From 6e3e9a337dbbcbf20e4990f0492364244ee58b40 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Wed, 28 Nov 2018 16:35:42 +0200 Subject: [PATCH 01/21] Added alert_notification configuration --- .../alert_notifications/sample.yaml | 20 ++ docs/sources/administration/provisioning.md | 178 ++++++++++++++ pkg/services/alerting/rule.go | 18 +- pkg/services/alerting/rule_test.go | 156 +++++++++++++ .../alert_notifications.go | 151 ++++++++++++ .../alert_notifications/config_reader.go | 115 +++++++++ .../alert_notifications/config_reader_test.go | 219 ++++++++++++++++++ .../test-configs/broken-yaml/broken.yaml | 9 + .../test-configs/broken-yaml/not.yaml.text | 6 + .../correct-properties.yaml | 26 +++ .../test-configs/double-default/default-1.yml | 4 + .../double-default/default-2.yaml | 4 + .../test-configs/empty/empty.yaml | 0 .../test-configs/empty_folder/.gitignore | 4 + .../two-notifications/two-notifications.yaml | 5 + .../unknown-notifier/notification.yaml | 3 + .../provisioning/alert_notifications/types.go | 19 ++ pkg/services/provisioning/provisioning.go | 6 + 18 files changed, 941 insertions(+), 2 deletions(-) create mode 100644 conf/provisioning/alert_notifications/sample.yaml create mode 100644 pkg/services/provisioning/alert_notifications/alert_notifications.go create mode 100644 pkg/services/provisioning/alert_notifications/config_reader.go create mode 100644 pkg/services/provisioning/alert_notifications/config_reader_test.go create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/not.yaml.text create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/empty/empty.yaml create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/empty_folder/.gitignore create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml create mode 100644 pkg/services/provisioning/alert_notifications/types.go diff --git a/conf/provisioning/alert_notifications/sample.yaml b/conf/provisioning/alert_notifications/sample.yaml new file mode 100644 index 00000000000..92d03a81720 --- /dev/null +++ b/conf/provisioning/alert_notifications/sample.yaml @@ -0,0 +1,20 @@ +# # config file version +apiVersion: 1 + +# alert_notifications: +# - name: default-slack +# type: slack +# org_id: 1 +# is_default: true +# settings: +# recipient: "XXX" +# token: "xoxb" +# uploadImage: true +# - name: default-email +# type: email +# org_id: 1 +# is_default: false +# delete_alert_notifications: +# - name: default-slack +# org_id: 1 +# - name: default-email \ No newline at end of file diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index b2a1b1f42e7..6b8a1a992da 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -230,4 +230,182 @@ By default Grafana will delete dashboards in the database if the file is removed > **Note.** Provisioning allows you to overwrite existing dashboards > which leads to problems if you re-use settings that are supposed to be unique. > Be careful not to re-use the same `title` multiple times within a folder +<<<<<<< HEAD > or `uid` within the same installation as this will cause weird behaviors. +======= +> or `uid` within the same installation as this will cause weird behaviours. + +## Alert Notification Channels + +Alert Notification Channels can be provisionned by adding one or more yaml config files in the [`provisioning/alert_notifications`](/installation/configuration/#provisioning) directory. + +Each config file can contain the following top-level fields: +- `alert_notifications`, a list of alert notifications that will be added or updated during start up. If the notification channel already exists, Grafana will update it to match the configuration file. +- `delete_alert_notifications`, a list of alert notifications to be deleted before before inserting/updating those in the `alert_notifications` list. + +Provisionning looks up alert notifications by name, and will update any existing notification with the provided name. + +By default, exporting a dashboard as JSON will use a sequential identifier to refer to alert notifications. The field `name` can be optionally specified to specify a string identifier for the alert name. + +```json +{ + ... + "alert": { + ..., + "conditions": [...], + "frequency": "24h", + "noDataState": "ok", + "notifications": [ + {"name": "notification-channel-1"}, + {"name": "notification-channel-2"}, + ] + } + ... +} +``` + +### Example Alert Notification Channels Config File + +```yaml +alert_notifications: + - name: notification-channel-1 + type: slack + org_id: 2 + is_default: true + # See `Supported Settings` section for settings supporter for each + # alert notification type. + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + +delete_alert_notifications: + - name: notification-channel-1 + org_id: 2 + - name: notification-channel-2 +``` + +### Supported Settings + +The following sections detail the supported settings for each alert notification type. + +#### Alert notification `pushover` + +| Name | +| ---- | +| apiToken | +| userKey | +| device | +| retry | +| expire | + +#### Alert notification `slack` + +| Name | +| ---- | +| url | +| recipient | +| username | +| iconEmoji | +| iconUrl | +| uploadImage | +| mention | +| token | + +#### Alert notification `victorops` + +| Name | +| ---- | +| url | + +#### Alert notification `kafka` + +| Name | +| ---- | +| kafkaRestProxy | +| kafkaTopic | + +#### Alert notification `LINE` + +| Name | +| ---- | +| token | + +#### Alert notification `pagerduty` + +| Name | +| ---- | +| integrationKey | + +#### Alert notification `sensu` + +| Name | +| ---- | +| url | +| source | +| handler | +| username | +| password | + +#### Alert notification `prometheus-alertmanager` + +| Name | +| ---- | +| url | + +#### Alert notification `teams` + +| Name | +| ---- | +| url | + +#### Alert notification `dingding` + +| Name | +| ---- | +| url | + +#### Alert notification `email` + +| Name | +| ---- | +| addresses | + +#### Alert notification `hipchat` + +| Name | +| ---- | +| url | +| apikey | +| roomid | + +#### Alert notification `opsgenie` + +| Name | +| ---- | +| apiKey | +| apiUrl | + +#### Alert notification `telegram` + +| Name | +| ---- | +| bottoken | +| chatid | + +#### Alert notification `threema` + +| Name | +| ---- | +| gateway_id | +| recipient_id | +| api_secret | + +#### Alert notification `webhook` + +| Name | +| ---- | +| url | +| username | +| password | +>>>>>>> Added alert_notification configuration diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index 4423046d600..180e013a856 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -7,6 +7,8 @@ import ( "strconv" "time" + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" ) @@ -128,9 +130,21 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) { jsonModel := simplejson.NewFromAny(v) id, err := jsonModel.Get("id").Int64() if err != nil { - return nil, ValidationError{Reason: "Invalid notification schema", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} + 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) } - model.Notifications = append(model.Notifications, id) + } for index, condition := range ruleDef.Settings.Get("conditions").MustArray() { diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index cf25cc118f4..c81af138f6e 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -3,11 +3,17 @@ 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) { @@ -129,5 +135,155 @@ 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/provisioning/alert_notifications/alert_notifications.go b/pkg/services/provisioning/alert_notifications/alert_notifications.go new file mode 100644 index 00000000000..eae5b4bb44e --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/alert_notifications.go @@ -0,0 +1,151 @@ +package alert_notifications + +import ( + "errors" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/models" +) + +var ( + ErrInvalidConfigTooManyDefault = errors.New("Alert notification provisioning config is invalid. Only one alert notification can be marked as default") + ErrInvalidNotifierType = errors.New("Unknown notifier type") +) + +func Provision(configDirectory string) error { + dc := newNotificationProvisioner(log.New("provisioning.alert_notifications")) + return dc.applyChanges(configDirectory) +} + +type NotificationProvisioner struct { + log log.Logger + cfgProvider *configReader +} + +func newNotificationProvisioner(log log.Logger) NotificationProvisioner { + return NotificationProvisioner{ + log: log, + cfgProvider: &configReader{log: log}, + } +} + +func (dc *NotificationProvisioner) apply(cfg *notificationsAsConfig) error { + if err := dc.deleteNotifications(cfg.DeleteNotifications); err != nil { + return err + } + + if err := dc.mergeNotifications(cfg.Notifications); err != nil { + return err + } + + return nil +} + +func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*deleteNotificationConfig) error { + for _, notification := range notificationToDelete { + dc.log.Info("Deleting alert notification", "name", notification.Name) + + getNotification := &models.GetAlertNotificationsQuery{Name: notification.Name, 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} + if err := bus.Dispatch(cmd); err != nil { + return err + } + } + } + + return nil +} + +func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*notificationFromConfig) error { + for _, notification := range notificationToMerge { + cmd := &models.GetAlertNotificationsQuery{OrgId: notification.OrgId, Name: notification.Name} + err := bus.Dispatch(cmd) + if err != nil { + return err + } + + settings := simplejson.New() + if len(notification.Settings) > 0 { + for k, v := range notification.Settings { + settings.Set(k, v) + } + } + + if cmd.Result == nil { + dc.log.Info("Inserting alert notification from configuration ", "name", notification.Name) + insertCmd := &models.CreateAlertNotificationCommand{ + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Settings: settings, + OrgId: notification.OrgId, + } + 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: settings, + OrgId: notification.OrgId, + } + if err := bus.Dispatch(updateCmd); err != nil { + return err + } + } + } + + return nil +} +func (cfg *notificationsAsConfig) mapToNotificationFromConfig() *notificationsAsConfig { + r := ¬ificationsAsConfig{} + if cfg == nil { + return r + } + + for _, notification := range cfg.Notifications { + r.Notifications = append(r.Notifications, ¬ificationFromConfig{ + OrgId: notification.OrgId, + Name: notification.Name, + Type: notification.Type, + IsDefault: notification.IsDefault, + Settings: notification.Settings, + }) + } + + for _, notification := range cfg.DeleteNotifications { + r.DeleteNotifications = append(r.DeleteNotifications, &deleteNotificationConfig{ + OrgId: notification.OrgId, + Name: notification.Name, + }) + } + + return r +} + +func (dc *NotificationProvisioner) applyChanges(configPath string) error { + configs, err := dc.cfgProvider.readConfig(configPath) + if err != nil { + return err + } + + for _, cfg := range configs { + if err := dc.apply(cfg); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/services/provisioning/alert_notifications/config_reader.go b/pkg/services/provisioning/alert_notifications/config_reader.go new file mode 100644 index 00000000000..b69f9bf15b1 --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/config_reader.go @@ -0,0 +1,115 @@ +package alert_notifications + +import ( + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/services/alerting" + "gopkg.in/yaml.v2" +) + +type configReader struct { + log log.Logger +} + +func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error) { + var notifications []*notificationsAsConfig + cr.log.Debug("Looking for alert notification provisioning files", "path", path) + + files, err := ioutil.ReadDir(path) + if err != nil { + cr.log.Error("Can't read alert notification provisioning files from directory", "path", path) + return notifications, nil + } + + for _, file := range files { + if strings.HasSuffix(file.Name(), ".yaml") || strings.HasSuffix(file.Name(), ".yml") { + cr.log.Debug("Parsing alert notifications provisioning file", "path", path, "file.Name", file.Name()) + notifs, err := cr.parseNotificationConfig(path, file) + if err != nil { + return nil, err + } + + if notifs != nil { + notifications = append(notifications, notifs) + } + } + } + + cr.log.Debug("Validating alert notifications") + err = validateDefaultUniqueness(notifications) + if err != nil { + return nil, err + } + + err = validateType(notifications) + if err != nil { + return nil, err + } + + return notifications, nil +} + +func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) (*notificationsAsConfig, error) { + filename, _ := filepath.Abs(filepath.Join(path, file.Name())) + yamlFile, err := ioutil.ReadFile(filename) + if err != nil { + return nil, err + } + + var cfg *notificationsAsConfig + err = yaml.Unmarshal(yamlFile, &cfg) + if err != nil { + return nil, err + } + + return cfg.mapToNotificationFromConfig(), nil +} + +func validateDefaultUniqueness(notifications []*notificationsAsConfig) error { + for i := range notifications { + for _, notification := range notifications[i].Notifications { + if notification.OrgId < 1 { + notification.OrgId = 1 + } + } + + for _, notification := range notifications[i].DeleteNotifications { + if notification.OrgId < 1 { + notification.OrgId = 1 + } + } + } + + return nil +} + +func validateType(notifications []*notificationsAsConfig) error { + notifierTypes := alerting.GetNotifiers() + + for i := range notifications { + if notifications[i].Notifications == nil { + continue + } + + for _, notification := range notifications[i].Notifications { + foundNotifier := false + + for _, notifier := range notifierTypes { + if notifier.Type == notification.Type { + foundNotifier = true + break + } + } + + if !foundNotifier { + return ErrInvalidNotifierType + } + } + } + + return nil +} diff --git a/pkg/services/provisioning/alert_notifications/config_reader_test.go b/pkg/services/provisioning/alert_notifications/config_reader_test.go new file mode 100644 index 00000000000..e028f5bac40 --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/config_reader_test.go @@ -0,0 +1,219 @@ +package alert_notifications + +import ( + "testing" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/alerting" + . "github.com/smartystreets/goconvey/convey" +) + +var ( + logger = log.New("fake.log") + + correct_properties = "./test-configs/correct-properties" + brokenYaml = "./test-configs/broken-yaml" + doubleNotificationsConfig = "./test-configs/double-default" + emptyFolder = "./test-configs/empty_folder" + 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) + + alerting.RegisterNotifier(&alerting.NotifierPlugin{ + Type: "slack", + Name: "slack", + }) + alerting.RegisterNotifier(&alerting.NotifierPlugin{ + Type: "email", + Name: "email", + }) + Convey("Can read correct properties", func() { + cfgProvifer := &configReader{log: log.New("test logger")} + cfg, err := cfgProvifer.readConfig(correct_properties) + if err != nil { + t.Fatalf("readConfig return an error %v", err) + } + So(len(cfg), ShouldEqual, 1) + + ntCfg := cfg[0] + nts := ntCfg.Notifications + So(len(nts), ShouldEqual, 4) + + nt := nts[0] + So(nt.Name, ShouldEqual, "default-slack-notification") + So(nt.Type, ShouldEqual, "slack") + So(nt.OrgId, ShouldEqual, 2) + So(nt.IsDefault, ShouldBeTrue) + So(nt.Settings, ShouldResemble, map[string]interface{}{ + "recipient": "XXX", "token": "xoxb", "uploadImage": true, + }) + + nt = nts[1] + So(nt.Name, ShouldEqual, "another-not-default-notification") + So(nt.Type, ShouldEqual, "email") + So(nt.OrgId, ShouldEqual, 3) + 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.IsDefault, ShouldBeFalse) + + nt = nts[3] + So(nt.Name, ShouldEqual, "Added notification with whitespaces in name") + So(nt.Type, ShouldEqual, "email") + So(nt.OrgId, ShouldEqual, 3) + + deleteNts := ntCfg.DeleteNotifications + So(len(deleteNts), ShouldEqual, 4) + + deleteNt := deleteNts[0] + So(deleteNt.Name, ShouldEqual, "default-slack-notification") + So(deleteNt.OrgId, ShouldEqual, 2) + + deleteNt = deleteNts[1] + So(deleteNt.Name, ShouldEqual, "deleted-notification-without-orgId") + So(deleteNt.OrgId, ShouldEqual, 1) + + deleteNt = deleteNts[2] + So(deleteNt.Name, ShouldEqual, "deleted-notification-with-0-orgId") + So(deleteNt.OrgId, ShouldEqual, 1) + + deleteNt = deleteNts[3] + So(deleteNt.Name, ShouldEqual, "Deleted notification with whitespaces in name") + So(deleteNt.OrgId, ShouldEqual, 1) + }) + + Convey("One configured notification", func() { + Convey("no notification in database", 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, 2) + So(len(fakeRepo.updated), ShouldEqual, 0) + }) + Convey("One notification in database with same name", func() { + fakeRepo.loadAll = []*models.AlertNotification{ + {Name: "channel1", OrgId: 1, Id: 1}, + } + Convey("should update one notification", 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) + }) + }) + Convey("Two notifications with is_default", func() { + dc := newNotificationProvisioner(logger) + 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) + }) + }) + }) + + 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}, + } + 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) + }) + }) + }) + Convey("Empty yaml file", func() { + Convey("should have not changed repo", func() { + dc := newNotificationProvisioner(logger) + err := dc.applyChanges(emptyFile) + 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) + }) + }) + Convey("Broken yaml should return error", func() { + reader := &configReader{log: log.New("test logger")} + _, err := reader.readConfig(brokenYaml) + So(err, ShouldNotBeNil) + }) + Convey("Skip invalid directory", func() { + cfgProvifer := &configReader{log: log.New("test logger")} + cfg, err := cfgProvifer.readConfig(emptyFolder) + if err != nil { + t.Fatalf("readConfig return an error %v", err) + } + So(len(cfg), ShouldEqual, 0) + }) + Convey("Unknown notifier should return error", func() { + cfgProvifer := &configReader{log: log.New("test logger")} + _, err := cfgProvifer.readConfig(unknownNotifier) + So(err, ShouldEqual, ErrInvalidNotifierType) + }) + + }) +} + +type fakeRepository struct { + inserted []*models.CreateAlertNotificationCommand + deleted []*models.DeleteAlertNotificationCommand + updated []*models.UpdateAlertNotificationCommand + loadAll []*models.AlertNotification +} + +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 +} diff --git a/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml b/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml new file mode 100644 index 00000000000..e7c38d22f2c --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml @@ -0,0 +1,9 @@ +alert_notifications: + - name: notification-channel-1 + type: slack + org_id: 2 + 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/broken-yaml/not.yaml.text b/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/not.yaml.text new file mode 100644 index 00000000000..9050f543cef --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/not.yaml.text @@ -0,0 +1,6 @@ +#sfxzgnsxzcvnbzcvn +cvbn +cvbn +c +vbn +cvbncvbn \ 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 new file mode 100644 index 00000000000..d18f538d4ee --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml @@ -0,0 +1,26 @@ +alert_notifications: + - name: default-slack-notification + type: slack + org_id: 2 + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + - name: another-not-default-notification + type: email + org_id: 3 + is_default: false + - name: check-unset-is_default-is-false + type: slack + org_id: 3 + - name: Added notification with whitespaces in name + type: email + org_id: 3 +delete_alert_notifications: + - name: default-slack-notification + org_id: 2 + - name: deleted-notification-without-orgId + - name: deleted-notification-with-0-orgId + org_id: 0 + - name: Deleted notification with whitespaces in name \ 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 new file mode 100644 index 00000000000..aaa4541b993 --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml @@ -0,0 +1,4 @@ +alert_notifications: + - name: first-default + type: slack + is_default: true \ 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 new file mode 100644 index 00000000000..0636316614a --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml @@ -0,0 +1,4 @@ +alert_notifications: + - name: second-default + type: email + is_default: true \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/test-configs/empty/empty.yaml b/pkg/services/provisioning/alert_notifications/test-configs/empty/empty.yaml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/pkg/services/provisioning/alert_notifications/test-configs/empty_folder/.gitignore b/pkg/services/provisioning/alert_notifications/test-configs/empty_folder/.gitignore new file mode 100644 index 00000000000..86d0cb2726c --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/empty_folder/.gitignore @@ -0,0 +1,4 @@ +# Ignore everything in this directory +* +# Except this file +!.gitignore \ 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 new file mode 100644 index 00000000000..a5e34d92fd5 --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml @@ -0,0 +1,5 @@ +alert_notifications: + - name: channel1 + type: slack + - name: channel2 + type: email \ 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 new file mode 100644 index 00000000000..a59c38ea84b --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml @@ -0,0 +1,3 @@ +alert_notifications: + - name: unknown-notifier + type: nonexisting \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/types.go b/pkg/services/provisioning/alert_notifications/types.go new file mode 100644 index 00000000000..7718605cd67 --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/types.go @@ -0,0 +1,19 @@ +package alert_notifications + +type notificationsAsConfig struct { + Notifications []*notificationFromConfig `json:"alert_notifications" yaml:"alert_notifications"` + DeleteNotifications []*deleteNotificationConfig `json:"delete_alert_notifications" yaml:"delete_alert_notifications"` +} + +type deleteNotificationConfig struct { + Name string `json:"name" yaml:"name"` + OrgId int64 `json:"org_id" yaml:"org_id"` +} + +type notificationFromConfig struct { + OrgId int64 `json:"org_id" yaml:"org_id"` + 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"` +} diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 9044ae97389..fdecd03a3da 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -6,6 +6,7 @@ import ( "path" "github.com/grafana/grafana/pkg/registry" + "github.com/grafana/grafana/pkg/services/provisioning/alert_notifications" "github.com/grafana/grafana/pkg/services/provisioning/dashboards" "github.com/grafana/grafana/pkg/services/provisioning/datasources" "github.com/grafana/grafana/pkg/setting" @@ -25,6 +26,11 @@ func (ps *ProvisioningService) Init() error { return fmt.Errorf("Datasource provisioning error: %v", err) } + alertNotificationsPath := path.Join(ps.Cfg.ProvisioningPath, "alert_notifications") + if err := alert_notifications.Provision(alertNotificationsPath); err != nil { + return fmt.Errorf("Alert notification provisioning error: %v", err) + } + return nil } From 4fa45253cf30eb8af96c9c36b7f7fc80705704cb Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Thu, 29 Nov 2018 10:22:56 +0200 Subject: [PATCH 02/21] Added orgName parameter for alert_notifications --- .../alert_notifications.go | 27 +++++++- .../alert_notifications/config_reader.go | 12 +++- .../alert_notifications/config_reader_test.go | 64 +++++++++++++++---- .../correct-properties-with-orgName.yaml | 18 ++++++ .../provisioning/alert_notifications/types.go | 6 +- 5 files changed, 110 insertions(+), 17 deletions(-) create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml diff --git a/pkg/services/provisioning/alert_notifications/alert_notifications.go b/pkg/services/provisioning/alert_notifications/alert_notifications.go index eae5b4bb44e..2fa899dcd20 100644 --- a/pkg/services/provisioning/alert_notifications/alert_notifications.go +++ b/pkg/services/provisioning/alert_notifications/alert_notifications.go @@ -47,6 +47,15 @@ func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*d for _, notification := range notificationToDelete { dc.log.Info("Deleting alert notification", "name", notification.Name) + if notification.OrgId == 0 && notification.OrgName != "" { + getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} + if err := bus.Dispatch(getOrg); err != nil { + return err + } + notification.OrgId = getOrg.Result.Id + } else if notification.OrgId < 0 { + notification.OrgId = 1 + } getNotification := &models.GetAlertNotificationsQuery{Name: notification.Name, OrgId: notification.OrgId} if err := bus.Dispatch(getNotification); err != nil { @@ -66,6 +75,17 @@ func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*d func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*notificationFromConfig) error { for _, notification := range notificationToMerge { + + if notification.OrgId == 0 && notification.OrgName != "" { + getOrg := &models.GetOrgByNameQuery{Name: notification.OrgName} + if err := bus.Dispatch(getOrg); err != nil { + return err + } + notification.OrgId = getOrg.Result.Id + } else if notification.OrgId < 0 { + notification.OrgId = 1 + } + cmd := &models.GetAlertNotificationsQuery{OrgId: notification.OrgId, Name: notification.Name} err := bus.Dispatch(cmd) if err != nil { @@ -109,6 +129,7 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not return nil } + func (cfg *notificationsAsConfig) mapToNotificationFromConfig() *notificationsAsConfig { r := ¬ificationsAsConfig{} if cfg == nil { @@ -118,6 +139,7 @@ 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, @@ -127,8 +149,9 @@ func (cfg *notificationsAsConfig) mapToNotificationFromConfig() *notificationsAs for _, notification := range cfg.DeleteNotifications { r.DeleteNotifications = append(r.DeleteNotifications, &deleteNotificationConfig{ - OrgId: notification.OrgId, - Name: notification.Name, + 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 b69f9bf15b1..8d38bc76a1f 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader.go +++ b/pkg/services/provisioning/alert_notifications/config_reader.go @@ -73,13 +73,21 @@ func validateDefaultUniqueness(notifications []*notificationsAsConfig) error { for i := range notifications { for _, notification := range notifications[i].Notifications { if notification.OrgId < 1 { - notification.OrgId = 1 + if notification.OrgName == "" { + notification.OrgId = 1 + } else { + notification.OrgId = 0 + } } } for _, notification := range notifications[i].DeleteNotifications { if notification.OrgId < 1 { - notification.OrgId = 1 + if notification.OrgName == "" { + notification.OrgId = 1 + } else { + notification.OrgId = 0 + } } } } diff --git a/pkg/services/provisioning/alert_notifications/config_reader_test.go b/pkg/services/provisioning/alert_notifications/config_reader_test.go index e028f5bac40..11dddd3e475 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader_test.go +++ b/pkg/services/provisioning/alert_notifications/config_reader_test.go @@ -13,13 +13,14 @@ import ( var ( logger = log.New("fake.log") - correct_properties = "./test-configs/correct-properties" - brokenYaml = "./test-configs/broken-yaml" - doubleNotificationsConfig = "./test-configs/double-default" - emptyFolder = "./test-configs/empty_folder" - emptyFile = "./test-configs/empty" - twoNotificationsConfig = "./test-configs/two-notifications" - unknownNotifier = "./test-configs/unknown-notifier" + correct_properties = "./test-configs/correct-properties" + correct_properties_with_orgName = "./test-configs/correct-properties-with-orgName" + brokenYaml = "./test-configs/broken-yaml" + doubleNotificationsConfig = "./test-configs/double-default" + emptyFolder = "./test-configs/empty_folder" + emptyFile = "./test-configs/empty" + twoNotificationsConfig = "./test-configs/two-notifications" + unknownNotifier = "./test-configs/unknown-notifier" fakeRepo *fakeRepository ) @@ -32,6 +33,7 @@ func TestNotificationAsConfig(t *testing.T) { bus.AddHandler("test", mockInsert) bus.AddHandler("test", mockUpdate) bus.AddHandler("test", mockGet) + bus.AddHandler("test", mockGetOrg) alerting.RegisterNotifier(&alerting.NotifierPlugin{ Type: "slack", @@ -155,6 +157,36 @@ func TestNotificationAsConfig(t *testing.T) { }) }) }) + + 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}, + } + + fakeRepo.loadAll = []*models.AlertNotification{ + {Name: "default-slack-notification", OrgId: 1, Id: 1}, + {Name: "another-not-default-notification", OrgId: 2, Id: 2}, + } + dc := newNotificationProvisioner(logger) + 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) + + }) + Convey("Empty yaml file", func() { Convey("should have not changed repo", func() { dc := newNotificationProvisioner(logger) @@ -190,10 +222,11 @@ func TestNotificationAsConfig(t *testing.T) { } type fakeRepository struct { - inserted []*models.CreateAlertNotificationCommand - deleted []*models.DeleteAlertNotificationCommand - updated []*models.UpdateAlertNotificationCommand - loadAll []*models.AlertNotification + inserted []*models.CreateAlertNotificationCommand + deleted []*models.DeleteAlertNotificationCommand + updated []*models.UpdateAlertNotificationCommand + loadAll []*models.AlertNotification + loadAllOrg []*models.Org } func mockDelete(cmd *models.DeleteAlertNotificationCommand) error { @@ -217,3 +250,12 @@ func mockGet(cmd *models.GetAlertNotificationsQuery) error { } 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 new file mode 100644 index 00000000000..eafca5e77de --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml @@ -0,0 +1,18 @@ +alert_notifications: + - name: default-slack-notification + type: slack + org_name: Main Org. 1 + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + - name: another-not-default-notification + type: email + 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 diff --git a/pkg/services/provisioning/alert_notifications/types.go b/pkg/services/provisioning/alert_notifications/types.go index 7718605cd67..469ce1a3586 100644 --- a/pkg/services/provisioning/alert_notifications/types.go +++ b/pkg/services/provisioning/alert_notifications/types.go @@ -6,12 +6,14 @@ type notificationsAsConfig struct { } type deleteNotificationConfig struct { - Name string `json:"name" yaml:"name"` - OrgId int64 `json:"org_id" yaml:"org_id"` + 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"` From e1b87fc597c634ca2f7317af85dd922f76558184 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Tue, 4 Dec 2018 11:20:40 +0200 Subject: [PATCH 03/21] Added parameter org_name of alert notification to documentation --- docs/sources/administration/provisioning.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index 6b8a1a992da..a926f926ca9 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -230,10 +230,7 @@ By default Grafana will delete dashboards in the database if the file is removed > **Note.** Provisioning allows you to overwrite existing dashboards > which leads to problems if you re-use settings that are supposed to be unique. > Be careful not to re-use the same `title` multiple times within a folder -<<<<<<< HEAD > or `uid` within the same installation as this will cause weird behaviors. -======= -> or `uid` within the same installation as this will cause weird behaviours. ## Alert Notification Channels @@ -270,7 +267,10 @@ By default, exporting a dashboard as JSON will use a sequential identifier to re alert_notifications: - name: notification-channel-1 type: slack + # either org_id: 2 + # or + org_name: Main Org. is_default: true # See `Supported Settings` section for settings supporter for each # alert notification type. @@ -281,8 +281,12 @@ alert_notifications: delete_alert_notifications: - name: notification-channel-1 + # either org_id: 2 + # or + org_name: Main Org. - name: notification-channel-2 + # default org_id: 1 ``` ### Supported Settings @@ -407,5 +411,4 @@ The following sections detail the supported settings for each alert notification | ---- | | url | | username | -| password | ->>>>>>> Added alert_notification configuration +| password | \ No newline at end of file From 5c10a897f831f261ab746e0fdce9d2b75314926f Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Wed, 5 Dec 2018 17:42:53 +0200 Subject: [PATCH 04/21] Instantiating notifiers from config before using --- .../alert_notifications/sample.yaml | 37 ++++++++++--------- docs/sources/administration/provisioning.md | 1 + .../alert_notifications.go | 12 +----- .../alert_notifications/config_reader.go | 10 ++++- .../alert_notifications/config_reader_test.go | 21 ++++++++--- .../correct-properties-with-orgName.yaml | 3 ++ .../correct-properties.yaml | 7 ++++ .../test-configs/double-default/default-1.yml | 4 +- .../double-default/default-2.yaml | 4 +- .../incorrect-properties.yaml | 9 +++++ .../two-notifications/two-notifications.yaml | 6 ++- .../provisioning/alert_notifications/types.go | 12 ++++++ 12 files changed, 90 insertions(+), 36 deletions(-) create mode 100644 pkg/services/provisioning/alert_notifications/test-configs/incorrect-properties/incorrect-properties.yaml diff --git a/conf/provisioning/alert_notifications/sample.yaml b/conf/provisioning/alert_notifications/sample.yaml index 92d03a81720..9af25277738 100644 --- a/conf/provisioning/alert_notifications/sample.yaml +++ b/conf/provisioning/alert_notifications/sample.yaml @@ -1,20 +1,23 @@ # # config file version apiVersion: 1 -# alert_notifications: -# - name: default-slack -# type: slack -# org_id: 1 -# is_default: true -# settings: -# recipient: "XXX" -# token: "xoxb" -# uploadImage: true -# - name: default-email -# type: email -# org_id: 1 -# is_default: false -# delete_alert_notifications: -# - name: default-slack -# org_id: 1 -# - name: default-email \ No newline at end of file +alert_notifications: + - name: default-slack + type: slack + org_id: 1 + is_default: true + settings: + recipient: "XXX" + token: "xoxb" + uploadImage: true + url: https://slack.com + - name: default-email + type: email + org_id: 1 + is_default: false + settings: + addresses: example@example.com +delete_alert_notifications: + - name: default-slack + org_id: 1 + - name: default-email \ No newline at end of file diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index a926f926ca9..065db762e52 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -278,6 +278,7 @@ alert_notifications: recipient: "XXX" token: "xoxb" uploadImage: true + url: https://slack.com delete_alert_notifications: - name: notification-channel-1 diff --git a/pkg/services/provisioning/alert_notifications/alert_notifications.go b/pkg/services/provisioning/alert_notifications/alert_notifications.go index 2fa899dcd20..53b28d93c52 100644 --- a/pkg/services/provisioning/alert_notifications/alert_notifications.go +++ b/pkg/services/provisioning/alert_notifications/alert_notifications.go @@ -4,7 +4,6 @@ import ( "errors" "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" ) @@ -92,20 +91,13 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not return err } - settings := simplejson.New() - if len(notification.Settings) > 0 { - for k, v := range notification.Settings { - settings.Set(k, v) - } - } - if cmd.Result == nil { dc.log.Info("Inserting alert notification from configuration ", "name", notification.Name) insertCmd := &models.CreateAlertNotificationCommand{ Name: notification.Name, Type: notification.Type, IsDefault: notification.IsDefault, - Settings: settings, + Settings: notification.SettingsToJson(), OrgId: notification.OrgId, } if err := bus.Dispatch(insertCmd); err != nil { @@ -118,7 +110,7 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not Name: notification.Name, Type: notification.Type, IsDefault: notification.IsDefault, - Settings: settings, + Settings: notification.SettingsToJson(), OrgId: notification.OrgId, } if err := bus.Dispatch(updateCmd); err != nil { diff --git a/pkg/services/provisioning/alert_notifications/config_reader.go b/pkg/services/provisioning/alert_notifications/config_reader.go index 8d38bc76a1f..058159e49a4 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader.go +++ b/pkg/services/provisioning/alert_notifications/config_reader.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/grafana/grafana/pkg/log" + m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" "gopkg.in/yaml.v2" ) @@ -109,10 +110,17 @@ func validateType(notifications []*notificationsAsConfig) error { for _, notifier := range notifierTypes { if notifier.Type == notification.Type { foundNotifier = true + _, notifierError := notifier.Factory(&m.AlertNotification{ + Name: notification.Name, + Settings: notification.SettingsToJson(), + Type: notification.Type, + }) + if notifierError != nil { + return notifierError + } break } } - if !foundNotifier { return ErrInvalidNotifierType } diff --git a/pkg/services/provisioning/alert_notifications/config_reader_test.go b/pkg/services/provisioning/alert_notifications/config_reader_test.go index 11dddd3e475..274335e82bb 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader_test.go +++ b/pkg/services/provisioning/alert_notifications/config_reader_test.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" + "github.com/grafana/grafana/pkg/services/alerting/notifiers" . "github.com/smartystreets/goconvey/convey" ) @@ -14,6 +15,7 @@ var ( logger = log.New("fake.log") correct_properties = "./test-configs/correct-properties" + incorrect_properties = "./test-configs/incorrect-properties" correct_properties_with_orgName = "./test-configs/correct-properties-with-orgName" brokenYaml = "./test-configs/broken-yaml" doubleNotificationsConfig = "./test-configs/double-default" @@ -36,12 +38,14 @@ func TestNotificationAsConfig(t *testing.T) { bus.AddHandler("test", mockGetOrg) alerting.RegisterNotifier(&alerting.NotifierPlugin{ - Type: "slack", - Name: "slack", + Type: "slack", + Name: "slack", + Factory: notifiers.NewSlackNotifier, }) alerting.RegisterNotifier(&alerting.NotifierPlugin{ - Type: "email", - Name: "email", + Type: "email", + Name: "email", + Factory: notifiers.NewEmailNotifier, }) Convey("Can read correct properties", func() { cfgProvifer := &configReader{log: log.New("test logger")} @@ -61,7 +65,7 @@ func TestNotificationAsConfig(t *testing.T) { So(nt.OrgId, ShouldEqual, 2) So(nt.IsDefault, ShouldBeTrue) So(nt.Settings, ShouldResemble, map[string]interface{}{ - "recipient": "XXX", "token": "xoxb", "uploadImage": true, + "recipient": "XXX", "token": "xoxb", "uploadImage": true, "url": "https://slack.com", }) nt = nts[1] @@ -218,6 +222,13 @@ func TestNotificationAsConfig(t *testing.T) { So(err, ShouldEqual, ErrInvalidNotifierType) }) + Convey("Read incorrect properties", func() { + cfgProvifer := &configReader{log: log.New("test logger")} + _, err := cfgProvifer.readConfig(incorrect_properties) + So(err, ShouldNotBeNil) + So(err.Error(), ShouldEqual, "Alert validation error: Could not find url property in settings") + }) + }) } 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 eafca5e77de..45631c38b9f 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 @@ -7,8 +7,11 @@ alert_notifications: recipient: "XXX" token: "xoxb" uploadImage: true + url: https://slack.com - name: another-not-default-notification type: email + settings: + addresses: example@example.com org_name: Main Org. 2 is_default: false delete_alert_notifications: 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 d18f538d4ee..c4b7d7cb224 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 @@ -7,16 +7,23 @@ alert_notifications: recipient: "XXX" token: "xoxb" uploadImage: true + url: https://slack.com - name: another-not-default-notification type: email + settings: + addresses: example@exmaple.com org_id: 3 is_default: false - name: check-unset-is_default-is-false type: slack org_id: 3 + settings: + url: https://slack.com - name: Added notification with whitespaces in name type: email org_id: 3 + settings: + addresses: example@exmaple.com delete_alert_notifications: - name: default-slack-notification org_id: 2 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 aaa4541b993..ca6c347433f 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,4 +1,6 @@ alert_notifications: - name: first-default type: slack - is_default: true \ No newline at end of file + 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 0636316614a..8b07e4ca9c4 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,4 +1,6 @@ alert_notifications: - name: second-default type: email - is_default: true \ No newline at end of file + 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-properties/incorrect-properties.yaml new file mode 100644 index 00000000000..f01234564e0 --- /dev/null +++ b/pkg/services/provisioning/alert_notifications/test-configs/incorrect-properties/incorrect-properties.yaml @@ -0,0 +1,9 @@ +alert_notifications: + - name: slack-notification-without-url-in-settings + type: slack + org_id: 2 + 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 a5e34d92fd5..dd93c815b04 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,5 +1,9 @@ alert_notifications: - name: channel1 type: slack + settings: + url: http://slack.com - name: channel2 - type: email \ No newline at end of file + type: email + settings: + addresses: example@example.com \ 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 469ce1a3586..7ac2e6900e5 100644 --- a/pkg/services/provisioning/alert_notifications/types.go +++ b/pkg/services/provisioning/alert_notifications/types.go @@ -1,5 +1,7 @@ package alert_notifications +import "github.com/grafana/grafana/pkg/components/simplejson" + type notificationsAsConfig struct { Notifications []*notificationFromConfig `json:"alert_notifications" yaml:"alert_notifications"` DeleteNotifications []*deleteNotificationConfig `json:"delete_alert_notifications" yaml:"delete_alert_notifications"` @@ -19,3 +21,13 @@ type notificationFromConfig struct { IsDefault bool `json:"is_default" yaml:"is_default"` Settings map[string]interface{} `json:"settings" yaml:"settings"` } + +func (notification notificationFromConfig) SettingsToJson() *simplejson.Json { + settings := simplejson.New() + if len(notification.Settings) > 0 { + for k, v := range notification.Settings { + settings.Set(k, v) + } + } + return settings +} From 7b09dd38d8805b18895556c2c4ef90d6d2c0902b Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Wed, 5 Dec 2018 17:58:27 +0200 Subject: [PATCH 05/21] Commented alert_notifications sample config --- .../alert_notifications/sample.yaml | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/conf/provisioning/alert_notifications/sample.yaml b/conf/provisioning/alert_notifications/sample.yaml index 9af25277738..1e609f07a0f 100644 --- a/conf/provisioning/alert_notifications/sample.yaml +++ b/conf/provisioning/alert_notifications/sample.yaml @@ -1,23 +1,23 @@ # # config file version apiVersion: 1 -alert_notifications: - - name: default-slack - type: slack - org_id: 1 - is_default: true - settings: - recipient: "XXX" - token: "xoxb" - uploadImage: true - url: https://slack.com - - name: default-email - type: email - org_id: 1 - is_default: false - settings: - addresses: example@example.com -delete_alert_notifications: - - name: default-slack - org_id: 1 - - name: default-email \ No newline at end of file +# alert_notifications: +# - name: default-slack +# type: slack +# org_id: 1 +# is_default: true +# settings: +# recipient: "XXX" +# token: "xoxb" +# uploadImage: true +# url: https://slack.com +# - name: default-email +# type: email +# org_id: 1 +# is_default: false +# settings: +# addresses: example@example.com +# delete_alert_notifications: +# - name: default-slack +# org_id: 1 +# - name: default-email \ No newline at end of file From 6d1ec19fe9a75f57ebc8be63bfa4ab8149f20df0 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Wed, 5 Dec 2018 18:17:47 +0200 Subject: [PATCH 06/21] Renamed validation funcs for alert notification --- .../alert_notifications/config_reader.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/pkg/services/provisioning/alert_notifications/config_reader.go b/pkg/services/provisioning/alert_notifications/config_reader.go index 058159e49a4..ee99ae2d6e8 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader.go +++ b/pkg/services/provisioning/alert_notifications/config_reader.go @@ -41,12 +41,9 @@ func (cr *configReader) readConfig(path string) ([]*notificationsAsConfig, error } cr.log.Debug("Validating alert notifications") - err = validateDefaultUniqueness(notifications) - if err != nil { - return nil, err - } + validateOrgIdAndSet(notifications) - err = validateType(notifications) + err = validateNotifications(notifications) if err != nil { return nil, err } @@ -70,7 +67,7 @@ func (cr *configReader) parseNotificationConfig(path string, file os.FileInfo) ( return cfg.mapToNotificationFromConfig(), nil } -func validateDefaultUniqueness(notifications []*notificationsAsConfig) error { +func validateOrgIdAndSet(notifications []*notificationsAsConfig) { for i := range notifications { for _, notification := range notifications[i].Notifications { if notification.OrgId < 1 { @@ -93,10 +90,9 @@ func validateDefaultUniqueness(notifications []*notificationsAsConfig) error { } } - return nil } -func validateType(notifications []*notificationsAsConfig) error { +func validateNotifications(notifications []*notificationsAsConfig) error { notifierTypes := alerting.GetNotifiers() for i := range notifications { From f132e929cebe40c1aea2c8b96c2acae3c78661f3 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Fri, 14 Dec 2018 11:53:50 +0200 Subject: [PATCH 07/21] 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, + })) } From d0370ea7ad00b3935731a29dd60a362e2b422f5f Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Thu, 20 Dec 2018 12:23:44 +0200 Subject: [PATCH 08/21] Using func InitNotifier for verifying notification settings --- .../alert_notifications.go | 1 - .../alert_notifications/config_reader.go | 26 +++++-------------- .../alert_notifications/config_reader_test.go | 3 ++- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/pkg/services/provisioning/alert_notifications/alert_notifications.go b/pkg/services/provisioning/alert_notifications/alert_notifications.go index 345446aa98b..0f155dda4a8 100644 --- a/pkg/services/provisioning/alert_notifications/alert_notifications.go +++ b/pkg/services/provisioning/alert_notifications/alert_notifications.go @@ -10,7 +10,6 @@ import ( var ( ErrInvalidConfigTooManyDefault = errors.New("Alert notification provisioning config is invalid. Only one alert notification can be marked as default") - ErrInvalidNotifierType = errors.New("Unknown notifier type") ) func Provision(configDirectory string) error { diff --git a/pkg/services/provisioning/alert_notifications/config_reader.go b/pkg/services/provisioning/alert_notifications/config_reader.go index bd97947de36..56bfdab2f19 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader.go +++ b/pkg/services/provisioning/alert_notifications/config_reader.go @@ -136,7 +136,6 @@ func validateRequiredField(notifications []*notificationsAsConfig) error { } func validateNotifications(notifications []*notificationsAsConfig) error { - notifierTypes := alerting.GetNotifiers() for i := range notifications { if notifications[i].Notifications == nil { @@ -144,24 +143,13 @@ func validateNotifications(notifications []*notificationsAsConfig) error { } for _, notification := range notifications[i].Notifications { - foundNotifier := false - - for _, notifier := range notifierTypes { - if notifier.Type == notification.Type { - foundNotifier = true - _, notifierError := notifier.Factory(&m.AlertNotification{ - Name: notification.Name, - Settings: notification.SettingsToJson(), - Type: notification.Type, - }) - if notifierError != nil { - return notifierError - } - break - } - } - if !foundNotifier { - return ErrInvalidNotifierType + _, err := alerting.InitNotifier(&m.AlertNotification{ + Name: notification.Name, + Settings: notification.SettingsToJson(), + Type: notification.Type, + }) + if err != nil { + return err } } } diff --git a/pkg/services/provisioning/alert_notifications/config_reader_test.go b/pkg/services/provisioning/alert_notifications/config_reader_test.go index 0ef3d5abcb9..d66dded8407 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader_test.go +++ b/pkg/services/provisioning/alert_notifications/config_reader_test.go @@ -293,7 +293,8 @@ func TestNotificationAsConfig(t *testing.T) { Convey("Unknown notifier should return error", func() { cfgProvifer := &configReader{log: log.New("test logger")} _, err := cfgProvifer.readConfig(unknownNotifier) - So(err, ShouldEqual, ErrInvalidNotifierType) + So(err, ShouldNotBeNil) + So(err.Error(), ShouldEqual, "Unsupported notification type") }) Convey("Read incorrect properties", func() { From 4bcace567bc84557b209dc5fbd9d9eff70282359 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Thu, 20 Dec 2018 12:45:18 +0200 Subject: [PATCH 09/21] Formatted errors to err --- pkg/services/sqlstore/alert_notification.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 392f985ae61..9231c896cf1 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -46,8 +46,8 @@ 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 err := getAlertNotificationWithUidInternal(existingNotification, newSession()); err != nil { + return err } if existingNotification.Result != nil { @@ -55,8 +55,8 @@ func DeleteAlertNotificationWithUid(cmd *m.DeleteAlertNotificationWithUidCommand Id: existingNotification.Result.Id, OrgId: existingNotification.Result.OrgId, } - if deleteErr := bus.Dispatch(deleteCommand); deleteErr != nil { - return deleteErr + if err := bus.Dispatch(deleteCommand); err != nil { + return err } } @@ -343,10 +343,8 @@ 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 + if err := getAlertNotificationWithUidInternal(getAlertNotificationWithUidQuery, newSession()); err != nil { + return err } current := getAlertNotificationWithUidQuery.Result @@ -368,8 +366,8 @@ func UpdateAlertNotificationWithUid(cmd *m.UpdateAlertNotificationWithUidCommand OrgId: cmd.OrgId, } - if updateErr := bus.Dispatch(updateNotification); updateErr != nil { - return updateErr + if err := bus.Dispatch(updateNotification); err != nil { + return err } return nil From 2de32756c257941703f22d0e1afbaa523dee55b1 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Thu, 20 Dec 2018 17:12:47 +0200 Subject: [PATCH 10/21] Returned id for alert notifications which were created without uid --- pkg/services/alerting/extractor_test.go | 129 ++++++++++-------- pkg/services/alerting/rule.go | 33 ++++- pkg/services/alerting/rule_test.go | 127 ++++++++++++----- .../alerting/testdata/dash-without-id.json | 3 + .../alerting/testdata/influxdb-alert.json | 5 +- pkg/services/sqlstore/alert_notification.go | 1 + 6 files changed, 197 insertions(+), 101 deletions(-) diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go index 9665a657bb7..66adf951269 100644 --- a/pkg/services/alerting/extractor_test.go +++ b/pkg/services/alerting/extractor_test.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore" . "github.com/smartystreets/goconvey/convey" ) @@ -197,74 +198,86 @@ func TestAlertRuleExtraction(t *testing.T) { }) }) - Convey("Parse and validate dashboard containing influxdb alert", func() { - json, err := ioutil.ReadFile("./testdata/influxdb-alert.json") + Convey("Alert notifications are in DB", func() { + sqlstore.InitTestDB(t) + err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."}) + So(err, ShouldBeNil) + firstNotification := m.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"} + err = sqlstore.CreateAlertNotificationCommand(&firstNotification) + So(err, ShouldBeNil) + secondNotification := m.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"} + err = sqlstore.CreateAlertNotificationCommand(&secondNotification) So(err, ShouldBeNil) - dashJson, err := simplejson.NewJson(json) - So(err, ShouldBeNil) - dash := m.NewDashboardFromJson(dashJson) - extractor := NewDashAlertExtractor(dash, 1, nil) - - alerts, err := extractor.GetAlerts() - - Convey("Get rules without error", func() { + Convey("Parse and validate dashboard containing influxdb alert", func() { + json, err := ioutil.ReadFile("./testdata/influxdb-alert.json") So(err, ShouldBeNil) - }) - Convey("should be able to read interval", func() { - So(len(alerts), ShouldEqual, 1) - - for _, alert := range alerts { - So(alert.DashboardId, ShouldEqual, 4) - - conditions := alert.Settings.Get("conditions").MustArray() - cond := simplejson.NewFromAny(conditions[0]) - - So(cond.Get("query").Get("model").Get("interval").MustString(), ShouldEqual, ">10s") - } - }) - }) - - Convey("Should be able to extract collapsed panels", func() { - json, err := ioutil.ReadFile("./testdata/collapsed-panels.json") - So(err, ShouldBeNil) - - dashJson, err := simplejson.NewJson(json) - So(err, ShouldBeNil) - - dash := m.NewDashboardFromJson(dashJson) - extractor := NewDashAlertExtractor(dash, 1, nil) - - alerts, err := extractor.GetAlerts() - - Convey("Get rules without error", func() { + dashJson, err := simplejson.NewJson(json) So(err, ShouldBeNil) + dash := m.NewDashboardFromJson(dashJson) + extractor := NewDashAlertExtractor(dash, 1, nil) + + alerts, err := extractor.GetAlerts() + + Convey("Get rules without error", func() { + So(err, ShouldBeNil) + }) + + Convey("should be able to read interval", func() { + So(len(alerts), ShouldEqual, 1) + + for _, alert := range alerts { + So(alert.DashboardId, ShouldEqual, 4) + + conditions := alert.Settings.Get("conditions").MustArray() + cond := simplejson.NewFromAny(conditions[0]) + + So(cond.Get("query").Get("model").Get("interval").MustString(), ShouldEqual, ">10s") + } + }) }) - Convey("should be able to extract collapsed alerts", func() { - So(len(alerts), ShouldEqual, 4) - }) - }) - - Convey("Parse and validate dashboard without id and containing an alert", func() { - json, err := ioutil.ReadFile("./testdata/dash-without-id.json") - So(err, ShouldBeNil) - - dashJSON, err := simplejson.NewJson(json) - So(err, ShouldBeNil) - dash := m.NewDashboardFromJson(dashJSON) - extractor := NewDashAlertExtractor(dash, 1, nil) - - err = extractor.ValidateAlerts() - - Convey("Should validate without error", func() { + Convey("Should be able to extract collapsed panels", func() { + json, err := ioutil.ReadFile("./testdata/collapsed-panels.json") So(err, ShouldBeNil) + + dashJson, err := simplejson.NewJson(json) + So(err, ShouldBeNil) + + dash := m.NewDashboardFromJson(dashJson) + extractor := NewDashAlertExtractor(dash, 1, nil) + + alerts, err := extractor.GetAlerts() + + Convey("Get rules without error", func() { + So(err, ShouldBeNil) + }) + + Convey("should be able to extract collapsed alerts", func() { + So(len(alerts), ShouldEqual, 4) + }) }) - Convey("Should fail on save", func() { - _, err := extractor.GetAlerts() - So(err.Error(), ShouldEqual, "Alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1") + Convey("Parse and validate dashboard without id and containing an alert", func() { + json, err := ioutil.ReadFile("./testdata/dash-without-id.json") + So(err, ShouldBeNil) + + dashJSON, err := simplejson.NewJson(json) + So(err, ShouldBeNil) + dash := m.NewDashboardFromJson(dashJSON) + extractor := NewDashAlertExtractor(dash, 1, nil) + + err = extractor.ValidateAlerts() + + Convey("Should validate without error", func() { + So(err, ShouldBeNil) + }) + + Convey("Should fail on save", func() { + _, err := extractor.GetAlerts() + So(err.Error(), ShouldEqual, "Alert validation error: Panel id is not correct, alertName=Influxdb, panelId=1") + }) }) }) }) diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index 75901ac5e2e..529f3b9ff95 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -9,6 +9,8 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" + + "github.com/grafana/grafana/pkg/bus" ) var ( @@ -126,12 +128,33 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) { for _, v := range ruleDef.Settings.Get("notifications").MustArray() { jsonModel := simplejson.NewFromAny(v) - uid, err := jsonModel.Get("uid").String() - if err != nil { - return nil, ValidationError{Reason: "Invalid notification schema", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} - } - model.Notifications = append(model.Notifications, uid) + if id, err := jsonModel.Get("id").Int64(); err == nil { + cmd := m.GetAlertNotificationsQuery{ + Id: id, + OrgId: ruleDef.OrgId, + } + if err = bus.Dispatch(&cmd); err != nil { + return nil, err + } + + if cmd.Result == nil { + errString := fmt.Sprintf("Alert notification id %d doesn't exist", id) + return nil, ValidationError{Reason: errString, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} + } + + if cmd.Result.Uid == "" { + errString := fmt.Sprintf("Alert notification id %d has empty uid", id) + return nil, ValidationError{Reason: errString, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} + } + model.Notifications = append(model.Notifications, cmd.Result.Uid) + } else { + if uid, err := jsonModel.Get("uid").String(); err != nil { + return nil, ValidationError{Reason: "Neither id nor uid is specified", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} + } else { + model.Notifications = append(model.Notifications, uid) + } + } } for index, condition := range ruleDef.Settings.Get("conditions").MustArray() { diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index 5ebae97caed..9b99278a5aa 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -5,6 +5,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore" . "github.com/smartystreets/goconvey/convey" ) @@ -45,6 +46,7 @@ func TestAlertRuleFrequencyParsing(t *testing.T) { } func TestAlertRuleModel(t *testing.T) { + sqlstore.InitTestDB(t) Convey("Testing alert rule", t, func() { RegisterCondition("test", func(model *simplejson.Json, index int) (Condition, error) { @@ -57,46 +59,59 @@ func TestAlertRuleModel(t *testing.T) { }) Convey("can construct alert rule model", func() { - json := ` - { - "name": "name2", - "description": "desc2", - "handler": 0, - "noDataMode": "critical", - "enabled": true, - "frequency": "60s", - "conditions": [ - { - "type": "test", - "prop": 123 - } - ], - "notifications": [ - {"uid": "1134"}, - {"uid": "22"} - ] - } - ` - - alertJSON, jsonErr := simplejson.NewJson([]byte(json)) - So(jsonErr, ShouldBeNil) - - alert := &m.Alert{ - Id: 1, - OrgId: 1, - DashboardId: 1, - PanelId: 1, - - Settings: alertJSON, - } - - alertRule, err := NewRuleFromDBAlert(alert) + err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."}) + So(err, ShouldBeNil) + firstNotification := m.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"} + err = sqlstore.CreateAlertNotificationCommand(&firstNotification) + So(err, ShouldBeNil) + secondNotification := m.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"} + err = sqlstore.CreateAlertNotificationCommand(&secondNotification) So(err, ShouldBeNil) - So(len(alertRule.Conditions), ShouldEqual, 1) + Convey("with notification id and uid", func() { + json := ` + { + "name": "name2", + "description": "desc2", + "handler": 0, + "noDataMode": "critical", + "enabled": true, + "frequency": "60s", + "conditions": [ + { + "type": "test", + "prop": 123 + } + ], + "notifications": [ + {"id": 1}, + {"uid": "notifier2"} + ] + } + ` - Convey("Can read notifications", func() { - So(len(alertRule.Notifications), ShouldEqual, 2) + alertJSON, jsonErr := simplejson.NewJson([]byte(json)) + So(jsonErr, ShouldBeNil) + + alert := &m.Alert{ + Id: 1, + OrgId: 1, + DashboardId: 1, + PanelId: 1, + + Settings: alertJSON, + } + + alertRule, err := NewRuleFromDBAlert(alert) + So(err, ShouldBeNil) + + So(len(alertRule.Conditions), ShouldEqual, 1) + + Convey("Can read notifications", func() { + So(len(alertRule.Notifications), ShouldEqual, 2) + So(alertRule.Notifications, ShouldContain, "notifier1") + So(alertRule.Notifications, ShouldContain, "notifier2") + }) }) }) @@ -130,5 +145,43 @@ func TestAlertRuleModel(t *testing.T) { So(alertRule.Frequency, ShouldEqual, 60) }) + Convey("raise error in case of missing notification id and uid", func() { + json := ` + { + "name": "name2", + "description": "desc2", + "noDataMode": "critical", + "enabled": true, + "frequency": "60s", + "conditions": [ + { + "type": "test", + "prop": 123 + } + ], + "notifications": [ + {"not_id_uid": "1134"} + ] + } + ` + + alertJSON, jsonErr := simplejson.NewJson([]byte(json)) + So(jsonErr, ShouldBeNil) + + alert := &m.Alert{ + Id: 1, + OrgId: 1, + DashboardId: 1, + PanelId: 1, + Frequency: 0, + + Settings: alertJSON, + } + + _, err := NewRuleFromDBAlert(alert) + So(err, ShouldNotBeNil) + So(err.Error(), ShouldEqual, "Alert validation error: Neither id nor uid is specified AlertId: 1 PanelId: 1 DashboardId: 1") + }) + }) } diff --git a/pkg/services/alerting/testdata/dash-without-id.json b/pkg/services/alerting/testdata/dash-without-id.json index 59fbd679cf2..02cd2c002f0 100644 --- a/pkg/services/alerting/testdata/dash-without-id.json +++ b/pkg/services/alerting/testdata/dash-without-id.json @@ -45,6 +45,9 @@ "notifications": [ { "uid": "notifier1" + }, + { + "id": 2 } ] }, diff --git a/pkg/services/alerting/testdata/influxdb-alert.json b/pkg/services/alerting/testdata/influxdb-alert.json index 2dcec447b15..29f1a0c8e5e 100644 --- a/pkg/services/alerting/testdata/influxdb-alert.json +++ b/pkg/services/alerting/testdata/influxdb-alert.json @@ -45,7 +45,10 @@ "noDataState": "no_data", "notifications": [ { - "uid": "notifier1" + "id": 1 + }, + { + "uid": "notifier2" } ] }, diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 9231c896cf1..0b958489624 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -130,6 +130,7 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS sql.WriteString(`SELECT alert_notification.id, + alert_notification.uid, alert_notification.org_id, alert_notification.name, alert_notification.type, From f461d5200465de994cbaa32718ed98651cb907c6 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Wed, 2 Jan 2019 10:34:07 +0200 Subject: [PATCH 11/21] Converted notification id to uid via fmt for old alert notification settings --- pkg/services/alerting/rule.go | 24 ++------------------- pkg/services/alerting/rule_test.go | 6 +++--- pkg/services/sqlstore/alert_notification.go | 1 - 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index 529f3b9ff95..902c1660976 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -9,8 +9,6 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" - - "github.com/grafana/grafana/pkg/bus" ) var ( @@ -129,28 +127,10 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) { for _, v := range ruleDef.Settings.Get("notifications").MustArray() { jsonModel := simplejson.NewFromAny(v) if id, err := jsonModel.Get("id").Int64(); err == nil { - cmd := m.GetAlertNotificationsQuery{ - Id: id, - OrgId: ruleDef.OrgId, - } - - if err = bus.Dispatch(&cmd); err != nil { - return nil, err - } - - if cmd.Result == nil { - errString := fmt.Sprintf("Alert notification id %d doesn't exist", id) - return nil, ValidationError{Reason: errString, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} - } - - if cmd.Result.Uid == "" { - errString := fmt.Sprintf("Alert notification id %d has empty uid", id) - return nil, ValidationError{Reason: errString, DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} - } - model.Notifications = append(model.Notifications, cmd.Result.Uid) + model.Notifications = append(model.Notifications, fmt.Sprintf("%09d", id)) } else { if uid, err := jsonModel.Get("uid").String(); err != nil { - return nil, ValidationError{Reason: "Neither id nor uid is specified", DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} + return nil, ValidationError{Reason: "Neither id nor uid is specified, " + err.Error(), DashboardId: model.DashboardId, Alertid: model.Id, PanelId: model.PanelId} } else { model.Notifications = append(model.Notifications, uid) } diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index 9b99278a5aa..7e8af888338 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -61,7 +61,7 @@ func TestAlertRuleModel(t *testing.T) { Convey("can construct alert rule model", func() { err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."}) So(err, ShouldBeNil) - firstNotification := m.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"} + firstNotification := m.CreateAlertNotificationCommand{OrgId: 1, Name: "1"} err = sqlstore.CreateAlertNotificationCommand(&firstNotification) So(err, ShouldBeNil) secondNotification := m.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"} @@ -109,7 +109,7 @@ func TestAlertRuleModel(t *testing.T) { Convey("Can read notifications", func() { So(len(alertRule.Notifications), ShouldEqual, 2) - So(alertRule.Notifications, ShouldContain, "notifier1") + So(alertRule.Notifications, ShouldContain, "000000001") So(alertRule.Notifications, ShouldContain, "notifier2") }) }) @@ -180,7 +180,7 @@ func TestAlertRuleModel(t *testing.T) { _, err := NewRuleFromDBAlert(alert) So(err, ShouldNotBeNil) - So(err.Error(), ShouldEqual, "Alert validation error: Neither id nor uid is specified AlertId: 1 PanelId: 1 DashboardId: 1") + So(err.Error(), ShouldEqual, "Alert validation error: Neither id nor uid is specified, type assertion to string failed AlertId: 1 PanelId: 1 DashboardId: 1") }) }) diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 0b958489624..9231c896cf1 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -130,7 +130,6 @@ func getAlertNotificationInternal(query *m.GetAlertNotificationsQuery, sess *DBS sql.WriteString(`SELECT alert_notification.id, - alert_notification.uid, alert_notification.org_id, alert_notification.name, alert_notification.type, From 6a8e39ba1741acba9ddef6fac5a43c2b97ddf627 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Wed, 16 Jan 2019 16:34:32 +0200 Subject: [PATCH 12/21] Added uid to AlertNotification json --- pkg/api/dtos/alerting.go | 2 ++ public/app/features/alerting/AlertTabCtrl.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/api/dtos/alerting.go b/pkg/api/dtos/alerting.go index c037831f341..dcdc3976ec5 100644 --- a/pkg/api/dtos/alerting.go +++ b/pkg/api/dtos/alerting.go @@ -50,6 +50,7 @@ func formatShort(interval time.Duration) string { func NewAlertNotification(notification *models.AlertNotification) *AlertNotification { return &AlertNotification{ Id: notification.Id, + Uid: notification.Uid, Name: notification.Name, Type: notification.Type, IsDefault: notification.IsDefault, @@ -64,6 +65,7 @@ func NewAlertNotification(notification *models.AlertNotification) *AlertNotifica type AlertNotification struct { Id int64 `json:"id"` + Uid string `json:"uid"` Name string `json:"name"` Type string `json:"type"` IsDefault bool `json:"isDefault"` diff --git a/public/app/features/alerting/AlertTabCtrl.ts b/public/app/features/alerting/AlertTabCtrl.ts index af00e79b085..d1accae6787 100644 --- a/public/app/features/alerting/AlertTabCtrl.ts +++ b/public/app/features/alerting/AlertTabCtrl.ts @@ -141,7 +141,7 @@ export class AlertTabCtrl { iconClass: this.getNotificationIcon(model.type), isDefault: false, }); - this.alert.notifications.push({ id: model.id }); + this.alert.notifications.push({ uid: model.uid }); // reset plus button this.addNotificationSegment.value = this.uiSegmentSrv.newPlusButton().value; From c27bf6e688ab43cc2ef233d015d65938917f7fda Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Fri, 18 Jan 2019 16:19:06 +0200 Subject: [PATCH 13/21] Check that alert notification with id already exists in notification settings --- public/app/features/alerting/AlertTabCtrl.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/public/app/features/alerting/AlertTabCtrl.ts b/public/app/features/alerting/AlertTabCtrl.ts index d1accae6787..ecde4aa0a4b 100644 --- a/public/app/features/alerting/AlertTabCtrl.ts +++ b/public/app/features/alerting/AlertTabCtrl.ts @@ -141,7 +141,9 @@ export class AlertTabCtrl { iconClass: this.getNotificationIcon(model.type), isDefault: false, }); - this.alert.notifications.push({ uid: model.uid }); + if (!_.find(this.alert.notifications, { id: model.id})) { + this.alert.notifications.push({ uid: model.uid }); + } // reset plus button this.addNotificationSegment.value = this.uiSegmentSrv.newPlusButton().value; From 43c3d5b8bafa51a2f3bf8ecf36a23be16d5d4a35 Mon Sep 17 00:00:00 2001 From: Pavel Bakulev Date: Fri, 18 Jan 2019 16:26:23 +0200 Subject: [PATCH 14/21] Updated removing notification channel by uid --- public/app/features/alerting/AlertTabCtrl.ts | 7 ++++--- public/app/features/alerting/partials/alert_tab.html | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/public/app/features/alerting/AlertTabCtrl.ts b/public/app/features/alerting/AlertTabCtrl.ts index ecde4aa0a4b..fe27f45e0f3 100644 --- a/public/app/features/alerting/AlertTabCtrl.ts +++ b/public/app/features/alerting/AlertTabCtrl.ts @@ -140,6 +140,7 @@ export class AlertTabCtrl { name: model.name, iconClass: this.getNotificationIcon(model.type), isDefault: false, + uid: model.uid }); if (!_.find(this.alert.notifications, { id: model.id})) { this.alert.notifications.push({ uid: model.uid }); @@ -151,9 +152,9 @@ export class AlertTabCtrl { this.addNotificationSegment.fake = true; } - removeNotification(index) { - this.alert.notifications.splice(index, 1); - this.alertNotifications.splice(index, 1); + removeNotification(deleteUid) { + _.remove(this.alert.notifications, { uid: deleteUid}); + _.remove(this.alertNotifications, { uid: deleteUid}); } initModel() { diff --git a/public/app/features/alerting/partials/alert_tab.html b/public/app/features/alerting/partials/alert_tab.html index 9dfd3da47f9..27518a1e44b 100644 --- a/public/app/features/alerting/partials/alert_tab.html +++ b/public/app/features/alerting/partials/alert_tab.html @@ -135,7 +135,7 @@
 {{nc.name}}  - +
