fix: parameter store should be called only once (#3584)

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

@@ -29,6 +29,7 @@ type Client struct {
 	GetParameterWithContextFn        GetParameterWithContextFn
 	GetParametersByPathWithContextFn GetParametersByPathWithContextFn
 	PutParameterWithContextFn        PutParameterWithContextFn
+	PutParameterWithContextCalledN   int
 	DeleteParameterWithContextFn     DeleteParameterWithContextFn
 	DescribeParametersWithContextFn  DescribeParametersWithContextFn
 	ListTagsForResourceWithContextFn ListTagsForResourceWithContextFn
@@ -86,6 +87,7 @@ func NewDescribeParametersWithContextFn(output *ssm.DescribeParametersOutput, er
 }
 
 func (sm *Client) PutParameterWithContext(ctx aws.Context, input *ssm.PutParameterInput, options ...request.Option) (*ssm.PutParameterOutput, error) {
+	sm.PutParameterWithContextCalledN++
 	return sm.PutParameterWithContextFn(ctx, input, options...)
 }
 

pkg/provider/aws/parameterstore/parameterstore.go

@@ -41,12 +41,10 @@ import (
 
 // Declares metadata information for pushing secrets to AWS Parameter Store.
 const (
-	PushSecretType                 = "parameterStoreType"
-	ParameterStoreTypeString       = "String"
-	ParameterStoreTypeStringList   = "StringList"
-	ParameterStoreTypeSecureString = "SecureString"
-	ParameterStoreKeyID            = "parameterStoreKeyID"
-	PushSecretKeyID                = "keyID"
+	PushSecretType  = "parameterStoreType"
+	StoreTypeString = "String"
+	StoreKeyID      = "parameterStoreKeyID"
+	PushSecretKeyID = "keyID"
 )
 
 // https://github.com/external-secrets/external-secrets/issues/644
@@ -153,12 +151,12 @@ func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret,
 		err   error
 	)
 
-	parameterTypeFormat, err := utils.FetchValueFromMetadata(PushSecretType, data.GetMetadata(), ParameterStoreTypeString)
+	parameterTypeFormat, err := utils.FetchValueFromMetadata(PushSecretType, data.GetMetadata(), StoreTypeString)
 	if err != nil {
 		return fmt.Errorf("failed to parse metadata: %w", err)
 	}
 
-	parameterKeyIDFormat, err := utils.FetchValueFromMetadata(ParameterStoreKeyID, data.GetMetadata(), PushSecretKeyID)
+	parameterKeyIDFormat, err := utils.FetchValueFromMetadata(StoreKeyID, data.GetMetadata(), PushSecretKeyID)
 	if err != nil {
 		return fmt.Errorf("failed to parse metadata: %w", err)
 	}
@@ -208,7 +206,6 @@ 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 {
-		fmt.Println("The existing value contains data:", existing.String())
 		tags, err := pm.getTagsByName(ctx, existing)
 		if err != nil {
 			return fmt.Errorf("error getting the existing tags for the parameter %v: %w", secretName, err)

pkg/provider/aws/parameterstore/parameterstore_test.go

@@ -25,6 +25,8 @@ import (
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ssm"
 	"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"
@@ -239,7 +241,7 @@ func TestDeleteSecret(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			ref := fake.PushSecretData{RemoteKey: "fake-key"}
+			ref := fake.PushSecretData{RemoteKey: remoteKey}
 			ps := ParameterStore{
 				client: &tc.args.client,
 			}
@@ -262,6 +264,9 @@ func TestDeleteSecret(t *testing.T) {
 		})
 	}
 }
+
+const remoteKey = "fake-key"
+
 func TestPushSecret(t *testing.T) {
 	invalidParameters := errors.New(ssm.ErrCodeInvalidParameters)
 	alreadyExistsError := errors.New(ssm.ErrCodeAlreadyExistsException)
@@ -489,7 +494,7 @@ func TestPushSecret(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			psd := fake.PushSecretData{SecretKey: fakeSecretKey, RemoteKey: "fake-key"}
+			psd := fake.PushSecretData{SecretKey: fakeSecretKey, RemoteKey: remoteKey}
 			if tc.args.metadata != nil {
 				psd.Metadata = tc.args.metadata
 			}
@@ -513,6 +518,48 @@ func TestPushSecret(t *testing.T) {
 	}
 }
 
+func TestPushSecretCalledOnlyOnce(t *testing.T) {
+	fakeSecretKey := "fakeSecretKey"
+	fakeValue := "fakeValue"
+	fakeSecret := &corev1.Secret{
+		Data: map[string][]byte{
+			fakeSecretKey: []byte(fakeValue),
+		},
+	}
+
+	managedByESO := ssm.Tag{
+		Key:   &managedBy,
+		Value: &externalSecrets,
+	}
+
+	putParameterOutput := &ssm.PutParameterOutput{}
+	validGetParameterOutput := &ssm.GetParameterOutput{
+		Parameter: &ssm.Parameter{
+			Value: &fakeValue,
+		},
+	}
+	describeParameterOutput := &ssm.DescribeParametersOutput{}
+	validListTagsForResourceOutput := &ssm.ListTagsForResourceOutput{
+		TagList: []*ssm.Tag{&managedByESO},
+	}
+
+	client := fakeps.Client{
+		PutParameterWithContextFn:        fakeps.NewPutParameterWithContextFn(putParameterOutput, nil),
+		GetParameterWithContextFn:        fakeps.NewGetParameterWithContextFn(validGetParameterOutput, nil),
+		DescribeParametersWithContextFn:  fakeps.NewDescribeParametersWithContextFn(describeParameterOutput, nil),
+		ListTagsForResourceWithContextFn: fakeps.NewListTagsForResourceWithContextFn(validListTagsForResourceOutput, nil),
+	}
+
+	psd := fake.PushSecretData{SecretKey: fakeSecretKey, RemoteKey: remoteKey}
+	ps := ParameterStore{
+		client: &client,
+	}
+
+	require.NoError(t, ps.PushSecret(context.TODO(), fakeSecret, psd))
+
+	assert.Equal(t, 0, client.PutParameterWithContextCalledN)
+}
+
 // test the ssm<->aws interface
 // make sure correct values are passed and errors are handled accordingly.
 func TestGetSecret(t *testing.T) {

pkg/utils/utils.go

@@ -512,16 +512,6 @@ func CompareStringAndByteSlices(valueString *string, valueByte []byte) bool {
 	if valueString == nil {
 		return false
 	}
-	stringToByteSlice := []byte(*valueString)
-	if len(stringToByteSlice) != len(valueByte) {
-		return false
-	}
-
-	for sb := range valueByte {
-		if stringToByteSlice[sb] != valueByte[sb] {
-			return false
-		}
-	}
 
-	return true
+	return bytes.Equal(valueByte, []byte(*valueString))
 }