diff --git a/deploy/charts/external-secrets/templates/cert-controller-service.yaml b/deploy/charts/external-secrets/templates/cert-controller-service.yaml index 3b63a59b0..aafdb1d48 100644 --- a/deploy/charts/external-secrets/templates/cert-controller-service.yaml +++ b/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: diff --git a/deploy/charts/external-secrets/templates/cert-controller-serviceaccount.yaml b/deploy/charts/external-secrets/templates/cert-controller-serviceaccount.yaml index 2823d6a91..acf7d6dea 100644 --- a/deploy/charts/external-secrets/templates/cert-controller-serviceaccount.yaml +++ b/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: diff --git a/deploy/charts/external-secrets/templates/validatingwebhook.yaml b/deploy/charts/external-secrets/templates/validatingwebhook.yaml index c50325035..d81171b2e 100644 --- a/deploy/charts/external-secrets/templates/validatingwebhook.yaml +++ b/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 }} diff --git a/deploy/charts/external-secrets/templates/webhook-secret.yaml b/deploy/charts/external-secrets/templates/webhook-secret.yaml index c20ac74c6..1ec25c31e 100644 --- a/deploy/charts/external-secrets/templates/webhook-secret.yaml +++ b/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 }} \ No newline at end of file diff --git a/deploy/charts/external-secrets/templates/webhook-service.yaml b/deploy/charts/external-secrets/templates/webhook-service.yaml index 548b4fa14..5992ddf25 100644 --- a/deploy/charts/external-secrets/templates/webhook-service.yaml +++ b/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 }} \ No newline at end of file diff --git a/deploy/charts/external-secrets/templates/webhook-serviceaccount.yaml b/deploy/charts/external-secrets/templates/webhook-serviceaccount.yaml index 295d9f3c4..5a8b95917 100644 --- a/deploy/charts/external-secrets/templates/webhook-serviceaccount.yaml +++ b/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: diff --git a/e2e/framework/addon/eso.go b/e2e/framework/addon/eso.go index c7a5fd8d7..f207bdac1 100644 --- a/e2e/framework/addon/eso.go +++ b/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{ diff --git a/e2e/suite/aws/parameterstore/parameterstore_managed.go b/e2e/suite/aws/parameterstore/parameterstore_managed.go index 5b611cd65..cb0a59bdd 100644 --- a/e2e/suite/aws/parameterstore/parameterstore_managed.go +++ b/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(), )) }) diff --git a/e2e/suite/aws/secretsmanager/secretsmanager_managed.go b/e2e/suite/aws/secretsmanager/secretsmanager_managed.go index 84f2e9509..c84c8d3fe 100644 --- a/e2e/suite/aws/secretsmanager/secretsmanager_managed.go +++ b/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(), )) }) diff --git a/e2e/suite/gcp/gcp_managed.go b/e2e/suite/gcp/gcp_managed.go index 0f8ee4e75..c421da79b 100644 --- a/e2e/suite/gcp/gcp_managed.go +++ b/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(), )) }) diff --git a/pkg/controllers/secretstore/common.go b/pkg/controllers/secretstore/common.go index fa5ed4a60..cfb5d86f7 100644 --- a/pkg/controllers/secretstore/common.go +++ b/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 { diff --git a/pkg/provider/gcp/secretmanager/secretsmanager.go b/pkg/provider/gcp/secretmanager/secretsmanager.go index dbdd14d2a..681d15196 100644 --- a/pkg/provider/gcp/secretmanager/secretsmanager.go +++ b/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) }