Skip to content

Commit

Permalink
Merge pull request #908 from LloydW93/lloyd/depcheck-segfault
Browse files Browse the repository at this point in the history
Fix nil pointer exceptions in certain broken dependency scenarios
  • Loading branch information
prymitive authored Mar 14, 2024
2 parents 4e245e0 + c3716b0 commit eb933aa
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
6 changes: 3 additions & 3 deletions internal/checks/rule_dependency.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ func (c RuleDependencyCheck) Check(_ context.Context, _ string, rule parser.Rule

var details strings.Builder
details.WriteString("If you remove the ")
details.WriteString(dep.kind)
details.WriteString(broken[0].kind)
details.WriteString(" rule generating `")
details.WriteString(dep.metric)
details.WriteString(broken[0].metric)
details.WriteString("`, and there is no other source of this metric, then any other rule depending on it will break.\n")
details.WriteString("List of found rules that are using `")
details.WriteString(dep.metric)
details.WriteString(broken[0].metric)
details.WriteString("`:\n\n")
for _, b := range broken {
details.WriteString("- `")
Expand Down
28 changes: 28 additions & 0 deletions internal/checks/rule_dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,34 @@ func TestRuleDependencyCheck(t *testing.T) {
`, discovery.Noop, "foo.yaml", "foo.yaml")[0],
},
},
{
description: "warns about removed dependency without crashing if it is not the last rule",
content: "- record: foo\n expr: sum(foo)\n",
checker: func(_ *promapi.FailoverGroup) checks.RuleChecker {
return checks.NewRuleDependencyCheck()
},
prometheus: newSimpleProm,
problems: func(_ string) []checks.Problem {
return []checks.Problem{
{
Anchor: checks.AnchorBefore,
Lines: parser.LineRange{
First: 1,
Last: 2,
},
Reporter: checks.RuleDependencyCheckName,
Text: textDependencyRule(1),
Details: detailsDependencyRule("recording", "foo", "- `alert` at `foo.yaml:2`\n"),
Severity: checks.Warning,
},
}
},
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: bar\n expr: vector(0)\n", discovery.Noop, "foo.yaml", "foo.yaml")[0],
},
},
}

runTests(t, testCases)
Expand Down

0 comments on commit eb933aa

Please sign in to comment.