diff --git a/lint/results.go b/lint/results.go index 7cf9be0..2841f8f 100644 --- a/lint/results.go +++ b/lint/results.go @@ -34,12 +34,26 @@ type TargetRuleResults struct { Results []TargetResult } +func TargetMessage(d Dashboard, p Panel, t Target, message string) string { + return fmt.Sprintf("Dashboard '%s', panel '%s', target idx '%d' %s", d.Title, p.Title, t.Idx, message) +} + func (r *TargetRuleResults) AddError(d Dashboard, p Panel, t Target, message string) { r.Results = append(r.Results, TargetResult{ Result: Result{ Severity: Error, - Message: fmt.Sprintf("Dashboard '%s', panel '%s', target idx '%d' %s", d.Title, p.Title, t.Idx, message), + Message: TargetMessage(d, p, t, message), + }, + }) +} + +func (r *TargetRuleResults) AddFixableError(d Dashboard, p Panel, t Target, message string, fix func(Dashboard, Panel, *Target)) { + r.Results = append(r.Results, TargetResult{ + Result: Result{ + Severity: Error, + Message: TargetMessage(d, p, t, message), }, + Fix: fix, }) } diff --git a/lint/rule_target_job_instance.go b/lint/rule_target_job_instance.go index 968f679..c3586b1 100644 --- a/lint/rule_target_job_instance.go +++ b/lint/rule_target_job_instance.go @@ -21,15 +21,15 @@ func newTargetRequiredMatcherRule(matcher string) *TargetRuleFunc { return r } - node, err := parsePromQL(t.Expr, d.Templating.List) + expr, err := parsePromQL(t.Expr, d.Templating.List) if err != nil { // Invalid PromQL is another rule return r } - for _, selector := range parser.ExtractSelectors(node) { + for _, selector := range parser.ExtractSelectors(expr) { if err := checkForMatcher(selector, matcher, labels.MatchRegexp, fmt.Sprintf("$%s", matcher)); err != nil { - r.AddError(d, p, t, fmt.Sprintf("invalid PromQL query '%s': %v", t.Expr, err)) + r.AddFixableError(d, p, t, fmt.Sprintf("invalid PromQL query '%s': %v", t.Expr, err), fixTargetRequiredMatcherRule(matcher, labels.MatchRegexp, fmt.Sprintf("$%s", matcher))) } } @@ -45,3 +45,46 @@ func NewTargetJobRule() *TargetRuleFunc { func NewTargetInstanceRule() *TargetRuleFunc { return newTargetRequiredMatcherRule("instance") } + +func fixTargetRequiredMatcherRule(name string, ty labels.MatchType, value string) func(Dashboard, Panel, *Target) { + return func(d Dashboard, p Panel, t *Target) { + // using t.Expr to ensure matchers added earlier in the loop are not lost + // no need to check for errors here, as the expression was already parsed and validated + expr, _ := parsePromQL(t.Expr, d.Templating.List) + // Walk the expression tree and add the matcher to all vector selectors + parser.Walk(addMatchers(name, ty, value), expr, nil) + t.Expr = expr.String() + } +} + +type matcherAdder func(node parser.Node) error + +func (f matcherAdder) Visit(node parser.Node, path []parser.Node) (w parser.Visitor, err error) { + err = f(node) + return f, err +} + +func addMatchers(name string, ty labels.MatchType, value string) matcherAdder { + return func(node parser.Node) error { + if n, ok := node.(*parser.VectorSelector); ok { + matcherfixed := false + for _, m := range n.LabelMatchers { + if m.Name == name { + if m.Type != ty || m.Value != value { + m.Type = ty + m.Value = value + } + matcherfixed = true + } + } + if !matcherfixed { + n.LabelMatchers = append(n.LabelMatchers, &labels.Matcher{ + Name: name, + Type: ty, + Value: value, + }) + } + } + return nil + } +} diff --git a/lint/rule_target_job_instance_test.go b/lint/rule_target_job_instance_test.go index 3b36277..712a501 100644 --- a/lint/rule_target_job_instance_test.go +++ b/lint/rule_target_job_instance_test.go @@ -1,8 +1,11 @@ package lint import ( + "encoding/json" "fmt" "testing" + + "github.com/stretchr/testify/require" ) func testTargetRequiredMatcherRule(t *testing.T, matcher string) { @@ -19,11 +22,14 @@ func testTargetRequiredMatcherRule(t *testing.T, matcher string) { } for _, tc := range []struct { + name string result Result target Target + fixed *Target }{ // Happy path { + name: "OK", result: ResultSuccess, target: Target{ Expr: fmt.Sprintf(`sum(rate(foo{%s=~"$%s"}[5m]))`, matcher, matcher), @@ -31,6 +37,7 @@ func testTargetRequiredMatcherRule(t *testing.T, matcher string) { }, // Also happy when the promql is invalid { + name: "OK-invalid-promql", result: ResultSuccess, target: Target{ Expr: `foo(bar.baz))`, @@ -38,33 +45,45 @@ func testTargetRequiredMatcherRule(t *testing.T, matcher string) { }, // Missing matcher { + name: "autofix-missing-matcher", result: Result{ - Severity: Error, + Severity: Fixed, Message: fmt.Sprintf("Dashboard 'dashboard', panel 'panel', target idx '0' invalid PromQL query 'sum(rate(foo[5m]))': %s selector not found", matcher), }, target: Target{ Expr: `sum(rate(foo[5m]))`, }, + fixed: &Target{ + Expr: fmt.Sprintf(`sum(rate(foo{%s=~"$%s"}[5m]))`, matcher, matcher), + }, }, // Not a regex matcher { + name: "autofix-not-regex-matcher", result: Result{ - Severity: Error, + Severity: Fixed, Message: fmt.Sprintf("Dashboard 'dashboard', panel 'panel', target idx '0' invalid PromQL query 'sum(rate(foo{%s=\"$%s\"}[5m]))': %s selector is =, not =~", matcher, matcher, matcher), }, target: Target{ Expr: fmt.Sprintf(`sum(rate(foo{%s="$%s"}[5m]))`, matcher, matcher), }, + fixed: &Target{ + Expr: fmt.Sprintf(`sum(rate(foo{%s=~"$%s"}[5m]))`, matcher, matcher), + }, }, // Wrong template variable { + name: "autofix-wrong-template-variable", result: Result{ - Severity: Error, + Severity: Fixed, Message: fmt.Sprintf("Dashboard 'dashboard', panel 'panel', target idx '0' invalid PromQL query 'sum(rate(foo{%s=~\"$foo\"}[5m]))': %s selector is $foo, not $%s", matcher, matcher, matcher), }, target: Target{ Expr: fmt.Sprintf(`sum(rate(foo{%s=~"$foo"}[5m]))`, matcher), }, + fixed: &Target{ + Expr: fmt.Sprintf(`sum(rate(foo{%s=~"$%s"}[5m]))`, matcher, matcher), + }, }, } { dashboard := Dashboard{ @@ -87,8 +106,35 @@ func testTargetRequiredMatcherRule(t *testing.T, matcher string) { }, }, } - - testRule(t, linter, dashboard, tc.result) + t.Run(tc.name, func(t *testing.T) { + autofix := tc.fixed != nil + testRuleWithAutofix(t, linter, &dashboard, []Result{tc.result}, autofix) + if autofix { + fixedDashboard := Dashboard{ + Title: "dashboard", + Templating: struct { + List []Template `json:"list"` + }{ + List: []Template{ + { + Type: "datasource", + Query: "prometheus", + }, + }, + }, + Panels: []Panel{ + { + Title: "panel", + Type: "singlestat", + Targets: []Target{*tc.fixed}, + }, + }, + } + expected, _ := json.Marshal(fixedDashboard) + actual, _ := json.Marshal(dashboard) + require.Equal(t, string(expected), string(actual)) + } + }) } }