feat(cli): `cdk diff` works for Nested Stacks #18207

comcalvi · 2021-12-29T00:59:33Z

cdk diff now compares all template objects (Resources, Outputs, etc) across nested stacks.

Closes #5722.

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

gitpod-io · 2021-12-29T00:59:36Z

skinny85

Nice work @comcalvi! While the experience is good, the code needs a few more iterations before we can merge it.

I think it's worth to think about the responsibilities here. I believe most of the problems with the code right now are because too much has been put in the CdkToolkit class. I think we need to spread the responsibilities around, so that the code in each class is smaller, and easier to understand.

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

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

skinny85

This is shaping up better, but I think the production code needs some more polish.

Also, this is the experience I see when going from new sns.Topic(this, 'Topic'); to

    this.topic = new sns.Topic(this, 'Topic', {
      topicName: 'myFifoQueue',
      fifo: true,
      contentBasedDeduplication: true,
    });

Stack UsingNestedStacks
Resources
[~] AWS::CloudFormation::Stack SnsNestedStack.NestedStack/SnsNestedStack.NestedStackResource SnsNestedStackNestedStackSnsNestedStackNestedStackResourceAB4E8721 
 └─ [~] Resources
     └─ [~] .TopicBFC7AF6E:
         └─ [+] Added: .Properties

I don't think that's expected, right? (I would expect to see the values of the properties) Probably needs some work there too.

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

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

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

packages/aws-cdk/lib/api/list-stack-resources.ts

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

skinny85

This is getting closer, but still needs some work.

The part I'm most unsure of is changing the formatObjectDiff() code, and how it changes the rendered diff, and whether the way we handle the nested stacks, by just jamming the entire template in place of the resource, will give us the correct classes from running diff that we need for hotswapping.

packages/@aws-cdk/cloudformation-diff/lib/format.ts

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

packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts

packages/aws-cdk/lib/api/hotswap-deployments.ts

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

skinny85

Looks almost perfect! Just a few last finishing touches, and we will get this merged.

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

packages/aws-cdk/test/api/cloudformation-deployments.test.ts

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

skinny85

Looks almost perfect 🙂

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

skinny85 · 2022-02-02T22:24:57Z

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

