WIP: first deletion test now working

Signed-off-by: Gustavo <gusfcarvalho@gmail.com>

pkg/controllers/pushsecret/pushsecret_controller.go

@@ -106,12 +106,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 	switch ps.Spec.DeletionPolicy {
 	case esapi.PushSecretDeletionPolicyDelete:
-		err = r.DeleteSecretFromProviders(ctx, &ps, syncedSecrets)
+		badSyncState, err := r.DeleteSecretFromProviders(ctx, &ps, syncedSecrets)
 		if err != nil {
 			msg := fmt.Sprintf("Failed to Delete Secrets from Provider: %v", err)
 			cond := NewPushSecretCondition(esapi.PushSecretReady, v1.ConditionFalse, esapi.ReasonErrored, msg)
 			ps = SetPushSecretCondition(ps, *cond)
-			r.SetSyncedSecrets(&ps, syncedSecrets)
+			r.SetSyncedSecrets(&ps, badSyncState)
 			r.recorder.Event(&ps, v1.EventTypeWarning, esapi.ReasonErrored, msg)
 			return ctrl.Result{}, err
 		}
@@ -129,7 +129,17 @@ func (r *Reconciler) SetSyncedSecrets(ps *esapi.PushSecret, status esapi.SyncedP
 	ps.Status.SyncedPushSecrets = status
 }
 
-func (r *Reconciler) DeleteSecretFromProviders(ctx context.Context, ps *esapi.PushSecret, newMap esapi.SyncedPushSecretsMap) error {
+func (r *Reconciler) DeleteSecretFromProviders(ctx context.Context, ps *esapi.PushSecret, newMap esapi.SyncedPushSecretsMap) (esapi.SyncedPushSecretsMap, error) {
+	out := newMap.DeepCopy()
+	for k, v := range ps.Status.SyncedPushSecrets {
+		_, ok := out[k]
+		if !ok {
+			out[k] = make(map[string]esapi.PushSecretData)
+		}
+		for kk, vv := range v {
+			out[k][kk] = vv
+		}
+	}
 	for storeName, oldData := range ps.Status.SyncedPushSecrets {
 		storeRef := esapi.PushSecretStoreRef{
 			Name: strings.Split(storeName, "/")[1],
@@ -137,11 +147,11 @@ func (r *Reconciler) DeleteSecretFromProviders(ctx context.Context, ps *esapi.Pu
 		}
 		store, err := r.getSecretStoreFromName(ctx, storeRef, ps.Namespace)
 		if err != nil {
-			return err
+			return out, err
 		}
 		client, err := r.getClientFromStore(ctx, store, ps)
 		if err != nil {
-			return fmt.Errorf("could not get secrets client for store %v: %w", store.GetName(), err)
+			return out, fmt.Errorf("could not get secrets client for store %v: %w", store.GetName(), err)
 		}
 		defer func() { //nolint
 			err := client.Close(ctx)
@@ -153,8 +163,9 @@ func (r *Reconciler) DeleteSecretFromProviders(ctx context.Context, ps *esapi.Pu
 		if !ok {
 			err = r.DeleteAllSecretsFromStore(ctx, client, oldData)
 			if err != nil {
-				return err
+				return out, err
 			}
+			delete(out, storeName)
 			continue
 		}
 		for oldEntry, oldRef := range oldData {
@@ -162,12 +173,13 @@ func (r *Reconciler) DeleteSecretFromProviders(ctx context.Context, ps *esapi.Pu
 			if !ok {
 				err = r.DeleteSecretFromStore(ctx, client, oldRef)
 				if err != nil {
-					return err
+					return out, err
 				}
+				delete(out[storeName], oldRef.Match.RemoteRef.RemoteKey)
 			}
 		}
 	}
-	return nil
+	return out, nil
 }
 
 func (r *Reconciler) DeleteAllSecretsFromStore(ctx context.Context, client v1beta1.SecretsClient, data map[string]esapi.PushSecretData) error {
@@ -198,8 +210,8 @@ func (r *Reconciler) getClientFromStore(ctx context.Context, store v1beta1.Gener
 
 func (r *Reconciler) PushSecretToProviders(ctx context.Context, stores map[esapi.PushSecretStoreRef]v1beta1.GenericStore, ps esapi.PushSecret, secret *v1.Secret) (esapi.SyncedPushSecretsMap, error) {
 	out := esapi.SyncedPushSecretsMap{}
-	for _, store := range stores {
-		storeKey := fmt.Sprintf("%v/%v", store.GetName(), store.GetObjectKind().GroupVersionKind().Kind)
+	for ref, store := range stores {
+		storeKey := fmt.Sprintf("%v/%v", ref.Kind, store.GetName())
 		out[storeKey] = make(map[string]esapi.PushSecretData)
 		client, err := r.getClientFromStore(ctx, store, &ps)
 		if err != nil {

pkg/controllers/pushsecret/pushsecret_controller_test.go

@@ -169,6 +169,9 @@ var _ = Describe("ExternalSecret controller", func() {
 					Name:      PushSecretStore,
 					Namespace: PushSecretNamespace,
 				},
+				TypeMeta: metav1.TypeMeta{
+					Kind: "SecretStore",
+				},
 				Spec: v1beta1.SecretStoreSpec{
 					Provider: &v1beta1.SecretStoreProvider{
 						Fake: &v1beta1.FakeProvider{
@@ -186,15 +189,72 @@ var _ = Describe("ExternalSecret controller", func() {
 			return nil
 		}
 		tc.assert = func(ps *v1alpha1.PushSecret, secret *v1.Secret) bool {
-			secretValue := secret.Data["key"]
-			providerValue := fakeProvider.SetSecretArgs[ps.Spec.Data[0].Match.RemoteRef.RemoteKey].Value
-			expected := v1alpha1.PushSecretStatusCondition{
-				Type:    v1alpha1.PushSecretReady,
-				Status:  v1.ConditionTrue,
-				Reason:  v1alpha1.ReasonSynced,
-				Message: "PushSecret synced successfully",
-			}
-			return bytes.Equal(secretValue, providerValue) && checkCondition(ps.Status, expected)
+			Eventually(func() bool {
+				By("checking if Provider value got updated")
+				secretValue := secret.Data["key"]
+				providerValue, ok := fakeProvider.SetSecretArgs[ps.Spec.Data[0].Match.RemoteRef.RemoteKey]
+				if !ok {
+					return false
+				}
+				got := providerValue.Value
+				return bytes.Equal(got, secretValue)
+			}, time.Second*10, time.Second).Should(BeTrue())
+			return true
+		}
+	}
+	// if target Secret name is not specified it should use the ExternalSecret name.
+	syncAndDeleteSuccessfully := func(tc *testCase) {
+		fakeProvider.SetSecretFn = func() error {
+			return nil
+		}
+		tc.pushsecret = &v1alpha1.PushSecret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      PushSecretName,
+				Namespace: PushSecretNamespace,
+			},
+			Spec: v1alpha1.PushSecretSpec{
+				DeletionPolicy: v1alpha1.PushSecretDeletionPolicyDelete,
+				SecretStoreRefs: []v1alpha1.PushSecretStoreRef{
+					{
+						Name: PushSecretStore,
+						Kind: "SecretStore",
+					},
+				},
+				Selector: v1alpha1.PushSecretSelector{
+					Secret: v1alpha1.PushSecretSecret{
+						Name: SecretName,
+					},
+				},
+				Data: []v1alpha1.PushSecretData{
+					{
+						Match: v1alpha1.PushSecretMatch{
+							SecretKey: "key",
+							RemoteRef: v1alpha1.PushSecretRemoteRef{
+								RemoteKey: "path/to/key",
+							},
+						},
+					},
+				},
+			},
+		}
+		tc.assert = func(ps *v1alpha1.PushSecret, secret *v1.Secret) bool {
+			ps.Spec.Data[0].Match.RemoteRef.RemoteKey = "different-key"
+			updatedPS := &v1alpha1.PushSecret{}
+			Expect(k8sClient.Update(context.Background(), ps, &client.UpdateOptions{})).Should(Succeed())
+			Eventually(func() bool {
+				psKey := types.NamespacedName{Name: PushSecretName, Namespace: PushSecretNamespace}
+				By("checking if Provider value got updated")
+				err := k8sClient.Get(context.Background(), psKey, updatedPS)
+				if err != nil {
+					return false
+				}
+				key, ok := updatedPS.Status.SyncedPushSecrets[fmt.Sprintf("SecretStore/%v", PushSecretStore)]["different-key"]
+				if !ok {
+					return false
+				}
+				return key.Match.SecretKey == "key"
+			}, time.Second*10, time.Second).Should(BeTrue())
+			return true
 		}
 	}
 	// if target Secret name is not specified it should use the ExternalSecret name.
@@ -236,6 +296,9 @@ var _ = Describe("ExternalSecret controller", func() {
 			},
 		}
 		tc.store = &v1beta1.SecretStore{
+			TypeMeta: metav1.TypeMeta{
+				Kind: "SecretStore",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      PushSecretStore,
 				Namespace: PushSecretNamespace,
@@ -268,6 +331,9 @@ var _ = Describe("ExternalSecret controller", func() {
 			return nil
 		}
 		tc.store = &v1beta1.ClusterSecretStore{
+			TypeMeta: metav1.TypeMeta{
+				Kind: "ClusterSecretStore",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name: PushSecretStore,
 			},
@@ -483,6 +549,7 @@ var _ = Describe("ExternalSecret controller", func() {
 			// this must be optional so we can test faulty es configuration
 		},
 		Entry("should sync", syncSuccessfully),
+		Entry("should delete if DeletionPolicy=Delete", syncAndDeleteSuccessfully),
 		Entry("should sync to stores matching labels", syncMatchingLabels),
 		Entry("should sync with ClusterStore", syncWithClusterStore),
 		Entry("should sync with ClusterStore matching labels", syncWithClusterStoreMatchingLabels),

pkg/provider/testing/fake/fake.go

@@ -16,7 +16,6 @@ package fake
 
 import (
 	"context"
-	"fmt"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -38,6 +37,7 @@ type Client struct {
 	GetSecretMapFn  func(context.Context, esv1beta1.ExternalSecretDataRemoteRef) (map[string][]byte, error)
 	GetAllSecretsFn func(context.Context, esv1beta1.ExternalSecretFind) (map[string][]byte, error)
 	SetSecretFn     func() error
+	DeleteSecretFn  func() error
 }
 
 // New returns a fake provider/client.
@@ -55,6 +55,9 @@ func New() *Client {
 		SetSecretFn: func() error {
 			return nil
 		},
+		DeleteSecretFn: func() error {
+			return nil
+		},
 		SetSecretArgs: map[string]SetSecretCallArgs{},
 	}
 
@@ -85,7 +88,7 @@ func (v *Client) SetSecret(ctx context.Context, value []byte, remoteRef esv1beta
 }
 
 func (v *Client) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemoteRef) error {
-	return fmt.Errorf("not implemented")
+	return v.DeleteSecretFn()
 }
 
 // GetSecret implements the provider.Provider interface.