Alerting: Fix config api POST provenance guard (#93244)

* Add failing tests

* Fix bug in provenance guard on renaming receivers or moving integrations

* Linting

pkg/services/ngalert/api/api_alertmanager_guards.go

@@ -3,6 +3,7 @@ package api
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
@@ -10,7 +11,6 @@ import (
 	amConfig "github.com/prometheus/alertmanager/config"
 	"github.com/prometheus/alertmanager/pkg/labels"
 
-	"github.com/grafana/grafana/pkg/infra/log"
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/util/cmputil"
@@ -23,7 +23,7 @@ func (srv AlertmanagerSrv) provenanceGuard(currentConfig apimodels.GettableUserC
 	if err := checkTemplates(currentConfig, newConfig); err != nil {
 		return err
 	}
-	if err := checkContactPoints(srv.log, currentConfig.AlertmanagerConfig.Receivers, newConfig.AlertmanagerConfig.Receivers); err != nil {
+	if err := checkContactPoints(currentConfig.AlertmanagerConfig.Receivers, newConfig.AlertmanagerConfig.Receivers); err != nil {
 		return err
 	}
 	if err := checkMuteTimes(currentConfig, newConfig); err != nil {
@@ -69,55 +69,109 @@ func checkTemplates(currentConfig apimodels.GettableUserConfig, newConfig apimod
 	return nil
 }
 
-func checkContactPoints(l log.Logger, currReceivers []*apimodels.GettableApiReceiver, newReceivers []*apimodels.PostableApiReceiver) error {
-	newCPs := make(map[string]*apimodels.PostableGrafanaReceiver)
+func checkContactPoints(currReceivers []*apimodels.GettableApiReceiver, newReceivers []*apimodels.PostableApiReceiver) error {
+	delta, err := calculateReceiversDelta(currReceivers, newReceivers)
+	if err != nil {
+		return err
+	}
+	delta = delta.ProvisionedSubset()
+	if !delta.IsEmpty() {
+		return fmt.Errorf("cannot modify provisioned contact points: %v", delta.String())
+	}
+	return nil
+}
+
+// calculateReceiversDelta calculates the changes to receivers between the current and new configuration.
+func calculateReceiversDelta(currReceivers []*apimodels.GettableApiReceiver, newReceivers []*apimodels.PostableApiReceiver) (ReceiversDelta, error) {
+	newReceiversByName := make(map[string]*apimodels.PostableApiReceiver) // Receiver Name -> Integration UID -> ContactPoint
 	for _, postedReceiver := range newReceivers {
-		for _, postedContactPoint := range postedReceiver.GrafanaManagedReceivers {
-			newCPs[postedContactPoint.UID] = postedContactPoint
-		}
+		newReceiversByName[postedReceiver.Name] = postedReceiver
 	}
+	delta := ReceiversDelta{}
 	for _, existingReceiver := range currReceivers {
-		for _, contactPoint := range existingReceiver.GrafanaManagedReceivers {
-			if contactPoint.Provenance == apimodels.Provenance(ngmodels.ProvenanceNone) {
-				continue // we are only interested in non none
-			}
-			postedContactPoint, present := newCPs[contactPoint.UID]
-			if !present {
-				return fmt.Errorf("cannot delete provisioned contact point '%s'", contactPoint.Name)
-			}
-			editErr := fmt.Errorf("cannot save provisioned contact point '%s'", contactPoint.Name)
-			if contactPoint.DisableResolveMessage != postedContactPoint.DisableResolveMessage {
-				return editErr
-			}
-			if contactPoint.Name != postedContactPoint.Name {
-				return editErr
-			}
-			if contactPoint.Type != postedContactPoint.Type {
-				return editErr
-			}
-			for key := range contactPoint.SecureFields {
-				if value, present := postedContactPoint.SecureSettings[key]; present && value != "" {
-					return editErr
-				}
-			}
-			existingSettings := map[string]any{}
-			err := json.Unmarshal(contactPoint.Settings, &existingSettings)
-			if err != nil {
-				return err
-			}
-			newSettings := map[string]any{}
-			err = json.Unmarshal(postedContactPoint.Settings, &newSettings)
-			if err != nil {
-				return err
-			}
-			d := cmp.Diff(existingSettings, newSettings)
-			if len(d) > 0 {
-				l.Warn("Settings of contact point with provenance status cannot be changed via regular API.", "contactPoint", postedContactPoint.Name, "settingsDiff", d, "error", editErr)
-				return editErr
-			}
+		postedReceiver, present := newReceiversByName[existingReceiver.Name]
+		if !present {
+			delta.Deleted = append(delta.Deleted, existingReceiver) // Receiver has been deleted.
+			continue
+		}
+		// Keep track of which new receivers existed in the old config so we can add the rest to the created list.
+		delete(newReceiversByName, existingReceiver.Name)
+
+		updated, err := receiverUpdated(existingReceiver, postedReceiver)
+		if err != nil {
+			return ReceiversDelta{}, err
+		}
+		if updated {
+			delta.Updated = append(delta.Updated, existingReceiver) // Integration has been updated.
 		}
 	}
-	return nil
+
+	for _, postedContactPoint := range newReceiversByName {
+		delta.Created = append(delta.Created, postedContactPoint) // New receiver has been added.
+	}
+
+	return delta, nil
+}
+
+// receiverUpdated returns true if the existing and posted receivers differ.
+func receiverUpdated(existing *apimodels.GettableApiReceiver, posted *apimodels.PostableApiReceiver) (bool, error) {
+	newCPs := make(map[string]*apimodels.PostableGrafanaReceiver)
+	for _, postedContactPoint := range posted.GrafanaManagedReceivers {
+		newCPs[postedContactPoint.UID] = postedContactPoint
+	}
+
+	// Check if integrations have been modified.
+	for _, contactPoint := range existing.GrafanaManagedReceivers {
+		postedContactPoint, present := newCPs[contactPoint.UID]
+		if !present {
+			return true, nil // Integration has been removed.
+		}
+		// Keep track of which new integrations existed in the old config so we can detect if any new integrations have been added.
+		delete(newCPs, contactPoint.UID)
+
+		updated, err := integrationUpdated(contactPoint, postedContactPoint)
+		if err != nil {
+			return false, err
+		}
+		if updated {
+			return true, nil // Integration has been updated.
+		}
+	}
+	return len(newCPs) > 0, nil // New integrations have been added.
+}
+
+// integrationUpdated returns true if the existing and posted integrations differ.
+func integrationUpdated(existing *apimodels.GettableGrafanaReceiver, posted *apimodels.PostableGrafanaReceiver) (bool, error) {
+	if existing.DisableResolveMessage != posted.DisableResolveMessage {
+		return true, nil
+	}
+	if existing.Name != posted.Name {
+		return true, nil
+	}
+	if existing.Type != posted.Type {
+		return true, nil
+	}
+	for key := range existing.SecureFields {
+		if value, present := posted.SecureSettings[key]; present && value != "" {
+			return true, nil
+		}
+	}
+	existingSettings := map[string]any{}
+	err := json.Unmarshal(existing.Settings, &existingSettings)
+	if err != nil {
+		return false, err
+	}
+	newSettings := map[string]any{}
+	err = json.Unmarshal(posted.Settings, &newSettings)
+	if err != nil {
+		return false, err
+	}
+	d := cmp.Diff(existingSettings, newSettings)
+	if len(d) > 0 {
+		return true, nil
+	}
+
+	return false, nil
 }
 
 func checkMuteTimes(currentConfig apimodels.GettableUserConfig, newConfig apimodels.PostableUserConfig) error {
@@ -153,3 +207,86 @@ func checkMuteTimes(currentConfig apimodels.GettableUserConfig, newConfig apimod
 	}
 	return nil
 }
+
+// ReceiversDelta represents the changes to receivers in the alertmanager configuration.
+type ReceiversDelta struct {
+	Created []*apimodels.PostableApiReceiver
+	Updated []*apimodels.GettableApiReceiver
+	Deleted []*apimodels.GettableApiReceiver
+}
+
+// ProvisionedSubset returns a subset of the delta containing only integrations that were provisioned.
+func (d ReceiversDelta) ProvisionedSubset() ReceiversDelta {
+	subset := ReceiversDelta{}
+
+	for _, cp := range d.Updated {
+		if hasProvisionIntegration(cp) {
+			subset.Updated = append(subset.Updated, cp)
+		}
+	}
+
+	for _, cp := range d.Deleted {
+		if hasProvisionIntegration(cp) {
+			subset.Deleted = append(subset.Deleted, cp)
+		}
+	}
+
+	// Don't include created integrations in the subset, as they cannot have been provisioned.
+	return subset
+}
+
+func hasProvisionIntegration(gettable *apimodels.GettableApiReceiver) bool {
+	for _, integration := range gettable.GrafanaManagedReceivers {
+		if integration.Provenance != apimodels.Provenance(ngmodels.ProvenanceNone) {
+			return true
+		}
+	}
+	return false
+}
+
+// IsEmpty returns true if the delta contains no changes.
+func (d ReceiversDelta) IsEmpty() bool {
+	return len(d.Created) == 0 && len(d.Updated) == 0 && len(d.Deleted) == 0
+}
+
+// String returns a human-readable representation of the delta for error messages.
+func (d ReceiversDelta) String() string {
+	res := strings.Builder{}
+	if len(d.Created) > 0 {
+		res.WriteString("created: ")
+	}
+	for i, cp := range d.Created {
+		if i > 0 {
+			res.WriteString(", ")
+		}
+		res.WriteString(cp.Name)
+	}
+
+	if len(d.Updated) > 0 {
+		if res.Len() > 0 {
+			res.WriteString(", ")
+		}
+		res.WriteString("updated: ")
+	}
+	for i, cp := range d.Updated {
+		if i > 0 {
+			res.WriteString(", ")
+		}
+		res.WriteString(cp.Name)
+	}
+
+	if len(d.Deleted) > 0 {
+		if res.Len() > 0 {
+			res.WriteString(", ")
+		}
+		res.WriteString("deleted: ")
+	}
+	for i, cp := range d.Deleted {
+		if i > 0 {
+			res.WriteString(", ")
+		}
+		res.WriteString(cp.Name)
+	}
+
+	return res.String()
+}

pkg/services/ngalert/api/api_alertmanager_guards_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 
-	"github.com/grafana/grafana/pkg/infra/log/logtest"
 	"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
 )
@@ -308,10 +307,45 @@ func TestCheckContactPoints(t *testing.T) {
 				}(),
 			},
 		},
+		{
+			name:      "editing name of a provisioned receiver should fail",
+			shouldErr: true,
+			currentConfig: []*definitions.GettableApiReceiver{
+				defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+			},
+			newConfig: []*definitions.PostableApiReceiver{
+				func() *definitions.PostableApiReceiver {
+					receiver := defaultPostableReceiver(t, "test-1")
+					receiver.Name = "updated name"
+					return receiver
+				}(),
+			},
+		},
+		{
+			name:      "Moving provisioned integration to different receiver should fail",
+			shouldErr: true,
+			currentConfig: []*definitions.GettableApiReceiver{
+				defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-2", models.ProvenanceAPI),
+			},
+			newConfig: []*definitions.PostableApiReceiver{
+				func() *definitions.PostableApiReceiver { // Move integration from test-1 to test-2
+					receiver := defaultPostableReceiver(t, "test-1")
+					receiver.GrafanaManagedReceivers = []*definitions.PostableGrafanaReceiver{}
+					return receiver
+				}(),
+				func() *definitions.PostableApiReceiver {
+					receiver2 := defaultPostableReceiver(t, "test-2")
+					integration1 := defaultPostableReceiver(t, "test-1").GrafanaManagedReceivers[0]
+					receiver2.GrafanaManagedReceivers = append(receiver2.GrafanaManagedReceivers, integration1)
+					return receiver2
+				}(),
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := checkContactPoints(&logtest.Fake{}, test.currentConfig, test.newConfig)
+			err := checkContactPoints(test.currentConfig, test.newConfig)
 			if test.shouldErr {
 				require.Error(t, err)
 			} else {
@@ -321,14 +355,131 @@ func TestCheckContactPoints(t *testing.T) {
 	}
 }
 
+func TestReceiversDelta(t *testing.T) {
+	tests := []struct {
+		name          string
+		currentConfig []*definitions.GettableApiReceiver
+		newConfig     []*definitions.PostableApiReceiver
+		expectedDelta ReceiversDelta
+	}{
+		{
+			name: "no changes",
+			currentConfig: []*definitions.GettableApiReceiver{
+				defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-2", models.ProvenanceAPI),
+			},
+			newConfig: []*definitions.PostableApiReceiver{
+				defaultPostableReceiver(t, "test-1"),
+				defaultPostableReceiver(t, "test-2"),
+			},
+			expectedDelta: ReceiversDelta{},
+		},
+		{
+			name: "create one",
+			currentConfig: []*definitions.GettableApiReceiver{
+				defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+			},
+			newConfig: []*definitions.PostableApiReceiver{
+				defaultPostableReceiver(t, "test-1"),
+				defaultPostableReceiver(t, "test-2"),
+			},
+			expectedDelta: ReceiversDelta{
+				Created: []*definitions.PostableApiReceiver{
+					defaultPostableReceiver(t, "test-2"),
+				},
+			},
+		},
+		{
+			name: "delete one",
+			currentConfig: []*definitions.GettableApiReceiver{
+				defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-2", models.ProvenanceAPI),
+			},
+			newConfig: []*definitions.PostableApiReceiver{
+				defaultPostableReceiver(t, "test-1"),
+			},
+			expectedDelta: ReceiversDelta{
+				Deleted: []*definitions.GettableApiReceiver{
+					defaultGettableReceiver(t, "test-2", models.ProvenanceAPI),
+				},
+			},
+		},
+		{
+			name: "update some",
+			currentConfig: []*definitions.GettableApiReceiver{
+				defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-2", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-3", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-4", models.ProvenanceAPI),
+				defaultGettableReceiver(t, "test-5", models.ProvenanceAPI),
+			},
+			newConfig: []*definitions.PostableApiReceiver{
+				func() *definitions.PostableApiReceiver {
+					receiver := defaultPostableReceiver(t, "test-1")
+					receiver.GrafanaManagedReceivers[0].Settings = definitions.RawMessage(`{ "hello": "data", "data": { "test": "test"}}`)
+					return receiver
+				}(),
+				func() *definitions.PostableApiReceiver {
+					receiver := defaultPostableReceiver(t, "test-2")
+					receiver.Name = "updated name"
+					return receiver
+				}(),
+				func() *definitions.PostableApiReceiver {
+					receiver := defaultPostableReceiver(t, "test-3")
+					receiver.GrafanaManagedReceivers[0].DisableResolveMessage = !receiver.GrafanaManagedReceivers[0].DisableResolveMessage
+					return receiver
+				}(),
+				func() *definitions.PostableApiReceiver {
+					receiver := defaultPostableReceiver(t, "test-4")
+					receiver.GrafanaManagedReceivers[0].Name = "updated name"
+					return receiver
+				}(),
+				func() *definitions.PostableApiReceiver {
+					receiver := defaultPostableReceiver(t, "test-5")
+					receiver.GrafanaManagedReceivers = append(receiver.GrafanaManagedReceivers, defaultPostableReceiver(t, "test-1").GrafanaManagedReceivers[0])
+					return receiver
+				}(),
+			},
+			expectedDelta: ReceiversDelta{
+				Updated: []*definitions.GettableApiReceiver{
+					defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
+					defaultGettableReceiver(t, "test-3", models.ProvenanceAPI),
+					defaultGettableReceiver(t, "test-4", models.ProvenanceAPI),
+					defaultGettableReceiver(t, "test-5", models.ProvenanceAPI),
+				},
+				Created: []*definitions.PostableApiReceiver{
+					func() *definitions.PostableApiReceiver {
+						receiver := defaultPostableReceiver(t, "test-2")
+						receiver.Name = "updated name"
+						return receiver
+					}(),
+				},
+				Deleted: []*definitions.GettableApiReceiver{
+					defaultGettableReceiver(t, "test-2", models.ProvenanceAPI),
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			delta, err := calculateReceiversDelta(test.currentConfig, test.newConfig)
+			require.NoError(t, err)
+			require.Equalf(t, test.expectedDelta, delta, "unexpected delta: %q, expected: %q", delta.String(), test.expectedDelta.String())
+		})
+	}
+}
+
 func defaultGettableReceiver(t *testing.T, uid string, provenance models.Provenance) *definitions.GettableApiReceiver {
 	t.Helper()
 	return &definitions.GettableApiReceiver{
+		Receiver: amConfig.Receiver{
+			Name: uid,
+		},
 		GettableGrafanaReceivers: definitions.GettableGrafanaReceivers{
 			GrafanaManagedReceivers: []*definitions.GettableGrafanaReceiver{
 				{
-					UID:                   "123",
-					Name:                  "yeah",
+					UID:                   uid,
+					Name:                  uid,
 					Type:                  "slack",
 					DisableResolveMessage: true,
 					Provenance:            definitions.Provenance(provenance),
@@ -348,11 +499,14 @@ func defaultGettableReceiver(t *testing.T, uid string, provenance models.Provena
 func defaultPostableReceiver(t *testing.T, uid string) *definitions.PostableApiReceiver {
 	t.Helper()
 	return &definitions.PostableApiReceiver{
+		Receiver: amConfig.Receiver{
+			Name: uid,
+		},
 		PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
 			GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
 				{
-					UID:                   "123",
-					Name:                  "yeah",
+					UID:                   uid,
+					Name:                  uid,
 					Type:                  "slack",
 					DisableResolveMessage: true,
 					Settings: definitions.RawMessage(`{