Browse Source

fix: dependent kind=secret are not recreated in case of deletion. (#349)

* chore: whitespace, typos, superflous aliases

* fix: deleted child secret is not recreated straight away.

* fix: e2e run
Alexander Chernov 4 years ago
parent
commit
280964f84e

+ 5 - 0
apis/externalsecrets/v1alpha1/externalsecret_types.go

@@ -207,6 +207,11 @@ type ExternalSecret struct {
 	Status ExternalSecretStatus `json:"status,omitempty"`
 	Status ExternalSecretStatus `json:"status,omitempty"`
 }
 }
 
 
+const (
+	// AnnotationDataHash is used to ensure consistency.
+	AnnotationDataHash = "reconcile.external-secrets.io/data-hash"
+)
+
 // +kubebuilder:object:root=true
 // +kubebuilder:object:root=true
 
 
 // ExternalSecretList contains a list of ExternalSecret resources.
 // ExternalSecretList contains a list of ExternalSecret resources.

+ 9 - 0
e2e/framework/eso.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
 See the License for the specific language governing permissions and
 limitations under the License.
 limitations under the License.
 */
 */
+
 package framework
 package framework
 
 
 import (
 import (
@@ -23,6 +24,8 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/util/wait"
+
+	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 )
 )
 
 
 // WaitForSecretValue waits until a secret comes into existence and compares the secret.Data
 // WaitForSecretValue waits until a secret comes into existence and compares the secret.Data
@@ -52,6 +55,12 @@ func equalSecrets(exp, ts *v1.Secret) bool {
 		return false
 		return false
 	}
 	}
 
 
