From cac486e74d1be45769442f09ecccb9677f915a70 Mon Sep 17 00:00:00 2001
From: Lukasz Mierzwa <lukasz@cloudflare.com>
Date: Thu, 11 Jul 2024 16:49:24 +0100
Subject: [PATCH] Don't warn about re-added rules

---
 docs/changelog.md                       |  6 +++++
 internal/checks/rule_dependency.go      | 36 ++++++++++++++++++-------
 internal/checks/rule_dependency_test.go | 14 ++++++++++
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/docs/changelog.md b/docs/changelog.md
index c158413f..76f16ab4 100644
--- a/docs/changelog.md
+++ b/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
diff --git a/internal/checks/rule_dependency.go b/internal/checks/rule_dependency.go
index 4813523b..97abc673 100644
--- a/internal/checks/rule_dependency.go
+++ b/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
+}
diff --git a/internal/checks/rule_dependency_test.go b/internal/checks/rule_dependency_test.go
index f88e6295..79a78bfd 100644
--- a/internal/checks/rule_dependency_test.go
+++ b/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)