Alerting: Update rules delete endpoint to handle rules in group (#53790)

* update RouteDeleteAlertRules rules to update as a group
* remove expecter from scheduler mock to support variadic function
* create function to check for provisioning status + tests

Co-authored-by: Alexander Weaver <weaver.alex.d@gmail.com>

pkg/services/ngalert/api/api_ruler.go

@@ -44,8 +44,10 @@ var (
 	errProvisionedResource = errors.New("request affects resources created via provisioning API")
 )
 
-// RouteDeleteAlertRules deletes all alert rules user is authorized to access in the given namespace
+// RouteDeleteAlertRules deletes all alert rules the user is authorized to access in the given namespace
-// or, if non-empty, a specific group of rules in the namespace
+// or, if non-empty, a specific group of rules in the namespace.
+// Returns http.StatusUnauthorized if user does not have access to any of the rules that match the filter.
+// Returns http.StatusBadRequest if all rules that match the filter and the user is authorized to delete are provisioned.
 func (srv RulerSrv) RouteDeleteAlertRules(c *models.ReqContext, namespaceTitle string, group string) response.Response {
 	namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser, true)
 	if err != nil {
@@ -71,8 +73,9 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *models.ReqContext, namespaceTitle s
 		return ErrResp(http.StatusInternalServerError, err, "failed to fetch provenances of alert rules")
 	}
 
-	var deletableRules []string
+	deletedGroups := make(map[ngmodels.AlertRuleGroupKey][]ngmodels.AlertRuleKey)
 	err = srv.xactManager.InTransaction(c.Req.Context(), func(ctx context.Context) error {
+		unauthz, provisioned := false, false
 		q := ngmodels.ListAlertRulesQuery{
 			OrgID:         c.SignedInUser.OrgID,
 			NamespaceUIDs: []string{namespace.Uid},
@@ -87,68 +90,60 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *models.ReqContext, namespaceTitle s
 			return nil
 		}
 
-		var canDelete []*ngmodels.AlertRule
+		var deletionCandidates = make(map[ngmodels.AlertRuleGroupKey][]*ngmodels.AlertRule)
-		var cannotDelete []string
+		for _, rule := range q.Result {
-
+			key := rule.GetGroupKey()
-		// partition will partation the given rules in two, one partition
+			deletionCandidates[key] = append(deletionCandidates[key], rule)
-		// being the rules that fulfill the predicate the other partation being
+		}
-		// the ruleIDs not fulfilling it.
+		rulesToDelete := make([]string, 0, len(q.Result))
-		partition := func(alerts []*ngmodels.AlertRule, predicate func(rule *ngmodels.AlertRule) bool) ([]*ngmodels.AlertRule, []string) {
+		for groupKey, rules := range deletionCandidates {
-			positive, negative := make([]*ngmodels.AlertRule, 0, len(alerts)), make([]string, 0, len(alerts))
+			if !authorizeAccessToRuleGroup(rules, hasAccess) {
-			for _, rule := range alerts {
+				unauthz = true
-				if predicate(rule) {
+				continue
-					positive = append(positive, rule)
-					continue
-				}
-				negative = append(negative, rule.UID)
 			}
-			return positive, negative
+			if containsProvisionedAlerts(provenances, rules) {
+				provisioned = true
+				continue
+			}
+			uid := make([]string, 0, len(rules))
+			keys := make([]ngmodels.AlertRuleKey, 0, len(rules))
+			for _, rule := range rules {
+				uid = append(uid, rule.UID)
+				keys = append(keys, rule.GetKey())
+			}
+			rulesToDelete = append(rulesToDelete, uid...)
+			deletedGroups[groupKey] = keys
 		}
-
+		if len(rulesToDelete) > 0 {
-		canDelete, cannotDelete = partition(q.Result, func(rule *ngmodels.AlertRule) bool {
+			return srv.store.DeleteAlertRulesByUID(ctx, c.SignedInUser.OrgID, rulesToDelete...)
-			return authorizeDatasourceAccessForRule(rule, hasAccess)
-		})
-		if len(canDelete) == 0 {
-			return fmt.Errorf("%w to delete rules because user is not authorized to access data sources used by the rules", ErrAuthorization)
 		}
-		if len(cannotDelete) > 0 {
+		// if none rules were deleted return an error.
-			logger.Info("user cannot delete one or many alert rules because it does not have access to data sources. Those rules will be skipped", "expected", len(q.Result), "authorized", len(canDelete), "unauthorized", cannotDelete)
+		// Check whether provisioned check failed first because if it is true, then all rules that the user can access (actually read via GET API) are provisioned.
+		if provisioned {
+			return errProvisionedResource
 		}
-
+		if unauthz {
-		canDelete, cannotDelete = partition(canDelete, func(rule *ngmodels.AlertRule) bool {
+			if group == "" {
-			provenance, exists := provenances[rule.UID]
+				return fmt.Errorf("%w to delete any existing rules in the namespace", ErrAuthorization)
-			return (exists && provenance == ngmodels.ProvenanceNone) || !exists
+			}
-		})
+			return fmt.Errorf("%w to delete group of the rules", ErrAuthorization)
-
-		if len(canDelete) == 0 {
-			return fmt.Errorf("all rules have been provisioned and cannot be deleted through this api")
 		}
-
+		return nil
-		if len(cannotDelete) > 0 {
-			logger.Info("user cannot delete one or many alert rules because it does have a provenance set. Those rules will be skipped", "expected", len(q.Result), "provenance_none", len(canDelete), "provenance_set", cannotDelete)
-		}
-
-		for _, rule := range canDelete {
-			deletableRules = append(deletableRules, rule.UID)
-		}
-
-		return srv.store.DeleteAlertRulesByUID(ctx, c.SignedInUser.OrgID, deletableRules...)
 	})
 
 	if err != nil {
 		if errors.Is(err, ErrAuthorization) {
-			return ErrResp(http.StatusUnauthorized, err, "")
+			return ErrResp(http.StatusUnauthorized, err, "failed to delete rule group")
+		}
+		if errors.Is(err, errProvisionedResource) {
+			return ErrResp(http.StatusBadRequest, err, "failed to delete rule group")
 		}
 		return ErrResp(http.StatusInternalServerError, err, "failed to delete rule group")
 	}
 
 	logger.Debug("rules have been deleted from the store. updating scheduler")
-
+	for _, ruleKeys := range deletedGroups {
-	for _, uid := range deletableRules {
+		srv.scheduleService.DeleteAlertRule(ruleKeys...)
-		srv.scheduleService.DeleteAlertRule(ngmodels.AlertRuleKey{
-			OrgID: c.SignedInUser.OrgID,
-			UID:   uid,
-		})
 	}
 
 	return response.JSON(http.StatusAccepted, util.DynMap{"message": "rules deleted"})
@@ -427,11 +422,12 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *models.ReqContext, groupKey ngmod
 		}, rule.Existing.Version+1)
 	}
 
-	for _, rule := range finalChanges.Delete {
+	if len(finalChanges.Delete) > 0 {
-		srv.scheduleService.DeleteAlertRule(ngmodels.AlertRuleKey{
+		keys := make([]ngmodels.AlertRuleKey, 0, len(finalChanges.Delete))
-			OrgID: c.SignedInUser.OrgID,
+		for _, rule := range finalChanges.Delete {
-			UID:   rule.UID,
+			keys = append(keys, rule.GetKey())
-		})
+		}
+		srv.scheduleService.DeleteAlertRule(keys...)
 	}
 
 	if finalChanges.IsEmpty() {
@@ -510,16 +506,13 @@ func verifyProvisionedRulesNotAffected(ctx context.Context, provenanceStore prov
 	}
 	errorMsg := strings.Builder{}
 	for group, alertRules := range ch.AffectedGroups {
-		for _, rule := range alertRules {
+		if !containsProvisionedAlerts(provenances, alertRules) {
-			if provenance, exists := provenances[rule.UID]; (exists && provenance == ngmodels.ProvenanceNone) || !exists {
+			continue
-				continue
-			}
-			if errorMsg.Len() > 0 {
-				errorMsg.WriteRune(',')
-			}
-			errorMsg.WriteString(group.String())
-			break
 		}
+		if errorMsg.Len() > 0 {
+			errorMsg.WriteRune(',')
+		}
+		errorMsg.WriteString(group.String())
 	}
 	if errorMsg.Len() == 0 {
 		return nil

pkg/services/ngalert/api/api_ruler_test.go

@@ -55,217 +55,208 @@ func TestRouteDeleteAlertRules(t *testing.T) {
 			require.Containsf(t, actualUIDs, rule.UID, "Rule %s was expected to be deleted but it wasn't", rule.UID)
 		}
 
-		require.Len(t, scheduler.Calls, len(expectedRules))
+		notDeletedRules := make(map[models.AlertRuleKey]struct{}, len(expectedRules))
+		for _, rule := range expectedRules {
+			notDeletedRules[rule.GetKey()] = struct{}{}
+		}
 		for _, call := range scheduler.Calls {
 			require.Equal(t, "DeleteAlertRule", call.Method)
-			key, ok := call.Arguments.Get(0).(models.AlertRuleKey)
+			keys, ok := call.Arguments.Get(0).([]models.AlertRuleKey)
 			require.Truef(t, ok, "Expected AlertRuleKey but got something else")
-			found := false
+			for _, key := range keys {
-			for _, rule := range expectedRules {
+				delete(notDeletedRules, key)
-				if rule.GetKey() == key {
-					found = true
-					break
-				}
 			}
-			require.Truef(t, found, "Key %v was not expected to be submitted to scheduler", key)
 		}
+		require.Emptyf(t, notDeletedRules, "Not all rules were deleted")
+	}
+
+	orgID := rand.Int63()
+	folder := randFolder()
+
+	initFakeRuleStore := func(t *testing.T) *store.FakeRuleStore {
+		ruleStore := store.NewFakeRuleStore(t)
+		ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
+		// add random data
+		ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID)))...)
+		return ruleStore
 	}
 
 	t.Run("when fine-grained access is disabled", func(t *testing.T) {
+		ac := acMock.New().WithDisabled()
 		t.Run("viewer should not be authorized", func(t *testing.T) {
-			ruleStore := store.NewFakeRuleStore(t)
+			ruleStore := initFakeRuleStore(t)
-			orgID := rand.Int63()
+			ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
-			folder := randFolder()
-			ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
 
 			scheduler := &schedule.FakeScheduleService{}
-			scheduler.On("DeleteAlertRule", mock.Anything).Panic("should not be called")
+			scheduler.On("DeleteAlertRule", mock.Anything)
 
-			ac := acMock.New().WithDisabled()
 			request := createRequestContext(orgID, org.RoleViewer, nil)
 			response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, "")
-			require.Equalf(t, 401, response.Status(), "Expected 403 but got %d: %v", response.Status(), string(response.Body()))
+			require.Equalf(t, 401, response.Status(), "Expected 401 but got %d: %v", response.Status(), string(response.Body()))
 
 			scheduler.AssertNotCalled(t, "DeleteAlertRule")
 			require.Empty(t, getRecordedCommand(ruleStore))
 		})
-		t.Run("editor should be able to delete all rules in folder", func(t *testing.T) {
+		t.Run("editor should be able to delete all non-provisioned rules in folder", func(t *testing.T) {
-			ruleStore := store.NewFakeRuleStore(t)
+			ruleStore := initFakeRuleStore(t)
-			orgID := rand.Int63()
+			rulesInFolder := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
-			folder := randFolder()
-			ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
-			rulesInFolder := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
 			ruleStore.PutRule(context.Background(), rulesInFolder...)
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
 
 			scheduler := &schedule.FakeScheduleService{}
 			scheduler.On("DeleteAlertRule", mock.Anything)
 
-			ac := acMock.New().WithDisabled()
 			request := createRequestContext(orgID, org.RoleEditor, nil)
 			response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, "")
+
 			require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
 			assertRulesDeleted(t, rulesInFolder, ruleStore, scheduler)
 		})
-		t.Run("editor should be able to delete rules in a group in a folder", func(t *testing.T) {
+		t.Run("editor should be able to delete rules group if it is not provisioned", func(t *testing.T) {
-			ruleStore := store.NewFakeRuleStore(t)
-			orgID := rand.Int63()
 			groupName := util.GenerateShortUID()
-			folder := randFolder()
+			rulesInFolderInGroup := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))
-			ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
+
-			rulesInFolderInGroup := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))
+			ruleStore := initFakeRuleStore(t)
 			ruleStore.PutRule(context.Background(), rulesInFolderInGroup...)
 			// rules in different groups but in the same namespace
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
+			ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
 			// rules in the same group but different folder
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withGroup(groupName)))...)
+			ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withGroup(groupName)))...)
 
 			scheduler := &schedule.FakeScheduleService{}
