From 0cdce20a8688bb233930542d01358e26e5ba8a61 Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Tue, 13 Aug 2024 15:54:24 -0700 Subject: [PATCH] feat(core): configure SNS topics to receive stack events on the Stack construct (#30551) ### Issue # (if applicable) #8581. ### Reason for this change It is easier and clearer to specify the SNS Topic ARNs on the stack construct itself instead of passing it as a command line argument. ### Description of changes Added a new optional stack prop, `notificationArns`, that is written to the CloudAssembly and concatenated with the CLI option `--notification-arns`. When I added CLI integ tests, I discovered that the existing framework is unable to use your local code. It always retrieves the latest release, which is not what you want when running it locally. This fixes that. Don't forget to select stacks by hierarchical ID (currently display name, in our tests) when writing certain test code. Otherwise, the tests may not select the stack you expected. ### Description of how you validated changes Unit tests + CLI integ test. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/package-sources/repo-source.ts | 9 +- .../cli-integ/lib/with-cdk-app.ts | 12 + .../cli-integ/resources/cdk-apps/app/app.js | 11 + .../tests/cli-integ-tests/cli.integtest.ts | 33 +- .../lib/cloud-assembly/artifact-schema.ts | 7 + .../schema/cloud-assembly.schema.json | 6 + .../schema/cloud-assembly.version.json | 2 +- packages/aws-cdk-lib/core/README.md | 12 + .../core/lib/stack-synthesizers/_shared.ts | 1 + packages/aws-cdk-lib/core/lib/stack.ts | 15 + packages/aws-cdk-lib/core/test/stack.test.ts | 15 + .../lib/artifacts/cloudformation-artifact.ts | 6 + .../cx-api/test/stack-artifact.test.ts | 18 + packages/aws-cdk/lib/api/deploy-stack.ts | 10 + .../aws-cdk/lib/api/util/cloudformation.ts | 11 +- packages/aws-cdk/lib/cdk-toolkit.ts | 23 +- .../aws-cdk/test/api/deploy-stack.test.ts | 38 +- packages/aws-cdk/test/cdk-toolkit.test.ts | 377 +++++++++++++----- packages/aws-cdk/test/util.ts | 2 + 19 files changed, 489 insertions(+), 119 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/package-sources/repo-source.ts b/packages/@aws-cdk-testing/cli-integ/lib/package-sources/repo-source.ts index 45a8f4e5d4dfc..7a5f08ec71b98 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/package-sources/repo-source.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/package-sources/repo-source.ts @@ -75,13 +75,14 @@ const YARN_MONOREPO_CACHE: Record = {}; * * Cached in YARN_MONOREPO_CACHE. */ -async function findYarnPackages(root: string): Promise> { +export async function findYarnPackages(root: string): Promise> { if (!(root in YARN_MONOREPO_CACHE)) { - const output: YarnWorkspacesOutput = JSON.parse(await shell(['yarn', 'workspaces', '--silent', 'info'], { + const outputDataString: string = JSON.parse(await shell(['yarn', 'workspaces', '--json', 'info'], { captureStderr: false, cwd: root, show: 'error', - })); + })).data; + const output: YarnWorkspacesOutput = JSON.parse(outputDataString); const ret: Record = {}; for (const [k, v] of Object.entries(output)) { @@ -96,7 +97,7 @@ async function findYarnPackages(root: string): Promise> { * Find the root directory of the repo from the current directory */ export async function autoFindRoot() { - const found = await findUp('release.json'); + const found = findUp('release.json'); if (!found) { throw new Error(`Could not determine repository root: 'release.json' not found from ${process.cwd()}`); } diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts index 16226c4cde259..f2b5263df06a5 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts @@ -4,6 +4,7 @@ import * as os from 'os'; import * as path from 'path'; import { outputFromStack, AwsClients } from './aws'; import { TestContext } from './integ-test'; +import { findYarnPackages } from './package-sources/repo-source'; import { IPackageSource } from './package-sources/source'; import { packageSourceInSubprocess } from './package-sources/subprocess'; import { RESOURCES_DIR } from './resources'; @@ -612,6 +613,17 @@ function defined(x: A): x is NonNullable { * for Node's dependency lookup mechanism). */ export async function installNpmPackages(fixture: TestFixture, packages: Record) { + if (process.env.REPO_ROOT) { + const monoRepo = await findYarnPackages(process.env.REPO_ROOT); + + // Replace the install target with the physical location of this package + for (const key of Object.keys(packages)) { + if (key in monoRepo) { + packages[key] = monoRepo[key]; + } + } + } + fs.writeFileSync(path.join(fixture.integTestDir, 'package.json'), JSON.stringify({ name: 'cdk-integ-tests', private: true, 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 d094055795e27..7e91514bb6d94 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 @@ -637,6 +637,13 @@ class BuiltinLambdaStack extends cdk.Stack { } } +class NotificationArnPropStack extends cdk.Stack { + constructor(parent, id, props) { + super(parent, id, props); + new sns.Topic(this, 'topic'); + } +} + const app = new cdk.App({ context: { '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build @@ -677,6 +684,10 @@ switch (stackSet) { new DockerStack(app, `${stackPrefix}-docker`); 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`], + }); + // SSO stacks new SsoInstanceAccessControlConfig(app, `${stackPrefix}-sso-access-control`); new SsoAssignment(app, `${stackPrefix}-sso-assignment`); 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 f323110eecfa4..1ce7fe3ef7751 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 @@ -1,7 +1,7 @@ import { promises as fs, existsSync } from 'fs'; import * as os from 'os'; import * as path from 'path'; -import { integTest, cloneDirectory, shell, withDefaultFixture, retry, sleep, randomInteger, withSamIntegrationFixture, RESOURCES_DIR, withCDKMigrateFixture, withExtendedTimeoutFixture, randomString } from '../../lib'; +import { integTest, cloneDirectory, shell, withDefaultFixture, retry, sleep, randomInteger, withSamIntegrationFixture, RESOURCES_DIR, withCDKMigrateFixture, withExtendedTimeoutFixture, randomString, withoutBootstrap } from '../../lib'; jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime @@ -187,7 +187,10 @@ integTest('context setting', withDefaultFixture(async (fixture) => { } })); -integTest('context in stage propagates to top', withDefaultFixture(async (fixture) => { +// bootstrapping also performs synthesis. As it turns out, bootstrap-stage synthesis still causes the lookups to be cached, meaning that the lookup never +// happens when we actually call `cdk synth --no-lookups`. This results in the error never being thrown, because it never tries to lookup anything. +// Fix this by not trying to bootstrap; there's no need to bootstrap anyway, since the test never tries to deploy anything. +integTest('context in stage propagates to top', withoutBootstrap(async (fixture) => { await expect(fixture.cdkSynth({ // This will make it error to prove that the context bubbles up, and also that we can fail on command options: ['--no-lookups'], @@ -466,11 +469,12 @@ integTest('deploy with parameters multi', withDefaultFixture(async (fixture) => ); })); -integTest('deploy with notification ARN', withDefaultFixture(async (fixture) => { - const topicName = `${fixture.stackNamePrefix}-test-topic`; +integTest('deploy with notification ARN as flag', withDefaultFixture(async (fixture) => { + const topicName = `${fixture.stackNamePrefix}-test-topic-flag`; const response = await fixture.aws.sns('createTopic', { Name: topicName }); const topicArn = response.TopicArn!; + try { await fixture.cdkDeploy('test-2', { options: ['--notification-arns', topicArn], @@ -488,6 +492,27 @@ integTest('deploy with notification ARN', withDefaultFixture(async (fixture) => } })); +integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixture) => { + const topicName = `${fixture.stackNamePrefix}-test-topic-prop`; + + const response = await fixture.aws.sns('createTopic', { Name: topicName }); + const topicArn = response.TopicArn!; + + try { + await fixture.cdkDeploy('notification-arn-prop'); + + // verify that the stack we deployed has our notification ARN + const describeResponse = await fixture.aws.cloudFormation('describeStacks', { + StackName: fixture.fullStackName('notification-arn-prop'), + }); + expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]); + } finally { + await fixture.aws.sns('deleteTopic', { + 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('deploy with role', withDefaultFixture(async (fixture) => { diff --git a/packages/aws-cdk-lib/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts b/packages/aws-cdk-lib/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts index 66872401251aa..644d110e70bd9 100644 --- a/packages/aws-cdk-lib/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts +++ b/packages/aws-cdk-lib/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts @@ -55,6 +55,13 @@ export interface AwsCloudFormationStackProperties { */ readonly tags?: { [id: string]: string }; + /** + * SNS Notification ARNs that should receive CloudFormation Stack Events. + * + * @default - No notification arns + */ + readonly notificationArns?: string[]; + /** * The name to use for the CloudFormation stack. * @default - name derived from artifact ID diff --git a/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.schema.json b/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.schema.json index 279dfbe369073..240834b8c4830 100644 --- a/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.schema.json +++ b/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.schema.json @@ -345,6 +345,12 @@ "type": "string" } }, + "notificationArns": { + "type": "array", + "items": { + "type": "string" + } + }, "stackName": { "description": "The name to use for the CloudFormation stack. (Default - name derived from artifact ID)", "type": "string" diff --git a/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.version.json b/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.version.json index 1f0068d32659a..079dd58c72d69 100644 --- a/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.version.json +++ b/packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.version.json @@ -1 +1 @@ -{"version":"36.0.0"} \ No newline at end of file +{"version":"37.0.0"} \ No newline at end of file diff --git a/packages/aws-cdk-lib/core/README.md b/packages/aws-cdk-lib/core/README.md index 140d8920c44de..aac7abe87c167 100644 --- a/packages/aws-cdk-lib/core/README.md +++ b/packages/aws-cdk-lib/core/README.md @@ -1242,6 +1242,18 @@ const stack = new Stack(app, 'StackName', { }); ``` +### Receiving CloudFormation Stack Events + +You can add one or more SNS Topic ARNs to any Stack: + +```ts +const stack = new Stack(app, 'StackName', { + notificationArns: ['arn:aws:sns:us-east-1:23456789012:Topic'], +}); +``` + +Stack events will be sent to any SNS Topics in this list. + ### CfnJson `CfnJson` allows you to postpone the resolution of a JSON blob from diff --git a/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts b/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts index 1017f172a850e..c985c538cac81 100644 --- a/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts +++ b/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts @@ -48,6 +48,7 @@ export function addStackArtifactToAssembly( terminationProtection: stack.terminationProtection, tags: nonEmptyDict(stack.tags.tagValues()), validateOnSynth: session.validateOnSynth, + notificationArns: stack._notificationArns, ...stackProps, ...stackNameProperty, }; diff --git a/packages/aws-cdk-lib/core/lib/stack.ts b/packages/aws-cdk-lib/core/lib/stack.ts index ce3cb9c9b9fd8..f66a571f534e1 100644 --- a/packages/aws-cdk-lib/core/lib/stack.ts +++ b/packages/aws-cdk-lib/core/lib/stack.ts @@ -127,6 +127,13 @@ export interface StackProps { */ readonly tags?: { [key: string]: string }; + /** + * SNS Topic ARNs that will receive stack events. + * + * @default - no notfication arns. + */ + readonly notificationArns?: string[]; + /** * Synthesis method to use while deploying this stack * @@ -364,6 +371,13 @@ export class Stack extends Construct implements ITaggable { */ public readonly _crossRegionReferences: boolean; + /** + * SNS Notification ARNs to receive stack events. + * + * @internal + */ + public readonly _notificationArns: string[]; + /** * Logical ID generation strategy */ @@ -450,6 +464,7 @@ export class Stack extends Construct implements ITaggable { throw new Error(`Stack name must be <= 128 characters. Stack name: '${this._stackName}'`); } this.tags = new TagManager(TagType.KEY_VALUE, 'aws:cdk:stack', props.tags); + 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 82be67b19499b..4846ff3b0cc05 100644 --- a/packages/aws-cdk-lib/core/test/stack.test.ts +++ b/packages/aws-cdk-lib/core/test/stack.test.ts @@ -2075,6 +2075,21 @@ describe('stack', () => { expect(asm.getStackArtifact(stack2.artifactId).tags).toEqual(expected); }); + test('stack notification arns are reflected in the stack artifact properties', () => { + // GIVEN + const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic']; + const app = new App({ stackTraces: false }); + const stack1 = new Stack(app, 'stack1', { + notificationArns: NOTIFICATION_ARNS, + }); + + // THEN + const asm = app.synth(); + const expected = { foo: 'bar' }; + + expect(asm.getStackArtifact(stack1.artifactId).notificationArns).toEqual(NOTIFICATION_ARNS); + }); + test('Termination Protection is reflected in Cloud Assembly artifact', () => { // if the root is an app, invoke "synth" to avoid double synthesis const app = new App(); 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 7cf279c96d924..d73e2a5b33dd7 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 @@ -54,6 +54,11 @@ export class CloudFormationStackArtifact extends CloudArtifact { */ public readonly tags: { [id: string]: string }; + /** + * SNS Topics that will receive stack events. + */ + public readonly notificationArns: string[]; + /** * The physical name of this stack. */ @@ -158,6 +163,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.assumeRoleArn = properties.assumeRoleArn; this.assumeRoleExternalId = properties.assumeRoleExternalId; this.cloudFormationExecutionRoleArn = properties.cloudFormationExecutionRoleArn; diff --git a/packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts b/packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts index 85009cedd7c23..81d5b4a0c3186 100644 --- a/packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts +++ b/packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts @@ -21,6 +21,24 @@ afterEach(() => { rimraf(builder.outdir); }); +test('read notification arns from artifact properties', () => { +// GIVEN + const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic']; + builder.addArtifact('Stack', { + ...stackBase, + properties: { + ...stackBase.properties, + notificationArns: NOTIFICATION_ARNS, + }, + }); + + // WHEN + const assembly = builder.buildAssembly(); + + // THEN + expect(assembly.getStackByName('Stack').notificationArns).toEqual(NOTIFICATION_ARNS); +}); + test('read tags from artifact properties', () => { // GIVEN builder.addArtifact('Stack', { diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index 28af2d39616b0..93a57eb2e4629 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -644,6 +644,12 @@ async function canSkipDeploy( return false; } + // Notification arns have changed + if (!arrayEquals(cloudFormationStack.notificationArns, deployStackOptions.notificationArns ?? [])) { + debug(`${deployName}: notification arns have changed`); + return false; + } + // Termination protection has been updated if (!!deployStackOptions.stack.terminationProtection !== !!cloudFormationStack.terminationProtection) { debug(`${deployName}: termination protection has been updated`); @@ -694,3 +700,7 @@ function suffixWithErrors(msg: string, errors?: string[]) { ? `${msg}: ${errors.join(', ')}` : msg; } + +function arrayEquals(a: any[], b: any[]): boolean { + return a.every(item => b.includes(item)) && b.every(item => a.includes(item)); +} diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts index 23e95f6d618e5..2361871e2bef0 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation.ts @@ -138,12 +138,21 @@ export class CloudFormationStack { /** * The stack's current tags * - * Empty list of the stack does not exist + * Empty list if the stack does not exist */ public get tags(): CloudFormation.Tags { return this.stack?.Tags || []; } + /** + * SNS Topic ARNs that will receive stack events. + * + * Empty list if the stack does not exist + */ + public get notificationArns(): CloudFormation.NotificationARNs { + return this.stack?.NotificationARNs ?? []; + } + /** * Return the names of all current parameters to the stack * diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 854b7ec6419c2..f876634484c46 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -161,7 +161,6 @@ export class CdkToolkit { let changeSet = undefined; if (options.changeSet) { - let stackExists = false; try { stackExists = await this.props.deployments.stackExists({ @@ -214,14 +213,6 @@ export class CdkToolkit { return this.watch(options); } - if (options.notificationArns) { - options.notificationArns.map( arn => { - if (!validateSnsTopicArn(arn)) { - throw new Error(`Notification arn ${arn} is not a valid arn for an SNS topic`); - } - }); - } - const startSynthTime = new Date().getTime(); const stackCollection = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly, options.ignoreNoStacks); @@ -318,7 +309,17 @@ export class CdkToolkit { } } - const stackIndex = stacks.indexOf(stack)+1; + 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`); + } + }); + + const stackIndex = stacks.indexOf(stack) + 1; print('%s: deploying... [%s/%s]', chalk.bold(stack.displayName), stackIndex, stackCollection.stackCount); const startDeployTime = new Date().getTime(); @@ -335,7 +336,7 @@ export class CdkToolkit { roleArn: options.roleArn, toolkitStackName: options.toolkitStackName, reuseAssets: options.reuseAssets, - notificationArns: options.notificationArns, + notificationArns, tags, execute: options.execute, changeSetName: options.changeSetName, diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts index 666d4f43410ec..4aec7cc9ff7d1 100644 --- a/packages/aws-cdk/test/api/deploy-stack.test.ts +++ b/packages/aws-cdk/test/api/deploy-stack.test.ts @@ -460,6 +460,42 @@ test('deploy is not skipped if parameters are different', async () => { })); }); +test('deploy is skipped if notificationArns are the same', async () => { + // GIVEN + givenTemplateIs(FAKE_STACK.template); + givenStackExists({ + NotificationARNs: ['arn:aws:sns:bermuda-triangle-1337:123456789012:TestTopic'], + }); + + // WHEN + await deployStack({ + ...standardDeployStackArguments(), + stack: FAKE_STACK, + notificationArns: ['arn:aws:sns:bermuda-triangle-1337:123456789012:TestTopic'], + }); + + // THEN + expect(cfnMocks.createChangeSet).not.toHaveBeenCalled(); +}); + +test('deploy is not skipped if notificationArns are different', async () => { + // GIVEN + givenTemplateIs(FAKE_STACK.template); + givenStackExists({ + NotificationARNs: ['arn:aws:sns:bermuda-triangle-1337:123456789012:TestTopic'], + }); + + // WHEN + await deployStack({ + ...standardDeployStackArguments(), + stack: FAKE_STACK, + notificationArns: ['arn:aws:sns:bermuda-triangle-1337:123456789012:MagicTopic'], + }); + + // THEN + expect(cfnMocks.createChangeSet).toHaveBeenCalled(); +}); + test('if existing stack failed to create, it is deleted and recreated', async () => { // GIVEN givenStackExists( @@ -624,7 +660,7 @@ test('deploy is not skipped if stack is in a _FAILED state', async () => { await deployStack({ ...standardDeployStackArguments(), usePreviousParameters: true, - }).catch(() => {}); + }).catch(() => { }); // THEN expect(cfnMocks.createChangeSet).toHaveBeenCalled(); diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index 7f70bbc8434de..b8d998b97ef84 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -71,6 +71,8 @@ import { CdkToolkit, Tag } from '../lib/cdk-toolkit'; import { RequireApproval } from '../lib/diff'; import { flatten } from '../lib/util'; +process.env.CXAPI_DISABLE_SELECT_BY_ID = '1'; + let cloudExecutable: MockCloudExecutable; let bootstrapper: jest.Mocked; let stderrMock: jest.SpyInstance; @@ -290,11 +292,11 @@ describe('readCurrentTemplate', () => { // GIVEN // throw error first for the 'prepareSdkWithLookupRoleFor' call and succeed for the rest mockForEnvironment = jest.fn().mockImplementationOnce(() => { throw new Error('error'); }) - .mockImplementation(() => { return { sdk: mockCloudExecutable.sdkProvider.sdk, didAssumeRole: true };}); + .mockImplementation(() => { return { sdk: mockCloudExecutable.sdkProvider.sdk, didAssumeRole: true }; }); mockCloudExecutable.sdkProvider.forEnvironment = mockForEnvironment; mockCloudExecutable.sdkProvider.stubSSM({ getParameter() { - return { }; + return {}; }, }); const cdkToolkit = new CdkToolkit({ @@ -336,7 +338,7 @@ describe('readCurrentTemplate', () => { }); mockCloudExecutable.sdkProvider.stubSSM({ getParameter() { - return { }; + return {}; }, }); @@ -482,108 +484,253 @@ describe('deploy', () => { }); }); - test('with sns notification arns', async () => { - // GIVEN - const notificationArns = [ - 'arn:aws:sns:us-east-2:444455556666:MyTopic', - 'arn:aws:sns:eu-west-1:111155556666:my-great-topic', - ]; - const toolkit = new CdkToolkit({ - cloudExecutable, - configuration: cloudExecutable.configuration, - sdkProvider: cloudExecutable.sdkProvider, - deployments: new FakeCloudFormation({ - 'Test-Stack-A': { Foo: 'Bar' }, - 'Test-Stack-B': { Baz: 'Zinga!' }, - }, notificationArns), + describe('sns notification arns', () => { + beforeEach(() => { + cloudExecutable = new MockCloudExecutable({ + stacks: [ + MockStack.MOCK_STACK_A, + MockStack.MOCK_STACK_B, + MockStack.MOCK_STACK_WITH_NOTIFICATION_ARNS, + MockStack.MOCK_STACK_WITH_BAD_NOTIFICATION_ARNS, + ], + }); }); - // WHEN - await toolkit.deploy({ - selector: { patterns: ['Test-Stack-A', 'Test-Stack-B'] }, - notificationArns, - hotswap: HotswapMode.FULL_DEPLOYMENT, + test('with sns notification arns as options', async () => { + // GIVEN + const notificationArns = [ + 'arn:aws:sns:us-east-2:444455556666:MyTopic', + 'arn:aws:sns:eu-west-1:111155556666:my-great-topic', + ]; + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-A': { Foo: 'Bar' }, + }, notificationArns), + }); + + // WHEN + await toolkit.deploy({ + // Stacks should be selected by their hierarchical ID, which is their displayName, not by the stack ID. + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + notificationArns, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }); }); - }); - test('fail with incorrect sns notification arns', async () => { - // GIVEN - const notificationArns = ['arn:::cfn-my-cool-topic']; - const toolkit = new CdkToolkit({ - cloudExecutable, - configuration: cloudExecutable.configuration, - sdkProvider: cloudExecutable.sdkProvider, - deployments: new FakeCloudFormation({ - 'Test-Stack-A': { Foo: 'Bar' }, - }, notificationArns), + test('fail with incorrect sns notification arns as options', async () => { + // GIVEN + const notificationArns = ['arn:::cfn-my-cool-topic']; + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-A': { Foo: 'Bar' }, + }, notificationArns), + }); + + // WHEN + await expect(() => + toolkit.deploy({ + // Stacks should be selected by their hierarchical ID, which is their displayName, not by the stack ID. + selector: { patterns: ['Test-Stack-A-Display-Name'] }, + notificationArns, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }), + ).rejects.toThrow('Notification arn arn:::cfn-my-cool-topic is not a valid arn for an SNS topic'); }); - // WHEN - await expect(() => - toolkit.deploy({ - selector: { patterns: ['Test-Stack-A'] }, + test('with sns notification arns in the executable', async () => { + // GIVEN + const expectedNotificationArns = [ + 'arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic', + ]; + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-Notification-Arns': { Foo: 'Bar' }, + }, expectedNotificationArns), + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-Notification-Arns'] }, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }); + }); + + test('fail with incorrect sns notification arns in the executable', async () => { + // GIVEN + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-Bad-Notification-Arns': { Foo: 'Bar' }, + }), + }); + + // WHEN + await expect(() => + toolkit.deploy({ + selector: { patterns: ['Test-Stack-Bad-Notification-Arns'] }, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }), + ).rejects.toThrow('Notification arn arn:1337:123456789012:sns:bad is not a valid arn for an SNS topic'); + }); + + test('with sns notification arns in the executable and as options', async () => { + // GIVEN + const notificationArns = [ + 'arn:aws:sns:us-east-2:444455556666:MyTopic', + 'arn:aws:sns:eu-west-1:111155556666:my-great-topic', + ]; + + const expectedNotificationArns = notificationArns.concat(['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic']); + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-Notification-Arns': { Foo: 'Bar' }, + }, expectedNotificationArns), + }); + + // WHEN + await toolkit.deploy({ + selector: { patterns: ['Test-Stack-Notification-Arns'] }, notificationArns, hotswap: HotswapMode.FULL_DEPLOYMENT, - }), - ).rejects.toThrow('Notification arn arn:::cfn-my-cool-topic is not a valid arn for an SNS topic'); + }); + }); + + test('fail with incorrect sns notification arns in the executable and incorrect sns notification arns as options', async () => { + // GIVEN + const notificationArns = ['arn:::cfn-my-cool-topic']; + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-Bad-Notification-Arns': { Foo: 'Bar' }, + }, notificationArns), + }); + + // WHEN + await expect(() => + toolkit.deploy({ + selector: { patterns: ['Test-Stack-Bad-Notification-Arns'] }, + notificationArns, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }), + ).rejects.toThrow('Notification arn arn:::cfn-my-cool-topic is not a valid arn for an SNS topic'); + }); + + test('fail with incorrect sns notification arns in the executable and correct sns notification arns as options', async () => { + // GIVEN + const notificationArns = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic']; + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-Bad-Notification-Arns': { Foo: 'Bar' }, + }, notificationArns), + }); + + // WHEN + await expect(() => + toolkit.deploy({ + selector: { patterns: ['Test-Stack-Bad-Notification-Arns'] }, + notificationArns, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }), + ).rejects.toThrow('Notification arn arn:1337:123456789012:sns:bad is not a valid arn for an SNS topic'); + }); + test('fail with correct sns notification arns in the executable and incorrect sns notification arns as options', async () => { + // GIVEN + const notificationArns = ['arn:::cfn-my-cool-topic']; + const toolkit = new CdkToolkit({ + cloudExecutable, + configuration: cloudExecutable.configuration, + sdkProvider: cloudExecutable.sdkProvider, + deployments: new FakeCloudFormation({ + 'Test-Stack-Notification-Arns': { Foo: 'Bar' }, + }, notificationArns), + }); + + // WHEN + await expect(() => + toolkit.deploy({ + selector: { patterns: ['Test-Stack-Notification-Arns'] }, + notificationArns, + hotswap: HotswapMode.FULL_DEPLOYMENT, + }), + ).rejects.toThrow('Notification arn arn:::cfn-my-cool-topic is not a valid arn for an SNS topic'); + }); }); + }); - test('globless bootstrap uses environment without question', async () => { + test('globless bootstrap uses environment without question', async () => { // GIVEN - const toolkit = defaultToolkitSetup(); - - // WHEN - await toolkit.bootstrap(['aws://56789/south-pole'], bootstrapper, {}); + const toolkit = defaultToolkitSetup(); - // THEN - expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledWith({ - account: '56789', - region: 'south-pole', - name: 'aws://56789/south-pole', - }, expect.anything(), expect.anything()); - expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledTimes(1); - }); + // WHEN + await toolkit.bootstrap(['aws://56789/south-pole'], bootstrapper, {}); - test('globby bootstrap uses whats in the stacks', async () => { - // GIVEN - const toolkit = defaultToolkitSetup(); - cloudExecutable.configuration.settings.set(['app'], 'something'); + // THEN + expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledWith({ + account: '56789', + region: 'south-pole', + name: 'aws://56789/south-pole', + }, expect.anything(), expect.anything()); + expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledTimes(1); + }); - // WHEN - await toolkit.bootstrap(['aws://*/bermuda-triangle-1'], bootstrapper, {}); + test('globby bootstrap uses whats in the stacks', async () => { + // GIVEN + const toolkit = defaultToolkitSetup(); + cloudExecutable.configuration.settings.set(['app'], 'something'); - // THEN - expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledWith({ - account: '123456789012', - region: 'bermuda-triangle-1', - name: 'aws://123456789012/bermuda-triangle-1', - }, expect.anything(), expect.anything()); - expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledTimes(1); - }); + // WHEN + await toolkit.bootstrap(['aws://*/bermuda-triangle-1'], bootstrapper, {}); - test('bootstrap can be invoked without the --app argument', async () => { - // GIVEN - cloudExecutable.configuration.settings.clear(); - const mockSynthesize = jest.fn(); - cloudExecutable.synthesize = mockSynthesize; + // THEN + expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledWith({ + account: '123456789012', + region: 'bermuda-triangle-1', + name: 'aws://123456789012/bermuda-triangle-1', + }, expect.anything(), expect.anything()); + expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledTimes(1); + }); - const toolkit = defaultToolkitSetup(); + test('bootstrap can be invoked without the --app argument', async () => { + // GIVEN + cloudExecutable.configuration.settings.clear(); + const mockSynthesize = jest.fn(); + cloudExecutable.synthesize = mockSynthesize; - // WHEN - await toolkit.bootstrap(['aws://123456789012/west-pole'], bootstrapper, {}); + const toolkit = defaultToolkitSetup(); - // THEN - expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledWith({ - account: '123456789012', - region: 'west-pole', - name: 'aws://123456789012/west-pole', - }, expect.anything(), expect.anything()); - expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledTimes(1); + // WHEN + await toolkit.bootstrap(['aws://123456789012/west-pole'], bootstrapper, {}); - expect(cloudExecutable.hasApp).toEqual(false); - expect(mockSynthesize).not.toHaveBeenCalled(); - }); + // THEN + expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledWith({ + account: '123456789012', + region: 'west-pole', + name: 'aws://123456789012/west-pole', + }, expect.anything(), expect.anything()); + expect(bootstrapper.bootstrapEnvironment).toHaveBeenCalledTimes(1); + + expect(cloudExecutable.hasApp).toEqual(false); + expect(mockSynthesize).not.toHaveBeenCalled(); }); }); @@ -591,7 +738,7 @@ describe('destroy', () => { test('destroy correct stack', async () => { const toolkit = defaultToolkitSetup(); - await expect(() => { + expect(() => { return toolkit.destroy({ selector: { patterns: ['Test-Stack-A/Test-Stack-C'] }, exclusively: true, @@ -854,10 +1001,6 @@ describe('synth', () => { expect(mockData.mock.calls.length).toEqual(0); }); - afterEach(() => { - process.env.STACKS_TO_VALIDATE = undefined; - }); - describe('migrate', () => { const testResourcePath = [__dirname, 'commands', 'test-resources']; const templatePath = [...testResourcePath, 'templates']; @@ -993,13 +1136,13 @@ describe('synth', () => { }); }); - test('causes synth to fail if autoValidate=true', async() => { + test('causes synth to fail if autoValidate=true', async () => { const toolkit = defaultToolkitSetup(); const autoValidate = true; await expect(toolkit.synth([], false, true, autoValidate)).rejects.toBeDefined(); }); - test('causes synth to succeed if autoValidate=false', async() => { + test('causes synth to succeed if autoValidate=false', async () => { const toolkit = defaultToolkitSetup(); const autoValidate = false; await toolkit.synth([], false, true, autoValidate); @@ -1007,7 +1150,7 @@ describe('synth', () => { }); }); - test('stack has error and was explicitly selected', async() => { + test('stack has error and was explicitly selected', async () => { cloudExecutable = new MockCloudExecutable({ stacks: [ MockStack.MOCK_STACK_A, @@ -1123,7 +1266,8 @@ class MockStack { ], }, depends: [MockStack.MOCK_STACK_C.stackName], - } + }; + public static readonly MOCK_STACK_WITH_ERROR: TestStackArtifact = { stackName: 'witherrors', env: 'aws://123456789012/bermuda-triangle-1', @@ -1155,6 +1299,39 @@ class MockStack { }, }, } + public static readonly MOCK_STACK_WITH_NOTIFICATION_ARNS: TestStackArtifact = { + stackName: 'Test-Stack-Notification-Arns', + notificationArns: ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'], + template: { Resources: { TemplateName: 'Test-Stack-Notification-Arns' } }, + env: 'aws://123456789012/bermuda-triangle-1337', + metadata: { + '/Test-Stack-Notification-Arns': [ + { + type: cxschema.ArtifactMetadataEntryType.STACK_TAGS, + data: [ + { key: 'Foo', value: 'Bar' }, + ], + }, + ], + }, + } + + public static readonly MOCK_STACK_WITH_BAD_NOTIFICATION_ARNS: TestStackArtifact = { + stackName: 'Test-Stack-Bad-Notification-Arns', + notificationArns: ['arn:1337:123456789012:sns:bad'], + template: { Resources: { TemplateName: 'Test-Stack-Bad-Notification-Arns' } }, + env: 'aws://123456789012/bermuda-triangle-1337', + metadata: { + '/Test-Stack-Bad-Notification-Arns': [ + { + type: cxschema.ArtifactMetadataEntryType.STACK_TAGS, + data: [ + { key: 'Foo', value: 'Bar' }, + ], + }, + ], + }, + } } class FakeCloudFormation extends Deployments { @@ -1172,9 +1349,7 @@ class FakeCloudFormation extends Deployments { Object.entries(tags).map(([Key, Value]) => ({ Key, Value })) .sort((l, r) => l.Key.localeCompare(r.Key)); } - if (expectedNotificationArns) { - this.expectedNotificationArns = expectedNotificationArns; - } + this.expectedNotificationArns = expectedNotificationArns ?? []; } public deployStack(options: DeployStackOptions): Promise { @@ -1182,7 +1357,11 @@ class FakeCloudFormation extends Deployments { MockStack.MOCK_STACK_A.stackName, MockStack.MOCK_STACK_B.stackName, MockStack.MOCK_STACK_C.stackName, + // MockStack.MOCK_STACK_D deliberately omitted. MockStack.MOCK_STACK_WITH_ASSET.stackName, + MockStack.MOCK_STACK_WITH_ERROR.stackName, + MockStack.MOCK_STACK_WITH_NOTIFICATION_ARNS.stackName, + MockStack.MOCK_STACK_WITH_BAD_NOTIFICATION_ARNS.stackName, ]).toContain(options.stack.stackName); if (this.expectedTags[options.stack.stackName]) { @@ -1213,8 +1392,12 @@ class FakeCloudFormation extends Deployments { return Promise.resolve({}); case MockStack.MOCK_STACK_WITH_ASSET.stackName: return Promise.resolve({}); + case MockStack.MOCK_STACK_WITH_NOTIFICATION_ARNS.stackName: + return Promise.resolve({}); + case MockStack.MOCK_STACK_WITH_BAD_NOTIFICATION_ARNS.stackName: + return Promise.resolve({}); default: - return Promise.reject(`Not an expected mock stack: ${stack.stackName}`); + throw new Error(`not an expected mock stack: ${stack.stackName}`); } } } diff --git a/packages/aws-cdk/test/util.ts b/packages/aws-cdk/test/util.ts index 879d6572f369b..1f059836d670d 100644 --- a/packages/aws-cdk/test/util.ts +++ b/packages/aws-cdk/test/util.ts @@ -16,6 +16,7 @@ export interface TestStackArtifact { env?: string; depends?: string[]; metadata?: cxapi.StackMetadata; + notificationArns?: string[]; /** Old-style assets */ assets?: cxschema.AssetMetadataEntry[]; @@ -101,6 +102,7 @@ function addAttributes(assembly: TestAssembly, builder: cxapi.CloudAssemblyBuild ...stack.properties, templateFile, terminationProtection: stack.terminationProtection, + notificationArns: stack.notificationArns, }, displayName: stack.displayName, });