IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fix panic when Slack API sends unexpected response (#40721)

Browse Source 
* panic when unexpected Slack response fixed, tests added

* fix linting errors, add test
This commit is contained in:
Santiago 2021-10-21 03:12:15 -03:00 committed by GitHub
parent 27ba6b37ab
commit e54fe220e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 210 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
pkg/services/alerting/notifiers/slack.go
View File

@ -382,15 +382,17 @@ func (sn *SlackNotifier) sendRequest(ctx context.Context, data []byte) error {
return fmt.Errorf("failed to read response body: %w", err)
}
if resp.StatusCode/100 == 2 {
var rslt map[string]interface{}
if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices {
// Slack responds to some requests with a JSON document, that might contain an error
rslt := struct {
Ok bool `json:"ok"`
Err string `json:"error"`
}{}
if err := json.Unmarshal(body, &rslt); err == nil {
if !rslt["ok"].(bool) {
errMsg := rslt["error"].(string)
if !rslt.Ok && rslt.Err != "" {
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)
"err", rslt.Err)
return fmt.Errorf("failed to make Slack API request: %s", rslt.Err)
}
}

102
pkg/services/alerting/notifiers/slack_test.go
View File

@ -2,6 +2,9 @@ package notifiers
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"github.com/grafana/grafana/pkg/components/simplejson"
@ -222,3 +225,102 @@ func TestSlackNotifier(t *testing.T) {
assert.Equal(t, "1ABCDE", slackNotifier.recipient)
})
}
func TestSendSlackRequest(t *testing.T) {
tests := []struct {
name string
slackResponse string
statusCode int
expectError bool
}{
{
name: "Example error",
slackResponse: `{
"ok": false,
"error": "too_many_attachments"
}`,
statusCode: http.StatusBadRequest,
expectError: true,
},
{
name: "Non 200 status code, no response body",
statusCode: http.StatusMovedPermanently,
expectError: true,
},
{
name: "Success case, normal response body",
slackResponse: `{
"ok": true,
"channel": "C1H9RESGL",
"ts": "1503435956.000247",
"message": {
"text": "Here's a message for you",
"username": "ecto1",
"bot_id": "B19LU7CSY",
"attachments": [
{
"text": "This is an attachment",
"id": 1,
"fallback": "This is an attachment's fallback"
}
],
"type": "message",
"subtype": "bot_message",
"ts": "1503435956.000247"
}
}`,
statusCode: http.StatusOK,
expectError: false,
},
{
name: "Success case, no response body",
statusCode: http.StatusOK,
expectError: false,
},
{
name: "Success case, unexpected response body",
statusCode: http.StatusOK,
slackResponse: "{}",
expectError: false,
},
{
name: "Success case, ok: true",
statusCode: http.StatusOK,
slackResponse: "{\"ok\": true}",
expectError: false,
},
{
name: "200 status code, error in body",
statusCode: http.StatusOK,
slackResponse: `{"ok": false, "error": "test error"}`,
expectError: true,
},
}
for _, test := range tests {
t.Run(test.name, func(tt *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(test.statusCode)
_, err := w.Write([]byte(test.slackResponse))
require.NoError(tt, err)
}))
settingsJSON, err := simplejson.NewJson([]byte(fmt.Sprintf(`{"url": %q}`, server.URL)))
require.NoError(t, err)
model := &models.AlertNotification{
Settings: settingsJSON,
}
not, err := NewSlackNotifier(model, ossencryption.ProvideService().GetDecryptedValue)
require.NoError(t, err)
slackNotifier := not.(*SlackNotifier)
err = slackNotifier.sendRequest(context.TODO(), []byte("test"))
if !test.expectError {
require.NoError(tt, err)
} else {
require.Error(tt, err)
}
})
}
}

14
pkg/services/ngalert/notifier/channels/slack.go
View File

@ -219,19 +219,21 @@ var sendSlackRequest = func(request *http.Request, logger log.Logger) error {
return fmt.Errorf("failed to read response body: %w", err)
}
if resp.StatusCode/100 != 2 {
if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
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
rslt := struct {
Ok bool `json:"ok"`
Err string `json:"error"`
}{}
if err := json.Unmarshal(body, &rslt); err == nil {
if !rslt["ok"].(bool) {
errMsg := rslt["error"].(string)
if !rslt.Ok && rslt.Err != "" {
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)
"err", rslt.Err)
return fmt.Errorf("failed to make Slack API request: %s", rslt.Err)
}
}

92
pkg/services/ngalert/notifier/channels/slack_test.go
View File

@ -5,6 +5,7 @@ import (
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"net/url"
"testing"
@ -221,3 +222,94 @@ func TestSlackNotifier(t *testing.T) {
})
}
}
func TestSendSlackRequest(t *testing.T) {
tests := []struct {
name string
slackResponse string
statusCode int
expectError bool
}{
{
name: "Example error",
slackResponse: `{
"ok": false,
"error": "too_many_attachments"
}`,
statusCode: http.StatusBadRequest,
expectError: true,
},
{
name: "Non 200 status code, no response body",
statusCode: http.StatusMovedPermanently,
expectError: true,
},
{
name: "Success case, normal response body",
slackResponse: `{
"ok": true,
"channel": "C1H9RESGL",
"ts": "1503435956.000247",
"message": {
"text": "Here's a message for you",
"username": "ecto1",
"bot_id": "B19LU7CSY",
"attachments": [
{
"text": "This is an attachment",
"id": 1,
"fallback": "This is an attachment's fallback"
}
],
"type": "message",
"subtype": "bot_message",
"ts": "1503435956.000247"
}
}`,
statusCode: http.StatusOK,
expectError: false,
},
{
name: "Success case, no response body",
statusCode: http.StatusOK,
expectError: false,
},
{
name: "Success case, unexpected response body",
statusCode: http.StatusOK,
slackResponse: "{}",
expectError: false,
},
{
name: "Success case, ok: true",
statusCode: http.StatusOK,
slackResponse: "{\"ok\": true}",
expectError: false,
},
{
name: "200 status code, error in body",
statusCode: http.StatusOK,
slackResponse: `{"ok": false, "error": "test error"}`,
expectError: true,
},
}
for _, test := range tests {
t.Run(test.name, func(tt *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(test.statusCode)
_, err := w.Write([]byte(test.slackResponse))
require.NoError(tt, err)
}))
req, err := http.NewRequest(http.MethodGet, server.URL, nil)
require.NoError(tt, err)
err = sendSlackRequest(req, log.New("test"))
if !test.expectError {
require.NoError(tt, err)
} else {
require.Error(tt, err)
}
})
}
}