fix(toolkit-lib): stack definitions for refactor are not considering resource refrences #547
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rences The `createStackRefactor` method, from the CloudFormation API, requires the client to pass not only the mappings, but also the templates in the state they are supposed to be after the refactor. Previously, this was done by iterating over the mappings (either computed by the toolkit or prescribed by the user), and using them as a list of instructions on how to modify the deployed stacks, moving resources around as necessary. This is a simple way to put the resources in their correct locations. But it becomes unwieldy when it comes to updating references between resources. Every time a resource is moved, all references to it -- and sometimes from it -- have to be updated as well. This PR changes implements a different algorithm: take all the synthesized templates as a starting point, and adjust them so that the final templates are equal to the deployed ones up to logical IDs and references. The result is what will be sent to CloudFormation. Specifically this involves: - Removing resources that exist locally but not remotely. - Adding resources that exist remotely but not locally. - Update the CDK construct path to the deployed value. (we will drop this once CloudFormation starts allowing changes in the construct path along with refactors). One case that is not yet covered is when physical IDs are set. Because we use the physical ID plus the type of the resource to identify it, users are free to change other properties, and we can still recognize that it's the same resource. Since we are simply taking the local resource as the basis and not updating anything other than the construct path, it's possible that it differs from the deployed one. This case will be handled in a following PR.
otaviomacedo
temporarily deployed
to
integ-approval
GitHub Actions
Inactive
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #547 +/- ##
==========================================
+ Coverage 79.09% 79.21% +0.12%
==========================================
Files 47 47
Lines 7074 7074
Branches 790 796 +6
==========================================
+ Hits 5595 5604 +9
+ Misses 1460 1451 -9
Partials 19 19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mrgrain
changed the title
mrgrain
approved these changes
May 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
createStackRefactormethod, from the CloudFormation API, requires the client to pass not only the mappings, but also the templates in the state they are supposed to be after the refactor.Previously, this was done by iterating over the mappings (either computed by the toolkit or prescribed by the user), and using them as a list of instructions on how to modify the deployed stacks, moving resources around as necessary. This is a simple way to put the resources in their correct locations. But it becomes unwieldy when it comes to updating references between resources. Every time a resource is moved, all references to it -- and sometimes from it -- have to be updated as well.
This PR changes implements a different algorithm: take all the synthesized templates as a starting point, and adjust them so that the final templates are equal to the deployed ones up to logical IDs and references. The result is what will be sent to CloudFormation. Specifically this involves:
One case that is not yet covered is when physical IDs are set. Because we use the physical ID plus the type of the resource to identify it, users are free to change other properties, and we can still recognize that it's the same resource. Since we are simply taking the local resource as the basis and not updating anything other than the construct path, it's possible that it differs from the deployed one. This case will be handled in a following PR.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license