+	// secret contains data hash property which must be ignored
+	delete(ts.ObjectMeta.Annotations, esv1alpha1.AnnotationDataHash)
+	if len(ts.ObjectMeta.Annotations) == 0 {
+		ts.ObjectMeta.Annotations = nil
+	}
+
 	expAnnotations, _ := json.Marshal(exp.ObjectMeta.Annotations)
 	expAnnotations, _ := json.Marshal(exp.ObjectMeta.Annotations)
 	tsAnnotations, _ := json.Marshal(ts.ObjectMeta.Annotations)
 	tsAnnotations, _ := json.Marshal(ts.ObjectMeta.Annotations)
 	if !bytes.Equal(expAnnotations, tsAnnotations) {
 	if !bytes.Equal(expAnnotations, tsAnnotations) {

+ 1 - 0
e2e/suite/aws/provider.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
 See the License for the specific language governing permissions and
 limitations under the License.
 limitations under the License.
 */
 */
+
 package aws
 package aws
 
 
 import (
 import (

+ 1 - 0
e2e/suite/aws/secretsmanager.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
 See the License for the specific language governing permissions and
 limitations under the License.
 limitations under the License.
 */
 */
+
 package aws
 package aws
 
 
 import (
 import (

+ 1 - 1
main.go

@@ -59,7 +59,7 @@ func main() {
 	var lvl zapcore.Level
 	var lvl zapcore.Level
 	err := lvl.UnmarshalText([]byte(loglevel))
 	err := lvl.UnmarshalText([]byte(loglevel))
 	if err != nil {
 	if err != nil {
-		setupLog.Error(err, "error unmarshaling loglevel")
+		setupLog.Error(err, "error unmarshalling loglevel")
 		os.Exit(1)
 		os.Exit(1)
 	}
 	}
 	logger := zap.New(zap.Level(lvl))
 	logger := zap.New(zap.Level(lvl))

+ 40 - 19
pkg/controllers/externalsecret/externalsecret_controller.go

@@ -16,9 +16,6 @@ package externalsecret
 
 
 import (
 import (
 	"context"
 	"context"
-
-	// nolint
-	"crypto/md5"
 	"fmt"
 	"fmt"
 	"time"
 	"time"
 
 
@@ -38,8 +35,8 @@ import (
 
 
 	// Loading registered providers.
 	// Loading registered providers.
 	_ "github.com/external-secrets/external-secrets/pkg/provider/register"
 	_ "github.com/external-secrets/external-secrets/pkg/provider/register"
-	schema "github.com/external-secrets/external-secrets/pkg/provider/schema"
-	utils "github.com/external-secrets/external-secrets/pkg/utils"
+	"github.com/external-secrets/external-secrets/pkg/provider/schema"
+	"github.com/external-secrets/external-secrets/pkg/utils"
 )
 )
 
 
 const (
 const (
@@ -53,6 +50,7 @@ const (
 	errStoreRef              = "could not get store reference"
 	errStoreRef              = "could not get store reference"
 	errStoreProvider         = "could not get store provider"
 	errStoreProvider         = "could not get store provider"
 	errStoreClient           = "could not get provider client"
 	errStoreClient           = "could not get provider client"
+	errGetExistingSecret     = "could not get existing secret: %w"
 	errCloseStoreClient      = "could not close provider client"
 	errCloseStoreClient      = "could not close provider client"
 	errSetCtrlReference      = "could not set ExternalSecret controller reference: %w"
 	errSetCtrlReference      = "could not set ExternalSecret controller reference: %w"
 	errFetchTplFrom          = "error fetching templateFrom data: %w"
 	errFetchTplFrom          = "error fetching templateFrom data: %w"
@@ -126,7 +124,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 
 	// check if store should be handled by this controller instance
 	// check if store should be handled by this controller instance
 	if !shouldProcessStore(store, r.ControllerClass) {
 	if !shouldProcessStore(store, r.ControllerClass) {
-		log.Info("skippig unmanaged store")
+		log.Info("skipping unmanaged store")
 		return ctrl.Result{}, nil
 		return ctrl.Result{}, nil
 	}
 	}
 
 
@@ -158,21 +156,31 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		refreshInt = externalSecret.Spec.RefreshInterval.Duration
 		refreshInt = externalSecret.Spec.RefreshInterval.Duration
 	}
 	}
 
 
+	// Target Secret Name should default to the ExternalSecret name if not explicitly specified
+	secretName := externalSecret.Spec.Target.Name
+	if secretName == "" {
+		secretName = externalSecret.ObjectMeta.Name
+	}
+
+	// fetch external secret, we need to ensure that it exists, and it's hashmap corresponds
+	var existingSecret v1.Secret
+	err = r.Get(ctx, types.NamespacedName{
+		Name:      secretName,
+		Namespace: externalSecret.Namespace,
+	}, &existingSecret)
+	if err != nil && !apierrors.IsNotFound(err) {
+		log.Error(err, errGetExistingSecret)
+	}
+
 	// refresh should be skipped if
 	// refresh should be skipped if
 	// 1. resource generation hasn't changed
 	// 1. resource generation hasn't changed
 	// 2. refresh interval is 0
 	// 2. refresh interval is 0
 	// 3. if we're still within refresh-interval
 	// 3. if we're still within refresh-interval
-	if !shouldRefresh(externalSecret) {
+	if !shouldRefresh(externalSecret) && isSecretValid(existingSecret) {
 		log.V(1).Info("skipping refresh", "rv", getResourceVersion(externalSecret))
 		log.V(1).Info("skipping refresh", "rv", getResourceVersion(externalSecret))
 		return ctrl.Result{RequeueAfter: refreshInt}, nil
 		return ctrl.Result{RequeueAfter: refreshInt}, nil
 	}
 	}
 
 
-	// Target Secret Name should default to the ExternalSecret name if not explicitly specified
-	secretName := externalSecret.Spec.Target.Name
-	if secretName == "" {
-		secretName = externalSecret.ObjectMeta.Name
-	}
-
 	secret := &v1.Secret{
 	secret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      secretName,
 			Name:      secretName,
@@ -202,7 +210,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return nil
 		return nil
 	}
 	}
 
 
-	//nolint
+	// nolint
 	switch externalSecret.Spec.Target.CreationPolicy {
 	switch externalSecret.Spec.Target.CreationPolicy {
 	case esv1alpha1.Merge:
 	case esv1alpha1.Merge:
 		err = patchSecret(ctx, r.Client, r.Scheme, secret, mutationFunc)
 		err = patchSecret(ctx, r.Client, r.Scheme, secret, mutationFunc)
@@ -249,7 +257,7 @@ func patchSecret(ctx context.Context, c client.Client, scheme *runtime.Scheme, s
 	// https://github.com/kubernetes-sigs/controller-runtime/issues/526
 	// https://github.com/kubernetes-sigs/controller-runtime/issues/526
 	// https://github.com/kubernetes-sigs/controller-runtime/issues/1517
 	// https://github.com/kubernetes-sigs/controller-runtime/issues/1517
 	// https://github.com/kubernetes/kubernetes/issues/80609
 	// https://github.com/kubernetes/kubernetes/issues/80609
-	// we need to manually set it befor doing a Patch() as it depends on the GVK
+	// we need to manually set it before doing a Patch() as it depends on the GVK
 	gvks, unversioned, err := scheme.ObjectKinds(secret)
 	gvks, unversioned, err := scheme.ObjectKinds(secret)
 	if err != nil {
 	if err != nil {
 		return err
 		return err
@@ -284,12 +292,10 @@ func hashMeta(m metav1.ObjectMeta) string {
 		annotations map[string]string
 		annotations map[string]string
 		labels      map[string]string
 		labels      map[string]string
 	}
 	}
-	h := md5.New() //nolint
-	_, _ = h.Write([]byte(fmt.Sprintf("%v", meta{
+	return utils.ObjectHash(meta{
 		annotations: m.Annotations,
 		annotations: m.Annotations,
 		labels:      m.Labels,
 		labels:      m.Labels,
-	})))
-	return fmt.Sprintf("%x", h.Sum(nil))
+	})
 }
 }
 
 
 func shouldRefresh(es esv1alpha1.ExternalSecret) bool {
 func shouldRefresh(es esv1alpha1.ExternalSecret) bool {
@@ -297,6 +303,7 @@ func shouldRefresh(es esv1alpha1.ExternalSecret) bool {
 	if es.Status.SyncedResourceVersion != getResourceVersion(es) {
 	if es.Status.SyncedResourceVersion != getResourceVersion(es) {
 		return true
 		return true
 	}
 	}
+
 	// skip refresh if refresh interval is 0
 	// skip refresh if refresh interval is 0
 	if es.Spec.RefreshInterval.Duration == 0 && es.Status.SyncedResourceVersion != "" {
 	if es.Spec.RefreshInterval.Duration == 0 && es.Status.SyncedResourceVersion != "" {
 		return false
 		return false
@@ -307,6 +314,20 @@ func shouldRefresh(es esv1alpha1.ExternalSecret) bool {
 	return !es.Status.RefreshTime.Add(es.Spec.RefreshInterval.Duration).After(time.Now())
 	return !es.Status.RefreshTime.Add(es.Spec.RefreshInterval.Duration).After(time.Now())
 }
 }
 
 
+// isSecretValid checks if the secret exists, and it's data is consistent with the calculated hash.
+func isSecretValid(existingSecret v1.Secret) bool {
+	// if target secret doesn't exist, or annotations as not set, we need to refresh
+	if existingSecret.UID == "" || existingSecret.Annotations == nil {
+		return false
+	}
+
+	// if the calculated hash is different from the calculation, then it's invalid
+	if existingSecret.Annotations[esv1alpha1.AnnotationDataHash] != utils.ObjectHash(existingSecret.Data) {
+		return false
+	}
+	return true
+}
+
 // getStore returns the store with the provided ExternalSecret.
 // getStore returns the store with the provided ExternalSecret.
 func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1alpha1.ExternalSecret) (esv1alpha1.GenericStore, error) {
 func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1alpha1.ExternalSecret) (esv1alpha1.GenericStore, error) {
 	ref := types.NamespacedName{
 	ref := types.NamespacedName{

+ 2 - 0
pkg/controllers/externalsecret/externalsecret_controller_template.go

@@ -40,6 +40,7 @@ func (r *Reconciler) applyTemplate(ctx context.Context, es *esv1alpha1.ExternalS
 	// no template: copy data and return
 	// no template: copy data and return
 	if es.Spec.Target.Template == nil {
 	if es.Spec.Target.Template == nil {
 		secret.Data = dataMap
 		secret.Data = dataMap
+		secret.Annotations[esv1alpha1.AnnotationDataHash] = utils.ObjectHash(secret.Data)
 		return nil
 		return nil
 	}
 	}
 
 
@@ -67,6 +68,7 @@ func (r *Reconciler) applyTemplate(ctx context.Context, es *esv1alpha1.ExternalS
 			secret.Data[k] = v
 			secret.Data[k] = v
 		}
 		}
 	}
 	}
+	secret.Annotations[esv1alpha1.AnnotationDataHash] = utils.ObjectHash(secret.Data)
 
 
 	return nil
 	return nil
 }
 }

+ 195 - 51
pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -16,6 +16,8 @@ package externalsecret
 import (
 import (
 	"context"
 	"context"
 	"fmt"
 	"fmt"
+	"os"
+	"strconv"
 	"time"
 	"time"
 
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/ginkgo"
@@ -59,6 +61,74 @@ type testCase struct {
 
 
 type testTweaks func(*testCase)
 type testTweaks func(*testCase)
 
 
+var _ = Describe("Kind=secret existence logic", func() {
+	type testCase struct {
+		Name           string
+		Input          v1.Secret
+		ExpectedOutput bool
+	}
+	tests := []testCase{
+		{
+			Name:           "Should not be valid in case of missing uid",
+			Input:          v1.Secret{},
+			ExpectedOutput: false,
+		},
+		{
+			Name: "A nil annotation should not be valid",
+			Input: v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					UID:         "xxx",
+					Annotations: map[string]string{},
+				},
+			},
+			ExpectedOutput: false,
+		},
+		{
+			Name: "A nil annotation should not be valid",
+			Input: v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					UID:         "xxx",
+					Annotations: map[string]string{},
+				},
+			},
+			ExpectedOutput: false,
+		},
+		{
+			Name: "An invalid annotation hash should not be valid",
+			Input: v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					UID: "xxx",
+					Annotations: map[string]string{
+						esv1alpha1.AnnotationDataHash: "xxxxxx",
+					},
+				},
+			},
+			ExpectedOutput: false,
+		},
+		{
+			Name: "A valid config map should return true",
+			Input: v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					UID: "xxx",
+					Annotations: map[string]string{
+						esv1alpha1.AnnotationDataHash: "caa0155759a6a9b3b6ada5a6883ee2bb",
+					},
+				},
+				Data: map[string][]byte{
+					"foo": []byte("value1"),
+					"bar": []byte("value2"),
+				},
+			},
+			ExpectedOutput: true,
+		},
+	}
+
+	for _, tt := range tests {
+		It(tt.Name, func() {
+			Expect(isSecretValid(tt.Input)).To(BeEquivalentTo(tt.ExpectedOutput))
+		})
+	}
+})
 var _ = Describe("ExternalSecret controller", func() {
 var _ = Describe("ExternalSecret controller", func() {
 	const (
 	const (
 		ExternalSecretName             = "test-es"
 		ExternalSecretName             = "test-es"
@@ -68,6 +138,13 @@ var _ = Describe("ExternalSecret controller", func() {
 
 
 	var ExternalSecretNamespace string
 	var ExternalSecretNamespace string
 
 
+	// if we are in debug and need to increase the timeout for testing, we can do so by using an env var
+	if customTimeout := os.Getenv("TEST_CUSTOM_TIMEOUT_SEC"); customTimeout != "" {
+		if t, err := strconv.Atoi(customTimeout); err == nil {
+			timeout = time.Second * time.Duration(t)
+		}
+	}
+
 	BeforeEach(func() {
 	BeforeEach(func() {
 		var err error
 		var err error
 		ExternalSecretNamespace, err = CreateNamespace("test-ns", k8sClient)
 		ExternalSecretNamespace, err = CreateNamespace("test-ns", k8sClient)
@@ -158,24 +235,32 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 		}
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
-			Eventually(func() bool {
-				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
-				return metric.GetCounter().GetValue() == 1.0
-			}, timeout, interval).Should(BeTrue())
-
 			// check value
 			// check value
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 
 
 			// check labels & annotations
 			// check labels & annotations
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.ObjectMeta.Labels))
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.ObjectMeta.Labels))
-			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.ObjectMeta.Annotations))
+			for k, v := range es.ObjectMeta.Annotations {
+				Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(k, v))
+			}
 			// ownerRef must not not be set!
 			// ownerRef must not not be set!
 			Expect(hasOwnerRef(secret.ObjectMeta, "ExternalSecret", ExternalSecretName)).To(BeTrue())
 			Expect(hasOwnerRef(secret.ObjectMeta, "ExternalSecret", ExternalSecretName)).To(BeTrue())
 		}
 		}
 	}
 	}
 
 
+	checkPrometheusCounters := func(tc *testCase) {
+		const secretVal = "someValue"
+		fakeProvider.WithGetSecret([]byte(secretVal), nil)
+		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
+			Eventually(func() bool {
+				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
+				return metric.GetCounter().GetValue() == 1.0
+			}, timeout, interval).Should(BeTrue())
+		}
+	}
+
 	// merge with existing secret using creationPolicy=Merge
 	// merge with existing secret using creationPolicy=Merge
 	// it should NOT have a ownerReference
 	// it should NOT have a ownerReference
 	// metadata.managedFields with the correct owner should be added to the secret
 	// metadata.managedFields with the correct owner should be added to the secret
@@ -198,23 +283,22 @@ var _ = Describe("ExternalSecret controller", func() {
 
 
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
-			Eventually(func() bool {
-				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
-				return metric.GetCounter().GetValue() == 1.0
-			}, timeout, interval).Should(BeTrue())
-
 			// check value
 			// check value
 			Expect(string(secret.Data[existingKey])).To(Equal(existingVal))
 			Expect(string(secret.Data[existingKey])).To(Equal(existingVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 
 
 			// check labels & annotations
 			// check labels & annotations
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.ObjectMeta.Labels))
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.ObjectMeta.Labels))
-			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.ObjectMeta.Annotations))
+			for k, v := range es.ObjectMeta.Annotations {
+				Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(k, v))
+			}
 			Expect(hasOwnerRef(secret.ObjectMeta, "ExternalSecret", ExternalSecretName)).To(BeFalse())
 			Expect(hasOwnerRef(secret.ObjectMeta, "ExternalSecret", ExternalSecretName)).To(BeFalse())
 			Expect(secret.ObjectMeta.ManagedFields).To(HaveLen(2))
 			Expect(secret.ObjectMeta.ManagedFields).To(HaveLen(2))
-			Expect(hasFieldOwnership(secret.ObjectMeta, "external-secrets", "{\"f:data\":{\"f:targetProperty\":{}}}")).To(BeTrue())
+			Expect(hasFieldOwnership(
+				secret.ObjectMeta,
+				"external-secrets",
+				fmt.Sprintf("{\"f:data\":{\"f:targetProperty\":{}},\"f:metadata\":{\"f:annotations\":{\"f:%s\":{}}}}", esv1alpha1.AnnotationDataHash)),
+			).To(BeTrue())
 			Expect(hasFieldOwnership(secret.ObjectMeta, "fake.manager", "{\"f:data\":{\".\":{},\"f:pre-existing-key\":{}},\"f:type\":{}}")).To(BeTrue())
 			Expect(hasFieldOwnership(secret.ObjectMeta, "fake.manager", "{\"f:data\":{\".\":{},\"f:pre-existing-key\":{}},\"f:type\":{}}")).To(BeTrue())
 		}
 		}
 	}
 	}
@@ -313,20 +397,15 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 		}
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
-			Eventually(func() bool {
-				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
-				return metric.GetCounter().GetValue() == 1.0
-			}, timeout, interval).Should(BeTrue())
-
 			// check values
 			// check values
 			Expect(string(secret.Data[targetProp])).To(Equal(expectedSecretVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(expectedSecretVal))
 			Expect(string(secret.Data[tplStaticKey])).To(Equal(tplStaticVal))
 			Expect(string(secret.Data[tplStaticKey])).To(Equal(tplStaticVal))
 
 
 			// labels/annotations should be taken from the template
 			// labels/annotations should be taken from the template
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
-			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Annotations))
+			for k, v := range es.Spec.Target.Template.Metadata.Annotations {
+				Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(k, v))
+			}
 		}
 		}
 	}
 	}
 
 
@@ -443,7 +522,12 @@ var _ = Describe("ExternalSecret controller", func() {
 
 
 			// labels/annotations should be taken from the template
 			// labels/annotations should be taken from the template
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
-			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Annotations))
+
+			// a secret will always have some extra annotations (i.e. hashmap check), so we only check for specific
+			// source annotations
+			for k, v := range es.Spec.Target.Template.Metadata.Annotations {
+				Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(k, v))
+			}
 
 
 			cleanEs := tc.externalSecret.DeepCopy()
 			cleanEs := tc.externalSecret.DeepCopy()
 
 
