From b193eaed6eb197a7c919e0bbe35c1f43ddda9910 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Wed, 7 Sep 2022 09:28:10 -0500 Subject: [PATCH] Alerting: Resetting the notification policy tree to the default policy will also restore default contact points (#54608) * Add test that resetting the route restores the default receiver * Return error instead of panic * Adjust error string to match styleguide --- .../provisioning/notification_policies.go | 24 ++++ .../notification_policies_test.go | 107 +++++++++++------- pkg/services/ngalert/provisioning/testing.go | 9 ++ 3 files changed, 102 insertions(+), 38 deletions(-) diff --git a/pkg/services/ngalert/provisioning/notification_policies.go b/pkg/services/ngalert/provisioning/notification_policies.go index cbd8d3d23c6..b361a15335d 100644 --- a/pkg/services/ngalert/provisioning/notification_policies.go +++ b/pkg/services/ngalert/provisioning/notification_policies.go @@ -132,6 +132,10 @@ func (nps *NotificationPolicyService) ResetPolicyTree(ctx context.Context, orgID return definitions.Route{}, err } revision.cfg.AlertmanagerConfig.Config.Route = route + err = nps.ensureDefaultReceiverExists(revision.cfg, defaultCfg) + if err != nil { + return definitions.Route{}, err + } serialized, err := serializeAlertmanagerConfig(*revision.cfg) if err != nil { @@ -169,3 +173,23 @@ func (nps *NotificationPolicyService) receiversToMap(records []*definitions.Post } return receivers, nil } + +func (nps *NotificationPolicyService) ensureDefaultReceiverExists(cfg *definitions.PostableUserConfig, defaultCfg *definitions.PostableUserConfig) error { + defaultRcv := cfg.AlertmanagerConfig.Route.Receiver + + for _, rcv := range cfg.AlertmanagerConfig.Receivers { + if rcv.Name == defaultRcv { + return nil + } + } + + for _, rcv := range defaultCfg.AlertmanagerConfig.Receivers { + if rcv.Name == defaultRcv { + cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, rcv) + return nil + } + } + + nps.log.Error("Grafana Alerting has been configured with a default configuration that is internally inconsistent! The default configuration's notification policy must have a corresponding receiver.") + return fmt.Errorf("inconsistent default configuration") +} diff --git a/pkg/services/ngalert/provisioning/notification_policies_test.go b/pkg/services/ngalert/provisioning/notification_policies_test.go index 27b0de8b62a..11e551a20e2 100644 --- a/pkg/services/ngalert/provisioning/notification_policies_test.go +++ b/pkg/services/ngalert/provisioning/notification_policies_test.go @@ -31,19 +31,13 @@ func TestNotificationPolicyService(t *testing.T) { sut.amStore.(*MockAMConfigStore).On("GetLatestAlertmanagerConfiguration", mock.Anything, mock.Anything). Return( func(ctx context.Context, query *models.GetLatestAlertmanagerConfigurationQuery) error { - cfg, _ := deserializeAlertmanagerConfig([]byte(defaultConfig)) - mti := config.MuteTimeInterval{ - Name: "not-the-one-we-need", - TimeIntervals: []timeinterval.TimeInterval{}, + cfg := createTestAlertingConfig() + cfg.AlertmanagerConfig.MuteTimeIntervals = []config.MuteTimeInterval{ + { + Name: "not-the-one-we-need", + TimeIntervals: []timeinterval.TimeInterval{}, + }, } - cfg.AlertmanagerConfig.MuteTimeIntervals = append(cfg.AlertmanagerConfig.MuteTimeIntervals, mti) - cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, - &definitions.PostableApiReceiver{ - Receiver: config.Receiver{ - // default one from createTestRoutingTree() - Name: "a new receiver", - }, - }) data, _ := serializeAlertmanagerConfig(*cfg) query.Result = &models.AlertConfiguration{ AlertmanagerConfiguration: string(data), @@ -69,19 +63,13 @@ func TestNotificationPolicyService(t *testing.T) { sut.amStore.(*MockAMConfigStore).On("GetLatestAlertmanagerConfiguration", mock.Anything, mock.Anything). Return( func(ctx context.Context, query *models.GetLatestAlertmanagerConfigurationQuery) error { - cfg, _ := deserializeAlertmanagerConfig([]byte(defaultConfig)) - mti := config.MuteTimeInterval{ - Name: "existing", - TimeIntervals: []timeinterval.TimeInterval{}, + cfg := createTestAlertingConfig() + cfg.AlertmanagerConfig.MuteTimeIntervals = []config.MuteTimeInterval{ + { + Name: "existing", + TimeIntervals: []timeinterval.TimeInterval{}, + }, } - cfg.AlertmanagerConfig.MuteTimeIntervals = append(cfg.AlertmanagerConfig.MuteTimeIntervals, mti) - cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, - &definitions.PostableApiReceiver{ - Receiver: config.Receiver{ - // default one from createTestRoutingTree() - Name: "a new receiver", - }, - }) data, _ := serializeAlertmanagerConfig(*cfg) query.Result = &models.AlertConfiguration{ AlertmanagerConfiguration: string(data), @@ -132,20 +120,7 @@ func TestNotificationPolicyService(t *testing.T) { sut.amStore.(*MockAMConfigStore).On("GetLatestAlertmanagerConfiguration", mock.Anything, mock.Anything). Return( func(ctx context.Context, query *models.GetLatestAlertmanagerConfigurationQuery) error { - cfg, _ := deserializeAlertmanagerConfig([]byte(defaultConfig)) - cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, - &definitions.PostableApiReceiver{ - Receiver: config.Receiver{ - // default one from createTestRoutingTree() - Name: "a new receiver", - }, - }) - cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, - &definitions.PostableApiReceiver{ - Receiver: config.Receiver{ - Name: "existing", - }, - }) + cfg := createTestAlertingConfig() data, _ := serializeAlertmanagerConfig(*cfg) query.Result = &models.AlertConfiguration{ AlertmanagerConfiguration: string(data), @@ -225,6 +200,44 @@ func TestNotificationPolicyService(t *testing.T) { require.Nil(t, tree.Routes) require.Equal(t, []model.LabelName{models.FolderTitleLabel, model.AlertNameLabel}, tree.GroupBy) }) + + t.Run("deleting route with missing default receiver restores receiver", func(t *testing.T) { + sut := createNotificationPolicyServiceSut() + sut.amStore = &MockAMConfigStore{} + sut.amStore.(*MockAMConfigStore).On("GetLatestAlertmanagerConfiguration", mock.Anything, mock.Anything). + Return( + func(ctx context.Context, query *models.GetLatestAlertmanagerConfigurationQuery) error { + cfg := createTestAlertingConfig() + cfg.AlertmanagerConfig.Route = &definitions.Route{ + Receiver: "a new receiver", + } + cfg.AlertmanagerConfig.Receivers = []*definitions.PostableApiReceiver{ + { + Receiver: config.Receiver{ + Name: "a new receiver", + }, + }, + // No default receiver! Only our custom one. + } + data, _ := serializeAlertmanagerConfig(*cfg) + query.Result = &models.AlertConfiguration{ + AlertmanagerConfiguration: string(data), + } + return nil + }) + var interceptedSave = models.SaveAlertmanagerConfigurationCmd{} + sut.amStore.(*MockAMConfigStore).EXPECT().SaveSucceedsIntercept(&interceptedSave) + + tree, err := sut.ResetPolicyTree(context.Background(), 1) + + require.NoError(t, err) + require.Equal(t, "grafana-default-email", tree.Receiver) + require.NotEmpty(t, interceptedSave.AlertmanagerConfiguration) + // Deserializing with no error asserts that the saved config is semantically valid. + newCfg, err := deserializeAlertmanagerConfig([]byte(interceptedSave.AlertmanagerConfiguration)) + require.NoError(t, err) + require.Len(t, newCfg.AlertmanagerConfig.Receivers, 2) + }) } func createNotificationPolicyServiceSut() *NotificationPolicyService { @@ -244,3 +257,21 @@ func createTestRoutingTree() definitions.Route { Receiver: "a new receiver", } } + +func createTestAlertingConfig() *definitions.PostableUserConfig { + cfg, _ := deserializeAlertmanagerConfig([]byte(defaultConfig)) + cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, + &definitions.PostableApiReceiver{ + Receiver: config.Receiver{ + // default one from createTestRoutingTree() + Name: "a new receiver", + }, + }) + cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, + &definitions.PostableApiReceiver{ + Receiver: config.Receiver{ + Name: "existing", + }, + }) + return cfg +} diff --git a/pkg/services/ngalert/provisioning/testing.go b/pkg/services/ngalert/provisioning/testing.go index 28601edb95a..3afb57daf87 100644 --- a/pkg/services/ngalert/provisioning/testing.go +++ b/pkg/services/ngalert/provisioning/testing.go @@ -160,6 +160,15 @@ func (m *MockAMConfigStore_Expecter) SaveSucceeds() *MockAMConfigStore_Expecter return m } +func (m *MockAMConfigStore_Expecter) SaveSucceedsIntercept(intercepted *models.SaveAlertmanagerConfigurationCmd) *MockAMConfigStore_Expecter { + m.UpdateAlertmanagerConfiguration(mock.Anything, mock.Anything). + Return(nil). + Run(func(ctx context.Context, cmd *models.SaveAlertmanagerConfigurationCmd) { + *intercepted = *cmd + }) + return m +} + func (m *MockProvisioningStore_Expecter) GetReturns(p models.Provenance) *MockProvisioningStore_Expecter { m.GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(p, nil) return m