Skip to content

Commit

Permalink
Merge pull request juju#18380 from manadart/dqlite-remove-BackendAdmi…
Browse files Browse the repository at this point in the history
…nConfigGetter

juju#18380

The `Secret` method on the model service factory is the only one that takes any arguments. 

This argument is populated with two _getter_ functions backed by the secret back-end service. The back-end service is backed by the controller database, whereas the service acquired is model-database-backed.

This is an architectural mistake in that:
- A consumer of the service needs to know internal details:
 - that a back-end service is required; and
 - special typed getters must be created using this service.
- The mistake is compounded by the addition of unimplemented manifestations of the getters, the marathon `NotImplementedBackendUserSecretConfigGetter` being an example - we burden the consumer further by requiring knowledge of _when_ the real or unimplemented arguments are actually needed.

And in the end the argumentation is needless, because we pass a secret back-end state to the secret service anyway!

So in this patch, we demonstrate how to remove one of the service parameters, by calling though to the secret-backend state directly for the data required in the secret service.

Other miscellaneous housekeeping is included:
- Removal of redundant or unused assets.
- File renaming to better indicate contents.
- Usage of _internal/errors_ for new code.
- Sensible line lengths. 

## QA steps

`cd tests && ./main secrets_iaas`

## Documentation changes

None.

## Links

<!-- Link to all relevant specification, documentation, bug, issue or JIRA card. -->

**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/

**Jira card:** [JUJU-7175](https://warthogs.atlassian.net/browse/JUJU-7175)

[JUJU-7175]: https://warthogs.atlassian.net/browse/JUJU-7175?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
jujubot authored Nov 26, 2024
2 parents f811616 + 5224dfe commit 46b75e7
Show file tree
Hide file tree
Showing 31 changed files with 793 additions and 722 deletions.
3 changes: 0 additions & 3 deletions apiserver/facades/agent/secretsdrain/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ func newSecretsDrainAPI(stdCtx context.Context, ctx facade.ModelContext) (*commo
leadershipChecker,
ctx.ModelUUID(),
domainServices.Secret(secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
Expand Down
3 changes: 0 additions & 3 deletions apiserver/facades/agent/secretsmanager/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ func NewSecretManagerAPI(stdCtx context.Context, ctx facade.ModelContext) (*Secr
backendService := domainServices.SecretBackend()
secretService := domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
Expand Down
3 changes: 0 additions & 3 deletions apiserver/facades/agent/uniter/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ func newUniterAPI(stdCtx context.Context, ctx facade.ModelContext) (*UniterAPI,
backendService := domainServices.SecretBackend()
secretService := domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
Expand Down
1 change: 0 additions & 1 deletion apiserver/facades/agent/uniter/uniter_apierror_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func (s *uniterAPIErrorSuite) TestGetStorageStateError(c *gc.C) {
domainServices.ModelInfo(),
domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretservice.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: secretservice.NotImplementedBackendUserSecretConfigGetter,
},
),
Expand Down
2 changes: 0 additions & 2 deletions apiserver/facades/agent/uniter/uniter_cloudspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ func (s *cloudSpecUniterSuite) TestGetCloudSpecReturnsSpecWhenTrusted(c *gc.C) {
domainServices.ModelInfo(),
domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretservice.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: secretservice.NotImplementedBackendUserSecretConfigGetter,
},
),
Expand Down Expand Up @@ -89,7 +88,6 @@ func (s *cloudSpecUniterSuite) TestCloudAPIVersion(c *gc.C) {
domainServices.ModelInfo(),
domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretservice.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: secretservice.NotImplementedBackendUserSecretConfigGetter,
},
),
Expand Down
3 changes: 0 additions & 3 deletions apiserver/facades/client/secrets/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ func newSecretsAPI(stdCtx context.Context, ctx facade.ModelContext) (*SecretsAPI

secretService := domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func makeStateCrossModelRelationsAPI(stdCtx context.Context, ctx facade.ModelCon
authCtxt.(*commoncrossmodel.AuthContext),
ctx.DomainServices().Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretservice.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: secretservice.NotImplementedBackendUserSecretConfigGetter,
},
),
Expand Down
3 changes: 0 additions & 3 deletions apiserver/facades/controller/crossmodelsecrets/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ func makeStateCrossModelSecretsAPI(stdCtx context.Context, ctx facade.MultiModel
secretInfoGetter := func(modelUUID coremodel.UUID) SecretService {
return ctx.DomainServicesForModel(modelUUID).Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
domainServices.SecretBackend(), modelUUID,
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
domainServices.SecretBackend(), modelUUID,
),
Expand Down
1 change: 0 additions & 1 deletion apiserver/facades/controller/remoterelations/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func makeAPI(stdCtx context.Context, ctx facade.ModelContext) (*API, error) {
externalControllerService,
domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretservice.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: secretservice.NotImplementedBackendUserSecretConfigGetter,
},
),
Expand Down
3 changes: 0 additions & 3 deletions apiserver/facades/controller/usersecrets/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ func NewUserSecretsManager(stdCtx stdcontext.Context, ctx facade.ModelContext) (
watcherRegistry: ctx.WatcherRegistry(),
secretService: domainServices.Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
Expand Down
3 changes: 0 additions & 3 deletions apiserver/facades/controller/usersecretsdrain/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ func newUserSecretsDrainAPI(stdCtx context.Context, ctx facade.ModelContext) (*S

secretService := ctx.DomainServices().Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretbackendservice.AdminBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
BackendUserSecretConfigGetter: secretbackendservice.UserSecretBackendConfigGetterFunc(
backendService, ctx.ModelUUID(),
),
Expand Down
1 change: 0 additions & 1 deletion apiserver/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,6 @@ func newSecretsRevisionWatcher(_ context.Context, context facade.ModelContext) (
watcherCommon: newWatcherCommon(context),
secretService: context.DomainServices().Secret(
secretservice.SecretServiceParams{
BackendAdminConfigGetter: secretservice.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: secretservice.NotImplementedBackendUserSecretConfigGetter,
},
),
Expand Down
242 changes: 242 additions & 0 deletions domain/secret/backend_mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion domain/secret/modelmigration/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func (e *exportOperation) Setup(scope modelmigration.Scope) error {
secretbackendstate.NewState(scope.ControllerDB(), e.logger),
e.logger,
service.SecretServiceParams{
BackendAdminConfigGetter: service.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: service.NotImplementedBackendUserSecretConfigGetter,
},
)
Expand Down
1 change: 0 additions & 1 deletion domain/secret/modelmigration/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ func (i *importOperation) Setup(scope modelmigration.Scope) error {
state.NewState(scope.ModelDB(), i.logger),
backendstate, i.logger,
service.SecretServiceParams{
BackendAdminConfigGetter: service.NotImplementedBackendConfigGetter,
BackendUserSecretConfigGetter: service.NotImplementedBackendUserSecretConfigGetter,
},
)
Expand Down
2 changes: 1 addition & 1 deletion domain/secret/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
gc "gopkg.in/check.v1"
)

//go:generate go run go.uber.org/mock/mockgen -typed -package secret -destination refcount_mock_test.go github.com/juju/juju/domain/secret/service SecretBackendReferenceMutator
//go:generate go run go.uber.org/mock/mockgen -typed -package secret -destination backend_mock_test.go github.com/juju/juju/domain/secret/service SecretBackendState

func TestPackage(t *testing.T) {
gc.TestingT(t)
Expand Down
Loading

0 comments on commit 46b75e7

Please sign in to comment.