feat: Make matcher errors fixable

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,
 	})
 }
 

lint/rule_target_job_instance.go

@@ -21,15 +21,15 @@
 				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 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
+	}
+}

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,52 +22,68 @@ 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),
 			},
 		},
 		// Also happy when the promql is invalid
 		{
+			name:   "OK-invalid-promql",
 			result: ResultSuccess,
 			target: Target{
 				Expr: `foo(bar.baz))`,
 			},
 		},
 		// 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))
+			}
+		})
 	}
 }