From 0375647d21517a5dd35c8a0707fc8ce5cfbfd9f5 Mon Sep 17 00:00:00 2001 From: Joseph Phillips Date: Tue, 3 Dec 2024 17:23:44 +0100 Subject: [PATCH] *** WIP. ROLL BACK. *** --- apiserver/common/secrets/drain.go | 30 ++++----- apiserver/facade/interface.go | 4 +- .../facades/agent/secretsdrain/register.go | 4 +- .../facades/agent/secretsmanager/register.go | 4 +- .../facades/agent/secretsmanager/secrets.go | 28 ++++---- .../facades/agent/secretsmanager/service.go | 4 +- apiserver/facades/agent/uniter/register.go | 8 +-- apiserver/facades/agent/uniter/secrets.go | 65 +++++++++---------- apiserver/facades/agent/uniter/uniter.go | 2 +- .../controller/usersecretsdrain/register.go | 4 +- apiserver/root.go | 4 +- core/leadership/interface.go | 3 +- domain/leaseservice.go | 5 +- domain/secret/service/access.go | 16 ++--- domain/secret/service/delete.go | 2 +- domain/secret/service/params.go | 20 +++--- domain/secret/service/service.go | 46 +++++++------ 17 files changed, 125 insertions(+), 124 deletions(-) diff --git a/apiserver/common/secrets/drain.go b/apiserver/common/secrets/drain.go index f289884e7a3e..9755f912139a 100644 --- a/apiserver/common/secrets/drain.go +++ b/apiserver/common/secrets/drain.go @@ -24,7 +24,7 @@ import ( type SecretsDrainAPI struct { authTag names.Tag logger logger.Logger - leadershipChecker leadership.Checker + leadershipEnsurer leadership.Ensurer watcherRegistry facade.WatcherRegistry modelUUID model.UUID @@ -37,7 +37,7 @@ func NewSecretsDrainAPI( authTag names.Tag, authorizer facade.Authorizer, logger logger.Logger, - leadershipChecker leadership.Checker, + leadershipEnsurer leadership.Ensurer, modelUUID model.UUID, secretService SecretService, secretBackendService SecretBackendService, @@ -49,7 +49,7 @@ func NewSecretsDrainAPI( return &SecretsDrainAPI{ authTag: authTag, logger: logger, - leadershipChecker: leadershipChecker, + leadershipEnsurer: leadershipEnsurer, modelUUID: modelUUID, secretService: secretService, secretBackendService: secretBackendService, @@ -130,9 +130,9 @@ func secretAccessorFromTag(authTag names.Tag) (secretservice.SecretAccessor, err } // isLeaderUnit returns true if the authenticated caller is the unit leader of its application. -func isLeaderUnit(authTag names.Tag, leadershipChecker leadership.Checker) (bool, error) { +func isLeaderUnit(authTag names.Tag, leadershipEnsurer leadership.Ensurer) (bool, error) { appName, _ := names.UnitApplication(authTag.Id()) - token := leadershipChecker.LeadershipCheck(appName, authTag.Id()) + token := leadershipEnsurer.LeadershipCheck(appName, authTag.Id()) err := token.Check() if err != nil && !leadership.IsNotLeaderError(err) { return false, errors.Trace(err) @@ -146,7 +146,7 @@ func (s *SecretsDrainAPI) getCharmSecretsToDrain(ctx context.Context) ([]*corese ID: s.authTag.Id(), }} // Unit leaders can also get metadata for secrets owned by the app. - isLeader, err := isLeaderUnit(s.authTag, s.leadershipChecker) + isLeader, err := isLeaderUnit(s.authTag, s.leadershipEnsurer) if err != nil { return nil, errors.Trace(err) } @@ -181,18 +181,18 @@ func (s *SecretsDrainAPI) changeSecretBackendForOne(ctx context.Context, arg par if err != nil { return } - token, err := LeadershipToken(s.authTag, s.leadershipChecker) - if err != nil { - return errors.Trace(err) - } - return s.secretService.ChangeSecretBackend(ctx, uri, arg.Revision, toChangeSecretBackendParams(accessor, token, arg)) + return s.secretService.ChangeSecretBackend( + ctx, uri, arg.Revision, toChangeSecretBackendParams(accessor, s.leadershipEnsurer, arg), + ) } -func toChangeSecretBackendParams(accessor secretservice.SecretAccessor, token leadership.Token, arg params.ChangeSecretBackendArg) secretservice.ChangeSecretBackendParams { +func toChangeSecretBackendParams( + accessor secretservice.SecretAccessor, ensurer leadership.Ensurer, arg params.ChangeSecretBackendArg, +) secretservice.ChangeSecretBackendParams { params := secretservice.ChangeSecretBackendParams{ - LeaderToken: token, - Accessor: accessor, - Data: arg.Content.Data, + LeaderEnsurer: ensurer, + Accessor: accessor, + Data: arg.Content.Data, } if arg.Content.ValueRef != nil { params.ValueRef = &coresecrets.ValueRef{ diff --git a/apiserver/facade/interface.go b/apiserver/facade/interface.go index 2bc5f28e2e8e..494559b1de95 100644 --- a/apiserver/facade/interface.go +++ b/apiserver/facade/interface.go @@ -53,9 +53,9 @@ type LeadershipModelContext interface { // context's model. LeadershipReader() (leadership.Reader, error) - // LeadershipChecker returns a leadership.Checker for this + // LeadershipEnsurer returns a leadership.Ensurer for this // context's model. - LeadershipChecker() (leadership.Checker, error) + LeadershipEnsurer() (leadership.Ensurer, error) // SingularClaimer returns a lease.Claimer for singular leases for // this context's model. diff --git a/apiserver/facades/agent/secretsdrain/register.go b/apiserver/facades/agent/secretsdrain/register.go index 57d49c602a2e..bf4ff73749b2 100644 --- a/apiserver/facades/agent/secretsdrain/register.go +++ b/apiserver/facades/agent/secretsdrain/register.go @@ -28,7 +28,7 @@ func newSecretsDrainAPI(stdCtx context.Context, ctx facade.ModelContext) (*commo if !ctx.Auth().AuthUnitAgent() { return nil, apiservererrors.ErrPerm } - leadershipChecker, err := ctx.LeadershipChecker() + leadershipEnsurer, err := ctx.LeadershipEnsurer() if err != nil { return nil, errors.Trace(err) } @@ -41,7 +41,7 @@ func newSecretsDrainAPI(stdCtx context.Context, ctx facade.ModelContext) (*commo authTag, ctx.Auth(), ctx.Logger().Child("secretsdrain"), - leadershipChecker, + leadershipEnsurer, ctx.ModelUUID(), domainServices.Secret(secretservice.SecretServiceParams{ BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc( diff --git a/apiserver/facades/agent/secretsmanager/register.go b/apiserver/facades/agent/secretsmanager/register.go index bc05b5865642..3b665ca1f8a9 100644 --- a/apiserver/facades/agent/secretsmanager/register.go +++ b/apiserver/facades/agent/secretsmanager/register.go @@ -37,7 +37,7 @@ func NewSecretManagerAPI(stdCtx context.Context, ctx facade.ModelContext) (*Secr return nil, apiservererrors.ErrPerm } domainServices := ctx.DomainServices() - leadershipChecker, err := ctx.LeadershipChecker() + leadershipChecker, err := ctx.LeadershipEnsurer() if err != nil { return nil, errors.Trace(err) } @@ -85,7 +85,7 @@ func NewSecretManagerAPI(stdCtx context.Context, ctx facade.ModelContext) (*Secr return &SecretsManagerAPI{ authTag: ctx.Auth().GetAuthTag(), authorizer: ctx.Auth(), - leadershipChecker: leadershipChecker, + leadershipEnsurer: leadershipChecker, watcherRegistry: ctx.WatcherRegistry(), secretBackendService: backendService, secretService: secretService, diff --git a/apiserver/facades/agent/secretsmanager/secrets.go b/apiserver/facades/agent/secretsmanager/secrets.go index 58dca0c261f2..aa66dd4cb43c 100644 --- a/apiserver/facades/agent/secretsmanager/secrets.go +++ b/apiserver/facades/agent/secretsmanager/secrets.go @@ -39,7 +39,7 @@ type CrossModelSecretsClient interface { // SecretsManagerAPI is the implementation for the SecretsManager facade. type SecretsManagerAPI struct { authorizer facade.Authorizer - leadershipChecker leadership.Checker + leadershipEnsurer leadership.Ensurer secretBackendService SecretBackendService secretService SecretService watcherRegistry facade.WatcherRegistry @@ -87,7 +87,7 @@ func (s *SecretsManagerAPI) getBackendConfigForDrain(ctx context.Context, arg pa Results: make(map[string]params.SecretBackendConfigResult, 1), } appName, _ := names.UnitApplication(s.authTag.Id()) - token := s.leadershipChecker.LeadershipCheck(appName, s.authTag.Id()) + token := s.leadershipEnsurer.LeadershipCheck(appName, s.authTag.Id()) cfgInfo, err := s.secretBackendService.DrainBackendConfigInfo(ctx, secretbackendservice.DrainBackendConfigParams{ GrantedSecretsGetter: s.secretService.ListGrantedSecretsForBackend, LeaderToken: token, @@ -123,7 +123,7 @@ func (s *SecretsManagerAPI) getBackendConfigForDrain(ctx context.Context, arg pa // GetSecretBackendConfig gets the config needed to create a client to secret backends. func (s *SecretsManagerAPI) getSecretBackendConfig(ctx context.Context, backendIDs []string) (map[string]params.SecretBackendConfigResult, string, error) { appName, _ := names.UnitApplication(s.authTag.Id()) - token := s.leadershipChecker.LeadershipCheck(appName, s.authTag.Id()) + token := s.leadershipEnsurer.LeadershipCheck(appName, s.authTag.Id()) cfgInfo, err := s.secretBackendService.BackendConfigInfo(ctx, secretbackendservice.BackendConfigParams{ GrantedSecretsGetter: s.secretService.ListGrantedSecretsForBackend, LeaderToken: token, @@ -268,7 +268,7 @@ func secretOwnersFromAuthTag(authTag names.Tag, leadershipChecker leadership.Che // GetSecretMetadata returns metadata for the caller's secrets. func (s *SecretsManagerAPI) GetSecretMetadata(ctx context.Context) (params.ListSecretResults, error) { var result params.ListSecretResults - owners, err := secretOwnersFromAuthTag(s.authTag, s.leadershipChecker) + owners, err := secretOwnersFromAuthTag(s.authTag, s.leadershipEnsurer) if err != nil { return result, errors.Trace(err) } @@ -494,7 +494,7 @@ func (s *SecretsManagerAPI) GetSecretRevisionContentInfo(ctx context.Context, ar ID: s.authTag.Id(), } appName, _ := names.UnitApplication(s.authTag.Id()) - token := s.leadershipChecker.LeadershipCheck(appName, s.authTag.Id()) + token := s.leadershipEnsurer.LeadershipCheck(appName, s.authTag.Id()) for i, rev := range arg.Revisions { // TODO(wallworld) - if pendingDelete is true, mark the revision for deletion val, valueRef, err := s.secretService.GetSecretValue(ctx, uri, rev, accessor) @@ -555,9 +555,9 @@ func (s *SecretsManagerAPI) getSecretContent(ctx context.Context, arg params.Get } unitName := s.authTag.Id() - appName, _ := names.UnitApplication(unitName) - token := s.leadershipChecker.LeadershipCheck(appName, unitName) - uri, labelToUpdate, err := s.secretService.ProcessCharmSecretConsumerLabel(ctx, unitName, uri, arg.Label, token) + uri, labelToUpdate, err := s.secretService.ProcessCharmSecretConsumerLabel( + ctx, unitName, uri, arg.Label, s.leadershipEnsurer, + ) if err != nil { return nil, nil, false, errors.Trace(err) } @@ -583,6 +583,9 @@ func (s *SecretsManagerAPI) getSecretContent(ctx context.Context, arg params.Get if err != nil || content.ValueRef == nil { return content, nil, false, errors.Trace(err) } + + appName, _ := names.UnitApplication(unitName) + token := s.leadershipEnsurer.LeadershipCheck(appName, unitName) backend, draining, err := s.getBackend(ctx, content.ValueRef.BackendID, accessor, token) return content, backend, draining, errors.Trace(err) } @@ -620,7 +623,7 @@ func (s *SecretsManagerAPI) charmSecretOwnersFromArgs(authTag names.Tag, args pa // Only unit leaders can watch application secrets. if ownerTag.Kind() == names.ApplicationTagKind { appName, _ := names.UnitApplication(authTag.Id()) - token := s.leadershipChecker.LeadershipCheck(appName, authTag.Id()) + token := s.leadershipEnsurer.LeadershipCheck(appName, authTag.Id()) if err := token.Check(); err != nil { return result, errors.Trace(err) } @@ -739,15 +742,12 @@ func (s *SecretsManagerAPI) SecretsRotated(ctx context.Context, args params.Secr if err != nil { return errors.Trace(err) } - unitName := s.authTag.Id() - appName, _ := names.UnitApplication(unitName) - token := s.leadershipChecker.LeadershipCheck(appName, unitName) accessor := secretservice.SecretAccessor{ Kind: secretservice.UnitAccessor, - ID: unitName, + ID: s.authTag.Id(), } return s.secretsTriggers.SecretRotated(ctx, uri, secretservice.SecretRotatedParams{ - LeaderToken: token, + LeaderEnsurer: s.leadershipEnsurer, Accessor: accessor, OriginalRevision: arg.OriginalRevision, Skip: arg.Skip, diff --git a/apiserver/facades/agent/secretsmanager/service.go b/apiserver/facades/agent/secretsmanager/service.go index 916bdd93208d..d5dc5d09ed39 100644 --- a/apiserver/facades/agent/secretsmanager/service.go +++ b/apiserver/facades/agent/secretsmanager/service.go @@ -5,8 +5,8 @@ package secretsmanager import ( "context" - "github.com/juju/juju/core/leadership" + "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/watcher" secretservice "github.com/juju/juju/domain/secret/service" @@ -42,7 +42,7 @@ type SecretService interface { GetSecretValue(context.Context, *secrets.URI, int, secretservice.SecretAccessor) (secrets.SecretValue, *secrets.ValueRef, error) ListCharmSecrets(context.Context, ...secretservice.CharmSecretOwner) ([]*secrets.SecretMetadata, [][]*secrets.SecretRevisionMetadata, error) ProcessCharmSecretConsumerLabel( - ctx context.Context, unitName string, uri *secrets.URI, label string, token leadership.Token, + ctx context.Context, unitName string, uri *secrets.URI, label string, ensurer leadership.Ensurer, ) (*secrets.URI, *string, error) ChangeSecretBackend(ctx context.Context, uri *secrets.URI, revision int, params secretservice.ChangeSecretBackendParams) error GetSecretGrants(ctx context.Context, uri *secrets.URI, role secrets.SecretRole) ([]secretservice.SecretAccess, error) diff --git a/apiserver/facades/agent/uniter/register.go b/apiserver/facades/agent/uniter/register.go index ee671b555c4c..c05ffefea6f5 100644 --- a/apiserver/facades/agent/uniter/register.go +++ b/apiserver/facades/agent/uniter/register.go @@ -81,7 +81,7 @@ func newUniterAPIWithServices( aClock := context.StatePool().Clock() resources := context.Resources() watcherRegistry := context.WatcherRegistry() - leadershipChecker, err := context.LeadershipChecker() + leadershipEnsurer, err := context.LeadershipEnsurer() if err != nil { return nil, errors.Trace(err) } @@ -136,11 +136,11 @@ func newUniterAPIWithServices( ModelConfigWatcher: common.NewModelConfigWatcher(modelConfigService, context.WatcherRegistry()), RebootRequester: common.NewRebootRequester(machineService, accessMachine), UnitStateAPI: common.NewExternalUnitStateAPI(controllerConfigService, unitStateService, st, resources, authorizer, accessUnit, logger), - LeadershipSettingsAccessor: leadershipSettingsAccessorFactory(st, leadershipChecker, resources, authorizer), + LeadershipSettingsAccessor: leadershipSettingsAccessorFactory(st, leadershipEnsurer, resources, authorizer), lxdProfileAPI: NewExternalLXDProfileAPIv2(st, machineService, context.WatcherRegistry(), authorizer, accessUnit, logger, modelInfoService), // TODO(fwereade): so *every* unit should be allowed to get/set its // own status *and* its application's? This is not a pleasing arrangement. - StatusAPI: NewStatusAPI(m, accessUnitOrApplication, leadershipChecker), + StatusAPI: NewStatusAPI(m, accessUnitOrApplication, leadershipEnsurer), m: m, st: st, @@ -158,7 +158,7 @@ func newUniterAPIWithServices( clock: aClock, auth: authorizer, resources: resources, - leadershipChecker: leadershipChecker, + leadershipEnsurer: leadershipEnsurer, leadershipRevoker: leadershipRevoker, accessUnit: accessUnit, accessApplication: accessApplication, diff --git a/apiserver/facades/agent/uniter/secrets.go b/apiserver/facades/agent/uniter/secrets.go index 91825d87c132..f5c61ce0afa4 100644 --- a/apiserver/facades/agent/uniter/secrets.go +++ b/apiserver/facades/agent/uniter/secrets.go @@ -64,8 +64,6 @@ func (u *UniterAPI) createSecret(ctx context.Context, arg params.CreateSecretArg return "", apiServerErrors.ErrPerm } - appName, _ := names.UnitApplication(authTag.Id()) - token := u.leadershipChecker.LeadershipCheck(appName, authTag.Id()) var uri *coresecrets.URI if arg.URI != nil { uri, err = coresecrets.ParseURI(*arg.URI) @@ -76,10 +74,13 @@ func (u *UniterAPI) createSecret(ctx context.Context, arg params.CreateSecretArg uri = coresecrets.NewURI() } + accessor := secretservice.SecretAccessor{ + Kind: names.UnitTagKind, + ID: authTag.Id(), + } params := secretservice.CreateCharmSecretParams{ - Version: secrets.Version, - // Secret accessor not needed when creating a secret since we are using the secret owner. - UpdateCharmSecretParams: fromUpsertParams(arg.UpsertSecretArg, secretservice.SecretAccessor{}, token), + Version: secrets.Version, + UpdateCharmSecretParams: fromUpsertParams(arg.UpsertSecretArg, accessor, u.leadershipEnsurer), } switch kind := secretOwner.Kind(); kind { case names.UnitTagKind: @@ -96,7 +97,9 @@ func (u *UniterAPI) createSecret(ctx context.Context, arg params.CreateSecretArg return uri.String(), nil } -func fromUpsertParams(p params.UpsertSecretArg, accessor secretservice.SecretAccessor, token leadership.Token) secretservice.UpdateCharmSecretParams { +func fromUpsertParams( + p params.UpsertSecretArg, accessor secretservice.SecretAccessor, ensurer leadership.Ensurer, +) secretservice.UpdateCharmSecretParams { var valueRef *coresecrets.ValueRef if p.Content.ValueRef != nil { valueRef = &coresecrets.ValueRef{ @@ -105,16 +108,16 @@ func fromUpsertParams(p params.UpsertSecretArg, accessor secretservice.SecretAcc } } return secretservice.UpdateCharmSecretParams{ - LeaderToken: token, - Accessor: accessor, - RotatePolicy: p.RotatePolicy, - ExpireTime: p.ExpireTime, - Description: p.Description, - Label: p.Label, - Params: p.Params, - Data: p.Content.Data, - ValueRef: valueRef, - Checksum: p.Content.Checksum, + LeaderEnsurer: ensurer, + Accessor: accessor, + RotatePolicy: p.RotatePolicy, + ExpireTime: p.ExpireTime, + Description: p.Description, + Label: p.Label, + Params: p.Params, + Data: p.Content.Data, + ValueRef: valueRef, + Checksum: p.Content.Checksum, } } @@ -143,14 +146,11 @@ func (u *UniterAPI) updateSecret(ctx context.Context, arg params.UpdateSecretArg return errors.New("at least one attribute to update must be specified") } - authTag := u.auth.GetAuthTag() accessor := secretservice.SecretAccessor{ Kind: secretservice.UnitAccessor, - ID: authTag.Id(), + ID: u.auth.GetAuthTag().Id(), } - appName, _ := names.UnitApplication(authTag.Id()) - token := u.leadershipChecker.LeadershipCheck(appName, authTag.Id()) - err = u.secretService.UpdateCharmSecret(ctx, uri, fromUpsertParams(arg.UpsertSecretArg, accessor, token)) + err = u.secretService.UpdateCharmSecret(ctx, uri, fromUpsertParams(arg.UpsertSecretArg, accessor, u.leadershipEnsurer)) return errors.Trace(err) } @@ -164,13 +164,10 @@ func (u *UniterAPI) removeSecrets(ctx context.Context, args params.DeleteSecretA return result, nil } - authTag := u.auth.GetAuthTag() accessor := secretservice.SecretAccessor{ Kind: secretservice.UnitAccessor, - ID: authTag.Id(), + ID: u.auth.GetAuthTag().Id(), } - appName, _ := names.UnitApplication(authTag.Id()) - token := u.leadershipChecker.LeadershipCheck(appName, authTag.Id()) for i, arg := range args.Args { uri, err := coresecrets.ParseURI(arg.URI) if err != nil { @@ -178,9 +175,9 @@ func (u *UniterAPI) removeSecrets(ctx context.Context, args params.DeleteSecretA continue } p := secretservice.DeleteSecretParams{ - LeaderToken: token, - Accessor: accessor, - Revisions: arg.Revisions, + LeaderEnsurer: u.leadershipEnsurer, + Accessor: accessor, + Revisions: arg.Revisions, } err = u.secretService.DeleteSecret(ctx, uri, p) if err != nil { @@ -289,8 +286,6 @@ func (u *UniterAPI) secretsGrantRevoke(ctx context.Context, args params.GrantRev Kind: secretservice.UnitAccessor, ID: authTag.Id(), } - appName, _ := names.UnitApplication(authTag.Id()) - token := u.leadershipChecker.LeadershipCheck(appName, authTag.Id()) for _, tagStr := range arg.SubjectTags { subjectTag, err := names.ParseTag(tagStr) @@ -302,11 +297,11 @@ func (u *UniterAPI) secretsGrantRevoke(ctx context.Context, args params.GrantRev return errors.Trace(err) } if err := op(ctx, uri, secretservice.SecretAccessParams{ - LeaderToken: token, - Accessor: accessor, - Scope: scope, - Subject: subject, - Role: role, + LeaderEnsurer: u.leadershipEnsurer, + Accessor: accessor, + Scope: scope, + Subject: subject, + Role: role, }); err != nil { return errors.Annotatef(err, "cannot change access to %q for %q", uri, tagStr) } diff --git a/apiserver/facades/agent/uniter/uniter.go b/apiserver/facades/agent/uniter/uniter.go index 854e63e1976e..ca527d83493f 100644 --- a/apiserver/facades/agent/uniter/uniter.go +++ b/apiserver/facades/agent/uniter/uniter.go @@ -59,7 +59,7 @@ type UniterAPI struct { clock clock.Clock auth facade.Authorizer resources facade.Resources - leadershipChecker leadership.Checker + leadershipEnsurer leadership.Ensurer leadershipRevoker leadership.Revoker accessUnit common.GetAuthFunc accessApplication common.GetAuthFunc diff --git a/apiserver/facades/controller/usersecretsdrain/register.go b/apiserver/facades/controller/usersecretsdrain/register.go index 79e535cf5db0..a5aba86b2e8f 100644 --- a/apiserver/facades/controller/usersecretsdrain/register.go +++ b/apiserver/facades/controller/usersecretsdrain/register.go @@ -28,7 +28,7 @@ func newUserSecretsDrainAPI(stdCtx context.Context, ctx facade.ModelContext) (*S if !ctx.Auth().AuthController() { return nil, apiservererrors.ErrPerm } - leadershipChecker, err := ctx.LeadershipChecker() + leadershipEnsurer, err := ctx.LeadershipEnsurer() if err != nil { return nil, errors.Trace(err) } @@ -52,7 +52,7 @@ func newUserSecretsDrainAPI(stdCtx context.Context, ctx facade.ModelContext) (*S authTag, ctx.Auth(), ctx.Logger().Child("usersecretsdrain"), - leadershipChecker, + leadershipEnsurer, ctx.ModelUUID(), secretService, backendService, diff --git a/apiserver/root.go b/apiserver/root.go index 6cd57ad40c4b..f951929d579d 100644 --- a/apiserver/root.go +++ b/apiserver/root.go @@ -814,8 +814,8 @@ func (ctx *facadeContext) RequestRecorder() facade.RequestRecorder { return ctx.r.requestRecorder } -// LeadershipChecker is part of the facade.ModelContext interface. -func (ctx *facadeContext) LeadershipChecker() (leadership.Checker, error) { +// LeadershipEnsurer is part of the facade.ModelContext interface. +func (ctx *facadeContext) LeadershipEnsurer() (leadership.Ensurer, error) { checker, err := ctx.r.shared.leaseManager.Checker( lease.ApplicationLeadershipNamespace, ctx.ModelUUID().String(), diff --git a/core/leadership/interface.go b/core/leadership/interface.go index 35ed93def01a..043c71cea1ae 100644 --- a/core/leadership/interface.go +++ b/core/leadership/interface.go @@ -106,7 +106,6 @@ type Token interface { // Checker exposes leadership checking capabilities. type Checker interface { - // LeadershipCheck returns a Token representing the supplied unit's // application leadership. The existence of the token does not imply // its accuracy; you need to Check() it. @@ -120,6 +119,8 @@ type Checker interface { // Ensurer describes the ability to guarantee leadership // for the duration of some operation. type Ensurer interface { + Checker + // WithLeader ensures that the input unit, holds leadership of the input // application for the duration of execution of the input function. WithLeader(ctx context.Context, appName, unitName string, fn func(context.Context) error) error diff --git a/domain/leaseservice.go b/domain/leaseservice.go index c46e9faeae38..4ea25b53d619 100644 --- a/domain/leaseservice.go +++ b/domain/leaseservice.go @@ -43,7 +43,7 @@ func (s *LeaseService) WithLeader( } // The leaseCtx will be cancelled when the lease is no longer held by the - // lease holder. This may or may not be the same as the holderName for the + // leaseholder. This may or may not be the same as the holderName for the // lease. That check is done by the Token checker. leaseCtx, leaseCancel := context.WithCancel(ctx) defer leaseCancel() @@ -105,8 +105,7 @@ func (s *LeaseService) WithLeader( // Ensure that the lease is held by the holder before proceeding. // We're guaranteed that the lease is held by the holder, otherwise the // context will have been cancelled. - token := leaseChecker.Token(appName, unitName) - if err := token.Check(); err != nil { + if err := leaseChecker.Token(appName, unitName).Check(); err != nil { return internalerrors.Errorf("checking lease token: %w", err) } diff --git a/domain/secret/service/access.go b/domain/secret/service/access.go index a0753404646c..3cc1c67a7379 100644 --- a/domain/secret/service/access.go +++ b/domain/secret/service/access.go @@ -129,7 +129,7 @@ func (s *SecretService) getSecretAccess(ctx context.Context, uri *secrets.URI, a // satisfying [secreterrors.InvalidSecretPermissionChange] is returned. // It returns [secreterrors.PermissionDenied] if the secret cannot be managed by the accessor. func (s *SecretService) GrantSecretAccess(ctx context.Context, uri *secrets.URI, params SecretAccessParams) error { - withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken) + withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderEnsurer) if err != nil { return errors.Trace(err) } @@ -172,7 +172,7 @@ func grantParams(in SecretAccessParams) domainsecret.GrantParams { // RevokeSecretAccess revokes access to the secret for the specified subject. // It returns an error satisfying [secreterrors.SecretNotFound] if the secret is not found. func (s *SecretService) RevokeSecretAccess(ctx context.Context, uri *secrets.URI, params SecretAccessParams) error { - withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken) + withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderEnsurer) if err != nil { return errors.Trace(err) } @@ -205,7 +205,7 @@ func (s *SecretService) RevokeSecretAccess(ctx context.Context, uri *secrets.URI // If the caveat can never be satisfied, an error is returned - the input // accessor can never manage the input secret. func (s *SecretService) getManagementCaveat( - ctx context.Context, uri *secrets.URI, accessor SecretAccessor, leaderToken leadership.Token, + ctx context.Context, uri *secrets.URI, accessor SecretAccessor, ensurer leadership.Ensurer, ) (func(context.Context, func(context.Context) error) error, error) { hasRole, err := s.getSecretAccess(ctx, uri, accessor) if err != nil { @@ -219,14 +219,12 @@ func (s *SecretService) getManagementCaveat( } // Units can manage app owned secrets if they are the leader. if accessor.Kind == UnitAccessor { - if leaderToken == nil { + if ensurer == nil { return nil, secreterrors.PermissionDenied } - // TODO (manadart 2024-11-29): This section will be updated to - // return a WithLease wrapper in a patch to follow. - if err := leaderToken.Check(); err == nil { - appName, _ := names.UnitApplication(accessor.ID) + appName, _ := names.UnitApplication(accessor.ID) + if err := ensurer.LeadershipCheck(appName, accessor.ID).Check(); err == nil { hasRole, err = s.getSecretAccess(ctx, uri, SecretAccessor{ Kind: ApplicationAccessor, ID: appName, @@ -238,7 +236,7 @@ func (s *SecretService) getManagementCaveat( if hasRole.Allowed(secrets.RoleManage) { return func(ctx context.Context, fn func(context.Context) error) error { - return fn(ctx) + return ensurer.WithLeader(ctx, appName, accessor.ID, fn) }, nil } } diff --git a/domain/secret/service/delete.go b/domain/secret/service/delete.go index deaa0c5c1408..253a1f84cee2 100644 --- a/domain/secret/service/delete.go +++ b/domain/secret/service/delete.go @@ -28,7 +28,7 @@ func (s *SecretService) DeleteObsoleteUserSecretRevisions(ctx context.Context) e // If revisions is nil or the last remaining revisions are removed. // It returns [secreterrors.PermissionDenied] if the secret cannot be managed by the accessor. func (s *SecretService) DeleteSecret(ctx context.Context, uri *secrets.URI, params DeleteSecretParams) error { - withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken) + withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderEnsurer) if err != nil { return errors.Capture(err) } diff --git a/domain/secret/service/params.go b/domain/secret/service/params.go index ab6b900ae1fc..6f8480e46588 100644 --- a/domain/secret/service/params.go +++ b/domain/secret/service/params.go @@ -28,8 +28,8 @@ type CreateCharmSecretParams struct { // UpdateCharmSecretParams are used to update a charm secret. type UpdateCharmSecretParams struct { - LeaderToken leadership.Token - Accessor SecretAccessor + LeaderEnsurer leadership.Ensurer + Accessor SecretAccessor RotatePolicy *secrets.RotatePolicy ExpireTime *time.Time @@ -61,16 +61,16 @@ type UpdateUserSecretParams struct { // DeleteSecretParams are used to delete a secret. type DeleteSecretParams struct { - LeaderToken leadership.Token - Accessor SecretAccessor + LeaderEnsurer leadership.Ensurer + Accessor SecretAccessor Revisions []int } // SecretRotatedParams are used to mark a secret as rotated. type SecretRotatedParams struct { - LeaderToken leadership.Token - Accessor SecretAccessor + LeaderEnsurer leadership.Ensurer + Accessor SecretAccessor OriginalRevision int Skip bool @@ -78,8 +78,8 @@ type SecretRotatedParams struct { // SecretAccessParams are used to define access to a secret. type SecretAccessParams struct { - LeaderToken leadership.Token - Accessor SecretAccessor + LeaderEnsurer leadership.Ensurer + Accessor SecretAccessor Scope SecretAccessScope Subject SecretAccessor @@ -88,8 +88,8 @@ type SecretAccessParams struct { // ChangeSecretBackendParams are used to change the backend of a secret. type ChangeSecretBackendParams struct { - LeaderToken leadership.Token - Accessor SecretAccessor + LeaderEnsurer leadership.Ensurer + Accessor SecretAccessor ValueRef *secrets.ValueRef Data secrets.SecretData diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index a8464077214f..5dd7a3d23d67 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -250,9 +250,9 @@ func ptr[T any](s T) *T { return &s } -// CreateCharmSecret creates a charm secret with the specified parameters, returning an error -// satisfying [secreterrors.SecretLabelAlreadyExists] if the secret owner already has -// a secret with the same label. +// CreateCharmSecret creates a charm secret with the specified parameters, +// returning an error satisfying [secreterrors.SecretLabelAlreadyExists] if the +// secret owner already has a secret with the same label. func (s *SecretService) CreateCharmSecret(ctx context.Context, uri *secrets.URI, params CreateCharmSecretParams) (errOut error) { if len(params.Data) > 0 && params.ValueRef != nil { return jujuerrors.New("must specify either content or a value reference but not both") @@ -301,10 +301,12 @@ func (s *SecretService) CreateCharmSecret(ctx context.Context, uri *secrets.URI, }() if params.CharmOwner.Kind == ApplicationOwner { // Only unit leaders can create application secrets. - if params.LeaderToken == nil { + if params.LeaderEnsurer == nil { return secreterrors.PermissionDenied } - if err := params.LeaderToken.Check(); err != nil { + + appName, _ := names.UnitApplication(params.Accessor.ID) + if err := params.LeaderEnsurer.LeadershipCheck(appName, params.Accessor.ID).Check(); err != nil { if leadership.IsNotLeaderError(err) { return secreterrors.PermissionDenied } @@ -433,7 +435,7 @@ func (s *SecretService) UpdateCharmSecret(ctx context.Context, uri *secrets.URI, return jujuerrors.New("must specify either content or a value reference but not both") } - withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken) + withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderEnsurer) if err != nil { return errors.Capture(err) } @@ -720,7 +722,7 @@ func (s *SecretService) GetSecretContentFromBackend(ctx context.Context, uri *se // This method returns the resulting uri, and optionally the label to update for // the consumer. func (s *SecretService) ProcessCharmSecretConsumerLabel( - ctx context.Context, unitName string, uri *secrets.URI, label string, token leadership.Token, + ctx context.Context, unitName string, uri *secrets.URI, label string, ensurer leadership.Ensurer, ) (_ *secrets.URI, _ *string, err error) { modelUUID, err := s.secretState.GetModelUUID(ctx) if err != nil { @@ -743,7 +745,7 @@ func (s *SecretService) ProcessCharmSecretConsumerLabel( // If the label has is to be changed by the secret owner, update the secret metadata. // TODO(wallyworld) - the label staying the same should be asserted in a txn. if labelToUpdate != nil && *labelToUpdate != md.Label { - isOwner, err := checkUnitOwner(unitName, md.Owner, token) + isOwner, err := checkUnitOwner(unitName, md.Owner, ensurer) if err != nil { return nil, nil, jujuerrors.Trace(err) } @@ -754,8 +756,8 @@ func (s *SecretService) ProcessCharmSecretConsumerLabel( // can ge done in a single txn. // Update the label. err := s.UpdateCharmSecret(ctx, uri, UpdateCharmSecretParams{ - LeaderToken: token, - Label: &label, + LeaderEnsurer: ensurer, + Label: &label, Accessor: SecretAccessor{ Kind: UnitAccessor, ID: unitName, @@ -767,12 +769,15 @@ func (s *SecretService) ProcessCharmSecretConsumerLabel( } } // 1. secrets can be accessed by the owner; - // 2. application owned secrets can be accessed by all the units of the application using owner label or URI. + // 2. application owned secrets can be accessed by all the units of + // the application using owner label or URI. uri = md.URI - // We don't update the consumer label in this case since the label comes - // from the owner metadata and we don't want to violate uniqueness checks. + // We don't update the consumer label in this case since the label + // comes from the owner metadata, and we don't want to violate + // uniqueness checks. // 1. owners use owner label; - // 2. the leader and peer units use the owner label for application-owned secrets. + // 2. the leader and peer units use the owner label for + // application-owned secrets. // So, no need to update the consumer label. labelToUpdate = nil } @@ -791,15 +796,18 @@ func (s *SecretService) ProcessCharmSecretConsumerLabel( return uri, labelToUpdate, nil } -func checkUnitOwner(unitName string, owner secrets.Owner, token leadership.Token) (bool, error) { +func checkUnitOwner(unitName string, owner secrets.Owner, ensurer leadership.Ensurer) (bool, error) { if owner.Kind == secrets.UnitOwner && owner.ID == unitName { return true, nil } + // Only unit leaders can "own" application secrets. - if token == nil { + if ensurer == nil { return false, secreterrors.PermissionDenied } - if err := token.Check(); err != nil { + + appName, _ := names.UnitApplication(owner.ID) + if err := ensurer.LeadershipCheck(appName, owner.ID).Check(); err != nil { if leadership.IsNotLeaderError(err) { return false, nil } @@ -848,7 +856,7 @@ func (s *SecretService) getAppOwnedOrUnitOwnedSecretMetadata(ctx context.Context func (s *SecretService) ChangeSecretBackend( ctx context.Context, uri *secrets.URI, revision int, params ChangeSecretBackendParams, ) error { - withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken) + withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderEnsurer) if err != nil { return errors.Capture(err) } @@ -892,7 +900,7 @@ func (s *SecretService) ChangeSecretBackend( // SecretRotated rotates the secret with the specified URI. func (s *SecretService) SecretRotated(ctx context.Context, uri *secrets.URI, params SecretRotatedParams) error { - withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderToken) + withCaveat, err := s.getManagementCaveat(ctx, uri, params.Accessor, params.LeaderEnsurer) if err != nil { return errors.Capture(err) }