-			scheduler.On("DeleteAlertRule", mock.Anything)
+			scheduler.On("DeleteAlertRule", mock.Anything).Return()
 
-			ac := acMock.New().WithDisabled()
 			request := createRequestContext(orgID, org.RoleEditor, nil)
 			response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, groupName)
+
 			require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
 			assertRulesDeleted(t, rulesInFolderInGroup, ruleStore, scheduler)
 		})
-		t.Run("editor shouldn't be able to delete provisioned rules", func(t *testing.T) {
+		t.Run("should return 202 if folder is empty", func(t *testing.T) {
-			ruleStore := store.NewFakeRuleStore(t)
+			ruleStore := initFakeRuleStore(t)
-			orgID := rand.Int63()
-			folder := randFolder()
-			ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
-			rulesInFolder := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
-			ruleStore.PutRule(context.Background(), rulesInFolder...)
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
 
 			scheduler := &schedule.FakeScheduleService{}
 			scheduler.On("DeleteAlertRule", mock.Anything)
 
-			ac := acMock.New().WithDisabled()
+			requestCtx := createRequestContext(orgID, org.RoleEditor, nil)
+			response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(requestCtx, folder.Title, "")
 
-			svc := createService(ac, ruleStore, scheduler)
-
-			err := svc.provenanceStore.SetProvenance(context.Background(), rulesInFolder[0], orgID, models.ProvenanceAPI)
-			require.NoError(t, err)
-
-			request := createRequestContext(orgID, org.RoleEditor, nil)
-			response := svc.RouteDeleteAlertRules(request, folder.Title, "")
 			require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
-			assertRulesDeleted(t, rulesInFolder[1:], ruleStore, scheduler)
-		})
-	})
-	t.Run("when fine-grained access is enabled", func(t *testing.T) {
-		t.Run("and user does not have access to any of data sources used by alert rules", func(t *testing.T) {
-			ruleStore := store.NewFakeRuleStore(t)
-			orgID := rand.Int63()
-			folder := randFolder()
-			ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
-			ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
-
-			scheduler := &schedule.FakeScheduleService{}
-			scheduler.On("DeleteAlertRule", mock.Anything).Panic("should not be called")
-
-			ac := acMock.New()
-			request := createRequestContext(orgID, "None", nil)
-			response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, "")
-			require.Equalf(t, 401, response.Status(), "Expected 403 but got %d: %v", response.Status(), string(response.Body()))
-
 			scheduler.AssertNotCalled(t, "DeleteAlertRule")
 			require.Empty(t, getRecordedCommand(ruleStore))
 		})
-		t.Run("and user has access to all alert rules", func(t *testing.T) {
+	})
-			t.Run("should delete all rules", func(t *testing.T) {
+	t.Run("when fine-grained access is enabled", func(t *testing.T) {
-				ruleStore := store.NewFakeRuleStore(t)
+		requestCtx := createRequestContext(orgID, "None", nil)
-				orgID := rand.Int63()
+
-				folder := randFolder()
+		t.Run("and group argument is empty", func(t *testing.T) {
-				ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
+			t.Run("return 401 if user is not authorized to access any group in the folder", func(t *testing.T) {
-				rulesInFolder := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
+				ruleStore := initFakeRuleStore(t)
-				ruleStore.PutRule(context.Background(), rulesInFolder...)
+				ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
 
 				scheduler := &schedule.FakeScheduleService{}
-				scheduler.On("DeleteAlertRule", mock.Anything)
+				scheduler.On("DeleteAlertRule", mock.Anything).Panic("should not be called")
 
-				ac := acMock.New().WithPermissions(createPermissionsForRules(rulesInFolder))
+				ac := acMock.New()
 				request := createRequestContext(orgID, "None", nil)
 
 				response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, "")
-				require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
+				require.Equalf(t, 401, response.Status(), "Expected 401 but got %d: %v", response.Status(), string(response.Body()))
-				assertRulesDeleted(t, rulesInFolder, ruleStore, scheduler)
+
+				scheduler.AssertNotCalled(t, "DeleteAlertRule")
+				require.Empty(t, getRecordedCommand(ruleStore))
 			})
-			t.Run("shouldn't be able to delete provisioned rules", func(t *testing.T) {
+			t.Run("delete only non-provisioned groups that user is authorized", func(t *testing.T) {
-				ruleStore := store.NewFakeRuleStore(t)
+				ruleStore := initFakeRuleStore(t)
-				orgID := rand.Int63()
+				provisioningStore := provisioning.NewFakeProvisioningStore()
-				folder := randFolder()
-				ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
-				rulesInFolder := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
-				ruleStore.PutRule(context.Background(), rulesInFolder...)
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
 
 				scheduler := &schedule.FakeScheduleService{}
 				scheduler.On("DeleteAlertRule", mock.Anything)
 
-				ac := acMock.New().WithPermissions(createPermissionsForRules(rulesInFolder))
+				authorizedRulesInFolder := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup("authz_"+util.GenerateShortUID())))
-				svc := createService(ac, ruleStore, scheduler)
 
-				err := svc.provenanceStore.SetProvenance(context.Background(), rulesInFolder[0], orgID, models.ProvenanceAPI)
+				provisionedRulesInFolder := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup("provisioned_"+util.GenerateShortUID())))
+				err := provisioningStore.SetProvenance(context.Background(), provisionedRulesInFolder[0], orgID, models.ProvenanceAPI)
 				require.NoError(t, err)
 
-				request := createRequestContext(orgID, "None", nil)
-
-				response := svc.RouteDeleteAlertRules(request, folder.Title, "")
-				require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
-				assertRulesDeleted(t, rulesInFolder[1:], ruleStore, scheduler)
-			})
-		})
-		t.Run("and user has access to data sources of some of alert rules", func(t *testing.T) {
-			t.Run("should delete only those that are accessible in folder", func(t *testing.T) {
-				ruleStore := store.NewFakeRuleStore(t)
-				orgID := rand.Int63()
-				folder := randFolder()
-				ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
-				authorizedRulesInFolder := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))
 				ruleStore.PutRule(context.Background(), authorizedRulesInFolder...)
+				ruleStore.PutRule(context.Background(), provisionedRulesInFolder...)
 				// more rules in the same namespace but user does not have access to them
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
+				ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup("unauthz"+util.GenerateShortUID())))...)
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID)))...)
 
