Pārlūkot izejas kodu

chore: refactor provider (#1529)

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>
Moritz Johner 3 gadi atpakaļ
vecāks
revīzija
af367e9933

+ 1 - 1
e2e/suites/provider/cases/gcp/provider.go

@@ -3,7 +3,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
 
-    http://www.apache.org/licenses/LICENSE-2.0
+	http://www.apache.org/licenses/LICENSE-2.0
 
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,

+ 29 - 31
pkg/provider/aws/auth/auth.go

@@ -81,11 +81,12 @@ func New(ctx context.Context, store esv1beta1.GenericStore, kube client.Client,
 		return nil, err
 	}
 	var creds *credentials.Credentials
+	isClusterKind := store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind
 
 	// use credentials via service account token
 	jwtAuth := prov.Auth.JWTAuth
 	if jwtAuth != nil {
-		creds, err = sessionFromServiceAccount(ctx, prov, store, kube, namespace, jwtProvider)
+		creds, err = sessionFromServiceAccount(ctx, prov.Auth, prov.Region, isClusterKind, kube, namespace, jwtProvider)
 		if err != nil {
 			return nil, err
 		}
@@ -95,7 +96,7 @@ func New(ctx context.Context, store esv1beta1.GenericStore, kube client.Client,
 	secretRef := prov.Auth.SecretRef
 	if secretRef != nil {
 		log.V(1).Info("using credentials from secretRef")
-		creds, err = sessionFromSecretRef(ctx, prov, store, kube, namespace)
+		creds, err = sessionFromSecretRef(ctx, prov.Auth, isClusterKind, kube, namespace)
 		if err != nil {
 			return nil, err
 		}
@@ -109,7 +110,7 @@ func New(ctx context.Context, store esv1beta1.GenericStore, kube client.Client,
 		config.WithRegion(prov.Region)
 	}
 
-	sess, err := getAWSSession(config, store, namespace)
+	sess, err := getAWSSession(config, EnableCache, store.GetName(), store.GetTypeMeta().Kind, namespace, store.GetObjectMeta().ResourceVersion)
 	if err != nil {
 		return nil, err
 	}
@@ -122,17 +123,17 @@ func New(ctx context.Context, store esv1beta1.GenericStore, kube client.Client,
 	return sess, nil
 }
 
-func sessionFromSecretRef(ctx context.Context, prov *esv1beta1.AWSProvider, store esv1beta1.GenericStore, kube client.Client, namespace string) (*credentials.Credentials, error) {
+func sessionFromSecretRef(ctx context.Context, auth esv1beta1.AWSAuth, isClusterKind bool, kube client.Client, namespace string) (*credentials.Credentials, error) {
 	ke := client.ObjectKey{
-		Name:      prov.Auth.SecretRef.AccessKeyID.Name,
+		Name:      auth.SecretRef.AccessKeyID.Name,
 		Namespace: namespace, // default to ExternalSecret namespace
 	}
 	// only ClusterStore is allowed to set namespace (and then it's required)
-	if store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind {
-		if prov.Auth.SecretRef.AccessKeyID.Namespace == nil {
+	if isClusterKind {
+		if auth.SecretRef.AccessKeyID.Namespace == nil {
 			return nil, fmt.Errorf(errInvalidClusterStoreMissingAKIDNamespace)
 		}
-		ke.Namespace = *prov.Auth.SecretRef.AccessKeyID.Namespace
+		ke.Namespace = *auth.SecretRef.AccessKeyID.Namespace
 	}
 	akSecret := v1.Secret{}
 	err := kube.Get(ctx, ke, &akSecret)
@@ -140,23 +141,23 @@ func sessionFromSecretRef(ctx context.Context, prov *esv1beta1.AWSProvider, stor
 		return nil, fmt.Errorf(errFetchAKIDSecret, err)
 	}
 	ke = client.ObjectKey{
-		Name:      prov.Auth.SecretRef.SecretAccessKey.Name,
+		Name:      auth.SecretRef.SecretAccessKey.Name,
 		Namespace: namespace, // default to ExternalSecret namespace
 	}
 	// only ClusterStore is allowed to set namespace (and then it's required)
-	if store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind {
-		if prov.Auth.SecretRef.SecretAccessKey.Namespace == nil {
+	if isClusterKind {
+		if auth.SecretRef.SecretAccessKey.Namespace == nil {
 			return nil, fmt.Errorf(errInvalidClusterStoreMissingSAKNamespace)
 		}
-		ke.Namespace = *prov.Auth.SecretRef.SecretAccessKey.Namespace
+		ke.Namespace = *auth.SecretRef.SecretAccessKey.Namespace
 	}
 	sakSecret := v1.Secret{}
 	err = kube.Get(ctx, ke, &sakSecret)
 	if err != nil {
 		return nil, fmt.Errorf(errFetchSAKSecret, err)
 	}
-	sak := string(sakSecret.Data[prov.Auth.SecretRef.SecretAccessKey.Key])
-	aks := string(akSecret.Data[prov.Auth.SecretRef.AccessKeyID.Key])
+	sak := string(sakSecret.Data[auth.SecretRef.SecretAccessKey.Key])
+	aks := string(akSecret.Data[auth.SecretRef.AccessKeyID.Key])
 	if sak == "" {
 		return nil, fmt.Errorf(errMissingSAK)
 	}
@@ -167,14 +168,11 @@ func sessionFromSecretRef(ctx context.Context, prov *esv1beta1.AWSProvider, stor
 	return credentials.NewStaticCredentials(aks, sak, ""), err
 }
 
-func sessionFromServiceAccount(ctx context.Context, prov *esv1beta1.AWSProvider, store esv1beta1.GenericStore, kube client.Client, namespace string, jwtProvider jwtProviderFactory) (*credentials.Credentials, error) {
-	if store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind {
-		if prov.Auth.JWTAuth.ServiceAccountRef.Namespace == nil {
-			return nil, fmt.Errorf("serviceAccountRef has no Namespace field (mandatory for ClusterSecretStore specs)")
-		}
-		namespace = *prov.Auth.JWTAuth.ServiceAccountRef.Namespace
+func sessionFromServiceAccount(ctx context.Context, auth esv1beta1.AWSAuth, region string, isClusterKind bool, kube client.Client, namespace string, jwtProvider jwtProviderFactory) (*credentials.Credentials, error) {
+	name := auth.JWTAuth.ServiceAccountRef.Name
+	if isClusterKind {
+		namespace = *auth.JWTAuth.ServiceAccountRef.Namespace
 	}
-	name := prov.Auth.JWTAuth.ServiceAccountRef.Name
 	sa := v1.ServiceAccount{}
 	err := kube.Get(ctx, types.NamespacedName{
 		Name:      name,
@@ -195,16 +193,16 @@ func sessionFromServiceAccount(ctx context.Context, prov *esv1beta1.AWSProvider,
 		tokenAud = defaultTokenAudience
 	}
 	audiences := []string{tokenAud}
-	if len(prov.Auth.JWTAuth.ServiceAccountRef.Audiences) > 0 {
-		audiences = append(audiences, prov.Auth.JWTAuth.ServiceAccountRef.Audiences...)
+	if len(auth.JWTAuth.ServiceAccountRef.Audiences) > 0 {
+		audiences = append(audiences, auth.JWTAuth.ServiceAccountRef.Audiences...)
 	}
 
-	jwtProv, err := jwtProvider(name, namespace, roleArn, audiences, prov.Region)
+	jwtProv, err := jwtProvider(name, namespace, roleArn, audiences, region)
 	if err != nil {
 		return nil, err
 	}
 
-	log.V(1).Info("using credentials via service account", "role", roleArn, "region", prov.Region)
+	log.V(1).Info("using credentials via service account", "role", roleArn, "region", region)
 	return credentials.NewCredentials(jwtProv), nil
 }
 
@@ -255,15 +253,15 @@ func DefaultSTSProvider(sess *session.Session) stsiface.STSAPI {
 
 // getAWSSession check if an AWS session should be reused
 // it returns the aws session or an error.
-func getAWSSession(config *aws.Config, store esv1beta1.GenericStore, namespace string) (*session.Session, error) {
+func getAWSSession(config *aws.Config, enableCache bool, name, kind, namespace, resourceVersion string) (*session.Session, error) {
 	tmpSession := SessionCache{
-		Name:            store.GetObjectMeta().Name,
+		Name:            name,
 		Namespace:       namespace,
-		Kind:            store.GetTypeMeta().Kind,
-		ResourceVersion: store.GetObjectMeta().ResourceVersion,
+		Kind:            kind,
+		ResourceVersion: resourceVersion,
 	}
 
-	if EnableCache {
+	if enableCache {
 		sess, ok := sessions[tmpSession]
 		if ok {
 			log.Info("reusing aws session", "SecretStore", tmpSession.Name, "namespace", tmpSession.Namespace, "kind", tmpSession.Kind, "resourceversion", tmpSession.ResourceVersion)
@@ -282,7 +280,7 @@ func getAWSSession(config *aws.Config, store esv1beta1.GenericStore, namespace s
 		return nil, err
 	}
 
-	if EnableCache {
+	if enableCache {
 		sessions[tmpSession] = sess
 	}
 	return sess, nil

+ 21 - 21
pkg/provider/azure/keyvault/keyvault.go

@@ -49,9 +49,9 @@ const (
 	defaultObjType       = "secret"
 	objectTypeCert       = "cert"
 	objectTypeKey        = "key"
-	azureDefaultAudience = "api://AzureADTokenExchange"
-	annotationClientID   = "azure.workload.identity/client-id"
-	annotationTenantID   = "azure.workload.identity/tenant-id"
+	AzureDefaultAudience = "api://AzureADTokenExchange"
+	AnnotationClientID   = "azure.workload.identity/client-id"
+	AnnotationTenantID   = "azure.workload.identity/tenant-id"
 
 	errUnexpectedStoreSpec   = "unexpected store spec"
 	errMissingAuthType       = "cannot initialize Azure Client: no valid authType was specified"
@@ -140,7 +140,7 @@ func newClient(ctx context.Context, store esv1beta1.GenericStore, kube client.Cl
 	case esv1beta1.AzureServicePrincipal:
 		authorizer, err = az.authorizerForServicePrincipal(ctx)
 	case esv1beta1.AzureWorkloadIdentity:
-		authorizer, err = az.authorizerForWorkloadIdentity(ctx, newTokenProvider)
+		authorizer, err = az.authorizerForWorkloadIdentity(ctx, NewTokenProvider)
 	default:
 		err = fmt.Errorf(errMissingAuthType)
 	}
@@ -423,8 +423,8 @@ func getSecretMapProperties(tags map[string]*string, key, property string) map[s
 }
 
 func (a *Azure) authorizerForWorkloadIdentity(ctx context.Context, tokenProvider tokenProviderFunc) (autorest.Authorizer, error) {
-	aadEndpoint := aadEndpointForProviderConfig(a.provider)
-	kvResource := kvResourceForProviderConfig(a.provider)
+	aadEndpoint := AadEndpointForType(a.provider.EnvironmentType)
+	kvResource := kvResourceForProviderConfig(a.provider.EnvironmentType)
 	// if no serviceAccountRef was provided
 	// we expect certain env vars to be present.
 	// They are set by the azure workload identity webhook.
@@ -457,19 +457,19 @@ func (a *Azure) authorizerForWorkloadIdentity(ctx context.Context, tokenProvider
 	if err != nil {
 		return nil, err
 	}
-	clientID, ok := sa.ObjectMeta.Annotations[annotationClientID]
+	clientID, ok := sa.ObjectMeta.Annotations[AnnotationClientID]
 	if !ok {
-		return nil, fmt.Errorf(errMissingSAAnnotation, annotationClientID)
+		return nil, fmt.Errorf(errMissingSAAnnotation, AnnotationClientID)
 	}
-	tenantID, ok := sa.ObjectMeta.Annotations[annotationTenantID]
+	tenantID, ok := sa.ObjectMeta.Annotations[AnnotationTenantID]
 	if !ok {
-		return nil, fmt.Errorf(errMissingSAAnnotation, annotationTenantID)
+		return nil, fmt.Errorf(errMissingSAAnnotation, AnnotationTenantID)
 	}
-	audiences := []string{azureDefaultAudience}
+	audiences := []string{AzureDefaultAudience}
 	if len(a.provider.ServiceAccountRef.Audiences) > 0 {
 		audiences = append(audiences, a.provider.ServiceAccountRef.Audiences...)
 	}
-	token, err := fetchSAToken(ctx, ns, a.provider.ServiceAccountRef.Name, audiences, a.kubeClient)
+	token, err := FetchSAToken(ctx, ns, a.provider.ServiceAccountRef.Name, audiences, a.kubeClient)
 	if err != nil {
 		return nil, err
 	}
@@ -480,7 +480,7 @@ func (a *Azure) authorizerForWorkloadIdentity(ctx context.Context, tokenProvider
 	return autorest.NewBearerAuthorizer(tp), nil
 }
 
-func fetchSAToken(ctx context.Context, ns, name string, audiences []string, kubeClient kcorev1.CoreV1Interface) (string, error) {
+func FetchSAToken(ctx context.Context, ns, name string, audiences []string, kubeClient kcorev1.CoreV1Interface) (string, error) {
 	token, err := kubeClient.ServiceAccounts(ns).CreateToken(ctx, name, &authv1.TokenRequest{
 		Spec: authv1.TokenRequestSpec{
 			Audiences: audiences,
@@ -499,7 +499,7 @@ type tokenProvider struct {
 
 type tokenProviderFunc func(ctx context.Context, token, clientID, tenantID, aadEndpoint, kvResource string) (adal.OAuthTokenProvider, error)
 
-func newTokenProvider(ctx context.Context, token, clientID, tenantID, aadEndpoint, kvResource string) (adal.OAuthTokenProvider, error) {
+func NewTokenProvider(ctx context.Context, token, clientID, tenantID, aadEndpoint, kvResource string) (adal.OAuthTokenProvider, error) {
 	// exchange token with Azure AccessToken
 	cred := confidential.NewCredFromAssertionCallback(func(ctx context.Context, aro confidential.AssertionRequestOptions) (string, error) {
 		return token, nil
@@ -532,7 +532,7 @@ func (t *tokenProvider) OAuthToken() string {
 
 func (a *Azure) authorizerForManagedIdentity() (autorest.Authorizer, error) {
 	msiConfig := kvauth.NewMSIConfig()
-	msiConfig.Resource = kvResourceForProviderConfig(a.provider)
+	msiConfig.Resource = kvResourceForProviderConfig(a.provider.EnvironmentType)
 	if a.provider.IdentityID != nil {
 		msiConfig.ClientID = *a.provider.IdentityID
 	}
@@ -562,8 +562,8 @@ func (a *Azure) authorizerForServicePrincipal(ctx context.Context) (autorest.Aut
 		return nil, err
 	}
 	clientCredentialsConfig := kvauth.NewClientCredentialsConfig(cid, csec, *a.provider.TenantID)
-	clientCredentialsConfig.Resource = kvResourceForProviderConfig(a.provider)
-	clientCredentialsConfig.AADEndpoint = aadEndpointForProviderConfig(a.provider)
+	clientCredentialsConfig.Resource = kvResourceForProviderConfig(a.provider.EnvironmentType)
+	clientCredentialsConfig.AADEndpoint = AadEndpointForType(a.provider.EnvironmentType)
 	return clientCredentialsConfig.Authorizer()
 }
 
@@ -597,8 +597,8 @@ func (a *Azure) Validate() (esv1beta1.ValidationResult, error) {
 	return esv1beta1.ValidationResultReady, nil
 }
 
-func aadEndpointForProviderConfig(prov *esv1beta1.AzureKVProvider) string {
-	switch prov.EnvironmentType {
+func AadEndpointForType(t esv1beta1.AzureEnvironmentType) string {
+	switch t {
 	case esv1beta1.AzureEnvironmentPublicCloud:
 		return azure.PublicCloud.ActiveDirectoryEndpoint
 	case esv1beta1.AzureEnvironmentChinaCloud:
@@ -612,9 +612,9 @@ func aadEndpointForProviderConfig(prov *esv1beta1.AzureKVProvider) string {
 	}
 }
 
-func kvResourceForProviderConfig(prov *esv1beta1.AzureKVProvider) string {
+func kvResourceForProviderConfig(t esv1beta1.AzureEnvironmentType) string {
 	var res string
-	switch prov.EnvironmentType {
+	switch t {
 	case esv1beta1.AzureEnvironmentPublicCloud:
 		res = azure.PublicCloud.KeyVaultEndpoint
 	case esv1beta1.AzureEnvironmentChinaCloud:

+ 2 - 2
pkg/provider/azure/keyvault/keyvault_auth_test.go

@@ -169,8 +169,8 @@ func TestGetAuthorizorForWorkloadIdentity(t *testing.T) {
 						Name:      saName,
 						Namespace: namespace,
 						Annotations: map[string]string{
-							annotationClientID: clientID,
-							annotationTenantID: tenantID,
+							AnnotationClientID: clientID,
+							AnnotationTenantID: tenantID,
 						},
 					},
 				},

+ 79 - 0
pkg/provider/gcp/secretmanager/auth.go

@@ -0,0 +1,79 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package secretmanager
+
+import (
+	"context"
+	"fmt"
+
+	"golang.org/x/oauth2"
+	"golang.org/x/oauth2/google"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+	kclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
+)
+
+func NewTokenSource(ctx context.Context, auth esv1beta1.GCPSMAuth, projectID string, isClusterKind bool, kube kclient.Client, namespace string) (oauth2.TokenSource, error) {
+	ts, err := serviceAccountTokenSource(ctx, auth, isClusterKind, kube, namespace)
+	if ts != nil || err != nil {
+		return ts, err
+	}
+	wi, err := newWorkloadIdentity(ctx, projectID)
+	if err != nil {
+		useMu.Unlock()
+		return nil, fmt.Errorf("unable to initialize workload identity")
+	}
+	ts, err = wi.TokenSource(ctx, auth, isClusterKind, kube, namespace)
+	if ts != nil || err != nil {
+		return ts, err
+	}
+	return google.DefaultTokenSource(ctx, CloudPlatformRole)
+}
+
+func serviceAccountTokenSource(ctx context.Context, auth esv1beta1.GCPSMAuth, isClusterKind bool, kube kclient.Client, namespace string) (oauth2.TokenSource, error) {
+	sr := auth.SecretRef
+	if sr == nil {
+		return nil, nil
+	}
+	credentialsSecret := &v1.Secret{}
+	credentialsSecretName := sr.SecretAccessKey.Name
+	objectKey := types.NamespacedName{
+		Name:      credentialsSecretName,
+		Namespace: namespace,
+	}
+
+	// only ClusterStore is allowed to set namespace (and then it's required)
+	if isClusterKind {
+		if credentialsSecretName != "" && sr.SecretAccessKey.Namespace == nil {
+			return nil, fmt.Errorf(errInvalidClusterStoreMissingSAKNamespace)
+		} else if credentialsSecretName != "" {
+			objectKey.Namespace = *sr.SecretAccessKey.Namespace
+		}
+	}
+	err := kube.Get(ctx, objectKey, credentialsSecret)
+	if err != nil {
+		return nil, fmt.Errorf(errFetchSAKSecret, err)
+	}
+	credentials := credentialsSecret.Data[sr.SecretAccessKey.Key]
+	if (credentials == nil) || (len(credentials) == 0) {
+		return nil, fmt.Errorf(errMissingSAK)
+	}
+	config, err := google.JWTConfigFromJSON(credentials, CloudPlatformRole)
+	if err != nil {
+		return nil, fmt.Errorf(errUnableProcessJSONCredentials, err)
+	}
+	return config.TokenSource(ctx), nil
+}

+ 43 - 210
pkg/provider/gcp/secretmanager/secretsmanager.go

@@ -20,18 +20,12 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
-	"sync"
 
 	secretmanager "cloud.google.com/go/secretmanager/apiv1"
 	"github.com/googleapis/gax-go/v2"
 	"github.com/tidwall/gjson"
-	"golang.org/x/oauth2"
-	"golang.org/x/oauth2/google"
 	"google.golang.org/api/iterator"
-	"google.golang.org/api/option"
 	secretmanagerpb "google.golang.org/genproto/googleapis/cloud/secretmanager/v1"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -66,7 +60,15 @@ const (
 	errUnexpectedFindOperator = "unexpected find operator"
 )
 
-var log = ctrl.Log.WithName("provider").WithName("gcp").WithName("secretsmanager")
+type Client struct {
+	smClient GoogleSecretManagerClient
+	kube     kclient.Client
+	store    *esv1beta1.GCPSMProvider
+
+	// namespace of the external secret
+	namespace        string
+	workloadIdentity *workloadIdentity
+}
 
 type GoogleSecretManagerClient interface {
 	AccessSecretVersion(ctx context.Context, req *secretmanagerpb.AccessSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error)
@@ -74,171 +76,33 @@ type GoogleSecretManagerClient interface {
 	Close() error
 }
 
-/*
-Currently, GCPSM client has a limitation around how concurrent connections work
-This limitation causes memory leaks due to random disconnects from living clients
-and also payload switches when sending a call (such as using a credential from one
-thread to ask secrets from another thread).
-A Mutex was implemented to make sure only one connection can be in place at a time.
-*/
-var useMu = sync.Mutex{}
-
-// https://github.com/external-secrets/external-secrets/issues/644
-var _ esv1beta1.SecretsClient = &ProviderGCP{}
-var _ esv1beta1.Provider = &ProviderGCP{}
-
-// ProviderGCP is a provider for GCP Secret Manager.
-type ProviderGCP struct {
-	projectID           string
-	SecretManagerClient GoogleSecretManagerClient
-	gClient             *gClient
-}
-
-type gClient struct {
-	kube      kclient.Client
-	store     *esv1beta1.GCPSMProvider
-	namespace string
-	storeKind string
-
-	workloadIdentity *workloadIdentity
-}
-
-func (c *gClient) getTokenSource(ctx context.Context, store esv1beta1.GenericStore, kube kclient.Client, namespace string) (oauth2.TokenSource, error) {
-	ts, err := serviceAccountTokenSource(ctx, store, kube, namespace)
-	if ts != nil || err != nil {
-		return ts, err
-	}
-	ts, err = c.workloadIdentity.TokenSource(ctx, store, kube, namespace)
-	if ts != nil || err != nil {
-		return ts, err
-	}
-
-	return google.DefaultTokenSource(ctx, CloudPlatformRole)
-}
-
-func (c *gClient) Close() error {
-	return c.workloadIdentity.Close()
-}
-
-func serviceAccountTokenSource(ctx context.Context, store esv1beta1.GenericStore, kube kclient.Client, namespace string) (oauth2.TokenSource, error) {
-	spec := store.GetSpec()
-	if spec == nil || spec.Provider.GCPSM == nil {
-		return nil, fmt.Errorf(errMissingStoreSpec)
-	}
-	sr := spec.Provider.GCPSM.Auth.SecretRef
-	if sr == nil {
-		return nil, nil
-	}
-	storeKind := store.GetObjectKind().GroupVersionKind().Kind
-	credentialsSecret := &v1.Secret{}
-	credentialsSecretName := sr.SecretAccessKey.Name
-	objectKey := types.NamespacedName{
-		Name:      credentialsSecretName,
-		Namespace: namespace,
-	}
-
-	// only ClusterStore is allowed to set namespace (and then it's required)
-	if storeKind == esv1beta1.ClusterSecretStoreKind {
-		if credentialsSecretName != "" && sr.SecretAccessKey.Namespace == nil {
-			return nil, fmt.Errorf(errInvalidClusterStoreMissingSAKNamespace)
-		} else if credentialsSecretName != "" {
-			objectKey.Namespace = *sr.SecretAccessKey.Namespace
-		}
-	}
-	err := kube.Get(ctx, objectKey, credentialsSecret)
-	if err != nil {
-		return nil, fmt.Errorf(errFetchSAKSecret, err)
-	}
-	credentials := credentialsSecret.Data[sr.SecretAccessKey.Key]
-	if (credentials == nil) || (len(credentials) == 0) {
-		return nil, fmt.Errorf(errMissingSAK)
-	}
-	config, err := google.JWTConfigFromJSON(credentials, CloudPlatformRole)
-	if err != nil {
-		return nil, fmt.Errorf(errUnableProcessJSONCredentials, err)
-	}
-	return config.TokenSource(ctx), nil
-}
-
-// NewClient constructs a GCP Provider.
-func (sm *ProviderGCP) NewClient(ctx context.Context, store esv1beta1.GenericStore, kube kclient.Client, namespace string) (esv1beta1.SecretsClient, error) {
-	storeSpec := store.GetSpec()
-	if storeSpec == nil || storeSpec.Provider == nil || storeSpec.Provider.GCPSM == nil {
-		return nil, fmt.Errorf(errGCPSMStore)
-	}
-	storeSpecGCPSM := storeSpec.Provider.GCPSM
-
-	useMu.Lock()
-	wi, err := newWorkloadIdentity(ctx)
-	if err != nil {
-		useMu.Unlock()
-		return nil, fmt.Errorf("unable to initialize workload identity")
-	}
-
-	cliStore := gClient{
-		kube:             kube,
-		store:            storeSpecGCPSM,
-		namespace:        namespace,
-		storeKind:        store.GetObjectKind().GroupVersionKind().Kind,
-		workloadIdentity: wi,
-	}
-	sm.gClient = &cliStore
-	defer func() {
-		// closes IAMClient to prevent gRPC connection leak in case of an error.
-		if sm.SecretManagerClient == nil {
-			_ = sm.gClient.Close()
-		}
-	}()
-
-	sm.projectID = cliStore.store.ProjectID
-
-	ts, err := cliStore.getTokenSource(ctx, store, kube, namespace)
-	if err != nil {
-		useMu.Unlock()
-		return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
-	}
-
-	// check if we can get credentials
-	_, err = ts.Token()
-	if err != nil {
-		useMu.Unlock()
-		return nil, fmt.Errorf(errUnableGetCredentials, err)
-	}
-
-	clientGCPSM, err := secretmanager.NewClient(ctx, option.WithTokenSource(ts))
-	if err != nil {
-		useMu.Unlock()
-		return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
-	}
-	sm.SecretManagerClient = clientGCPSM
-	return sm, nil
-}
+var log = ctrl.Log.WithName("provider").WithName("gcp").WithName("secretsmanager")
 
 // GetAllSecrets syncs multiple secrets from gcp provider into a single Kubernetes Secret.
-func (sm *ProviderGCP) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
+func (c *Client) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	if ref.Name != nil {
-		return sm.findByName(ctx, ref)
+		return c.findByName(ctx, ref)
 	}
 	if len(ref.Tags) > 0 {
-		return sm.findByTags(ctx, ref)
+		return c.findByTags(ctx, ref)
 	}
 	return nil, errors.New(errUnexpectedFindOperator)
 }
 
-func (sm *ProviderGCP) findByName(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
+func (c *Client) findByName(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	// regex matcher
 	matcher, err := find.New(*ref.Name)
 	if err != nil {
 		return nil, err
 	}
 	req := &secretmanagerpb.ListSecretsRequest{
-		Parent: fmt.Sprintf("projects/%s", sm.projectID),
+		Parent: fmt.Sprintf("projects/%s", c.store.ProjectID),
 	}
 	if ref.Path != nil {
 		req.Filter = fmt.Sprintf("name:%s", *ref.Path)
 	}
 	// Call the API.
-	it := sm.SecretManagerClient.ListSecrets(ctx, req)
+	it := c.smClient.ListSecrets(ctx, req)
 	secretMap := make(map[string][]byte)
 	for {
 		resp, err := it.Next()
@@ -249,7 +113,7 @@ func (sm *ProviderGCP) findByName(ctx context.Context, ref esv1beta1.ExternalSec
 			return nil, fmt.Errorf("failed to list secrets: %w", err)
 		}
 		log.V(1).Info("gcp sm findByName found", "secrets", strconv.Itoa(it.PageInfo().Remaining()))
-		key := sm.trimName(resp.Name)
+		key := c.trimName(resp.Name)
 		// If we don't match we skip.
 		// Also, if we have path, and it is not at the beguining we skip.
 		// We have to check if path is at the beguining of the key because
@@ -260,7 +124,7 @@ func (sm *ProviderGCP) findByName(ctx context.Context, ref esv1beta1.ExternalSec
 			continue
 		}
 		log.V(1).Info("gcp sm findByName matches", "name", resp.Name)
-		secretMap[key], err = sm.getData(ctx, key)
+		secretMap[key], err = c.getData(ctx, key)
 		if err != nil {
 			return nil, err
 		}
@@ -269,18 +133,18 @@ func (sm *ProviderGCP) findByName(ctx context.Context, ref esv1beta1.ExternalSec
 	return utils.ConvertKeys(ref.ConversionStrategy, secretMap)
 }
 
-func (sm *ProviderGCP) getData(ctx context.Context, key string) ([]byte, error) {
+func (c *Client) getData(ctx context.Context, key string) ([]byte, error) {
 	dataRef := esv1beta1.ExternalSecretDataRemoteRef{
 		Key: key,
 	}
-	data, err := sm.GetSecret(ctx, dataRef)
+	data, err := c.GetSecret(ctx, dataRef)
 	if err != nil {
 		return []byte(""), err
 	}
 	return data, nil
 }
 
-func (sm *ProviderGCP) findByTags(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
+func (c *Client) findByTags(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	var tagFilter string
 	for k, v := range ref.Tags {
 		tagFilter = fmt.Sprintf("%slabels.%s=%s ", tagFilter, k, v)
@@ -290,12 +154,12 @@ func (sm *ProviderGCP) findByTags(ctx context.Context, ref esv1beta1.ExternalSec
 		tagFilter = fmt.Sprintf("%s name:%s", tagFilter, *ref.Path)
 	}
 	req := &secretmanagerpb.ListSecretsRequest{
-		Parent: fmt.Sprintf("projects/%s", sm.projectID),
+		Parent: fmt.Sprintf("projects/%s", c.store.ProjectID),
 	}
 	log.V(1).Info("gcp sm findByTags", "tagFilter", tagFilter)
 	req.Filter = tagFilter
 	// Call the API.
-	it := sm.SecretManagerClient.ListSecrets(ctx, req)
+	it := c.smClient.ListSecrets(ctx, req)
 	secretMap := make(map[string][]byte)
 	for {
 		resp, err := it.Next()
@@ -305,12 +169,12 @@ func (sm *ProviderGCP) findByTags(ctx context.Context, ref esv1beta1.ExternalSec
 		if err != nil {
 			return nil, fmt.Errorf("failed to list secrets: %w", err)
 		}
-		key := sm.trimName(resp.Name)
+		key := c.trimName(resp.Name)
 		if ref.Path != nil && !strings.HasPrefix(key, *ref.Path) {
 			continue
 		}
 		log.V(1).Info("gcp sm findByTags matches tags", "name", resp.Name)
-		secretMap[key], err = sm.getData(ctx, key)
+		secretMap[key], err = c.getData(ctx, key)
 		if err != nil {
 			return nil, err
 		}
@@ -319,8 +183,8 @@ func (sm *ProviderGCP) findByTags(ctx context.Context, ref esv1beta1.ExternalSec
 	return utils.ConvertKeys(ref.ConversionStrategy, secretMap)
 }
 
-func (sm *ProviderGCP) trimName(name string) string {
-	projectIDNumuber := sm.extractProjectIDNumber(name)
+func (c *Client) trimName(name string) string {
+	projectIDNumuber := c.extractProjectIDNumber(name)
 	key := strings.TrimPrefix(name, fmt.Sprintf("projects/%s/secrets/", projectIDNumuber))
 	return key
 }
@@ -328,15 +192,15 @@ func (sm *ProviderGCP) trimName(name string) string {
 // extractProjectIDNumber grabs the project id from the full name returned by gcp api
 // gcp api seems to always return the number and not the project name
 // (and users would always use the name, while requests accept both).
-func (sm *ProviderGCP) extractProjectIDNumber(secretFullName string) string {
+func (c *Client) extractProjectIDNumber(secretFullName string) string {
 	s := strings.Split(secretFullName, "/")
 	projectIDNumuber := s[1]
 	return projectIDNumuber
 }
 
 // GetSecret returns a single secret from the provider.
-func (sm *ProviderGCP) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
-	if utils.IsNil(sm.SecretManagerClient) || sm.projectID == "" {
+func (c *Client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
+	if utils.IsNil(c.smClient) || c.store.ProjectID == "" {
 		return nil, fmt.Errorf(errUninitalizedGCPProvider)
 	}
 
@@ -346,9 +210,9 @@ func (sm *ProviderGCP) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecr
 	}
 
 	req := &secretmanagerpb.AccessSecretVersionRequest{
-		Name: fmt.Sprintf("projects/%s/secrets/%s/versions/%s", sm.projectID, ref.Key, version),
+		Name: fmt.Sprintf("projects/%s/secrets/%s/versions/%s", c.store.ProjectID, ref.Key, version),
 	}
-	result, err := sm.SecretManagerClient.AccessSecretVersion(ctx, req)
+	result, err := c.smClient.AccessSecretVersion(ctx, req)
 	if err != nil {
 		return nil, fmt.Errorf(errClientGetSecretAccess, err)
 	}
@@ -381,12 +245,12 @@ func (sm *ProviderGCP) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecr
 }
 
 // GetSecretMap returns multiple k/v pairs from the provider.
-func (sm *ProviderGCP) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
-	if sm.SecretManagerClient == nil || sm.projectID == "" {
+func (c *Client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
+	if c.smClient == nil || c.store.ProjectID == "" {
 		return nil, fmt.Errorf(errUninitalizedGCPProvider)
 	}
 
-	data, err := sm.GetSecret(ctx, ref)
+	data, err := c.GetSecret(ctx, ref)
 	if err != nil {
 		return nil, err
 	}
@@ -411,10 +275,13 @@ func (sm *ProviderGCP) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalS
 	return secretData, nil
 }
 
-func (sm *ProviderGCP) Close(ctx context.Context) error {
-	err := sm.SecretManagerClient.Close()
-	if sm.gClient != nil {
-		err = sm.gClient.Close()
+func (c *Client) Close(ctx context.Context) error {
+	var err error
+	if c.smClient != nil {
+		err = c.smClient.Close()
+	}
+	if c.workloadIdentity != nil {
+		err = c.workloadIdentity.Close()
 	}
 	useMu.Unlock()
 	if err != nil {
@@ -423,40 +290,6 @@ func (sm *ProviderGCP) Close(ctx context.Context) error {
 	return nil
 }
 
-func (sm *ProviderGCP) Validate() (esv1beta1.ValidationResult, error) {
+func (c *Client) Validate() (esv1beta1.ValidationResult, error) {
 	return esv1beta1.ValidationResultReady, nil
 }
-
-func (sm *ProviderGCP) ValidateStore(store esv1beta1.GenericStore) error {
-	if store == nil {
-		return fmt.Errorf(errInvalidStore)
-	}
-	spc := store.GetSpec()
-	if spc == nil {
-		return fmt.Errorf(errInvalidStoreSpec)
-	}
-	if spc.Provider == nil {
-		return fmt.Errorf(errInvalidStoreProv)
-	}
-	p := spc.Provider.GCPSM
-	if p == nil {
-		return fmt.Errorf(errInvalidGCPProv)
-	}
-	if p.Auth.SecretRef != nil {
-		if err := utils.ValidateSecretSelector(store, p.Auth.SecretRef.SecretAccessKey); err != nil {
-			return fmt.Errorf(errInvalidAuthSecretRef, err)
-		}
-	}
-	if p.Auth.WorkloadIdentity != nil {
-		if err := utils.ValidateServiceAccountSelector(store, p.Auth.WorkloadIdentity.ServiceAccountRef); err != nil {
-			return fmt.Errorf(errInvalidWISARef, err)
-		}
-	}
-	return nil
-}
-
-func init() {
-	esv1beta1.Register(&ProviderGCP{}, &esv1beta1.SecretStoreProvider{
-		GCPSM: &esv1beta1.GCPSMProvider{},
-	})
-}

+ 7 - 7
pkg/provider/gcp/secretmanager/secretsmanager_test.go

@@ -167,10 +167,10 @@ func TestSecretManagerGetSecret(t *testing.T) {
 		makeValidSecretManagerTestCaseCustom(setNilMockClient),
 	}
 
-	sm := ProviderGCP{}
+	sm := Client{}
 	for k, v := range successCases {
-		sm.projectID = v.projectID
-		sm.SecretManagerClient = v.mockClient
+		sm.store = &esv1beta1.GCPSMProvider{ProjectID: v.projectID}
+		sm.smClient = v.mockClient
 		out, err := sm.GetSecret(context.Background(), *v.ref)
 		if !ErrorContains(err, v.expectError) {
 			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
@@ -209,10 +209,10 @@ func TestGetSecretMap(t *testing.T) {
 		makeValidSecretManagerTestCaseCustom(setNestedJSON),
 	}
 
-	sm := ProviderGCP{}
+	sm := Client{}
 	for k, v := range successCases {
-		sm.projectID = v.projectID
-		sm.SecretManagerClient = v.mockClient
+		sm.store = &esv1beta1.GCPSMProvider{ProjectID: v.projectID}
+		sm.smClient = v.mockClient
 		out, err := sm.GetSecretMap(context.Background(), *v.ref)
 		if !ErrorContains(err, v.expectError) {
 			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
@@ -278,7 +278,7 @@ func TestValidateStore(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			sm := &ProviderGCP{}
+			sm := &Provider{}
 			store := &esv1beta1.SecretStore{
 				Spec: esv1beta1.SecretStoreSpec{
 					Provider: &esv1beta1.SecretStoreProvider{

+ 134 - 0
pkg/provider/gcp/secretmanager/provider.go

@@ -0,0 +1,134 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package secretmanager
+
+import (
+	"context"
+	"fmt"
+	"sync"
+
+	secretmanager "cloud.google.com/go/secretmanager/apiv1"
+	"google.golang.org/api/option"
+	kclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
+	"github.com/external-secrets/external-secrets/pkg/utils"
+)
+
+// Provider is a secrets provider for GCP Secret Manager.
+// It implements the necessary NewClient() and ValidateStore() funcs.
+type Provider struct{}
+
+// https://github.com/external-secrets/external-secrets/issues/644
+var _ esv1beta1.SecretsClient = &Client{}
+var _ esv1beta1.Provider = &Provider{}
+
+func init() {
+	esv1beta1.Register(&Provider{}, &esv1beta1.SecretStoreProvider{
+		GCPSM: &esv1beta1.GCPSMProvider{},
+	})
+}
+
+/*
+Currently, GCPSM client has a limitation around how concurrent connections work
+This limitation causes memory leaks due to random disconnects from living clients
+and also payload switches when sending a call (such as using a credential from one
+thread to ask secrets from another thread).
+A Mutex was implemented to make sure only one connection can be in place at a time.
+*/
+var useMu = sync.Mutex{}
+
+// NewClient constructs a GCP Provider.
+func (p *Provider) NewClient(ctx context.Context, store esv1beta1.GenericStore, kube kclient.Client, namespace string) (esv1beta1.SecretsClient, error) {
+	storeSpec := store.GetSpec()
+	if storeSpec == nil || storeSpec.Provider == nil || storeSpec.Provider.GCPSM == nil {
+		return nil, fmt.Errorf(errGCPSMStore)
+	}
+	gcpStore := storeSpec.Provider.GCPSM
+
+	useMu.Lock()
+
+	client := &Client{
+		kube:      kube,
+		store:     gcpStore,
+		namespace: namespace,
+	}
+	defer func() {
+		if client.smClient == nil {
+			_ = client.Close(ctx)
+		}
+	}()
+
+	// this project ID is used for authentication (currently only relevant for workload identity)
+	clusterProjectID, err := clusterProjectID(storeSpec)
+	if err != nil {
+		return nil, err
+	}
+	isClusterKind := store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind
+	ts, err := NewTokenSource(ctx, gcpStore.Auth, clusterProjectID, isClusterKind, kube, namespace)
+	if err != nil {
+		return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
+	}
+
+	// check if we can get credentials
+	_, err = ts.Token()
+	if err != nil {
+		return nil, fmt.Errorf(errUnableGetCredentials, err)
+	}
+
+	clientGCPSM, err := secretmanager.NewClient(ctx, option.WithTokenSource(ts))
+	if err != nil {
+		return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
+	}
+	client.smClient = clientGCPSM
+	return client, nil
+}
+
+func (p *Provider) ValidateStore(store esv1beta1.GenericStore) error {
+	if store == nil {
+		return fmt.Errorf(errInvalidStore)
+	}
+	spc := store.GetSpec()
+	if spc == nil {
+		return fmt.Errorf(errInvalidStoreSpec)
+	}
+	if spc.Provider == nil {
+		return fmt.Errorf(errInvalidStoreProv)
+	}
+	g := spc.Provider.GCPSM
+	if p == nil {
+		return fmt.Errorf(errInvalidGCPProv)
+	}
+	if g.Auth.SecretRef != nil {
+		if err := utils.ValidateSecretSelector(store, g.Auth.SecretRef.SecretAccessKey); err != nil {
+			return fmt.Errorf(errInvalidAuthSecretRef, err)
+		}
+	}
+	if g.Auth.WorkloadIdentity != nil {
+		if err := utils.ValidateServiceAccountSelector(store, g.Auth.WorkloadIdentity.ServiceAccountRef); err != nil {
+			return fmt.Errorf(errInvalidWISARef, err)
+		}
+	}
+	return nil
+}
+
+func clusterProjectID(spec *esv1beta1.SecretStoreSpec) (string, error) {
+	if spec.Provider.GCPSM.Auth.WorkloadIdentity != nil && spec.Provider.GCPSM.Auth.WorkloadIdentity.ClusterProjectID != "" {
+		return spec.Provider.GCPSM.Auth.WorkloadIdentity.ClusterProjectID, nil
+	} else if spec.Provider.GCPSM.ProjectID != "" {
+		return spec.Provider.GCPSM.ProjectID, nil
+	} else {
+		return "", fmt.Errorf(errNoProjectID)
+	}
+}

+ 13 - 27
pkg/provider/gcp/secretmanager/secretsmanager_workload_identity.go

@@ -58,6 +58,7 @@ type workloadIdentity struct {
 	iamClient            IamClient
 	idBindTokenGenerator idBindTokenGenerator
 	saTokenGenerator     saTokenGenerator
+	clusterProjectID     string
 }
 
 // interface to GCP IAM API.
@@ -76,7 +77,7 @@ type saTokenGenerator interface {
 	Generate(context.Context, []string, string, string) (*authenticationv1.TokenRequest, error)
 }
 
-func newWorkloadIdentity(ctx context.Context) (*workloadIdentity, error) {
+func newWorkloadIdentity(ctx context.Context, projectID string) (*workloadIdentity, error) {
 	iamc, err := newIAMClient(ctx)
 	if err != nil {
 		return nil, err
@@ -89,47 +90,39 @@ func newWorkloadIdentity(ctx context.Context) (*workloadIdentity, error) {
 		iamClient:            iamc,
 		idBindTokenGenerator: newIDBindTokenGenerator(),
 		saTokenGenerator:     satg,
+		clusterProjectID:     projectID,
 	}, nil
 }
 
-func (w *workloadIdentity) TokenSource(ctx context.Context, store esv1beta1.GenericStore, kube kclient.Client, namespace string) (oauth2.TokenSource, error) {
-	spec := store.GetSpec()
-	if spec == nil || spec.Provider == nil || spec.Provider.GCPSM == nil {
-		return nil, fmt.Errorf(errMissingStoreSpec)
-	}
-	wi := spec.Provider.GCPSM.Auth.WorkloadIdentity
+func (w *workloadIdentity) TokenSource(ctx context.Context, auth esv1beta1.GCPSMAuth, isClusterKind bool, kube kclient.Client, namespace string) (oauth2.TokenSource, error) {
+	wi := auth.WorkloadIdentity
 	if wi == nil {
 		return nil, nil
 	}
-	storeKind := store.GetObjectKind().GroupVersionKind().Kind
 	saKey := types.NamespacedName{
 		Name:      wi.ServiceAccountRef.Name,
 		Namespace: namespace,
 	}
 
 	// only ClusterStore is allowed to set namespace (and then it's required)
-	if storeKind == esv1beta1.ClusterSecretStoreKind {
+	if isClusterKind {
 		if wi.ServiceAccountRef.Namespace == nil {
 			return nil, fmt.Errorf(errInvalidClusterStoreMissingSANamespace)
 		}
 		saKey.Namespace = *wi.ServiceAccountRef.Namespace
 	}
 
-	clusterProjectID, err := clusterProjectID(spec)
-	if err != nil {
-		return nil, err
-	}
 	sa := &v1.ServiceAccount{}
-	err = kube.Get(ctx, saKey, sa)
+	err := kube.Get(ctx, saKey, sa)
 	if err != nil {
 		return nil, err
 	}
 
 	idProvider := fmt.Sprintf("https://container.googleapis.com/v1/projects/%s/locations/%s/clusters/%s",
-		clusterProjectID,
+		w.clusterProjectID,
 		wi.ClusterLocation,
 		wi.ClusterName)
-	idPool := fmt.Sprintf("%s.svc.id.goog", clusterProjectID)
+	idPool := fmt.Sprintf("%s.svc.id.goog", w.clusterProjectID)
 	audiences := []string{idPool}
 	if len(wi.ServiceAccountRef.Audiences) > 0 {
 		audiences = append(audiences, wi.ServiceAccountRef.Audiences...)
@@ -165,7 +158,10 @@ func (w *workloadIdentity) TokenSource(ctx context.Context, store esv1beta1.Gene
 }
 
 func (w *workloadIdentity) Close() error {
-	return w.iamClient.Close()
+	if w.iamClient != nil {
+		return w.iamClient.Close()
+	}
+	return nil
 }
 
 func newIAMClient(ctx context.Context) (IamClient, error) {
@@ -266,13 +262,3 @@ func (g *gcpIDBindTokenGenerator) Generate(ctx context.Context, client *http.Cli
 	}
 	return idBindToken, nil
 }
-
-func clusterProjectID(spec *esv1beta1.SecretStoreSpec) (string, error) {
-	if spec.Provider.GCPSM.Auth.WorkloadIdentity.ClusterProjectID != "" {
-		return spec.Provider.GCPSM.Auth.WorkloadIdentity.ClusterProjectID, nil
-	} else if spec.Provider.GCPSM.ProjectID != "" {
-		return spec.Provider.GCPSM.ProjectID, nil
-	} else {
-		return "", fmt.Errorf(errNoProjectID)
-	}
-}

+ 2 - 12
pkg/provider/gcp/secretmanager/secretsmanager_workload_identity_test.go

@@ -52,11 +52,6 @@ func TestWorkloadIdentity(t *testing.T) {
 	clusterSANamespace := "foobar"
 	tbl := []*workloadIdentityTest{
 		composeTestcase(
-			defaultTestCase("missing store spec should result in error"),
-			withErr("invalid: missing store spec"),
-			withStore(&esv1beta1.SecretStore{}),
-		),
-		composeTestcase(
 			defaultTestCase("should skip when no workload identity is configured: TokenSource and error must be nil"),
 			withStore(&esv1beta1.SecretStore{
 				Spec: esv1beta1.SecretStoreSpec{
@@ -137,7 +132,8 @@ func TestWorkloadIdentity(t *testing.T) {
 			cb := clientfake.NewClientBuilder()
 			cb.WithObjects(row.kubeObjects...)
 			client := cb.Build()
-			ts, err := w.TokenSource(context.Background(), row.store, client, "default")
+			isCluster := row.store.GetTypeMeta().Kind == esv1beta1.ClusterSecretStoreKind
+			ts, err := w.TokenSource(context.Background(), row.store.GetSpec().Provider.GCPSM.Auth, isCluster, client, "default")
 			// assert err
 			if row.expErr == "" {
 				assert.NoError(t, err)
@@ -213,12 +209,6 @@ func composeTestcase(tc *workloadIdentityTest, mutators ...testCaseMutator) *wor
 	return tc
 }
 
-func withErr(err string) testCaseMutator {
-	return func(tc *workloadIdentityTest) {
-		tc.expErr = err
-	}
-}
-
 func withStore(store esv1beta1.GenericStore) testCaseMutator {
 	return func(tc *workloadIdentityTest) {
 		tc.store = store