Merge branch 'main' into feature/conversion-webhook

Disabled secrets cache for cert controller.

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>

cmd/certcontroller.go

@@ -21,7 +21,9 @@ import (
 
 	"github.com/spf13/cobra"
 	"go.uber.org/zap/zapcore"
+	v1 "k8s.io/api/core/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
@@ -49,7 +51,15 @@ var certcontrollerCmd = &cobra.Command{
 			Port:               9443,
 			LeaderElection:     enableLeaderElection,
 			LeaderElectionID:   "crd-certs-controller",
-		})
+			ClientDisableCacheFor: []client.Object{
+				// the client creates a ListWatch for all resource kinds that
+				// are requested with .Get().
+				// We want to avoid to cache all secrets or configmaps in memory.
+				// The ES controller uses v1.PartialObjectMetadata for the secrets
+				// that he owns.
+				// see #721
+				&v1.Secret{},
+			}})
 		if err != nil {
 			setupLog.Error(err, "unable to start manager")
 			os.Exit(1)

cmd/root.go

@@ -21,12 +21,14 @@ import (
 
 	"github.com/spf13/cobra"
 	"go.uber.org/zap/zapcore"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 
 	// To allow using gcp auth.
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
@@ -84,7 +86,17 @@ var rootCmd = &cobra.Command{
 			Port:               9443,
 			LeaderElection:     enableLeaderElection,
 			LeaderElectionID:   "external-secrets-controller",
-			Namespace:          namespace,
+			ClientDisableCacheFor: []client.Object{
+				// the client creates a ListWatch for all resource kinds that
+				// are requested with .Get().
+				// We want to avoid to cache all secrets or configmaps in memory.
+				// The ES controller uses v1.PartialObjectMetadata for the secrets
+				// that he owns.
+				// see #721
+				&v1.Secret{},
+				&v1.ConfigMap{},
+			},
+			Namespace: namespace,
 		})
 		if err != nil {
 			setupLog.Error(err, "unable to start manager")

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -432,6 +433,6 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, opts controller.Options)
 	return ctrl.NewControllerManagedBy(mgr).
 		WithOptions(opts).
 		For(&esv1beta1.ExternalSecret{}).
-		Owns(&v1.Secret{}).
+		Owns(&v1.Secret{}, builder.OnlyMetadata).
 		Complete(r)
 }

pkg/provider/azure/keyvault/keyvault.go