@@ -470,7 +554,9 @@ var _ = Describe("ExternalSecret controller", func() {
 
 
 			// also check labels/annotations have been updated
 			// also check labels/annotations have been updated
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
-			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Annotations))
+			for k, v := range es.Spec.Target.Template.Metadata.Annotations {
+				Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(k, v))
+			}
 		}
 		}
 	}
 	}
 
 
@@ -490,7 +576,9 @@ var _ = Describe("ExternalSecret controller", func() {
 
 
 			// labels/annotations should be taken from the template
 			// labels/annotations should be taken from the template
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
 			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
-			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Annotations))
+			for k, v := range es.Spec.Target.Template.Metadata.Annotations {
+				Expect(secret.ObjectMeta.Annotations).To(HaveKeyWithValue(k, v))
+			}
 		}
 		}
 	}
 	}
 
 
@@ -502,13 +590,6 @@ var _ = Describe("ExternalSecret controller", func() {
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
 		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
-			Eventually(func() bool {
-				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
-				return metric.GetCounter().GetValue() == 1.0
-			}, timeout, interval).Should(BeTrue())
-
 			// check values
 			// check values
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 
 
@@ -537,13 +618,6 @@ var _ = Describe("ExternalSecret controller", func() {
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: 0}
 		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: 0}
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
-			Eventually(func() bool {
-				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
-				return metric.GetCounter().GetValue() == 1.0
-			}, timeout, interval).Should(BeTrue())
-
 			// check values
 			// check values
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 
 
@@ -580,13 +654,6 @@ var _ = Describe("ExternalSecret controller", func() {
 			"bar": []byte("map-bar-value"),
 			"bar": []byte("map-bar-value"),
 		}, nil)
 		}, nil)
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
-			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
-			Eventually(func() bool {
-				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
-				return metric.GetCounter().GetValue() == 1.0
-			}, timeout, interval).Should(BeTrue())
-
 			// check values
 			// check values
 			Expect(string(secret.Data["foo"])).To(Equal("map-foo-value"))
 			Expect(string(secret.Data["foo"])).To(Equal("map-foo-value"))
 			Expect(string(secret.Data["bar"])).To(Equal("map-bar-value"))
 			Expect(string(secret.Data["bar"])).To(Equal("map-bar-value"))
