From 2448123a652b3d8f61ee35c32412d34850fb41da Mon Sep 17 00:00:00 2001 From: gotjosh Date: Tue, 12 Oct 2021 23:55:39 +0100 Subject: [PATCH] Alerting: Remove invalid Slack URL as we migrate notification channels (#40344) * Alerting: Remove invalid Slack URL as we migrate notification channels Grafana will accept any type of utf8 valid string as the Slack URL and will simply fail as we try to deliver the notification of the channel. The Alertmanager will fail to apply a configuration if the URL of the Slack Receiver is invalid. This change takes that into account by removing the URL for the receiver as we migrate notification channels that do not pass the url validation. As we assume the notification was not being delivered to being with. * Add a log line when we modify the channel Co-authored-by: Yuriy Tseretyan --- .../sqlstore/migrations/ualert/channel.go | 17 +++ .../migrations/ualert/channel_test.go | 119 ++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 pkg/services/sqlstore/migrations/ualert/channel_test.go diff --git a/pkg/services/sqlstore/migrations/ualert/channel.go b/pkg/services/sqlstore/migrations/ualert/channel.go index 333fc3ff2df..d55d4e63e2c 100644 --- a/pkg/services/sqlstore/migrations/ualert/channel.go +++ b/pkg/services/sqlstore/migrations/ualert/channel.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "net/url" "sort" "strings" @@ -127,6 +128,22 @@ func (m *migration) makeReceiverAndRoute(ruleUid string, orgID int64, channelUid if err != nil { return err } + + // Grafana accepts any type of string as a URL for the Slack notification channel. + // However, the Alertmanager will fail if provided with an invalid URL we have two options at this point: + // Either we fail the migration or remove the URL, we've chosen the latter and assume that the notification + // channel was broken to begin with. + if c.Type == "slack" { + u, ok := decryptedSecureSettings["url"] + if ok { + _, err := url.Parse(u) + if err != nil { + m.mg.Logger.Warn("slack notification channel had invalid URL, removing", "name", c.Name, "uid", c.Uid, "org", c.OrgID) + delete(decryptedSecureSettings, "url") + } + } + } + portedChannels = append(portedChannels, &PostableGrafanaReceiver{ UID: uid, Name: c.Name, diff --git a/pkg/services/sqlstore/migrations/ualert/channel_test.go b/pkg/services/sqlstore/migrations/ualert/channel_test.go new file mode 100644 index 00000000000..26ea14e6a95 --- /dev/null +++ b/pkg/services/sqlstore/migrations/ualert/channel_test.go @@ -0,0 +1,119 @@ +package ualert + +import ( + "fmt" + "math/rand" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" + "github.com/grafana/grafana/pkg/util" +) + +func Test_makeReceiverAndRoute(t *testing.T) { + emptyMigration := func() *migration { + return &migration{ + mg: &migrator.Migrator{ + Logger: log.New("test"), + }, + migratedChannelsPerOrg: make(map[int64]map[*notificationChannel]struct{}), + portedChannelGroupsPerOrg: make(map[int64]map[string]string), + seenChannelUIDs: make(map[string]struct{}), + } + } + + generateChannel := func(channelType string, settings map[string]interface{}, secureSettings map[string]string) *notificationChannel { + uid := util.GenerateShortUID() + return ¬ificationChannel{ + ID: rand.Int63(), + OrgID: rand.Int63(), + Uid: uid, + Name: fmt.Sprintf("Test-%s", uid), + Type: channelType, + DisableResolveMessage: rand.Int63()%2 == 0, + IsDefault: rand.Int63()%2 == 0, + Settings: simplejson.NewFromAny(settings), + SecureSettings: GetEncryptedJsonData(secureSettings), + } + } + + t.Run("Slack channel is migrated", func(t *testing.T) { + t.Run("url is removed if it is invalid (secure settings)", func(t *testing.T) { + secureSettings := map[string]string{ + "url": invalidUri, + "token": util.GenerateShortUID(), + } + settings := map[string]interface{}{ + "test": "data", + "some_map": map[string]interface{}{ + "test": rand.Int63(), + }, + } + + channel := generateChannel("slack", settings, secureSettings) + channelsUid := []interface{}{ + channel.Uid, + } + defaultChannels := make([]*notificationChannel, 0) + allChannels := map[interface{}]*notificationChannel{ + channel.Uid: channel, + } + + apiReceiver, _, err := emptyMigration().makeReceiverAndRoute(util.GenerateShortUID(), channel.OrgID, channelsUid, defaultChannels, allChannels) + require.NoError(t, err) + + require.Len(t, apiReceiver.GrafanaManagedReceivers, 1) + + receiver := apiReceiver.GrafanaManagedReceivers[0] + + require.NotContains(t, receiver.SecureSettings, "url") + require.Contains(t, receiver.SecureSettings, "token") + require.Equal(t, secureSettings["token"], receiver.SecureSettings["token"]) + actualSettings, err := receiver.Settings.Map() + require.NoError(t, err) + require.Equal(t, settings, actualSettings) + }) + + t.Run("url is removed if it is invalid (settings)", func(t *testing.T) { + secureSettings := map[string]string{ + "token": util.GenerateShortUID(), + } + settings := map[string]interface{}{ + "url": invalidUri, + "test": "data", + "some_map": map[string]interface{}{ + "test": rand.Int63(), + }, + } + + channel := generateChannel("slack", settings, secureSettings) + channelsUid := []interface{}{ + channel.Uid, + } + defaultChannels := make([]*notificationChannel, 0) + allChannels := map[interface{}]*notificationChannel{ + channel.Uid: channel, + } + + apiReceiver, _, err := emptyMigration().makeReceiverAndRoute(util.GenerateShortUID(), channel.OrgID, channelsUid, defaultChannels, allChannels) + require.NoError(t, err) + + require.Len(t, apiReceiver.GrafanaManagedReceivers, 1) + + receiver := apiReceiver.GrafanaManagedReceivers[0] + + require.NotContains(t, receiver.SecureSettings, "url") + require.Contains(t, receiver.SecureSettings, "token") + require.Equal(t, secureSettings["token"], receiver.SecureSettings["token"]) + actualSettings, err := receiver.Settings.Map() + require.NoError(t, err) + delete(settings, "url") + require.Equal(t, settings, actualSettings) + }) + }) +} + +const invalidUri = "�6�M��)uk譹1(�h`$�o�N>mĕ����cS2�dh![ę� ���`csB�!��OSxP�{�"