@@ -17,6 +17,7 @@ package keyvault
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -35,7 +36,22 @@ import (
 
 const (
 	defaultObjType = "secret"
+	objectTypeCert = "cert"
+	objectTypeKey  = "key"
 	vaultResource  = "https://vault.azure.net"
+
+	errUnexpectedStoreSpec   = "unexpected store spec"
+	errMissingAuthType       = "cannot initialize Azure Client: no valid authType was specified"
+	errPropNotExist          = "property %s does not exist in key %s"
+	errUnknownObjectType     = "unknown Azure Keyvault object Type for %s"
+	errUnmarshalJSONData     = "error unmarshalling json data: %w"
+	errDataFromCert          = "cannot get use dataFrom to get certificate secret"
+	errDataFromKey           = "cannot get use dataFrom to get key secret"
+	errMissingTenant         = "missing tenantID in store config"
+	errMissingSecretRef      = "missing secretRef in provider config"
+	errMissingClientIDSecret = "missing accessKeyID/secretAccessKey in store config"
+	errFindSecret            = "could not find secret %s/%s: %w"
+	errFindDataKey           = "no data for %q in secret '%s/%s'"
 )
 
 // interface to keyvault.BaseClient.
@@ -49,8 +65,8 @@ type SecretClient interface {
 type Azure struct {
 	kube       client.Client
 	store      esv1beta1.GenericStore
+	provider   *esv1beta1.AzureKVProvider
 	baseClient SecretClient
-	vaultURL   string
 	namespace  string
 }
 
@@ -66,23 +82,37 @@ func (a *Azure) NewClient(ctx context.Context, store esv1beta1.GenericStore, kub
 }
 
 func newClient(ctx context.Context, store esv1beta1.GenericStore, kube client.Client, namespace string) (provider.SecretsClient, error) {
-	anAzure := &Azure{
+	provider, err := getProvider(store)
+	if err != nil {
+		return nil, err
+	}
+	az := &Azure{
 		kube:      kube,
 		store:     store,
 		namespace: namespace,
+		provider:  provider,
 	}
 
-	clientSet, err := anAzure.setAzureClientWithManagedIdentity()
-	if clientSet {
-		return anAzure, err
+	ok, err := az.setAzureClientWithManagedIdentity()
+	if ok {
+		return az, err
 	}
 
-	clientSet, err = anAzure.setAzureClientWithServicePrincipal(ctx)
-	if clientSet {
-		return anAzure, err
+	ok, err = az.setAzureClientWithServicePrincipal(ctx)
+	if ok {
+		return az, err
+	}
+
+	return nil, fmt.Errorf(errMissingAuthType)
+}
+
+func getProvider(store esv1beta1.GenericStore) (*esv1beta1.AzureKVProvider, error) {
+	spc := store.GetSpec()
+	if spc == nil || spc.Provider.AzureKV == nil {
+		return nil, errors.New(errUnexpectedStoreSpec)
 	}
 
-	return nil, fmt.Errorf("cannot initialize Azure Client: no valid authType was specified")
+	return spc.Provider.AzureKV, nil
 }
 
 // Empty GetAllSecrets.
@@ -96,7 +126,6 @@ func (a *Azure) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecretF
 // The Object Type is defined as a prefix in the ref.Name , if no prefix is defined , we assume a secret is required.
 func (a *Azure) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	version := ""
-	basicClient := a.baseClient
 	objectType, secretName := getObjType(ref)
 
 	if ref.Version != "" {
@@ -107,7 +136,7 @@ func (a *Azure) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataR
 	case defaultObjType:
 		// returns a SecretBundle with the secret value
 		// https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault#SecretBundle
-		secretResp, err := basicClient.GetSecret(context.Background(), a.vaultURL, secretName, version)
+		secretResp, err := a.baseClient.GetSecret(context.Background(), *a.provider.VaultURL, secretName, version)
 		if err != nil {
 			return nil, err
 		}
@@ -116,29 +145,29 @@ func (a *Azure) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataR
 		}
 		res := gjson.Get(*secretResp.Value, ref.Property)
 		if !res.Exists() {
-			return nil, fmt.Errorf("property %s does not exist in key %s", ref.Property, ref.Key)
+			return nil, fmt.Errorf(errPropNotExist, ref.Property, ref.Key)
 		}
 		return []byte(res.String()), err
-	case "cert":
+	case objectTypeCert:
 		// returns a CertBundle. We return CER contents of x509 certificate
 		// see: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault#CertificateBundle
-		secretResp, err := basicClient.GetCertificate(context.Background(), a.vaultURL, secretName, version)
+		secretResp, err := a.baseClient.GetCertificate(context.Background(), *a.provider.VaultURL, secretName, version)
 		if err != nil {
 			return nil, err
 		}
 		return *secretResp.Cer, nil
-	case "key":
+	case objectTypeKey:
 		// returns a KeyBundle that contains a jwk
 		// azure kv returns only public keys
 		// see: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault#KeyBundle
-		keyResp, err := basicClient.GetKey(context.Background(), a.vaultURL, secretName, version)
+		keyResp, err := a.baseClient.GetKey(context.Background(), *a.provider.VaultURL, secretName, version)
 		if err != nil {
 			return nil, err
 		}
 		return json.Marshal(keyResp.Key)
 	}
 
-	return nil, fmt.Errorf("unknown Azure Keyvault object Type for %s", secretName)
+	return nil, fmt.Errorf(errUnknownObjectType, secretName)
 }
 
 // Implements store.Client.GetSecretMap Interface.
@@ -156,7 +185,7 @@ func (a *Azure) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDa
 		kv := make(map[string]string)
 		err = json.Unmarshal(data, &kv)
 		if err != nil {
-			return nil, fmt.Errorf("error unmarshalling json data: %w", err)
+			return nil, fmt.Errorf(errUnmarshalJSONData, err)
 		}
 
 		secretData := make(map[string][]byte)
@@ -165,83 +194,73 @@ func (a *Azure) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDa
 		}
 
 		return secretData, nil
-	case "cert":
-		return nil, fmt.Errorf("cannot get use dataFrom to get certificate secret")
-	case "key":
-		return nil, fmt.Errorf("cannot get use dataFrom to get key secret")
+	case objectTypeCert:
+		return nil, fmt.Errorf(errDataFromCert)
+	case objectTypeKey:
+		return nil, fmt.Errorf(errDataFromKey)
 	}
 
-	return nil, fmt.Errorf("unknown Azure Keyvault object Type for %s", secretName)
+	return nil, fmt.Errorf(errUnknownObjectType, secretName)
 }
 
 func (a *Azure) setAzureClientWithManagedIdentity() (bool, error) {
-	spec := *a.store.GetSpec().Provider.AzureKV
-
-	if *spec.AuthType != esv1beta1.ManagedIdentity {
+	if *a.provider.AuthType != esv1beta1.ManagedIdentity {
 		return false, nil
 	}
 
 	msiConfig := kvauth.NewMSIConfig()
 	msiConfig.Resource = vaultResource
-	if spec.IdentityID != nil {
-		msiConfig.ClientID = *spec.IdentityID
+	if a.provider.IdentityID != nil {
+		msiConfig.ClientID = *a.provider.IdentityID
 	}
 	authorizer, err := msiConfig.Authorizer()
 	if err != nil {
 		return true, err
 	}
 
-	basicClient := keyvault.New()
-	basicClient.Authorizer = authorizer
-
-	a.baseClient = basicClient
-	a.vaultURL = *spec.VaultURL
-
+	cl := keyvault.New()
+	cl.Authorizer = authorizer
+	a.baseClient = &cl
 	return true, nil
 }
 
 func (a *Azure) setAzureClientWithServicePrincipal(ctx context.Context) (bool, error) {
-	spec := *a.store.GetSpec().Provider.AzureKV
-
-	if *spec.AuthType != esv1beta1.ServicePrincipal {
+	if *a.provider.AuthType != esv1beta1.ServicePrincipal {
 		return false, nil
 	}
 
-	if spec.TenantID == nil {
-		return true, fmt.Errorf("missing tenantID in store config")
+	if a.provider.TenantID == nil {
+		return true, fmt.Errorf(errMissingTenant)
 	}
-	if spec.AuthSecretRef == nil {
-		return true, fmt.Errorf("missing clientID/clientSecret in store config")
+	if a.provider.AuthSecretRef == nil {
+		return true, fmt.Errorf(errMissingSecretRef)
 	}
-	if spec.AuthSecretRef.ClientID == nil || spec.AuthSecretRef.ClientSecret == nil {
-		return true, fmt.Errorf("missing accessKeyID/secretAccessKey in store config")
+	if a.provider.AuthSecretRef.ClientID == nil || a.provider.AuthSecretRef.ClientSecret == nil {
+		return true, fmt.Errorf(errMissingClientIDSecret)
 	}
 	clusterScoped := false
 	if a.store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind {
 		clusterScoped = true
 	}
-	cid, err := a.secretKeyRef(ctx, a.store.GetNamespace(), *spec.AuthSecretRef.ClientID, clusterScoped)
+	cid, err := a.secretKeyRef(ctx, a.store.GetNamespace(), *a.provider.AuthSecretRef.ClientID, clusterScoped)
 	if err != nil {
 		return true, err
 	}
-	csec, err := a.secretKeyRef(ctx, a.store.GetNamespace(), *spec.AuthSecretRef.ClientSecret, clusterScoped)
+	csec, err := a.secretKeyRef(ctx, a.store.GetNamespace(), *a.provider.AuthSecretRef.ClientSecret, clusterScoped)
 	if err != nil {
 		return true, err
 	}
 
-	clientCredentialsConfig := kvauth.NewClientCredentialsConfig(cid, csec, *spec.TenantID)
+	clientCredentialsConfig := kvauth.NewClientCredentialsConfig(cid, csec, *a.provider.TenantID)
 	clientCredentialsConfig.Resource = vaultResource
 	authorizer, err := clientCredentialsConfig.Authorizer()
 	if err != nil {
 		return true, err
 	}
 
-	basicClient := keyvault.New()
-	basicClient.Authorizer = authorizer
-
-	a.baseClient = &basicClient
-	a.vaultURL = *spec.VaultURL
-
+	cl := keyvault.New()
+	cl.Authorizer = authorizer
+	a.baseClient = &cl
 	return true, nil
 }
 
@@ -256,11 +275,11 @@ func (a *Azure) secretKeyRef(ctx context.Context, namespace string, secretRef sm
 	}
 	err := a.kube.Get(ctx, ref, &secret)
 	if err != nil {
-		return "", fmt.Errorf("could not find secret %s/%s: %w", ref.Namespace, ref.Name, err)
+		return "", fmt.Errorf(errFindSecret, ref.Namespace, ref.Name, err)
 	}
 	keyBytes, ok := secret.Data[secretRef.Key]
 	if !ok {
-		return "", fmt.Errorf("no data for %q in secret '%s/%s'", secretRef.Key, secretRef.Name, namespace)
+		return "", fmt.Errorf(errFindDataKey, secretRef.Key, secretRef.Name, namespace)
 	}
 	value := strings.TrimSpace(string(keyBytes))
 	return value, nil

pkg/provider/azure/keyvault/keyvault_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault"
 	tassert "github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 	clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
@@ -130,7 +131,7 @@ func TestNewClientNoCreds(t *testing.T) {
 	tassert.Nil(t, err, "the return err should be nil")
 	k8sClient := clientfake.NewClientBuilder().Build()
 	_, err = provider.NewClient(context.Background(), &store, k8sClient, namespace)
-	tassert.EqualError(t, err, "missing clientID/clientSecret in store config")
+	tassert.EqualError(t, err, "missing secretRef in provider config")
 
 	store.Spec.Provider.AzureKV.AuthSecretRef = &esv1beta1.AzureKVAuth{}
 	_, err = provider.NewClient(context.Background(), &store, k8sClient, namespace)
@@ -263,7 +264,9 @@ func TestAzureKeyVaultSecretManagerGetSecret(t *testing.T) {
 		makeValidSecretManagerTestCaseCustom(badSecretType),
 	}
 
-	sm := Azure{}
+	sm := Azure{
+		provider: &esv1beta1.AzureKVProvider{VaultURL: pointer.StringPtr("noop")},
+	}
 	for k, v := range successCases {
 		sm.baseClient = v.mockClient
 		out, err := sm.GetSecret(context.Background(), *v.ref)
@@ -357,7 +360,9 @@ func TestAzureKeyVaultSecretManagerGetSecretMap(t *testing.T) {
 		makeValidSecretManagerTestCaseCustom(badSecretType),
 	}
 
-	sm := Azure{}
+	sm := Azure{
+		provider: &esv1beta1.AzureKVProvider{VaultURL: pointer.StringPtr("noop")},
+	}
 	for k, v := range successCases {
 		sm.baseClient = v.mockClient
 		out, err := sm.GetSecretMap(context.Background(), *v.ref)