Alerting: Rule api to fail update if provisioned rules are affected (#50835)

* add function that checks whether changes mention provisioned rules
* update API that updates group of rules to fail if check does not pass

pkg/services/ngalert/CHANGELOG.md

@@ -45,6 +45,7 @@ Scopes must have an order to ensure consistency and ease of search, this helps u
 
 ## Grafana Alerting - main / unreleased
 
+- [CHANGE] Rule API to reject request to update rules that affects provisioned rules #50835
 - [FEATURE] use optimistic lock by version field when updating alert rules #50274
 - [ENHANCEMENT] Scheduler: Drop ticks if rule evaluation is too slow and adds a metric grafana_alerting_schedule_rule_evaluations_missed_total to track missed evaluations per rule #48885
 - [ENHANCEMENT] Ticker to tick at predictable time #50197

pkg/services/ngalert/api/api_ruler.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
@@ -41,7 +42,8 @@ type RulerSrv struct {
 }
 
 var (
-	errQuotaReached = errors.New("quota has been exceeded")
+	errQuotaReached        = errors.New("quota has been exceeded")
+	errProvisionedResource = errors.New("request affects resources created via provisioning API")
 )
 
 // RouteDeleteAlertRules deletes all alert rules user is authorized to access in the namespace (request parameter :Namespace)
@@ -341,7 +343,6 @@ func (srv RulerSrv) RoutePostNameRulesConfig(c *models.ReqContext, ruleGroupConf
 
 // updateAlertRulesInGroup calculates changes (rules to add,update,delete), verifies that the user is authorized to do the calculated changes and updates database.
 // All operations are performed in a single transaction
-// nolint: gocyclo
 func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, groupKey ngmodels.AlertRuleGroupKey, rules []*ngmodels.AlertRule) response.Response {
 	var finalChanges *changes
 	hasAccess := accesscontrol.HasAccess(srv.ac, c)
@@ -368,45 +369,11 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, groupKey ngmod
 			}
 		}
 
-		provenances, err := srv.provenanceStore.GetProvenances(c.Req.Context(), c.OrgId, (&ngmodels.AlertRule{}).ResourceType())
-		if err != nil {
+		if err := verifyProvisionedRulesNotAffected(c.Req.Context(), srv.provenanceStore, c.OrgId, groupChanges); err != nil {
 			return err
 		}
 
-		// New rules don't need to be checked for provenance, just copy the whole slice.
-		finalChanges = &changes{}
-		finalChanges.New = groupChanges.New
-		for _, rule := range groupChanges.Update {
-			if provenance, exists := provenances[rule.Existing.UID]; (exists && provenance == ngmodels.ProvenanceNone) || !exists {
-				finalChanges.Update = append(finalChanges.Update, rule)
-			}
-		}
-		for _, rule := range groupChanges.Delete {
-			if provenance, exists := provenances[rule.UID]; (exists && provenance == ngmodels.ProvenanceNone) || !exists {
-				finalChanges.Delete = append(finalChanges.Delete, rule)
-			}
-		}
-
-		if finalChanges.isEmpty() {
-			logger.Info("no changes detected that have 'none' provenance in the request. Do nothing",
-				"provenance_invalid_add", len(groupChanges.New),
-				"provenance_invalid_update", len(groupChanges.Update),
-				"provenance_invalid_delete", len(groupChanges.Delete))
-			return nil
-		}
-
-		if len(groupChanges.Delete) > len(finalChanges.Delete) {
-			logger.Info("provenance is not 'none' for one or many rules in the group that should be deleted. those rules will be skipped",
-				"expected", len(groupChanges.Delete),
-				"allowed", len(groupChanges.Delete))
-		}
-
-		if len(groupChanges.Update) > len(finalChanges.Update) {
-			logger.Info("provenance is not 'none' for one or many rules in the group that should be updated. those rules will be skipped",
-				"expected", len(groupChanges.Update),
-				"allowed", len(groupChanges.Update))
-		}
-
+		finalChanges = groupChanges
 		logger.Debug("updating database with the authorized changes", "add", len(finalChanges.New), "update", len(finalChanges.New), "delete", len(finalChanges.Delete))
 
 		if len(finalChanges.Update) > 0 || len(finalChanges.New) > 0 {
@@ -461,7 +428,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, groupKey ngmod
 	if err != nil {
 		if errors.Is(err, ngmodels.ErrAlertRuleNotFound) {
 			return ErrResp(http.StatusNotFound, err, "failed to update rule group")
-		} else if errors.Is(err, ngmodels.ErrAlertRuleFailedValidation) {
+		} else if errors.Is(err, ngmodels.ErrAlertRuleFailedValidation) || errors.Is(err, errProvisionedResource) {
 			return ErrResp(http.StatusBadRequest, err, "failed to update rule group")
 		} else if errors.Is(err, errQuotaReached) {
 			return ErrResp(http.StatusForbidden, err, "")
@@ -559,7 +526,9 @@ type ruleUpdate struct {
 }
 
 type changes struct {
-	GroupKey       ngmodels.AlertRuleGroupKey
+	GroupKey ngmodels.AlertRuleGroupKey
+	// AffectedGroups contains all rules of all groups that are affected by these changes.
+	// For example, during moving a rule from one group to another this map will contain all rules from two groups
 	AffectedGroups map[ngmodels.AlertRuleGroupKey][]*ngmodels.AlertRule
 	New            []*ngmodels.AlertRule
 	Update         []ruleUpdate
@@ -570,6 +539,32 @@ func (c *changes) isEmpty() bool {
 	return len(c.Update)+len(c.New)+len(c.Delete) == 0
 }
 
+// verifyProvisionedRulesNotAffected check that neither of provisioned alerts are affected by changes.
+// Returns errProvisionedResource if there is at least one rule in groups affected by changes that was provisioned.
+func verifyProvisionedRulesNotAffected(ctx context.Context, provenanceStore provisioning.ProvisioningStore, orgID int64, ch *changes) error {
+	provenances, err := provenanceStore.GetProvenances(ctx, orgID, (&ngmodels.AlertRule{}).ResourceType())
+	if err != nil {
+		return err
+	}
+	errorMsg := strings.Builder{}
+	for group, alertRules := range ch.AffectedGroups {
+		for _, rule := range alertRules {
+			if provenance, exists := provenances[rule.UID]; (exists && provenance == ngmodels.ProvenanceNone) || !exists {
+				continue
+			}
+			if errorMsg.Len() > 0 {
+				errorMsg.WriteRune(',')
+			}
+			errorMsg.WriteString(group.String())
+			break
+		}
+	}
+	if errorMsg.Len() == 0 {
+		return nil
+	}
+	return fmt.Errorf("%w: alert rule group [%s]", errProvisionedResource, errorMsg.String())
+}
+
 // calculateChanges calculates the difference between rules in the group in the database and the submitted rules. If a submitted rule has UID it tries to find it in the database (in other groups).
 // returns a list of rules that need to be added, updated and deleted. Deleted considered rules in the database that belong to the group but do not exist in the list of submitted rules.
 func calculateChanges(ctx context.Context, ruleStore store.RuleStore, groupKey ngmodels.AlertRuleGroupKey, submittedRules []*ngmodels.AlertRule) (*changes, error) {

pkg/services/ngalert/api/api_ruler_test.go

@@ -659,6 +659,67 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) {
 	})
 }
 
+func TestVerifyProvisionedRulesNotAffected(t *testing.T) {
+	orgID := rand.Int63()
+	group := models.GenerateGroupKey(orgID)
+	affectedGroups := make(map[models.AlertRuleGroupKey][]*models.AlertRule)
+	var allRules []*models.AlertRule
+	{
+		rules := models.GenerateAlertRules(rand.Intn(3)+1, models.AlertRuleGen(withGroupKey(group)))
+		allRules = append(allRules, rules...)
+		affectedGroups[group] = rules
+		for i := 0; i < rand.Intn(3)+1; i++ {
+			g := models.GenerateGroupKey(orgID)
+			rules := models.GenerateAlertRules(rand.Intn(3)+1, models.AlertRuleGen(withGroupKey(g)))
+			allRules = append(allRules, rules...)
+			affectedGroups[g] = rules
+		}
+	}
+	ch := &changes{
+		GroupKey:       group,
+		AffectedGroups: affectedGroups,
+	}
+
+	t.Run("should return error if at least one rule in affected groups is provisioned", func(t *testing.T) {
+		rand.Shuffle(len(allRules), func(i, j int) {
+			allRules[j], allRules[i] = allRules[i], allRules[j]
+		})
+		storeResult := make(map[string]models.Provenance, len(allRules))
+		storeResult[allRules[0].UID] = models.ProvenanceAPI
+		storeResult[allRules[1].UID] = models.ProvenanceFile
+
+		provenanceStore := &provisioning.MockProvisioningStore{}
+		provenanceStore.EXPECT().GetProvenances(mock.Anything, orgID, "alertRule").Return(storeResult, nil)
+
+		result := verifyProvisionedRulesNotAffected(context.Background(), provenanceStore, orgID, ch)
+		require.Error(t, result)
+		require.ErrorIs(t, result, errProvisionedResource)
+		assert.Contains(t, result.Error(), allRules[0].GetGroupKey().String())
+		assert.Contains(t, result.Error(), allRules[1].GetGroupKey().String())
+	})
+
+	t.Run("should return nil if all have ProvenanceNone", func(t *testing.T) {
+		storeResult := make(map[string]models.Provenance, len(allRules))
+		for _, rule := range allRules {
+			storeResult[rule.UID] = models.ProvenanceNone
+		}
+
+		provenanceStore := &provisioning.MockProvisioningStore{}
+		provenanceStore.EXPECT().GetProvenances(mock.Anything, orgID, "alertRule").Return(storeResult, nil)
+
+		result := verifyProvisionedRulesNotAffected(context.Background(), provenanceStore, orgID, ch)
+		require.NoError(t, result)
+	})
+
+	t.Run("should return nil if no alerts have provisioning status", func(t *testing.T) {
+		provenanceStore := &provisioning.MockProvisioningStore{}
+		provenanceStore.EXPECT().GetProvenances(mock.Anything, orgID, "alertRule").Return(make(map[string]models.Provenance, len(allRules)), nil)
+
+		result := verifyProvisionedRulesNotAffected(context.Background(), provenanceStore, orgID, ch)
+		require.NoError(t, result)
+	})
+}
+
 func createService(ac *acMock.Mock, store *store.FakeRuleStore, scheduler schedule.ScheduleService) *RulerSrv {
 	return &RulerSrv{
 		xactManager:     store,