[Alerting]: Assign UUID to grafana receivers (#34241)

* [Alerting]: Assign UUID to grafana receivers

* Apply suggestions from code review

* Add test for updating invalid receiver

Co-authored-by: Domas <domasx2@gmail.com>

pkg/services/ngalert/api/api_alertmanager.go

@@ -1,7 +1,6 @@
 package api
 
 import (
-	"encoding/base64"
 	"errors"
 	"fmt"
 	"net/http"
@@ -13,7 +12,6 @@ import (
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
-	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -86,6 +84,13 @@ func (srv AlertmanagerSrv) RouteGetAlertingConfig(c *models.ReqContext) response
 		for _, pr := range recv.PostableGrafanaReceivers.GrafanaManagedReceivers {
 			secureFields := make(map[string]bool, len(pr.SecureSettings))
 			for k := range pr.SecureSettings {
+				decryptedValue, err := pr.GetDecryptedSecret(k)
+				if err != nil {
+					return response.Error(http.StatusInternalServerError, fmt.Sprintf("failed to decrypt stored secure setting: %s", k), err)
+				}
+				if decryptedValue == "" {
+					continue
+				}
 				secureFields[k] = true
 			}
 			gr := apimodels.GettableGrafanaReceiver{
@@ -191,49 +196,43 @@ func (srv AlertmanagerSrv) RoutePostAlertingConfig(c *models.ReqContext, body ap
 	if err != nil {
 		return response.Error(http.StatusInternalServerError, "failed to load lastest configuration", err)
 	}
+	currentReceiverMap := currentConfig.GetGrafanaReceiverMap()
 
 	// Copy the previously known secure settings
 	for i, r := range body.AlertmanagerConfig.Receivers {
 		for j, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
-			if len(currentConfig.AlertmanagerConfig.Receivers) <= i { // this is a receiver we don't have any stored for - skip it.
+			if gr.UID == "" { // new receiver
 				continue
 			}
-			cr := currentConfig.AlertmanagerConfig.Receivers[i]
 
-			if len(cr.PostableGrafanaReceivers.GrafanaManagedReceivers) <= j { //  this is a receiver we don't have anything stored for - skip it.
+			cgmr, ok := currentReceiverMap[gr.UID]
-				continue
+			if !ok {
+				// it tries to update a receiver that didn't previously exist
+				return response.Error(http.StatusBadRequest, fmt.Sprintf("unknown receiver: %s", gr.UID), nil)
 			}
-			cgmr := cr.PostableGrafanaReceivers.GrafanaManagedReceivers[j]
 
-			//TODO: We use the name and type to match current stored receivers againt sent ones, but we should ideally use something unique e.g. UUID
+			// frontend sends only the secure settings that have to be updated
-			if cgmr.Name == gr.Name && cgmr.Type == gr.Type {
+			// therefore we have to copy from the last configuration only those secure settings not included in the request
-				// frontend sends only the secure settings that have to be updated
+			for key := range cgmr.SecureSettings {
-				// therefore we have to copy from the last configuration only those secure settings not included in the request
+				_, ok := body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings[key]
-				for key, storedValue := range cgmr.SecureSettings {
+				if !ok {
-					_, ok := body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings[key]
+					decryptedValue, err := cgmr.GetDecryptedSecret(key)
-					if !ok {
+					if err != nil {
-						decodeValue, err := base64.StdEncoding.DecodeString(storedValue)
+						return response.Error(http.StatusInternalServerError, fmt.Sprintf("failed to decrypt stored secure setting: %s", key), err)
-						if err != nil {
-							return response.Error(http.StatusInternalServerError, fmt.Sprintf("failed to decode stored secure setting: %s", key), err)
-						}
-						decryptedValue, err := util.Decrypt(decodeValue, setting.SecretKey)
-						if err != nil {
-							return response.Error(http.StatusInternalServerError, fmt.Sprintf("failed to decrypt stored secure setting: %s", key), err)
-						}
-
-						if body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings == nil {
-							body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings = make(map[string]string, len(cgmr.SecureSettings))
-						}
-
-						body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings[key] = string(decryptedValue)
 					}
+
+					if body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings == nil {
+						body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings = make(map[string]string, len(cgmr.SecureSettings))
+					}
+
+					body.AlertmanagerConfig.Receivers[i].PostableGrafanaReceivers.GrafanaManagedReceivers[j].SecureSettings[key] = decryptedValue
 				}
 			}
 		}
 	}
 
-	if err := body.EncryptSecureSettings(); err != nil {
+	if err := body.ProcessConfig(); err != nil {
-		return response.Error(http.StatusInternalServerError, "failed to encrypt receiver secrets", err)
+		return response.Error(http.StatusInternalServerError, "failed to post process Alertmanager configuration", err)
 	}
 
 	if err := srv.am.SaveAndApplyConfig(&body); err != nil {

pkg/services/ngalert/api/tooling/definitions/alertmanager.go

@@ -245,7 +245,24 @@ func (c *PostableUserConfig) validate() error {
 	return nil
 }
 
-func (c *PostableUserConfig) EncryptSecureSettings() error {
+// GetGrafanaReceiverMap returns a map that associates UUIDs to grafana receivers
+func (c *PostableUserConfig) GetGrafanaReceiverMap() map[string]*PostableGrafanaReceiver {
+	UIDs := make(map[string]*PostableGrafanaReceiver)
+	for _, r := range c.AlertmanagerConfig.Receivers {
+		switch r.Type() {
+		case GrafanaReceiverType:
+			for _, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers {
+				UIDs[gr.UID] = gr
+			}
+		default:
+		}
+	}
+	return UIDs
+}
+
+// ProcessConfig parses grafana receivers, encrypts secrets and assigns UUIDs (if they are missing)
+func (c *PostableUserConfig) ProcessConfig() error {
+	seenUIDs := make(map[string]struct{})
 	// encrypt secure settings for storing them in DB
 	for _, r := range c.AlertmanagerConfig.Receivers {
 		switch r.Type() {
@@ -258,6 +275,21 @@ func (c *PostableUserConfig) EncryptSecureSettings() error {
 					}
 					gr.SecureSettings[k] = base64.StdEncoding.EncodeToString(encryptedData)
 				}
+				if gr.UID == "" {
+					retries := 5
+					for i := 0; i < retries; i++ {
+						gen := util.GenerateShortUID()
+						_, ok := seenUIDs[gen]
+						if !ok {
+							gr.UID = gen
+							break
+						}
+					}
+					if gr.UID == "" {
+						return fmt.Errorf("all %d attempts to generate UID for receiver have failed; please retry", retries)
+					}
+				}
+				seenUIDs[gr.UID] = struct{}{}
 			}
 		default:
 		}
@@ -353,6 +385,21 @@ func (c *GettableUserConfig) MarshalJSON() ([]byte, error) {
 	return json.Marshal(tmp)
 }
 
+// GetGrafanaReceiverMap returns a map that associates UUIDs to grafana receivers
+func (c *GettableUserConfig) GetGrafanaReceiverMap() map[string]*GettableGrafanaReceiver {
+	UIDs := make(map[string]*GettableGrafanaReceiver)
+	for _, r := range c.AlertmanagerConfig.Receivers {
+		switch r.Type() {
+		case GrafanaReceiverType:
+			for _, gr := range r.GettableGrafanaReceivers.GrafanaManagedReceivers {
+				UIDs[gr.UID] = gr
+			}
+		default:
+		}
+	}
+	return UIDs
+}
+
 type GettableApiAlertingConfig struct {
 	Config `yaml:",inline"`
 
@@ -521,6 +568,22 @@ type PostableGrafanaReceiver struct {
 	SecureSettings        map[string]string `json:"secureSettings"`
 }
 
+func (r *PostableGrafanaReceiver) GetDecryptedSecret(key string) (string, error) {
+	storedValue, ok := r.SecureSettings[key]
+	if !ok {
+		return "", nil
+	}
+	decodeValue, err := base64.StdEncoding.DecodeString(storedValue)
+	if err != nil {
+		return "", err
+	}
+	decryptedValue, err := util.Decrypt(decodeValue, setting.SecretKey)
+	if err != nil {
+		return "", err
+	}
+	return string(decryptedValue), nil
+}
+
 type ReceiverType int
 
 const (

pkg/tests/api/alerting/api_alertmanager_configuration_test.go

@@ -1,12 +1,15 @@
 package alerting
 
 import (
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/tests/testinfra"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -74,6 +77,7 @@ func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
 	store := testinfra.SetUpDatabase(t, dir)
 	grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
 	alertConfigURL := fmt.Sprintf("http://%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
+	generatedUID := ""
 
 	// create a new configuration that has a secret
 	{
@@ -105,9 +109,56 @@ func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
 		resp := postRequest(t, alertConfigURL, payload, http.StatusAccepted) // nolint
 		require.JSONEq(t, `{"message":"configuration created"}`, getBody(t, resp.Body))
 	}
-	// Then, update the recipient
+
+	// Try to update a receiver with unknown UID
 	{
+		// Then, update the recipient
 		payload := `
+	{
+		"template_files": {},
+		"alertmanager_config": {
+			"route": {
+				"receiver": "slack.receiver"
+			},
+			"templates": null,
+			"receivers": [{
+				"name": "slack.receiver",
+				"grafana_managed_receiver_configs": [{
+					"settings": {
+						"recipient": "#unified-alerting-test-but-updated"
+					},
+					"secureFields": {
+						"url": true
+					},
+					"type": "slack",
+					"name": "slack.receiver",
+					"disableResolveMessage": false,
+					"uid": "invalid"
+				}]
+			}]
+		}
+	}
+	`
+
+		resp := postRequest(t, alertConfigURL, payload, http.StatusBadRequest) // nolint
+		require.JSONEq(t, `{"message": "unknown receiver: invalid"}`, getBody(t, resp.Body))
+	}
+
+	// The secure settings must be present
+	{
+		resp := getRequest(t, alertConfigURL, http.StatusOK) // nolint
+		var c definitions.GettableUserConfig
+		bb := getBody(t, resp.Body)
+		err := json.Unmarshal([]byte(bb), &c)
+		require.NoError(t, err)
+		m := c.GetGrafanaReceiverMap()
+		assert.Len(t, m, 1)
+		for k := range m {
+			generatedUID = m[k].UID
+		}
+
+		// Then, update the recipient
+		payload := fmt.Sprintf(`
 {
 	"template_files": {},
 	"alertmanager_config": {
@@ -126,20 +177,22 @@ func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
 				},
 				"type": "slack",
 				"name": "slack.receiver",
-				"disableResolveMessage": false
+				"disableResolveMessage": false,
+				"uid": %q
 			}]
 		}]
 	}
 }
-`
+`, generatedUID)
-		resp := postRequest(t, alertConfigURL, payload, http.StatusAccepted) // nolint
+
+		resp = postRequest(t, alertConfigURL, payload, http.StatusAccepted) // nolint
 		require.JSONEq(t, `{"message": "configuration created"}`, getBody(t, resp.Body))
 	}
 
 	// The secure settings must be present
 	{
 		resp := getRequest(t, alertConfigURL, http.StatusOK) // nolint
-		require.JSONEq(t, `
+		require.JSONEq(t, fmt.Sprintf(`
 {
 	"template_files": {},
 	"alertmanager_config": {
@@ -150,7 +203,7 @@ func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
 		"receivers": [{
 			"name": "slack.receiver",
 			"grafana_managed_receiver_configs": [{
-				"uid": "",
+				"uid": %q,
 				"name": "slack.receiver",
 				"type": "slack",
 				"disableResolveMessage": false,
@@ -164,6 +217,6 @@ func TestAlertmanagerConfigurationPersistSecrets(t *testing.T) {
 		}]
 	}
 }
-`, getBody(t, resp.Body))
+`, generatedUID), getBody(t, resp.Body))
 	}
 }

pkg/tests/api/alerting/api_notification_channel_test.go

@@ -67,8 +67,9 @@ func TestNotificationChannels(t *testing.T) {
 		alertsURL := fmt.Sprintf("http://grafana:password@%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)
 		resp := getRequest(t, alertsURL, http.StatusOK) // nolint
 		b := getBody(t, resp.Body)
+		re := regexp.MustCompile(`"uid":"([\w|-]*)"`)
 		e := getExpAlertmanagerConfigFromAPI(mockChannel.server.Addr)
-		require.JSONEq(t, e, b)
+		require.JSONEq(t, e, string(re.ReplaceAll([]byte(b), []byte(`"uid":""`))))
 	}
 
 	{

public/app/features/alerting/unified/utils/receiver-form.ts

@@ -224,6 +224,9 @@ function formChannelValuesToGrafanaChannelConfig(
     disableResolveMessage:
       values.disableResolveMessage ?? existing?.disableResolveMessage ?? defaults.disableResolveMessage,
   };
+  if (existing) {
+    channel.uid = existing.uid;
+  }
   return channel;
 }
 

public/app/plugins/datasource/alertmanager/types.ts

@@ -68,6 +68,7 @@ export type WebhookConfig = {
 };
 
 export type GrafanaManagedReceiverConfig = {
+  uid?: string;
   disableResolveMessage: boolean;
   secureFields?: Record<string, boolean>;
   secureSettings?: Record<string, unknown>;