diff --git a/pkg/services/ngalert/notifier/channels/slack.go b/pkg/services/ngalert/notifier/channels/slack.go index e1132d3c819..d6b76f8dbfd 100644 --- a/pkg/services/ngalert/notifier/channels/slack.go +++ b/pkg/services/ngalert/notifier/channels/slack.go @@ -8,8 +8,6 @@ import ( "errors" "fmt" "io" - "math/rand" - "mime/multipart" "net" "net/http" "net/url" @@ -27,7 +25,6 @@ import ( ) var SlackAPIEndpoint = "https://slack.com/api/chat.postMessage" -var SlackImageAPIEndpoint = "https://slack.com/api/files.upload" // SlackNotifier is responsible for sending // alert notification to Slack. @@ -39,7 +36,6 @@ type SlackNotifier struct { webhookSender notifications.WebhookSender URL *url.URL - ImageUploadURL string Username string IconEmoji string IconURL string @@ -55,7 +51,6 @@ type SlackNotifier struct { type SlackConfig struct { *NotificationChannelConfig URL *url.URL - ImageUploadURL string Username string IconEmoji string IconURL string @@ -83,7 +78,6 @@ func NewSlackConfig(factoryConfig FactoryConfig) (*SlackConfig, error) { channelConfig := factoryConfig.Config decryptFunc := factoryConfig.DecryptFunc endpointURL := channelConfig.Settings.Get("endpointUrl").MustString(SlackAPIEndpoint) - imageUploadURL := channelConfig.Settings.Get("imageUploadUrl").MustString(SlackImageAPIEndpoint) slackURL := decryptFunc(context.Background(), channelConfig.SecureSettings, "url", channelConfig.Settings.Get("url").MustString()) if slackURL == "" { slackURL = endpointURL @@ -127,7 +121,6 @@ func NewSlackConfig(factoryConfig FactoryConfig) (*SlackConfig, error) { MentionUsers: mentionUsers, MentionGroups: mentionGroups, URL: apiURL, - ImageUploadURL: imageUploadURL, Username: channelConfig.Settings.Get("username").MustString("Grafana"), IconEmoji: channelConfig.Settings.Get("icon_emoji").MustString(), IconURL: channelConfig.Settings.Get("icon_url").MustString(), @@ -152,7 +145,6 @@ func NewSlackNotifier(config *SlackConfig, Settings: config.Settings, }), URL: config.URL, - ImageUploadURL: config.ImageUploadURL, Recipient: config.Recipient, MentionUsers: config.MentionUsers, MentionGroups: config.MentionGroups, @@ -196,6 +188,7 @@ type attachment struct { // Notify sends an alert notification to Slack. func (sn *SlackNotifier) Notify(ctx context.Context, alerts ...*types.Alert) (bool, error) { + sn.log.Debug("building slack message", "alerts", len(alerts)) msg, err := sn.buildSlackMessage(ctx, alerts) if err != nil { return false, fmt.Errorf("build slack message: %w", err) @@ -227,46 +220,18 @@ func (sn *SlackNotifier) Notify(ctx context.Context, alerts ...*types.Alert) (bo return false, err } - var imgData io.ReadCloser - - // Try to upload if we have an image path but no image URL. This uploads the file - // immediately after the message. A bit of a hack, but it doesn't require the - // user to have an image host set up. - // TODO: We need a refactoring so we don't do two database reads for the same data. - if len(msg.Attachments[0].ImageURL) == 0 { - _ = withStoredImage(ctx, sn.log, sn.images, - func(index int, image *ngmodels.Image) error { - if image == nil || len(image.Path) == 0 { - return nil - } - - imgData, err = openImage(image.Path) - if err != nil { - imgData = nil - } - - return nil - }, - 0, alerts...) - - if imgData != nil { - defer func() { - _ = imgData.Close() - }() - - err = sn.slackFileUpload(ctx, imgData, sn.Recipient, sn.Token) - if err != nil { - sn.log.Warn("Error reading screenshot data from ImageStore: %v", 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 { +var sendSlackRequest = func(request *http.Request, logger log.Logger) (retErr error) { + defer func() { + if retErr != nil { + logger.Warn("failed to send slack request", "err", retErr) + } + }() + netTransport := &http.Transport{ TLSClientConfig: &tls.Config{ Renegotiation: tls.RenegotiateFreelyAsClient, @@ -407,59 +372,3 @@ func (sn *SlackNotifier) buildSlackMessage(ctx context.Context, alrts []*types.A func (sn *SlackNotifier) SendResolved() bool { return !sn.GetDisableResolveMessage() } - -func (sn *SlackNotifier) slackFileUpload(ctx context.Context, data io.Reader, recipient, token string) error { - sn.log.Info("Uploading to slack via file.upload API") - headers, uploadBody, err := sn.generateFileUploadBody(data, token, recipient) - if err != nil { - return err - } - cmd := &models.SendWebhookSync{ - Url: sn.ImageUploadURL, Body: uploadBody.String(), HttpHeader: headers, HttpMethod: "POST", - } - if err := sn.webhookSender.SendWebhookSync(ctx, cmd); err != nil { - sn.log.Error("Failed to upload slack image", "error", err, "webhook", "file.upload") - return err - } - return nil -} - -func (sn *SlackNotifier) generateFileUploadBody(data io.Reader, token string, recipient string) (map[string]string, bytes.Buffer, error) { - // Slack requires all POSTs to files.upload to present - // an "application/x-www-form-urlencoded" encoded querystring - // See https://api.slack.com/methods/files.upload - var b bytes.Buffer - w := multipart.NewWriter(&b) - defer func() { - if err := w.Close(); err != nil { - // Shouldn't matter since we already close w explicitly on the non-error path - sn.log.Warn("Failed to close multipart writer", "err", err) - } - }() - - // TODO: perhaps we should pass the filename through to here to use the local name. - // https://github.com/grafana/grafana/issues/49375 - fw, err := w.CreateFormFile("file", fmt.Sprintf("screenshot-%v", rand.Intn(2e6))) - if err != nil { - return nil, b, err - } - if _, err := io.Copy(fw, data); err != nil { - return nil, b, err - } - // Add the authorization token - if err := w.WriteField("token", token); err != nil { - return nil, b, err - } - // Add the channel(s) to POST to - if err := w.WriteField("channels", recipient); err != nil { - return nil, b, err - } - if err := w.Close(); err != nil { - return nil, b, fmt.Errorf("failed to close multipart writer: %w", err) - } - headers := map[string]string{ - "Content-Type": w.FormDataContentType(), - "Authorization": "auth_token=\"" + token + "\"", - } - return headers, b, nil -} diff --git a/pkg/services/ngalert/notifier/channels/slack_test.go b/pkg/services/ngalert/notifier/channels/slack_test.go index 82fc387a986..83899b44f59 100644 --- a/pkg/services/ngalert/notifier/channels/slack_test.go +++ b/pkg/services/ngalert/notifier/channels/slack_test.go @@ -39,14 +39,6 @@ func TestSlackNotifier(t *testing.T) { Token: "test-with-url", URL: "https://www.example.com/image.jpg", }, - { - Token: "test-with-path-not-found", - Path: "usr/home/nouser/noway.jpg", - }, - { - Token: "test-with-path-found", - Path: f.Name(), // Has the full path because of how CreateTemp works. - }, }, } @@ -172,78 +164,6 @@ func TestSlackNotifier(t *testing.T) { }, expMsgError: nil, }, - { - name: "Image URL with path but no file creates message with no error", - settings: `{ - "token": "1234", - "image_upload_url": "https://www.webhook.com", - "recipient": "#testchannel", - "icon_emoji": ":emoji:" - }`, - alerts: []*types.Alert{ - { - Alert: model.Alert{ - Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, - Annotations: model.LabelSet{"ann1": "annv1", "__dashboardUid__": "abcd", "__panelId__": "efgh", "__alertScreenshotToken__": "test-with-path-not-found"}, - }, - }, - }, - expMsg: &slackMessage{ - Channel: "#testchannel", - Username: "Grafana", - IconEmoji: ":emoji:", - Attachments: []attachment{ - { - Title: "[FIRING:1] (val1)", - TitleLink: "http://localhost/alerting/list", - Text: "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n", - Fallback: "[FIRING:1] (val1)", - Fields: nil, - Footer: "Grafana v" + setting.BuildVersion, - FooterIcon: "https://grafana.com/assets/img/fav32.png", - Color: "#D63232", - Ts: 0, - }, - }, - }, - expMsgError: nil, - }, - { - name: "Image URL with path and file creates message and uploads image", - settings: `{ - "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", "__dashboardUid__": "abcd", "__panelId__": "efgh", "__alertScreenshotToken__": "test-with-path-found"}, - }, - }, - }, - expMsg: &slackMessage{ - Channel: "#testchannel", - Username: "Grafana", - IconEmoji: ":emoji:", - Attachments: []attachment{ - { - Title: "[FIRING:1] (val1)", - TitleLink: "http://localhost/alerting/list", - Text: "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n", - Fallback: "[FIRING:1] (val1)", - Fields: nil, - Footer: "Grafana v" + setting.BuildVersion, - FooterIcon: "https://grafana.com/assets/img/fav32.png", - Color: "#D63232", - Ts: 0, - }, - }, - }, - expMsgError: nil, - expWebhookURL: SlackImageAPIEndpoint, - }, { name: "Correct config with multiple alerts and template", settings: `{ @@ -334,43 +254,6 @@ func TestSlackNotifier(t *testing.T) { }, expMsgError: nil, }, - { - name: "Custom image upload URL", - settings: `{ - "token": "1234", - "recipient": "#testchannel", - "icon_emoji": ":emoji:", - "imageUploadUrl": "https://custom-domain.upload" - }`, - alerts: []*types.Alert{ - { - Alert: model.Alert{ - Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, - Annotations: model.LabelSet{"ann1": "annv1", "__dashboardUid__": "abcd", "__panelId__": "efgh", "__alertScreenshotToken__": "test-with-path-found"}, - }, - }, - }, - expMsg: &slackMessage{ - Channel: "#testchannel", - Username: "Grafana", - IconEmoji: ":emoji:", - Attachments: []attachment{ - { - Title: "[FIRING:1] (val1)", - TitleLink: "http://localhost/alerting/list", - Text: "**Firing**\n\nValue: [no value]\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matcher=alertname%3Dalert1&matcher=lbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n", - Fallback: "[FIRING:1] (val1)", - Fields: nil, - Footer: "Grafana v" + setting.BuildVersion, - FooterIcon: "https://grafana.com/assets/img/fav32.png", - Color: "#D63232", - Ts: 0, - }, - }, - }, - expMsgError: nil, - expWebhookURL: "https://custom-domain.upload", - }, } for _, c := range cases {