-				scheduler := &schedule.FakeScheduleService{}
+				ac := acMock.New().WithPermissions(createPermissionsForRules(append(authorizedRulesInFolder, provisionedRulesInFolder...)))
-				scheduler.On("DeleteAlertRule", mock.Anything)
 
-				ac := acMock.New().WithPermissions(createPermissionsForRules(authorizedRulesInFolder))
+				response := createServiceWithProvenanceStore(ac, ruleStore, scheduler, provisioningStore).RouteDeleteAlertRules(requestCtx, folder.Title, "")
-				request := createRequestContext(orgID, "None", nil)
 
-				response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, "")
 				require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
 				assertRulesDeleted(t, authorizedRulesInFolder, ruleStore, scheduler)
 			})
-			t.Run("should delete only rules in a group that are authorized", func(t *testing.T) {
+			t.Run("return 400 if all rules user can access are provisioned", func(t *testing.T) {
-				ruleStore := store.NewFakeRuleStore(t)
+				ruleStore := initFakeRuleStore(t)
-				orgID := rand.Int63()
+				provisioningStore := provisioning.NewFakeProvisioningStore()
-				groupName := util.GenerateShortUID()
+
-				folder := randFolder()
+				provisionedRulesInFolder := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(util.GenerateShortUID())))
-				ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], folder)
+				err := provisioningStore.SetProvenance(context.Background(), provisionedRulesInFolder[0], orgID, models.ProvenanceAPI)
-				authorizedRulesInGroup := models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))
+				require.NoError(t, err)
+
+				ruleStore.PutRule(context.Background(), provisionedRulesInFolder...)
+				// more rules in the same namespace but user does not have access to them
+				ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(util.GenerateShortUID())))...)
+
+				scheduler := &schedule.FakeScheduleService{}
+				scheduler.On("DeleteAlertRule", mock.Anything)
+
+				ac := acMock.New().WithPermissions(createPermissionsForRules(provisionedRulesInFolder))
+
+				response := createServiceWithProvenanceStore(ac, ruleStore, scheduler, provisioningStore).RouteDeleteAlertRules(requestCtx, folder.Title, "")
+
+				require.Equalf(t, 400, response.Status(), "Expected 400 but got %d: %v", response.Status(), string(response.Body()))
+				scheduler.AssertNotCalled(t, "DeleteAlertRule")
+				require.Empty(t, getRecordedCommand(ruleStore))
+			})
+		})
+		t.Run("and group argument is not empty", func(t *testing.T) {
+			groupName := util.GenerateShortUID()
+			t.Run("return 401 if user is not authorized to access the group", func(t *testing.T) {
+				ruleStore := initFakeRuleStore(t)
+
+				authorizedRulesInGroup := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))
 				ruleStore.PutRule(context.Background(), authorizedRulesInGroup...)
 				// more rules in the same group but user is not authorized to access them
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))...)
+				ruleStore.PutRule(context.Background(), models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))...)
-				// rules in different groups but in the same namespace
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withNamespace(folder)))...)
-				// rules in the same group but different folder
-				ruleStore.PutRule(context.Background(), models.GenerateAlertRules(rand.Intn(4)+2, models.AlertRuleGen(withOrgID(orgID), withGroup(groupName)))...)
 
 				scheduler := &schedule.FakeScheduleService{}
 				scheduler.On("DeleteAlertRule", mock.Anything)
 
 				ac := acMock.New().WithPermissions(createPermissionsForRules(authorizedRulesInGroup))
