diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index c42e1f89c0c..99bceb0e9e1 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -431,6 +431,11 @@ func NotificationTest(c *models.ReqContext, dto dtos.NotificationTestCommand) re if errors.Is(err, models.ErrSmtpNotEnabled) { return response.Error(412, err.Error(), err) } + var alertingErr alerting.ValidationError + if errors.As(err, &alertingErr) { + return response.Error(400, err.Error(), err) + } + return response.Error(500, "Failed to send alert notifications", err) } diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 238ef466685..4df31e87045 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -294,7 +294,7 @@ type NotifierFactory func(notification *models.AlertNotification) (Notifier, err var notifierFactories = make(map[string]*NotifierPlugin) -// RegisterNotifier register an notifier +// RegisterNotifier registers a notifier. func RegisterNotifier(plugin *NotifierPlugin) { notifierFactories[plugin.Type] = plugin } diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 40107dd5425..42363a6bdcd 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -30,8 +30,7 @@ type NotifierBase struct { // NewNotifierBase returns a new `NotifierBase`. func NewNotifierBase(model *models.AlertNotification) NotifierBase { uploadImage := true - value, exist := model.Settings.CheckGet("uploadImage") - if exist { + if value, exists := model.Settings.CheckGet("uploadImage"); exists { uploadImage = value.MustBool() } diff --git a/pkg/services/alerting/notifiers/email.go b/pkg/services/alerting/notifiers/email.go index fadeaf0e3b0..bc5444fd739 100644 --- a/pkg/services/alerting/notifiers/email.go +++ b/pkg/services/alerting/notifiers/email.go @@ -117,9 +117,7 @@ func (en *EmailNotifier) Notify(evalContext *alerting.EvalContext) error { } } - err = bus.DispatchCtx(evalContext.Ctx, cmd) - - if err != nil { + if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { en.log.Error("Failed to send alert notification email", "error", err) return err } diff --git a/pkg/services/alerting/notifiers/slack.go b/pkg/services/alerting/notifiers/slack.go index 656e0aba05b..edb63c90b5f 100644 --- a/pkg/services/alerting/notifiers/slack.go +++ b/pkg/services/alerting/notifiers/slack.go @@ -2,11 +2,15 @@ package notifiers import ( "bytes" + "context" + "crypto/tls" "encoding/json" "fmt" "io" "mime/multipart" + "net" "net/http" + "net/url" "os" "path/filepath" "regexp" @@ -24,26 +28,29 @@ func init() { alerting.RegisterNotifier(&alerting.NotifierPlugin{ Type: "slack", Name: "Slack", - Description: "Sends notifications to Slack via Slack Webhooks", + Description: "Sends notifications to Slack", Heading: "Slack settings", Factory: NewSlackNotifier, Options: []alerting.NotifierOption{ - { - Label: "Url", - Element: alerting.ElementTypeInput, - InputType: alerting.InputTypeText, - Placeholder: "Slack incoming webhook url", - PropertyName: "url", - Required: true, - Secure: true, - }, { Label: "Recipient", Element: alerting.ElementTypeInput, InputType: alerting.InputTypeText, - Description: "Override default channel or user, use #channel-name, @username (has to be all lowercase, no whitespace), or user/channel Slack ID", + Description: "Specify channel or user, use #channel-name, @username (has to be all lowercase, no whitespace), or user/channel Slack ID - required unless you provide a webhook", PropertyName: "recipient", }, + // Logically, this field should be required when not using a webhook, since the Slack API needs a token. + // However, since the UI doesn't allow to say that a field is required or not depending on another field, + // we've gone with the compromise of making this field optional and instead return a validation error + // if it's necessary and missing. + { + Label: "Token", + Element: alerting.ElementTypeInput, + InputType: alerting.InputTypeText, + Description: "Provide a Slack API token (starts with \"xoxb\") - required unless you provide a webhook", + PropertyName: "token", + Secure: true, + }, { Label: "Username", Element: alerting.ElementTypeInput, @@ -100,11 +107,12 @@ func init() { PropertyName: "mentionChannel", }, { - Label: "Token", + Label: "Webhook URL", Element: alerting.ElementTypeInput, InputType: alerting.InputTypeText, - Description: "Provide a bot token to use the Slack file.upload API (starts with \"xoxb\"). Specify Recipient for this to work", - PropertyName: "token", + Description: "Optionally provide a Slack incoming webhook URL for sending messages, in this case the token isn't necessary", + Placeholder: "Slack incoming webhook URL", + PropertyName: "url", Secure: true, }, }, @@ -113,16 +121,28 @@ func init() { 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 +const slackAPIEndpoint = "https://slack.com/api/chat.postMessage" + +// NewSlackNotifier is the constructor for the Slack notifier. func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error) { - url := model.DecryptedValue("url", model.Settings.Get("url").MustString()) - if url == "" { - return nil, alerting.ValidationError{Reason: "Could not find url property in settings"} + urlStr := model.DecryptedValue("url", model.Settings.Get("url").MustString()) + if urlStr == "" { + urlStr = slackAPIEndpoint + } + apiURL, err := url.Parse(urlStr) + if err != nil { + return nil, fmt.Errorf("invalid URL %q: %w", urlStr, err) } 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)} + if recipient != "" { + if !reRecipient.MatchString(recipient) { + return nil, alerting.ValidationError{Reason: fmt.Sprintf("recipient on invalid format: %q", recipient)} + } + } else if apiURL.String() == slackAPIEndpoint { + return nil, alerting.ValidationError{ + Reason: "recipient must be specified when using the Slack chat API", + } } username := model.Settings.Get("username").MustString() iconEmoji := model.Settings.Get("icon_emoji").MustString() @@ -131,6 +151,11 @@ func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error mentionGroupsStr := model.Settings.Get("mentionGroups").MustString() mentionChannel := model.Settings.Get("mentionChannel").MustString() token := model.DecryptedValue("token", model.Settings.Get("token").MustString()) + if token == "" && apiURL.String() == slackAPIEndpoint { + return nil, alerting.ValidationError{ + Reason: "token must be specified when using the Slack chat API", + } + } uploadImage := model.Settings.Get("uploadImage").MustBool(true) @@ -155,17 +180,17 @@ func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error } return &SlackNotifier{ + url: apiURL, NotifierBase: NewNotifierBase(model), - URL: url, - Recipient: recipient, - Username: username, - IconEmoji: iconEmoji, - IconURL: iconURL, - MentionUsers: mentionUsers, - MentionGroups: mentionGroups, - MentionChannel: mentionChannel, - Token: token, - Upload: uploadImage, + recipient: recipient, + username: username, + iconEmoji: iconEmoji, + iconURL: iconURL, + mentionUsers: mentionUsers, + mentionGroups: mentionGroups, + mentionChannel: mentionChannel, + token: token, + upload: uploadImage, log: log.New("alerting.notifier.slack"), }, nil } @@ -174,26 +199,26 @@ func NewSlackNotifier(model *models.AlertNotification) (alerting.Notifier, error // alert notification to Slack. type SlackNotifier struct { NotifierBase - URL string - Recipient string - Username string - IconEmoji string - IconURL string - MentionUsers []string - MentionGroups []string - MentionChannel string - Token string - Upload bool + url *url.URL + recipient string + username string + iconEmoji string + iconURL string + mentionUsers []string + mentionGroups []string + mentionChannel string + token string + upload bool log log.Logger } -// Notify send alert notification to Slack. +// Notify sends an alert notification to Slack. func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { sn.log.Info("Executing slack notification", "ruleId", evalContext.Rule.ID, "notification", sn.Name) ruleURL, err := evalContext.GetRuleURL() if err != nil { - sn.log.Error("Failed get rule link", "error", err) + sn.log.Error("Failed to get rule link", "error", err) return err } @@ -220,19 +245,19 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { mentionsBuilder.WriteString(" ") } } - mentionChannel := strings.TrimSpace(sn.MentionChannel) + mentionChannel := strings.TrimSpace(sn.mentionChannel) if mentionChannel != "" { mentionsBuilder.WriteString(fmt.Sprintf("", mentionChannel, mentionChannel)) } - if len(sn.MentionGroups) > 0 { + if len(sn.mentionGroups) > 0 { appendSpace() - for _, g := range sn.MentionGroups { + for _, g := range sn.mentionGroups { mentionsBuilder.WriteString(fmt.Sprintf("", g)) } } - if len(sn.MentionUsers) > 0 { + if len(sn.mentionUsers) > 0 { appendSpace() - for _, u := range sn.MentionUsers { + for _, u := range sn.mentionUsers { mentionsBuilder.WriteString(fmt.Sprintf("<@%s>", u)) } } @@ -242,7 +267,7 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { } imageURL := "" // default to file.upload API method if a token is provided - if sn.Token == "" { + if sn.token == "" { imageURL = evalContext.ImagePublicURL } @@ -273,7 +298,8 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { attachment["image_url"] = imageURL } body := map[string]interface{}{ - "text": evalContext.GetNotificationTitle(), + "channel": sn.recipient, + "text": evalContext.GetNotificationTitle(), "attachments": []map[string]interface{}{ attachment, }, @@ -282,49 +308,103 @@ func (sn *SlackNotifier) Notify(evalContext *alerting.EvalContext) error { body["blocks"] = blocks } - // recipient override - if sn.Recipient != "" { - body["channel"] = sn.Recipient + if sn.username != "" { + body["username"] = sn.username } - if sn.Username != "" { - body["username"] = sn.Username + if sn.iconEmoji != "" { + body["icon_emoji"] = sn.iconEmoji } - if sn.IconEmoji != "" { - body["icon_emoji"] = sn.IconEmoji - } - if sn.IconURL != "" { - body["icon_url"] = sn.IconURL + if sn.iconURL != "" { + body["icon_url"] = sn.iconURL } data, err := json.Marshal(&body) if err != nil { return err } - cmd := &models.SendWebhookSync{ - Url: sn.URL, - Body: string(data), - HttpMethod: http.MethodPost, - } - if sn.Token != "" { - sn.log.Debug("Adding authorization header to HTTP request") - cmd.HttpHeader = map[string]string{ - "Authorization": fmt.Sprintf("Bearer %s", sn.Token), - } - } - if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { - sn.log.Error("Failed to send slack notification", "error", err, "webhook", sn.Name) + if err := sn.sendRequest(evalContext.Ctx, data); err != nil { return err } - if sn.Token != "" && sn.UploadImage { - err = sn.slackFileUpload(evalContext, sn.log, "https://slack.com/api/files.upload", sn.Recipient, sn.Token) + + if sn.token != "" && sn.UploadImage { + err := sn.slackFileUpload(evalContext, sn.log, sn.recipient, sn.token) if err != nil { return err } } + return nil } -func (sn *SlackNotifier) slackFileUpload(evalContext *alerting.EvalContext, log log.Logger, url string, recipient string, token string) error { +func (sn *SlackNotifier) sendRequest(ctx context.Context, data []byte) error { + sn.log.Debug("Sending Slack API request", "url", sn.url.String(), "data", string(data)) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, sn.url.String(), bytes.NewReader(data)) + if err != nil { + return fmt.Errorf("failed to create HTTP request: %w", err) + } + + request.Header.Set("Content-Type", "application/json") + request.Header.Set("User-Agent", "Grafana") + if sn.token == "" { + if sn.url.String() == slackAPIEndpoint { + panic("Token should be set when using the Slack chat API") + } + } else { + sn.log.Debug("Adding authorization header to HTTP request") + request.Header.Set("Authorization", fmt.Sprintf("Bearer %s", sn.token)) + } + + netTransport := &http.Transport{ + TLSClientConfig: &tls.Config{ + Renegotiation: tls.RenegotiateFreelyAsClient, + }, + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + }).DialContext, + TLSHandshakeTimeout: 5 * time.Second, + } + netClient := &http.Client{ + Timeout: time.Second * 30, + Transport: netTransport, + } + resp, err := netClient.Do(request) + if err != nil { + return err + } + defer func() { + if err := resp.Body.Close(); err != nil { + sn.log.Warn("Failed to close response body", "err", err) + } + }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode/100 == 2 { + var rslt map[string]interface{} + // Slack responds to some requests with a JSON document, that might contain an error + if err := json.Unmarshal(body, &rslt); err == nil { + if !rslt["ok"].(bool) { + errMsg := rslt["error"].(string) + sn.log.Warn("Sending Slack API request failed", "url", sn.url.String(), "statusCode", resp.Status, + "err", errMsg) + return fmt.Errorf("failed to make Slack API request: %s", errMsg) + } + } + + sn.log.Debug("Sending Slack API request succeeded", "url", sn.url.String(), "statusCode", resp.Status) + + return nil + } + + sn.log.Warn("Slack API request failed", "url", sn.url.String(), "statusCode", resp.Status, "body", string(body)) + return fmt.Errorf("request to Slack API failed with status code %d", resp.StatusCode) +} + +func (sn *SlackNotifier) slackFileUpload(evalContext *alerting.EvalContext, log log.Logger, recipient, token string) error { if evalContext.ImageOnDiskPath == "" { // nolint:gosec // We can ignore the gosec G304 warning on this one because `setting.HomePath` comes from Grafana's configuration file. @@ -335,7 +415,9 @@ func (sn *SlackNotifier) slackFileUpload(evalContext *alerting.EvalContext, log if err != nil { return err } - cmd := &models.SendWebhookSync{Url: url, Body: uploadBody.String(), HttpHeader: headers, HttpMethod: "POST"} + cmd := &models.SendWebhookSync{ + Url: "https://slack.com/api/files.upload", Body: uploadBody.String(), HttpHeader: headers, HttpMethod: "POST", + } if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { log.Error("Failed to upload slack image", "error", err, "webhook", "file.upload") return err diff --git a/pkg/services/alerting/notifiers/slack_test.go b/pkg/services/alerting/notifiers/slack_test.go index f4529185242..a232aac9c2b 100644 --- a/pkg/services/alerting/notifiers/slack_test.go +++ b/pkg/services/alerting/notifiers/slack_test.go @@ -6,60 +6,58 @@ import ( "github.com/grafana/grafana/pkg/components/securejsondata" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" - . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSlackNotifier(t *testing.T) { - Convey("Slack notifier tests", t, func() { - Convey("Parsing alert notification from settings", func() { - Convey("empty settings should return error", func() { - json := `{ }` + t.Run("empty settings should return error", func(t *testing.T) { + json := `{ }` - settingsJSON, err := simplejson.NewJson([]byte(json)) - So(err, ShouldBeNil) - model := &models.AlertNotification{ - Name: "ops", - Type: "slack", - Settings: settingsJSON, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - _, err = NewSlackNotifier(model) - So(err, ShouldBeError, "alert validation error: Could not find url property in settings") - }) + _, err = NewSlackNotifier(model) + assert.EqualError(t, err, "alert validation error: recipient must be specified when using the Slack chat API") + }) - //nolint:goconst - Convey("from settings", func() { - json := ` + t.Run("from settings", func(t *testing.T) { + json := ` { "url": "http://google.com" }` - settingsJSON, _ := simplejson.NewJson([]byte(json)) - model := &models.AlertNotification{ - Name: "ops", - Type: "slack", - Settings: settingsJSON, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - not, err := NewSlackNotifier(model) - slackNotifier := not.(*SlackNotifier) + not, err := NewSlackNotifier(model) + require.NoError(t, err) + slackNotifier := not.(*SlackNotifier) + assert.Equal(t, "ops", slackNotifier.Name) + assert.Equal(t, "slack", slackNotifier.Type) + assert.Equal(t, "http://google.com", slackNotifier.url.String()) + assert.Empty(t, slackNotifier.recipient) + assert.Empty(t, slackNotifier.username) + assert.Empty(t, slackNotifier.iconEmoji) + assert.Empty(t, slackNotifier.iconURL) + assert.Empty(t, slackNotifier.mentionUsers) + assert.Empty(t, slackNotifier.mentionGroups) + assert.Empty(t, slackNotifier.mentionChannel) + assert.Empty(t, slackNotifier.token) + }) - So(err, ShouldBeNil) - So(slackNotifier.Name, ShouldEqual, "ops") - So(slackNotifier.Type, ShouldEqual, "slack") - So(slackNotifier.URL, ShouldEqual, "http://google.com") - So(slackNotifier.Recipient, ShouldEqual, "") - So(slackNotifier.Username, ShouldEqual, "") - So(slackNotifier.IconEmoji, ShouldEqual, "") - So(slackNotifier.IconURL, ShouldEqual, "") - So(slackNotifier.MentionUsers, ShouldResemble, []string{}) - So(slackNotifier.MentionGroups, ShouldResemble, []string{}) - So(slackNotifier.MentionChannel, ShouldEqual, "") - So(slackNotifier.Token, ShouldEqual, "") - }) - - Convey("from settings with Recipient, Username, IconEmoji, IconUrl, MentionUsers, MentionGroups, MentionChannel, and Token", func() { - json := ` + t.Run("from settings with Recipient, Username, IconEmoji, IconUrl, MentionUsers, MentionGroups, MentionChannel, and Token", func(t *testing.T) { + json := ` { "url": "http://google.com", "recipient": "#ds-opentsdb", @@ -72,33 +70,32 @@ func TestSlackNotifier(t *testing.T) { "token": "xoxb-XXXXXXXX-XXXXXXXX-XXXXXXXXXX" }` - settingsJSON, err := simplejson.NewJson([]byte(json)) - So(err, ShouldBeNil) - model := &models.AlertNotification{ - Name: "ops", - Type: "slack", - Settings: settingsJSON, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - not, err := NewSlackNotifier(model) - slackNotifier := not.(*SlackNotifier) + not, err := NewSlackNotifier(model) + require.NoError(t, err) + slackNotifier := not.(*SlackNotifier) + assert.Equal(t, "ops", slackNotifier.Name) + assert.Equal(t, "slack", slackNotifier.Type) + assert.Equal(t, "http://google.com", slackNotifier.url.String()) + assert.Equal(t, "#ds-opentsdb", slackNotifier.recipient) + assert.Equal(t, "Grafana Alerts", slackNotifier.username) + assert.Equal(t, ":smile:", slackNotifier.iconEmoji) + assert.Equal(t, "https://grafana.com/img/fav32.png", slackNotifier.iconURL) + assert.Equal(t, []string{"user1", "user2"}, slackNotifier.mentionUsers) + assert.Equal(t, []string{"group1", "group2"}, slackNotifier.mentionGroups) + assert.Equal(t, "here", slackNotifier.mentionChannel) + assert.Equal(t, "xoxb-XXXXXXXX-XXXXXXXX-XXXXXXXXXX", slackNotifier.token) + }) - So(err, ShouldBeNil) - So(slackNotifier.Name, ShouldEqual, "ops") - So(slackNotifier.Type, ShouldEqual, "slack") - So(slackNotifier.URL, ShouldEqual, "http://google.com") - So(slackNotifier.Recipient, ShouldEqual, "#ds-opentsdb") - So(slackNotifier.Username, ShouldEqual, "Grafana Alerts") - So(slackNotifier.IconEmoji, ShouldEqual, ":smile:") - So(slackNotifier.IconURL, ShouldEqual, "https://grafana.com/img/fav32.png") - So(slackNotifier.MentionUsers, ShouldResemble, []string{"user1", "user2"}) - So(slackNotifier.MentionGroups, ShouldResemble, []string{"group1", "group2"}) - So(slackNotifier.MentionChannel, ShouldEqual, "here") - So(slackNotifier.Token, ShouldEqual, "xoxb-XXXXXXXX-XXXXXXXX-XXXXXXXXXX") - }) - - Convey("from settings with Recipient, Username, IconEmoji, IconUrl, MentionUsers, MentionGroups, MentionChannel, and Secured Token", func() { - json := ` + t.Run("from settings with Recipient, Username, IconEmoji, IconUrl, MentionUsers, MentionGroups, MentionChannel, and Secured Token", func(t *testing.T) { + json := ` { "url": "http://google.com", "recipient": "#ds-opentsdb", @@ -111,116 +108,109 @@ func TestSlackNotifier(t *testing.T) { "token": "uenc-XXXXXXXX-XXXXXXXX-XXXXXXXXXX" }` - settingsJSON, err := simplejson.NewJson([]byte(json)) - securedSettingsJSON := securejsondata.GetEncryptedJsonData(map[string]string{ - "token": "xenc-XXXXXXXX-XXXXXXXX-XXXXXXXXXX", - }) - So(err, ShouldBeNil) - model := &models.AlertNotification{ - Name: "ops", - Type: "slack", - Settings: settingsJSON, - SecureSettings: securedSettingsJSON, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + securedSettingsJSON := securejsondata.GetEncryptedJsonData(map[string]string{ + "token": "xenc-XXXXXXXX-XXXXXXXX-XXXXXXXXXX", + }) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + SecureSettings: securedSettingsJSON, + } - not, err := NewSlackNotifier(model) - slackNotifier := not.(*SlackNotifier) + not, err := NewSlackNotifier(model) + require.NoError(t, err) + slackNotifier := not.(*SlackNotifier) + assert.Equal(t, "ops", slackNotifier.Name) + assert.Equal(t, "slack", slackNotifier.Type) + assert.Equal(t, "http://google.com", slackNotifier.url.String()) + assert.Equal(t, "#ds-opentsdb", slackNotifier.recipient) + assert.Equal(t, "Grafana Alerts", slackNotifier.username) + assert.Equal(t, ":smile:", slackNotifier.iconEmoji) + assert.Equal(t, "https://grafana.com/img/fav32.png", slackNotifier.iconURL) + assert.Equal(t, []string{"user1", "user2"}, slackNotifier.mentionUsers) + assert.Equal(t, []string{"group1", "group2"}, slackNotifier.mentionGroups) + assert.Equal(t, "here", slackNotifier.mentionChannel) + assert.Equal(t, "xenc-XXXXXXXX-XXXXXXXX-XXXXXXXXXX", slackNotifier.token) + }) - So(err, ShouldBeNil) - So(slackNotifier.Name, ShouldEqual, "ops") - So(slackNotifier.Type, ShouldEqual, "slack") - So(slackNotifier.URL, ShouldEqual, "http://google.com") - So(slackNotifier.Recipient, ShouldEqual, "#ds-opentsdb") - So(slackNotifier.Username, ShouldEqual, "Grafana Alerts") - So(slackNotifier.IconEmoji, ShouldEqual, ":smile:") - So(slackNotifier.IconURL, ShouldEqual, "https://grafana.com/img/fav32.png") - So(slackNotifier.MentionUsers, ShouldResemble, []string{"user1", "user2"}) - So(slackNotifier.MentionGroups, ShouldResemble, []string{"group1", "group2"}) - So(slackNotifier.MentionChannel, ShouldEqual, "here") - So(slackNotifier.Token, ShouldEqual, "xenc-XXXXXXXX-XXXXXXXX-XXXXXXXXXX") - }) - - Convey("with channel recipient with spaces should return an error", func() { - json := ` + t.Run("with channel recipient with spaces should return an error", func(t *testing.T) { + 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, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - _, err = NewSlackNotifier(model) + _, err = NewSlackNotifier(model) + assert.EqualError(t, err, "alert validation error: recipient on invalid format: \"#open tsdb\"") + }) - So(err, ShouldBeError, "alert validation error: Recipient on invalid format: \"#open tsdb\"") - }) - - Convey("with user recipient with spaces should return an error", func() { - json := ` + t.Run("with user recipient with spaces should return an error", func(t *testing.T) { + 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, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - _, err = NewSlackNotifier(model) + _, err = NewSlackNotifier(model) + assert.EqualError(t, err, "alert validation error: recipient on invalid format: \"@user name\"") + }) - 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 := ` + t.Run("with user recipient with uppercase letters should return an error", func(t *testing.T) { + 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, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - _, err = NewSlackNotifier(model) + _, err = NewSlackNotifier(model) + assert.EqualError(t, err, "alert validation error: recipient on invalid format: \"@User\"") + }) - So(err, ShouldBeError, "alert validation error: Recipient on invalid format: \"@User\"") - }) - - Convey("with Slack ID for recipient should work", func() { - json := ` + t.Run("with Slack ID for recipient should work", func(t *testing.T) { + 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, - } + settingsJSON, err := simplejson.NewJson([]byte(json)) + require.NoError(t, err) + model := &models.AlertNotification{ + Name: "ops", + Type: "slack", + Settings: settingsJSON, + } - not, err := NewSlackNotifier(model) - So(err, ShouldBeNil) - slackNotifier := not.(*SlackNotifier) - - So(slackNotifier.Recipient, ShouldEqual, "1ABCDE") - }) - }) + not, err := NewSlackNotifier(model) + require.NoError(t, err) + slackNotifier := not.(*SlackNotifier) + assert.Equal(t, "1ABCDE", slackNotifier.recipient) }) } diff --git a/pkg/services/ngalert/notifier/channels/slack.go b/pkg/services/ngalert/notifier/channels/slack.go index ad7a5ec7ceb..0beaf44c194 100644 --- a/pkg/services/ngalert/notifier/channels/slack.go +++ b/pkg/services/ngalert/notifier/channels/slack.go @@ -1,9 +1,13 @@ package channels import ( + "bytes" "context" + "crypto/tls" "encoding/json" "fmt" + "io" + "net" "net/http" "net/url" "regexp" @@ -16,7 +20,6 @@ import ( "github.com/prometheus/alertmanager/template" "github.com/prometheus/alertmanager/types" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" @@ -32,7 +35,7 @@ type SlackNotifier struct { tmpl *template.Template externalUrl *url.URL - URL string + URL *url.URL Username string IconEmoji string IconURL string @@ -48,6 +51,8 @@ type SlackNotifier struct { var reRecipient *regexp.Regexp = regexp.MustCompile("^((@[a-z0-9][a-zA-Z0-9._-]*)|(#[^ .A-Z]{1,79})|([a-zA-Z0-9]+))$") +const slackAPIEndpoint = "https://slack.com/api/chat.postMessage" + // NewSlackNotifier is the constructor for the Slack notifier func NewSlackNotifier(model *models.AlertNotification, t *template.Template, externalUrl *url.URL) (*SlackNotifier, error) { if model.Settings == nil { @@ -56,12 +61,22 @@ func NewSlackNotifier(model *models.AlertNotification, t *template.Template, ext slackURL := model.DecryptedValue("url", model.Settings.Get("url").MustString()) if slackURL == "" { - return nil, alerting.ValidationError{Reason: "Could not find url property in settings"} + slackURL = slackAPIEndpoint + } + apiURL, err := url.Parse(slackURL) + if err != nil { + return nil, alerting.ValidationError{Reason: fmt.Sprintf("invalid URL %q: %s", slackURL, err)} } 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)} + if recipient != "" { + if !reRecipient.MatchString(recipient) { + return nil, alerting.ValidationError{Reason: fmt.Sprintf("recipient on invalid format: %q", recipient)} + } + } else if apiURL.String() == slackAPIEndpoint { + return nil, alerting.ValidationError{ + Reason: "recipient must be specified when using the Slack chat API", + } } mentionChannel := model.Settings.Get("mentionChannel").MustString() @@ -89,9 +104,16 @@ func NewSlackNotifier(model *models.AlertNotification, t *template.Template, ext } } + token := model.DecryptedValue("token", model.Settings.Get("token").MustString()) + if token == "" && apiURL.String() == slackAPIEndpoint { + return nil, alerting.ValidationError{ + Reason: "token must be specified when using the Slack chat API", + } + } + return &SlackNotifier{ NotifierBase: old_notifiers.NewNotifierBase(model), - URL: slackURL, + URL: apiURL, Recipient: recipient, MentionUsers: mentionUsers, MentionGroups: mentionGroups, @@ -99,7 +121,7 @@ func NewSlackNotifier(model *models.AlertNotification, t *template.Template, ext Username: model.Settings.Get("username").MustString("Grafana"), IconEmoji: model.Settings.Get("icon_emoji").MustString(), IconURL: model.Settings.Get("icon_url").MustString(), - Token: model.DecryptedValue("token", model.Settings.Get("token").MustString()), + Token: token, Text: model.Settings.Get("text").MustString(`{{ template "slack.default.text" . }}`), Title: model.Settings.Get("title").MustString(`{{ template "slack.default.title" . }}`), Fallback: model.Settings.Get("fallback").MustString(`{{ template "slack.default.title" . }}`), @@ -132,6 +154,7 @@ type attachment struct { Ts int64 `json:"ts,omitempty"` } +// Notify sends an alert notification to Slack. func (sn *SlackNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { msg, err := sn.buildSlackMessage(ctx, as) if err != nil { @@ -143,31 +166,85 @@ func (sn *SlackNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, return false, fmt.Errorf("marshal json: %w", err) } - cmd := &models.SendWebhookSync{ - Url: sn.URL, - Body: string(b), - HttpMethod: http.MethodPost, + sn.log.Debug("Sending Slack API request", "url", sn.URL.String(), "data", string(b)) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, sn.URL.String(), bytes.NewReader(b)) + if err != nil { + return false, fmt.Errorf("failed to create HTTP request: %w", err) } - if sn.Token != "" { - sn.log.Debug("Adding authorization header to HTTP request") - cmd.HttpHeader = map[string]string{ - "Authorization": fmt.Sprintf("Bearer %s", sn.Token), + request.Header.Set("Content-Type", "application/json") + request.Header.Set("User-Agent", "Grafana") + if sn.Token == "" { + if sn.URL.String() == slackAPIEndpoint { + panic("Token should be set when using the Slack chat API") } + } else { + sn.log.Debug("Adding authorization header to HTTP request") + request.Header.Set("Authorization", fmt.Sprintf("Bearer %s", sn.Token)) } - if err := bus.DispatchCtx(ctx, cmd); err != nil { - sn.log.Error("Failed to send slack notification", "error", err, "webhook", sn.Name) + if err := sendSlackRequest(request, sn.log); err != nil { return false, err } - return true, nil } +// sendSlackRequest sends a request to the Slack API. +// Stubbable by tests. +var sendSlackRequest = func(request *http.Request, logger log.Logger) error { + netTransport := &http.Transport{ + TLSClientConfig: &tls.Config{ + Renegotiation: tls.RenegotiateFreelyAsClient, + }, + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + }).DialContext, + TLSHandshakeTimeout: 5 * time.Second, + } + netClient := &http.Client{ + Timeout: time.Second * 30, + Transport: netTransport, + } + resp, err := netClient.Do(request) + if err != nil { + return err + } + defer func() { + if err := resp.Body.Close(); err != nil { + logger.Warn("Failed to close response body", "err", err) + } + }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("failed to read response body: %w", err) + } + + if resp.StatusCode/100 != 2 { + logger.Warn("Slack API request failed", "url", request.URL.String(), "statusCode", resp.Status, "body", string(body)) + return fmt.Errorf("request to Slack API failed with status code %d", resp.StatusCode) + } + + var rslt map[string]interface{} + // Slack responds to some requests with a JSON document, that might contain an error + if err := json.Unmarshal(body, &rslt); err == nil { + if !rslt["ok"].(bool) { + errMsg := rslt["error"].(string) + logger.Warn("Sending Slack API request failed", "url", request.URL.String(), "statusCode", resp.Status, + "err", errMsg) + return fmt.Errorf("failed to make Slack API request: %s", errMsg) + } + } + + logger.Debug("Sending Slack API request succeeded", "url", request.URL.String(), "statusCode", resp.Status) + return nil +} + func (sn *SlackNotifier) buildSlackMessage(ctx context.Context, as []*types.Alert) (*slackMessage, error) { - var tmplErr error data := notify.GetTemplateData(ctx, &template.Template{ExternalURL: sn.externalUrl}, as, gokit_log.NewNopLogger()) alerts := types.Alerts(as...) + var tmplErr error tmpl := notify.TmplText(sn.tmpl, data, &tmplErr) req := &slackMessage{ @@ -189,6 +266,9 @@ func (sn *SlackNotifier) buildSlackMessage(ctx context.Context, as []*types.Aler }, }, } + if tmplErr != nil { + return nil, fmt.Errorf("failed to template Slack message: %w", tmplErr) + } mentionsBuilder := strings.Builder{} appendSpace := func() { @@ -225,11 +305,7 @@ func (sn *SlackNotifier) buildSlackMessage(ctx context.Context, as []*types.Aler } } - if tmplErr != nil { - tmplErr = fmt.Errorf("failed to template Slack message: %w", tmplErr) - } - - return req, tmplErr + return req, nil } func (sn *SlackNotifier) SendResolved() bool { diff --git a/pkg/services/ngalert/notifier/channels/slack_test.go b/pkg/services/ngalert/notifier/channels/slack_test.go index fb2720d5d67..0baec37964a 100644 --- a/pkg/services/ngalert/notifier/channels/slack_test.go +++ b/pkg/services/ngalert/notifier/channels/slack_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "errors" + "io" + "net/http" "net/url" "testing" @@ -13,8 +15,8 @@ import ( "github.com/prometheus/common/model" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/alerting" ) @@ -34,7 +36,43 @@ func TestSlackNotifier(t *testing.T) { { name: "Correct config with one alert", settings: `{ - "url": "https://test.slack.com", + "token": "1234", + "recipient": "#testchannel", + "icon_emoji": ":emoji:" + }`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1"}, + }, + }, + }, + expMsg: &slackMessage{ + Channel: "#testchannel", + Username: "Grafana", + IconEmoji: ":emoji:", + Attachments: []attachment{ + { + Title: "[FIRING:1] (val1)", + TitleLink: "TODO: rule URL", + Text: "", + Fallback: "[FIRING:1] (val1)", + Fields: nil, + Footer: "Grafana v", + FooterIcon: "https://grafana.com/assets/img/fav32.png", + Color: "#D63232", + Ts: 0, + }, + }, + }, + expInitError: nil, + expMsgError: nil, + }, + { + name: "Correct config with webhook", + settings: `{ + "url": "https://webhook.com", "recipient": "#testchannel", "icon_emoji": ":emoji:" }`, @@ -70,7 +108,7 @@ func TestSlackNotifier(t *testing.T) { { name: "Correct config with multiple alerts and template", settings: `{ - "url": "https://test.slack.com", + "token": "1234", "recipient": "#testchannel", "icon_emoji": ":emoji:", "title": "{{ .Alerts.Firing | len }} firing, {{ .Alerts.Resolved | len }} resolved" @@ -110,9 +148,17 @@ func TestSlackNotifier(t *testing.T) { expInitError: nil, expMsgError: nil, }, { - name: "Error in initing", - settings: `{}`, - expInitError: alerting.ValidationError{Reason: "Could not find url property in settings"}, + name: "Missing token", + settings: `{ + "recipient": "#testchannel" + }`, + expInitError: alerting.ValidationError{Reason: "token must be specified when using the Slack chat API"}, + }, { + name: "Missing recipient", + settings: `{ + "token": "1234" + }`, + expInitError: alerting.ValidationError{Reason: "recipient must be specified when using the Slack chat API"}, }, { name: "Error in building message", settings: `{ @@ -145,17 +191,28 @@ func TestSlackNotifier(t *testing.T) { require.NoError(t, err) body := "" - bus.AddHandlerCtx("test", func(ctx context.Context, webhook *models.SendWebhookSync) error { - body = webhook.Body - return nil + origSendSlackRequest := sendSlackRequest + t.Cleanup(func() { + sendSlackRequest = origSendSlackRequest }) + sendSlackRequest = func(request *http.Request, log log.Logger) error { + t.Helper() + defer func() { + _ = request.Body.Close() + }() + + b, err := io.ReadAll(request.Body) + require.NoError(t, err) + body = string(b) + return nil + } ctx := notify.WithGroupKey(context.Background(), "alertname") ctx = notify.WithGroupLabels(ctx, model.LabelSet{"alertname": ""}) ok, err := pn.Notify(ctx, c.alerts...) if c.expMsgError != nil { - require.False(t, ok) require.Error(t, err) + require.False(t, ok) require.Equal(t, c.expMsgError.Error(), err.Error()) return } @@ -163,8 +220,8 @@ func TestSlackNotifier(t *testing.T) { require.NoError(t, err) // Getting Ts from actual since that can't be predicted. - obj := &slackMessage{} - require.NoError(t, json.Unmarshal([]byte(body), obj)) + var obj slackMessage + require.NoError(t, json.Unmarshal([]byte(body), &obj)) c.expMsg.Attachments[0].Ts = obj.Attachments[0].Ts expBody, err := json.Marshal(c.expMsg) diff --git a/pkg/services/provisioning/notifiers/config_reader_test.go b/pkg/services/provisioning/notifiers/config_reader_test.go index d655b7d5318..423dc5a25c8 100644 --- a/pkg/services/provisioning/notifiers/config_reader_test.go +++ b/pkg/services/provisioning/notifiers/config_reader_test.go @@ -321,7 +321,7 @@ func TestNotificationAsConfig(t *testing.T) { cfgProvider := &configReader{log: log.New("test logger")} _, err := cfgProvider.readConfig(incorrectSettings) So(err, ShouldNotBeNil) - So(err.Error(), ShouldEqual, "alert validation error: Could not find url property in settings") + So(err.Error(), ShouldEqual, "alert validation error: token must be specified when using the Slack chat API") }) }) } diff --git a/pkg/services/provisioning/notifiers/testdata/test-configs/incorrect-settings/incorrect-settings.yaml b/pkg/services/provisioning/notifiers/testdata/test-configs/incorrect-settings/incorrect-settings.yaml index b7ecfbdf012..b79c6e9761c 100644 --- a/pkg/services/provisioning/notifiers/testdata/test-configs/incorrect-settings/incorrect-settings.yaml +++ b/pkg/services/provisioning/notifiers/testdata/test-configs/incorrect-settings/incorrect-settings.yaml @@ -1,10 +1,9 @@ notifiers: - - name: slack-notification-without-url-in-settings + - name: slack-notification-without-token-in-settings type: slack org_id: 2 uid: notifier1 is_default: true settings: recipient: "XXX" - token: "xoxb" - uploadImage: true \ No newline at end of file + uploadImage: true