From d50e2a94a354e73d7c327ed89222676b24f6875d Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Tue, 25 Nov 2025 11:20:28 +0100 Subject: [PATCH] Alerting: Implement setting of version message in convert API. (#114331) Part 2 of adding version messages to the `alert_rule_version` able. This allows setting the message via a header when using the Prometheus conversion API, which can be useful for e.g. linking changes back to source control. --- .../loki/historian_store_test.go | 4 +- .../snapshot_mgmt_alerts_test.go | 2 +- .../ngalert/api/api_convert_prometheus.go | 21 +++- .../api/api_convert_prometheus_test.go | 62 ++++++++++++ pkg/services/ngalert/api/api_provisioning.go | 7 +- pkg/services/ngalert/api/api_ruler.go | 4 +- pkg/services/ngalert/api/persist.go | 2 +- pkg/services/ngalert/models/alert_rule.go | 6 ++ .../ngalert/provisioning/alert_rules.go | 31 +++--- .../ngalert/provisioning/alert_rules_test.go | 96 +++++++++---------- pkg/services/ngalert/provisioning/persist.go | 2 +- pkg/services/ngalert/store/alert_rule.go | 11 ++- pkg/services/ngalert/store/alert_rule_test.go | 49 ++++++---- pkg/services/ngalert/tests/fakes/rules.go | 4 +- pkg/services/ngalert/tests/util.go | 6 +- pkg/storage/unified/federated/stats_test.go | 6 +- 16 files changed, 212 insertions(+), 101 deletions(-) diff --git a/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go b/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go index 348aa38d477..66d447e8bf9 100644 --- a/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go +++ b/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go @@ -687,7 +687,7 @@ func createAlertRule(t *testing.T, sql *sqlstore.SQLStore, title string, generat rule.PanelID = nil ruleStore := store.SetupStoreForTesting(t, sql) - ids, err := ruleStore.InsertAlertRules(context.Background(), nil, []ngmodels.AlertRule{rule}) + ids, err := ruleStore.InsertAlertRules(context.Background(), nil, []ngmodels.InsertRule{{AlertRule: rule}}) require.NoError(t, err) result, err := ruleStore.GetAlertRuleByUID(context.Background(), &ngmodels.GetAlertRuleByUIDQuery{OrgID: rule.OrgID, UID: ids[0].UID}) require.NoError(t, err) @@ -719,7 +719,7 @@ func createAlertRuleFromDashboard(t *testing.T, sql *sqlstore.SQLStore, title st rule.PanelID = panelID } ruleStore := store.SetupStoreForTesting(t, sql) - ids, err := ruleStore.InsertAlertRules(context.Background(), nil, []ngmodels.AlertRule{rule}) + ids, err := ruleStore.InsertAlertRules(context.Background(), nil, []ngmodels.InsertRule{{AlertRule: rule}}) require.NoError(t, err) result, err := ruleStore.GetAlertRuleByUID(context.Background(), &ngmodels.GetAlertRuleByUIDQuery{OrgID: rule.OrgID, UID: ids[0].UID}) require.NoError(t, err) diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go index ee2c9889c20..eaa95962a23 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go @@ -457,7 +457,7 @@ func createAlertRuleGroup(t *testing.T, ctx context.Context, service *Service, u Rules: rules, } - err := service.ngAlert.Api.AlertRules.ReplaceRuleGroup(ctx, user, group, "") + err := service.ngAlert.Api.AlertRules.ReplaceRuleGroup(ctx, user, group, "", "") require.NoError(t, err) return group diff --git a/pkg/services/ngalert/api/api_convert_prometheus.go b/pkg/services/ngalert/api/api_convert_prometheus.go index 7e999010795..588f6dee213 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus.go +++ b/pkg/services/ngalert/api/api_convert_prometheus.go @@ -63,6 +63,9 @@ const ( // configIdentifierHeader is the header that specifies the identifier for imported Alertmanager config. configIdentifierHeader = "X-Grafana-Alerting-Config-Identifier" defaultConfigIdentifier = "default" + + // versionMessageHeader is the header that specifies an optional message for rule versions. + versionMessageHeader = "X-Grafana-Alerting-Version-Message" ) var ( @@ -407,6 +410,11 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostRuleGroups(c *context return errorToResponse(err) } + versionMessage, err := parseVersionMessageHeader(c) + if err != nil { + logger.Error("Failed to parse version message header", "error", err) + return errorToResponse(err) + } // 2. Convert Prometheus Rules to GMA grafanaGroups := make([]*models.AlertRuleGroup, 0, len(promNamespaces)) for ns, rgs := range promNamespaces { @@ -444,12 +452,13 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostRuleGroups(c *context logger.Error("Failed to convert Prometheus rules to Grafana rules", "error", err) return errorToResponse(err) } + grafanaGroups = append(grafanaGroups, grafanaGroup) } } // 3. Update the GMA Rules in the DB - err = srv.alertRuleService.ReplaceRuleGroups(c.Req.Context(), c.SignedInUser, grafanaGroups, provenance) + err = srv.alertRuleService.ReplaceRuleGroups(c.Req.Context(), c.SignedInUser, grafanaGroups, provenance, versionMessage) if err != nil { logger.Error("Failed to replace rule groups", "error", err) return errorToResponse(err) @@ -856,6 +865,16 @@ func parseExtraLabelsHeader(c *contextmodel.ReqContext) (map[string]string, erro return parseKeyValuePairs(labelsStr, extraLabelsHeader) } +// parseVersionMessageHeader obtains and validates the message header value. +func parseVersionMessageHeader(c *contextmodel.ReqContext) (string, error) { + str := strings.TrimSpace(c.Req.Header.Get(versionMessageHeader)) + // Limit message to the same as the dashboards message. + if len(str) > 500 { + return "", errInvalidHeaderValue(versionMessageHeader, errors.New("must be less than 500 characters")) + } + return str, nil +} + func formatMergeMatchers(matchers amconfig.Matchers) string { var pairs []string for _, matcher := range matchers { diff --git a/pkg/services/ngalert/api/api_convert_prometheus_test.go b/pkg/services/ngalert/api/api_convert_prometheus_test.go index 9371ca6a230..08ce147a5c3 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus_test.go +++ b/pkg/services/ngalert/api/api_convert_prometheus_test.go @@ -506,6 +506,68 @@ func TestRouteConvertPrometheusPostRuleGroup(t *testing.T) { } }) + t.Run("with version message header should pass message to rule store", func(t *testing.T) { + provenanceStore := fakes.NewFakeProvisioningStore() + folderService := foldertest.NewFakeService() + srv, _, ruleStore := createConvertPrometheusSrv(t, withProvenanceStore(provenanceStore), withFolderService(folderService)) + + // Create a folder in the root + fldr := randFolder() + fldr.ParentUID = "" + folderService.ExpectedFolder = fldr + folderService.ExpectedFolders = []*folder.Folder{fldr} + ruleStore.Folders[1] = append(ruleStore.Folders[1], fldr) + + makeGroup := func(alertname string) apimodels.PrometheusRuleGroup { + return apimodels.PrometheusRuleGroup{ + Name: "Test Group", + Interval: prommodel.Duration(1 * time.Minute), + Rules: []apimodels.PrometheusRule{ + { + Alert: alertname, + Expr: "up == 0", + For: util.Pointer(prommodel.Duration(5 * time.Minute)), + Labels: map[string]string{ + "severity": "critical", + }, + }, + }, + } + } + + // Create a rule with the X-Grafana-Alerting-Version-Message to check it's passed to InsertRule. + + rc1 := createRequestCtx() + rc1.Req.Header.Set("X-Grafana-Alerting-Version-Message", "version #1") + response1 := srv.RouteConvertPrometheusPostRuleGroup(rc1, fldr.Title, makeGroup("AlertV1")) + require.Equal(t, http.StatusAccepted, response1.Status()) + + inserts := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { + a, ok := cmd.([]models.InsertRule) + return a, ok + }) + require.Len(t, inserts, 1) + cmd1 := inserts[0].([]models.InsertRule) + require.Len(t, cmd1, 1) + require.Equal(t, "version #1", cmd1[0].Message) + + // Now update the rule to check it gets passed to UpdateRule. + + rc2 := createRequestCtx() + rc2.Req.Header.Set("X-Grafana-Alerting-Version-Message", "version #2") + response2 := srv.RouteConvertPrometheusPostRuleGroup(rc2, fldr.Title, makeGroup("AlertV2")) + require.Equal(t, http.StatusAccepted, response2.Status()) + + updates := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { + a, ok := cmd.([]models.UpdateRule) + return a, ok + }) + require.Len(t, updates, 1) + cmd2 := updates[0].([]models.UpdateRule) + require.Len(t, cmd2, 1) + require.Equal(t, "version #2", cmd2[0].Message) + }) + t.Run("returns error when target datasource does not exist", func(t *testing.T) { srv, _, _ := createConvertPrometheusSrv(t) rc := createRequestCtx() diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index dc8863e1fb6..8d24fafa351 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -75,7 +75,7 @@ type AlertRuleService interface { UpdateAlertRule(ctx context.Context, user identity.Requester, rule alerting_models.AlertRule, provenance alerting_models.Provenance) (alerting_models.AlertRule, error) DeleteAlertRule(ctx context.Context, user identity.Requester, ruleUID string, provenance alerting_models.Provenance) error GetRuleGroup(ctx context.Context, user identity.Requester, folder, group string) (alerting_models.AlertRuleGroup, error) - ReplaceRuleGroup(ctx context.Context, user identity.Requester, group alerting_models.AlertRuleGroup, provenance alerting_models.Provenance) error + ReplaceRuleGroup(ctx context.Context, user identity.Requester, group alerting_models.AlertRuleGroup, provenance alerting_models.Provenance, message string) error DeleteRuleGroup(ctx context.Context, user identity.Requester, folder, group string, provenance alerting_models.Provenance) error DeleteRuleGroups(ctx context.Context, user identity.Requester, provenance alerting_models.Provenance, opts *provisioning.FilterOptions) error GetAlertRuleWithFolderFullpath(ctx context.Context, u identity.Requester, ruleUID string) (provisioning.AlertRuleWithFolderFullpath, error) @@ -492,7 +492,10 @@ func (srv *ProvisioningSrv) RoutePutAlertRuleGroup(c *contextmodel.ReqContext, a ErrResp(http.StatusBadRequest, err, "") } provenance := determineProvenance(c) - err = srv.alertRules.ReplaceRuleGroup(c.Req.Context(), c.SignedInUser, groupModel, alerting_models.Provenance(provenance)) + // TODO: https://github.com/grafana/grafana/issues/114197 + // Support passing change messages. + changeMessage := "" + err = srv.alertRules.ReplaceRuleGroup(c.Req.Context(), c.SignedInUser, groupModel, alerting_models.Provenance(provenance), changeMessage) if errors.Is(err, alerting_models.ErrAlertRuleFailedValidation) { return ErrResp(http.StatusBadRequest, err, "") } diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 268ed7b8c5d..d81f448f600 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -576,9 +576,9 @@ func (srv RulerSrv) performUpdateAlertRules(ctx context.Context, c *contextmodel } if len(finalChanges.New) > 0 { - inserts := make([]ngmodels.AlertRule, 0, len(finalChanges.New)) + inserts := make([]ngmodels.InsertRule, 0, len(finalChanges.New)) for _, rule := range finalChanges.New { - inserts = append(inserts, *rule) + inserts = append(inserts, ngmodels.InsertRule{AlertRule: *rule}) } added, err := srv.store.InsertAlertRules(tranCtx, ngmodels.NewUserUID(c.SignedInUser), inserts) if err != nil { diff --git a/pkg/services/ngalert/api/persist.go b/pkg/services/ngalert/api/persist.go index 9e7df81f48e..0123af54d52 100644 --- a/pkg/services/ngalert/api/persist.go +++ b/pkg/services/ngalert/api/persist.go @@ -28,7 +28,7 @@ type RuleStore interface { // InsertAlertRules will insert all alert rules passed into the function // and return the map of uuid to id. - InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.AlertRule) ([]ngmodels.AlertRuleKeyWithId, error) + InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.InsertRule) ([]ngmodels.AlertRuleKeyWithId, error) UpdateAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.UpdateRule) error DeleteAlertRulesByUID(ctx context.Context, orgID int64, user *ngmodels.UserUID, permanently bool, ruleUID ...string) error DeleteRuleFromTrashByGUID(ctx context.Context, orgID int64, ruleGUID string) (int64, error) diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index a4e1e8fcc04..e6177843e8a 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -1036,9 +1036,15 @@ type ListNamespaceAlertRulesQuery struct { NamespaceUID string } +type InsertRule struct { + AlertRule + Message string +} + type UpdateRule struct { Existing *AlertRule New AlertRule + Message string } // Condition contains backend expressions and queries and the RefID diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index d8cb5c9783d..aa6c4c55b34 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -304,8 +304,10 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, user ident } } err = service.xact.InTransaction(ctx, func(ctx context.Context) error { - ids, err := service.ruleStore.InsertAlertRules(ctx, userUidOrFallback(user), []models.AlertRule{ - rule, + ids, err := service.ruleStore.InsertAlertRules(ctx, userUidOrFallback(user), []models.InsertRule{ + { + AlertRule: rule, + }, }) if err != nil { return err @@ -464,7 +466,7 @@ func (service *AlertRuleService) UpdateRuleGroup(ctx context.Context, user ident }) } -func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, user identity.Requester, group models.AlertRuleGroup, provenance models.Provenance) error { +func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, user identity.Requester, group models.AlertRuleGroup, provenance models.Provenance, versionMessage string) error { if err := models.ValidateRuleGroupInterval(group.Interval, service.baseIntervalSeconds); err != nil { return err } @@ -518,13 +520,13 @@ func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, user iden } } - return service.persistDelta(ctx, user, delta, provenance) + return service.persistDelta(ctx, user, delta, provenance, versionMessage) } -func (service *AlertRuleService) ReplaceRuleGroups(ctx context.Context, user identity.Requester, groups []*models.AlertRuleGroup, provenance models.Provenance) error { +func (service *AlertRuleService) ReplaceRuleGroups(ctx context.Context, user identity.Requester, groups []*models.AlertRuleGroup, provenance models.Provenance, versionMessage string) error { err := service.xact.InTransaction(ctx, func(ctx context.Context) error { for _, group := range groups { - err := service.ReplaceRuleGroup(ctx, user, *group, provenance) + err := service.ReplaceRuleGroup(ctx, user, *group, provenance, versionMessage) if err != nil { return err } @@ -565,7 +567,7 @@ func (service *AlertRuleService) DeleteRuleGroups(ctx context.Context, user iden return err } } - err = service.persistDelta(ctx, user, delta, provenance) + err = service.persistDelta(ctx, user, delta, provenance, "") // Message not used for deletes. if err != nil { return err } @@ -621,7 +623,7 @@ func (service *AlertRuleService) calcDelta(ctx context.Context, user identity.Re return store.UpdateCalculatedRuleFields(delta), nil } -func (service *AlertRuleService) persistDelta(ctx context.Context, user identity.Requester, delta *store.GroupDelta, provenance models.Provenance) error { +func (service *AlertRuleService) persistDelta(ctx context.Context, user identity.Requester, delta *store.GroupDelta, provenance models.Provenance, versionMessage string) error { return service.xact.InTransaction(ctx, func(ctx context.Context) error { // Delete first as this could prevent future unique constraint violations. if len(delta.Delete) > 0 { @@ -669,6 +671,7 @@ func (service *AlertRuleService) persistDelta(ctx context.Context, user identity updates = append(updates, models.UpdateRule{ Existing: update.Existing, New: *update.New, + Message: versionMessage, }) } if err := service.ruleStore.UpdateAlertRules(ctx, userUidOrFallback(user), updates); err != nil { @@ -682,7 +685,7 @@ func (service *AlertRuleService) persistDelta(ctx context.Context, user identity } if len(delta.New) > 0 { - uids, err := service.ruleStore.InsertAlertRules(ctx, userUidOrFallback(user), withoutNilAlertRules(delta.New)) + uids, err := service.ruleStore.InsertAlertRules(ctx, userUidOrFallback(user), makeInsertRules(delta.New, versionMessage)) if err != nil { return fmt.Errorf("failed to insert alert rules: %w", err) } @@ -994,11 +997,15 @@ func syncGroupRuleFields(group *models.AlertRuleGroup, orgID int64) *models.Aler return group } -func withoutNilAlertRules(ptrs []*models.AlertRule) []models.AlertRule { - result := make([]models.AlertRule, 0, len(ptrs)) +// makeInsertRules converts a slice of AlertRule pointers to InsertRule, removing nils and adding version message. +func makeInsertRules(ptrs []*models.AlertRule, versionMessage string) []models.InsertRule { + result := make([]models.InsertRule, 0, len(ptrs)) for _, ptr := range ptrs { if ptr != nil { - result = append(result, *ptr) + result = append(result, models.InsertRule{ + AlertRule: *ptr, + Message: versionMessage, + }) } } return result diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index 145f1df2bb8..c552f797429 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -49,7 +49,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { t.Run("group creation should set the right provenance", func(t *testing.T) { group := createDummyGroup("group-test-1", orgID) - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "group-test-1") @@ -96,7 +96,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { group := createDummyGroup("namespace-test", orgID) group.Rules[0].NamespaceUID = "" - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "namespace-test") @@ -109,7 +109,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { group := createDummyGroup("group-test-3", orgID) group.Rules[0].RuleGroup = "something different" - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "group-test-3") @@ -169,13 +169,13 @@ func TestIntegrationAlertRuleService(t *testing.T) { t.Run("updating a group by updating a rule should bump that rule's data and version number", func(t *testing.T) { group := createDummyGroup("group-test-5", orgID) - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "group-test-5") require.NoError(t, err) updatedGroup.Rules[0].Title = "some-other-title-asdf" - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "group-test-5") @@ -198,7 +198,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { }, } rule.Metadata = ruleMetadata - r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule}) + r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.Len(t, r, 1) @@ -215,7 +215,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { Rules: []models.AlertRule{rule}, } - err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, namespaceUID, groupTitle) @@ -242,7 +242,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { }, } rule.Metadata = ruleMetadata - r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule}) + r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.Len(t, r, 1) @@ -263,7 +263,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { Rules: []models.AlertRule{rule}, } - err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, namespaceUID, groupTitle) @@ -289,7 +289,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { }, } rule.Metadata = ruleMetadata - r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule}) + r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.Len(t, r, 1) @@ -306,7 +306,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { Rules: []models.AlertRule{rule}, } - err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, namespaceUID, groupTitle) @@ -326,7 +326,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { }, } rule.Metadata = ruleMetadata - r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule}) + r, err := ruleService.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.Len(t, r, 1) @@ -356,14 +356,14 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("overlap-test-rule-2", orgID), }, } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "overlap-test") require.NoError(t, err) updatedGroup.Rules[0].Title = "overlap-test-rule-2" updatedGroup.Rules[1].Title = "overlap-test-rule-3" - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "overlap-test") @@ -387,14 +387,14 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("swap-test-rule-2", orgID), }, } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "swap-test") require.NoError(t, err) updatedGroup.Rules[0].Title = "swap-test-rule-2" updatedGroup.Rules[1].Title = "swap-test-rule-1" - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "swap-test") @@ -419,7 +419,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("cycle-test-rule-3", orgID), }, } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "cycle-test") require.NoError(t, err) @@ -427,7 +427,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { updatedGroup.Rules[0].Title = "cycle-test-rule-2" updatedGroup.Rules[1].Title = "cycle-test-rule-3" updatedGroup.Rules[2].Title = "cycle-test-rule-1" - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "cycle-test") @@ -457,7 +457,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("multi-cycle-test-rule-5", orgID), }, } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "multi-cycle-test") require.NoError(t, err) @@ -469,7 +469,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { updatedGroup.Rules[3].Title = "multi-cycle-test-rule-5" updatedGroup.Rules[4].Title = "multi-cycle-test-rule-3" - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "multi-cycle-test") @@ -498,7 +498,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("recreate-test-rule-1", orgID), }, } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup := models.AlertRuleGroup{ Title: "recreate-test", @@ -508,7 +508,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("recreate-test-rule-1", orgID), }, } - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "recreate-test") @@ -529,14 +529,14 @@ func TestIntegrationAlertRuleService(t *testing.T) { dummyRule("create-overlap-test-rule-1", orgID), }, } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "create-overlap-test") require.NoError(t, err) updatedGroup.Rules[0].Title = "create-overlap-test-rule-2" updatedGroup.Rules = append(updatedGroup.Rules, dummyRule("create-overlap-test-rule-1", orgID)) - err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI) + err = ruleService.ReplaceRuleGroup(context.Background(), u, updatedGroup, models.ProvenanceAPI, "") require.NoError(t, err) readGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "create-overlap-test") @@ -558,7 +558,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { models.PanelIDAnnotation: strconv.FormatInt(panelId, 10), } - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) updatedGroup, err := ruleService.GetRuleGroup(context.Background(), u, "my-namespace", "group-test-5") require.NoError(t, err) @@ -713,11 +713,11 @@ func TestIntegrationAlertRuleService(t *testing.T) { t.Run(test.name, func(t *testing.T) { var orgID int64 = 1 group := createDummyGroup(t.Name(), orgID) - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, test.from) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, test.from, "") require.NoError(t, err) group.Rules[0].Title = t.Name() - err = ruleService.ReplaceRuleGroup(context.Background(), u, group, test.to) + err = ruleService.ReplaceRuleGroup(context.Background(), u, group, test.to, "") if test.errNil { require.NoError(t, err) } else { @@ -745,7 +745,7 @@ func TestIntegrationAlertRuleService(t *testing.T) { ruleService.quotas = checker group := createDummyGroup("quota-reached", orgID) - err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := ruleService.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.ErrorIs(t, err, models.ErrQuotaReached) }) @@ -912,11 +912,11 @@ func TestIntegrationCreateAlertRule(t *testing.T) { t.Run("inserts to database", func(t *testing.T) { inserts := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { - a, ok := cmd.([]models.AlertRule) + a, ok := cmd.([]models.InsertRule) return a, ok }) require.Len(t, inserts, 1) - cmd := inserts[0].([]models.AlertRule) + cmd := inserts[0].([]models.InsertRule) require.Len(t, cmd, 1) }) @@ -946,11 +946,11 @@ func TestIntegrationCreateAlertRule(t *testing.T) { t.Run("inserts to database", func(t *testing.T) { inserts := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { - a, ok := cmd.([]models.AlertRule) + a, ok := cmd.([]models.InsertRule) return a, ok }) require.Len(t, inserts, 1) - cmd := inserts[0].([]models.AlertRule) + cmd := inserts[0].([]models.InsertRule) require.Len(t, cmd, 1) }) @@ -993,11 +993,11 @@ func TestIntegrationCreateAlertRule(t *testing.T) { t.Run("inserts to database", func(t *testing.T) { inserts := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { - a, ok := cmd.([]models.AlertRule) + a, ok := cmd.([]models.InsertRule) return a, ok }) require.Len(t, inserts, 1) - cmd := inserts[0].([]models.AlertRule) + cmd := inserts[0].([]models.InsertRule) require.Len(t, cmd, 1) }) @@ -1040,11 +1040,11 @@ func TestIntegrationCreateAlertRule(t *testing.T) { t.Run("inserts to database", func(t *testing.T) { inserts := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { - a, ok := cmd.([]models.AlertRule) + a, ok := cmd.([]models.InsertRule) return a, ok }) require.Len(t, inserts, 1) - cmd := inserts[0].([]models.AlertRule) + cmd := inserts[0].([]models.InsertRule) require.Len(t, cmd, 1) }) @@ -1075,7 +1075,7 @@ func TestIntegrationCreateAlertRule(t *testing.T) { assert.Equal(t, "AuthorizeRuleGroupWrite", ac.Calls[1].Method) inserts := ruleStore.GetRecordedCommands(func(cmd any) (any, bool) { - a, ok := cmd.([]models.AlertRule) + a, ok := cmd.([]models.InsertRule) return a, ok }) require.Empty(t, inserts) @@ -1276,7 +1276,7 @@ func TestUpdateAlertRule(t *testing.T) { rule := models.CopyRule(rules[0]) - _, err := service.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{*rule}) + _, err := service.ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: *rule}}) require.NoError(t, err) ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { @@ -1300,7 +1300,7 @@ func TestUpdateAlertRule(t *testing.T) { ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { return true, nil } rule := createNoGroupRule("nogroup-update", orgID, "my-namespace") - _, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{rule}) + _, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.NoError(t, provenanceStore.SetProvenance(context.Background(), &rule, orgID, models.ProvenanceNone)) @@ -1418,7 +1418,7 @@ func TestDeleteAlertRule(t *testing.T) { // create two NoGroup rules in the same namespace (distinct sentinel groups) r1 := createNoGroupRule("nogroup-del-1", orgID, "my-namespace") r2 := createNoGroupRule("nogroup-del-2", orgID, "my-namespace") - _, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{r1, r2}) + _, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: r1}, {AlertRule: r2}}) require.NoError(t, err) require.NoError(t, provenanceStore.SetProvenance(context.Background(), &r1, orgID, models.ProvenanceNone)) require.NoError(t, provenanceStore.SetProvenance(context.Background(), &r2, orgID, models.ProvenanceNone)) @@ -1444,7 +1444,7 @@ func TestDeleteAlertRule(t *testing.T) { ac.CanWriteAllRulesFunc = func(ctx context.Context, user identity.Requester) (bool, error) { return false, nil } r := createNoGroupRule("nogroup-del-auth", orgID, "my-namespace") - _, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.AlertRule{r}) + _, err := ruleStore.InsertAlertRules(context.Background(), models.NewUserUID(u), []models.InsertRule{{AlertRule: r}}) require.NoError(t, err) require.NoError(t, provenanceStore.SetProvenance(context.Background(), &r, orgID, models.ProvenanceNone)) @@ -1917,7 +1917,7 @@ func TestReplaceGroup(t *testing.T) { return true, nil } - err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) require.Len(t, ac.Calls, 1) @@ -1953,7 +1953,7 @@ func TestReplaceGroup(t *testing.T) { return expectedErr } - err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.ErrorIs(t, err, expectedErr) require.Len(t, ac.Calls, 2) @@ -1976,7 +1976,7 @@ func TestReplaceGroup(t *testing.T) { return nil } - err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI) + err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceAPI, "") require.NoError(t, err) require.Len(t, ac.Calls, 2) @@ -2013,11 +2013,11 @@ func TestReplaceGroup(t *testing.T) { Rules: []models.AlertRule{rule}, } - err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone) + err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone, "") require.NoError(t, err) rule.Metadata.PrometheusStyleRule.OriginalRuleDefinition = "new" - err = service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone) + err = service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone, "") require.NoError(t, err) rule, _, err = service.GetAlertRule(context.Background(), u, rule.UID) @@ -2036,7 +2036,7 @@ func TestReplaceGroup(t *testing.T) { second.RuleGroup = group.Title group.Rules = append(group.Rules, second) - err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone) + err := service.ReplaceRuleGroup(context.Background(), u, group, models.ProvenanceNone, "") require.Error(t, err) require.ErrorContains(t, err, "cannot be used for rule groups with multiple rules") }) @@ -2050,7 +2050,7 @@ func TestReplaceGroup(t *testing.T) { // change the group name away from the sentinel value groupSeed.Title = "some-other-group" // not the sentinel group name - err := service.ReplaceRuleGroup(context.Background(), u, groupSeed, models.ProvenanceNone) + err := service.ReplaceRuleGroup(context.Background(), u, groupSeed, models.ProvenanceNone, "") require.Error(t, err) require.ErrorContains(t, err, "cannot move rule out of this group") }) diff --git a/pkg/services/ngalert/provisioning/persist.go b/pkg/services/ngalert/provisioning/persist.go index 753c874cce3..914a3644984 100644 --- a/pkg/services/ngalert/provisioning/persist.go +++ b/pkg/services/ngalert/provisioning/persist.go @@ -34,7 +34,7 @@ type RuleStore interface { ListAlertRules(ctx context.Context, query *models.ListAlertRulesQuery) (models.RulesGroup, error) ListAlertRulesPaginated(ctx context.Context, query *models.ListAlertRulesExtendedQuery) (models.RulesGroup, string, error) GetRuleGroupInterval(ctx context.Context, orgID int64, namespaceUID string, ruleGroup string) (int64, error) - InsertAlertRules(ctx context.Context, user *models.UserUID, rule []models.AlertRule) ([]models.AlertRuleKeyWithId, error) + InsertAlertRules(ctx context.Context, user *models.UserUID, rule []models.InsertRule) ([]models.AlertRuleKeyWithId, error) UpdateAlertRules(ctx context.Context, user *models.UserUID, rule []models.UpdateRule) error DeleteAlertRulesByUID(ctx context.Context, orgID int64, user *models.UserUID, permanently bool, ruleUID ...string) error GetAlertRulesGroupByRuleUID(ctx context.Context, query *models.GetAlertRulesGroupByRuleUIDQuery) ([]*models.AlertRule, error) diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 076974ea9c9..3feefac6d73 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -336,7 +336,7 @@ func (st DBstore) GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmode // InsertAlertRules is a handler for creating/updating alert rules. // Returns the UID and ID of rules that were created in the same order as the input rules. -func (st DBstore) InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.AlertRule) ([]ngmodels.AlertRuleKeyWithId, error) { +func (st DBstore) InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, rules []ngmodels.InsertRule) ([]ngmodels.AlertRuleKeyWithId, error) { ids := make([]ngmodels.AlertRuleKeyWithId, 0, len(rules)) keys := make([]ngmodels.AlertRuleKey, 0, len(rules)) return ids, st.SQLStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { @@ -352,14 +352,14 @@ func (st DBstore) InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, r.UID = uid } r.Version = 1 - if err := st.validateAlertRule(r); err != nil { + if err := st.validateAlertRule(r.AlertRule); err != nil { return err } if err := (&r).PreSave(TimeNow, user); err != nil { return err } - converted, err := alertRuleFromModelsAlertRule(r) + converted, err := alertRuleFromModelsAlertRule(r.AlertRule) if err != nil { return fmt.Errorf("failed to convert alert rule %q to storage model: %w", r.Title, err) } @@ -369,7 +369,9 @@ func (st DBstore) InsertAlertRules(ctx context.Context, user *ngmodels.UserUID, converted.GUID = uuid.NewString() newRules = append(newRules, converted) - ruleVersions = append(ruleVersions, alertRuleToAlertRuleVersion(converted)) + v := alertRuleToAlertRuleVersion(converted) + v.Message = r.Message + ruleVersions = append(ruleVersions, v) } if len(newRules) > 0 { // we have to insert the rules one by one as otherwise we are @@ -444,6 +446,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, user *ngmodels.UserUID, v := alertRuleToAlertRuleVersion(converted) v.Version++ v.ParentVersion = r.Existing.Version + v.Message = r.Message // check if there is diff between existing and new, and if no, skip saving version. existingConverted, err := alertRuleFromModelsAlertRule(*r.Existing) diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index b0b7592c2d1..b9a574d33fc 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -38,6 +38,17 @@ import ( tutil "github.com/grafana/grafana/pkg/util/testutil" ) +func toInsertRules(rules []models.AlertRule) []models.InsertRule { + result := make([]models.InsertRule, len(rules)) + for i := range rules { + result[i] = models.InsertRule{ + AlertRule: rules[i], + Message: "", + } + } + return result +} + func TestIntegrationUpdateAlertRules(t *testing.T) { tutil.SkipIntegrationTestInShortMode(t) @@ -567,7 +578,7 @@ func TestIntegration_DeleteAlertRulesByUID(t *testing.T) { store := createTestStore(sqlStore, folderService, logger, cfg.UnifiedAlerting, b) store.FeatureToggles = featuremgmt.WithFeatures(featuremgmt.FlagAlertRuleRestore) - result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, gen.GenerateMany(3)) + result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, toInsertRules(gen.GenerateMany(3))) uids := make([]string, 0, len(result)) for _, rule := range result { uids = append(uids, rule.UID) @@ -634,7 +645,7 @@ func TestIntegration_DeleteAlertRulesByUID(t *testing.T) { store := createTestStore(sqlStore, folderService, logger, cfg.UnifiedAlerting, b) store.FeatureToggles = featuremgmt.WithFeatures(featuremgmt.FlagAlertRuleRestore) - result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, gen.GenerateMany(3)) + result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, toInsertRules(gen.GenerateMany(3))) uids := make([]string, 0, len(result)) for _, rule := range result { uids = append(uids, rule.UID) @@ -687,7 +698,7 @@ func TestIntegration_DeleteAlertRulesByUID(t *testing.T) { store := createTestStore(sqlStore, folderService, logger, cfg.UnifiedAlerting, b) store.FeatureToggles = featuremgmt.WithFeatures(featuremgmt.FlagAlertRuleRestore) - result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, gen.GenerateMany(3)) + result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, toInsertRules(gen.GenerateMany(3))) uids := make([]string, 0, len(result)) for _, rule := range result { uids = append(uids, rule.UID) @@ -754,7 +765,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { rules := append(gen.GenerateMany(5), recordingRulesGen.GenerateMany(5)...) - ids, err := store.InsertAlertRules(context.Background(), &usr, rules) + ids, err := store.InsertAlertRules(context.Background(), &usr, toInsertRules(rules)) require.NoError(t, err) require.Len(t, ids, len(rules)) @@ -819,7 +830,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { invalidMetric := "my_metric\x80" invalidRule := recordingRulesGen.Generate() invalidRule.Record.Metric = invalidMetric - _, err := store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{invalidRule}) + _, err := store.InsertAlertRules(context.Background(), &usr, []models.InsertRule{{AlertRule: invalidRule}}) require.ErrorIs(t, err, models.ErrAlertRuleFailedValidation) require.ErrorContains(t, err, "metric name for recording rule must be a valid utf8 string") }) @@ -827,7 +838,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { t.Run("clears fields that should not exist on recording rules", func(t *testing.T) { rule := recordingRulesGen.Generate() - rules, err := store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{rule}) + rules, err := store.InsertAlertRules(context.Background(), &usr, []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.Len(t, rules, 1) ruleUID := rules[0].UID @@ -844,19 +855,19 @@ func TestIntegrationInsertAlertRules(t *testing.T) { }) t.Run("fail to insert rules with same ID", func(t *testing.T) { - _, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{rules[0]}) + _, err = store.InsertAlertRules(context.Background(), &usr, []models.InsertRule{{AlertRule: rules[0]}}) require.ErrorIs(t, err, models.ErrAlertRuleConflictBase) }) t.Run("should not fail insert rules with the same title in a folder", func(t *testing.T) { cp := models.CopyRule(&rules[0]) cp.UID = cp.UID + "-new" - _, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{*cp}) + _, err = store.InsertAlertRules(context.Background(), &usr, []models.InsertRule{{AlertRule: *cp}}) require.NoError(t, err) }) t.Run("should not let insert rules with the same UID", func(t *testing.T) { cp := models.CopyRule(&rules[0]) cp.Title = "unique-test-title" - _, err = store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{*cp}) + _, err = store.InsertAlertRules(context.Background(), &usr, []models.InsertRule{{AlertRule: *cp}}) require.ErrorIs(t, err, models.ErrAlertRuleConflictBase) require.ErrorContains(t, err, fmt.Sprintf("Failed to save alert rule '%s' in organization %d due to conflict", cp.UID, cp.OrgID)) }) @@ -877,7 +888,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { return nil } - rules, err := store.InsertAlertRules(context.Background(), &usr, []models.AlertRule{rule}) + rules, err := store.InsertAlertRules(context.Background(), &usr, []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) require.Len(t, rules, 1) require.True(t, called) @@ -885,7 +896,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { t.Run("nil identity should be handled correctly", func(t *testing.T) { rule := gen.Generate() - ids, err = store.InsertAlertRules(context.Background(), nil, []models.AlertRule{rule}) + ids, err = store.InsertAlertRules(context.Background(), nil, []models.InsertRule{{AlertRule: rule}}) require.NoError(t, err) insertedRule, err := store.GetRuleByID(context.Background(), models.GetAlertRuleByIDQuery{ ID: ids[0].ID, @@ -942,7 +953,7 @@ func TestIntegrationAlertRulesNotificationSettings(t *testing.T) { require.NoError(t, store.SetProvenance(context.Background(), rule, rule.OrgID, p)) } - _, err := store.InsertAlertRules(context.Background(), &usr, deref) + _, err := store.InsertAlertRules(context.Background(), &usr, toInsertRules(deref)) require.NoError(t, err) t.Run("should find rules by receiver name", func(t *testing.T) { @@ -1220,7 +1231,7 @@ func TestIntegrationListNotificationSettings(t *testing.T) { orgRules := append(rulesWithNotificationsAndReceiver, rulesWithNotificationsAndTimeInterval...) - _, err := store.InsertAlertRules(context.Background(), &usr, deref) + _, err := store.InsertAlertRules(context.Background(), &usr, toInsertRules(deref)) require.NoError(t, err) result, err := store.ListNotificationSettings(context.Background(), models.ListNotificationSettingsQuery{OrgID: 1}) @@ -1319,7 +1330,7 @@ func TestIntegrationGetNamespacesByRuleUID(t *testing.T) { store := createTestStore(sqlStore, folderService, logger, cfg.UnifiedAlerting, b) rules := models.RuleGen.With(models.RuleMuts.WithOrgID(1), models.RuleMuts.WithRandomRecordingRules()).GenerateMany(5) - _, err := store.InsertAlertRules(context.Background(), &usr, rules) + _, err := store.InsertAlertRules(context.Background(), &usr, toInsertRules(rules)) require.NoError(t, err) uids := make([]string, 0, len(rules)) @@ -1381,7 +1392,7 @@ func TestIntegrationRuleGroupsCaseSensitive(t *testing.T) { group2 := gen.With(gen.WithGroupKey(groupKey2)).GenerateMany(1, 3) group3 := gen.With(gen.WithGroupKey(groupKey3)).GenerateMany(1, 3) - _, err := store.InsertAlertRules(context.Background(), &usr, append(append(append(misc, group1...), group2...), group3...)) + _, err := store.InsertAlertRules(context.Background(), &usr, toInsertRules(append(append(append(misc, group1...), group2...), group3...))) require.NoError(t, err) t.Run("GetAlertRulesGroupByRuleUID", func(t *testing.T) { @@ -1497,7 +1508,7 @@ func TestIntegrationListAlertRulesByGroupCaseSensitiveOrdering(t *testing.T) { // Insert all rules allRules := append(append(groupUpper, groupMixed...), groupLower...) - _, err := store.InsertAlertRules(context.Background(), &usr, allRules) + _, err := store.InsertAlertRules(context.Background(), &usr, toInsertRules(allRules)) require.NoError(t, err) t.Run("should order groups case-sensitively", func(t *testing.T) { @@ -1661,7 +1672,7 @@ func TestIntegrationGetRuleVersions(t *testing.T) { gen := models.RuleGen gen = gen.With(gen.WithIntervalMatching(store.Cfg.BaseInterval), gen.WithOrgID(orgID), gen.WithVersion(1)) - inserted, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.AlertRule{gen.Generate()}) + inserted, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.InsertRule{{AlertRule: gen.Generate()}}) require.NoError(t, err) ruleV1, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: inserted[0].UID}) require.NoError(t, err) @@ -2377,7 +2388,7 @@ func TestIntegration_ListDeletedRules(t *testing.T) { gen := models.RuleGen gen = gen.With(gen.WithIntervalMatching(store.Cfg.BaseInterval), gen.WithOrgID(orgID)) - result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.AlertRule{gen.Generate(), gen.Generate()}) + result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, []models.InsertRule{{AlertRule: gen.Generate()}, {AlertRule: gen.Generate()}}) require.NoError(t, err) rule1, err := store.GetAlertRuleByUID(context.Background(), &models.GetAlertRuleByUIDQuery{UID: result[0].UID}) require.NoError(t, err) @@ -2465,7 +2476,7 @@ func TestIntegration_CleanUpDeletedAlertRules(t *testing.T) { gen = gen.With(gen.WithOrgID(orgID)) - result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, gen.GenerateMany(3)) + result, err := store.InsertAlertRules(context.Background(), &models.AlertingUserUID, toInsertRules(gen.GenerateMany(3))) uids := make([]string, 0, len(result)) for _, rule := range result { uids = append(uids, rule.UID) diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go index 09abeaffba3..cfe790b9853 100644 --- a/pkg/services/ngalert/tests/fakes/rules.go +++ b/pkg/services/ngalert/tests/fakes/rules.go @@ -464,7 +464,7 @@ func (f *RuleStore) UpdateAlertRules(_ context.Context, _ *models.UserUID, q []m return nil } -func (f *RuleStore) InsertAlertRules(_ context.Context, _ *models.UserUID, q []models.AlertRule) ([]models.AlertRuleKeyWithId, error) { +func (f *RuleStore) InsertAlertRules(_ context.Context, _ *models.UserUID, q []models.InsertRule) ([]models.AlertRuleKeyWithId, error) { f.mtx.Lock() defer f.mtx.Unlock() f.RecordedOps = append(f.RecordedOps, q) @@ -475,7 +475,7 @@ func (f *RuleStore) InsertAlertRules(_ context.Context, _ *models.UserUID, q []m AlertRuleKey: rule.GetKey(), ID: rand.Int63(), }) - rulesPerOrg[rule.OrgID] = append(rulesPerOrg[rule.OrgID], rule) + rulesPerOrg[rule.OrgID] = append(rulesPerOrg[rule.OrgID], rule.AlertRule) } for orgID, rules := range rulesPerOrg { diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 60674e8153c..4a8434d5616 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -134,8 +134,8 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s } require.NoError(t, err) - _, err = dbstore.InsertAlertRules(ctx, models.NewUserUID(user), []models.AlertRule{ - { + _, err = dbstore.InsertAlertRules(ctx, models.NewUserUID(user), []models.InsertRule{{ + AlertRule: models.AlertRule{ ID: 0, OrgID: orgID, @@ -162,7 +162,7 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s RuleGroup: ruleGroup, NoDataState: models.NoData, ExecErrState: models.AlertingErrState, - }, + }}, }) require.NoError(t, err) diff --git a/pkg/storage/unified/federated/stats_test.go b/pkg/storage/unified/federated/stats_test.go index 89d50d1da87..04812084f65 100644 --- a/pkg/storage/unified/federated/stats_test.go +++ b/pkg/storage/unified/federated/stats_test.go @@ -79,8 +79,8 @@ func TestIntegrationDirectSQLStats(t *testing.T) { ruleStore := ngalertstore.SetupStoreForTesting(t, db) dashboardUID := "test" - _, err = ruleStore.InsertAlertRules(context.Background(), ngmodels.NewUserUID(tempUser), []ngmodels.AlertRule{ - { + _, err = ruleStore.InsertAlertRules(context.Background(), ngmodels.NewUserUID(tempUser), []ngmodels.InsertRule{{ + AlertRule: ngmodels.AlertRule{ DashboardUID: &dashboardUID, UID: "test", Title: "test", @@ -102,7 +102,7 @@ func TestIntegrationDirectSQLStats(t *testing.T) { ExecErrState: ngmodels.ExecutionErrorState(ngmodels.Alerting), NoDataState: ngmodels.Alerting, IntervalSeconds: 60, - }}) + }}}) require.NoError(t, err) _, err = dashStore.SaveDashboard(ctx, dashboards.SaveDashboardCommand{