-				request := createRequestContext(orgID, "None", nil)
+
-				response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(request, folder.Title, groupName)
+				response := createService(ac, ruleStore, scheduler).RouteDeleteAlertRules(requestCtx, folder.Title, groupName)
-				require.Equalf(t, 202, response.Status(), "Expected 202 but got %d: %v", response.Status(), string(response.Body()))
+
-				assertRulesDeleted(t, authorizedRulesInGroup, ruleStore, scheduler)
+				require.Equalf(t, 401, response.Status(), "Expected 401 but got %d: %v", response.Status(), string(response.Body()))
+				scheduler.AssertNotCalled(t, "DeleteAlertRule", mock.Anything)
+				deleteCommands := getRecordedCommand(ruleStore)
+				require.Empty(t, deleteCommands)
+			})
+			t.Run("return 400 if group is provisioned", func(t *testing.T) {
+				ruleStore := initFakeRuleStore(t)
+				provisioningStore := provisioning.NewFakeProvisioningStore()
+
+				provisionedRulesInFolder := models.GenerateAlertRulesSmallNonEmpty(models.AlertRuleGen(withOrgID(orgID), withNamespace(folder), withGroup(groupName)))
+				err := provisioningStore.SetProvenance(context.Background(), provisionedRulesInFolder[0], orgID, models.ProvenanceAPI)
+				require.NoError(t, err)
+
+				ruleStore.PutRule(context.Background(), provisionedRulesInFolder...)
+
+				scheduler := &schedule.FakeScheduleService{}
+				scheduler.On("DeleteAlertRule", mock.Anything)
+
+				ac := acMock.New().WithPermissions(createPermissionsForRules(provisionedRulesInFolder))
+
+				response := createServiceWithProvenanceStore(ac, ruleStore, scheduler, provisioningStore).RouteDeleteAlertRules(requestCtx, folder.Title, groupName)
+
+				require.Equalf(t, 400, response.Status(), "Expected 400 but got %d: %v", response.Status(), string(response.Body()))
+				scheduler.AssertNotCalled(t, "DeleteAlertRule", mock.Anything)
+				deleteCommands := getRecordedCommand(ruleStore)
+				require.Empty(t, deleteCommands)
 			})
 		})
 	})
