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

Alert rule validation does not accept valid templates II #5343

Closed
kfindeisen opened this issue Jan 8, 2020 · 1 comment · Fixed by #5516
Closed

Alert rule validation does not accept valid templates II #5343

kfindeisen opened this issue Jan 8, 2020 · 1 comment · Fixed by #5516
Assignees
Milestone

Comments

@kfindeisen
Copy link

kfindeisen commented Jan 8, 2020

Chronograf version: 1.7.15
Installation method: built from source
Related issues: #5090, #5137
Related PRs: #5228

This is a re-opening of #5137, which was reported fixed in 1.7.13 through #5228.

As of Chronograf 1.7.15 I still see the old behavior: the alert builder GUI rejects messages with valid Go templates such as:

CI ID = {{ with (index .Tags "ci_id") }}{{ . }}{{ else }}Unknown{{ end }}

CI ID = {{ if (index .Tags "ci_id") }}{{ index .Tags "ci_id" }}{{ else -}} Unknown {{ end }}

# of Tags = {{ len .Tags }}

The messages pass validation if one removes {{ . }}, -, and len, respectively. The validator's aversion to {{- and -}} is a bit harder to reproduce than the other two; - tends to be accepted in simple expressions but rejected in complex ones.

In none of the above examples do I see the parse error added in #5228; all triggering strings return "Please correct templates in alert message." I do see the parse error if I reproduce the demo from #5228 (i.e., closing with } instead of }}), confirming that I'm using the new code at least some of the time.

@russorat
Copy link
Contributor

@kfindeisen thanks for opening. we will take a look. this code seems to keep causing us issues, so many we should just remove the template validation altogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants