diff --git a/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts b/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts index 536f1800ea6aa..1f0430ab08b50 100644 --- a/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts +++ b/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'; @@ -124,7 +125,14 @@ export class BucketNotifications extends Construct { role: this.handlerRole, }); - const managed = this.bucket instanceof Bucket; + let managed = this.bucket instanceof Bucket; + + // We should treat buckets as unmanaged because it will not remove notifications added somewhere else + // 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; + } if (!managed) { handler.addToRolePolicy(new iam.PolicyStatement({ diff --git a/packages/aws-cdk-lib/aws-s3/test/notification.test.ts b/packages/aws-cdk-lib/aws-s3/test/notification.test.ts index 8c41965b38646..5b6d5b7ca6242 100644 --- a/packages/aws-cdk-lib/aws-s3/test/notification.test.ts +++ b/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,45 @@ 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, + }); + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 's3:PutBucketNotification', + Effect: 'Allow', + Resource: '*', + }, + { + Action: 's3:GetBucketNotification', + Effect: 'Allow', + Resource: '*', + }, + ], + Version: '2012-10-17', + }, + }); + }); + test('check notifications handler runtime version', () => { const stack = new cdk.Stack(); diff --git a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md b/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md index 9e980cc71faae..fc77b116c1c76 100644 --- a/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md +++ b/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) | @@ -1338,4 +1339,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` | `false` | + + diff --git a/packages/aws-cdk-lib/cx-api/lib/features.ts b/packages/aws-cdk-lib/cx-api/lib/features.ts index 233529bf8fa5b..8d19b140a4e35 100644 --- a/packages/aws-cdk-lib/cx-api/lib/features.ts +++ b/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 = { ////////////////////////////////////////////////////////////////////// @@ -1092,6 +1093,20 @@ export const FLAGS: Record = { 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' }, + recommendedValue: false, + }, }; const CURRENT_MV = 'v2'; diff --git a/packages/aws-cdk-lib/cx-api/test/features.test.ts b/packages/aws-cdk-lib/cx-api/test/features.test.ts index c7770a260ca98..c1aeb3b80c0d1 100644 --- a/packages/aws-cdk-lib/cx-api/test/features.test.ts +++ b/packages/aws-cdk-lib/cx-api/test/features.test.ts @@ -37,9 +37,8 @@ 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 }); });