fix(aws): stop incrementing the UUID for versions (#5175)

* fix: stop incrementing the UUID for versions

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

On-behalf-of: gergely.brautigam@sap.com

* removed enforced entropy because system entropy should sufice

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Co-authored-by: Jakob Moeller <jakob.moeller@sap.com>

---------

Co-authored-by: Jakob Moeller <jakob.moeller@sap.com>
Co-authored-by: Moritz Johner <moolen@users.noreply.github.com>

pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -20,7 +20,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"math/big"
 	"slices"
 	"strings"
 
@@ -71,6 +70,7 @@ type SecretsManager struct {
 	cache        map[string]*awssm.GetSecretValueOutput
 	config       *esv1.SecretsManager
 	prefix       string
+	newUUID      func() string
 }
 
 // SMInterface is a subset of the smiface api.
@@ -275,41 +275,6 @@ func (sm *SecretsManager) getNewSecretValue(value []byte, property string, exist
 	return value, nil
 }
 
-func padOrTrim(b []byte) []byte {
-	l := len(b)
-	size := 16
-	if l == size {
-		return b
-	}
-	if l > size {
-		return b[l-size:]
-	}
-	tmp := make([]byte, size)
-	copy(tmp[size-l:], b)
-	return tmp
-}
-
-func bumpVersionNumber(id *string) (*string, error) {
-	if id == nil {
-		output := initialVersion
-		return &output, nil
-	}
-	n := new(big.Int)
-	oldVersion, ok := n.SetString(strings.ReplaceAll(*id, "-", ""), 16)
-	if !ok {
-		return nil, fmt.Errorf("expected secret version in AWS SSM to be a UUID but got '%s'", *id)
-	}
-	newVersionRaw := oldVersion.Add(oldVersion, big.NewInt(1)).Bytes()
-
-	newVersion, err := uuid.FromBytes(padOrTrim(newVersionRaw))
-	if err != nil {
-		return nil, err
-	}
-
-	s := newVersion.String()
-	return &s, nil
-}
-
 func isManagedByESO(data *awssm.DescribeSecretOutput) bool {
 	managedBy := managedBy
 	externalSecrets := externalSecrets
@@ -575,11 +540,11 @@ func (sm *SecretsManager) putSecretValueWithContext(ctx context.Context, secretA
 
 	newVersionNumber := initialVersion
 	if awsSecret != nil {
-		bumpedVersionNumber, err := bumpVersionNumber(awsSecret.VersionId)
-		if err != nil {
-			return err
+		if sm.newUUID == nil {
+			newVersionNumber = uuid.NewString()
+		} else {
+			newVersionNumber = sm.newUUID()
 		}
-		newVersionNumber = *bumpedVersionNumber
 	}
 	input := &awssm.PutSecretValueInput{
 		SecretId:           &secretArn,

pkg/provider/aws/secretsmanager/secretsmanager_test.go

@@ -472,9 +472,9 @@ func TestSetSecret(t *testing.T) {
 	}
 
 	initialVersion := "00000000-0000-0000-0000-000000000001"
-	defaultUpdatedVersion := "00000000-0000-0000-0000-000000000003"
-	randomUUIDVersion := "c2812e8d-84ce-4858-abec-7163d8ab312b"
-	randomUUIDVersionIncremented := "c2812e8d-84ce-4858-abec-7163d8ab312c"
+	defaultUpdatedVersion := "6c70d57a-f53d-bf4d-9525-3503dd5abe8c"
+	randomUUIDVersion := "9d6202c2-c216-433e-a2f0-5836c4f025af"
+	randomUUIDVersionIncremented := "4346824b-7da1-4d82-addf-dee197fd5d71"
 	unparsableVersion := "IAM UNPARSABLE"
 
 	secretValueOutput := &awssm.GetSecretValueOutput{
@@ -530,6 +530,7 @@ func TestSetSecret(t *testing.T) {
 		store          *esv1.AWSProvider
 		client         fakesm.Client
 		pushSecretData fake.PushSecretData
+		newUUID        string
 	}
 
 	type want struct {
@@ -707,7 +708,8 @@ func TestSetSecret(t *testing.T) {
 		"SetSecretWithPropertySucceedsWithExistingSecretAndNewPropertyBinary": {
 			reason: "when a pushSecretData property is specified, this property will be added to the sm secret if it is currently absent (sm secret is binary)",
 			args: args{
-				store: makeValidSecretStore().Spec.Provider.AWS,
+				newUUID: defaultUpdatedVersion,
+				store:   makeValidSecretStore().Spec.Provider.AWS,
 				client: fakesm.Client{
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutputFrom(params{b: []byte((`{"fake-property":"fake-value"}`))}), nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
@@ -727,7 +729,8 @@ func TestSetSecret(t *testing.T) {
 		"SetSecretWithPropertySucceedsWithExistingSecretAndRandomUUIDVersion": {
 			reason: "When a secret version is not specified, the client sets a random uuid by default. We should treat a version that can't be parsed to an int as not having a version",
 			args: args{
-				store: makeValidSecretStore().Spec.Provider.AWS,
+				store:   makeValidSecretStore().Spec.Provider.AWS,
+				newUUID: randomUUIDVersionIncremented,
 				client: fakesm.Client{
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutputFrom(params{
 						b:       []byte((`{"fake-property":"fake-value"}`)),
@@ -748,26 +751,34 @@ func TestSetSecret(t *testing.T) {
 			},
 		},
 		"SetSecretWithPropertySucceedsWithExistingSecretAndVersionThatCantBeParsed": {
-			reason: "A manually set secret version doesn't have to be a UUID, we only support UUID secret versions though",
+			reason: "A manually set secret version doesn't have to be a UUID",
 			args: args{
-				store: makeValidSecretStore().Spec.Provider.AWS,
+				newUUID: unparsableVersion,
+				store:   makeValidSecretStore().Spec.Provider.AWS,
 				client: fakesm.Client{
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutputFrom(params{
 						b:       []byte((`{"fake-property":"fake-value"}`)),
 						version: &unparsableVersion,
 					}), nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
+					PutSecretValueFn: fakesm.NewPutSecretValueFn(putSecretOutput, nil, fakesm.ExpectedPutSecretValueInput{
+						SecretBinary: []byte((`fake-value`)),
+						Version:      &unparsableVersion,
+					}),
+					TagResourceFn:   fakesm.NewTagResourceFn(&awssm.TagResourceOutput{}, nil),
+					UntagResourceFn: fakesm.NewUntagResourceFn(&awssm.UntagResourceOutput{}, nil),
 				},
-				pushSecretData: pushSecretDataWithProperty,
+				pushSecretData: pushSecretDataWithoutProperty,
 			},
 			want: want{
-				err: fmt.Errorf("expected secret version in AWS SSM to be a UUID but got '%s'", unparsableVersion),
+				err: nil,
 			},
 		},
 		"SetSecretWithPropertySucceedsWithExistingSecretAndAbsentVersion": {
 			reason: "When a secret version is not specified, set it to 1",
 			args: args{
-				store: makeValidSecretStore().Spec.Provider.AWS,
+				newUUID: initialVersion,
+				store:   makeValidSecretStore().Spec.Provider.AWS,
 				client: fakesm.Client{
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(&awssm.GetSecretValueOutput{
 						ARN:          &arn,
@@ -790,7 +801,8 @@ func TestSetSecret(t *testing.T) {
 		"SetSecretWithPropertySucceedsWithExistingSecretAndNewPropertyString": {
 			reason: "when a pushSecretData property is specified, this property will be added to the sm secret if it is currently absent (sm secret is a string)",
 			args: args{
-				store: makeValidSecretStore().Spec.Provider.AWS,
+				newUUID: defaultUpdatedVersion,
+				store:   makeValidSecretStore().Spec.Provider.AWS,
 				client: fakesm.Client{
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutputFrom(params{s: `{"fake-property":"fake-value"}`}), nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
@@ -810,7 +822,8 @@ func TestSetSecret(t *testing.T) {
 		"SetSecretWithPropertySucceedsWithExistingSecretAndNewPropertyWithDot": {
 			reason: "when a pushSecretData property is specified, this property will be added to the sm secret if it is currently absent (pushSecretData property is a sub-object)",
 			args: args{
-				store: makeValidSecretStore().Spec.Provider.AWS,
+				newUUID: defaultUpdatedVersion,
+				store:   makeValidSecretStore().Spec.Provider.AWS,
 				client: fakesm.Client{
 					GetSecretValueFn: fakesm.NewGetSecretValueFn(secretValueOutputFrom(params{s: `{"fake-property":{"fake-property":"fake-value"}}`}), nil),
 					DescribeSecretFn: fakesm.NewDescribeSecretFn(tagSecretOutput, nil),
@@ -989,8 +1002,9 @@ func TestSetSecret(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			sm := SecretsManager{
-				client: &tc.args.client,
-				prefix: tc.args.store.Prefix,
+				client:  &tc.args.client,
+				prefix:  tc.args.store.Prefix,
+				newUUID: func() string { return tc.args.newUUID },
 			}
 
 			err := sm.PushSecret(context.Background(), fakeSecret, tc.args.pushSecretData)