Skip to content

Commit be4be99

Browse files
authored
fix(s3-deployment): physical id not set during failure scenario (#24428)
Send the correct physical id to CloudFormation to prevent unintended object deletion during rollback. Closes #22670 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent dc064be commit be4be99

File tree

2 files changed

+19
-1
lines changed

2 files changed

+19
-1
lines changed

Diff for: packages/@aws-cdk/aws-s3-deployment/lib/lambda/index.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ def handler(event, context):
2828

2929
def cfn_error(message=None):
3030
logger.error("| cfn_error: %s" % message)
31-
cfn_send(event, context, CFN_FAILED, reason=message)
31+
cfn_send(event, context, CFN_FAILED, reason=message, physicalResourceId=event.get('PhysicalResourceId', None))
32+
3233

3334
try:
3435
# We are not logging ResponseURL as this is a pre-signed S3 URL, and could be used to tamper

Diff for: packages/@aws-cdk/aws-s3-deployment/test/lambda/test.py

+17
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,23 @@ def mock_make_api_call(self, operation_name, kwarg):
595595

596596
self.assertEqual(update_resp['Reason'], "invalid request: request type is 'Delete' but 'PhysicalResourceId' is not defined")
597597

598+
def test_physical_id_on_cloud_front_error(self):
599+
def mock_make_api_call(self, operation_name, kwarg):
600+
if operation_name == 'CreateInvalidation':
601+
raise ClientError({'Error': {'Code': '500', 'Message': 'Invalidation error'}}, operation_name)
602+
603+
with patch('botocore.client.BaseClient._make_api_call', new=mock_make_api_call):
604+
resp = invoke_handler("Update", {
605+
"SourceBucketNames": ["<source-bucket>"],
606+
"SourceObjectKeys": ["<source-object-key>"],
607+
"DestinationBucketName": "<dest-bucket-name>",
608+
"DistributionId": "<cf-dist-id>",
609+
}, old_resource_props={
610+
"DestinationBucketName": "<dest-bucket-name>",
611+
}, physical_id="<physical-id>", expected_status="FAILED")
612+
613+
self.assertEqual(resp["PhysicalResourceId"], "<physical-id>")
614+
598615
# no bucket tags removes content
599616
def test_no_tags_on_bucket(self):
600617
def mock_make_api_call(self, operation_name, kwarg):

0 commit comments

Comments
 (0)