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.

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

aws_s3_notifications : CDK destroy deletes existing S3 event notifications #29004

Closed
nleut opened this issue Feb 6, 2024 · 5 comments · Fixed by #29939
Closed

aws_s3_notifications : CDK destroy deletes existing S3 event notifications #29004

nleut opened this issue Feb 6, 2024 · 5 comments · Fixed by #29939
Assignees
Labels
@aws-cdk/aws-s3-notifications bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@nleut
Copy link

nleut commented Feb 6, 2024

Describe the bug

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.

Expected Behavior

cdk destroy or a rollback from a failed stack should only delete S3 event notifications created by the stack

Current Behavior

cdk destroy or a rollback from a failed stack removes all S3 event notifications on the bucket

Reproduction Steps

  1. Create S3 bucket manually through the console
  2. Create SNS topic manually through the console
  3. Add event notification on the S3 bucket to the SNS topic through the console
  4. Create CDK stack that references an existing bucket and creates a new event notification:
bucket = s3.Bucket.from_bucket_arn(self, id="mybucket", bucket_arn="arn:aws:s3:::mybucket")
topic = sns.Topic(self, id="mytopic")
bucket.add_event_notification(s3.EventType.OBJECT_CREATED, s3_notifications.SnsDestination(topic), s3.NotificationKeyFilter(suffix=".1"))
  1. cdk deploy the stack
  2. cdk destroy the stack
  3. Observe that all event notifications are now removed from the bucket, including the manually created event notification from step 3.

Possible Solution

The BucketNotificationHandler Lambda function described in #2004 appears to include handling for unmanaged event notifications. Modifying this function to support this scenario should resolve the issue.

Additional Information/Context

No response

CDK CLI Version

2.126.0 (build fb74c41)

Framework Version

No response

Node.js Version

v20.11.0

OS

macOS Ventura 13.6.4

Language

Python

Language Version

Python (3.9.7)

Other information

No response

@nleut nleut added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 6, 2024
@spKavvadias
Copy link

We also had the same issue. We have multiple CDK stacks. When I tried to add or remove bucket notifications using the method you described (to a specific stack) it also removed all existing ones (not part of the stack).

@pahud
Copy link
Contributor

pahud commented Feb 8, 2024

probably related to #28915

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 8, 2024
@moelasmar moelasmar assigned moelasmar and unassigned moelasmar Feb 8, 2024
@wchaws
Copy link
Contributor

wchaws commented Feb 29, 2024

This is caused by 37be7b9#r137776660

@alpcaniscan
Copy link

We also had the same issue. We have multiple CDK stacks. When I tried to add or remove bucket notifications using the method you described (to a specific stack) it also removed all existing ones (not part of the stack).

having same issue here.

@aaythapa aaythapa removed their assignment Apr 18, 2024
@GavinZZ GavinZZ self-assigned this Apr 23, 2024
@mergify mergify bot closed this as completed in #29939 Apr 24, 2024
mergify bot pushed a commit that referenced this issue Apr 24, 2024
…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*
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3-notifications bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants