Alerting: Rule Version API to Ignore versions without diff (#100093)

pkg/services/ngalert/models/testing.go

@@ -563,6 +563,12 @@ func (a *AlertRuleMutators) WithKey(key AlertRuleKey) AlertRuleMutator {
 	}
 }
 
+func (a *AlertRuleMutators) WithVersion(version int64) AlertRuleMutator {
+	return func(r *AlertRule) {
+		r.Version = version
+	}
+}
+
 func (g *AlertRuleGenerator) GenerateLabels(min, max int, prefix string) data.Labels {
 	count := max
 	if min > max {

pkg/services/ngalert/store/alert_rule.go

@@ -121,11 +121,12 @@ func (st DBstore) GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAler
 func (st DBstore) GetAlertRuleVersions(ctx context.Context, key ngmodels.AlertRuleKey) ([]*ngmodels.AlertRule, error) {
 	alertRules := make([]*ngmodels.AlertRule, 0)
 	err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error {
-		rows, err := sess.Table(new(alertRuleVersion)).Where("rule_org_id = ? AND rule_uid = ?", key.OrgID, key.UID).Desc("id").Rows(new(alertRuleVersion))
+		rows, err := sess.Table(new(alertRuleVersion)).Where("rule_org_id = ? AND rule_uid = ?", key.OrgID, key.UID).Asc("id").Rows(new(alertRuleVersion))
 		if err != nil {
 			return err
 		}
 		// Deserialize each rule separately in case any of them contain invalid JSON.
+		var previousVersion *alertRuleVersion
 		for rows.Next() {
 			rule := new(alertRuleVersion)
 			err = rows.Scan(rule)
@@ -133,11 +134,17 @@ func (st DBstore) GetAlertRuleVersions(ctx context.Context, key ngmodels.AlertRu
 				st.Logger.Error("Invalid rule version found in DB store, ignoring it", "func", "GetAlertRuleVersions", "error", err)
 				continue
 			}
+			// skip version that has no diff with previous version
+			// this is pretty basic comparison, it may have false negatives
+			if previousVersion != nil && previousVersion.EqualSpec(*rule) {
+				continue
+			}
 			converted, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(*rule), st.Logger)
 			if err != nil {
 				st.Logger.Error("Invalid rule found in DB store, cannot convert, ignoring it", "func", "GetAlertRuleVersions", "error", err, "version_id", rule.ID)
 				continue
 			}
+			previousVersion = rule
 			alertRules = append(alertRules, &converted)
 		}
 		return nil
@@ -145,6 +152,15 @@ func (st DBstore) GetAlertRuleVersions(ctx context.Context, key ngmodels.AlertRu
 	if err != nil {
 		return nil, err
 	}
+	slices.SortFunc(alertRules, func(a, b *ngmodels.AlertRule) int {
+		if a.ID > b.ID {
+			return -1
+		}
+		if a.ID < b.ID {
+			return 1
+		}
+		return 0
+	})
 	return alertRules, nil
 }
 

pkg/services/ngalert/store/alert_rule_test.go

@@ -1543,6 +1543,74 @@ func TestIncreaseVersionForAllRulesInNamespaces(t *testing.T) {
 	})
 }
 
+func TestGetRuleVersions(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)
+	folderService := setupFolderService(t, sqlStore, cfg, featuremgmt.WithFeatures())
+	b := &fakeBus{}
+	store := createTestStore(sqlStore, folderService, &logtest.Fake{}, cfg.UnifiedAlerting, b)
+	orgID := int64(1)
+	gen := models.RuleGen
+	gen = gen.With(gen.WithIntervalMatching(store.Cfg.BaseInterval), gen.WithOrgID(orgID), gen.WithVersion(1))
+
+	inserted, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.AlertRule{gen.Generate()})
+	require.NoError(t, err)
+	ruleV1, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: inserted[0].UID})
+	require.NoError(t, err)
+	ruleV2 := models.CopyRule(ruleV1, gen.WithTitle(util.GenerateShortUID()), gen.WithGroupIndex(rand.Int()))
+
+	err = store.UpdateAlertRules(context.Background(), &models.AlertingUserUID, []models.UpdateRule{
+		{
+			Existing: ruleV1,
+			New:      *ruleV2,
+		},
+	})
+	require.NoError(t, err)
+
+	t.Run("should return rule versions sorted in decreasing order", func(t *testing.T) {
+		versions, err := store.GetAlertRuleVersions(context.Background(), ruleV2.GetKey())
+		require.NoError(t, err)
+		assert.Len(t, versions, 2)
+		assert.IsDecreasing(t, versions[0].ID, versions[1].ID)
+		diff := versions[1].Diff(versions[0], AlertRuleFieldsToIgnoreInDiff[:]...)
+		assert.ElementsMatch(t, []string{"Title", "RuleGroupIndex"}, diff.Paths())
+	})
+
+	t.Run("should not remove versions without diff", func(t *testing.T) {
+		for i := 0; i < rand.Intn(2)+1; i++ {
+			r, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: ruleV2.UID})
+			require.NoError(t, err)
+			rn := models.CopyRule(r)
+			err = store.UpdateAlertRules(context.Background(), &models.AlertingUserUID, []models.UpdateRule{
+				{
+					Existing: r,
+					New:      *rn,
+				},
+			})
+			require.NoError(t, err)
+		}
+		ruleV2, err = store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: ruleV2.UID})
+		ruleV3 := models.CopyRule(ruleV2, gen.WithGroupName(util.GenerateShortUID()), gen.WithNamespaceUID(util.GenerateShortUID()))
+
+		err = store.UpdateAlertRules(context.Background(), &models.AlertingUserUID, []models.UpdateRule{
+			{
+				Existing: ruleV2,
+				New:      *ruleV3,
+			},
+		})
+
+		versions, err := store.GetAlertRuleVersions(context.Background(), ruleV3.GetKey())
+		require.NoError(t, err)
+		assert.Len(t, versions, 3)
+		diff := versions[0].Diff(versions[1], AlertRuleFieldsToIgnoreInDiff[:]...)
+		assert.ElementsMatch(t, []string{"RuleGroup", "NamespaceUID"}, diff.Paths())
+	})
+}
+
 // createAlertRule creates an alert rule in the database and returns it.
 // If a generator is not specified, uniqueness of primary key is not guaranteed.
 func createRule(t *testing.T, store *DBstore, generator *models.AlertRuleGenerator) *models.AlertRule {

pkg/services/ngalert/store/models.go

@@ -65,6 +65,29 @@ type alertRuleVersion struct {
 	Metadata             string `xorm:"metadata"`
 }
 
+// EqualSpec compares two alertRuleVersion objects for equality based on their specifications and returns true if they match.
+// The comparison is very basic and can produce false-negative. Fields excluded: ID, ParentVersion, RestoredFrom, Version, Created and CreatedBy
+func (a alertRuleVersion) EqualSpec(b alertRuleVersion) bool {
+	return a.RuleOrgID == b.RuleOrgID &&
+		a.RuleUID == b.RuleUID &&
+		a.RuleNamespaceUID == b.RuleNamespaceUID &&
+		a.RuleGroup == b.RuleGroup &&
+		a.RuleGroupIndex == b.RuleGroupIndex &&
+		a.Title == b.Title &&
+		a.Condition == b.Condition &&
+		a.Data == b.Data &&
+		a.IntervalSeconds == b.IntervalSeconds &&
+		a.Record == b.Record &&
+		a.NoDataState == b.NoDataState &&
+		a.ExecErrState == b.ExecErrState &&
+		a.For == b.For &&
+		a.Annotations == b.Annotations &&
+		a.Labels == b.Labels &&
+		a.IsPaused == b.IsPaused &&
+		a.NotificationSettings == b.NotificationSettings &&
+		a.Metadata == b.Metadata
+}
+
 func (a alertRuleVersion) TableName() string {
 	return "alert_rule_version"
 }