Skip to content

Commit

Permalink
fix(s3-notifications): cdk destroy deletes external/existing s3 notif…
Browse files Browse the repository at this point in the history
…ication events (#29939)

### Issue # (if applicable)

Closes #29004

### Reason for this change

`cdk destroy` removes all event notifications configured on an existing S3 bucket instead of only CDK managed event notifications. This occurs whenever a stack that creates an event notification for an existing bucket is deleted or rolled back.

### Description of changes

Add a `Delete` statement which will only remove the ones created from within the stack

### Description of how you validated changes

Manually tested this.

### Checklist
- [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
GavinZZ authored Apr 24, 2024
1 parent 209ffd9 commit 7360a88
Show file tree
Hide file tree
Showing 46 changed files with 686 additions and 331 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"Arn"
]
},
"BucketName": "cdk-test-bucket-event-source",
"BucketName": "cdk-integ-test-imported-bucket-name",
"NotificationConfiguration": {
"LambdaFunctionConfigurations": [
{
Expand All @@ -28,10 +28,10 @@
"Managed": false
},
"DependsOn": [
"ImportedAllowBucketNotificationsTotesterFD715D26C4DE5336A"
"ImportedAllowBucketNotificationsToTestStackF6B9A922242C13EDE"
]
},
"ImportedAllowBucketNotificationsTotesterFD715D26C4DE5336A": {
"ImportedAllowBucketNotificationsToTestStackF6B9A922242C13EDE": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand All @@ -53,7 +53,7 @@
{
"Ref": "AWS::Partition"
},
":s3:::cdk-test-bucket-event-source"
":s3:::cdk-integ-test-imported-bucket-name"
]
]
}
Expand Down Expand Up @@ -169,7 +169,7 @@
"Properties": {
"Description": "AWS CloudFormation handler for \"Custom::S3BucketNotifications\" resources (@aws-cdk/aws-s3)",
"Code": {
"ZipFile": "import boto3 # type: ignore\nimport json\nimport logging\nimport urllib.request\n\ns3 = boto3.client(\"s3\")\n\nEVENTBRIDGE_CONFIGURATION = 'EventBridgeConfiguration'\nCONFIGURATION_TYPES = [\"TopicConfigurations\", \"QueueConfigurations\", \"LambdaFunctionConfigurations\"]\n\ndef handler(event: dict, context):\n response_status = \"SUCCESS\"\n error_message = \"\"\n try:\n props = event[\"ResourceProperties\"]\n notification_configuration = props[\"NotificationConfiguration\"]\n managed = props.get('Managed', 'true').lower() == 'true'\n stack_id = event['StackId']\n old = event.get(\"OldResourceProperties\", {}).get(\"NotificationConfiguration\", {})\n if managed:\n config = handle_managed(event[\"RequestType\"], notification_configuration)\n else:\n config = handle_unmanaged(props[\"BucketName\"], stack_id, event[\"RequestType\"], notification_configuration, old)\n s3.put_bucket_notification_configuration(Bucket=props[\"BucketName\"], NotificationConfiguration=config)\n except Exception as e:\n logging.exception(\"Failed to put bucket notification configuration\")\n response_status = \"FAILED\"\n error_message = f\"Error: {str(e)}. \"\n finally:\n submit_response(event, context, response_status, error_message)\n\ndef handle_managed(request_type, notification_configuration):\n if request_type == 'Delete':\n return {}\n return notification_configuration\n\ndef handle_unmanaged(bucket, stack_id, request_type, notification_configuration, old):\n def with_id(n):\n n['Id'] = f\"{stack_id}-{hash(json.dumps(n, sort_keys=True))}\"\n return n\n\n external_notifications = {}\n existing_notifications = s3.get_bucket_notification_configuration(Bucket=bucket)\n for t in CONFIGURATION_TYPES:\n if request_type == 'Update':\n ids = [with_id(n) for n in old.get(t, [])]\n old_incoming_ids = [n['Id'] for n in ids]\n external_notifications[t] = [n for n in existing_notifications.get(t, []) if not n['Id'] in old_incoming_ids]\n elif request_type == 'Create':\n external_notifications[t] = [n for n in existing_notifications.get(t, [])]\n if EVENTBRIDGE_CONFIGURATION in existing_notifications:\n external_notifications[EVENTBRIDGE_CONFIGURATION] = existing_notifications[EVENTBRIDGE_CONFIGURATION]\n\n if request_type == 'Delete':\n return external_notifications\n\n notifications = {}\n for t in CONFIGURATION_TYPES:\n external = external_notifications.get(t, [])\n incoming = [with_id(n) for n in notification_configuration.get(t, [])]\n notifications[t] = external + incoming\n\n if EVENTBRIDGE_CONFIGURATION in notification_configuration:\n notifications[EVENTBRIDGE_CONFIGURATION] = notification_configuration[EVENTBRIDGE_CONFIGURATION]\n elif EVENTBRIDGE_CONFIGURATION in external_notifications:\n notifications[EVENTBRIDGE_CONFIGURATION] = external_notifications[EVENTBRIDGE_CONFIGURATION]\n\n return notifications\n\ndef submit_response(event: dict, context, response_status: str, error_message: str):\n response_body = json.dumps(\n {\n \"Status\": response_status,\n \"Reason\": f\"{error_message}See the details in CloudWatch Log Stream: {context.log_stream_name}\",\n \"PhysicalResourceId\": event.get(\"PhysicalResourceId\") or event[\"LogicalResourceId\"],\n \"StackId\": event[\"StackId\"],\n \"RequestId\": event[\"RequestId\"],\n \"LogicalResourceId\": event[\"LogicalResourceId\"],\n \"NoEcho\": False,\n }\n ).encode(\"utf-8\")\n headers = {\"content-type\": \"\", \"content-length\": str(len(response_body))}\n try:\n req = urllib.request.Request(url=event[\"ResponseURL\"], headers=headers, data=response_body, method=\"PUT\")\n with urllib.request.urlopen(req) as response:\n print(response.read().decode(\"utf-8\"))\n print(\"Status code: \" + response.reason)\n except Exception as e:\n print(\"send(..) failed executing request.urlopen(..): \" + str(e))\n"
"ZipFile": "import boto3 # type: ignore\nimport json\nimport logging\nimport urllib.request\n\ns3 = boto3.client(\"s3\")\n\nEVENTBRIDGE_CONFIGURATION = 'EventBridgeConfiguration'\nCONFIGURATION_TYPES = [\"TopicConfigurations\", \"QueueConfigurations\", \"LambdaFunctionConfigurations\"]\n\ndef handler(event: dict, context):\n response_status = \"SUCCESS\"\n error_message = \"\"\n try:\n props = event[\"ResourceProperties\"]\n notification_configuration = props[\"NotificationConfiguration\"]\n managed = props.get('Managed', 'true').lower() == 'true'\n stack_id = event['StackId']\n old = event.get(\"OldResourceProperties\", {}).get(\"NotificationConfiguration\", {})\n if managed:\n config = handle_managed(event[\"RequestType\"], notification_configuration)\n else:\n config = handle_unmanaged(props[\"BucketName\"], stack_id, event[\"RequestType\"], notification_configuration, old)\n s3.put_bucket_notification_configuration(Bucket=props[\"BucketName\"], NotificationConfiguration=config)\n except Exception as e:\n logging.exception(\"Failed to put bucket notification configuration\")\n response_status = \"FAILED\"\n error_message = f\"Error: {str(e)}. \"\n finally:\n submit_response(event, context, response_status, error_message)\n\ndef handle_managed(request_type, notification_configuration):\n if request_type == 'Delete':\n return {}\n return notification_configuration\n\ndef handle_unmanaged(bucket, stack_id, request_type, notification_configuration, old):\n def with_id(n):\n n['Id'] = f\"{stack_id}-{hash(json.dumps(n, sort_keys=True))}\"\n return n\n\n external_notifications = {}\n existing_notifications = s3.get_bucket_notification_configuration(Bucket=bucket)\n for t in CONFIGURATION_TYPES:\n if request_type == 'Update':\n ids = [with_id(n) for n in old.get(t, [])]\n old_incoming_ids = [n['Id'] for n in ids]\n external_notifications[t] = [n for n in existing_notifications.get(t, []) if not n['Id'] in old_incoming_ids]\n elif request_type == 'Delete':\n external_notifications[t] = [n for n in existing_notifications.get(t, []) if not n['Id'].startswith(f\"{stack_id}-\")]\n elif request_type == 'Create':\n external_notifications[t] = [n for n in existing_notifications.get(t, [])]\n if EVENTBRIDGE_CONFIGURATION in existing_notifications:\n external_notifications[EVENTBRIDGE_CONFIGURATION] = existing_notifications[EVENTBRIDGE_CONFIGURATION]\n\n if request_type == 'Delete':\n return external_notifications\n\n notifications = {}\n for t in CONFIGURATION_TYPES:\n external = external_notifications.get(t, [])\n incoming = [with_id(n) for n in notification_configuration.get(t, [])]\n notifications[t] = external + incoming\n\n if EVENTBRIDGE_CONFIGURATION in notification_configuration:\n notifications[EVENTBRIDGE_CONFIGURATION] = notification_configuration[EVENTBRIDGE_CONFIGURATION]\n elif EVENTBRIDGE_CONFIGURATION in external_notifications:\n notifications[EVENTBRIDGE_CONFIGURATION] = external_notifications[EVENTBRIDGE_CONFIGURATION]\n\n return notifications\n\ndef submit_response(event: dict, context, response_status: str, error_message: str):\n response_body = json.dumps(\n {\n \"Status\": response_status,\n \"Reason\": f\"{error_message}See the details in CloudWatch Log Stream: {context.log_stream_name}\",\n \"PhysicalResourceId\": event.get(\"PhysicalResourceId\") or event[\"LogicalResourceId\"],\n \"StackId\": event[\"StackId\"],\n \"RequestId\": event[\"RequestId\"],\n \"LogicalResourceId\": event[\"LogicalResourceId\"],\n \"NoEcho\": False,\n }\n ).encode(\"utf-8\")\n headers = {\"content-type\": \"\", \"content-length\": str(len(response_body))}\n try:\n req = urllib.request.Request(url=event[\"ResponseURL\"], headers=headers, data=response_body, method=\"PUT\")\n with urllib.request.urlopen(req) as response:\n print(response.read().decode(\"utf-8\"))\n print(\"Status code: \" + response.reason)\n except Exception as e:\n print(\"send(..) failed executing request.urlopen(..): \" + str(e))\n"
},
"Handler": "index.handler",
"Role": {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"bucket43879C71": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "cdk-test-bucket-event-source"
"BucketName": "cdk-integ-test-imported-bucket-name"
},
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 7360a88

Please sign in to comment.