From ce55d70fa57e509cb12543ad6baa387b82204e23 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Fri, 5 Sep 2025 10:46:46 -0400 Subject: [PATCH] Alerting: Refactor notification legacy storage (#110619) * make legacy store expose only model.Receiver * use integration as provenance type provider * use revision RenameReceiverInRoutes * introduce function GetReceiversNames in config revision --------- Co-authored-by: Matthew Jacobson --- .../notifications/receiver/legacy_storage.go | 13 ++- pkg/services/ngalert/models/receivers.go | 8 ++ .../ngalert/notifier/legacy_storage/compat.go | 2 +- .../notifier/legacy_storage/receivers.go | 82 +++++++++----- pkg/services/ngalert/notifier/receiver_svc.go | 100 ++++++++---------- .../ngalert/notifier/receiver_svc_test.go | 24 +++-- .../ngalert/provisioning/contactpoints.go | 4 +- .../provisioning/contactpoints_test.go | 7 +- .../provisioning/notification_policies.go | 5 +- pkg/services/ngalert/provisioning/testing.go | 10 +- 10 files changed, 136 insertions(+), 119 deletions(-) diff --git a/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go b/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go index cd61c164017..e224c210ff8 100644 --- a/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go +++ b/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go @@ -16,7 +16,6 @@ import ( grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" alertingac "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" - "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier/legacy_storage" ) @@ -30,7 +29,7 @@ type ReceiverService interface { GetReceivers(ctx context.Context, q ngmodels.GetReceiversQuery, user identity.Requester) ([]*ngmodels.Receiver, error) CreateReceiver(ctx context.Context, r *ngmodels.Receiver, orgID int64, user identity.Requester) (*ngmodels.Receiver, error) UpdateReceiver(ctx context.Context, r *ngmodels.Receiver, storedSecureFields map[string][]string, orgID int64, user identity.Requester) (*ngmodels.Receiver, error) - DeleteReceiver(ctx context.Context, name string, provenance definitions.Provenance, version string, orgID int64, user identity.Requester) error + DeleteReceiver(ctx context.Context, name string, provenance ngmodels.Provenance, version string, orgID int64, user identity.Requester) error } type MetadataService interface { @@ -76,9 +75,9 @@ func (s *legacyStorage) List(ctx context.Context, opts *internalversion.ListOpti q := ngmodels.GetReceiversQuery{ OrgID: orgId, Decrypt: false, - //Names: ctx.QueryStrings("names"), // TODO: Query params. - //Limit: ctx.QueryInt("limit"), - //Offset: ctx.QueryInt("offset"), + // Names: ctx.QueryStrings("names"), // TODO: Query params. + // Limit: ctx.QueryInt("limit"), + // Offset: ctx.QueryInt("offset"), } user, err := identity.GetRequester(ctx) @@ -273,8 +272,8 @@ func (s *legacyStorage) Delete(ctx context.Context, uid string, deleteValidation version = *options.Preconditions.ResourceVersion } - err = s.service.DeleteReceiver(ctx, uid, definitions.Provenance(ngmodels.ProvenanceNone), version, info.OrgID, user) // TODO add support for dry-run option - return old, false, err // false - will be deleted async + err = s.service.DeleteReceiver(ctx, uid, ngmodels.ProvenanceNone, version, info.OrgID, user) // TODO add support for dry-run option + return old, false, err // false - will be deleted async } func (s *legacyStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) { diff --git a/pkg/services/ngalert/models/receivers.go b/pkg/services/ngalert/models/receivers.go index 3a0996c68f6..56c6ad06841 100644 --- a/pkg/services/ngalert/models/receivers.go +++ b/pkg/services/ngalert/models/receivers.go @@ -145,6 +145,14 @@ type Integration struct { SecureSettings map[string]string } +func (integration *Integration) ResourceType() string { + return "contactPoint" +} + +func (integration *Integration) ResourceID() string { + return integration.UID +} + // IntegrationConfig represents the configuration of an integration. It contains the type and information about the fields. type IntegrationConfig struct { Type string diff --git a/pkg/services/ngalert/notifier/legacy_storage/compat.go b/pkg/services/ngalert/notifier/legacy_storage/compat.go index ec3c8a8aec3..ccc9f030933 100644 --- a/pkg/services/ngalert/notifier/legacy_storage/compat.go +++ b/pkg/services/ngalert/notifier/legacy_storage/compat.go @@ -94,7 +94,7 @@ func PostableApiReceiverToReceiver(postable *apimodels.PostableApiReceiver, prov // GetReceiverProvenance determines the provenance of a definitions.PostableApiReceiver based on the provenance of its integrations. func GetReceiverProvenance(storedProvenances map[string]models.Provenance, r *apimodels.PostableApiReceiver) models.Provenance { - if len(r.GrafanaManagedReceivers) == 0 { + if len(r.GrafanaManagedReceivers) == 0 || len(storedProvenances) == 0 { return models.ProvenanceNone } diff --git a/pkg/services/ngalert/notifier/legacy_storage/receivers.go b/pkg/services/ngalert/notifier/legacy_storage/receivers.go index 36e9884b876..6272a14b731 100644 --- a/pkg/services/ngalert/notifier/legacy_storage/receivers.go +++ b/pkg/services/ngalert/notifier/legacy_storage/receivers.go @@ -1,15 +1,18 @@ package legacy_storage import ( - "errors" "fmt" "slices" + "github.com/grafana/alerting/definition" + "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/util" ) +type Provenances map[string]models.Provenance + func (rev *ConfigRevision) DeleteReceiver(uid string) { // Remove the receiver from the configuration. rev.Config.AlertmanagerConfig.Receivers = slices.DeleteFunc(rev.Config.AlertmanagerConfig.Receivers, func(r *definitions.PostableApiReceiver) bool { @@ -17,15 +20,13 @@ func (rev *ConfigRevision) DeleteReceiver(uid string) { }) } -func (rev *ConfigRevision) CreateReceiver(receiver *models.Receiver) (*definitions.PostableApiReceiver, error) { - // Check if the receiver already exists. - _, err := rev.GetReceiver(receiver.GetUID()) - if err == nil { +func (rev *ConfigRevision) CreateReceiver(receiver *models.Receiver) (*models.Receiver, error) { + exists := slices.ContainsFunc(rev.Config.AlertmanagerConfig.Receivers, func(r *definition.PostableApiReceiver) bool { + return NameToUid(r.Name) == receiver.GetUID() + }) + if exists { return nil, ErrReceiverExists.Errorf("") } - if !errors.Is(err, ErrReceiverNotFound) { - return nil, err - } if err := validateAndSetIntegrationUIDs(receiver); err != nil { return nil, err @@ -42,32 +43,33 @@ func (rev *ConfigRevision) CreateReceiver(receiver *models.Receiver) (*definitio return nil, err } - return postable, nil + return PostableApiReceiverToReceiver(postable, receiver.Provenance) } -func (rev *ConfigRevision) UpdateReceiver(receiver *models.Receiver) (*definitions.PostableApiReceiver, error) { - existing, err := rev.GetReceiver(receiver.GetUID()) - if err != nil { - return nil, err +func (rev *ConfigRevision) UpdateReceiver(receiver *models.Receiver) (*models.Receiver, error) { + existingIdx := slices.IndexFunc(rev.Config.AlertmanagerConfig.Receivers, func(postable *definitions.PostableApiReceiver) bool { + return NameToUid(postable.GetName()) == receiver.GetUID() + }) + if existingIdx < 0 { + return nil, ErrReceiverNotFound.Errorf("") } if err := validateAndSetIntegrationUIDs(receiver); err != nil { return nil, err } - postable, err := ReceiverToPostableApiReceiver(receiver) + newReceiver, err := ReceiverToPostableApiReceiver(receiver) if err != nil { return nil, err } - // Update receiver in the configuration. - *existing = *postable + rev.Config.AlertmanagerConfig.Receivers[existingIdx] = newReceiver - if err := rev.ValidateReceiver(existing); err != nil { + if err := rev.ValidateReceiver(newReceiver); err != nil { return nil, err } - return postable, nil + return PostableApiReceiverToReceiver(newReceiver, receiver.Provenance) } // ReceiverNameUsedByRoutes checks if a receiver name is used in any routes. @@ -82,23 +84,47 @@ func (rev *ConfigRevision) ReceiverUseByName() map[string]int { return m } -func (rev *ConfigRevision) GetReceiver(uid string) (*definitions.PostableApiReceiver, error) { +func (rev *ConfigRevision) GetReceiver(uid string, prov Provenances) (*models.Receiver, error) { for _, r := range rev.Config.AlertmanagerConfig.Receivers { - if NameToUid(r.GetName()) == uid { - return r, nil + if NameToUid(r.GetName()) != uid { + continue } + recv, err := PostableApiReceiverToReceiver(r, GetReceiverProvenance(prov, r)) + if err != nil { + return nil, fmt.Errorf("failed to convert receiver %q: %w", r.Name, err) + } + return recv, nil } return nil, ErrReceiverNotFound.Errorf("") } -func (rev *ConfigRevision) GetReceivers(uids []string) []*definitions.PostableApiReceiver { - receivers := make([]*definitions.PostableApiReceiver, 0, len(uids)) - for _, r := range rev.Config.AlertmanagerConfig.Receivers { - if len(uids) == 0 || slices.Contains(uids, NameToUid(r.GetName())) { - receivers = append(receivers, r) - } +func (rev *ConfigRevision) GetReceivers(uids []string, prov Provenances) ([]*models.Receiver, error) { + capacity := len(uids) + if capacity == 0 { + capacity = len(rev.Config.AlertmanagerConfig.Receivers) } - return receivers + receivers := make([]*models.Receiver, 0, capacity) + for _, r := range rev.Config.AlertmanagerConfig.Receivers { + uid := NameToUid(r.GetName()) + if len(uids) > 0 && !slices.Contains(uids, uid) { + continue + } + recv, err := PostableApiReceiverToReceiver(r, GetReceiverProvenance(prov, r)) + if err != nil { + return nil, fmt.Errorf("failed to convert receiver %q: %w", r.Name, err) + } + receivers = append(receivers, recv) + } + return receivers, nil +} + +// GetReceiversNames returns a map of receiver names +func (rev *ConfigRevision) GetReceiversNames() map[string]struct{} { + result := make(map[string]struct{}, len(rev.Config.AlertmanagerConfig.Receivers)) + for _, r := range rev.Config.AlertmanagerConfig.Receivers { + result[r.GetName()] = struct{}{} + } + return result } func DecryptedReceivers(receivers []*definitions.PostableApiReceiver, decryptFn models.DecryptFn) ([]*definitions.PostableApiReceiver, error) { diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index 4fb76c6f26d..a2332cbe6cd 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" ac "github.com/grafana/grafana/pkg/services/accesscontrol" - "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/notifier/legacy_storage" "github.com/grafana/grafana/pkg/services/ngalert/provisioning/validation" @@ -119,6 +118,10 @@ func NewReceiverService( } } +func (rs *ReceiverService) loadProvenances(ctx context.Context, orgID int64) (map[string]models.Provenance, error) { + return rs.provisioningStore.GetProvenances(ctx, orgID, (&models.Integration{}).ResourceType()) +} + // GetReceiver returns a receiver by name. // The receiver's secure settings are decrypted if requested and the user has access to do so. func (rs *ReceiverService) GetReceiver(ctx context.Context, q models.GetReceiverQuery, user identity.Requester) (*models.Receiver, error) { @@ -133,7 +136,13 @@ func (rs *ReceiverService) GetReceiver(ctx context.Context, q models.GetReceiver if err != nil { return nil, err } - postable, err := revision.GetReceiver(legacy_storage.NameToUid(q.Name)) + + prov, err := rs.loadProvenances(ctx, q.OrgID) + if err != nil { + return nil, err + } + + rcv, err := revision.GetReceiver(legacy_storage.NameToUid(q.Name), prov) if err != nil { return nil, err } @@ -142,15 +151,6 @@ func (rs *ReceiverService) GetReceiver(ctx context.Context, q models.GetReceiver attribute.String("concurrency_token", revision.ConcurrencyToken), )) - storedProvenances, err := rs.provisioningStore.GetProvenances(ctx, q.OrgID, (&definitions.EmbeddedContactPoint{}).ResourceType()) - if err != nil { - return nil, err - } - rcv, err := legacy_storage.PostableApiReceiverToReceiver(postable, legacy_storage.GetReceiverProvenance(storedProvenances, postable)) - if err != nil { - return nil, err - } - auth := rs.authz.AuthorizeReadDecrypted if !q.Decrypt { auth = rs.authz.AuthorizeRead @@ -195,22 +195,22 @@ func (rs *ReceiverService) GetReceivers(ctx context.Context, q models.GetReceive if err != nil { return nil, err } - postables := revision.GetReceivers(uids) + + prov, err := rs.loadProvenances(ctx, q.OrgID) + if err != nil { + return nil, err + } + + receivers, err := revision.GetReceivers(uids, prov) + if err != nil { + return nil, err + } span.AddEvent("Loaded receivers", trace.WithAttributes( attribute.String("concurrency_token", revision.ConcurrencyToken), - attribute.Int("count", len(postables)), + attribute.Int("count", len(receivers)), )) - storedProvenances, err := rs.provisioningStore.GetProvenances(ctx, q.OrgID, (&definitions.EmbeddedContactPoint{}).ResourceType()) - if err != nil { - return nil, err - } - receivers, err := legacy_storage.PostableApiReceiversToReceivers(postables, storedProvenances) - if err != nil { - return nil, err - } - filterFn := rs.authz.FilterReadDecrypted if !q.Decrypt { filterFn = rs.authz.FilterRead @@ -243,7 +243,7 @@ func (rs *ReceiverService) GetReceivers(ctx context.Context, q models.GetReceive // DeleteReceiver deletes a receiver by uid. // UID field currently does not exist, we assume the uid is a particular hashed value of the receiver name. -func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, callerProvenance definitions.Provenance, version string, orgID int64, user identity.Requester) error { +func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, callerProvenance models.Provenance, version string, orgID int64, user identity.Requester) error { ctx, span := rs.tracer.Start(ctx, "alerting.receivers.delete", trace.WithAttributes( attribute.String("receiver_uid", uid), attribute.String("receiver_version", version), @@ -257,7 +257,13 @@ func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, calle if err != nil { return err } - postable, err := revision.GetReceiver(uid) + + prov, err := rs.loadProvenances(ctx, orgID) + if err != nil { + return err + } + + existing, err := revision.GetReceiver(uid, prov) if err != nil { if errors.Is(err, legacy_storage.ErrReceiverNotFound) { return nil @@ -265,15 +271,6 @@ func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, calle return err } - storedProvenances, err := rs.provisioningStore.GetProvenances(ctx, orgID, (&definitions.EmbeddedContactPoint{}).ResourceType()) - if err != nil { - return err - } - existing, err := legacy_storage.PostableApiReceiverToReceiver(postable, legacy_storage.GetReceiverProvenance(storedProvenances, postable)) - if err != nil { - return err - } - logger := rs.log.FromContext(ctx).New("receiver", existing.Name, "uid", uid, "version", version, "integrations", existing.GetIntegrationTypes()) // Check optimistic concurrency. @@ -287,7 +284,7 @@ func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, calle logger.Debug("Ignoring optimistic concurrency check because version was not provided", "operation", "delete") } - if err := rs.provenanceValidator(existing.Provenance, models.Provenance(callerProvenance)); err != nil { + if err := rs.provenanceValidator(existing.Provenance, callerProvenance); err != nil { return err } @@ -354,7 +351,7 @@ func (rs *ReceiverService) CreateReceiver(ctx context.Context, r *models.Receive // Generate UID from name. createdReceiver.UID = legacy_storage.NameToUid(createdReceiver.Name) - created, err := revision.CreateReceiver(&createdReceiver) + result, err = revision.CreateReceiver(&createdReceiver) if err != nil { return nil, err } @@ -371,10 +368,6 @@ func (rs *ReceiverService) CreateReceiver(ctx context.Context, r *models.Receive return nil, err } - result, err = legacy_storage.PostableApiReceiverToReceiver(created, createdReceiver.Provenance) - if err != nil { - return nil, err - } span.AddEvent("Created a new receiver", trace.WithAttributes( attribute.String("uid", result.UID), attribute.String("version", result.Version), @@ -403,16 +396,13 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive if err != nil { return nil, err } - postable, err := revision.GetReceiver(r.GetUID()) + + prov, err := rs.loadProvenances(ctx, orgID) if err != nil { return nil, err } - storedProvenances, err := rs.provisioningStore.GetProvenances(ctx, orgID, (&definitions.EmbeddedContactPoint{}).ResourceType()) - if err != nil { - return nil, err - } - existing, err := legacy_storage.PostableApiReceiverToReceiver(postable, legacy_storage.GetReceiverProvenance(storedProvenances, postable)) + existing, err := revision.GetReceiver(r.GetUID(), prov) if err != nil { return nil, err } @@ -460,7 +450,7 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive return nil, legacy_storage.MakeErrReceiverInvalid(err) } - updated, err := revision.UpdateReceiver(&updatedReceiver) + result, err := revision.UpdateReceiver(&updatedReceiver) if err != nil { return nil, err } @@ -468,7 +458,7 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive err = rs.xact.InTransaction(ctx, func(ctx context.Context) error { // If the name of the receiver changed, we must update references to it in both routes and notification settings. if existing.Name != r.Name { - err := rs.RenameReceiverInDependentResources(ctx, orgID, revision.Config.AlertmanagerConfig.Route, existing.Name, r.Name, r.Provenance) + err := rs.RenameReceiverInDependentResources(ctx, orgID, revision, existing.Name, r.Name, r.Provenance) if err != nil { return err } @@ -499,10 +489,6 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive return nil, err } - result, err := legacy_storage.PostableApiReceiverToReceiver(updated, updatedReceiver.Provenance) - if err != nil { - return nil, err - } logger.Info("Updated receiver", "new_version", result.Version) return result, nil } @@ -575,8 +561,7 @@ func removedIntegrations(old, new *models.Receiver) []*models.Integration { func (rs *ReceiverService) setReceiverProvenance(ctx context.Context, orgID int64, receiver *models.Receiver) error { // Add provenance for all integrations in the receiver. for _, integration := range receiver.Integrations { - target := definitions.EmbeddedContactPoint{UID: integration.UID} - if err := rs.provisioningStore.SetProvenance(ctx, &target, orgID, receiver.Provenance); err != nil { // TODO: Should we set ProvenanceNone? + if err := rs.provisioningStore.SetProvenance(ctx, integration, orgID, receiver.Provenance); err != nil { // TODO: Should we set ProvenanceNone? return err } } @@ -586,8 +571,7 @@ func (rs *ReceiverService) setReceiverProvenance(ctx context.Context, orgID int6 func (rs *ReceiverService) deleteProvenances(ctx context.Context, orgID int64, integrations []*models.Integration) error { // Delete provenance for all integrations. for _, integration := range integrations { - target := definitions.EmbeddedContactPoint{UID: integration.UID} - if err := rs.provisioningStore.DeleteProvenance(ctx, &target, orgID); err != nil { + if err := rs.provisioningStore.DeleteProvenance(ctx, integration, orgID); err != nil { return err } } @@ -700,7 +684,7 @@ func makeErrReceiverDependentResourcesProvenance(usedByRoutes bool, rules []mode }) } -func (rs *ReceiverService) RenameReceiverInDependentResources(ctx context.Context, orgID int64, route *definitions.Route, oldName, newName string, receiverProvenance models.Provenance) error { +func (rs *ReceiverService) RenameReceiverInDependentResources(ctx context.Context, orgID int64, revision *legacy_storage.ConfigRevision, oldName, newName string, receiverProvenance models.Provenance) error { ctx, span := rs.tracer.Start(ctx, "alerting.receivers.rename-dependent-resources", trace.WithAttributes( attribute.String("oldName", oldName), attribute.String("newName", newName), @@ -710,10 +694,10 @@ func (rs *ReceiverService) RenameReceiverInDependentResources(ctx context.Contex validate := validation.ValidateProvenanceOfDependentResources(receiverProvenance) // if there are no references to the old time interval, exit - updatedRoutes := legacy_storage.RenameReceiverInRoute(oldName, newName, route) + updatedRoutes := revision.RenameReceiverInRoutes(oldName, newName) canUpdate := true if updatedRoutes > 0 { - routeProvenance, err := rs.provisioningStore.GetProvenance(ctx, route, orgID) + routeProvenance, err := rs.provisioningStore.GetProvenance(ctx, revision.Config.AlertmanagerConfig.Route, orgID) if err != nil { return err } diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index 5451a53a859..0954f7239f9 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -221,7 +221,7 @@ func TestReceiverService_Delete(t *testing.T) { name string user identity.Requester deleteUID string - callerProvenance definitions.Provenance + callerProvenance models.Provenance version string storeSettings map[models.AlertRuleKey][]models.NotificationSettings existing *models.Receiver @@ -243,7 +243,7 @@ func TestReceiverService_Delete(t *testing.T) { name: "service deletes receiver with provenance", user: writer, deleteUID: baseReceiver.UID, - callerProvenance: definitions.Provenance(models.ProvenanceAPI), + callerProvenance: models.ProvenanceAPI, existing: util.Pointer(models.CopyReceiverWith(baseReceiver, models.ReceiverMuts.WithProvenance(models.ProvenanceAPI), models.ReceiverMuts.WithIntegrations(slackIntegration, emailIntegration))), }, { @@ -274,7 +274,7 @@ func TestReceiverService_Delete(t *testing.T) { name: "delete provisioning provenance fails when caller is ProvenanceNone", user: writer, deleteUID: baseReceiver.UID, - callerProvenance: definitions.Provenance(models.ProvenanceNone), + callerProvenance: models.ProvenanceNone, existing: util.Pointer(models.CopyReceiverWith(baseReceiver, models.ReceiverMuts.WithProvenance(models.ProvenanceFile))), expectedErr: validation.MakeErrProvenanceChangeNotAllowed(models.ProvenanceFile, models.ProvenanceNone), }, @@ -282,7 +282,7 @@ func TestReceiverService_Delete(t *testing.T) { name: "delete provisioning provenance fails when caller is a different type", // TODO: This should fail once we move from lenient to strict validation. user: writer, deleteUID: baseReceiver.UID, - callerProvenance: definitions.Provenance(models.ProvenanceFile), + callerProvenance: models.ProvenanceFile, existing: util.Pointer(models.CopyReceiverWith(baseReceiver, models.ReceiverMuts.WithProvenance(models.ProvenanceAPI))), // expectedErr: validation.MakeErrProvenanceChangeNotAllowed(models.ProvenanceAPI, models.ProvenanceFile), }, @@ -528,9 +528,13 @@ func TestReceiverService_Create(t *testing.T) { if tc.expectedStored != nil { revision, err := sut.cfgStore.Get(context.Background(), writer.GetOrgID()) require.NoError(t, err) - rcv, err := revision.GetReceiver(legacy_storage.NameToUid(tc.expectedStored.Name)) - require.NoError(t, err) - assert.Equal(t, tc.expectedStored, rcv) + for _, apiReceiver := range revision.Config.AlertmanagerConfig.Receivers { + if apiReceiver.Name == tc.expectedStored.Name { + assert.Equal(t, tc.expectedStored, apiReceiver) + return + } + } + t.Fatalf("expected to find receiver %q in revision", tc.expectedStored.Name) } }) } @@ -734,11 +738,9 @@ func TestReceiverService_Update(t *testing.T) { // Create route after receivers as they will be referenced. revision, err := sut.cfgStore.Get(context.Background(), tc.user.GetOrgID()) require.NoError(t, err) - result, err := revision.CreateReceiver(tc.existing) + created, err := revision.CreateReceiver(tc.existing) require.NoError(t, err) - created, err := legacy_storage.PostableApiReceiverToReceiver(result, tc.existing.Provenance) - require.NoError(t, err) err = sut.cfgStore.Save(context.Background(), revision, tc.user.GetOrgID()) require.NoError(t, err) @@ -1413,7 +1415,7 @@ func TestReceiverServiceAC_Delete(t *testing.T) { return false } for _, recv := range allReceivers() { - err := sut.DeleteReceiver(context.Background(), recv.UID, definitions.Provenance(models.ProvenanceNone), versions[recv.UID], orgId, usr) + err := sut.DeleteReceiver(context.Background(), recv.UID, models.ProvenanceNone, versions[recv.UID], orgId, usr) if hasAccess(recv.UID) { require.NoErrorf(t, err, "should have access to receiver '%s', but doesn't", recv.Name) } else { diff --git a/pkg/services/ngalert/provisioning/contactpoints.go b/pkg/services/ngalert/provisioning/contactpoints.go index 2d95bfd37a8..88be77fe0c5 100644 --- a/pkg/services/ngalert/provisioning/contactpoints.go +++ b/pkg/services/ngalert/provisioning/contactpoints.go @@ -43,7 +43,7 @@ type ContactPointService struct { type receiverService interface { GetReceivers(ctx context.Context, query models.GetReceiversQuery, user identity.Requester) ([]*models.Receiver, error) - RenameReceiverInDependentResources(ctx context.Context, orgID int64, route *apimodels.Route, oldName, newName string, receiverProvenance models.Provenance) error + RenameReceiverInDependentResources(ctx context.Context, orgID int64, route *legacy_storage.ConfigRevision, oldName, newName string, receiverProvenance models.Provenance) error } func NewContactPointService( @@ -321,7 +321,7 @@ func (ecp *ContactPointService) UpdateContactPoint(ctx context.Context, orgID in } if fullRemoval { - if err := ecp.receiverService.RenameReceiverInDependentResources(ctx, orgID, revision.Config.AlertmanagerConfig.Route, oldReceiverName, mergedReceiver.Name, provenance); err != nil { + if err := ecp.receiverService.RenameReceiverInDependentResources(ctx, orgID, revision, oldReceiverName, mergedReceiver.Name, provenance); err != nil { return err } if err := ecp.resourcePermissions.DeleteResourcePermissions(ctx, orgID, legacy_storage.NameToUid(oldReceiverName)); err != nil { diff --git a/pkg/services/ngalert/provisioning/contactpoints_test.go b/pkg/services/ngalert/provisioning/contactpoints_test.go index 5f98590e857..a6670af2521 100644 --- a/pkg/services/ngalert/provisioning/contactpoints_test.go +++ b/pkg/services/ngalert/provisioning/contactpoints_test.go @@ -198,8 +198,8 @@ func TestIntegrationContactPointService(t *testing.T) { newCp.Name = newName - svc.RenameReceiverInDependentResourcesFunc = func(ctx context.Context, orgID int64, route *definitions.Route, oldName, newName string, receiverProvenance models.Provenance) error { - legacy_storage.RenameReceiverInRoute(oldName, newName, route) + svc.RenameReceiverInDependentResourcesFunc = func(ctx context.Context, orgID int64, revision *legacy_storage.ConfigRevision, oldName, newName string, receiverProvenance models.Provenance) error { + revision.RenameReceiverInRoutes(oldName, newName) return nil } @@ -213,7 +213,8 @@ func TestIntegrationContactPointService(t *testing.T) { assert.Equal(t, "RenameReceiverInDependentResources", svc.Calls[0].Method) assertInTransaction(t, svc.Calls[0].Args[0].(context.Context)) assert.Equal(t, int64(1), svc.Calls[0].Args[1]) - assert.EqualValues(t, parsed.AlertmanagerConfig.Route, svc.Calls[0].Args[2]) + revision := svc.Calls[0].Args[2].(*legacy_storage.ConfigRevision) + assert.EqualValues(t, parsed.AlertmanagerConfig.Route, revision.Config.AlertmanagerConfig.Route) assert.Equal(t, oldName, svc.Calls[0].Args[3]) assert.Equal(t, newName, svc.Calls[0].Args[4]) assert.Equal(t, models.ProvenanceAPI, svc.Calls[0].Args[5]) diff --git a/pkg/services/ngalert/provisioning/notification_policies.go b/pkg/services/ngalert/provisioning/notification_policies.go index 220783b962c..330caf64731 100644 --- a/pkg/services/ngalert/provisioning/notification_policies.go +++ b/pkg/services/ngalert/provisioning/notification_policies.go @@ -88,11 +88,8 @@ func (nps *NotificationPolicyService) UpdatePolicyTree(ctx context.Context, orgI return definitions.Route{}, "", err } - receivers := map[string]struct{}{} + receivers := revision.GetReceiversNames() receivers[""] = struct{}{} // Allow empty receiver (inheriting from parent) - for _, receiver := range revision.GetReceivers(nil) { - receivers[receiver.Name] = struct{}{} - } err = tree.ValidateReceivers(receivers) if err != nil { diff --git a/pkg/services/ngalert/provisioning/testing.go b/pkg/services/ngalert/provisioning/testing.go index 059ef542d39..9ffad7985a5 100644 --- a/pkg/services/ngalert/provisioning/testing.go +++ b/pkg/services/ngalert/provisioning/testing.go @@ -9,9 +9,9 @@ import ( mock "github.com/stretchr/testify/mock" "github.com/grafana/grafana/pkg/apimachinery/identity" - apimodels "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/notifier" + "github.com/grafana/grafana/pkg/services/ngalert/notifier/legacy_storage" "github.com/grafana/grafana/pkg/services/ngalert/store" ) @@ -232,7 +232,7 @@ func (f *fakeAlertRuleNotificationStore) ListNotificationSettings(ctx context.Co type fakeReceiverService struct { Calls []call GetReceiversFunc func(ctx context.Context, query models.GetReceiversQuery, user identity.Requester) ([]*models.Receiver, error) - RenameReceiverInDependentResourcesFunc func(ctx context.Context, orgID int64, route *apimodels.Route, oldName, newName string, receiverProvenance models.Provenance) error + RenameReceiverInDependentResourcesFunc func(ctx context.Context, orgID int64, revision *legacy_storage.ConfigRevision, oldName, newName string, receiverProvenance models.Provenance) error } func (f *fakeReceiverService) GetReceivers(ctx context.Context, query models.GetReceiversQuery, user identity.Requester) ([]*models.Receiver, error) { @@ -243,10 +243,10 @@ func (f *fakeReceiverService) GetReceivers(ctx context.Context, query models.Get return nil, nil } -func (f *fakeReceiverService) RenameReceiverInDependentResources(ctx context.Context, orgID int64, route *apimodels.Route, oldName, newName string, receiverProvenance models.Provenance) error { - f.Calls = append(f.Calls, call{Method: "RenameReceiverInDependentResources", Args: []interface{}{ctx, orgID, route, oldName, newName, receiverProvenance}}) +func (f *fakeReceiverService) RenameReceiverInDependentResources(ctx context.Context, orgID int64, revision *legacy_storage.ConfigRevision, oldName, newName string, receiverProvenance models.Provenance) error { + f.Calls = append(f.Calls, call{Method: "RenameReceiverInDependentResources", Args: []interface{}{ctx, orgID, revision, oldName, newName, receiverProvenance}}) if f.RenameReceiverInDependentResourcesFunc != nil { - return f.RenameReceiverInDependentResourcesFunc(ctx, orgID, route, oldName, newName, receiverProvenance) + return f.RenameReceiverInDependentResourcesFunc(ctx, orgID, revision, oldName, newName, receiverProvenance) } return nil }