@@ -645,6 +636,12 @@ func TestVerifyProvisionedRulesNotAffected(t *testing.T) {
 	})
 }
 
+func createServiceWithProvenanceStore(ac *acMock.Mock, store *store.FakeRuleStore, scheduler schedule.ScheduleService, provenanceStore provisioning.ProvisioningStore) *RulerSrv {
+	svc := createService(ac, store, scheduler)
+	svc.provenanceStore = provenanceStore
+	return svc
+}
+
 func createService(ac *acMock.Mock, store *store.FakeRuleStore, scheduler schedule.ScheduleService) *RulerSrv {
 	return &RulerSrv{
 		xactManager:     store,

pkg/services/ngalert/api/util.go

@@ -276,3 +276,16 @@ func ErrResp(status int, err error, msg string, args ...interface{}) *response.N
 func accessForbiddenResp() response.Response {
 	return ErrResp(http.StatusForbidden, errors.New("Permission denied"), "")
 }
+
+func containsProvisionedAlerts(provenances map[string]ngmodels.Provenance, rules []*ngmodels.AlertRule) bool {
+	if len(provenances) == 0 {
+		return false
+	}
+	for _, rule := range rules {
+		provenance, ok := provenances[rule.UID]
+		if ok && provenance != ngmodels.ProvenanceNone {
+			return true
+		}
+	}
+	return false
+}

pkg/services/ngalert/api/util_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	models2 "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/util"
@@ -156,3 +157,21 @@ func TestAlertingProxy_createProxyContext(t *testing.T) {
 		})
 	})
 }
