Alerting: Create algorithm to process receiver changes and keep them consistent internally (#50738)

* Algorithm to fix up receivers

* Extract for tests

* Add tests, fix bug

* Add test which demonstrates how it fixes up broken groups

* Fix package prefix

pkg/services/ngalert/provisioning/contactpoints.go

@@ -252,21 +252,12 @@ func (ecp *ContactPointService) UpdateContactPoint(ctx context.Context, orgID in
 	if err != nil {
 		return err
 	}
-	for _, receiver := range revision.cfg.AlertmanagerConfig.Receivers {
+
-		if receiver.Name == contactPoint.Name {
+	configModified := stitchReceiver(revision.cfg, mergedReceiver)
-			receiverNotFound := true
+	if !configModified {
-			for i, grafanaReceiver := range receiver.GrafanaManagedReceivers {
+		return fmt.Errorf("contact point with uid '%s' not found", mergedReceiver.UID)
-				if grafanaReceiver.UID == mergedReceiver.UID {
-					receiverNotFound = false
-					receiver.GrafanaManagedReceivers[i] = mergedReceiver
-					break
-				}
-			}
-			if receiverNotFound {
-				return fmt.Errorf("contact point with uid '%s' not found", mergedReceiver.UID)
-			}
-		}
 	}
+
 	data, err := json.Marshal(revision.cfg)
 	if err != nil {
 		return err
@@ -378,3 +369,73 @@ func (ecp *ContactPointService) encryptValue(value string) (string, error) {
 	}
 	return base64.StdEncoding.EncodeToString(encryptedData), nil
 }
+
+// stitchReceiver modifies a receiver, target, in an alertmanager config. It modifies the given config in-place.
+// Returns true if the config was altered in any way, and false otherwise.
+func stitchReceiver(cfg *apimodels.PostableUserConfig, target *apimodels.PostableGrafanaReceiver) bool {
+	// Algorithm to fix up receivers. Receivers are very complex and depend heavily on internal consistency.
+	// All receivers in a given receiver group have the same name. We must maintain this across renames.
+	configModified := false
+groupLoop:
+	for _, receiverGroup := range cfg.AlertmanagerConfig.Receivers {
+		// Does the current group contain the grafana receiver we're interested in?
+		for i, grafanaReceiver := range receiverGroup.GrafanaManagedReceivers {
+			if grafanaReceiver.UID == target.UID {
+				// If it's a basic field change, simply replace it. Done!
+				//
+				// NOTE:
+				// In a "normal" database, receiverGroup.Name should always == grafanaReceiver.Name.
+				// Check it regardless.
+				// If these values are out of sync due to some bug elsewhere in the code, let's fix it up.
+				// Our receiver group fixing logic below will handle it.
+				if grafanaReceiver.Name == target.Name && receiverGroup.Name == grafanaReceiver.Name {
+					receiverGroup.GrafanaManagedReceivers[i] = target
+					configModified = true
+					break groupLoop
+				}
+
+				// If we're renaming, we'll need to fix up the macro receiver group for consistency.
+				// Firstly, if we're the only receiver in the group, simply rename the group to match. Done!
+				if len(receiverGroup.GrafanaManagedReceivers) == 1 {
+					receiverGroup.Name = target.Name
+					receiverGroup.GrafanaManagedReceivers[i] = target
+					configModified = true
+					break groupLoop
+				}
+
+				// Otherwise, we only want to rename the receiver we are touching... NOT all of them.
+				// Check to see whether a different group with the name we want already exists.
+				for i, candidateExistingGroup := range cfg.AlertmanagerConfig.Receivers {
+					// If so, put our modified receiver into that group. Done!
+					if candidateExistingGroup.Name == target.Name {
+						// Drop it from the old group...
+						receiverGroup.GrafanaManagedReceivers = append(receiverGroup.GrafanaManagedReceivers[:i], receiverGroup.GrafanaManagedReceivers[i+1:]...)
+						// Add the modified receiver to the new group...
+						candidateExistingGroup.GrafanaManagedReceivers = append(candidateExistingGroup.GrafanaManagedReceivers, target)
+						configModified = true
+						break groupLoop
+					}
+				}
+
+				// Doesn't exist? Create a new group just for the receiver.
+				newGroup := &apimodels.PostableApiReceiver{
+					Receiver: config.Receiver{
+						Name: target.Name,
+					},
+					PostableGrafanaReceivers: apimodels.PostableGrafanaReceivers{
+						GrafanaManagedReceivers: []*apimodels.PostableGrafanaReceiver{
+							target,
+						},
+					},
+				}
+				cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, newGroup)
+				// Drop it from the old spot.
+				receiverGroup.GrafanaManagedReceivers = append(receiverGroup.GrafanaManagedReceivers[:i], receiverGroup.GrafanaManagedReceivers[i+1:]...)
+				configModified = true
+				break groupLoop
+			}
+		}
+	}
+
+	return configModified
+}

pkg/services/ngalert/provisioning/contactpoints_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/secrets/database"
 	"github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
+	"github.com/prometheus/alertmanager/config"
 	"github.com/stretchr/testify/require"
 )
 
@@ -254,3 +255,407 @@ func createTestContactPoint() definitions.EmbeddedContactPoint {
 		Settings: settings,
 	}
 }
