Skip to content

Commit

Permalink
Fix a false positive when bool modifier is used
Browse files Browse the repository at this point in the history
  • Loading branch information
prymitive committed Oct 27, 2022
1 parent 687236f commit c839563
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
12 changes: 10 additions & 2 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,23 @@
### Fixed
- Fixed a false positive in [promql/vector_matching](checks/promql/vector_matching.md)
- Fixed false positive reports in [promql/vector_matching](checks/promql/vector_matching.md)
for rules using `on(...)`. Example:
```
sum(foo) without(instance) * on(app_name) group_left() bar
```
- Don't log passwords when Prometheus URI is using basic authentication.
- Fixed a false positive reports in [alerts/template](checks/alerts/template.md)
- Fixed false positive reports in [alerts/template](checks/alerts/template.md)
suggeting to use `humanize` on queries that already use `round()`.
- Fixed false positive reports in [alerts/comparison](checks/alerts/comparison.md)
when `bool` modifier is used on a condtion that is guarded by another conditon.
Example:
```yaml
alert: Foo
expr: (foo > 1) > bool 1
```
## v0.30.2
Expand Down
10 changes: 5 additions & 5 deletions internal/checks/alerts_comparison.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ func (c ComparisonCheck) Check(ctx context.Context, rule parser.Rule, entries []

expr := rule.Expr().Query

if expr := hasComparision(expr); expr != nil {
if expr.ReturnBool {
if n := hasComparision(expr.Node); n != nil {
if n.ReturnBool && hasComparision(n.LHS) == nil && hasComparision(n.RHS) == nil {
problems = append(problems, Problem{
Fragment: rule.AlertingRule.Expr.Value.Value,
Lines: rule.AlertingRule.Expr.Lines(),
Expand All @@ -70,8 +70,8 @@ func (c ComparisonCheck) Check(ctx context.Context, rule parser.Rule, entries []
return
}

func hasComparision(n *parser.PromQLNode) *promParser.BinaryExpr {
if node, ok := n.Node.(*promParser.BinaryExpr); ok {
func hasComparision(n promParser.Node) *promParser.BinaryExpr {
if node, ok := n.(*promParser.BinaryExpr); ok {
if node.Op.IsComparisonOperator() {
return node
}
Expand All @@ -80,7 +80,7 @@ func hasComparision(n *parser.PromQLNode) *promParser.BinaryExpr {
}
}

for _, child := range n.Children {
for _, child := range promParser.Children(n) {
if node := hasComparision(child); node != nil {
return node
}
Expand Down
7 changes: 7 additions & 0 deletions internal/checks/alerts_comparison_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ func TestComparisonCheck(t *testing.T) {
prometheus: noProm,
problems: noProblems,
},
{
description: "(foo > 1) > bool 1",
content: "- alert: Foo Is Missing\n expr: (foo > 1) > bool 1\n",
checker: newComparisonCheck,
prometheus: noProm,
problems: noProblems,
},
}

runTests(t, testCases)
Expand Down

0 comments on commit c839563

Please sign in to comment.