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

fix(cli): empty non top-level stack does not get deleted #21624

Merged
merged 8 commits into from
Aug 17, 2022

Conversation

arewa
Copy link
Contributor

@arewa arewa commented Aug 16, 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.

if (minimatch(stack.hierarchicalId, pattern)) {

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

integTest('deploy stack without resource', withDefaultFixture(async (fixture) => {

How I tested it locally?

closes #20822


All Submissions:

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

@gitpod-io
Copy link

gitpod-io bot commented Aug 16, 2022

@github-actions github-actions bot added the p2 label Aug 16, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team August 16, 2022 15:04
Copy link
Contributor

@TheRealAmazonKendra TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure that your PR title confirms to the conventional commit standard (fix, feat, chore) and that it is written in a style that will reflect correctly in the change log (See Contributing Guide, Pull Requests). Your PR body should describe the problem the PR is solving, and the design approach and alternatives considered. Explain why the PR solves the problem. A link to an issue is helpful, but does not replace an explanation of your thought process.

Additionally, we cannot provide a meaningful review for PRs without any testing. Please add tests for this change. If this is not a change that automated tests can be written for, please explain how this change has been manually tested.

@arewa arewa changed the title fix(cdk cli): Delete empty stack on deploy #20822 wip: fix(cli): fixes #20822 Aug 16, 2022
@arewa arewa changed the title wip: fix(cli): fixes #20822 WIP: fix(cli): fixes #20822 Aug 16, 2022
@arewa arewa changed the title WIP: fix(cli): fixes #20822 fix(cli): fixes #20822 Aug 16, 2022
@arewa arewa marked this pull request as draft August 16, 2022 16:52
@mergify mergify bot dismissed TheRealAmazonKendra’s stale review August 17, 2022 09:39

Pull request has been modified.

@arewa arewa marked this pull request as ready for review August 17, 2022 12:15
@TheRealAmazonKendra TheRealAmazonKendra changed the title fix(cli): fixes #20822 fix(cli): empty non top-level stack does not get deleted Aug 17, 2022
@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 and removed p2 labels Aug 17, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit a6757b0 into aws:main Aug 17, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: b5eae3b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

josephedward pushed a commit to josephedward/aws-cdk that referenced this pull request 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cdk cli: Deploy of empty non-toplevel stack does not get deleted
3 participants