From 2e02dbf3e48c1347efb46ea3056bf7eb1cc45c2c Mon Sep 17 00:00:00 2001 From: epolon Date: Sun, 17 Nov 2024 12:44:10 +0200 Subject: [PATCH 1/6] mid work --- .../cli-integ/resources/cdk-apps/app/app.js | 12 ++++++ .../tests/cli-integ-tests/cli.integtest.ts | 39 +++++++++++++++++++ packages/aws-cdk-lib/core/lib/stack.ts | 4 +- packages/aws-cdk-lib/core/test/stack.test.ts | 15 +++++++ .../lib/artifacts/cloudformation-artifact.ts | 4 +- packages/aws-cdk/lib/cdk-toolkit.ts | 30 ++++++++++---- 6 files changed, 92 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index 0f88ddf3bd467..e57104fe62645 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -30,6 +30,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { aws_lambda: lambda, aws_ecr_assets: docker, aws_appsync: appsync, + aws_cloudformation: cfn, Stack } = require('aws-cdk-lib'); } @@ -748,6 +749,16 @@ class NotificationArnPropStack extends cdk.Stack { new sns.Topic(this, 'topic'); } } + +class EmptyStack extends cdk.Stack { + constructor(parent, id, props) { + super(parent, id, props); + + new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle'); + + } +} + class AppSyncHotswapStack extends cdk.Stack { constructor(parent, id, props) { super(parent, id, props); @@ -812,6 +823,7 @@ switch (stackSet) { new MissingSSMParameterStack(app, `${stackPrefix}-missing-ssm-parameter`, { env: defaultEnv }); new LambdaStack(app, `${stackPrefix}-lambda`); + new EmptyStack(app, `${stackPrefix}-empty`); // This stack is used to test diff with large templates by creating a role with a ton of metadata new IamRolesStack(app, `${stackPrefix}-iam-roles`); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index de888c9062f22..278c2d55285c4 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -7,6 +7,7 @@ import { DescribeStacksCommand, GetTemplateCommand, ListChangeSetsCommand, + UpdateStackCommand, } from '@aws-sdk/client-cloudformation'; import { DescribeServicesCommand } from '@aws-sdk/client-ecs'; import { @@ -679,6 +680,44 @@ integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixt } })); +// 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('empty'); + + // add notification arns externally to cdk + await fixture.aws.cloudFormation.send( + new UpdateStackCommand({ + StackName: fixture.fullStackName('empty'), + UsePreviousTemplate: true, + NotificationARNs: [topicArn], + }), + ); + + // deploy again + await fixture.cdkDeploy('empty'); + + // make sure the notification arn is preserved + const describeResponse = await fixture.aws.cloudFormation.send( + new DescribeStacksCommand({ + StackName: fixture.fullStackName('empty'), + }), + ); + expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]); + } finally { + await fixture.aws.sns.send( + new DeleteTopicCommand({ + TopicArn: topicArn, + }), + ); + } +})); + // 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( diff --git a/packages/aws-cdk-lib/core/lib/stack.ts b/packages/aws-cdk-lib/core/lib/stack.ts index 0ccb560636718..e3b21ea57de28 100644 --- a/packages/aws-cdk-lib/core/lib/stack.ts +++ b/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}'`); diff --git a/packages/aws-cdk-lib/core/test/stack.test.ts b/packages/aws-cdk-lib/core/test/stack.test.ts index 399e355063dfb..6fad4e4922045 100644 --- a/packages/aws-cdk-lib/core/test/stack.test.ts +++ b/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']; diff --git a/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts b/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts index 8fcea525b4907..3009129056e93 100644 --- a/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts +++ b/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; diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 8ae254f6813ed..baaf7e84d26c3 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -370,15 +370,29 @@ 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`); + // 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); From 293e4d69b3832b7a1b8425d0484230bd4f28b7f6 Mon Sep 17 00:00:00 2001 From: epolon Date: Sun, 17 Nov 2024 16:54:42 +0200 Subject: [PATCH 2/6] mid work --- .../cli-integ/resources/cdk-apps/app/app.js | 23 +++---- .../tests/cli-integ-tests/cli.integtest.ts | 65 ++++++++++++++++--- packages/aws-cdk-lib/core/README.md | 11 +++- packages/aws-cdk/lib/cdk-toolkit.ts | 9 --- packages/aws-cdk/lib/config.ts | 2 +- .../lib/parse-command-line-arguments.ts | 2 +- packages/aws-cdk/test/cdk-toolkit.test.ts | 2 +- 7 files changed, 80 insertions(+), 34 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index e57104fe62645..d38593aca0a35 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -30,7 +30,6 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { aws_lambda: lambda, aws_ecr_assets: docker, aws_appsync: appsync, - aws_cloudformation: cfn, Stack } = require('aws-cdk-lib'); } @@ -743,16 +742,17 @@ 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'); - } -} -class EmptyStack extends cdk.Stack { - constructor(parent, id, props) { - super(parent, id, props); + const arnsFromEnv = process.env.INTEG_NOTIFICATION_ARNS; + super(parent, id, { + ...props, + // comma separated list of arns. + // empty strings means empty list. + // undefined means undefined + notificationArns: arnsFromEnv == '' ? [] : (arnsFromEnv ? arnsFromEnv.split(',') : undefined) + }); new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle'); @@ -823,7 +823,6 @@ switch (stackSet) { new MissingSSMParameterStack(app, `${stackPrefix}-missing-ssm-parameter`, { env: defaultEnv }); new LambdaStack(app, `${stackPrefix}-lambda`); - new EmptyStack(app, `${stackPrefix}-empty`); // This stack is used to test diff with large templates by creating a role with a ton of metadata new IamRolesStack(app, `${stackPrefix}-iam-roles`); @@ -842,9 +841,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`); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 278c2d55285c4..3795feb1636f8 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -634,14 +634,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]); @@ -662,12 +662,17 @@ 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]); @@ -688,27 +693,71 @@ integTest('deploy preserves existing notification arns when not specified', with const topicArn = response.TopicArn!; try { - await fixture.cdkDeploy('empty'); + await fixture.cdkDeploy('notification-arns'); // add notification arns externally to cdk await fixture.aws.cloudFormation.send( new UpdateStackCommand({ - StackName: fixture.fullStackName('empty'), + StackName: fixture.fullStackName('notification-arns'), UsePreviousTemplate: true, NotificationARNs: [topicArn], }), ); // deploy again - await fixture.cdkDeploy('empty'); + await fixture.cdkDeploy('notification-arns'); // make sure the notification arn is preserved const describeResponse = await fixture.aws.cloudFormation.send( new DescribeStacksCommand({ - StackName: fixture.fullStackName('empty'), + StackName: fixture.fullStackName('notification-arns'), + }), + ); + expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]); + } finally { + await fixture.aws.sns.send( + new DeleteTopicCommand({ + TopicArn: topicArn, + }), + ); + } +})); + +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({ diff --git a/packages/aws-cdk-lib/core/README.md b/packages/aws-cdk-lib/core/README.md index a6402770d6c66..ecec3322f5351 100644 --- a/packages/aws-cdk-lib/core/README.md +++ b/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 diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index baaf7e84d26c3..dc8b6cf34aca1 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -370,15 +370,6 @@ 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). diff --git a/packages/aws-cdk/lib/config.ts b/packages/aws-cdk/lib/config.ts index d8f490dac67dc..349faf1126384 100644 --- a/packages/aws-cdk/lib/config.ts +++ b/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 }, diff --git a/packages/aws-cdk/lib/parse-command-line-arguments.ts b/packages/aws-cdk/lib/parse-command-line-arguments.ts index 3f7ebd16aa4d8..a8d637cd28a98 100644 --- a/packages/aws-cdk/lib/parse-command-line-arguments.ts +++ b/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, }) diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index cfc9aca1bc398..241e5987014f1 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/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 { From c940d552ff56f92bbadd34a97adfdce50176fe8e Mon Sep 17 00:00:00 2001 From: epolon Date: Sun, 17 Nov 2024 16:56:18 +0200 Subject: [PATCH 3/6] mid work --- .../@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index d38593aca0a35..4981a75b2a0f1 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -749,7 +749,7 @@ class NotificationArnsStack extends cdk.Stack { super(parent, id, { ...props, // comma separated list of arns. - // empty strings means empty list. + // empty string means empty list. // undefined means undefined notificationArns: arnsFromEnv == '' ? [] : (arnsFromEnv ? arnsFromEnv.split(',') : undefined) }); From 8a082debd2215872365af979ea801225e15cb377 Mon Sep 17 00:00:00 2001 From: epolon Date: Sun, 17 Nov 2024 17:04:19 +0200 Subject: [PATCH 4/6] mid work --- .../tests/cli-integ-tests/cli.integtest.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 3795feb1636f8..e80840adbcd6e 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -767,6 +767,43 @@ integTest('deploy deletes ALL notification arns when empty array is passed', wit } })); +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( From cc2c4b1e329b4be34c11febbfba1730f9f1f2326 Mon Sep 17 00:00:00 2001 From: epolon Date: Mon, 18 Nov 2024 09:45:36 +0200 Subject: [PATCH 5/6] allow breaking change --- allowed-breaking-changes.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/allowed-breaking-changes.txt b/allowed-breaking-changes.txt index 99d48726abbee..3e514afeee38b 100644 --- a/allowed-breaking-changes.txt +++ b/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 \ No newline at end of file +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 \ No newline at end of file From b63d5c0c88acbc0f2ba6cf6b8c411d46e1e8633d Mon Sep 17 00:00:00 2001 From: epolon Date: Mon, 18 Nov 2024 13:33:27 +0200 Subject: [PATCH 6/6] wait for stack update complete --- .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 8e12934c571ae..8c73c74bb84f3 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -8,6 +8,7 @@ import { GetTemplateCommand, ListChangeSetsCommand, UpdateStackCommand, + waitUntilStackUpdateComplete, } from '@aws-sdk/client-cloudformation'; import { DescribeServicesCommand } from '@aws-sdk/client-ecs'; import { @@ -704,6 +705,14 @@ integTest('deploy preserves existing notification arns when not specified', with }), ); + await waitUntilStackUpdateComplete( + { + client: fixture.aws.cloudFormation, + maxWaitTime: 600, + }, + { StackName: fixture.fullStackName('notification-arns') }, + ); + // deploy again await fixture.cdkDeploy('notification-arns');