diff --git a/pkg/services/accesscontrol/ossaccesscontrol/receivers.go b/pkg/services/accesscontrol/ossaccesscontrol/receivers.go index f7ef1bc3edc..ac5f5f6d33c 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/receivers.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/receivers.go @@ -39,10 +39,6 @@ func ProvideReceiverPermissionsService( license licensing.Licensing, service accesscontrol.Service, teamService team.Service, userService user.Service, actionSetService resourcepermissions.ActionSetService, ) (*ReceiverPermissionsService, error) { - if !features.IsEnabledGlobally(featuremgmt.FlagAlertingApiServer) { - return nil, nil - } - options := resourcepermissions.Options{ Resource: "receivers", ResourceAttribute: "uid", @@ -66,7 +62,7 @@ func ProvideReceiverPermissionsService( if err != nil { return nil, err } - return &ReceiverPermissionsService{srv, service, log.New("resourcepermissions.receivers")}, nil + return &ReceiverPermissionsService{Service: srv, ac: service, log: log.New("resourcepermissions.receivers")}, nil } var _ accesscontrol.ReceiverPermissionsService = new(ReceiverPermissionsService) @@ -79,6 +75,7 @@ type ReceiverPermissionsService struct { // SetDefaultPermissions sets the default permissions for a newly created receiver. func (r ReceiverPermissionsService) SetDefaultPermissions(ctx context.Context, orgID int64, user identity.Requester, uid string) { + r.log.Debug("Setting default permissions for receiver", "receiver_uid", uid) permissions := defaultPermissions() clearCache := false if user != nil && user.IsIdentityType(claims.TypeUser) { @@ -122,6 +119,7 @@ func copyPermissionUser(orgID int64) identity.Requester { // method to be used during receiver renaming that is necessitated by receiver uids being generated from the receiver // name. func (r ReceiverPermissionsService) CopyPermissions(ctx context.Context, orgID int64, user identity.Requester, oldUID, newUID string) (int, error) { + r.log.Debug("Copying permissions from receiver", "old_uid", oldUID, "new_uid", newUID) currentPermissions, err := r.GetPermissions(ctx, copyPermissionUser(orgID), oldUID) if err != nil { return 0, err diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index dc2131ebbdd..082881c1fbb 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -38,7 +38,7 @@ type ProvisioningSrv struct { type ContactPointService interface { GetContactPoints(ctx context.Context, q provisioning.ContactPointQuery, user identity.Requester) ([]definitions.EmbeddedContactPoint, error) - CreateContactPoint(ctx context.Context, orgID int64, contactPoint definitions.EmbeddedContactPoint, p alerting_models.Provenance) (definitions.EmbeddedContactPoint, error) + CreateContactPoint(ctx context.Context, orgID int64, user identity.Requester, contactPoint definitions.EmbeddedContactPoint, p alerting_models.Provenance) (definitions.EmbeddedContactPoint, error) UpdateContactPoint(ctx context.Context, orgID int64, contactPoint definitions.EmbeddedContactPoint, p alerting_models.Provenance) error DeleteContactPoint(ctx context.Context, orgID int64, uid string) error } @@ -165,7 +165,7 @@ func (srv *ProvisioningSrv) RouteGetContactPointsExport(c *contextmodel.ReqConte func (srv *ProvisioningSrv) RoutePostContactPoint(c *contextmodel.ReqContext, cp definitions.EmbeddedContactPoint) response.Response { provenance := determineProvenance(c) - contactPoint, err := srv.contactPointService.CreateContactPoint(c.Req.Context(), c.SignedInUser.GetOrgID(), cp, alerting_models.Provenance(provenance)) + contactPoint, err := srv.contactPointService.CreateContactPoint(c.Req.Context(), c.SignedInUser.GetOrgID(), c.SignedInUser, cp, alerting_models.Provenance(provenance)) if errors.Is(err, provisioning.ErrValidation) { return ErrResp(http.StatusBadRequest, err, "") } diff --git a/pkg/services/ngalert/api/api_provisioning_test.go b/pkg/services/ngalert/api/api_provisioning_test.go index 7832ef59a30..785d433769f 100644 --- a/pkg/services/ngalert/api/api_provisioning_test.go +++ b/pkg/services/ngalert/api/api_provisioning_test.go @@ -1907,7 +1907,7 @@ func createProvisioningSrvSutFromEnv(t *testing.T, env *testEnvironment) Provisi return ProvisioningSrv{ log: env.log, policies: newFakeNotificationPolicyService(), - contactPointService: provisioning.NewContactPointService(configStore, env.secrets, env.prov, env.xact, receiverSvc, env.log, env.store), + contactPointService: provisioning.NewContactPointService(configStore, env.secrets, env.prov, env.xact, receiverSvc, env.log, env.store, ngalertfakes.NewFakeReceiverPermissionsService()), templates: provisioning.NewTemplateService(configStore, env.prov, env.xact, env.log), muteTimings: provisioning.NewMuteTimingService(configStore, env.prov, env.xact, env.log, env.store), alertRules: provisioning.NewAlertRuleService(env.store, env.prov, env.folderService, env.quotas, env.xact, 60, 10, 100, env.log, &provisioning.NotificationSettingsValidatorProviderFake{}, env.rulesAuthz), diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 75f38395233..74ca6331bcd 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -441,7 +441,7 @@ func (ng *AlertNG) init() error { // Provisioning policyService := provisioning.NewNotificationPolicyService(configStore, ng.store, ng.store, ng.Cfg.UnifiedAlerting, ng.Log) - contactPointService := provisioning.NewContactPointService(configStore, ng.SecretsService, ng.store, ng.store, provisioningReceiverService, ng.Log, ng.store) + contactPointService := provisioning.NewContactPointService(configStore, ng.SecretsService, ng.store, ng.store, provisioningReceiverService, ng.Log, ng.store, ng.ResourcePermissions) templateService := provisioning.NewTemplateService(configStore, ng.store, ng.store, ng.Log) muteTimingService := provisioning.NewMuteTimingService(configStore, ng.store, ng.store, ng.Log, ng.store) alertRuleService := provisioning.NewAlertRuleService(ng.store, ng.store, ng.folderService, ng.QuotaService, ng.store, diff --git a/pkg/services/ngalert/notifier/receiver_svc.go b/pkg/services/ngalert/notifier/receiver_svc.go index 2e2bdd99ba8..1ecf12594cc 100644 --- a/pkg/services/ngalert/notifier/receiver_svc.go +++ b/pkg/services/ngalert/notifier/receiver_svc.go @@ -316,11 +316,9 @@ func (rs *ReceiverService) DeleteReceiver(ctx context.Context, uid string, calle if err != nil { return err } - if rs.resourcePermissions != nil { - err = rs.resourcePermissions.DeleteResourcePermissions(ctx, orgID, uid) - if err != nil { - rs.log.Error("Could not delete receiver permissions", "receiver", existing.Name, "error", err) - } + err = rs.resourcePermissions.DeleteResourcePermissions(ctx, orgID, uid) + if err != nil { + rs.log.Error("Could not delete receiver permissions", "receiver", existing.Name, "error", err) } return rs.deleteProvenances(ctx, orgID, existing.Integrations) }) @@ -359,9 +357,7 @@ func (rs *ReceiverService) CreateReceiver(ctx context.Context, r *models.Receive if err != nil { return err } - if rs.resourcePermissions != nil { - rs.resourcePermissions.SetDefaultPermissions(ctx, orgID, user, createdReceiver.GetUID()) - } + rs.resourcePermissions.SetDefaultPermissions(ctx, orgID, user, createdReceiver.GetUID()) return rs.setReceiverProvenance(ctx, orgID, &createdReceiver) }) if err != nil { @@ -439,17 +435,15 @@ func (rs *ReceiverService) UpdateReceiver(ctx context.Context, r *models.Receive return err } // Update receiver permissions - if rs.resourcePermissions != nil { - permissionsUpdated, err := rs.resourcePermissions.CopyPermissions(ctx, orgID, user, legacy_storage.NameToUid(existing.Name), legacy_storage.NameToUid(r.Name)) - if err != nil { - return err - } - if permissionsUpdated > 0 { - rs.log.FromContext(ctx).Debug("Moved custom receiver permissions", "oldName", existing.Name, "newName", r.Name, "count", permissionsUpdated) - } - if err := rs.resourcePermissions.DeleteResourcePermissions(ctx, orgID, legacy_storage.NameToUid(existing.Name)); err != nil { - return err - } + permissionsUpdated, err := rs.resourcePermissions.CopyPermissions(ctx, orgID, user, legacy_storage.NameToUid(existing.Name), legacy_storage.NameToUid(r.Name)) + if err != nil { + return err + } + if permissionsUpdated > 0 { + rs.log.FromContext(ctx).Info("Moved custom receiver permissions", "oldName", existing.Name, "newName", r.Name, "count", permissionsUpdated) + } + if err := rs.resourcePermissions.DeleteResourcePermissions(ctx, orgID, legacy_storage.NameToUid(existing.Name)); err != nil { + return err } } err = rs.cfgStore.Save(ctx, revision, orgID) diff --git a/pkg/services/ngalert/provisioning/contactpoints.go b/pkg/services/ngalert/provisioning/contactpoints.go index 2fad1402fb9..9bde1c86b17 100644 --- a/pkg/services/ngalert/provisioning/contactpoints.go +++ b/pkg/services/ngalert/provisioning/contactpoints.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" 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/channels_config" @@ -37,6 +38,7 @@ type ContactPointService struct { xact TransactionManager receiverService receiverService log log.Logger + resourcePermissions ac.ReceiverPermissionsService } type receiverService interface { @@ -44,9 +46,16 @@ type receiverService interface { RenameReceiverInDependentResources(ctx context.Context, orgID int64, route *apimodels.Route, oldName, newName string, receiverProvenance models.Provenance) error } -func NewContactPointService(store alertmanagerConfigStore, encryptionService secrets.Service, - provenanceStore ProvisioningStore, xact TransactionManager, receiverService receiverService, log log.Logger, - nsStore AlertRuleNotificationSettingsStore) *ContactPointService { +func NewContactPointService( + store alertmanagerConfigStore, + encryptionService secrets.Service, + provenanceStore ProvisioningStore, + xact TransactionManager, + receiverService receiverService, + log log.Logger, + nsStore AlertRuleNotificationSettingsStore, + resourcePermissions ac.ReceiverPermissionsService, +) *ContactPointService { return &ContactPointService{ configStore: store, receiverService: receiverService, @@ -55,6 +64,7 @@ func NewContactPointService(store alertmanagerConfigStore, encryptionService sec xact: xact, log: log, notificationSettingsStore: nsStore, + resourcePermissions: resourcePermissions, } } @@ -134,8 +144,13 @@ func (ecp *ContactPointService) getContactPointDecrypted(ctx context.Context, or return apimodels.EmbeddedContactPoint{}, fmt.Errorf("%w: contact point with uid '%s' not found", ErrNotFound, uid) } -func (ecp *ContactPointService) CreateContactPoint(ctx context.Context, orgID int64, - contactPoint apimodels.EmbeddedContactPoint, provenance models.Provenance) (apimodels.EmbeddedContactPoint, error) { +func (ecp *ContactPointService) CreateContactPoint( + ctx context.Context, + orgID int64, + user identity.Requester, + contactPoint apimodels.EmbeddedContactPoint, + provenance models.Provenance, +) (apimodels.EmbeddedContactPoint, error) { if err := ValidateContactPoint(ctx, contactPoint, ecp.encryptionService.GetDecryptedValue); err != nil { return apimodels.EmbeddedContactPoint{}, fmt.Errorf("%w: %s", ErrValidation, err.Error()) } @@ -210,6 +225,11 @@ func (ecp *ContactPointService) CreateContactPoint(ctx context.Context, orgID in if err := ecp.configStore.Save(ctx, revision, orgID); err != nil { return err } + if !receiverFound { + // Compatibility with new receiver resource permissions. + // Since this is a new receiver, we need to set default resource permissions so that viewers and editors can see and edit it. + ecp.resourcePermissions.SetDefaultPermissions(ctx, orgID, user, legacy_storage.NameToUid(contactPoint.Name)) + } return ecp.provenanceStore.SetProvenance(ctx, &contactPoint, orgID, provenance) }) if err != nil { @@ -285,16 +305,31 @@ func (ecp *ContactPointService) UpdateContactPoint(ctx context.Context, orgID in return err } - configModified, renamedReceiver := stitchReceiver(revision.Config, mergedReceiver) - if !configModified { + oldReceiverName, fullRemoval, newReceiverCreated := stitchReceiver(revision.Config, mergedReceiver) + if oldReceiverName == "" { return fmt.Errorf("contact point with uid '%s' not found", mergedReceiver.UID) } err = ecp.xact.InTransaction(ctx, func(ctx context.Context) error { - if renamedReceiver != "" && renamedReceiver != mergedReceiver.Name { - err := ecp.receiverService.RenameReceiverInDependentResources(ctx, orgID, revision.Config.AlertmanagerConfig.Route, renamedReceiver, mergedReceiver.Name, provenance) - if err != nil { - return err + if mergedReceiver.Name != oldReceiverName { + if newReceiverCreated { + // Copy receiver permissions + permissionsUpdated, err := ecp.resourcePermissions.CopyPermissions(ctx, orgID, nil, legacy_storage.NameToUid(oldReceiverName), legacy_storage.NameToUid(mergedReceiver.Name)) + if err != nil { + return err + } + if permissionsUpdated > 0 { + ecp.log.FromContext(ctx).Debug("Moved custom receiver permissions", "oldName", oldReceiverName, "newName", mergedReceiver.Name, "count", permissionsUpdated) + } + } + + if fullRemoval { + if err := ecp.receiverService.RenameReceiverInDependentResources(ctx, orgID, revision.Config.AlertmanagerConfig.Route, oldReceiverName, mergedReceiver.Name, provenance); err != nil { + return err + } + if err := ecp.resourcePermissions.DeleteResourcePermissions(ctx, orgID, legacy_storage.NameToUid(oldReceiverName)); err != nil { + return err + } } } if err := ecp.configStore.Save(ctx, revision, orgID); err != nil { @@ -352,6 +387,12 @@ func (ecp *ContactPointService) DeleteContactPoint(ctx context.Context, orgID in ecp.log.Error("Cannot delete contact point because it is used in rule's notification settings", "receiverName", name, "rulesUid", strings.Join(uids, ",")) return ErrContactPointUsedInRule.Errorf("") } + + // Compatibility with new receiver resource permissions. + // We need to cleanup resource permissions. + if err := ecp.resourcePermissions.DeleteResourcePermissions(ctx, orgID, legacy_storage.NameToUid(name)); err != nil { + ecp.log.Error("Could not delete receiver permissions", "receiverName", name, "error", err) + } } if err := ecp.configStore.Save(ctx, revision, orgID); err != nil { @@ -398,16 +439,15 @@ func (ecp *ContactPointService) encryptValue(value string) (string, error) { // stitchReceiver modifies a receiver, target, in an alertmanager configStore. It modifies the given configStore in-place. // Returns true if the configStore was altered in any way, and false otherwise. // If integration was moved to another group and it was the last in the previous group, the second parameter contains the name of the old group that is gone -func stitchReceiver(cfg *apimodels.PostableUserConfig, target *apimodels.PostableGrafanaReceiver) (bool, string) { +func stitchReceiver(cfg *apimodels.PostableUserConfig, target *apimodels.PostableGrafanaReceiver) (oldReceiverName string, fullRemoval bool, newReceiverCreated bool) { // Algorithm to fix up receivers. Receivers are very complex and depend heavily on internal consistency. // All receivers in a given receiver group have the same name. We must maintain this across renames. - configModified := false - renamedReceiver := "" groupLoop: for groupIdx, receiverGroup := range cfg.AlertmanagerConfig.Receivers { // Does the current group contain the grafana receiver we're interested in? for i, grafanaReceiver := range receiverGroup.GrafanaManagedReceivers { if grafanaReceiver.UID == target.UID { + oldReceiverName = receiverGroup.Name // If it's a basic field change, simply replace it. Done! // // NOTE: @@ -417,16 +457,13 @@ groupLoop: // Our receiver group fixing logic below will handle it. if grafanaReceiver.Name == target.Name && receiverGroup.Name == grafanaReceiver.Name { receiverGroup.GrafanaManagedReceivers[i] = target - configModified = true break groupLoop } // If we're renaming, we'll need to fix up the macro receiver group for consistency. // Firstly, if we're the only receiver in the group, simply rename the group to match. Done! if len(receiverGroup.GrafanaManagedReceivers) == 1 { - renamedReceiver = receiverGroup.Name // remember the old name of the receiver. - receiverGroup.Name = target.Name - receiverGroup.GrafanaManagedReceivers[i] = target + fullRemoval = true } // Otherwise, we only want to rename the receiver we are touching... NOT all of them. @@ -438,7 +475,6 @@ groupLoop: receiverGroup.GrafanaManagedReceivers = append(receiverGroup.GrafanaManagedReceivers[:i], receiverGroup.GrafanaManagedReceivers[i+1:]...) // Add the modified receiver to the new group... candidateExistingGroup.GrafanaManagedReceivers = append(candidateExistingGroup.GrafanaManagedReceivers, target) - configModified = true // if the old receiver group turns out to be empty. Remove it. if len(receiverGroup.GrafanaManagedReceivers) == 0 { @@ -448,6 +484,14 @@ groupLoop: } } + newReceiverCreated = true + if fullRemoval { + // Since we're going to remove the receiver group anyways, we reuse the old group to retain order. + receiverGroup.Name = target.Name + receiverGroup.GrafanaManagedReceivers[i] = target + break groupLoop + } + // Doesn't exist? Create a new group just for the receiver. newGroup := &apimodels.PostableApiReceiver{ Receiver: config.Receiver{ @@ -462,13 +506,12 @@ groupLoop: cfg.AlertmanagerConfig.Receivers = append(cfg.AlertmanagerConfig.Receivers, newGroup) // Drop it from the old spot. receiverGroup.GrafanaManagedReceivers = append(receiverGroup.GrafanaManagedReceivers[:i], receiverGroup.GrafanaManagedReceivers[i+1:]...) - configModified = true break groupLoop } } } - return configModified, renamedReceiver + return oldReceiverName, fullRemoval, newReceiverCreated } func ValidateContactPoint(ctx context.Context, e apimodels.EmbeddedContactPoint, decryptFunc alertingNotify.GetDecryptedValueFn) error { diff --git a/pkg/services/ngalert/provisioning/contactpoints_test.go b/pkg/services/ngalert/provisioning/contactpoints_test.go index 879a1558b28..045b4030cdf 100644 --- a/pkg/services/ngalert/provisioning/contactpoints_test.go +++ b/pkg/services/ngalert/provisioning/contactpoints_test.go @@ -84,7 +84,7 @@ func TestContactPointService(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) newCp := createTestContactPoint() - _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) cps, err := sut.GetContactPoints(context.Background(), cpsQuery(1), redactedUser) @@ -100,7 +100,7 @@ func TestContactPointService(t *testing.T) { newCp := createTestContactPoint() newCp.UID = customUID - _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), redactedUser) @@ -115,7 +115,7 @@ func TestContactPointService(t *testing.T) { newCp := createTestContactPoint() newCp.UID = customUID - _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.ErrorIs(t, err, ErrValidation) }) @@ -125,10 +125,10 @@ func TestContactPointService(t *testing.T) { newCp := createTestContactPoint() newCp.UID = customUID - _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) - _, err = sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err = sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.Error(t, err) }) @@ -137,7 +137,7 @@ func TestContactPointService(t *testing.T) { newCp := createTestContactPoint() newCp.Type = "" - _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.ErrorIs(t, err, ErrValidation) }) @@ -145,7 +145,7 @@ func TestContactPointService(t *testing.T) { t.Run("update rejects contact points with no settings", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) newCp := createTestContactPoint() - newCp, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + newCp, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) newCp.Settings = nil @@ -157,7 +157,7 @@ func TestContactPointService(t *testing.T) { t.Run("update rejects contact points with no type", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) newCp := createTestContactPoint() - newCp, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + newCp, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) newCp.Type = "" @@ -169,7 +169,7 @@ func TestContactPointService(t *testing.T) { t.Run("update rejects contact points which fail validation after merging", func(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) newCp := createTestContactPoint() - newCp, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + newCp, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) newCp.Settings, _ = simplejson.NewJson([]byte(`{}`)) @@ -190,7 +190,7 @@ func TestContactPointService(t *testing.T) { oldName := newCp.Name newName := "new-name" - newCp, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + newCp, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) newCp.Name = newName @@ -274,7 +274,7 @@ func TestContactPointService(t *testing.T) { sut := createContactPointServiceSut(t, secretsService) newCp := createTestContactPoint() - newCp, err := sut.CreateContactPoint(context.Background(), 1, newCp, test.from) + newCp, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, test.from) require.NoError(t, err) cps, err := sut.GetContactPoints(context.Background(), cpsQueryWithName(1, newCp.Name), redactedUser) @@ -306,7 +306,7 @@ func TestContactPointService(t *testing.T) { require.NoError(t, err) expectedConcurrencyToken := config.ConcurrencyToken - _, err = sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err = sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) intercepted := fakeConfigStore.LastSaveCommand @@ -346,7 +346,7 @@ func TestContactPointService(t *testing.T) { settings, _ := simplejson.NewJson([]byte(tc.settingsJSON)) newCp.Settings = settings - _, err := sut.CreateContactPoint(context.Background(), 1, newCp, models.ProvenanceAPI) + _, err := sut.CreateContactPoint(context.Background(), 1, redactedUser, newCp, models.ProvenanceAPI) require.NoError(t, err) q := cpsQueryWithName(1, newCp.Name) @@ -496,14 +496,16 @@ func createContactPointServiceSutWithConfigStore(t *testing.T, secretService sec fakes.NewFakeReceiverPermissionsService(), ) - return &ContactPointService{ - configStore: legacy_storage.NewAlertmanagerConfigStore(configStore), - provenanceStore: provisioningStore, - receiverService: receiverService, - xact: xact, - encryptionService: secretService, - log: log.NewNopLogger(), - } + return NewContactPointService( + legacy_storage.NewAlertmanagerConfigStore(configStore), + secretService, + provisioningStore, + xact, + receiverService, + log.NewNopLogger(), + nil, + fakes.NewFakeReceiverPermissionsService(), + ) } func createTestContactPoint() definitions.EmbeddedContactPoint { @@ -546,9 +548,10 @@ func TestStitchReceivers(t *testing.T) { name string initial *definitions.PostableUserConfig new *definitions.PostableGrafanaReceiver - expModified bool expCfg definitions.PostableApiAlertingConfig - expRenamedReceiver string + expOldReceiver string + expCreatedReceiver bool + expFullRemoval bool } cases := []testCase{ @@ -557,8 +560,8 @@ func TestStitchReceivers(t *testing.T) { new: &definitions.PostableGrafanaReceiver{ UID: "does not exist", }, - expModified: false, - expCfg: createTestConfigWithReceivers().AlertmanagerConfig, + expOldReceiver: "", + expCfg: createTestConfigWithReceivers().AlertmanagerConfig, }, { name: "matching receiver with unchanged name, replaces", @@ -567,7 +570,7 @@ func TestStitchReceivers(t *testing.T) { Name: "receiver-2", Type: "teams", }, - expModified: true, + expOldReceiver: "receiver-2", expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -628,8 +631,9 @@ func TestStitchReceivers(t *testing.T) { Name: "new-receiver", Type: "slack", }, - expModified: true, - expRenamedReceiver: "receiver-1", + expOldReceiver: "receiver-1", + expCreatedReceiver: true, + expFullRemoval: true, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -690,7 +694,8 @@ func TestStitchReceivers(t *testing.T) { Name: "receiver-1", Type: "slack", }, - expModified: true, + expOldReceiver: "receiver-2", + expCreatedReceiver: false, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -810,7 +815,8 @@ func TestStitchReceivers(t *testing.T) { Name: "receiver-2", Type: "slack", }, - expModified: true, + expOldReceiver: "receiver-1", + expCreatedReceiver: false, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -953,7 +959,8 @@ func TestStitchReceivers(t *testing.T) { Name: "receiver-4", Type: "slack", }, - expModified: true, + expOldReceiver: "receiver-1", + expCreatedReceiver: false, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -1037,7 +1044,8 @@ func TestStitchReceivers(t *testing.T) { Name: "brand-new-group", Type: "opsgenie", }, - expModified: true, + expOldReceiver: "receiver-2", + expCreatedReceiver: true, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -1108,7 +1116,8 @@ func TestStitchReceivers(t *testing.T) { Name: "brand-new-group", Type: "opsgenie", }, - expModified: true, + expOldReceiver: "receiver-2", // Not the inconsistent receiver-3? + expCreatedReceiver: true, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -1230,8 +1239,9 @@ func TestStitchReceivers(t *testing.T) { Name: "receiver-1", Type: "slack", }, - expModified: true, - expRenamedReceiver: "receiver-2", + expOldReceiver: "receiver-2", + expCreatedReceiver: false, + expFullRemoval: true, expCfg: definitions.PostableApiAlertingConfig{ Config: definitions.Config{ Route: &definitions.Route{ @@ -1283,13 +1293,10 @@ func TestStitchReceivers(t *testing.T) { cfg = c.initial } - modified, renamedReceiver := stitchReceiver(cfg, c.new) - if c.expRenamedReceiver != "" { - assert.Equal(t, c.expRenamedReceiver, renamedReceiver) - } else { - assert.Empty(t, renamedReceiver) - } - require.Equal(t, c.expModified, modified) + renamedReceiver, fullRemoval, createdReceiver := stitchReceiver(cfg, c.new) + assert.Equalf(t, c.expOldReceiver, renamedReceiver, "expected old receiver to be %s, got %s", c.expOldReceiver, renamedReceiver) + assert.Equalf(t, c.expFullRemoval, fullRemoval, "expected full removal to be %t, got %t", c.expFullRemoval, fullRemoval) + assert.Equalf(t, c.expCreatedReceiver, createdReceiver, "expected created receiver to be %t, got %t", c.expCreatedReceiver, createdReceiver) require.Equal(t, c.expCfg, cfg.AlertmanagerConfig) }) } diff --git a/pkg/services/provisioning/alerting/contact_point_provisioner.go b/pkg/services/provisioning/alerting/contact_point_provisioner.go index b0fca535d6a..4a0e8bae1b4 100644 --- a/pkg/services/provisioning/alerting/contact_point_provisioner.go +++ b/pkg/services/provisioning/alerting/contact_point_provisioner.go @@ -55,8 +55,13 @@ func (c *defaultContactPointProvisioner) Provision(ctx context.Context, continue outer } } - _, err := c.contactPointService.CreateContactPoint(ctx, contactPointsConfig.OrgID, - contactPoint, models.ProvenanceFile) + _, err := c.contactPointService.CreateContactPoint( + ctx, + contactPointsConfig.OrgID, + provisionerUser(contactPointsConfig.OrgID), + contactPoint, + models.ProvenanceFile, + ) if err != nil { return err } diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 14548a576f2..e2b3fc70333 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -293,7 +293,7 @@ func (ps *ProvisioningServiceImpl) ProvisionAlerting(ctx context.Context) error ps.resourcePermissions, ) contactPointService := provisioning.NewContactPointService(configStore, ps.secretService, - st, ps.SQLStore, receiverSvc, ps.log, &st) + st, ps.SQLStore, receiverSvc, ps.log, &st, ps.resourcePermissions) notificationPolicyService := provisioning.NewNotificationPolicyService(configStore, st, ps.SQLStore, ps.Cfg.UnifiedAlerting, ps.log) mutetimingsService := provisioning.NewMuteTimingService(configStore, st, &st, ps.log, &st)