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

[Alerting] Validate action message in ActionForm for mustache compatibility #62928

Closed
patrykkopycinski opened this issue Apr 8, 2020 · 2 comments · Fixed by #83919
Closed
Labels
bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.1 v8.0.0

Comments

@patrykkopycinski
Copy link
Contributor

Scheduled action failed due to typo in action message and I would not know that if I don't look at the Kibana logs, so it would be great if ActionForm would validate the message against mustache compatibility
Zrzut ekranu 2020-04-8 o 12 45 37

@patrykkopycinski patrykkopycinski added bug Fixes for quality problems that affect the customer experience v8.0.0 v7.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Apr 8, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@pmuellr
Copy link
Member

pmuellr commented Nov 24, 2020

On the face of it, this is pretty simple. You can render a template string with no variables just to see if it throws an error if the mustache template has syntax errors.

The problem is, when do you do this? Not in actions themselves, but somewhere in alerting. And as part of not-yet-merged-ATM PR #83919, actions will have the ability to choose which parameters from alerting should treated as templates - eg, for email, to/cc/etc would not be treated as a mustache template, but subject and message would.

Also in PR #83919, ATM, when mustache throws an error, we are outputting the error message into the rendered string. This is pretty safe, mustache only seems to print out values from the template, and not the variables. And useful, it actually provides an integer position within the template at the syntax error. Perhaps that will be enough for now.

I'd think the only way to test this before actually using it in an alert would be part of alert simulation.

pmuellr added a commit to pmuellr/kibana that referenced this issue Dec 8, 2020
resolves elastic#79371
resolves elastic#75601
resolves elastic#62928

In this PR, we allow action types to determine how to escape the
variables used in their parameters, when rendered as mustache
templates.  Prior to this, action parameters were recursively
rendered as mustache templates using the default mustache
templating, by the alerts library.  The default mustache
templating used html escaping.

Action types opt-in to the new capability via a new optional
method in the action type, `renderParameterTemplates()`.  If not
provided, the previous recursive rendering is done, but now with
no escaping at all.

For elastic#75601, added toString() methods to mustache object variables
which allow them to be used in a template and expanded to
JSON, for experimentation / discovery of context variables.

For elastic#62928, changed the mustache template rendering to be
replaced with the error message, if an error occurred,
so at least you can now see that an error occurred.  Useful
to diagnose problems with invalid mustache templates.
pmuellr added a commit to pmuellr/kibana that referenced this issue Dec 8, 2020
resolves elastic#79371
resolves elastic#62928

In this PR, we allow action types to determine how to escape the
variables used in their parameters, when rendered as mustache
templates.  Prior to this, action parameters were recursively
rendered as mustache templates using the default mustache
templating, by the alerts library.  The default mustache
templating used html escaping.

Action types opt-in to the new capability via a new optional
method in the action type, `renderParameterTemplates()`.  If not
provided, the previous recursive rendering is done, but now with
no escaping at all.

For elastic#62928, changed the mustache template rendering to be
replaced with the error message, if an error occurred,
so at least you can now see that an error occurred.  Useful
to diagnose problems with invalid mustache templates.
pmuellr added a commit that referenced this issue Dec 15, 2020
… parameter templates (#83919)

resolves #79371
resolves #62928

In this PR, we allow action types to determine how to escape the
variables used in their parameters, when rendered as mustache
templates.  Prior to this, action parameters were recursively
rendered as mustache templates using the default mustache
templating, by the alerts library.  The default mustache
templating used html escaping.

Action types opt-in to the new capability via a new optional
method in the action type, `renderParameterTemplates()`.  If not
provided, the previous recursive rendering is done, but now with
no escaping at all.

For #62928, changed the mustache template rendering to be
replaced with the error message, if an error occurred,
so at least you can now see that an error occurred.  Useful
to diagnose problems with invalid mustache templates.
pmuellr added a commit to pmuellr/kibana that referenced this issue Dec 15, 2020
… parameter templates (elastic#83919)

resolves elastic#79371
resolves elastic#62928

In this PR, we allow action types to determine how to escape the
variables used in their parameters, when rendered as mustache
templates.  Prior to this, action parameters were recursively
rendered as mustache templates using the default mustache
templating, by the alerts library.  The default mustache
templating used html escaping.

Action types opt-in to the new capability via a new optional
method in the action type, `renderParameterTemplates()`.  If not
provided, the previous recursive rendering is done, but now with
no escaping at all.

For elastic#62928, changed the mustache template rendering to be
replaced with the error message, if an error occurred,
so at least you can now see that an error occurred.  Useful
to diagnose problems with invalid mustache templates.
pmuellr added a commit that referenced this issue Dec 15, 2020
… parameter templates (#83919) (#85901)

resolves #79371
resolves #62928

In this PR, we allow action types to determine how to escape the
variables used in their parameters, when rendered as mustache
templates.  Prior to this, action parameters were recursively
rendered as mustache templates using the default mustache
templating, by the alerts library.  The default mustache
templating used html escaping.

Action types opt-in to the new capability via a new optional
method in the action type, `renderParameterTemplates()`.  If not
provided, the previous recursive rendering is done, but now with
no escaping at all.

For #62928, changed the mustache template rendering to be
replaced with the error message, if an error occurred,
so at least you can now see that an error occurred.  Useful
to diagnose problems with invalid mustache templates.
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.1 v8.0.0
Projects
None yet
6 participants