Browse Source

feat: fix a bunch of Sonar issues (#4208)

* feat: fix a bunch of Sonar issues

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* fix build issues and correct two more sonarcloud issues

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* fix the unit test and the bitwarden refactor and use an index in the loop

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* added even more changes and refactors

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* refactor fetchSecretData

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* fix the test typo

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* fix dockerfile and increase python version

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

---------

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Gergely Brautigam 1 year ago
parent
commit
05a1814b1b
52 changed files with 988 additions and 699 deletions
  1. 3 6
      Dockerfile.ubi
  2. 6 5
      apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go
  3. 47 14
      apis/externalsecrets/v1beta1/externalsecret_validator.go
  4. 7 3
      apis/externalsecrets/v1beta1/externalsecret_validator_test.go
  5. 15 3
      cmd/certcontroller.go
  6. 1 1
      deploy/charts/external-secrets/templates/deployment.yaml
  7. 1 1
      e2e/Dockerfile
  8. 2 1
      e2e/framework/addon/conjur.go
  9. 7 4
      e2e/framework/addon/eso.go
  10. 32 24
      e2e/suites/provider/cases/aws/common.go
  11. 2 2
      e2e/suites/provider/cases/aws/parameterstore/provider.go
  12. 15 4
      e2e/suites/provider/cases/aws/secretsmanager/provider.go
  13. 8 8
      hack/api-docs/Dockerfile
  14. 1 1
      overrides/main.html
  15. 58 26
      pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller.go
  16. 30 19
      pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller_test.go
  17. 14 6
      pkg/controllers/crds/crds_controller.go
  18. 8 2
      pkg/controllers/crds/suite_test.go
  19. 27 18
      pkg/controllers/externalsecret/externalsecret_controller_test.go
  20. 12 12
      pkg/controllers/pushsecret/pushsecret_controller.go
  21. 4 11
      pkg/controllers/pushsecret/pushsecret_controller_test.go
  22. 6 1
      pkg/controllers/secretstore/clustersecretstore_controller.go
  23. 14 8
      pkg/controllers/secretstore/common.go
  24. 6 1
      pkg/controllers/secretstore/secretstore_controller.go
  25. 7 1
      pkg/controllers/webhookconfig/suite_test.go
  26. 14 8
      pkg/controllers/webhookconfig/webhookconfig.go
  27. 28 17
      pkg/generator/vault/vault.go
  28. 36 34
      pkg/provider/akeyless/akeyless.go
  29. 30 27
      pkg/provider/akeyless/akeyless_api.go
  30. 3 3
      pkg/provider/akeyless/akeyless_test.go
  31. 42 39
      pkg/provider/aws/parameterstore/parameterstore.go
  32. 52 48
      pkg/provider/aws/secretsmanager/secretsmanager.go
  33. 7 6
      pkg/provider/aws/secretsmanager/secretsmanager_test.go
  34. 7 6
      pkg/provider/azure/keyvault/keyvault.go
  35. 51 48
      pkg/provider/azure/keyvault/keyvault_test.go
  36. 6 4
      pkg/provider/bitwarden/bitwarden_sdk.go
  37. 1 1
      pkg/provider/bitwarden/bitwarden_sdk_test.go
  38. 40 26
      pkg/provider/bitwarden/client.go
  39. 62 56
      pkg/provider/bitwarden/client_test.go
  40. 10 10
      pkg/provider/gcp/secretmanager/client.go
  41. 30 27
      pkg/provider/gcp/secretmanager/client_test.go
  42. 76 47
      pkg/provider/gitlab/gitlab.go
  43. 23 0
      pkg/provider/gitlab/provider.go
  44. 20 25
      pkg/provider/ibm/provider.go
  45. 2 12
      pkg/provider/ibm/provider_test.go
  46. 11 10
      pkg/provider/kubernetes/auth_test.go
  47. 74 43
      pkg/provider/oracle/oracle.go
  48. 1 1
      pkg/provider/oracle/oracle_test.go
  49. 18 11
      pkg/provider/passbolt/passbolt_test.go
  50. 3 2
      pkg/provider/vault/auth.go
  51. 7 3
      pkg/provider/vault/fake/vault.go
  52. 1 3
      tilt.debug.dockerfile

+ 3 - 6
Dockerfile.ubi

@@ -1,4 +1,4 @@
-FROM registry.access.redhat.com/ubi8/ubi as minimal-ubi
+FROM registry.redhat.io/ubi8/ubi@sha256:17b8ee77f5c03bedf40bfe23557d35f2c2f67be11b78a8a7a6aec0db4d818a25 AS minimal-ubi
 
 ARG TARGETOS
 ARG TARGETARCH
@@ -16,10 +16,7 @@ COPY ubi-build-files-${TARGETARCH}.txt /tmp
 # Copy all the required files from the base UBI image into the image directory
 # As the go binary is not statically compiled this includes everything needed for CGO to work, cacerts, tzdata and RH release files
 RUN tar cf /tmp/files.tar -T /tmp/ubi-build-files-${TARGETARCH}.txt && tar xf /tmp/files.tar -C /image/ \
-  && strip --strip-unneeded /image/usr/lib64/*[0-9].so
-
-# Generate a rpm database which contains all the packages that you said were needed in ubi-build-files-*.txt
-RUN rpm --root /image --initdb \
+  && strip --strip-unneeded /image/usr/lib64/*[0-9].so && rpm --root /image --initdb \
   && PACKAGES=$(rpm -qf $(cat /tmp/ubi-build-files-${TARGETARCH}.txt) | grep -v "is not owned by any package" | sort -u) \
   && echo dnf install -y 'dnf-command(download)' \
   && dnf download --destdir / ${PACKAGES} \
@@ -32,4 +29,4 @@ USER 65534
 ARG TARGETOS
 ARG TARGETARCH
 COPY bin/external-secrets-${TARGETOS}-${TARGETARCH} /bin/external-secrets
-ENTRYPOINT ["/bin/external-secrets"]
+ENTRYPOINT ["/bin/external-secrets"]

+ 6 - 5
apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go

@@ -25,7 +25,8 @@ import (
 )
 
 const (
-	keyName = "my-key"
+	keyName    = "my-key"
+	testTarget = "test-target"
 )
 
 func newExternalSecretV1Alpha1() *ExternalSecret {
@@ -45,7 +46,7 @@ func newExternalSecretV1Alpha1() *ExternalSecret {
 				},
 			},
 			Binding: corev1.LocalObjectReference{
-				Name: "test-target",
+				Name: testTarget,
 			},
 		},
 		Spec: ExternalSecretSpec{
@@ -54,7 +55,7 @@ func newExternalSecretV1Alpha1() *ExternalSecret {
 				Kind: "ClusterSecretStore",
 			},
 			Target: ExternalSecretTarget{
-				Name:           "test-target",
+				Name:           testTarget,
 				CreationPolicy: Owner,
 				Immutable:      false,
 				Template: &ExternalSecretTemplate{
@@ -130,7 +131,7 @@ func newExternalSecretV1Beta1() *esv1beta1.ExternalSecret {
 				},
 			},
 			Binding: corev1.LocalObjectReference{
-				Name: "test-target",
+				Name: testTarget,
 			},
 		},
 		Spec: esv1beta1.ExternalSecretSpec{
@@ -139,7 +140,7 @@ func newExternalSecretV1Beta1() *esv1beta1.ExternalSecret {
 				Kind: "ClusterSecretStore",
 			},
 			Target: esv1beta1.ExternalSecretTarget{
-				Name:           "test-target",
+				Name:           testTarget,
 				CreationPolicy: esv1beta1.CreatePolicyOwner,
 				Immutable:      false,
 				Template: &esv1beta1.ExternalSecretTemplate{

+ 47 - 14
apis/externalsecrets/v1beta1/externalsecret_validator.go

@@ -44,13 +44,8 @@ func validateExternalSecret(obj runtime.Object) (admission.Warnings, error) {
 	}
 
 	var errs error
-	if (es.Spec.Target.DeletionPolicy == DeletionPolicyDelete && es.Spec.Target.CreationPolicy == CreatePolicyMerge) ||
-		(es.Spec.Target.DeletionPolicy == DeletionPolicyDelete && es.Spec.Target.CreationPolicy == CreatePolicyNone) {
-		errs = errors.Join(errs, errors.New("deletionPolicy=Delete must not be used when the controller doesn't own the secret. Please set creationPolicy=Owner"))
-	}
-
-	if es.Spec.Target.DeletionPolicy == DeletionPolicyMerge && es.Spec.Target.CreationPolicy == CreatePolicyNone {
-		errs = errors.Join(errs, errors.New("deletionPolicy=Merge must not be used with creationPolicy=None. There is no Secret to merge with"))
+	if err := validatePolicies(es); err != nil {
+		errs = errors.Join(errs, err)
 	}
 
 	if len(es.Spec.Data) == 0 && len(es.Spec.DataFrom) == 0 {
@@ -58,17 +53,16 @@ func validateExternalSecret(obj runtime.Object) (admission.Warnings, error) {
 	}
 
 	for _, ref := range es.Spec.DataFrom {
-		generatorRef := ref.SourceRef != nil && ref.SourceRef.GeneratorRef != nil
-		if (ref.Find != nil && (ref.Extract != nil || generatorRef)) || (ref.Extract != nil && (ref.Find != nil || generatorRef)) || (generatorRef && (ref.Find != nil || ref.Extract != nil)) {
-			errs = errors.Join(errs, errors.New("extract, find, or generatorRef cannot be set at the same time"))
+		if err := validateExtractFindGenerator(ref); err != nil {
+			errs = errors.Join(errs, err)
 		}
 
-		if ref.Find == nil && ref.Extract == nil && ref.SourceRef == nil {
-			errs = errors.Join(errs, errors.New("either extract, find, or sourceRef must be set to dataFrom"))
+		if err := validateFindExtractSourceRef(ref); err != nil {
+			errs = errors.Join(errs, err)
 		}
 
-		if ref.SourceRef != nil && ref.SourceRef.GeneratorRef == nil && ref.SourceRef.SecretStoreRef == nil {
-			errs = errors.Join(errs, errors.New("generatorRef or storeRef must be set when using sourceRef in dataFrom"))
+		if err := validateSourceRef(ref); err != nil {
+			errs = errors.Join(errs, err)
 		}
 	}
 
@@ -76,6 +70,45 @@ func validateExternalSecret(obj runtime.Object) (admission.Warnings, error) {
 	return nil, errs
 }
 
+func validateSourceRef(ref ExternalSecretDataFromRemoteRef) error {
+	if ref.SourceRef != nil && ref.SourceRef.GeneratorRef == nil && ref.SourceRef.SecretStoreRef == nil {
+		return errors.New("generatorRef or storeRef must be set when using sourceRef in dataFrom")
+	}
+
+	return nil
+}
+
+func validateFindExtractSourceRef(ref ExternalSecretDataFromRemoteRef) error {
+	if ref.Find == nil && ref.Extract == nil && ref.SourceRef == nil {
+		return errors.New("either extract, find, or sourceRef must be set to dataFrom")
+	}
+
+	return nil
+}
+
+func validateExtractFindGenerator(ref ExternalSecretDataFromRemoteRef) error {
+	generatorRef := ref.SourceRef != nil && ref.SourceRef.GeneratorRef != nil
+	if (ref.Find != nil && (ref.Extract != nil || generatorRef)) || (ref.Extract != nil && (ref.Find != nil || generatorRef)) || (generatorRef && (ref.Find != nil || ref.Extract != nil)) {
+		return errors.New("extract, find, or generatorRef cannot be set at the same time")
+	}
+
+	return nil
+}
+
+func validatePolicies(es *ExternalSecret) error {
+	var errs error
+	if (es.Spec.Target.DeletionPolicy == DeletionPolicyDelete && es.Spec.Target.CreationPolicy == CreatePolicyMerge) ||
+		(es.Spec.Target.DeletionPolicy == DeletionPolicyDelete && es.Spec.Target.CreationPolicy == CreatePolicyNone) {
+		errs = errors.Join(errs, errors.New("deletionPolicy=Delete must not be used when the controller doesn't own the secret. Please set creationPolicy=Owner"))
+	}
+
+	if es.Spec.Target.DeletionPolicy == DeletionPolicyMerge && es.Spec.Target.CreationPolicy == CreatePolicyNone {
+		errs = errors.Join(errs, errors.New("deletionPolicy=Merge must not be used with creationPolicy=None. There is no Secret to merge with"))
+	}
+
+	return errs
+}
+
 func validateDuplicateKeys(es *ExternalSecret, errs error) error {
 	if es.Spec.Target.DeletionPolicy == DeletionPolicyRetain {
 		seenKeys := make(map[string]struct{})

+ 7 - 3
apis/externalsecrets/v1beta1/externalsecret_validator_test.go

@@ -20,6 +20,10 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+const (
+	errExtractFindGenerator = "extract, find, or generatorRef cannot be set at the same time"
+)
+
 func TestValidateExternalSecret(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -80,7 +84,7 @@ func TestValidateExternalSecret(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "extract, find, or generatorRef cannot be set at the same time",
+			expectedErr: errExtractFindGenerator,
 		},
 		{
 			name: "generator with find",
@@ -96,7 +100,7 @@ func TestValidateExternalSecret(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "extract, find, or generatorRef cannot be set at the same time",
+			expectedErr: errExtractFindGenerator,
 		},
 		{
 			name: "generator with extract",
@@ -112,7 +116,7 @@ func TestValidateExternalSecret(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "extract, find, or generatorRef cannot be set at the same time",
+			expectedErr: errExtractFindGenerator,
 		},
 		{
 			name: "empty dataFrom",

+ 15 - 3
cmd/certcontroller.go

@@ -115,7 +115,14 @@ var certcontrollerCmd = &cobra.Command{
 
 		crdctrl := crds.New(mgr.GetClient(), mgr.GetScheme(), mgr.Elected(),
 			ctrl.Log.WithName("controllers").WithName("webhook-certs-updater"),
-			crdRequeueInterval, serviceName, serviceNamespace, secretName, secretNamespace, crdNames)
+			crdRequeueInterval,
+			crds.Opts{
+				SvcName:         serviceName,
+				SvcNamespace:    serviceNamespace,
+				SecretName:      secretName,
+				SecretNamespace: secretNamespace,
+				Resources:       crdNames,
+			})
 		if err := crdctrl.SetupWithManager(mgr, controller.Options{
 			MaxConcurrentReconciles: concurrent,
 		}); err != nil {
@@ -125,8 +132,13 @@ var certcontrollerCmd = &cobra.Command{
 
 		whc := webhookconfig.New(mgr.GetClient(), mgr.GetScheme(), mgr.Elected(),
 			ctrl.Log.WithName("controllers").WithName("webhook-certs-updater"),
-			serviceName, serviceNamespace,
-			secretName, secretNamespace, crdRequeueInterval)
+			webhookconfig.Opts{
+				SvcName:         serviceName,
+				SvcNamespace:    serviceNamespace,
+				SecretName:      secretName,
+				SecretNamespace: secretNamespace,
+				RequeueInterval: crdRequeueInterval,
+			})
 		if err := whc.SetupWithManager(mgr, controller.Options{
 			MaxConcurrentReconciles: concurrent,
 		}); err != nil {

+ 1 - 1
deploy/charts/external-secrets/templates/deployment.yaml

@@ -110,7 +110,7 @@ spec:
           {{- toYaml .Values.extraVolumeMounts | nindent 12 }}
           {{- end }}
         {{- if .Values.extraContainers }}
-          {{ toYaml .Values.extraContainers | nindent 8}}
+          {{ toYaml .Values.extraContainers | nindent 8 }}
         {{- end }}
       dnsPolicy: {{ .Values.dnsPolicy }}
       {{- if .Values.dnsConfig }}

+ 1 - 1
e2e/Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.23.4-bookworm@sha256:ef30001eeadd12890c7737c26f3be5b3a8479ccdcdc553b999c84879875a27ce as builder
+FROM golang:1.23.4-bookworm@sha256:ef30001eeadd12890c7737c26f3be5b3a8479ccdcdc553b999c84879875a27ce AS builder
 
 ENV KUBECTL_VERSION="v1.28.3"
 ENV HELM_VERSION="v3.13.1"

+ 2 - 1
e2e/framework/addon/conjur.go

@@ -11,6 +11,7 @@ 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 addon
 
 import (
@@ -127,7 +128,7 @@ func (l *Conjur) initConjur() error {
 		return fmt.Errorf("error fetching admin API key: %w", err)
 	}
 
-	// TODO: ExecCmdWithContainer includes the StdErr output with a warning about config directory.
+	// Note: ExecCmdWithContainer includes the StdErr output with a warning about config directory.
 	// Therefore we need to split the output and only use the first line.
 	l.AdminApiKey = strings.Split(apiKey, "\n")[0]
 

+ 7 - 4
e2e/framework/addon/eso.go

@@ -25,7 +25,10 @@ type ESO struct {
 	*HelmChart
 }
 
-const installCRDsVar = "installCRDs"
+const (
+	installCRDsVar = "installCRDs"
+	esoImage       = "ghcr.io/external-secrets/external-secrets"
+)
 
 func NewESO(mutators ...MutationFunc) *ESO {
 	eso := &ESO{
@@ -44,7 +47,7 @@ func NewESO(mutators ...MutationFunc) *ESO {
 				},
 				{
 					Key:   "webhook.image.repository",
-					Value: "ghcr.io/external-secrets/external-secrets",
+					Value: esoImage,
 				},
 				{
 					Key:   "certController.image.tag",
@@ -52,7 +55,7 @@ func NewESO(mutators ...MutationFunc) *ESO {
 				},
 				{
 					Key:   "certController.image.repository",
-					Value: "ghcr.io/external-secrets/external-secrets",
+					Value: esoImage,
 				},
 				{
 					Key:   "image.tag",
@@ -60,7 +63,7 @@ func NewESO(mutators ...MutationFunc) *ESO {
 				},
 				{
 					Key:   "image.repository",
-					Value: "ghcr.io/external-secrets/external-secrets",
+					Value: esoImage,
 				},
 				{
 					Key:   "extraArgs.loglevel",

+ 32 - 24
e2e/suites/provider/cases/aws/common.go

@@ -94,9 +94,17 @@ func newStaticStoreProvider(serviceType esv1beta1.AWSServiceType, region, secret
 	}
 }
 
-// SessionTagsStore is namespaced and references
-// static credentials from a secret. It assumes a role and specifies session tags
-func SetupSessionTagsStore(f *framework.Framework, kid, sak, st, region, role string, sessionTags []*esv1beta1.Tag, serviceType esv1beta1.AWSServiceType) {
+type AccessOpts struct {
+	KID    string
+	SAK    string
+	ST     string
+	Region string
+	Role   string
+}
+
+// SetupSessionTagsStore is namespaced and references
+// static credentials from a secret. It assumes a Role and specifies session tags
+func SetupSessionTagsStore(f *framework.Framework, access AccessOpts, sessionTags []*esv1beta1.Tag, serviceType esv1beta1.AWSServiceType) {
 	credsName := "provider-secret-sess-tags"
 	awsCreds := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -104,9 +112,9 @@ func SetupSessionTagsStore(f *framework.Framework, kid, sak, st, region, role st
 			Namespace: f.Namespace.Name,
 		},
 		StringData: map[string]string{
-			staticKeyID:           kid,
-			staticSecretAccessKey: sak,
-			staticySessionToken:   st,
+			staticKeyID:           access.KID,
+			staticSecretAccessKey: access.SAK,
+			staticySessionToken:   access.ST,
 		},
 	}
 	err := f.CRClient.Create(context.Background(), awsCreds)
@@ -118,16 +126,16 @@ func SetupSessionTagsStore(f *framework.Framework, kid, sak, st, region, role st
 			Namespace: f.Namespace.Name,
 		},
 		Spec: esv1beta1.SecretStoreSpec{
-			Provider: newStaticStoreProvider(serviceType, region, credsName, role, "", sessionTags),
+			Provider: newStaticStoreProvider(serviceType, access.Region, credsName, access.Role, "", sessionTags),
 		},
 	}
 	err = f.CRClient.Create(context.Background(), secretStore)
 	Expect(err).ToNot(HaveOccurred())
 }
 
-// ExternalIDStore is namespaced and references
+// SetupExternalIDStore is namespaced and references
 // static credentials from a secret. It assumes a role and specifies an externalID
-func SetupExternalIDStore(f *framework.Framework, kid, sak, st, region, role, externalID string, sessionTags []*esv1beta1.Tag, serviceType esv1beta1.AWSServiceType) {
+func SetupExternalIDStore(f *framework.Framework, access AccessOpts, externalID string, sessionTags []*esv1beta1.Tag, serviceType esv1beta1.AWSServiceType) {
 	credsName := "provider-secret-ext-id"
 	awsCreds := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -135,9 +143,9 @@ func SetupExternalIDStore(f *framework.Framework, kid, sak, st, region, role, ex
 			Namespace: f.Namespace.Name,
 		},
 		StringData: map[string]string{
-			staticKeyID:           kid,
-			staticSecretAccessKey: sak,
-			staticySessionToken:   st,
+			staticKeyID:           access.KID,
+			staticSecretAccessKey: access.SAK,
+			staticySessionToken:   access.ST,
 		},
 	}
 	err := f.CRClient.Create(context.Background(), awsCreds)
@@ -149,25 +157,25 @@ func SetupExternalIDStore(f *framework.Framework, kid, sak, st, region, role, ex
 			Namespace: f.Namespace.Name,
 		},
 		Spec: esv1beta1.SecretStoreSpec{
-			Provider: newStaticStoreProvider(serviceType, region, credsName, role, externalID, sessionTags),
+			Provider: newStaticStoreProvider(serviceType, access.Region, credsName, access.Role, externalID, sessionTags),
 		},
 	}
 	err = f.CRClient.Create(context.Background(), secretStore)
 	Expect(err).ToNot(HaveOccurred())
 }
 
-// StaticStore is namespaced and references
+// SetupStaticStore is namespaced and references
 // static credentials from a secret.
-func SetupStaticStore(f *framework.Framework, kid, sak, st, region string, serviceType esv1beta1.AWSServiceType) {
+func SetupStaticStore(f *framework.Framework, access AccessOpts, serviceType esv1beta1.AWSServiceType) {
 	awsCreds := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      StaticCredentialsSecretName,
 			Namespace: f.Namespace.Name,
 		},
 		StringData: map[string]string{
-			staticKeyID:           kid,
-			staticSecretAccessKey: sak,
-			staticySessionToken:   st,
+			staticKeyID:           access.KID,
+			staticSecretAccessKey: access.SAK,
+			staticySessionToken:   access.ST,
 		},
 	}
 	err := f.CRClient.Create(context.Background(), awsCreds)
@@ -179,7 +187,7 @@ func SetupStaticStore(f *framework.Framework, kid, sak, st, region string, servi
 			Namespace: f.Namespace.Name,
 		},
 		Spec: esv1beta1.SecretStoreSpec{
-			Provider: newStaticStoreProvider(serviceType, region, StaticCredentialsSecretName, "", "", nil),
+			Provider: newStaticStoreProvider(serviceType, access.Region, StaticCredentialsSecretName, "", "", nil),
 		},
 	}
 	err = f.CRClient.Create(context.Background(), secretStore)
@@ -188,7 +196,7 @@ func SetupStaticStore(f *framework.Framework, kid, sak, st, region string, servi
 
 // CreateReferentStaticStore creates a CSS with referent auth and
 // creates a secret with static authentication credentials in the ExternalSecret namespace.
-func CreateReferentStaticStore(f *framework.Framework, kid, sak, st, region string, serviceType esv1beta1.AWSServiceType) {
+func CreateReferentStaticStore(f *framework.Framework, access AccessOpts, serviceType esv1beta1.AWSServiceType) {
 	ns := f.Namespace.Name
 
 	awsCreds := &corev1.Secret{
@@ -197,9 +205,9 @@ func CreateReferentStaticStore(f *framework.Framework, kid, sak, st, region stri
 			Namespace: ns,
 		},
 		StringData: map[string]string{
-			staticKeyID:           kid,
-			staticSecretAccessKey: sak,
-			staticySessionToken:   st,
+			staticKeyID:           access.KID,
+			staticSecretAccessKey: access.SAK,
+			staticySessionToken:   access.ST,
 		},
 	}
 	err := f.CRClient.Create(context.Background(), awsCreds)
@@ -210,7 +218,7 @@ func CreateReferentStaticStore(f *framework.Framework, kid, sak, st, region stri
 			Name: ReferentSecretStoreName(f),
 		},
 		Spec: esv1beta1.SecretStoreSpec{
-			Provider: newStaticStoreProvider(serviceType, region, StaticReferentCredentialsSecretName, "", "", nil),
+			Provider: newStaticStoreProvider(serviceType, access.Region, StaticReferentCredentialsSecretName, "", "", nil),
 		},
 	}
 	err = f.CRClient.Create(context.Background(), secretStore)

+ 2 - 2
e2e/suites/provider/cases/aws/parameterstore/provider.go

@@ -68,8 +68,8 @@ func NewProvider(f *framework.Framework, kid, sak, st, region, saName, saNamespa
 	}
 
 	BeforeEach(func() {
-		awscommon.SetupStaticStore(f, kid, sak, st, region, esv1beta1.AWSServiceParameterStore)
-		awscommon.CreateReferentStaticStore(f, kid, sak, st, region, esv1beta1.AWSServiceParameterStore)
+		awscommon.SetupStaticStore(f, awscommon.AccessOpts{KID: kid, SAK: sak, ST: st, Region: region}, esv1beta1.AWSServiceParameterStore)
+		awscommon.CreateReferentStaticStore(f, awscommon.AccessOpts{KID: kid, SAK: sak, ST: st, Region: region}, esv1beta1.AWSServiceParameterStore)
 		prov.SetupReferencedIRSAStore()
 		prov.SetupMountedIRSAStore()
 	})

+ 15 - 4
e2e/suites/provider/cases/aws/secretsmanager/provider.go

@@ -69,10 +69,21 @@ func NewProvider(f *framework.Framework, kid, sak, st, region, saName, saNamespa
 	}
 
 	BeforeEach(func() {
-		awscommon.SetupStaticStore(f, kid, sak, st, region, esv1beta1.AWSServiceSecretsManager)
-		awscommon.SetupExternalIDStore(f, kid, sak, st, region, awscommon.IAMRoleExternalID, awscommon.IAMTrustedExternalID, nil, esv1beta1.AWSServiceSecretsManager)
-		awscommon.SetupSessionTagsStore(f, kid, sak, st, region, awscommon.IAMRoleSessionTags, nil, esv1beta1.AWSServiceSecretsManager)
-		awscommon.CreateReferentStaticStore(f, kid, sak, st, region, esv1beta1.AWSServiceSecretsManager)
+		awscommon.SetupStaticStore(f, awscommon.AccessOpts{KID: kid, SAK: sak, ST: st, Region: region}, esv1beta1.AWSServiceSecretsManager)
+		awscommon.SetupExternalIDStore(
+			f,
+			awscommon.AccessOpts{KID: kid, SAK: sak, ST: st, Region: region, Role: awscommon.IAMRoleExternalID},
+			awscommon.IAMTrustedExternalID,
+			nil,
+			esv1beta1.AWSServiceSecretsManager,
+		)
+		awscommon.SetupSessionTagsStore(
+			f,
+			awscommon.AccessOpts{KID: kid, SAK: sak, ST: st, Region: region, Role: awscommon.IAMRoleSessionTags},
+			nil,
+			esv1beta1.AWSServiceSecretsManager,
+		)
+		awscommon.CreateReferentStaticStore(f, awscommon.AccessOpts{KID: kid, SAK: sak, ST: st, Region: region}, esv1beta1.AWSServiceSecretsManager)
 		prov.SetupReferencedIRSAStore()
 		prov.SetupMountedIRSAStore()
 	})

+ 8 - 8
hack/api-docs/Dockerfile

@@ -13,16 +13,16 @@
 
 FROM alpine:3.21@sha256:21dc6063fd678b478f57c0e13f47560d0ea4eeba26dfc947b2a4f81f686b9f45
 RUN apk add -U --no-cache \
-    python3 \
-    python3-dev \
-    py3-pip \
-    musl-dev \
-    git \
-    openssh \
-    git-fast-import \
     bash \
+    diffutils \
     gcc \
-    diffutils
+    git \
+    git-fast-import \
+    musl-dev \
+    openssh \
+    py3-pip \
+    python3 \
+    python3-dev
 
 ENV PATH=$PATH:/.venv/bin
 COPY requirements.txt /

+ 1 - 1
overrides/main.html

@@ -7,6 +7,6 @@
 {% endblock %}
 
 {% block footer %}
-<img referrerpolicy="no-referrer-when-downgrade" src="https://static.scarf.sh/a.png?x-pxid=6658a9eb-067d-49f1-94f2-b8b00f21451e" />
+<img referrerpolicy="no-referrer-when-downgrade" src="https://static.scarf.sh/a.png?x-pxid=6658a9eb-067d-49f1-94f2-b8b00f21451e"  alt=""/>
   {{ super() }}
 {% endblock %}

+ 58 - 26
pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller.go

@@ -45,7 +45,7 @@ import (
 	ctrlmetrics "github.com/external-secrets/external-secrets/pkg/controllers/metrics"
 )
 
-// ClusterExternalSecretReconciler reconciles a ClusterExternalSecret object.
+// Reconciler reconciles a ClusterExternalSecret object.
 type Reconciler struct {
 	client.Client
 	Log             logr.Logger
@@ -91,6 +91,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	p := client.MergeFrom(clusterExternalSecret.DeepCopy())
 	defer r.deferPatch(ctx, log, &clusterExternalSecret, p)
 
+	return r.reconcile(ctx, log, &clusterExternalSecret)
+}
+
+func (r *Reconciler) reconcile(ctx context.Context, log logr.Logger, clusterExternalSecret *esv1beta1.ClusterExternalSecret) (ctrl.Result, error) {
 	refreshInt := r.RequeueInterval
 	if clusterExternalSecret.Spec.RefreshInterval != nil {
 		refreshInt = clusterExternalSecret.Spec.RefreshInterval.Duration
@@ -102,30 +106,20 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 	if prevName := clusterExternalSecret.Status.ExternalSecretName; prevName != esName {
 		// ExternalSecretName has changed, so remove the old ones
-		failedNamespaces := map[string]error{}
-		for _, ns := range clusterExternalSecret.Status.ProvisionedNamespaces {
-			if err := r.deleteExternalSecret(ctx, prevName, clusterExternalSecret.Name, ns); err != nil {
-				log.Error(err, "could not delete ExternalSecret")
-				failedNamespaces[ns] = err
-			}
-		}
-		if len(failedNamespaces) > 0 {
-			condition := NewClusterExternalSecretCondition(failedNamespaces)
-			SetClusterExternalSecretCondition(&clusterExternalSecret, *condition)
-			clusterExternalSecret.Status.FailedNamespaces = toNamespaceFailures(failedNamespaces)
+		if err := r.removeOldSecrets(ctx, log, clusterExternalSecret, prevName); err != nil {
 			return ctrl.Result{}, err
 		}
 	}
 	clusterExternalSecret.Status.ExternalSecretName = esName
 
-	namespaces, err := r.getTargetNamespaces(ctx, &clusterExternalSecret)
+	namespaces, err := r.getTargetNamespaces(ctx, clusterExternalSecret)
 	if err != nil {
 		log.Error(err, "failed to get target Namespaces")
 		failedNamespaces := map[string]error{
 			"unknown": err,
 		}
 		condition := NewClusterExternalSecretCondition(failedNamespaces)
-		SetClusterExternalSecretCondition(&clusterExternalSecret, *condition)
+		SetClusterExternalSecretCondition(clusterExternalSecret, *condition)
 
 		clusterExternalSecret.Status.FailedNamespaces = toNamespaceFailures(failedNamespaces)
 
@@ -134,10 +128,30 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	failedNamespaces := r.deleteOutdatedExternalSecrets(ctx, namespaces, esName, clusterExternalSecret.Name, clusterExternalSecret.Status.ProvisionedNamespaces)
 
-	provisionedNamespaces := []string{}
+	provisionedNamespaces := r.gatherProvisionedNamespaces(ctx, log, clusterExternalSecret, namespaces, esName, failedNamespaces)
+
+	condition := NewClusterExternalSecretCondition(failedNamespaces)
+	SetClusterExternalSecretCondition(clusterExternalSecret, *condition)
+
+	clusterExternalSecret.Status.FailedNamespaces = toNamespaceFailures(failedNamespaces)
+	sort.Strings(provisionedNamespaces)
+	clusterExternalSecret.Status.ProvisionedNamespaces = provisionedNamespaces
+
+	return ctrl.Result{RequeueAfter: refreshInt}, nil
+}
+
+func (r *Reconciler) gatherProvisionedNamespaces(
+	ctx context.Context,
+	log logr.Logger,
+	clusterExternalSecret *esv1beta1.ClusterExternalSecret,
+	namespaces []v1.Namespace,
+	esName string,
+	failedNamespaces map[string]error,
+) []string {
+	var provisionedNamespaces []string //nolint:prealloc // we don't know the size
 	for _, namespace := range namespaces {
 		var existingES esv1beta1.ExternalSecret
-		err = r.Get(ctx, types.NamespacedName{
+		err := r.Get(ctx, types.NamespacedName{
 			Name:      esName,
 			Namespace: namespace.Name,
 		}, &existingES)
@@ -152,7 +166,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			continue
 		}
 
-		if err := r.createOrUpdateExternalSecret(ctx, &clusterExternalSecret, namespace, esName, clusterExternalSecret.Spec.ExternalSecretMetadata); err != nil {
+		if err := r.createOrUpdateExternalSecret(ctx, clusterExternalSecret, namespace, esName, clusterExternalSecret.Spec.ExternalSecretMetadata); err != nil {
 			log.Error(err, "failed to create or update external secret")
 			failedNamespaces[namespace.Name] = err
 			continue
@@ -160,19 +174,33 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 		provisionedNamespaces = append(provisionedNamespaces, namespace.Name)
 	}
+	return provisionedNamespaces
+}
 
-	condition := NewClusterExternalSecretCondition(failedNamespaces)
-	SetClusterExternalSecretCondition(&clusterExternalSecret, *condition)
-
-	clusterExternalSecret.Status.FailedNamespaces = toNamespaceFailures(failedNamespaces)
-	sort.Strings(provisionedNamespaces)
-	clusterExternalSecret.Status.ProvisionedNamespaces = provisionedNamespaces
+func (r *Reconciler) removeOldSecrets(ctx context.Context, log logr.Logger, clusterExternalSecret *esv1beta1.ClusterExternalSecret, prevName string) error {
+	var (
+		failedNamespaces = map[string]error{}
+		lastErr          error
+	)
+	for _, ns := range clusterExternalSecret.Status.ProvisionedNamespaces {
+		if err := r.deleteExternalSecret(ctx, prevName, clusterExternalSecret.Name, ns); err != nil {
+			log.Error(err, "could not delete ExternalSecret")
+			failedNamespaces[ns] = err
+			lastErr = err
+		}
+	}
+	if len(failedNamespaces) > 0 {
+		condition := NewClusterExternalSecretCondition(failedNamespaces)
+		SetClusterExternalSecretCondition(clusterExternalSecret, *condition)
+		clusterExternalSecret.Status.FailedNamespaces = toNamespaceFailures(failedNamespaces)
+		return lastErr
+	}
 
-	return ctrl.Result{RequeueAfter: refreshInt}, nil
+	return nil
 }
 
 func (r *Reconciler) getTargetNamespaces(ctx context.Context, ces *esv1beta1.ClusterExternalSecret) ([]v1.Namespace, error) {
-	selectors := []*metav1.LabelSelector{}
+	var selectors []*metav1.LabelSelector //nolint:prealloc // ces.Spec.NamespaceSelector might be empty.
 	if s := ces.Spec.NamespaceSelector; s != nil {
 		selectors = append(selectors, s)
 	}
@@ -341,9 +369,13 @@ func (r *Reconciler) findObjectsForNamespace(ctx context.Context, namespace clie
 		return []reconcile.Request{}
 	}
 
+	return r.queueRequestsForItem(&clusterExternalSecrets, namespace)
+}
+
+func (r *Reconciler) queueRequestsForItem(clusterExternalSecrets *esv1beta1.ClusterExternalSecretList, namespace client.Object) []reconcile.Request {
 	var requests []reconcile.Request
 	for i := range clusterExternalSecrets.Items {
-		clusterExternalSecret := &clusterExternalSecrets.Items[i]
+		clusterExternalSecret := clusterExternalSecrets.Items[i]
 		var selectors []*metav1.LabelSelector
 		if s := clusterExternalSecret.Spec.NamespaceSelector; s != nil {
 			selectors = append(selectors, s)

+ 30 - 19
pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller_test.go

@@ -39,6 +39,17 @@ func init() {
 	cesmetrics.SetUpMetrics()
 }
 
+const (
+	metadataLabelName       = "kubernetes.io/metadata.name"
+	testLabelKey            = "test-label-key"
+	testAnnotationKey       = "test-annotation-key"
+	testLabelValue          = "test-label-value"
+	testAnnotationValue     = "test-annotation-value"
+	updatedTestStore        = "updated-test-store"
+	noLongerMatchLabelKey   = "no-longer-match-label-key"
+	noLongerMatchLabelValue = "no-longer-match-label-value"
+)
+
 var (
 	timeout  = time.Second * 10
 	interval = time.Millisecond * 250
@@ -158,7 +169,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name},
+					MatchLabels: map[string]string{metadataLabelName: namespaces[0].Name},
 				}
 				return *ces
 			},
@@ -199,7 +210,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name},
+					MatchLabels: map[string]string{metadataLabelName: namespaces[0].Name},
 				}
 				ces.Spec.ExternalSecretName = "test-es"
 				ces.Spec.ExternalSecretMetadata = esv1beta1.ExternalSecretMetadata{
@@ -247,7 +258,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name},
+					MatchLabels: map[string]string{metadataLabelName: namespaces[0].Name},
 				}
 				ces.Spec.ExternalSecretName = "old-es-name"
 				return *ces
@@ -304,7 +315,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name},
+					MatchLabels: map[string]string{metadataLabelName: namespaces[0].Name},
 				}
 				return *ces
 			},
@@ -321,19 +332,19 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 
 				copied := created.DeepCopy()
 				copied.Spec.ExternalSecretMetadata = esv1beta1.ExternalSecretMetadata{
-					Labels:      map[string]string{"test-label-key": "test-label-value"},
-					Annotations: map[string]string{"test-annotation-key": "test-annotation-value"},
+					Labels:      map[string]string{testLabelKey: testLabelValue},
+					Annotations: map[string]string{testAnnotationKey: testAnnotationValue},
 				}
-				copied.Spec.ExternalSecretSpec.SecretStoreRef.Name = "updated-test-store" //nolint:goconst
+				copied.Spec.ExternalSecretSpec.SecretStoreRef.Name = updatedTestStore
 				Expect(k8sClient.Patch(ctx, copied, crclient.MergeFrom(created.DeepCopy()))).ShouldNot(HaveOccurred())
 			},
 			expectedClusterExternalSecret: func(namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) esv1beta1.ClusterExternalSecret {
 				updatedSpec := created.Spec.DeepCopy()
 				updatedSpec.ExternalSecretMetadata = esv1beta1.ExternalSecretMetadata{
-					Labels:      map[string]string{"test-label-key": "test-label-value"},
-					Annotations: map[string]string{"test-annotation-key": "test-annotation-value"},
+					Labels:      map[string]string{testLabelKey: testLabelValue},
+					Annotations: map[string]string{testAnnotationKey: testAnnotationValue},
 				}
-				updatedSpec.ExternalSecretSpec.SecretStoreRef.Name = "updated-test-store"
+				updatedSpec.ExternalSecretSpec.SecretStoreRef.Name = updatedTestStore
 
 				return esv1beta1.ClusterExternalSecret{
 					ObjectMeta: metav1.ObjectMeta{
@@ -354,15 +365,15 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			},
 			expectedExternalSecrets: func(namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) []esv1beta1.ExternalSecret {
 				updatedSpec := created.Spec.ExternalSecretSpec.DeepCopy()
-				updatedSpec.SecretStoreRef.Name = "updated-test-store"
+				updatedSpec.SecretStoreRef.Name = updatedTestStore
 
 				return []esv1beta1.ExternalSecret{
 					{
 						ObjectMeta: metav1.ObjectMeta{
 							Namespace:   namespaces[0].Name,
 							Name:        created.Name,
-							Labels:      map[string]string{"test-label-key": "test-label-value"},
-							Annotations: map[string]string{"test-annotation-key": "test-annotation-value"},
+							Labels:      map[string]string{testLabelKey: testLabelValue},
+							Annotations: map[string]string{testAnnotationKey: testAnnotationValue},
 						},
 						Spec: *updatedSpec,
 					},
@@ -376,7 +387,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name},
+					MatchLabels: map[string]string{metadataLabelName: namespaces[0].Name},
 				}
 
 				es := &esv1beta1.ExternalSecret{
@@ -438,7 +449,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name},
+					MatchLabels: map[string]string{metadataLabelName: namespaces[0].Name},
 				}
 
 				es := &esv1beta1.ExternalSecret{
@@ -501,13 +512,13 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   randomNamespaceName(),
-						Labels: map[string]string{"no-longer-match-label-key": "no-longer-match-label-value"},
+						Labels: map[string]string{noLongerMatchLabelKey: noLongerMatchLabelValue},
 					},
 				},
 				{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   randomNamespaceName(),
-						Labels: map[string]string{"no-longer-match-label-key": "no-longer-match-label-value"},
+						Labels: map[string]string{noLongerMatchLabelKey: noLongerMatchLabelValue},
 					},
 				},
 			},
@@ -515,7 +526,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.RefreshInterval = &metav1.Duration{Duration: 100 * time.Millisecond}
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"no-longer-match-label-key": "no-longer-match-label-value"},
+					MatchLabels: map[string]string{noLongerMatchLabelKey: noLongerMatchLabelValue},
 				}
 				return *ces
 			},
@@ -646,7 +657,7 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
 				ces := defaultClusterExternalSecret()
 				ces.Spec.NamespaceSelector = &metav1.LabelSelector{
-					MatchLabels: map[string]string{"kubernetes.io/metadata.name": "no-namespace-matches"},
+					MatchLabels: map[string]string{metadataLabelName: "no-namespace-matches"},
 				}
 				return *ces
 			},

+ 14 - 6
pkg/controllers/crds/crds_controller.go

@@ -81,18 +81,26 @@ type Reconciler struct {
 	readyStatusMap   map[string]bool
 }
 
+type Opts struct {
+	SvcName         string
+	SvcNamespace    string
+	SecretName      string
+	SecretNamespace string
+	Resources       []string
+}
+
 func New(k8sClient client.Client, scheme *runtime.Scheme, leaderChan <-chan struct{}, logger logr.Logger,
-	interval time.Duration, svcName, svcNamespace, secretName, secretNamespace string, resources []string) *Reconciler {
+	interval time.Duration, opts Opts) *Reconciler {
 	return &Reconciler{
 		Client:           k8sClient,
 		Log:              logger,
 		Scheme:           scheme,
-		SvcName:          svcName,
-		SvcNamespace:     svcNamespace,
-		SecretName:       secretName,
-		SecretNamespace:  secretNamespace,
+		SvcName:          opts.SvcName,
+		SvcNamespace:     opts.SvcNamespace,
+		SecretName:       opts.SecretName,
+		SecretNamespace:  opts.SecretNamespace,
 		RequeueInterval:  interval,
-		CrdResources:     resources,
+		CrdResources:     opts.Resources,
 		CAName:           "external-secrets",
 		CAOrganization:   "external-secrets",
 		leaderChan:       leaderChan,

+ 8 - 2
pkg/controllers/crds/suite_test.go

@@ -81,8 +81,14 @@ var _ = BeforeSuite(func() {
 	leaderChan := make(chan struct{})
 	close(leaderChan)
 	rec := New(k8sClient, k8sManager.GetScheme(), leaderChan, log, time.Second*1,
-		"foo", "default", "foo", "default", []string{
-			"secretstores.test.io",
+		Opts{
+			SvcName:         "foo",
+			SvcNamespace:    "default",
+			SecretName:      "foo",
+			SecretNamespace: "default",
+			Resources: []string{
+				"secretstores.test.io",
+			},
 		})
 	rec.SetupWithManager(k8sManager, controller.Options{})
 	Expect(err).ToNot(HaveOccurred())

+ 27 - 18
pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -48,6 +48,15 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+const (
+	labelKey           = "label-key"
+	labelValue         = "label-value"
+	annotationKey      = "annotation-key"
+	annotationValue    = "annotation-value"
+	existingLabelKey   = "existing-label-key"
+	existingLabelValue = "existing-label-value"
+)
+
 var (
 	fakeProvider   *fake.Client
 	metric         dto.Metric
@@ -320,16 +329,16 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 	// should be copied over to the Kind=Secret
 	syncLabelsAnnotations := func(tc *testCase) {
 		tc.externalSecret.ObjectMeta.Labels = map[string]string{
-			"label-key": "label-value",
+			labelKey: labelValue,
 		}
 		tc.externalSecret.ObjectMeta.Annotations = map[string]string{
-			"annotation-key": "annotation-value",
+			annotationKey: annotationValue,
 		}
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 
 		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
-			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue("label-key", "label-value"))
-			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue("annotation-key", "annotation-value"))
+			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue(labelKey, labelValue))
+			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(annotationKey, annotationValue))
 
 			// ownerRef must not be set!
 			Expect(ctest.HasOwnerRef(secret.ObjectMeta, "ExternalSecret", ExternalSecretName)).To(BeTrue())
@@ -340,10 +349,10 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 	// should be merged to the Secret if exists
 	mergeLabelsAnnotations := func(tc *testCase) {
 		tc.externalSecret.ObjectMeta.Labels = map[string]string{
-			"label-key": "label-value",
+			labelKey: labelValue,
 		}
 		tc.externalSecret.ObjectMeta.Annotations = map[string]string{
-			"annotation-key": "annotation-value",
+			annotationKey: annotationValue,
 		}
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		// Create a secret owned by another entity to test if the pre-existing metadata is preserved
@@ -352,7 +361,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 				Name:      ExternalSecretTargetSecretName,
 				Namespace: ExternalSecretNamespace,
 				Labels: map[string]string{
-					"existing-label-key": "existing-label-value",
+					existingLabelKey: existingLabelValue,
 				},
 				Annotations: map[string]string{
 					"existing-annotation-key": "existing-annotation-value",
@@ -361,19 +370,19 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}, client.FieldOwner(FakeManager))).To(Succeed())
 
 		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
-			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue("label-key", "label-value"))
-			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue("existing-label-key", "existing-label-value"))
-			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue("annotation-key", "annotation-value"))
+			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue(labelKey, labelValue))
+			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue(existingLabelKey, existingLabelValue))
+			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(annotationKey, annotationValue))
 			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue("existing-annotation-key", "existing-annotation-value"))
 		}
 	}
 
 	removeOutdatedLabelsAnnotations := func(tc *testCase) {
 		tc.externalSecret.ObjectMeta.Labels = map[string]string{
-			"label-key": "label-value",
+			labelKey: labelValue,
 		}
 		tc.externalSecret.ObjectMeta.Annotations = map[string]string{
-			"annotation-key": "annotation-value",
+			annotationKey: annotationValue,
 		}
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		// Create a secret owned by the operator to test if the outdated pre-existing metadata is removed
@@ -382,7 +391,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 				Name:      ExternalSecretTargetSecretName,
 				Namespace: ExternalSecretNamespace,
 				Labels: map[string]string{
-					"existing-label-key": "existing-label-value",
+					existingLabelKey: existingLabelValue,
 				},
 				Annotations: map[string]string{
 					"existing-annotation-key": "existing-annotation-value",
@@ -391,9 +400,9 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		}, client.FieldOwner(ExternalSecretFQDN))).To(Succeed())
 
 		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
-			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue("label-key", "label-value"))
-			Expect(secret.ObjectMeta.Labels).NotTo(HaveKeyWithValue("existing-label-key", "existing-label-value"))
-			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue("annotation-key", "annotation-value"))
+			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue(labelKey, labelValue))
+			Expect(secret.ObjectMeta.Labels).NotTo(HaveKeyWithValue(existingLabelKey, existingLabelValue))
+			Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(annotationKey, annotationValue))
 			Expect(secret.ObjectMeta.Annotations).NotTo(HaveKeyWithValue("existing-annotation-key", "existing-annotation-value"))
 		}
 	}
@@ -432,7 +441,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 				Name:      ExternalSecretTargetSecretName,
 				Namespace: ExternalSecretNamespace,
 				Labels: map[string]string{
-					"existing-label-key": "existing-label-value",
+					existingLabelKey: existingLabelValue,
 				},
 				Annotations: map[string]string{
 					"existing-annotation-key": "existing-annotation-value",
@@ -450,7 +459,7 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 
 			Expect(secret.ObjectMeta.Labels).To(HaveLen(3))
-			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue("existing-label-key", "existing-label-value"))
+			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue(existingLabelKey, existingLabelValue))
 			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue("es-label-key", "es-label-value"))
 			Expect(secret.ObjectMeta.Labels).To(HaveKeyWithValue(esv1beta1.LabelManaged, esv1beta1.LabelManagedValue))
 

+ 12 - 12
pkg/controllers/pushsecret/pushsecret_controller.go

@@ -49,15 +49,15 @@ import (
 )
 
 const (
-	errFailedGetSecret       = "could not get source secret"
-	errPatchStatus           = "error merging"
-	errGetSecretStore        = "could not get SecretStore %q, %w"
-	errGetClusterSecretStore = "could not get ClusterSecretStore %q, %w"
-	errSetSecretFailed       = "could not write remote ref %v to target secretstore %v: %v"
-	errFailedSetSecret       = "set secret failed: %v"
-	errConvert               = "could not apply conversion strategy to keys: %v"
-	errUnmanagedStores       = "PushSecret %q has no managed stores to push to"
-	pushSecretFinalizer      = "pushsecret.externalsecrets.io/finalizer"
+	errFailedGetSecret         = "could not get source secret"
+	errPatchStatus             = "error merging"
+	errGetSecretStore          = "could not get SecretStore %q, %w"
+	errGetClusterSecretStore   = "could not get ClusterSecretStore %q, %w"
+	errSetSecretFailed         = "could not write remote ref %v to target secretstore %v: %v"
+	errFailedSetSecret         = "set secret failed: %v"
+	errConvert                 = "could not apply conversion strategy to keys: %v"
+	pushSecretFinalizer        = "pushsecret.externalsecrets.io/finalizer"
+	errCloudNotUpdateFinalizer = "could not update finalizers: %w"
 )
 
 type Reconciler struct {
@@ -122,7 +122,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			if !controllerutil.ContainsFinalizer(&ps, pushSecretFinalizer) {
 				controllerutil.AddFinalizer(&ps, pushSecretFinalizer)
 				if err := r.Client.Update(ctx, &ps, &client.UpdateOptions{}); err != nil {
-					return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
+					return ctrl.Result{}, fmt.Errorf(errCloudNotUpdateFinalizer, err)
 				}
 
 				return ctrl.Result{}, nil
@@ -140,7 +140,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 				controllerutil.RemoveFinalizer(&ps, pushSecretFinalizer)
 				if err := r.Client.Update(ctx, &ps, &client.UpdateOptions{}); err != nil {
-					return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
+					return ctrl.Result{}, fmt.Errorf(errCloudNotUpdateFinalizer, err)
 				}
 
 				return ctrl.Result{}, nil
@@ -150,7 +150,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		if controllerutil.ContainsFinalizer(&ps, pushSecretFinalizer) {
 			controllerutil.RemoveFinalizer(&ps, pushSecretFinalizer)
 			if err := r.Client.Update(ctx, &ps, &client.UpdateOptions{}); err != nil {
-				return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
+				return ctrl.Result{}, fmt.Errorf(errCloudNotUpdateFinalizer, err)
 			}
 		}
 	default:

+ 4 - 11
pkg/controllers/pushsecret/pushsecret_controller_test.go

@@ -125,7 +125,6 @@ var _ = Describe("PushSecret controller", func() {
 			},
 		})
 		// give a time for reconciler to remove finalizers before removing SecretStores
-		// TODO: Secret Stores should have finalizers bound to PushSecrets if DeletionPolicy == Delete
 		time.Sleep(2 * time.Second)
 		k8sClient.Delete(context.Background(), &v1beta1.SecretStore{
 			ObjectMeta: metav1.ObjectMeta{
@@ -742,7 +741,7 @@ var _ = Describe("PushSecret controller", func() {
 						Match: v1alpha1.PushSecretMatch{
 							SecretKey: "some-array[0].entity",
 							RemoteRef: v1alpha1.PushSecretRemoteRef{
-								RemoteKey: "path/to/key",
+								RemoteKey: defaultPath,
 							},
 						},
 					},
@@ -1181,15 +1180,9 @@ var _ = Describe("PushSecret Controller Un/Managed Stores", func() {
 	})
 
 	const (
-		defaultKey          = "key"
-		defaultVal          = "value"
-		defaultPath         = "path/to/key"
-		otherKey            = "other-key"
-		otherVal            = "other-value"
-		otherPath           = "path/to/other-key"
-		newKey              = "new-key"
-		newVal              = "new-value"
-		storePrefixTemplate = "SecretStore/%v"
+		defaultKey  = "key"
+		defaultVal  = "value"
+		defaultPath = "path/to/key"
 	)
 
 	makeDefaultTestcase := func() *testCase {

+ 6 - 1
pkg/controllers/secretstore/clustersecretstore_controller.go

@@ -63,7 +63,12 @@ func (r *ClusterStoreReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return ctrl.Result{}, err
 	}
 
-	return reconcile(ctx, req, &css, r.Client, log, r.ControllerClass, cssmetrics.GetGaugeVec, r.recorder, r.RequeueInterval)
+	return reconcile(ctx, req, &css, r.Client, log, Opts{
+		ControllerClass: r.ControllerClass,
+		GaugeVecGetter:  cssmetrics.GetGaugeVec,
+		Recorder:        r.recorder,
+		RequeueInterval: r.RequeueInterval,
+	})
 }
 
 // SetupWithManager returns a new controller builder that will be started by the provided Manager.

+ 14 - 8
pkg/controllers/secretstore/common.go

@@ -30,24 +30,30 @@ import (
 )
 
 const (
-	errStoreProvider       = "could not get store provider: %w"
 	errStoreClient         = "could not get provider client: %w"
 	errValidationFailed    = "could not validate provider: %w"
 	errPatchStatus         = "unable to patch status: %w"
 	errUnableCreateClient  = "unable to create client"
 	errUnableValidateStore = "unable to validate store: %s"
-	errUnableGetProvider   = "unable to get store provider"
 
 	msgStoreValidated = "store validated"
 )
 
-func reconcile(ctx context.Context, req ctrl.Request, ss esapi.GenericStore, cl client.Client, log logr.Logger,
-	controllerClass string, gaugeVecGetter metrics.GaugeVevGetter, recorder record.EventRecorder, requeueInterval time.Duration) (ctrl.Result, error) {
-	if !ShouldProcessStore(ss, controllerClass) {
+type Opts struct {
+	ControllerClass string
+	GaugeVecGetter  metrics.GaugeVevGetter
+	Recorder        record.EventRecorder
+	RequeueInterval time.Duration
+}
+
+func reconcile(ctx context.Context, req ctrl.Request, ss esapi.GenericStore, cl client.Client, log logr.Logger, opts Opts) (ctrl.Result, error) {
+	if !ShouldProcessStore(ss, opts.ControllerClass) {
 		log.V(1).Info("skip store")
 		return ctrl.Result{}, nil
 	}
 
+	requeueInterval := opts.RequeueInterval
+
 	if ss.GetSpec().RefreshInterval != 0 {
 		requeueInterval = time.Second * time.Duration(ss.GetSpec().RefreshInterval)
 	}
@@ -64,7 +70,7 @@ func reconcile(ctx context.Context, req ctrl.Request, ss esapi.GenericStore, cl
 	// validateStore modifies the store conditions
 	// we have to patch the status
 	log.V(1).Info("validating")
-	err := validateStore(ctx, req.Namespace, controllerClass, ss, cl, gaugeVecGetter, recorder)
+	err := validateStore(ctx, req.Namespace, opts.ControllerClass, ss, cl, opts.GaugeVecGetter, opts.Recorder)
 	if err != nil {
 		log.Error(err, "unable to validate store")
 		return ctrl.Result{}, err
@@ -79,9 +85,9 @@ func reconcile(ctx context.Context, req ctrl.Request, ss esapi.GenericStore, cl
 	}
 	ss.SetStatus(capStatus)
 
-	recorder.Event(ss, v1.EventTypeNormal, esapi.ReasonStoreValid, msgStoreValidated)
+	opts.Recorder.Event(ss, v1.EventTypeNormal, esapi.ReasonStoreValid, msgStoreValidated)
 	cond := NewSecretStoreCondition(esapi.SecretStoreReady, v1.ConditionTrue, esapi.ReasonStoreValid, msgStoreValidated)
-	SetExternalSecretCondition(ss, *cond, gaugeVecGetter)
+	SetExternalSecretCondition(ss, *cond, opts.GaugeVecGetter)
 
 	return ctrl.Result{
 		RequeueAfter: requeueInterval,

+ 6 - 1
pkg/controllers/secretstore/secretstore_controller.go

@@ -63,7 +63,12 @@ func (r *StoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		return ctrl.Result{}, err
 	}
 
-	return reconcile(ctx, req, &ss, r.Client, log, r.ControllerClass, ssmetrics.GetGaugeVec, r.recorder, r.RequeueInterval)
+	return reconcile(ctx, req, &ss, r.Client, log, Opts{
+		ControllerClass: r.ControllerClass,
+		GaugeVecGetter:  ssmetrics.GetGaugeVec,
+		Recorder:        r.recorder,
+		RequeueInterval: r.RequeueInterval,
+	})
 }
 
 // SetupWithManager returns a new controller builder that will be started by the provided Manager.

+ 7 - 1
pkg/controllers/webhookconfig/suite_test.go

@@ -87,7 +87,13 @@ var _ = BeforeSuite(func() {
 	Expect(k8sClient).ToNot(BeNil())
 	leaderChan := make(chan struct{})
 	close(leaderChan)
-	reconciler = New(k8sClient, k8sManager.GetScheme(), leaderChan, ctrl.Log, ctrlSvcName, ctrlSvcNamespace, ctrlSecretName, ctrlSecretNamespace, time.Second)
+	reconciler = New(k8sClient, k8sManager.GetScheme(), leaderChan, ctrl.Log, Opts{
+		SvcName:         ctrlSvcName,
+		SvcNamespace:    ctrlSvcNamespace,
+		SecretName:      ctrlSecretName,
+		SecretNamespace: ctrlSecretNamespace,
+		RequeueInterval: time.Second,
+	})
 	reconciler.SetupWithManager(k8sManager, controller.Options{})
 	Expect(err).ToNot(HaveOccurred())
 

+ 14 - 8
pkg/controllers/webhookconfig/webhookconfig.go

@@ -57,18 +57,24 @@ type Reconciler struct {
 	webhookReady   bool
 }
 
-func New(k8sClient client.Client, scheme *runtime.Scheme, leaderChan <-chan struct{},
-	log logr.Logger, svcName, svcNamespace, secretName, secretNamespace string,
-	requeueInterval time.Duration) *Reconciler {
+type Opts struct {
+	SvcName         string
+	SvcNamespace    string
+	SecretName      string
+	SecretNamespace string
+	RequeueInterval time.Duration
+}
+
+func New(k8sClient client.Client, scheme *runtime.Scheme, leaderChan <-chan struct{}, log logr.Logger, opts Opts) *Reconciler {
 	return &Reconciler{
 		Client:          k8sClient,
 		Scheme:          scheme,
 		Log:             log,
-		RequeueDuration: requeueInterval,
-		SvcName:         svcName,
-		SvcNamespace:    svcNamespace,
-		SecretName:      secretName,
-		SecretNamespace: secretNamespace,
+		RequeueDuration: opts.RequeueInterval,
+		SvcName:         opts.SvcName,
+		SvcNamespace:    opts.SvcNamespace,
+		SecretName:      opts.SecretName,
+		SecretNamespace: opts.SecretNamespace,
 		leaderChan:      leaderChan,
 		leaderElected:   false,
 		webhookReadyMu:  &sync.Mutex{},

+ 28 - 17
pkg/generator/vault/vault.go

@@ -30,6 +30,7 @@ import (
 
 	genv1alpha1 "github.com/external-secrets/external-secrets/apis/generators/v1alpha1"
 	provider "github.com/external-secrets/external-secrets/pkg/provider/vault"
+	"github.com/external-secrets/external-secrets/pkg/provider/vault/util"
 	"github.com/external-secrets/external-secrets/pkg/utils"
 )
 
@@ -76,23 +77,7 @@ func (g *Generator) generate(ctx context.Context, c *provider.Provider, jsonSpec
 		return nil, fmt.Errorf(errVaultClient, err)
 	}
 
-	var result *vault.Secret
-	if res.Spec.Method == "" || res.Spec.Method == "GET" {
-		result, err = cl.Logical().ReadWithDataWithContext(ctx, res.Spec.Path, nil)
-	} else if res.Spec.Method == "LIST" {
-		result, err = cl.Logical().ListWithContext(ctx, res.Spec.Path)
-	} else if res.Spec.Method == "DELETE" {
-		result, err = cl.Logical().DeleteWithContext(ctx, res.Spec.Path)
-	} else {
-		params := make(map[string]any)
-		if res.Spec.Parameters != nil {
-			err = json.Unmarshal(res.Spec.Parameters.Raw, &params)
-			if err != nil {
-				return nil, err
-			}
-		}
-		result, err = cl.Logical().WriteWithContext(ctx, res.Spec.Path, params)
-	}
+	result, err := g.fetchVaultSecret(ctx, res, cl)
 	if err != nil {
 		return nil, err
 	}
@@ -124,6 +109,32 @@ func (g *Generator) generate(ctx context.Context, c *provider.Provider, jsonSpec
 	return response, nil
 }
 
+func (g *Generator) fetchVaultSecret(ctx context.Context, res *genv1alpha1.VaultDynamicSecret, cl util.Client) (*vault.Secret, error) {
+	var (
+		result *vault.Secret
+		err    error
+	)
+
+	if res.Spec.Method == "" || res.Spec.Method == "GET" {
+		result, err = cl.Logical().ReadWithDataWithContext(ctx, res.Spec.Path, nil)
+	} else if res.Spec.Method == "LIST" {
+		result, err = cl.Logical().ListWithContext(ctx, res.Spec.Path)
+	} else if res.Spec.Method == "DELETE" {
+		result, err = cl.Logical().DeleteWithContext(ctx, res.Spec.Path)
+	} else {
+		params := make(map[string]any)
+		if res.Spec.Parameters != nil {
+			if err := json.Unmarshal(res.Spec.Parameters.Raw, &params); err != nil {
+				return nil, err
+			}
+		}
+
+		result, err = cl.Logical().WriteWithContext(ctx, res.Spec.Path, params)
+	}
+
+	return result, err
+}
+
 func parseSpec(data []byte) (*genv1alpha1.VaultDynamicSecret, error) {
 	var spec genv1alpha1.VaultDynamicSecret
 	err := yaml.Unmarshal(data, &spec)

+ 36 - 34
pkg/provider/akeyless/akeyless.go

@@ -47,9 +47,8 @@ type AkeylessCtx string
 
 const (
 	defaultAPIUrl                   = "https://api.akeyless.io"
-	errNotImplemented               = "not implemented"
-	ExtSecretManagedTag             = "k8s-external-secrets"
-	AkeylessToken       AkeylessCtx = "AKEYLESS_TOKEN"
+	extSecretManagedTag             = "k8s-external-secrets"
+	aKeylessToken       AkeylessCtx = "AKEYLESS_TOKEN"
 )
 
 // https://github.com/external-secrets/external-secrets/issues/644
@@ -230,14 +229,14 @@ func newClient(ctx context.Context, store esv1beta1.GenericStore, kube client.Cl
 }
 
 func (a *Akeyless) contextWithToken(ctx context.Context) (context.Context, error) {
-	if v := ctx.Value(AkeylessToken); v != nil {
+	if v := ctx.Value(aKeylessToken); v != nil {
 		return ctx, nil
 	}
 	token, err := a.Client.TokenFromSecretRef(ctx)
 	if err != nil {
 		return nil, err
 	}
-	return context.WithValue(ctx, AkeylessToken, token), nil
+	return context.WithValue(ctx, aKeylessToken, token), nil
 }
 
 func (a *Akeyless) Close(_ context.Context) error {
@@ -299,8 +298,8 @@ func (a *Akeyless) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDa
 	return []byte(val.String()), nil
 }
 
-// Implements store.Client.GetAllSecrets Interface.
-// Retrieves a all secrets with defined in ref.Name or tags.
+// GetAllSecrets Implements store.Client.GetAllSecrets Interface.
+// Retrieves all secrets with defined in ref.Name or tags.
 func (a *Akeyless) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	if utils.IsNil(a.Client) {
 		return nil, errors.New(errUninitalizedAkeylessProvider)
@@ -321,37 +320,32 @@ func (a *Akeyless) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecr
 		}
 	}
 	if ref.Name != nil {
-		potentialSecrets, err := a.Client.ListSecrets(ctx, searchPath, "")
+		return a.findSecretsFromName(ctx, searchPath, *ref.Name)
+	}
+	if len(ref.Tags) > 0 {
+		return a.getSecrets(ctx, searchPath, ref.Tags)
+	}
+
+	return nil, errors.New("unexpected find operator")
+}
+
+func (a *Akeyless) getSecrets(ctx context.Context, searchPath string, tags map[string]string) (map[string][]byte, error) {
+	var potentialSecretsName []string
+	for _, v := range tags {
+		potentialSecrets, err := a.Client.ListSecrets(ctx, searchPath, v)
 		if err != nil {
 			return nil, err
 		}
-		if len(potentialSecrets) == 0 {
-			return nil, nil
+		if len(potentialSecrets) > 0 {
+			potentialSecretsName = append(potentialSecretsName, potentialSecrets...)
 		}
-		return a.findSecretsFromName(ctx, potentialSecrets, *ref.Name)
 	}
-	if len(ref.Tags) > 0 {
-		var potentialSecretsName []string
-		for _, v := range ref.Tags {
-			potentialSecrets, err := a.Client.ListSecrets(ctx, searchPath, v)
-			if err != nil {
-				return nil, err
-			}
-			if len(potentialSecrets) > 0 {
-				potentialSecretsName = append(potentialSecretsName, potentialSecrets...)
-			}
-		}
-		if len(potentialSecretsName) == 0 {
-			return nil, nil
-		}
-		return a.getSecrets(ctx, potentialSecretsName)
+	if len(potentialSecretsName) == 0 {
+		return nil, nil
 	}
-	return nil, errors.New("unexpected find operator")
-}
 
-func (a *Akeyless) getSecrets(ctx context.Context, candidates []string) (map[string][]byte, error) {
 	secrets := make(map[string][]byte)
-	for _, name := range candidates {
+	for _, name := range potentialSecretsName {
 		secretValue, err := a.Client.GetSecretByType(ctx, name, 0)
 		if err != nil {
 			return nil, err
@@ -363,13 +357,21 @@ func (a *Akeyless) getSecrets(ctx context.Context, candidates []string) (map[str
 	return secrets, nil
 }
 
-func (a *Akeyless) findSecretsFromName(ctx context.Context, candidates []string, ref esv1beta1.FindName) (map[string][]byte, error) {
+func (a *Akeyless) findSecretsFromName(ctx context.Context, searchPath string, ref esv1beta1.FindName) (map[string][]byte, error) {
+	potentialSecrets, err := a.Client.ListSecrets(ctx, searchPath, "")
+	if err != nil {
+		return nil, err
+	}
+	if len(potentialSecrets) == 0 {
+		return nil, nil
+	}
+
 	secrets := make(map[string][]byte)
 	matcher, err := find.New(ref)
 	if err != nil {
 		return nil, err
 	}
-	for _, name := range candidates {
+	for _, name := range potentialSecrets {
 		ok := matcher.MatchName(name)
 		if ok {
 			secretValue, err := a.Client.GetSecretByType(ctx, name, 0)
@@ -384,7 +386,7 @@ func (a *Akeyless) findSecretsFromName(ctx context.Context, candidates []string,
 	return secrets, nil
 }
 
-// Implements store.Client.GetSecretMap Interface.
+// GetSecretMap implements store.Client.GetSecretMap Interface.
 // New version of GetSecretMap.
 func (a *Akeyless) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	if utils.IsNil(a.Client) {
@@ -495,7 +497,7 @@ func (a *Akeyless) DeleteSecret(ctx context.Context, psr esv1beta1.PushSecretRem
 	if err != nil {
 		return err
 	}
-	if item == nil || item.ItemTags == nil || !slices.Contains(*item.ItemTags, ExtSecretManagedTag) {
+	if item == nil || item.ItemTags == nil || !slices.Contains(*item.ItemTags, extSecretManagedTag) {
 		return nil
 	}
 	if psr.GetProperty() == "" {

+ 30 - 27
pkg/provider/akeyless/akeyless_api.go

@@ -114,7 +114,7 @@ func (a *akeylessBase) GetSecretByType(ctx context.Context, secretName string, v
 }
 
 func SetBodyToken(t Tokener, ctx context.Context) error {
-	token, ok := ctx.Value(AkeylessToken).(string)
+	token, ok := ctx.Value(aKeylessToken).(string)
 	if !ok {
 		return ErrTokenNotExists
 	}
@@ -321,7 +321,7 @@ func (a *akeylessBase) CreateSecret(ctx context.Context, remoteKey, data string)
 	body := akeyless.CreateSecret{
 		Name:  remoteKey,
 		Value: data,
-		Tags:  &[]string{ExtSecretManagedTag},
+		Tags:  &[]string{extSecretManagedTag},
 	}
 	if err := SetBodyToken(&body, ctx); err != nil {
 		return err
@@ -360,33 +360,36 @@ func (a *akeylessBase) DeleteSecret(ctx context.Context, remoteKey string) error
 }
 
 func (a *akeylessBase) getK8SServiceAccountJWT(ctx context.Context, kubernetesAuth *esv1beta1.AkeylessKubernetesAuth) (string, error) {
-	if kubernetesAuth != nil {
-		if kubernetesAuth.ServiceAccountRef != nil {
-			// Kubernetes <v1.24 fetch token via ServiceAccount.Secrets[]
-			jwt, err := a.getJWTFromServiceAccount(ctx, kubernetesAuth.ServiceAccountRef)
-			if jwt != "" {
-				return jwt, err
-			}
-			// Kubernetes >=v1.24: fetch token via TokenRequest API
-			jwt, err = a.getJWTfromServiceAccountToken(ctx, *kubernetesAuth.ServiceAccountRef, nil, 600)
-			if err != nil {
-				return "", err
-			}
-			return jwt, nil
-		} else if kubernetesAuth.SecretRef != nil {
-			tokenRef := kubernetesAuth.SecretRef
-			if tokenRef.Key == "" {
-				tokenRef = kubernetesAuth.SecretRef.DeepCopy()
-				tokenRef.Key = "token"
-			}
-			jwt, err := resolvers.SecretKeyRef(ctx, a.kube, a.storeKind, a.namespace, tokenRef)
-			if err != nil {
-				return "", err
-			}
-			return jwt, nil
+	if kubernetesAuth == nil {
+		return readK8SServiceAccountJWT()
+	}
+
+	switch {
+	case kubernetesAuth.ServiceAccountRef != nil:
+		jwt, err := a.getJWTFromServiceAccount(ctx, kubernetesAuth.ServiceAccountRef)
+		if jwt != "" {
+			return jwt, err
+		}
+		// Kubernetes >=v1.24: fetch token via TokenRequest API
+		jwt, err = a.getJWTfromServiceAccountToken(ctx, *kubernetesAuth.ServiceAccountRef, nil, 600)
+		if err != nil {
+			return "", err
 		}
+		return jwt, nil
+	case kubernetesAuth.SecretRef != nil:
+		tokenRef := kubernetesAuth.SecretRef
+		if tokenRef.Key == "" {
+			tokenRef = kubernetesAuth.SecretRef.DeepCopy()
+			tokenRef.Key = "token"
+		}
+		jwt, err := resolvers.SecretKeyRef(ctx, a.kube, a.storeKind, a.namespace, tokenRef)
+		if err != nil {
+			return "", err
+		}
+		return jwt, nil
 	}
-	return readK8SServiceAccountJWT()
+
+	return "", fmt.Errorf("can't determine k8s service account jwt")
 }
 
 func (a *akeylessBase) getJWTFromServiceAccount(ctx context.Context, serviceAccountRef *esmeta.ServiceAccountSelector) (string, error) {

+ 3 - 3
pkg/provider/akeyless/akeyless_test.go

@@ -402,7 +402,7 @@ func TestDeleteSecret(t *testing.T) {
 			})),
 		makeValidAkeylessTestCase("delete whole secret").SetExpectInput(&testingfake.PushSecretData{RemoteKey: "42"}).
 			SetMockClient(fakeakeyless.New().SetDescribeItemFn(func(ctx context.Context, itemName string) (*akeyless.Item, error) {
-				return &akeyless.Item{ItemTags: &[]string{ExtSecretManagedTag}}, nil
+				return &akeyless.Item{ItemTags: &[]string{extSecretManagedTag}}, nil
 			}).SetDeleteSecretFn(func(ctx context.Context, remoteKey string) error {
 				if remoteKey != "42" {
 					return fmt.Errorf("remote key %s expected %s", remoteKey, "42")
@@ -411,7 +411,7 @@ func TestDeleteSecret(t *testing.T) {
 			})),
 		makeValidAkeylessTestCase("delete property of secret").SetExpectInput(&testingfake.PushSecretData{Property: "Foo"}).
 			SetMockClient(fakeakeyless.New().SetDescribeItemFn(func(ctx context.Context, itemName string) (*akeyless.Item, error) {
-				return &akeyless.Item{ItemTags: &[]string{ExtSecretManagedTag}}, nil
+				return &akeyless.Item{ItemTags: &[]string{extSecretManagedTag}}, nil
 			}).SetGetSecretFn(func(secretName string, version int32) (string, error) {
 				return `{"Dio": "Brando", "Foo": "Fighters"}`, nil
 			}).
@@ -424,7 +424,7 @@ func TestDeleteSecret(t *testing.T) {
 				})),
 		makeValidAkeylessTestCase("delete secret if one property left").SetExpectInput(&testingfake.PushSecretData{RemoteKey: "Rings", Property: "Annatar"}).
 			SetMockClient(fakeakeyless.New().SetDescribeItemFn(func(ctx context.Context, itemName string) (*akeyless.Item, error) {
-				return &akeyless.Item{ItemTags: &[]string{ExtSecretManagedTag}}, nil
+				return &akeyless.Item{ItemTags: &[]string{extSecretManagedTag}}, nil
 			}).SetGetSecretFn(func(secretName string, version int32) (string, error) {
 				return `{"Annatar": "The Lord of Gifts"}`, nil
 			}).

+ 42 - 39
pkg/provider/aws/parameterstore/parameterstore.go

@@ -29,7 +29,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ssm"
 	"github.com/tidwall/gjson"
 	corev1 "k8s.io/api/core/v1"
-	utilpointer "k8s.io/utils/ptr"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
@@ -42,10 +42,10 @@ import (
 
 // Declares metadata information for pushing secrets to AWS Parameter Store.
 const (
-	PushSecretType  = "parameterStoreType"
-	StoreTypeString = "String"
-	StoreKeyID      = "parameterStoreKeyID"
-	PushSecretKeyID = "keyID"
+	pushSecretType  = "parameterStoreType"
+	storeTypeString = "String"
+	storeKeyID      = "parameterStoreKeyID"
+	pushSecretKeyID = "keyID"
 )
 
 // https://github.com/external-secrets/external-secrets/issues/644
@@ -154,12 +154,12 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 		err   error
 	)
 
-	parameterTypeFormat, err := utils.FetchValueFromMetadata(PushSecretType, data.GetMetadata(), StoreTypeString)
+	parameterTypeFormat, err := utils.FetchValueFromMetadata(pushSecretType, data.GetMetadata(), storeTypeString)
 	if err != nil {
 		return fmt.Errorf("failed to parse metadata: %w", err)
 	}
 
-	parameterKeyIDFormat, err := utils.FetchValueFromMetadata(StoreKeyID, data.GetMetadata(), PushSecretKeyID)
+	parameterKeyIDFormat, err := utils.FetchValueFromMetadata(storeKeyID, data.GetMetadata(), pushSecretKeyID)
 	if err != nil {
 		return fmt.Errorf("failed to parse metadata: %w", err)
 	}
@@ -168,8 +168,6 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 		parameterKeyIDFormat = "alias/aws/ssm"
 	}
 
-	overwrite := true
-
 	key := data.GetSecretKey()
 
 	if key == "" {
@@ -181,14 +179,12 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 		value = secret.Data[key]
 	}
 
-	stringValue := string(value)
 	secretName := pm.prefix + data.GetRemoteKey()
-
 	secretRequest := ssm.PutParameterInput{
-		Name:      &secretName,
-		Value:     &stringValue,
-		Type:      &parameterTypeFormat,
-		Overwrite: &overwrite,
+		Name:      ptr.To(pm.prefix + data.GetRemoteKey()),
+		Value:     ptr.To(string(value)),
+		Type:      ptr.To(parameterTypeFormat),
+		Overwrite: ptr.To(true),
 	}
 
 	if parameterTypeFormat == "SecureString" {
@@ -210,33 +206,37 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 
 	// If we have a valid parameter returned to us, check its tags
 	if existing != nil && existing.Parameter != nil {
-		tags, err := pm.getTagsByName(ctx, existing)
-		if err != nil {
-			return fmt.Errorf("error getting the existing tags for the parameter %v: %w", secretName, err)
-		}
+		return pm.setExisting(ctx, existing, secretName, value, secretRequest)
+	}
 
-		isManaged := isManagedByESO(tags)
+	// let's set the secret
+	// Do we need to delete the existing parameter on the remote?
+	return pm.setManagedRemoteParameter(ctx, secretRequest, true)
+}
 
-		if !isManaged {
-			return errors.New("secret not managed by external-secrets")
-		}
+func (pm *ParameterStore) setExisting(ctx context.Context, existing *ssm.GetParameterOutput, secretName string, value []byte, secretRequest ssm.PutParameterInput) error {
+	tags, err := pm.getTagsByName(ctx, existing)
+	if err != nil {
+		return fmt.Errorf("error getting the existing tags for the parameter %v: %w", secretName, err)
+	}
 
-		// When fetching a remote SecureString parameter without decrypting, the default value will always be 'sensitive'
-		// in this case, no updates will be pushed remotely
-		if existing.Parameter.Value != nil && *existing.Parameter.Value == "sensitive" {
-			return errors.New("unable to compare 'sensitive' result, ensure to request a decrypted value")
-		}
+	isManaged := isManagedByESO(tags)
 
-		if existing.Parameter.Value != nil && *existing.Parameter.Value == string(value) {
-			return nil
-		}
+	if !isManaged {
+		return errors.New("secret not managed by external-secrets")
+	}
 
-		return pm.setManagedRemoteParameter(ctx, secretRequest, false)
+	// When fetching a remote SecureString parameter without decrypting, the default value will always be 'sensitive'
+	// in this case, no updates will be pushed remotely
+	if existing.Parameter.Value != nil && *existing.Parameter.Value == "sensitive" {
+		return errors.New("unable to compare 'sensitive' result, ensure to request a decrypted value")
 	}
 
-	// let's set the secret
-	// Do we need to delete the existing parameter on the remote?
-	return pm.setManagedRemoteParameter(ctx, secretRequest, true)
+	if existing.Parameter.Value != nil && *existing.Parameter.Value == string(value) {
+		return nil
+	}
+
+	return pm.setManagedRemoteParameter(ctx, secretRequest, false)
 }
 
 func isManagedByESO(tags []*ssm.Tag) bool {
@@ -309,14 +309,17 @@ func (pm *ParameterStore) findByName(ctx context.Context, ref esv1beta1.External
 				logger.Info("GetParametersByPath: access denied. using fallback to describe parameters. It is recommended to add ssm:GetParametersByPath permissions", "path", ref.Path)
 				return pm.fallbackFindByName(ctx, ref)
 			}
+
 			return nil, err
 		}
+
 		for _, param := range it.Parameters {
 			if !matcher.MatchName(*param.Name) {
 				continue
 			}
 			data[*param.Name] = []byte(*param.Value)
 		}
+
 		nextToken = it.NextToken
 		if nextToken == nil {
 			break
@@ -375,9 +378,9 @@ func (pm *ParameterStore) findByTags(ctx context.Context, ref esv1beta1.External
 	filters := make([]*ssm.ParameterStringFilter, 0)
 	for k, v := range ref.Tags {
 		filters = append(filters, &ssm.ParameterStringFilter{
-			Key:    utilpointer.To(fmt.Sprintf("tag:%s", k)),
-			Values: []*string{utilpointer.To(v)},
-			Option: utilpointer.To("Equals"),
+			Key:    ptr.To(fmt.Sprintf("tag:%s", k)),
+			Values: []*string{ptr.To(v)},
+			Option: ptr.To("Equals"),
 		})
 	}
 
@@ -419,7 +422,7 @@ func (pm *ParameterStore) findByTags(ctx context.Context, ref esv1beta1.External
 
 func (pm *ParameterStore) fetchAndSet(ctx context.Context, data map[string][]byte, name string) error {
 	out, err := pm.client.GetParameterWithContext(ctx, &ssm.GetParameterInput{
-		Name:           utilpointer.To(name),
+		Name:           ptr.To(name),
 		WithDecryption: aws.Bool(true),
 	})
 	metrics.ObserveAPICall(constants.ProviderAWSPS, constants.CallAWSPSGetParameter, err)

+ 52 - 48
pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -113,55 +113,11 @@ func (sm *SecretsManager) fetch(ctx context.Context, ref esv1beta1.ExternalSecre
 		return secretOut, nil
 	}
 
-	var secretOut *awssm.GetSecretValueOutput
-	var err error
-
-	if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch {
-		describeSecretInput := &awssm.DescribeSecretInput{
-			SecretId: &ref.Key,
-		}
-
-		descOutput, err := sm.client.DescribeSecretWithContext(ctx, describeSecretInput)
-		if err != nil {
-			return nil, err
-		}
-		log.Info("found metadata secret", "key", ref.Key, "output", descOutput)
-
-		jsonTags, err := util.SecretTagsToJSONString(descOutput.Tags)
-		if err != nil {
-			return nil, err
-		}
-		secretOut = &awssm.GetSecretValueOutput{
-			ARN:          descOutput.ARN,
-			CreatedDate:  descOutput.CreatedDate,
-			Name:         descOutput.Name,
-			SecretString: &jsonTags,
-			VersionId:    &ver,
-		}
-	} else {
-		var getSecretValueInput *awssm.GetSecretValueInput
-		if strings.HasPrefix(ver, "uuid/") {
-			versionID := strings.TrimPrefix(ver, "uuid/")
-			getSecretValueInput = &awssm.GetSecretValueInput{
-				SecretId:  &ref.Key,
-				VersionId: &versionID,
-			}
-		} else {
-			getSecretValueInput = &awssm.GetSecretValueInput{
-				SecretId:     &ref.Key,
-				VersionStage: &ver,
-			}
-		}
-		secretOut, err = sm.client.GetSecretValue(getSecretValueInput)
-		metrics.ObserveAPICall(constants.ProviderAWSSM, constants.CallAWSSMGetSecretValue, err)
-		var nf *awssm.ResourceNotFoundException
-		if errors.As(err, &nf) {
-			return nil, esv1beta1.NoSecretErr
-		}
-		if err != nil {
-			return nil, err
-		}
+	secretOut, err := sm.constructSecretValue(ctx, ref, ver)
+	if err != nil {
+		return nil, err
 	}
+
 	sm.cache[cacheKey] = secretOut
 
 	return secretOut, nil
@@ -634,3 +590,51 @@ func (sm *SecretsManager) setSecretValues(secret *awssm.SecretValueEntry, data m
 		data[*secret.Name] = secret.SecretBinary
 	}
 }
+
+func (sm *SecretsManager) constructSecretValue(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef, ver string) (*awssm.GetSecretValueOutput, error) {
+	if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch {
+		describeSecretInput := &awssm.DescribeSecretInput{
+			SecretId: &ref.Key,
+		}
+
+		descOutput, err := sm.client.DescribeSecretWithContext(ctx, describeSecretInput)
+		if err != nil {
+			return nil, err
+		}
+		log.Info("found metadata secret", "key", ref.Key, "output", descOutput)
+
+		jsonTags, err := util.SecretTagsToJSONString(descOutput.Tags)
+		if err != nil {
+			return nil, err
+		}
+		return &awssm.GetSecretValueOutput{
+			ARN:          descOutput.ARN,
+			CreatedDate:  descOutput.CreatedDate,
+			Name:         descOutput.Name,
+			SecretString: &jsonTags,
+			VersionId:    &ver,
+		}, nil
+	}
+
+	var getSecretValueInput *awssm.GetSecretValueInput
+	if strings.HasPrefix(ver, "uuid/") {
+		versionID := strings.TrimPrefix(ver, "uuid/")
+		getSecretValueInput = &awssm.GetSecretValueInput{
+			SecretId:  &ref.Key,
+			VersionId: &versionID,
+		}
+	} else {
+		getSecretValueInput = &awssm.GetSecretValueInput{
+			SecretId:     &ref.Key,
+			VersionStage: &ver,
+		}
+	}
+	secretOut, err := sm.client.GetSecretValue(getSecretValueInput)
+	metrics.ObserveAPICall(constants.ProviderAWSSM, constants.CallAWSSMGetSecretValue, err)
+	var nf *awssm.ResourceNotFoundException
+	if errors.As(err, &nf) {
+		return nil, esv1beta1.NoSecretErr
+	}
+
+	return secretOut, err
+}

+ 7 - 6
pkg/provider/aws/secretsmanager/secretsmanager_test.go

@@ -62,6 +62,7 @@ const (
 	tagvalue1 = "tagvalue1"
 	tagname2  = "tagname2"
 	tagvalue2 = "tagvalue2"
+	fakeKey   = "fake-key"
 )
 
 func makeValidSecretsManagerTestCase() *secretsManagerTestCase {
@@ -464,11 +465,11 @@ func TestSetSecret(t *testing.T) {
 		ARN: &arn,
 	}
 
-	pushSecretDataWithoutProperty := fake.PushSecretData{SecretKey: secretKey, RemoteKey: "fake-key", Property: ""}
-	pushSecretDataWithMetadata := fake.PushSecretData{SecretKey: secretKey, RemoteKey: "fake-key", Property: "", Metadata: &apiextensionsv1.JSON{
+	pushSecretDataWithoutProperty := fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: ""}
+	pushSecretDataWithMetadata := fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: "", Metadata: &apiextensionsv1.JSON{
 		Raw: []byte(`{"secretPushFormat": "string"}`),
 	}}
-	pushSecretDataWithProperty := fake.PushSecretData{SecretKey: secretKey, RemoteKey: "fake-key", Property: "other-fake-property"}
+	pushSecretDataWithProperty := fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: "other-fake-property"}
 
 	type args struct {
 		store          *esv1beta1.AWSProvider
@@ -655,7 +656,7 @@ func TestSetSecret(t *testing.T) {
 						Version:      &defaultUpdatedVersion,
 					}),
 				},
-				pushSecretData: fake.PushSecretData{SecretKey: secretKey, RemoteKey: "fake-key", Property: "fake-property.other-fake-property"},
+				pushSecretData: fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: "fake-property.other-fake-property"},
 			},
 			want: want{
 				err: nil,
@@ -992,7 +993,7 @@ func TestDeleteSecret(t *testing.T) {
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			ref := fake.PushSecretData{RemoteKey: "fake-key"}
+			ref := fake.PushSecretData{RemoteKey: fakeKey}
 			sm := SecretsManager{
 				client: &tc.args.client,
 				config: &tc.args.config,
@@ -1333,7 +1334,7 @@ func TestSecretExists(t *testing.T) {
 	getSecretCorrectErr := awssm.ResourceNotFoundException{}
 	getSecretWrongErr := awssm.InvalidRequestException{}
 
-	pushSecretDataWithoutProperty := fake.PushSecretData{SecretKey: "fake-secret-key", RemoteKey: "fake-key", Property: ""}
+	pushSecretDataWithoutProperty := fake.PushSecretData{SecretKey: "fake-secret-key", RemoteKey: fakeKey, Property: ""}
 
 	type args struct {
 		store          *esv1beta1.AWSProvider

+ 7 - 6
pkg/provider/azure/keyvault/keyvault.go

@@ -66,6 +66,7 @@ const (
 	AnnotationClientID   = "azure.workload.identity/client-id"
 	AnnotationTenantID   = "azure.workload.identity/tenant-id"
 	managerLabel         = "external-secrets"
+	managedBy            = "managed-by"
 
 	errUnexpectedStoreSpec      = "unexpected store spec"
 	errMissingAuthType          = "cannot initialize Azure Client: no valid authType was specified"
@@ -246,7 +247,7 @@ func canDelete(tags map[string]*string, err error) (bool, error) {
 	if aerr.StatusCode == 404 {
 		return false, nil
 	}
-	manager, ok := tags["managed-by"]
+	manager, ok := tags[managedBy]
 	if !ok || manager == nil || *manager != managerLabel {
 		return false, errors.New("not managed by external-secrets")
 	}
@@ -410,7 +411,7 @@ func canCreate(tags map[string]*string, err error) (bool, error) {
 		return false, fmt.Errorf("unexpected api error: %w", err)
 	}
 	if err == nil {
-		manager, ok := tags["managed-by"]
+		manager, ok := tags[managedBy]
 		if !ok || manager == nil || *manager != managerLabel {
 			return false, errors.New("not managed by external-secrets")
 		}
@@ -441,7 +442,7 @@ func (a *Azure) setKeyVaultSecret(ctx context.Context, secretName string, value
 	secretParams := keyvault.SecretSetParameters{
 		Value: &val,
 		Tags: map[string]*string{
-			"managed-by": pointer.To(managerLabel),
+			managedBy: pointer.To(managerLabel),
 		},
 		SecretAttributes: &keyvault.SecretAttributes{
 			Enabled: pointer.To(true),
@@ -482,7 +483,7 @@ func (a *Azure) setKeyVaultCertificate(ctx context.Context, secretName string, v
 	params := keyvault.CertificateImportParameters{
 		Base64EncodedCertificate: &val,
 		Tags: map[string]*string{
-			"managed-by": pointer.To(managerLabel),
+			managedBy: pointer.To(managerLabel),
 		},
 	}
 	_, err = a.baseClient.ImportCertificate(ctx, *a.provider.VaultURL, secretName, params)
@@ -538,7 +539,7 @@ func (a *Azure) setKeyVaultKey(ctx context.Context, secretName string, value []b
 		Key:           &azkey,
 		KeyAttributes: &keyvault.KeyAttributes{},
 		Tags: map[string]*string{
-			"managed-by": pointer.To(managerLabel),
+			managedBy: pointer.To(managerLabel),
 		},
 	}
 	_, err = a.baseClient.ImportKey(ctx, *a.provider.VaultURL, secretName, params)
@@ -698,7 +699,7 @@ func parseError(err error) error {
 	return err
 }
 
-// Implements store.Client.GetSecret Interface.
+// GetSecret implements store.Client.GetSecret Interface.
 // Retrieves a secret/Key/Certificate/Tag with the secret name defined in ref.Name
 // 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) {

+ 51 - 48
pkg/provider/azure/keyvault/keyvault_test.go

@@ -140,6 +140,9 @@ const (
 	foo                  = "foo"
 	bar                  = "bar"
 	errStore             = "Azure.ValidateStore() error = %v, wantErr %v"
+	externalSecrets      = "external-secrets"
+	notFoundMessage      = "Not Found"
+	forbiddenMessage     = "Forbidden"
 )
 
 func getTagMap() map[string]*string {
@@ -176,7 +179,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: pointer.To("foo"),
 		}
@@ -187,8 +190,8 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		smtc.pushData = testingfake.PushSecretData{
 			RemoteKey: secretName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
-		smtc.deleteErr = autorest.DetailedError{StatusCode: 404, Method: "DELETE", Message: "Not Found"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
+		smtc.deleteErr = autorest.DetailedError{StatusCode: 404, Method: "DELETE", Message: notFoundMessage}
 	}
 
 	secretNotManaged := func(smtc *secretManagerTestCase) {
@@ -216,7 +219,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: pointer.To("foo"),
 		}
@@ -238,7 +241,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		}
 		smtc.certOutput = keyvault.CertificateBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 		smtc.deleteCertificateOutput = keyvault.DeletedCertificateBundle{}
@@ -248,7 +251,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 			RemoteKey: certName,
 		}
 		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Certificate Not Found"}
-		smtc.deleteErr = autorest.DetailedError{StatusCode: 404, Method: "DELETE", Message: "Not Found"}
+		smtc.deleteErr = autorest.DetailedError{StatusCode: 404, Method: "DELETE", Message: notFoundMessage}
 	}
 
 	certNotManaged := func(smtc *secretManagerTestCase) {
@@ -274,7 +277,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		}
 		smtc.certOutput = keyvault.CertificateBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 		smtc.expectError = "No certificate delete Permissions"
@@ -295,7 +298,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 		smtc.deleteKeyOutput = keyvault.DeletedKeyBundle{}
@@ -304,8 +307,8 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		smtc.pushData = testingfake.PushSecretData{
 			RemoteKey: keyName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
-		smtc.deleteErr = autorest.DetailedError{StatusCode: 404, Method: "DELETE", Message: "Not Found"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
+		smtc.deleteErr = autorest.DetailedError{StatusCode: 404, Method: "DELETE", Message: notFoundMessage}
 	}
 
 	keyNotManaged := func(smtc *secretManagerTestCase) {
@@ -331,7 +334,7 @@ func TestAzureKeyVaultDeleteSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 		smtc.expectError = errNoPermission
@@ -405,7 +408,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: &goodSecret,
 		}
@@ -418,7 +421,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: &goodSecret,
 		}
@@ -445,7 +448,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: &goodSecret,
 			Attributes: &keyvault.SecretAttributes{
@@ -454,7 +457,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.setSecretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: &goodSecret,
 			Attributes: &keyvault.SecretAttributes{
@@ -470,7 +473,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("nope"),
+				managedBy: pointer.To("nope"),
 			},
 			Value: &goodSecret,
 		}
@@ -486,7 +489,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 			Value: &wholeSecretString,
 		}
@@ -512,7 +515,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			SecretKey: secretKey,
 			RemoteKey: secretName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
 	}
 	failedGetSecret := func(smtc *secretManagerTestCase) {
 		smtc.setValue = []byte(goodSecret)
@@ -520,7 +523,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			SecretKey: secretKey,
 			RemoteKey: secretName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 403, Method: "GET", Message: "Forbidden"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 403, Method: "GET", Message: forbiddenMessage}
 		smtc.expectError = errAPI
 	}
 	failedNotParseableError := func(smtc *secretManagerTestCase) {
@@ -538,8 +541,8 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			SecretKey: secretKey,
 			RemoteKey: secretName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
-		smtc.setErr = autorest.DetailedError{StatusCode: 403, Method: "POST", Message: "Forbidden"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
+		smtc.setErr = autorest.DetailedError{StatusCode: 403, Method: "POST", Message: forbiddenMessage}
 		smtc.expectError = "could not set secret example-1: #POST: Forbidden: StatusCode=403"
 	}
 	keySuccess := func(smtc *secretManagerTestCase) {
@@ -550,7 +553,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To(managerLabel),
+				managedBy: pointer.To(managerLabel),
 			},
 			Key: &keyvault.JSONWebKey{},
 		}
@@ -563,7 +566,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To(managerLabel),
+				managedBy: pointer.To(managerLabel),
 			},
 			Key: &keyvault.JSONWebKey{},
 		}
@@ -576,7 +579,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To(managerLabel),
+				managedBy: pointer.To(managerLabel),
 			},
 			Key: &keyvault.JSONWebKey{},
 		}
@@ -589,7 +592,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To(managerLabel),
+				managedBy: pointer.To(managerLabel),
 			},
 			Key: &keyvault.JSONWebKey{},
 		}
@@ -602,7 +605,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To(managerLabel),
+				managedBy: pointer.To(managerLabel),
 			},
 			Key: &keyvault.JSONWebKey{},
 		}
@@ -629,7 +632,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		}
 		smtc.keyOutput = keyvault.KeyBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("internal-secrets"),
+				managedBy: pointer.To("internal-secrets"),
 			},
 			Key: &keyvault.JSONWebKey{},
 		}
@@ -641,7 +644,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			SecretKey: secretKey,
 			RemoteKey: keyName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 403, Method: "GET", Message: "Forbidden"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 403, Method: "GET", Message: forbiddenMessage}
 		smtc.expectError = errAPI
 	}
 	keyNotFound := func(smtc *secretManagerTestCase) {
@@ -650,7 +653,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			SecretKey: secretKey,
 			RemoteKey: keyName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
 		smtc.expectError = ""
 	}
 	importKeyFailed := func(smtc *secretManagerTestCase) {
@@ -659,8 +662,8 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			SecretKey: secretKey,
 			RemoteKey: keyName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
-		smtc.setErr = autorest.DetailedError{StatusCode: 403, Method: "POST", Message: "Forbidden"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
+		smtc.setErr = autorest.DetailedError{StatusCode: 403, Method: "POST", Message: forbiddenMessage}
 		smtc.expectError = "could not import key keyname: #POST: Forbidden: StatusCode=403"
 	}
 	certP12Success := func(smtc *secretManagerTestCase) {
@@ -672,7 +675,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -685,7 +688,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -698,7 +701,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -712,7 +715,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -727,7 +730,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -742,7 +745,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -757,7 +760,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 		smtc.expectError = "could not import certificate certname: error"
@@ -774,7 +777,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			Cer: &cert,
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				managedBy: pointer.To(externalSecrets),
 			},
 		}
 	}
@@ -788,7 +791,7 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 		smtc.certOutput = keyvault.CertificateBundle{
 			X509Thumbprint: pointer.To("123"),
 			Tags: map[string]*string{
-				"managed-by": pointer.To("foobar"),
+				managedBy: pointer.To("foobar"),
 			},
 		}
 		smtc.expectError = "certificate certname: not managed by external-secrets"
@@ -888,17 +891,17 @@ func TestAzureKeyVaultPushSecret(t *testing.T) {
 			if err == nil {
 				t.Errorf("[%d] unexpected error: <nil>, expected: '%s'", k, v.expectError)
 			} else {
-				t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+				t.Errorf(unexpectedError, k, err.Error(), v.expectError)
 			}
 		}
 		if len(v.expectedData) > 0 {
 			sm.baseClient = v.mockClient
 			out, err := sm.GetSecretMap(context.Background(), *v.ref)
 			if !utils.ErrorContains(err, v.expectError) {
-				t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+				t.Errorf(unexpectedError, k, err.Error(), v.expectError)
 			}
 			if err == nil && !reflect.DeepEqual(out, v.expectedData) {
-				t.Errorf("[%d] unexpected secret data: expected %#v, got %#v", k, v.expectedData, out)
+				t.Errorf(unexpectedSecretData, k, v.expectedData, out)
 			}
 		}
 	}
@@ -1271,7 +1274,7 @@ func TestAzureKeyVaultSecretManagerGetSecret(t *testing.T) {
 		sm.baseClient = v.mockClient
 		out, err := sm.GetSecret(context.Background(), *v.ref)
 		if !utils.ErrorContains(err, v.expectError) {
-			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+			t.Errorf(unexpectedError, k, err.Error(), v.expectError)
 		}
 		if string(out) != v.expectedSecret {
 			t.Errorf("[%d] unexpected secret: expected %s, got %s", k, v.expectedSecret, string(out))
@@ -1430,10 +1433,10 @@ func TestAzureKeyVaultSecretManagerGetSecretMap(t *testing.T) {
 		sm.baseClient = v.mockClient
 		out, err := sm.GetSecretMap(context.Background(), *v.ref)
 		if !utils.ErrorContains(err, v.expectError) {
-			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
+			t.Errorf(unexpectedError, k, err.Error(), v.expectError)
 		}
 		if err == nil && !reflect.DeepEqual(out, v.expectedData) {
-			t.Errorf("[%d] unexpected secret data: expected %#v, got %#v", k, v.expectedData, out)
+			t.Errorf(unexpectedSecretData, k, v.expectedData, out)
 		}
 	}
 }
@@ -1734,7 +1737,7 @@ func TestAzureKeyVaultSecretExists(t *testing.T) {
 		}
 		smtc.secretOutput = keyvault.SecretBundle{
 			Tags: map[string]*string{
-				"managed-by": pointer.To("external-secrets"),
+				"managed-by": pointer.To(externalSecrets),
 			},
 			Value: pointer.To("foo"),
 		}
@@ -1758,7 +1761,7 @@ func TestAzureKeyVaultSecretExists(t *testing.T) {
 		smtc.pushData = testingfake.PushSecretData{
 			RemoteKey: secretName,
 		}
-		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: "Not Found"}
+		smtc.apiErr = autorest.DetailedError{StatusCode: 404, Method: "GET", Message: notFoundMessage}
 		smtc.expectedExistence = false
 	}
 

+ 6 - 4
pkg/provider/bitwarden/bitwarden_sdk.go

@@ -33,6 +33,8 @@ const (
 	WardenHeaderAccessToken = "Warden-Access-Token"
 	WardenHeaderAPIURL      = "Warden-Api-Url"
 	WardenHeaderIdentityURL = "Warden-Identity-Url"
+
+	restAPIURL = "/rest/api/1/secret"
 )
 
 type SecretResponse struct {
@@ -130,7 +132,7 @@ func (s *SdkClient) GetSecret(ctx context.Context, id string) (*SecretResponse,
 
 	if err := s.performHTTPRequestOperation(ctx, params{
 		method: http.MethodGet,
-		url:    s.bitwardenSdkServerURL + "/rest/api/1/secret",
+		url:    s.bitwardenSdkServerURL + restAPIURL,
 		body:   body,
 		result: &secretResp,
 	}); err != nil {
@@ -150,7 +152,7 @@ func (s *SdkClient) DeleteSecret(ctx context.Context, ids []string) (*SecretsDel
 	secretResp := &SecretsDeleteResponse{}
 	if err := s.performHTTPRequestOperation(ctx, params{
 		method: http.MethodDelete,
-		url:    s.bitwardenSdkServerURL + "/rest/api/1/secret",
+		url:    s.bitwardenSdkServerURL + restAPIURL,
 		body:   body,
 		result: &secretResp,
 	}); err != nil {
@@ -164,7 +166,7 @@ func (s *SdkClient) CreateSecret(ctx context.Context, createReq SecretCreateRequ
 	secretResp := &SecretResponse{}
 	if err := s.performHTTPRequestOperation(ctx, params{
 		method: http.MethodPost,
-		url:    s.bitwardenSdkServerURL + "/rest/api/1/secret",
+		url:    s.bitwardenSdkServerURL + restAPIURL,
 		body:   createReq,
 		result: &secretResp,
 	}); err != nil {
@@ -178,7 +180,7 @@ func (s *SdkClient) UpdateSecret(ctx context.Context, putReq SecretPutRequest) (
 	secretResp := &SecretResponse{}
 	if err := s.performHTTPRequestOperation(ctx, params{
 		method: http.MethodPut,
-		url:    s.bitwardenSdkServerURL + "/rest/api/1/secret",
+		url:    s.bitwardenSdkServerURL + restAPIURL,
 		body:   putReq,
 		result: &secretResp,
 	}); err != nil {

+ 1 - 1
pkg/provider/bitwarden/bitwarden_sdk_test.go

@@ -26,7 +26,7 @@ import (
 // The rest of the tests much look the same, it would be nice if I could find a way
 // to nicely unify the tests for all of them.
 
-func TestSdkClient_CreateSecret(t *testing.T) {
+func TestSdkClientCreateSecret(t *testing.T) {
 	type fields struct {
 		apiURL                func(c *httptest.Server) string
 		identityURL           func(c *httptest.Server) string

+ 40 - 26
pkg/provider/bitwarden/client.go

@@ -24,6 +24,7 @@ import (
 	"gopkg.in/yaml.v3"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/kube-openapi/pkg/validation/strfmt"
+	"k8s.io/utils/ptr"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 	"github.com/external-secrets/external-secrets/pkg/utils"
@@ -32,6 +33,13 @@ import (
 const (
 	// NoteMetadataKey defines the note for the pushed secret.
 	NoteMetadataKey = "note"
+
+	errNoProvider = "store does not have a provider"
+)
+
+var (
+	errFailedToGetAllSecrets = "failed to get all secrets: %w"
+	errFailedToGetSecret     = "failed to get secret: %w"
 )
 
 // PushSecret will write a single secret into the provider.
@@ -46,7 +54,7 @@ const (
 func (p *Provider) PushSecret(ctx context.Context, secret *corev1.Secret, data esv1beta1.PushSecretData) error {
 	spec := p.store.GetSpec()
 	if spec == nil || spec.Provider == nil {
-		return errors.New("store does not have a provider")
+		return errors.New(errNoProvider)
 	}
 
 	if data.GetRemoteKey() == "" {
@@ -84,7 +92,7 @@ func (p *Provider) PushSecret(ctx context.Context, secret *corev1.Secret, data e
 	// ListAll Secrets for an organization. If the key matches our key, we GetSecret that and do a compare.
 	remoteSecrets, err := p.bitwardenSdkClient.ListSecrets(ctx, spec.Provider.BitwardenSecretsManager.OrganizationID)
 	if err != nil {
-		return fmt.Errorf("failed to get all secrets: %w", err)
+		return fmt.Errorf(errFailedToGetAllSecrets, err)
 	}
 
 	for _, d := range remoteSecrets.Data {
@@ -98,18 +106,10 @@ func (p *Provider) PushSecret(ctx context.Context, secret *corev1.Secret, data e
 		}
 
 		// If all pushed data matches, we won't push this secret.
-		if sec.Key == data.GetRemoteKey() &&
-			sec.Value == string(value) &&
-			sec.Note == note &&
-			sec.ProjectID != nil &&
-			*sec.ProjectID == spec.Provider.BitwardenSecretsManager.ProjectID {
+		if p.isExactlySameSecret(sec, data.GetRemoteKey(), note, spec.Provider.BitwardenSecretsManager.ProjectID, value) {
 			// we have a complete match, skip pushing.
 			return nil
-		} else if sec.Key == data.GetRemoteKey() &&
-			sec.Value != string(value) &&
-			sec.Note == note &&
-			sec.ProjectID != nil &&
-			*sec.ProjectID == spec.Provider.BitwardenSecretsManager.ProjectID {
+		} else if p.isOnlyValueDifferent(sec, data.GetRemoteKey(), note, spec.Provider.BitwardenSecretsManager.ProjectID, value) {
 			// only the value is different, update the existing secret.
 			_, err = p.bitwardenSdkClient.UpdateSecret(ctx, SecretPutRequest{
 				ID:             sec.ID,
@@ -136,12 +136,26 @@ func (p *Provider) PushSecret(ctx context.Context, secret *corev1.Secret, data e
 	return err
 }
 
+func (p *Provider) isExactlySameSecret(sec *SecretResponse, remoteKey, note, projectID string, value []byte) bool {
+	return sec.Key == remoteKey &&
+		sec.Value == string(value) &&
+		sec.Note == note &&
+		ptr.Deref(sec.ProjectID, "") == projectID
+}
+
+func (p *Provider) isOnlyValueDifferent(sec *SecretResponse, remoteKey, note, projectID string, value []byte) bool {
+	return sec.Key == remoteKey &&
+		sec.Value != string(value) &&
+		sec.Note == note &&
+		ptr.Deref(sec.ProjectID, "") == projectID
+}
+
 // GetSecret returns a single secret from the provider.
 func (p *Provider) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	if strfmt.IsUUID(ref.Key) {
 		resp, err := p.bitwardenSdkClient.GetSecret(ctx, ref.Key)
 		if err != nil {
-			return nil, fmt.Errorf("error getting secret: %w", err)
+			return nil, fmt.Errorf(errFailedToGetSecret, err)
 		}
 
 		return []byte(resp.Value), nil
@@ -149,12 +163,12 @@ func (p *Provider) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDa
 
 	spec := p.store.GetSpec()
 	if spec == nil || spec.Provider == nil {
-		return nil, errors.New("store does not have a provider")
+		return nil, errors.New(errNoProvider)
 	}
 
 	secret, err := p.findSecretByRef(ctx, ref.Key, spec.Provider.BitwardenSecretsManager.ProjectID)
 	if err != nil {
-		return nil, fmt.Errorf("error getting secret: %w", err)
+		return nil, fmt.Errorf(errFailedToGetSecret, err)
 	}
 
 	if secret == nil {
@@ -172,12 +186,12 @@ func (p *Provider) DeleteSecret(ctx context.Context, ref esv1beta1.PushSecretRem
 
 	spec := p.store.GetSpec()
 	if spec == nil || spec.Provider == nil {
-		return errors.New("store does not have a provider")
+		return errors.New(errNoProvider)
 	}
 
 	secret, err := p.findSecretByRef(ctx, ref.GetRemoteKey(), spec.Provider.BitwardenSecretsManager.ProjectID)
 	if err != nil {
-		return fmt.Errorf("error getting secret: %w", err)
+		return fmt.Errorf(errFailedToGetSecret, err)
 	}
 
 	if secret == nil {
@@ -210,7 +224,7 @@ func (p *Provider) SecretExists(ctx context.Context, ref esv1beta1.PushSecretRem
 	if strfmt.IsUUID(ref.GetRemoteKey()) {
 		_, err := p.bitwardenSdkClient.GetSecret(ctx, ref.GetRemoteKey())
 		if err != nil {
-			return false, fmt.Errorf("error getting secret: %w", err)
+			return false, fmt.Errorf(errFailedToGetSecret, err)
 		}
 
 		return true, nil
@@ -218,12 +232,12 @@ func (p *Provider) SecretExists(ctx context.Context, ref esv1beta1.PushSecretRem
 
 	spec := p.store.GetSpec()
 	if spec == nil || spec.Provider == nil {
-		return false, errors.New("store does not have a provider")
+		return false, errors.New(errNoProvider)
 	}
 
 	secret, err := p.findSecretByRef(ctx, ref.GetRemoteKey(), spec.Provider.BitwardenSecretsManager.ProjectID)
 	if err != nil {
-		return false, fmt.Errorf("error getting secret: %w", err)
+		return false, fmt.Errorf(errFailedToGetSecret, err)
 	}
 
 	if secret == nil {
@@ -296,19 +310,19 @@ func (p *Provider) parseYamlSecretData(data []byte) (map[string][]byte, error) {
 func (p *Provider) GetAllSecrets(ctx context.Context, _ esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	spec := p.store.GetSpec()
 	if spec == nil {
-		return nil, errors.New("store does not have a provider")
+		return nil, errors.New(errNoProvider)
 	}
 
 	secrets, err := p.bitwardenSdkClient.ListSecrets(ctx, spec.Provider.BitwardenSecretsManager.OrganizationID)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get all secrets: %w", err)
+		return nil, fmt.Errorf(errFailedToGetAllSecrets, err)
 	}
 
 	result := map[string][]byte{}
 	for _, d := range secrets.Data {
 		sec, err := p.bitwardenSdkClient.GetSecret(ctx, d.ID)
 		if err != nil {
-			return nil, fmt.Errorf("failed to get secret: %w", err)
+			return nil, fmt.Errorf(errFailedToGetSecret, err)
 		}
 
 		result[d.ID] = []byte(sec.Value)
@@ -330,13 +344,13 @@ func (p *Provider) Close(_ context.Context) error {
 func (p *Provider) findSecretByRef(ctx context.Context, key, projectID string) (*SecretResponse, error) {
 	spec := p.store.GetSpec()
 	if spec == nil || spec.Provider == nil {
-		return nil, errors.New("store does not have a provider")
+		return nil, errors.New(errNoProvider)
 	}
 
 	// ListAll Secrets for an organization. If the key matches our key, we GetSecret that and do a compare.
 	secrets, err := p.bitwardenSdkClient.ListSecrets(ctx, spec.Provider.BitwardenSecretsManager.OrganizationID)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get all secrets: %w", err)
+		return nil, fmt.Errorf(errFailedToGetAllSecrets, err)
 	}
 
 	var remoteSecret *SecretResponse
@@ -347,7 +361,7 @@ func (p *Provider) findSecretByRef(ctx context.Context, key, projectID string) (
 
 		sec, err := p.bitwardenSdkClient.GetSecret(ctx, d.ID)
 		if err != nil {
-			return nil, fmt.Errorf("failed to get secret: %w", err)
+			return nil, fmt.Errorf(errFailedToGetSecret, err)
 		}
 
 		if sec.ProjectID != nil && *sec.ProjectID == projectID {

+ 62 - 56
pkg/provider/bitwarden/client_test.go

@@ -28,6 +28,11 @@ import (
 	"github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 )
 
+const (
+	remoteID = "d8f29773-3019-4973-9bbc-66327d077fe2"
+	testKey  = "this-is-a-name"
+)
+
 var projectID = "e8fc8f9c-2208-446e-9e89-9bc358f39b47"
 
 func TestProviderDeleteSecret(t *testing.T) {
@@ -72,7 +77,7 @@ func TestProviderDeleteSecret(t *testing.T) {
 			args: args{
 				ctx: context.TODO(),
 				ref: v1alpha1.PushSecretRemoteRef{
-					RemoteKey: "d8f29773-3019-4973-9bbc-66327d077fe2",
+					RemoteKey: remoteID,
 				},
 			},
 		},
@@ -94,15 +99,15 @@ func TestProviderDeleteSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
 					})
 
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -118,7 +123,7 @@ func TestProviderDeleteSecret(t *testing.T) {
 			args: args{
 				ctx: context.TODO(),
 				ref: v1alpha1.PushSecretRemoteRef{
-					RemoteKey: "d8f29773-3019-4973-9bbc-66327d077fe2",
+					RemoteKey: remoteID,
 				},
 			},
 		},
@@ -140,8 +145,8 @@ func TestProviderDeleteSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
@@ -149,8 +154,8 @@ func TestProviderDeleteSecret(t *testing.T) {
 
 					projectID := "another-project"
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-						Key:            "this-is-a-name",
+						ID:             remoteID,
+						Key:            testKey,
 						Note:           "note",
 						OrganizationID: "orgid",
 						Value:          "value",
@@ -165,7 +170,7 @@ func TestProviderDeleteSecret(t *testing.T) {
 			args: args{
 				ctx: context.TODO(),
 				ref: v1alpha1.PushSecretRemoteRef{
-					RemoteKey: "this-is-a-name",
+					RemoteKey: testKey,
 				},
 			},
 		},
@@ -226,7 +231,7 @@ func TestProviderGetAllSecrets(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+								ID:             remoteID,
 								Key:            "key1",
 								OrganizationID: "orgid",
 							},
@@ -239,7 +244,7 @@ func TestProviderGetAllSecrets(t *testing.T) {
 					})
 
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:    "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:    remoteID,
 						Key:   "key1",
 						Value: "value1",
 					})
@@ -255,7 +260,7 @@ func TestProviderGetAllSecrets(t *testing.T) {
 				ref: v1beta1.ExternalSecretFind{},
 			},
 			want: map[string][]byte{
-				"d8f29773-3019-4973-9bbc-66327d077fe2": []byte("value1"),
+				remoteID:                               []byte("value1"),
 				"7c0d21ec-10d9-4972-bdf8-ec52df99cc86": []byte("value2"),
 			},
 		},
@@ -322,7 +327,7 @@ func TestProviderGetSecret(t *testing.T) {
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key: "d8f29773-3019-4973-9bbc-66327d077fe2",
+					Key: remoteID,
 				},
 			},
 			want: []byte("value"),
@@ -348,15 +353,15 @@ func TestProviderGetSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
 					})
 
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -368,7 +373,7 @@ func TestProviderGetSecret(t *testing.T) {
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key: "this-is-a-name",
+					Key: testKey,
 				},
 			},
 			want: []byte("value"),
@@ -429,7 +434,7 @@ func TestProviderPushSecret(t *testing.T) {
 					Match: v1alpha1.PushSecretMatch{
 						SecretKey: "key",
 						RemoteRef: v1alpha1.PushSecretRemoteRef{
-							RemoteKey: "this-is-a-name",
+							RemoteKey: testKey,
 						},
 					},
 				},
@@ -453,14 +458,14 @@ func TestProviderPushSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
 					})
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "no-match", // if this is this-is-a-name it would match
 						Note:           "",
 						OrganizationID: "orgid",
@@ -472,7 +477,7 @@ func TestProviderPushSecret(t *testing.T) {
 				assertMock: func(t *testing.T, c *FakeClient) {
 					cargs := c.createSecretCallArguments[0]
 					assert.Equal(t, cargs, SecretCreateRequest{
-						Key:            "this-is-a-name",
+						Key:            testKey,
 						Note:           "",
 						OrganizationID: "orgid",
 						ProjectIDS:     []string{projectID},
@@ -493,7 +498,7 @@ func TestProviderPushSecret(t *testing.T) {
 				data: v1alpha1.PushSecretData{
 					Match: v1alpha1.PushSecretMatch{
 						RemoteRef: v1alpha1.PushSecretRemoteRef{
-							RemoteKey: "this-is-a-name",
+							RemoteKey: testKey,
 						},
 					},
 				},
@@ -517,14 +522,14 @@ func TestProviderPushSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
 					})
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "no-match", // if this is this-is-a-name it would match
 						Note:           "",
 						OrganizationID: "orgid",
@@ -536,7 +541,7 @@ func TestProviderPushSecret(t *testing.T) {
 				assertMock: func(t *testing.T, c *FakeClient) {
 					cargs := c.createSecretCallArguments[0]
 					assert.Equal(t, SecretCreateRequest{
-						Key:            "this-is-a-name",
+						Key:            testKey,
 						Note:           "",
 						OrganizationID: "orgid",
 						ProjectIDS:     []string{projectID},
@@ -546,7 +551,7 @@ func TestProviderPushSecret(t *testing.T) {
 			},
 		},
 		{
-			name: "push secret is successful for a existing remote secret but only the value differs will call update",
+			name: "push secret is successful for an existing remote secret but only the value differs will call update",
 			args: args{
 				ctx: context.Background(),
 				secret: &corev1.Secret{
@@ -558,7 +563,7 @@ func TestProviderPushSecret(t *testing.T) {
 					Match: v1alpha1.PushSecretMatch{
 						SecretKey: "key",
 						RemoteRef: v1alpha1.PushSecretRemoteRef{
-							RemoteKey: "this-is-a-name",
+							RemoteKey: testKey,
 						},
 					},
 				},
@@ -582,15 +587,15 @@ func TestProviderPushSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
 					})
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-						Key:            "this-is-a-name",
+						ID:             remoteID,
+						Key:            testKey,
 						Note:           "",
 						OrganizationID: "orgid",
 						Value:          "value",
@@ -601,8 +606,8 @@ func TestProviderPushSecret(t *testing.T) {
 				assertMock: func(t *testing.T, c *FakeClient) {
 					pargs := c.updateSecretCallArguments[0]
 					assert.Equal(t, pargs, SecretPutRequest{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-						Key:            "this-is-a-name",
+						ID:             remoteID,
+						Key:            testKey,
 						Note:           "",
 						OrganizationID: "orgid",
 						ProjectIDS:     []string{projectID},
@@ -624,7 +629,7 @@ func TestProviderPushSecret(t *testing.T) {
 					Match: v1alpha1.PushSecretMatch{
 						SecretKey: "key",
 						RemoteRef: v1alpha1.PushSecretRemoteRef{
-							RemoteKey: "this-is-a-name",
+							RemoteKey: testKey,
 						},
 					},
 				},
@@ -648,15 +653,15 @@ func TestProviderPushSecret(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-								Key:            "this-is-a-name",
+								ID:             remoteID,
+								Key:            testKey,
 								OrganizationID: "orgid",
 							},
 						},
 					})
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
-						Key:            "this-is-a-name",
+						ID:             remoteID,
+						Key:            testKey,
 						OrganizationID: "orgid",
 						Value:          "value",
 						ProjectID:      &projectID,
@@ -735,7 +740,7 @@ func TestProviderSecretExists(t *testing.T) {
 				ref: v1alpha1.PushSecretData{
 					Match: v1alpha1.PushSecretMatch{
 						RemoteRef: v1alpha1.PushSecretRemoteRef{
-							RemoteKey: "d8f29773-3019-4973-9bbc-66327d077fe2",
+							RemoteKey: remoteID,
 						},
 					},
 				},
@@ -759,14 +764,14 @@ func TestProviderSecretExists(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+								ID:             remoteID,
 								Key:            "name",
 								OrganizationID: "orgid",
 							},
 						},
 					})
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "name",
 						OrganizationID: "orgid",
 						Value:          "value",
@@ -803,7 +808,7 @@ func TestProviderSecretExists(t *testing.T) {
 					c.ListSecretReturnsOnCallN(0, &SecretIdentifiersResponse{
 						Data: []SecretIdentifierResponse{
 							{
-								ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+								ID:             remoteID,
 								Key:            "name",
 								OrganizationID: "orgid",
 							},
@@ -811,7 +816,7 @@ func TestProviderSecretExists(t *testing.T) {
 					})
 					projectIDDifferent := "different-project"
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "name",
 						OrganizationID: "orgid",
 						Value:          "value",
@@ -845,6 +850,7 @@ func TestProviderSecretExists(t *testing.T) {
 					},
 				},
 				mock: func(c *FakeClient) {
+					// no mocking needed
 				},
 				assertMock: func(t *testing.T, c *FakeClient) {
 					assert.Equal(t, 0, c.listSecretsCalledN)
@@ -916,7 +922,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 				store:     &v1beta1.SecretStore{},
 				mock: func(c *FakeClient) {
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -927,7 +933,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key:      "d8f29773-3019-4973-9bbc-66327d077fe2",
+					Key:      remoteID,
 					Property: "key",
 				},
 				key: "key",
@@ -944,7 +950,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 				store:     &v1beta1.SecretStore{},
 				mock: func(c *FakeClient) {
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -955,7 +961,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key:      "d8f29773-3019-4973-9bbc-66327d077fe2",
+					Key:      remoteID,
 					Property: "key",
 				},
 				key: "key",
@@ -972,7 +978,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 				store:     &v1beta1.SecretStore{},
 				mock: func(c *FakeClient) {
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -984,7 +990,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key:      "d8f29773-3019-4973-9bbc-66327d077fe2",
+					Key:      remoteID,
 					Property: "key",
 				},
 				key: "key",
@@ -1001,7 +1007,7 @@ func TestProviderGetSecretMap(t *testing.T) {
 				store:     &v1beta1.SecretStore{},
 				mock: func(c *FakeClient) {
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -1013,7 +1019,7 @@ key2: !!binary VGhpcyBpcyBhIHRlc3Q=`,
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key:      "d8f29773-3019-4973-9bbc-66327d077fe2",
+					Key:      remoteID,
 					Property: "key2",
 				},
 				key: "key2",
@@ -1030,7 +1036,7 @@ key2: !!binary VGhpcyBpcyBhIHRlc3Q=`,
 				store:     &v1beta1.SecretStore{},
 				mock: func(c *FakeClient) {
 					c.GetSecretReturnsOnCallN(0, &SecretResponse{
-						ID:             "d8f29773-3019-4973-9bbc-66327d077fe2",
+						ID:             remoteID,
 						Key:            "key",
 						Note:           "note",
 						OrganizationID: "org",
@@ -1041,7 +1047,7 @@ key2: !!binary VGhpcyBpcyBhIHRlc3Q=`,
 			args: args{
 				ctx: context.Background(),
 				ref: v1beta1.ExternalSecretDataRemoteRef{
-					Key:      "d8f29773-3019-4973-9bbc-66327d077fe2",
+					Key:      remoteID,
 					Property: "nope",
 				},
 			},

+ 10 - 10
pkg/provider/gcp/secretmanager/client.go

@@ -519,15 +519,7 @@ func (c *Client) getSecretMetadata(ctx context.Context, ref esv1beta1.ExternalSe
 		labels      = "labels"
 	)
 
-	extractMetadataKey := func(s string, p string) string {
-		prefix := p + "."
-		if !strings.HasPrefix(s, prefix) {
-			return ""
-		}
-		return strings.TrimPrefix(s, prefix)
-	}
-
-	if annotation := extractMetadataKey(ref.Property, annotations); annotation != "" {
+	if annotation := c.extractMetadataKey(ref.Property, annotations); annotation != "" {
 		v, ok := secret.GetAnnotations()[annotation]
 		if !ok {
 			return nil, fmt.Errorf("annotation with key %s does not exist in secret %s", annotation, ref.Key)
@@ -536,7 +528,7 @@ func (c *Client) getSecretMetadata(ctx context.Context, ref esv1beta1.ExternalSe
 		return []byte(v), nil
 	}
 
-	if label := extractMetadataKey(ref.Property, labels); label != "" {
+	if label := c.extractMetadataKey(ref.Property, labels); label != "" {
 		v, ok := secret.GetLabels()[label]
 		if !ok {
 			return nil, fmt.Errorf("label with key %s does not exist in secret %s", label, ref.Key)
@@ -578,6 +570,14 @@ func (c *Client) getSecretMetadata(ctx context.Context, ref esv1beta1.ExternalSe
 	return j, nil
 }
 
+func (c *Client) extractMetadataKey(s, p string) string {
+	prefix := p + "."
+	if !strings.HasPrefix(s, prefix) {
+		return ""
+	}
+	return strings.TrimPrefix(s, prefix)
+}
+
 // GetSecretMap returns multiple k/v pairs from the provider.
 func (c *Client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	if c.smClient == nil || c.store.ProjectID == "" {

+ 30 - 27
pkg/provider/gcp/secretmanager/client_test.go

@@ -44,6 +44,9 @@ const (
 	errCallNotFoundAtIndex0   = "index 0 for call not found in the list of calls"
 	usEast1                   = "us-east1"
 	errInvalidReplicationType = "req.Secret.Replication.Replication was not of type *secretmanagerpb.Replication_UserManaged_ but: %T"
+	testSecretName            = "projects/foo/secret/bar"
+	managedBy                 = "managed-by"
+	externalSecrets           = "external-secrets"
 )
 
 type secretManagerTestCase struct {
@@ -207,7 +210,7 @@ func TestSecretManagerGetSecret(t *testing.T) {
 	}
 }
 
-func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
+func TestGetSecretMetadataPolicyFetch(t *testing.T) {
 	tests := []struct {
 		name                string
 		ref                 esv1beta1.ExternalSecretDataRemoteRef
@@ -224,14 +227,14 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Annotations: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy: externalSecrets,
 					},
 				},
 				Err: nil,
 			},
-			expectedSecret: "external-secrets",
+			expectedSecret: externalSecrets,
 		},
 		{
 			name: "label is specified",
@@ -242,14 +245,14 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Labels: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy: externalSecrets,
 					},
 				},
 				Err: nil,
 			},
-			expectedSecret: "external-secrets",
+			expectedSecret: externalSecrets,
 		},
 		{
 			name: "annotations is specified",
@@ -260,7 +263,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Annotations: map[string]string{
 						"annotationKey1": "annotationValue1",
 						"annotationKey2": "annotationValue2",
@@ -283,7 +286,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Annotations: map[string]string{
 						"annotationKey1": "annotationValue1",
 						"annotationKey2": "annotationValue2",
@@ -305,7 +308,7 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Labels: map[string]string{
 						"label-key": "label-value",
 					},
@@ -326,9 +329,9 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Annotations: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy: externalSecrets,
 					},
 				},
 				Err: nil,
@@ -344,9 +347,9 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Labels: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy: externalSecrets,
 					},
 				},
 				Err: nil,
@@ -362,9 +365,9 @@ func TestGetSecret_MetadataPolicyFetch(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 					Labels: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy: externalSecrets,
 					},
 				},
 				Err: nil,
@@ -434,9 +437,9 @@ func TestDeleteSecret(t *testing.T) {
 				getSecretOutput: fakesm.SecretMockReturn{
 					Secret: &secretmanagerpb.Secret{
 
-						Name: "projects/foo/secret/bar",
+						Name: testSecretName,
 						Labels: map[string]string{
-							"managed-by": "external-secrets",
+							managedBy: externalSecrets,
 						},
 					},
 					Err: nil,
@@ -449,7 +452,7 @@ func TestDeleteSecret(t *testing.T) {
 				getSecretOutput: fakesm.SecretMockReturn{
 					Secret: &secretmanagerpb.Secret{
 
-						Name:   "projects/foo/secret/bar",
+						Name:   testSecretName,
 						Labels: map[string]string{},
 					},
 					Err: nil,
@@ -537,7 +540,7 @@ func TestPushSecret(t *testing.T) {
 			},
 		},
 		Labels: map[string]string{
-			"managed-by": "external-secrets",
+			managedBy: externalSecrets,
 		},
 	}
 	secretWithTopics := secretmanagerpb.Secret{
@@ -548,7 +551,7 @@ func TestPushSecret(t *testing.T) {
 			},
 		},
 		Labels: map[string]string{
-			"managed-by": "external-secrets",
+			managedBy: externalSecrets,
 		},
 		Topics: []*secretmanagerpb.Topic{
 			{
@@ -567,7 +570,7 @@ func TestPushSecret(t *testing.T) {
 			},
 		},
 		Labels: map[string]string{
-			"managed-by": "not-external-secrets",
+			managedBy: "not-external-secrets",
 		},
 	}
 
@@ -660,7 +663,7 @@ func TestPushSecret(t *testing.T) {
 						},
 					},
 					Labels: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy:    externalSecrets,
 						"label-key1": "label-value1",
 					},
 					Annotations: map[string]string{
@@ -693,7 +696,7 @@ func TestPushSecret(t *testing.T) {
 						},
 					},
 					Labels: map[string]string{
-						"managed-by": "external-secrets",
+						managedBy:    externalSecrets,
 						"label-key1": "label-value1",
 					},
 					Annotations: map[string]string{
@@ -950,7 +953,7 @@ func TestSecretExists(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 				},
 				Err: nil,
 			},
@@ -979,7 +982,7 @@ func TestSecretExists(t *testing.T) {
 			},
 			getSecretMockReturn: fakesm.SecretMockReturn{
 				Secret: &secretmanagerpb.Secret{
-					Name: "projects/foo/secret/bar",
+					Name: testSecretName,
 				},
 				Err: errors.New("some error"),
 			},
@@ -1011,7 +1014,7 @@ func TestSecretExists(t *testing.T) {
 	}
 }
 
-func TestPushSecret_Property(t *testing.T) {
+func TestPushSecretProperty(t *testing.T) {
 	secretKey := "secret-key"
 	defaultAddSecretVersionMockReturn := func(gotPayload, expectedPayload string) (*secretmanagerpb.SecretVersion, error) {
 		if gotPayload != expectedPayload {

+ 76 - 47
pkg/provider/gitlab/gitlab.go

@@ -38,19 +38,16 @@ import (
 )
 
 const (
-	errGitlabCredSecretName                   = "credentials are empty"
-	errInvalidClusterStoreMissingSAKNamespace = "invalid clusterStore missing SAK namespace"
-	errFetchSAKSecret                         = "couldn't find secret on cluster: %w"
-	errList                                   = "could not verify whether the gitlabClient is valid: %w"
-	errProjectAuth                            = "gitlabClient is not allowed to get secrets for project id [%s]"
-	errGroupAuth                              = "gitlabClient is not allowed to get secrets for group id [%s]"
-	errUninitializedGitlabProvider            = "provider gitlab is not initialized"
-	errNameNotDefined                         = "'find.name' is mandatory"
-	errEnvironmentIsConstricted               = "'find.tags' is constrained by 'environment_scope' of the store"
-	errTagsOnlyEnvironmentSupported           = "'find.tags' only supports 'environment_scope'"
-	errPathNotImplemented                     = "'find.path' is not implemented in the GitLab provider"
-	errJSONSecretUnmarshal                    = "unable to unmarshal secret: %w"
-	errNotImplemented                         = "not implemented"
+	errList                         = "could not verify whether the gitlabClient is valid: %w"
+	errProjectAuth                  = "gitlabClient is not allowed to get secrets for project id [%s]"
+	errGroupAuth                    = "gitlabClient is not allowed to get secrets for group id [%s]"
+	errUninitializedGitlabProvider  = "provider gitlab is not initialized"
+	errNameNotDefined               = "'find.name' is mandatory"
+	errEnvironmentIsConstricted     = "'find.tags' is constrained by 'environment_scope' of the store"
+	errTagsOnlyEnvironmentSupported = "'find.tags' only supports 'environment_scope'"
+	errPathNotImplemented           = "'find.path' is not implemented in the GitLab provider"
+	errJSONSecretUnmarshal          = "unable to unmarshal secret: %w"
+	errNotImplemented               = "not implemented"
 )
 
 // https://github.com/external-secrets/external-secrets/issues/644
@@ -138,36 +135,27 @@ func (g *gitlabBase) GetAllSecrets(_ context.Context, ref esv1beta1.ExternalSecr
 		return nil, err
 	}
 
-	var gopts = &gitlab.ListGroupVariablesOptions{PerPage: 100}
-	secretData := make(map[string][]byte)
-	for _, groupID := range g.store.GroupIDs {
-		for groupPage := 1; ; groupPage++ {
-			gopts.Page = groupPage
-			groupVars, response, err := g.groupVariablesClient.ListVariables(groupID, gopts)
-			metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabGroupListVariables, err)
-			if err != nil {
-				return nil, err
-			}
-			for _, data := range groupVars {
-				matching, key, isWildcard := matchesFilter(effectiveEnvironment, data.EnvironmentScope, data.Key, matcher)
-				if !matching && !isWildcard {
-					continue
-				}
-				secretData[key] = []byte(data.Value)
-			}
-			if response.CurrentPage >= response.TotalPages {
-				break
-			}
-		}
+	secretData, err := g.fetchSecretData(effectiveEnvironment, matcher)
+	if err != nil {
+		return nil, err
+	}
+
+	// _Note_: fetchProjectVariables alters secret data map
+	if err := g.fetchProjectVariables(effectiveEnvironment, matcher, secretData); err != nil {
+		return nil, err
 	}
 
+	return secretData, nil
+}
+
+func (g *gitlabBase) fetchProjectVariables(effectiveEnvironment string, matcher *find.Matcher, secretData map[string][]byte) error {
 	var popts = &gitlab.ListProjectVariablesOptions{PerPage: 100}
 	for projectPage := 1; ; projectPage++ {
 		popts.Page = projectPage
 		projectData, response, err := g.projectVariablesClient.ListVariables(g.store.ProjectID, popts)
 		metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabProjectListVariables, err)
 		if err != nil {
-			return nil, err
+			return err
 		}
 
 		for _, data := range projectData {
@@ -187,9 +175,59 @@ func (g *gitlabBase) GetAllSecrets(_ context.Context, ref esv1beta1.ExternalSecr
 		}
 	}
 
+	return nil
+}
+
+func (g *gitlabBase) fetchSecretData(effectiveEnvironment string, matcher *find.Matcher) (map[string][]byte, error) {
+	var gopts = &gitlab.ListGroupVariablesOptions{PerPage: 100}
+	secretData := make(map[string][]byte)
+	for _, groupID := range g.store.GroupIDs {
+		if err := g.setVariablesForGroupID(effectiveEnvironment, matcher, gopts, groupID, secretData); err != nil {
+			return nil, err
+		}
+	}
+
 	return secretData, nil
 }
 
+func (g *gitlabBase) setVariablesForGroupID(
+	effectiveEnvironment string,
+	matcher *find.Matcher,
+	gopts *gitlab.ListGroupVariablesOptions,
+	groupID string,
+	secretData map[string][]byte,
+) error {
+	for groupPage := 1; ; groupPage++ {
+		gopts.Page = groupPage
+		groupVars, response, err := g.groupVariablesClient.ListVariables(groupID, gopts)
+		metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabGroupListVariables, err)
+		if err != nil {
+			return err
+		}
+		g.setGroupValues(effectiveEnvironment, matcher, groupVars, secretData)
+
+		if response.CurrentPage >= response.TotalPages {
+			break
+		}
+	}
+	return nil
+}
+
+func (g *gitlabBase) setGroupValues(
+	effectiveEnvironment string,
+	matcher *find.Matcher,
+	groupVars []*gitlab.GroupVariable,
+	secretData map[string][]byte,
+) {
+	for _, data := range groupVars {
+		matching, key, isWildcard := matchesFilter(effectiveEnvironment, data.EnvironmentScope, data.Key, matcher)
+		if !matching && !isWildcard {
+			continue
+		}
+		secretData[key] = []byte(data.Value)
+	}
+}
+
 func ExtractTag(tags map[string]string) (string, error) {
 	var environmentScope string
 	for tag, value := range tags {
@@ -222,19 +260,10 @@ func (g *gitlabBase) GetSecret(_ context.Context, ref esv1beta1.ExternalSecretDa
 		vopts = &gitlab.GetProjectVariableOptions{Filter: &gitlab.VariableFilter{EnvironmentScope: g.store.Environment}}
 	}
 
-	data, resp, err := g.projectVariablesClient.GetVariable(g.store.ProjectID, ref.Key, vopts)
-	metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabProjectVariableGet, err)
+	// _Note_: getVariables potentially alters vopts environment variable.
+	data, resp, err := g.getVariables(ref, vopts)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound && !isEmptyOrWildcard(g.store.Environment) {
-			vopts.Filter.EnvironmentScope = "*"
-			data, resp, err = g.projectVariablesClient.GetVariable(g.store.ProjectID, ref.Key, vopts)
-			metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabProjectVariableGet, err)
-			if err != nil || resp == nil {
-				return nil, fmt.Errorf("error getting variable %s from GitLab: %w", ref.Key, err)
-			}
-		} else {
-			return nil, err
-		}
+		return nil, err
 	}
 
 	err = g.ResolveGroupIds()

+ 23 - 0
pkg/provider/gitlab/provider.go

@@ -17,12 +17,16 @@ package gitlab
 import (
 	"context"
 	"errors"
+	"fmt"
+	"net/http"
 
 	gitlab "gitlab.com/gitlab-org/api/client-go"
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
+	"github.com/external-secrets/external-secrets/pkg/constants"
+	"github.com/external-secrets/external-secrets/pkg/metrics"
 	"github.com/external-secrets/external-secrets/pkg/utils"
 )
 
@@ -96,6 +100,25 @@ func (g *gitlabBase) getClient(ctx context.Context, provider *esv1beta1.GitlabPr
 	return client, nil
 }
 
+func (g *gitlabBase) getVariables(ref esv1beta1.ExternalSecretDataRemoteRef, vopts *gitlab.GetProjectVariableOptions) (*gitlab.ProjectVariable, *gitlab.Response, error) {
+	data, resp, err := g.projectVariablesClient.GetVariable(g.store.ProjectID, ref.Key, vopts)
+	metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabProjectVariableGet, err)
+	if err != nil {
+		if resp != nil && resp.StatusCode == http.StatusNotFound && !isEmptyOrWildcard(g.store.Environment) {
+			vopts.Filter.EnvironmentScope = "*"
+			data, resp, err = g.projectVariablesClient.GetVariable(g.store.ProjectID, ref.Key, vopts)
+			metrics.ObserveAPICall(constants.ProviderGitLab, constants.CallGitLabProjectVariableGet, err)
+			if err != nil || resp == nil {
+				return nil, nil, fmt.Errorf("error getting variable %s from GitLab: %w", ref.Key, err)
+			}
+		} else {
+			return nil, nil, err
+		}
+	}
+
+	return data, resp, nil
+}
+
 func (g *Provider) ValidateStore(store esv1beta1.GenericStore) (admission.Warnings, error) {
 	storeSpec := store.GetSpec()
 	gitlabSpec := storeSpec.Provider.Gitlab

+ 20 - 25
pkg/provider/ibm/provider.go

@@ -39,10 +39,6 @@ import (
 )
 
 const (
-	SecretsManagerEndpointEnv = "IBM_SECRETSMANAGER_ENDPOINT"
-	STSEndpointEnv            = "IBM_STS_ENDPOINT"
-	SSMEndpointEnv            = "IBM_SSM_ENDPOINT"
-
 	certificateConst  = "certificate"
 	intermediateConst = "intermediate"
 	privateKeyConst   = "private_key"
@@ -54,14 +50,13 @@ const (
 	payloadConst      = "payload"
 	smAPIKeyConst     = "api_key"
 
-	errIBMClient               = "cannot setup new ibm client: %w"
-	errIBMCredSecretName       = "invalid IBM SecretStore resource: missing IBM APIKey"
-	errUninitalizedIBMProvider = "provider IBM is not initialized"
-	errFetchSAKSecret          = "could not fetch SecretAccessKey secret: %w"
-	errJSONSecretUnmarshal     = "unable to unmarshal secret: %w"
-	errJSONSecretMarshal       = "unable to marshal secret: %w"
-	errExtractingSecret        = "unable to extract the fetched secret %s of type %s while performing %s"
-	errNotImplemented          = "not implemented"
+	errIBMClient                = "cannot setup new ibm client: %w"
+	errUninitializedIBMProvider = "provider IBM is not initialized"
+	errJSONSecretUnmarshal      = "unable to unmarshal secret: %w"
+	errJSONSecretMarshal        = "unable to marshal secret: %w"
+	errExtractingSecret         = "unable to extract the fetched secret %s of type %s while performing %s"
+	errNotImplemented           = "not implemented"
+	errKeyDoesNotExist          = "key %s does not exist in secret %s"
 )
 
 var contextTimeout = time.Minute * 2
@@ -106,12 +101,12 @@ func (ibm *providerIBM) SecretExists(_ context.Context, _ esv1beta1.PushSecretRe
 	return false, errors.New(errNotImplemented)
 }
 
-// Not Implemented PushSecret.
+// PushSecret not implemented.
 func (ibm *providerIBM) PushSecret(_ context.Context, _ *corev1.Secret, _ esv1beta1.PushSecretData) error {
 	return errors.New(errNotImplemented)
 }
 
-// Empty GetAllSecrets.
+// GetAllSecrets empty.
 func (ibm *providerIBM) GetAllSecrets(_ context.Context, _ esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	// TO be implemented
 	return nil, errors.New(errNotImplemented)
@@ -119,7 +114,7 @@ func (ibm *providerIBM) GetAllSecrets(_ context.Context, _ esv1beta1.ExternalSec
 
 func (ibm *providerIBM) GetSecret(_ context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	if utils.IsNil(ibm.IBMClient) {
-		return nil, errors.New(errUninitalizedIBMProvider)
+		return nil, errors.New(errUninitializedIBMProvider)
 	}
 
 	var secretGroupName string
@@ -209,7 +204,7 @@ func getArbitrarySecret(ibm *providerIBM, secretName *string, secretGroupName st
 	if val, ok := secMap[payloadConst]; ok {
 		return []byte(val.(string)), nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", payloadConst, *secretName)
+	return nil, fmt.Errorf(errKeyDoesNotExist, payloadConst, *secretName)
 }
 
 func getImportCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.ExternalSecretDataRemoteRef, secretGroupName string) ([]byte, error) {
@@ -230,7 +225,7 @@ func getImportCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.Ext
 		fmt.Printf("warn: %s is empty for secret %s\n", privateKeyConst, *secretName)
 		return []byte(""), nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+	return nil, fmt.Errorf(errKeyDoesNotExist, ref.Property, ref.Key)
 }
 
 func getPublicCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.ExternalSecretDataRemoteRef, secretGroupName string) ([]byte, error) {
@@ -245,7 +240,7 @@ func getPublicCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.Ext
 	if val, ok := secMap[ref.Property]; ok {
 		return []byte(val.(string)), nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+	return nil, fmt.Errorf(errKeyDoesNotExist, ref.Property, ref.Key)
 }
 
 func getPrivateCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.ExternalSecretDataRemoteRef, secretGroupName string) ([]byte, error) {
@@ -260,7 +255,7 @@ func getPrivateCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.Ex
 	if val, ok := secMap[ref.Property]; ok {
 		return []byte(val.(string)), nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+	return nil, fmt.Errorf(errKeyDoesNotExist, ref.Property, ref.Key)
 }
 
 func getIamCredentialsSecret(ibm *providerIBM, secretName *string, secretGroupName string) ([]byte, error) {
@@ -275,7 +270,7 @@ func getIamCredentialsSecret(ibm *providerIBM, secretName *string, secretGroupNa
 	if val, ok := secMap[smAPIKeyConst]; ok {
 		return []byte(val.(string)), nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", smAPIKeyConst, *secretName)
+	return nil, fmt.Errorf(errKeyDoesNotExist, smAPIKeyConst, *secretName)
 }
 
 func getServiceCredentialsSecret(ibm *providerIBM, secretName *string, secretGroupName string) ([]byte, error) {
@@ -294,7 +289,7 @@ func getServiceCredentialsSecret(ibm *providerIBM, secretName *string, secretGro
 		}
 		return mval, nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", credentialsConst, *secretName)
+	return nil, fmt.Errorf(errKeyDoesNotExist, credentialsConst, *secretName)
 }
 
 func getUsernamePasswordSecret(ibm *providerIBM, secretName *string, ref esv1beta1.ExternalSecretDataRemoteRef, secretGroupName string) ([]byte, error) {
@@ -309,7 +304,7 @@ func getUsernamePasswordSecret(ibm *providerIBM, secretName *string, ref esv1bet
 	if val, ok := secMap[ref.Property]; ok {
 		return []byte(val.(string)), nil
 	}
-	return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+	return nil, fmt.Errorf(errKeyDoesNotExist, ref.Property, ref.Key)
 }
 
 // Returns a secret of type kv and supports json path.
@@ -348,7 +343,7 @@ func getKVSecret(ref esv1beta1.ExternalSecretDataRemoteRef, secret *sm.KVSecret)
 		// try to get value for this path
 		val := gjson.Get(payloadJSON, ref.Property)
 		if !val.Exists() {
-			return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
+			return nil, fmt.Errorf(errKeyDoesNotExist, ref.Property, ref.Key)
 		}
 		return []byte(val.String()), nil
 	}
@@ -399,7 +394,7 @@ func getSecretData(ibm *providerIBM, secretName *string, secretType, secretGroup
 
 func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	if utils.IsNil(ibm.IBMClient) {
-		return nil, errors.New(errUninitalizedIBMProvider)
+		return nil, errors.New(errUninitializedIBMProvider)
 	}
 	var secretGroupName string
 	secretType := sm.Secret_SecretType_Arbitrary
@@ -434,7 +429,7 @@ func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1beta1.ExternalSe
 	checkNilFn := func(propertyList []string) error {
 		for _, prop := range propertyList {
 			if _, ok := secMap[prop]; !ok {
-				return fmt.Errorf("key %s does not exist in secret %s", prop, secretName)
+				return fmt.Errorf(errKeyDoesNotExist, prop, secretName)
 			}
 		}
 		return nil

+ 2 - 12
pkg/provider/ibm/provider_test.go

@@ -68,7 +68,7 @@ func makeValidSecretManagerTestCase() *secretManagerTestCase {
 		ref:             makeValidRef(),
 		apiOutput:       makeValidAPIOutput(),
 		getByNameInput:  makeValidGetByNameInput(),
-		getByNameOutput: makeValidGetByNameOutput(),
+		getByNameOutput: makeValidAPIOutput(),
 		getByNameError:  nil,
 		serviceURL:      nil,
 		apiErr:          nil,
@@ -115,16 +115,6 @@ func makeValidGetByNameInput() *sm.GetSecretByNameTypeOptions {
 	return &sm.GetSecretByNameTypeOptions{}
 }
 
-func makeValidGetByNameOutput() sm.SecretIntf {
-	secret := &sm.Secret{
-		SecretType: utilpointer.To(sm.Secret_SecretType_Arbitrary),
-		Name:       utilpointer.To("testyname"),
-		ID:         utilpointer.To(secretUUID),
-	}
-	var i sm.SecretIntf = secret
-	return i
-}
-
 func makeValidSecretManagerTestCaseCustom(tweaks ...func(smtc *secretManagerTestCase)) *secretManagerTestCase {
 	smtc := makeValidSecretManagerTestCase()
 	for _, fn := range tweaks {
@@ -151,7 +141,7 @@ var setAPIErr = func(smtc *secretManagerTestCase) {
 
 var setNilMockClient = func(smtc *secretManagerTestCase) {
 	smtc.mockClient = nil
-	smtc.expectError = errUninitalizedIBMProvider
+	smtc.expectError = errUninitializedIBMProvider
 }
 
 // simple tests for Validate Store.

+ 11 - 10
pkg/provider/kubernetes/auth_test.go

@@ -67,6 +67,7 @@ users:
   user:
     token: eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJPbmxpbmUgSldUIEJ1aWxkZXIiLCJpYXQiOjE3MTkzOTY4OTksImV4cCI6MTc1MDkzMjg4NywiYXVkIjoid3d3LmV4YW1wbGUuY29tIiwic3ViIjoianJvY2tldEBleGFtcGxlLmNvbSIsIkdpdmVuTmFtZSI6IkpvaG5ueSIsIlN1cm5hbWUiOiJSb2NrZXQiLCJFbWFpbCI6Impyb2NrZXRAZXhhbXBsZS5jb20iLCJSb2xlIjpbIk1hbmFnZXIiLCJQcm9qZWN0IEFkbWluaXN0cmF0b3IiXX0.xXrfIl0akhfjWU_BDl7Ad54SXje0YlJdnugzwh96VmM
 `
+	serverURL = "https://my.test.tld"
 )
 
 func TestSetAuth(t *testing.T) {
@@ -122,7 +123,7 @@ func TestSetAuth(t *testing.T) {
 				}).Build(),
 				store: &esv1beta1.KubernetesProvider{
 					Server: esv1beta1.KubernetesServer{
-						URL: "https://my.test.tld",
+						URL: serverURL,
 						CAProvider: &esv1beta1.CAProvider{
 							Type: esv1beta1.CAProviderTypeSecret,
 							Name: "foobar",
@@ -141,7 +142,7 @@ func TestSetAuth(t *testing.T) {
 				},
 			},
 			want: &want{
-				Host:        "https://my.test.tld",
+				Host:        serverURL,
 				BearerToken: "mytoken",
 				TLSClientConfig: rest.TLSClientConfig{
 					CAData: []byte(caCert),
@@ -172,7 +173,7 @@ func TestSetAuth(t *testing.T) {
 				}).Build(),
 				store: &esv1beta1.KubernetesProvider{
 					Server: esv1beta1.KubernetesServer{
-						URL: "https://my.test.tld",
+						URL: serverURL,
 						CAProvider: &esv1beta1.CAProvider{
 							Type: esv1beta1.CAProviderTypeConfigMap,
 							Name: "foobar",
@@ -191,7 +192,7 @@ func TestSetAuth(t *testing.T) {
 				},
 			},
 			want: &want{
-				Host:        "https://my.test.tld",
+				Host:        serverURL,
 				BearerToken: "mytoken",
 				TLSClientConfig: rest.TLSClientConfig{
 					CAData: []byte("1234"),
@@ -214,7 +215,7 @@ func TestSetAuth(t *testing.T) {
 				}).Build(),
 				store: &esv1beta1.KubernetesProvider{
 					Server: esv1beta1.KubernetesServer{
-						URL:      "https://my.test.tld",
+						URL:      serverURL,
 						CABundle: []byte(caCert),
 					},
 					Auth: esv1beta1.KubernetesAuth{
@@ -229,7 +230,7 @@ func TestSetAuth(t *testing.T) {
 				},
 			},
 			want: &want{
-				Host:        "https://my.test.tld",
+				Host:        serverURL,
 				BearerToken: "mytoken",
 				TLSClientConfig: rest.TLSClientConfig{
 					CAData: []byte(caCert),
@@ -253,7 +254,7 @@ func TestSetAuth(t *testing.T) {
 				}).Build(),
 				store: &esv1beta1.KubernetesProvider{
 					Server: esv1beta1.KubernetesServer{
-						URL:      "https://my.test.tld",
+						URL:      serverURL,
 						CABundle: []byte(caCert),
 					},
 					Auth: esv1beta1.KubernetesAuth{
@@ -271,7 +272,7 @@ func TestSetAuth(t *testing.T) {
 				},
 			},
 			want: &want{
-				Host: "https://my.test.tld",
+				Host: serverURL,
 				TLSClientConfig: rest.TLSClientConfig{
 					CAData:   []byte(caCert),
 					CertData: []byte("my-cert"),
@@ -293,7 +294,7 @@ func TestSetAuth(t *testing.T) {
 				kubeclientset: utilfake.NewCreateTokenMock().WithToken("my-sa-token"),
 				store: &esv1beta1.KubernetesProvider{
 					Server: esv1beta1.KubernetesServer{
-						URL:      "https://my.test.tld",
+						URL:      serverURL,
 						CABundle: []byte(caCert),
 					},
 					Auth: esv1beta1.KubernetesAuth{
@@ -305,7 +306,7 @@ func TestSetAuth(t *testing.T) {
 				},
 			},
 			want: &want{
-				Host:        "https://my.test.tld",
+				Host:        serverURL,
 				BearerToken: "my-sa-token",
 				TLSClientConfig: rest.TLSClientConfig{
 					CAData: []byte(caCert),

+ 74 - 43
pkg/provider/oracle/oracle.go

@@ -58,6 +58,7 @@ const (
 	errJSONSecretUnmarshal        = "unable to unmarshal secret: %w"
 	errMissingKey                 = "missing Key in secret: %s"
 	errUnexpectedContent          = "unexpected secret bundle content"
+	errSettingOCIEnvVariables     = "unable to set OCI SDK environment variable %s: %w"
 )
 
 // https://github.com/external-secrets/external-secrets/issues/644
@@ -273,20 +274,9 @@ func (vms *VaultManagementService) NewClient(ctx context.Context, store esv1beta
 		return nil, errors.New(errMissingRegion)
 	}
 
-	var (
-		err                   error
-		configurationProvider common.ConfigurationProvider
-	)
-
-	if oracleSpec.PrincipalType == esv1beta1.WorkloadPrincipal {
-		configurationProvider, err = vms.getWorkloadIdentityProvider(store, oracleSpec.ServiceAccountRef, oracleSpec.Region, namespace)
-	} else if oracleSpec.PrincipalType == esv1beta1.InstancePrincipal || oracleSpec.Auth == nil {
-		configurationProvider, err = auth.InstancePrincipalConfigurationProvider()
-	} else {
-		configurationProvider, err = getUserAuthConfigurationProvider(ctx, kube, oracleSpec, namespace, store.GetObjectKind().GroupVersionKind().Kind, oracleSpec.Region)
-	}
+	configurationProvider, err := vms.constructProvider(ctx, store, oracleSpec, kube, namespace)
 	if err != nil {
-		return nil, fmt.Errorf(errOracleClient, err)
+		return nil, err
 	}
 
 	secretManagementService, err := secrets.NewSecretsClientWithConfigurationProvider(configurationProvider)
@@ -308,34 +298,9 @@ func (vms *VaultManagementService) NewClient(ctx context.Context, store esv1beta
 	vaultClient.SetRegion(oracleSpec.Region)
 
 	if storeSpec.RetrySettings != nil {
-		opts := []common.RetryPolicyOption{common.WithShouldRetryOperation(common.DefaultShouldRetryOperation)}
-
-		if mr := storeSpec.RetrySettings.MaxRetries; mr != nil {
-			attempts := safeConvert(*mr)
-			opts = append(opts, common.WithMaximumNumberAttempts(attempts))
-		}
-
-		if ri := storeSpec.RetrySettings.RetryInterval; ri != nil {
-			i, err := time.ParseDuration(*storeSpec.RetrySettings.RetryInterval)
-			if err != nil {
-				return nil, fmt.Errorf(errOracleClient, err)
-			}
-			opts = append(opts, common.WithFixedBackoff(i))
+		if err := vms.configureRetryPolicy(storeSpec, secretManagementService, kmsVaultClient, vaultClient); err != nil {
+			return nil, fmt.Errorf(errOracleClient, err)
 		}
-
-		customRetryPolicy := common.NewRetryPolicyWithOptions(opts...)
-
-		secretManagementService.SetCustomClientConfiguration(common.CustomClientConfiguration{
-			RetryPolicy: &customRetryPolicy,
-		})
-
-		kmsVaultClient.SetCustomClientConfiguration(common.CustomClientConfiguration{
-			RetryPolicy: &customRetryPolicy,
-		})
-
-		vaultClient.SetCustomClientConfiguration(common.CustomClientConfiguration{
-			RetryPolicy: &customRetryPolicy,
-		})
 	}
 
 	return &VaultManagementService{
@@ -348,6 +313,24 @@ func (vms *VaultManagementService) NewClient(ctx context.Context, store esv1beta
 	}, nil
 }
 
+func (vms *VaultManagementService) constructOptions(storeSpec *esv1beta1.SecretStoreSpec) ([]common.RetryPolicyOption, error) {
+	opts := []common.RetryPolicyOption{common.WithShouldRetryOperation(common.DefaultShouldRetryOperation)}
+
+	if mr := storeSpec.RetrySettings.MaxRetries; mr != nil {
+		attempts := safeConvert(*mr)
+		opts = append(opts, common.WithMaximumNumberAttempts(attempts))
+	}
+
+	if ri := storeSpec.RetrySettings.RetryInterval; ri != nil {
+		i, err := time.ParseDuration(*storeSpec.RetrySettings.RetryInterval)
+		if err != nil {
+			return nil, fmt.Errorf(errOracleClient, err)
+		}
+		opts = append(opts, common.WithFixedBackoff(i))
+	}
+	return opts, nil
+}
+
 func safeConvert(i int32) uint {
 	if i < 0 {
 		return 0
@@ -573,7 +556,7 @@ func (vms *VaultManagementService) ValidateStore(store esv1beta1.GenericStore) (
 func (vms *VaultManagementService) getWorkloadIdentityProvider(store esv1beta1.GenericStore, serviceAcccountRef *esmeta.ServiceAccountSelector, region, namespace string) (configurationProvider common.ConfigurationProvider, err error) {
 	defer func() {
 		if uerr := os.Unsetenv(auth.ResourcePrincipalVersionEnvVar); uerr != nil {
-			err = errors.Join(err, fmt.Errorf("unable to set OCI SDK environment variable %s: %w", auth.ResourcePrincipalRegionEnvVar, uerr))
+			err = errors.Join(err, fmt.Errorf(errSettingOCIEnvVariables, auth.ResourcePrincipalRegionEnvVar, uerr))
 		}
 		if uerr := os.Unsetenv(auth.ResourcePrincipalRegionEnvVar); uerr != nil {
 			err = errors.Join(err, fmt.Errorf("unabled to unset OCI SDK environment variable %s: %w", auth.ResourcePrincipalVersionEnvVar, uerr))
@@ -583,10 +566,10 @@ func (vms *VaultManagementService) getWorkloadIdentityProvider(store esv1beta1.G
 	vms.workloadIdentityMutex.Lock()
 	// OCI SDK requires specific environment variables for workload identity.
 	if err := os.Setenv(auth.ResourcePrincipalVersionEnvVar, auth.ResourcePrincipalVersion2_2); err != nil {
-		return nil, fmt.Errorf("unable to set OCI SDK environment variable %s: %w", auth.ResourcePrincipalVersionEnvVar, err)
+		return nil, fmt.Errorf(errSettingOCIEnvVariables, auth.ResourcePrincipalVersionEnvVar, err)
 	}
 	if err := os.Setenv(auth.ResourcePrincipalRegionEnvVar, region); err != nil {
-		return nil, fmt.Errorf("unable to set OCI SDK environment variable %s: %w", auth.ResourcePrincipalRegionEnvVar, err)
+		return nil, fmt.Errorf(errSettingOCIEnvVariables, auth.ResourcePrincipalRegionEnvVar, err)
 	}
 	// If no service account is specified, use the pod service account to create the Workload Identity provider.
 	if serviceAcccountRef == nil {
@@ -608,6 +591,54 @@ func (vms *VaultManagementService) getWorkloadIdentityProvider(store esv1beta1.G
 	return auth.OkeWorkloadIdentityConfigurationProviderWithServiceAccountTokenProvider(tokenProvider)
 }
 
+func (vms *VaultManagementService) constructProvider(ctx context.Context, store esv1beta1.GenericStore, oracleSpec *esv1beta1.OracleProvider, kube kclient.Client, namespace string) (common.ConfigurationProvider, error) {
+	var (
+		configurationProvider common.ConfigurationProvider
+		err                   error
+	)
+
+	if oracleSpec.PrincipalType == esv1beta1.WorkloadPrincipal {
+		configurationProvider, err = vms.getWorkloadIdentityProvider(store, oracleSpec.ServiceAccountRef, oracleSpec.Region, namespace)
+	} else if oracleSpec.PrincipalType == esv1beta1.InstancePrincipal || oracleSpec.Auth == nil {
+		configurationProvider, err = auth.InstancePrincipalConfigurationProvider()
+	} else {
+		configurationProvider, err = getUserAuthConfigurationProvider(ctx, kube, oracleSpec, namespace, store.GetObjectKind().GroupVersionKind().Kind, oracleSpec.Region)
+	}
+	if err != nil {
+		return nil, fmt.Errorf(errOracleClient, err)
+	}
+
+	return configurationProvider, nil
+}
+
+func (vms *VaultManagementService) configureRetryPolicy(
+	storeSpec *esv1beta1.SecretStoreSpec,
+	secretManagementService secrets.SecretsClient,
+	kmsVaultClient keymanagement.KmsVaultClient,
+	vaultClient vault.VaultsClient,
+) error {
+	opts, err := vms.constructOptions(storeSpec)
+	if err != nil {
+		return err
+	}
+
+	customRetryPolicy := common.NewRetryPolicyWithOptions(opts...)
+
+	secretManagementService.SetCustomClientConfiguration(common.CustomClientConfiguration{
+		RetryPolicy: &customRetryPolicy,
+	})
+
+	kmsVaultClient.SetCustomClientConfiguration(common.CustomClientConfiguration{
+		RetryPolicy: &customRetryPolicy,
+	})
+
+	vaultClient.SetCustomClientConfiguration(common.CustomClientConfiguration{
+		RetryPolicy: &customRetryPolicy,
+	})
+
+	return err
+}
+
 func sanitizeOCISDKErr(err error) error {
 	if err == nil {
 		return nil

+ 1 - 1
pkg/provider/oracle/oracle_test.go

@@ -321,7 +321,7 @@ func TestValidateStore(t *testing.T) {
 	}
 }
 
-func TestVaultManagementService_NewClient(t *testing.T) {
+func TestVaultManagementServiceNewClient(t *testing.T) {
 	t.Parallel()
 
 	namespace := "default"

+ 18 - 11
pkg/provider/passbolt/passbolt_test.go

@@ -28,6 +28,13 @@ import (
 	esmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
 )
 
+const (
+	someKey1 = "some-key1"
+	someKey2 = "some-key2"
+	someURI1 = "some-uri1"
+	someURI2 = "some-uri2"
+)
+
 type PassboltClientMock struct {
 }
 
@@ -42,8 +49,8 @@ func (p *PassboltClientMock) Logout(_ context.Context) error {
 }
 func (p *PassboltClientMock) GetResource(_ context.Context, resourceID string) (*api.Resource, error) {
 	resmap := map[string]api.Resource{
-		"some-key1": {ID: "some-key1", Name: "some-name1", URI: "some-uri1"},
-		"some-key2": {ID: "some-key2", Name: "some-name2", URI: "some-uri2"},
+		someKey1: {ID: someKey1, Name: "some-name1", URI: someURI1},
+		someKey2: {ID: someKey2, Name: "some-name2", URI: someURI2},
 	}
 
 	if res, ok := resmap[resourceID]; ok {
@@ -55,8 +62,8 @@ func (p *PassboltClientMock) GetResource(_ context.Context, resourceID string) (
 
 func (p *PassboltClientMock) GetResources(_ context.Context, _ *api.GetResourcesOptions) ([]api.Resource, error) {
 	res := []api.Resource{
-		{ID: "some-key1", Name: "some-name1", URI: "some-uri1"},
-		{ID: "some-key2", Name: "some-name2", URI: "some-uri2"},
+		{ID: someKey1, Name: "some-name1", URI: someURI1},
+		{ID: someKey2, Name: "some-name2", URI: someURI2},
 	}
 	return res, nil
 }
@@ -72,8 +79,8 @@ func (p *PassboltClientMock) DecryptMessage(message string) (string, error) {
 
 func (p *PassboltClientMock) GetSecret(_ context.Context, resourceID string) (*api.Secret, error) {
 	resmap := map[string]api.Secret{
-		"some-key1": {Data: `{"password": "some-password1", "description": "some-description1"}`},
-		"some-key2": {Data: `{"password": "some-password2", "description": "some-description2"}`},
+		someKey1: {Data: `{"password": "some-password1", "description": "some-description1"}`},
+		someKey2: {Data: `{"password": "some-password2", "description": "some-description2"}`},
 	}
 
 	if res, ok := resmap[resourceID]; ok {
@@ -167,8 +174,8 @@ func TestGetAllSecrets(t *testing.T) {
 				},
 			},
 			expected: map[string][]byte{
-				"some-key1": []byte(`{"name":"some-name1","username":"","password":"some-password1","uri":"some-uri1","description":"some-description1"}`),
-				"some-key2": []byte(`{"name":"some-name2","username":"","password":"some-password2","uri":"some-uri2","description":"some-description2"}`),
+				someKey1: []byte(`{"name":"some-name1","username":"","password":"some-password1","uri":"some-uri1","description":"some-description1"}`),
+				someKey2: []byte(`{"name":"some-name2","username":"","password":"some-password2","uri":"some-uri2","description":"some-description2"}`),
 			},
 		},
 		{
@@ -234,7 +241,7 @@ func TestGetSecret(t *testing.T) {
 		{
 			name: "get property from secret",
 			request: esv1beta1.ExternalSecretDataRemoteRef{
-				Key:      "some-key1",
+				Key:      someKey1,
 				Property: "password",
 			},
 			expValue: "some-password1",
@@ -242,14 +249,14 @@ func TestGetSecret(t *testing.T) {
 		{
 			name: "get full secret",
 			request: esv1beta1.ExternalSecretDataRemoteRef{
-				Key: "some-key1",
+				Key: someKey1,
 			},
 			expValue: `{"name":"some-name1","username":"","password":"some-password1","uri":"some-uri1","description":"some-description1"}`,
 		},
 		{
 			name: "return err when using invalid property",
 			request: esv1beta1.ExternalSecretDataRemoteRef{
-				Key:      "some-key1",
+				Key:      someKey1,
 				Property: "invalid",
 			},
 			expErr: errPassboltSecretPropertyInvalid,

+ 3 - 2
pkg/provider/vault/auth.go

@@ -222,6 +222,7 @@ func (c *client) useAuthNamespace(_ context.Context) func() {
 		}
 	}
 
-	// no-op
-	return func() {}
+	return func() {
+		// no-op
+	}
 }

+ 7 - 3
pkg/provider/vault/fake/vault.go

@@ -24,7 +24,7 @@ import (
 
 	vault "github.com/hashicorp/vault/api"
 
-	util "github.com/external-secrets/external-secrets/pkg/provider/vault/util"
+	"github.com/external-secrets/external-secrets/pkg/provider/vault/util"
 )
 
 type LoginFn func(ctx context.Context, authMethod vault.AuthMethod) (*vault.Secret, error)
@@ -189,11 +189,15 @@ func NewTokenFn(v string) MockTokenFn {
 }
 
 func NewClearTokenFn() MockClearTokenFn {
-	return func() {}
+	return func() {
+		// no-op
+	}
 }
 
 func NewAddHeaderFn() MockAddHeaderFn {
-	return func(key, value string) {}
+	return func(key, value string) {
+		// no header
+	}
 }
 
 type VaultClient struct {

+ 1 - 3
tilt.debug.dockerfile

@@ -2,9 +2,7 @@ FROM golang:1.23.4@sha256:70031844b8c225351d0bb63e2c383f80db85d92ba894e3da7e13bc
 WORKDIR /
 COPY ./bin/external-secrets /external-secrets
 
-RUN go install github.com/go-delve/delve/cmd/dlv@v1.22.0
-RUN chmod +x /go/bin/dlv
-RUN mv /go/bin/dlv /
+RUN go install github.com/go-delve/delve/cmd/dlv@v1.22.0 && chmod +x /go/bin/dlv && mv /go/bin/dlv /
 
 EXPOSE 30000