@@ -703,6 +770,80 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 		}
 	}
 	}
 
 
+	// When the ownership is set to owner, and we delete a dependent child kind=secret
+	// it should be recreated without waiting for refresh interval
+	checkDeletion := func(tc *testCase) {
+		const secretVal = "someValue"
+		fakeProvider.WithGetSecret([]byte(secretVal), nil)
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Minute * 10}
+		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
+
+			// check values
+			oldUID := secret.UID
+			Expect(oldUID).NotTo(BeEmpty())
+
+			// delete the related config
+			Expect(k8sClient.Delete(context.TODO(), secret))
+
+			var newSecret v1.Secret
+			secretLookupKey := types.NamespacedName{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			Eventually(func() bool {
+				err := k8sClient.Get(context.Background(), secretLookupKey, &newSecret)
+				if err != nil {
+					return false
+				}
+				// new secret should be a new, recreated object with a different UID
+				return newSecret.UID != oldUID
+			}, timeout, interval).Should(BeTrue())
+		}
+	}
+
+	// Checks that secret annotation has been written based on the data
+	checkSecretDataHashAnnotation := func(tc *testCase) {
+		const secretVal = "someValue"
+		fakeProvider.WithGetSecret([]byte(secretVal), nil)
+		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
+			Expect(secret.Annotations[esv1alpha1.AnnotationDataHash]).To(Equal("9d30b95ca81e156f9454b5ef3bfcc6ee"))
+		}
+	}
+
+	// When we amend the created kind=secret, refresh operation should be run again regardless of refresh interval
+	checkSecretDataHashAnnotationChange := func(tc *testCase) {
+		fakeData := map[string][]byte{
+			"targetProperty": []byte("map-foo-value"),
+		}
+		fakeProvider.WithGetSecretMap(fakeData, nil)
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Minute * 10}
+		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
+			oldHash := secret.Annotations[esv1alpha1.AnnotationDataHash]
+			oldResourceVersion := secret.ResourceVersion
+			Expect(oldHash).NotTo(BeEmpty())
+
+			cleanSecret := secret.DeepCopy()
+			secret.Data["new"] = []byte("value")
+			secret.ObjectMeta.Annotations[esv1alpha1.AnnotationDataHash] = "thisiswronghash"
+			Expect(k8sClient.Patch(context.Background(), secret, client.MergeFrom(cleanSecret))).To(Succeed())
+
+			var refreshedSecret v1.Secret
+			secretLookupKey := types.NamespacedName{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			Eventually(func() bool {
+				err := k8sClient.Get(context.Background(), secretLookupKey, &refreshedSecret)
+				if err != nil {
+					return false
+				}
+				// refreshed secret should have a different generation (sign that it was updated), but since
+				// the secret source is the same (not changed), the hash should be reverted to an old value
+				return refreshedSecret.ResourceVersion != oldResourceVersion && refreshedSecret.Annotations[esv1alpha1.AnnotationDataHash] == oldHash
+			}, timeout, interval).Should(BeTrue())
+		}
+	}
+
 	DescribeTable("When reconciling an ExternalSecret",
 	DescribeTable("When reconciling an ExternalSecret",
 		func(tweaks ...testTweaks) {
 		func(tweaks ...testTweaks) {
 			tc := makeDefaultTestcase()
 			tc := makeDefaultTestcase()
@@ -739,9 +880,13 @@ var _ = Describe("ExternalSecret controller", func() {
 				tc.checkSecret(createdES, syncedSecret)
 				tc.checkSecret(createdES, syncedSecret)
 			}
 			}
 		},
 		},
+		Entry("should recreate deleted secret", checkDeletion),
+		Entry("should create proper hash annotation for the external secret", checkSecretDataHashAnnotation),
+		Entry("should refresh when the hash annotation doesn't correspond to secret data", checkSecretDataHashAnnotationChange),
 		Entry("should set the condition eventually", syncLabelsAnnotations),
 		Entry("should set the condition eventually", syncLabelsAnnotations),
+		Entry("should set prometheus counters", checkPrometheusCounters),
 		Entry("should merge with existing secret using creationPolicy=Merge", mergeWithSecret),
 		Entry("should merge with existing secret using creationPolicy=Merge", mergeWithSecret),
-		Entry("should error if sceret doesn't exist when using creationPolicy=Merge", mergeWithSecretErr),
+		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 resolve conflicts with creationPolicy=Merge", mergeWithConflict),
 		Entry("should sync with template", syncWithTemplate),
 		Entry("should sync with template", syncWithTemplate),
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
@@ -766,7 +911,6 @@ var _ = Describe("ExternalSecret refresh logic", func() {
 				},
 				},
 			})).To(BeTrue())
 			})).To(BeTrue())
 		})
 		})
