feat(aws): parametersstore support for aws tags (#4967)

* feat(aws): parametersstore support for aws tags

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

* feat(aws): parametersstore support for aws tags

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

* feat(aws): parametersstore support for aws tags

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

* feat(aws): parametersstore support for aws tags

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

* feat(aws): parametersstore support for aws tags

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

* feat(aws): parametersstore support for aws tags

Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Signed-off-by: Ivan Ka <5395690+ivankatliarchuk@users.noreply.github.com>

* feat(aws): parametersstore support for aws tags

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

---------

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: Ivan Ka <5395690+ivankatliarchuk@users.noreply.github.com>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

docs/snippets/aws-pm-push-secret-with-metadata.yaml

@@ -20,6 +20,7 @@ spec:
         apiVersion: kubernetes.external-secrets.io/v1alpha1
         kind: PushSecretMetadata
         spec:
+          description: "This is a secret for the API credentials"
           secretType: SecureString
           kmsKeyID: bb123123-b2b0-4f60-ac3a-44a13f0e6b6c
           tier:
@@ -44,3 +45,8 @@ spec:
                 attributes:
                   after: "30"
                   unit: "Days"
+          tags:
+            environment: sandbox
+            team: pokedex
+            secret-store: aws-parameterstore
+            refresh-interval: 1h

pkg/constants/constants.go

@@ -30,6 +30,8 @@ const (
 	CallAWSPSPutParameter        = "PutParameter"
 	CallAWSPSDeleteParameter     = "DeleteParameter"
 	CallAWSPSDescribeParameter   = "DescribeParameter"
+	CallAWSPSRemoveTagsParameter = "RemoveTagsFromResource"
+	CallAWSPSAddTagsParameter    = "AddTagsToResource"
 	CallAWSPSListTagsForResource = "ListTagsForResource"
 
 	ProviderAzureKV              = "Azure/KeyVault"

pkg/provider/aws/parameterstore/fake/fake.go

@@ -33,6 +33,8 @@ type Client struct {
 	DeleteParameterFn        DeleteParameterFn
 	DescribeParametersFn     DescribeParametersFn
 	ListTagsForResourceFn    ListTagsForResourceFn
+	RemoveTagsFromResourceFn RemoveTagsFromResourceFn
+	AddTagsToResourceFn      AddTagsToResourceFn
 }
 
 type GetParameterFn func(context.Context, *ssm.GetParameterInput, ...func(*ssm.Options)) (*ssm.GetParameterOutput, error)
@@ -42,6 +44,10 @@ type DescribeParametersFn func(context.Context, *ssm.DescribeParametersInput, ..
 type ListTagsForResourceFn func(context.Context, *ssm.ListTagsForResourceInput, ...func(*ssm.Options)) (*ssm.ListTagsForResourceOutput, error)
 type DeleteParameterFn func(ctx context.Context, input *ssm.DeleteParameterInput, opts ...func(*ssm.Options)) (*ssm.DeleteParameterOutput, error)
 
+type RemoveTagsFromResourceFn func(ctx context.Context, params *ssm.RemoveTagsFromResourceInput, optFns ...func(*ssm.Options)) (*ssm.RemoveTagsFromResourceOutput, error)
+
+type AddTagsToResourceFn func(ctx context.Context, params *ssm.AddTagsToResourceInput, optFns ...func(*ssm.Options)) (*ssm.AddTagsToResourceOutput, error)
+
 func (sm *Client) ListTagsForResource(ctx context.Context, input *ssm.ListTagsForResourceInput, options ...func(*ssm.Options)) (*ssm.ListTagsForResourceOutput, error) {
 	return sm.ListTagsForResourceFn(ctx, input, options...)
 }
@@ -106,3 +112,23 @@ func (sm *Client) WithValue(in *ssm.GetParameterInput, val *ssm.GetParameterOutp
 		return val, err
 	}
 }
+
+func (sm *Client) RemoveTagsFromResource(_ context.Context, params *ssm.RemoveTagsFromResourceInput, optFns ...func(*ssm.Options)) (*ssm.RemoveTagsFromResourceOutput, error) {
+	return nil, errors.New("RemoveTagsFromResource is not implemented in fake client")
+}
+
+func NewRemoveTagsFromResourceFn(output *ssm.RemoveTagsFromResourceOutput, err error) RemoveTagsFromResourceFn {
+	return func(ctx context.Context, params *ssm.RemoveTagsFromResourceInput, optFns ...func(*ssm.Options)) (*ssm.RemoveTagsFromResourceOutput, error) {
+		return output, err
+	}
+}
+
+func (sm *Client) AddTagsToResource(_ context.Context, params *ssm.AddTagsToResourceInput, optFns ...func(*ssm.Options)) (*ssm.AddTagsToResourceOutput, error) {
+	return nil, errors.New("AddTagsToResource is not implemented in fake client")
+}
+
+func NewAddTagsToResourceFn(output *ssm.AddTagsToResourceOutput, err error) AddTagsToResourceFn {
+	return func(ctx context.Context, params *ssm.AddTagsToResourceInput, optFns ...func(*ssm.Options)) (*ssm.AddTagsToResourceOutput, error) {
+		return output, err
+	}
+}

pkg/provider/aws/parameterstore/parameterstore.go

@@ -19,7 +19,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"slices"
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -53,6 +52,8 @@ type PushSecretMetadataSpec struct {
 	KMSKeyID        string                 `json:"kmsKeyID,omitempty"`
 	Tier            Tier                   `json:"tier,omitempty"`
 	EncodeAsDecoded bool                   `json:"encodeAsDecoded,omitempty"`
+	Tags            map[string]string      `json:"tags,omitempty"`
+	Description     string                 `json:"description,omitempty"`
 }
 
 // https://github.com/external-secrets/external-secrets/issues/644
@@ -79,17 +80,18 @@ type PMInterface interface {
 	PutParameter(ctx context.Context, input *ssm.PutParameterInput, opts ...func(*ssm.Options)) (*ssm.PutParameterOutput, error)
 	DescribeParameters(ctx context.Context, input *ssm.DescribeParametersInput, opts ...func(*ssm.Options)) (*ssm.DescribeParametersOutput, error)
 	ListTagsForResource(ctx context.Context, input *ssm.ListTagsForResourceInput, opts ...func(*ssm.Options)) (*ssm.ListTagsForResourceOutput, error)
+	RemoveTagsFromResource(ctx context.Context, params *ssm.RemoveTagsFromResourceInput, optFns ...func(*ssm.Options)) (*ssm.RemoveTagsFromResourceOutput, error)
+	AddTagsToResource(ctx context.Context, params *ssm.AddTagsToResourceInput, optFns ...func(*ssm.Options)) (*ssm.AddTagsToResourceOutput, error)
 	DeleteParameter(ctx context.Context, input *ssm.DeleteParameterInput, opts ...func(*ssm.Options)) (*ssm.DeleteParameterOutput, error)
 }
 
 const (
 	errUnexpectedFindOperator    = "unexpected find operator"
-	errAccessDeniedException     = "AccessDeniedException"
 	errCodeAccessDeniedException = "AccessDeniedException"
 )
 
 // New constructs a ParameterStore Provider that is specific to a store.
-func New(ctx context.Context, cfg *aws.Config, prefix string, referentAuth bool) (*ParameterStore, error) {
+func New(_ context.Context, cfg *aws.Config, prefix string, referentAuth bool) (*ParameterStore, error) {
 	return &ParameterStore{
 		cfg:          cfg,
 		referentAuth: referentAuth,
@@ -100,7 +102,7 @@ func New(ctx context.Context, cfg *aws.Config, prefix string, referentAuth bool)
 	}, nil
 }
 
-func (pm *ParameterStore) getTagsByName(ctx context.Context, ref *ssm.GetParameterOutput) ([]ssmTypes.Tag, error) {
+func (pm *ParameterStore) getTagsByName(ctx context.Context, ref *ssm.GetParameterOutput) (map[string]string, error) {
 	parameterType := "Parameter"
 
 	parameterTags := ssm.ListTagsForResourceInput{
@@ -114,7 +116,11 @@ func (pm *ParameterStore) getTagsByName(ctx context.Context, ref *ssm.GetParamet
 		return nil, fmt.Errorf("error listing tags %w", err)
 	}
 
-	return data.TagList, nil
+	tags := map[string]string{}
+	for _, tag := range data.TagList {
+		tags[*tag.Key] = *tag.Value
+	}
+	return tags, nil
 }
 
 func (pm *ParameterStore) DeleteSecret(ctx context.Context, remoteRef esv1.PushSecretRemoteRef) error {
@@ -200,12 +206,22 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 		value = secret.Data[key]
 	}
 
+	tags := make([]ssmTypes.Tag, 0, len(meta.Spec.Tags))
+
+	for k, v := range meta.Spec.Tags {
+		tags = append(tags, ssmTypes.Tag{
+			Key:   ptr.To(k),
+			Value: ptr.To(v),
+		})
+	}
+
 	secretName := pm.prefix + data.GetRemoteKey()
 	secretRequest := ssm.PutParameterInput{
-		Name:      ptr.To(pm.prefix + data.GetRemoteKey()),
-		Value:     ptr.To(string(value)),
-		Type:      meta.Spec.SecretType,
-		Overwrite: ptr.To(true),
+		Name:        ptr.To(pm.prefix + data.GetRemoteKey()),
+		Value:       ptr.To(string(value)),
+		Type:        meta.Spec.SecretType,
+		Overwrite:   ptr.To(true),
+		Description: ptr.To(meta.Spec.Description),
 	}
 
 	if meta.Spec.SecretType == "SecureString" {
@@ -234,12 +250,12 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 
 	// If we have a valid parameter returned to us, check its tags
 	if existing != nil && existing.Parameter != nil {
-		return pm.setExisting(ctx, existing, secretName, value, secretRequest)
+		return pm.setExisting(ctx, existing, secretName, value, secretRequest, meta.Spec.Tags)
 	}
 
 	// let's set the secret
 	// Do we need to delete the existing parameter on the remote?
-	return pm.setManagedRemoteParameter(ctx, secretRequest, true)
+	return pm.setManagedRemoteParameter(ctx, secretRequest, tags, true)
 }
 
 func (pm *ParameterStore) encodeSecretData(encodeAsDecoded bool, data map[string][]byte) ([]byte, error) {
@@ -259,7 +275,7 @@ func convertMap(in map[string][]byte) map[string]string {
 	return m
 }
 
-func (pm *ParameterStore) setExisting(ctx context.Context, existing *ssm.GetParameterOutput, secretName string, value []byte, secretRequest ssm.PutParameterInput) error {
+func (pm *ParameterStore) setExisting(ctx context.Context, existing *ssm.GetParameterOutput, secretName string, value []byte, secretRequest ssm.PutParameterInput, metaTags map[string]string) error {
 	tags, err := pm.getTagsByName(ctx, existing)
 	if err != nil {
 		return fmt.Errorf("error getting the existing tags for the parameter %v: %w", secretName, err)
@@ -281,25 +297,49 @@ func (pm *ParameterStore) setExisting(ctx context.Context, existing *ssm.GetPara
 		return nil
 	}
 
-	return pm.setManagedRemoteParameter(ctx, secretRequest, false)
-}
+	err = pm.setManagedRemoteParameter(ctx, secretRequest, []ssmTypes.Tag{}, false)
+	if err != nil {
+		return err
+	}
 
-func isManagedByESO(tags []ssmTypes.Tag) bool {
-	return slices.ContainsFunc(tags, func(tag ssmTypes.Tag) bool {
-		return *tag.Key == managedBy && *tag.Value == externalSecrets
-	})
-}
+	tagKeysToRemove := findTagKeysToRemove(tags, metaTags)
+	if len(tagKeysToRemove) > 0 {
+		_, err = pm.client.RemoveTagsFromResource(ctx, &ssm.RemoveTagsFromResourceInput{
+			ResourceId:   existing.Parameter.Name,
+			ResourceType: ssmTypes.ResourceTypeForTaggingParameter,
+			TagKeys:      tagKeysToRemove,
+		})
+		metrics.ObserveAPICall(constants.ProviderAWSPS, constants.CallAWSPSRemoveTagsParameter, err)
+		if err != nil {
+			return err
+		}
+	}
 
-func (pm *ParameterStore) setManagedRemoteParameter(ctx context.Context, secretRequest ssm.PutParameterInput, createManagedByTags bool) error {
-	externalSecretsTag := ssmTypes.Tag{
-		Key:   &managedBy,
-		Value: &externalSecrets,
+	tagsToUpdate, isModified := computeTagsToUpdate(tags, metaTags)
+	if isModified {
+		_, err = pm.client.AddTagsToResource(ctx, &ssm.AddTagsToResourceInput{
+			ResourceId:   existing.Parameter.Name,
+			ResourceType: ssmTypes.ResourceTypeForTaggingParameter,
+			Tags:         tagsToUpdate,
+		})
+		metrics.ObserveAPICall(constants.ProviderAWSPS, constants.CallAWSPSAddTagsParameter, err)
+		if err != nil {
+			return err
+		}
 	}
 
+	return nil
+}
+
+func isManagedByESO(tags map[string]string) bool {
+	return tags[managedBy] == externalSecrets
+}
+
+func (pm *ParameterStore) setManagedRemoteParameter(ctx context.Context, secretRequest ssm.PutParameterInput, tags []ssmTypes.Tag, createManagedByTags bool) error {
 	overwrite := true
 	secretRequest.Overwrite = &overwrite
 	if createManagedByTags {
-		secretRequest.Tags = append(secretRequest.Tags, externalSecretsTag)
+		secretRequest.Tags = append(secretRequest.Tags, tags...)
 		overwrite = false
 	}
 
@@ -609,6 +649,10 @@ func (pm *ParameterStore) constructMetadataWithDefaults(data *apiextensionsv1.JS
 		meta = &metadata.PushSecretMetadata[PushSecretMetadataSpec]{}
 	}
 
+	if meta.Spec.Description == "" {
+		meta.Spec.Description = fmt.Sprintf("secret '%s:%s'", managedBy, externalSecrets)
+	}
+
 	if meta.Spec.Tier.Type == "" {
 		meta.Spec.Tier.Type = "Standard"
 	}
@@ -621,5 +665,45 @@ func (pm *ParameterStore) constructMetadataWithDefaults(data *apiextensionsv1.JS
 		meta.Spec.KMSKeyID = "alias/aws/ssm"
 	}
 
+	if len(meta.Spec.Tags) > 0 {
+		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)
+	}
+	// always add the managedBy tag
+	meta.Spec.Tags[managedBy] = externalSecrets
+
 	return meta, nil
 }
+
+// findTagKeysToRemove returns a slice of tag keys that exist in the current tags
+// but are not present in the desired metaTags. These keys should be removed to
+// synchronize the tags with the desired state.
+func findTagKeysToRemove(tags, metaTags map[string]string) []string {
+	var diff []string
+	for key, _ := range tags {
+		if _, ok := metaTags[key]; !ok {
+			diff = append(diff, key)
+		}
+	}
+	return diff
+}
+
+// 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) ([]ssmTypes.Tag, bool) {
+	result := make([]ssmTypes.Tag, 0, len(metaTags))
+	modified := false
+	for k, v := range metaTags {
+		if _, exists := tags[k]; !exists || tags[k] != v {
+			modified = true
+		}
+		result = append(result, ssmTypes.Tag{
+			Key:   ptr.To(k),
+			Value: ptr.To(v),
+		})
+	}
+	return result, modified
+}

pkg/provider/aws/parameterstore/parameterstore_test.go

@@ -17,18 +17,21 @@ package parameterstore
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/ssm"
 	ssmtypes "github.com/aws/aws-sdk-go-v2/service/ssm/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"
+	"k8s.io/utils/ptr"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 	fakeps "github.com/external-secrets/external-secrets/pkg/provider/aws/parameterstore/fake"
@@ -339,10 +342,12 @@ func TestPushSecret(t *testing.T) {
 			args: args{
 				store: makeValidParameterStore().Spec.Provider.AWS,
 				client: fakeps.Client{
-					PutParameterFn:        fakeps.NewPutParameterFn(putParameterOutput, nil),
-					GetParameterFn:        fakeps.NewGetParameterFn(getParameterOutput, nil),
-					DescribeParametersFn:  fakeps.NewDescribeParametersFn(describeParameterOutput, nil),
-					ListTagsForResourceFn: fakeps.NewListTagsForResourceFn(validListTagsForResourceOutput, nil),
+					PutParameterFn:           fakeps.NewPutParameterFn(putParameterOutput, nil),
+					GetParameterFn:           fakeps.NewGetParameterFn(getParameterOutput, nil),
+					DescribeParametersFn:     fakeps.NewDescribeParametersFn(describeParameterOutput, nil),
+					ListTagsForResourceFn:    fakeps.NewListTagsForResourceFn(validListTagsForResourceOutput, nil),
+					RemoveTagsFromResourceFn: fakeps.NewRemoveTagsFromResourceFn(&ssm.RemoveTagsFromResourceOutput{}, nil),
+					AddTagsToResourceFn:      fakeps.NewAddTagsToResourceFn(&ssm.AddTagsToResourceOutput{}, nil),
 				},
 			},
 			want: want{
@@ -586,11 +591,40 @@ func TestPushSecret(t *testing.T) {
 				err: errors.New("unable to compare 'sensitive' result, ensure to request a decrypted value"),
 			},
 		},
+		"SecretWithTags": {
+			reason: "test if we can configure tags for the secret",
+			args: args{
+				store: makeValidParameterStore().Spec.Provider.AWS,
+				metadata: &apiextensionsv1.JSON{
+					Raw: []byte(`{
+						"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+						"kind": "PushSecretMetadata",
+						"spec": {
+							"tags": {
+								"tagname1": "value1"
+							},
+						}
+					}`),
+				},
+				client: fakeps.Client{
+					PutParameterFn:       fakeps.NewPutParameterFn(putParameterOutput, nil),
+					GetParameterFn:       fakeps.NewGetParameterFn(sameGetParameterOutput, nil),
+					DescribeParametersFn: fakeps.NewDescribeParametersFn(describeParameterOutput, nil),
+					ListTagsForResourceFn: fakeps.NewListTagsForResourceFn(&ssm.ListTagsForResourceOutput{
+						TagList: []ssmtypes.Tag{managedByESO, {Key: ptr.To("tagname1"), Value: ptr.To("value1")}},
+					}, nil),
+				},
+			},
+			want: want{
+				err: nil,
+			},
+		},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			psd := fake.PushSecretData{SecretKey: fakeSecretKey, RemoteKey: remoteKey}
+			fmt.Println("line 635:", psd, tc.args.metadata)
 			if tc.args.metadata != nil {
 				psd.Metadata = tc.args.metadata
 			}
@@ -599,6 +633,8 @@ func TestPushSecret(t *testing.T) {
 			}
 			err := ps.PushSecret(context.TODO(), fakeSecret, psd)
 
+			fmt.Println("line 644:", err)
+
 			// 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)
@@ -806,7 +842,7 @@ func TestGetSecret(t *testing.T) {
 			TagList: getTagSlice(),
 		}
 		pstc.fakeClient.ListTagsForResourceFn = fakeps.NewListTagsForResourceFn(&output, nil)
-		pstc.expectedSecret, _ = util.ParameterTagsToJSONString(getTagSlice())
+		pstc.expectedSecret, _ = util.ParameterTagsToJSONString(normaliseTags(getTagSlice()))
 	}
 
 	// good case: metadata property returned
@@ -952,6 +988,16 @@ func getTagSlice() []ssmtypes.Tag {
 	}
 }
 
+func normaliseTags(input []ssmtypes.Tag) map[string]string {
+	tags := make(map[string]string, len(input))
+	for _, tag := range input {
+		if tag.Key != nil && tag.Value != nil {
+			tags[*tag.Key] = *tag.Value
+		}
+	}
+	return tags
+}
+
 func TestSecretExists(t *testing.T) {
 	parameterOutput := &ssm.GetParameterOutput{
 		Parameter: &ssmtypes.Parameter{
@@ -1038,3 +1084,248 @@ 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",
+					"tier": {"type": "Advanced"},
+					"secretType":"SecureString",
+					"kmsKeyID": "custom-kms-key",
+					"tags": {
+						"customKey": "customValue"
+					},
+				}
+			}`)},
+			expected: &metadata.PushSecretMetadata[PushSecretMetadataSpec]{
+				APIVersion: "kubernetes.external-secrets.io/v1alpha1",
+				Kind:       "PushSecretMetadata",
+				Spec: PushSecretMetadataSpec{
+					Description: "test description",
+					Tier: Tier{
+						Type: "Advanced",
+					},
+					SecretType: "SecureString",
+					KMSKeyID:   "custom-kms-key",
+					Tags: map[string]string{
+						"customKey":  "customValue",
+						"managed-by": "external-secrets",
+					},
+				},
+			},
+		},
+		{
+			name:  "Empty metadata, defaults applied",
+			input: nil,
+			expected: &metadata.PushSecretMetadata[PushSecretMetadataSpec]{
+				Spec: PushSecretMetadataSpec{
+					Description: "secret 'managed-by:external-secrets'",
+					Tier: Tier{
+						Type: "Standard",
+					},
+					SecretType: "String",
+					KMSKeyID:   "alias/aws/ssm",
+					Tags: map[string]string{
+						"managed-by": "external-secrets",
+					},
+				},
+			},
+		},
+		{
+			name: "Added default metadata with 'managed-by' tag",
+			input: &apiextensionsv1.JSON{Raw: []byte(`{
+				"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+				"kind": "PushSecretMetadata",
+				"spec": {
+ 					"description": "adding managed-by tag explicitly",
+					"tags": {
+						"managed-by": "external-secrets",
+						"customKey": "customValue"
+					},
+				}
+			}`)},
+			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 := (&ParameterStore{}).constructMetadataWithDefaults(tt.input)
+
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestFindTagKeysToRemove(t *testing.T) {
+	tests := []struct {
+		name     string
+		tags     map[string]string
+		metaTags map[string]string
+		expected []string
+	}{
+		{
+			name: "No tags to remove",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			metaTags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expected: []string{},
+		},
+		{
+			name: "Some tags to remove",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+				"key3": "value3",
+			},
+			metaTags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expected: []string{"key3"},
+		},
+		{
+			name: "All tags to remove",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			metaTags: map[string]string{},
+			expected: []string{"key1", "key2"},
+		},
+		{
+			name:     "Empty tags and metaTags",
+			tags:     map[string]string{},
+			metaTags: map[string]string{},
+			expected: []string{},
+		},
+		{
+			name: "Empty metaTags with non-empty tags",
+			tags: map[string]string{
+				"key1": "value1",
+			},
+			metaTags: map[string]string{},
+			expected: []string{"key1"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := findTagKeysToRemove(tt.tags, tt.metaTags)
+			assert.ElementsMatch(t, tt.expected, result)
+		})
+	}
+}
+
+func TestComputeTagsToUpdate(t *testing.T) {
+	tests := []struct {
+		name     string
+		tags     map[string]string
+		metaTags map[string]string
+		expected []ssmtypes.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: []ssmtypes.Tag{
+				{Key: ptr.To("key1"), Value: ptr.To("value1")},
+				{Key: ptr.To("key2"), Value: ptr.To("value2")},
+			},
+			modified: false,
+		},
+		{
+			name: "Add new tag",
+			tags: map[string]string{
+				"key1": "value1",
+			},
+			metaTags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expected: []ssmtypes.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: []ssmtypes.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: []ssmtypes.Tag{},
+			modified: false,
+		},
+		{
+			name: "Empty tags with non-empty metaTags",
+			tags: map[string]string{},
+			metaTags: map[string]string{
+				"key1": "value1",
+			},
+			expected: []ssmtypes.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)
+		})
+	}
+}

pkg/provider/aws/util/provider.go

@@ -20,8 +20,6 @@ import (
 	"fmt"
 
 	awssm "github.com/aws/aws-sdk-go-v2/service/secretsmanager/types"
-	ssm "github.com/aws/aws-sdk-go-v2/service/ssm/types"
-
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 )
 
@@ -80,13 +78,8 @@ func SecretTagsToJSONString(tags []awssm.Tag) (string, error) {
 	return string(byteArr), nil
 }
 
-func ParameterTagsToJSONString(tags []ssm.Tag) (string, error) {
-	tagMap := make(map[string]string, len(tags))
-	for _, tag := range tags {
-		tagMap[*tag.Key] = *tag.Value
-	}
-
-	byteArr, err := json.Marshal(tagMap)
+func ParameterTagsToJSONString(tags map[string]string) (string, error) {
+	byteArr, err := json.Marshal(tags)
 	if err != nil {
 		return "", err
 	}

pkg/provider/aws/util/provider_test.go

@@ -0,0 +1,68 @@
+/*
+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 util
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParameterTagsToJSONString(t *testing.T) {
+	tests := []struct {
+		name     string
+		tags     map[string]string
+		expected string
+		wantErr  bool
+	}{
+		{
+			name: "Valid tags",
+			tags: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+			expected: `{"key1":"value1","key2":"value2"}`,
+			wantErr:  false,
+		},
+		{
+			name:     "Empty tags",
+			tags:     map[string]string{},
+			expected: `{}`,
+			wantErr:  false,
+		},
+		{
+			name:     "Nil tags",
+			tags:     nil,
+			wantErr:  false,
+			expected: "null",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := ParameterTagsToJSONString(tt.tags)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				var resultMap map[string]string
+				err := json.Unmarshal([]byte(result), &resultMap)
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}