fix: pre-existing dynamodb versions table should be populated with version if it doesn't contain it already (#3699)

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

pkg/meta/dynamodb/dynamodb.go

@@ -2267,11 +2267,10 @@ func (dwr *DynamoDB) createVersionTable() error {
 	})
 	if err != nil {
 		inUseException := new(types.ResourceInUseException)
-		if errors.As(err, &inUseException) {
-			return nil
+		if !errors.As(err, &inUseException) {
+			return err
 		}
-
-		return err
+		// Table already exists, continue to ensure version data is set
 	}
 
 	err = dwr.waitTableToBeCreated(dwr.VersionTablename)
@@ -2296,10 +2295,17 @@ func (dwr *DynamoDB) createVersionTable() error {
 				Value: version.DBVersionKey,
 			},
 		},
-		TableName:        aws.String(dwr.VersionTablename),
-		UpdateExpression: aws.String("SET #V = :Version"),
+		TableName:           aws.String(dwr.VersionTablename),
+		UpdateExpression:    aws.String("SET #V = :Version"),
+		ConditionExpression: aws.String("attribute_not_exists(#V)"),
 	})
 	if err != nil {
+		conditionalCheckFailedException := new(types.ConditionalCheckFailedException)
+		if errors.As(err, &conditionalCheckFailedException) {
+			// Version already exists, skip setting it - patches will handle version updates
+			return nil
+		}
+
 		return err
 	}
 

pkg/meta/dynamodb/dynamodb_internal_test.go

@@ -7,7 +7,9 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
+	"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"
 	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
+	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
 	guuid "github.com/gofrs/uuid"
 	. "github.com/smartystreets/goconvey/convey"
 
@@ -100,4 +102,196 @@ func TestWrapperErrors(t *testing.T) {
 		err = dynamoWrapper.deleteTable(dynamoWrapper.RepoMetaTablename)
 		So(err, ShouldBeNil)
 	})
