Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(aws-s3): adding prop to enable EventBridge in S3 BucketProps #18076

Closed
1 of 2 tasks
mickychetta opened this issue Dec 17, 2021 · 3 comments · Fixed by #18150 or #18614
Closed
1 of 2 tasks

(aws-s3): adding prop to enable EventBridge in S3 BucketProps #18076

mickychetta opened this issue Dec 17, 2021 · 3 comments · Fixed by #18150 or #18614
Assignees
@otaviomacedo
Labels
@aws-cdk/aws-s3 Related to Amazon S3 feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@mickychetta
Copy link

mickychetta commented Dec 17, 2021 •
edited
Loading

Description

We can now use Amazon S3 Event Notifications with EventBridge to create new event driven applications after this announcement.

In order to take advantage of this feature, S3 must have enable EventBridge in the properties sections:
image

It is a resource in CloudFormation but not a resource in CfnBucket yet.

Use Case

In Solutions Constructs, we have a construct aws-s3-stepfunctions that uses S3 Event Notifications to send to EventBridge then trigger a state machine. At the moment we are using escape hatches to override the prop.

cfnBucket.addPropertyOverride('NotificationConfiguration.EventBridgeConfiguration.EventBridgeEnabled', true);

It is in CloudFormation but not yet in the CDK, It would a nice feature to have a prop to enable EventBridge.

Proposed Solution

Add an optional prop to the CDK S3 BucketProps called eventBridgeEnabled: boolean. The resource is currently missing in CfnBucket and i'm not sure how to approach this but any pointers would be great so I can start working on it.

Another feature up for discussion is if it can be changed for an existing bucket? If it can, then will probably want an enableEventBridge() method on Bucket and IBucket.

Other information

No response

Acknowledge

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@mickychetta mickychetta added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 17, 2021
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Dec 17, 2021
@otaviomacedo
Copy link
Contributor

otaviomacedo commented Dec 18, 2021 •
edited
Loading

We could do that. But if it is in the CloudFormation spec, it should be available in CfnBucket in the next release of the CDK.

@biffgaut biffgaut mentioned this issue Dec 20, 2021
Closed
2 tasks
@yerzhan7 yerzhan7 mentioned this issue Dec 23, 2021
Merged
@michaelbrewer
Copy link
Contributor

Oops, i did not see #18150 when i started #18236

Hopefully @yerzhan7 your PR is reviewed and approved :)

