From ca1cbf8c7e0ee8cf3b8bd3e0197ab233df39fa97 Mon Sep 17 00:00:00 2001 From: Irene Li Date: Tue, 26 Nov 2024 11:47:11 -0800 Subject: [PATCH 1/6] Fix issue with UpdateTable command missing entries by extracting table schema after update --- localstack-core/localstack/services/dynamodb/provider.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/localstack-core/localstack/services/dynamodb/provider.py b/localstack-core/localstack/services/dynamodb/provider.py index cac009a009b6a..72fcfbce03519 100644 --- a/localstack-core/localstack/services/dynamodb/provider.py +++ b/localstack-core/localstack/services/dynamodb/provider.py @@ -851,6 +851,10 @@ def update_table( SchemaExtractor.invalidate_table_schema(table_name, context.account_id, global_table_region) + schema = SchemaExtractor.get_table_schema( + table_name, context.account_id, global_table_region + ) + # TODO: DDB streams must also be created for replicas if update_table_input.get("StreamSpecification"): create_dynamodb_stream( @@ -860,7 +864,7 @@ def update_table( result["TableDescription"].get("LatestStreamLabel"), ) - return result + return UpdateTableOutput(TableDescription=schema["Table"]) def list_tables( self, From baf822f02292d31382595115dcd937224e46f3fa Mon Sep 17 00:00:00 2001 From: Irene Li Date: Fri, 6 Dec 2024 23:50:21 -0800 Subject: [PATCH 2/6] Add dynamodb aws service test through update table without changing sse specification --- tests/aws/services/dynamodb/test_dynamodb.py | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/aws/services/dynamodb/test_dynamodb.py b/tests/aws/services/dynamodb/test_dynamodb.py index 07651bb388ca2..fc978591140d4 100644 --- a/tests/aws/services/dynamodb/test_dynamodb.py +++ b/tests/aws/services/dynamodb/test_dynamodb.py @@ -4,6 +4,7 @@ from datetime import datetime from time import sleep from typing import Dict +import sys import botocore.exceptions import pytest @@ -1653,6 +1654,37 @@ def test_dynamodb_create_table_with_partial_sse_specification( result = aws_client.dynamodb.describe_table(TableName=table_name) assert "SSESpecification" not in result["Table"] + @markers.aws.validated + def test_dynamodb_update_table_without_sse_specification_change( + self, dynamodb_create_table_with_parameters, aws_client, account_id, region_name + ): + table_name = f"ddb-table-{short_uid()}" + + kms_master_key_id = long_uid() + sse_specification = {"Enabled": True, "SSEType": "KMS", "KMSMasterKeyId": kms_master_key_id} + kms_master_key_arn = arns.kms_key_arn( + kms_master_key_id, account_id=account_id, region_name=region_name + ) + + _ = dynamodb_create_table_with_parameters( + TableName=table_name, + KeySchema=[{"AttributeName": PARTITION_KEY, "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": PARTITION_KEY, "AttributeType": "S"}], + ProvisionedThroughput={"ReadCapacityUnits": 5, "WriteCapacityUnits": 5}, + SSESpecification=sse_specification, + Tags=TEST_DDB_TAGS, + ) + + update_result = aws_client.dynamodb.update_table( + TableName=table_name, + BillingMode="PAY_PER_REQUEST" + ) + + assert update_result["TableDescription"]["SSEDescription"] + assert update_result["TableDescription"]["SSEDescription"]["Status"] == "ENABLED" + assert update_result['TableDescription']['SSEDescription']['SSEType'] == 'KMS' + assert update_result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] == kms_master_key_arn + @markers.aws.validated def test_dynamodb_get_batch_items( self, dynamodb_create_table_with_parameters, snapshot, aws_client From ca9b98a2a08a0b69cdf67578ac19f7ae244ec438 Mon Sep 17 00:00:00 2001 From: Irene Li Date: Fri, 6 Dec 2024 23:53:21 -0800 Subject: [PATCH 3/6] Reformat test_dynamodb_update_table_without_sse_specification_change tester --- tests/aws/services/dynamodb/test_dynamodb.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/aws/services/dynamodb/test_dynamodb.py b/tests/aws/services/dynamodb/test_dynamodb.py index fc978591140d4..31c4947b98a8e 100644 --- a/tests/aws/services/dynamodb/test_dynamodb.py +++ b/tests/aws/services/dynamodb/test_dynamodb.py @@ -4,7 +4,6 @@ from datetime import datetime from time import sleep from typing import Dict -import sys import botocore.exceptions import pytest @@ -1676,14 +1675,16 @@ def test_dynamodb_update_table_without_sse_specification_change( ) update_result = aws_client.dynamodb.update_table( - TableName=table_name, - BillingMode="PAY_PER_REQUEST" + TableName=table_name, BillingMode="PAY_PER_REQUEST" ) assert update_result["TableDescription"]["SSEDescription"] assert update_result["TableDescription"]["SSEDescription"]["Status"] == "ENABLED" - assert update_result['TableDescription']['SSEDescription']['SSEType'] == 'KMS' - assert update_result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] == kms_master_key_arn + assert update_result["TableDescription"]["SSEDescription"]["SSEType"] == "KMS" + assert ( + update_result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] + == kms_master_key_arn + ) @markers.aws.validated def test_dynamodb_get_batch_items( From b5de0af7932f050f544d681a0ee5e081bb51f03f Mon Sep 17 00:00:00 2001 From: Irene Li Date: Sat, 7 Dec 2024 00:06:11 -0800 Subject: [PATCH 4/6] Implement missing SSESpecification update_table changes on dynamodb v2 --- .../localstack/services/dynamodb/v2/provider.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/localstack-core/localstack/services/dynamodb/v2/provider.py b/localstack-core/localstack/services/dynamodb/v2/provider.py index 500c07f4c201d..c264febc672c5 100644 --- a/localstack-core/localstack/services/dynamodb/v2/provider.py +++ b/localstack-core/localstack/services/dynamodb/v2/provider.py @@ -614,7 +614,7 @@ def update_table( global_table_region = self.get_global_table_region(context, table_name) try: - result = self._forward_request(context=context, region=global_table_region) + self._forward_request(context=context, region=global_table_region) except CommonServiceException as exc: # DynamoDBLocal refuses to update certain table params and raises. # But we still need to update this info in LocalStack stores @@ -689,7 +689,11 @@ def update_table( SchemaExtractor.invalidate_table_schema(table_name, context.account_id, global_table_region) - return result + schema = SchemaExtractor.get_table_schema( + table_name, context.account_id, global_table_region + ) + + return UpdateTableOutput(TableDescription=schema["Table"]) def list_tables( self, From de0d977e0a659ee3760b0f01fd6ca18187444e75 Mon Sep 17 00:00:00 2001 From: Irene Li Date: Mon, 9 Dec 2024 08:35:23 -0800 Subject: [PATCH 5/6] Convert test_dynamodb_update_table_without_sse_specification_change to parity test and verify the behaviour --- tests/aws/services/dynamodb/test_dynamodb.py | 30 +++++++------- .../dynamodb/test_dynamodb.snapshot.json | 39 +++++++++++++++++++ .../dynamodb/test_dynamodb.validation.json | 3 ++ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/tests/aws/services/dynamodb/test_dynamodb.py b/tests/aws/services/dynamodb/test_dynamodb.py index 31c4947b98a8e..9a25c1f5aaa46 100644 --- a/tests/aws/services/dynamodb/test_dynamodb.py +++ b/tests/aws/services/dynamodb/test_dynamodb.py @@ -1655,17 +1655,13 @@ def test_dynamodb_create_table_with_partial_sse_specification( @markers.aws.validated def test_dynamodb_update_table_without_sse_specification_change( - self, dynamodb_create_table_with_parameters, aws_client, account_id, region_name + self, dynamodb_create_table_with_parameters, snapshot, aws_client ): - table_name = f"ddb-table-{short_uid()}" + table_name = f"test_table_{short_uid()}" - kms_master_key_id = long_uid() - sse_specification = {"Enabled": True, "SSEType": "KMS", "KMSMasterKeyId": kms_master_key_id} - kms_master_key_arn = arns.kms_key_arn( - kms_master_key_id, account_id=account_id, region_name=region_name - ) + sse_specification = {"Enabled": True} - _ = dynamodb_create_table_with_parameters( + result = dynamodb_create_table_with_parameters( TableName=table_name, KeySchema=[{"AttributeName": PARTITION_KEY, "KeyType": "HASH"}], AttributeDefinitions=[{"AttributeName": PARTITION_KEY, "AttributeType": "S"}], @@ -1673,16 +1669,24 @@ def test_dynamodb_update_table_without_sse_specification_change( SSESpecification=sse_specification, Tags=TEST_DDB_TAGS, ) + snapshot.match("SSEDescription", result["TableDescription"]["SSEDescription"]) + + kms_master_key_arn = result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] + result = aws_client.kms.describe_key(KeyId=kms_master_key_arn) + snapshot.match("KMSDescription", result) - update_result = aws_client.dynamodb.update_table( + result = aws_client.dynamodb.update_table( TableName=table_name, BillingMode="PAY_PER_REQUEST" ) + snapshot.match( + "update-table-unchanged-sse-spec", result["TableDescription"]["SSEDescription"] + ) - assert update_result["TableDescription"]["SSEDescription"] - assert update_result["TableDescription"]["SSEDescription"]["Status"] == "ENABLED" - assert update_result["TableDescription"]["SSEDescription"]["SSEType"] == "KMS" + # Verify that SSEDescription exists and remains unchanged after update_table + assert result["TableDescription"]["SSEDescription"]["Status"] == "ENABLED" + assert result["TableDescription"]["SSEDescription"]["SSEType"] == "KMS" assert ( - update_result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] + result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] == kms_master_key_arn ) diff --git a/tests/aws/services/dynamodb/test_dynamodb.snapshot.json b/tests/aws/services/dynamodb/test_dynamodb.snapshot.json index 2ad2829c47716..04796e5e5ac11 100644 --- a/tests/aws/services/dynamodb/test_dynamodb.snapshot.json +++ b/tests/aws/services/dynamodb/test_dynamodb.snapshot.json @@ -1417,5 +1417,44 @@ ] } } + }, + "tests/aws/services/dynamodb/test_dynamodb.py::TestDynamoDB::test_dynamodb_update_table_without_sse_specification_change": { + "recorded-date": "09-12-2024, 16:09:53", + "recorded-content": { + "SSEDescription": { + "KMSMasterKeyArn": "arn::kms::111111111111:key/", + "SSEType": "KMS", + "Status": "ENABLED" + }, + "KMSDescription": { + "KeyMetadata": { + "AWSAccountId": "111111111111", + "Arn": "arn::kms::111111111111:key/", + "CreationDate": "datetime", + "CustomerMasterKeySpec": "SYMMETRIC_DEFAULT", + "Description": "Default key that protects my DynamoDB data when no other key is defined", + "Enabled": true, + "EncryptionAlgorithms": [ + "SYMMETRIC_DEFAULT" + ], + "KeyId": "", + "KeyManager": "AWS", + "KeySpec": "SYMMETRIC_DEFAULT", + "KeyState": "Enabled", + "KeyUsage": "ENCRYPT_DECRYPT", + "MultiRegion": false, + "Origin": "AWS_KMS" + }, + "ResponseMetadata": { + "HTTPHeaders": {}, + "HTTPStatusCode": 200 + } + }, + "update-table-unchanged-sse-spec": { + "KMSMasterKeyArn": "arn::kms::111111111111:key/", + "SSEType": "KMS", + "Status": "ENABLED" + } + } } } diff --git a/tests/aws/services/dynamodb/test_dynamodb.validation.json b/tests/aws/services/dynamodb/test_dynamodb.validation.json index ef5f8c6df8514..423483aefc2e2 100644 --- a/tests/aws/services/dynamodb/test_dynamodb.validation.json +++ b/tests/aws/services/dynamodb/test_dynamodb.validation.json @@ -53,6 +53,9 @@ "tests/aws/services/dynamodb/test_dynamodb.py::TestDynamoDB::test_dynamodb_streams_describe_with_exclusive_start_shard_id": { "last_validated_date": "2023-10-22T20:27:28+00:00" }, + "tests/aws/services/dynamodb/test_dynamodb.py::TestDynamoDB::test_dynamodb_update_table_without_sse_specification_change": { + "last_validated_date": "2024-12-09T16:09:21+00:00" + }, "tests/aws/services/dynamodb/test_dynamodb.py::TestDynamoDB::test_empty_and_binary_values": { "last_validated_date": "2023-08-23T14:32:29+00:00" }, From ad219daad2573b1bccf49d2a250fe3f8ab276089 Mon Sep 17 00:00:00 2001 From: Irene Li Date: Mon, 9 Dec 2024 18:31:18 -0800 Subject: [PATCH 6/6] Reformat test_dynamodb_update_table_without_sse_specification_change tester --- tests/aws/services/dynamodb/test_dynamodb.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/aws/services/dynamodb/test_dynamodb.py b/tests/aws/services/dynamodb/test_dynamodb.py index 9a25c1f5aaa46..6bc47a51d4907 100644 --- a/tests/aws/services/dynamodb/test_dynamodb.py +++ b/tests/aws/services/dynamodb/test_dynamodb.py @@ -1685,10 +1685,7 @@ def test_dynamodb_update_table_without_sse_specification_change( # Verify that SSEDescription exists and remains unchanged after update_table assert result["TableDescription"]["SSEDescription"]["Status"] == "ENABLED" assert result["TableDescription"]["SSEDescription"]["SSEType"] == "KMS" - assert ( - result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] - == kms_master_key_arn - ) + assert result["TableDescription"]["SSEDescription"]["KMSMasterKeyArn"] == kms_master_key_arn @markers.aws.validated def test_dynamodb_get_batch_items(