Adds SecretManager DeleteSecret

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

pkg/controllers/pushsecret/pushsecret_controller.go

@@ -82,35 +82,40 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			log.Error(err, errPatchStatus)
 		}
 	}()
-	// finalizer logic
-	if ps.ObjectMeta.DeletionTimestamp.IsZero() {
-		if !controllerutil.ContainsFinalizer(&ps, pushSecretFinalizer) {
-			controllerutil.AddFinalizer(&ps, pushSecretFinalizer)
-			err := r.Client.Update(ctx, &ps, &client.UpdateOptions{})
-			if err != nil {
-				return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
-			}
-			return ctrl.Result{}, nil
-		}
-	} else {
-		if controllerutil.ContainsFinalizer(&ps, pushSecretFinalizer) {
-			// trigger a cleanup with no Synced Map
-			badState, err := r.DeleteSecretFromProviders(ctx, &ps, esapi.SyncedPushSecretsMap{})
-			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, badState)
-				r.recorder.Event(&ps, v1.EventTypeWarning, esapi.ReasonErrored, msg)
-				return ctrl.Result{}, err
+	switch ps.Spec.DeletionPolicy {
+	case esapi.PushSecretDeletionPolicyDelete:
+		// finalizer logic. Only added if we should delete the secrets
+		if ps.ObjectMeta.DeletionTimestamp.IsZero() {
+			if !controllerutil.ContainsFinalizer(&ps, pushSecretFinalizer) {
+				controllerutil.AddFinalizer(&ps, pushSecretFinalizer)
+				err := r.Client.Update(ctx, &ps, &client.UpdateOptions{})
+				if err != nil {
+					return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
+				}
+				return ctrl.Result{}, nil
 			}
-			controllerutil.RemoveFinalizer(&ps, pushSecretFinalizer)
-			err = r.Client.Update(ctx, &ps, &client.UpdateOptions{})
-			if err != nil {
-				return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
+		} else {
+			if controllerutil.ContainsFinalizer(&ps, pushSecretFinalizer) {
+				// trigger a cleanup with no Synced Map
+				badState, err := r.DeleteSecretFromProviders(ctx, &ps, esapi.SyncedPushSecretsMap{})
+				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, badState)
+					r.recorder.Event(&ps, v1.EventTypeWarning, esapi.ReasonErrored, msg)
+					return ctrl.Result{}, err
+				}
+				controllerutil.RemoveFinalizer(&ps, pushSecretFinalizer)
+				err = r.Client.Update(ctx, &ps, &client.UpdateOptions{})
+				if err != nil {
+					return ctrl.Result{}, fmt.Errorf("could not update finalizers: %w", err)
+				}
+				return ctrl.Result{}, nil
 			}
-			return ctrl.Result{}, nil
 		}
+	case esapi.PushSecretDeletionPolicyNone:
+	default:
 	}
 
 	secret, err := r.GetSecret(ctx, ps)

pkg/controllers/pushsecret/pushsecret_controller_test.go

@@ -107,7 +107,7 @@ var _ = Describe("ExternalSecret controller", func() {
 			},
 		})
 		// give a time for reconciler to remove finalizers before removing SecretStores
-		// TODO: Secret Stores should have finalizers bound to External-Secrets and PushSecrets
+		// 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{

pkg/provider/aws/secretsmanager/fake/fake.go

@@ -31,12 +31,14 @@ type Client struct {
 	GetSecretValueWithContextFn GetSecretValueWithContextFn
 	PutSecretValueWithContextFn PutSecretValueWithContextFn
 	DescribeSecretWithContextFn DescribeSecretWithContextFn
+	DeleteSecretWithContextFn   DeleteSecretWithContextFn
 }
 
 type CreateSecretWithContextFn func(aws.Context, *awssm.CreateSecretInput, ...request.Option) (*awssm.CreateSecretOutput, error)
 type GetSecretValueWithContextFn func(aws.Context, *awssm.GetSecretValueInput, ...request.Option) (*awssm.GetSecretValueOutput, error)
 type PutSecretValueWithContextFn func(aws.Context, *awssm.PutSecretValueInput, ...request.Option) (*awssm.PutSecretValueOutput, error)
 type DescribeSecretWithContextFn func(aws.Context, *awssm.DescribeSecretInput, ...request.Option) (*awssm.DescribeSecretOutput, error)