@mergify mergify bot closed this as completed in #18150 Jan 10, 2022
mergify bot pushed a commit that referenced this issue Jan 10, 2022
@yerzhan7
feat(s3): add EventBridge bucket notifications (#18150)
912aeda 
### **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.

----

**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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@mickychetta mickychetta mentioned this issue Jan 10, 2022
Closed
2 tasks
@yerzhan7 yerzhan7 mentioned this issue Jan 22, 2022
Merged
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
@yerzhan7 @TikiTDO
feat(s3): add EventBridge bucket notifications (aws#18150)
1634135 
### **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*
mergify bot pushed a commit that referenced this issue Apr 1, 2022
@yerzhan7
feat(s3): EventBridge bucket notifications (#18614)
d8e602b 
## 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*
This was referenced Apr 7, 2022
Merged
Merged
mergify bot added a commit that referenced this issue Apr 7, 2022
@mergify
chore(release): 1.152.0 (#19781)
9487b39 
See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/1.152.0/CHANGELOG.md)

For convenience, extracted the relevant CHANGELOG entry:

## [1.152.0](v1.151.0...v1.152.0) (2022-04-06)


### Features

* **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9))
* **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64))
* **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667)
* **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209))
* **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e))
* **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5))
* **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e))
* add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010))
* **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df))
* **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605)
* **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076)
* **synthetics:** new puppeteer 3.5 runtime ([#19673](#19673)) ([ce2b91b](ce2b91b)), closes [#19634](#19634)


### Bug Fixes

* **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969)
* **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160)
* **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421)
* **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2))
* **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042))
* **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399)
* **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad))
* **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550)
* **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3))
* **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648)
* **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259)
* **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751)
* **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)
mergify bot added a commit that referenced this issue Apr 7, 2022
@mergify
chore(release): 2.20.0 (#19801)
738ef49 
See [CHANGELOG](https://github.com/aws/aws-cdk/blob/bump/2.20.0/CHANGELOG.md)

For convenience, extracted the relevant CHANGELOG entry:

## [2.20.0](v2.19.0...v2.20.0) (2022-04-07)


### Features

* **cfnspec:** cloudformation spec v63.0.0 ([#19679](#19679)) ([dba96a9](dba96a9))
* **cfnspec:** cloudformation spec v65.0.0 ([#19745](#19745)) ([796fc64](796fc64))
* **cli:** add --build option ([#19663](#19663)) ([eb9b8e2](eb9b8e2)), closes [#19667](#19667)
* **cli:** preview of `cdk import` ([#17666](#17666)) ([4f12209](4f12209))
* **core:** throw error when stack name exceeds max length ([#19725](#19725)) ([1ffd45e](1ffd45e))
* **eks:** add k8s v1.22 ([#19756](#19756)) ([9a518c5](9a518c5))
* **opensearch:** Add latest Opensearch Version 1.2 ([#19749](#19749)) ([a2ac36e](a2ac36e))
* add new integration test runner ([#19754](#19754)) ([1b4d010](1b4d010))
* **eks:** alb-controller v2.4.1 ([#19653](#19653)) ([1ec08df](1ec08df))
* **lambda:** add support for ephemeral storage ([#19552](#19552)) ([f1d9b6a](f1d9b6a)), closes [#19605](#19605)
* **s3:** EventBridge bucket notifications ([#18614](#18614)) ([d8e602b](d8e602b)), closes [#18076](#18076)


### Bug Fixes

* **aws_applicationautoscaling:** Add missing members to PredefinedMetric enum ([#18978](#18978)) ([75a6fa7](75a6fa7)), closes [#18969](#18969)
* **cli:** apps with many resources scroll resource output offscreen ([#19742](#19742)) ([053d22c](053d22c)), closes [#19160](#19160)
* **cli:** support attributes of DynamoDB Tables for hotswapping ([#19620](#19620)) ([2321ece](2321ece)), closes [#19421](#19421)
* **cloudwatch:** automatic metric math label cannot be suppressed ([#17639](#17639)) ([7fa3bf2](7fa3bf2))
* **codedeploy:** add name validation for Application, Deployment Group and Deployment Configuration ([#19473](#19473)) ([9185042](9185042))
* **codedeploy:** the Service Principal is wrong in isolated regions ([#19729](#19729)) ([7e9a43d](7e9a43d)), closes [#19399](#19399)
* **core:** `Fn.select` incorrectly short-circuits complex expressions ([#19680](#19680)) ([7f26fad](7f26fad))
* **core:** detect and resolve stringified number tokens ([#19578](#19578)) ([7d9ab2a](7d9ab2a)), closes [#19546](#19546) [#19550](#19550)
* **core:** reduce CFN template indent size to save bytes ([#19656](#19656)) ([fd63ca3](fd63ca3))
* **ecs:** 'desiredCount' and 'ephemeralStorageGiB' cannot be tokens ([#19453](#19453)) ([c852239](c852239)), closes [#16648](#16648)
* **ecs:** remove unnecessary error when adding volume to external task definition ([#19774](#19774)) ([5446ded](5446ded)), closes [#19259](#19259)
* **iam:** policies aren't minimized as far as possible ([#19764](#19764)) ([876ed8a](876ed8a)), closes [#19751](#19751)
* **logs:** Faulty Resource Policy Generated ([#19640](#19640)) ([1fdf122](1fdf122)), closes [#17544](#17544)
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this issue Apr 27, 2022
@yerzhan7
feat(s3): EventBridge bucket notifications (aws#18614)
2ba4d1a 
## 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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@otaviomacedo otaviomacedo

Labels
@aws-cdk/aws-s3 Related to Amazon S3 feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(s3): add EventBridge bucket notifications
feat(s3): EventBridge bucket notifications
3 participants
@otaviomacedo @michaelbrewer @mickychetta