diff --git a/pkg/services/ngalert/api/api_ruler_validation_test.go b/pkg/services/ngalert/api/api_ruler_validation_test.go index 33574690840..b56705f625f 100644 --- a/pkg/services/ngalert/api/api_ruler_validation_test.go +++ b/pkg/services/ngalert/api/api_ruler_validation_test.go @@ -301,6 +301,36 @@ func TestValidateRuleGroupFailures(t *testing.T) { require.Contains(t, err.Error(), apiModel.Rules[0].GrafanaManagedAlert.UID) }, }, + { + name: "fail with 4xx if rule contains only panelID", + group: func() *apimodels.PostableRuleGroupConfig { + r1 := validRule() + panelId := int64(42) + r1.Annotations = map[string]string{ + models.PanelIDAnnotation: strconv.FormatInt(panelId, 10), + } + g := validGroup(cfg, r1) + return &g + }, + assert: func(t *testing.T, apiModel *apimodels.PostableRuleGroupConfig, err error) { + require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation) + }, + }, + { + name: "fail with 4xx if rule contains only dashboardUID", + group: func() *apimodels.PostableRuleGroupConfig { + r1 := validRule() + dashboardUid := "oinwerfgiuac" + r1.Annotations = map[string]string{ + models.DashboardUIDAnnotation: dashboardUid, + } + g := validGroup(cfg, r1) + return &g + }, + assert: func(t *testing.T, apiModel *apimodels.PostableRuleGroupConfig, err error) { + require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation) + }, + }, } for _, testCase := range testCases { diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index c299a9f6aa1..ef8d15827df 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -436,13 +436,13 @@ func (alertRule *AlertRule) SetDashboardAndPanelFromAnnotations() error { dashUID := alertRule.Annotations[DashboardUIDAnnotation] panelID := alertRule.Annotations[PanelIDAnnotation] if dashUID != "" && panelID == "" || dashUID == "" && panelID != "" { - return fmt.Errorf("both annotations %s and %s must be specified", + return fmt.Errorf("%w: both annotations %s and %s must be specified", ErrAlertRuleFailedValidation, DashboardUIDAnnotation, PanelIDAnnotation) } if dashUID != "" { panelIDValue, err := strconv.ParseInt(panelID, 10, 64) if err != nil { - return fmt.Errorf("annotation %s must be a valid integer Panel ID", + return fmt.Errorf("%w: annotation %s must be a valid integer Panel ID", ErrAlertRuleFailedValidation, PanelIDAnnotation) } alertRule.DashboardUID = &dashUID diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index 619d50d90df..bb7bfdadf33 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/pkg/services/ngalert/models/alert_rule_test.go @@ -137,6 +137,7 @@ func TestSetDashboardAndPanelFromAnnotations(t *testing.T) { name string annotations map[string]string expectedError error + expectedErrContains string expectedDashboardUID string expectedPanelID int64 }{ @@ -148,41 +149,42 @@ func TestSetDashboardAndPanelFromAnnotations(t *testing.T) { 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), + name: "dashboardUID is not present", + annotations: map[string]string{PanelIDAnnotation: "1234567890"}, + expectedError: ErrAlertRuleFailedValidation, + expectedErrContains: fmt.Sprintf("%s and %s", 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), + name: "dashboardUID is present but empty", + annotations: map[string]string{DashboardUIDAnnotation: "", PanelIDAnnotation: "1234567890"}, + expectedError: ErrAlertRuleFailedValidation, + expectedErrContains: fmt.Sprintf("%s and %s", 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), + name: "panelID is not present", + annotations: map[string]string{DashboardUIDAnnotation: "cKy7f6Hk"}, + expectedError: ErrAlertRuleFailedValidation, + expectedErrContains: fmt.Sprintf("%s and %s", 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), + name: "panelID is present but empty", + annotations: map[string]string{DashboardUIDAnnotation: "cKy7f6Hk", PanelIDAnnotation: ""}, + expectedError: ErrAlertRuleFailedValidation, + expectedErrContains: fmt.Sprintf("%s and %s", 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), + expectedError: ErrAlertRuleFailedValidation, + expectedErrContains: PanelIDAnnotation, expectedDashboardUID: "", expectedPanelID: -1, }, @@ -203,7 +205,10 @@ func TestSetDashboardAndPanelFromAnnotations(t *testing.T) { ).Generate() err := rule.SetDashboardAndPanelFromAnnotations() - require.Equal(t, tc.expectedError, err) + require.ErrorIs(t, err, tc.expectedError) + if tc.expectedErrContains != "" { + require.ErrorContains(t, err, tc.expectedErrContains) + } require.Equal(t, tc.expectedDashboardUID, rule.GetDashboardUID()) require.Equal(t, tc.expectedPanelID, rule.GetPanelID()) }) diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index 98693e427d5..ce0651f4eaf 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -867,6 +867,37 @@ func TestCreateAlertRule(t *testing.T) { require.NoError(t, err) }) }) + t.Run("when dashboard is specified", func(t *testing.T) { + t.Run("return no error when both specified", func(t *testing.T) { + rule := dummyRule("test#4", orgID) + dashboardUid := "oinwerfgiuac" + panelId := int64(42) + rule.Annotations = map[string]string{ + models.DashboardUIDAnnotation: dashboardUid, + models.PanelIDAnnotation: strconv.FormatInt(panelId, 10), + } + rule, err := ruleService.CreateAlertRule(context.Background(), u, rule, models.ProvenanceNone) + require.NoError(t, err) + }) + t.Run("return 4xx error when missing dashboard uid", func(t *testing.T) { + rule := dummyRule("test#3", orgID) + panelId := int64(42) + rule.Annotations = map[string]string{ + models.PanelIDAnnotation: strconv.FormatInt(panelId, 10), + } + rule, err := ruleService.CreateAlertRule(context.Background(), u, rule, models.ProvenanceNone) + require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation) + }) + t.Run("return 4xx error when missing panel id", func(t *testing.T) { + rule := dummyRule("test#3", orgID) + dashboardUid := "oinwerfgiuac" + rule.Annotations = map[string]string{ + models.DashboardUIDAnnotation: dashboardUid, + } + rule, err := ruleService.CreateAlertRule(context.Background(), u, rule, models.ProvenanceNone) + require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation) + }) + }) } func TestUpdateAlertRule(t *testing.T) { diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 2e81b4c6c52..bfae05b1538 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -215,7 +215,10 @@ func TestIntegrationPrometheusRules(t *testing.T) { assert.Equal(t, 400, resp.StatusCode) var res map[string]any require.NoError(t, json.Unmarshal(b, &res)) - require.Equal(t, "invalid rule specification at index [0]: both annotations __dashboardUid__ and __panelId__ must be specified", res["message"]) + require.Contains(t, res["message"], "[0]") // Index of the invalid rule. + require.Contains(t, res["message"], ngmodels.ErrAlertRuleFailedValidation.Error()) + require.Contains(t, res["message"], ngmodels.DashboardUIDAnnotation) + require.Contains(t, res["message"], ngmodels.PanelIDAnnotation) } // Now, let's see how this looks like.