+type DeleteSecretWithContextFn func(ctx aws.Context, input *awssm.DeleteSecretInput, opts ...request.Option) (*awssm.DeleteSecretOutput, error)
 
 func (sm Client) CreateSecretWithContext(ctx aws.Context, input *awssm.CreateSecretInput, options ...request.Option) (*awssm.CreateSecretOutput, error) {
 	return sm.CreateSecretWithContextFn(ctx, input, options...)
@@ -47,6 +49,15 @@ func NewCreateSecretWithContextFn(output *awssm.CreateSecretOutput, err error) C
 		return output, err
 	}
 }
+func (sm Client) DeleteSecretWithContext(ctx aws.Context, input *awssm.DeleteSecretInput, opts ...request.Option) (*awssm.DeleteSecretOutput, error) {
+	return sm.DeleteSecretWithContextFn(ctx, input, opts...)
+}
+
+func NewDeleteSecretWithContextFn(output *awssm.DeleteSecretOutput, err error) DeleteSecretWithContextFn {
+	return func(ctx aws.Context, input *awssm.DeleteSecretInput, opts ...request.Option) (output *awssm.DeleteSecretOutput, err error) {
+		return output, err
+	}
+}
 
 func (sm Client) GetSecretValueWithContext(ctx aws.Context, input *awssm.GetSecretValueInput, options ...request.Option) (*awssm.GetSecretValueOutput, error) {
 	return sm.GetSecretValueWithContextFn(ctx, input, options...)

pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -15,11 +15,11 @@ limitations under the License.
 package secretsmanager
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"reflect"
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -55,10 +55,13 @@ type SMInterface interface {
 	GetSecretValueWithContext(aws.Context, *awssm.GetSecretValueInput, ...request.Option) (*awssm.GetSecretValueOutput, error)
 	PutSecretValueWithContext(aws.Context, *awssm.PutSecretValueInput, ...request.Option) (*awssm.PutSecretValueOutput, error)
 	DescribeSecretWithContext(aws.Context, *awssm.DescribeSecretInput, ...request.Option) (*awssm.DescribeSecretOutput, error)
+	DeleteSecretWithContext(ctx aws.Context, input *awssm.DeleteSecretInput, opts ...request.Option) (*awssm.DeleteSecretOutput, error)
 }
 
 const (
 	errUnexpectedFindOperator = "unexpected find operator"
+	managedBy                 = "managed-by"
+	externalSecrets           = "external-secrets"
 )
 
 var log = ctrl.Log.WithName("provider").WithName("aws").WithName("secretsmanager")
@@ -112,13 +115,42 @@ func (sm *SecretsManager) fetch(_ context.Context, ref esv1beta1.ExternalSecretD
 }
 
 func (sm *SecretsManager) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemoteRef) error {
-	return fmt.Errorf("not implemented")
+	secretName := remoteRef.GetRemoteKey()
+	secretValue := awssm.GetSecretValueInput{
+		SecretId: &secretName,
+	}
+	secretInput := awssm.DescribeSecretInput{
+		SecretId: &secretName,
+	}
+	awsSecret, err := sm.client.GetSecretValueWithContext(ctx, &secretValue)
+	var aerr awserr.Error
+	if err != nil {
+		if ok := errors.As(err, &aerr); !ok {
+			return err
+		}
+		if aerr.Code() == awssm.ErrCodeResourceNotFoundException {
+			return nil
+		}
+		return err
+	}
+	data, err := sm.client.DescribeSecretWithContext(ctx, &secretInput)
+	if err != nil {
+		return err
+	}
+	if !isManagedByESO(data) {
+		return nil
+	}
+	deleteInput := &awssm.DeleteSecretInput{
+		SecretId: awsSecret.ARN,
+	}
+	_, err = sm.client.DeleteSecretWithContext(ctx, deleteInput)
+	return err
 }
 
 func (sm *SecretsManager) SetSecret(ctx context.Context, value []byte, remoteRef esv1beta1.PushRemoteRef) error {
 	secretName := remoteRef.GetRemoteKey()
-	managedBy := "managed-by"
-	externalSecrets := "external-secrets"
+	managedBy := managedBy
+	externalSecrets := externalSecrets
 	externalSecretsTag := []*awssm.Tag{
 		{
 			Key:   &managedBy,
@@ -140,50 +172,44 @@ func (sm *SecretsManager) SetSecret(ctx context.Context, value []byte, remoteRef
 	}
 
 	awsSecret, err := sm.client.GetSecretValueWithContext(ctx, &secretValue)
-
-	if err == nil {
-		data, err := sm.client.DescribeSecretWithContext(ctx, &secretInput)
-		if err != nil {
-			return err
-		}
-
-		for _, tag := range data.Tags {
-			if *tag.Key == managedBy && *tag.Value == externalSecrets {
-				goto TAGGED
-			} else {
-				return fmt.Errorf("secret not managed by external-secrets")
-			}
-		}
-	}
-TAGGED:
-	if awsSecret != nil && reflect.DeepEqual(awsSecret.SecretBinary, secretRequest.SecretBinary) {
-		return nil
-	} else if awsSecret.ARN != nil {
-		input := &awssm.PutSecretValueInput{
-			SecretId:     awsSecret.ARN,
-			SecretBinary: value,
-		}
-		_, err := sm.client.PutSecretValueWithContext(ctx, input)
-		if err != nil {
+	var aerr awserr.Error
+	if err != nil {
+		if ok := errors.As(err, &aerr); !ok {
 			return err
 		}
-	}
-
-	var aerr awserr.Error
-	if ok := errors.As(err, &aerr); ok {
-		if aerr.Code() != awssm.ErrCodeResourceNotFoundException {
+		if aerr.Code() == awssm.ErrCodeResourceNotFoundException {
+			_, err = sm.client.CreateSecretWithContext(ctx, &secretRequest)
 			return err
 		}
-	} else if err != nil {
 		return err
 	}
-
-	_, err = sm.client.CreateSecretWithContext(ctx, &secretRequest)
-
+	data, err := sm.client.DescribeSecretWithContext(ctx, &secretInput)
 	if err != nil {
 		return err
 	}
-	return nil
+	if !isManagedByESO(data) {
+		return fmt.Errorf("secret not managed by external-secrets")
+	}
+	if awsSecret != nil && bytes.Equal(awsSecret.SecretBinary, value) {
+		return nil
+	}
+	input := &awssm.PutSecretValueInput{
+		SecretId:     awsSecret.ARN,
+		SecretBinary: value,
+	}
+	_, err = sm.client.PutSecretValueWithContext(ctx, input)
+	return err
+}
+
+func isManagedByESO(data *awssm.DescribeSecretOutput) bool {
+	managedBy := managedBy
+	externalSecrets := externalSecrets
+	for _, tag := range data.Tags {
+		if *tag.Key == managedBy && *tag.Value == externalSecrets {
+			return true
+		}
+	}
+	return false
 }
 
 // GetAllSecrets syncs multiple secrets from aws provider into a single Kubernetes Secret.

pkg/provider/aws/secretsmanager/secretsmanager_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	awssm "github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/google/go-cmp/cmp"
@@ -329,10 +330,10 @@ func (f fakeRef) GetRemoteKey() string {
 }
 
 func TestSetSecret(t *testing.T) {
-	managedBy := "managed-by"
+	managedBy := managedBy
 	notManagedBy := "not-managed-by"
 	secretValue := []byte("fake-value")
-	externalSecrets := "external-secrets"
+	externalSecrets := externalSecrets
 	noPermission := errors.New("no permission")
 	arn := "arn:aws:secretsmanager:us-east-1:702902267788:secret:foo-bar5-Robbgh"
 
@@ -530,6 +531,152 @@ func TestSetSecret(t *testing.T) {
 	}
 }
 
+func TestDeleteSecret(t *testing.T) {
+	fakeClient := fakesm.Client{}
+	managed := managedBy
+	manager := externalSecrets
+	secretTag := awssm.Tag{
+		Key:   &managed,
+		Value: &manager,
+	}
+	type args struct {
+		client               fakesm.Client
+		getSecretOutput      *awssm.GetSecretValueOutput
+		describeSecretOutput *awssm.DescribeSecretOutput
+		deleteSecretOutput   *awssm.DeleteSecretOutput
+		getSecretErr         error
+		describeSecretErr    error
+		deleteSecretErr      error
+	}
+	type want struct {
+		err error
+	}
+	type testCase struct {
+		args   args
+		want   want
+		reason string
+	}
+	tests := map[string]testCase{
+		"Deletes Successfully": {
+			args: args{
+
+				client:          fakeClient,
+				getSecretOutput: &awssm.GetSecretValueOutput{},
+				describeSecretOutput: &awssm.DescribeSecretOutput{
+					Tags: []*awssm.Tag{&secretTag},
+				},
+				deleteSecretOutput: &awssm.DeleteSecretOutput{},
+				getSecretErr:       nil,
+				describeSecretErr:  nil,
+				deleteSecretErr:    nil,
+			},
+			want: want{
+				err: nil,
+			},
+			reason: "",
+		},
+		"Not Managed by ESO": {
+			args: args{
+
+				client:          fakeClient,
+				getSecretOutput: &awssm.GetSecretValueOutput{},
+				describeSecretOutput: &awssm.DescribeSecretOutput{
+					Tags: []*awssm.Tag{},
+				},
+				deleteSecretOutput: &awssm.DeleteSecretOutput{},
+				getSecretErr:       nil,
+				describeSecretErr:  nil,
+				deleteSecretErr:    nil,
+			},
+			want: want{
+				err: nil,
+			},
+			reason: "",
+		},
+		"Failed to get Tags": {
+			args: args{
+
+				client:               fakeClient,
+				getSecretOutput:      &awssm.GetSecretValueOutput{},
+				describeSecretOutput: nil,
+				deleteSecretOutput:   nil,
+				getSecretErr:         nil,
+				describeSecretErr:    errors.New("failed to get tags"),
+				deleteSecretErr:      nil,
+			},
+			want: want{
+				err: errors.New("failed to get tags"),
+			},
+			reason: "",
+		},
+		"Secret Not Found": {
+			args: args{
+				client:               fakeClient,
+				getSecretOutput:      nil,
+				describeSecretOutput: nil,
+				deleteSecretOutput:   nil,
+				getSecretErr:         awserr.New(awssm.ErrCodeResourceNotFoundException, "not here, sorry dude", nil),
+				describeSecretErr:    nil,
+				deleteSecretErr:      nil,
+			},
+			want: want{
+				err: nil,
+			},
+		},
+		"Not expected AWS error": {
+			args: args{
+				client:               fakeClient,
+				getSecretOutput:      nil,
+				describeSecretOutput: nil,
+				deleteSecretOutput:   nil,
+				getSecretErr:         awserr.New(awssm.ErrCodeEncryptionFailure, "aws unavailable", nil),
+				describeSecretErr:    nil,
+				deleteSecretErr:      nil,
+			},
+			want: want{
+				err: errors.New("aws unavailable"),
+			},
+		},
+		"unexpected error": {
+			args: args{
+				client:               fakeClient,
+				getSecretOutput:      nil,
+				describeSecretOutput: nil,
+				deleteSecretOutput:   nil,
+				getSecretErr:         errors.New("timeout"),
+				describeSecretErr:    nil,
+				deleteSecretErr:      nil,
+			},
+			want: want{
+				err: errors.New("timeout"),
+			},
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			ref := fakeRef{key: "fake-key"}
+			sm := SecretsManager{
+				client: &tc.args.client,
+			}
+			tc.args.client.GetSecretValueWithContextFn = fakesm.NewGetSecretValueWithContextFn(tc.args.getSecretOutput, tc.args.getSecretErr)
+			tc.args.client.DescribeSecretWithContextFn = fakesm.NewDescribeSecretWithContextFn(tc.args.describeSecretOutput, tc.args.describeSecretErr)
+			tc.args.client.DeleteSecretWithContextFn = fakesm.NewDeleteSecretWithContextFn(tc.args.deleteSecretOutput, tc.args.deleteSecretErr)
+			err := sm.DeleteSecret(context.TODO(), ref)
+
+			// Error nil XOR tc.want.err nil
+			if ((err == nil) || (tc.want.err == nil)) && !((err == nil) && (tc.want.err == nil)) {
+				t.Errorf("\nTesting SetSecret:\nName: %v\nReason: %v\nWant error: %v\nGot error: %v", name, tc.reason, tc.want.err, err)
+			}
+
+			// if errors are the same type but their contents do not match
+			if err != nil && tc.want.err != nil {
+				if !strings.Contains(err.Error(), tc.want.err.Error()) {
+					t.Errorf("\nTesting SetSecret:\nName: %v\nReason: %v\nWant error: %v\nGot error got nil", name, tc.reason, tc.want.err)
+				}
+			}
+		})
+	}
+}
 func makeValidSecretStore() *esv1beta1.SecretStore {
 	return &esv1beta1.SecretStore{
 		ObjectMeta: metav1.ObjectMeta{