Merge pull request #1029 from cloudflare/deps

Don't warn about re-added rules

docs/changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## v0.62.1
+
+### Fixed
+
+- Fixed false positive warnings from [rule/dependency](checks/rule/dependency.md) check.
+
 ## v0.62.0
 
 ### Added

internal/checks/rule_dependency.go

@@ -47,18 +47,18 @@ func (c RuleDependencyCheck) Check(_ context.Context, path discovery.Path, rule
 		return problems
 	}
 
+	filtered := nonRemovedEntries(entries)
+
+	for _, entry := range filtered {
+		if entry.Rule.Type() == rule.Type() && entry.Rule.Name() == rule.Name() {
+			// There's another rule with same type & name, do nothing.
+			return problems
+		}
+	}
+
 	var broken []*brokenDependency
 	var dep *brokenDependency
-	for _, entry := range entries {
-		if entry.State == discovery.Removed {
-			continue
-		}
-		if entry.PathError != nil {
-			continue
-		}
-		if entry.Rule.Error.Err != nil {
-			continue
-		}
+	for _, entry := range filtered {
 		if rule.RecordingRule != nil {
 			dep = c.usesVector(entry, rule.RecordingRule.Record.Value)
 		}
@@ -178,3 +178,19 @@ type brokenDependency struct {
 	name   string
 	line   int
 }
+
+func nonRemovedEntries(src []discovery.Entry) (dst []discovery.Entry) {
+	for _, entry := range src {
+		if entry.State == discovery.Removed {
+			continue
+		}
+		if entry.PathError != nil {
+			continue
+		}
+		if entry.Rule.Error.Err != nil {
+			continue
+		}
+		dst = append(dst, entry)
+	}
+	return dst
+}

internal/checks/rule_dependency_test.go

@@ -315,6 +315,20 @@ func TestRuleDependencyCheck(t *testing.T) {
 				parseWithState("- record: bar\n  expr: vector(0)\n", discovery.Noop, "foo.yaml", "foo.yaml")[0],
 			},
 		},
+		{
+			description: "ignores re-added rules",
+			content:     "- record: foo\n  expr: sum(foo)\n",
+			checker: func(_ *promapi.FailoverGroup) checks.RuleChecker {
+				return checks.NewRuleDependencyCheck()
+			},
+			prometheus: newSimpleProm,
+			problems:   noProblems,
+			entries: []discovery.Entry{
+				parseWithState("- record: foo\n  expr: sum(foo)\n", discovery.Removed, "foo.yaml", "foo.yaml")[0],
+				parseWithState("- alert: alert\n  expr: foo == 0\n", discovery.Noop, "foo.yaml", "foo.yaml")[0],
+				parseWithState("- record: foo\n  expr: sum(foo)\n", discovery.Added, "bar.yaml", "foo.yaml")[0],
+			},
+		},
 	}
 
 	runTests(t, testCases)