diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index 903188c496b..a8f86eee6ab 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/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 } diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 2e759b818c7..872833941ba 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/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 } diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index b15ab5bcb2e..bf1bbd4007e 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/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 { diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index 8482a90730b..27d6b531946 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/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 } diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index e726b83758f..43c37a2efbd 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/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