+
+func Test_containsProvisionedAlerts(t *testing.T) {
+	t.Run("should return true if at least one rule is provisioned", func(t *testing.T) {
+		_, rules := models2.GenerateUniqueAlertRules(rand.Intn(4)+2, models2.AlertRuleGen())
+		provenance := map[string]models2.Provenance{
+			rules[rand.Intn(len(rules)-1)].UID: []models2.Provenance{models2.ProvenanceAPI, models2.ProvenanceFile}[rand.Intn(2)],
+		}
+		require.Truef(t, containsProvisionedAlerts(provenance, rules), "the group of rules is expected to be considered as provisioned but it isn't. Provenances: %v", provenance)
+	})
+	t.Run("should return false if map does not contain or has ProvenanceNone", func(t *testing.T) {
+		_, rules := models2.GenerateUniqueAlertRules(rand.Intn(5)+1, models2.AlertRuleGen())
+		provenance := make(map[string]models2.Provenance)
+		for i := 0; i < rand.Intn(len(rules)); i++ {
+			provenance[rules[i].UID] = models2.ProvenanceNone
+		}
+		require.Falsef(t, containsProvisionedAlerts(provenance, rules), "the group of rules is not expected to be provisioned but it is. Provenances: %v", provenance)
+	})
+}

pkg/services/ngalert/models/testing.go

@@ -188,6 +188,11 @@ func GenerateUniqueAlertRules(count int, f func() *AlertRule) (map[string]*Alert
 	return uIDs, result
 }
 
