feat(core): configure Stack SNS notification ARNs on the Stack constr…

…uct (aws#31107) ### Issue # (if applicable) aws#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`. 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. ### Description of how you validated changes 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. ### 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*
michelle-wangg · Sep 23, 2024 · 7e32056 · 7e32056
1 parent 8f49e11
commit 7e32056
Show file tree

Hide file tree

Showing 20 changed files with 527 additions and 165 deletions.
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
@@ -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);
@@ -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`);

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
@@ -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
@@ -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
@@ -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],
@@ -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(

diff --git a/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md b/packages/@aws-cdk/cx-api/FEATURE_FLAGS.md
@@ -73,6 +73,7 @@ Flags come in three types:
 | [@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault](#aws-cdkcustom-resourceslogapiresponsedatapropertytruedefault) | When enabled, the custom resource used for `AwsCustomResource` will configure the `logApiResponseData` property as true by default | 2.145.0 | (fix) |
 | [@aws-cdk/aws-s3:keepNotificationInImportedBucket](#aws-cdkaws-s3keepnotificationinimportedbucket) | When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack. | 2.155.0 | (fix) |
 | [@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask](#aws-cdkaws-stepfunctions-tasksusenews3uriparametersforbedrockinvokemodeltask) | When enabled, use new props for S3 URI field in task definition of state machine for bedrock invoke model. | 2.156.0 | (fix) |
+| [@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions](#aws-cdkaws-ecsreduceec2fargatecloudwatchpermissions) | When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration | 2.159.0 | (fix) |
 
 <!-- END table -->
 
@@ -134,7 +135,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
     "@aws-cdk/aws-ec2:ebsDefaultGp3Volume": true,
     "@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm": true,
     "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false,
-    "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false
+    "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false,
+    "@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true
   }
 }
 ```
@@ -1378,4 +1380,22 @@ When this feature flag is enabled, specify newly introduced props 's3InputUri' a
 **Compatibility with old behavior:** Disable the feature flag to use input and output path fields for s3 URI
 
 
+### @aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions
+
+*When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration* (fix)
+
+Currently, we automatically add a number of cloudwatch permissions to the task role when no cloudwatch log group is
+specified as logConfiguration and it will grant 'Resources': ['*'] to the task role.
+
+When this feature flag is enabled, we will only grant the necessary permissions when users specify cloudwatch log group.
+
+
+| Since | Default | Recommended |
+| ----- | ----- | ----- |
+| (not in v1) |  |  |
+| 2.159.0 | `false` | `true` |
+
+**Compatibility with old behavior:** Disable the feature flag to continue grant permissions to log group when no log group is specified
+
+
 <!-- END details -->
diff --git a/packages/@aws-cdk/cx-api/package.json b/packages/@aws-cdk/cx-api/package.json
@@ -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",

diff --git a/packages/@aws-cdk/integ-runner/package.json b/packages/@aws-cdk/integ-runner/package.json
@@ -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",

diff --git 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
@@ -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
@@ -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
    */
@@ -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}'`);
     }

diff --git a/packages/aws-cdk-lib/core/test/stack.test.ts b/packages/aws-cdk-lib/core/test/stack.test.ts
@@ -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();

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,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
@@ -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/package.json b/packages/aws-cdk-lib/package.json
@@ -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",

diff --git 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
@@ -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
    *