Skip to content

Commit

Permalink
fix(s3-notifications): unable to delete the existing S3 event notific…
Browse files Browse the repository at this point in the history
…ations (under feature flag) (#30610)

### Issue # (if applicable)

Closes #28915 

### Reason for this change

Fix to address the issues deleting the existing S3 event notifications and adding new event notifications on top of existing notifications.

### Description of changes
We fixed the hashing logic used to identify old vs external S3 event notifications

### Description of how you validated changes
Manually tested, integration and unit tested the changes

yes
### Checklist
- [x] 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
sarangarav authored Jun 26, 2024
1 parent 1ad41e7 commit b880067
Show file tree
Hide file tree
Showing 56 changed files with 37,704 additions and 2,461 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-integ-test-imported-bucket-name",
"BucketName": "cdk-integration-test-imported-bucket-name",
"NotificationConfiguration": {
"LambdaFunctionConfigurations": [
{
Expand All @@ -25,7 +25,8 @@
}
]
},
"Managed": false
"Managed": false,
"ApplyNameTransformations": true
},
"DependsOn": [
"ImportedAllowBucketNotificationsToTestStackF6B9A922242C13EDE"
Expand Down Expand Up @@ -53,7 +54,7 @@
{
"Ref": "AWS::Partition"
},
":s3:::cdk-integ-test-imported-bucket-name"
":s3:::cdk-integration-test-imported-bucket-name"
]
]
}
Expand Down Expand Up @@ -169,7 +170,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 == '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"
"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 applyNameTransformations = props.get('ApplyNameTransformations', 'false').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, applyNameTransformations)\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, applyNameTransformations):\n def get_id(n):\n n['Id'] = ''\n strToHash=json.dumps(n, sort_keys=True).replace('\"Name\": \"prefix\"', '\"Name\": \"Prefix\"').replace('\"Name\": \"suffix\"', '\"Name\": \"Suffix\"')\n return f\"{stack_id}-{hash(strToHash)}\"\n def with_id(n):\n if applyNameTransformations:\n n['Id'] = get_id(n)\n else:\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 if applyNameTransformations:\n old_incoming_ids = [get_id(n) for n in old.get(t, [])]\n external_notifications[t] = [n for n in existing_notifications.get(t, []) if not get_id(n) in old_incoming_ids]\n else:\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))"
},
"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-integ-test-imported-bucket-name"
"BucketName": "cdk-integration-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.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { TestFunction } from './test-function';
import { IntegTest } from '@aws-cdk/integ-tests-alpha';
import { Construct } from 'constructs';

const bucketName = 'cdk-integ-test-imported-bucket-name';
const bucketName = 'cdk-integration-test-imported-bucket-name';
class TestStack extends Stack {
constructor(scope: Construct, id: string) {
super(scope, id);
Expand All @@ -30,7 +30,11 @@ class TestStack extends Stack {
}
}

const app = new App();
const app = new App({
postCliContext: {
'@aws-cdk/aws-s3:s3ExistingNotificationsDeleteEnabled': true,
},
});

new IntegTest(app, 'issue-4324', {
testCases: [
Expand Down

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

Loading

0 comments on commit b880067

Please sign in to comment.