Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CFE-3489: Mustache: Added warning when trying to use {{.}} to expand containers #97

Merged
merged 1 commit into from Nov 13, 2020

Conversation

olehermanse
Copy link
Contributor

@olehermanse olehermanse commented Nov 11, 2020

Example:

root@OH-WIN:/northern.tech/cfengine/core# cat test.cf
bundle agent firewall
{
  vars:
      "_defaults" data => '{ "source": "0.0.0.0/0", "protocol": "tcp" }';
      "cfengine" data => mergedata( @(_defaults), '{"name": "cfengine" "destination-port": "5308" }' );
      "RULES" data => mergedata( '{ "ACCEPT": [ cfengine ]}' );
    reports:
      "$(with)" with => string_mustache( "{{#ACCEPT}}{{.}}{{/ACCEPT}}", RULES );
}
bundle agent __main__
{
    methods:
      "firewall";
    reports:
      "CFEngine $(sys.cf_version)";
}
root@OH-WIN:/northern.tech/cfengine/core# /var/cfengine/bin/cf-agent -KI test.cf
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
R: $(with)
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
 warning: {{.}} mustache tag cannot expand a container without specifying conversion - consider {{$.}} or {{%.}}
R: CFEngine 3.17.0a.782c013a0
root@OH-WIN:/northern.tech/cfengine/core# sed "s/{{.}}/{{$.}}/g" -i test.cf
root@OH-WIN:/northern.tech/cfengine/core# cat test.cf
bundle agent firewall
{
  vars:
      "_defaults" data => '{ "source": "0.0.0.0/0", "protocol": "tcp" }';
      "cfengine" data => mergedata( @(_defaults), '{"name": "cfengine" "destination-port": "5308" }' );
      "RULES" data => mergedata( '{ "ACCEPT": [ cfengine ]}' );
    reports:
      "$(with)" with => string_mustache( "{{#ACCEPT}}{{$.}}{{/ACCEPT}}", RULES );
}
bundle agent __main__
{
    methods:
      "firewall";
    reports:
      "CFEngine $(sys.cf_version)";
}
root@OH-WIN:/northern.tech/cfengine/core# /var/cfengine/bin/cf-agent -KI test.cf
R: {"destination-port":"5308","name":"cfengine","protocol":"tcp","source":"0.0.0.0/0"}
R: CFEngine 3.17.0a.782c013a0
root@OH-WIN:/northern.tech/cfengine/core#

The mustache manual doesn't mention {{.}} at all:

http://mustache.github.io/mustache.5.html

Mustache.js documentation does mention it:

https://github.com/janl/mustache.js/#non-empty-lists

But only what it should do when iterating over a list of strings.

Some references in the "spec":

mustache/mustache#129
https://github.com/mustache/spec/blob/master/specs/sections.yml#L141-L169

But no mention of what should happen in these edge cases / wrong usage.

So, I don't think we should copy the behavior of the mustache demo in this case, because

  1. It's not described anywhere in the spec
  2. It seems accidental:

Mustache template

{{.}}

Data:

[1,"a",{},[1,2]]

Output:

1,a,[object Object],1,2

JavaScript:

$ node
Welcome to Node.js v14.15.0.
Type ".help" for more information.
> String([1,2,3])
'1,2,3'
> String({})
'[object Object]'
> String(["a","b","c"])
'a,b,c'
> String([1,"a",{},[1,2]])
'1,a,[object Object],1,2'

See JIRA tickets for further explanation:
https://tracker.mender.io/browse/CFE-3457
https://tracker.mender.io/browse/CFE-3489

@olehermanse olehermanse changed the title Mustache: Added warning when trying to use {{.}} to expand containers CFE-3489: Mustache: Added warning when trying to use {{.}} to expand containers Nov 11, 2020
Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me otherwise.

libutils/mustache.c Show resolved Hide resolved
The mustache manual doesn't mention `{{.}}` at all:

http://mustache.github.io/mustache.5.html

Mustache.js documentation does mention it:

https://github.com/janl/mustache.js/#non-empty-lists

But only what it should do when iterating over a list of strings.

Some references in the "spec":

mustache/mustache#129
https://github.com/mustache/spec/blob/master/specs/sections.yml#L141-L169

But no mention of what should happen in these edge cases / wrong usage.

So, I don't think we should copy the behavior of the mustache demo in this case, because

1. It's not described anywhere in the spec
2. It seems accidental:

Mustache template
```
{{.}}
```

Data:
```
[1,"a",{},[1,2]]
```

Output:
```
1,a,[object Object],1,2
```

JavaScript:

```
$ node
Welcome to Node.js v14.15.0.
Type ".help" for more information.
> String([1,2,3])
'1,2,3'
> String({})
'[object Object]'
> String(["a","b","c"])
'a,b,c'
> String([1,"a",{},[1,2]])
'1,a,[object Object],1,2'
```

See JIRA tickets for further explanation:
https://tracker.mender.io/browse/CFE-3457
https://tracker.mender.io/browse/CFE-3489

Ticket: CFE-3489 CFE-3457
Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech>
Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@olehermanse olehermanse merged commit 4e9efcb into NorthernTechHQ:master Nov 13, 2020
@olehermanse olehermanse deleted the mustache branch November 13, 2020 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants