Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cdk cli: Deploy of empty non-toplevel stack does not get deleted #20822

Closed
rabbittsoup opened this issue Jun 22, 2022 · 2 comments · Fixed by #21624
Closed

cdk cli: Deploy of empty non-toplevel stack does not get deleted #20822

rabbittsoup opened this issue Jun 22, 2022 · 2 comments · Fixed by #21624
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI

Comments

@rabbittsoup
Copy link
Contributor

rabbittsoup commented Jun 22, 2022

Describe the bug

After deploying a stack that uses a stage as it's scope (and not the App), so that it is not defined at the top level of the construct tree, a subsequent deploy of that same stack with all resources removed (including analytics_reporting=False) will recognize the stack as empty and existing, but does not delete it.

Expected Behavior

A previously existing stack deployed with no resources should delete the stack.

Current Behavior

The previously existing stack is not deleted.

Reproduction Steps

#app.py
from aws_cdk import App, Stage, Stack
app = App()
stage = Stage(app, 'Stage')
stack = Stack(stage, 'Stack')

cdk deploy '**'

#changed app.py
from aws_cdk import App, Stage, Stack
app = App()
stage = Stage(app, 'Stage')
stack = Stack(stage, 'Stack', analytics_reporting=False)

cdk deploy '**'

Possible Solution

I suspect the stack.stackName reference during the deploy of an existing empty stack should be stack.node.path instead.

selector: { patterns: [stack.stackName] },

>>> app = App()
>>> top_level_stack = Stack(app, 'TopLevelStack')
>>> stage = Stage(app, 'Stage')
>>> staged_level_stack = Stack(stage, 'StagedLevelStack')
>>> top_level_stack.stack_name; top_level_stack.node.path
'TopLevelStack'
'TopLevelStack'
>>> staged_level_stack.stack_name; staged_level_stack.node.path
'Stage-StagedLevelStack'
'Stage/StagedLevelStack'

Additional Information/Context

No response

CDK CLI Version

2.28.1 (build d035432)

Framework Version

2.28.1

Node.js Version

14.16.0

OS

AL2

Language

Python

Language Version

Python 3.8

Other information

No response

@rabbittsoup rabbittsoup added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 22, 2022
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jun 22, 2022
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 7, 2022
@rix0rrr rix0rrr removed their assignment Jul 7, 2022
arewa added a commit to arewa/aws-cdk that referenced this issue Aug 16, 2022
@arewa
Copy link
Contributor

arewa commented Aug 16, 2022

@rabbittsoup, thanks for pointing the possible suspect, I tested locally and prepared the PR, stack.stackName must be actually stack.hierarchicalId.

@mergify mergify bot closed this as completed in #21624 Aug 17, 2022
mergify bot pushed a commit that referenced this issue Aug 17, 2022
Fixed selector pattern for the empty stack which must be deleted during the deployment. I set stack selector pattern as 
`stack.hierarchicalId` because hierarchicalId is used in `selectMatchingStacks` method which is called when destroy logic is looking for the stack to be deleted.

https://github.com/aws/aws-cdk/blob/92d6d58029595735df6902db5f820b1182dfb27b/packages/aws-cdk/lib/api/cxapp/cloud-assembly.ts#L138

There is also existing integration test which covers destroy logic and it works now without additional modifications:

https://github.com/aws/aws-cdk/blob/92d6d58029595735df6902db5f820b1182dfb27b/packages/aws-cdk/test/integ/cli/cli.integtest.ts#L685

**How I tested it locally?**

- Prepared a package with fix with `yarn package` and installed with `npm install -g dist/js/aws-cdk-0.0.0.tgz`
- Reproduced steps from the bug #20822
- Ensured that the issue is fixed

closes #20822 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
Fixed selector pattern for the empty stack which must be deleted during the deployment. I set stack selector pattern as 
`stack.hierarchicalId` because hierarchicalId is used in `selectMatchingStacks` method which is called when destroy logic is looking for the stack to be deleted.

https://github.com/aws/aws-cdk/blob/92d6d58029595735df6902db5f820b1182dfb27b/packages/aws-cdk/lib/api/cxapp/cloud-assembly.ts#L138

There is also existing integration test which covers destroy logic and it works now without additional modifications:

https://github.com/aws/aws-cdk/blob/92d6d58029595735df6902db5f820b1182dfb27b/packages/aws-cdk/test/integ/cli/cli.integtest.ts#L685

**How I tested it locally?**

- Prepared a package with fix with `yarn package` and installed with `npm install -g dist/js/aws-cdk-0.0.0.tgz`
- Reproduced steps from the bug aws#20822
- Ensured that the issue is fixed

closes aws#20822 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants