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

comcalvi · 2024-06-13T21:56:16Z

Issue # (if applicable)

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.

Description of how you validated changes

Unit tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…ion-arns

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

…ion-arns

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

…ion-arns

aws-cdk-automation · 2024-07-15T16:26:09Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

aws-cdk-automation · 2024-07-15T16:53:50Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: f704997
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

scanlonp

Looks solid. Couple clarifying questions so I can get a better sense of a few parts of the PR.

Other than the individual comments, main question would be about the notificationArns being put into the toolkit in some tests. From a user perspective, these options are added on the stack. Are the toolkit Arns a product of the cli option?

scanlonp · 2024-07-17T00:42:45Z

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

+  if (process.env.REPO_ROOT) {
+    const monoRepo = await findYarnPackages(process.env.REPO_ROOT ?? '');


When we go into this if block, will REPO_ROOT ever be undefined?

scanlonp · 2024-07-17T00:44:08Z

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

@@ -75,13 +75,14 @@ const YARN_MONOREPO_CACHE: Record<string, any> = {};
  *
  * Cached in YARN_MONOREPO_CACHE.
  */
-async function findYarnPackages(root: string): Promise<Record<string, string>> {
+export async function findYarnPackages(root: string): Promise<Record<string, string>> {


Quick blurb about the purpose of these changes? Is this cli integ test related?

scanlonp · 2024-07-17T00:46:23Z

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

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

-integTest('context in stage propagates to top', withDefaultFixture(async (fixture) => {
+// bootstrapping also performs synthesis. As it turns out, bootstarp-stage synthesis still causes the lookups to be cached, meaning that the lookup never


Suggested change

// bootstrapping also performs synthesis. As it turns out, bootstarp-stage synthesis still causes the lookups to be cached, meaning that the lookup never

// bootstrapping also performs synthesis. As it turns out, bootstrap-stage synthesis still causes the lookups to be cached, meaning that the lookup never

scanlonp · 2024-07-17T00:46:58Z

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

+// bootstrapping also performs synthesis. As it turns out, bootstarp-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) => {


scanlonp · 2024-07-17T00:56:48Z

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

+          sdkProvider: cloudExecutable.sdkProvider,
+          deployments: new FakeCloudFormation({
+            'Test-Stack-A': { Foo: 'Bar' },
+          }, notificationArns),


Why are the arns being passed into the toolkit?

comcalvi added 10 commits June 11, 2024 15:59

working basic implementation

48f5b63

Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…

f05b908

…ion-arns

Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…

868cb1f

…ion-arns

fix

f5c930b

don't skip deploy if stack notifications arnrns have changed

662697d

more changes

bc0afea

cleanup

b3a5716

most tests working

4b88b03

toolkit tests

af40c5e

docs

ffaf14e

github-actions bot added the p2 label Jun 13, 2024

aws-cdk-automation requested a review from a team June 13, 2024 21:56

mergify bot added the contribution/core This is a PR that came from AWS. label Jun 13, 2024

comcalvi had a problem deploying to test-pipeline June 13, 2024 21:57 — with GitHub Actions Failure

aws-cdk-automation previously requested changes Jun 13, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Jun 13, 2024

comcalvi added 2 commits June 13, 2024 15:07

remove the bad cast, it is evil

055d496

cleanup

fc184cc

comcalvi had a problem deploying to test-pipeline June 13, 2024 22:08 — with GitHub Actions Failure

comcalvi had a problem deploying to test-pipeline June 13, 2024 22:09 — with GitHub Actions Failure

comcalvi added 3 commits June 13, 2024 15:25

Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…

fac1245

…ion-arns

silly licenses

5e606af

newline\n...

180066b

comcalvi had a problem deploying to test-pipeline June 13, 2024 22:27 — with GitHub Actions Failure

foo

2bc9320

comcalvi added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Jun 13, 2024

comcalvi temporarily deployed to test-pipeline June 13, 2024 23:02 — with GitHub Actions Inactive

comcalvi temporarily deployed to test-pipeline June 14, 2024 17:08 — with GitHub Actions Inactive

small change to trigger test pipeline

e9f9fa4

comcalvi temporarily deployed to test-pipeline June 14, 2024 18:38 — with GitHub Actions Inactive

comcalvi added the p1 label Jul 2, 2024

wowowo cli integ test

c38e7a4

comcalvi temporarily deployed to test-pipeline July 2, 2024 15:58 — with GitHub Actions Inactive

integ-fix

f79368f

comcalvi temporarily deployed to test-pipeline July 2, 2024 20:19 — with GitHub Actions Inactive

fix deploy all test

b03209d

comcalvi requested a deployment to test-pipeline July 2, 2024 20:24 — with GitHub Actions Waiting

wowowo

22bb7e8

comcalvi temporarily deployed to test-pipeline July 2, 2024 20:26 — with GitHub Actions Inactive

comcalvi removed the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Jul 2, 2024

aws-cdk-automation previously requested changes Jul 2, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Jul 2, 2024

fixing the cli test framework

bf5deb9

comcalvi temporarily deployed to test-pipeline July 10, 2024 23:06 — with GitHub Actions Inactive

fix

95e8dac

comcalvi temporarily deployed to test-pipeline July 10, 2024 23:29 — with GitHub Actions Inactive

fix the busted test

4331c91

comcalvi temporarily deployed to test-pipeline July 12, 2024 21:08 — with GitHub Actions Inactive

fix

f4c65a2

comcalvi temporarily deployed to test-pipeline July 12, 2024 23:33 — with GitHub Actions Inactive

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 13, 2024

Merge branch 'main' of github.com:aws/aws-cdk into comcalvi/notificat…

f704997

…ion-arns

comcalvi temporarily deployed to test-pipeline July 15, 2024 16:25 — with GitHub Actions Inactive

comcalvi added pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Jul 15, 2024

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 15, 2024

scanlonp reviewed Jul 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

comcalvi commented Jun 13, 2024 •

edited

Loading

aws-cdk-automation left a comment

aws-cdk-automation left a comment

aws-cdk-automation commented Jul 15, 2024

aws-cdk-automation commented Jul 15, 2024

scanlonp left a comment

scanlonp Jul 17, 2024

scanlonp Jul 17, 2024

scanlonp Jul 17, 2024

scanlonp Jul 17, 2024

scanlonp Jul 17, 2024

		if (process.env.REPO_ROOT) {
		const monoRepo = await findYarnPackages(process.env.REPO_ROOT ?? '');

	// bootstrapping also performs synthesis. As it turns out, bootstarp-stage synthesis still causes the lookups to be cached, meaning that the lookup never
	// bootstrapping also performs synthesis. As it turns out, bootstrap-stage synthesis still causes the lookups to be cached, meaning that the lookup never

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

Are you sure you want to change the base?

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

Conversation

comcalvi commented Jun 13, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jul 15, 2024

aws-cdk-automation commented Jul 15, 2024

AWS CodeBuild CI Report

scanlonp left a comment

Choose a reason for hiding this comment

scanlonp Jul 17, 2024

Choose a reason for hiding this comment

scanlonp Jul 17, 2024

Choose a reason for hiding this comment

scanlonp Jul 17, 2024

Choose a reason for hiding this comment

scanlonp Jul 17, 2024

Choose a reason for hiding this comment

scanlonp Jul 17, 2024

Choose a reason for hiding this comment

comcalvi commented Jun 13, 2024 •

edited

Loading