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

comcalvi · 2022-02-12T01:10:01Z

Resources in nested stacks can now be hotswapped.

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

…sure that resource lookups work

…ent resource type (with the same logical ID) results in a full deployment, and vice versa

…K calls or duplicated code

gitpod-io · 2022-02-12T01:10:08Z

skinny85

I just skimmed for now (this is quite complex), but one thing stuck out to me.

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

skinny85

Thanks for the PR @comcalvi! This is a good first attempt, but there are some functional problems with the current code, and some design and organizational ones, before we can merge this in.

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

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

packages/aws-cdk/test/api/hotswap/nested-stacks-hotswap.test.ts

skinny85

Looks great @comcalvi! I don't have any comments on the functionality, I only have some comments on the code details.

The most important thing I want to see before we merge this in is getting rid of the multiple calls to diff the Stacks. Ideally, we should perform a single diff, that includes the Nested Stacks, and use that for hotswapping. The same way we don't call diff multiple times for the cdk diff command for Nested Stacks.

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

packages/aws-cdk/lib/api/nested-stack-helpers.ts

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

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

packages/aws-cdk/lib/api/nested-stack-helpers.ts

skinny85

Looks almost perfect! One last iteration, and we'll get this merged in.

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

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

packages/aws-cdk/lib/api/nested-stack-helpers.ts

packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts

skinny85 · 2022-02-25T19:23:07Z

packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts

@@ -60,16 +96,22 @@ export function stackSummaryOf(logicalId: string, resourceType: string, physical
 export class HotswapMockSdkProvider {
  public readonly mockSdkProvider: MockSdkProvider;

-  constructor() {
+  constructor(rootStackName?: string, childStacks?: string[]) {


How about

Suggested change

constructor(rootStackName?: string, childStacks?: string[]) {

constructor(rootStackName?: string, childStacks: string[] = []) {

? Will save you the null checks on childStacks below.

I think we don't actually need childStacks at all, I've reworked this to be better.

packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts

skinny85 · 2022-02-25T19:30:34Z

packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts

@@ -454,7 +454,7 @@ test("will not perform a hotswap deployment if it doesn't know how to handle a s
    template: {
      Resources: {
        Bucket: {
-          Type: 'AWS::Lambda::Function',
+          Type: 'AWS::S3::Bucket',


Nice catch 😉

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

skinny85

Looks great @comcalvi!

mergify · 2022-02-25T22:15:59Z

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-25T22:58:50Z

AWS CodeBuild CI Report

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

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

mergify · 2022-02-25T22:59:09Z

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).

comcalvi added 18 commits February 3, 2022 15:17

improved testing framework to allow hotswap nestedStack testing

5454912

first test case working

fe40a74

found broken case

d729b5c

nested resources don't destroy their ancestors

576dd36

sibling stacks no longer break hotswap deployments

0adaa27

new evaluateCfnTemplates are now created for each nested stack, to en…

e401a2e

…sure that resource lookups work

ensured that changing the resource type of a nested stack to a differ…

32d1c57

…ent resource type (with the same logical ID) results in a full deployment, and vice versa

notes

ced3ca2

added new return value, still need to add type, but no more wasted SD…

9ce0acc

…K calls or duplicated code

made all unit tests pass

4a78161

test refactoring to use same file

c2a3482

use .filter() instead of array mutation

7f83a06

1

d2092a5

updated comment

05d2921

add blank line

082ff5d

old-style synthesis mostly works

a86b11e

finished implementing support for old-style synthesis

03ac7b8

revisions

cd7e430

comcalvi requested a review from skinny85 February 12, 2022 01:10

github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Feb 12, 2022

github-actions bot assigned rix0rrr Feb 12, 2022

mergify bot added the contribution/core This is a PR that came from AWS. label Feb 12, 2022

readme

f99d4e0

comcalvi assigned skinny85 and unassigned rix0rrr Feb 12, 2022

skinny85 reviewed Feb 12, 2022

View reviewed changes

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

reused variable that should have been used

3dcfaed

comcalvi requested a review from skinny85 February 14, 2022 16:23

skinny85 suggested changes Feb 15, 2022

View reviewed changes

comcalvi added 7 commits February 23, 2022 16:01

moved nested logic to new helper file

dc86ec2

evaluateCfnExpression() used instead of uneeded loop

c31b098

combined into one loop

a6c8eea

comments

9005ce1

updated comment

a2a658d

comment

6b01df9

comment

8034f84

comcalvi force-pushed the hotswap-nested-stacks branch from d8d5cd1 to 8034f84 Compare February 24, 2022 00:25

comcalvi requested a review from skinny85 February 24, 2022 00:26

skinny85 suggested changes Feb 25, 2022

View reviewed changes

comcalvi added 2 commits February 24, 2022 21:00

review comments

8c6c423

function now returns nestedStackNames

573cfb7

comcalvi requested a review from skinny85 February 25, 2022 18:28

added new type

50a1edc

skinny85 suggested changes Feb 25, 2022

View reviewed changes

review comments

37bf486

skinny85 reviewed Feb 25, 2022

View reviewed changes

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

comcalvi added 3 commits February 25, 2022 12:39

review comments

c757cd7

removed unneeded childStacks from testing logic

9d8d824

finalized comments

9115312

comcalvi requested a review from skinny85 February 25, 2022 21:10

uncomment

d733325

skinny85 approved these changes Feb 25, 2022

View reviewed changes

Merge branch 'master' into hotswap-nested-stacks

d70f600

mergify bot merged commit 2ea9da1 into aws:master Feb 25, 2022

comcalvi mentioned this pull request Dec 12, 2022

(cli): support hotswapping resources in Nested Stacks #16508

Closed

peterwoodworth mentioned this pull request Jan 4, 2023

(aws-tools): CLI --hotswap -> We don't support attributes of the 'AWS::CloudFormation::Stack' resource #23533

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

comcalvi commented Feb 12, 2022

gitpod-io bot commented Feb 12, 2022

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment •

edited

Loading

skinny85 left a comment

skinny85 Feb 25, 2022

comcalvi Feb 25, 2022

skinny85 Feb 25, 2022

skinny85 left a comment

mergify bot commented Feb 25, 2022

aws-cdk-automation commented Feb 25, 2022

mergify bot commented Feb 25, 2022

	constructor(rootStackName?: string, childStacks?: string[]) {
	constructor(rootStackName?: string, childStacks: string[] = []) {

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

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

Conversation

comcalvi commented Feb 12, 2022

gitpod-io bot commented Feb 12, 2022

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 • edited Loading

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 Feb 25, 2022

Choose a reason for hiding this comment

comcalvi Feb 25, 2022

Choose a reason for hiding this comment

skinny85 Feb 25, 2022

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 25, 2022

aws-cdk-automation commented Feb 25, 2022

AWS CodeBuild CI Report

mergify bot commented Feb 25, 2022

skinny85 left a comment •

edited

Loading