feat(s3): add EventBridge bucket notifications #18150

yerzhan7 · 2021-12-23T11:30:31Z

Description

Adds EventBridge bucket notification configuration.

See https://aws.amazon.com/blogs/aws/new-use-amazon-s3-event-notifications-with-amazon-eventbridge/

Implementation

Added new Bucket property to enable this feature (eventBridgeEnabled: true)
Added EventBridge config to S3BucketNotifications Custom Resource
Added unit tests
Added integration test (currently fails, see below for more info)
Fixed dependent integration tests

Closes #18076

FAQ

Why not simply expose EventBridge Cfn property via S3 BucketProps?

Currently CDK manages NotificationConfigurations via CustomResource. If we were to expose that way, then e.g. SNS configuration would override EventBridge config.

Why not create new IBucketNotificationDestination class for EventBridge?

We can, but there is no need. Usually we create a subclass to IBucketNotificationDestination in order to adjust resource permissions, however in this case there is no need to adjust permissions: default EventBridge does not require any additional permissions unlike SQS/SNS/Lambda destinations. Additionally, enabling this feature via bucket props is much cleaner/simpler API than creating new dummy object of type IBucketNotificationDestination for customers.

However, if you still think that we need to create new IBucketNotificationDestination subclass for EventBridge for consistency, let me know and I will refactor.

BLOCKED ON LAMBDA RUNTIME SDK UPDATE TO BOTOCORE >= v1.23.16 (Integration test currently fails as current version (v1.21.55) does not contain EventBridge configuration)

Check latest version here: https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2021-12-23T11:30:33Z

mergify · 2021-12-23T11:31:04Z

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

mergify · 2022-01-10T11:28:36Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-01-10T12:13:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 62e47e5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-01-10T12:14:22Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

yerzhan7 · 2022-01-11T05:20:38Z

@otaviomacedo , thanks for reviewing this PR.

However, as I mentioned in the description above, this feature is not working yet due to old AWS SDK version in Lambda.

Let's revert and wait for SDK update?

orekav · 2022-01-17T13:59:23Z

Is there an expected release date for the SDK update?

I want to use this for StepFunctions but CloudFormation does not like it
Received response status [FAILED] from custom resource. Message returned: Error: Parameter validation failed: Unknown parameter in NotificationConfiguration: "EventBridgeConfiguration", must be one of: TopicConfigurations, QueueConfigurations, LambdaFunctionConfigurations

rankovr · 2022-01-18T12:27:18Z

Hi team, I have the same problem as orekav:
I have upgraded my CDK to 1.139.0, but it is failing during the deploy with an error:
Unknown parameter in NotificationConfiguration: "EventBridgeConfiguration", must be one of: TopicConfigurations, QueueConfigurations, LambdaFunctionConfigurations.
my CDK version: 1.139.0, region: eu-central-1
https://docs.aws.amazon.com/cdk/api/v1/python/aws_cdk.aws_s3/README.html
bucket = s3.Bucket(self, "MyEventBridgeBucket",
event_bridge_enabled=True
)

This reverts commit 912aeda. ## Why? [PR#18150](#18150) was merged prematurely (See comments). To avoid customer confusion this PR reverts it. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

In #18150, a change was merged that blew up the size of the inline Lambda beyond its limit of 4096 characters. This change was not detected because the Lambda constructs being used there didn't use the regular `aws-lambda` module, but escape hatches that bypass the regular validation (released in 1.139.0, 2.8.0). Because this effectively broke S3 notifications, it was rolled back in #18507 (released in 1.140.0, not yet released in 2.x line). In this PR, add validation to make sure an event like this doesn't happen again. This will be relevant for #18614.

In #18150, a change was merged that blew up the size of the inline Lambda beyond its limit of 4096 characters. This change was not detected because the Lambda constructs being used there didn't use the regular `aws-lambda` module, but escape hatches that bypass the regular validation (released in 1.139.0, 2.8.0). Because this effectively broke S3 notifications, it was rolled back in #18507 (released in 1.140.0, not yet released in 2.x line). In this PR, add validation to make sure an event like this doesn't happen again. This will be relevant for #18614. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