From 48aa173f67c3c8525985532b8d3dc4a1cf44f57b Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 13:20:29 +0100 Subject: [PATCH 15/21] support both uid and id for showing/removing notifiers --- public/app/features/alerting/AlertTabCtrl.ts | 21 ++++++++++++++----- .../features/alerting/partials/alert_tab.html | 2 +- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/public/app/features/alerting/AlertTabCtrl.ts b/public/app/features/alerting/AlertTabCtrl.ts index fe27f45e0f3..12943805c2c 100644 --- a/public/app/features/alerting/AlertTabCtrl.ts +++ b/public/app/features/alerting/AlertTabCtrl.ts @@ -142,7 +142,9 @@ export class AlertTabCtrl { isDefault: false, uid: model.uid }); - if (!_.find(this.alert.notifications, { id: model.id})) { + + // avoid duplicates using both id and uid to be backwards compatible. + if (!_.find(this.alert.notifications, n => n.id === model.id || n.uid === model.uid)) { this.alert.notifications.push({ uid: model.uid }); } @@ -152,9 +154,11 @@ export class AlertTabCtrl { this.addNotificationSegment.fake = true; } - removeNotification(deleteUid) { - _.remove(this.alert.notifications, { uid: deleteUid}); - _.remove(this.alertNotifications, { uid: deleteUid}); + removeNotification(an) { + // remove notifiers refeered to by id and uid to support notifiers added + // before and after we added support for uid + _.remove(this.alert.notifications, n => n.uid === an.uid || n.id === an.id); + _.remove(this.alertNotifications, n => n.uid === an.uid || n.id === an.id); } initModel() { @@ -190,7 +194,14 @@ export class AlertTabCtrl { ThresholdMapper.alertToGraphThresholds(this.panel); for (const addedNotification of alert.notifications) { - const model = _.find(this.notifications, { id: addedNotification.id }); + // lookup notifier type by uid + let model = _.find(this.notifications, { uid: addedNotification.uid }); + + // fallback to using id if uid is missing + if (!model) { + model = _.find(this.notifications, { id: addedNotification.id }); + } + if (model && model.isDefault === false) { model.iconClass = this.getNotificationIcon(model.type); this.alertNotifications.push(model); diff --git a/public/app/features/alerting/partials/alert_tab.html b/public/app/features/alerting/partials/alert_tab.html index 27518a1e44b..b99859fd847 100644 --- a/public/app/features/alerting/partials/alert_tab.html +++ b/public/app/features/alerting/partials/alert_tab.html @@ -135,7 +135,7 @@
 {{nc.name}}  - +
From 935da14f7d1ccc9d9223f5fee117ff09247f9999 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 15:27:02 +0100 Subject: [PATCH 16/21] tab/spaces formatting --- pkg/services/alerting/rule_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index 7e8af888338..f0c7ea0c9d6 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -77,13 +77,13 @@ func TestAlertRuleModel(t *testing.T) { "noDataMode": "critical", "enabled": true, "frequency": "60s", - "conditions": [ - { - "type": "test", - "prop": 123 + "conditions": [ + { + "type": "test", + "prop": 123 } - ], - "notifications": [ + ], + "notifications": [ {"id": 1}, {"uid": "notifier2"} ] @@ -123,8 +123,8 @@ func TestAlertRuleModel(t *testing.T) { "noDataMode": "critical", "enabled": true, "frequency": "0s", - "conditions": [ { "type": "test", "prop": 123 } ], - "notifications": [] + "conditions": [ { "type": "test", "prop": 123 } ], + "notifications": [] }` alertJSON, jsonErr := simplejson.NewJson([]byte(json)) @@ -153,13 +153,13 @@ func TestAlertRuleModel(t *testing.T) { "noDataMode": "critical", "enabled": true, "frequency": "60s", - "conditions": [ - { - "type": "test", - "prop": 123 + "conditions": [ + { + "type": "test", + "prop": 123 } - ], - "notifications": [ + ], + "notifications": [ {"not_id_uid": "1134"} ] } From 21fff415ed6dc405435e83c8a214926f61850af9 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 15:37:52 +0100 Subject: [PATCH 17/21] removes unnessecary db request --- pkg/services/alerting/extractor_test.go | 2 -- pkg/services/alerting/rule_test.go | 5 +---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/services/alerting/extractor_test.go b/pkg/services/alerting/extractor_test.go index 66adf951269..9c689fec921 100644 --- a/pkg/services/alerting/extractor_test.go +++ b/pkg/services/alerting/extractor_test.go @@ -200,8 +200,6 @@ func TestAlertRuleExtraction(t *testing.T) { Convey("Alert notifications are in DB", func() { sqlstore.InitTestDB(t) - err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."}) - So(err, ShouldBeNil) firstNotification := m.CreateAlertNotificationCommand{Uid: "notifier1", OrgId: 1, Name: "1"} err = sqlstore.CreateAlertNotificationCommand(&firstNotification) So(err, ShouldBeNil) diff --git a/pkg/services/alerting/rule_test.go b/pkg/services/alerting/rule_test.go index f0c7ea0c9d6..f4172a57e74 100644 --- a/pkg/services/alerting/rule_test.go +++ b/pkg/services/alerting/rule_test.go @@ -59,10 +59,8 @@ func TestAlertRuleModel(t *testing.T) { }) Convey("can construct alert rule model", func() { - err := sqlstore.CreateOrg(&m.CreateOrgCommand{Name: "Main Org."}) - So(err, ShouldBeNil) firstNotification := m.CreateAlertNotificationCommand{OrgId: 1, Name: "1"} - err = sqlstore.CreateAlertNotificationCommand(&firstNotification) + err := sqlstore.CreateAlertNotificationCommand(&firstNotification) So(err, ShouldBeNil) secondNotification := m.CreateAlertNotificationCommand{Uid: "notifier2", OrgId: 1, Name: "2"} err = sqlstore.CreateAlertNotificationCommand(&secondNotification) @@ -182,6 +180,5 @@ func TestAlertRuleModel(t *testing.T) { So(err, ShouldNotBeNil) So(err.Error(), ShouldEqual, "Alert validation error: Neither id nor uid is specified, type assertion to string failed AlertId: 1 PanelId: 1 DashboardId: 1") }) - }) } From 8f0e65a150f71e187aaf8d51dd7d4d99b54b8897 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 20:39:09 +0100 Subject: [PATCH 18/21] renames alert_notifications -> notifiers --- .../{alert_notifications => notifiers}/sample.yaml | 4 ++-- docs/sources/administration/provisioning.md | 10 +++++----- .../alert_notifications.go | 7 +++++-- .../config_reader.go | 9 +++++++-- .../config_reader_test.go | 9 +++++++-- .../test-configs/broken-yaml/broken.yaml | 2 +- .../test-configs/broken-yaml/not.yaml.text | 0 .../correct-properties-with-orgName.yaml | 4 ++-- .../correct-properties/correct-properties.yaml | 4 ++-- .../test-configs/double-default/default-1.yml | 2 +- .../test-configs/double-default/default-2.yaml | 2 +- .../test-configs/empty/empty.yaml | 0 .../test-configs/empty_folder/.gitignore | 0 .../incorrect-settings/incorrect-settings.yaml | 2 +- .../no-required-fields/no-required-fields.yaml | 4 ++-- .../two-notifications/two-notifications.yaml | 2 +- .../test-configs/unknown-notifier/notification.yaml | 2 +- .../{alert_notifications => notifiers}/types.go | 6 +++--- pkg/services/provisioning/provisioning.go | 6 +++--- 19 files changed, 44 insertions(+), 31 deletions(-) rename conf/provisioning/{alert_notifications => notifiers}/sample.yaml (90%) rename pkg/services/provisioning/{alert_notifications => notifiers}/alert_notifications.go (98%) rename pkg/services/provisioning/{alert_notifications => notifiers}/config_reader.go (99%) rename pkg/services/provisioning/{alert_notifications => notifiers}/config_reader_test.go (99%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/broken-yaml/broken.yaml (88%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/broken-yaml/not.yaml.text (100%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml (83%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/correct-properties/correct-properties.yaml (95%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/double-default/default-1.yml (85%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/double-default/default-2.yaml (86%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/empty/empty.yaml (100%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/empty_folder/.gitignore (100%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/incorrect-settings/incorrect-settings.yaml (90%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/no-required-fields/no-required-fields.yaml (92%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/two-notifications/two-notifications.yaml (90%) rename pkg/services/provisioning/{alert_notifications => notifiers}/test-configs/unknown-notifier/notification.yaml (76%) rename pkg/services/provisioning/{alert_notifications => notifiers}/types.go (84%) diff --git a/conf/provisioning/alert_notifications/sample.yaml b/conf/provisioning/notifiers/sample.yaml similarity index 90% rename from conf/provisioning/alert_notifications/sample.yaml rename to conf/provisioning/notifiers/sample.yaml index d08df2545e9..7d909839412 100644 --- a/conf/provisioning/alert_notifications/sample.yaml +++ b/conf/provisioning/notifiers/sample.yaml @@ -1,7 +1,7 @@ # # config file version apiVersion: 1 -# alert_notifications: +# notifiers: # - name: default-slack-temp # type: slack # org_name: Main Org. @@ -19,7 +19,7 @@ apiVersion: 1 # is_default: false # settings: # addresses: example11111@example.com -# delete_alert_notifications: +# delete_notifiers: # - 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 9d3fd9385d4..3c94cb79f21 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -234,11 +234,11 @@ By default Grafana will delete dashboards in the database if the file is removed ## Alert Notification Channels -Alert Notification Channels can be provisionned by adding one or more yaml config files in the [`provisioning/alert_notifications`](/installation/configuration/#provisioning) directory. +Alert Notification Channels can be provisionned by adding one or more yaml config files in the [`provisioning/notifiers`](/installation/configuration/#provisioning) directory. Each config file can contain the following top-level fields: -- `alert_notifications`, a list of alert notifications that will be added or updated during start up. If the notification channel already exists, Grafana will update it to match the configuration file. -- `delete_alert_notifications`, a list of alert notifications to be deleted before before inserting/updating those in the `alert_notifications` list. +- `notifiers`, a list of alert notifications that will be added or updated during start up. If the notification channel already exists, Grafana will update it to match the configuration file. +- `delete_notifiers`, a list of alert notifications to be deleted before before inserting/updating those in the `notifiers` list. Provisionning looks up alert notifications by name, and will update any existing notification with the provided name. @@ -264,7 +264,7 @@ By default, exporting a dashboard as JSON will use a sequential identifier to re ### Example Alert Notification Channels Config File ```yaml -alert_notifications: +notifiers: - name: notification-channel-1 type: slack uid: notifier1 @@ -281,7 +281,7 @@ alert_notifications: uploadImage: true url: https://slack.com -delete_alert_notifications: +delete_notifiers: - name: notification-channel-1 uid: notifier1 # either diff --git a/pkg/services/provisioning/alert_notifications/alert_notifications.go b/pkg/services/provisioning/notifiers/alert_notifications.go similarity index 98% rename from pkg/services/provisioning/alert_notifications/alert_notifications.go rename to pkg/services/provisioning/notifiers/alert_notifications.go index 0f155dda4a8..514f11379c8 100644 --- a/pkg/services/provisioning/alert_notifications/alert_notifications.go +++ b/pkg/services/provisioning/notifiers/alert_notifications.go @@ -1,4 +1,4 @@ -package alert_notifications +package notifiers import ( "errors" @@ -13,7 +13,7 @@ var ( ) func Provision(configDirectory string) error { - dc := newNotificationProvisioner(log.New("provisioning.alert_notifications")) + dc := newNotificationProvisioner(log.New("provisioning.notifiers")) return dc.applyChanges(configDirectory) } @@ -54,6 +54,7 @@ func (dc *NotificationProvisioner) deleteNotifications(notificationToDelete []*d } else if notification.OrgId < 0 { notification.OrgId = 1 } + getNotification := &models.GetAlertNotificationsWithUidQuery{Uid: notification.Uid, OrgId: notification.OrgId} if err := bus.Dispatch(getNotification); err != nil { @@ -103,6 +104,7 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not Frequency: notification.Frequency, SendReminder: notification.SendReminder, } + if err := bus.Dispatch(insertCmd); err != nil { return err } @@ -119,6 +121,7 @@ func (dc *NotificationProvisioner) mergeNotifications(notificationToMerge []*not Frequency: notification.Frequency, SendReminder: notification.SendReminder, } + if err := bus.Dispatch(updateCmd); err != nil { return err } diff --git a/pkg/services/provisioning/alert_notifications/config_reader.go b/pkg/services/provisioning/notifiers/config_reader.go similarity index 99% rename from pkg/services/provisioning/alert_notifications/config_reader.go rename to pkg/services/provisioning/notifiers/config_reader.go index 56bfdab2f19..e712e8e3eff 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader.go +++ b/pkg/services/provisioning/notifiers/config_reader.go @@ -1,4 +1,4 @@ -package alert_notifications +package notifiers import ( "fmt" @@ -94,8 +94,8 @@ func checkOrgIdAndOrgName(notifications []*notificationsAsConfig) { } } } - } + func validateRequiredField(notifications []*notificationsAsConfig) error { for i := range notifications { var errStrings []string @@ -106,6 +106,7 @@ func validateRequiredField(notifications []*notificationsAsConfig) error { fmt.Sprintf("Added alert notification item %d in configuration doesn't contain required field name", index+1), ) } + if notification.Uid == "" { errStrings = append( errStrings, @@ -121,6 +122,7 @@ func validateRequiredField(notifications []*notificationsAsConfig) error { fmt.Sprintf("Deleted alert notification item %d in configuration doesn't contain required field name", index+1), ) } + if notification.Uid == "" { errStrings = append( errStrings, @@ -128,10 +130,12 @@ func validateRequiredField(notifications []*notificationsAsConfig) error { ) } } + if len(errStrings) != 0 { return fmt.Errorf(strings.Join(errStrings, "\n")) } } + return nil } @@ -148,6 +152,7 @@ func validateNotifications(notifications []*notificationsAsConfig) error { Settings: notification.SettingsToJson(), Type: notification.Type, }) + if err != nil { return err } diff --git a/pkg/services/provisioning/alert_notifications/config_reader_test.go b/pkg/services/provisioning/notifiers/config_reader_test.go similarity index 99% rename from pkg/services/provisioning/alert_notifications/config_reader_test.go rename to pkg/services/provisioning/notifiers/config_reader_test.go index d66dded8407..18074d7614e 100644 --- a/pkg/services/provisioning/alert_notifications/config_reader_test.go +++ b/pkg/services/provisioning/notifiers/config_reader_test.go @@ -1,4 +1,4 @@ -package alert_notifications +package notifiers import ( "testing" @@ -35,11 +35,13 @@ func TestNotificationAsConfig(t *testing.T) { Name: "slack", Factory: notifiers.NewSlackNotifier, }) + alerting.RegisterNotifier(&alerting.NotifierPlugin{ Type: "email", Name: "email", Factory: notifiers.NewEmailNotifier, }) + Convey("Can read correct properties", func() { cfgProvifer := &configReader{log: log.New("test logger")} cfg, err := cfgProvifer.readConfig(correct_properties) @@ -264,6 +266,7 @@ func TestNotificationAsConfig(t *testing.T) { 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) @@ -277,11 +280,13 @@ func TestNotificationAsConfig(t *testing.T) { So(notificationsQuery.Result, ShouldBeEmpty) }) }) + Convey("Broken yaml should return error", func() { reader := &configReader{log: log.New("test logger")} _, err := reader.readConfig(brokenYaml) So(err, ShouldNotBeNil) }) + Convey("Skip invalid directory", func() { cfgProvifer := &configReader{log: log.New("test logger")} cfg, err := cfgProvifer.readConfig(emptyFolder) @@ -290,6 +295,7 @@ func TestNotificationAsConfig(t *testing.T) { } So(len(cfg), ShouldEqual, 0) }) + Convey("Unknown notifier should return error", func() { cfgProvifer := &configReader{log: log.New("test logger")} _, err := cfgProvifer.readConfig(unknownNotifier) @@ -303,6 +309,5 @@ func TestNotificationAsConfig(t *testing.T) { So(err, ShouldNotBeNil) So(err.Error(), ShouldEqual, "Alert validation error: Could not find url property in settings") }) - }) } diff --git a/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml b/pkg/services/provisioning/notifiers/test-configs/broken-yaml/broken.yaml similarity index 88% rename from pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml rename to pkg/services/provisioning/notifiers/test-configs/broken-yaml/broken.yaml index e7c38d22f2c..72f2fbdbf63 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/broken.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/broken-yaml/broken.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: notification-channel-1 type: slack org_id: 2 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/not.yaml.text b/pkg/services/provisioning/notifiers/test-configs/broken-yaml/not.yaml.text similarity index 100% rename from pkg/services/provisioning/alert_notifications/test-configs/broken-yaml/not.yaml.text rename to pkg/services/provisioning/notifiers/test-configs/broken-yaml/not.yaml.text diff --git a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml b/pkg/services/provisioning/notifiers/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml similarity index 83% rename from pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml rename to pkg/services/provisioning/notifiers/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml index 214396982ea..25c4536d1f3 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: default-notification-create type: email uid: notifier2 @@ -6,7 +6,7 @@ alert_notifications: addresses: example@example.com org_name: Main Org. 2 is_default: false -delete_alert_notifications: +delete_notifiers: - 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/notifiers/test-configs/correct-properties/correct-properties.yaml similarity index 95% rename from pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml rename to pkg/services/provisioning/notifiers/test-configs/correct-properties/correct-properties.yaml index 4f4cd171852..af0736f35a4 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/correct-properties/correct-properties.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/correct-properties/correct-properties.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: default-slack-notification type: slack uid: notifier1 @@ -29,7 +29,7 @@ alert_notifications: uid: "notifier4" settings: addresses: example@exmaple.com -delete_alert_notifications: +delete_notifiers: - name: default-slack-notification org_id: 2 uid: notifier1 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml b/pkg/services/provisioning/notifiers/test-configs/double-default/default-1.yml similarity index 85% rename from pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml rename to pkg/services/provisioning/notifiers/test-configs/double-default/default-1.yml index d3ae32c5ae7..d9d2fe66081 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-1.yml +++ b/pkg/services/provisioning/notifiers/test-configs/double-default/default-1.yml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: first-default type: slack uid: notifier1 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml b/pkg/services/provisioning/notifiers/test-configs/double-default/default-2.yaml similarity index 86% rename from pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml rename to pkg/services/provisioning/notifiers/test-configs/double-default/default-2.yaml index 3c2e8953f6b..878f8b48aa5 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/double-default/default-2.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/double-default/default-2.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: second-default type: email uid: notifier2 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/empty/empty.yaml b/pkg/services/provisioning/notifiers/test-configs/empty/empty.yaml similarity index 100% rename from pkg/services/provisioning/alert_notifications/test-configs/empty/empty.yaml rename to pkg/services/provisioning/notifiers/test-configs/empty/empty.yaml diff --git a/pkg/services/provisioning/alert_notifications/test-configs/empty_folder/.gitignore b/pkg/services/provisioning/notifiers/test-configs/empty_folder/.gitignore similarity index 100% rename from pkg/services/provisioning/alert_notifications/test-configs/empty_folder/.gitignore rename to pkg/services/provisioning/notifiers/test-configs/empty_folder/.gitignore diff --git a/pkg/services/provisioning/alert_notifications/test-configs/incorrect-settings/incorrect-settings.yaml b/pkg/services/provisioning/notifiers/test-configs/incorrect-settings/incorrect-settings.yaml similarity index 90% rename from pkg/services/provisioning/alert_notifications/test-configs/incorrect-settings/incorrect-settings.yaml rename to pkg/services/provisioning/notifiers/test-configs/incorrect-settings/incorrect-settings.yaml index 2d720d9d2d9..b7ecfbdf012 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/incorrect-settings/incorrect-settings.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/incorrect-settings/incorrect-settings.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: slack-notification-without-url-in-settings type: slack org_id: 2 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml b/pkg/services/provisioning/notifiers/test-configs/no-required-fields/no-required-fields.yaml similarity index 92% rename from pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml rename to pkg/services/provisioning/notifiers/test-configs/no-required-fields/no-required-fields.yaml index 582abefe14d..55ff545525e 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/no-required-fields/no-required-fields.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/no-required-fields/no-required-fields.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - type: slack org_id: 2 uid: no-name_added-notification @@ -15,7 +15,7 @@ alert_notifications: recipient: "XXX" token: "xoxb" uploadImage: true -delete_alert_notifications: +delete_notifiers: - name: no-uid type: slack org_id: 2 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml b/pkg/services/provisioning/notifiers/test-configs/two-notifications/two-notifications.yaml similarity index 90% rename from pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml rename to pkg/services/provisioning/notifiers/test-configs/two-notifications/two-notifications.yaml index 23fff0aff23..aeeb718e6de 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/two-notifications/two-notifications.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/two-notifications/two-notifications.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: channel1 type: email uid: notifier1 diff --git a/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml b/pkg/services/provisioning/notifiers/test-configs/unknown-notifier/notification.yaml similarity index 76% rename from pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml rename to pkg/services/provisioning/notifiers/test-configs/unknown-notifier/notification.yaml index e46db7b8b6e..ca0d3fa3c75 100644 --- a/pkg/services/provisioning/alert_notifications/test-configs/unknown-notifier/notification.yaml +++ b/pkg/services/provisioning/notifiers/test-configs/unknown-notifier/notification.yaml @@ -1,4 +1,4 @@ -alert_notifications: +notifiers: - name: unknown-notifier type: nonexisting uid: notifier1 \ No newline at end of file diff --git a/pkg/services/provisioning/alert_notifications/types.go b/pkg/services/provisioning/notifiers/types.go similarity index 84% rename from pkg/services/provisioning/alert_notifications/types.go rename to pkg/services/provisioning/notifiers/types.go index d3a858ae956..f788da79c79 100644 --- a/pkg/services/provisioning/alert_notifications/types.go +++ b/pkg/services/provisioning/notifiers/types.go @@ -1,10 +1,10 @@ -package alert_notifications +package notifiers import "github.com/grafana/grafana/pkg/components/simplejson" type notificationsAsConfig struct { - Notifications []*notificationFromConfig `json:"alert_notifications" yaml:"alert_notifications"` - DeleteNotifications []*deleteNotificationConfig `json:"delete_alert_notifications" yaml:"delete_alert_notifications"` + Notifications []*notificationFromConfig `json:"notifiers" yaml:"notifiers"` + DeleteNotifications []*deleteNotificationConfig `json:"delete_notifiers" yaml:"delete_notifiers"` } type deleteNotificationConfig struct { diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index fdecd03a3da..eb19cb2c5e8 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -6,7 +6,7 @@ import ( "path" "github.com/grafana/grafana/pkg/registry" - "github.com/grafana/grafana/pkg/services/provisioning/alert_notifications" + "github.com/grafana/grafana/pkg/services/provisioning/notifiers" "github.com/grafana/grafana/pkg/services/provisioning/dashboards" "github.com/grafana/grafana/pkg/services/provisioning/datasources" "github.com/grafana/grafana/pkg/setting" @@ -26,8 +26,8 @@ func (ps *ProvisioningService) Init() error { return fmt.Errorf("Datasource provisioning error: %v", err) } - alertNotificationsPath := path.Join(ps.Cfg.ProvisioningPath, "alert_notifications") - if err := alert_notifications.Provision(alertNotificationsPath); err != nil { + alertNotificationsPath := path.Join(ps.Cfg.ProvisioningPath, "notifiers") + if err := notifiers.Provision(alertNotificationsPath); err != nil { return fmt.Errorf("Alert notification provisioning error: %v", err) } From 809019d4eeaf490dd905e601d2171c89a0b056d9 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 20:43:53 +0100 Subject: [PATCH 19/21] moves test files into testdata folder --- .../notifiers/config_reader_test.go | 24 +++++++++---------- .../test-configs/broken-yaml/broken.yaml | 0 .../test-configs/broken-yaml/not.yaml.text | 0 .../correct-properties-with-orgName.yaml | 0 .../correct-properties.yaml | 0 .../test-configs/double-default/default-1.yml | 0 .../double-default/default-2.yaml | 0 .../test-configs/empty/empty.yaml | 0 .../test-configs/empty_folder/.gitignore | 0 .../incorrect-settings.yaml | 0 .../no-required-fields.yaml | 0 .../two-notifications/two-notifications.yaml | 0 .../unknown-notifier/notification.yaml | 0 13 files changed, 12 insertions(+), 12 deletions(-) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/broken-yaml/broken.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/broken-yaml/not.yaml.text (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/correct-properties/correct-properties.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/double-default/default-1.yml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/double-default/default-2.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/empty/empty.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/empty_folder/.gitignore (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/incorrect-settings/incorrect-settings.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/no-required-fields/no-required-fields.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/two-notifications/two-notifications.yaml (100%) rename pkg/services/provisioning/notifiers/{ => testdata}/test-configs/unknown-notifier/notification.yaml (100%) diff --git a/pkg/services/provisioning/notifiers/config_reader_test.go b/pkg/services/provisioning/notifiers/config_reader_test.go index 18074d7614e..87645ee7d31 100644 --- a/pkg/services/provisioning/notifiers/config_reader_test.go +++ b/pkg/services/provisioning/notifiers/config_reader_test.go @@ -12,21 +12,21 @@ import ( ) var ( - logger = log.New("fake.log") - - correct_properties = "./test-configs/correct-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" - emptyFolder = "./test-configs/empty_folder" - emptyFile = "./test-configs/empty" - twoNotificationsConfig = "./test-configs/two-notifications" - unknownNotifier = "./test-configs/unknown-notifier" + correct_properties = "./testdata/test-configs/correct-properties" + incorrect_settings = "./testdata/test-configs/incorrect-settings" + no_required_fields = "./testdata/test-configs/no-required-fields" + correct_properties_with_orgName = "./testdata/test-configs/correct-properties-with-orgName" + brokenYaml = "./testdata/test-configs/broken-yaml" + doubleNotificationsConfig = "./testdata/test-configs/double-default" + emptyFolder = "./testdata/test-configs/empty_folder" + emptyFile = "./testdata/test-configs/empty" + twoNotificationsConfig = "./testdata/test-configs/two-notifications" + unknownNotifier = "./testdata/test-configs/unknown-notifier" ) func TestNotificationAsConfig(t *testing.T) { + logger := log.New("fake.log") + Convey("Testing notification as configuration", t, func() { sqlstore.InitTestDB(t) diff --git a/pkg/services/provisioning/notifiers/test-configs/broken-yaml/broken.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/broken-yaml/broken.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/broken-yaml/broken.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/broken-yaml/broken.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/broken-yaml/not.yaml.text b/pkg/services/provisioning/notifiers/testdata/test-configs/broken-yaml/not.yaml.text similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/broken-yaml/not.yaml.text rename to pkg/services/provisioning/notifiers/testdata/test-configs/broken-yaml/not.yaml.text diff --git a/pkg/services/provisioning/notifiers/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/correct-properties-with-orgName/correct-properties-with-orgName.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/correct-properties/correct-properties.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/correct-properties/correct-properties.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/correct-properties/correct-properties.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/correct-properties/correct-properties.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/double-default/default-1.yml b/pkg/services/provisioning/notifiers/testdata/test-configs/double-default/default-1.yml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/double-default/default-1.yml rename to pkg/services/provisioning/notifiers/testdata/test-configs/double-default/default-1.yml diff --git a/pkg/services/provisioning/notifiers/test-configs/double-default/default-2.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/double-default/default-2.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/double-default/default-2.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/double-default/default-2.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/empty/empty.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/empty/empty.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/empty/empty.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/empty/empty.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/empty_folder/.gitignore b/pkg/services/provisioning/notifiers/testdata/test-configs/empty_folder/.gitignore similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/empty_folder/.gitignore rename to pkg/services/provisioning/notifiers/testdata/test-configs/empty_folder/.gitignore diff --git a/pkg/services/provisioning/notifiers/test-configs/incorrect-settings/incorrect-settings.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/incorrect-settings/incorrect-settings.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/incorrect-settings/incorrect-settings.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/incorrect-settings/incorrect-settings.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/no-required-fields/no-required-fields.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/no-required-fields/no-required-fields.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/no-required-fields/no-required-fields.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/no-required-fields/no-required-fields.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/two-notifications/two-notifications.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/two-notifications/two-notifications.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/two-notifications/two-notifications.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/two-notifications/two-notifications.yaml diff --git a/pkg/services/provisioning/notifiers/test-configs/unknown-notifier/notification.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/unknown-notifier/notification.yaml similarity index 100% rename from pkg/services/provisioning/notifiers/test-configs/unknown-notifier/notification.yaml rename to pkg/services/provisioning/notifiers/testdata/test-configs/unknown-notifier/notification.yaml From 7c93335d28d6ad3c6e06d6340ef51cf7d52f5ff3 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 21:04:08 +0100 Subject: [PATCH 20/21] gofmt issue --- pkg/services/provisioning/provisioning.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index eb19cb2c5e8..45f0972b885 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -6,9 +6,9 @@ import ( "path" "github.com/grafana/grafana/pkg/registry" - "github.com/grafana/grafana/pkg/services/provisioning/notifiers" "github.com/grafana/grafana/pkg/services/provisioning/dashboards" "github.com/grafana/grafana/pkg/services/provisioning/datasources" + "github.com/grafana/grafana/pkg/services/provisioning/notifiers" "github.com/grafana/grafana/pkg/setting" ) From e218cc7637e78f8ebb5767a3051517824f9730ab Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 28 Jan 2019 22:03:16 +0100 Subject: [PATCH 21/21] docs: updates docs to refer to using uid --- docs/sources/administration/provisioning.md | 6 +++--- pkg/services/sqlstore/alert_notification.go | 2 ++ pkg/services/sqlstore/migrations/alert_mig.go | 3 +++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index 3c94cb79f21..cce25e4cf2b 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -234,15 +234,15 @@ By default Grafana will delete dashboards in the database if the file is removed ## Alert Notification Channels -Alert Notification Channels can be provisionned by adding one or more yaml config files in the [`provisioning/notifiers`](/installation/configuration/#provisioning) directory. +Alert Notification Channels can be provisioned by adding one or more yaml config files in the [`provisioning/notifiers`](/installation/configuration/#provisioning) directory. Each config file can contain the following top-level fields: - `notifiers`, a list of alert notifications that will be added or updated during start up. If the notification channel already exists, Grafana will update it to match the configuration file. - `delete_notifiers`, a list of alert notifications to be deleted before before inserting/updating those in the `notifiers` list. -Provisionning looks up alert notifications by name, and will update any existing notification with the provided name. +Provisioning looks up alert notifications by uid, and will update any existing notification with the provided uid. -By default, exporting a dashboard as JSON will use a sequential identifier to refer to alert notifications. The field `name` can be optionally specified to specify a string identifier for the alert name. +By default, exporting a dashboard as JSON will use a sequential identifier to refer to alert notifications. The field `uid` can be optionally specified to specify a string identifier for the alert name. ```json { diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 9231c896cf1..efb5f621392 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -281,10 +281,12 @@ func generateNewAlertNotificationUid(sess *DBSession, orgId int64) (string, erro if err != nil { return "", err } + if !exists { return uid, nil } } + return "", m.ErrAlertNotificationFailedGenerateUniqueUid } diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index 2ed9732687a..84014bd386f 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -141,13 +141,16 @@ func addAlertMigrations(mg *Migrator) { 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, }))