diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index b10bdfab314..881fc311d87 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -27,7 +27,7 @@ var ( ErrCannotEditNamespace = errors.New("user does not have permissions to edit the namespace") ErrRuleGroupNamespaceNotFound = errors.New("rule group not found under this namespace") ErrAlertRuleFailedValidation = errors.New("invalid alert rule") - ErrAlertRuleUniqueConstraintViolation = errors.New("a conflicting alert rule is found: rule title under the same organisation and folder should be unique") + ErrAlertRuleUniqueConstraintViolation = errors.New("rule title under the same organisation and folder should be unique") ErrQuotaReached = errors.New("quota has been exceeded") // ErrNoDashboard is returned when the alert rule does not have a Dashboard UID // in its annotations or the dashboard does not exist. diff --git a/pkg/services/ngalert/models/errors.go b/pkg/services/ngalert/models/errors.go new file mode 100644 index 00000000000..9851d1c5919 --- /dev/null +++ b/pkg/services/ngalert/models/errors.go @@ -0,0 +1,15 @@ +package models + +import ( + "github.com/grafana/grafana/pkg/util/errutil" +) + +var ( + errAlertRuleConflictMsg = "conflicting alert rule found [rule_uid: '{{ .Public.RuleUID }}', title: '{{ .Public.Title }}', namespace_uid: '{{ .Public.NamespaceUID }}']: {{ .Public.Error }}" + ErrAlertRuleConflictBase = errutil.Conflict("alerting.alert-rule.conflict"). + MustTemplate(errAlertRuleConflictMsg, errutil.WithPublic(errAlertRuleConflictMsg)) +) + +func ErrAlertRuleConflict(rule AlertRule, underlying error) error { + return ErrAlertRuleConflictBase.Build(errutil.TemplateData{Public: map[string]any{"RuleUID": rule.UID, "Title": rule.Title, "NamespaceUID": rule.NamespaceUID, "Error": underlying.Error()}, Error: underlying}) +} diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 19c75ea82d4..585ddf14261 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -165,7 +165,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu for i := range newRules { if _, err := sess.Insert(&newRules[i]); err != nil { if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { - return ngmodels.ErrAlertRuleUniqueConstraintViolation + return ngmodels.ErrAlertRuleConflict(newRules[i], ngmodels.ErrAlertRuleUniqueConstraintViolation) } return fmt.Errorf("failed to create new rules: %w", err) } @@ -208,7 +208,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR if updated, err := sess.ID(r.Existing.ID).AllCols().Update(r.New); err != nil || updated == 0 { if err != nil { if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) { - return ngmodels.ErrAlertRuleUniqueConstraintViolation + return ngmodels.ErrAlertRuleConflict(r.New, ngmodels.ErrAlertRuleUniqueConstraintViolation) } return fmt.Errorf("failed to update rule [%s] %s: %w", r.New.UID, r.New.Title, err) } diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 1313ea028b4..844173297d6 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -317,6 +317,28 @@ func TestIntegrationUpdateAlertRulesWithUniqueConstraintViolation(t *testing.T) require.Equal(t, newRule3.Title, dbrule3.Title) require.Equal(t, newRule4.Title, dbrule4.Title) }) + + t.Run("should fail with unique constraint violation", func(t *testing.T) { + rule1 := createRuleInFolder("unique-rule1", 1, "my-namespace") + rule2 := createRuleInFolder("unique-rule2", 1, "my-namespace") + + newRule1 := models.CopyRule(rule1) + newRule2 := models.CopyRule(rule2) + newRule2.Title = newRule1.Title + + err := store.UpdateAlertRules(context.Background(), []models.UpdateRule{{ + Existing: rule2, + New: *newRule2, + }, + }) + require.ErrorIs(t, err, models.ErrAlertRuleUniqueConstraintViolation) + require.NotEqual(t, newRule2.UID, "") + require.NotEqual(t, newRule2.Title, "") + require.NotEqual(t, newRule2.NamespaceUID, "") + require.ErrorContains(t, err, newRule2.UID) + require.ErrorContains(t, err, newRule2.Title) + require.ErrorContains(t, err, newRule2.NamespaceUID) + }) } func TestIntegration_GetAlertRulesForScheduling(t *testing.T) { @@ -617,6 +639,15 @@ func TestIntegrationInsertAlertRules(t *testing.T) { } require.Truef(t, found, "Rule with key %#v was not found in database", keyWithID) } + + _, err = store.InsertAlertRules(context.Background(), []models.AlertRule{deref[0]}) + require.ErrorIs(t, err, models.ErrAlertRuleUniqueConstraintViolation) + require.NotEqual(t, deref[0].UID, "") + require.NotEqual(t, deref[0].Title, "") + require.NotEqual(t, deref[0].NamespaceUID, "") + require.ErrorContains(t, err, deref[0].UID) + require.ErrorContains(t, err, deref[0].Title) + require.ErrorContains(t, err, deref[0].NamespaceUID) } // createAlertRule creates an alert rule in the database and returns it. diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index 5844a387c22..e634492f224 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -841,11 +841,11 @@ func TestIntegrationAlertRuleConflictingTitle(t *testing.T) { rulesWithUID.Rules = append(rulesWithUID.Rules, rules.Rules[0]) // Create new copy of first rule. _, status, body := apiClient.PostRulesGroupWithStatus(t, "folder1", &rulesWithUID) - assert.Equal(t, http.StatusInternalServerError, status) + assert.Equal(t, http.StatusConflict, status) var res map[string]any require.NoError(t, json.Unmarshal([]byte(body), &res)) - require.Equal(t, "failed to update rule group: failed to add rules: a conflicting alert rule is found: rule title under the same organisation and folder should be unique", res["message"]) + require.Contains(t, res["message"], ngmodels.ErrAlertRuleUniqueConstraintViolation.Error()) }) t.Run("trying to update an alert to the title of an existing alert in the same folder should fail", func(t *testing.T) { @@ -853,11 +853,11 @@ func TestIntegrationAlertRuleConflictingTitle(t *testing.T) { rulesWithUID.Rules[1].GrafanaManagedAlert.Title = "AlwaysFiring" _, status, body := apiClient.PostRulesGroupWithStatus(t, "folder1", &rulesWithUID) - assert.Equal(t, http.StatusInternalServerError, status) + assert.Equal(t, http.StatusConflict, status) var res map[string]any require.NoError(t, json.Unmarshal([]byte(body), &res)) - require.Equal(t, "failed to update rule group: failed to update rules: a conflicting alert rule is found: rule title under the same organisation and folder should be unique", res["message"]) + require.Contains(t, res["message"], ngmodels.ErrAlertRuleUniqueConstraintViolation.Error()) }) t.Run("trying to create alert with same title under another folder should succeed", func(t *testing.T) {