Alerting: Fix incorrect 500 code on missing alert rule dashboardUID / panelID (#96491)

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 {

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

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",
+			name:                 "dashboardUID is not present",
-			annotations: map[string]string{PanelIDAnnotation: "1234567890"},
+			annotations:          map[string]string{PanelIDAnnotation: "1234567890"},
-			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+			expectedError:        ErrAlertRuleFailedValidation,
-				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedErrContains:  fmt.Sprintf("%s and %s", DashboardUIDAnnotation, PanelIDAnnotation),
 			expectedDashboardUID: "",
 			expectedPanelID:      -1,
 		},
 		{
-			name:        "dashboardUID is present but empty",
+			name:                 "dashboardUID is present but empty",
-			annotations: map[string]string{DashboardUIDAnnotation: "", PanelIDAnnotation: "1234567890"},
+			annotations:          map[string]string{DashboardUIDAnnotation: "", PanelIDAnnotation: "1234567890"},
-			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+			expectedError:        ErrAlertRuleFailedValidation,
-				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedErrContains:  fmt.Sprintf("%s and %s", DashboardUIDAnnotation, PanelIDAnnotation),
 			expectedDashboardUID: "",
 			expectedPanelID:      -1,
 		},
 		{
-			name:        "panelID is not present",
+			name:                 "panelID is not present",
-			annotations: map[string]string{DashboardUIDAnnotation: "cKy7f6Hk"},
+			annotations:          map[string]string{DashboardUIDAnnotation: "cKy7f6Hk"},
-			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+			expectedError:        ErrAlertRuleFailedValidation,
-				DashboardUIDAnnotation, PanelIDAnnotation),
+			expectedErrContains:  fmt.Sprintf("%s and %s", DashboardUIDAnnotation, PanelIDAnnotation),
 			expectedDashboardUID: "",
 			expectedPanelID:      -1,
 		},
 		{
-			name:        "panelID is present but empty",
+			name:                 "panelID is present but empty",
-			annotations: map[string]string{DashboardUIDAnnotation: "cKy7f6Hk", PanelIDAnnotation: ""},
+			annotations:          map[string]string{DashboardUIDAnnotation: "cKy7f6Hk", PanelIDAnnotation: ""},
-			expectedError: fmt.Errorf("both annotations %s and %s must be specified",
+			expectedError:        ErrAlertRuleFailedValidation,
-				DashboardUIDAnnotation, PanelIDAnnotation),
+			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())
 		})

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) {

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.