diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index 22d2f65a9e6..f875a73d9d7 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/pkg/services/ngalert/models/alert_rule_test.go @@ -347,7 +347,7 @@ func TestPatchPartialAlertRule(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { var existing *AlertRule for { - existing = AlertRuleGen()() + existing = AlertRuleGen(WithUniqueID())() cloned := *existing // make sure the generated rule does not match the mutated one testCase.mutator(&cloned) diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index 71ffd9312a9..f22f357f251 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -71,7 +71,7 @@ func AlertRuleGen(mutators ...AlertRuleMutator) func() *AlertRule { } rule := &AlertRule{ - ID: rand.Int63n(1500), + ID: 0, OrgID: rand.Int63n(1500) + 1, // Prevent OrgID=0 as this does not pass alert rule validation. Title: "TEST-ALERT-" + util.GenerateShortUID(), Condition: "A", @@ -110,7 +110,7 @@ func WithUniqueID() AlertRuleMutator { usedID := make(map[int64]struct{}) return func(rule *AlertRule) { for { - id := rand.Int63n(1500) + id := rand.Int63n(1500) + 1 if _, ok := usedID[id]; !ok { usedID[id] = struct{}{} rule.ID = id diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 2570657ea01..0ae77f063d5 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -169,7 +169,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.ErrAlertRuleConflict(newRules[i], ngmodels.ErrAlertRuleUniqueConstraintViolation) + return ruleConstraintViolationToErr(newRules[i], err) } return fmt.Errorf("failed to create new rules: %w", err) } @@ -215,7 +215,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.ErrAlertRuleConflict(r.New, ngmodels.ErrAlertRuleUniqueConstraintViolation) + return ruleConstraintViolationToErr(r.New, err) } return fmt.Errorf("failed to update rule [%s] %s: %w", r.New.UID, r.New.Title, err) } @@ -758,3 +758,14 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI } return len(updates), st.UpdateAlertRules(ctx, updates) } + +func ruleConstraintViolationToErr(rule ngmodels.AlertRule, err error) error { + msg := err.Error() + if strings.Contains(msg, "UQE_alert_rule_org_id_namespace_uid_title") || strings.Contains(msg, "alert_rule.org_id, alert_rule.namespace_uid, alert_rule.title") { + return ngmodels.ErrAlertRuleConflict(rule, ngmodels.ErrAlertRuleUniqueConstraintViolation) + } else if strings.Contains(msg, "UQE_alert_rule_org_id_uid") || strings.Contains(msg, "alert_rule.org_id, alert_rule.uid") { + return ngmodels.ErrAlertRuleConflict(rule, errors.New("rule UID under the same organisation should be unique")) + } else { + return ngmodels.ErrAlertRuleConflict(rule, err) + } +} diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index 97aa3433d3e..fe470991d73 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -603,6 +603,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { t.Skip("skipping integration test") } + orgID := int64(1) sqlStore := db.InitTestDB(t) cfg := setting.NewCfg() cfg.UnifiedAlerting.BaseInterval = 1 * time.Second @@ -613,7 +614,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { Cfg: cfg.UnifiedAlerting, } - rules := models.GenerateAlertRules(5, models.AlertRuleGen(models.WithOrgID(1), withIntervalMatching(store.Cfg.BaseInterval))) + rules := models.GenerateAlertRules(5, models.AlertRuleGen(models.WithOrgID(orgID), withIntervalMatching(store.Cfg.BaseInterval))) deref := make([]models.AlertRule, 0, len(rules)) for _, rule := range rules { deref = append(deref, *rule) @@ -641,14 +642,30 @@ 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) + t.Run("fail to insert rules with same ID", func(t *testing.T) { + _, err = store.InsertAlertRules(context.Background(), []models.AlertRule{deref[0]}) + require.ErrorIs(t, err, models.ErrAlertRuleConflictBase) + }) + t.Run("fail insert rules with the same title in a folder", func(t *testing.T) { + cp := models.CopyRule(&deref[0]) + cp.UID = cp.UID + "-new" + _, err = store.InsertAlertRules(context.Background(), []models.AlertRule{*cp}) + require.ErrorIs(t, err, models.ErrAlertRuleConflictBase) + 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) + }) + t.Run("should not let insert rules with the same UID", func(t *testing.T) { + cp := models.CopyRule(&deref[0]) + cp.Title = "unique-test-title" + _, err = store.InsertAlertRules(context.Background(), []models.AlertRule{*cp}) + require.ErrorIs(t, err, models.ErrAlertRuleConflictBase) + require.ErrorContains(t, err, "rule UID under the same organisation should be unique") + }) } func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {