From 687408f6c752733f482ef89b31064a49a11736a8 Mon Sep 17 00:00:00 2001 From: TonyAdo <71679464+adohe@users.noreply.github.com> Date: Mon, 8 Jan 2024 11:39:28 +0800 Subject: [PATCH] refactor: rename SecretStoreFactory to SecretStoreProvider (#738) --- pkg/secrets/interfaces.go | 4 ++-- pkg/secrets/providers.go | 16 ++++++++-------- .../alicloud/secretsmanager/secretsmanager.go | 12 ++++++------ .../secretsmanager/secretsmanager_test.go | 2 +- .../aws/secretsmanager/secretsmanager.go | 10 +++++----- .../aws/secretsmanager/secretsmanager_test.go | 2 +- pkg/secrets/providers/azure/keyvault/keyvault.go | 8 ++++---- .../providers/azure/keyvault/keyvault_test.go | 2 +- pkg/secrets/providers/hashivault/vault.go | 14 +++++--------- pkg/secrets/providers/hashivault/vault_test.go | 2 +- pkg/secrets/providers_test.go | 10 +++++----- 11 files changed, 39 insertions(+), 43 deletions(-) diff --git a/pkg/secrets/interfaces.go b/pkg/secrets/interfaces.go index da42e9f3..1ea14394 100644 --- a/pkg/secrets/interfaces.go +++ b/pkg/secrets/interfaces.go @@ -12,8 +12,8 @@ type SecretStore interface { GetSecret(ctx context.Context, ref v1.ExternalSecretRef) ([]byte, error) } -// SecretStoreFactory is a factory type for secret store. -type SecretStoreFactory interface { +// SecretStoreProvider is a factory type for secret store. +type SecretStoreProvider interface { // NewSecretStore constructs a usable secret store with specific provider spec. NewSecretStore(spec v1.SecretStoreSpec) (SecretStore, error) } diff --git a/pkg/secrets/providers.go b/pkg/secrets/providers.go index a9cff62a..bb998e5a 100644 --- a/pkg/secrets/providers.go +++ b/pkg/secrets/providers.go @@ -19,23 +19,23 @@ var ( func init() { createOnce.Do(func() { secretStoreProviders = &Providers{ - registry: make(map[string]SecretStoreFactory), + registry: make(map[string]SecretStoreProvider), } }) } // Register a secret store provider with target spec. -func Register(ssf SecretStoreFactory, spec *v1.ProviderSpec) { +func Register(ssf SecretStoreProvider, spec *v1.ProviderSpec) { secretStoreProviders.register(ssf, spec) } // GetProviderByName returns registered provider by name. -func GetProviderByName(providerName string) (SecretStoreFactory, bool) { +func GetProviderByName(providerName string) (SecretStoreProvider, bool) { return secretStoreProviders.getProviderByName(providerName) } // GetProvider returns the provider from the provider spec. -func GetProvider(spec *v1.ProviderSpec) (SecretStoreFactory, bool) { +func GetProvider(spec *v1.ProviderSpec) (SecretStoreProvider, bool) { if spec == nil { return nil, false } @@ -50,12 +50,12 @@ func GetProvider(spec *v1.ProviderSpec) (SecretStoreFactory, bool) { type Providers struct { lock sync.RWMutex - registry map[string]SecretStoreFactory + registry map[string]SecretStoreProvider } // register registers a provider with associated spec. This // is expected to happen during app startup. -func (ps *Providers) register(ssf SecretStoreFactory, spec *v1.ProviderSpec) { +func (ps *Providers) register(ssf SecretStoreProvider, spec *v1.ProviderSpec) { providerName, err := getProviderName(spec) if err != nil { panic(fmt.Sprintf("provider registery failed to parse spec: %s", err.Error())) @@ -69,7 +69,7 @@ func (ps *Providers) register(ssf SecretStoreFactory, spec *v1.ProviderSpec) { log.Warnf("Provider %s was registered twice", providerName) } } else { - ps.registry = map[string]SecretStoreFactory{} + ps.registry = map[string]SecretStoreProvider{} } log.Infof("Registered secret store provider %s", providerName) @@ -77,7 +77,7 @@ func (ps *Providers) register(ssf SecretStoreFactory, spec *v1.ProviderSpec) { } // getProviderByName returns registered provider by name. -func (ps *Providers) getProviderByName(providerName string) (SecretStoreFactory, bool) { +func (ps *Providers) getProviderByName(providerName string) (SecretStoreProvider, bool) { ps.lock.RLock() defer ps.lock.RUnlock() provider, found := ps.registry[providerName] diff --git a/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager.go b/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager.go index 944afad2..55d9f6e6 100644 --- a/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager.go +++ b/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager.go @@ -26,14 +26,14 @@ var ( accessKeySecret = os.Getenv("credentials_access_secret") ) -// DefaultFactory should implement the secrets.SecretStoreFactory interface. -var _ secrets.SecretStoreFactory = &DefaultFactory{} +// DefaultSecretStoreProvider should implement the secrets.SecretStoreProvider interface. +var _ secrets.SecretStoreProvider = &DefaultSecretStoreProvider{} // smSecretStore should implement the secrets.SecretStore interface. var _ secrets.SecretStore = &smSecretStore{} -// DefaultFactory implements the secrets.SecretStoreFactory interface. -type DefaultFactory struct{} +// DefaultSecretStoreProvider implements the secrets.SecretStoreProvider interface. +type DefaultSecretStoreProvider struct{} // smSecretStore implements the secrets.SecretStore interface. type smSecretStore struct { @@ -41,7 +41,7 @@ type smSecretStore struct { } // NewSecretStore constructs a Vault based secret store with specific secret store spec. -func (p *DefaultFactory) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { +func (p *DefaultSecretStoreProvider) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { providerSpec := spec.Provider if providerSpec == nil { return nil, fmt.Errorf(errMissingProviderSpec) @@ -115,7 +115,7 @@ func (s *smSecretStore) convertSecretToGjson(secretInfo *models.SecretInfo, refP } func init() { - secrets.Register(&DefaultFactory{}, &v1.ProviderSpec{ + secrets.Register(&DefaultSecretStoreProvider{}, &v1.ProviderSpec{ Alicloud: &v1.AlicloudProvider{}, }) } diff --git a/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager_test.go b/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager_test.go index 37292cf4..aa180644 100644 --- a/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager_test.go +++ b/pkg/secrets/providers/alicloud/secretsmanager/secretsmanager_test.go @@ -139,7 +139,7 @@ func TestNewSecretStore(t *testing.T) { }, } - factory := DefaultFactory{} + factory := DefaultSecretStoreProvider{} for name, tc := range testCases { _, err := factory.NewSecretStore(tc.spec) if diff := cmp.Diff(err, tc.expectedErr, EquateErrors()); diff != "" { diff --git a/pkg/secrets/providers/aws/secretsmanager/secretsmanager.go b/pkg/secrets/providers/aws/secretsmanager/secretsmanager.go index c4745ccf..91ae2b6d 100644 --- a/pkg/secrets/providers/aws/secretsmanager/secretsmanager.go +++ b/pkg/secrets/providers/aws/secretsmanager/secretsmanager.go @@ -21,16 +21,16 @@ const ( errFailedToCreateSession = "failed to create usable AWS session: %w" ) -// DefaultFactory should implement the secrets.SecretStoreFactory interface -var _ secrets.SecretStoreFactory = &DefaultFactory{} +// DefaultSecretStoreProvider should implement the secrets.SecretStoreProvider interface +var _ secrets.SecretStoreProvider = &DefaultSecretStoreProvider{} // smSecretStore should implement the secrets.SecretStore interface var _ secrets.SecretStore = &smSecretStore{} -type DefaultFactory struct{} +type DefaultSecretStoreProvider struct{} // NewSecretStore constructs a Vault based secret store with specific secret store spec. -func (p *DefaultFactory) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { +func (p *DefaultSecretStoreProvider) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { providerSpec := spec.Provider if providerSpec == nil { return nil, fmt.Errorf(errMissingProviderSpec) @@ -126,7 +126,7 @@ func (s *smSecretStore) convertSecretToGjson(secretValueOutput *secretsmanager.G } func init() { - secrets.Register(&DefaultFactory{}, &v1.ProviderSpec{ + secrets.Register(&DefaultSecretStoreProvider{}, &v1.ProviderSpec{ AWS: &v1.AWSProvider{}, }) } diff --git a/pkg/secrets/providers/aws/secretsmanager/secretsmanager_test.go b/pkg/secrets/providers/aws/secretsmanager/secretsmanager_test.go index 2c8bbd1b..ecc77b68 100644 --- a/pkg/secrets/providers/aws/secretsmanager/secretsmanager_test.go +++ b/pkg/secrets/providers/aws/secretsmanager/secretsmanager_test.go @@ -159,7 +159,7 @@ func TestNewSecretStore(t *testing.T) { }, } - factory := DefaultFactory{} + factory := DefaultSecretStoreProvider{} for name, tc := range testCases { _, err := factory.NewSecretStore(tc.spec) if diff := cmp.Diff(err, tc.expectedErr, EquateErrors()); diff != "" { diff --git a/pkg/secrets/providers/azure/keyvault/keyvault.go b/pkg/secrets/providers/azure/keyvault/keyvault.go index 023d651b..8b35ea6f 100644 --- a/pkg/secrets/providers/azure/keyvault/keyvault.go +++ b/pkg/secrets/providers/azure/keyvault/keyvault.go @@ -29,16 +29,16 @@ const ( errUnknownObjectType = "unknown Azure KeyVault object Type for %s" ) -// DefaultFactory should implement the secrets.SecretStoreFactory interface -var _ secrets.SecretStoreFactory = &DefaultFactory{} +// DefaultSecretStoreProvider should implement the secrets.SecretStoreProvider interface +var _ secrets.SecretStoreProvider = &DefaultSecretStoreProvider{} // kvSecretStore should implement the secrets.SecretStore interface var _ secrets.SecretStore = &kvSecretStore{} -type DefaultFactory struct{} +type DefaultSecretStoreProvider struct{} // NewSecretStore constructs an Azure KeyVault based secret store with specific secret store spec. -func (p *DefaultFactory) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { +func (p *DefaultSecretStoreProvider) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { providerSpec := spec.Provider if providerSpec == nil { return nil, fmt.Errorf(errMissingProviderSpec) diff --git a/pkg/secrets/providers/azure/keyvault/keyvault_test.go b/pkg/secrets/providers/azure/keyvault/keyvault_test.go index a9ef9526..0f74b526 100644 --- a/pkg/secrets/providers/azure/keyvault/keyvault_test.go +++ b/pkg/secrets/providers/azure/keyvault/keyvault_test.go @@ -149,7 +149,7 @@ func TestNewSecretStore(t *testing.T) { }, } - factory := DefaultFactory{} + factory := DefaultSecretStoreProvider{} for name, tc := range testCases { t.Run(name, func(t *testing.T) { if tc.initEnv { diff --git a/pkg/secrets/providers/hashivault/vault.go b/pkg/secrets/providers/hashivault/vault.go index 79a5ade2..da666132 100644 --- a/pkg/secrets/providers/hashivault/vault.go +++ b/pkg/secrets/providers/hashivault/vault.go @@ -28,20 +28,16 @@ const ( errBuildVaultClient = "failed to new Vault client: %w" ) -// DefaultFactory should implement the secrets.SecretStoreFactory interface -var _ secrets.SecretStoreFactory = &DefaultFactory{} +// DefaultSecretStoreProvider should implement the secrets.SecretStoreProvider interface +var _ secrets.SecretStoreProvider = &DefaultSecretStoreProvider{} // vaultSecretStore should implement the secrets.SecretStore interface var _ secrets.SecretStore = &vaultSecretStore{} -type DefaultFactory struct{} - -func (p *DefaultFactory) Type() string { - return "Vault" -} +type DefaultSecretStoreProvider struct{} // NewSecretStore constructs a Vault based secret store with specific secret store spec. -func (p *DefaultFactory) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { +func (p *DefaultSecretStoreProvider) NewSecretStore(spec v1.SecretStoreSpec) (secrets.SecretStore, error) { providerSpec := spec.Provider if providerSpec == nil || providerSpec.Vault == nil { return nil, errors.New(errInvalidVaultSecretStore) @@ -226,7 +222,7 @@ func getTypedKey(data map[string]interface{}, key string) ([]byte, error) { } func init() { - secrets.Register(&DefaultFactory{}, &v1.ProviderSpec{ + secrets.Register(&DefaultSecretStoreProvider{}, &v1.ProviderSpec{ Vault: &v1.VaultProvider{}, }) } diff --git a/pkg/secrets/providers/hashivault/vault_test.go b/pkg/secrets/providers/hashivault/vault_test.go index 7b086421..3f954c3f 100644 --- a/pkg/secrets/providers/hashivault/vault_test.go +++ b/pkg/secrets/providers/hashivault/vault_test.go @@ -297,7 +297,7 @@ func TestNewSecretStore(t *testing.T) { }, } - factory := DefaultFactory{} + factory := DefaultSecretStoreProvider{} for name, tc := range testCases { _, err := factory.NewSecretStore(tc.spec) if diff := cmp.Diff(err, tc.expectedErr, EquateErrors()); diff != "" { diff --git a/pkg/secrets/providers_test.go b/pkg/secrets/providers_test.go index 5fdffe91..29cec9db 100644 --- a/pkg/secrets/providers_test.go +++ b/pkg/secrets/providers_test.go @@ -17,11 +17,11 @@ func (fss *FakeSecretStore) GetSecret(_ context.Context, _ v1.ExternalSecretRef) return []byte("NOOP"), nil } -// FakeSecretStoreFactory is the fake implementation of SecretStoreFactory. -type FakeSecretStoreFactory struct{} +// FakeSecretStoreProvider is the fake implementation of SecretStoreProvider. +type FakeSecretStoreProvider struct{} -// Fake implementation of SecretStoreFactory.NewSecretStore. -func (fsf *FakeSecretStoreFactory) NewSecretStore(_ v1.SecretStoreSpec) (SecretStore, error) { +// Fake implementation of SecretStoreProvider.NewSecretStore. +func (fsf *FakeSecretStoreProvider) NewSecretStore(_ v1.SecretStoreSpec) (SecretStore, error) { return &FakeSecretStore{}, nil } @@ -49,7 +49,7 @@ func TestRegister(t *testing.T) { }, } - fsp := &FakeSecretStoreFactory{} + fsp := &FakeSecretStoreProvider{} for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { if tc.shouldPanic {