Skip to content

Commit

Permalink
feat(core): configure Stack SNS notification ARNs on the Stack constr…
Browse files Browse the repository at this point in the history
…uct (#31107)

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.

Added a new optional stack prop, `notificationArns`, that is written to the CloudAssembly and concatenated with the CLI option `--notification-arns`.

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 expect.

Depends on: cdklabs/cdk-assets#87 and cdklabs/cloud-assembly-schema#58.

Unit tests + CLI integ test. Framework integ tests not included because they would require an externally-created SNS Topic, which is not what we want in integ tests; besides, the case is covered by the CLI integ test.

- [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*
  • Loading branch information
comcalvi authored and HBobertz committed Sep 24, 2024
1 parent 20ceec8 commit f8e5469
Show file tree
Hide file tree
Showing 19 changed files with 506 additions and 164 deletions.
10 changes: 10 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,12 @@ class BuiltinLambdaStack extends cdk.Stack {
}
}

class NotificationArnPropStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
new sns.Topic(this, 'topic');
}
}
class AppSyncHotswapStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -708,6 +714,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`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
withCDKMigrateFixture,
withExtendedTimeoutFixture,
randomString,
withoutBootstrap,
} from '../../lib';

jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
Expand Down Expand Up @@ -276,9 +277,12 @@ integTest(
}),
);

// 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',
withDefaultFixture(async (fixture) => {
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
Expand Down Expand Up @@ -613,12 +617,13 @@ integTest(
);

integTest(
'deploy with notification ARN',
'deploy with notification ARN as flag',
withDefaultFixture(async (fixture) => {
const topicName = `${fixture.stackNamePrefix}-test-topic`;
const topicName = `${fixture.stackNamePrefix}-test-topic-flag`;

const response = await fixture.aws.sns.send(new CreateTopicCommand({ Name: topicName }));
const topicArn = response.TopicArn!;

try {
await fixture.cdkDeploy('test-2', {
options: ['--notification-arns', topicArn],
Expand All @@ -641,6 +646,31 @@ integTest(
}),
);

integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixture) => {
const topicName = `${fixture.stackNamePrefix}-test-topic-prop`;

const response = await fixture.aws.sns.send(new CreateTopicCommand({ 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.send(
new DescribeStacksCommand({
StackName: fixture.fullStackName('notification-arn-prop'),
}),
);
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(
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cx-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@
"semver": "^7.6.3"
},
"peerDependencies": {
"@aws-cdk/cloud-assembly-schema": "^36.0.5"
"@aws-cdk/cloud-assembly-schema": "^38.0.0"
},
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^29.5.12",
"@types/mock-fs": "^4.13.4",
Expand Down
5 changes: 3 additions & 2 deletions packages/@aws-cdk/integ-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@
},
"dependencies": {
"chokidar": "^3.6.0",
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"cdk-assets": "^2.154.0",
"@aws-cdk/aws-service-spec": "^0.1.24",
"cdk-assets": "^2.151.29",

"@aws-cdk/cdk-cli-wrapper": "0.0.0",
"aws-cdk": "0.0.0",
"chalk": "^4",
Expand Down
12 changes: 12 additions & 0 deletions packages/aws-cdk-lib/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function addStackArtifactToAssembly(
terminationProtection: stack.terminationProtection,
tags: nonEmptyDict(stack.tags.tagValues()),
validateOnSynth: session.validateOnSynth,
notificationArns: stack._notificationArns,
...stackProps,
...stackNameProperty,
};
Expand Down
22 changes: 22 additions & 0 deletions packages/aws-cdk-lib/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -451,6 +465,14 @@ export class Stack extends Construct implements ITaggable {
}
this.tags = new TagManager(TagType.KEY_VALUE, 'aws:cdk:stack', props.tags);

for (const notificationArn of props.notificationArns ?? []) {
if (Token.isUnresolved(notificationArn)) {
throw new Error(`Stack '${id}' includes one or more tokens in its notification ARNs: ${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}'`);
}
Expand Down
26 changes: 26 additions & 0 deletions packages/aws-cdk-lib/core/test/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,32 @@ 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,
});

// WHEN
const asm = app.synth();

// THEN
expect(asm.getStackArtifact(stack1.artifactId).notificationArns).toEqual(NOTIFICATION_ARNS);
});

test('throws if stack notification arns contain tokens', () => {
// GIVEN
const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];
const app = new App({ stackTraces: false });

// THEN
expect(() => new Stack(app, 'stack1', {
notificationArns: [...NOTIFICATION_ARNS, Aws.URL_SUFFIX],
})).toThrow('includes one or more tokens in its 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
Expand Down
18 changes: 18 additions & 0 deletions packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
"@aws-cdk/asset-awscli-v1": "^2.2.202",
"@aws-cdk/asset-kubectl-v20": "^2.1.2",
"@aws-cdk/asset-node-proxy-agent-v6": "^2.1.0",
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@balena/dockerignore": "^1.0.2",
"case": "1.6.3",
"fs-extra": "^11.2.0",
Expand Down
10 changes: 10 additions & 0 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down Expand Up @@ -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));
}
11 changes: 10 additions & 1 deletion packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
23 changes: 12 additions & 11 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ export class CdkToolkit {
let changeSet = undefined;

if (options.changeSet) {

let stackExists = false;
try {
stackExists = await this.props.deployments.stackExists({
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();

Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
"xml-js": "^1.6.11"
},
"dependencies": {
"@aws-cdk/cloud-assembly-schema": "^36.0.24",
"@aws-cdk/cloud-assembly-schema": "^38.0.0",
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/region-info": "0.0.0",
Expand Down Expand Up @@ -124,7 +124,7 @@
"aws-sdk": "^2.1691.0",
"aws-sdk-js-codemod": "^2.3.2",
"camelcase": "^6.3.0",
"cdk-assets": "^2.151.29",
"cdk-assets": "^2.154.0",
"cdk-from-cfn": "^0.162.0",
"chalk": "^4",
"chokidar": "^3.6.0",
Expand Down
Loading

0 comments on commit f8e5469

Please sign in to comment.