feat(aws): secretsmanager to update/patch/delete tags (#4984)

* feat(aws): secretsmanager to update/patch/delete tags

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

* feat(aws): secretsmanager to update/patch/delete tags

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

* feat(aws): secretsmanager to update/patch/delete tags

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

---------

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

pkg/constants/constants.go

@@ -24,6 +24,8 @@ const (
 	CallAWSSMPutSecretValue      = "PutSecretValue"
 	CallAWSSMListSecrets         = "ListSecrets"
 	CallAWSSMBatchGetSecretValue = "BatchGetSecretValue"
+	CallAWSSMUntagResource       = "UntagResource"
+	CallAWSSMTagResource         = "TagResource"
 
 	ProviderAWSPS                = "AWS/ParameterStore"
 	CallAWSPSGetParameter        = "GetParameter"

pkg/provider/aws/parameterstore/parameterstore.go

@@ -685,7 +685,9 @@ func computeTagsToUpdate(tags, metaTags map[string]string) ([]ssmTypes.Tag, bool
 	modified := false
 	for k, v := range metaTags {
 		if _, exists := tags[k]; !exists || tags[k] != v {
-			modified = true
+			if k != managedBy {
+				modified = true
+			}
 		}
 		result = append(result, ssmTypes.Tag{
 			Key:   ptr.To(k),

pkg/provider/aws/parameterstore/parameterstore_test.go

@@ -1168,7 +1168,7 @@ func TestConstructMetadataWithDefaults(t *testing.T) {
 				"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
 				"kind": "PushSecretMetadata",
 				"spec": {
- 					"description": "adding managed-by tag explicitly",
+					"description": "adding managed-by tag explicitly",
 					"tags": {
 						"managed-by": "external-secrets",
 						"customKey": "customValue"
@@ -1230,6 +1230,24 @@ func TestComputeTagsToUpdate(t *testing.T) {
 			modified: false,
 		},
 		{
+			name: "No tags to update as managed-by tag is ignored",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			metaTags: map[string]string{
+				"key1":    "value1",
+				"key2":    "value2",
+				managedBy: externalSecrets,
+			},
+			expected: []ssmtypes.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("value1")},
+				{Key: ptr.To("key2"), Value: ptr.To("value2")},
+				{Key: ptr.To(managedBy), Value: ptr.To(externalSecrets)},
+			},
+			modified: false,
+		},
+		{
 			name: "Add new tag",
 			tags: map[string]string{
 				"key1": "value1",

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

@@ -38,6 +38,8 @@ type Client struct {
 	DeleteSecretFn        DeleteSecretFn
 	ListSecretsFn         ListSecretsFn
 	BatchGetSecretValueFn BatchGetSecretValueFn
+	TagResourceFn         TagResourceFn
+	UntagResourceFn       UntagResourceFn
 }
 type CreateSecretFn func(context.Context, *awssm.CreateSecretInput, ...func(*awssm.Options)) (*awssm.CreateSecretOutput, error)
 type GetSecretValueFn func(context.Context, *awssm.GetSecretValueInput, ...func(*awssm.Options)) (*awssm.GetSecretValueOutput, error)
@@ -47,7 +49,10 @@ type DeleteSecretFn func(context.Context, *awssm.DeleteSecretInput, ...func(*aws
 type ListSecretsFn func(context.Context, *awssm.ListSecretsInput, ...func(*awssm.Options)) (*awssm.ListSecretsOutput, error)
 type BatchGetSecretValueFn func(context.Context, *awssm.BatchGetSecretValueInput, ...func(*awssm.Options)) (*awssm.BatchGetSecretValueOutput, error)
 
-func (sm Client) CreateSecret(ctx context.Context, input *awssm.CreateSecretInput, options ...func(*awssm.Options)) (*awssm.CreateSecretOutput, error) {
+type TagResourceFn func(context.Context, *awssm.TagResourceInput, ...func(*awssm.Options)) (*awssm.TagResourceOutput, error)
+type UntagResourceFn func(context.Context, *awssm.UntagResourceInput, ...func(*awssm.Options)) (*awssm.UntagResourceOutput, error)
+
+func (sm *Client) CreateSecret(ctx context.Context, input *awssm.CreateSecretInput, options ...func(*awssm.Options)) (*awssm.CreateSecretOutput, error) {
 	return sm.CreateSecretFn(ctx, input, options...)
 }
 
@@ -66,7 +71,7 @@ func NewCreateSecretFn(output *awssm.CreateSecretOutput, err error, expectedSecr
 	}
 }
 
-func (sm Client) DeleteSecret(ctx context.Context, input *awssm.DeleteSecretInput, opts ...func(*awssm.Options)) (*awssm.DeleteSecretOutput, error) {
+func (sm *Client) DeleteSecret(ctx context.Context, input *awssm.DeleteSecretInput, opts ...func(*awssm.Options)) (*awssm.DeleteSecretOutput, error) {
 	return sm.DeleteSecretFn(ctx, input, opts...)
 }
 
@@ -85,7 +90,7 @@ func NewGetSecretValueFn(output *awssm.GetSecretValueOutput, err error) GetSecre
 	}
 }
 
-func (sm Client) PutSecretValue(ctx context.Context, input *awssm.PutSecretValueInput, options ...func(*awssm.Options)) (*awssm.PutSecretValueOutput, error) {
+func (sm *Client) PutSecretValue(ctx context.Context, input *awssm.PutSecretValueInput, options ...func(*awssm.Options)) (*awssm.PutSecretValueOutput, error) {
 	return sm.PutSecretValueFn(ctx, input, options...)
 }
 
@@ -133,7 +138,7 @@ func NewPutSecretValueFn(output *awssm.PutSecretValueOutput, err error, expected
 	}
 }
 
-func (sm Client) DescribeSecret(ctx context.Context, input *awssm.DescribeSecretInput, options ...func(*awssm.Options)) (*awssm.DescribeSecretOutput, error) {
+func (sm *Client) DescribeSecret(ctx context.Context, input *awssm.DescribeSecretInput, options ...func(*awssm.Options)) (*awssm.DescribeSecretOutput, error) {
 	return sm.DescribeSecretFn(ctx, input, options...)
 }
 
@@ -189,3 +194,29 @@ func (sm *Client) WithValue(in *awssm.GetSecretValueInput, val *awssm.GetSecretV
 		return val, err
 	}
 }
+
+func (sm *Client) TagResource(ctx context.Context, params *awssm.TagResourceInput, optFns ...func(*awssm.Options)) (*awssm.TagResourceOutput, error) {
+	return sm.TagResourceFn(ctx, params, optFns...)
+}
+
+func NewTagResourceFn(output *awssm.TagResourceOutput, err error, aFunc ...func(input *awssm.TagResourceInput)) TagResourceFn {
+	return func(ctx context.Context, params *awssm.TagResourceInput, optFns ...func(*awssm.Options)) (*awssm.TagResourceOutput, error) {
+		for _, f := range aFunc {
+			f(params)
+		}
+		return output, err
+	}
+}
+
+func (sm *Client) UntagResource(ctx context.Context, params *awssm.UntagResourceInput, optFuncs ...func(*awssm.Options)) (*awssm.UntagResourceOutput, error) {
+	return sm.UntagResourceFn(ctx, params, optFuncs...)
+}
+
+func NewUntagResourceFn(output *awssm.UntagResourceOutput, err error, aFunc ...func(input *awssm.UntagResourceInput)) UntagResourceFn {
+	return func(ctx context.Context, params *awssm.UntagResourceInput, optFuncs ...func(*awssm.Options)) (*awssm.UntagResourceOutput, error) {
+		for _, f := range aFunc {
+			f(params)
+		}
+		return output, err
+	}
+}

pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -83,6 +83,8 @@ type SMInterface interface {
 	PutSecretValue(ctx context.Context, params *awssm.PutSecretValueInput, optFuncs ...func(*awssm.Options)) (*awssm.PutSecretValueOutput, error)
 	DescribeSecret(ctx context.Context, params *awssm.DescribeSecretInput, optFuncs ...func(*awssm.Options)) (*awssm.DescribeSecretOutput, error)
 	DeleteSecret(ctx context.Context, params *awssm.DeleteSecretInput, optFuncs ...func(*awssm.Options)) (*awssm.DeleteSecretOutput, error)
+	TagResource(ctx context.Context, params *awssm.TagResourceInput, optFuncs ...func(*awssm.Options)) (*awssm.TagResourceOutput, error)
+	UntagResource(ctx context.Context, params *awssm.UntagResourceInput, optFuncs ...func(*awssm.Options)) (*awssm.UntagResourceOutput, error)
 }
 
 const (
@@ -95,7 +97,7 @@ const (
 var log = ctrl.Log.WithName("provider").WithName("aws").WithName("secretsmanager")
 
 // New creates a new SecretsManager client.
-func New(ctx context.Context, cfg *aws.Config, secretsManagerCfg *esv1.SecretsManager, prefix string, referentAuth bool) (*SecretsManager, error) {
+func New(_ context.Context, cfg *aws.Config, secretsManagerCfg *esv1.SecretsManager, prefix string, referentAuth bool) (*SecretsManager, error) {
 	return &SecretsManager{
 		cfg: cfg,
 		client: awssm.NewFromConfig(*cfg, func(o *awssm.Options) {
@@ -513,12 +515,7 @@ func (sm *SecretsManager) createSecretWithContext(ctx context.Context, secretNam
 		return fmt.Errorf("failed to parse push secret metadata: %w", err)
 	}
 
-	tags := []types.Tag{
-		{
-			Key:   utilpointer.To(managedBy),
-			Value: utilpointer.To(externalSecrets),
-		},
-	}
+	tags := make([]types.Tag, 0)
 
 	for k, v := range mdata.Spec.Tags {
 		tags = append(tags, types.Tag{
@@ -579,8 +576,47 @@ func (sm *SecretsManager) putSecretValueWithContext(ctx context.Context, secretI
 
 	_, err = sm.client.PutSecretValue(ctx, input)
 	metrics.ObserveAPICall(constants.ProviderAWSSM, constants.CallAWSSMPutSecretValue, err)
+	if err != nil {
+		return err
+	}
 
-	return err
+	currentTags := make(map[string]string, len(data.Tags))
+	for _, tag := range data.Tags {
+		currentTags[*tag.Key] = *tag.Value
+	}
+	return sm.patchTags(ctx, psd.GetMetadata(), awsSecret.ARN, currentTags)
+}
+
+func (sm *SecretsManager) patchTags(ctx context.Context, metadata *apiextensionsv1.JSON, secretId *string, tags map[string]string) error {
+	meta, err := sm.constructMetadataWithDefaults(metadata)
+	if err != nil {
+		return err
+	}
+
+	tagKeysToRemove := util.FindTagKeysToRemove(tags, meta.Spec.Tags)
+	if len(tagKeysToRemove) > 0 {
+		_, err = sm.client.UntagResource(ctx, &awssm.UntagResourceInput{
+			SecretId: secretId,
+			TagKeys:  tagKeysToRemove,
+		})
+		metrics.ObserveAPICall(constants.ProviderAWSSM, constants.CallAWSSMUntagResource, err)
+		if err != nil {
+			return err
+		}
+	}
+
+	tagsToUpdate, isModified := computeTagsToUpdate(tags, meta.Spec.Tags)
+	if isModified {
+		_, err = sm.client.TagResource(ctx, &awssm.TagResourceInput{
+			SecretId: secretId,
+			Tags:     tagsToUpdate,
+		})
+		metrics.ObserveAPICall(constants.ProviderAWSSM, constants.CallAWSSMTagResource, err)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func (sm *SecretsManager) fetchWithBatch(ctx context.Context, filters []types.Filter, matcher *find.Matcher) (map[string][]byte, error) {
@@ -711,7 +747,29 @@ func (sm *SecretsManager) constructMetadataWithDefaults(data *apiextensionsv1.JS
 		if _, exists := meta.Spec.Tags[managedBy]; exists {
 			return nil, fmt.Errorf("error parsing tags in metadata: Cannot specify a '%s' tag", managedBy)
 		}
+	} else {
+		meta.Spec.Tags = make(map[string]string)
 	}
+	meta.Spec.Tags[managedBy] = externalSecrets
 
 	return meta, nil
 }
+
+// computeTagsToUpdate compares the current tags with the desired metaTags and returns a slice of ssmTypes.Tag
+// that should be set on the resource. It also returns a boolean indicating if any tag was added or modified.
+func computeTagsToUpdate(tags, metaTags map[string]string) ([]types.Tag, bool) {
+	result := make([]types.Tag, 0, len(metaTags))
+	modified := false
+	for k, v := range metaTags {
+		if _, exists := tags[k]; !exists || tags[k] != v {
+			if k != managedBy {
+				modified = true
+			}
+		}
+		result = append(result, types.Tag{
+			Key:   utilpointer.To(k),
+			Value: utilpointer.To(v),
+		})
+	}
+	return result, modified
+}

pkg/provider/aws/secretsmanager/secretsmanager_test.go

@@ -16,6 +16,7 @@ package secretsmanager
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"reflect"
@@ -27,8 +28,10 @@ import (
 	"github.com/aws/aws-sdk-go-v2/credentials"
 	awssm "github.com/aws/aws-sdk-go-v2/service/secretsmanager"
 	"github.com/aws/aws-sdk-go-v2/service/secretsmanager/types"
+	"github.com/external-secrets/external-secrets/pkg/utils/metadata"
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -527,6 +530,8 @@ func TestSetSecret(t *testing.T) {
 					CreateSecretFn:   fakesm.NewCreateSecretFn(secretOutput, nil),
 					PutSecretValueFn: fakesm.NewPutSecretValueFn(putSecretOutput, nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
+					TagResourceFn:    fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn:  fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithoutProperty,
 			},
@@ -543,6 +548,8 @@ func TestSetSecret(t *testing.T) {
 					CreateSecretFn:   fakesm.NewCreateSecretFn(secretOutput, nil),
 					PutSecretValueFn: fakesm.NewPutSecretValueFn(putSecretOutput, nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
+					TagResourceFn:    fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn:  fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithoutSecretKey,
 			},
@@ -559,6 +566,8 @@ func TestSetSecret(t *testing.T) {
 					CreateSecretFn:   fakesm.NewCreateSecretFn(secretOutput, nil),
 					PutSecretValueFn: fakesm.NewPutSecretValueFn(putSecretOutput, nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
+					TagResourceFn:    fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn:  fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithMetadata,
 			},
@@ -599,6 +608,8 @@ func TestSetSecret(t *testing.T) {
 					CreateSecretFn:   fakesm.NewCreateSecretFn(secretOutput, nil),
 					PutSecretValueFn: fakesm.NewPutSecretValueFn(putSecretOutput, nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
+					TagResourceFn:    fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn:  fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: "", Metadata: &apiextensionsv1.JSON{
 					Raw: []byte(`{
@@ -652,6 +663,8 @@ func TestSetSecret(t *testing.T) {
 						SecretBinary: []byte(`{"fake-property":"fake-value","other-fake-property":"fake-value"}`),
 						Version:      &defaultUpdatedVersion,
 					}),
+					TagResourceFn:   fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithProperty,
 			},
@@ -673,6 +686,8 @@ func TestSetSecret(t *testing.T) {
 						SecretBinary: []byte(`{"fake-property":"fake-value","other-fake-property":"fake-value"}`),
 						Version:      &randomUUIDVersionIncremented,
 					}),
+					TagResourceFn:   fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithProperty,
 			},
@@ -715,6 +730,8 @@ func TestSetSecret(t *testing.T) {
 						SecretBinary: []byte(`{"fake-property":"fake-value","other-fake-property":"fake-value"}`),
 						Version:      &initialVersion,
 					}),
+					TagResourceFn:   fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithProperty,
 			},
@@ -733,6 +750,8 @@ func TestSetSecret(t *testing.T) {
 						SecretBinary: []byte(`{"fake-property":"fake-value","other-fake-property":"fake-value"}`),
 						Version:      &defaultUpdatedVersion,
 					}),
+					TagResourceFn:   fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithProperty,
 			},
@@ -751,6 +770,8 @@ func TestSetSecret(t *testing.T) {
 						SecretBinary: []byte(`{"fake-property":{"fake-property":"fake-value","other-fake-property":"fake-value"}}`),
 						Version:      &defaultUpdatedVersion,
 					}),
+					TagResourceFn:   fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: "fake-property.other-fake-property"},
 			},
@@ -821,6 +842,8 @@ func TestSetSecret(t *testing.T) {
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutput, nil),
 					PutSecretValueFn: fakesm.NewPutSecretValueFn(nil, noPermission),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
+					TagResourceFn:    fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn:  fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
 				pushSecretData: pushSecretDataWithoutProperty,
 			},
@@ -869,26 +892,45 @@ func TestSetSecret(t *testing.T) {
 				err: errors.New("secret not managed by external-secrets"),
 			},
 		},
-		"SetSecretWithPrefix": {
-			reason: "secret key is properly prefixed when creating a new secret",
+		"PatchSecretTags": {
+			reason: "secret key is configured with tags to remove and add",
 			args: args{
-
 				store: &esv1.AWSProvider{
 					Service: esv1.AWSServiceSecretsManager,
 					Region:  "eu-west-2",
-					Prefix:  "prefix-",
 				},
 				client: fakesm.Client{
-					GetSecretValueFn: fakesm.NewGetSecretValueFn(blankSecretValueOutput, &getSecretCorrectErr),
-					CreateSecretFn: func(ctx context.Context, input *awssm.CreateSecretInput, opts ...func(*awssm.Options)) (*awssm.CreateSecretOutput, error) {
-						// Verify that the input name has the prefix applied
-						if *input.Name != "prefix-"+fakeKey {
-							return nil, fmt.Errorf("expected secret name to be prefixed with 'prefix-', got %s", *input.Name)
-						}
-						return secretOutput, nil
-					},
+					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutputFrom(params{s: `{"fake-property":{"fake-property":"fake-value"}}`}), nil),
+					DescribeSecretFn: fakesm.NewDescribeSecretFn(&awssm.DescribeSecretOutput{
+						ARN: &arn,
+						Tags: []types.Tag{
+							{Key: &managedBy, Value: &externalSecrets},
+							{Key: ptr.To("team"), Value: ptr.To("paradox")},
+						},
+					}, nil),
+					PutSecretValueFn: fakesm.NewPutSecretValueFn(putSecretOutput, nil),
+					TagResourceFn: fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil, func(input *awssm.TagResourceInput) {
+						assert.Len(t, input.Tags, 2)
+						assert.Contains(t, input.Tags, types.Tag{Key: &managedBy, Value: &externalSecrets})
+						assert.Contains(t, input.Tags, types.Tag{Key: ptr.To("env"), Value: ptr.To("sandbox")})
+					}),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil, func(input *awssm.UntagResourceInput) {
+						assert.Len(t, input.TagKeys, 1)
+						assert.Equal(t, []string{"team"}, input.TagKeys)
+						assert.NotContains(t, input.TagKeys, managedBy)
+					}),
 				},
-				pushSecretData: pushSecretDataWithoutProperty,
+				pushSecretData: fake.PushSecretData{SecretKey: secretKey, RemoteKey: fakeKey, Property: "", Metadata: &apiextensionsv1.JSON{
+					Raw: []byte(`{
+					"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+					"kind": "PushSecretMetadata",
+					"spec": {
+						"secretPushFormat": "string",
+						"tags": {
+							"env": "sandbox"
+						}
+					}
+				}`)}},
 			},
 			want: want{
 				err: nil,
@@ -1591,6 +1633,304 @@ func TestSecretExists(t *testing.T) {
 	}
 }
 
+func TestConstructMetadataWithDefaults(t *testing.T) {
+	tests := []struct {
+		name        string
+		input       *apiextensionsv1.JSON
+		expected    *metadata.PushSecretMetadata[PushSecretMetadataSpec]
+		expectError bool
+	}{
+		{
+			name: "Valid metadata with multiple fields",
+			input: &apiextensionsv1.JSON{Raw: []byte(`{
+				"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+				"kind": "PushSecretMetadata",
+				"spec": {
+					"description": "test description",
+					"secretPushFormat":"string",
+					"kmsKeyID": "custom-kms-key",
+					"tags": {
+						"customKey": "customValue"
+					},
+				}
+			}`)},
+			expected: &metadata.PushSecretMetadata[PushSecretMetadataSpec]{
+				APIVersion: "kubernetes.external-secrets.io/v1alpha1",
+				Kind:       "PushSecretMetadata",
+				Spec: PushSecretMetadataSpec{
+					Description:      "test description",
+					SecretPushFormat: "string",
+					KMSKeyID:         "custom-kms-key",
+					Tags: map[string]string{
+						"customKey": "customValue",
+						managedBy:   externalSecrets,
+					},
+				},
+			},
+		},
+		{
+			name:  "Empty metadata, defaults applied",
+			input: nil,
+			expected: &metadata.PushSecretMetadata[PushSecretMetadataSpec]{
+				Spec: PushSecretMetadataSpec{
+					Description:      fmt.Sprintf("secret '%s:%s'", managedBy, externalSecrets),
+					SecretPushFormat: "binary",
+					KMSKeyID:         "alias/aws/secretsmanager",
+					Tags: map[string]string{
+						managedBy: externalSecrets,
+					},
+				},
+			},
+		},
+		{
+			name: "Added default metadata with 'managed-by' tag",
+			input: &apiextensionsv1.JSON{Raw: []byte(`{
+				"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+				"kind": "PushSecretMetadata",
+				"spec": {
+					"tags": {
+                        "managed-by": "external-secrets",
+						"customKey": "customValue"
+					},
+				}
+			}`)},
+			expected:    nil,
+			expectError: true,
+		},
+		{
+			name:        "Invalid metadata format",
+			input:       &apiextensionsv1.JSON{Raw: []byte(`invalid-json`)},
+			expected:    nil,
+			expectError: true,
+		},
+		{
+			name:        "Metadata with 'managed-by' tag specified",
+			input:       &apiextensionsv1.JSON{Raw: []byte(`{"tags":{"managed-by":"invalid"}}`)},
+			expected:    nil,
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := (&SecretsManager{}).constructMetadataWithDefaults(tt.input)
+
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestComputeTagsToUpdate(t *testing.T) {
+	tests := []struct {
+		name     string
+		tags     map[string]string
+		metaTags map[string]string
+		expected []types.Tag
+		modified bool
+	}{
+		{
+			name: "No tags to update",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			metaTags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expected: []types.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("value1")},
+				{Key: ptr.To("key2"), Value: ptr.To("value2")},
+			},
+			modified: false,
+		},
+		{
+			name: "No tags to update as managed-by tag is ignored",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			metaTags: map[string]string{
+				"key1":    "value1",
+				"key2":    "value2",
+				managedBy: externalSecrets,
+			},
+			expected: []types.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("value1")},
+				{Key: ptr.To("key2"), Value: ptr.To("value2")},
+				{Key: ptr.To(managedBy), Value: ptr.To(externalSecrets)},
+			},
+			modified: false,
+		},
+		{
+			name: "Add new tag",
+			tags: map[string]string{
+				"key1": "value1",
+			},
+			metaTags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expected: []types.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("value1")},
+				{Key: ptr.To("key2"), Value: ptr.To("value2")},
+			},
+			modified: true,
+		},
+		{
+			name: "Update existing tag value",
+			tags: map[string]string{
+				"key1": "value1",
+			},
+			metaTags: map[string]string{
+				"key1": "newValue",
+			},
+			expected: []types.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("newValue")},
+			},
+			modified: true,
+		},
+		{
+			name:     "Empty tags and metaTags",
+			tags:     map[string]string{},
+			metaTags: map[string]string{},
+			expected: []types.Tag{},
+			modified: false,
+		},
+		{
+			name: "Empty tags with non-empty metaTags",
+			tags: map[string]string{},
+			metaTags: map[string]string{
+				"key1": "value1",
+			},
+			expected: []types.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("value1")},
+			},
+			modified: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, modified := computeTagsToUpdate(tt.tags, tt.metaTags)
+			assert.ElementsMatch(t, tt.expected, result)
+			assert.Equal(t, tt.modified, modified)
+		})
+	}
+}
+
+func TestPatchTags(t *testing.T) {
+	type call struct {
+		untagCalled bool
+		tagCalled   bool
+	}
+	tests := []struct {
+		name         string
+		existingTags map[string]string
+		metaTags     map[string]string
+		expectUntag  bool
+		expectTag    bool
+		assertsTag   func(input *awssm.TagResourceInput)
+		assertsUntag func(input *awssm.UntagResourceInput)
+	}{
+		{
+			name:         "no changes",
+			existingTags: map[string]string{"a": "1"},
+			metaTags:     map[string]string{"a": "1"},
+			expectUntag:  false,
+			expectTag:    false,
+			assertsTag: func(input *awssm.TagResourceInput) {
+				assert.Fail(t, "Expected TagResource to not be called")
+			},
+			assertsUntag: func(input *awssm.UntagResourceInput) {
+				assert.Fail(t, "Expected UntagResource to not be called")
+			},
+		},
+		{
+			name:         "update tag value",
+			existingTags: map[string]string{"a": "1"},
+			metaTags:     map[string]string{"a": "2"},
+			expectUntag:  false,
+			expectTag:    true,
+			assertsTag: func(input *awssm.TagResourceInput) {
+				assert.Contains(t, input.Tags, types.Tag{Key: ptr.To(managedBy), Value: ptr.To(externalSecrets)})
+				assert.Contains(t, input.Tags, types.Tag{Key: ptr.To("a"), Value: ptr.To("2")})
+			},
+			assertsUntag: func(input *awssm.UntagResourceInput) {
+				assert.Fail(t, "Expected UntagResource to not be called")
+			},
+		},
+		{
+			name:         "remove tag",
+			existingTags: map[string]string{"a": "1", "b": "2"},
+			metaTags:     map[string]string{"a": "1"},
+			expectUntag:  true,
+			expectTag:    false,
+			assertsTag: func(input *awssm.TagResourceInput) {
+				assert.Fail(t, "Expected TagResource to not be called")
+			},
+			assertsUntag: func(input *awssm.UntagResourceInput) {
+				assert.Equal(t, []string{"b"}, input.TagKeys)
+			},
+		},
+		{
+			name:         "add tags",
+			existingTags: map[string]string{"a": "1"},
+			metaTags:     map[string]string{"a": "1", "b": "2"},
+			expectUntag:  false,
+			expectTag:    true,
+			assertsTag: func(input *awssm.TagResourceInput) {
+				assert.Contains(t, input.Tags, types.Tag{Key: ptr.To(managedBy), Value: ptr.To(externalSecrets)})
+				assert.Contains(t, input.Tags, types.Tag{Key: ptr.To("a"), Value: ptr.To("1")})
+				assert.Contains(t, input.Tags, types.Tag{Key: ptr.To("b"), Value: ptr.To("2")})
+			},
+			assertsUntag: func(input *awssm.UntagResourceInput) {
+				assert.Fail(t, "Expected UntagResource to not be called")
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			calls := call{}
+			fakeClient := &fakesm.Client{
+				TagResourceFn: fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil, func(input *awssm.TagResourceInput) {
+					tt.assertsTag(input)
+					calls.tagCalled = true
+				}),
+				UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil, func(input *awssm.UntagResourceInput) {
+					tt.assertsUntag(input)
+					calls.untagCalled = true
+				}),
+			}
+
+			sm := &SecretsManager{client: fakeClient}
+			metaMap := map[string]interface{}{
+				"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+				"kind":       "PushSecretMetadata",
+				"spec": map[string]interface{}{
+					"description": "adding managed-by tag explicitly",
+					"tags":        tt.metaTags,
+				},
+			}
+			raw, err := json.Marshal(metaMap)
+			require.NoError(t, err)
+			meta := &apiextensionsv1.JSON{Raw: raw}
+
+			secretId := "secret"
+			err = sm.patchTags(context.Background(), meta, &secretId, tt.existingTags)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectUntag, calls.untagCalled)
+			assert.Equal(t, tt.expectTag, calls.tagCalled)
+		})
+	}
+}
+
 // FakeCredProvider implements the AWS credentials.Provider interface
 // It is used to inject an error into the AWS config to cause a
 // validation error.