From fc3636acf26d8e44bff3f6ac04f5e10190f1ea11 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Thu, 11 Sep 2025 13:44:06 +0200 Subject: [PATCH] Alerting: Fix bug where rules with identical mute/active intervals produced conflicting routes (#110935) Alerting: Fix hash collision in NotificationSettings fingerprint --- .../ngalert/api/api_alertmanager_test.go | 10 ++-- pkg/services/ngalert/models/notifications.go | 4 ++ .../ngalert/models/notifications_test.go | 47 ++++++++++++++- .../notifier/autogen_alertmanager_test.go | 58 +++++++++---------- 4 files changed, 82 insertions(+), 37 deletions(-) diff --git a/pkg/services/ngalert/api/api_alertmanager_test.go b/pkg/services/ngalert/api/api_alertmanager_test.go index bd253f5a685..bbe1df17155 100644 --- a/pkg/services/ngalert/api/api_alertmanager_test.go +++ b/pkg/services/ngalert/api/api_alertmanager_test.go @@ -688,15 +688,15 @@ var validConfigWithAutogen = `{ "receiver": "some email", "object_matchers": [["__grafana_autogenerated__", "=", "true"]], "routes": [{ - "receiver": "some email", - "group_by": ["grafana_folder", "alertname"], - "object_matchers": [["__grafana_receiver__", "=", "some email"]], - "continue": false - },{ "receiver": "other email", "group_by": ["grafana_folder", "alertname"], "object_matchers": [["__grafana_receiver__", "=", "other email"]], "continue": false + },{ + "receiver": "some email", + "group_by": ["grafana_folder", "alertname"], + "object_matchers": [["__grafana_receiver__", "=", "some email"]], + "continue": false }] },{ "receiver": "other email", diff --git a/pkg/services/ngalert/models/notifications.go b/pkg/services/ngalert/models/notifications.go index f6f9ad3dce1..36e4df6ae0d 100644 --- a/pkg/services/ngalert/models/notifications.go +++ b/pkg/services/ngalert/models/notifications.go @@ -190,8 +190,12 @@ func (s *NotificationSettings) Fingerprint() data.Fingerprint { for _, interval := range s.MuteTimeIntervals { writeString(interval) } + // Add a separator between the time intervals to avoid collisions + // when all settings are the same including interval names except for the interval type (mute vs active). + _, _ = h.Write([]byte{255}) for _, interval := range s.ActiveTimeIntervals { writeString(interval) } + return data.Fingerprint(h.Sum64()) } diff --git a/pkg/services/ngalert/models/notifications_test.go b/pkg/services/ngalert/models/notifications_test.go index 0af9f3dfa14..2e7df039895 100644 --- a/pkg/services/ngalert/models/notifications_test.go +++ b/pkg/services/ngalert/models/notifications_test.go @@ -113,6 +113,8 @@ func TestValidate(t *testing.T) { } func TestNotificationSettingsLabels(t *testing.T) { + timeInterval := "time-interval-1" + testCases := []struct { name string notificationSettings NotificationSettings @@ -135,7 +137,7 @@ func TestNotificationSettingsLabels(t *testing.T) { labels: data.Labels{ AutogeneratedRouteLabel: "true", AutogeneratedRouteReceiverNameLabel: "receiver name", - AutogeneratedRouteSettingsHashLabel: "6027cdeaff62ba3f", + AutogeneratedRouteSettingsHashLabel: "c65d254ff4c279f2", }, }, { @@ -151,7 +153,7 @@ func TestNotificationSettingsLabels(t *testing.T) { labels: data.Labels{ AutogeneratedRouteLabel: "true", AutogeneratedRouteReceiverNameLabel: "receiver name", - AutogeneratedRouteSettingsHashLabel: "47164c92f2986a35", + AutogeneratedRouteSettingsHashLabel: "634e52b238fc78f0", }, }, { @@ -168,7 +170,25 @@ func TestNotificationSettingsLabels(t *testing.T) { labels: data.Labels{ AutogeneratedRouteLabel: "true", AutogeneratedRouteReceiverNameLabel: "receiver name", - AutogeneratedRouteSettingsHashLabel: "a173df6210e43af0", + AutogeneratedRouteSettingsHashLabel: "9ac606ba0f6bcfb5", + }, + }, + { + name: "default notification settings with active time interval", + notificationSettings: CopyNotificationSettings(NewDefaultNotificationSettings("receiver name"), NSMuts.WithActiveTimeIntervals(timeInterval)), + labels: data.Labels{ + AutogeneratedRouteLabel: "true", + AutogeneratedRouteReceiverNameLabel: "receiver name", + AutogeneratedRouteSettingsHashLabel: "8304d9c06fda36e2", + }, + }, + { + name: "default notification settings with mute time interval", + notificationSettings: CopyNotificationSettings(NewDefaultNotificationSettings("receiver name"), NSMuts.WithMuteTimeIntervals(timeInterval)), + labels: data.Labels{ + AutogeneratedRouteLabel: "true", + AutogeneratedRouteReceiverNameLabel: "receiver name", + AutogeneratedRouteSettingsHashLabel: "171cfd2d4e0810fa", }, }, } @@ -181,6 +201,27 @@ func TestNotificationSettingsLabels(t *testing.T) { } } +func TestNotificationSettings_TimeIntervals(t *testing.T) { + // Create notification settings with default settings and usign the same + // time interval, but in one case as a mute time interval and in another case + // as an active time interval. They should produce different hashes. + + receiver := "receiver name" + timeInterval := "time interval name" + + muteSettings := NotificationSettings{ + Receiver: receiver, + MuteTimeIntervals: []string{timeInterval}, + } + + activeSettings := NotificationSettings{ + Receiver: receiver, + ActiveTimeIntervals: []string{timeInterval}, + } + + require.NotEqual(t, activeSettings.Fingerprint(), muteSettings.Fingerprint()) +} + func TestNormalizedGroupBy(t *testing.T) { validNotificationSettings := NotificationSettingsGen() diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go index 7d47c0974d4..78239c06479 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go @@ -86,9 +86,9 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - basicContactRoute("receiver1"), - basicContactRoute("receiver3"), basicContactRoute("receiver2"), + basicContactRoute("receiver3"), + basicContactRoute("receiver1"), }, }), }, @@ -100,9 +100,9 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - basicContactRoute("receiver1"), - basicContactRoute("receiver3"), basicContactRoute("receiver2"), + basicContactRoute("receiver3"), + basicContactRoute("receiver1"), }, }), }, @@ -130,42 +130,42 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - withChildRoutes(basicContactRoute("receiver5"), &definitions.Route{ - Receiver: "receiver5", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "030d6474aec0b553"), - MuteTimeIntervals: []string{"maintenance"}, - }, &definitions.Route{ - Receiver: "receiver5", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "cd6cd2089632453c"), - ActiveTimeIntervals: []string{"active"}, - }), withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "dde34b8127e68f31"), - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, &definitions.Route{ Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "f134b8faf7db083c"), + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "02466789dc88da23"), GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), GroupWait: util.Pointer(model.Duration(2 * time.Minute)), RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), MuteTimeIntervals: []string{"maintenance"}, ActiveTimeIntervals: []string{"active"}, + }, &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), }), withChildRoutes(basicContactRoute("receiver2"), &definitions.Route{ Receiver: "receiver2", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "27e1d1717c9ef621"), + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "63ad04d6c21c3aec"), GroupWait: util.Pointer(model.Duration(2 * time.Minute)), }), + withChildRoutes(basicContactRoute("receiver5"), &definitions.Route{ + Receiver: "receiver5", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "8cd5f9adeac58123"), + ActiveTimeIntervals: []string{"active"}, + }, &definitions.Route{ + Receiver: "receiver5", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "f0770544f1741cf6"), + MuteTimeIntervals: []string{"maintenance"}, + }), withChildRoutes(basicContactRoute("receiver4"), &definitions.Route{ Receiver: "receiver4", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "b3a2fa5e615dcc7e"), + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "9bbbec5f72627ae5"), GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, }), withChildRoutes(basicContactRoute("receiver3"), &definitions.Route{ Receiver: "receiver3", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "9e282ef0193d830a"), + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "fbcacbfae385a901"), RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), }), }, @@ -183,7 +183,7 @@ func TestAddAutogenConfig(t *testing.T) { Routes: []*definitions.Route{ withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "dde34b8127e68f31"), + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), GroupByStr: nil, GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), }), @@ -203,13 +203,13 @@ func TestAddAutogenConfig(t *testing.T) { Routes: []*definitions.Route{ withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "dde34b8127e68f31"), - GroupByStr: nil, + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "828092ed6f427a00"), // Different hash. + GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), }, &definitions.Route{ Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "e1f3a275a8918385"), // Different hash. - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), + GroupByStr: nil, GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), }), }, @@ -229,7 +229,7 @@ func TestAddAutogenConfig(t *testing.T) { Routes: []*definitions.Route{ withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "e1f3a275a8918385"), + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "828092ed6f427a00"), GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), }), @@ -249,9 +249,9 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - basicContactRoute("receiver1"), - basicContactRoute("receiver3"), basicContactRoute("receiver2"), + basicContactRoute("receiver3"), + basicContactRoute("receiver1"), }, }), },