From 05726c5ea28edeac9b871ef938775752db009311 Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar Date: Thu, 13 Sep 2018 18:25:58 +0530 Subject: [PATCH] Test template expansion while loading groups (#4537) Signed-off-by: Ganesh Vernekar --- pkg/rulefmt/rulefmt.go | 49 ++++++++++++++++++++++ pkg/rulefmt/rulefmt_test.go | 82 +++++++++++++++++++++++++++++++++++++ rules/alerting.go | 8 +--- template/template.go | 20 +++++++++ 4 files changed, 152 insertions(+), 7 deletions(-) diff --git a/pkg/rulefmt/rulefmt.go b/pkg/rulefmt/rulefmt.go index a60b2887b..8acb65512 100644 --- a/pkg/rulefmt/rulefmt.go +++ b/pkg/rulefmt/rulefmt.go @@ -14,11 +14,16 @@ package rulefmt import ( + "context" + "fmt" "io/ioutil" + "time" "github.com/pkg/errors" "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/pkg/timestamp" "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/template" yaml "gopkg.in/yaml.v2" ) @@ -137,6 +142,50 @@ func (r *Rule) Validate() (errs []error) { } } + errs = append(errs, testTemplateParsing(r)...) + return errs +} + +// testTemplateParsing checks if the templates used in labels and annotations +// of the alerting rules are parsed correctly. +func testTemplateParsing(rl *Rule) (errs []error) { + if rl.Alert == "" { + // Not an alerting rule. + return errs + } + + // Trying to parse templates. + tmplData := template.AlertTemplateData(make(map[string]string), 0) + defs := "{{$labels := .Labels}}{{$value := .Value}}" + parseTest := func(text string) error { + tmpl := template.NewTemplateExpander( + context.TODO(), + defs+text, + "__alert_"+rl.Alert, + tmplData, + model.Time(timestamp.FromTime(time.Now())), + nil, + nil, + ) + return tmpl.ParseTest() + } + + // Parsing Labels. + for _, val := range rl.Labels { + err := parseTest(val) + if err != nil { + errs = append(errs, fmt.Errorf("msg=%s", err.Error())) + } + } + + // Parsing Annotations. + for _, val := range rl.Annotations { + err := parseTest(val) + if err != nil { + errs = append(errs, fmt.Errorf("msg=%s", err.Error())) + } + } + return errs } diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go index 0104d5751..26151c8c1 100644 --- a/pkg/rulefmt/rulefmt_test.go +++ b/pkg/rulefmt/rulefmt_test.go @@ -17,6 +17,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/prometheus/prometheus/util/testutil" ) func TestParseFileSuccess(t *testing.T) { @@ -79,3 +81,83 @@ func TestParseFileFailure(t *testing.T) { } } + +func TestTemplateParsing(t *testing.T) { + tests := []struct { + ruleString string + shouldPass bool + }{ + { + ruleString: ` +groups: +- name: example + rules: + - alert: InstanceDown + expr: up == 0 + for: 5m + labels: + severity: "page" + annotations: + summary: "Instance {{ $labels.instance }} down" +`, + shouldPass: true, + }, + { + // `$label` instead of `$labels`. + ruleString: ` +groups: +- name: example + rules: + - alert: InstanceDown + expr: up == 0 + for: 5m + labels: + severity: "page" + annotations: + summary: "Instance {{ $label.instance }} down" +`, + shouldPass: false, + }, + { + // `$this_is_wrong`. + ruleString: ` +groups: +- name: example + rules: + - alert: InstanceDown + expr: up == 0 + for: 5m + labels: + severity: "{{$this_is_wrong}}" + annotations: + summary: "Instance {{ $labels.instance }} down" +`, + shouldPass: false, + }, + { + // `$labels.quantile * 100`. + ruleString: ` +groups: +- name: example + rules: + - alert: InstanceDown + expr: up == 0 + for: 5m + labels: + severity: "page" + annotations: + summary: "Instance {{ $labels.instance }} down" + description: "{{$labels.quantile * 100}}" +`, + shouldPass: false, + }, + } + + for _, tst := range tests { + rgs, errs := Parse([]byte(tst.ruleString)) + testutil.Assert(t, rgs != nil, "Rule parsing, rule=\n"+tst.ruleString) + passed := (tst.shouldPass && len(errs) == 0) || (!tst.shouldPass && len(errs) > 0) + testutil.Assert(t, passed, "Rule validation failed, rule=\n"+tst.ruleString) + } + +} diff --git a/rules/alerting.go b/rules/alerting.go index a337936e0..01701f1c0 100644 --- a/rules/alerting.go +++ b/rules/alerting.go @@ -292,13 +292,7 @@ func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, l[lbl.Name] = lbl.Value } - tmplData := struct { - Labels map[string]string - Value float64 - }{ - Labels: l, - Value: smpl.V, - } + tmplData := template.AlertTemplateData(l, smpl.V) // Inject some convenience variables that are easier to remember for users // who are not used to Go's templating system. defs := "{{$labels := .Labels}}{{$value := .Value}}" diff --git a/template/template.go b/template/template.go index 91ff9d5eb..f25fe67db 100644 --- a/template/template.go +++ b/template/template.go @@ -243,6 +243,17 @@ func NewTemplateExpander( } } +// AlertTemplateData returns the interface to be used in expanding the template. +func AlertTemplateData(labels map[string]string, value float64) interface{} { + return struct { + Labels map[string]string + Value float64 + }{ + Labels: labels, + Value: value, + } +} + // Funcs adds the functions in fm to the Expander's function map. // Existing functions will be overwritten in case of conflict. func (te Expander) Funcs(fm text_template.FuncMap) { @@ -315,3 +326,12 @@ func (te Expander) ExpandHTML(templateFiles []string) (result string, resultErr } return buffer.String(), nil } + +// ParseTest parses the templates and returns the error if any. +func (te Expander) ParseTest() error { + _, err := text_template.New(te.name).Funcs(te.funcMap).Option("missingkey=zero").Parse(te.text) + if err != nil { + return err + } + return nil +}