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

TheRealAmazonKendra · web-flow · commit 5f30aa5f81ef · 2024-08-13T23:55:50.000Z
Reverts #30551
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
@@ -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)) {
@@ -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()}`);
   }
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
@@ -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';
@@ -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,
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
@@ -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
@@ -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`);
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
@@ -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
 
@@ -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'],
@@ -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],
@@ -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) => {
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
@@ -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
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
@@ -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"
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
@@ -1 +1 @@
-{"version":"37.0.0"}
+{"version":"36.0.0"}
diff --git a/packages/aws-cdk-lib/core/README.md b/packages/aws-cdk-lib/core/README.md
@@ -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
diff --git a/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts b/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts
@@ -48,7 +48,6 @@ 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
@@ -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
    *
@@ -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
    */
@@ -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}'`);
diff --git a/packages/aws-cdk-lib/core/test/stack.test.ts b/packages/aws-cdk-lib/core/test/stack.test.ts
@@ -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();
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
@@ -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.
    */
@@ -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;
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
@@ -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', {
diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -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`);
@@ -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));
-}
diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts
@@ -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
    *
diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts
@@ -161,6 +161,7 @@ export class CdkToolkit {
         let changeSet = undefined;
 
         if (options.changeSet) {
+
           let stackExists = false;
           try {
             stackExists = await this.props.deployments.stackExists({
@@ -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);
@@ -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();
 
@@ -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,
diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts
diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts
diff --git a/packages/aws-cdk/test/util.ts b/packages/aws-cdk/test/util.ts

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-{"version":"37.0.0"}`
	`1`	`+{"version":"36.0.0"}`