Alerting: Reject receiver update in config API when FlagAlertingApiServer enabled (#93300)

* Reject receiver update in config API when FlagAlertingApiServer enabled

pkg/services/ngalert/api/api.go

@@ -96,10 +96,11 @@ func (api *API) RegisterAPIEndpoints(m *metrics.API) {
 		api.DatasourceCache,
 		NewLotexAM(proxy, logger),
 		&AlertmanagerSrv{
-			crypto: api.MultiOrgAlertmanager.Crypto,
-			log:    logger,
-			ac:     api.AccessControl,
-			mam:    api.MultiOrgAlertmanager,
+			crypto:         api.MultiOrgAlertmanager.Crypto,
+			log:            logger,
+			ac:             api.AccessControl,
+			mam:            api.MultiOrgAlertmanager,
+			featureManager: api.FeatureManager,
 			silenceSvc: notifier.NewSilenceService(
 				accesscontrol.NewSilenceService(api.AccessControl, api.RuleStore),
 				api.TransactionManager,

pkg/services/ngalert/api/api_alertmanager.go

@@ -15,6 +15,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
@@ -28,11 +29,12 @@ const (
 )
 
 type AlertmanagerSrv struct {
-	log        log.Logger
-	ac         accesscontrol.AccessControl
-	mam        *notifier.MultiOrgAlertmanager
-	crypto     notifier.Crypto
-	silenceSvc SilenceService
+	log            log.Logger
+	ac             accesscontrol.AccessControl
+	mam            *notifier.MultiOrgAlertmanager
+	crypto         notifier.Crypto
+	silenceSvc     SilenceService
+	featureManager featuremgmt.FeatureToggles
 }
 
 type UnknownReceiverError struct {
@@ -195,6 +197,18 @@ func (srv AlertmanagerSrv) RoutePostAlertingConfig(c *contextmodel.ReqContext, b
 			return ErrResp(http.StatusBadRequest, err, "")
 		}
 	}
+	if srv.featureManager.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingApiServer) {
+		if err != nil {
+			// Unclear if returning an error here is the right thing to do, preventing the user from posting a new config
+			// when the current one is legitimately invalid is not optimal, but we need to ensure receiver
+			// permissions are maintained and prevent potential access control bypasses. The workaround is to use the
+			// various new k8s API endpoints to fix the configuration.
+			return ErrResp(http.StatusInternalServerError, err, "")
+		}
+		if err := srv.k8sApiServiceGuard(currentConfig, body); err != nil {
+			return ErrResp(http.StatusBadRequest, err, "")
+		}
+	}
 	err = srv.mam.SaveAndApplyAlertmanagerConfiguration(c.Req.Context(), c.SignedInUser.GetOrgID(), body)
 	if err == nil {
 		return response.JSON(http.StatusAccepted, util.DynMap{"message": "configuration created"})

pkg/services/ngalert/api/api_alertmanager_guards.go

@@ -32,6 +32,27 @@ func (srv AlertmanagerSrv) provenanceGuard(currentConfig apimodels.GettableUserC
 	return nil
 }
 
+func (srv AlertmanagerSrv) k8sApiServiceGuard(currentConfig apimodels.GettableUserConfig, newConfig apimodels.PostableUserConfig) error {
+	// Modifications to receivers via this API is tricky with new per-receiver RBAC. Assuming we restrict the API to only
+	// those users with global edit permissions, we would still need to consider the following:
+	// - 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.
+
+	// 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.
+	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)")
+	}
+	return nil
+}
+
 func checkRoutes(currentConfig apimodels.GettableUserConfig, newConfig apimodels.PostableUserConfig) error {
 	reporter := cmputil.DiffReporter{}
 	options := []cmp.Option{cmp.Reporter(&reporter), cmpopts.EquateEmpty(), cmpopts.IgnoreUnexported(labels.Matcher{})}

pkg/services/ngalert/api/api_alertmanager_test.go

@@ -569,11 +569,12 @@ func createSut(t *testing.T) AlertmanagerSrv {
 	ruleStore := ngfakes.NewRuleStore(t)
 	ruleAuthzService := accesscontrol.NewRuleService(acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()))
 	return AlertmanagerSrv{
-		mam:        mam,
-		crypto:     mam.Crypto,
-		ac:         ac,
-		log:        log,
-		silenceSvc: notifier.NewSilenceService(accesscontrol.NewSilenceService(ac, ruleStore), ruleStore, log, mam, ruleStore, ruleAuthzService),
+		mam:            mam,
+		crypto:         mam.Crypto,
+		ac:             ac,
+		log:            log,
+		featureManager: featuremgmt.WithFeatures(),
+		silenceSvc:     notifier.NewSilenceService(accesscontrol.NewSilenceService(ac, ruleStore), ruleStore, log, mam, ruleStore, ruleAuthzService),
 	}
 }
 

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

@@ -24,6 +24,7 @@ import (
 	common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
 	"github.com/grafana/grafana/pkg/apis/alerting_notifications/v0alpha1"
 	"github.com/grafana/grafana/pkg/generated/clientset/versioned"
+	notificationsv0alpha1 "github.com/grafana/grafana/pkg/generated/clientset/versioned/typed/alerting_notifications/v0alpha1"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
@@ -426,6 +427,10 @@ func TestIntegrationInUseMetadata(t *testing.T) {
 	cliCfg := helper.Org1.Admin.NewRestConfig()
 	legacyCli := alerting.NewAlertingLegacyAPIClient(helper.GetEnv().Server.HTTPServer.Listener.Addr().String(), cliCfg.Username, cliCfg.Password)
 
+	adminK8sClient, err := versioned.NewForConfig(cliCfg)
+	require.NoError(t, err)
+	adminClient := adminK8sClient.NotificationsV0alpha1().Receivers("default")
+
 	// Prepare environment and create notification policy and rule that use receiver
 	alertmanagerRaw, err := testData.ReadFile(path.Join("test-data", "notification-settings.json"))
 	require.NoError(t, err)
@@ -440,8 +445,7 @@ func TestIntegrationInUseMetadata(t *testing.T) {
 	parentRoute.Routes = []*definitions.Route{&route1, &route2}
 	amConfig.AlertmanagerConfig.Route.Routes = append(amConfig.AlertmanagerConfig.Route.Routes, &parentRoute)
 
-	success, err := legacyCli.PostConfiguration(t, amConfig)
-	require.Truef(t, success, "Failed to post Alertmanager configuration: %s", err)
+	persistInitialConfig(t, amConfig, adminClient, legacyCli)
 
 	postGroupRaw, err := testData.ReadFile(path.Join("test-data", "rulegroup-1.json"))
 	require.NoError(t, err)
@@ -471,10 +475,6 @@ func TestIntegrationInUseMetadata(t *testing.T) {
 	_, status, data := legacyCli.PostRulesGroupWithStatus(t, folderUID, &ruleGroup)
 	require.Equalf(t, http.StatusAccepted, status, "Failed to post Rule: %s", data)
 
-	adminK8sClient, err := versioned.NewForConfig(cliCfg)
-	require.NoError(t, err)
-	adminClient := adminK8sClient.NotificationsV0alpha1().Receivers("default")
-
 	requestReceivers := func(t *testing.T, title string) (v0alpha1.Receiver, v0alpha1.Receiver) {
 		t.Helper()
 		receivers, err := adminClient.List(ctx, v1.ListOptions{})
@@ -508,7 +508,7 @@ func TestIntegrationInUseMetadata(t *testing.T) {
 
 	// Removing the new extra route should leave only 1.
 	amConfig.AlertmanagerConfig.Route.Routes = amConfig.AlertmanagerConfig.Route.Routes[:1]
-	success, err = legacyCli.PostConfiguration(t, amConfig)
+	success, err := legacyCli.PostConfiguration(t, amConfig)
 	require.Truef(t, success, "Failed to post Alertmanager configuration: %s", err)
 
 	receiverListed, receiverGet = requestReceivers(t, "user-defined")
@@ -786,6 +786,26 @@ func TestIntegrationPatch(t *testing.T) {
 	})
 }
 
+func TestIntegrationRejectConfigApiReceiverModification(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
+	helper := getTestHelper(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.
+	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))
+
+	success, err := legacyCli.PostConfiguration(t, amConfig)
+	require.Falsef(t, success, "Expected receiver modification to be rejected, but got %s", err)
+}
+
 func TestIntegrationReferentialIntegrity(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
@@ -802,14 +822,17 @@ func TestIntegrationReferentialIntegrity(t *testing.T) {
 	cliCfg := helper.Org1.Admin.NewRestConfig()
 	legacyCli := alerting.NewAlertingLegacyAPIClient(helper.GetEnv().Server.HTTPServer.Listener.Addr().String(), cliCfg.Username, cliCfg.Password)
 
+	adminK8sClient, err := versioned.NewForConfig(cliCfg)
+	require.NoError(t, err)
+	adminClient := adminK8sClient.NotificationsV0alpha1().Receivers("default")
+
 	// Prepare environment and create notification policy and rule that use time receiver
 	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))
 
-	success, err := legacyCli.PostConfiguration(t, amConfig)
-	require.Truef(t, success, "Failed to post Alertmanager configuration: %s", err)
+	persistInitialConfig(t, amConfig, adminClient, legacyCli)
 
 	postGroupRaw, err := testData.ReadFile(path.Join("test-data", "rulegroup-1.json"))
 	require.NoError(t, err)
@@ -821,10 +844,6 @@ func TestIntegrationReferentialIntegrity(t *testing.T) {
 	_, status, data := legacyCli.PostRulesGroupWithStatus(t, folderUID, &ruleGroup)
 	require.Equalf(t, http.StatusAccepted, status, "Failed to post Rule: %s", data)
 
-	adminK8sClient, err := versioned.NewForConfig(cliCfg)
-	require.NoError(t, err)
-	adminClient := adminK8sClient.NotificationsV0alpha1().Receivers("default")
-
 	receivers, err := adminClient.List(ctx, v1.ListOptions{})
 	require.NoError(t, err)
 	require.Len(t, receivers.Items, 2)
@@ -1171,14 +1190,72 @@ func TestIntegrationReceiverListSelector(t *testing.T) {
 	})
 }
 
+// persistInitialConfig helps create an initial config with new receivers using legacy json. Config API blocks receiver
+// modifications, so we need to use k8s API to create new receivers before posting the config.
+func persistInitialConfig(t *testing.T, amConfig definitions.PostableUserConfig, adminClient notificationsv0alpha1.ReceiverInterface, legacyCli alerting.LegacyApiClient) {
+	ctx := context.Background()
+
+	var defaultReceiver *definitions.PostableApiReceiver
+	for _, receiver := range amConfig.AlertmanagerConfig.Receivers {
+		if receiver.Name == "grafana-default-email" {
+			defaultReceiver = receiver
+			continue
+		}
+
+		toCreate := v0alpha1.Receiver{
+			ObjectMeta: v1.ObjectMeta{
+				Namespace: "default",
+			},
+			Spec: v0alpha1.ReceiverSpec{
+				Title:        receiver.Name,
+				Integrations: []v0alpha1.Integration{},
+			},
+		}
+
+		for _, integration := range receiver.GrafanaManagedReceivers {
+			settings := common.Unstructured{}
+			require.NoError(t, settings.UnmarshalJSON(integration.Settings))
+			toCreate.Spec.Integrations = append(toCreate.Spec.Integrations, v0alpha1.Integration{
+				Settings:              settings,
+				Type:                  integration.Type,
+				DisableResolveMessage: util.Pointer(false),
+			})
+		}
+
+		created, err := adminClient.Create(ctx, &toCreate, v1.CreateOptions{})
+		require.NoError(t, err)
+
+		for i, integration := range created.Spec.Integrations {
+			receiver.GrafanaManagedReceivers[i].UID = *integration.Uid
+		}
+	}
+
+	success, err := legacyCli.PostConfiguration(t, amConfig)
+	require.Truef(t, success, "Failed to post Alertmanager configuration: %s", err)
+
+	gettable, status, body := legacyCli.GetAlertmanagerConfigWithStatus(t)
+	require.Equalf(t, http.StatusOK, status, body)
+
+	idx := slices.IndexFunc(gettable.AlertmanagerConfig.Receivers, func(recv *definitions.GettableApiReceiver) bool {
+		return recv.Name == "grafana-default-email"
+	})
+	gettableDefault := gettable.AlertmanagerConfig.Receivers[idx]
+
+	// Assign uid of default receiver as well.
+	defaultReceiver.GrafanaManagedReceivers[0].UID = gettableDefault.GrafanaManagedReceivers[0].UID
+}
+
 func createIntegration(t *testing.T, integrationType string) v0alpha1.Integration {
 	cfg, ok := notify.AllKnownConfigsForTesting[integrationType]
 	require.Truef(t, ok, "no known config for integration type %s", integrationType)
+	return createIntegrationWithSettings(t, integrationType, cfg.Config)
+}
+func createIntegrationWithSettings(t *testing.T, integrationType string, settingsJson string) v0alpha1.Integration {
 	settings := common.Unstructured{}
-	require.NoError(t, settings.UnmarshalJSON([]byte(cfg.Config)))
+	require.NoError(t, settings.UnmarshalJSON([]byte(settingsJson)))
 	return v0alpha1.Integration{
 		Settings:              settings,
-		Type:                  cfg.NotifierType,
+		Type:                  integrationType,
 		DisableResolveMessage: util.Pointer(false),
 	}
 }

pkg/tests/apis/alerting/notifications/receivers/test-data/notification-settings.json

@@ -23,8 +23,9 @@
         "grafana_managed_receiver_configs": [
           {
             "type": "email",
+            "name": "email receiver",
             "settings": {
-              "addresses": "example@email.com"
+              "addresses": "<example@email.com>"
             }
           }
         ]
@@ -34,6 +35,7 @@
         "grafana_managed_receiver_configs": [
           {
             "type": "email",
+            "name": "user-defined",
             "settings": {
               "addresses": "example@email.com"
             }
@@ -42,4 +44,4 @@
       }
     ]
   }
-}
+}

pkg/tests/apis/alerting/notifications/timeinterval/test-data/notification-settings.json

@@ -45,6 +45,7 @@
         "grafana_managed_receiver_configs": [
           {
             "type": "email",
+            "name": "email receiver",
             "settings": {
               "addresses": "<example@email.com>"
             }
@@ -53,4 +54,4 @@
       }
     ]
   }
-}
+}