@@ -104,7 +104,7 @@ export class CdkToolkit {
      // Compare N stacks against deployed templates
      for (const stack of stacks.stackArtifacts) {
        stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
-        const currentTemplate = await this.props.cloudFormation.readCurrentTemplate(stack);
+        const currentTemplate = await this.props.cloudFormation.readCurrentTemplateWithNestedStacks(stack); // TODO; renmae


Can we remove the ToDo then...? 🙂

packages/aws-cdk/test/api/cloudformation-deployments.test.ts

skinny85

👍

mergify · 2022-02-03T00:45:23Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-02-03T01:35:47Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 3bb10f5
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2022-02-03T01:36:07Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

* origin/master: (27 commits) chore(eks): deprecate older versions of EKS (aws#18842) fix(tooling): update vscode devcontainer image (aws#18455) chore: npm-check-updates && yarn upgrade (aws#18832) chore(docs): Fix broken md links (aws#18384) chore(lambda-layer-awscli): install awscli with pip and requirements.txt (aws#18800) fix(aws-appsync): Strip unsupported characters from Lambda DataSource (aws#18765) feat(cfnspec): cloudformation spec v55.0.0 (aws#18827) docs(cfnspec): update CloudFormation documentation (aws#18826) chore(cxapi): plugin context provider limited by cx schema (aws#18709) feat(iotevents): add grant method to Input class (aws#18617) chore(cx-api): break circular dependencies (aws#18767) docs(core): clarify that `addOverride` does not change property casing (aws#18687) feat(s3-deployment): deploy data with deploy-time values (aws#18659) docs(cfnspec): update CloudFormation documentation (aws#18808) feat(cli): `cdk diff` works for Nested Stacks (aws#18207) docs(cfnspec): update CloudFormation documentation (aws#18783) chore(lambda-layer-awscli): add update mechanism for AWS CLI (aws#18780) chore(release): 1.143.0 feat(fsx): add support for FSx Lustre Persistent_2 deployment type (aws#18626) feat(amplify): support performance mode in Branch (aws#18598) ...

`cdk diff` now compares all template objects (Resources, Outputs, etc) across nested stacks. Closes aws#5722. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

comcalvi requested a review from skinny85 December 29, 2021 00:59

github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Dec 29, 2021

github-actions bot assigned rix0rrr Dec 29, 2021

comcalvi assigned skinny85 and unassigned rix0rrr Dec 29, 2021

mergify bot added the contribution/core This is a PR that came from AWS. label Dec 29, 2021

github-actions bot assigned rix0rrr Dec 29, 2021

comcalvi unassigned rix0rrr Dec 29, 2021

skinny85 suggested changes Dec 30, 2021

View reviewed changes

comcalvi requested a review from skinny85 January 4, 2022 23:57

skinny85 suggested changes Jan 5, 2022

View reviewed changes

skinny85 mentioned this pull request Jan 5, 2022

feat(cli): diff now uses the lookup Role for new-style synthesis #18277

Merged

comcalvi requested a review from skinny85 January 13, 2022 22:07

skinny85 suggested changes Jan 14, 2022

View reviewed changes

skinny85 reviewed Jan 14, 2022

View reviewed changes

packages/aws-cdk/lib/api/cloudformation-deployments.ts Outdated Show resolved Hide resolved

comcalvi changed the base branch from master to nija-at/jest-eks January 20, 2022 22:35

comcalvi changed the base branch from nija-at/jest-eks to master January 20, 2022 22:35

github-actions bot assigned rix0rrr Jan 20, 2022

comcalvi closed this Jan 20, 2022

comcalvi force-pushed the diffNestedStacks branch from ff137fd to d735ce4 Compare January 20, 2022 22:39

file deletion

e6bdd04

comcalvi reopened this Jan 20, 2022

removed line

2b0ca60

comcalvi force-pushed the diffNestedStacks branch from fffe982 to 2b0ca60 Compare January 20, 2022 23:47

updated some tests

63d7116

comcalvi unassigned rix0rrr Jan 21, 2022

comcalvi added 3 commits January 21, 2022 12:09

improved testing logic

c3c928f

moving nested template to Properties

3a22820

made tests pass with Properties

de73931

skinny85 reviewed Feb 1, 2022

View reviewed changes

packages/aws-cdk/lib/api/cloudformation-deployments.ts Outdated Show resolved Hide resolved

comcalvi added 3 commits February 1, 2022 11:21

reviews comments

0b4a1c8

renamed magic url in testing

acc0142

removed redundant test

38b5723

comcalvi requested a review from skinny85 February 1, 2022 23:37

skinny85 suggested changes Feb 2, 2022

View reviewed changes

skinny85 reviewed Feb 2, 2022

View reviewed changes

packages/aws-cdk/lib/api/cloudformation-deployments.ts Outdated Show resolved Hide resolved

comcalvi force-pushed the diffNestedStacks branch from 9b78354 to 6f614b7 Compare February 2, 2022 20:15

review comments

54ba003

comcalvi force-pushed the diffNestedStacks branch from 6f614b7 to 54ba003 Compare February 2, 2022 20:23

comcalvi requested a review from skinny85 February 2, 2022 20:29

skinny85 changed the title ~~fix(cli): cdk diff compares template objects across nested stacks~~ feat(cli): cdk diff works for Nested Stacks Feb 2, 2022

skinny85 added the pr-linter/exempt-readme The PR linter will not require README changes label Feb 2, 2022

skinny85 reviewed Feb 2, 2022

View reviewed changes

renamed NestedStackA/B

35e2c3b

comcalvi requested a review from skinny85 February 2, 2022 23:56

skinny85 approved these changes Feb 3, 2022

View reviewed changes

Merge branch 'master' into diffNestedStacks

3bb10f5

mergify bot merged commit 1337b24 into aws:master Feb 3, 2022

skinny85 mentioned this pull request Feb 25, 2022

feat(cli): hotswap support for resources in nested stacks #18950

Merged

peterwoodworth mentioned this pull request Mar 11, 2022

(cdk-diff): >=2.11.0 not authorized to perform: cloudformation:ListStackResources #19351

Closed

rectalogic mentioned this pull request Oct 7, 2022

NestedStack: more mening full diff log output #20392

Open

2 tasks

oxc mentioned this pull request Dec 19, 2022

cli: let cdk diff show nested templates also with --template #23395

Open

2 tasks

skinny85 mentioned this pull request Apr 30, 2023

chore(aws-cdk): Revert "fix(aws-cdk): include nested stacks when building changesets … #19618

Merged

4 tasks

comcalvi mentioned this pull request Jan 3, 2024

(diff): Support Change Set Diffs for Nested Stacks #28568

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): `cdk diff` works for Nested Stacks #18207

feat(cli): `cdk diff` works for Nested Stacks #18207

comcalvi commented Dec 29, 2021 •

edited by skinny85

Loading

gitpod-io bot commented Dec 29, 2021

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment

skinny85 Feb 2, 2022

skinny85 left a comment

mergify bot commented Feb 3, 2022

aws-cdk-automation commented Feb 3, 2022

mergify bot commented Feb 3, 2022

feat(cli): cdk diff works for Nested Stacks #18207

feat(cli): cdk diff works for Nested Stacks #18207

Conversation

comcalvi commented Dec 29, 2021 • edited by skinny85 Loading

gitpod-io bot commented Dec 29, 2021

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 Feb 2, 2022

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 3, 2022

aws-cdk-automation commented Feb 3, 2022

AWS CodeBuild CI Report

mergify bot commented Feb 3, 2022

feat(cli): `cdk diff` works for Nested Stacks #18207

feat(cli): `cdk diff` works for Nested Stacks #18207

comcalvi commented Dec 29, 2021 •

edited by skinny85

Loading