diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index b7dd3c61e50..d86fc7a3d50 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -17,6 +17,8 @@ var ( ErrRuleGroupNamespaceNotFound = errors.New("rule group not found under this namespace") // ErrAlertRuleFailedValidation ErrAlertRuleFailedValidation = errors.New("invalid alert rule") + // ErrAlertRuleUniqueConstraintViolation + ErrAlertRuleUniqueConstraintViolation = errors.New("a conflicting alert rule is found: rule title under the same organisation and folder should be unique") ) type NoDataState string diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 5cd139c6d95..8851dc79881 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -522,6 +522,9 @@ func (st DBstore) UpdateRuleGroup(cmd UpdateRuleGroupCmd) error { } if err := st.UpsertAlertRules(upsertRules); err != nil { + if st.SQLStore.Dialect.IsUniqueConstraintViolation(err) { + return ngmodels.ErrAlertRuleUniqueConstraintViolation + } return err } diff --git a/pkg/services/sqlstore/migrations/ualert/tables.go b/pkg/services/sqlstore/migrations/ualert/tables.go index 712e4f2aa81..f59a89d809d 100644 --- a/pkg/services/sqlstore/migrations/ualert/tables.go +++ b/pkg/services/sqlstore/migrations/ualert/tables.go @@ -7,7 +7,6 @@ import ( ) // AddMigration defines database migrations. -// If Alerting NG is not enabled does nothing. func AddTablesMigrations(mg *migrator.Migrator) { AddAlertDefinitionMigrations(mg, 60) AddAlertDefinitionVersionMigrations(mg) @@ -194,6 +193,14 @@ func AddAlertRuleMigrations(mg *migrator.Migrator, defaultIntervalSeconds int64) // add labels column mg.AddMigration("add column labels to alert_rule", migrator.NewAddColumnMigration(alertRule, &migrator.Column{Name: "labels", Type: migrator.DB_Text, Nullable: true})) + + mg.AddMigration("remove unique index from alert_rule on org_id, title columns", migrator.NewDropIndexMigration(alertRule, &migrator.Index{ + Cols: []string{"org_id", "title"}, Type: migrator.UniqueIndex, + })) + + mg.AddMigration("add index in alert_rule on org_id, namespase_uid and title columns", migrator.NewAddIndexMigration(alertRule, &migrator.Index{ + Cols: []string{"org_id", "namespace_uid", "title"}, Type: migrator.UniqueIndex, + })) } func AddAlertRuleVersionMigrations(mg *migrator.Migrator) { diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index 99da0369d0c..8823504964d 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -298,3 +298,123 @@ func createRule(t *testing.T, grafanaListedAddr string, folder string, user, pas assert.Equal(t, http.StatusAccepted, resp.StatusCode) require.JSONEq(t, `{"message":"rule group updated successfully"}`, string(b)) } + +func TestAlertRuleConflictingTitle(t *testing.T) { + // Setup Grafana and its Database + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + EnableFeatureToggles: []string{"ngalert"}, + EnableQuota: true, + DisableAnonymous: true, + ViewersCanEdit: true, + }) + + store := testinfra.SetUpDatabase(t, dir) + // override bus to get the GetSignedInUserQuery handler + store.Bus = bus.GetBus() + grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store) + + // Create the namespace we'll save our alerts to. + _, err := createFolder(t, store, 0, "folder1") + require.NoError(t, err) + _, err = createFolder(t, store, 0, "folder2") + require.NoError(t, err) + + // Create user + require.NoError(t, createUser(t, store, models.ROLE_ADMIN, "admin", "admin")) + + interval, err := model.ParseDuration("1m") + require.NoError(t, err) + + rules := apimodels.PostableRuleGroupConfig{ + Name: "arulegroup", + Rules: []apimodels.PostableExtendedRuleNode{ + { + ApiRuleNode: &apimodels.ApiRuleNode{ + For: interval, + Labels: map[string]string{"label1": "val1"}, + Annotations: map[string]string{"annotation1": "val1"}, + }, + // this rule does not explicitly set no data and error states + // therefore it should get the default values + GrafanaManagedAlert: &apimodels.PostableGrafanaRule{ + Title: "AlwaysFiring", + Condition: "A", + Data: []ngmodels.AlertQuery{ + { + RefID: "A", + RelativeTimeRange: ngmodels.RelativeTimeRange{ + From: ngmodels.Duration(time.Duration(5) * time.Hour), + To: ngmodels.Duration(time.Duration(3) * time.Hour), + }, + DatasourceUID: "-100", + Model: json.RawMessage(`{ + "type": "math", + "expression": "2 + 3 > 1" + }`), + }, + }, + }, + }, + }, + } + buf := bytes.Buffer{} + enc := json.NewEncoder(&buf) + err = enc.Encode(&rules) + require.NoError(t, err) + + u := fmt.Sprintf("http://admin:admin@%s/api/ruler/grafana/api/v1/rules/folder1", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", &buf) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, http.StatusAccepted, resp.StatusCode) + require.JSONEq(t, `{"message":"rule group updated successfully"}`, string(b)) + + t.Run("trying to create alert with same title under same folder should fail", func(t *testing.T) { + buf := bytes.Buffer{} + enc := json.NewEncoder(&buf) + err = enc.Encode(&rules) + require.NoError(t, err) + + u := fmt.Sprintf("http://admin:admin@%s/api/ruler/grafana/api/v1/rules/folder1", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", &buf) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + require.JSONEq(t, `{"message":"failed to update rule group: a conflicting alert rule is found: rule title under the same organisation and folder should be unique"}`, string(b)) + }) + + t.Run("trying to create alert with same title under another folder should succeed", func(t *testing.T) { + buf := bytes.Buffer{} + enc := json.NewEncoder(&buf) + err = enc.Encode(&rules) + require.NoError(t, err) + + u := fmt.Sprintf("http://admin:admin@%s/api/ruler/grafana/api/v1/rules/folder2", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", &buf) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, http.StatusAccepted, resp.StatusCode) + require.JSONEq(t, `{"message":"rule group updated successfully"}`, string(b)) + }) +}