Merge pull request #873 from gusfcarvalho/fix/mutex-on-gcp

Adding mutexes only for GCP provider

deploy/charts/external-secrets/templates/cert-controller-service.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.certController.prometheus.enabled }}
+{{- if and .Values.certController.create .Values.certController.prometheus.enabled }}
 apiVersion: v1
 kind: Service
 metadata:

deploy/charts/external-secrets/templates/cert-controller-serviceaccount.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.certController.serviceAccount.create -}}
+{{- if and .Values.certController.create .Values.certController.serviceAccount.create -}}
 apiVersion: v1
 kind: ServiceAccount
 metadata:

deploy/charts/external-secrets/templates/validatingwebhook.yaml

@@ -1,3 +1,4 @@
+{{- if .Values.webhook.create }}
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
@@ -39,3 +40,4 @@ webhooks:
   admissionReviewVersions: ["v1", "v1beta1"]
   sideEffects: None
   timeoutSeconds: 5
+{{- end }}

deploy/charts/external-secrets/templates/webhook-secret.yaml

@@ -1,3 +1,4 @@
+{{- if .Values.webhook.create }}
 apiVersion: v1
 kind: Secret
 metadata:
@@ -6,3 +7,4 @@ metadata:
   labels:
     {{- include "external-secrets-webhook.labels" . | nindent 4 }}
     external-secrets.io/component : webhook
+{{- end }}

deploy/charts/external-secrets/templates/webhook-service.yaml

@@ -1,3 +1,4 @@
+{{- if .Values.webhook.create }}
 apiVersion: v1
 kind: Service
 metadata:
@@ -27,3 +28,4 @@ spec:
   {{- end }}
   selector:
     {{- include "external-secrets-webhook.selectorLabels" . | nindent 4 }}
+{{- end }}

deploy/charts/external-secrets/templates/webhook-serviceaccount.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.webhook.serviceAccount.create -}}
+{{- if and .Values.webhook.create .Values.webhook.serviceAccount.create -}}
 apiVersion: v1
 kind: ServiceAccount
 metadata:

e2e/framework/addon/eso.go

@@ -93,6 +93,24 @@ func WithNamespaceScope(namespace string) MutationFunc {
 	}
 }
 
+func WithoutWebhook() MutationFunc {
+	return func(eso *ESO) {
+		eso.HelmChart.Vars = append(eso.HelmChart.Vars, StringTuple{
+			Key:   "webhook.create",
+			Value: "false",
+		})
+	}
+}
+
+func WithoutCertController() MutationFunc {
+	return func(eso *ESO) {
+		eso.HelmChart.Vars = append(eso.HelmChart.Vars, StringTuple{
+			Key:   "certController.create",
+			Value: "false",
+		})
+	}
+}
+
 func WithServiceAccount(saName string) MutationFunc {
 	return func(eso *ESO) {
 		eso.HelmChart.Vars = append(eso.HelmChart.Vars, []StringTuple{

e2e/suite/aws/parameterstore/parameterstore_managed.go

@@ -63,6 +63,8 @@ var _ = Describe("[awsmanaged] with mounted IRSA", Label("aws", "parameterstore"
 			addon.WithServiceAccount(prov.ServiceAccountName),
 			addon.WithReleaseName(f.Namespace.Name),
 			addon.WithNamespace("default"),
+			addon.WithoutWebhook(),
+			addon.WithoutCertController(),
 		))
 	})
 

e2e/suite/aws/secretsmanager/secretsmanager_managed.go

@@ -63,6 +63,8 @@ var _ = Describe("[awsmanaged] with mounted IRSA", Label("aws", "secretsmanager"
 			addon.WithServiceAccount(prov.ServiceAccountName),
 			addon.WithReleaseName(f.Namespace.Name),
 			addon.WithNamespace("default"),
+			addon.WithoutWebhook(),
+			addon.WithoutCertController(),
 		))
 	})
 

e2e/suite/gcp/gcp_managed.go

@@ -44,6 +44,8 @@ var _ = Describe("[gcpmanaged] with pod identity", Label("gcp", "secretsmanager"
 			addon.WithServiceAccount(prov.ServiceAccountName),
 			addon.WithReleaseName(f.Namespace.Name),
 			addon.WithNamespace("default"),
+			addon.WithoutWebhook(),
+			addon.WithoutCertController(),
 		))
 	})
 
@@ -78,6 +80,8 @@ var _ = Describe("[gcpmanaged] with service account", Label("gcp", "secretsmanag
 			addon.WithControllerClass(f.BaseName),
 			addon.WithReleaseName(f.Namespace.Name),
 			addon.WithNamespace(f.Namespace.Name),
+			addon.WithoutWebhook(),
+			addon.WithoutCertController(),
 		))
 	})
 

pkg/controllers/secretstore/common.go

@@ -91,6 +91,7 @@ func validateStore(ctx context.Context, namespace string, store esapi.GenericSto
 		recorder.Event(store, v1.EventTypeWarning, esapi.ReasonInvalidProviderConfig, err.Error())
 		return fmt.Errorf(errStoreClient, err)
 	}
+	defer cl.Close(ctx)
 
 	err = cl.Validate()
 	if err != nil {

pkg/provider/gcp/secretmanager/secretsmanager.go

@@ -17,6 +17,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"sync"
 
 	secretmanager "cloud.google.com/go/secretmanager/apiv1"
 	"github.com/googleapis/gax-go/v2"
@@ -64,6 +65,15 @@ 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{}
+
 // ProviderGCP is a provider for GCP Secret Manager.
 type ProviderGCP struct {
 	projectID           string
@@ -144,8 +154,10 @@ func (sm *ProviderGCP) NewClient(ctx context.Context, store esv1beta1.GenericSto
 	}
 	storeSpecGCPSM := storeSpec.Provider.GCPSM
 
+	useMu.Lock()
 	wi, err := newWorkloadIdentity(ctx)
 	if err != nil {
+		useMu.Unlock()
 		return nil, fmt.Errorf("unable to initialize workload identity")
 	}
 
@@ -168,17 +180,20 @@ func (sm *ProviderGCP) NewClient(ctx context.Context, store esv1beta1.GenericSto
 
 	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
@@ -265,6 +280,7 @@ func (sm *ProviderGCP) Close(ctx context.Context) error {
 	if sm.gClient != nil {
 		err = sm.gClient.Close()
 	}
+	useMu.Unlock()
 	if err != nil {
 		return fmt.Errorf(errClientClose, err)
 	}