mirror of
https://github.com/grafana/grafana.git
synced 2025-02-11 16:15:42 -06:00
Alerting: Fix template validation in provisioning api (#62530)
* Alerting: Fix template validation in provisioning api Fix issue where provisioning API accepts a malformed template having extra text outside of definition block and template name matching definition name.
This commit is contained in:
parent
121e384fab
commit
f9ec16e74f
@ -5,6 +5,7 @@ import (
|
||||
tmplhtml "html/template"
|
||||
"regexp"
|
||||
"strings"
|
||||
tmpltext "text/template"
|
||||
"time"
|
||||
|
||||
"github.com/prometheus/alertmanager/template"
|
||||
@ -65,13 +66,6 @@ func (t *NotificationTemplate) Validate() error {
|
||||
return fmt.Errorf("template must have content")
|
||||
}
|
||||
|
||||
tmpl := tmplhtml.New("").Option("missingkey=zero")
|
||||
tmpl.Funcs(tmplhtml.FuncMap(template.DefaultFuncs))
|
||||
_, err := tmpl.Parse(t.Template)
|
||||
if err != nil {
|
||||
return fmt.Errorf("invalid template: %w", err)
|
||||
}
|
||||
|
||||
content := strings.TrimSpace(t.Template)
|
||||
found, err := regexp.MatchString(`\{\{\s*define`, content)
|
||||
if err != nil {
|
||||
@ -87,6 +81,21 @@ func (t *NotificationTemplate) Validate() error {
|
||||
}
|
||||
t.Template = content
|
||||
|
||||
// Validate template contents. We try to stick as close to what will actually happen when the templates are parsed
|
||||
// by the alertmanager as possible. That means parsing with both the text and html parsers and making sure we set
|
||||
// the template name and options.
|
||||
ttext := tmpltext.New(t.Name).Option("missingkey=zero")
|
||||
ttext.Funcs(tmpltext.FuncMap(template.DefaultFuncs))
|
||||
if _, err := ttext.Parse(t.Template); err != nil {
|
||||
return fmt.Errorf("invalid template: %w", err)
|
||||
}
|
||||
|
||||
thtml := tmplhtml.New(t.Name).Option("missingkey=zero")
|
||||
thtml.Funcs(tmplhtml.FuncMap(template.DefaultFuncs))
|
||||
if _, err := thtml.Parse(t.Template); err != nil {
|
||||
return fmt.Errorf("invalid template: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
@ -1,6 +1,7 @@
|
||||
package definitions
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/prometheus/alertmanager/config"
|
||||
@ -365,3 +366,116 @@ func TestValidateMuteTimeInterval(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestValidateNotificationTemplates(t *testing.T) {
|
||||
tc := []struct {
|
||||
name string
|
||||
template NotificationTemplate
|
||||
expContent string
|
||||
expError error
|
||||
}{
|
||||
{
|
||||
name: "Same template name as definition",
|
||||
template: NotificationTemplate{
|
||||
Name: "Same name as definition",
|
||||
Template: `{{ define "Same name as definition" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expContent: `{{ define "Same name as definition" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
expError: nil,
|
||||
},
|
||||
{
|
||||
name: "Different template name than definition",
|
||||
template: NotificationTemplate{
|
||||
Name: "Different name than definition",
|
||||
Template: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expContent: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
expError: nil,
|
||||
},
|
||||
{
|
||||
name: "Fix template - missing both {{ define }} and {{ end }}",
|
||||
template: NotificationTemplate{
|
||||
Name: "Alert Instance Template",
|
||||
Template: `Firing: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expContent: "{{ define \"Alert Instance Template\" }}\n Firing: {{ .Labels.alertname }}\\nSilence: {{ .SilenceURL }}\n{{ end }}",
|
||||
expError: nil,
|
||||
},
|
||||
{
|
||||
name: "Multiple definitions",
|
||||
template: NotificationTemplate{
|
||||
Name: "Alert Instance Template",
|
||||
Template: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}{{ define "Alert Instance Template2" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expContent: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}{{ define "Alert Instance Template2" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
expError: nil,
|
||||
},
|
||||
{
|
||||
name: "Malformed template - missing {{ define }}",
|
||||
template: NotificationTemplate{
|
||||
Name: "Alert Instance Template",
|
||||
Template: `\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expError: errors.New("invalid template: template: Alert Instance Template:3: unexpected {{end}}"),
|
||||
},
|
||||
{
|
||||
name: "Malformed template - missing {{ end }}",
|
||||
template: NotificationTemplate{
|
||||
Name: "Alert Instance Template",
|
||||
Template: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expError: errors.New("invalid template: template: Alert Instance Template:1: unexpected EOF"),
|
||||
},
|
||||
{
|
||||
name: "Malformed template - multiple definitions duplicate name",
|
||||
template: NotificationTemplate{
|
||||
Name: "Alert Instance Template",
|
||||
Template: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}\n{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expError: errors.New("invalid template: template: Different name than definition:1: template: multiple definition of template \"Alert Instance Template\""),
|
||||
},
|
||||
{
|
||||
// This is fine as long as the template name is different from the definition, it just ignores the extra text.
|
||||
name: "Extra text outside definition block - different template name and definition",
|
||||
template: NotificationTemplate{
|
||||
Name: "Different name than definition",
|
||||
Template: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}[what is this?]`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expContent: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}[what is this?]`,
|
||||
expError: nil,
|
||||
},
|
||||
{
|
||||
// This is NOT fine as the template name is the same as the definition.
|
||||
// GO template parser will treat it as if it's wrapped in {{ define "Alert Instance Template" }}, thus creating a duplicate definition.
|
||||
name: "Extra text outside definition block - same template name and definition",
|
||||
template: NotificationTemplate{
|
||||
Name: "Alert Instance Template",
|
||||
Template: `{{ define "Alert Instance Template" }}\nFiring: {{ .Labels.alertname }}\nSilence: {{ .SilenceURL }}\n{{ end }}[what is this?]`,
|
||||
Provenance: "test",
|
||||
},
|
||||
expError: errors.New("invalid template: template: Alert Instance Template:1: template: multiple definition of template \"Alert Instance Template\""),
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tc {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := tt.template.Validate()
|
||||
if tt.expError == nil {
|
||||
require.NoError(t, err)
|
||||
} else {
|
||||
require.Error(t, err)
|
||||
return
|
||||
}
|
||||
|
||||
require.Equal(t, tt.expContent, tt.template.Template)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user