Alerting: Allow config api receiver create/delete with alertingApiServer (#95207)

Previously all receiver modifications were denied with alertingApiServer
enabled. This allows pure creates and deletes through as these specific
cases can be handled simply and without risk of rbac shenanigans.

pkg/services/ngalert/api/api_alertmanager_guards.go

@@ -38,17 +38,20 @@ func (srv AlertmanagerSrv) k8sApiServiceGuard(currentConfig apimodels.GettableUs
 	// - Since the UIDs stored in the database for the purposes of per-receiver RBAC are generated based on the receiver
 	//   name, we would need to ensure continuity of permissions when a receiver is renamed. This would, preferably,
 	//   require detecting renames and updating the permissions UID in the database.
-	// - It would need to determine newly created and deleted receivers so it can populate per-receiver access control defaults.
+	// - Editors don't have permission to manage all receiver permissions by default, only admin users do. This means that
+	//   certain combined operations (e.g. swapping the name of receivers) would require careful handling to ensure
+	//   that the correct permissions are maintained, and considering receivers don't have a unique identifier outside
+	//   their name, this is non-trivial.
 
 	// Neither of these are insurmountable, but considering this endpoint will be removed once FlagAlertingApiServer
 	// becomes GA, the complexity may not be worthwhile. To that end, for now we reject any request that attempts to
-	// modify receivers.
+	// update receivers, while allowing operations that add or remove receivers.
 	delta, err := calculateReceiversDelta(currentConfig.AlertmanagerConfig.Receivers, newConfig.AlertmanagerConfig.Receivers)
 	if err != nil {
 		return err
 	}
-	if !delta.IsEmpty() {
-		return fmt.Errorf("cannot modify receivers using this API while per-receiver RBAC is enabled; either disable the `alertingApiServer` feature flag or use an API that supports per-receiver RBAC (e.g. provisioning or receivers API)")
+	if len(delta.Updated) > 0 {
+		return fmt.Errorf("cannot update receivers using this API while per-receiver RBAC is enabled; either disable the `alertingApiServer` feature flag or use an API that supports per-receiver RBAC (e.g. provisioning or receivers API)")
 	}
 	return nil
 }

pkg/tests/apis/alerting/notifications/receivers/receiver_test.go

@@ -14,6 +14,7 @@ import (
 	"testing"
 
 	"github.com/grafana/alerting/notify"
+	"github.com/prometheus/alertmanager/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -1127,14 +1128,65 @@ func TestIntegrationRejectConfigApiReceiverModification(t *testing.T) {
 	cliCfg := helper.Org1.Admin.NewRestConfig()
 	legacyCli := alerting.NewAlertingLegacyAPIClient(helper.GetEnv().Server.HTTPServer.Listener.Addr().String(), cliCfg.Username, cliCfg.Password)
 
-	// This config has new and modified receivers.
+	adminK8sClient, err := versioned.NewForConfig(cliCfg)
+	require.NoError(t, err)
+	adminClient := adminK8sClient.NotificationsV0alpha1().Receivers("default")
+
 	alertmanagerRaw, err := testData.ReadFile(path.Join("test-data", "notification-settings.json"))
 	require.NoError(t, err)
 	var amConfig definitions.PostableUserConfig
 	require.NoError(t, json.Unmarshal(alertmanagerRaw, &amConfig))
 
+	persistInitialConfig(t, amConfig, adminClient, legacyCli)
+
+	// We modify the receiver, this should cause the POST to be rejected.
+	userDefinedReceiver := amConfig.AlertmanagerConfig.Receivers[slices.IndexFunc(amConfig.AlertmanagerConfig.Receivers, func(receiver *definitions.PostableApiReceiver) bool {
+		return receiver.Receiver.Name == "user-defined"
+	})]
+	userDefinedReceiver.GrafanaManagedReceivers[0].DisableResolveMessage = !userDefinedReceiver.GrafanaManagedReceivers[0].DisableResolveMessage
 	success, err := legacyCli.PostConfiguration(t, amConfig)
-	require.Falsef(t, success, "Expected receiver modification to be rejected, but got %s", err)
+	require.Falsef(t, success, "Expected receiver modification to be rejected, but got %t", success)
+	require.ErrorContainsf(t, err, "alertingApiServer", "Expected error message to contain 'alertingApiServer', but got %s", err)
+
+	// Revert the change.
+	userDefinedReceiver.GrafanaManagedReceivers[0].DisableResolveMessage = !userDefinedReceiver.GrafanaManagedReceivers[0].DisableResolveMessage
+
+	// We add a receiver, this should be accepted.
+	amConfig.AlertmanagerConfig.Receivers = append(amConfig.AlertmanagerConfig.Receivers, &definitions.PostableApiReceiver{
+		Receiver: config.Receiver{
+			Name: "new receiver",
+		},
+		PostableGrafanaReceivers: definitions.PostableGrafanaReceivers{
+			GrafanaManagedReceivers: []*definitions.PostableGrafanaReceiver{
+				{
+					Name:     "new receiver",
+					Type:     "email",
+					Settings: []byte(`{"addresses": "<some@email.com>"}`),
+				},
+			},
+		},
+	})
+	success, err = legacyCli.PostConfiguration(t, amConfig)
+	require.Truef(t, success, "Expected receiver modification to be accepted, but got %s", err)
+	require.NoError(t, err)
+
+	// Sanity check.
+	gettable, status, body := legacyCli.GetAlertmanagerConfigWithStatus(t)
+	require.Equalf(t, http.StatusOK, status, body)
+	require.Lenf(t, gettable.AlertmanagerConfig.Receivers, 3, "Expected 3 receivers, got %d", len(gettable.AlertmanagerConfig.Receivers))
+
+	// We remove the receiver, this should be accepted.
+	amConfig.AlertmanagerConfig.Receivers = slices.DeleteFunc(amConfig.AlertmanagerConfig.Receivers, func(receiver *definitions.PostableApiReceiver) bool {
+		return receiver.GrafanaManagedReceivers[0].Name == "new receiver"
+	})
+	success, err = legacyCli.PostConfiguration(t, amConfig)
+	require.Truef(t, success, "Expected receiver modification to be accepted, but got %s", err)
+	require.NoError(t, err)
+
+	// Sanity check.
+	gettable, status, body = legacyCli.GetAlertmanagerConfigWithStatus(t)
+	require.Equalf(t, http.StatusOK, status, body)
+	require.Lenf(t, gettable.AlertmanagerConfig.Receivers, 2, "Expected 2 receivers, got %d", len(gettable.AlertmanagerConfig.Receivers))
 }
 
 func TestIntegrationReferentialIntegrity(t *testing.T) {