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

[Fleet] Add helpers for escaping special characters in YAML variables #127268

Closed
2 tasks done
joshdover opened this issue Mar 9, 2022 · 5 comments · Fixed by #135992
Closed
2 tasks done

[Fleet] Add helpers for escaping special characters in YAML variables #127268

joshdover opened this issue Mar 9, 2022 · 5 comments · Fixed by #135992
Assignees
Labels
enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@joshdover
Copy link
Contributor

joshdover commented Mar 9, 2022

We've had many issues around interpolating string variables that contain special YAML characters into Agent policy handlebar templates. See:

@jen-huang suggested in #121934 (comment):

There is a related discussion around the difficulties of yaml parsing here, where Andrew points out that similar issues was solved in Beats module templates by adding a to_json helper: elastic/package-spec#280 (comment)

I wonder if this kind of helper can help us in this kind of case too?

We should explore adding helpers to our template engine to ease package developers and users to avoid issues with special characters in strings. As noted by @nchaulet below, there are different use cases to consider and we should be clear about what we want to support and whether or not these should be solved by a single helper or separate helpers for each use cases.

Use cases

  • Escaping special characters in strings
  • Embedding JSON fields

cc @mtojek @jsoriano for any feedback

Implementation

  • Implement an escape_string Handlebars helper in Fleet's templating implementation that performs as follows
    • Wraps the interpolated variable in single quotes '
    • If the variable is a string, replaces any single quotes within the variable with their escaped YML equivalent of two single quotes ''
  • Implement a to_json Handlebars helper that runs the variable through JSON.stringify and returns it
    • Note: Do not wrap the result in quotes of any kind

Documentation

These helpers should be documented somewhere accessible to integration developers who rely on Fleet's templating engine to generate elastic-agent.yml files. We should specifically be sure to capture the caveat that using escape_string and wrapping the result in single or double quotes is not supported and will likely cause errors.

@kpollich to determine the best location for these docs

@joshdover joshdover added enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team labels Mar 9, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@mtojek
Copy link
Contributor

mtojek commented Mar 9, 2022

I agree that we need to improve this area. Based on the investigation around GCP credentials, I was thinking about base64 vars just to prevent dangerous escaping or trimming. It might be a stupid idea to disable all escaping and simply passed based input down to agent, which will unbase there.

@nchaulet
Copy link
Member

nchaulet commented Mar 9, 2022

I guess the to_json could help to escape some fields in the template but it's probably not magic and there is still some template that will not work the one with key level variables lie

test: 123
{{custom}}

or (this one is currently not supported)

test:
  foo: bar
  {{custom}}

the to_json helper or maybe escape_string will be more helpful I think to be sure string variables are correctly escaped

@joshdover joshdover changed the title [Fleet] Add a to_json helper for YAML variables [Fleet] Add helpers for escaping special characters in YAML variables Mar 14, 2022
@joshdover
Copy link
Contributor Author

the to_json helper or maybe escape_string will be more helpful I think to be sure string variables are correctly escaped

Good feedback here, thanks @nchaulet. I agree, let's explore separate helpers for these. I think escape_string would make sense. I've updated the issue title and description to capture this idea, but we still need further feedback from Integration teams on what would be helpful here.

@kpollich
Copy link
Member

kpollich commented Jun 13, 2022

One interesting development I've noticed in reviewing all the context here is the difference in usage of quotes we have in various integrations today.

Let's take for example two of the integrations linked in this issue mentioned as having issues with escaping in the past: vSphere and Cloudflare

In vSphere's case, we reported an issue elastic/integrations#3429 around special characters in a password variable. That password variable's definition lives here and is interpolated in several places throughout the eventual agent.yml file, but here is a single example.

So, in the vSphere integration, we're interpolating the password value without any quotes, e.g.

password: {{password}}

This results in special characters like @ - { :, etc coming through unescaped. This opens us up to various YAML parsing issues and cryptic errors as evidenced in the linked issue.

In the Cloudflare integration, we've observed a similar issue elastic/integrations#3388 around interpolation of a password variable containing special characters: elastic/integrations#3388. This is working off of an auth_key variable defined here.

The difference in Cloudflare's case, however, is that we wrap this value in double quotes. When we interpolate this variable, we have it wrapped in double quotes in an effort to escape special characters, e.g.

value: "{{auth_key}}"

This results in a slightly different issue around escaping because double quotes within double quotes must be escaped in YAML. e.g.

value: "my\"auth\"key"

See relevant section of the YAML specification here: https://yaml.org/spec/1.2.2/#731-double-quoted-style.


Single quotes are probably the best option for the escaping behavior we want, because they

  • Allow us to escape all special characters
  • Don't honor return/line-ending characters like \n

Single quotes do, however, require any nested single quotes to be duplicated as an escaping mechanism, e.g.

- my_yaml_property: 'hello''world'

So, I think what we can do in Fleet is register an escape_string Handlebars helper that wraps the given variable in single quotes and replaces any nested single quotes with '' (two single quotes), e.g.

# .yml.hbs file
- my_yaml_property: {{escape_string my_variable }}

# { my_variable: "hello'world" }

# elastic-agent.yml output file
- my_yaml_property: 'hello''world'

Integration developers will just need to make certain they don't "double up" on quoting mechanisms when using the escape_string helper method. This is important to avoid malformed YAML and errors in the Fleet policy editor.


There's also been mention of a to_json Handlebar helper similar to the beats module one. This one should be trivial to implement, as it just needs to run a given variable through JSON.stringify. Example use case:

# .yml.hbs
- my_yaml_property: {{to_json my_variable }}

# { my_variable: { foo: ["bar"] } }

# elastic-agent.yml
- my_yaml_property: {"my_variable":{"foo":["bar"]}}

I think the above should satisfy the need for an easier way to generate JSON blocks in YAML templates. @nchaulet do you know of any explicit examples where we need this behavior so I can get some context?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants