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] Handle invalid YAML string values at API level when adding / editing integrations in policies #93926

Closed
skh opened this issue Mar 8, 2021 · 2 comments
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture

Comments

@skh
Copy link
Contributor

skh commented Mar 8, 2021

Currently, when an invalid YAML string value to be used in a policy is sent to the /api/fleet/package_policies endpoint, the response contains the raw error from the YAML parser. As an example, this request, adding nginx to the existing policy with id e233dc90-800a-11eb-b84c-bfaec6f37113:

curl -X POST -u elastic:changeme http://localhost:5601/byp/api/fleet/package_policies -H 'kbn-xsrf: xyz' -H "Content-Type: application/json" -d '{"name":"nginx-1","description":"","namespace":"default","policy_id":"e233dc90-800a-11eb-b84c-bfaec6f37113","enabled":true,"output_id":"","inputs":[{"type":"logfile","enabled":true,"streams":[{"enabled":true,"data_stream":{"type":"logs","dataset":"nginx.access"},"vars":{"paths":{"value":["/var/log/nginx/access.log*"],"type":"text"}}},{"enabled":true,"data_stream":{"type":"logs","dataset":"nginx.error"},"vars":{"paths":{"value":["/var/log/nginx/error.log*"],"type":"text"}}}]},{"type":"nginx/metrics","enabled":true,"streams":[{"enabled":true,"data_stream":{"type":"metrics","dataset":"nginx.stubstatus"},"vars":{"period":{"value":"10s","type":"text"},"server_status_path":{"type":"text","value":"*foo"}}}],"vars":{"hosts":{"value":["http://127.0.0.1:80"],"type":"text"}}}],"package":{"name":"nginx","title":"Nginx","version":"0.3.10"}}'

(note the "*foo" in the value for "server_status_path") will result in the error

{"statusCode":500,"error":"Internal Server Error","message":"unidentified alias \"foo\" at line 5, column 25:\n    server_status_path: *foo\n                            ^"}

A valid string in this place would be "\"*foo\"". It needs to contain escaped double quotes so that the string that ends up in the policy is "*foo". Without that, *foo is interpreted to be a YAML alias, which is not defined at this place and hence an invalid value.

This error is thrown after the values have been replaced in the policy template, at this place https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/services/epm/agent/agent.ts#L21 in the line

const yamlFromCompiledTemplate = safeLoad(compiledTemplate, {});

Invalid strings are already being caught by UI-side form validation when a user adds an integration to a policy (or edits it). (This was added in #93585 .) This does not help in the case when users use the API directly.

This task is to provide a better error message here to inform the API user of the problematic value:

  • wrap the YAML error in a better error message, or
  • validate the string values roughly at this place in any other way, or
  • something even better.

NOTE Please see the discussion in #93585 -- at the time of this writing, the best known solution is NOT to add additional quotes to the string in this place, because this introduces other, different errors.

@skh skh added bug Fixes for quality problems that affect the customer experience Team:Fleet Team label for Observability Data Collection Fleet team labels Mar 8, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jen-huang jen-huang added technical debt Improvement of the software architecture and operational architecture and removed bug Fixes for quality problems that affect the customer experience labels Aug 18, 2021
@joshdover
Copy link
Contributor

Closing in favor of #127268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

4 participants