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

Filter main route tables #953

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

leighpascoe
Copy link
Contributor

@leighpascoe leighpascoe commented Mar 3, 2023

Description

AWS does not allow you to delete main route tables, see AWS docs.
Aws-nuke wants to delete a VPC, it expects the main route table to be deleted first, the main route table cannot be deleted, therefore the VPC fails to delete due to a dependency error.

Proposed Solution
After Describing the route tables, check for any Associations where Main is true. There may be zero to many Associations. If at least one main Association is found on the Toute Table, then the Route Table will will not be added to the returned collection of Route Tables

I have tested this on VPC with just a main route table, as well as VPC's with multiple route tables.

Related Issues: #946 #785

@leighpascoe leighpascoe requested a review from a team as a code owner March 3, 2023 19:46
@leighpascoe
Copy link
Contributor Author

Any idea when this PR will get looked at?

@der-eismann
Copy link
Member

Hey @leighpascoe, sorry for the late reply. We already have a dedicated Filter() function for these things, see this for example. Would be great if you could use this instead of your solution.

@leighpascoe
Copy link
Contributor Author

great. thank you. I'll make that change

@leighpascoe
Copy link
Contributor Author

@der-eismann I've made the update, and tested it. Ready for re-review

@leighpascoe
Copy link
Contributor Author

@svenwltr and @der-eismann Could I please get this re-reviewed. We are looking forward to using the released container instead of building our own forked image

@leighpascoe
Copy link
Contributor Author

@svenwltr, Am I missing anything in this PR?

@leighpascoe
Copy link
Contributor Author

@der-eismann We have been using aws-nuke on this custom branch for the last 4 months to delete VPC, it's well tested. Could we please get this reviewed and merged?

@bjoernhaeuser bjoernhaeuser enabled auto-merge (squash) August 15, 2023 11:17
@bjoernhaeuser bjoernhaeuser merged commit df499d5 into rebuy-de:main Aug 15, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants