Alerting: Set Dashboard and Panel IDs on rule group replacement (#60374)

* Set Dashboard and Panel IDs on rule group replacement

* fix comments and abbreviate test variable name

* Update pkg/services/ngalert/provisioning/alert_rules.go

Co-authored-by: Jean-Philippe Quéméner <JohnnyQQQQ@users.noreply.github.com>

Co-authored-by: Jean-Philippe Quéméner <JohnnyQQQQ@users.noreply.github.com>

pkg/services/ngalert/api/api_ruler_validation.go

@@ -108,7 +108,7 @@ func validateRuleNode(
 		newAlertRule.Annotations = ruleNode.ApiRuleNode.Annotations
 		newAlertRule.Labels = ruleNode.ApiRuleNode.Labels
 
-		err = newAlertRule.SetDashboardAndPanel()
+		err = newAlertRule.SetDashboardAndPanelFromAnnotations()
 		if err != nil {
 			return nil, err
 		}

pkg/services/ngalert/models/alert_rule.go

@@ -232,10 +232,9 @@ func (alertRule *AlertRule) Diff(rule *AlertRule, ignore ...string) cmputil.Diff
 	return reporter.Diffs
 }
 
-// SetDashboardAndPanel will set the DashboardUID and PanlID
-// field be doing a lookup in the annotations. Errors when
-// the found annotations are not valid.
-func (alertRule *AlertRule) SetDashboardAndPanel() error {
+// SetDashboardAndPanelFromAnnotations will set the DashboardUID and PanelID field by doing a lookup in the annotations.
+// Errors when the found annotations are not valid.
+func (alertRule *AlertRule) SetDashboardAndPanelFromAnnotations() error {
 	if alertRule.Annotations == nil {
 		return nil
 	}

pkg/services/ngalert/models/alert_rule_test.go

@@ -2,6 +2,7 @@ package models
 
 import (
 	"encoding/json"
+	"fmt"
 	"math/rand"
 	"sort"
 	"strings"
@@ -82,6 +83,85 @@ func TestErrStateFromString(t *testing.T) {
 	})
 }
 
+func TestSetDashboardAndPanelFromAnnotations(t *testing.T) {
+	testCases := []struct {
+		name                 string
+		annotations          map[string]string
+		expectedError        error
+		expectedDashboardUID string
+		expectedPanelID      int64
+	}{
+		{
+			name:                 "annotations is empty",
+			annotations:          nil,
+			expectedError:        nil,
+			expectedDashboardUID: "",
+			expectedPanelID:      -1,
+		},
+		{
+			name:        "dashboardUID is not present",
+			annotations: map[string]string{PanelIDAnnotation: "1234567890"},
+			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedDashboardUID: "",
+			expectedPanelID:      -1,
+		},
+		{
+			name:        "dashboardUID is present but empty",
+			annotations: map[string]string{DashboardUIDAnnotation: "", PanelIDAnnotation: "1234567890"},
+			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedDashboardUID: "",
+			expectedPanelID:      -1,
+		},
+		{
+			name:        "panelID is not present",
+			annotations: map[string]string{DashboardUIDAnnotation: "cKy7f6Hk"},
+			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedDashboardUID: "",
+			expectedPanelID:      -1,
+		},
+		{
+			name:        "panelID is present but empty",
+			annotations: map[string]string{DashboardUIDAnnotation: "cKy7f6Hk", PanelIDAnnotation: ""},
+			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedDashboardUID: "",
+			expectedPanelID:      -1,
+		},
+		{
+			name:                 "dashboardUID and panelID are present but panelID is not a correct int64",
+			annotations:          map[string]string{DashboardUIDAnnotation: "cKy7f6Hk", PanelIDAnnotation: "fgh"},
+			expectedError:        fmt.Errorf("annotation %s must be a valid integer Panel ID", PanelIDAnnotation),
+			expectedDashboardUID: "",
+			expectedPanelID:      -1,
+		},
+		{
+			name:                 "dashboardUID and panelID are present and correct",
+			annotations:          map[string]string{DashboardUIDAnnotation: "cKy7f6Hk", PanelIDAnnotation: "65"},
+			expectedError:        nil,
+			expectedDashboardUID: "cKy7f6Hk",
+			expectedPanelID:      65,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			rule := AlertRuleGen(func(rule *AlertRule) {
+				rule.Annotations = tc.annotations
+				rule.DashboardUID = nil
+				rule.PanelID = nil
+			})()
+			err := rule.SetDashboardAndPanelFromAnnotations()
+
+			require.Equal(t, tc.expectedError, err)
+			require.Equal(t, tc.expectedDashboardUID, rule.GetDashboardUID())
+			require.Equal(t, tc.expectedPanelID, rule.GetPanelID())
+		})
+	}
+}
+
 func TestPatchPartialAlertRule(t *testing.T) {
 	t.Run("patches", func(t *testing.T) {
 		testCases := []struct {

pkg/services/ngalert/provisioning/alert_rules.go

@@ -84,7 +84,7 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model
 		return models.AlertRule{}, err
 	}
 	rule.IntervalSeconds = interval
-	err = rule.SetDashboardAndPanel()
+	err = rule.SetDashboardAndPanelFromAnnotations()
 	if err != nil {
 		return models.AlertRule{}, err
 	}
@@ -203,6 +203,9 @@ func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, orgID int
 	rules := make([]*models.AlertRule, len(group.Rules))
 	group = *syncGroupRuleFields(&group, orgID)
 	for i := range group.Rules {
+		if err := group.Rules[i].SetDashboardAndPanelFromAnnotations(); err != nil {
+			return err
+		}
 		rules = append(rules, &group.Rules[i])
 	}
 	delta, err := store.CalculateChanges(ctx, service.ruleStore, key, rules)
@@ -288,7 +291,7 @@ func (service *AlertRuleService) UpdateAlertRule(ctx context.Context, rule model
 	rule.Updated = time.Now()
 	rule.ID = storedRule.ID
 	rule.IntervalSeconds = storedRule.IntervalSeconds
-	err = rule.SetDashboardAndPanel()
+	err = rule.SetDashboardAndPanelFromAnnotations()
 	if err != nil {
 		return models.AlertRule{}, err
 	}

pkg/services/ngalert/provisioning/alert_rules_test.go

@@ -3,6 +3,7 @@ package provisioning
 import (
 	"context"
 	"encoding/json"
+	"strconv"
 	"testing"
 	"time"
 
@@ -164,6 +165,27 @@ func TestAlertRuleService(t *testing.T) {
 		require.Equal(t, int64(2), readGroup.Rules[0].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"
+		panelId := int64(5678)
+		group := createDummyGroup("group-test-5", orgID)
+		group.Rules[0].Annotations = map[string]string{
+			models.DashboardUIDAnnotation: dashboardUid,
+			models.PanelIDAnnotation:      strconv.FormatInt(panelId, 10),
+		}
+
+		err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI)
+		require.NoError(t, err)
+		updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "group-test-5")
+		require.NoError(t, err)
+
+		require.NotNil(t, updatedGroup.Rules[0].DashboardUID)
+		require.NotNil(t, updatedGroup.Rules[0].PanelID)
+		require.Equal(t, dashboardUid, *updatedGroup.Rules[0].DashboardUID)
+		require.Equal(t, panelId, *updatedGroup.Rules[0].PanelID)
+	})
+
 	t.Run("alert rule provenace should be correctly checked", func(t *testing.T) {
 		tests := []struct {
 			name   string