fix(s3): BucketNotification in owning stack deletes BucketNotificatio…

…ns from other stacks

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

@@ -2,6 +2,7 @@ import { Construct, IConstruct } from 'constructs';
 import { NotificationsResourceHandler } from './notifications-resource-handler';
 import * as iam from '../../../aws-iam';
 import * as cdk from '../../../core';
+import * as cxapi from '../../../cx-api';
 import { Bucket, IBucket, EventType, NotificationKeyFilter } from '../bucket';
 import { BucketNotificationDestinationType, IBucketNotificationDestination } from '../destination';
 
@@ -117,7 +118,7 @@ export class BucketNotifications extends Construct {
         role: this.handlerRole,
       });
 
-      const managed = this.bucket instanceof Bucket;
+      let managed = this.bucket instanceof Bucket;
 
       if (!managed) {
         handler.addToRolePolicy(new iam.PolicyStatement({
@@ -126,6 +127,13 @@ export class BucketNotifications extends Construct {
         }));
       }
 
+      // The customer resource handles unmanaged bucket is the right way so setting the managed flag to false
+      // Ading a feature flag to prevent it brings unexpected changes to customers
+      // Put it here because we still need to create the permission if it's unmanaged bucket.
+      if (cdk.FeatureFlags.of(this).isEnabled(cxapi.S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET)) {
+        managed = false;
+      }
+
       this.resource = new cdk.CfnResource(this, 'Resource', {
         type: 'Custom::S3BucketNotifications',
         properties: {

packages/aws-cdk-lib/aws-s3/test/notification.test.ts

@@ -1,6 +1,7 @@
 import { Match, Template } from '../../assertions';
 import * as iam from '../../aws-iam';
 import * as cdk from '../../core';
+import * as cxapi from '../../cx-api';
 import * as s3 from '../lib';
 
 describe('notification', () => {
@@ -226,6 +227,28 @@ describe('notification', () => {
       },
     });
   });
+
+  test('Notification custom resource uses always treat bucket as unmanaged', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    stack.node.setContext(cxapi.S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET, true);
+
+    // WHEN
+    new s3.Bucket(stack, 'MyBucket', {
+      eventBridgeEnabled: true,
+    });
+
+    // THEN
+    Template.fromStack(stack).resourceCountIs('AWS::S3::Bucket', 1);
+    Template.fromStack(stack).hasResourceProperties('Custom::S3BucketNotifications', {
+      NotificationConfiguration: {
+        EventBridgeConfiguration: {},
+      },
+      Managed: false,
+    });
+  });
+
   test('check notifications handler runtime version', () => {
     const stack = new cdk.Stack();
 

packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

@@ -71,6 +71,7 @@ Flags come in three types:
 | [@aws-cdk/pipelines:reduceAssetRoleTrustScope](#aws-cdkpipelinesreduceassetroletrustscope) | Remove the root account principal from PipelineAssetsFileRole trust policy | 2.141.0 | (default) |
 | [@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm](#aws-cdkaws-ecsremovedefaultdeploymentalarm) | When enabled, remove default deployment alarm settings | 2.143.0 | (default) |
 | [@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault](#aws-cdkcustom-resourceslogapiresponsedatapropertytruedefault) | When enabled, the custom resource used for `AwsCustomResource` will configure the `logApiResponseData` property as true by default | 2.145.0 | (fix) |
+| [@aws-cdk/aws-s3:keepNotificationInImportedBucket](#aws-cdkaws-s3keepnotificationinimportedbucket) | When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack. | V2NEXT | (fix) |
 
 <!-- END table -->
 
@@ -132,6 +133,9 @@ The following json shows the current recommended set of flags, as `cdk init` wou
     "@aws-cdk/aws-ec2:ebsDefaultGp3Volume": true,
     "@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm": true,
     "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false
+    "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false,
+    "@aws-cdk/aws-s3:keepNotificationInImportedBucket": true,
+    "@aws-cdk/aws-stepfunctions-tasks:ecsReduceRunTaskPermissions": true
   }
 }
 ```
@@ -1338,4 +1342,20 @@ property from the event object.
 | 2.145.0 | `false` | `false` |
 
 
+### @aws-cdk/aws-s3:keepNotificationInImportedBucket
+
+*When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack.* (fix)
+
+Currently, adding notifications to a bucket where it was created by ourselves will override notification added where it is imported.
+
+When this feature flag is enabled, adding notifications to a bucket in the current stack will only update notification defined in this stack.
+Other notifications that are not managed by this stack will be kept.
+
+
+| Since | Default | Recommended |
+| ----- | ----- | ----- |
+| (not in v1) |  |  |
+| V2NEXT | `false` | `true` |
+
+
 <!-- END details -->

packages/aws-cdk-lib/cx-api/lib/features.ts

@@ -105,6 +105,7 @@ export const EKS_NODEGROUP_NAME = '@aws-cdk/aws-eks:nodegroupNameAttribute';
 export const EBS_DEFAULT_GP3 = '@aws-cdk/aws-ec2:ebsDefaultGp3Volume';
 export const ECS_REMOVE_DEFAULT_DEPLOYMENT_ALARM = '@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm';
 export const LOG_API_RESPONSE_DATA_PROPERTY_TRUE_DEFAULT = '@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault';
+export const S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET = '@aws-cdk/aws-s3:keepNotificationInImportedBucket';
 
 export const FLAGS: Record<string, FlagInfo> = {
   //////////////////////////////////////////////////////////////////////
@@ -1092,6 +1093,21 @@ export const FLAGS: Record<string, FlagInfo> = {
     introducedIn: { v2: '2.145.0' },
     recommendedValue: false,
   },
+
+  //////////////////////////////////////////////////////////////////////
+  [S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET]: {
+    type: FlagType.BugFix,
+    summary: 'When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack.',
+    detailsMd: `
+      Currently, adding notifications to a bucket where it was created by ourselves will override notification added where it is imported.
+
+      When this feature flag is enabled, adding notifications to a bucket in the current stack will only update notification defined in this stack.
+      Other notifications that are not managed by this stack will be kept.
+    `,
+    introducedIn: { v2: 'V2NEXT' },
+    defaults: { v2: false },
+    recommendedValue: true,
+  },
 };
 
 const CURRENT_MV = 'v2';

packages/aws-cdk-lib/cx-api/test/features.test.ts

@@ -37,9 +37,9 @@ test('feature flag defaults may not be changed anymore', () => {
     [feats.EFS_DEFAULT_ENCRYPTION_AT_REST]: true,
     [feats.LAMBDA_RECOGNIZE_VERSION_PROPS]: true,
     [feats.CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021]: true,
-    // Add new disabling feature flags below this line
     [feats.PIPELINE_REDUCE_ASSET_ROLE_TRUST_SCOPE]: true,
-
+    // Add new disabling feature flags below this line
+    [feats.S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET]: false,
   });
 });