### **Description** Adds EventBridge bucket notification configuration. See https://aws.amazon.com/blogs/aws/new-use-amazon-s3-event-notifications-with-amazon-eventbridge/ ### **Implementation** - Added new Bucket property to enable this feature (`eventBridgeEnabled: true`) - Added EventBridge config to `S3BucketNotifications` Custom Resource - Added unit tests - Added integration test (currently fails, see below for more info) - Fixed dependent integration tests Closes aws#18076 ### **FAQ** 1. **Why not simply expose EventBridge Cfn property via S3 BucketProps?** Currently CDK manages `NotificationConfigurations `via CustomResource. If we were to expose that way, then e.g. SNS configuration would override EventBridge config. 2. **Why not create new `IBucketNotificationDestination` class for EventBridge?** We can, but there is no need. Usually we create a subclass to `IBucketNotificationDestination` in order to adjust resource permissions, however in this case there is no need to adjust permissions: [default EventBridge does not require any additional permissions](https://docs.aws.amazon.com/AmazonS3/latest/userguide/ev-permissions.html) unlike SQS/SNS/Lambda destinations. Additionally, enabling this feature via bucket props is much cleaner/simpler API than creating new dummy object of type `IBucketNotificationDestination` for customers. However, if you still think that we need to create new `IBucketNotificationDestination` subclass for EventBridge for consistency, let me know and I will refactor. ---- **BLOCKED ON LAMBDA RUNTIME SDK UPDATE TO BOTOCORE >= v1.23.16 (Integration test currently fails as current version (v1.21.55) does not contain EventBridge configuration)** Check latest version here: https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

This reverts commit 912aeda. ## Why? [PR#18150](aws#18150) was merged prematurely (See comments). To avoid customer confusion this PR reverts it. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

In aws#18150, a change was merged that blew up the size of the inline Lambda beyond its limit of 4096 characters. This change was not detected because the Lambda constructs being used there didn't use the regular `aws-lambda` module, but escape hatches that bypass the regular validation (released in 1.139.0, 2.8.0). Because this effectively broke S3 notifications, it was rolled back in aws#18507 (released in 1.140.0, not yet released in 2.x line). In this PR, add validation to make sure an event like this doesn't happen again. This will be relevant for aws#18614. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

## Duplicate of #18150 ## ~~Blocked on Lambda runtime SDK update to Botocore >= v1.23.16 (Integration test currently fails as current version (v1.21.55) does not contain EventBridge configuration)~~ ## ~~Check latest version here: https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html~~ ### **Description** Adds EventBridge bucket notification configuration. See https://aws.amazon.com/blogs/aws/new-use-amazon-s3-event-notifications-with-amazon-eventbridge/ ### **Implementation** - Added new Bucket property to enable this feature (`eventBridgeEnabled: true`) - Added EventBridge config to `S3BucketNotifications` Custom Resource - Added unit tests - Added integration test (currently fails, see below for more info) - Fixed dependent integration tests Closes #18076 ### **FAQ** 1. **Why not simply expose EventBridge Cfn property via S3 BucketProps?** Currently CDK manages `NotificationConfigurations `via CustomResource. If we were to expose that way, then e.g. SNS configuration would override EventBridge config. 2. **Why not create new `IBucketNotificationDestination` class for EventBridge?** We can, but there is no need. Usually we create a subclass to `IBucketNotificationDestination` in order to adjust resource permissions, however in this case there is no need to adjust permissions: [default EventBridge does not require any additional permissions](https://docs.aws.amazon.com/AmazonS3/latest/userguide/ev-permissions.html) unlike SQS/SNS/Lambda destinations. Additionally, enabling this feature via bucket props is much cleaner/simpler API than creating new dummy object of type `IBucketNotificationDestination` for customers. However, if you still think that we need to create new `IBucketNotificationDestination` subclass for EventBridge for consistency, let me know and I will refactor. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