+
+func TestStitchReceivers(t *testing.T) {
+	type testCase struct {
+		name        string
+		initial     *definitions.PostableUserConfig
+		new         *definitions.PostableGrafanaReceiver
+		expModified bool
+		expCfg      definitions.PostableApiAlertingConfig
+	}
+
+	cases := []testCase{
+		{
+			name: "non matching receiver by UID, no change",
+			new: &definitions.PostableGrafanaReceiver{
+				UID: "does not exist",
+			},
+			expModified: false,
+			expCfg:      createTestConfigWithReceivers().AlertmanagerConfig,
+		},
+		{
+			name: "matching receiver with unchanged name, replaces",
+			new: &definitions.PostableGrafanaReceiver{
+				UID:  "ghi",
+				Name: "receiver-2",
+				Type: "teams",
+			},
+			expModified: true,
+			expCfg: definitions.PostableApiAlertingConfig{
+				Receivers: []*definitions.PostableApiReceiver{
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-1",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "abc",
+									Name: "receiver-1",
+									Type: "slack",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-2",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "def",
+									Name: "receiver-2",
+									Type: "slack",
+								},
+								{
+									UID:  "ghi",
+									Name: "receiver-2",
+									Type: "teams",
+								},
+								{
+									UID:  "jkl",
+									Name: "receiver-2",
+									Type: "discord",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "rename with only one receiver in group, renames group",
+			new: &definitions.PostableGrafanaReceiver{
+				UID:  "abc",
+				Name: "new-receiver",
+				Type: "slack",
+			},
+			expModified: true,
+			expCfg: definitions.PostableApiAlertingConfig{
+				Receivers: []*definitions.PostableApiReceiver{
+					{
+						Receiver: config.Receiver{
+							Name: "new-receiver",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "abc",
+									Name: "new-receiver",
+									Type: "slack",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-2",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "def",
+									Name: "receiver-2",
+									Type: "slack",
+								},
+								{
+									UID:  "ghi",
+									Name: "receiver-2",
+									Type: "email",
+								},
+								{
+									UID:  "jkl",
+									Name: "receiver-2",
+									Type: "discord",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "rename to another existing group, moves receiver",
+			new: &definitions.PostableGrafanaReceiver{
+				UID:  "def",
+				Name: "receiver-1",
+				Type: "slack",
+			},
+			expModified: true,
+			expCfg: definitions.PostableApiAlertingConfig{
+				Receivers: []*definitions.PostableApiReceiver{
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-1",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "abc",
+									Name: "receiver-1",
+									Type: "slack",
+								},
+								{
+									UID:  "def",
+									Name: "receiver-1",
+									Type: "slack",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-2",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "ghi",
+									Name: "receiver-2",
+									Type: "email",
+								},
+								{
+									UID:  "jkl",
+									Name: "receiver-2",
+									Type: "discord",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "rename to a name that doesn't exist, creates new group and moves",
+			new: &definitions.PostableGrafanaReceiver{
+				UID:  "jkl",
+				Name: "brand-new-group",
+				Type: "opsgenie",
+			},
+			expModified: true,
+			expCfg: definitions.PostableApiAlertingConfig{
+				Receivers: []*definitions.PostableApiReceiver{
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-1",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "abc",
+									Name: "receiver-1",
+									Type: "slack",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-2",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "def",
+									Name: "receiver-2",
+									Type: "slack",
+								},
+								{
+									UID:  "ghi",
+									Name: "receiver-2",
+									Type: "email",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "brand-new-group",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "jkl",
+									Name: "brand-new-group",
+									Type: "opsgenie",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:    "rename an inconsistent group in the database, algorithm fixes it",
+			initial: createInconsistentTestConfigWithReceivers(),
+			new: &definitions.PostableGrafanaReceiver{
+				UID:  "ghi",
+				Name: "brand-new-group",
+				Type: "opsgenie",
+			},
+			expModified: true,
+			expCfg: definitions.PostableApiAlertingConfig{
+				Receivers: []*definitions.PostableApiReceiver{
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-1",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "abc",
+									Name: "receiver-1",
+									Type: "slack",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "receiver-2",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "def",
+									Name: "receiver-2",
+									Type: "slack",
+								},
+								{
+									UID:  "jkl",
+									Name: "receiver-2",
+									Type: "discord",
+								},
+							},
+						},
+					},
+					{
+						Receiver: config.Receiver{
+							Name: "brand-new-group",
+						},
+						PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+							GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+								{
+									UID:  "ghi",
+									Name: "brand-new-group",
+									Type: "opsgenie",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			cfg := createTestConfigWithReceivers()
+			if c.initial != nil {
+				cfg = c.initial
+			}
+
+			modified := stitchReceiver(cfg, c.new)
+
+			require.Equal(t, c.expModified, modified)
+			require.Equal(t, c.expCfg, cfg.AlertmanagerConfig)
+		})
+	}
+}
+
+func createTestConfigWithReceivers() *definitions.PostableUserConfig {
+	return &definitions.PostableUserConfig{
+		AlertmanagerConfig: definitions.PostableApiAlertingConfig{
+			Receivers: []*definitions.PostableApiReceiver{
+				{
+					Receiver: config.Receiver{
+						Name: "receiver-1",
+					},
+					PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+						GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+							{
+								UID:  "abc",
+								Name: "receiver-1",
+								Type: "slack",
+							},
+						},
+					},
+				},
+				{
+					Receiver: config.Receiver{
+						Name: "receiver-2",
+					},
+					PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+						GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+							{
+								UID:  "def",
+								Name: "receiver-2",
+								Type: "slack",
+							},
+							{
+								UID:  "ghi",
+								Name: "receiver-2",
+								Type: "email",
+							},
+							{
+								UID:  "jkl",
+								Name: "receiver-2",
+								Type: "discord",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+// This is an invalid config, with inconsistently named receivers (intentionally).
+func createInconsistentTestConfigWithReceivers() *definitions.PostableUserConfig {
+	return &definitions.PostableUserConfig{
+		AlertmanagerConfig: definitions.PostableApiAlertingConfig{
+			Receivers: []*definitions.PostableApiReceiver{
+				{
+					Receiver: config.Receiver{
+						Name: "receiver-1",
+					},
+					PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+						GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+							{
+								UID:  "abc",
+								Name: "receiver-1",
+								Type: "slack",
+							},
+						},
+					},
+				},
+				{
+					Receiver: config.Receiver{
+						Name: "receiver-2",
+					},
+					PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+						GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+							{
+								UID:  "def",
+								Name: "receiver-2",
+								Type: "slack",
+							},
+							{
+								UID:  "ghi",
+								Name: "receiver-3",
+								Type: "email",
+							},
+							{
+								UID:  "jkl",
+								Name: "receiver-2",
+								Type: "discord",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}