+// GenerateAlertRulesSmallNonEmpty generates 1 to 5 rules using the provided generator
+func GenerateAlertRulesSmallNonEmpty(f func() *AlertRule) []*AlertRule {
+	return GenerateAlertRules(rand.Intn(4)+1, f)
+}
+
 // GenerateAlertRules generates many random alert rules. Does not guarantee that rules are unique (by UID)
 func GenerateAlertRules(count int, f func() *AlertRule) []*AlertRule {
 	result := make([]*AlertRule, 0, count)

pkg/services/ngalert/ngalert_test.go

@@ -34,7 +34,7 @@ func Test_subscribeToFolderChanges(t *testing.T) {
 	db.PutRule(context.Background(), rules...)
 
 	scheduler := &schedule.FakeScheduleService{}
-	scheduler.EXPECT().UpdateAlertRule(mock.Anything, mock.Anything).Return()
+	scheduler.On("UpdateAlertRule", mock.Anything, mock.Anything).Return()
 
 	subscribeToFolderChanges(log.New("test"), bus, db, scheduler)
 

pkg/services/ngalert/schedule/schedule.go

@@ -26,15 +26,15 @@ import (
 
 // ScheduleService is an interface for a service that schedules the evaluation
 // of alert rules.
-//go:generate mockery --name ScheduleService --structname FakeScheduleService --inpackage --filename schedule_mock.go --with-expecter
+//go:generate mockery --name ScheduleService --structname FakeScheduleService --inpackage --filename schedule_mock.go --unroll-variadic=False
 type ScheduleService interface {
 	// Run the scheduler until the context is canceled or the scheduler returns
 	// an error. The scheduler is terminated when this function returns.
 	Run(context.Context) error
 	// UpdateAlertRule notifies scheduler that a rule has been changed
 	UpdateAlertRule(key ngmodels.AlertRuleKey, lastVersion int64)
-	// DeleteAlertRule notifies scheduler that a rule has been changed
+	// DeleteAlertRule notifies scheduler that rules have been deleted
-	DeleteAlertRule(key ngmodels.AlertRuleKey)
+	DeleteAlertRule(keys ...ngmodels.AlertRuleKey)
 	// the following are used by tests only used for tests
 	evalApplied(ngmodels.AlertRuleKey, time.Time)
 	stopApplied(ngmodels.AlertRuleKey)
@@ -153,23 +153,23 @@ func (sch *schedule) UpdateAlertRule(key ngmodels.AlertRuleKey, lastVersion int6
 }
 
 // DeleteAlertRule stops evaluation of the rule, deletes it from active rules, and cleans up state cache.
-func (sch *schedule) DeleteAlertRule(key ngmodels.AlertRuleKey) {
+func (sch *schedule) DeleteAlertRule(keys ...ngmodels.AlertRuleKey) {
-	// It can happen that the scheduler has deleted the alert rule before the
+	for _, key := range keys {
-	// Ruler API has called DeleteAlertRule. This can happen as requests to
+		// It can happen that the scheduler has deleted the alert rule before the
-	// the Ruler API do not hold an exclusive lock over all scheduler operations.
+		// Ruler API has called DeleteAlertRule. This can happen as requests to
-	if _, ok := sch.schedulableAlertRules.del(key); !ok {
+		// the Ruler API do not hold an exclusive lock over all scheduler operations.
-		sch.log.Info("alert rule cannot be removed from the scheduler as it is not scheduled", "uid", key.UID, "org_id", key.OrgID)
+		if _, ok := sch.schedulableAlertRules.del(key); !ok {
+			sch.log.Info("alert rule cannot be removed from the scheduler as it is not scheduled", "uid", key.UID, "org_id", key.OrgID)
+		}
+		// Delete the rule routine
+		ruleInfo, ok := sch.registry.del(key)
+		if !ok {
+			sch.log.Info("alert rule cannot be stopped as it is not running", "uid", key.UID, "org_id", key.OrgID)
+			continue
+		}
+		// stop rule evaluation
+		ruleInfo.stop()
 	}
-
-	// Delete the rule routine
-	ruleInfo, ok := sch.registry.del(key)
-	if !ok {
-		sch.log.Info("alert rule cannot be stopped as it is not running", "uid", key.UID, "org_id", key.OrgID)
-		return
-	}
-	// stop rule evaluation
-	ruleInfo.stop()
-
 	// Our best bet at this point is that we update the metrics with what we hope to schedule in the next tick.
 	alertRules := sch.schedulableAlertRules.all()
 	sch.metrics.SchedulableAlertRules.Set(float64(len(alertRules)))

pkg/services/ngalert/schedule/schedule_mock.go

@@ -16,40 +16,9 @@ type FakeScheduleService struct {
 	mock.Mock
 }
 
-type FakeScheduleService_Expecter struct {
+// DeleteAlertRule provides a mock function with given fields: keys
-	mock *mock.Mock
+func (_m *FakeScheduleService) DeleteAlertRule(keys ...models.AlertRuleKey) {
-}
+	_m.Called(keys)
-
-func (_m *FakeScheduleService) EXPECT() *FakeScheduleService_Expecter {
-	return &FakeScheduleService_Expecter{mock: &_m.Mock}
-}
-
-// DeleteAlertRule provides a mock function with given fields: key
-func (_m *FakeScheduleService) DeleteAlertRule(key models.AlertRuleKey) {
-	_m.Called(key)
-}
-
-// FakeScheduleService_DeleteAlertRule_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteAlertRule'
-type FakeScheduleService_DeleteAlertRule_Call struct {
-	*mock.Call
-}
-
-// DeleteAlertRule is a helper method to define mock.On call
-//  - key models.AlertRuleKey
-func (_e *FakeScheduleService_Expecter) DeleteAlertRule(key interface{}) *FakeScheduleService_DeleteAlertRule_Call {
-	return &FakeScheduleService_DeleteAlertRule_Call{Call: _e.mock.On("DeleteAlertRule", key)}
-}
-
-func (_c *FakeScheduleService_DeleteAlertRule_Call) Run(run func(key models.AlertRuleKey)) *FakeScheduleService_DeleteAlertRule_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(models.AlertRuleKey))
-	})
-	return _c
-}
-
-func (_c *FakeScheduleService_DeleteAlertRule_Call) Return() *FakeScheduleService_DeleteAlertRule_Call {
-	_c.Call.Return()
-	return _c
 }
 
 // Run provides a mock function with given fields: _a0
@@ -66,139 +35,22 @@ func (_m *FakeScheduleService) Run(_a0 context.Context) error {
 	return r0
 }
 
-// FakeScheduleService_Run_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Run'
-type FakeScheduleService_Run_Call struct {
-	*mock.Call
-}
-
-// Run is a helper method to define mock.On call
-//  - _a0 context.Context
-func (_e *FakeScheduleService_Expecter) Run(_a0 interface{}) *FakeScheduleService_Run_Call {
-	return &FakeScheduleService_Run_Call{Call: _e.mock.On("Run", _a0)}
-}
-
-func (_c *FakeScheduleService_Run_Call) Run(run func(_a0 context.Context)) *FakeScheduleService_Run_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(context.Context))
-	})
-	return _c
-}
-
-func (_c *FakeScheduleService_Run_Call) Return(_a0 error) *FakeScheduleService_Run_Call {
-	_c.Call.Return(_a0)
-	return _c
-}
-
 // UpdateAlertRule provides a mock function with given fields: key, lastVersion
 func (_m *FakeScheduleService) UpdateAlertRule(key models.AlertRuleKey, lastVersion int64) {
 	_m.Called(key, lastVersion)
 }
 
