From 6705efef6fed2b0795af80502e43648e47d2357e Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 16 Jun 2016 16:18:40 +0200 Subject: [PATCH] feat(alerting): make some settings properties required --- pkg/services/alerting/notifier.go | 33 ++++++-- pkg/services/alerting/notifier_test.go | 109 ++++++++++++++++--------- 2 files changed, 97 insertions(+), 45 deletions(-) diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 6e31fed7db3..661f761b6ff 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -1,6 +1,8 @@ package alerting import ( + "fmt" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/log" @@ -108,27 +110,44 @@ func (n *NotifierImpl) getNotifiers(orgId int64, notificationGroups []int64) []* } func NewNotificationFromDBModel(model *m.AlertNotification) (*Notification, error) { + notifier, err := createNotifier(model.Type, model.Settings) + + if err != nil { + return nil, err + } + return &Notification{ Name: model.Name, Type: model.Type, - Notifierr: createNotifier(model.Type, model.Settings), + Notifierr: notifier, SendCritical: !model.Settings.Get("ignoreCrit").MustBool(), SendWarning: !model.Settings.Get("ignoreWarn").MustBool(), }, nil } -var createNotifier = func(notificationType string, settings *simplejson.Json) NotificationDispatcher { +var createNotifier = func(notificationType string, settings *simplejson.Json) (NotificationDispatcher, error) { if notificationType == "email" { - return &EmailNotifier{ - To: settings.Get("to").MustString(), - log: log.New("alerting.notification.email"), + to := settings.Get("to").MustString() + + if to == "" { + return nil, fmt.Errorf("Could not find to propertie in settings") } + + return &EmailNotifier{ + To: to, + log: log.New("alerting.notification.email"), + }, nil + } + + url := settings.Get("url").MustString() + if url == "" { + return nil, fmt.Errorf("Could not find url propertie in settings") } return &WebhookNotifier{ - Url: settings.Get("url").MustString(), + Url: url, User: settings.Get("user").MustString(), Password: settings.Get("password").MustString(), log: log.New("alerting.notification.webhook"), - } + }, nil } diff --git a/pkg/services/alerting/notifier_test.go b/pkg/services/alerting/notifier_test.go index 4249238cfa0..a7d720d01d9 100644 --- a/pkg/services/alerting/notifier_test.go +++ b/pkg/services/alerting/notifier_test.go @@ -13,54 +13,87 @@ import ( func TestAlertNotificationExtraction(t *testing.T) { Convey("Parsing alert notification from settings", t, func() { - Convey("Parsing email notification from settings", func() { - json := ` - { - "to": "ops@grafana.org" - }` + Convey("Parsing email", func() { + Convey("empty settings should return error", func() { + json := `{ }` - settingsJSON, _ := simplejson.NewJson([]byte(json)) - model := &m.AlertNotification{ - Name: "ops", - Type: "email", - Settings: settingsJSON, - } + settingsJSON, _ := simplejson.NewJson([]byte(json)) + model := &m.AlertNotification{ + Name: "ops", + Type: "email", + Settings: settingsJSON, + } - not, err := NewNotificationFromDBModel(model) + _, err := NewNotificationFromDBModel(model) + So(err, ShouldNotBeNil) + }) - So(err, ShouldBeNil) - So(not.Name, ShouldEqual, "ops") - So(not.Type, ShouldEqual, "email") - So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.EmailNotifier") + Convey("from settings", func() { + json := ` + { + "to": "ops@grafana.org" + }` - email := not.Notifierr.(*EmailNotifier) - So(email.To, ShouldEqual, "ops@grafana.org") + settingsJSON, _ := simplejson.NewJson([]byte(json)) + model := &m.AlertNotification{ + Name: "ops", + Type: "email", + Settings: settingsJSON, + } + + not, err := NewNotificationFromDBModel(model) + + So(err, ShouldBeNil) + So(not.Name, ShouldEqual, "ops") + So(not.Type, ShouldEqual, "email") + So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.EmailNotifier") + + email := not.Notifierr.(*EmailNotifier) + So(email.To, ShouldEqual, "ops@grafana.org") + }) }) - Convey("Parsing webhook notification from settings", func() { - json := ` - { - "url": "http://localhost:3000", - "username": "username", - "password": "password" - }` + Convey("Parsing webhook", func() { + Convey("empty settings should return error", func() { + json := `{ }` - settingsJSON, _ := simplejson.NewJson([]byte(json)) - model := &m.AlertNotification{ - Name: "slack", - Type: "webhook", - Settings: settingsJSON, - } + settingsJSON, _ := simplejson.NewJson([]byte(json)) + model := &m.AlertNotification{ + Name: "ops", + Type: "webhook", + Settings: settingsJSON, + } - not, err := NewNotificationFromDBModel(model) + _, err := NewNotificationFromDBModel(model) + So(err, ShouldNotBeNil) + }) - So(err, ShouldBeNil) - So(not.Name, ShouldEqual, "slack") - So(not.Type, ShouldEqual, "webhook") - So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.WebhookNotifier") + Convey("from settings", func() { + json := ` + { + "url": "http://localhost:3000", + "username": "username", + "password": "password" + }` - webhook := not.Notifierr.(*WebhookNotifier) - So(webhook.Url, ShouldEqual, "http://localhost:3000") + settingsJSON, _ := simplejson.NewJson([]byte(json)) + model := &m.AlertNotification{ + Name: "slack", + Type: "webhook", + Settings: settingsJSON, + } + + not, err := NewNotificationFromDBModel(model) + + So(err, ShouldBeNil) + So(not.Name, ShouldEqual, "slack") + So(not.Type, ShouldEqual, "webhook") + So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.WebhookNotifier") + + webhook := not.Notifierr.(*WebhookNotifier) + So(webhook.Url, ShouldEqual, "http://localhost:3000") + }) }) + }) }