From 019834ae1212a38872a6a037cfb752f476083d89 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 14 Feb 2020 17:11:23 +0100 Subject: [PATCH] Alerting: Update the types of recipient supported by the Slack notifier (#22205) * Alerting: Update types of supported Slack recipients * Document Slack recipient requirements --- docs/sources/alerting/notifications.md | 2 +- pkg/services/alerting/notifiers/slack.go | 17 +++- pkg/services/alerting/notifiers/slack_test.go | 92 ++++++++++++++++++- 3 files changed, 101 insertions(+), 10 deletions(-) diff --git a/docs/sources/alerting/notifications.md b/docs/sources/alerting/notifications.md index a7f92a152c0..08779bc6aaf 100644 --- a/docs/sources/alerting/notifications.md +++ b/docs/sources/alerting/notifications.md @@ -85,7 +85,7 @@ Setting | Description ---------- | ----------- Url | Slack incoming webhook URL. Username | Set the username for the bot's message. -Recipient | Allows you to override the Slack recipient. +Recipient | Allows you to override the Slack recipient. You must either provide a channel Slack ID, a user Slack ID, a username reference (@<user>, all lowercase, no whitespace), or a channel reference (#<channel>, all lowercase, no whitespace). Icon emoji | Provide an emoji to use as the icon for the bot's message. Ex :smile: Icon URL | Provide a URL to an image to use as the icon for the bot's message. Mention Users | Optionally mention one or more users in the Slack notification sent by Grafana. You have to refer to users, comma-separated, via their corresponding Slack IDs (which you can find by clicking the overflow button on each user's Slack profile). diff --git a/pkg/services/alerting/notifiers/slack.go b/pkg/services/alerting/notifiers/slack.go index 199ac565d88..ebd621a981e 100644 --- a/pkg/services/alerting/notifiers/slack.go +++ b/pkg/services/alerting/notifiers/slack.go @@ -8,6 +8,7 @@ import ( "mime/multipart" "os" "path/filepath" + "regexp" "strings" "time" @@ -38,7 +39,7 @@ func init() { data-placement="right"> - Override default channel or user, use #channel-name or @username + Override default channel or user, use #channel-name, @username (has to be all lowercase, no whitespace), or user/channel Slack ID
@@ -118,14 +119,15 @@ func init() { data-placement="right"> - Provide a bot token to use the Slack file.upload API (starts with "xoxb"). Specify #channel-name or @username in Recipient for this to work + Provide a bot token to use the Slack file.upload API (starts with "xoxb"). Specify Recipient for this to work
`, }) - } +var reRecipient *regexp.Regexp = regexp.MustCompile("^((@[a-z0-9][a-zA-Z0-9._-]*)|(#[^ .A-Z]{1,79})|([a-zA-Z0-9]+))$") + // NewSlackNotifier is the constructor for the Slack notifier func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error) { url := model.Settings.Get("url").MustString() @@ -133,7 +135,10 @@ func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error return nil, alerting.ValidationError{Reason: "Could not find url property in settings"} } - recipient := model.Settings.Get("recipient").MustString() + recipient := strings.TrimSpace(model.Settings.Get("recipient").MustString()) + if recipient != "" && !reRecipient.MatchString(recipient) { + return nil, alerting.ValidationError{Reason: fmt.Sprintf("Recipient on invalid format: %q", recipient)} + } username := model.Settings.Get("username").MustString() iconEmoji := model.Settings.Get("icon_emoji").MustString() iconURL := model.Settings.Get("icon_url").MustString() @@ -144,7 +149,9 @@ func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error uploadImage := model.Settings.Get("uploadImage").MustBool(true) if mentionChannel != "" && mentionChannel != "here" && mentionChannel != "channel" { - return nil, fmt.Errorf(fmt.Sprintf("invalid value for mentionChannel: %q", mentionChannel)) + return nil, alerting.ValidationError{ + Reason: fmt.Sprintf("Invalid value for mentionChannel: %q", mentionChannel), + } } mentionUsers := []string{} for _, u := range strings.Split(mentionUsersStr, ",") { diff --git a/pkg/services/alerting/notifiers/slack_test.go b/pkg/services/alerting/notifiers/slack_test.go index e14469ed9d9..a27471cacc5 100644 --- a/pkg/services/alerting/notifiers/slack_test.go +++ b/pkg/services/alerting/notifiers/slack_test.go @@ -15,15 +15,16 @@ func TestSlackNotifier(t *testing.T) { Convey("empty settings should return error", func() { json := `{ }` - settingsJSON, _ := simplejson.NewJson([]byte(json)) + settingsJSON, err := simplejson.NewJson([]byte(json)) + So(err, ShouldBeNil) model := &models.AlertNotification{ Name: "ops", Type: "slack", Settings: settingsJSON, } - _, err := NewSlackNotifier(model) - So(err, ShouldNotBeNil) + _, err = NewSlackNotifier(model) + So(err, ShouldBeError, "Alert validation error: Could not find url property in settings") }) //nolint:goconst @@ -71,7 +72,8 @@ func TestSlackNotifier(t *testing.T) { "token": "xoxb-XXXXXXXX-XXXXXXXX-XXXXXXXXXX" }` - settingsJSON, _ := simplejson.NewJson([]byte(json)) + settingsJSON, err := simplejson.NewJson([]byte(json)) + So(err, ShouldBeNil) model := &models.AlertNotification{ Name: "ops", Type: "slack", @@ -94,6 +96,88 @@ func TestSlackNotifier(t *testing.T) { So(slackNotifier.MentionChannel, ShouldEqual, "here") So(slackNotifier.Token, ShouldEqual, "xoxb-XXXXXXXX-XXXXXXXX-XXXXXXXXXX") }) + + Convey("with channel recipient with spaces should return an error", func() { + json := ` + { + "url": "http://google.com", + "recipient": "#open tsdb" + }` + + settingsJSON, err := simplejson.NewJson([]byte(json)) + So(err, ShouldBeNil) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } + + _, err = NewSlackNotifier(model) + + So(err, ShouldBeError, "Alert validation error: Recipient on invalid format: \"#open tsdb\"") + }) + + Convey("with user recipient with spaces should return an error", func() { + json := ` + { + "url": "http://google.com", + "recipient": "@user name" + }` + + settingsJSON, err := simplejson.NewJson([]byte(json)) + So(err, ShouldBeNil) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } + + _, err = NewSlackNotifier(model) + + So(err, ShouldBeError, "Alert validation error: Recipient on invalid format: \"@user name\"") + }) + + Convey("with user recipient with uppercase letters should return an error", func() { + json := ` + { + "url": "http://google.com", + "recipient": "@User" + }` + + settingsJSON, err := simplejson.NewJson([]byte(json)) + So(err, ShouldBeNil) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } + + _, err = NewSlackNotifier(model) + + So(err, ShouldBeError, "Alert validation error: Recipient on invalid format: \"@User\"") + }) + + Convey("with Slack ID for recipient should work", func() { + json := ` + { + "url": "http://google.com", + "recipient": "1ABCDE" + }` + + settingsJSON, err := simplejson.NewJson([]byte(json)) + So(err, ShouldBeNil) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } + + not, err := NewSlackNotifier(model) + So(err, ShouldBeNil) + slackNotifier := not.(*SlackNotifier) + + So(slackNotifier.Recipient, ShouldEqual, "1ABCDE") + }) }) }) }