diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index a01ee84e333..7df5b256476 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -56,7 +56,7 @@ type AlertRuleService interface { UpdateAlertRule(ctx context.Context, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error) DeleteAlertRule(ctx context.Context, orgID int64, ruleUID string, provenance alerting_models.Provenance) error GetRuleGroup(ctx context.Context, orgID int64, folder, group string) (definitions.AlertRuleGroup, error) - UpdateRuleGroup(ctx context.Context, orgID int64, folderUID, rulegroup string, interval int64) error + ReplaceRuleGroup(ctx context.Context, orgID int64, group definitions.AlertRuleGroup, userID int64, provenance alerting_models.Provenance) error } func (srv *ProvisioningSrv) RouteGetPolicyTree(c *models.ReqContext) response.Response { @@ -312,8 +312,13 @@ func (srv *ProvisioningSrv) RouteGetAlertRuleGroup(c *models.ReqContext, folder return response.JSON(http.StatusOK, g) } -func (srv *ProvisioningSrv) RoutePutAlertRuleGroup(c *models.ReqContext, ag definitions.AlertRuleGroupMetadata, folderUID string, group string) response.Response { - err := srv.alertRules.UpdateRuleGroup(c.Req.Context(), c.OrgId, folderUID, group, ag.Interval) +func (srv *ProvisioningSrv) RoutePutAlertRuleGroup(c *models.ReqContext, ag definitions.AlertRuleGroup, folderUID string, group string) response.Response { + ag.FolderUID = folderUID + ag.Title = group + err := srv.alertRules.ReplaceRuleGroup(c.Req.Context(), c.OrgId, ag, c.UserId, alerting_models.ProvenanceAPI) + if errors.Is(err, alerting_models.ErrAlertRuleFailedValidation) { + return ErrResp(http.StatusBadRequest, err, "") + } if err != nil { if errors.Is(err, store.ErrOptimisticLock) { return ErrResp(http.StatusConflict, err, "") diff --git a/pkg/services/ngalert/api/api_provisioning_test.go b/pkg/services/ngalert/api/api_provisioning_test.go index 984885549d2..e3e132dc999 100644 --- a/pkg/services/ngalert/api/api_provisioning_test.go +++ b/pkg/services/ngalert/api/api_provisioning_test.go @@ -299,6 +299,37 @@ func TestProvisioningApi(t *testing.T) { require.Equal(t, 404, response.Status()) }) + + t.Run("are invalid at group level", func(t *testing.T) { + t.Run("PUT returns 400", func(t *testing.T) { + sut := createProvisioningSrvSut(t) + rc := createTestRequestCtx() + insertRule(t, sut, createTestAlertRule("rule", 1)) + group := createInvalidAlertRuleGroup() + group.Interval = 0 + + response := sut.RoutePutAlertRuleGroup(&rc, group, "folder-uid", group.Title) + + require.Equal(t, 400, response.Status()) + require.NotEmpty(t, response.Body()) + require.Contains(t, string(response.Body()), "invalid alert rule") + }) + }) + + t.Run("are invalid at rule level", func(t *testing.T) { + t.Run("PUT returns 400", func(t *testing.T) { + sut := createProvisioningSrvSut(t) + rc := createTestRequestCtx() + insertRule(t, sut, createTestAlertRule("rule", 1)) + group := createInvalidAlertRuleGroup() + + response := sut.RoutePutAlertRuleGroup(&rc, group, "folder-uid", group.Title) + + require.Equal(t, 400, response.Status()) + require.NotEmpty(t, response.Body()) + require.Contains(t, string(response.Body()), "invalid alert rule") + }) + }) }) } @@ -477,6 +508,14 @@ func createInvalidAlertRule() definitions.ProvisionedAlertRule { return definitions.ProvisionedAlertRule{} } +func createInvalidAlertRuleGroup() definitions.AlertRuleGroup { + return definitions.AlertRuleGroup{ + Title: "invalid", + Interval: 10, + Rules: []models.AlertRule{{}}, + } +} + func createTestAlertRule(title string, orgID int64) definitions.ProvisionedAlertRule { return definitions.ProvisionedAlertRule{ OrgID: orgID, diff --git a/pkg/services/ngalert/api/generated_base_api_provisioning.go b/pkg/services/ngalert/api/generated_base_api_provisioning.go index 484524672b1..f6399502489 100644 --- a/pkg/services/ngalert/api/generated_base_api_provisioning.go +++ b/pkg/services/ngalert/api/generated_base_api_provisioning.go @@ -135,7 +135,7 @@ func (f *ProvisioningApiHandler) RoutePutAlertRuleGroup(ctx *models.ReqContext) folderUIDParam := web.Params(ctx.Req)[":FolderUID"] groupParam := web.Params(ctx.Req)[":Group"] // Parse Request Body - conf := apimodels.AlertRuleGroupMetadata{} + conf := apimodels.AlertRuleGroup{} if err := web.Bind(ctx.Req, &conf); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } diff --git a/pkg/services/ngalert/api/provisioning.go b/pkg/services/ngalert/api/provisioning.go index 192f8c1c3f0..703a697ed31 100644 --- a/pkg/services/ngalert/api/provisioning.go +++ b/pkg/services/ngalert/api/provisioning.go @@ -100,6 +100,6 @@ func (f *ProvisioningApiHandler) handleRouteGetAlertRuleGroup(ctx *models.ReqCon return f.svc.RouteGetAlertRuleGroup(ctx, folder, group) } -func (f *ProvisioningApiHandler) handleRoutePutAlertRuleGroup(ctx *models.ReqContext, ag apimodels.AlertRuleGroupMetadata, folder, group string) response.Response { +func (f *ProvisioningApiHandler) handleRoutePutAlertRuleGroup(ctx *models.ReqContext, ag apimodels.AlertRuleGroup, folder, group string) response.Response { return f.svc.RoutePutAlertRuleGroup(ctx, ag, folder, group) } diff --git a/pkg/services/ngalert/api/tooling/api.json b/pkg/services/ngalert/api/tooling/api.json index 26bc4456210..2868195258f 100644 --- a/pkg/services/ngalert/api/tooling/api.json +++ b/pkg/services/ngalert/api/tooling/api.json @@ -3513,7 +3513,6 @@ "$ref": "#/definitions/Duration" }, "gettableAlert": { - "description": "GettableAlert gettable alert", "properties": { "annotations": { "$ref": "#/definitions/labelSet" @@ -3569,7 +3568,6 @@ "type": "object" }, "gettableAlerts": { - "description": "GettableAlerts gettable alerts", "items": { "$ref": "#/definitions/gettableAlert" }, @@ -3735,7 +3733,6 @@ "type": "array" }, "postableSilence": { - "description": "PostableSilence postable silence", "properties": { "comment": { "description": "comment", @@ -4183,15 +4180,15 @@ "in": "body", "name": "Body", "schema": { - "$ref": "#/definitions/AlertRuleGroupMetadata" + "$ref": "#/definitions/AlertRuleGroup" } } ], "responses": { "200": { - "description": "AlertRuleGroupMetadata", + "description": "AlertRuleGroup", "schema": { - "$ref": "#/definitions/AlertRuleGroupMetadata" + "$ref": "#/definitions/AlertRuleGroup" } }, "400": { diff --git a/pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go b/pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go index 65c1f650a76..2c9363e888e 100644 --- a/pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go +++ b/pkg/services/ngalert/api/tooling/definitions/provisioning_alert_rules.go @@ -151,7 +151,7 @@ func NewAlertRule(rule models.AlertRule, provenance models.Provenance) Provision // - application/json // // Responses: -// 200: AlertRuleGroupMetadata +// 200: AlertRuleGroup // 400: ValidationError // swagger:parameters RouteGetAlertRuleGroup RoutePutAlertRuleGroup @@ -169,7 +169,7 @@ type RuleGroupPathParam struct { // swagger:parameters RoutePutAlertRuleGroup type AlertRuleGroupPayload struct { // in:body - Body AlertRuleGroupMetadata + Body AlertRuleGroup } // swagger:model diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index 60a8e6c8aa5..5e7bca9f230 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -3381,6 +3381,7 @@ "type": "object" }, "alertGroup": { + "description": "AlertGroup alert group", "properties": { "alerts": { "description": "alerts", @@ -3568,7 +3569,6 @@ "type": "object" }, "gettableAlerts": { - "description": "GettableAlerts gettable alerts", "items": { "$ref": "#/definitions/gettableAlert" }, @@ -3624,7 +3624,6 @@ "type": "object" }, "gettableSilences": { - "description": "GettableSilences gettable silences", "items": { "$ref": "#/definitions/gettableSilence" }, @@ -3735,7 +3734,6 @@ "type": "array" }, "postableSilence": { - "description": "PostableSilence postable silence", "properties": { "comment": { "description": "comment", @@ -5923,15 +5921,15 @@ "in": "body", "name": "Body", "schema": { - "$ref": "#/definitions/AlertRuleGroupMetadata" + "$ref": "#/definitions/AlertRuleGroup" } } ], "responses": { "200": { - "description": "AlertRuleGroupMetadata", + "description": "AlertRuleGroup", "schema": { - "$ref": "#/definitions/AlertRuleGroupMetadata" + "$ref": "#/definitions/AlertRuleGroup" } }, "400": { diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index 78316945b67..2110d7c98d4 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -2075,15 +2075,15 @@ "name": "Body", "in": "body", "schema": { - "$ref": "#/definitions/AlertRuleGroupMetadata" + "$ref": "#/definitions/AlertRuleGroup" } } ], "responses": { "200": { - "description": "AlertRuleGroupMetadata", + "description": "AlertRuleGroup", "schema": { - "$ref": "#/definitions/AlertRuleGroupMetadata" + "$ref": "#/definitions/AlertRuleGroup" } }, "400": { @@ -5886,6 +5886,7 @@ } }, "alertGroup": { + "description": "AlertGroup alert group", "type": "object", "required": [ "alerts", @@ -6076,7 +6077,6 @@ "$ref": "#/definitions/gettableAlert" }, "gettableAlerts": { - "description": "GettableAlerts gettable alerts", "type": "array", "items": { "$ref": "#/definitions/gettableAlert" @@ -6134,7 +6134,6 @@ "$ref": "#/definitions/gettableSilence" }, "gettableSilences": { - "description": "GettableSilences gettable silences", "type": "array", "items": { "$ref": "#/definitions/gettableSilence" @@ -6246,7 +6245,6 @@ } }, "postableSilence": { - "description": "PostableSilence postable silence", "type": "object", "required": [ "comment", diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index 82e07f37617..047bbacb2a9 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -91,15 +91,8 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model return errors.New("couldn't find newly created id") } - limitReached, err := service.quotas.CheckQuotaReached(ctx, "alert_rule", "a.ScopeParameters{ - OrgID: rule.OrgID, - UserID: userID, - }) - if err != nil { - return fmt.Errorf("failed to check alert rule quota: %w", err) - } - if limitReached { - return models.ErrQuotaReached + if err = service.checkLimitsTransactionCtx(ctx, rule.OrgID, userID); err != nil { + return err } return service.provenanceStore.SetProvenance(ctx, &rule, rule.OrgID, provenance) @@ -167,6 +160,109 @@ func (service *AlertRuleService) UpdateRuleGroup(ctx context.Context, orgID int6 }) } +func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, orgID int64, group definitions.AlertRuleGroup, userID int64, provenance models.Provenance) error { + if err := models.ValidateRuleGroupInterval(group.Interval, service.baseIntervalSeconds); err != nil { + return err + } + + // If the provided request did not provide the rules list at all, treat it as though it does not wish to change rules. + // This is done for backwards compatibility. Requests which specify only the interval must update only the interval. + if group.Rules == nil { + listRulesQuery := models.ListAlertRulesQuery{ + OrgID: orgID, + NamespaceUIDs: []string{group.FolderUID}, + RuleGroup: group.Title, + } + if err := service.ruleStore.ListAlertRules(ctx, &listRulesQuery); err != nil { + return fmt.Errorf("failed to list alert rules: %w", err) + } + group.Rules = make([]models.AlertRule, 0, len(listRulesQuery.Result)) + for _, r := range listRulesQuery.Result { + if r != nil { + group.Rules = append(group.Rules, *r) + } + } + } + + key := models.AlertRuleGroupKey{ + OrgID: orgID, + NamespaceUID: group.FolderUID, + RuleGroup: group.Title, + } + rules := make([]*models.AlertRule, len(group.Rules)) + group = *syncGroupRuleFields(&group, orgID) + for i := range group.Rules { + rules = append(rules, &group.Rules[i]) + } + delta, err := store.CalculateChanges(ctx, service.ruleStore, key, rules) + if err != nil { + return fmt.Errorf("failed to calculate diff for alert rules: %w", err) + } + + // Refresh all calculated fields across all rules. + delta = store.UpdateCalculatedRuleFields(delta) + + if len(delta.New) == 0 && len(delta.Update) == 0 && len(delta.Delete) == 0 { + return nil + } + + return service.xact.InTransaction(ctx, func(ctx context.Context) error { + uids, err := service.ruleStore.InsertAlertRules(ctx, withoutNilAlertRules(delta.New)) + if err != nil { + return fmt.Errorf("failed to insert alert rules: %w", err) + } + for uid := range uids { + if err := service.provenanceStore.SetProvenance(ctx, &models.AlertRule{UID: uid}, orgID, provenance); err != nil { + return err + } + } + + updates := make([]store.UpdateRule, 0, len(delta.Update)) + for _, update := range delta.Update { + // check that provenance is not changed in a invalid way + storedProvenance, err := service.provenanceStore.GetProvenance(ctx, update.New, orgID) + if err != nil { + return err + } + if storedProvenance != provenance && storedProvenance != models.ProvenanceNone { + return fmt.Errorf("cannot update with provided provenance '%s', needs '%s'", provenance, storedProvenance) + } + updates = append(updates, store.UpdateRule{ + Existing: update.Existing, + New: *update.New, + }) + } + if err = service.ruleStore.UpdateAlertRules(ctx, updates); err != nil { + return fmt.Errorf("failed to update alert rules: %w", err) + } + for _, update := range delta.Update { + if err := service.provenanceStore.SetProvenance(ctx, update.New, orgID, provenance); err != nil { + return err + } + } + + for _, delete := range delta.Delete { + // check that provenance is not changed in a invalid way + storedProvenance, err := service.provenanceStore.GetProvenance(ctx, delete, orgID) + if err != nil { + return err + } + if storedProvenance != provenance && storedProvenance != models.ProvenanceNone { + return fmt.Errorf("cannot update with provided provenance '%s', needs '%s'", provenance, storedProvenance) + } + } + if err := service.deleteRules(ctx, orgID, delta.Delete...); err != nil { + return err + } + + if err = service.checkLimitsTransactionCtx(ctx, orgID, userID); err != nil { + return err + } + + return nil + }) +} + // CreateAlertRule creates a new alert rule. This function will ignore any // interval that is set in the rule struct and fetch the current group interval // from database. @@ -220,10 +316,62 @@ func (service *AlertRuleService) DeleteAlertRule(ctx context.Context, orgID int6 return fmt.Errorf("cannot delete with provided provenance '%s', needs '%s'", provenance, storedProvenance) } return service.xact.InTransaction(ctx, func(ctx context.Context) error { - err := service.ruleStore.DeleteAlertRulesByUID(ctx, orgID, ruleUID) - if err != nil { - return err - } - return service.provenanceStore.DeleteProvenance(ctx, rule, rule.OrgID) + return service.deleteRules(ctx, orgID, rule) }) } + +// checkLimitsTransactionCtx checks whether the current transaction (as identified by the ctx) breaches configured alert rule limits. +func (service *AlertRuleService) checkLimitsTransactionCtx(ctx context.Context, orgID, userID int64) error { + limitReached, err := service.quotas.CheckQuotaReached(ctx, "alert_rule", "a.ScopeParameters{ + OrgID: orgID, + UserID: userID, + }) + if err != nil { + return fmt.Errorf("failed to check alert rule quota: %w", err) + } + if limitReached { + return models.ErrQuotaReached + } + return nil +} + +// deleteRules deletes a set of target rules and associated data, while checking for database consistency. +func (service *AlertRuleService) deleteRules(ctx context.Context, orgID int64, targets ...*models.AlertRule) error { + uids := make([]string, 0, len(targets)) + for _, tgt := range targets { + if tgt != nil { + uids = append(uids, tgt.UID) + } + } + if err := service.ruleStore.DeleteAlertRulesByUID(ctx, orgID, uids...); err != nil { + return err + } + for _, uid := range uids { + if err := service.provenanceStore.DeleteProvenance(ctx, &models.AlertRule{UID: uid}, orgID); err != nil { + // We failed to clean up the record, but this doesn't break things. Log it and move on. + service.log.Warn("failed to delete provenance record for rule: %w", err) + } + } + return nil +} + +// syncRuleGroupFields synchronizes calculated fields across multiple rules in a group. +func syncGroupRuleFields(group *definitions.AlertRuleGroup, orgID int64) *definitions.AlertRuleGroup { + for i := range group.Rules { + group.Rules[i].IntervalSeconds = group.Interval + group.Rules[i].RuleGroup = group.Title + group.Rules[i].NamespaceUID = group.FolderUID + group.Rules[i].OrgID = orgID + } + return group +} + +func withoutNilAlertRules(ptrs []*models.AlertRule) []models.AlertRule { + result := make([]models.AlertRule, 0, len(ptrs)) + for _, ptr := range ptrs { + if ptr != nil { + result = append(result, *ptr) + } + } + return result +} diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index 87728a9db9b..190ac769762 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -35,6 +36,22 @@ func TestAlertRuleService(t *testing.T) { require.Equal(t, models.ProvenanceAPI, provenance) }) + t.Run("group creation should set the right provenance", func(t *testing.T) { + var orgID int64 = 1 + group := createDummyGroup("group-test-1", orgID) + err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI) + require.NoError(t, err) + + readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "group-test-1") + require.NoError(t, err) + require.NotEmpty(t, readGroup.Rules) + for _, rule := range readGroup.Rules { + _, provenance, err := ruleService.GetAlertRule(context.Background(), orgID, rule.UID) + require.NoError(t, err) + require.Equal(t, models.ProvenanceAPI, provenance) + } + }) + t.Run("alert rule group should be updated correctly", func(t *testing.T) { var orgID int64 = 1 rule := dummyRule("test#3", orgID) @@ -52,6 +69,22 @@ func TestAlertRuleService(t *testing.T) { require.Equal(t, interval, rule.IntervalSeconds) }) + t.Run("group creation should propagate group title correctly", func(t *testing.T) { + var orgID int64 = 1 + group := createDummyGroup("group-test-3", orgID) + group.Rules[0].RuleGroup = "something different" + + err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI) + require.NoError(t, err) + + readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "group-test-3") + require.NoError(t, err) + require.NotEmpty(t, readGroup.Rules) + for _, rule := range readGroup.Rules { + require.Equal(t, "group-test-3", rule.RuleGroup) + } + }) + t.Run("alert rule should get interval from existing rule group", func(t *testing.T) { var orgID int64 = 1 rule := dummyRule("test#4", orgID) @@ -70,7 +103,7 @@ func TestAlertRuleService(t *testing.T) { require.Equal(t, interval, rule.IntervalSeconds) }) - t.Run("updating a rule group should bump the version number", func(t *testing.T) { + t.Run("updating a rule group's top level fields should bump the version number", func(t *testing.T) { const ( orgID = 123 namespaceUID = "abc" @@ -99,6 +132,26 @@ func TestAlertRuleService(t *testing.T) { require.Equal(t, newInterval, rule.IntervalSeconds) }) + t.Run("updating a group by updating a rule should bump that rule's data and version number", func(t *testing.T) { + var orgID int64 = 1 + group := createDummyGroup("group-test-5", orgID) + err := ruleService.ReplaceRuleGroup(context.Background(), orgID, group, 0, models.ProvenanceAPI) + require.NoError(t, err) + updatedGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "group-test-5") + require.NoError(t, err) + + updatedGroup.Rules[0].Title = "some-other-title-asdf" + err = ruleService.ReplaceRuleGroup(context.Background(), orgID, updatedGroup, 0, models.ProvenanceAPI) + require.NoError(t, err) + + readGroup, err := ruleService.GetRuleGroup(context.Background(), orgID, "my-namespace", "group-test-5") + require.NoError(t, err) + require.NotEmpty(t, readGroup.Rules) + require.Len(t, readGroup.Rules, 1) + require.Equal(t, "some-other-title-asdf", readGroup.Rules[0].Title) + require.Equal(t, int64(2), readGroup.Rules[0].Version) + }) + t.Run("alert rule provenace should be correctly checked", func(t *testing.T) { tests := []struct { name string @@ -160,6 +213,68 @@ func TestAlertRuleService(t *testing.T) { } }) + t.Run("alert rule provenace should be correctly checked when writing groups", func(t *testing.T) { + tests := []struct { + name string + from models.Provenance + to models.Provenance + errNil bool + }{ + { + name: "should be able to update from provenance none to api", + from: models.ProvenanceNone, + to: models.ProvenanceAPI, + errNil: true, + }, + { + name: "should be able to update from provenance none to file", + from: models.ProvenanceNone, + to: models.ProvenanceFile, + errNil: true, + }, + { + name: "should not be able to update from provenance api to file", + from: models.ProvenanceAPI, + to: models.ProvenanceFile, + errNil: false, + }, + { + name: "should not be able to update from provenance api to none", + from: models.ProvenanceAPI, + to: models.ProvenanceNone, + errNil: false, + }, + { + name: "should not be able to update from provenance file to api", + from: models.ProvenanceFile, + to: models.ProvenanceAPI, + errNil: false, + }, + { + name: "should not be able to update from provenance file to none", + from: models.ProvenanceFile, + to: models.ProvenanceNone, + errNil: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var orgID int64 = 1 + group := createDummyGroup(t.Name(), orgID) + err := ruleService.ReplaceRuleGroup(context.Background(), 1, group, 0, test.from) + require.NoError(t, err) + + group.Rules[0].Title = t.Name() + err = ruleService.ReplaceRuleGroup(context.Background(), 1, group, 0, test.to) + if test.errNil { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } + }) + t.Run("quota met causes create to be rejected", func(t *testing.T) { ruleService := createAlertRuleService(t) checker := &MockQuotaChecker{} @@ -170,6 +285,18 @@ func TestAlertRuleService(t *testing.T) { require.ErrorIs(t, err, models.ErrQuotaReached) }) + + t.Run("quota met causes group write to be rejected", func(t *testing.T) { + ruleService := createAlertRuleService(t) + checker := &MockQuotaChecker{} + checker.EXPECT().LimitExceeded() + ruleService.quotas = checker + + group := createDummyGroup("quota-reached", 1) + err := ruleService.ReplaceRuleGroup(context.Background(), 1, group, 0, models.ProvenanceAPI) + + require.ErrorIs(t, err, models.ErrQuotaReached) + }) } func createAlertRuleService(t *testing.T) AlertRuleService { @@ -180,6 +307,7 @@ func createAlertRuleService(t *testing.T) AlertRuleService { Cfg: setting.UnifiedAlertingSettings{ BaseInterval: time.Second * 10, }, + Logger: log.NewNopLogger(), } quotas := MockQuotaChecker{} quotas.EXPECT().LimitOK() @@ -195,6 +323,10 @@ func createAlertRuleService(t *testing.T) AlertRuleService { } func dummyRule(title string, orgID int64) models.AlertRule { + return createTestRule(title, "my-cool-group", orgID) +} + +func createTestRule(title string, groupTitle string, orgID int64) models.AlertRule { return models.AlertRule{ OrgID: orgID, Title: title, @@ -212,9 +344,21 @@ func dummyRule(title string, orgID int64) models.AlertRule { }, }, }, - RuleGroup: "my-cool-group", + NamespaceUID: "my-namespace", + RuleGroup: groupTitle, For: time.Second * 60, NoDataState: models.OK, ExecErrState: models.OkErrState, } } + +func createDummyGroup(title string, orgID int64) definitions.AlertRuleGroup { + return definitions.AlertRuleGroup{ + Title: title, + Interval: 60, + FolderUID: "my-namespace", + Rules: []models.AlertRule{ + dummyRule(title+"-"+"rule-1", orgID), + }, + } +} diff --git a/pkg/services/ngalert/provisioning/persist.go b/pkg/services/ngalert/provisioning/persist.go index ed99265b7de..37cafa6815a 100644 --- a/pkg/services/ngalert/provisioning/persist.go +++ b/pkg/services/ngalert/provisioning/persist.go @@ -37,6 +37,7 @@ type RuleStore interface { InsertAlertRules(ctx context.Context, rule []models.AlertRule) (map[string]int64, error) UpdateAlertRules(ctx context.Context, rule []store.UpdateRule) error DeleteAlertRulesByUID(ctx context.Context, orgID int64, ruleUID ...string) error + GetAlertRulesGroupByRuleUID(ctx context.Context, query *models.GetAlertRulesGroupByRuleUIDQuery) error } // QuotaChecker represents the ability to evaluate whether quotas are met. diff --git a/pkg/services/ngalert/store/deltas.go b/pkg/services/ngalert/store/deltas.go index 12814c8ee1a..a3f589a8cc7 100644 --- a/pkg/services/ngalert/store/deltas.go +++ b/pkg/services/ngalert/store/deltas.go @@ -62,6 +62,9 @@ func CalculateChanges(ctx context.Context, ruleReader RuleReader, groupKey model var toUpdate []RuleDelta loadedRulesByUID := map[string]*models.AlertRule{} // auxiliary cache to avoid unnecessary queries if there are multiple moves from the same group for _, r := range submittedRules { + if r == nil { + continue + } var existing *models.AlertRule = nil if r.UID != "" { if existingGroupRule, ok := existingGroupRulesUIDs[r.UID]; ok {