-// FakeScheduleService_UpdateAlertRule_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'UpdateAlertRule'
-type FakeScheduleService_UpdateAlertRule_Call struct {
-	*mock.Call
-}
-
-// UpdateAlertRule is a helper method to define mock.On call
-//  - key models.AlertRuleKey
-//  - lastVersion int64
-func (_e *FakeScheduleService_Expecter) UpdateAlertRule(key interface{}, lastVersion interface{}) *FakeScheduleService_UpdateAlertRule_Call {
-	return &FakeScheduleService_UpdateAlertRule_Call{Call: _e.mock.On("UpdateAlertRule", key, lastVersion)}
-}
-
-func (_c *FakeScheduleService_UpdateAlertRule_Call) Run(run func(key models.AlertRuleKey, lastVersion int64)) *FakeScheduleService_UpdateAlertRule_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(models.AlertRuleKey), args[1].(int64))
-	})
-	return _c
-}
-
-func (_c *FakeScheduleService_UpdateAlertRule_Call) Return() *FakeScheduleService_UpdateAlertRule_Call {
-	_c.Call.Return()
-	return _c
-}
-
 // evalApplied provides a mock function with given fields: _a0, _a1
 func (_m *FakeScheduleService) evalApplied(_a0 models.AlertRuleKey, _a1 time.Time) {
 	_m.Called(_a0, _a1)
 }
 
-// FakeScheduleService_evalApplied_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'evalApplied'
-type FakeScheduleService_evalApplied_Call struct {
-	*mock.Call
-}
-
-// evalApplied is a helper method to define mock.On call
-//  - _a0 models.AlertRuleKey
-//  - _a1 time.Time
-func (_e *FakeScheduleService_Expecter) evalApplied(_a0 interface{}, _a1 interface{}) *FakeScheduleService_evalApplied_Call {
-	return &FakeScheduleService_evalApplied_Call{Call: _e.mock.On("evalApplied", _a0, _a1)}
-}
-
-func (_c *FakeScheduleService_evalApplied_Call) Run(run func(_a0 models.AlertRuleKey, _a1 time.Time)) *FakeScheduleService_evalApplied_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(models.AlertRuleKey), args[1].(time.Time))
-	})
-	return _c
-}
-
-func (_c *FakeScheduleService_evalApplied_Call) Return() *FakeScheduleService_evalApplied_Call {
-	_c.Call.Return()
-	return _c
-}
-
 // overrideCfg provides a mock function with given fields: cfg
 func (_m *FakeScheduleService) overrideCfg(cfg SchedulerCfg) {
 	_m.Called(cfg)
 }
 
-// FakeScheduleService_overrideCfg_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'overrideCfg'
-type FakeScheduleService_overrideCfg_Call struct {
-	*mock.Call
-}
-
-// overrideCfg is a helper method to define mock.On call
-//  - cfg SchedulerCfg
-func (_e *FakeScheduleService_Expecter) overrideCfg(cfg interface{}) *FakeScheduleService_overrideCfg_Call {
-	return &FakeScheduleService_overrideCfg_Call{Call: _e.mock.On("overrideCfg", cfg)}
-}
-
-func (_c *FakeScheduleService_overrideCfg_Call) Run(run func(cfg SchedulerCfg)) *FakeScheduleService_overrideCfg_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(SchedulerCfg))
-	})
-	return _c
-}
-
-func (_c *FakeScheduleService_overrideCfg_Call) Return() *FakeScheduleService_overrideCfg_Call {
-	_c.Call.Return()
-	return _c
-}
-
 // stopApplied provides a mock function with given fields: _a0
 func (_m *FakeScheduleService) stopApplied(_a0 models.AlertRuleKey) {
 	_m.Called(_a0)
 }
-
-// FakeScheduleService_stopApplied_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'stopApplied'
-type FakeScheduleService_stopApplied_Call struct {
-	*mock.Call
-}
-
-// stopApplied is a helper method to define mock.On call
-//  - _a0 models.AlertRuleKey
-func (_e *FakeScheduleService_Expecter) stopApplied(_a0 interface{}) *FakeScheduleService_stopApplied_Call {
-	return &FakeScheduleService_stopApplied_Call{Call: _e.mock.On("stopApplied", _a0)}
-}
-
-func (_c *FakeScheduleService_stopApplied_Call) Run(run func(_a0 models.AlertRuleKey)) *FakeScheduleService_stopApplied_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		run(args[0].(models.AlertRuleKey))
-	})
-	return _c
-}
-
-func (_c *FakeScheduleService_stopApplied_Call) Return() *FakeScheduleService_stopApplied_Call {
-	_c.Call.Return()
-	return _c
-}