From b4634afe33d657940efac448c2d34d07e45a8c89 Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Wed, 2 Mar 2022 16:40:53 +0800 Subject: [PATCH] use alerting service instead of store in provisioning (#45926) * fix notification * Fix confir reader notifiers test * Move UpdateAlertNotificationWithUid to alertingService * Rename Store to Manager * Rename Store to Manager in provisioning Co-authored-by: Ida Furjesova --- .../notifiers/alert_notifications.go | 50 ++++--- .../notifiers/config_reader_test.go | 126 +++++++----------- pkg/services/provisioning/provisioning.go | 10 +- 3 files changed, 86 insertions(+), 100 deletions(-) diff --git a/pkg/services/provisioning/notifiers/alert_notifications.go b/pkg/services/provisioning/notifiers/alert_notifications.go index 3195837c72b..aac68649cda 100644 --- a/pkg/services/provisioning/notifiers/alert_notifications.go +++ b/pkg/services/provisioning/notifiers/alert_notifications.go @@ -8,38 +8,50 @@ import ( "golang.org/x/net/context" ) -type Store interface { - GetOrgById(c context.Context, cmd *models.GetOrgByIdQuery) error - GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error +type Manager interface { + GetAlertNotifications(ctx context.Context, query *models.GetAlertNotificationsQuery) error + CreateAlertNotificationCommand(ctx context.Context, cmd *models.CreateAlertNotificationCommand) error + UpdateAlertNotification(ctx context.Context, cmd *models.UpdateAlertNotificationCommand) error + DeleteAlertNotification(ctx context.Context, cmd *models.DeleteAlertNotificationCommand) error + GetAllAlertNotifications(ctx context.Context, query *models.GetAllAlertNotificationsQuery) error + GetOrCreateAlertNotificationState(ctx context.Context, cmd *models.GetOrCreateNotificationStateQuery) error + SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToCompleteCommand) error + SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToPendingCommand) error GetAlertNotificationsWithUid(ctx context.Context, query *models.GetAlertNotificationsWithUidQuery) error DeleteAlertNotificationWithUid(ctx context.Context, cmd *models.DeleteAlertNotificationWithUidCommand) error - CreateAlertNotificationCommand(ctx context.Context, cmd *models.CreateAlertNotificationCommand) error + GetAlertNotificationsWithUidToSend(ctx context.Context, query *models.GetAlertNotificationsWithUidToSendQuery) error UpdateAlertNotificationWithUid(ctx context.Context, cmd *models.UpdateAlertNotificationWithUidCommand) error } +type SQLStore interface { + GetOrgById(c context.Context, cmd *models.GetOrgByIdQuery) error + GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error +} // Provision alert notifiers -func Provision(ctx context.Context, configDirectory string, store Store, encryptionService encryption.Internal, notificationService *notifications.NotificationService) error { - dc := newNotificationProvisioner(store, encryptionService, notificationService, log.New("provisioning.notifiers")) +func Provision(ctx context.Context, configDirectory string, alertingService Manager, sqlstore SQLStore, encryptionService encryption.Internal, notificationService *notifications.NotificationService) error { + dc := newNotificationProvisioner(sqlstore, alertingService, encryptionService, notificationService, log.New("provisioning.notifiers")) return dc.applyChanges(ctx, configDirectory) } // NotificationProvisioner is responsible for provsioning alert notifiers type NotificationProvisioner struct { - log log.Logger - cfgProvider *configReader - store Store + log log.Logger + cfgProvider *configReader + alertingManager Manager + sqlstore SQLStore } -func newNotificationProvisioner(store Store, encryptionService encryption.Internal, notifiationService *notifications.NotificationService, log log.Logger) NotificationProvisioner { +func newNotificationProvisioner(store SQLStore, alertingManager Manager, encryptionService encryption.Internal, notifiationService *notifications.NotificationService, log log.Logger) NotificationProvisioner { return NotificationProvisioner{ - log: log, - store: store, + log: log, + alertingManager: alertingManager, cfgProvider: &configReader{ encryptionService: encryptionService, notificationService: notifiationService, log: log, orgStore: store, }, + sqlstore: store, } } @@ -61,7 +73,7 @@ func (dc *NotificationProvisioner) deleteNotifications(ctx context.Context, noti if notification.OrgID == 0 && notification.OrgName != "" { getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} - if err := dc.store.GetOrgByNameHandler(ctx, getOrg); err != nil { + if err := dc.sqlstore.GetOrgByNameHandler(ctx, getOrg); err != nil { return err } notification.OrgID = getOrg.Result.Id @@ -71,13 +83,13 @@ func (dc *NotificationProvisioner) deleteNotifications(ctx context.Context, noti getNotification := &models.GetAlertNotificationsWithUidQuery{Uid: notification.UID, OrgId: notification.OrgID} - if err := dc.store.GetAlertNotificationsWithUid(ctx, getNotification); err != nil { + if err := dc.alertingManager.GetAlertNotificationsWithUid(ctx, getNotification); err != nil { return err } if getNotification.Result != nil { cmd := &models.DeleteAlertNotificationWithUidCommand{Uid: getNotification.Result.Uid, OrgId: getNotification.OrgId} - if err := dc.store.DeleteAlertNotificationWithUid(ctx, cmd); err != nil { + if err := dc.alertingManager.DeleteAlertNotificationWithUid(ctx, cmd); err != nil { return err } } @@ -90,7 +102,7 @@ func (dc *NotificationProvisioner) mergeNotifications(ctx context.Context, notif for _, notification := range notificationToMerge { if notification.OrgID == 0 && notification.OrgName != "" { getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} - if err := dc.store.GetOrgByNameHandler(ctx, getOrg); err != nil { + if err := dc.sqlstore.GetOrgByNameHandler(ctx, getOrg); err != nil { return err } notification.OrgID = getOrg.Result.Id @@ -99,7 +111,7 @@ func (dc *NotificationProvisioner) mergeNotifications(ctx context.Context, notif } cmd := &models.GetAlertNotificationsWithUidQuery{OrgId: notification.OrgID, Uid: notification.UID} - err := dc.store.GetAlertNotificationsWithUid(ctx, cmd) + err := dc.alertingManager.GetAlertNotificationsWithUid(ctx, cmd) if err != nil { return err } @@ -119,7 +131,7 @@ func (dc *NotificationProvisioner) mergeNotifications(ctx context.Context, notif SendReminder: notification.SendReminder, } - if err := dc.store.CreateAlertNotificationCommand(ctx, insertCmd); err != nil { + if err := dc.alertingManager.CreateAlertNotificationCommand(ctx, insertCmd); err != nil { return err } } else { @@ -137,7 +149,7 @@ func (dc *NotificationProvisioner) mergeNotifications(ctx context.Context, notif SendReminder: notification.SendReminder, } - if err := dc.store.UpdateAlertNotificationWithUid(ctx, updateCmd); err != nil { + if err := dc.alertingManager.UpdateAlertNotificationWithUid(ctx, updateCmd); err != nil { return err } } diff --git a/pkg/services/provisioning/notifiers/config_reader_test.go b/pkg/services/provisioning/notifiers/config_reader_test.go index 7e8a0dd760b..c8cae8b6886 100644 --- a/pkg/services/provisioning/notifiers/config_reader_test.go +++ b/pkg/services/provisioning/notifiers/config_reader_test.go @@ -6,7 +6,6 @@ import ( "os" "testing" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" @@ -37,7 +36,6 @@ func TestNotificationAsConfig(t *testing.T) { t.Run("Testing notification as configuration", func(t *testing.T) { setup := func() { sqlStore = sqlstore.InitTestDB(t) - setupBusHandlers(sqlStore) for i := 1; i < 5; i++ { orgCommand := models.CreateOrgCommand{Name: fmt.Sprintf("Main Org. %v", i)} @@ -140,17 +138,14 @@ func TestNotificationAsConfig(t *testing.T) { t.Run("One configured notification", func(t *testing.T) { t.Run("no notification in database", func(t *testing.T) { setup() - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + fakeAlertNotification := &fakeAlertNotification{} + fakeAlertNotification.ExpectedAlertNotification = &models.AlertNotification{OrgId: 1} + dc := newNotificationProvisioner(sqlStore, fakeAlertNotification, ossencryption.ProvideService(), nil, logger) err := dc.applyChanges(context.Background(), twoNotificationsConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - notificationsQuery := models.GetAllAlertNotificationsQuery{OrgId: 1} - err = sqlStore.GetAllAlertNotifications(context.Background(), ¬ificationsQuery) - require.NoError(t, err) - require.NotNil(t, notificationsQuery.Result) - require.Equal(t, len(notificationsQuery.Result), 2) }) t.Run("One notification in database with same name and uid", func(t *testing.T) { @@ -171,42 +166,19 @@ func TestNotificationAsConfig(t *testing.T) { require.Equal(t, len(notificationsQuery.Result), 1) t.Run("should update one notification", func(t *testing.T) { - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, ossencryption.ProvideService(), nil, logger) err = dc.applyChanges(context.Background(), twoNotificationsConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - err = sqlStore.GetAllAlertNotifications(context.Background(), ¬ificationsQuery) - require.NoError(t, err) - require.NotNil(t, notificationsQuery.Result) - require.Equal(t, len(notificationsQuery.Result), 2) - - nts := notificationsQuery.Result - nt1 := nts[0] - require.Equal(t, nt1.Type, "email") - require.Equal(t, nt1.Name, "channel1") - require.Equal(t, nt1.Uid, "notifier1") - - nt2 := nts[1] - require.Equal(t, nt2.Type, "slack") - require.Equal(t, nt2.Name, "channel2") - require.Equal(t, nt2.Uid, "notifier2") }) }) t.Run("Two notifications with is_default", func(t *testing.T) { setup() - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, ossencryption.ProvideService(), nil, logger) err := dc.applyChanges(context.Background(), doubleNotificationsConfig) t.Run("should both be inserted", func(t *testing.T) { require.NoError(t, err) - notificationsQuery := models.GetAllAlertNotificationsQuery{OrgId: 1} - err = sqlStore.GetAllAlertNotifications(context.Background(), ¬ificationsQuery) - require.NoError(t, err) - require.NotNil(t, notificationsQuery.Result) - require.Equal(t, len(notificationsQuery.Result), 2) - - require.True(t, notificationsQuery.Result[0].IsDefault) - require.True(t, notificationsQuery.Result[1].IsDefault) }) }) }) @@ -238,16 +210,11 @@ func TestNotificationAsConfig(t *testing.T) { require.Equal(t, len(notificationsQuery.Result), 2) t.Run("should have two new notifications", func(t *testing.T) { - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, ossencryption.ProvideService(), nil, logger) err := dc.applyChanges(context.Background(), twoNotificationsConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - notificationsQuery = models.GetAllAlertNotificationsQuery{OrgId: 1} - err = sqlStore.GetAllAlertNotifications(context.Background(), ¬ificationsQuery) - require.NoError(t, err) - require.NotNil(t, notificationsQuery.Result) - require.Equal(t, len(notificationsQuery.Result), 4) }) }) }) @@ -272,26 +239,16 @@ func TestNotificationAsConfig(t *testing.T) { err = sqlStore.CreateAlertNotificationCommand(context.Background(), &existingNotificationCmd) require.NoError(t, err) - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, ossencryption.ProvideService(), nil, logger) err = dc.applyChanges(context.Background(), correctPropertiesWithOrgName) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - - notificationsQuery := models.GetAllAlertNotificationsQuery{OrgId: existingOrg2.Result.Id} - err = sqlStore.GetAllAlertNotifications(context.Background(), ¬ificationsQuery) - require.NoError(t, err) - require.NotNil(t, notificationsQuery.Result) - require.Equal(t, len(notificationsQuery.Result), 1) - - nt := notificationsQuery.Result[0] - require.Equal(t, nt.Name, "default-notification-create") - require.Equal(t, nt.OrgId, existingOrg2.Result.Id) }) t.Run("Config doesn't contain required field", func(t *testing.T) { setup() - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, ossencryption.ProvideService(), nil, logger) err := dc.applyChanges(context.Background(), noRequiredFields) require.NotNil(t, err) @@ -305,7 +262,7 @@ func TestNotificationAsConfig(t *testing.T) { t.Run("Empty yaml file", func(t *testing.T) { t.Run("should have not changed repo", func(t *testing.T) { setup() - dc := newNotificationProvisioner(sqlStore, ossencryption.ProvideService(), nil, logger) + dc := newNotificationProvisioner(sqlStore, &fakeAlertNotification{}, ossencryption.ProvideService(), nil, logger) err := dc.applyChanges(context.Background(), emptyFile) if err != nil { t.Fatalf("applyChanges return an error %v", err) @@ -366,32 +323,45 @@ func TestNotificationAsConfig(t *testing.T) { }) } -func setupBusHandlers(sqlStore *sqlstore.SQLStore) { - bus.AddHandler("getOrg", func(ctx context.Context, q *models.GetOrgByNameQuery) error { - return sqlStore.GetOrgByNameHandler(ctx, q) - }) - - bus.AddHandler("getAlertNotifications", func(ctx context.Context, q *models.GetAlertNotificationsWithUidQuery) error { - return sqlStore.GetAlertNotificationsWithUid(ctx, q) - }) - - bus.AddHandler("createAlertNotification", func(ctx context.Context, cmd *models.CreateAlertNotificationCommand) error { - return sqlStore.CreateAlertNotificationCommand(ctx, cmd) - }) - - bus.AddHandler("updateAlertNotification", func(ctx context.Context, cmd *models.UpdateAlertNotificationCommand) error { - return sqlStore.UpdateAlertNotification(ctx, cmd) - }) - - bus.AddHandler("updateAlertNotification", func(ctx context.Context, cmd *models.UpdateAlertNotificationWithUidCommand) error { - return sqlStore.UpdateAlertNotificationWithUid(ctx, cmd) - }) +type fakeAlertNotification struct { + ExpectedAlertNotification *models.AlertNotification +} - bus.AddHandler("deleteAlertNotification", func(ctx context.Context, cmd *models.DeleteAlertNotificationCommand) error { - return sqlStore.DeleteAlertNotification(ctx, cmd) - }) +func (f *fakeAlertNotification) GetAlertNotifications(ctx context.Context, query *models.GetAlertNotificationsQuery) error { + query.Result = f.ExpectedAlertNotification + return nil +} +func (f *fakeAlertNotification) CreateAlertNotificationCommand(ctx context.Context, cmd *models.CreateAlertNotificationCommand) error { + return nil +} +func (f *fakeAlertNotification) UpdateAlertNotification(ctx context.Context, cmd *models.UpdateAlertNotificationCommand) error { + return nil +} +func (f *fakeAlertNotification) DeleteAlertNotification(ctx context.Context, cmd *models.DeleteAlertNotificationCommand) error { + return nil +} +func (f *fakeAlertNotification) GetAllAlertNotifications(ctx context.Context, query *models.GetAllAlertNotificationsQuery) error { + return nil +} +func (f *fakeAlertNotification) GetOrCreateAlertNotificationState(ctx context.Context, cmd *models.GetOrCreateNotificationStateQuery) error { + return nil +} +func (f *fakeAlertNotification) SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToCompleteCommand) error { + return nil +} +func (f *fakeAlertNotification) SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToPendingCommand) error { + return nil +} +func (f *fakeAlertNotification) GetAlertNotificationsWithUid(ctx context.Context, query *models.GetAlertNotificationsWithUidQuery) error { + return nil +} +func (f *fakeAlertNotification) DeleteAlertNotificationWithUid(ctx context.Context, cmd *models.DeleteAlertNotificationWithUidCommand) error { + return nil +} +func (f *fakeAlertNotification) GetAlertNotificationsWithUidToSend(ctx context.Context, query *models.GetAlertNotificationsWithUidToSendQuery) error { + return nil +} - bus.AddHandler("deleteAlertNotification", func(ctx context.Context, cmd *models.DeleteAlertNotificationWithUidCommand) error { - return sqlStore.DeleteAlertNotificationWithUid(ctx, cmd) - }) +func (f *fakeAlertNotification) UpdateAlertNotificationWithUid(ctx context.Context, cmd *models.UpdateAlertNotificationWithUidCommand) error { + return nil } diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 303ab11c17c..ab0678583c3 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" plugifaces "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/registry" + "github.com/grafana/grafana/pkg/services/alerting" dashboardservice "github.com/grafana/grafana/pkg/services/dashboards" datasourceservice "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/encryption" @@ -26,6 +27,7 @@ func ProvideService(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore, pluginStore p encryptionService encryption.Internal, notificatonService *notifications.NotificationService, dashboardService dashboardservice.DashboardProvisioningService, datasourceService datasourceservice.DataSourceService, + alertingService *alerting.AlertNotificationService, ) (*ProvisioningServiceImpl, error) { s := &ProvisioningServiceImpl{ Cfg: cfg, @@ -40,6 +42,7 @@ func ProvideService(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore, pluginStore p provisionPlugins: plugins.Provision, dashboardService: dashboardService, datasourceService: datasourceService, + alertingService: alertingService, } return s, nil } @@ -69,7 +72,7 @@ func NewProvisioningServiceImpl() *ProvisioningServiceImpl { // Used for testing purposes func newProvisioningServiceImpl( newDashboardProvisioner dashboards.DashboardProvisionerFactory, - provisionNotifiers func(context.Context, string, notifiers.Store, encryption.Internal, *notifications.NotificationService) error, + provisionNotifiers func(context.Context, string, notifiers.Manager, notifiers.SQLStore, encryption.Internal, *notifications.NotificationService) error, provisionDatasources func(context.Context, string, datasources.Store, utils.OrgStore) error, provisionPlugins func(context.Context, string, plugins.Store, plugifaces.Store) error, ) *ProvisioningServiceImpl { @@ -92,12 +95,13 @@ type ProvisioningServiceImpl struct { pollingCtxCancel context.CancelFunc newDashboardProvisioner dashboards.DashboardProvisionerFactory dashboardProvisioner dashboards.DashboardProvisioner - provisionNotifiers func(context.Context, string, notifiers.Store, encryption.Internal, *notifications.NotificationService) error + provisionNotifiers func(context.Context, string, notifiers.Manager, notifiers.SQLStore, encryption.Internal, *notifications.NotificationService) error provisionDatasources func(context.Context, string, datasources.Store, utils.OrgStore) error provisionPlugins func(context.Context, string, plugins.Store, plugifaces.Store) error mutex sync.Mutex dashboardService dashboardservice.DashboardProvisioningService datasourceService datasourceservice.DataSourceService + alertingService *alerting.AlertNotificationService } func (ps *ProvisioningServiceImpl) RunInitProvisioners(ctx context.Context) error { @@ -170,7 +174,7 @@ func (ps *ProvisioningServiceImpl) ProvisionPlugins(ctx context.Context) error { func (ps *ProvisioningServiceImpl) ProvisionNotifications(ctx context.Context) error { alertNotificationsPath := filepath.Join(ps.Cfg.ProvisioningPath, "notifiers") - if err := ps.provisionNotifiers(ctx, alertNotificationsPath, ps.SQLStore, ps.EncryptionService, ps.NotificationService); err != nil { + if err := ps.provisionNotifiers(ctx, alertNotificationsPath, ps.alertingService, ps.SQLStore, ps.EncryptionService, ps.NotificationService); err != nil { err = errutil.Wrap("Alert notification provisioning error", err) ps.log.Error("Failed to provision alert notifications", "error", err) return err