+
+	Convey("Create version table behavior", t, func() {
+		customResolver := aws.EndpointResolverWithOptionsFunc( //nolint: staticcheck
+			func(service, region string, options ...any) (aws.Endpoint, error) {
+				return aws.Endpoint{ //nolint: staticcheck
+					PartitionID:   "aws",
+					URL:           endpoint,
+					SigningRegion: region,
+				}, nil
+			},
+		)
+
+		cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(region),
+			config.WithEndpointResolverWithOptions(customResolver)) //nolint: staticcheck
+		So(err, ShouldBeNil)
+
+		Convey("createVersionTable sets version for new table", func() {
+			uuid, err := guuid.NewV4()
+			So(err, ShouldBeNil)
+			versionTablename := "Version" + uuid.String()
+
+			dynamoWrapper := DynamoDB{
+				Client:           dynamodb.NewFromConfig(cfg),
+				VersionTablename: versionTablename,
+				Patches:          version.GetDynamoDBPatches(),
+				Log:              log.NewTestLogger(),
+			}
+
+			// Create version table - should set version
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+			defer func() {
+				_ = dynamoWrapper.deleteTable(versionTablename)
+			}()
+
+			// Verify version was set
+			actualVersion, err := getVersion(dynamoWrapper.Client, versionTablename)
+			So(err, ShouldBeNil)
+			So(actualVersion, ShouldEqual, version.CurrentVersion)
+		})
+
+		Convey("createVersionTable sets version when table exists but version doesn't", func() {
+			uuid, err := guuid.NewV4()
+			So(err, ShouldBeNil)
+			versionTablename := "Version" + uuid.String()
+
+			dynamoWrapper := DynamoDB{
+				Client:           dynamodb.NewFromConfig(cfg),
+				VersionTablename: versionTablename,
+				Patches:          version.GetDynamoDBPatches(),
+				Log:              log.NewTestLogger(),
+			}
+
+			// Create table first without version
+			err = dynamoWrapper.createTable(versionTablename)
+			So(err, ShouldBeNil)
+			defer func() {
+				_ = dynamoWrapper.deleteTable(versionTablename)
+			}()
+
+			// Now create version table - should set version even though table exists
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+
+			// Verify version was set
+			actualVersion, err := getVersion(dynamoWrapper.Client, versionTablename)
+			So(err, ShouldBeNil)
+			So(actualVersion, ShouldEqual, version.CurrentVersion)
+		})
+
+		Convey("createVersionTable does not overwrite existing version", func() {
+			uuid, err := guuid.NewV4()
+			So(err, ShouldBeNil)
+			versionTablename := "Version" + uuid.String()
+
+			dynamoWrapper := DynamoDB{
+				Client:           dynamodb.NewFromConfig(cfg),
+				VersionTablename: versionTablename,
+				Patches:          version.GetDynamoDBPatches(),
+				Log:              log.NewTestLogger(),
+			}
+
+			// Create version table first - sets version to CurrentVersion
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+			defer func() {
+				_ = dynamoWrapper.deleteTable(versionTablename)
+			}()
+
+			// Manually set a different version
+			err = setVersion(dynamoWrapper.Client, versionTablename, "V2")
+			So(err, ShouldBeNil)
+
+			// Verify version is V2
+			actualVersion, err := getVersion(dynamoWrapper.Client, versionTablename)
+			So(err, ShouldBeNil)
+			So(actualVersion, ShouldEqual, "V2")
+
+			// Call createVersionTable again - should not overwrite existing version
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+
+			// Verify version is still V2, not overwritten
+			actualVersion, err = getVersion(dynamoWrapper.Client, versionTablename)
+			So(err, ShouldBeNil)
+			So(actualVersion, ShouldEqual, "V2")
+		})
+
+		Convey("createVersionTable is idempotent - can be called multiple times", func() {
+			uuid, err := guuid.NewV4()
+			So(err, ShouldBeNil)
+			versionTablename := "Version" + uuid.String()
+
+			dynamoWrapper := DynamoDB{
+				Client:           dynamodb.NewFromConfig(cfg),
+				VersionTablename: versionTablename,
+				Patches:          version.GetDynamoDBPatches(),
+				Log:              log.NewTestLogger(),
+			}
+
+			// Call createVersionTable multiple times
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+			defer func() {
+				_ = dynamoWrapper.deleteTable(versionTablename)
+			}()
+
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+
+			err = dynamoWrapper.createVersionTable()
+			So(err, ShouldBeNil)
+
+			// Verify version is set correctly
+			actualVersion, err := getVersion(dynamoWrapper.Client, versionTablename)
+			So(err, ShouldBeNil)
+			So(actualVersion, ShouldEqual, version.CurrentVersion)
+		})
+	})
+}
+
+// Helper function to get version from DynamoDB
+func getVersion(client *dynamodb.Client, versionTablename string) (string, error) {
+	resp, err := client.GetItem(context.TODO(), &dynamodb.GetItemInput{
+		TableName: aws.String(versionTablename),
+		Key: map[string]types.AttributeValue{
+			"TableKey": &types.AttributeValueMemberS{
+				Value: version.DBVersionKey,
+			},
+		},
+	})
+	if err != nil {
+		return "", err
+	}
+
+	if resp.Item == nil {
+		return "", nil
+	}
+
+	var versionValue string
+	err = attributevalue.Unmarshal(resp.Item["Version"], &versionValue)
+	if err != nil {
+		return "", err
+	}
+
+	return versionValue, nil
+}
+
+// Helper function to set version in DynamoDB
+func setVersion(client *dynamodb.Client, versionTablename string, versionValue string) error {
+	mdAttributeValue, err := attributevalue.Marshal(versionValue)
+	if err != nil {
+		return err
+	}
+
+	_, err = client.UpdateItem(context.TODO(), &dynamodb.UpdateItemInput{
+		ExpressionAttributeNames: map[string]string{
+			"#V": "Version",
+		},
+		ExpressionAttributeValues: map[string]types.AttributeValue{
+			":Version": mdAttributeValue,
+		},
+		Key: map[string]types.AttributeValue{
+			"TableKey": &types.AttributeValueMemberS{
+				Value: version.DBVersionKey,
+			},
+		},
+		TableName:        aws.String(versionTablename),
+		UpdateExpression: aws.String("SET #V = :Version"),
+	})
+
+	return err
 }