Alerting: Allow rules with same title across folders (#35270)

* Alerting: Allow rules with same title across folders

* Add test

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

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
 		}
 

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

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))
+	})
+}