From f132e929cebe40c1aea2c8b96c2acae3c78661f3 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Fri, 14 Dec 2018 11:53:50 +0200 Subject: [PATCH] Added uid for alert notifications --- .../alert_notifications/sample.yaml | 14 +- docs/sources/administration/provisioning.md | 6 +- pkg/models/alert_notifications.go | 40 +++- pkg/services/alerting/interfaces.go | 2 +- pkg/services/alerting/notifier.go | 12 +- pkg/services/alerting/notifiers/base.go | 8 +- pkg/services/alerting/notifiers/base_test.go | 2 +- pkg/services/alerting/rule.go | 21 +- pkg/services/alerting/rule_test.go | 159 +------------ .../alerting/testdata/dash-without-id.json | 2 +- .../alerting/testdata/influxdb-alert.json | 2 +- .../alert_notifications.go | 58 +++-- .../alert_notifications/config_reader.go | 47 +++- .../alert_notifications/config_reader_test.go | 219 ++++++++++-------- .../correct-properties-with-orgName.yaml | 19 +- .../correct-properties.yaml | 11 +- .../test-configs/double-default/default-1.yml | 1 + .../double-default/default-2.yaml | 1 + .../incorrect-settings.yaml} | 1 + .../no-required-fields.yaml | 35 +++ .../two-notifications/two-notifications.yaml | 13 +- .../unknown-notifier/notification.yaml | 3 +- .../provisioning/alert_notifications/types.go | 17 +- pkg/services/sqlstore/alert_notification.go | 150 +++++++++++- .../sqlstore/alert_notification_test.go | 33 ++- pkg/services/sqlstore/migrations/alert_mig.go | 14 ++ 26 files changed, 533 insertions(+), 357 deletions(-) rename pkg/services/provisioning/alert_notifications/test-configs/{incorrect-properties/incorrect-properties.yaml => incorrect-settings/incorrect-settings.yaml} (91%) create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml diff --git a/conf/provisioning/alert_notifications/sample.yaml b/conf/provisioning/alert_notifications/sample.yaml index 1e609f07a0f..d08df2545e9 100644 --- a/conf/provisioning/alert_notifications/sample.yaml +++ b/conf/provisioning/alert_notifications/sample.yaml @@ -2,10 +2,11 @@ apiVersion: 1 # alert_notifications: -# - name: default-slack +# - name: default-slack-temp # type: slack -# org_id: 1 +# org_name: Main Org. # is_default: true +# uid: notifier1 # settings: # recipient: "XXX" # token: "xoxb" @@ -14,10 +15,11 @@ apiVersion: 1 # - name: default-email # type: email # org_id: 1 +# uid: notifier2 # is_default: false # settings: -# addresses: example@example.com +# addresses: example11111@example.com # delete_alert_notifications: -# - name: default-slack -# org_id: 1 -# - name: default-email \ No newline at end of file +# - name: default-slack-temp +# org_name: Main Org. +# uid: notifier1 \ No newline at end of file diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index 065db762e52..9d3fd9385d4 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -253,8 +253,8 @@ By default, exporting a dashboard as JSON will use a sequential identifier to re "frequency": "24h", "noDataState": "ok", "notifications": [ - {"name": "notification-channel-1"}, - {"name": "notification-channel-2"}, + {"uid": "notifier1"}, + {"uid": "notifier2"}, ] } ... @@ -267,6 +267,7 @@ By default, exporting a dashboard as JSON will use a sequential identifier to re alert_notifications: - name: notification-channel-1 type: slack + uid: notifier1 # either org_id: 2 # or @@ -282,6 +283,7 @@ alert_notifications: delete_alert_notifications: - name: notification-channel-1 + uid: notifier1 # either org_id: 2 # or diff --git a/pkg/models/alert_notifications.go b/pkg/models/alert_notifications.go index e0fd12937ed..0a26276e787 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -8,10 +8,11 @@ import ( ) var ( - ErrNotificationFrequencyNotFound = errors.New("Notification frequency not specified") - ErrAlertNotificationStateNotFound = errors.New("alert notification state not found") - ErrAlertNotificationStateVersionConflict = errors.New("alert notification state update version conflict") - ErrAlertNotificationStateAlreadyExist = errors.New("alert notification state already exists.") + ErrNotificationFrequencyNotFound = errors.New("Notification frequency not specified") + ErrAlertNotificationStateNotFound = errors.New("alert notification state not found") + ErrAlertNotificationStateVersionConflict = errors.New("alert notification state update version conflict") + ErrAlertNotificationStateAlreadyExist = errors.New("alert notification state already exists.") + ErrAlertNotificationFailedGenerateUniqueUid = errors.New("Failed to generate unique alert notification uid") ) type AlertNotificationStateType string @@ -24,6 +25,7 @@ var ( type AlertNotification struct { Id int64 `json:"id"` + Uid string `json:"-"` OrgId int64 `json:"-"` Name string `json:"name"` Type string `json:"type"` @@ -37,6 +39,7 @@ type AlertNotification struct { } type CreateAlertNotificationCommand struct { + Uid string `json:"-"` Name string `json:"name" binding:"Required"` Type string `json:"type" binding:"Required"` SendReminder bool `json:"sendReminder"` @@ -63,10 +66,28 @@ type UpdateAlertNotificationCommand struct { Result *AlertNotification } +type UpdateAlertNotificationWithUidCommand struct { + Uid string + Name string + Type string + SendReminder bool + DisableResolveMessage bool + Frequency string + IsDefault bool + Settings *simplejson.Json + + OrgId int64 + Result *AlertNotification +} + type DeleteAlertNotificationCommand struct { Id int64 OrgId int64 } +type DeleteAlertNotificationWithUidCommand struct { + Uid string + OrgId int64 +} type GetAlertNotificationsQuery struct { Name string @@ -76,8 +97,15 @@ type GetAlertNotificationsQuery struct { Result *AlertNotification } -type GetAlertNotificationsToSendQuery struct { - Ids []int64 +type GetAlertNotificationsWithUidQuery struct { + Uid string + OrgId int64 + + Result *AlertNotification +} + +type GetAlertNotificationsWithUidToSendQuery struct { + Uids []string OrgId int64 Result []*AlertNotification diff --git a/pkg/services/alerting/interfaces.go b/pkg/services/alerting/interfaces.go index 040d0991861..bd7ca087769 100644 --- a/pkg/services/alerting/interfaces.go +++ b/pkg/services/alerting/interfaces.go @@ -24,7 +24,7 @@ type Notifier interface { // ShouldNotify checks this evaluation should send an alert notification ShouldNotify(ctx context.Context, evalContext *EvalContext, notificationState *models.AlertNotificationState) bool - GetNotifierId() int64 + GetNotifierUid() string GetIsDefault() bool GetSendReminder() bool GetDisableResolveMessage() bool diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 75c68615750..e1a550d48f4 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -60,13 +60,13 @@ func (n *notificationService) SendIfNeeded(context *EvalContext) error { func (n *notificationService) sendAndMarkAsComplete(evalContext *EvalContext, notifierState *notifierState) error { notifier := notifierState.notifier - n.log.Debug("Sending notification", "type", notifier.GetType(), "id", notifier.GetNotifierId(), "isDefault", notifier.GetIsDefault()) + n.log.Debug("Sending notification", "type", notifier.GetType(), "uid", notifier.GetNotifierUid(), "isDefault", notifier.GetIsDefault()) metrics.M_Alerting_Notification_Sent.WithLabelValues(notifier.GetType()).Inc() err := notifier.Notify(evalContext) if err != nil { - n.log.Error("failed to send notification", "id", notifier.GetNotifierId(), "error", err) + n.log.Error("failed to send notification", "uid", notifier.GetNotifierUid(), "error", err) } if evalContext.IsTestRun { @@ -110,7 +110,7 @@ func (n *notificationService) sendNotifications(evalContext *EvalContext, notifi for _, notifierState := range notifierStates { err := n.sendNotification(evalContext, notifierState) if err != nil { - n.log.Error("failed to send notification", "id", notifierState.notifier.GetNotifierId(), "error", err) + n.log.Error("failed to send notification", "uid", notifierState.notifier.GetNotifierUid(), "error", err) } } @@ -157,8 +157,8 @@ func (n *notificationService) uploadImage(context *EvalContext) (err error) { return nil } -func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds []int64, evalContext *EvalContext) (notifierStateSlice, error) { - query := &m.GetAlertNotificationsToSendQuery{OrgId: orgId, Ids: notificationIds} +func (n *notificationService) getNeededNotifiers(orgId int64, notificationUids []string, evalContext *EvalContext) (notifierStateSlice, error) { + query := &m.GetAlertNotificationsWithUidToSendQuery{OrgId: orgId, Uids: notificationUids} if err := bus.Dispatch(query); err != nil { return nil, err @@ -168,7 +168,7 @@ func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds [] for _, notification := range query.Result { not, err := InitNotifier(notification) if err != nil { - n.log.Error("Could not create notifier", "notifier", notification.Id, "error", err) + n.log.Error("Could not create notifier", "notifier", notification.Uid, "error", err) continue } diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index d4a9975bcba..9616c2ab7cf 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -16,7 +16,7 @@ const ( type NotifierBase struct { Name string Type string - Id int64 + Uid string IsDeault bool UploadImage bool SendReminder bool @@ -34,7 +34,7 @@ func NewNotifierBase(model *models.AlertNotification) NotifierBase { } return NotifierBase{ - Id: model.Id, + Uid: model.Uid, Name: model.Name, IsDeault: model.IsDefault, Type: model.Type, @@ -110,8 +110,8 @@ func (n *NotifierBase) NeedsImage() bool { return n.UploadImage } -func (n *NotifierBase) GetNotifierId() int64 { - return n.Id +func (n *NotifierBase) GetNotifierUid() string { + return n.Uid } func (n *NotifierBase) GetIsDefault() bool { diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index 3fd4447eefe..bf09ecab0cd 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -173,7 +173,7 @@ func TestBaseNotifier(t *testing.T) { bJson := simplejson.New() model := &m.AlertNotification{ - Id: 1, + Uid: "1", Name: "name", Type: "email", Settings: bJson, diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index 180e013a856..75901ac5e2e 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -7,8 +7,6 @@ import ( "strconv" "time" - "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" ) @@ -32,7 +30,7 @@ type Rule struct { ExecutionErrorState m.ExecutionErrorOption State m.AlertStateType Conditions []Condition - Notifications []int64 + Notifications []string StateChanges int64 } @@ -128,22 +126,11 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) { for _, v := range ruleDef.Settings.Get("notifications").MustArray() { jsonModel := simplejson.NewFromAny(v) - id, err := jsonModel.Get("id").Int64() + uid, err := jsonModel.Get("uid").String() if err != nil { - notificationName, notificationNameErr := jsonModel.Get("name").String() - if notificationNameErr != nil { - return nil, ValidationError{Reason: "Invalid notification schema", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} - } - cmd := &m.GetAlertNotificationsQuery{OrgId: ruleDef.OrgId, Name: notificationName} - nameErr := bus.Dispatch(cmd) - if nameErr != nil || cmd.Result == nil { - errorMsg := fmt.Sprintf("Cannot lookup notification by name %s and orgId %d", notificationName, ruleDef.OrgId) - return nil, ValidationError{Reason: errorMsg, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} - } - model.Notifications = append(model.Notifications, cmd.Result.Id) - } else { - model.Notifications = append(model.Notifications, id) + return nil, ValidationError{Reason: "Invalid notification schema", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} } + model.Notifications = append(model.Notifications, uid) } diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index c81af138f6e..5ebae97caed 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -3,17 +3,11 @@ package alerting import ( "testing" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/models" m "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" ) -var ( - fakeRepo *fakeRepository -) - type FakeCondition struct{} func (f *FakeCondition) Eval(context *EvalContext) (*ConditionResult, error) { @@ -78,8 +72,8 @@ func TestAlertRuleModel(t *testing.T) { } ], "notifications": [ - {"id": 1134}, - {"id": 22} + {"uid": "1134"}, + {"uid": "22"} ] } ` @@ -135,155 +129,6 @@ func TestAlertRuleModel(t *testing.T) { So(err, ShouldBeNil) So(alertRule.Frequency, ShouldEqual, 60) }) - Convey("can construct alert rule model mixed notification ids and names", func() { - json := ` - { - "name": "name2", - "description": "desc2", - "handler": 0, - "noDataMode": "critical", - "enabled": true, - "frequency": "60s", - "conditions": [ - { - "type": "test", - "prop": 123 - } - ], - "notifications": [ - {"id": 1134}, - {"id": 22}, - {"name": "channel1"}, - {"name": "channel2"} - ] - } - ` - alertJSON, jsonErr := simplejson.NewJson([]byte(json)) - So(jsonErr, ShouldBeNil) - - alert := &m.Alert{ - Id: 1, - OrgId: 1, - DashboardId: 1, - PanelId: 1, - - Settings: alertJSON, - } - - fakeRepo = &fakeRepository{} - bus.ClearBusHandlers() - bus.AddHandler("test", mockGet) - - fakeRepo.loadAll = []*models.AlertNotification{ - {Name: "channel1", OrgId: 1, Id: 1}, - {Name: "channel2", OrgId: 1, Id: 2}, - } - - alertRule, err := NewRuleFromDBAlert(alert) - So(err, ShouldBeNil) - - Convey("Can read notifications", func() { - So(len(alertRule.Notifications), ShouldEqual, 4) - So(alertRule.Notifications, ShouldResemble, []int64{1134, 22, 1, 2}) - }) - }) - - Convey("raise error in case of left id", func() { - json := ` - { - "name": "name2", - "description": "desc2", - "handler": 0, - "noDataMode": "critical", - "enabled": true, - "frequency": "60s", - "conditions": [ - { - "type": "test", - "prop": 123 - } - ], - "notifications": [ - {"not_id": 1134} - ] - } - ` - - alertJSON, jsonErr := simplejson.NewJson([]byte(json)) - So(jsonErr, ShouldBeNil) - - alert := &m.Alert{ - Id: 1, - OrgId: 1, - DashboardId: 1, - PanelId: 1, - - Settings: alertJSON, - } - - _, err := NewRuleFromDBAlert(alert) - So(err, ShouldNotBeNil) - }) - - Convey("raise error in case of left id but existed name with alien orgId", func() { - json := ` - { - "name": "name2", - "description": "desc2", - "handler": 0, - "noDataMode": "critical", - "enabled": true, - "frequency": "60s", - "conditions": [ - { - "type": "test", - "prop": 123 - } - ], - "notifications": [ - {"not_id": 1134, "name": "channel1"} - ] - } - ` - - alertJSON, jsonErr := simplejson.NewJson([]byte(json)) - So(jsonErr, ShouldBeNil) - - alert := &m.Alert{ - Id: 1, - OrgId: 1, - DashboardId: 1, - PanelId: 1, - - Settings: alertJSON, - } - - fakeRepo = &fakeRepository{} - bus.ClearBusHandlers() - bus.AddHandler("test", mockGet) - - fakeRepo.loadAll = []*models.AlertNotification{ - {Name: "channel1", OrgId: 2, Id: 1}, - } - - _, err := NewRuleFromDBAlert(alert) - - So(err, ShouldNotBeNil) - }) }) } - -type fakeRepository struct { - loadAll []*models.AlertNotification -} - -func mockGet(cmd *models.GetAlertNotificationsQuery) error { - for _, v := range fakeRepo.loadAll { - if cmd.Name == v.Name && cmd.OrgId == v.OrgId { - cmd.Result = v - return nil - } - } - return nil -} diff --git a/pkg/services/alerting/testdata/dash-without-id.json b/pkg/services/alerting/testdata/dash-without-id.json index e0a212695d8..59fbd679cf2 100644 --- a/pkg/services/alerting/testdata/dash-without-id.json +++ b/pkg/services/alerting/testdata/dash-without-id.json @@ -44,7 +44,7 @@ "noDataState": "no_data", "notifications": [ { - "id": 6 + "uid": "notifier1" } ] }, diff --git a/pkg/services/alerting/testdata/influxdb-alert.json b/pkg/services/alerting/testdata/influxdb-alert.json index fd6feb31a47..2dcec447b15 100644 --- a/pkg/services/alerting/testdata/influxdb-alert.json +++ b/pkg/services/alerting/testdata/influxdb-alert.json @@ -45,7 +45,7 @@ "noDataState": "no_data", "notifications": [ { - "id": 6 + "uid": "notifier1" } ] }, diff --git a/pkg/services/provisioning/alert_notifications/alert_notifications.go b/pkg/services/provisioning/alert_notifications/alert_notifications.go index 53b28d93c52..345446aa98b 100644 --- a/pkg/services/provisioning/alert_notifications/alert_notifications.go +++ b/pkg/services/provisioning/alert_notifications/alert_notifications.go @@ -44,7 +44,7 @@ func (dc *NotificationProvisioner) apply(cfg *notificationsAsConfig) error { func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*deleteNotificationConfig) error { for _, notification := range notificationToDelete { - dc.log.Info("Deleting alert notification", "name", notification.Name) + dc.log.Info("Deleting alert notification", "name", notification.Name, "uid", notification.Uid) if notification.OrgId == 0 && notification.OrgName != "" { getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} @@ -55,14 +55,14 @@ func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*d } else if notification.OrgId < 0 { notification.OrgId = 1 } - getNotification := &models.GetAlertNotificationsQuery{Name: notification.Name, OrgId: notification.OrgId} + getNotification := &models.GetAlertNotificationsWithUidQuery{Uid: notification.Uid, OrgId: notification.OrgId} if err := bus.Dispatch(getNotification); err != nil { return err } if getNotification.Result != nil { - cmd := &models.DeleteAlertNotificationCommand{Id: getNotification.Result.Id, OrgId: getNotification.OrgId} + cmd := &models.DeleteAlertNotificationWithUidCommand{Uid: getNotification.Result.Uid, OrgId: getNotification.OrgId} if err := bus.Dispatch(cmd); err != nil { return err } @@ -85,33 +85,40 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not notification.OrgId = 1 } - cmd := &models.GetAlertNotificationsQuery{OrgId: notification.OrgId, Name: notification.Name} + cmd := &models.GetAlertNotificationsWithUidQuery{OrgId: notification.OrgId, Uid: notification.Uid} err := bus.Dispatch(cmd) if err != nil { return err } if cmd.Result == nil { - dc.log.Info("Inserting alert notification from configuration ", "name", notification.Name) + dc.log.Info("Inserting alert notification from configuration ", "name", notification.Name, "uid", notification.Uid) insertCmd := &models.CreateAlertNotificationCommand{ - Name: notification.Name, - Type: notification.Type, - IsDefault: notification.IsDefault, - Settings: notification.SettingsToJson(), - OrgId: notification.OrgId, + Uid: notification.Uid, + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Settings: notification.SettingsToJson(), + OrgId: notification.OrgId, + DisableResolveMessage: notification.DisableResolveMessage, + Frequency: notification.Frequency, + SendReminder: notification.SendReminder, } if err := bus.Dispatch(insertCmd); err != nil { return err } } else { dc.log.Info("Updating alert notification from configuration", "name", notification.Name) - updateCmd := &models.UpdateAlertNotificationCommand{ - Id: cmd.Result.Id, - Name: notification.Name, - Type: notification.Type, - IsDefault: notification.IsDefault, - Settings: notification.SettingsToJson(), - OrgId: notification.OrgId, + updateCmd := &models.UpdateAlertNotificationWithUidCommand{ + Uid: notification.Uid, + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Settings: notification.SettingsToJson(), + OrgId: notification.OrgId, + DisableResolveMessage: notification.DisableResolveMessage, + Frequency: notification.Frequency, + SendReminder: notification.SendReminder, } if err := bus.Dispatch(updateCmd); err != nil { return err @@ -130,17 +137,22 @@ func (cfg *notificationsAsConfig) mapToNotificationFromConfig() *notificationsAs for _, notification := range cfg.Notifications { r.Notifications = append(r.Notifications, ¬ificationFromConfig{ - OrgId: notification.OrgId, - OrgName: notification.OrgName, - Name: notification.Name, - Type: notification.Type, - IsDefault: notification.IsDefault, - Settings: notification.Settings, + Uid: notification.Uid, + OrgId: notification.OrgId, + OrgName: notification.OrgName, + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Settings: notification.Settings, + DisableResolveMessage: notification.DisableResolveMessage, + Frequency: notification.Frequency, + SendReminder: notification.SendReminder, }) } for _, notification := range cfg.DeleteNotifications { r.DeleteNotifications = append(r.DeleteNotifications, &deleteNotificationConfig{ + Uid: notification.Uid, OrgId: notification.OrgId, OrgName: notification.OrgName, Name: notification.Name, diff --git a/pkg/services/provisioning/alert_notifications/config_reader.go b/pkg/services/provisioning/alert_notifications/config_reader.go index ee99ae2d6e8..bd97947de36 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader.go +++ b/pkg/services/provisioning/alert_notifications/config_reader.go @@ -1,6 +1,7 @@ package alert_notifications import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -41,7 +42,11 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error } cr.log.Debug("Validating alert notifications") - validateOrgIdAndSet(notifications) + if err = validateRequiredField(notifications); err != nil { + return nil, err + } + + checkOrgIdAndOrgName(notifications) err = validateNotifications(notifications) if err != nil { @@ -67,7 +72,7 @@ func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) ( return cfg.mapToNotificationFromConfig(), nil } -func validateOrgIdAndSet(notifications []*notificationsAsConfig) { +func checkOrgIdAndOrgName(notifications []*notificationsAsConfig) { for i := range notifications { for _, notification := range notifications[i].Notifications { if notification.OrgId < 1 { @@ -91,6 +96,44 @@ func validateOrgIdAndSet(notifications []*notificationsAsConfig) { } } +func validateRequiredField(notifications []*notificationsAsConfig) error { + for i := range notifications { + var errStrings []string + for index, notification := range notifications[i].Notifications { + if notification.Name == "" { + errStrings = append( + errStrings, + fmt.Sprintf("Added alert notification item %d in configuration doesn't contain required field name", index+1), + ) + } + if notification.Uid == "" { + errStrings = append( + errStrings, + fmt.Sprintf("Added alert notification item %d in configuration doesn't contain required field uid", index+1), + ) + } + } + + for index, notification := range notifications[i].DeleteNotifications { + if notification.Name == "" { + errStrings = append( + errStrings, + fmt.Sprintf("Deleted alert notification item %d in configuration doesn't contain required field name", index+1), + ) + } + if notification.Uid == "" { + errStrings = append( + errStrings, + fmt.Sprintf("Deleted alert notification item %d in configuration doesn't contain required field uid", index+1), + ) + } + } + if len(errStrings) != 0 { + return fmt.Errorf(strings.Join(errStrings, "\n")) + } + } + return nil +} func validateNotifications(notifications []*notificationsAsConfig) error { notifierTypes := alerting.GetNotifiers() diff --git a/pkg/services/provisioning/alert_notifications/config_reader_test.go b/pkg/services/provisioning/alert_notifications/config_reader_test.go index 274335e82bb..0ef3d5abcb9 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader_test.go +++ b/pkg/services/provisioning/alert_notifications/config_reader_test.go @@ -3,11 +3,11 @@ package alert_notifications import ( "testing" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" - "github.com/grafana/grafana/pkg/models" + m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/alerting/notifiers" + "github.com/grafana/grafana/pkg/services/sqlstore" . "github.com/smartystreets/goconvey/convey" ) @@ -15,7 +15,8 @@ var ( logger = log.New("fake.log") correct_properties = "./test-configs/correct-properties" - incorrect_properties = "./test-configs/incorrect-properties" + incorrect_settings = "./test-configs/incorrect-settings" + no_required_fields = "./test-configs/no-required-fields" correct_properties_with_orgName = "./test-configs/correct-properties-with-orgName" brokenYaml = "./test-configs/broken-yaml" doubleNotificationsConfig = "./test-configs/double-default" @@ -23,19 +24,11 @@ var ( emptyFile = "./test-configs/empty" twoNotificationsConfig = "./test-configs/two-notifications" unknownNotifier = "./test-configs/unknown-notifier" - - fakeRepo *fakeRepository ) func TestNotificationAsConfig(t *testing.T) { Convey("Testing notification as configuration", t, func() { - fakeRepo = &fakeRepository{} - bus.ClearBusHandlers() - bus.AddHandler("test", mockDelete) - bus.AddHandler("test", mockInsert) - bus.AddHandler("test", mockUpdate) - bus.AddHandler("test", mockGet) - bus.AddHandler("test", mockGetOrg) + sqlstore.InitTestDB(t) alerting.RegisterNotifier(&alerting.NotifierPlugin{ Type: "slack", @@ -63,6 +56,7 @@ func TestNotificationAsConfig(t *testing.T) { So(nt.Name, ShouldEqual, "default-slack-notification") So(nt.Type, ShouldEqual, "slack") So(nt.OrgId, ShouldEqual, 2) + So(nt.Uid, ShouldEqual, "notifier1") So(nt.IsDefault, ShouldBeTrue) So(nt.Settings, ShouldResemble, map[string]interface{}{ "recipient": "XXX", "token": "xoxb", "uploadImage": true, "url": "https://slack.com", @@ -72,17 +66,20 @@ func TestNotificationAsConfig(t *testing.T) { So(nt.Name, ShouldEqual, "another-not-default-notification") So(nt.Type, ShouldEqual, "email") So(nt.OrgId, ShouldEqual, 3) + So(nt.Uid, ShouldEqual, "notifier2") So(nt.IsDefault, ShouldBeFalse) nt = nts[2] So(nt.Name, ShouldEqual, "check-unset-is_default-is-false") So(nt.Type, ShouldEqual, "slack") So(nt.OrgId, ShouldEqual, 3) + So(nt.Uid, ShouldEqual, "notifier3") So(nt.IsDefault, ShouldBeFalse) nt = nts[3] So(nt.Name, ShouldEqual, "Added notification with whitespaces in name") So(nt.Type, ShouldEqual, "email") + So(nt.Uid, ShouldEqual, "notifier4") So(nt.OrgId, ShouldEqual, 3) deleteNts := ntCfg.DeleteNotifications @@ -90,19 +87,23 @@ func TestNotificationAsConfig(t *testing.T) { deleteNt := deleteNts[0] So(deleteNt.Name, ShouldEqual, "default-slack-notification") + So(deleteNt.Uid, ShouldEqual, "notifier1") So(deleteNt.OrgId, ShouldEqual, 2) deleteNt = deleteNts[1] So(deleteNt.Name, ShouldEqual, "deleted-notification-without-orgId") So(deleteNt.OrgId, ShouldEqual, 1) + So(deleteNt.Uid, ShouldEqual, "notifier2") deleteNt = deleteNts[2] So(deleteNt.Name, ShouldEqual, "deleted-notification-with-0-orgId") So(deleteNt.OrgId, ShouldEqual, 1) + So(deleteNt.Uid, ShouldEqual, "notifier3") deleteNt = deleteNts[3] So(deleteNt.Name, ShouldEqual, "Deleted notification with whitespaces in name") So(deleteNt.OrgId, ShouldEqual, 1) + So(deleteNt.Uid, ShouldEqual, "notifier4") }) Convey("One configured notification", func() { @@ -112,23 +113,50 @@ func TestNotificationAsConfig(t *testing.T) { if err != nil { t.Fatalf("applyChanges return an error %v", err) } - So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 2) - So(len(fakeRepo.updated), ShouldEqual, 0) + notificationsQuery := m.GetAllAlertNotificationsQuery{OrgId: 1} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 2) }) - Convey("One notification in database with same name", func() { - fakeRepo.loadAll = []*models.AlertNotification{ - {Name: "channel1", OrgId: 1, Id: 1}, + + Convey("One notification in database with same name and uid", func() { + existingNotificationCmd := m.CreateAlertNotificationCommand{ + Name: "channel1", + OrgId: 1, + Uid: "notifier1", + Type: "slack", } + err := sqlstore.CreateAlertNotificationCommand(&existingNotificationCmd) + So(err, ShouldBeNil) + So(existingNotificationCmd.Result, ShouldNotBeNil) + notificationsQuery := m.GetAllAlertNotificationsQuery{OrgId: 1} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 1) + Convey("should update one notification", func() { dc := newNotificationProvisioner(logger) - err := dc.applyChanges(twoNotificationsConfig) + err = dc.applyChanges(twoNotificationsConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 1) - So(len(fakeRepo.updated), ShouldEqual, 1) + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 2) + + nts := notificationsQuery.Result + nt1 := nts[0] + So(nt1.Type, ShouldEqual, "email") + So(nt1.Name, ShouldEqual, "channel1") + So(nt1.Uid, ShouldEqual, "notifier1") + + nt2 := nts[1] + So(nt2.Type, ShouldEqual, "slack") + So(nt2.Name, ShouldEqual, "channel2") + So(nt2.Uid, ShouldEqual, "notifier2") }) }) Convey("Two notifications with is_default", func() { @@ -136,61 +164,106 @@ func TestNotificationAsConfig(t *testing.T) { err := dc.applyChanges(doubleNotificationsConfig) Convey("should both be inserted", func() { So(err, ShouldBeNil) - So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 2) - So(len(fakeRepo.updated), ShouldEqual, 0) + notificationsQuery := m.GetAllAlertNotificationsQuery{OrgId: 1} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 2) + + So(notificationsQuery.Result[0].IsDefault, ShouldBeTrue) + So(notificationsQuery.Result[1].IsDefault, ShouldBeTrue) }) }) }) Convey("Two configured notification", func() { Convey("two other notifications in database", func() { - fakeRepo.loadAll = []*models.AlertNotification{ - {Name: "channel1", OrgId: 1, Id: 1}, - {Name: "channel3", OrgId: 1, Id: 2}, + existingNotificationCmd := m.CreateAlertNotificationCommand{ + Name: "channel0", + OrgId: 1, + Uid: "notifier0", + Type: "slack", } + err := sqlstore.CreateAlertNotificationCommand(&existingNotificationCmd) + So(err, ShouldBeNil) + existingNotificationCmd = m.CreateAlertNotificationCommand{ + Name: "channel3", + OrgId: 1, + Uid: "notifier3", + Type: "slack", + } + err = sqlstore.CreateAlertNotificationCommand(&existingNotificationCmd) + So(err, ShouldBeNil) + + notificationsQuery := m.GetAllAlertNotificationsQuery{OrgId: 1} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 2) + Convey("should have two new notifications", func() { dc := newNotificationProvisioner(logger) err := dc.applyChanges(twoNotificationsConfig) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 1) - So(len(fakeRepo.updated), ShouldEqual, 1) + notificationsQuery = m.GetAllAlertNotificationsQuery{OrgId: 1} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 4) }) }) }) Convey("Can read correct properties with orgName instead of orgId", func() { - fakeRepo.loadAllOrg = []*models.Org{ - {Name: "Main Org. 1", Id: 1}, - {Name: "Main Org. 2", Id: 2}, - } + existingOrg1 := m.CreateOrgCommand{Name: "Main Org. 1"} + err := sqlstore.CreateOrg(&existingOrg1) + So(err, ShouldBeNil) + So(existingOrg1.Result, ShouldNotBeNil) + existingOrg2 := m.CreateOrgCommand{Name: "Main Org. 2"} + err = sqlstore.CreateOrg(&existingOrg2) + So(err, ShouldBeNil) + So(existingOrg2.Result, ShouldNotBeNil) - fakeRepo.loadAll = []*models.AlertNotification{ - {Name: "default-slack-notification", OrgId: 1, Id: 1}, - {Name: "another-not-default-notification", OrgId: 2, Id: 2}, + existingNotificationCmd := m.CreateAlertNotificationCommand{ + Name: "default-notification-delete", + OrgId: existingOrg2.Result.Id, + Uid: "notifier2", + Type: "slack", } + err = sqlstore.CreateAlertNotificationCommand(&existingNotificationCmd) + So(err, ShouldBeNil) + dc := newNotificationProvisioner(logger) - err := dc.applyChanges(correct_properties_with_orgName) + err = dc.applyChanges(correct_properties_with_orgName) if err != nil { t.Fatalf("applyChanges return an error %v", err) } - So(len(fakeRepo.deleted), ShouldEqual, 2) - So(len(fakeRepo.inserted), ShouldEqual, 0) - So(len(fakeRepo.updated), ShouldEqual, 2) - updated := fakeRepo.updated - nt := updated[0] - So(nt.Name, ShouldEqual, "default-slack-notification") - So(nt.OrgId, ShouldEqual, 1) - nt = updated[1] - So(nt.Name, ShouldEqual, "another-not-default-notification") - So(nt.OrgId, ShouldEqual, 2) + notificationsQuery := m.GetAllAlertNotificationsQuery{OrgId: existingOrg2.Result.Id} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldNotBeNil) + So(len(notificationsQuery.Result), ShouldEqual, 1) + + nt := notificationsQuery.Result[0] + So(nt.Name, ShouldEqual, "default-notification-create") + So(nt.OrgId, ShouldEqual, existingOrg2.Result.Id) }) + Convey("Config doesn't contain required field", func() { + dc := newNotificationProvisioner(logger) + err := dc.applyChanges(no_required_fields) + So(err, ShouldNotBeNil) + + errString := err.Error() + So(errString, ShouldContainSubstring, "Deleted alert notification item 1 in configuration doesn't contain required field uid") + So(errString, ShouldContainSubstring, "Deleted alert notification item 2 in configuration doesn't contain required field name") + So(errString, ShouldContainSubstring, "Added alert notification item 1 in configuration doesn't contain required field name") + So(errString, ShouldContainSubstring, "Added alert notification item 2 in configuration doesn't contain required field uid") + }) Convey("Empty yaml file", func() { Convey("should have not changed repo", func() { dc := newNotificationProvisioner(logger) @@ -198,9 +271,10 @@ func TestNotificationAsConfig(t *testing.T) { if err != nil { t.Fatalf("applyChanges return an error %v", err) } - So(len(fakeRepo.deleted), ShouldEqual, 0) - So(len(fakeRepo.inserted), ShouldEqual, 0) - So(len(fakeRepo.updated), ShouldEqual, 0) + notificationsQuery := m.GetAllAlertNotificationsQuery{OrgId: 1} + err = sqlstore.GetAllAlertNotifications(¬ificationsQuery) + So(err, ShouldBeNil) + So(notificationsQuery.Result, ShouldBeEmpty) }) }) Convey("Broken yaml should return error", func() { @@ -224,49 +298,10 @@ func TestNotificationAsConfig(t *testing.T) { Convey("Read incorrect properties", func() { cfgProvifer := &configReader{log: log.New("test logger")} - _, err := cfgProvifer.readConfig(incorrect_properties) + _, err := cfgProvifer.readConfig(incorrect_settings) So(err, ShouldNotBeNil) So(err.Error(), ShouldEqual, "Alert validation error: Could not find url property in settings") }) }) } - -type fakeRepository struct { - inserted []*models.CreateAlertNotificationCommand - deleted []*models.DeleteAlertNotificationCommand - updated []*models.UpdateAlertNotificationCommand - loadAll []*models.AlertNotification - loadAllOrg []*models.Org -} - -func mockDelete(cmd *models.DeleteAlertNotificationCommand) error { - fakeRepo.deleted = append(fakeRepo.deleted, cmd) - return nil -} -func mockUpdate(cmd *models.UpdateAlertNotificationCommand) error { - fakeRepo.updated = append(fakeRepo.updated, cmd) - return nil -} -func mockInsert(cmd *models.CreateAlertNotificationCommand) error { - fakeRepo.inserted = append(fakeRepo.inserted, cmd) - return nil -} -func mockGet(cmd *models.GetAlertNotificationsQuery) error { - for _, v := range fakeRepo.loadAll { - if cmd.Name == v.Name && cmd.OrgId == v.OrgId { - cmd.Result = v - return nil - } - } - return nil -} -func mockGetOrg(cmd *models.GetOrgByNameQuery) error { - for _, v := range fakeRepo.loadAllOrg { - if cmd.Name == v.Name { - cmd.Result = v - return nil - } - } - return nil -} diff --git a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml b/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml index 45631c38b9f..214396982ea 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml +++ b/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml @@ -1,21 +1,12 @@ alert_notifications: - - name: default-slack-notification - type: slack - org_name: Main Org. 1 - is_default: true - settings: - recipient: "XXX" - token: "xoxb" - uploadImage: true - url: https://slack.com - - name: another-not-default-notification + - name: default-notification-create type: email + uid: notifier2 settings: addresses: example@example.com org_name: Main Org. 2 is_default: false delete_alert_notifications: - - name: default-slack-notification - org_name: Main Org. 1 - - name: another-not-default-notification - org_name: Main Org. 2 \ No newline at end of file + - name: default-notification-delete + org_name: Main Org. 2 + uid: notifier2 \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml b/pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml index c4b7d7cb224..4f4cd171852 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml +++ b/pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml @@ -1,7 +1,9 @@ alert_notifications: - name: default-slack-notification type: slack + uid: notifier1 org_id: 2 + uid: "notifier1" is_default: true settings: recipient: "XXX" @@ -13,21 +15,28 @@ alert_notifications: settings: addresses: example@exmaple.com org_id: 3 + uid: "notifier2" is_default: false - name: check-unset-is_default-is-false type: slack org_id: 3 + uid: "notifier3" settings: url: https://slack.com - name: Added notification with whitespaces in name type: email org_id: 3 + uid: "notifier4" settings: addresses: example@exmaple.com delete_alert_notifications: - name: default-slack-notification org_id: 2 + uid: notifier1 - name: deleted-notification-without-orgId + uid: "notifier2" - name: deleted-notification-with-0-orgId org_id: 0 - - name: Deleted notification with whitespaces in name \ No newline at end of file + uid: "notifier3" + - name: Deleted notification with whitespaces in name + uid: "notifier4" \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml b/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml index ca6c347433f..d3ae32c5ae7 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml +++ b/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml @@ -1,6 +1,7 @@ alert_notifications: - name: first-default type: slack + uid: notifier1 is_default: true settings: url: https://slack.com \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml b/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml index 8b07e4ca9c4..3c2e8953f6b 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml +++ b/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml @@ -1,6 +1,7 @@ alert_notifications: - name: second-default type: email + uid: notifier2 is_default: true settings: addresses: example@example.com \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/incorrect-properties/incorrect-properties.yaml b/pkg/services/provisioning/alert_notifications/test-configs/incorrect-settings/incorrect-settings.yaml similarity index 91% rename from pkg/services/provisioning/alert_notifications/test-configs/incorrect-properties/incorrect-properties.yaml rename to pkg/services/provisioning/alert_notifications/test-configs/incorrect-settings/incorrect-settings.yaml index f01234564e0..2d720d9d2d9 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/incorrect-properties/incorrect-properties.yaml +++ b/pkg/services/provisioning/alert_notifications/test-configs/incorrect-settings/incorrect-settings.yaml @@ -2,6 +2,7 @@ alert_notifications: - name: slack-notification-without-url-in-settings type: slack org_id: 2 + uid: notifier1 is_default: true settings: recipient: "XXX" diff --git a/pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml b/pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml new file mode 100644 index 00000000000..582abefe14d --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml @@ -0,0 +1,35 @@ +alert_notifications: + - type: slack + org_id: 2 + uid: no-name_added-notification + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + - name: no-uid + type: slack + org_id: 2 + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true +delete_alert_notifications: + - name: no-uid + type: slack + org_id: 2 + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + - type: slack + org_id: 2 + uid: no-name_added-notification + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml b/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml index dd93c815b04..23fff0aff23 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml +++ b/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml @@ -1,9 +1,12 @@ -alert_notifications: +alert_notifications: - name: channel1 + type: email + uid: notifier1 + org_id: 1 + settings: + addresses: example@example.com + - name: channel2 type: slack + uid: notifier2 settings: url: http://slack.com - - name: channel2 - type: email - settings: - addresses: example@example.com \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml b/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml index a59c38ea84b..e46db7b8b6e 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml +++ b/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml @@ -1,3 +1,4 @@ alert_notifications: - name: unknown-notifier - type: nonexisting \ No newline at end of file + type: nonexisting + uid: notifier1 \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/types.go b/pkg/services/provisioning/alert_notifications/types.go index 7ac2e6900e5..d3a858ae956 100644 --- a/pkg/services/provisioning/alert_notifications/types.go +++ b/pkg/services/provisioning/alert_notifications/types.go @@ -8,18 +8,23 @@ type notificationsAsConfig struct { } type deleteNotificationConfig struct { + Uid string `json:"uid" yaml:"uid"` Name string `json:"name" yaml:"name"` OrgId int64 `json:"org_id" yaml:"org_id"` OrgName string `json:"org_name" yaml:"org_name"` } type notificationFromConfig struct { - OrgId int64 `json:"org_id" yaml:"org_id"` - OrgName string `json:"org_name" yaml:"org_name"` - Name string `json:"name" yaml:"name"` - Type string `json:"type" yaml:"type"` - IsDefault bool `json:"is_default" yaml:"is_default"` - Settings map[string]interface{} `json:"settings" yaml:"settings"` + Uid string `json:"uid" yaml:"uid"` + OrgId int64 `json:"org_id" yaml:"org_id"` + OrgName string `json:"org_name" yaml:"org_name"` + Name string `json:"name" yaml:"name"` + Type string `json:"type" yaml:"type"` + SendReminder bool `json:"send_reminder" yaml:"send_reminder"` + DisableResolveMessage bool `json:"disable_resolve_message" yaml:"disable_resolve_message"` + Frequency string `json:"frequency" yaml:"frequency"` + IsDefault bool `json:"is_default" yaml:"is_default"` + Settings map[string]interface{} `json:"settings" yaml:"settings"` } func (notification notificationFromConfig) SettingsToJson() *simplejson.Json { diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index afe6269510f..392f985ae61 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/util" ) func init() { @@ -17,11 +18,15 @@ func init() { bus.AddHandler("sql", CreateAlertNotificationCommand) bus.AddHandler("sql", UpdateAlertNotification) bus.AddHandler("sql", DeleteAlertNotification) - bus.AddHandler("sql", GetAlertNotificationsToSend) bus.AddHandler("sql", GetAllAlertNotifications) bus.AddHandlerCtx("sql", GetOrCreateAlertNotificationState) bus.AddHandlerCtx("sql", SetAlertNotificationStateToCompleteCommand) bus.AddHandlerCtx("sql", SetAlertNotificationStateToPendingCommand) + + bus.AddHandler("sql", GetAlertNotificationsWithUid) + bus.AddHandler("sql", UpdateAlertNotificationWithUid) + bus.AddHandler("sql", DeleteAlertNotificationWithUid) + bus.AddHandler("sql", GetAlertNotificationsWithUidToSend) } func DeleteAlertNotification(cmd *m.DeleteAlertNotificationCommand) error { @@ -39,10 +44,33 @@ func DeleteAlertNotification(cmd *m.DeleteAlertNotificationCommand) error { }) } +func DeleteAlertNotificationWithUid(cmd *m.DeleteAlertNotificationWithUidCommand) error { + existingNotification := &m.GetAlertNotificationsWithUidQuery{OrgId: cmd.OrgId, Uid: cmd.Uid} + if getNotificationErr := getAlertNotificationWithUidInternal(existingNotification, newSession()); getNotificationErr != nil { + return getNotificationErr + } + + if existingNotification.Result != nil { + deleteCommand := &m.DeleteAlertNotificationCommand{ + Id: existingNotification.Result.Id, + OrgId: existingNotification.Result.OrgId, + } + if deleteErr := bus.Dispatch(deleteCommand); deleteErr != nil { + return deleteErr + } + } + + return nil +} + func GetAlertNotifications(query *m.GetAlertNotificationsQuery) error { return getAlertNotificationInternal(query, newSession()) } +func GetAlertNotificationsWithUid(query *m.GetAlertNotificationsWithUidQuery) error { + return getAlertNotificationWithUidInternal(query, newSession()) +} + func GetAllAlertNotifications(query *m.GetAllAlertNotificationsQuery) error { results := make([]*m.AlertNotification, 0) if err := x.Where("org_id = ?", query.OrgId).Find(&results); err != nil { @@ -53,12 +81,13 @@ func GetAllAlertNotifications(query *m.GetAllAlertNotificationsQuery) error { return nil } -func GetAlertNotificationsToSend(query *m.GetAlertNotificationsToSendQuery) error { +func GetAlertNotificationsWithUidToSend(query *m.GetAlertNotificationsWithUidToSendQuery) error { var sql bytes.Buffer params := make([]interface{}, 0) - sql.WriteString(`SELECT + sql.WriteString(`SELECT alert_notification.id, + alert_notification.uid, alert_notification.org_id, alert_notification.name, alert_notification.type, @@ -77,9 +106,10 @@ func GetAlertNotificationsToSend(query *m.GetAlertNotificationsToSendQuery) erro sql.WriteString(` AND ((alert_notification.is_default = ?)`) params = append(params, dialect.BooleanStr(true)) - if len(query.Ids) > 0 { - sql.WriteString(` OR alert_notification.id IN (?` + strings.Repeat(",?", len(query.Ids)-1) + ")") - for _, v := range query.Ids { + + if len(query.Uids) > 0 { + sql.WriteString(` OR alert_notification.uid IN (?` + strings.Repeat(",?", len(query.Uids)-1) + ")") + for _, v := range query.Uids { params = append(params, v) } } @@ -142,16 +172,70 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS return nil } +func getAlertNotificationWithUidInternal(query *m.GetAlertNotificationsWithUidQuery, sess *DBSession) error { + var sql bytes.Buffer + params := make([]interface{}, 0) + + sql.WriteString(`SELECT + alert_notification.id, + alert_notification.uid, + alert_notification.org_id, + alert_notification.name, + alert_notification.type, + alert_notification.created, + alert_notification.updated, + alert_notification.settings, + alert_notification.is_default, + alert_notification.disable_resolve_message, + alert_notification.send_reminder, + alert_notification.frequency + FROM alert_notification + `) + + sql.WriteString(` WHERE alert_notification.org_id = ? AND alert_notification.uid = ?`) + params = append(params, query.OrgId, query.Uid) + + results := make([]*m.AlertNotification, 0) + if err := sess.SQL(sql.String(), params...).Find(&results); err != nil { + return err + } + + if len(results) == 0 { + query.Result = nil + } else { + query.Result = results[0] + } + + return nil +} + func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error { return inTransaction(func(sess *DBSession) error { - existingQuery := &m.GetAlertNotificationsQuery{OrgId: cmd.OrgId, Name: cmd.Name} - err := getAlertNotificationInternal(existingQuery, sess) + if cmd.Uid == "" { + if uid, uidGenerationErr := generateNewAlertNotificationUid(sess, cmd.OrgId); uidGenerationErr != nil { + return uidGenerationErr + } else { + cmd.Uid = uid + } + } + existingQuery := &m.GetAlertNotificationsWithUidQuery{OrgId: cmd.OrgId, Uid: cmd.Uid} + err := getAlertNotificationWithUidInternal(existingQuery, sess) if err != nil { return err } if existingQuery.Result != nil { + return fmt.Errorf("Alert notification uid %s already exists", cmd.Uid) + } + + // check if name exists + sameNameQuery := &m.GetAlertNotificationsQuery{OrgId: cmd.OrgId, Name: cmd.Name} + if err := getAlertNotificationInternal(sameNameQuery, sess); err != nil { + return err + } + + if sameNameQuery.Result != nil { return fmt.Errorf("Alert notification name %s already exists", cmd.Name) } @@ -168,6 +252,7 @@ func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error } alertNotification := &m.AlertNotification{ + Uid: cmd.Uid, OrgId: cmd.OrgId, Name: cmd.Name, Type: cmd.Type, @@ -189,6 +274,20 @@ func CreateAlertNotificationCommand(cmd *m.CreateAlertNotificationCommand) error }) } +func generateNewAlertNotificationUid(sess *DBSession, orgId int64) (string, error) { + for i := 0; i < 3; i++ { + uid := util.GenerateShortUid() + exists, err := sess.Where("org_id=? AND uid=?", orgId, uid).Get(&m.AlertNotification{}) + if err != nil { + return "", err + } + if !exists { + return uid, nil + } + } + return "", m.ErrAlertNotificationFailedGenerateUniqueUid +} + func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { return inTransaction(func(sess *DBSession) (err error) { current := m.AlertNotification{} @@ -241,6 +340,41 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { }) } +func UpdateAlertNotificationWithUid(cmd *m.UpdateAlertNotificationWithUidCommand) error { + getAlertNotificationWithUidQuery := &m.GetAlertNotificationsWithUidQuery{OrgId: cmd.OrgId, Uid: cmd.Uid} + + getCurrentNotificationErr := getAlertNotificationWithUidInternal(getAlertNotificationWithUidQuery, newSession()) + + if getCurrentNotificationErr != nil { + return getCurrentNotificationErr + } + + current := getAlertNotificationWithUidQuery.Result + + if current == nil { + return fmt.Errorf("Cannot update, alert notification uid %s doesn't exist", cmd.Uid) + } + + updateNotification := &m.UpdateAlertNotificationCommand{ + Id: current.Id, + Name: cmd.Name, + Type: cmd.Type, + SendReminder: cmd.SendReminder, + DisableResolveMessage: cmd.DisableResolveMessage, + Frequency: cmd.Frequency, + IsDefault: cmd.IsDefault, + Settings: cmd.Settings, + + OrgId: cmd.OrgId, + } + + if updateErr := bus.Dispatch(updateNotification); updateErr != nil { + return updateErr + } + + return nil +} + func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetAlertNotificationStateToCompleteCommand) error { return inTransactionCtx(ctx, func(sess *DBSession) error { version := cmd.Version diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 629a6292eb5..91b84cb91d0 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -220,11 +220,38 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(cmd.Result.Type, ShouldEqual, "email") So(cmd.Result.Frequency, ShouldEqual, 10*time.Second) So(cmd.Result.DisableResolveMessage, ShouldBeFalse) + So(cmd.Result.Uid, ShouldNotBeEmpty) Convey("Cannot save Alert Notification with the same name", func() { err = CreateAlertNotificationCommand(cmd) So(err, ShouldNotBeNil) }) + Convey("Cannot save Alert Notification with the same name and another uid", func() { + anotherUidCmd := &models.CreateAlertNotificationCommand{ + Name: cmd.Name, + Type: cmd.Type, + OrgId: 1, + SendReminder: cmd.SendReminder, + Frequency: cmd.Frequency, + Settings: cmd.Settings, + Uid: "notifier1", + } + err = CreateAlertNotificationCommand(anotherUidCmd) + So(err, ShouldNotBeNil) + }) + Convey("Can save Alert Notification with another name and another uid", func() { + anotherUidCmd := &models.CreateAlertNotificationCommand{ + Name: "another ops", + Type: cmd.Type, + OrgId: 1, + SendReminder: cmd.SendReminder, + Frequency: cmd.Frequency, + Settings: cmd.Settings, + Uid: "notifier2", + } + err = CreateAlertNotificationCommand(anotherUidCmd) + So(err, ShouldBeNil) + }) Convey("Can update alert notification", func() { newCmd := &models.UpdateAlertNotificationCommand{ @@ -274,12 +301,12 @@ func TestAlertNotificationSQLAccess(t *testing.T) { So(CreateAlertNotificationCommand(&otherOrg), ShouldBeNil) Convey("search", func() { - query := &models.GetAlertNotificationsToSendQuery{ - Ids: []int64{cmd1.Result.Id, cmd2.Result.Id, 112341231}, + query := &models.GetAlertNotificationsWithUidToSendQuery{ + Uids: []string{cmd1.Result.Uid, cmd2.Result.Uid, "112341231"}, OrgId: 1, } - err := GetAlertNotificationsToSend(query) + err := GetAlertNotificationsWithUidToSend(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 3) }) diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index b5aeb26483c..2ed9732687a 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -137,4 +137,18 @@ func addAlertMigrations(mg *Migrator) { mg.AddMigration("Add for to alert table", NewAddColumnMigration(alertV1, &Column{ Name: "for", Type: DB_BigInt, Nullable: true, })) + + mg.AddMigration("Add column uid in alert_notification", NewAddColumnMigration(alert_notification, &Column{ + Name: "uid", Type: DB_NVarchar, Length: 40, Nullable: true, + })) + mg.AddMigration("Update uid column values in alert_notification", new(RawSqlMigration). + Sqlite("UPDATE alert_notification SET uid=printf('%09d',id) WHERE uid IS NULL;"). + Postgres("UPDATE alert_notification SET uid=lpad('' || id,9,'0') WHERE uid IS NULL;"). + Mysql("UPDATE alert_notification SET uid=lpad(id,9,'0') WHERE uid IS NULL;")) + mg.AddMigration("Add unique index alert_notification_org_id_uid", NewAddIndexMigration(alert_notification, &Index{ + Cols: []string{"org_id", "uid"}, Type: UniqueIndex, + })) + mg.AddMigration("Remove unique index org_id_name", NewDropIndexMigration(alert_notification, &Index{ + Cols: []string{"org_id", "name"}, Type: UniqueIndex, + })) }