-
 		It("should refresh when labels change", func() {
 		It("should refresh when labels change", func() {
 			es := esv1alpha1.ExternalSecret{
 			es := esv1alpha1.ExternalSecret{
 				ObjectMeta: metav1.ObjectMeta{
 				ObjectMeta: metav1.ObjectMeta{

+ 14 - 1
pkg/utils/utils.go

@@ -14,7 +14,13 @@ limitations under the License.
 
 
 package utils
 package utils
 
 
-import "reflect"
+import (
+
+	// nolint:gosec
+	"crypto/md5"
+	"fmt"
+	"reflect"
+)
 
 
 // MergeByteMap merges map of byte slices.
 // MergeByteMap merges map of byte slices.
 func MergeByteMap(dst, src map[string][]byte) map[string][]byte {
 func MergeByteMap(dst, src map[string][]byte) map[string][]byte {
@@ -35,3 +41,10 @@ func MergeStringMap(dest, src map[string]string) {
 func IsNil(i interface{}) bool {
 func IsNil(i interface{}) bool {
 	return i == nil || reflect.ValueOf(i).IsNil()
 	return i == nil || reflect.ValueOf(i).IsNil()
 }
 }
+
+// ObjectHash calculates md5 sum of the data contained in the secret.
+// nolint:gosec
+func ObjectHash(object interface{}) string {
+	textualVersion := fmt.Sprintf("%+v", object)
+	return fmt.Sprintf("%x", md5.Sum([]byte(textualVersion)))
+}

+ 62 - 0
pkg/utils/utils_test.go

@@ -0,0 +1,62 @@
+/*
+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 utils
+
+import (
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+)
+
+func TestObjectHash(t *testing.T) {
+	tests := []struct {
+		name  string
+		input interface{}
+		want  string
+	}{
+		{
+			name:  "A nil should be still working",
+			input: nil,
+			want:  "60046f14c917c18a9a0f923e191ba0dc",
+		},
+		{
+			name:  "We accept a simple scalar value, i.e. string",
+			input: "hello there",
+			want:  "161bc25962da8fed6d2f59922fb642aa",
+		},
+		{
+			name: "A complex object like a secret is not an issue",
+			input: v1.Secret{Data: map[string][]byte{
+				"xx": []byte("yyy"),
+			}},
+			want: "a9fe13fd43b20829b45f0a93372413dd",
+		},
+		{
+			name: "map also works",
+			input: map[string][]byte{
+				"foo": []byte("value1"),
+				"bar": []byte("value2"),
+			},
+			want: "caa0155759a6a9b3b6ada5a6883ee2bb",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ObjectHash(tt.input); got != tt.want {
+				t.Errorf("ObjectHash() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

+ 1 - 0
tools.go

@@ -1,3 +1,4 @@
+//go:build tools
 // +build tools
 // +build tools
 
 
 package tools
 package tools