feat: implement deletionPolicy (#900)

* feat: implement deletionPolicy

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>
Co-authored-by: Gustavo Fernandes de Carvalho <gustavo.carvalho@container-solutions.com>

apis/externalsecrets/v1alpha1/externalsecret_conversion.go

@@ -24,7 +24,7 @@ import (
 
 func (alpha *ExternalSecret) ConvertTo(betaRaw conversion.Hub) error {
 	beta := betaRaw.(*esv1beta1.ExternalSecret)
-	// Actual converted code thatn eeds to be like this
+	// Actual converted code that needs to be like this
 	v1beta1DataFrom := make([]esv1beta1.ExternalSecretDataFromRemoteRef, 0)
 	for _, v1alpha1RemoteRef := range alpha.Spec.DataFrom {
 		v1beta1RemoteRef := esv1beta1.ExternalSecretDataFromRemoteRef{

apis/externalsecrets/v1alpha1/externalsecret_conversion_test.go

@@ -134,7 +134,7 @@ func newExternalSecretV1Beta1() *esv1beta1.ExternalSecret {
 			},
 			Target: esv1beta1.ExternalSecretTarget{
 				Name:           "test-target",
-				CreationPolicy: esv1beta1.Owner,
+				CreationPolicy: esv1beta1.CreatePolicyOwner,
 				Immutable:      false,
 				Template: &esv1beta1.ExternalSecretTemplate{
 					Type: corev1.SecretTypeOpaque,

apis/externalsecrets/v1beta1/externalsecret_types.go

@@ -31,31 +31,45 @@ type SecretStoreRef struct {
 }
 
 // ExternalSecretCreationPolicy defines rules on how to create the resulting Secret.
+// +kubebuilder:validation:Enum=Owner;Orphan;Merge;None
 type ExternalSecretCreationPolicy string
 
 const (
 	// Owner creates the Secret and sets .metadata.ownerReferences to the ExternalSecret resource.
-	Owner ExternalSecretCreationPolicy = "Owner"
+	CreatePolicyOwner ExternalSecretCreationPolicy = "Owner"
+
+	// Orphan creates the Secret and does not set the ownerReference.
+	// I.e. it will be orphaned after the deletion of the ExternalSecret.
+	CreatePolicyOrphan ExternalSecretCreationPolicy = "Orphan"
 
 	// Merge does not create the Secret, but merges the data fields to the Secret.
-	Merge ExternalSecretCreationPolicy = "Merge"
+	CreatePolicyMerge ExternalSecretCreationPolicy = "Merge"
 
 	// None does not create a Secret (future use with injector).
-	None ExternalSecretCreationPolicy = "None"
+	CreatePolicyNone ExternalSecretCreationPolicy = "None"
 )
 
 // ExternalSecretDeletionPolicy defines rules on how to delete the resulting Secret.
+// +kubebuilder:validation:Enum=Delete;Merge;Retain
 type ExternalSecretDeletionPolicy string
 
 const (
-	// Owner creates the Secret and sets .metadata.ownerReferences to the ExternalSecret resource.
-	DeletionOwner ExternalSecretDeletionPolicy = "Owner"
-
-	// Merge does not create the Secret, but merges the data fields to the Secret.
-	DeletionMerge ExternalSecretDeletionPolicy = "Merge"
-
-	// None does not create a Secret (future use with injector).
-	DeletionNone ExternalSecretDeletionPolicy = "None"
+	// Delete deletes the secret if all provider secrets are deleted.
+	// If a secret gets deleted on the provider side and is not accessible
+	// anymore this is not considered an error and the ExternalSecret
+	// does not go into SecretSyncedError status.
+	DeletionPolicyDelete ExternalSecretDeletionPolicy = "Delete"
+
+	// Merge removes keys in the secret, but not the secret itself.
+	// If a secret gets deleted on the provider side and is not accessible
+	// anymore this is not considered an error and the ExternalSecret
+	// does not go into SecretSyncedError status.
+	DeletionPolicyMerge ExternalSecretDeletionPolicy = "Merge"
+
+	// Retain will retain the secret if all provider secrets have been deleted.
+	// If a provider secret does not exist the ExternalSecret gets into the
+	// SecretSyncedError status.
+	DeletionPolicyRetain ExternalSecretDeletionPolicy = "Retain"
 )
 
 // ExternalSecretTemplateMetadata defines metadata fields for the Secret blueprint.
@@ -127,9 +141,9 @@ type ExternalSecretTarget struct {
 	// +kubebuilder:default="Owner"
 	CreationPolicy ExternalSecretCreationPolicy `json:"creationPolicy,omitempty"`
 	// DeletionPolicy defines rules on how to delete the resulting Secret
-	// Defaults to 'None'
+	// Defaults to 'Retain'
 	// +optional
-	// +kubebuilder:default="None"
+	// +kubebuilder:default="Retain"
 	DeletionPolicy ExternalSecretDeletionPolicy `json:"deletionPolicy,omitempty"`
 	// Template defines a blueprint for the created Secret resource.
 	// +optional
@@ -262,6 +276,7 @@ const (
 	ReasonProviderClientConfig = "InvalidProviderClientConfig"
 	ReasonUpdateFailed         = "UpdateFailed"
 	ReasonUpdated              = "Updated"
+	ReasonDeleted              = "Deleted"
 )
 
 type ExternalSecretStatus struct {

apis/externalsecrets/v1beta1/externalsecret_validator.go

@@ -0,0 +1,52 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package v1beta1
+
+import (
+	"context"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+type ExternalSecretValidator struct{}
+
+func (esv *ExternalSecretValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
+	return validateExternalSecret(obj)
+}
+
+func (esv *ExternalSecretValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
+	return validateExternalSecret(newObj)
+}
+
+func (esv *ExternalSecretValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
+	return nil
+}
+
+func validateExternalSecret(obj runtime.Object) error {
+	es, ok := obj.(*ExternalSecret)
+	if !ok {
+		return fmt.Errorf("unexpected type")
+	}
+
+	if (es.Spec.Target.DeletionPolicy == DeletionPolicyDelete && es.Spec.Target.CreationPolicy == CreatePolicyMerge) ||
+		(es.Spec.Target.DeletionPolicy == DeletionPolicyDelete && es.Spec.Target.CreationPolicy == CreatePolicyNone) {
+		return fmt.Errorf("deletionPolicy=Delete must not be used when the controller doesn't own the secret. Please set creationPolcy=Owner")
+	}
+
+	if es.Spec.Target.DeletionPolicy == DeletionPolicyMerge && es.Spec.Target.CreationPolicy == CreatePolicyNone {
+		return fmt.Errorf("deletionPolicy=Merge must not be used with creationPolcy=None. There is no Secret to merge with")
+	}
+	return nil
+}

apis/externalsecrets/v1beta1/externalsecret_webhook.go

@@ -21,5 +21,6 @@ import (
 func (r *ExternalSecret) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
+		WithValidator(&ExternalSecretValidator{}).
 		Complete()
 }

apis/externalsecrets/v1beta1/provider.go

@@ -42,6 +42,8 @@ type Provider interface {
 // SecretsClient provides access to secrets.
 type SecretsClient interface {
 	// GetSecret returns a single secret from the provider
+	// if GetSecret returns an error with type NoSecretError
+	// then the secret entry will be deleted depending on the deletionPolicy.
 	GetSecret(ctx context.Context, ref ExternalSecretDataRemoteRef) ([]byte, error)
 
 	// Validate checks if the client is configured correctly
@@ -56,3 +58,13 @@ type SecretsClient interface {
 
 	Close(ctx context.Context) error
 }
+
+var NoSecretErr = NoSecretError{}
+
+// NoSecretError shall be returned when a GetSecret can not find the
+// desired secret. This is used for deletionPolicy.
+type NoSecretError struct{}
+
+func (NoSecretError) Error() string {
+	return "Secret does not exist"
+}

apis/externalsecrets/v1beta1/zz_generated.deepcopy.go

@@ -822,6 +822,21 @@ func (in *ExternalSecretTemplateMetadata) DeepCopy() *ExternalSecretTemplateMeta
 }
 
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *ExternalSecretValidator) DeepCopyInto(out *ExternalSecretValidator) {
+	*out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalSecretValidator.
+func (in *ExternalSecretValidator) DeepCopy() *ExternalSecretValidator {
+	if in == nil {
+		return nil
+	}
+	out := new(ExternalSecretValidator)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *FakeProvider) DeepCopyInto(out *FakeProvider) {
 	*out = *in
 	if in.Data != nil {
@@ -1142,6 +1157,21 @@ func (in *KubernetesServer) DeepCopy() *KubernetesServer {
 }
 
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *NoSecretError) DeepCopyInto(out *NoSecretError) {
+	*out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NoSecretError.
+func (in *NoSecretError) DeepCopy() *NoSecretError {
+	if in == nil {
+		return nil
+	}
+	out := new(NoSecretError)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *OracleAuth) DeepCopyInto(out *OracleAuth) {
 	*out = *in
 	in.SecretRef.DeepCopyInto(&out.SecretRef)

config/crds/bases/external-secrets.io_clusterexternalsecrets.yaml

@@ -168,11 +168,20 @@ spec:
                         default: Owner
                         description: CreationPolicy defines rules on how to create
                           the resulting Secret Defaults to 'Owner'
+                        enum:
+                        - Owner
+                        - Orphan
+                        - Merge
+                        - None
                         type: string
                       deletionPolicy:
-                        default: None
+                        default: Retain
                         description: DeletionPolicy defines rules on how to delete
-                          the resulting Secret Defaults to 'None'
+                          the resulting Secret Defaults to 'Retain'
+                        enum:
+                        - Delete
+                        - Merge
+                        - Retain
                         type: string
                       immutable:
                         description: Immutable defines if the final secret will be

config/crds/bases/external-secrets.io_externalsecrets.yaml

@@ -412,11 +412,20 @@ spec:
                     default: Owner
                     description: CreationPolicy defines rules on how to create the
                       resulting Secret Defaults to 'Owner'
+                    enum:
+                    - Owner
+                    - Orphan
+                    - Merge
+                    - None
                     type: string
                   deletionPolicy:
-                    default: None
+                    default: Retain
                     description: DeletionPolicy defines rules on how to delete the
-                      resulting Secret Defaults to 'None'
+                      resulting Secret Defaults to 'Retain'
+                    enum:
+                    - Delete
+                    - Merge
+                    - Retain
                     type: string
                   immutable:
                     description: Immutable defines if the final secret will be immutable

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

@@ -40,4 +40,29 @@ webhooks:
   admissionReviewVersions: ["v1", "v1beta1"]
   sideEffects: None
   timeoutSeconds: 5
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: externalsecret-validate
+  labels:
+    external-secrets.io/component: webhook
+webhooks:
+- name: "validate.externalsecret.external-secrets.io"
+  rules:
+  - apiGroups:   ["external-secrets.io"]
+    apiVersions: ["v1beta1"]
+    operations:  ["CREATE", "UPDATE", "DELETE"]
+    resources:   ["externalsecrets"]
+    scope:       "Namespaced"
+  clientConfig:
+    service:
+      namespace: {{ .Release.Namespace | quote }}
+      name: {{ include "external-secrets.fullname" . }}-webhook
+      path: /validate-external-secrets-io-v1beta1-externalsecret
+    # will be set by controller
+    caBundle: Cg==
+  admissionReviewVersions: ["v1", "v1beta1"]
+  sideEffects: None
+  timeoutSeconds: 5
 {{- end }}

deploy/crds/bundle.yaml

@@ -143,10 +143,19 @@ spec:
                         creationPolicy:
                           default: Owner
                           description: CreationPolicy defines rules on how to create the resulting Secret Defaults to 'Owner'
+                          enum:
+                            - Owner
+                            - Orphan
+                            - Merge
+                            - None
                           type: string
                         deletionPolicy:
-                          default: None
-                          description: DeletionPolicy defines rules on how to delete the resulting Secret Defaults to 'None'
+                          default: Retain
+                          description: DeletionPolicy defines rules on how to delete the resulting Secret Defaults to 'Retain'
+                          enum:
+                            - Delete
+                            - Merge
+                            - Retain
                           type: string
                         immutable:
                           description: Immutable defines if the final secret will be immutable
@@ -2730,10 +2739,19 @@ spec:
                     creationPolicy:
                       default: Owner
                       description: CreationPolicy defines rules on how to create the resulting Secret Defaults to 'Owner'
+                      enum:
+                        - Owner
+                        - Orphan
+                        - Merge
+                        - None
                       type: string
                     deletionPolicy:
-                      default: None
-                      description: DeletionPolicy defines rules on how to delete the resulting Secret Defaults to 'None'
+                      default: Retain
+                      description: DeletionPolicy defines rules on how to delete the resulting Secret Defaults to 'Retain'
+                      enum:
+                        - Delete
+                        - Merge
+                        - Retain
                       type: string
                     immutable:
                       description: Immutable defines if the final secret will be immutable

docs/guides-ownership-deletion-policy.md

@@ -0,0 +1,50 @@
+# Lifecycle
+The External Secrets Operator manages the lifecycle of secrets in Kubernetes. With `creationPolicy` and `deletionPolicy` you get fine-grained control of its lifecycle.
+
+!!! note "Creation/Deletion Policy Combinations"
+    Some combinations of creationPolicy/deletionPolicy are not allowed as they would delete existing secrets:
+    <br/>- `deletionPolicy=Delete` & `creationPolicy=Merge`
+    <br/>- `deletionPolicy=Delete` & `creationPolicy=None`
+    <br/>- `deletionPolicy=Merge` & `creationPolicy=None`
+
+## Creation Policy
+The field `spec.creationPolicy` defines how the operator creates the a secret.
+
+### Owner (default)
+The External Secret Operator creates secret and sets the `ownerReference` field on the Secret. This secret is subject to [garbage collection](https://kubernetes.io/docs/concepts/architecture/garbage-collection/) if the initial `ExternalSecret` is absent. If a secret with the same name already exists that is not owned by the controller it will result in a conflict. The operator will just error out, not claiming the ownership.
+
+### Orphan
+The operator creates ths secret but does not set the `ownerReference` on the Secret. That means the Secret will not be subject to garbage collection. If a secret with the same name already exists it will be updated.
+
+### Merge
+The operator does not create a secret. Instead, it expects the secret to already exist. Values from the secret provider will be merged into the existing secret. Note: the controller takes ownership of a field even if it is owned by a different entity. Multiple ExternalSecrets can use `creationPolicy=Merge` with a single secret as long as the fields don't collide - otherwise you end up in an oscillating state.
+
+### None
+The operator does not create or update the secret, this is basically a no-op.
+
+## Deletion Policy
+DeletionPolicy defines what should happen if a given secret gets deleted **from the provider**.
+
+DeletionPolicy is only supported on the following providers. Please feel free to contribute more:
+* AWS Secrets Manager
+* AWS Parameter Store
+
+### Retain (default)
+Retain will retain the secret if all provider secrets have been deleted.
+If a provider secret does not exist the ExternalSecret gets into the
+SecretSyncedError status.
+
+### Delete
+Delete deletes the secret if all provider secrets are deleted.
+If a secret gets deleted on the provider side and is not accessible
+anymore this is not considered an error and the ExternalSecret
+does not go into SecretSyncedError status. This is also true for new
+ExternalSecrets mapping to non-existing secrets in the provider.
+
+### Merge
+Merge removes keys in the secret, but not the secret itself.
+If a secret gets deleted on the provider side and is not accessible
+anymore this is not considered an error and the ExternalSecret
+does not go into SecretSyncedError status.
+
+

docs/snippets/full-external-secret.yaml

@@ -39,6 +39,11 @@ spec:
     # None does not create a secret (future use with injector)
     creationPolicy: 'Merge'
 
+    # DeletionPolicy defines how/when to delete the Secret in Kubernetes
+    # if the provider secret gets deleted.
+    # Valid values are Delete, Merge, Retain
+    deletionPolicy: "Retain"
+
     # Specify a blueprint for the resulting Kind=Secret
     template:
       type: kubernetes.io/dockerconfigjson # or TLS...
@@ -79,10 +84,10 @@ spec:
       property: provider-key-property
       conversionStrategy: Default
   - find:
-      path: path-to-filter 
-      name: 
+      path: path-to-filter
+      name:
         regexp: ".*foobar.*"
-      tags: 
+      tags:
         foo: bar
       conversionStrategy: Unicode
 

e2e/framework/log/log.go

@@ -19,5 +19,5 @@ import (
 
 // Logf logs the format string to ginkgo stdout.
 func Logf(format string, args ...interface{}) {
-	ginkgo.GinkgoWriter.Printf(format, args)
+	ginkgo.GinkgoWriter.Printf(format, args...)
 }

e2e/framework/testcase.go

@@ -35,6 +35,7 @@ type TestCase struct {
 	ExternalSecretV1Alpha1 *esv1alpha1.ExternalSecret
 	Secrets                map[string]SecretEntry
 	ExpectedSecret         *v1.Secret
+	AfterSync              func(SecretStoreProvider, *v1.Secret)
 }
 
 type SecretEntry struct {
@@ -92,11 +93,14 @@ func TableFunc(f *Framework, prov SecretStoreProvider) func(...func(*TestCase))
 		}
 
 		Expect(err).ToNot(HaveOccurred())
+
+		tc.AfterSync(prov, secret)
 	}
 }
 
 func makeDefaultTestCase(f *Framework) *TestCase {
 	return &TestCase{
+		AfterSync: func(ssp SecretStoreProvider, s *v1.Secret) {},
 		Framework: f,
 		ExternalSecret: &esv1beta1.ExternalSecret{
 			ObjectMeta: metav1.ObjectMeta{

e2e/suite/aws/parameterstore/parameterstore.go

@@ -42,6 +42,7 @@ var _ = Describe("[aws] ", Label("aws", "parameterstore"), func() {
 		Entry(common.SyncWithoutTargetName(f)),
 		Entry(common.JSONDataWithoutTargetName(f)),
 		Entry(common.SyncV1Alpha1(f)),
+		Entry(common.DeletionPolicyDelete(f)),
 
 		// These are specific to parameterstore
 		Entry(FindByName(f)),

e2e/suite/aws/parameterstore/provider.go

@@ -16,6 +16,7 @@ package aws
 
 import (
 	"context"
+	"errors"
 	"os"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -118,6 +119,10 @@ func (s *Provider) DeleteSecret(key string) {
 	_, err := s.client.DeleteParameter(&ssm.DeleteParameterInput{
 		Name: aws.String(key),
 	})
+	var nf *ssm.ParameterNotFound
+	if errors.As(err, &nf) {
+		return
+	}
 	Expect(err).ToNot(HaveOccurred())
 }
 

e2e/suite/aws/secretsmanager/provider.go

@@ -16,6 +16,7 @@ package aws
 
 import (
 	"context"
+	"errors"
 	"os"
 	"time"
 
@@ -136,6 +137,10 @@ func (s *Provider) DeleteSecret(key string) {
 		SecretId:                   aws.String(key),
 		ForceDeleteWithoutRecovery: aws.Bool(true),
 	})
+	var nf *secretsmanager.ResourceNotFoundException
+	if errors.As(err, &nf) {
+		return
+	}
 	Expect(err).ToNot(HaveOccurred())
 }
 

e2e/suite/aws/secretsmanager/secretsmanager.go

@@ -46,5 +46,6 @@ var _ = Describe("[aws] ", Label("aws", "secretsmanager"), func() {
 		Entry(common.FindByTag(f)),
 		Entry(common.FindByTagWithPath(f)),
 		Entry(common.SyncV1Alpha1(f)),
+		Entry(common.DeletionPolicyDelete(f)),
 	)
 })

e2e/suite/common/common.go

@@ -13,9 +13,13 @@ limitations under the License.
 package common
 
 import (
+	"context"
 	"fmt"
+	"time"
 
+	"github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
@@ -556,3 +560,48 @@ func SSHKeySyncDataProperty(f *framework.Framework) (string, func(*framework.Tes
 		}
 	}
 }
+
+func DeletionPolicyDelete(f *framework.Framework) (string, func(*framework.TestCase)) {
+	return "[common] should delete secret when provider secret was deleted using .data[]", func(tc *framework.TestCase) {
+		secretKey1 := fmt.Sprintf("%s-%s", f.Namespace.Name, "one")
+		secretKey2 := fmt.Sprintf("%s-%s", f.Namespace.Name, "other")
+		secretValue := "bazz"
+		tc.Secrets = map[string]framework.SecretEntry{
+			secretKey1: {Value: secretValue},
+			secretKey2: {Value: secretValue},
+		}
+		tc.ExpectedSecret = &v1.Secret{
+			Type: v1.SecretTypeOpaque,
+			Data: map[string][]byte{
+				secretKey1: []byte(secretValue),
+				secretKey2: []byte(secretValue),
+			},
+		}
+
+		tc.ExternalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second * 5}
+		tc.ExternalSecret.Spec.Target.DeletionPolicy = esv1beta1.DeletionPolicyDelete
+		tc.ExternalSecret.Spec.Data = []esv1beta1.ExternalSecretData{
+			{
+				SecretKey: secretKey1,
+				RemoteRef: esv1beta1.ExternalSecretDataRemoteRef{
+					Key: secretKey1,
+				},
+			},
+			{
+				SecretKey: secretKey2,
+				RemoteRef: esv1beta1.ExternalSecretDataRemoteRef{
+					Key: secretKey2,
+				},
+			},
+		}
+		tc.AfterSync = func(prov framework.SecretStoreProvider, secret *v1.Secret) {
+			prov.DeleteSecret(secretKey1)
+			prov.DeleteSecret(secretKey2)
+
+			gomega.Eventually(func() bool {
+				_, err := f.KubeClientSet.CoreV1().Secrets(f.Namespace.Name).Get(context.Background(), secret.Name, metav1.GetOptions{})
+				return errors.IsNotFound(err)
+			}, time.Minute, time.Second*5).Should(gomega.BeTrue())
+		}
+	}
+}

hack/api-docs/mkdocs.yml

@@ -41,6 +41,7 @@ nav:
     - All keys, One secret: guides-all-keys-one-secret.md
     - Common K8S Secret Types: guides-common-k8s-secret-types.md
     - Controller Classes: guides-controller-class.md
+    - "Lifecycle: ownership & deletion": guides-ownership-deletion-policy.md
     - Getting Multiple Secrets: guides-getallsecrets.md
     - Multi Tenancy: guides-multi-tenancy.md
     - Metrics: guides-metrics.md

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -16,7 +16,10 @@ package externalsecret
 
 import (
 	"context"
+	"encoding/json"
+	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -45,6 +48,8 @@ import (
 const (
 	requeueAfter = time.Second * 30
 
+	fieldOwner = "external-secrets"
+
 	errGetES                 = "could not get ExternalSecret"
 	errConvert               = "could not apply conversion strategy to keys: %v"
 	errUpdateSecret          = "could not update Secret"
@@ -58,9 +63,11 @@ const (
 	errCloseStoreClient      = "could not close provider client"
 	errSetCtrlReference      = "could not set ExternalSecret controller reference: %w"
 	errFetchTplFrom          = "error fetching templateFrom data: %w"
-	errGetSecretData         = "could not get secret data from provider: %w"
+	errGetSecretData         = "could not get secret data from provider"
+	errDeleteSecret          = "could not delete secret"
 	errApplyTemplate         = "could not apply template: %w"
 	errExecTpl               = "could not execute template: %w"
+	errInvalidCreatePolicy   = "invalid creationPolicy=%s. Can not delete secret i do not own"
 	errPolicyMergeNotFound   = "the desired secret %s was not found. With creationPolicy=Merge the secret won't be created"
 	errPolicyMergeGetSecret  = "unable to get secret %s: %w"
 	errPolicyMergeMutate     = "unable to mutate secret %s: %w"
@@ -209,32 +216,89 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		Data:      make(map[string][]byte),
 	}
 
+	dataMap, err := r.getProviderSecretData(ctx, secretClient, &externalSecret)
+	if err != nil {
+		log.Error(err, errGetSecretData)
+		r.recorder.Event(&externalSecret, v1.EventTypeWarning, esv1beta1.ReasonUpdateFailed, err.Error())
+		conditionSynced := NewExternalSecretCondition(esv1beta1.ExternalSecretReady, v1.ConditionFalse, esv1beta1.ConditionReasonSecretSyncedError, errGetSecretData)
+		SetExternalSecretCondition(&externalSecret, *conditionSynced)
+		syncCallsError.With(syncCallsMetricLabels).Inc()
+		return ctrl.Result{RequeueAfter: requeueAfter}, nil
+	}
+
+	// if no data was found we can delete the secret if needed.
+	if len(dataMap) == 0 {
+		switch externalSecret.Spec.Target.DeletionPolicy {
+		// delete secret and return early.
+		case esv1beta1.DeletionPolicyDelete:
+			// safeguard that we only can delete secrets we own
+			// this is also implemented in the es validation webhook
+			if externalSecret.Spec.Target.CreationPolicy != esv1beta1.CreatePolicyOwner {
+				err := fmt.Errorf(errInvalidCreatePolicy, externalSecret.Spec.Target.CreationPolicy)
+				log.Error(err, errDeleteSecret)
+				r.recorder.Event(&externalSecret, v1.EventTypeWarning, esv1beta1.ReasonUpdateFailed, err.Error())
+				conditionSynced := NewExternalSecretCondition(esv1beta1.ExternalSecretReady, v1.ConditionFalse, esv1beta1.ConditionReasonSecretSyncedError, errDeleteSecret)
+				SetExternalSecretCondition(&externalSecret, *conditionSynced)
+				syncCallsError.With(syncCallsMetricLabels).Inc()
+				return ctrl.Result{RequeueAfter: requeueAfter}, nil
+			}
+			err = r.Delete(ctx, secret)
+			if err != nil && !apierrors.IsNotFound(err) {
+				log.Error(err, errDeleteSecret)
+				r.recorder.Event(&externalSecret, v1.EventTypeWarning, esv1beta1.ReasonUpdateFailed, err.Error())
+				conditionSynced := NewExternalSecretCondition(esv1beta1.ExternalSecretReady, v1.ConditionFalse, esv1beta1.ConditionReasonSecretSyncedError, errDeleteSecret)
+				SetExternalSecretCondition(&externalSecret, *conditionSynced)
+				syncCallsError.With(syncCallsMetricLabels).Inc()
+			}
+
+			conditionSynced := NewExternalSecretCondition(esv1beta1.ExternalSecretReady, v1.ConditionTrue, esv1beta1.ConditionReasonSecretDeleted, "secret deleted due to DeletionPolicy")
+			SetExternalSecretCondition(&externalSecret, *conditionSynced)
+			return ctrl.Result{RequeueAfter: requeueAfter}, nil
+
+		case esv1beta1.DeletionPolicyMerge:
+			// noop, handled below
+
+		// In case provider secrets don't exist the kubernetes secret will be kept as-is.
+		case esv1beta1.DeletionPolicyRetain:
+			return ctrl.Result{RequeueAfter: requeueAfter}, nil
+		}
+	}
+
 	mutationFunc := func() error {
-		if externalSecret.Spec.Target.CreationPolicy == esv1beta1.Owner {
+		if externalSecret.Spec.Target.CreationPolicy == esv1beta1.CreatePolicyOwner {
 			err = controllerutil.SetControllerReference(&externalSecret, &secret.ObjectMeta, r.Scheme)
 			if err != nil {
 				return fmt.Errorf(errSetCtrlReference, err)
 			}
 		}
-
-		dataMap, err := r.getProviderSecretData(ctx, secretClient, &externalSecret)
-		if err != nil {
-			return fmt.Errorf(errGetSecretData, err)
+		if secret.Data == nil {
+			secret.Data = make(map[string][]byte)
 		}
-
 		err = r.applyTemplate(ctx, &externalSecret, secret, dataMap)
 		if err != nil {
 			return fmt.Errorf(errApplyTemplate, err)
 		}
 
+		// diff existing keys
+		if externalSecret.Spec.Target.DeletionPolicy == esv1beta1.DeletionPolicyMerge {
+			keys, err := getManagedKeys(&existingSecret)
+			if err != nil {
+				return err
+			}
+			for _, key := range keys {
+				if dataMap[key] == nil {
+					secret.Data[key] = nil
+				}
+			}
+		}
 		return nil
 	}
 
 	// nolint
 	switch externalSecret.Spec.Target.CreationPolicy {
-	case esv1beta1.Merge:
+	case esv1beta1.CreatePolicyMerge:
 		err = patchSecret(ctx, r.Client, r.Scheme, secret, mutationFunc)
-	case esv1beta1.None:
+	case esv1beta1.CreatePolicyNone:
 		log.V(1).Info("secret creation skipped due to creationPolicy=None")
 		err = nil
 	default:
@@ -302,13 +366,42 @@ func patchSecret(ctx context.Context, c client.Client, scheme *runtime.Scheme, s
 
 	// we're not able to resolve conflicts so we force ownership
 	// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#using-server-side-apply-in-a-controller
-	err = c.Patch(ctx, secret, client.Apply, client.FieldOwner("external-secrets"), client.ForceOwnership)
+	err = c.Patch(ctx, secret, client.Apply, client.FieldOwner(fieldOwner), client.ForceOwnership)
 	if err != nil {
 		return fmt.Errorf(errPolicyMergePatch, secret.Name, err)
 	}
 	return nil
 }
 
+func getManagedKeys(secret *v1.Secret) ([]string, error) {
+	var keys []string
+	for _, v := range secret.ObjectMeta.ManagedFields {
+		if v.Manager != fieldOwner {
+			continue
+		}
+		fields := make(map[string]interface{})
+		err := json.Unmarshal(v.FieldsV1.Raw, &fields)
+		if err != nil {
+			return nil, fmt.Errorf("error unmarshaling managed fields: %w", err)
+		}
+		dataFields := fields["f:data"]
+		if dataFields == nil {
+			continue
+		}
+		df, ok := dataFields.(map[string]string)
+		if !ok {
+			continue
+		}
+		for k := range df {
+			if k == "." {
+				continue
+			}
+			keys = append(keys, strings.TrimPrefix(k, "f:"))
+		}
+	}
+	return keys, nil
+}
+
 func getResourceVersion(es esv1beta1.ExternalSecret) string {
 	return fmt.Sprintf("%d-%s", es.ObjectMeta.GetGeneration(), hashMeta(es.ObjectMeta))
 }
@@ -404,11 +497,15 @@ func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1beta1.Ext
 func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient esv1beta1.SecretsClient, externalSecret *esv1beta1.ExternalSecret) (map[string][]byte, error) {
 	providerData := make(map[string][]byte)
 
-	for _, remoteRef := range externalSecret.Spec.DataFrom {
+	for i, remoteRef := range externalSecret.Spec.DataFrom {
 		var secretMap map[string][]byte
 		var err error
 		if remoteRef.Find != nil {
 			secretMap, err = providerClient.GetAllSecrets(ctx, *remoteRef.Find)
+			if errors.Is(err, esv1beta1.NoSecretErr) && externalSecret.Spec.Target.DeletionPolicy != esv1beta1.DeletionPolicyRetain {
+				r.recorder.Event(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonDeleted, fmt.Sprintf("secret does not exist at provider using .dataFrom[%d]", i))
+				continue
+			}
 			if err != nil {
 				return nil, err
 			}
@@ -418,6 +515,10 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient e
 			}
 		} else if remoteRef.Extract != nil {
 			secretMap, err = providerClient.GetSecretMap(ctx, *remoteRef.Extract)
+			if errors.Is(err, esv1beta1.NoSecretErr) && externalSecret.Spec.Target.DeletionPolicy != esv1beta1.DeletionPolicyRetain {
+				r.recorder.Event(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonDeleted, fmt.Sprintf("secret does not exist at provider using .dataFrom[%d]", i))
+				continue
+			}
 			if err != nil {
 				return nil, err
 			}
@@ -430,8 +531,12 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient e
 		providerData = utils.MergeByteMap(providerData, secretMap)
 	}
 
-	for _, secretRef := range externalSecret.Spec.Data {
+	for i, secretRef := range externalSecret.Spec.Data {
 		secretData, err := providerClient.GetSecret(ctx, secretRef.RemoteRef)
+		if errors.Is(err, esv1beta1.NoSecretErr) && externalSecret.Spec.Target.DeletionPolicy != esv1beta1.DeletionPolicyRetain {
+			r.recorder.Event(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonDeleted, fmt.Sprintf("secret does not exist at provider using .data[%d] key=%s", i, secretRef.RemoteRef.Key))
+			continue
+		}
 		if err != nil {
 			return nil, err
 		}

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -14,6 +14,7 @@ limitations under the License.
 package externalsecret
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"os"
@@ -24,6 +25,7 @@ import (
 	. "github.com/onsi/gomega"
 	dto "github.com/prometheus/client_model/go"
 	v1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -155,6 +157,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		syncCallsTotal.Reset()
 		syncCallsError.Reset()
 		externalSecretCondition.Reset()
+		fakeProvider.Reset()
 	})
 
 	AfterEach(func() {
@@ -162,13 +165,13 @@ var _ = Describe("ExternalSecret controller", func() {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: ExternalSecretNamespace,
 			},
-		}, client.PropagationPolicy(metav1.DeletePropagationBackground)), client.GracePeriodSeconds(0)).To(Succeed())
+		})).To(Succeed())
 		Expect(k8sClient.Delete(context.Background(), &esv1beta1.SecretStore{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      ExternalSecretStore,
 				Namespace: ExternalSecretNamespace,
 			},
-		}, client.PropagationPolicy(metav1.DeletePropagationBackground)), client.GracePeriodSeconds(0)).To(Succeed())
+		})).To(Succeed())
 	})
 
 	const targetProp = "targetProperty"
@@ -281,7 +284,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		const secretVal = "someValue"
 		const existingKey = "pre-existing-key"
 		existingVal := "pre-existing-value"
-		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.Merge
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyMerge
 
 		// create secret beforehand
 		Expect(k8sClient.Create(context.Background(), &v1.Secret{
@@ -320,7 +323,7 @@ var _ = Describe("ExternalSecret controller", func() {
 	mergeWithSecretNoChange := func(tc *testCase) {
 		const existingKey = "pre-existing-key"
 		existingVal := "someValue"
-		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.Merge
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyMerge
 
 		// create secret beforehand
 		Expect(k8sClient.Create(context.Background(), &v1.Secret{
@@ -360,7 +363,7 @@ var _ = Describe("ExternalSecret controller", func() {
 	// should not merge with secret if it doesn't exist
 	mergeWithSecretErr := func(tc *testCase) {
 		const secretVal = "someValue"
-		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.Merge
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyMerge
 
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		tc.checkCondition = func(es *esv1beta1.ExternalSecret) bool {
@@ -386,7 +389,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		// this should confict
 		const existingKey = targetProp
 		existingVal := "pre-existing-value"
-		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.Merge
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyMerge
 
 		// create secret beforehand
 		Expect(k8sClient.Create(context.Background(), &v1.Secret{
@@ -785,6 +788,162 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 	}
 
+	deleteSecretPolicy := func(tc *testCase) {
+		expVal := []byte("1234")
+		// set initial value
+		fakeProvider.WithGetAllSecrets(map[string][]byte{
+			"foo": expVal,
+			"bar": expVal,
+		}, nil)
+		tc.externalSecret.Spec.Data = nil
+		tc.externalSecret.Spec.DataFrom = []esv1beta1.ExternalSecretDataFromRemoteRef{
+			{
+				Find: &esv1beta1.ExternalSecretFind{
+					Tags: map[string]string{},
+				},
+			},
+		}
+		tc.externalSecret.Spec.Target.DeletionPolicy = esv1beta1.DeletionPolicyDelete
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
+		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
+			Expect(secret.Data["foo"]).To(Equal(expVal))
+
+			// update provider secret
+			fakeProvider.WithGetAllSecrets(map[string][]byte{
+				"foo": expVal,
+			}, nil)
+			sec := &v1.Secret{}
+			secretLookupKey := types.NamespacedName{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			Eventually(func() bool {
+				By("checking secret value for foo=1234 and bar=nil")
+				err := k8sClient.Get(context.Background(), secretLookupKey, sec)
+				if err != nil {
+					return false
+				}
+				return bytes.Equal(sec.Data["foo"], expVal) && sec.Data["bar"] == nil
+			}, time.Second*10, time.Second).Should(BeTrue())
+
+			// return specific delete err to indicate deletion
+			fakeProvider.WithGetAllSecrets(map[string][]byte{}, esv1beta1.NoSecretErr)
+			Eventually(func() bool {
+				By("checking that secret has been deleted")
+				err := k8sClient.Get(context.Background(), secretLookupKey, sec)
+				return apierrors.IsNotFound(err)
+			}, time.Second*10, time.Second).Should(BeTrue())
+		}
+	}
+
+	deleteSecretPolicyRetain := func(tc *testCase) {
+		expVal := []byte("1234")
+		// set initial value
+		fakeProvider.WithGetAllSecrets(map[string][]byte{
+			"foo": expVal,
+			"bar": expVal,
+		}, nil)
+		tc.externalSecret.Spec.DataFrom = []esv1beta1.ExternalSecretDataFromRemoteRef{
+			{
+				Find: &esv1beta1.ExternalSecretFind{
+					Tags: map[string]string{},
+				},
+			},
+		}
+		tc.externalSecret.Spec.Target.DeletionPolicy = esv1beta1.DeletionPolicyRetain
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
+		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
+			Expect(secret.Data["foo"]).To(Equal(expVal))
+
+			sec := &v1.Secret{}
+			secretLookupKey := types.NamespacedName{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			// return specific delete err to indicate deletion
+			// however this should not trigger a delete
+			fakeProvider.WithGetAllSecrets(map[string][]byte{}, esv1beta1.NoSecretErr)
+			Consistently(func() bool {
+				By("checking that secret has not been deleted")
+				err := k8sClient.Get(context.Background(), secretLookupKey, sec)
+				return apierrors.IsNotFound(err) && bytes.Equal(sec.Data["foo"], expVal)
+			}, time.Second*10, time.Second).Should(BeFalse())
+		}
+	}
+
+	// merge with existing secret using creationPolicy=Merge
+	// if provider secret gets deleted only the managed field should get deleted
+	deleteSecretPolicyMerge := func(tc *testCase) {
+		const secretVal = "someValue"
+		const existingKey = "some-existing-key"
+		existingVal := "some-existing-value"
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyMerge
+		tc.externalSecret.Spec.Target.DeletionPolicy = esv1beta1.DeletionPolicyMerge
+
+		// create secret beforehand
+		Expect(k8sClient.Create(context.Background(), &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			},
+			Data: map[string][]byte{
+				existingKey: []byte(existingVal),
+			},
+		}, client.FieldOwner(FakeManager))).To(Succeed())
+
+		fakeProvider.WithGetSecret([]byte(secretVal), nil)
+		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
+			// check value
+			Expect(string(secret.Data[existingKey])).To(Equal(existingVal))
+			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
+
+			sec := &v1.Secret{}
+			secretLookupKey := types.NamespacedName{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			// return specific delete err to indicate deletion
+			// however this should not trigger a delete
+			// instead expect that only the pre-existing value exists
+			fakeProvider.WithGetSecret(nil, esv1beta1.NoSecretErr)
+			Eventually(func() bool {
+				By("checking that secret has not been deleted and pre-existing key exists")
+				err := k8sClient.Get(context.Background(), secretLookupKey, sec)
+				return !apierrors.IsNotFound(err) &&
+					len(sec.Data) == 1 &&
+					bytes.Equal(sec.Data[existingKey], []byte(existingVal))
+			}, time.Second*30, time.Second).Should(BeTrue())
+
+		}
+	}
+
+	// orphan the secret after the external secret has been deleted
+	createSecretPolicyOrphan := func(tc *testCase) {
+		const secretVal = "someValue"
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1beta1.CreatePolicyOrphan
+
+		fakeProvider.WithGetSecret([]byte(secretVal), nil)
+		tc.checkSecret = func(es *esv1beta1.ExternalSecret, secret *v1.Secret) {
+			// check value
+			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
+
+			sec := &v1.Secret{}
+			secretLookupKey := types.NamespacedName{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			err := k8sClient.Delete(context.Background(), tc.externalSecret)
+			Expect(err).ToNot(HaveOccurred())
+			Consistently(func() bool {
+				By("checking that secret has not been deleted")
+				err := k8sClient.Get(context.Background(), secretLookupKey, sec)
+				return !apierrors.IsNotFound(err)
+			}, time.Second*15, time.Second).Should(BeTrue())
+		}
+	}
+
 	// with dataFrom all properties from the specified secret
 	// should be put into the secret
 	syncWithDataFrom := func(tc *testCase) {
@@ -1113,6 +1272,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		Entry("should error if secret doesn't exist when using creationPolicy=Merge", mergeWithSecretErr),
 		Entry("should not resolve conflicts with creationPolicy=Merge", mergeWithConflict),
 		Entry("should not update unchanged secret using creationPolicy=Merge", mergeWithSecretNoChange),
+		Entry("should not delete pre-existing secret with creationPolicy=Orphan", createSecretPolicyOrphan),
 		Entry("should sync with template", syncWithTemplate),
 		Entry("should sync with template engine v2", syncWithTemplateV2),
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
@@ -1130,6 +1290,9 @@ var _ = Describe("ExternalSecret controller", func() {
 		Entry("should set an error condition when store provider constructor fails", storeConstructErrCondition),
 		Entry("should not process store with mismatching controller field", ignoreMismatchController),
 		Entry("should not process cluster secret store when it is disabled", ignoreClusterSecretStoreWhenDisabled),
+		Entry("should eventually delete target secret with deletionPolicy=Delete", deleteSecretPolicy),
+		Entry("should not delete target secret with deletionPolicy=Retain", deleteSecretPolicyRetain),
+		Entry("should not delete pre-existing secret with deletionPolicy=Merge", deleteSecretPolicyMerge),
 	)
 })
 

pkg/provider/aws/parameterstore/parameterstore.go

@@ -170,6 +170,11 @@ func (pm *ParameterStore) GetSecret(ctx context.Context, ref esv1beta1.ExternalS
 		Name:           &ref.Key,
 		WithDecryption: aws.Bool(true),
 	})
+
+	var nf *ssm.ParameterNotFound
+	if errors.As(err, &nf) {
+		return nil, esv1beta1.NoSecretErr
+	}
 	if err != nil {
 		return nil, util.SanitizeErr(err)
 	}

pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -77,6 +77,10 @@ func (sm *SecretsManager) fetch(_ context.Context, ref esv1beta1.ExternalSecretD
 		SecretId:     &ref.Key,
 		VersionStage: &ver,
 	})
+	var nf *awssm.ResourceNotFoundException
+	if errors.As(err, &nf) {
+		return nil, esv1beta1.NoSecretErr
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -212,6 +216,9 @@ func (sm *SecretsManager) fetchAndSet(ctx context.Context, data map[string][]byt
 // GetSecret returns a single secret from the provider.
 func (sm *SecretsManager) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	secretOut, err := sm.fetch(ctx, ref)
+	if errors.Is(err, esv1beta1.NoSecretErr) {
+		return nil, err
+	}
 	if err != nil {
 		return nil, util.SanitizeErr(err)
 	}

pkg/provider/fake/fake.go

@@ -24,7 +24,6 @@ import (
 )
 
 var (
-	errNotFound            = fmt.Errorf("secret value not found")
 	errMissingStore        = fmt.Errorf("missing store provider")
 	errMissingFakeProvider = fmt.Errorf("missing store provider fake")
 	errMissingKeyField     = "key must be set in data %v"
@@ -69,7 +68,7 @@ func (p *Provider) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDa
 			return []byte(data.Value), nil
 		}
 	}
-	return nil, errNotFound
+	return nil, esv1beta1.NoSecretErr
 }
 
 // GetSecretMap returns multiple k/v pairs from the provider.
@@ -80,7 +79,7 @@ func (p *Provider) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecre
 		}
 		return convertMap(data.ValueMap), nil
 	}
-	return nil, errNotFound
+	return nil, esv1beta1.NoSecretErr
 }
 
 func convertMap(in map[string]string) map[string][]byte {

pkg/provider/fake/fake_test.go

@@ -95,7 +95,7 @@ func TestGetSecret(t *testing.T) {
 				Key:     "/foo",
 				Version: "v2",
 			},
-			expErr: "secret value not found",
+			expErr: esv1beta1.NoSecretErr.Error(),
 		},
 		{
 			name: "get correct value from multiple versions",
@@ -165,7 +165,7 @@ func TestGetSecretMap(t *testing.T) {
 				Key:     "/foo",
 				Version: "v2",
 			},
-			expErr: "secret value not found",
+			expErr: esv1beta1.NoSecretErr.Error(),
 		},
 		{
 			name: "get correct value from multiple versions",

pkg/provider/testing/fake/fake.go

@@ -125,3 +125,10 @@ func (v *Client) NewClient(ctx context.Context, store esv1beta1.GenericStore, ku
 	}
 	return c, nil
 }
+
+func (v *Client) Reset() {
+	v.WithNew(func(context.Context, esv1beta1.GenericStore, client.Client,
+		string) (esv1beta1.SecretsClient, error) {
+		return v, nil
+	})
+}