[Fleet] Escape YAML string values if necessary #91418
Conversation
skh
added
Feature:Fleet
| const maybeEscapeNumericString = (value: string) => { | ||
| return value.length && !isNaN(+value) ? `"${value}"` : value; | ||
| const maybeEscapeString = (value: string) => { | ||
| return safeDump(value); |
There was a problem hiding this comment.
@nchaulet This seems to do the right thing, but safeDump() is given only one string here, not a complete object. I wonder if this is how it is meant to be used -- I didn't find any documentation for that use case.
There was a problem hiding this comment.
Just tested the safeDump and looks like it had a \n at the end not sure it will work for our usecase
There was a problem hiding this comment.
Have a look at the test case output in 0409c3e , it looks ok to me. I'm happy to change it to a simple "${value}" as well.
There was a problem hiding this comment.
I did more test and it's breaking some of our templates where we have loop, yes I think we should implement the escape function ourself here.
There was a problem hiding this comment.
I've changed this to quote all strings containing YAML special characters as listed in https://yaml.org/spec/1.2/spec.html#id2772075
@nchaulet can you share what you're testing with, and which templates are being broken?
|
Pinging @elastic/fleet (Feature:Fleet) |
skh
force-pushed
the
91401-escape-specialchars-in-yamlstrings
branch
from
dafdf7b to
9d7de5f
Compare
| const yamlSpecialCharsRegex = /[{}\[\],&*?|\-<>=!%@:]/; | ||
|
|
||
| // In addition, numeric strings need to be quoted to stay strings. | ||
| if ((value.length && !isNaN(+value)) || value.match(yamlSpecialCharsRegex)) { |
There was a problem hiding this comment.
Could also do yamlSpecialCharsRegex.test(value) since we only want a boolean result; not the matched item. It's also faster. Both are likely Fast Enough ™️ but this seems like a good place to use the more efficient option.
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
* Use js-yaml.safeDump() to escape string values. * Add unit test. * Explicitly check for YAML special characters. * Remove unnecessary imports. * Use RegExp.prototype.test() for speed.
* Use js-yaml.safeDump() to escape string values. * Add unit test. * Explicitly check for YAML special characters. * Remove unnecessary imports. * Use RegExp.prototype.test() for speed. Co-authored-by: Kibana Machine <[email protected]>
Summary
Fixes #91401
How to test this
Have a local registry running with the apm-server package contained in elastic/apm-server#4690 .
Install the Elastic APM integration with the "Add Elastic APM" button and follow the workflow to add the integration to a policy. In that workflow, enable RUM, and install the package by clicking "Save Integration".
The policy should be created without error, and be correct.
Checklist