Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revert: feat(core): configure SNS topics to receive stack events on the Stack construct #31100

Merged
merged 1 commit into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,13 @@ const YARN_MONOREPO_CACHE: Record<string, any> = {};
*
* Cached in YARN_MONOREPO_CACHE.
*/
export async function findYarnPackages(root: string): Promise<Record<string, string>> {
async function findYarnPackages(root: string): Promise<Record<string, string>> {
if (!(root in YARN_MONOREPO_CACHE)) {
const outputDataString: string = JSON.parse(await shell(['yarn', 'workspaces', '--json', 'info'], {
const output: YarnWorkspacesOutput = JSON.parse(await shell(['yarn', 'workspaces', '--silent', 'info'], {
captureStderr: false,
cwd: root,
show: 'error',
})).data;
const output: YarnWorkspacesOutput = JSON.parse(outputDataString);
}));

const ret: Record<string, string> = {};
for (const [k, v] of Object.entries(output)) {
Expand All @@ -97,7 +96,7 @@ export async function findYarnPackages(root: string): Promise<Record<string, str
* Find the root directory of the repo from the current directory
*/
export async function autoFindRoot() {
const found = findUp('release.json');
const found = await findUp('release.json');
if (!found) {
throw new Error(`Could not determine repository root: 'release.json' not found from ${process.cwd()}`);
}
Expand Down
12 changes: 0 additions & 12 deletions packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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';
Expand Down Expand Up @@ -613,17 +612,6 @@ function defined<A>(x: A): x is NonNullable<A> {
* for Node's dependency lookup mechanism).
*/
export async function installNpmPackages(fixture: TestFixture, packages: Record<string, string>) {
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,
Expand Down
11 changes: 0 additions & 11 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 @@ -637,13 +637,6 @@ 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
Expand Down Expand Up @@ -684,10 +677,6 @@ 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
@@ -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, withoutBootstrap } from '../../lib';
import { integTest, cloneDirectory, shell, withDefaultFixture, retry, sleep, randomInteger, withSamIntegrationFixture, RESOURCES_DIR, withCDKMigrateFixture, withExtendedTimeoutFixture, randomString } from '../../lib';

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

Expand Down Expand Up @@ -187,10 +187,7 @@ integTest('context setting', 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) => {
integTest('context in stage propagates to top', withDefaultFixture(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'],
Expand Down Expand Up @@ -469,12 +466,11 @@ integTest('deploy with parameters multi', withDefaultFixture(async (fixture) =>
);
}));

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

const response = await fixture.aws.sns('createTopic', { Name: topicName });
const topicArn = response.TopicArn!;

try {
await fixture.cdkDeploy('test-2', {
options: ['--notification-arns', topicArn],
Expand All @@ -492,27 +488,6 @@ integTest('deploy with notification ARN as flag', withDefaultFixture(async (fixt
}
}));

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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,6 @@ 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,6 @@
"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"
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"37.0.0"}
{"version":"36.0.0"}
12 changes: 0 additions & 12 deletions packages/aws-cdk-lib/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1242,18 +1242,6 @@ 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,7 +48,6 @@ export function addStackArtifactToAssembly(
terminationProtection: stack.terminationProtection,
tags: nonEmptyDict(stack.tags.tagValues()),
validateOnSynth: session.validateOnSynth,
notificationArns: stack._notificationArns,
...stackProps,
...stackNameProperty,
};
Expand Down
15 changes: 0 additions & 15 deletions packages/aws-cdk-lib/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,6 @@ 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 @@ -371,13 +364,6 @@ 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 @@ -464,7 +450,6 @@ 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}'`);
Expand Down
15 changes: 0 additions & 15 deletions packages/aws-cdk-lib/core/test/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2075,21 +2075,6 @@ 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ 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 @@ -163,7 +158,6 @@ 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: 0 additions & 18 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,24 +21,6 @@ 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
10 changes: 0 additions & 10 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -644,12 +644,6 @@ 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 @@ -700,7 +694,3 @@ 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: 1 addition & 10 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,21 +138,12 @@ export class CloudFormationStack {
/**
* The stack's current tags
*
* Empty list if the stack does not exist
* Empty list of 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: 11 additions & 12 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ export class CdkToolkit {
let changeSet = undefined;

if (options.changeSet) {

let stackExists = false;
try {
stackExists = await this.props.deployments.stackExists({
Expand Down Expand Up @@ -213,6 +214,14 @@ 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 @@ -309,17 +318,7 @@ 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`);
}
});

const stackIndex = stacks.indexOf(stack) + 1;
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 @@ -336,7 +335,7 @@ export class CdkToolkit {
roleArn: options.roleArn,
toolkitStackName: options.toolkitStackName,
reuseAssets: options.reuseAssets,
notificationArns,
notificationArns: options.notificationArns,
tags,
execute: options.execute,
changeSetName: options.changeSetName,
Expand Down
Loading
Loading