diff --git a/pkg/services/ngalert/migration/alert_rule.go b/pkg/services/ngalert/migration/alert_rule.go index 04030e5b78ab..dd95668c73db 100644 --- a/pkg/services/ngalert/migration/alert_rule.go +++ b/pkg/services/ngalert/migration/alert_rule.go @@ -18,7 +18,7 @@ import ( "github.com/grafana/grafana/pkg/util" ) -func addLabelsAndAnnotations(l log.Logger, alert *legacymodels.Alert, dashboardUID string, channels []string) (data.Labels, data.Labels) { +func addLabelsAndAnnotations(l log.Logger, alert *legacymodels.Alert, dashboardUID string, channels []*legacymodels.AlertNotification) (data.Labels, data.Labels) { tags := alert.GetTagsFromSettings() lbls := make(data.Labels, len(tags)+len(channels)+1) @@ -29,7 +29,7 @@ func addLabelsAndAnnotations(l log.Logger, alert *legacymodels.Alert, dashboardU // Add a label for routing lbls[ngmodels.MigratedUseLegacyChannelsLabel] = "true" for _, c := range channels { - lbls[fmt.Sprintf(ngmodels.MigratedContactLabelTemplate, c)] = "true" + lbls[contactLabel(c.Name)] = "true" } annotations := make(data.Labels, 4) @@ -60,7 +60,7 @@ func (om *OrgMigration) migrateAlert(ctx context.Context, l log.Logger, alert *l return nil, fmt.Errorf("transform conditions: %w", err) } - channels := om.extractChannelUIDs(ctx, l, alert.OrgID, parsedSettings) + channels := om.extractChannels(l, parsedSettings) lbls, annotations := addLabelsAndAnnotations(l, alert, info.DashboardUID, channels) @@ -298,22 +298,27 @@ func truncate(daName string, length int) string { return daName } -// extractChannelUIDs extracts the notification channel UIDs from the given legacy dashboard alert parsed settings. -func (om *OrgMigration) extractChannelUIDs(ctx context.Context, l log.Logger, orgID int64, parsedSettings dashAlertSettings) (channelUids []string) { - // Extracting channel UID/ID. - for _, ui := range parsedSettings.Notifications { +// extractChannels extracts notification channels from the given legacy dashboard alert parsed settings. +func (om *OrgMigration) extractChannels(l log.Logger, parsedSettings dashAlertSettings) []*legacymodels.AlertNotification { + // Extracting channels. + channels := make([]*legacymodels.AlertNotification, 0, len(parsedSettings.Notifications)) + for _, key := range parsedSettings.Notifications { // Either id or uid can be defined in the dashboard alert notification settings. See alerting.NewRuleFromDBAlert. - if ui.ID > 0 { - uid, err := om.migrationStore.GetAlertNotificationUidWithId(ctx, orgID, ui.ID) - if err != nil { - l.Warn("Failed to get alert notification UID, skipping", "notificationId", ui.ID, "err", err) + if key.ID > 0 { + if c, ok := om.channelCache.GetChannelByID(key.ID); ok { + channels = append(channels, c) + continue + } + } + + if key.UID != "" { + if c, ok := om.channelCache.GetChannelByUID(key.UID); ok { + channels = append(channels, c) continue } - channelUids = append(channelUids, uid) - } else if ui.UID != "" { - channelUids = append(channelUids, ui.UID) } - } - return channelUids + l.Warn("Failed to get alert notification, skipping", "notificationKey", key) + } + return channels } diff --git a/pkg/services/ngalert/migration/channel.go b/pkg/services/ngalert/migration/channel.go index 229c81d1b44f..47b6bbf82bec 100644 --- a/pkg/services/ngalert/migration/channel.go +++ b/pkg/services/ngalert/migration/channel.go @@ -133,21 +133,23 @@ func (om *OrgMigration) createReceiver(channel *legacymodels.AlertNotification) // createRoute creates a route from a legacy notification channel, and matches using a label based on the channel UID. func createRoute(channel *legacymodels.AlertNotification, receiverName string) (*apimodels.Route, error) { - // We create a matchers based on channel UID so that we only need a single route per channel. - // All routes are stored in a nested route under the root. This is so we can keep the migrated channels separate + // We create a matchers based on channel name so that we only need a single route per channel. + // All channel routes are nested in a single route under the root. This is so we can keep the migrated channels separate // and organized. - // The matchers are created using a label that is unique to the channel UID. So, each migrated alert rule can define - // one label per migrated channel that it should send to. - // Default channels are matched using a catch-all matcher, because in legacy alerting they are attached to all - // alerts automatically. + // Since default channels are attached to all alerts in legacy, we use a catch-all matcher after migration instead + // of a specific label matcher. // - // For example, if an alert needs to send to channel1 and channel2 it will have two labels: + // For example, if an alert needs to send to channel1 and channel2 it will have one label to route to the nested + // policy and two channel-specific labels to route to the correct contact points: + // - __use_legacy_channels__="true" // - __contact_channel1__="true" // - __contact_channel2__="true" // - // These will match two routes as they are all defined with Continue=true. + // If an alert needs to send to channel1 and the default channel, it will have one label to route to the nested + // policy and one channel-specific label to route to channel1, and a catch-all policy will ensure it also routes to + // the default channel. - label := fmt.Sprintf(ngmodels.MigratedContactLabelTemplate, channel.UID) + label := contactLabel(channel.Name) mat, err := labels.NewMatcher(labels.MatchEqual, label, "true") if err != nil { return nil, err @@ -171,6 +173,11 @@ func createRoute(channel *legacymodels.AlertNotification, receiverName string) ( }, nil } +// contactLabel creates a label matcher key used to route alerts to a contact point. +func contactLabel(name string) string { + return fmt.Sprintf(ngmodels.MigratedContactLabelTemplate, name) +} + var secureKeysToMigrate = map[string][]string{ "slack": {"url", "token"}, "pagerduty": {"integrationKey"}, diff --git a/pkg/services/ngalert/migration/channel_test.go b/pkg/services/ngalert/migration/channel_test.go index 3cbc3417aa89..8bb5713f96fd 100644 --- a/pkg/services/ngalert/migration/channel_test.go +++ b/pkg/services/ngalert/migration/channel_test.go @@ -34,11 +34,11 @@ func TestCreateRoute(t *testing.T) { }{ { name: "when a receiver is passed in, the route should exact match based on channel uid with continue=true", - channel: &legacymodels.AlertNotification{UID: "uid1"}, - recv: createPostableApiReceiver("uid1", "recv1", nil), + channel: &legacymodels.AlertNotification{UID: "uid1", Name: "recv1"}, + recv: createPostableApiReceiver("uid1", "recv1"), expected: &apimodels.Route{ Receiver: "recv1", - ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, + ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("recv1"), Value: "true"}}, Routes: nil, Continue: true, GroupByStr: nil, @@ -46,12 +46,12 @@ func TestCreateRoute(t *testing.T) { }, }, { - name: "notification channel should be escaped for regex in the matcher", - channel: &legacymodels.AlertNotification{UID: "uid1"}, - recv: createPostableApiReceiver("uid1", `. ^ $ * + - ? ( ) [ ] { } \ |`, nil), + name: "notification channel labels matcher should work with special characters", + channel: &legacymodels.AlertNotification{UID: "uid1", Name: `. ^ $ * + - ? ( ) [ ] { } \ |`}, + recv: createPostableApiReceiver("uid1", `. ^ $ * + - ? ( ) [ ] { } \ |`), expected: &apimodels.Route{ Receiver: `. ^ $ * + - ? ( ) [ ] { } \ |`, - ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, + ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel(`. ^ $ * + - ? ( ) [ ] { } \ |`), Value: "true"}}, Routes: nil, Continue: true, GroupByStr: nil, @@ -60,11 +60,11 @@ func TestCreateRoute(t *testing.T) { }, { name: "when a channel has sendReminder=true, the route should use the frequency in repeat interval", - channel: &legacymodels.AlertNotification{SendReminder: true, Frequency: time.Duration(42) * time.Hour, UID: "uid1"}, - recv: createPostableApiReceiver("uid1", "recv1", nil), + channel: &legacymodels.AlertNotification{SendReminder: true, Frequency: time.Duration(42) * time.Hour, UID: "uid1", Name: "recv1"}, + recv: createPostableApiReceiver("uid1", "recv1"), expected: &apimodels.Route{ Receiver: "recv1", - ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, + ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("recv1"), Value: "true"}}, Routes: nil, Continue: true, GroupByStr: nil, @@ -73,11 +73,11 @@ func TestCreateRoute(t *testing.T) { }, { name: "when a channel has sendReminder=false, the route should ignore the frequency in repeat interval and use DisabledRepeatInterval", - channel: &legacymodels.AlertNotification{SendReminder: false, Frequency: time.Duration(42) * time.Hour, UID: "uid1"}, - recv: createPostableApiReceiver("uid1", "recv1", nil), + channel: &legacymodels.AlertNotification{SendReminder: false, Frequency: time.Duration(42) * time.Hour, UID: "uid1", Name: "recv1"}, + recv: createPostableApiReceiver("uid1", "recv1"), expected: &apimodels.Route{ Receiver: "recv1", - ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, + ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("recv1"), Value: "true"}}, Routes: nil, Continue: true, GroupByStr: nil, @@ -143,7 +143,7 @@ func TestCreateReceivers(t *testing.T) { { name: "when given notification channels migrate them to receivers", channel: createNotChannel(t, "uid1", int64(1), "name1", false, 0), - expRecv: createPostableApiReceiver("uid1", "name1", []string{"name1"}), + expRecv: createPostableApiReceiver("uid1", "name1"), }, { name: "when given hipchat return discontinued error", @@ -374,16 +374,16 @@ func TestSetupAlertmanagerConfig(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, }}, Receivers: []*apimodels.PostableApiReceiver{ {Receiver: config.Receiver{Name: "autogen-contact-point-default"}, PostableGrafanaReceivers: apimodels.PostableGrafanaReceivers{GrafanaManagedReceivers: []*apimodels.PostableGrafanaReceiver{}}}, - createPostableApiReceiver("uid1", "notifier1", []string{"notifier1"}), - createPostableApiReceiver("uid2", "notifier2", []string{"notifier2"}), + createPostableApiReceiver("uid1", "notifier1"), + createPostableApiReceiver("uid2", "notifier2"), }, }, }, @@ -402,15 +402,15 @@ func TestSetupAlertmanagerConfig(t *testing.T) { Continue: true, Routes: []*apimodels.Route{ {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchRegexp, Name: model.AlertNameLabel, Value: ".+"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, }}, Receivers: []*apimodels.PostableApiReceiver{ {Receiver: config.Receiver{Name: "autogen-contact-point-default"}, PostableGrafanaReceivers: apimodels.PostableGrafanaReceivers{GrafanaManagedReceivers: []*apimodels.PostableGrafanaReceiver{}}}, - createPostableApiReceiver("uid1", "notifier1", []string{"notifier1"}), - createPostableApiReceiver("uid2", "notifier2", []string{"notifier2"}), + createPostableApiReceiver("uid1", "notifier1"), + createPostableApiReceiver("uid2", "notifier2"), }, }, }, @@ -428,16 +428,16 @@ func TestSetupAlertmanagerConfig(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(42)}, - {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "uid2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(43)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(42)}, + {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(43)}, }, }, }, }}, Receivers: []*apimodels.PostableApiReceiver{ {Receiver: config.Receiver{Name: "autogen-contact-point-default"}, PostableGrafanaReceivers: apimodels.PostableGrafanaReceivers{GrafanaManagedReceivers: []*apimodels.PostableGrafanaReceiver{}}}, - createPostableApiReceiver("uid1", "notifier1", []string{"notifier1"}), - createPostableApiReceiver("uid2", "notifier2", []string{"notifier2"})}, + createPostableApiReceiver("uid1", "notifier1"), + createPostableApiReceiver("uid2", "notifier2")}, }, }, }, @@ -469,23 +469,21 @@ func TestSetupAlertmanagerConfig(t *testing.T) { } } -func createPostableApiReceiver(uid string, name string, integrationNames []string) *apimodels.PostableApiReceiver { - integrations := make([]*apimodels.PostableGrafanaReceiver, 0, len(integrationNames)) - for _, integrationName := range integrationNames { - integrations = append(integrations, &apimodels.PostableGrafanaReceiver{ - UID: uid, - Type: "email", - Name: integrationName, - Settings: apimodels.RawMessage("{}"), - SecureSettings: map[string]string{}, - }) - } +func createPostableApiReceiver(uid string, name string) *apimodels.PostableApiReceiver { return &apimodels.PostableApiReceiver{ Receiver: config.Receiver{ Name: name, }, PostableGrafanaReceivers: apimodels.PostableGrafanaReceivers{ - GrafanaManagedReceivers: integrations, + GrafanaManagedReceivers: []*apimodels.PostableGrafanaReceiver{ + { + UID: uid, + Type: "email", + Name: name, + Settings: apimodels.RawMessage("{}"), + SecureSettings: map[string]string{}, + }, + }, }, } } diff --git a/pkg/services/ngalert/migration/migration_test.go b/pkg/services/ngalert/migration/migration_test.go index c4acfb8631dd..40a5dd1f4341 100644 --- a/pkg/services/ngalert/migration/migration_test.go +++ b/pkg/services/ngalert/migration/migration_test.go @@ -14,6 +14,7 @@ import ( "github.com/prometheus/alertmanager/pkg/labels" "github.com/prometheus/common/model" "github.com/stretchr/testify/require" + "xorm.io/xorm" "github.com/grafana/grafana/pkg/components/simplejson" @@ -157,9 +158,9 @@ func TestAMConfigMigration(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier3", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier3"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier3", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier3"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -183,8 +184,8 @@ func TestAMConfigMigration(t *testing.T) { Continue: true, Routes: []*apimodels.Route{ {Receiver: "notifier6", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchRegexp, Name: model.AlertNameLabel, Value: ".+"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier4", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier4"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier5", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier5"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier4", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier4"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier5", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier5"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -216,7 +217,7 @@ func TestAMConfigMigration(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -283,8 +284,8 @@ func TestAMConfigMigration(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, - {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier2", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier2"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -315,7 +316,7 @@ func TestAMConfigMigration(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -347,7 +348,7 @@ func TestAMConfigMigration(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -380,7 +381,7 @@ func TestAMConfigMigration(t *testing.T) { ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: ngModels.MigratedUseLegacyChannelsLabel, Value: "true"}}, Continue: true, Routes: []*apimodels.Route{ - {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, + {Receiver: "notifier1", ObjectMatchers: apimodels.ObjectMatchers{{Type: labels.MatchEqual, Name: contactLabel("notifier1"), Value: "true"}}, Routes: nil, Continue: true, RepeatInterval: durationPointer(DisabledRepeatInterval)}, }, }, }, @@ -393,6 +394,17 @@ func TestAMConfigMigration(t *testing.T) { }, }, }, + { + name: "failed channel migration fails upgrade", + legacyChannels: []*models.AlertNotification{ + createAlertNotification(t, int64(1), "notifier1", "email", emailSettings, false), + createAlertNotification(t, int64(1), "notifier2", "slack", brokenSettings, false), + }, + alerts: []*models.Alert{ + createAlert(t, 1, 1, 1, "alert1", []string{"notifier1"}), + }, + expErrors: []string{"channel 'notifier2'"}, + }, } for _, tt := range tc { @@ -454,17 +466,15 @@ func TestAMConfigMigration(t *testing.T) { // TestDashAlertMigration tests the execution of the migration specifically for alert rules. func TestDashAlertMigration(t *testing.T) { - sqlStore := db.InitTestDB(t) - x := sqlStore.GetEngine() - service := NewTestMigrationService(t, sqlStore, &setting.Cfg{}) - withDefaults := func(lbls map[string]string) map[string]string { lbls[ngModels.MigratedUseLegacyChannelsLabel] = "true" return lbls } t.Run("when DashAlertMigration create ContactLabel on migrated AlertRules", func(t *testing.T) { - defer teardown(t, x, service) + sqlStore := db.InitTestDB(t) + x := sqlStore.GetEngine() + service := NewTestMigrationService(t, sqlStore, &setting.Cfg{}) legacyChannels := []*models.AlertNotification{ createAlertNotification(t, int64(1), "notifier1", "email", emailSettings, false), createAlertNotification(t, int64(1), "notifier2", "slack", slackSettings, false), @@ -483,14 +493,14 @@ func TestDashAlertMigration(t *testing.T) { } expected := map[int64]map[string]*ngModels.AlertRule{ int64(1): { - "alert1": {Labels: withDefaults(map[string]string{fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier1"): "true"})}, - "alert2": {Labels: withDefaults(map[string]string{fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier2"): "true", fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier3"): "true"})}, - "alert3": {Labels: withDefaults(map[string]string{fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier3"): "true"})}, + "alert1": {Labels: withDefaults(map[string]string{contactLabel("notifier1"): "true"})}, + "alert2": {Labels: withDefaults(map[string]string{contactLabel("notifier2"): "true", contactLabel("notifier3"): "true"})}, + "alert3": {Labels: withDefaults(map[string]string{contactLabel("notifier3"): "true"})}, }, int64(2): { // Don't include default channels. - "alert4": {Labels: withDefaults(map[string]string{fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier4"): "true"})}, - "alert5": {Labels: withDefaults(map[string]string{fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier4"): "true", fmt.Sprintf(ngModels.MigratedContactLabelTemplate, "notifier5"): "true"})}, + "alert4": {Labels: withDefaults(map[string]string{contactLabel("notifier4"): "true"})}, + "alert5": {Labels: withDefaults(map[string]string{contactLabel("notifier4"): "true", contactLabel("notifier5"): "true"})}, "alert6": {Labels: withDefaults(map[string]string{})}, }, } @@ -524,7 +534,9 @@ func TestDashAlertMigration(t *testing.T) { }) t.Run("when folder is missing put alert in General folder", func(t *testing.T) { - defer teardown(t, x, service) + sqlStore := db.InitTestDB(t) + x := sqlStore.GetEngine() + service := NewTestMigrationService(t, sqlStore, &setting.Cfg{}) o := createOrg(t, 1) folder1 := createFolder(t, 1, o.ID, "folder-1") dash1 := createDashboard(t, 3, o.ID, "dash1", folder1.ID, nil) @@ -558,6 +570,75 @@ func TestDashAlertMigration(t *testing.T) { require.Equal(t, expectedFolder.UID, rule.NamespaceUID) } }) + + t.Run("when alert notification settings contain different combinations of id and uid", func(t *testing.T) { + sqlStore := db.InitTestDB(t) + x := sqlStore.GetEngine() + service := NewTestMigrationService(t, sqlStore, &setting.Cfg{}) + legacyChannels := []*models.AlertNotification{ + createAlertNotification(t, int64(1), "notifier1", "email", emailSettings, false), + createAlertNotification(t, int64(1), "notifier2", "slack", slackSettings, false), + createAlertNotification(t, int64(1), "notifier3", "opsgenie", opsgenieSettings, false), + createAlertNotification(t, int64(2), "notifier4", "email", emailSettings, false), + createAlertNotification(t, int64(2), "notifier5", "slack", slackSettings, false), + createAlertNotification(t, int64(2), "notifier6", "opsgenie", opsgenieSettings, true), // default + } + alerts := []*models.Alert{ + createAlert(t, 1, 1, 1, "alert1", nil), + createAlert(t, 1, 1, 2, "alert2", nil), + createAlert(t, 1, 2, 3, "alert3", nil), + createAlert(t, 2, 3, 1, "alert4", nil), + createAlert(t, 2, 3, 2, "alert5", nil), + createAlert(t, 2, 4, 3, "alert6", nil), + } + alerts[0].Settings.Set("notifications", []notificationKey{{UID: "notifier1"}}) + alerts[1].Settings.Set("notifications", []notificationKey{{ID: 2}, {UID: "notifier3"}}) + alerts[2].Settings.Set("notifications", []notificationKey{{ID: 3, UID: "notifier4"}}) // This shouldn't happen, but if it does we choose the ID. + alerts[3].Settings.Set("notifications", []notificationKey{{ID: -99}}) // Unknown ID + alerts[4].Settings.Set("notifications", []notificationKey{{UID: "unknown"}}) // Unknown UID + alerts[5].Settings.Set("notifications", []notificationKey{{ID: -99}, {UID: "unknown"}, {UID: "notifier4"}, {ID: 5}}) // Mixed unknown and known. + + expected := map[int64]map[string]*ngModels.AlertRule{ + int64(1): { + "alert1": {Labels: withDefaults(map[string]string{contactLabel("notifier1"): "true"})}, + "alert2": {Labels: withDefaults(map[string]string{contactLabel("notifier2"): "true", contactLabel("notifier3"): "true"})}, + "alert3": {Labels: withDefaults(map[string]string{contactLabel("notifier3"): "true"})}, + }, + int64(2): { + // Don't include default channels. + "alert4": {Labels: withDefaults(map[string]string{})}, + "alert5": {Labels: withDefaults(map[string]string{})}, + "alert6": {Labels: withDefaults(map[string]string{contactLabel("notifier4"): "true", contactLabel("notifier5"): "true"})}, + }, + } + dashes := []*dashboards.Dashboard{ + createDashboard(t, 1, 1, "dash1-1", 5, nil), + createDashboard(t, 2, 1, "dash2-1", 5, nil), + createDashboard(t, 3, 2, "dash3-2", 6, nil), + createDashboard(t, 4, 2, "dash4-2", 6, nil), + } + folders := []*dashboards.Dashboard{ + createFolder(t, 5, 1, "folder5-1"), + createFolder(t, 6, 2, "folder6-2"), + } + setupLegacyAlertsTables(t, x, legacyChannels, alerts, folders, dashes) + err := service.Run(context.Background()) + require.NoError(t, err) + + for orgId := range expected { + rules := getAlertRules(t, x, orgId) + expectedRulesMap := expected[orgId] + require.Len(t, rules, len(expectedRulesMap)) + for _, r := range rules { + delete(r.Labels, "rule_uid") // Not checking this here. + exp := expectedRulesMap[r.Title].Labels + require.Lenf(t, r.Labels, len(exp), "rule doesn't have correct number of labels: %s", r.Title) + for l := range r.Labels { + require.Equal(t, exp[l], r.Labels[l]) + } + } + } + }) } // TestDashAlertQueryMigration tests the execution of the migration specifically for alert rule queries. @@ -657,6 +738,7 @@ func TestDashAlertQueryMigration(t *testing.T) { expectedFolder *dashboards.Dashboard expected map[int64][]*ngModels.AlertRule + expErrors []string } tc := []testcase{ @@ -931,6 +1013,14 @@ func TestDashAlertQueryMigration(t *testing.T) { }, }, }, + { + name: "failed alert migration fails upgrade", + alerts: []*models.Alert{ + createAlertWithCond(t, 1, 1, 1, "alert1", nil, + []dashAlertCondition{{}}), + }, + expErrors: []string{"migrate alert 'alert1'"}, + }, } for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { @@ -955,6 +1045,12 @@ func TestDashAlertQueryMigration(t *testing.T) { setupLegacyAlertsTables(t, x, nil, tt.alerts, folders, dashes) err := service.Run(context.Background()) + if len(tt.expErrors) > 0 { + for _, expErr := range tt.expErrors { + require.ErrorContains(t, err, expErr) + } + return + } require.NoError(t, err) for orgId, expected := range tt.expected { @@ -999,6 +1095,7 @@ const ( emailSettings = `{"addresses": "test"}` slackSettings = `{"recipient": "test", "token": "test"}` opsgenieSettings = `{"apiKey": "test"}` + brokenSettings = `[{"unknown": 1.5}]` ) var ( @@ -1081,9 +1178,7 @@ func createAlertWithCond(t *testing.T, orgId int, dashboardId int, panelsId int, if len(notifierUids) != 0 { notifiers := make([]any, 0) for _, n := range notifierUids { - notifiers = append(notifiers, struct { - Uid string - }{Uid: n}) + notifiers = append(notifiers, notificationKey{UID: n}) } settings.Set("notifications", notifiers) diff --git a/pkg/services/ngalert/migration/models.go b/pkg/services/ngalert/migration/models.go index 7ede252c8c92..06ba61891b6a 100644 --- a/pkg/services/ngalert/migration/models.go +++ b/pkg/services/ngalert/migration/models.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" + legacymodels "github.com/grafana/grafana/pkg/services/alerting/models" "github.com/grafana/grafana/pkg/services/folder" migmodels "github.com/grafana/grafana/pkg/services/ngalert/migration/models" migrationStore "github.com/grafana/grafana/pkg/services/ngalert/migration/store" @@ -29,6 +30,7 @@ type OrgMigration struct { silences []*pb.MeshSilence alertRuleTitleDedup map[string]Deduplicator // Folder -> Deduplicator (Title). alertRuleGroupDedup map[string]Deduplicator // Folder -> Deduplicator (Group). + channelCache *ChannelCache // Migrated folder for a dashboard based on permissions. Parent Folder ID -> unique dashboard permission -> custom folder. permissionsMap map[int64]map[permissionHash]*folder.Folder @@ -56,6 +58,7 @@ func (ms *migrationService) newOrgMigration(orgID int64) *OrgMigration { // We deduplicate alert rule groups so that we don't have to ensure that the alerts rules have the same interval. alertRuleGroupDedup: make(map[string]Deduplicator), + channelCache: &ChannelCache{cache: make(map[any]*legacymodels.AlertNotification)}, permissionsMap: make(map[int64]map[permissionHash]*folder.Folder), folderCache: make(map[int64]*folder.Folder), @@ -90,6 +93,28 @@ func (om *OrgMigration) AlertGroupDeduplicator(folderUID string) Deduplicator { return om.alertRuleGroupDedup[folderUID] } +// ChannelCache caches channels by ID and UID. +type ChannelCache struct { + cache map[any]*legacymodels.AlertNotification +} + +func (c *ChannelCache) LoadChannels(channels []*legacymodels.AlertNotification) { + for _, channel := range channels { + c.cache[channel.ID] = channel + c.cache[channel.UID] = channel + } +} + +func (c *ChannelCache) GetChannelByID(id int64) (*legacymodels.AlertNotification, bool) { + channel, ok := c.cache[id] + return channel, ok +} + +func (c *ChannelCache) GetChannelByUID(uid string) (*legacymodels.AlertNotification, bool) { + channel, ok := c.cache[uid] + return channel, ok +} + // Deduplicator is a wrapper around map[string]struct{} and util.GenerateShortUID() which aims help maintain and generate // unique strings (such as uids or titles). if caseInsensitive is true, all uniqueness is determined in a // case-insensitive manner. if maxLen is greater than 0, all strings will be truncated to maxLen before being checked in diff --git a/pkg/services/ngalert/migration/store/database.go b/pkg/services/ngalert/migration/store/database.go index 7611cc3e88b7..f4a41f2ea68f 100644 --- a/pkg/services/ngalert/migration/store/database.go +++ b/pkg/services/ngalert/migration/store/database.go @@ -41,7 +41,6 @@ type Store interface { GetDatasource(ctx context.Context, datasourceID int64, user identity.Requester) (*datasources.DataSource, error) - GetAlertNotificationUidWithId(ctx context.Context, orgID int64, id int64) (string, error) GetNotificationChannels(ctx context.Context, orgID int64) ([]*legacymodels.AlertNotification, error) GetOrgDashboardAlerts(ctx context.Context, orgID int64) (map[int64][]*legacymodels.Alert, int, error) @@ -79,7 +78,6 @@ type migrationStore struct { dashboardPermissions accesscontrol.DashboardPermissionsService orgService org.Service - legacyAlertStore legacyalerting.AlertStore legacyAlertNotificationService *legacyalerting.AlertNotificationService } @@ -97,7 +95,6 @@ func ProvideMigrationStore( folderPermissions accesscontrol.FolderPermissionsService, dashboardPermissions accesscontrol.DashboardPermissionsService, orgService org.Service, - legacyAlertStore legacyalerting.AlertStore, legacyAlertNotificationService *legacyalerting.AlertNotificationService, ) (Store, error) { return &migrationStore{ @@ -112,7 +109,6 @@ func ProvideMigrationStore( folderPermissions: folderPermissions, dashboardPermissions: dashboardPermissions, orgService: orgService, - legacyAlertStore: legacyAlertStore, legacyAlertNotificationService: legacyAlertNotificationService, }, nil } @@ -205,6 +201,7 @@ func (ms *migrationStore) InsertAlertRules(ctx context.Context, rules ...models. return nil } +// SaveAlertmanagerConfiguration saves the alertmanager configuration for the given org. func (ms *migrationStore) SaveAlertmanagerConfiguration(ctx context.Context, orgID int64, amConfig *apimodels.PostableUserConfig) error { rawAmConfig, err := json.Marshal(amConfig) if err != nil { @@ -361,27 +358,22 @@ func (ms *migrationStore) DeleteFolders(ctx context.Context, orgID int64, uids . return nil } +// GetDashboard returns a single dashboard for the given org and dashboard id. func (ms *migrationStore) GetDashboard(ctx context.Context, orgID int64, id int64) (*dashboards.Dashboard, error) { return ms.dashboardService.GetDashboard(ctx, &dashboards.GetDashboardQuery{ID: id, OrgID: orgID}) } +// GetAllOrgs returns all orgs. func (ms *migrationStore) GetAllOrgs(ctx context.Context) ([]*org.OrgDTO, error) { orgQuery := &org.SearchOrgsQuery{} return ms.orgService.Search(ctx, orgQuery) } +// GetDatasource returns a single datasource for the given org and datasource id. func (ms *migrationStore) GetDatasource(ctx context.Context, datasourceID int64, user identity.Requester) (*datasources.DataSource, error) { return ms.dataSourceCache.GetDatasource(ctx, datasourceID, user, false) } -func (ms *migrationStore) GetAlertNotificationUidWithId(ctx context.Context, orgID int64, id int64) (string, error) { - cmd := legacymodels.GetAlertNotificationUidQuery{ - ID: id, - OrgID: orgID, - } - return ms.legacyAlertStore.GetAlertNotificationUidWithId(ctx, &cmd) -} - // GetNotificationChannels returns all channels for this org. func (ms *migrationStore) GetNotificationChannels(ctx context.Context, orgID int64) ([]*legacymodels.AlertNotification, error) { return ms.legacyAlertNotificationService.GetAllAlertNotifications(ctx, &legacymodels.GetAllAlertNotificationsQuery{ diff --git a/pkg/services/ngalert/migration/store/testing.go b/pkg/services/ngalert/migration/store/testing.go index 0e83ae52e3ba..a5a0c320cc4a 100644 --- a/pkg/services/ngalert/migration/store/testing.go +++ b/pkg/services/ngalert/migration/store/testing.go @@ -45,6 +45,7 @@ func NewTestMigrationStore(t *testing.T, sqlStore *sqlstore.SQLStore, cfg *setti alertingStore := store.DBstore{ SQLStore: sqlStore, Cfg: cfg.UnifiedAlerting, + Logger: &logtest.Fake{}, } bus := bus.ProvideBus(tracing.InitializeTracerForTest()) folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore) @@ -86,8 +87,6 @@ func NewTestMigrationStore(t *testing.T, sqlStore *sqlstore.SQLStore, cfg *setti err = acSvc.RegisterFixedRoles(context.Background()) require.NoError(t, err) - legacyAlertStore := legacyalerting.ProvideAlertStore(sqlStore, cache, cfg, nil, features) - return &migrationStore{ log: &logtest.Fake{}, cfg: cfg, @@ -100,7 +99,6 @@ func NewTestMigrationStore(t *testing.T, sqlStore *sqlstore.SQLStore, cfg *setti folderPermissions: folderPermissions, dashboardPermissions: dashboardPermissions, orgService: orgService, - legacyAlertStore: legacyAlertStore, legacyAlertNotificationService: legacyalerting.ProvideService(sqlStore, encryptionservice.SetupTestService(t), nil), } } diff --git a/pkg/services/ngalert/migration/ualert.go b/pkg/services/ngalert/migration/ualert.go index fe552ee6ac43..f44783828b30 100644 --- a/pkg/services/ngalert/migration/ualert.go +++ b/pkg/services/ngalert/migration/ualert.go @@ -73,6 +73,9 @@ func (om *OrgMigration) migrateOrgChannels(ctx context.Context) (*migmodels.Aler return nil, fmt.Errorf("load notification channels: %w", err) } + // Cache for later use by alerts + om.channelCache.LoadChannels(channels) + amConfig, err := om.migrateChannels(channels) if err != nil { return nil, err @@ -83,16 +86,14 @@ func (om *OrgMigration) migrateOrgChannels(ctx context.Context) (*migmodels.Aler func (om *OrgMigration) migrateOrg(ctx context.Context) error { om.log.Info("Migrating alerts for organisation") - err := om.migrateOrgAlerts(ctx) + amConfig, err := om.migrateOrgChannels(ctx) if err != nil { - return fmt.Errorf("migrate alerts: %w", err) + return fmt.Errorf("migrate channels: %w", err) } - // This must happen before we insert the rules into the database because it modifies the alert labels. This will - // be changed in the future when we improve how notification policies are created. - amConfig, err := om.migrateOrgChannels(ctx) + err = om.migrateOrgAlerts(ctx) if err != nil { - return fmt.Errorf("migrate channels: %w", err) + return fmt.Errorf("migrate alerts: %w", err) } if err := om.writeSilencesFile(); err != nil {