fix(core): unable to reference resources across multiple nested stacks #7187

eladb · 2020-04-06T09:26:08Z

Commit Message

fix(core): unable to reference resources across multiple nested stacks (#7187)

Fixes #6473 by centralizing the logic to resolve cross-references and prepare nested stack template assets in App.prepare, which has a global view of the app and is the last prepare to execute before synthesis. This dramatically simplified reference resolution and allows dealing with nested stack assets only after all cross references have been resolved (the root cause for #7059).

This logic is implemented in a function called prepareApp which is normally called from App.prepare() but if a Stack is created as a root (normally in unit tests, we invoke this logic from there to retain current behavior).

This algorithm takes care of both resolving cross references and add template assets for nested stacks. This is because assets are currently addressed using CFN parameters, which means that when we adding them to the parent of a nested stack, the parent is mutated, so we need to rectify references again. To make sure this is done correctly, we always create assets in DFS order.

All changes to the test snapshots stem from new asset IDs of nested stack templates, not the template themselves. The change is a result of the fact that the refactor caused the "Parameters" section to appear in a different place in the template, but the template itself is identical.

Fixes #7059 by first resolving all references in the app and only then calculating the hash of the nested stack templates for their assets.

Fixes #5888 but this was not verified.

End Commit Message

TODO:

Validate: Nested stacks template will not be republished when a reference is added/updated #7059
Verify DDB Global is not a breaking change

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

aws-cdk-automation · 2020-04-06T09:31:00Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: cc6b168
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T09:54:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 344ede2
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T09:54:35Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 67cdc9b
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T11:47:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3353aca
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T11:56:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9e673ed
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T12:44:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e241660
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T13:02:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9957251
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T13:59:59Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 814a15b
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T15:03:17Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e792923
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T15:09:39Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: ae5a697
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T15:13:29Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: f11a29a
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-06T16:32:36Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 7b2f8be
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-07T07:02:23Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: da75010
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-07T08:45:39Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: a5bb590
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-09T12:47:16Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: b57dbab
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T14:59:59Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3b5b4e7
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T16:14:27Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 7903384
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T17:09:45Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9a4197c
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T17:10:51Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: efa0124
Result: FAILED
Build Logs (available for 30 days)

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

#7187) Fixes #6473 by centralizing all the logic to resolve cross-references into `App.prepare()`. This allows reasoning about the entire application once from a single code flow, dramatically simplified the algorithm and solved the issue at hand (and probably a handful of other issues). This logic is implemented in a function called `prepareApp` which is normally called from `App.prepare()` but if a `Stack` is created as a root (normally in unit tests, we invoke this logic from there to retain current behavior). This algorithm takes care of both resolving cross references and add template assets for nested stacks. This is because assets are currently addressed using CFN parameters, which means that when we adding them to the parent of a nested stack, the parent is mutated, so we need to rectify references again. To make sure this is done correctly, we always create assets in DFS order. Fixes #7059 Fixes #5888

aws-cdk-automation · 2020-04-10T17:45:52Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9f54f66
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T18:23:41Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 182838c
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T18:46:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9cc5d4d
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T18:50:20Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8074d34
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-10T19:12:51Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8f840fb
Result: FAILED
Build Logs (available for 30 days)

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

eladb · 2020-04-14T11:34:22Z

@rix0rrr let me know if you have additional notes here

aws-cdk-automation · 2020-04-14T12:06:02Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 60e214a
Result: SUCCEEDED
Build Logs (available for 30 days)

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

NetaNir · 2020-04-15T05:03:26Z

@eladb I added one comment which is more a style issue, other than that, looks great.

aws-cdk-automation · 2020-04-15T07:42:23Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3cf1206
Result: SUCCEEDED
Build Logs (available for 30 days)

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

After a reference is resolved (e.g. by adding a nested stack parameter or output), we loop back and look up all references again to make sure we resolve any new references that might be added. Loop stops when all references have values assigned to them. Also, simplify prepare-app to work on nested stacks as a queue.

aws-cdk-automation · 2020-04-15T13:39:59Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e306a5e
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-15T14:27:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 1d02733
Result: SUCCEEDED
Build Logs (available for 30 days)

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

eladb · 2020-04-15T15:13:16Z

@NetaNir made some updates following your comments. What do you think?

aws-cdk-automation · 2020-04-15T15:43:46Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 5f043cc
Result: SUCCEEDED
Build Logs (available for 30 days)

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

NetaNir · 2020-04-16T01:10:09Z

@eladb the changes to the main loop looks great.

Could the loop in resolveReferences be replaced with a recursive call in resolveValue? Similarly to the first version of this PR?

// complier does not like consumer.parentStack type, hacking it for the code sample
if (consumer.parentStack && isNested(consumer, producer)) {
    const param =  createNestedStackParameter(consumer, reference);
    resolveValue(consumer.parentStack, reference);
    return param;
 }

Since we know that createNestedStackParameter only creates unresolved references on the parent stack we can resolve them immediately.

eladb · 2020-04-16T04:19:44Z

Could the loop in resolveReferences be replaced with a recursive call in resolveValue? Similarly to the first version of this PR?

@NetaNir what’s the reason you think this would be better? Do you see an issue with the current version?

aws-cdk-automation · 2020-04-16T08:53:45Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8f55171
Result: SUCCEEDED
Build Logs (available for 30 days)

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

eladb · 2020-04-16T10:29:37Z

Okay @NetaNir last iteration. I think you’ll like it.

aws-cdk-automation · 2020-04-16T10:38:29Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8fb40cc
Result: SUCCEEDED
Build Logs (available for 30 days)

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

NetaNir

Magic

mergify · 2020-04-16T16:40:38Z

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

mergify · 2020-04-16T16:40:40Z

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

As a follow up to #7187, extract the rest of the code from `Stack.prepare`: the logic that converted construct-level dependencies to resource-level dependencies was moved to `App.prepare` and the logic that added top-level stack tags to the cloud assembly metadata was moved to `Stack.synthesize`.

mergify bot added the contribution/core This is a PR that came from AWS. label Apr 6, 2020

SomayaB requested review from rix0rrr and removed request for rix0rrr April 6, 2020 21:36

SomayaB assigned eladb Apr 6, 2020

eladb force-pushed the benisrae/nested-fix-multi-level-refs branch from efa0124 to 9f54f66 Compare April 10, 2020 17:37

rix0rrr approved these changes Apr 14, 2020

View reviewed changes

Merge branch 'master' into benisrae/nested-fix-multi-level-refs

3cf1206

update snapshot

1d02733

Merge branch 'master' into benisrae/nested-fix-multi-level-refs

5f043cc

Merge branch 'master' into benisrae/nested-fix-multi-level-refs

8f55171

replace iteration with recursion

8fb40cc

NetaNir approved these changes Apr 16, 2020

View reviewed changes

eladb removed the pr/do-not-merge This PR should not be merged at this time. label Apr 16, 2020

mergify bot merged commit 000f0c2 into master Apr 16, 2020

mergify bot deleted the benisrae/nested-fix-multi-level-refs branch April 16, 2020 16:40

jogold added a commit to jogold/aws-cdk that referenced this pull request Apr 16, 2020

update integ test after aws#7187 and aws#7361

c132769

eladb mentioned this pull request Apr 28, 2020

refactor(core): move dependency logic to app and stack tags to synth #7638

Merged

fix(core): unable to reference resources across multiple nested stacks #7187

fix(core): unable to reference resources across multiple nested stacks #7187

Conversation

eladb commented Apr 6, 2020 • edited Loading

Commit Message

End Commit Message

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 7, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 9, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 10, 2020

AWS CodeBuild CI Report

eladb commented Apr 14, 2020

aws-cdk-automation commented Apr 14, 2020

AWS CodeBuild CI Report

NetaNir commented Apr 15, 2020

aws-cdk-automation commented Apr 15, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 15, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 15, 2020

AWS CodeBuild CI Report

eladb commented Apr 15, 2020

aws-cdk-automation commented Apr 15, 2020

AWS CodeBuild CI Report

NetaNir commented Apr 16, 2020

eladb commented Apr 16, 2020

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

eladb commented Apr 16, 2020

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

NetaNir left a comment

Choose a reason for hiding this comment

mergify bot commented Apr 16, 2020

mergify bot commented Apr 16, 2020

eladb commented Apr 6, 2020 •

edited

Loading