Skip to content

Commit

Permalink
Don't warn about re-added rules
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Jul 11, 2024
1 parent 19dbe12 commit cac486e
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 10 deletions.
6 changes: 6 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
36 changes: 26 additions & 10 deletions internal/checks/rule_dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
14 changes: 14 additions & 0 deletions internal/checks/rule_dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit cac486e

Please sign in to comment.