Alerting: Fix alert rule copy to include metadata (#100212)

* copy metadata

* add tests for copy and generator

* extract copy rule to a production method and update usages

* fix tests

pkg/services/ngalert/models/alert_rule.go

@@ -680,6 +680,81 @@ func (alertRule *AlertRule) Type() RuleType {
 	return RuleTypeAlerting
 }
 
+// Copy creates and returns a deep copy of the AlertRule instance, duplicating all fields and nested data structures.
+func (alertRule *AlertRule) Copy() *AlertRule {
+	if alertRule == nil {
+		return nil
+	}
+	result := AlertRule{
+		ID:              alertRule.ID,
+		OrgID:           alertRule.OrgID,
+		Title:           alertRule.Title,
+		Condition:       alertRule.Condition,
+		Updated:         alertRule.Updated,
+		UpdatedBy:       alertRule.UpdatedBy,
+		IntervalSeconds: alertRule.IntervalSeconds,
+		Version:         alertRule.Version,
+		UID:             alertRule.UID,
+		NamespaceUID:    alertRule.NamespaceUID,
+		RuleGroup:       alertRule.RuleGroup,
+		RuleGroupIndex:  alertRule.RuleGroupIndex,
+		NoDataState:     alertRule.NoDataState,
+		ExecErrState:    alertRule.ExecErrState,
+		For:             alertRule.For,
+		Record:          alertRule.Record,
+		IsPaused:        alertRule.IsPaused,
+		Metadata:        alertRule.Metadata,
+	}
+
+	if alertRule.DashboardUID != nil {
+		dash := *alertRule.DashboardUID
+		result.DashboardUID = &dash
+	}
+	if alertRule.PanelID != nil {
+		p := *alertRule.PanelID
+		result.PanelID = &p
+	}
+
+	for _, d := range alertRule.Data {
+		q := AlertQuery{
+			RefID:             d.RefID,
+			QueryType:         d.QueryType,
+			RelativeTimeRange: d.RelativeTimeRange,
+			DatasourceUID:     d.DatasourceUID,
+		}
+		q.Model = make([]byte, 0, cap(d.Model))
+		q.Model = append(q.Model, d.Model...)
+		result.Data = append(result.Data, q)
+	}
+
+	if alertRule.Annotations != nil {
+		result.Annotations = make(map[string]string, len(alertRule.Annotations))
+		for s, s2 := range alertRule.Annotations {
+			result.Annotations[s] = s2
+		}
+	}
+
+	if alertRule.Labels != nil {
+		result.Labels = make(map[string]string, len(alertRule.Labels))
+		for s, s2 := range alertRule.Labels {
+			result.Labels[s] = s2
+		}
+	}
+
+	if alertRule.Record != nil {
+		result.Record = &Record{
+			From:   alertRule.Record.From,
+			Metric: alertRule.Record.Metric,
+		}
+	}
+
+	for _, s := range alertRule.NotificationSettings {
+		result.NotificationSettings = append(result.NotificationSettings, CopyNotificationSettings(s))
+	}
+
+	return &result
+}
+
 func ClearRecordingRuleIgnoredFields(rule *AlertRule) {
 	rule.NoDataState = ""
 	rule.ExecErrState = ""

pkg/services/ngalert/models/alert_rule_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/maps"
 	"gopkg.in/yaml.v3"
 
 	"github.com/grafana/grafana/pkg/util"
@@ -407,7 +408,7 @@ func TestDiff(t *testing.T) {
 		rule1 := RuleGen.GenerateRef()
 		rule2 := RuleGen.GenerateRef()
 
-		diffs := rule1.Diff(rule2, "Data", "Annotations", "Labels", "NotificationSettings") // these fields will be tested separately
+		diffs := rule1.Diff(rule2, "Data", "Annotations", "Labels", "NotificationSettings", "Metadata") // these fields will be tested separately
 
 		difCnt := 0
 		if rule1.ID != rule2.ID {
@@ -839,6 +840,24 @@ func TestDiff(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("should detect changes in Metadata", func(t *testing.T) {
+		rule1 := RuleGen.With(RuleGen.WithMetadata(AlertRuleMetadata{EditorSettings: EditorSettings{
+			SimplifiedQueryAndExpressionsSection: false,
+			SimplifiedNotificationsSection:       false,
+		}})).GenerateRef()
+
+		rule2 := CopyRule(rule1, RuleGen.WithMetadata(AlertRuleMetadata{EditorSettings: EditorSettings{
+			SimplifiedQueryAndExpressionsSection: true,
+			SimplifiedNotificationsSection:       true,
+		}}))
+
+		diff := rule1.Diff(rule2)
+		assert.ElementsMatch(t, []string{
+			"Metadata.EditorSettings.SimplifiedQueryAndExpressionsSection",
+			"Metadata.EditorSettings.SimplifiedNotificationsSection",
+		}, diff.Paths())
+	})
 }
 
 func TestSortByGroupIndex(t *testing.T) {
@@ -919,3 +938,48 @@ func TestAlertRuleGetKeyWithGroup(t *testing.T) {
 		require.Equal(t, expected, rule.GetKeyWithGroup())
 	})
 }
+
+func TestAlertRuleCopy(t *testing.T) {
+	for i := 0; i < 100; i++ {
+		rule := RuleGen.GenerateRef()
+		copied := rule.Copy()
+		require.Empty(t, rule.Diff(copied))
+	}
+}
+
+// This test makes sure the default generator
+func TestGeneratorFillsAllFields(t *testing.T) {
+	ignoredFields := map[string]struct{}{
+		"ID":       {},
+		"IsPaused": {},
+		"Record":   {},
+	}
+
+	tpe := reflect.TypeOf(AlertRule{})
+	fields := make(map[string]struct{}, tpe.NumField())
+	for i := 0; i < tpe.NumField(); i++ {
+		if _, ok := ignoredFields[tpe.Field(i).Name]; ok {
+			continue
+		}
+		fields[tpe.Field(i).Name] = struct{}{}
+	}
+
+	for i := 0; i < 1000; i++ {
+		rule := RuleGen.Generate()
+		v := reflect.ValueOf(rule)
+
+		for j := 0; j < tpe.NumField(); j++ {
+			field := tpe.Field(j)
+			value := v.Field(j)
+			if !value.IsValid() || value.Kind() == reflect.Ptr && value.IsNil() || value.IsZero() {
+				continue
+			}
+			delete(fields, field.Name)
+			if len(fields) == 0 {
+				return
+			}
+		}
+	}
+
+	require.FailNow(t, "AlertRule generator does not populate fields", "skipped fields: %v", maps.Keys(fields))
+}

pkg/services/ngalert/models/testing.go

@@ -123,6 +123,7 @@ func (g *AlertRuleGenerator) Generate() AlertRule {
 		Annotations:          annotations,
 		Labels:               labels,
 		NotificationSettings: ns,
+		Metadata:             GenerateMetadata(),
 	}
 
 	for _, mutator := range g.mutators {
@@ -569,6 +570,12 @@ func (a *AlertRuleMutators) WithVersion(version int64) AlertRuleMutator {
 	}
 }
 
+func (a *AlertRuleMutators) WithMetadata(meta AlertRuleMetadata) AlertRuleMutator {
+	return func(r *AlertRule) {
+		r.Metadata = meta
+	}
+}
+
 func (g *AlertRuleGenerator) GenerateLabels(min, max int, prefix string) data.Labels {
 	count := max
 	if min > max {
@@ -643,79 +650,13 @@ func GenerateGroupKey(orgID int64) AlertRuleGroupKey {
 
 // CopyRule creates a deep copy of AlertRule
 func CopyRule(r *AlertRule, mutators ...AlertRuleMutator) *AlertRule {
-	result := AlertRule{
+	result := r.Copy()
-		ID:              r.ID,
-		OrgID:           r.OrgID,
-		Title:           r.Title,
-		Condition:       r.Condition,
-		Updated:         r.Updated,
-		UpdatedBy:       r.UpdatedBy,
-		IntervalSeconds: r.IntervalSeconds,
-		Version:         r.Version,
-		UID:             r.UID,
-		NamespaceUID:    r.NamespaceUID,
-		RuleGroup:       r.RuleGroup,
-		RuleGroupIndex:  r.RuleGroupIndex,
-		NoDataState:     r.NoDataState,
-		ExecErrState:    r.ExecErrState,
-		For:             r.For,
-		Record:          r.Record,
-		IsPaused:        r.IsPaused,
-	}
-
-	if r.DashboardUID != nil {
-		dash := *r.DashboardUID
-		result.DashboardUID = &dash
-	}
-	if r.PanelID != nil {
-		p := *r.PanelID
-		result.PanelID = &p
-	}
-
-	for _, d := range r.Data {
-		q := AlertQuery{
-			RefID:             d.RefID,
-			QueryType:         d.QueryType,
-			RelativeTimeRange: d.RelativeTimeRange,
-			DatasourceUID:     d.DatasourceUID,
-		}
-		q.Model = make([]byte, 0, cap(d.Model))
-		q.Model = append(q.Model, d.Model...)
-		result.Data = append(result.Data, q)
-	}
-
-	if r.Annotations != nil {
-		result.Annotations = make(map[string]string, len(r.Annotations))
-		for s, s2 := range r.Annotations {
-			result.Annotations[s] = s2
-		}
-	}
-
-	if r.Labels != nil {
-		result.Labels = make(map[string]string, len(r.Labels))
-		for s, s2 := range r.Labels {
-			result.Labels[s] = s2
-		}
-	}
-
-	if r.Record != nil {
-		result.Record = &Record{
-			From:   r.Record.From,
-			Metric: r.Record.Metric,
-		}
-	}
-
-	for _, s := range r.NotificationSettings {
-		result.NotificationSettings = append(result.NotificationSettings, CopyNotificationSettings(s))
-	}
-
 	if len(mutators) > 0 {
 		for _, mutator := range mutators {
-			mutator(&result)
+			mutator(result)
 		}
 	}
-
+	return result
-	return &result
 }
 
 func CreateClassicConditionExpression(refID string, inputRefID string, reducer string, operation string, threshold int) AlertQuery {
@@ -862,6 +803,15 @@ func CreateHysteresisExpression(t *testing.T, refID string, inputRefID string, t
 	return q
 }
 
+func GenerateMetadata() AlertRuleMetadata {
+	return AlertRuleMetadata{
+		EditorSettings: EditorSettings{
+			SimplifiedQueryAndExpressionsSection: rand.Int()%2 == 0,
+			SimplifiedNotificationsSection:       rand.Int()%2 == 0,
+		},
+	}
+}
+
 type AlertInstanceMutator func(*AlertInstance)
 
 // AlertInstanceGen provides a factory function that generates a random AlertInstance.

pkg/services/ngalert/schedule/schedule_unit_test.go

@@ -784,6 +784,7 @@ func TestSchedule_updateRulesMetrics(t *testing.T) {
 
 		alertRuleWithAdvancedSettings := models.RuleGen.With(
 			models.RuleGen.WithOrgID(firstOrgID),
+			models.RuleGen.WithEditorSettingsSimplifiedNotificationsSection(false),
 			models.RuleGen.WithEditorSettingsSimplifiedQueryAndExpressionsSection(false),
 		).GenerateRef()
 
@@ -818,6 +819,7 @@ func TestSchedule_updateRulesMetrics(t *testing.T) {
 
 		alertRule2 := models.RuleGen.With(
 			models.RuleGen.WithOrgID(secondOrgID),
+			models.RuleGen.WithEditorSettingsSimplifiedNotificationsSection(false),
 			models.RuleGen.WithEditorSettingsSimplifiedQueryAndExpressionsSection(true),
 		).GenerateRef()
 

pkg/services/ngalert/store/alert_rule.go

@@ -984,7 +984,7 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI
 			continue
 		}
 
-		r := ngmodels.CopyRule(rule)
+		r := rule.Copy()
 		for idx := range r.NotificationSettings {
 			if r.NotificationSettings[idx].Receiver == oldReceiver {
 				r.NotificationSettings[idx].Receiver = newReceiver
@@ -1059,7 +1059,7 @@ func (st DBstore) RenameTimeIntervalInNotificationSettings(
 			continue
 		}
 
-		r := ngmodels.CopyRule(rule)
+		r := rule.Copy()
 		for idx := range r.NotificationSettings {
 			for mtIdx := range r.NotificationSettings[idx].MuteTimeIntervals {
 				if r.NotificationSettings[idx].MuteTimeIntervals[mtIdx] == oldTimeInterval {

pkg/services/ngalert/store/deltas.go

@@ -175,7 +175,7 @@ func UpdateCalculatedRuleFields(ch *GroupDelta) *GroupDelta {
 			}
 			if groupKey != ch.GroupKey {
 				if rule.RuleGroupIndex != idx {
-					upd.New = models.CopyRule(rule)
+					upd.New = rule.Copy()
 					upd.New.RuleGroupIndex = idx
 					upd.Diff = rule.Diff(upd.New, AlertRuleFieldsToIgnoreInDiff[:]...)
 				}

pkg/services/ngalert/store/deltas_test.go

@@ -81,7 +81,7 @@ func TestCalculateChanges(t *testing.T) {
 		submittedMap := groupByUID(t, rules)
 		submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules))
 		for _, rule := range rules {
-			submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule})
+			submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule, HasMetadata: true})
 		}
 
 		fakeStore := fakes.NewRuleStore(t)
@@ -216,7 +216,7 @@ func TestCalculateChanges(t *testing.T) {
 		submittedMap := groupByUID(t, rules)
 		submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules))
 		for _, rule := range rules {
-			submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule})
+			submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule, HasMetadata: true})
 		}
 
 		changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, submitted)