Alerting: Validate upgraded receivers early to display in preview (#82956)

Previously receivers were only validated before saving the alertmanager
configuration. This is a suboptimal experience for those upgrading with preview
as the failed channel upgrade will return an API error instead of being
summarized in the table.

pkg/services/ngalert/migration/channel.go

@@ -3,10 +3,12 @@ package migration
 import (
 	"context"
 	"encoding/base64"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"time"
 
+	alertingNotify "github.com/grafana/alerting/notify"
 	"github.com/prometheus/alertmanager/pkg/labels"
 	"github.com/prometheus/common/model"
 
@@ -74,14 +76,38 @@ func (om *OrgMigration) createReceiver(c *legacymodels.AlertNotification) (*apim
 		return nil, err
 	}
 
-	return &apimodels.PostableGrafanaReceiver{
+	recv := &apimodels.PostableGrafanaReceiver{
 		UID:                   c.UID,
 		Name:                  c.Name,
 		Type:                  c.Type,
 		DisableResolveMessage: c.DisableResolveMessage,
 		Settings:              data,
 		SecureSettings:        secureSettings,
-	}, nil
+	}
+	err = validateReceiver(recv, om.encryptionService.GetDecryptedValue)
+	if err != nil {
+		return nil, err
+	}
+	return recv, nil
+}
+
+// validateReceiver validates a receiver by building the configuration and checking for errors.
+func validateReceiver(receiver *apimodels.PostableGrafanaReceiver, decrypt func(ctx context.Context, sjd map[string][]byte, key, fallback string) string) error {
+	var (
+		cfg = &alertingNotify.GrafanaIntegrationConfig{
+			UID:                   receiver.UID,
+			Name:                  receiver.Name,
+			Type:                  receiver.Type,
+			DisableResolveMessage: receiver.DisableResolveMessage,
+			Settings:              json.RawMessage(receiver.Settings),
+			SecureSettings:        receiver.SecureSettings,
+		}
+	)
+
+	_, err := alertingNotify.BuildReceiverConfiguration(context.Background(), &alertingNotify.APIReceiver{
+		GrafanaIntegrations: alertingNotify.GrafanaIntegrations{Integrations: []*alertingNotify.GrafanaIntegrationConfig{cfg}},
+	}, decrypt)
+	return err
 }
 
 // createRoute creates a route from a legacy notification channel, and matches using a label based on the channel UID.

pkg/services/ngalert/migration/channel_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -118,7 +119,7 @@ func createNotChannel(t *testing.T, uid string, id int64, name string, isDefault
 		Type:         "email",
 		SendReminder: frequency > 0,
 		Frequency:    frequency,
-		Settings:     simplejson.New(),
+		Settings:     simplejson.NewFromAny(map[string]any{"addresses": "example"}),
 		IsDefault:    isDefault,
 		Created:      now,
 		Updated:      now,
@@ -132,6 +133,20 @@ func createBasicNotChannel(t *testing.T, notType string) *legacymodels.AlertNoti
 	return a
 }
 
+func createBrokenNotChannel(t *testing.T) *legacymodels.AlertNotification {
+	t.Helper()
+	return &legacymodels.AlertNotification{
+		UID:  "uid",
+		ID:   1,
+		Name: "broken email",
+		Type: "email",
+		Settings: simplejson.NewFromAny(map[string]any{
+			"something": "some value", // Missing required field addresses.
+		}),
+		SecureSettings: map[string][]byte{},
+	}
+}
+
 func TestCreateReceivers(t *testing.T) {
 	tc := []struct {
 		name    string
@@ -154,6 +169,11 @@ func TestCreateReceivers(t *testing.T) {
 			channel: createBasicNotChannel(t, "sensu"),
 			expErr:  fmt.Errorf("'sensu': %w", ErrDiscontinued),
 		},
+		{
+			name:    "when channel is misconfigured return error",
+			channel: createBrokenNotChannel(t),
+			expErr:  errors.New(`failed to validate integration "broken email" (UID uid) of type "email": could not find addresses in settings`),
+		},
 	}
 
 	sqlStore := db.InitTestDB(t)
@@ -266,7 +286,7 @@ func TestMigrateNotificationChannelSecureSettings(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			service := NewTestMigrationService(t, sqlStore, nil)
 			m := service.newOrgMigration(1)
-			recv, err := m.createReceiver(tt.channel)
+			settings, secureSettings, err := m.migrateSettingsToSecureSettings(tt.channel.Type, tt.channel.Settings, tt.channel.SecureSettings)
 			if tt.expErr != nil {
 				require.Error(t, err)
 				require.EqualError(t, err, tt.expErr.Error())
@@ -274,6 +294,8 @@ func TestMigrateNotificationChannelSecureSettings(t *testing.T) {
 			}
 			require.NoError(t, err)
 
+			recv := createReceiverNoValidation(t, tt.channel, settings, secureSettings)
+
 			if len(tt.expRecv.SecureSettings) > 0 {
 				require.NotEqual(t, tt.expRecv, recv) // Make sure they were actually encrypted at first.
 			}
@@ -300,8 +322,9 @@ func TestMigrateNotificationChannelSecureSettings(t *testing.T) {
 							channel.SecureSettings[key] = []byte(legacyEncryptFn("secure " + key))
 						}
 					})
-					recv, err := m.createReceiver(channel)
+					settings, secure, err := m.migrateSettingsToSecureSettings(channel.Type, channel.Settings, channel.SecureSettings)
 					require.NoError(t, err)
+					recv := createReceiverNoValidation(t, channel, settings, secure)
 
 					require.Equal(t, nType, recv.Type)
 					if len(secureSettings) > 0 {
@@ -335,8 +358,9 @@ func TestMigrateNotificationChannelSecureSettings(t *testing.T) {
 							channel.Settings.Set(key, "secure "+key)
 						}
 					})
-					recv, err := m.createReceiver(channel)
+					settings, secure, err := m.migrateSettingsToSecureSettings(channel.Type, channel.Settings, channel.SecureSettings)
 					require.NoError(t, err)
+					recv := createReceiverNoValidation(t, channel, settings, secure)
 
 					require.Equal(t, nType, recv.Type)
 					if len(secureSettings) > 0 {
@@ -439,12 +463,26 @@ func TestSetupAlertmanagerConfig(t *testing.T) {
 	}
 }
 
+func createReceiverNoValidation(t *testing.T, c *legacymodels.AlertNotification, settings *simplejson.Json, secureSettings map[string]string) *apimodels.PostableGrafanaReceiver {
+	data, err := settings.MarshalJSON()
+	require.NoError(t, err)
+
+	return &apimodels.PostableGrafanaReceiver{
+		UID:                   c.UID,
+		Name:                  c.Name,
+		Type:                  c.Type,
+		DisableResolveMessage: c.DisableResolveMessage,
+		Settings:              data,
+		SecureSettings:        secureSettings,
+	}
+}
+
 func createPostableGrafanaReceiver(uid string, name string) *apimodels.PostableGrafanaReceiver {
 	return &apimodels.PostableGrafanaReceiver{
 		UID:            uid,
 		Type:           "email",
 		Name:           name,
-		Settings:       apimodels.RawMessage("{}"),
+		Settings:       apimodels.RawMessage(`{"addresses":"example"}`),
 		SecureSettings: map[string]string{},
 	}
 }

pkg/services/ngalert/migration/persist.go

@@ -7,8 +7,6 @@ import (
 	"fmt"
 	"sort"
 
-	alertingNotify "github.com/grafana/alerting/notify"
-
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	legacymodels "github.com/grafana/grafana/pkg/services/alerting/models"
@@ -558,24 +556,7 @@ func (sync *sync) extractChannels(ctx context.Context, alert *legacymodels.Alert
 func (sync *sync) validateAlertmanagerConfig(config *apiModels.PostableUserConfig) error {
 	for _, r := range config.AlertmanagerConfig.Receivers {
 		for _, gr := range r.GrafanaManagedReceivers {
-			data, err := gr.Settings.MarshalJSON()
-			if err != nil {
-				return err
-			}
-			var (
-				cfg = &alertingNotify.GrafanaIntegrationConfig{
-					UID:                   gr.UID,
-					Name:                  gr.Name,
-					Type:                  gr.Type,
-					DisableResolveMessage: gr.DisableResolveMessage,
-					Settings:              data,
-					SecureSettings:        gr.SecureSettings,
-				}
-			)
-
-			_, err = alertingNotify.BuildReceiverConfiguration(context.Background(), &alertingNotify.APIReceiver{
-				GrafanaIntegrations: alertingNotify.GrafanaIntegrations{Integrations: []*alertingNotify.GrafanaIntegrationConfig{cfg}},
-			}, sync.getDecryptedValue)
+			err := validateReceiver(gr, sync.getDecryptedValue)
 			if err != nil {
 				return err
 			}