## Duplicate of aws#18150 ## ~~Blocked on Lambda runtime SDK update to Botocore >= v1.23.16 (Integration test currently fails as current version (v1.21.55) does not contain EventBridge configuration)~~ ## ~~Check latest version here: https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html~~ ### **Description** Adds EventBridge bucket notification configuration. See https://aws.amazon.com/blogs/aws/new-use-amazon-s3-event-notifications-with-amazon-eventbridge/ ### **Implementation** - Added new Bucket property to enable this feature (`eventBridgeEnabled: true`) - Added EventBridge config to `S3BucketNotifications` Custom Resource - Added unit tests - Added integration test (currently fails, see below for more info) - Fixed dependent integration tests Closes aws#18076 ### **FAQ** 1. **Why not simply expose EventBridge Cfn property via S3 BucketProps?** Currently CDK manages `NotificationConfigurations `via CustomResource. If we were to expose that way, then e.g. SNS configuration would override EventBridge config. 2. **Why not create new `IBucketNotificationDestination` class for EventBridge?** We can, but there is no need. Usually we create a subclass to `IBucketNotificationDestination` in order to adjust resource permissions, however in this case there is no need to adjust permissions: [default EventBridge does not require any additional permissions](https://docs.aws.amazon.com/AmazonS3/latest/userguide/ev-permissions.html) unlike SQS/SNS/Lambda destinations. Additionally, enabling this feature via bucket props is much cleaner/simpler API than creating new dummy object of type `IBucketNotificationDestination` for customers. However, if you still think that we need to create new `IBucketNotificationDestination` subclass for EventBridge for consistency, let me know and I will refactor. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

(aws-s3): add EventBridge bucket notifications

9629a23

github-actions bot assigned otaviomacedo Dec 23, 2021

github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Dec 23, 2021

yerzhan7 changed the title ~~(aws-s3): add EventBridge bucket notifications~~ feat(s3): add EventBridge bucket notifications Dec 23, 2021

yerzhan7 added 4 commits December 23, 2021 17:40

Add EventBridge section in ReadMe

21c3de3

Fix aws-s3-notifications integration tests

ecda8c7

Fix aws-lambda-event-sources integration tests

3fadf6e

Fix integ. test runtime

ab44887

otaviomacedo approved these changes Jan 10, 2022

View reviewed changes

Merge branch 'master' into s3-eventbridge

62e47e5

mergify bot merged commit 912aeda into aws:master Jan 10, 2022

yerzhan7 mentioned this pull request Jan 18, 2022

revert(s3): add EventBridge bucket notifications (#18150) #18507

Merged

yerzhan7 mentioned this pull request Jan 22, 2022

feat(s3): EventBridge bucket notifications #18614

Merged

rix0rrr mentioned this pull request Jan 26, 2022

chore(s3): ensure Lambda size doesn't grow too large #18660

Merged

mickychetta mentioned this pull request Mar 1, 2022

feat(aws-s3-stepfunctions): Replace EventBridge enabled property override with prop awslabs/aws-solutions-constructs#565

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(s3): add EventBridge bucket notifications #18150

feat(s3): add EventBridge bucket notifications #18150

yerzhan7 commented Dec 23, 2021 •

edited

Loading

gitpod-io bot commented Dec 23, 2021

mergify bot commented Dec 23, 2021

mergify bot commented Jan 10, 2022

aws-cdk-automation commented Jan 10, 2022

mergify bot commented Jan 10, 2022

yerzhan7 commented Jan 11, 2022

orekav commented Jan 17, 2022

rankovr commented Jan 18, 2022

feat(s3): add EventBridge bucket notifications #18150

feat(s3): add EventBridge bucket notifications #18150

Conversation

yerzhan7 commented Dec 23, 2021 • edited Loading

Description

Implementation

FAQ

gitpod-io bot commented Dec 23, 2021

mergify bot commented Dec 23, 2021

mergify bot commented Jan 10, 2022

aws-cdk-automation commented Jan 10, 2022

AWS CodeBuild CI Report

mergify bot commented Jan 10, 2022

yerzhan7 commented Jan 11, 2022

orekav commented Jan 17, 2022

rankovr commented Jan 18, 2022

yerzhan7 commented Dec 23, 2021 •

edited

Loading