From 405871d41dd84c44c6a999b4dd0f01876f2d5e11 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Wed, 22 Oct 2025 12:49:22 -0400 Subject: [PATCH] refactor: change GetReceiver to get by UID to avoid conversions of name to uid back an forth --- .../notifications/receiver/legacy_storage.go | 9 ++----- pkg/services/ngalert/notifier/receiver_svc.go | 20 +++++++------- .../ngalert/notifier/receiver_svc_test.go | 26 ++++++------------- 3 files changed, 20 insertions(+), 35 deletions(-) diff --git a/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go b/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go index 84c0fe9f7fd..9fad371a009 100644 --- a/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go +++ b/pkg/registry/apps/alerting/notifications/receiver/legacy_storage.go @@ -25,7 +25,7 @@ var ( ) type ReceiverService interface { - GetReceiver(ctx context.Context, q ngmodels.GetReceiverQuery, user identity.Requester) (*ngmodels.Receiver, error) + GetReceiver(ctx context.Context, uid string, decrypt bool, user identity.Requester) (*ngmodels.Receiver, error) 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) @@ -120,18 +120,13 @@ func (s *legacyStorage) Get(ctx context.Context, uid string, _ *metav1.GetOption if err != nil { return nil, apierrors.NewNotFound(ResourceInfo.GroupResource(), uid) } - q := ngmodels.GetReceiverQuery{ - OrgID: info.OrgID, - Name: name, - Decrypt: false, - } user, err := identity.GetRequester(ctx) if err != nil { return nil, err } - r, err := s.service.GetReceiver(ctx, q, user) + r, err := s.service.GetReceiver(ctx, name, false, user) if err != nil { return nil, err } diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index e05b12a2920..0997ee3bbca 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -133,27 +133,27 @@ func (rs *ReceiverService) loadProvenances(ctx context.Context, orgID int64) (ma return rs.provisioningStore.GetProvenances(ctx, orgID, (&models.Integration{}).ResourceType()) } -// GetReceiver returns a receiver by name. +// GetReceiver returns a receiver by its UID. // 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) { +func (rs *ReceiverService) GetReceiver(ctx context.Context, uid string, decrypt bool, user identity.Requester) (*models.Receiver, error) { ctx, span := rs.tracer.Start(ctx, "alerting.receivers.get", trace.WithAttributes( - attribute.Int64("query_org_id", q.OrgID), - attribute.String("query_name", q.Name), - attribute.Bool("query_decrypt", q.Decrypt), + attribute.Int64("query_org_id", user.GetOrgID()), + attribute.String("query_uid", uid), + attribute.Bool("query_decrypt", decrypt), )) defer span.End() - revision, err := rs.cfgStore.Get(ctx, q.OrgID) + revision, err := rs.cfgStore.Get(ctx, user.GetOrgID()) if err != nil { return nil, err } - prov, err := rs.loadProvenances(ctx, q.OrgID) + prov, err := rs.loadProvenances(ctx, user.GetOrgID()) if err != nil { return nil, err } - rcv, err := revision.GetReceiver(legacy_storage.NameToUid(q.Name), prov) + rcv, err := revision.GetReceiver(uid, prov) if err != nil { if errors.Is(err, legacy_storage.ErrReceiverNotFound) && rs.includeImported { imported := rs.getImportedReceivers(ctx, span, []string{legacy_storage.NameToUid(q.Name)}, revision) @@ -171,14 +171,14 @@ func (rs *ReceiverService) GetReceiver(ctx context.Context, q models.GetReceiver )) auth := rs.authz.AuthorizeReadDecrypted - if !q.Decrypt { + if !decrypt { auth = rs.authz.AuthorizeRead } if err := auth(ctx, user, rcv); err != nil { return nil, err } - if q.Decrypt { + if decrypt { err := rcv.Decrypt(rs.decryptor(ctx)) if err != nil { rs.log.FromContext(ctx).Warn("Failed to decrypt secure settings", "name", rcv.Name, "error", err) diff --git a/pkg/services/ngalert/notifier/receiver_svc_test.go b/pkg/services/ngalert/notifier/receiver_svc_test.go index 10e84f33b62..5805e920ebf 100644 --- a/pkg/services/ngalert/notifier/receiver_svc_test.go +++ b/pkg/services/ngalert/notifier/receiver_svc_test.go @@ -50,7 +50,7 @@ func TestIntegrationReceiverService_GetReceiver(t *testing.T) { t.Run("service gets receiver from AM config", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - recv, err := sut.GetReceiver(context.Background(), singleQ(1, "slack receiver"), redactedUser) + recv, err := sut.GetReceiver(context.Background(), legacy_storage.NameToUid("slack receiver"), false, redactedUser) require.NoError(t, err) require.Equal(t, "slack receiver", recv.Name) require.Len(t, recv.Integrations, 1) @@ -60,7 +60,7 @@ func TestIntegrationReceiverService_GetReceiver(t *testing.T) { t.Run("service returns error when receiver does not exist", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService) - _, err := sut.GetReceiver(context.Background(), singleQ(1, "receiver1"), redactedUser) + _, err := sut.GetReceiver(context.Background(), legacy_storage.NameToUid("receiver1"), redactedUser) require.ErrorIs(t, err, legacy_storage.ErrReceiverNotFound) }) @@ -81,7 +81,7 @@ func TestIntegrationReceiverService_GetReceiver(t *testing.T) { t.Run("falls to only Grafana if cannot read imported receivers", func(t *testing.T) { sut := createReceiverServiceSut(t, secretsService, withImportedIncluded, withInvalidExtraConfig) - _, err := sut.GetReceiver(context.Background(), singleQ(1, "receiver1"), redactedUser) + _, err := sut.GetReceiver(context.Background(), singleQ(1, "receiver1"), false, redactedUser) require.ErrorIs(t, err, legacy_storage.ErrReceiverNotFound) _, err = sut.GetReceiver(context.Background(), singleQ(1, "slack receiver"), redactedUser) require.NoError(t, err) @@ -412,8 +412,7 @@ func TestReceiverService_Delete(t *testing.T) { // Ensure receiver saved to store is correct. name, err := legacy_storage.UidToName(tc.deleteUID) require.NoError(t, err) - q := models.GetReceiverQuery{OrgID: tc.user.GetOrgID(), Name: name} - _, err = sut.GetReceiver(context.Background(), q, writer) + _, err = sut.GetReceiver(context.Background(), legacy_storage.NameToUid(name), false, writer) assert.ErrorIs(t, err, legacy_storage.ErrReceiverNotFound) provenances, err := sut.provisioningStore.GetProvenances(context.Background(), tc.user.GetOrgID(), (&definitions.EmbeddedContactPoint{}).ResourceType()) @@ -626,8 +625,7 @@ func TestReceiverService_Create(t *testing.T) { assert.Equal(t, tc.expectedCreate, *created) // Ensure receiver saved to store is correct. - q := models.GetReceiverQuery{OrgID: tc.user.GetOrgID(), Name: tc.receiver.Name, Decrypt: true} - stored, err := sut.GetReceiver(context.Background(), q, decryptUser) + stored, err := sut.GetReceiver(context.Background(), legacy_storage.NameToUid(tc.receiver.Name), true, decryptUser) require.NoError(t, err) decrypted := models.CopyReceiverWith(tc.expectedCreate, models.ReceiverMuts.Decrypted(models.Base64Decrypt)) decrypted.Version = tc.expectedCreate.Version // Version is calculated before decryption. @@ -931,8 +929,7 @@ func TestReceiverService_Update(t *testing.T) { assert.Equal(t, tc.expectedUpdate, *updated) // Ensure receiver saved to store is correct. - q := models.GetReceiverQuery{OrgID: tc.user.GetOrgID(), Name: tc.receiver.Name, Decrypt: true} - stored, err := sut.GetReceiver(context.Background(), q, decryptUser) + stored, err := sut.GetReceiver(context.Background(), legacy_storage.NameToUid(tc.receiver.Name), true, decryptUser) require.NoError(t, err) decrypted := models.CopyReceiverWith(tc.expectedUpdate, models.ReceiverMuts.Decrypted(models.Base64Decrypt)) decrypted.Version = tc.expectedUpdate.Version // Version is calculated before decryption. @@ -1185,7 +1182,7 @@ func TestReceiverServiceAC_Read(t *testing.T) { return false } for _, recv := range allReceivers() { - response, err := sut.GetReceiver(context.Background(), singleQ(orgId, recv.Name), usr) + response, err := sut.GetReceiver(context.Background(), legacy_storage.NameToUid(recv.Name), false, usr) if isVisible(recv.UID) { require.NoErrorf(t, err, "receiver '%s' should be visible, but isn't", recv.Name) assert.NotNil(t, response) @@ -1207,7 +1204,7 @@ func TestReceiverServiceAC_Read(t *testing.T) { } sut.authz = ac.NewReceiverAccess[*models.Receiver](acimpl.ProvideAccessControl(featuremgmt.WithFeatures()), true) for _, recv := range allReceivers() { - response, err := sut.GetReceiver(context.Background(), singleQ(orgId, recv.Name), usr) + response, err := sut.GetReceiver(context.Background(), legacy_storage.NameToUid(recv.Name), false, usr) if isVisibleInProvisioning(recv.UID) { require.NoErrorf(t, err, "receiver '%s' should be visible, but isn't", recv.Name) assert.NotNil(t, response) @@ -1842,13 +1839,6 @@ func createEncryptedConfig(t *testing.T, secretService secretService, extraConfi return string(bytes) } -func singleQ(orgID int64, name string) models.GetReceiverQuery { - return models.GetReceiverQuery{ - OrgID: orgID, - Name: name, - } -} - func multiQ(orgID int64, names ...string) models.GetReceiversQuery { return models.GetReceiversQuery{ OrgID: orgID,