public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[v10.4.x] Alerting: Fix incorrect 500 code on missing alert rule dashboardUID / panelID (#96717)

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

(cherry picked from commit 64c93217ff)

* Fix test

---------

Co-authored-by: Matthew Jacobson <matthew.jacobson@grafana.com>
This commit is contained in:
grafana-delivery-bot[bot]
2024-12-02 09:16:13 -05:00
committed by GitHub
co-authored by Matthew Jacobson
parent e1572e3fdc
commit 40213a06b3
5 changed files with 90 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/ngalert/api/api_ruler_validation_test.go
+30
View File
@@ -287,6 +287,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
+2 -2
View File
@@ -376,13 +376,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
+23 -18
View File
@@ -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,
},
@@ -204,7 +206,10 @@ func TestSetDashboardAndPanelFromAnnotations(t *testing.T) {
})()
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
+31
View File
@@ -556,6 +556,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(), rule, models.ProvenanceNone, 0)
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(), rule, models.ProvenanceNone, 0)
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(), rule, models.ProvenanceNone, 0)
require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation)
})
})
}
func createAlertRuleService(t *testing.T) AlertRuleService {
pkg/tests/api/alerting/api_prometheus_test.go
+4 -1
View File
@@ -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.