Fix absent checks

linode-obs · Apr 6, 2022 · f97b24c · f97b24c
1 parent 5d3783e
commit f97b24c
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 22 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## v0.17.4
+
+### Fixed
+
+- Fixed false positive reports from `alerts/template` check when `absent()` is
+  used inside a binary expression.
+
 ## v0.17.3
 
 ### Fixed

diff --git a/internal/checks/alerts_template_test.go b/internal/checks/alerts_template_test.go
@@ -572,6 +572,17 @@ func TestTemplateCheck(t *testing.T) {
   expr: bar * on() group_right() absent(foo{job="xxx"})
   annotations:
     summary: '{{ .Labels.job }} in cluster {{$labels.cluster}}/{{ $labels.env }} is missing'
+`,
+			checker:  newTemplateCheck,
+			problems: noProblems,
+		},
+		{
+			description: "",
+			content: `
+- alert: Foo
+  expr: foo and on() absent(bar)
+  annotations:
+    summary: '{{ .Labels.job }} is missing'
 `,
 			checker:  newTemplateCheck,
 			problems: noProblems,

diff --git a/internal/parser/utils/absent.go b/internal/parser/utils/absent.go
@@ -24,30 +24,44 @@ func HasOuterAbsent(node *parser.PromQLNode) (calls []PromQLFragment) {
 			// bar / absent(foo)
 			// absent(foo) / bar
 			case promParser.CardOneToOne:
-			// CardManyToOne:
+
 			// absent(foo{job="bar"}) * on(job) group_left(xxx) bar
 			// bar * on() group_left(xxx) absent(foo{job="bar"})
-			// CardOneToMany
-			// bar * on() group_right(xxx) absent(foo{job="bar"})
-			// absent(foo{job="bar"}) * on(job) group_right(xxx) bar
-			case promParser.CardManyToOne, promParser.CardOneToMany:
+			case promParser.CardManyToOne:
 				if ln, ok := n.LHS.(*promParser.Call); ok && ln.Func.Name == "absent" {
 					calls = append(calls, PromQLFragment{
 						Fragment: node.Children[0],
 						BinExpr:  n,
 					})
 				}
+
+			// bar * on() group_right(xxx) absent(foo{job="bar"})
+			// absent(foo{job="bar"}) * on(job) group_right(xxx) bar
+			case promParser.CardOneToMany:
 				if rn, ok := n.RHS.(*promParser.Call); ok && rn.Func.Name == "absent" {
 					calls = append(calls, PromQLFragment{
 						Fragment: node.Children[1],
 						BinExpr:  n,
 					})
 				}
+
 			// bar AND absent(foo{job="bar"})
+			// bar OR absent(foo{job="bar"})
+			// bar UNLESS absent(foo{job="bar"})
 			case promParser.CardManyToMany:
-				for _, child := range node.Children {
-					calls = append(calls, HasOuterAbsent(child)...)
+				if n.Op == promParser.LOR {
+					for _, child := range node.Children {
+						calls = append(calls, HasOuterAbsent(child)...)
+					}
+				} else {
+					if ln, ok := n.LHS.(*promParser.Call); ok && ln.Func.Name == "absent" {
+						calls = append(calls, PromQLFragment{
+							Fragment: node.Children[0],
+							BinExpr:  n,
+						})
+					}
 				}
+
 			default:
 				log.Warn().Str("matching", n.VectorMatching.Card.String()).Msg("Unsupported VectorMatching operation")
 			}

diff --git a/internal/parser/utils/absent_test.go b/internal/parser/utils/absent_test.go
@@ -35,19 +35,22 @@ func TestHasOuterAbsent(t *testing.T) {
 		{
 			expr: `absent(foo{job="bar"}) AND on(job) bar`,
 			output: []callT{{
-				call: `absent(foo{job="bar"})`,
+				call:    `absent(foo{job="bar"})`,
+				binExpr: `absent(foo{job="bar"}) and on(job) bar`,
 			}},
 		},
 		{
 			expr: `vector(1) or absent(foo{job="bar"}) AND on(job) bar`,
-			output: []callT{
-				{call: `absent(foo{job="bar"})`},
-			},
+			output: []callT{{
+				call:    `absent(foo{job="bar"})`,
+				binExpr: `absent(foo{job="bar"}) and on(job) bar`,
+			}},
 		},
 		{
 			expr: `up == 0 or absent(foo{job="bar"}) AND on(job) bar`,
 			output: []callT{{
-				call: `absent(foo{job="bar"})`,
+				call:    `absent(foo{job="bar"})`,
+				binExpr: `absent(foo{job="bar"}) and on(job) bar`,
 			}},
 		},
 		{
@@ -71,11 +74,8 @@ func TestHasOuterAbsent(t *testing.T) {
 			}},
 		},
 		{
-			expr: `bar * on() group_left(xxx) absent(foo{job="bar"})`,
-			output: []callT{{
-				call:    `absent(foo{job="bar"})`,
-				binExpr: `bar * on() group_left(xxx) absent(foo{job="bar"})`,
-			}},
+			expr:   `bar * on() group_left(xxx) absent(foo{job="bar"})`,
+			output: []callT{},
 		},
 		{
 			expr: `up == 0 or absent(foo{job="bar"}) * on(job) group_left() bar`,
@@ -92,18 +92,31 @@ func TestHasOuterAbsent(t *testing.T) {
 			}},
 		},
 		{
-			expr: `absent(foo{job="bar"}) * on(job) group_right(xxx) bar`,
-			output: []callT{{
-				call:    `absent(foo{job="bar"})`,
-				binExpr: `absent(foo{job="bar"}) * on(job) group_right(xxx) bar`,
-			}},
+			expr:   `absent(foo{job="bar"}) * on(job) group_right(xxx) bar`,
+			output: []callT{},
 		},
 		{
 			expr: `absent(foo{job="bar"}) OR bar`,
 			output: []callT{{
 				call: `absent(foo{job="bar"})`,
 			}},
 		},
+		{
+			expr: `absent(foo{job="bar"}) OR absent(foo{job="bob"})`,
+			output: []callT{
+				{call: `absent(foo{job="bar"})`},
+				{call: `absent(foo{job="bob"})`},
+			},
+		},
+		{
+			expr: `absent(foo{job="bar"}) UNLESS absent(foo{job="bob"})`,
+			output: []callT{
+				{
+					call:    `absent(foo{job="bar"})`,
+					binExpr: `absent(foo{job="bar"}) unless absent(foo{job="bob"})`,
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {