[v10.0.x] Alerting: Fix unique violation when updating rule group with title chains/cycles (#70467)

* Alerting: Fix TestIntegration_GetAlertRulesForScheduling to make sure rules are created in different org (#69088)

make sure rules are created in different org

(cherry picked from commit b57ef1f2c7)

* Alerting: Fix flaky TestIntegrationUpdateAlertRules (#69106)

Prevents duplicate alert rule ids and 0 value for BaseInterval and IntervalSeconds

(cherry picked from commit 97ae6ae6ef)

* Alerting: Fix unique violation when updating rule group with title chains/cycles (#67868)

* Alerting: Fix unique violation when updating rule group with title chains/cycles

The uniqueness constraint for titles within an org+folder is enforced on every update within a transaction instead of on commit (deferred constraint). This means that there could be a set of updates that will throw a unique constraint violation in an intermediate step even though the final state is valid. For example, a chain of updates RuleA -> RuleB -> RuleC could fail if not executed in the correct order, or a swap of titles RuleA <-> RuleB cannot be executed in any order without violating the constraint.

The exact solution to this is complex and requires determining directed paths and cycles in the update graph, adding in temporary updates to break cycles, and then executing the updates in reverse topological order (see first commit in PR if curious).

This is not implemented here.

Instead, we choose a simpler solution that works in all cases but might perform more updates than necessary. This simpler solution makes a determination of whether an intermediate collision could occur and if so, adds a temporary title on all updated rules to break any cycles and remove the need for specific ordering.

In addition, we make sure diffs are executed in the following order: DELETES, UPDATES, INSERTS.

(cherry picked from commit 0c688190f7)

* Linting

---------

Co-authored-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>

pkg/services/ngalert/api/api_ruler.go

@@ -353,29 +353,7 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey
 		finalChanges = store.UpdateCalculatedRuleFields(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 {
-			updates := make([]ngmodels.UpdateRule, 0, len(finalChanges.Update))
-			inserts := make([]ngmodels.AlertRule, 0, len(finalChanges.New))
-			for _, update := range finalChanges.Update {
-				logger.Debug("updating rule", "rule_uid", update.New.UID, "diff", update.Diff.String())
-				updates = append(updates, ngmodels.UpdateRule{
-					Existing: update.Existing,
-					New:      *update.New,
-				})
-			}
-			for _, rule := range finalChanges.New {
-				inserts = append(inserts, *rule)
-			}
-			_, err = srv.store.InsertAlertRules(tranCtx, inserts)
-			if err != nil {
-				return fmt.Errorf("failed to add rules: %w", err)
-			}
-			err = srv.store.UpdateAlertRules(tranCtx, updates)
-			if err != nil {
-				return fmt.Errorf("failed to update rules: %w", err)
-			}
-		}
-
+		// Delete first as this could prevent future unique constraint violations.
 		if len(finalChanges.Delete) > 0 {
 			UIDs := make([]string, 0, len(finalChanges.Delete))
 			for _, rule := range finalChanges.Delete {
@@ -387,6 +365,32 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey
 			}
 		}
 
+		if len(finalChanges.Update) > 0 {
+			updates := make([]ngmodels.UpdateRule, 0, len(finalChanges.Update))
+			for _, update := range finalChanges.Update {
+				logger.Debug("updating rule", "rule_uid", update.New.UID, "diff", update.Diff.String())
+				updates = append(updates, ngmodels.UpdateRule{
+					Existing: update.Existing,
+					New:      *update.New,
+				})
+			}
+			err = srv.store.UpdateAlertRules(tranCtx, updates)
+			if err != nil {
+				return fmt.Errorf("failed to update rules: %w", err)
+			}
+		}
+
+		if len(finalChanges.New) > 0 {
+			inserts := make([]ngmodels.AlertRule, 0, len(finalChanges.New))
+			for _, rule := range finalChanges.New {
+				inserts = append(inserts, *rule)
+			}
+			_, err = srv.store.InsertAlertRules(tranCtx, inserts)
+			if err != nil {
+				return fmt.Errorf("failed to add rules: %w", err)
+			}
+		}
+
 		if len(finalChanges.New) > 0 {
 			limitReached, err := srv.QuotaService.CheckQuotaReached(tranCtx, ngmodels.QuotaTargetSrv, &quota.ScopeParameters{
 				OrgID:  c.OrgID,

pkg/services/ngalert/models/testing.go

@@ -140,6 +140,21 @@ func WithOrgID(orgId int64) AlertRuleMutator {
 	}
 }
 
+func WithUniqueOrgID() AlertRuleMutator {
+	orgs := map[int64]struct{}{}
+	return func(rule *AlertRule) {
+		var orgID int64
+		for {
+			orgID = rand.Int63()
+			if _, ok := orgs[orgID]; !ok {
+				break
+			}
+		}
+		orgs[orgID] = struct{}{}
+		rule.OrgID = orgID
+	}
+}
+
 func WithNamespace(namespace *folder.Folder) AlertRuleMutator {
 	return func(rule *AlertRule) {
 		rule.NamespaceUID = namespace.UID

pkg/services/ngalert/provisioning/alert_rules.go

@@ -260,53 +260,60 @@ func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, orgID int
 	}
 
 	return service.xact.InTransaction(ctx, func(ctx context.Context) error {
-		uids, err := service.ruleStore.InsertAlertRules(ctx, withoutNilAlertRules(delta.New))
-		if err != nil {
-			return fmt.Errorf("failed to insert alert rules: %w", err)
-		}
-		for uid := range uids {
-			if err := service.provenanceStore.SetProvenance(ctx, &models.AlertRule{UID: uid}, orgID, provenance); err != nil {
+		// Delete first as this could prevent future unique constraint violations.
+		if len(delta.Delete) > 0 {
+			for _, del := range delta.Delete {
+				// check that provenance is not changed in an invalid way
+				storedProvenance, err := service.provenanceStore.GetProvenance(ctx, del, orgID)
+				if err != nil {
+					return err
+				}
+				if canUpdate := canUpdateProvenanceInRuleGroup(storedProvenance, provenance); !canUpdate {
+					return fmt.Errorf("cannot update with provided provenance '%s', needs '%s'", provenance, storedProvenance)
+				}
+			}
+			if err := service.deleteRules(ctx, orgID, delta.Delete...); err != nil {
 				return err
 			}
 		}
 
-		updates := make([]models.UpdateRule, 0, len(delta.Update))
-		for _, update := range delta.Update {
-			// check that provenance is not changed in an invalid way
-			storedProvenance, err := service.provenanceStore.GetProvenance(ctx, update.New, orgID)
-			if err != nil {
-				return err
+		if len(delta.Update) > 0 {
+			updates := make([]models.UpdateRule, 0, len(delta.Update))
+			for _, update := range delta.Update {
+				// check that provenance is not changed in an invalid way
+				storedProvenance, err := service.provenanceStore.GetProvenance(ctx, update.New, orgID)
+				if err != nil {
+					return err
+				}
+				if canUpdate := canUpdateProvenanceInRuleGroup(storedProvenance, provenance); !canUpdate {
+					return fmt.Errorf("cannot update with provided provenance '%s', needs '%s'", provenance, storedProvenance)
+				}
+				updates = append(updates, models.UpdateRule{
+					Existing: update.Existing,
+					New:      *update.New,
+				})
 			}
-			if canUpdate := canUpdateProvenanceInRuleGroup(storedProvenance, provenance); !canUpdate {
-				return fmt.Errorf("cannot update with provided provenance '%s', needs '%s'", provenance, storedProvenance)
+			if err = service.ruleStore.UpdateAlertRules(ctx, updates); err != nil {
+				return fmt.Errorf("failed to update alert rules: %w", err)
 			}
-			updates = append(updates, models.UpdateRule{
-				Existing: update.Existing,
-				New:      *update.New,
-			})
-		}
-		if err = service.ruleStore.UpdateAlertRules(ctx, updates); err != nil {
-			return fmt.Errorf("failed to update alert rules: %w", err)
-		}
-		for _, update := range delta.Update {
-			if err := service.provenanceStore.SetProvenance(ctx, update.New, orgID, provenance); err != nil {
-				return err
+			for _, update := range delta.Update {
+				if err := service.provenanceStore.SetProvenance(ctx, update.New, orgID, provenance); err != nil {
+					return err
+				}
 			}
 		}
 
-		for _, delete := range delta.Delete {
-			// check that provenance is not changed in an invalid way
-			storedProvenance, err := service.provenanceStore.GetProvenance(ctx, delete, orgID)
+		if len(delta.New) > 0 {
+			uids, err := service.ruleStore.InsertAlertRules(ctx, withoutNilAlertRules(delta.New))
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to insert alert rules: %w", err)
 			}
-			if canUpdate := canUpdateProvenanceInRuleGroup(storedProvenance, provenance); !canUpdate {
-				return fmt.Errorf("cannot update with provided provenance '%s', needs '%s'", provenance, storedProvenance)
+			for uid := range uids {
+				if err := service.provenanceStore.SetProvenance(ctx, &models.AlertRule{UID: uid}, orgID, provenance); err != nil {
+					return err
+				}
 			}
 		}
-		if err := service.deleteRules(ctx, orgID, delta.Delete...); err != nil {
-			return err
-		}
 
 		if err = service.checkLimitsTransactionCtx(ctx, orgID, userID); err != nil {
 			return err

pkg/services/ngalert/provisioning/alert_rules_test.go

@@ -166,6 +166,210 @@ func TestAlertRuleService(t *testing.T) {
 		require.Equal(t, int64(2), readGroup.Rules[0].Version)
 	})
 
+	t.Run("updating a group to temporarily overlap rule names should not throw unique constraint", func(t *testing.T) {
+		var orgID int64 = 1
+		group := models.AlertRuleGroup{
+			Title:     "overlap-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("overlap-test-rule-1", orgID),
+				dummyRule("overlap-test-rule-2", orgID),
+			},
+		}
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "overlap-test")
+		require.NoError(t, err)
+
+		updatedGroup.Rules[0].Title = "overlap-test-rule-2"
+		updatedGroup.Rules[1].Title = "overlap-test-rule-3"
+		err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "overlap-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Len(t, readGroup.Rules, 2)
+		require.Equal(t, "overlap-test-rule-2", readGroup.Rules[0].Title)
+		require.Equal(t, "overlap-test-rule-3", readGroup.Rules[1].Title)
+		require.Equal(t, int64(3), readGroup.Rules[0].Version)
+		require.Equal(t, int64(3), readGroup.Rules[1].Version)
+	})
+
+	t.Run("updating a group to swap the name of two rules should not throw unique constraint", func(t *testing.T) {
+		var orgID int64 = 1
+		group := models.AlertRuleGroup{
+			Title:     "swap-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("swap-test-rule-1", orgID),
+				dummyRule("swap-test-rule-2", orgID),
+			},
+		}
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "swap-test")
+		require.NoError(t, err)
+
+		updatedGroup.Rules[0].Title = "swap-test-rule-2"
+		updatedGroup.Rules[1].Title = "swap-test-rule-1"
+		err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "swap-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Len(t, readGroup.Rules, 2)
+		require.Equal(t, "swap-test-rule-2", readGroup.Rules[0].Title)
+		require.Equal(t, "swap-test-rule-1", readGroup.Rules[1].Title)
+		require.Equal(t, int64(3), readGroup.Rules[0].Version) // Needed an extra update to break the update cycle.
+		require.Equal(t, int64(3), readGroup.Rules[1].Version)
+	})
+
+	t.Run("updating a group that has a rule name cycle should not throw unique constraint", func(t *testing.T) {
+		var orgID int64 = 1
+		group := models.AlertRuleGroup{
+			Title:     "cycle-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("cycle-test-rule-1", orgID),
+				dummyRule("cycle-test-rule-2", orgID),
+				dummyRule("cycle-test-rule-3", orgID),
+			},
+		}
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "cycle-test")
+		require.NoError(t, err)
+
+		updatedGroup.Rules[0].Title = "cycle-test-rule-2"
+		updatedGroup.Rules[1].Title = "cycle-test-rule-3"
+		updatedGroup.Rules[2].Title = "cycle-test-rule-1"
+		err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "cycle-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Len(t, readGroup.Rules, 3)
+		require.Equal(t, "cycle-test-rule-2", readGroup.Rules[0].Title)
+		require.Equal(t, "cycle-test-rule-3", readGroup.Rules[1].Title)
+		require.Equal(t, "cycle-test-rule-1", readGroup.Rules[2].Title)
+		require.Equal(t, int64(3), readGroup.Rules[0].Version) // Needed an extra update to break the update cycle.
+		require.Equal(t, int64(3), readGroup.Rules[1].Version)
+		require.Equal(t, int64(3), readGroup.Rules[2].Version)
+	})
+
+	t.Run("updating a group that has multiple rule name cycles should not throw unique constraint", func(t *testing.T) {
+		var orgID int64 = 1
+		group := models.AlertRuleGroup{
+			Title:     "multi-cycle-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("multi-cycle-test-rule-1", orgID),
+				dummyRule("multi-cycle-test-rule-2", orgID),
+
+				dummyRule("multi-cycle-test-rule-3", orgID),
+				dummyRule("multi-cycle-test-rule-4", orgID),
+				dummyRule("multi-cycle-test-rule-5", orgID),
+			},
+		}
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "multi-cycle-test")
+		require.NoError(t, err)
+
+		updatedGroup.Rules[0].Title = "multi-cycle-test-rule-2"
+		updatedGroup.Rules[1].Title = "multi-cycle-test-rule-1"
+
+		updatedGroup.Rules[2].Title = "multi-cycle-test-rule-4"
+		updatedGroup.Rules[3].Title = "multi-cycle-test-rule-5"
+		updatedGroup.Rules[4].Title = "multi-cycle-test-rule-3"
+
+		err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "multi-cycle-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Len(t, readGroup.Rules, 5)
+		require.Equal(t, "multi-cycle-test-rule-2", readGroup.Rules[0].Title)
+		require.Equal(t, "multi-cycle-test-rule-1", readGroup.Rules[1].Title)
+		require.Equal(t, "multi-cycle-test-rule-4", readGroup.Rules[2].Title)
+		require.Equal(t, "multi-cycle-test-rule-5", readGroup.Rules[3].Title)
+		require.Equal(t, "multi-cycle-test-rule-3", readGroup.Rules[4].Title)
+		require.Equal(t, int64(3), readGroup.Rules[0].Version) // Needed an extra update to break the update cycle.
+		require.Equal(t, int64(3), readGroup.Rules[1].Version)
+		require.Equal(t, int64(3), readGroup.Rules[2].Version) // Needed an extra update to break the update cycle.
+		require.Equal(t, int64(3), readGroup.Rules[3].Version)
+		require.Equal(t, int64(3), readGroup.Rules[4].Version)
+	})
+
+	t.Run("updating a group to recreate a rule using the same name should not throw unique constraint", func(t *testing.T) {
+		var orgID int64 = 1
+		group := models.AlertRuleGroup{
+			Title:     "recreate-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("recreate-test-rule-1", orgID),
+			},
+		}
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup := models.AlertRuleGroup{
+			Title:     "recreate-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("recreate-test-rule-1", orgID),
+			},
+		}
+		err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "recreate-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Len(t, readGroup.Rules, 1)
+		require.Equal(t, "recreate-test-rule-1", readGroup.Rules[0].Title)
+		require.Equal(t, int64(1), readGroup.Rules[0].Version)
+	})
+
+	t.Run("updating a group to create a rule that temporarily overlaps an existing should not throw unique constraint", func(t *testing.T) {
+		var orgID int64 = 1
+		group := models.AlertRuleGroup{
+			Title:     "create-overlap-test",
+			Interval:  60,
+			FolderUID: "my-namespace",
+			Rules: []models.AlertRule{
+				dummyRule("create-overlap-test-rule-1", orgID),
+			},
+		}
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "create-overlap-test")
+		require.NoError(t, err)
+		updatedGroup.Rules[0].Title = "create-overlap-test-rule-2"
+		updatedGroup.Rules = append(updatedGroup.Rules, dummyRule("create-overlap-test-rule-1", orgID))
+
+		err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+
+		readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "create-overlap-test")
+		require.NoError(t, err)
+		require.NotEmpty(t, readGroup.Rules)
+		require.Len(t, readGroup.Rules, 2)
+		require.Equal(t, "create-overlap-test-rule-2", readGroup.Rules[0].Title)
+		require.Equal(t, "create-overlap-test-rule-1", readGroup.Rules[1].Title)
+		require.Equal(t, int64(2), readGroup.Rules[0].Version)
+		require.Equal(t, int64(1), readGroup.Rules[1].Version)
+	})
+
 	t.Run("updating a group by updating a rule should not remove dashboard and panel ids", func(t *testing.T) {
 		var orgID int64 = 1
 		dashboardUid := "huYnkl7H"

pkg/services/ngalert/store/alert_rule.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/google/uuid"
+
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -181,6 +183,11 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu
 // UpdateAlertRules is a handler for updating alert rules.
 func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateRule) error {
 	return st.SQLStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
+		err := st.preventIntermediateUniqueConstraintViolations(sess, rules)
+		if err != nil {
+			return fmt.Errorf("failed when preventing intermediate unique constraint violation: %w", err)
+		}
+
 		ruleVersions := make([]ngmodels.AlertRuleVersion, 0, len(rules))
 		for _, r := range rules {
 			var parentVersion int64
@@ -232,6 +239,77 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR
 	})
 }
 
+// preventIntermediateUniqueConstraintViolations prevents unique constraint violations caused by an intermediate update.
+// The uniqueness constraint for titles within an org+folder is enforced on every update within a transaction
+// instead of on commit (deferred constraint). This means that there could be a set of updates that will throw
+// a unique constraint violation in an intermediate step even though the final state is valid.
+// For example, a chain of updates RuleA -> RuleB -> RuleC could fail if not executed in the correct order, or
+// a swap of titles RuleA <-> RuleB cannot be executed in any order without violating the constraint.
+func (st DBstore) preventIntermediateUniqueConstraintViolations(sess *db.Session, updates []ngmodels.UpdateRule) error {
+	// The exact solution to this is complex and requires determining directed paths and cycles in the update graph,
+	// adding in temporary updates to break cycles, and then executing the updates in reverse topological order.
+	// This is not implemented here. Instead, we choose a simpler solution that works in all cases but might perform
+	// more updates than necessary. This simpler solution makes a determination of whether an intermediate collision
+	// could occur and if so, adds a temporary title on all updated rules to break any cycles and remove the need for
+	// specific ordering.
+
+	titleUpdates := make([]ngmodels.UpdateRule, 0)
+	for _, update := range updates {
+		if update.Existing.Title != update.New.Title {
+			titleUpdates = append(titleUpdates, update)
+		}
+	}
+
+	// If there is no overlap then an intermediate unique constraint violation is not possible. If there is an overlap,
+	// then there is the possibility of intermediate unique constraint violation.
+	if !newTitlesOverlapExisting(titleUpdates) {
+		return nil
+	}
+	st.Logger.Debug("detected possible intermediate unique constraint violation, creating temporary title updates", "updates", len(titleUpdates))
+
+	for _, update := range titleUpdates {
+		r := update.Existing
+		u := uuid.New().String()
+
+		// Some defensive programming in case the temporary title is somehow persisted it will still be recognizable.
+		uniqueTempTitle := r.Title + u
+		if len(uniqueTempTitle) > AlertRuleMaxTitleLength {
+			uniqueTempTitle = r.Title[:AlertRuleMaxTitleLength-len(u)] + uuid.New().String()
+		}
+
+		if updated, err := sess.ID(r.ID).Cols("title").Update(&ngmodels.AlertRule{Title: uniqueTempTitle, Version: r.Version}); err != nil || updated == 0 {
+			if err != nil {
+				return fmt.Errorf("failed to set temporary rule title [%s] %s: %w", r.UID, r.Title, err)
+			}
+			return fmt.Errorf("%w: alert rule UID %s version %d", ErrOptimisticLock, r.UID, r.Version)
+		}
+		// Otherwise optimistic locking will conflict on the 2nd update.
+		r.Version++
+		// For consistency.
+		r.Title = uniqueTempTitle
+	}
+
+	return nil
+}
+
+// newTitlesOverlapExisting returns true if any new titles overlap with existing titles.
+// It does so in a case-insensitive manner as some supported databases perform case-insensitive comparisons.
+func newTitlesOverlapExisting(rules []ngmodels.UpdateRule) bool {
+	existingTitles := make(map[string]struct{}, len(rules))
+	for _, r := range rules {
+		existingTitles[strings.ToLower(r.Existing.Title)] = struct{}{}
+	}
+
+	// Check if there is any overlap between lower case existing and new titles.
+	for _, r := range rules {
+		if _, ok := existingTitles[strings.ToLower(r.New.Title)]; ok {
+			return true
+		}
+	}
+
+	return false
+}
+
 // CountAlertRulesInFolder is a handler for retrieving the number of alert rules of
 // specific organisation associated with a given namespace (parent folder).
 func (st DBstore) CountAlertRulesInFolder(ctx context.Context, query *ngmodels.CountAlertRulesQuery) (int64, error) {

pkg/services/ngalert/store/alert_rule_test.go

@@ -4,10 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/infra/log/logtest"
 	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
@@ -30,16 +32,18 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	cfg := setting.NewCfg()
-	cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{BaseInterval: time.Duration(rand.Int63n(100)) * time.Second}
+	cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{BaseInterval: time.Duration(rand.Int63n(100)+1) * time.Second}
 	sqlStore := db.InitTestDB(t)
 	store := &DBstore{
 		SQLStore:      sqlStore,
 		Cfg:           cfg.UnifiedAlerting,
 		FolderService: setupFolderService(t, sqlStore, cfg),
+		Logger:        &logtest.Fake{},
 	}
+	generator := models.AlertRuleGen(withIntervalMatching(store.Cfg.BaseInterval), models.WithUniqueID())
 
 	t.Run("should increase version", func(t *testing.T) {
-		rule := createRule(t, store)
+		rule := createRule(t, store, generator)
 		newRule := models.CopyRule(rule)
 		newRule.Title = util.GenerateShortUID()
 		err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
@@ -61,7 +65,7 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
 	})
 
 	t.Run("should fail due to optimistic locking if version does not match", func(t *testing.T) {
-		rule := createRule(t, store)
+		rule := createRule(t, store, generator)
 		rule.Version-- // simulate version discrepancy
 
 		newRule := models.CopyRule(rule)
@@ -77,6 +81,236 @@ func TestIntegrationUpdateAlertRules(t *testing.T) {
 	})
 }
 
+func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+	cfg := setting.NewCfg()
+	cfg.UnifiedAlerting = setting.UnifiedAlertingSettings{BaseInterval: time.Duration(rand.Int63n(100)+1) * time.Second}
+	sqlStore := db.InitTestDB(t)
+	store := &DBstore{
+		SQLStore:      sqlStore,
+		Cfg:           cfg.UnifiedAlerting,
+		FolderService: setupFolderService(t, sqlStore, cfg),
+		Logger:        &logtest.Fake{},
+	}
+
+	idMutator := models.WithUniqueID()
+	createRuleInFolder := func(title string, orgID int64, namespaceUID string) *models.AlertRule {
+		generator := models.AlertRuleGen(withIntervalMatching(store.Cfg.BaseInterval), idMutator, models.WithNamespace(&folder.Folder{
+			UID:   namespaceUID,
+			Title: namespaceUID,
+		}), withOrgID(orgID), models.WithTitle(title))
+		return createRule(t, store, generator)
+	}
+
+	t.Run("should handle update chains without unique constraint violation", func(t *testing.T) {
+		rule1 := createRuleInFolder("chain-rule1", 1, "my-namespace")
+		rule2 := createRuleInFolder("chain-rule2", 1, "my-namespace")
+
+		newRule1 := models.CopyRule(rule1)
+		newRule2 := models.CopyRule(rule2)
+		newRule1.Title = rule2.Title
+		newRule2.Title = util.GenerateShortUID()
+
+		err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
+			Existing: rule1,
+			New:      *newRule1,
+		}, {
+			Existing: rule2,
+			New:      *newRule2,
+		},
+		})
+		require.NoError(t, err)
+
+		dbrule1 := &models.AlertRule{}
+		dbrule2 := &models.AlertRule{}
+		err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
+			exist, err := sess.Table(models.AlertRule{}).ID(rule1.ID).Get(dbrule1)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule1.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule2.ID).Get(dbrule2)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule2.ID))
+			return nil
+		})
+
+		require.NoError(t, err)
+		require.Equal(t, newRule1.Title, dbrule1.Title)
+		require.Equal(t, newRule2.Title, dbrule2.Title)
+	})
+
+	t.Run("should handle update chains with cycle without unique constraint violation", func(t *testing.T) {
+		rule1 := createRuleInFolder("cycle-rule1", 1, "my-namespace")
+		rule2 := createRuleInFolder("cycle-rule2", 1, "my-namespace")
+		rule3 := createRuleInFolder("cycle-rule3", 1, "my-namespace")
+
+		newRule1 := models.CopyRule(rule1)
+		newRule2 := models.CopyRule(rule2)
+		newRule3 := models.CopyRule(rule3)
+		newRule1.Title = rule2.Title
+		newRule2.Title = rule3.Title
+		newRule3.Title = rule1.Title
+
+		err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
+			Existing: rule1,
+			New:      *newRule1,
+		}, {
+			Existing: rule2,
+			New:      *newRule2,
+		}, {
+			Existing: rule3,
+			New:      *newRule3,
+		},
+		})
+		require.NoError(t, err)
+
+		dbrule1 := &models.AlertRule{}
+		dbrule2 := &models.AlertRule{}
+		dbrule3 := &models.AlertRule{}
+		err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
+			exist, err := sess.Table(models.AlertRule{}).ID(rule1.ID).Get(dbrule1)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule1.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule2.ID).Get(dbrule2)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule2.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule3.ID).Get(dbrule3)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule3.ID))
+			return nil
+		})
+
+		require.NoError(t, err)
+		require.Equal(t, newRule1.Title, dbrule1.Title)
+		require.Equal(t, newRule2.Title, dbrule2.Title)
+		require.Equal(t, newRule3.Title, dbrule3.Title)
+	})
+
+	t.Run("should handle case-insensitive intermediate collision without unique constraint violation", func(t *testing.T) {
+		rule1 := createRuleInFolder("case-cycle-rule1", 1, "my-namespace")
+		rule2 := createRuleInFolder("case-cycle-rule2", 1, "my-namespace")
+
+		newRule1 := models.CopyRule(rule1)
+		newRule2 := models.CopyRule(rule2)
+		newRule1.Title = strings.ToUpper(rule2.Title)
+		newRule2.Title = strings.ToUpper(rule1.Title)
+
+		err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
+			Existing: rule1,
+			New:      *newRule1,
+		}, {
+			Existing: rule2,
+			New:      *newRule2,
+		},
+		})
+		require.NoError(t, err)
+
+		dbrule1 := &models.AlertRule{}
+		dbrule2 := &models.AlertRule{}
+		err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
+			exist, err := sess.Table(models.AlertRule{}).ID(rule1.ID).Get(dbrule1)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule1.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule2.ID).Get(dbrule2)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule2.ID))
+			return nil
+		})
+
+		require.NoError(t, err)
+		require.Equal(t, newRule1.Title, dbrule1.Title)
+		require.Equal(t, newRule2.Title, dbrule2.Title)
+	})
+
+	t.Run("should handle update multiple chains in different folders without unique constraint violation", func(t *testing.T) {
+		rule1 := createRuleInFolder("multi-cycle-rule1", 1, "my-namespace")
+		rule2 := createRuleInFolder("multi-cycle-rule2", 1, "my-namespace")
+		rule3 := createRuleInFolder("multi-cycle-rule1", 1, "my-namespace2")
+		rule4 := createRuleInFolder("multi-cycle-rule2", 1, "my-namespace2")
+
+		newRule1 := models.CopyRule(rule1)
+		newRule2 := models.CopyRule(rule2)
+		newRule3 := models.CopyRule(rule3)
+		newRule4 := models.CopyRule(rule4)
+		newRule1.Title = rule2.Title
+		newRule2.Title = rule1.Title
+		newRule3.Title = rule4.Title
+		newRule4.Title = rule3.Title
+
+		err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{
+			Existing: rule1,
+			New:      *newRule1,
+		}, {
+			Existing: rule2,
+			New:      *newRule2,
+		}, {
+			Existing: rule3,
+			New:      *newRule3,
+		}, {
+			Existing: rule4,
+			New:      *newRule4,
+		},
+		})
+		require.NoError(t, err)
+
+		dbrule1 := &models.AlertRule{}
+		dbrule2 := &models.AlertRule{}
+		dbrule3 := &models.AlertRule{}
+		dbrule4 := &models.AlertRule{}
+		err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
+			exist, err := sess.Table(models.AlertRule{}).ID(rule1.ID).Get(dbrule1)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule1.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule2.ID).Get(dbrule2)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule2.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule3.ID).Get(dbrule3)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule3.ID))
+
+			exist, err = sess.Table(models.AlertRule{}).ID(rule4.ID).Get(dbrule4)
+			if err != nil {
+				return err
+			}
+			require.Truef(t, exist, fmt.Sprintf("rule with ID %d does not exist", rule4.ID))
+			return nil
+		})
+
+		require.NoError(t, err)
+		require.Equal(t, newRule1.Title, dbrule1.Title)
+		require.Equal(t, newRule2.Title, dbrule2.Title)
+		require.Equal(t, newRule3.Title, dbrule3.Title)
+		require.Equal(t, newRule4.Title, dbrule4.Title)
+	})
+}
+
 func TestIntegration_GetAlertRulesForScheduling(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
@@ -89,16 +323,17 @@ func TestIntegration_GetAlertRulesForScheduling(t *testing.T) {
 
 	sqlStore := db.InitTestDB(t)
 	store := &DBstore{
-		SQLStore: sqlStore,
-		Cfg: setting.UnifiedAlertingSettings{
-			BaseInterval: time.Duration(rand.Int63n(100)) * time.Second,
-		},
+		SQLStore:       sqlStore,
+		Cfg:            cfg.UnifiedAlerting,
 		FolderService:  setupFolderService(t, sqlStore, cfg),
 		FeatureToggles: featuremgmt.WithFeatures(),
 	}
 
-	rule1 := createRule(t, store)
-	rule2 := createRule(t, store)
+	generator := models.AlertRuleGen(withIntervalMatching(store.Cfg.BaseInterval), models.WithUniqueID(), models.WithUniqueOrgID())
+	rule1 := createRule(t, store, generator)
+	rule2 := createRule(t, store, generator)
+	createFolder(t, store, rule1.NamespaceUID, rule1.Title, rule1.OrgID)
+	createFolder(t, store, rule2.NamespaceUID, rule2.Title, rule2.OrgID)
 
 	tc := []struct {
 		name         string
@@ -171,7 +406,7 @@ func TestIntegration_GetAlertRulesForScheduling(t *testing.T) {
 
 func withIntervalMatching(baseInterval time.Duration) func(*models.AlertRule) {
 	return func(rule *models.AlertRule) {
-		rule.IntervalSeconds = int64(baseInterval.Seconds()) * rand.Int63n(10)
+		rule.IntervalSeconds = int64(baseInterval.Seconds()) * (rand.Int63n(10) + 1)
 		rule.For = time.Duration(rule.IntervalSeconds*rand.Int63n(9)+1) * time.Second
 	}
 }
@@ -184,7 +419,7 @@ func TestIntegration_CountAlertRules(t *testing.T) {
 	sqlStore := db.InitTestDB(t)
 	cfg := setting.NewCfg()
 	store := &DBstore{SQLStore: sqlStore, FolderService: setupFolderService(t, sqlStore, cfg)}
-	rule := createRule(t, store)
+	rule := createRule(t, store, nil)
 
 	tests := map[string]struct {
 		query     *models.CountAlertRulesQuery
@@ -222,10 +457,12 @@ func TestIntegration_CountAlertRules(t *testing.T) {
 	}
 }
 
-func createRule(t *testing.T, store *DBstore) *models.AlertRule {
+func createRule(t *testing.T, store *DBstore, generate func() *models.AlertRule) *models.AlertRule {
 	t.Helper()
-	rule := models.AlertRuleGen(withIntervalMatching(store.Cfg.BaseInterval), models.WithUniqueID())()
-	createFolder(t, store, rule.NamespaceUID, rule.Title, rule.OrgID)
+	if generate == nil {
+		generate = models.AlertRuleGen(withIntervalMatching(store.Cfg.BaseInterval), models.WithUniqueID())
+	}
+	rule := generate()
 	err := store.SQLStore.WithDbSession(context.Background(), func(sess *db.Session) error {
 		_, err := sess.Table(models.AlertRule{}).InsertOne(rule)
 		if err != nil {

pkg/tests/api/alerting/api_ruler_test.go

@@ -358,9 +358,10 @@ func TestIntegrationAlertRuleConflictingTitle(t *testing.T) {
 	require.Len(t, createdRuleGroup.Rules, 2)
 
 	t.Run("trying to create alert with same title under same folder should fail", func(t *testing.T) {
-		rules := newTestingRuleConfig(t)
+		rulesWithUID := convertGettableRuleGroupToPostable(createdRuleGroup)
+		rulesWithUID.Rules = append(rulesWithUID.Rules, rules.Rules[0]) // Create new copy of first rule.
 
-		status, body := apiClient.PostRulesGroup(t, "folder1", &rules)
+		status, body := apiClient.PostRulesGroup(t, "folder1", &rulesWithUID)
 		assert.Equal(t, http.StatusInternalServerError, status)
 
 		var res map[string]interface{}
@@ -369,12 +370,10 @@ func TestIntegrationAlertRuleConflictingTitle(t *testing.T) {
 	})
 
 	t.Run("trying to update an alert to the title of an existing alert in the same folder should fail", func(t *testing.T) {
-		rules := newTestingRuleConfig(t)
-		rules.Rules[0].GrafanaManagedAlert.UID = createdRuleGroup.Rules[0].GrafanaManagedAlert.UID
-		rules.Rules[1].GrafanaManagedAlert.UID = createdRuleGroup.Rules[1].GrafanaManagedAlert.UID
-		rules.Rules[1].GrafanaManagedAlert.Title = "AlwaysFiring"
+		rulesWithUID := convertGettableRuleGroupToPostable(createdRuleGroup)
+		rulesWithUID.Rules[1].GrafanaManagedAlert.Title = "AlwaysFiring"
 
-		status, body := apiClient.PostRulesGroup(t, "folder1", &rules)
+		status, body := apiClient.PostRulesGroup(t, "folder1", &rulesWithUID)
 		assert.Equal(t, http.StatusInternalServerError, status)
 
 		var res map[string]interface{}
@@ -388,6 +387,28 @@ func TestIntegrationAlertRuleConflictingTitle(t *testing.T) {
 		assert.Equal(t, http.StatusAccepted, status)
 		require.JSONEq(t, `{"message":"rule group updated successfully"}`, body)
 	})
+
+	t.Run("trying to swap titles of existing alerts in the same folder should work", func(t *testing.T) {
+		rulesWithUID := convertGettableRuleGroupToPostable(createdRuleGroup)
+		title0 := rulesWithUID.Rules[0].GrafanaManagedAlert.Title
+		title1 := rulesWithUID.Rules[1].GrafanaManagedAlert.Title
+		rulesWithUID.Rules[0].GrafanaManagedAlert.Title = title1
+		rulesWithUID.Rules[1].GrafanaManagedAlert.Title = title0
+
+		status, body := apiClient.PostRulesGroup(t, "folder1", &rulesWithUID)
+		assert.Equal(t, http.StatusAccepted, status)
+		require.JSONEq(t, `{"message":"rule group updated successfully"}`, body)
+	})
+
+	t.Run("trying to update titles of existing alerts in a chain in the same folder should work", func(t *testing.T) {
+		rulesWithUID := convertGettableRuleGroupToPostable(createdRuleGroup)
+		rulesWithUID.Rules[0].GrafanaManagedAlert.Title = rulesWithUID.Rules[1].GrafanaManagedAlert.Title
+		rulesWithUID.Rules[1].GrafanaManagedAlert.Title = "something new"
+
+		status, body := apiClient.PostRulesGroup(t, "folder1", &rulesWithUID)
+		assert.Equal(t, http.StatusAccepted, status)
+		require.JSONEq(t, `{"message":"rule group updated successfully"}`, body)
+	})
 }
 
 func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {