Skip to content

Commit 5f30aa5

Browse files
revert: feat(core): configure SNS topics to receive stack events on the Stack construct (#31100)
Reverts #30551
1 parent 0cdce20 commit 5f30aa5

File tree

19 files changed

+119
-489
lines changed

19 files changed

+119
-489
lines changed

packages/@aws-cdk-testing/cli-integ/lib/package-sources/repo-source.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,13 @@ const YARN_MONOREPO_CACHE: Record<string, any> = {};
7575
*
7676
* Cached in YARN_MONOREPO_CACHE.
7777
*/
78-
export async function findYarnPackages(root: string): Promise<Record<string, string>> {
78+
async function findYarnPackages(root: string): Promise<Record<string, string>> {
7979
if (!(root in YARN_MONOREPO_CACHE)) {
80-
const outputDataString: string = JSON.parse(await shell(['yarn', 'workspaces', '--json', 'info'], {
80+
const output: YarnWorkspacesOutput = JSON.parse(await shell(['yarn', 'workspaces', '--silent', 'info'], {
8181
captureStderr: false,
8282
cwd: root,
8383
show: 'error',
84-
})).data;
85-
const output: YarnWorkspacesOutput = JSON.parse(outputDataString);
84+
}));
8685

8786
const ret: Record<string, string> = {};
8887
for (const [k, v] of Object.entries(output)) {
@@ -97,7 +96,7 @@ export async function findYarnPackages(root: string): Promise<Record<string, str
9796
* Find the root directory of the repo from the current directory
9897
*/
9998
export async function autoFindRoot() {
100-
const found = findUp('release.json');
99+
const found = await findUp('release.json');
101100
if (!found) {
102101
throw new Error(`Could not determine repository root: 'release.json' not found from ${process.cwd()}`);
103102
}

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

-12
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import * as os from 'os';
44
import * as path from 'path';
55
import { outputFromStack, AwsClients } from './aws';
66
import { TestContext } from './integ-test';
7-
import { findYarnPackages } from './package-sources/repo-source';
87
import { IPackageSource } from './package-sources/source';
98
import { packageSourceInSubprocess } from './package-sources/subprocess';
109
import { RESOURCES_DIR } from './resources';
@@ -613,17 +612,6 @@ function defined<A>(x: A): x is NonNullable<A> {
613612
* for Node's dependency lookup mechanism).
614613
*/
615614
export async function installNpmPackages(fixture: TestFixture, packages: Record<string, string>) {
616-
if (process.env.REPO_ROOT) {
617-
const monoRepo = await findYarnPackages(process.env.REPO_ROOT);
618-
619-
// Replace the install target with the physical location of this package
620-
for (const key of Object.keys(packages)) {
621-
if (key in monoRepo) {
622-
packages[key] = monoRepo[key];
623-
}
624-
}
625-
}
626-
627615
fs.writeFileSync(path.join(fixture.integTestDir, 'package.json'), JSON.stringify({
628616
name: 'cdk-integ-tests',
629617
private: true,

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

-11
Original file line numberDiff line numberDiff line change
@@ -637,13 +637,6 @@ class BuiltinLambdaStack extends cdk.Stack {
637637
}
638638
}
639639

640-
class NotificationArnPropStack extends cdk.Stack {
641-
constructor(parent, id, props) {
642-
super(parent, id, props);
643-
new sns.Topic(this, 'topic');
644-
}
645-
}
646-
647640
const app = new cdk.App({
648641
context: {
649642
'@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build
@@ -684,10 +677,6 @@ switch (stackSet) {
684677
new DockerStack(app, `${stackPrefix}-docker`);
685678
new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);
686679

687-
new NotificationArnPropStack(app, `${stackPrefix}-notification-arn-prop`, {
688-
notificationArns: [`arn:aws:sns:${defaultEnv.region}:${defaultEnv.account}:${stackPrefix}-test-topic-prop`],
689-
});
690-
691680
// SSO stacks
692681
new SsoInstanceAccessControlConfig(app, `${stackPrefix}-sso-access-control`);
693682
new SsoAssignment(app, `${stackPrefix}-sso-assignment`);

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

+4-29
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { promises as fs, existsSync } from 'fs';
22
import * as os from 'os';
33
import * as path from 'path';
4-
import { integTest, cloneDirectory, shell, withDefaultFixture, retry, sleep, randomInteger, withSamIntegrationFixture, RESOURCES_DIR, withCDKMigrateFixture, withExtendedTimeoutFixture, randomString, withoutBootstrap } from '../../lib';
4+
import { integTest, cloneDirectory, shell, withDefaultFixture, retry, sleep, randomInteger, withSamIntegrationFixture, RESOURCES_DIR, withCDKMigrateFixture, withExtendedTimeoutFixture, randomString } from '../../lib';
55

66
jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
77

@@ -187,10 +187,7 @@ integTest('context setting', withDefaultFixture(async (fixture) => {
187187
}
188188
}));
189189

190-
// bootstrapping also performs synthesis. As it turns out, bootstrap-stage synthesis still causes the lookups to be cached, meaning that the lookup never
191-
// happens when we actually call `cdk synth --no-lookups`. This results in the error never being thrown, because it never tries to lookup anything.
192-
// Fix this by not trying to bootstrap; there's no need to bootstrap anyway, since the test never tries to deploy anything.
193-
integTest('context in stage propagates to top', withoutBootstrap(async (fixture) => {
190+
integTest('context in stage propagates to top', withDefaultFixture(async (fixture) => {
194191
await expect(fixture.cdkSynth({
195192
// This will make it error to prove that the context bubbles up, and also that we can fail on command
196193
options: ['--no-lookups'],
@@ -469,12 +466,11 @@ integTest('deploy with parameters multi', withDefaultFixture(async (fixture) =>
469466
);
470467
}));
471468

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

475472
const response = await fixture.aws.sns('createTopic', { Name: topicName });
476473
const topicArn = response.TopicArn!;
477-
478474
try {
479475
await fixture.cdkDeploy('test-2', {
480476
options: ['--notification-arns', topicArn],
@@ -492,27 +488,6 @@ integTest('deploy with notification ARN as flag', withDefaultFixture(async (fixt
492488
}
493489
}));
494490

495-
integTest('deploy with notification ARN as prop', withDefaultFixture(async (fixture) => {
496-
const topicName = `${fixture.stackNamePrefix}-test-topic-prop`;
497-
498-
const response = await fixture.aws.sns('createTopic', { Name: topicName });
499-
const topicArn = response.TopicArn!;
500-
501-
try {
502-
await fixture.cdkDeploy('notification-arn-prop');
503-
504-
// verify that the stack we deployed has our notification ARN
505-
const describeResponse = await fixture.aws.cloudFormation('describeStacks', {
506-
StackName: fixture.fullStackName('notification-arn-prop'),
507-
});
508-
expect(describeResponse.Stacks?.[0].NotificationARNs).toEqual([topicArn]);
509-
} finally {
510-
await fixture.aws.sns('deleteTopic', {
511-
TopicArn: topicArn,
512-
});
513-
}
514-
}));
515-
516491
// NOTE: this doesn't currently work with modern-style synthesis, as the bootstrap
517492
// role by default will not have permission to iam:PassRole the created role.
518493
integTest('deploy with role', withDefaultFixture(async (fixture) => {

packages/aws-cdk-lib/cloud-assembly-schema/lib/cloud-assembly/artifact-schema.ts

-7
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,6 @@ export interface AwsCloudFormationStackProperties {
5555
*/
5656
readonly tags?: { [id: string]: string };
5757

58-
/**
59-
* SNS Notification ARNs that should receive CloudFormation Stack Events.
60-
*
61-
* @default - No notification arns
62-
*/
63-
readonly notificationArns?: string[];
64-
6558
/**
6659
* The name to use for the CloudFormation stack.
6760
* @default - name derived from artifact ID

packages/aws-cdk-lib/cloud-assembly-schema/schema/cloud-assembly.schema.json

-6
Original file line numberDiff line numberDiff line change
@@ -345,12 +345,6 @@
345345
"type": "string"
346346
}
347347
},
348-
"notificationArns": {
349-
"type": "array",
350-
"items": {
351-
"type": "string"
352-
}
353-
},
354348
"stackName": {
355349
"description": "The name to use for the CloudFormation stack. (Default - name derived from artifact ID)",
356350
"type": "string"
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"version":"37.0.0"}
1+
{"version":"36.0.0"}

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

-12
Original file line numberDiff line numberDiff line change
@@ -1242,18 +1242,6 @@ const stack = new Stack(app, 'StackName', {
12421242
});
12431243
```
12441244

1245-
### Receiving CloudFormation Stack Events
1246-
1247-
You can add one or more SNS Topic ARNs to any Stack:
1248-
1249-
```ts
1250-
const stack = new Stack(app, 'StackName', {
1251-
notificationArns: ['arn:aws:sns:us-east-1:23456789012:Topic'],
1252-
});
1253-
```
1254-
1255-
Stack events will be sent to any SNS Topics in this list.
1256-
12571245
### CfnJson
12581246

12591247
`CfnJson` allows you to postpone the resolution of a JSON blob from

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

-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ export function addStackArtifactToAssembly(
4848
terminationProtection: stack.terminationProtection,
4949
tags: nonEmptyDict(stack.tags.tagValues()),
5050
validateOnSynth: session.validateOnSynth,
51-
notificationArns: stack._notificationArns,
5251
...stackProps,
5352
...stackNameProperty,
5453
};

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

-15
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,6 @@ export interface StackProps {
127127
*/
128128
readonly tags?: { [key: string]: string };
129129

130-
/**
131-
* SNS Topic ARNs that will receive stack events.
132-
*
133-
* @default - no notfication arns.
134-
*/
135-
readonly notificationArns?: string[];
136-
137130
/**
138131
* Synthesis method to use while deploying this stack
139132
*
@@ -371,13 +364,6 @@ export class Stack extends Construct implements ITaggable {
371364
*/
372365
public readonly _crossRegionReferences: boolean;
373366

374-
/**
375-
* SNS Notification ARNs to receive stack events.
376-
*
377-
* @internal
378-
*/
379-
public readonly _notificationArns: string[];
380-
381367
/**
382368
* Logical ID generation strategy
383369
*/
@@ -464,7 +450,6 @@ export class Stack extends Construct implements ITaggable {
464450
throw new Error(`Stack name must be <= 128 characters. Stack name: '${this._stackName}'`);
465451
}
466452
this.tags = new TagManager(TagType.KEY_VALUE, 'aws:cdk:stack', props.tags);
467-
this._notificationArns = props.notificationArns ?? [];
468453

469454
if (!VALID_STACK_NAME_REGEX.test(this.stackName)) {
470455
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

-15
Original file line numberDiff line numberDiff line change
@@ -2075,21 +2075,6 @@ describe('stack', () => {
20752075
expect(asm.getStackArtifact(stack2.artifactId).tags).toEqual(expected);
20762076
});
20772077

2078-
test('stack notification arns are reflected in the stack artifact properties', () => {
2079-
// GIVEN
2080-
const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];
2081-
const app = new App({ stackTraces: false });
2082-
const stack1 = new Stack(app, 'stack1', {
2083-
notificationArns: NOTIFICATION_ARNS,
2084-
});
2085-
2086-
// THEN
2087-
const asm = app.synth();
2088-
const expected = { foo: 'bar' };
2089-
2090-
expect(asm.getStackArtifact(stack1.artifactId).notificationArns).toEqual(NOTIFICATION_ARNS);
2091-
});
2092-
20932078
test('Termination Protection is reflected in Cloud Assembly artifact', () => {
20942079
// if the root is an app, invoke "synth" to avoid double synthesis
20952080
const app = new App();

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

-6
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,6 @@ export class CloudFormationStackArtifact extends CloudArtifact {
5454
*/
5555
public readonly tags: { [id: string]: string };
5656

57-
/**
58-
* SNS Topics that will receive stack events.
59-
*/
60-
public readonly notificationArns: string[];
61-
6257
/**
6358
* The physical name of this stack.
6459
*/
@@ -163,7 +158,6 @@ export class CloudFormationStackArtifact extends CloudArtifact {
163158
// We get the tags from 'properties' if available (cloud assembly format >= 6.0.0), otherwise
164159
// from the stack metadata
165160
this.tags = properties.tags ?? this.tagsFromMetadata();
166-
this.notificationArns = properties.notificationArns ?? [];
167161
this.assumeRoleArn = properties.assumeRoleArn;
168162
this.assumeRoleExternalId = properties.assumeRoleExternalId;
169163
this.cloudFormationExecutionRoleArn = properties.cloudFormationExecutionRoleArn;

packages/aws-cdk-lib/cx-api/test/stack-artifact.test.ts

-18
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,6 @@ afterEach(() => {
2121
rimraf(builder.outdir);
2222
});
2323

24-
test('read notification arns from artifact properties', () => {
25-
// GIVEN
26-
const NOTIFICATION_ARNS = ['arn:aws:sns:bermuda-triangle-1337:123456789012:MyTopic'];
27-
builder.addArtifact('Stack', {
28-
...stackBase,
29-
properties: {
30-
...stackBase.properties,
31-
notificationArns: NOTIFICATION_ARNS,
32-
},
33-
});
34-
35-
// WHEN
36-
const assembly = builder.buildAssembly();
37-
38-
// THEN
39-
expect(assembly.getStackByName('Stack').notificationArns).toEqual(NOTIFICATION_ARNS);
40-
});
41-
4224
test('read tags from artifact properties', () => {
4325
// GIVEN
4426
builder.addArtifact('Stack', {

packages/aws-cdk/lib/api/deploy-stack.ts

-10
Original file line numberDiff line numberDiff line change
@@ -644,12 +644,6 @@ async function canSkipDeploy(
644644
return false;
645645
}
646646

647-
// Notification arns have changed
648-
if (!arrayEquals(cloudFormationStack.notificationArns, deployStackOptions.notificationArns ?? [])) {
649-
debug(`${deployName}: notification arns have changed`);
650-
return false;
651-
}
652-
653647
// Termination protection has been updated
654648
if (!!deployStackOptions.stack.terminationProtection !== !!cloudFormationStack.terminationProtection) {
655649
debug(`${deployName}: termination protection has been updated`);
@@ -700,7 +694,3 @@ function suffixWithErrors(msg: string, errors?: string[]) {
700694
? `${msg}: ${errors.join(', ')}`
701695
: msg;
702696
}
703-
704-
function arrayEquals(a: any[], b: any[]): boolean {
705-
return a.every(item => b.includes(item)) && b.every(item => a.includes(item));
706-
}

packages/aws-cdk/lib/api/util/cloudformation.ts

+1-10
Original file line numberDiff line numberDiff line change
@@ -138,21 +138,12 @@ export class CloudFormationStack {
138138
/**
139139
* The stack's current tags
140140
*
141-
* Empty list if the stack does not exist
141+
* Empty list of the stack does not exist
142142
*/
143143
public get tags(): CloudFormation.Tags {
144144
return this.stack?.Tags || [];
145145
}
146146

147-
/**
148-
* SNS Topic ARNs that will receive stack events.
149-
*
150-
* Empty list if the stack does not exist
151-
*/
152-
public get notificationArns(): CloudFormation.NotificationARNs {
153-
return this.stack?.NotificationARNs ?? [];
154-
}
155-
156147
/**
157148
* Return the names of all current parameters to the stack
158149
*

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

+11-12
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ export class CdkToolkit {
161161
let changeSet = undefined;
162162

163163
if (options.changeSet) {
164+
164165
let stackExists = false;
165166
try {
166167
stackExists = await this.props.deployments.stackExists({
@@ -213,6 +214,14 @@ export class CdkToolkit {
213214
return this.watch(options);
214215
}
215216

217+
if (options.notificationArns) {
218+
options.notificationArns.map( arn => {
219+
if (!validateSnsTopicArn(arn)) {
220+
throw new Error(`Notification arn ${arn} is not a valid arn for an SNS topic`);
221+
}
222+
});
223+
}
224+
216225
const startSynthTime = new Date().getTime();
217226
const stackCollection = await this.selectStacksForDeploy(options.selector, options.exclusively,
218227
options.cacheCloudAssembly, options.ignoreNoStacks);
@@ -309,17 +318,7 @@ export class CdkToolkit {
309318
}
310319
}
311320

312-
let notificationArns: string[] = [];
313-
notificationArns = notificationArns.concat(options.notificationArns ?? []);
314-
notificationArns = notificationArns.concat(stack.notificationArns);
315-
316-
notificationArns.map(arn => {
317-
if (!validateSnsTopicArn(arn)) {
318-
throw new Error(`Notification arn ${arn} is not a valid arn for an SNS topic`);
319-
}
320-
});
321-
322-
const stackIndex = stacks.indexOf(stack) + 1;
321+
const stackIndex = stacks.indexOf(stack)+1;
323322
print('%s: deploying... [%s/%s]', chalk.bold(stack.displayName), stackIndex, stackCollection.stackCount);
324323
const startDeployTime = new Date().getTime();
325324

@@ -336,7 +335,7 @@ export class CdkToolkit {
336335
roleArn: options.roleArn,
337336
toolkitStackName: options.toolkitStackName,
338337
reuseAssets: options.reuseAssets,
339-
notificationArns,
338+
notificationArns: options.notificationArns,
340339
tags,
341340
execute: options.execute,
342341
changeSetName: options.changeSetName,

0 commit comments

Comments
 (0)