feat(cli): add cdk orphan command to detach resources from a stack#1324
feat(cli): add cdk orphan command to detach resources from a stack#1324
cdk orphan command to detach resources from a stack#1324Conversation
|
This is actually @LeeroyHannigan's change, I'm just turning it into a PR so I can leave comments more conveniently. |
rix0rrr
left a comment
There was a problem hiding this comment.
Great start!
Initial round of comments on this.
This also needs an integ test (and to be frank probably more than 1 😉 ).
Head branch was pushed to by a user without write access
82e0371 to
8ad5411
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1324 +/- ##
==========================================
- Coverage 88.29% 88.14% -0.15%
==========================================
Files 73 74 +1
Lines 10386 10494 +108
Branches 1413 1428 +15
==========================================
+ Hits 9170 9250 +80
- Misses 1189 1216 +27
- Partials 27 28 +1
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:
|
| // Only include the primary resource for each construct path | ||
| // e.g. for path "MyTable", match "StackName/MyTable/Resource" exactly | ||
| const cdkPath = resource.Metadata?.[PATH_METADATA_KEY] ?? ''; | ||
| const primaryPaths = constructPaths.map(p => `${stack.stackName}/${p}/Resource`); | ||
| if (!primaryPaths.includes(cdkPath)) continue; |
There was a problem hiding this comment.
This smells to me. I can't be bothered to try to understand what this is doing exactly, but it doesn't feel right.
Why the interpolation with stackName? Why only exactly this L1 reosurce? Why includes(cdkPath) instead of startsWith(cdkPath)?
What are we trying to do here?
There was a problem hiding this comment.
Removed the path filter entirely. Will need to discuss.
1036138 to
211b87e
Compare
deea40c to
06a93bd
Compare
06a93bd to
616d59a
Compare
| # `cdk import` | ||
| - cloudformation:GetTemplateSummary | ||
| # `cdk orphan` | ||
| - cloudformation:ListStackResources |
There was a problem hiding this comment.
This needs an update to the bootstrap stack version as well (in 2 places)
There was a problem hiding this comment.
I changed my mind. Can we change this to:
- cloudformation:List*
- cloudformation:Describe*
I'm tired of having to add every new read-only permissions one by one :(
| # `cdk import` | ||
| - cloudformation:GetTemplateSummary | ||
| # `cdk orphan` | ||
| - cloudformation:ListStackResources |
There was a problem hiding this comment.
I changed my mind. Can we change this to:
- cloudformation:List*
- cloudformation:Describe*
I'm tired of having to add every new read-only permissions one by one :(
|
|
||
| // Step 1/3: Resolve GetAtt attribute values via temporary stack outputs | ||
| await this.ioHelper.defaults.info('Step 1/3: Resolving attribute values...'); | ||
| const resolvedValues = await this.resolveGetAttValues(stack, cfn, logicalIds, currentTemplate, physicalIds); |
There was a problem hiding this comment.
I just realized there's something missing.
A {Fn::Sub} has implicit interpolation inside its format string. It can look like this:
{
"Fn::Sub": "before ${ImplicitRef} ${ImplicitAtt.AttrName} after"
}
{
"Fn::Sub": [
"before ${ImplicitRef} ${ImplicitAtt.AttrName} or ${Explicit} after",
{
"Explicit": { "Fn::GetAtt": ["Resource", "Attr"] }
}
}For the final case we don't have to do anything (Resource.Attr) because (I assume) it would already be found. But the 2 strings can contain implicit references:
${ImplicitRef}inside format string - resolves the same as{ "Ref": "ImplicitRef" }would.${ImplicitAtt.AttrName}inside format string - resolves the same as{ "Fn::GetAtt": ["ImplicitAtt", "AttrName"] }would.
We need to discover these references in the discovery stage, and replace them in the replacement stage (be sure to add tests).
Note: the format string can occur in 2 forms, either inside an array or as a direct argument.
Be sure to add tests on these.
| ⚠️**CAUTION**⚠️: CDK Orphan is currently experimental and may have | ||
| breaking changes in the future. Make sure to use the `--unstable=orphan` flag | ||
| when using this command. |
There was a problem hiding this comment.
Let's make this sound less scary.
| ⚠️**CAUTION**⚠️: CDK Orphan is currently experimental and may have | |
| breaking changes in the future. Make sure to use the `--unstable=orphan` flag | |
| when using this command. | |
| > `cdk orphan` is currently experimental, meaning we reserve the right to change | |
| > options and flag names in the future. Pass the `--unstable=orphan` flag when using | |
| > this command and be aware of this when using it in scripts. |
Adds a new CLI command that safely removes resources from a CloudFormation
stack without deleting them, enabling resource type migrations (e.g.
DynamoDB Table to GlobalTable).
`cdk orphan --path <ConstructPath>` will:
- Find all resources under the construct path via aws:cdk:path metadata
- Resolve {Ref} values via DescribeStackResources
- Resolve {Fn::GetAtt} values by injecting temporary stack Outputs
- Set DeletionPolicy: Retain on all matched resources
- Remove the resources from the stack
- Output an inline `cdk import` command with the resource mapping
Also adds inline JSON support for `--resource-mapping` in `cdk import`,
and exposes `stackSdk()` on Deployments for read-only SDK access.
e16472f to
921758f
Compare
921758f to
e6313a9
Compare
e6313a9 to
a5397aa
Compare
a5397aa to
1909dd4
Compare
Adds a new CLI command that safely removes resources from a CloudFormation stack without deleting them, enabling resource type migrations (e.g. DynamoDB Table to GlobalTable).
cdk orphan --path <ConstructPath>will:cdk importcommand with the resource mappingAlso adds inline JSON support for
--resource-mappingincdk import, and exposesstackSdk()on Deployments for read-only SDK access.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license