fix(cli): externally managed stack notification arns are deleted on deploy

allowed-breaking-changes.txt

@@ -936,4 +936,10 @@ removed:aws-cdk-lib.aws_ec2.WindowsVersion.WINDOWS_SERVER_2022_TURKISH_FULL_BASE
 
 # null() return a [] which is invalid filter rule. It should return [null].
 # Hence the return type changes from string[] to any
-change-return-type:aws-cdk-lib.aws_lambda.FilterRule.null
+change-return-type:aws-cdk-lib.aws_lambda.FilterRule.null
+
+
+# output property was mistakenly marked as required even though it should have allowed
+# for undefined, i.e optional
+changed-type:@aws-cdk/cx-api.CloudFormationStackArtifact.notificationArns
+changed-type:aws-cdk-lib.cx_api.CloudFormationStackArtifact.notificationArns

packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js

@@ -742,12 +742,23 @@ class BuiltinLambdaStack extends cdk.Stack {
   }
 }
 
-class NotificationArnPropStack extends cdk.Stack {
+class NotificationArnsStack extends cdk.Stack {
   constructor(parent, id, props) {
-    super(parent, id, props);
-    new sns.Topic(this, 'topic');
+
+    const arnsFromEnv = process.env.INTEG_NOTIFICATION_ARNS;
+    super(parent, id, {
+      ...props,
+      // comma separated list of arns. 
+      // empty string means empty list.
+      // undefined means undefined
+      notificationArns: arnsFromEnv == '' ? [] : (arnsFromEnv ? arnsFromEnv.split(',') : undefined)
+    });
+
+    new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle');
+
   }
 }
+
 class AppSyncHotswapStack extends cdk.Stack {
   constructor(parent, id, props) {
     super(parent, id, props);
@@ -839,9 +850,7 @@ switch (stackSet) {
     new DockerInUseStack(app, `${stackPrefix}-docker-in-use`);
     new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);
 
-    new NotificationArnPropStack(app, `${stackPrefix}-notification-arn-prop`, {
-      notificationArns: [`arn:aws:sns:${defaultEnv.region}:${defaultEnv.account}:${stackPrefix}-test-topic-prop`],
-    });
+    new NotificationArnsStack(app, `${stackPrefix}-notification-arns`);
 
     // SSO stacks
     new SsoInstanceAccessControlConfig(app, `${stackPrefix}-sso-access-control`);

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

@@ -7,6 +7,8 @@ import {
   DescribeStacksCommand,
   GetTemplateCommand,
   ListChangeSetsCommand,
+  UpdateStackCommand,
+  waitUntilStackUpdateComplete,
 } from '@aws-sdk/client-cloudformation';
 import { DescribeServicesCommand } from '@aws-sdk/client-ecs';
 import {
@@ -633,14 +635,14 @@ integTest(
     const topicArn = response.TopicArn!;
 
     try {
-      await fixture.cdkDeploy('test-2', {
+      await fixture.cdkDeploy('notification-arns', {
         options: ['--notification-arns', topicArn],
       });
 
       // verify that the stack we deployed has our notification ARN
       const describeResponse = await fixture.aws.cloudFormation.send(
         new DescribeStacksCommand({
-          StackName: fixture.fullStackName('test-2'),
+          StackName: fixture.fullStackName('notification-arns'),
         }),
       );
       expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]);
@@ -661,12 +663,63 @@ integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixt
   const topicArn = response.TopicArn!;
 
   try {
-    await fixture.cdkDeploy('notification-arn-prop');
+    await fixture.cdkDeploy('notification-arns', {
+      modEnv: {
+        INTEG_NOTIFICATION_ARNS: topicArn,
+
+      },
+    });
 
     // verify that the stack we deployed has our notification ARN
     const describeResponse = await fixture.aws.cloudFormation.send(
       new DescribeStacksCommand({
-        StackName: fixture.fullStackName('notification-arn-prop'),
+        StackName: fixture.fullStackName('notification-arns'),
+      }),
+    );
+    expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]);
+  } finally {
+    await fixture.aws.sns.send(
+      new DeleteTopicCommand({
+        TopicArn: topicArn,
+      }),
+    );
+  }
+}));
+
+// https://github.com/aws/aws-cdk/issues/32153
+integTest('deploy preserves existing notification arns when not specified', withDefaultFixture(async (fixture) => {
+  const topicName = `${fixture.stackNamePrefix}-topic`;
+
+  const response = await fixture.aws.sns.send(new CreateTopicCommand({ Name: topicName }));
+  const topicArn = response.TopicArn!;
+
+  try {
+    await fixture.cdkDeploy('notification-arns');
+
+    // add notification arns externally to cdk
+    await fixture.aws.cloudFormation.send(
+      new UpdateStackCommand({
+        StackName: fixture.fullStackName('notification-arns'),
+        UsePreviousTemplate: true,
+        NotificationARNs: [topicArn],
+      }),
+    );
+
+    await waitUntilStackUpdateComplete(
+      {
+        client: fixture.aws.cloudFormation,
+        maxWaitTime: 600,
+      },
+      { StackName: fixture.fullStackName('notification-arns') },
+    );
+
+    // deploy again
+    await fixture.cdkDeploy('notification-arns');
+
+    // make sure the notification arn is preserved
+    const describeResponse = await fixture.aws.cloudFormation.send(
+      new DescribeStacksCommand({
+        StackName: fixture.fullStackName('notification-arns'),
       }),
     );
     expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]);
@@ -679,6 +732,87 @@ integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixt
   }
 }));
 
