From dd1edf7f16a6d9ff132002eea175a5c1765777e3 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Mon, 22 Dec 2025 21:23:59 +0100 Subject: [PATCH] Alerting: Fix database-based filtering by labels when rules have no labels (#115657) Alerting: Fix database-based filtering by labels when rules have no labels at all --- pkg/services/ngalert/store/alert_rule_labels_test.go | 12 ++++++------ pkg/services/ngalert/store/alert_rule_test.go | 12 +++++++++--- pkg/services/ngalert/store/json.go | 12 ++++++------ pkg/services/ngalert/store/json_test.go | 12 ++++++------ 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/pkg/services/ngalert/store/alert_rule_labels_test.go b/pkg/services/ngalert/store/alert_rule_labels_test.go index 9b72d8f00f9..2006c49fd7a 100644 --- a/pkg/services/ngalert/store/alert_rule_labels_test.go +++ b/pkg/services/ngalert/store/alert_rule_labels_test.go @@ -73,42 +73,42 @@ func TestBuildLabelMatcherJSON(t *testing.T) { name: "MySQL MatchEqual with non-empty value", dialect: migrator.NewMysqlDialect(), matcher: &labels.Matcher{Type: labels.MatchEqual, Name: "team", Value: "alerting"}, - wantSQL: "JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) = ?", + wantSQL: "JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) = ?", wantArgs: []any{"team", "alerting"}, }, { name: "MySQL MatchEqual with empty value", dialect: migrator.NewMysqlDialect(), matcher: &labels.Matcher{Type: labels.MatchEqual, Name: "team", Value: ""}, - wantSQL: "(JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) = ? OR JSON_EXTRACT(labels, CONCAT('$.', ?)) IS NULL)", + wantSQL: "(JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) = ? OR JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?)) IS NULL)", wantArgs: []any{"team", "", "team"}, }, { name: "MySQL MatchNotEqual", dialect: migrator.NewMysqlDialect(), matcher: &labels.Matcher{Type: labels.MatchNotEqual, Name: "team", Value: "alerting"}, - wantSQL: "(JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) IS NULL OR JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) != ?)", + wantSQL: "(JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) IS NULL OR JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) != ?)", wantArgs: []any{"team", "team", "alerting"}, }, { name: "PostgreSQL MatchEqual with non-empty value", dialect: migrator.NewPostgresDialect(), matcher: &labels.Matcher{Type: labels.MatchEqual, Name: "team", Value: "alerting"}, - wantSQL: "jsonb_extract_path_text(labels::jsonb, ?) = ?", + wantSQL: "jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) = ?", wantArgs: []any{"team", "alerting"}, }, { name: "PostgreSQL MatchEqual with empty value", dialect: migrator.NewPostgresDialect(), matcher: &labels.Matcher{Type: labels.MatchEqual, Name: "team", Value: ""}, - wantSQL: "(jsonb_extract_path_text(labels::jsonb, ?) = ? OR jsonb_extract_path_text(labels::jsonb, ?) IS NULL)", + wantSQL: "(jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) = ? OR jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) IS NULL)", wantArgs: []any{"team", "", "team"}, }, { name: "PostgreSQL MatchNotEqual", dialect: migrator.NewPostgresDialect(), matcher: &labels.Matcher{Type: labels.MatchNotEqual, Name: "team", Value: "alerting"}, - wantSQL: "(jsonb_extract_path_text(labels::jsonb, ?) IS NULL OR jsonb_extract_path_text(labels::jsonb, ?) != ?)", + wantSQL: "(jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) IS NULL OR jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) != ?)", wantArgs: []any{"team", "team", "alerting"}, }, { diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 29b82f943ba..f7497c7b05a 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -2462,6 +2462,12 @@ func TestIntegration_ListAlertRules(t *testing.T) { ruleNonempty := createRule(t, store, ruleGen.With( ruleGen.WithLabels(map[string]string{"empty": "nonempty"}), ruleGen.WithTitle("rule_nonempty"))) + // include a rule with no labels at all, + // to ensure we handle that case correctly. + // JSON functions need to be able to handle null and empty string values. + ruleNoLabels := createRule(t, store, ruleGen.With( + ruleGen.WithLabels(map[string]string{}), + ruleGen.WithTitle("rule_no_labels"))) tc := []struct { name string @@ -2487,7 +2493,7 @@ func TestIntegration_ListAlertRules(t *testing.T) { labelMatchers: labels.Matchers{ func() *labels.Matcher { m, _ := labels.NewMatcher(labels.MatchNotEqual, "team", "alerting"); return m }(), }, - expectedRules: []*models.AlertRule{ruleUpper, ruleSpecial, ruleGlob, ruleSpecialChars, ruleEmpty, ruleNonempty}, + expectedRules: []*models.AlertRule{ruleUpper, ruleSpecial, ruleGlob, ruleSpecialChars, ruleEmpty, ruleNonempty, ruleNoLabels}, }, { name: "special characters in labels are handled correctly", @@ -2536,7 +2542,7 @@ func TestIntegration_ListAlertRules(t *testing.T) { labelMatchers: labels.Matchers{ func() *labels.Matcher { m, _ := labels.NewMatcher(labels.MatchEqual, "empty", ""); return m }(), }, - expectedRules: []*models.AlertRule{ruleLower, ruleUpper, ruleSpecial, ruleGlob, ruleSpecialChars, ruleEmpty}, + expectedRules: []*models.AlertRule{ruleLower, ruleUpper, ruleSpecial, ruleGlob, ruleSpecialChars, ruleEmpty, ruleNoLabels}, }, { name: "inequality matcher on non-existent label matches all rules", @@ -2546,7 +2552,7 @@ func TestIntegration_ListAlertRules(t *testing.T) { return m }(), }, - expectedRules: []*models.AlertRule{ruleLower, ruleUpper, ruleSpecial, ruleGlob, ruleSpecialChars, ruleEmpty, ruleNonempty}, + expectedRules: []*models.AlertRule{ruleLower, ruleUpper, ruleSpecial, ruleGlob, ruleSpecialChars, ruleEmpty, ruleNonempty, ruleNoLabels}, }, } diff --git a/pkg/services/ngalert/store/json.go b/pkg/services/ngalert/store/json.go index 7b634246951..e0c71975a76 100644 --- a/pkg/services/ngalert/store/json.go +++ b/pkg/services/ngalert/store/json.go @@ -13,9 +13,9 @@ import ( func jsonEquals(dialect migrator.Dialect, column, key, value string) (string, []any) { switch dialect.DriverName() { case migrator.MySQL: - return fmt.Sprintf("JSON_UNQUOTE(JSON_EXTRACT(%s, CONCAT('$.', ?))) = ?", column), []any{key, value} + return fmt.Sprintf("JSON_UNQUOTE(JSON_EXTRACT(NULLIF(%s, ''), CONCAT('$.', ?))) = ?", column), []any{key, value} case migrator.Postgres: - return fmt.Sprintf("jsonb_extract_path_text(%s::jsonb, ?) = ?", column), []any{key, value} + return fmt.Sprintf("jsonb_extract_path_text(NULLIF(%s, '')::jsonb, ?) = ?", column), []any{key, value} default: return "", nil } @@ -25,9 +25,9 @@ func jsonNotEquals(dialect migrator.Dialect, column, key, value string) (string, var jx string switch dialect.DriverName() { case migrator.MySQL: - jx = fmt.Sprintf("JSON_UNQUOTE(JSON_EXTRACT(%s, CONCAT('$.', ?)))", column) + jx = fmt.Sprintf("JSON_UNQUOTE(JSON_EXTRACT(NULLIF(%s, ''), CONCAT('$.', ?)))", column) case migrator.Postgres: - jx = fmt.Sprintf("jsonb_extract_path_text(%s::jsonb, ?)", column) + jx = fmt.Sprintf("jsonb_extract_path_text(NULLIF(%s, '')::jsonb, ?)", column) default: return "", nil } @@ -37,9 +37,9 @@ func jsonNotEquals(dialect migrator.Dialect, column, key, value string) (string, func jsonKeyMissing(dialect migrator.Dialect, column, key string) (string, []any) { switch dialect.DriverName() { case migrator.MySQL: - return fmt.Sprintf("JSON_EXTRACT(%s, CONCAT('$.', ?)) IS NULL", column), []any{key} + return fmt.Sprintf("JSON_EXTRACT(NULLIF(%s, ''), CONCAT('$.', ?)) IS NULL", column), []any{key} case migrator.Postgres: - return fmt.Sprintf("jsonb_extract_path_text(%s::jsonb, ?) IS NULL", column), []any{key} + return fmt.Sprintf("jsonb_extract_path_text(NULLIF(%s, '')::jsonb, ?) IS NULL", column), []any{key} default: return "", nil } diff --git a/pkg/services/ngalert/store/json_test.go b/pkg/services/ngalert/store/json_test.go index 89f85a027a6..d09d3741c4b 100644 --- a/pkg/services/ngalert/store/json_test.go +++ b/pkg/services/ngalert/store/json_test.go @@ -23,7 +23,7 @@ func TestJsonEquals(t *testing.T) { column: "labels", key: "team", value: "alerting", - wantSQL: "JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) = ?", + wantSQL: "JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) = ?", wantArgs: []any{"team", "alerting"}, }, { @@ -32,7 +32,7 @@ func TestJsonEquals(t *testing.T) { column: "labels", key: "team", value: "alerting", - wantSQL: "jsonb_extract_path_text(labels::jsonb, ?) = ?", + wantSQL: "jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) = ?", wantArgs: []any{"team", "alerting"}, }, } @@ -62,7 +62,7 @@ func TestJsonNotEquals(t *testing.T) { column: "labels", key: "team", value: "alerting", - wantSQL: "(JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) IS NULL OR JSON_UNQUOTE(JSON_EXTRACT(labels, CONCAT('$.', ?))) != ?)", + wantSQL: "(JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) IS NULL OR JSON_UNQUOTE(JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?))) != ?)", wantArgs: []any{"team", "team", "alerting"}, }, { @@ -71,7 +71,7 @@ func TestJsonNotEquals(t *testing.T) { column: "labels", key: "team", value: "alerting", - wantSQL: "(jsonb_extract_path_text(labels::jsonb, ?) IS NULL OR jsonb_extract_path_text(labels::jsonb, ?) != ?)", + wantSQL: "(jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) IS NULL OR jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) != ?)", wantArgs: []any{"team", "team", "alerting"}, }, } @@ -99,7 +99,7 @@ func TestJsonKeyMissing(t *testing.T) { dialect: migrator.NewMysqlDialect(), column: "labels", key: "team", - wantSQL: "JSON_EXTRACT(labels, CONCAT('$.', ?)) IS NULL", + wantSQL: "JSON_EXTRACT(NULLIF(labels, ''), CONCAT('$.', ?)) IS NULL", wantArgs: []any{"team"}, }, { @@ -107,7 +107,7 @@ func TestJsonKeyMissing(t *testing.T) { dialect: migrator.NewPostgresDialect(), column: "labels", key: "team", - wantSQL: "jsonb_extract_path_text(labels::jsonb, ?) IS NULL", + wantSQL: "jsonb_extract_path_text(NULLIF(labels, '')::jsonb, ?) IS NULL", wantArgs: []any{"team"}, }, }