aws_s3: BucketNotification in owning stack deletes BucketNotifications from other stacks

[sebastian-fredriksson-bernholtz]

Describe the bug

When making changes to the S3 event notifications in the stack that owns an S3 Bucket (on Bucket), it deletes event notifications for the bucket that have been configured in other stacks (on IBucket).

Expected Behavior

Event notifications configured in other stacks should not be deleted.

Current Behavior

Event notifications configured in other stacks are being deleted.

Reproduction Steps

1. Create and deploy Bucket in stack 1.

// stack 1
new Bucket(this, 'Bucket', {
    bucketName: 'bucketname',
});

2. Add and deploy event notification in stack 2

// stack 2
Bucket.fromBucketName(this, 'DataBucket', 'bucketName').addEventNotification(
    EventType.OBJECT_CREATED_PUT,
    new LambdaDestination(lambda)
);

3. Make a change to event notifications in stack 1 and deploy:

// stack 1
new Bucket(this, 'Bucket', {
    bucketName: 'bucketname',
+   eventBridgeEnabled: true,
});

The event handler configured and deployed in step 2 will be deleted when doing step 3.

Possible Solution

Use the same logic for handling BucketNotifications in the stack that owns the Bucket as in other stack:

aws-cdk/packages/@aws-cdk/custom-resource-handlers/lib/aws-s3/notifications-resource-handler/index.py

Line 37 in 7360a88

def handle_unmanaged(bucket, stack_id, request_type, notification_configuration, old):

Additional Information/Context

This is happening for Bucket (unlike IBucket) cdk sets Managed property on the custom resource that manages event notifications to true.

aws-cdk/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts

Line 120 in 71986ff

const managed = this.bucket instanceof Bucket;

aws-cdk/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts

Line 135 in 71986ff

Managed: managed,

And the code in the custom resource lambda handler disregards externally set notifications if Managed is set to true.

aws-cdk/packages/@aws-cdk/custom-resource-handlers/lib/aws-s3/notifications-resource-handler/index.py

Line 21 in 7360a88

config = handle_managed(event["RequestType"], notification_configuration)

CDK CLI Version

2.146.0 (build b368c78)

Framework Version

2.146.0

Node.js Version

v20.11.0

OS

macOS 14.3.1 (23D60)

Language

TypeScript

Language Version

5.5.2

Other information

Activating eventbridge for our S3 Bucket in cdk caused our site to break because a notification set up in another stack using cdk got deleted.

[pahud]

Yes I can reproduce that by following your steps. I think we need to look into the implementation from the custom resource to get it fixed.

[sebastian-fredriksson-bernholtz]

Yes I can reproduce that by following your steps. I think we need to look into the implementation from the custom resource to get it fixed.

It seems to me like a reasonable solution (the one I suggested) is to not have the special case for the owning stack (managed). If all the code related to managed was removed and just used the code path for "unmanaged" it seems like it would be a lot safer?

[xazhao]

I agree separating managed/unmanaged seems unnecessary here. unmanaged should be able handle all cases. However there might be some edge cases using managed path that we're not aware of. Removing it directly could be a breaking change. I'm thinking maybe we could put this change behind a feature flag which is safer.

Also it's the same issue as #29653

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.