+integTest('deploy deletes ALL notification arns when empty array is passed', withDefaultFixture(async (fixture) => {
+  const topicName = `${fixture.stackNamePrefix}-topic`;
+
+  const response = await fixture.aws.sns.send(new CreateTopicCommand({ Name: topicName }));
+  const topicArn = response.TopicArn!;
+
+  try {
+    await fixture.cdkDeploy('notification-arns', {
+      modEnv: {
+        INTEG_NOTIFICATION_ARNS: topicArn,
+      },
+    });
+
+    // make sure the arn was added
+    let describeResponse = await fixture.aws.cloudFormation.send(
+      new DescribeStacksCommand({
+        StackName: fixture.fullStackName('notification-arns'),
+      }),
+    );
+    expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]);
+
+    // deploy again with empty array
+    await fixture.cdkDeploy('notification-arns', {
+      modEnv: {
+        INTEG_NOTIFICATION_ARNS: '',
+      },
+    });
+
+    // make sure the arn was deleted
+    describeResponse = await fixture.aws.cloudFormation.send(
+      new DescribeStacksCommand({
+        StackName: fixture.fullStackName('notification-arns'),
+      }),
+    );
+    expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([]);
+  } finally {
+    await fixture.aws.sns.send(
+      new DeleteTopicCommand({
+        TopicArn: topicArn,
+      }),
+    );
+  }
+}));
+
+integTest('deploy with notification ARN as prop and flag', withDefaultFixture(async (fixture) => {
+  const topic1Name = `${fixture.stackNamePrefix}-topic1`;
+  const topic2Name = `${fixture.stackNamePrefix}-topic1`;
+
+  const topic1Arn = (await fixture.aws.sns.send(new CreateTopicCommand({ Name: topic1Name }))).TopicArn!;
+  const topic2Arn = (await fixture.aws.sns.send(new CreateTopicCommand({ Name: topic2Name }))).TopicArn!;
+
+  try {
+    await fixture.cdkDeploy('notification-arns', {
+      modEnv: {
+        INTEG_NOTIFICATION_ARNS: topic1Arn,
+
+      },
+      options: ['--notification-arns', topic2Arn],
+    });
+
+    // verify that the stack we deployed has our notification ARN
+    const describeResponse = await fixture.aws.cloudFormation.send(
+      new DescribeStacksCommand({
+        StackName: fixture.fullStackName('notification-arns'),
+      }),
+    );
+    expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topic1Arn, topic2Arn]);
+  } finally {
+    await fixture.aws.sns.send(
+      new DeleteTopicCommand({
+        TopicArn: topic1Arn,
+      }),
+    );
+    await fixture.aws.sns.send(
+      new DeleteTopicCommand({
+        TopicArn: topic2Arn,
+      }),
+    );
+  }
+}));
+
 // NOTE: this doesn't currently work with modern-style synthesis, as the bootstrap
 // role by default will not have permission to iam:PassRole the created role.
 integTest(

packages/aws-cdk-lib/core/README.md

@@ -1332,7 +1332,16 @@ const stack = new Stack(app, 'StackName', {
 });
 ```
 
-Stack events will be sent to any SNS Topics in this list.
+Stack events will be sent to any SNS Topics in this list. These ARNs are added to those specified using
+the `--notification-arns` command line option.
+
+Note that in order to do delete notification ARNs entirely, you must pass an empty array ([]) instead of omitting it.
+If you omit the property, no action on existing ARNs will take place.
+
+> [!NOTICE]
+> Adding the `notificationArns` property (or using the `--notification-arns` CLI options) will **override**
+> any existing ARNs configured on the stack. If you have an external system managing notification ARNs,
+> either migrate to use this mechanism, or avoid specfying notification ARNs with the CDK.
 
 ### CfnJson
 

packages/aws-cdk-lib/core/lib/stack.ts

@@ -376,7 +376,7 @@ export class Stack extends Construct implements ITaggable {
    *
    * @internal
    */
-  public readonly _notificationArns: string[];
+  public readonly _notificationArns?: string[];
 
   /**
    * Logical ID generation strategy
@@ -471,7 +471,7 @@ export class Stack extends Construct implements ITaggable {
       }
     }
 
-    this._notificationArns = props.notificationArns ?? [];
+    this._notificationArns = props.notificationArns;
 
     if (!VALID_STACK_NAME_REGEX.test(this.stackName)) {
       throw new Error(`Stack name must match the regular expression: ${VALID_STACK_NAME_REGEX.toString()}, got '${this.stackName}'`);

packages/aws-cdk-lib/core/test/stack.test.ts

@@ -2097,6 +2097,21 @@ describe('stack', () => {
     ]);
   });
 
+  test('stack notification arns defaults to undefined', () => {
+
+    const app = new App({ stackTraces: false });
+    const stack1 = new Stack(app, 'stack1', {});
+
+    const asm = app.synth();
+
+    // It must be undefined and not [] because:
+    //
+    //  - undefined  =>  cdk ignores it entirely, as if it wasn't supported (allows external management).
+    //  - []:        =>  cdk manages it, and the user wants to wipe it out.
+    //  - ['arn-1']  =>  cdk manages it, and the user wants to set it to ['arn-1'].
+    expect(asm.getStackArtifact(stack1.artifactId).notificationArns).toBeUndefined();
+  });
+
   test('stack notification arns are reflected in the stack artifact properties', () => {
     // GIVEN
     const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];

packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts

@@ -56,7 +56,7 @@ export class CloudFormationStackArtifact extends CloudArtifact {
   /**
    * SNS Topics that will receive stack events.
    */
-  public readonly notificationArns: string[];
+  public readonly notificationArns?: string[];
 
   /**
    * The physical name of this stack.
@@ -174,7 +174,7 @@ export class CloudFormationStackArtifact extends CloudArtifact {
     // We get the tags from 'properties' if available (cloud assembly format >= 6.0.0), otherwise
     // from the stack metadata
     this.tags = properties.tags ?? this.tagsFromMetadata();
-    this.notificationArns = properties.notificationArns ?? [];
+    this.notificationArns = properties.notificationArns;
     this.assumeRoleArn = properties.assumeRoleArn;
     this.assumeRoleExternalId = properties.assumeRoleExternalId;
     this.assumeRoleAdditionalOptions = properties.assumeRoleAdditionalOptions;

packages/aws-cdk/lib/cdk-toolkit.ts

@@ -370,15 +370,20 @@ export class CdkToolkit {
         }
       }
 
-      let notificationArns: string[] = [];
-      notificationArns = notificationArns.concat(options.notificationArns ?? []);
-      notificationArns = notificationArns.concat(stack.notificationArns);
-
-      notificationArns.map((arn) => {
-        if (!validateSnsTopicArn(arn)) {
-          throw new Error(`Notification arn ${arn} is not a valid arn for an SNS topic`);
+      // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK)
+      //
+      //  - undefined  =>  cdk ignores it, as if it wasn't supported (allows external management).
+      //  - []:        =>  cdk manages it, and the user wants to wipe it out.
+      //  - ['arn-1']  =>  cdk manages it, and the user wants to set it to ['arn-1'].
+      const notificationArns = (!!options.notificationArns || !!stack.notificationArns)
+        ? (options.notificationArns ?? []).concat(stack.notificationArns ?? [])
+        : undefined;
+
+      for (const notificationArn of notificationArns ?? []) {
+        if (!validateSnsTopicArn(notificationArn)) {
+          throw new Error(`Notification arn ${notificationArn} is not a valid arn for an SNS topic`);
         }
-      });
+      }
 
       const stackIndex = stacks.indexOf(stack) + 1;
       print('%s: deploying... [%s/%s]', chalk.bold(stack.displayName), stackIndex, stackCollection.stackCount);

packages/aws-cdk/lib/config.ts

@@ -112,7 +112,7 @@ export function makeConfig(): CliConfig {
           'build-exclude': { type: 'array', alias: 'E', nargs: 1, desc: 'Do not rebuild asset with the given ID. Can be specified multiple times', default: [] },
           'exclusively': { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' },
           'require-approval': { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'What security-sensitive changes need manual approval' },
-          'notification-arns': { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events', nargs: 1, requiresArg: true },
+          'notification-arns': { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events. These will be added to ARNs specified with the \'notificationArns\' stack property.', nargs: 1, requiresArg: true },
           // @deprecated(v2) -- tags are part of the Cloud Assembly and tags specified here will be overwritten on the next deployment
           'tags': { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE), overrides tags from Cloud Assembly (deprecated)', nargs: 1, requiresArg: true },
           'execute': { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet) (deprecated)', deprecated: true },

packages/aws-cdk/lib/parse-command-line-arguments.ts

@@ -355,7 +355,7 @@ export function parseCommandLineArguments(
         })
         .option('notification-arns', {
           type: 'array',
-          desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events',
+          desc: "ARNs of SNS topics that CloudFormation will notify with stack related events. These will be added to ARNs specified with the 'notificationArns' stack property.",
           nargs: 1,
           requiresArg: true,
         })

packages/aws-cdk/test/cdk-toolkit.test.ts

@@ -1658,7 +1658,7 @@ class FakeCloudFormation extends Deployments {
         .map(([Key, Value]) => ({ Key, Value }))
         .sort((l, r) => l.Key.localeCompare(r.Key));
     }
-    this.expectedNotificationArns = expectedNotificationArns ?? [];
+    this.expectedNotificationArns = expectedNotificationArns;
   }
 
   public deployStack(options: DeployStackOptions): Promise<SuccessfulDeployStackResult> {