From 007b734596dd470dfe8153bf29a89604966337b8 Mon Sep 17 00:00:00 2001
From: Moustafa Baiou <moustafa.baiou@grafana.com>
Date: Thu, 28 Aug 2025 13:04:57 -0400
Subject: [PATCH] Alerting: Fix copying of recording rule fields

Recording rule fields were not being copied correctly when duplicating an alert rule. This manifests as missing `TargetDataSourceUID` fields from the `Record` part of the rule when rules in a group are re-ordered.

Added some additional tests to ensure we cover the generation of recording rules in tests and fixed the copying logic to ensure all fields are copied correctly.

(cherry picked from commit c73b3ccf6e4298ded84c34b8e53f5a70916ed3f5)
---
 pkg/services/ngalert/models/alert_rule.go     |  5 +-
 .../ngalert/models/alert_rule_test.go         | 49 +++++++++++++++++++
 pkg/services/ngalert/models/testing.go        | 12 +++++
 pkg/services/ngalert/tests/fakes/rules.go     |  4 +-
 4 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go
index b4a4a94eaba..017641765f8 100644
--- a/pkg/services/ngalert/models/alert_rule.go
+++ b/pkg/services/ngalert/models/alert_rule.go
@@ -803,8 +803,9 @@ func (alertRule *AlertRule) Copy() *AlertRule {
 
 	if alertRule.Record != nil {
 		result.Record = &Record{
-			From:   alertRule.Record.From,
-			Metric: alertRule.Record.Metric,
+			From:                alertRule.Record.From,
+			Metric:              alertRule.Record.Metric,
+			TargetDatasourceUID: alertRule.Record.TargetDatasourceUID,
 		}
 	}
 
diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go
index 329e979ea03..25d49826828 100644
--- a/pkg/services/ngalert/models/alert_rule_test.go
+++ b/pkg/services/ngalert/models/alert_rule_test.go
@@ -1028,6 +1028,13 @@ func TestAlertRuleCopy(t *testing.T) {
 		copied := rule.Copy()
 		require.NotSame(t, rule.Metadata.PrometheusStyleRule, copied.Metadata.PrometheusStyleRule)
 	})
+	t.Run("should return an exact copy of recording rule", func(t *testing.T) {
+		for i := 0; i < 100; i++ {
+			rule := RuleGen.With(RuleGen.WithAllRecordingRules()).GenerateRef()
+			copied := rule.Copy()
+			require.Empty(t, rule.Diff(copied))
+		}
+	})
 }
 
 // This test makes sure the default generator
@@ -1067,6 +1074,48 @@ func TestGeneratorFillsAllFields(t *testing.T) {
 	require.FailNow(t, "AlertRule generator does not populate fields", "skipped fields: %v", maps.Keys(fields))
 }
 
+func TestGeneratorFillsAllRecordingRuleFields(t *testing.T) {
+	ignoredFields := map[string]struct{}{
+		"ID":                          {},
+		"IsPaused":                    {},
+		"NoDataState":                 {},
+		"ExecErrState":                {},
+		"Condition":                   {},
+		"KeepFiringFor":               {},
+		"MissingSeriesEvalsToResolve": {},
+		"For":                         {},
+		"NotificationSettings":        {},
+	}
+
+	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.With(RuleGen.WithAllRecordingRules()).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))
+}
+
 func TestValidateAlertRule(t *testing.T) {
 	t.Run("keepFiringFor", func(t *testing.T) {
 		testCases := []struct {
diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go
index aa8e7c8f32d..a33440e5f40 100644
--- a/pkg/services/ngalert/models/testing.go
+++ b/pkg/services/ngalert/models/testing.go
@@ -561,6 +561,14 @@ func (a *AlertRuleMutators) WithAllRecordingRules() AlertRuleMutator {
 	}
 }
 
+func (a *AlertRuleMutators) WithoutTargetDataSource() AlertRuleMutator {
+	return func(rule *AlertRule) {
+		if rule.Record != nil {
+			rule.Record.TargetDatasourceUID = ""
+		}
+	}
+}
+
 func (a *AlertRuleMutators) WithMetric(metric string) AlertRuleMutator {
 	return func(rule *AlertRule) {
 		if rule.Record == nil {
@@ -1374,10 +1382,14 @@ func ConvertToRecordingRule(rule *AlertRule) {
 	if rule.Record.Metric == "" {
 		rule.Record.Metric = fmt.Sprintf("some_metric_%s", util.GenerateShortUID())
 	}
+	if rule.Record.TargetDatasourceUID == "" {
+		rule.Record.TargetDatasourceUID = util.GenerateShortUID()
+	}
 	rule.Condition = ""
 	rule.NoDataState = ""
 	rule.ExecErrState = ""
 	rule.For = 0
+	rule.KeepFiringFor = 0
 	rule.NotificationSettings = nil
 	rule.MissingSeriesEvalsToResolve = nil
 }
diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go
index 046522f3214..d2196562b6d 100644
--- a/pkg/services/ngalert/tests/fakes/rules.go
+++ b/pkg/services/ngalert/tests/fakes/rules.go
@@ -231,8 +231,8 @@ func (f *RuleStore) ListAlertRules(_ context.Context, q *models.ListAlertRulesQu
 		if q.ReceiverName != "" && (len(r.NotificationSettings) < 1 || r.NotificationSettings[0].Receiver != q.ReceiverName) {
 			continue
 		}
-
-		ruleList = append(ruleList, r)
+		copyR := models.CopyRule(r)
+		ruleList = append(ruleList, copyR)
 	}
 
 	return ruleList, nil