cloud-nuke
cloud-nuke copied to clipboard
Add ability to exclude more resources via the cloud-nuke-excuded tag
Currently, you can only exclude S3 buckets that are tagged with key cloud-nuke-excluded and value set to true. This PR adds more resources that are protected via this tag. This functionality is used in case you have a test environment that you do not want to get wiped out, like an integration setup with an EKS cluster, ELB, workers in an ASG etc. - you can propagate the tag across your setup for more fine-grained control (in comparison to specifying a creation time). The current setup is backwards compatible and has been tested with every resource.
Nice work @merkata! What is preventing this from getting merged in @brikis98 @yorinasub17
It would be great if this feature can be reviewed / merged 💚
Hi 👋 Any updates on this ?
@merkata this change set looks great to me, could you resolve the conflicts in the PR and add test cases for the new tagging feature?
@merkata this change set looks great to me, could you resolve the conflicts in the PR and add test cases for the new tagging feature?
Thanks a lot @ellisonc - I've fixed the conflicts and I'll be writing the tests to cover the new functionality. Appreciate your input!
@merkata this change set looks great to me, could you resolve the conflicts in the PR and add test cases for the new tagging feature?
Thanks a lot @ellisonc - I've fixed the conflicts and I'll be writing the tests to cover the new functionality. Appreciate your input!
@merkata This looks like a great change! Could you also please update the README to reflect this expanded functionality as well?
@merkata this change set looks great to me, could you resolve the conflicts in the PR and add test cases for the new tagging feature?
Thanks a lot @ellisonc - I've fixed the conflicts and I'll be writing the tests to cover the new functionality. Appreciate your input!
@merkata This looks like a great change! Could you also please update the README to reflect this expanded functionality as well?
Thanks @zackproser , I'll update the README as well!
@merkata apologies for the delay, I attempted to fix the conflicts and get this merged but our tooling doesn't allow pushes to any branch named master and I don't want to steal your credit by making the changes on a new branch of my own. Could you resolve the conflicts on a feature branch and update the PR?
@merkata apologies for the delay, I attempted to fix the conflicts and get this merged but our tooling doesn't allow pushes to any branch named master and I don't want to steal your credit by making the changes on a new branch of my own. Could you resolve the conflicts on a feature branch and update the PR?
Hey @ellisonc , thanks for the feedback! I fixed the conflicts, two things stand out:
-
the current
README.md
doesn't seem to cover exclusion of resources based on tags, the previous list of resources described which resources can be filtered out based on tag and I extended the descriptions of the resources I added. -
please have a look at
aws/snapshot.go
, I believe the logic is correct, still there is the addition of not deleting a snapshot with the AWS backup tag and I want to make sure I got the logic right.
Thanks!
@merkata I also don't see the readme update you mentioned, did that not get staged?
@merkata I also don't see the readme update you mentioned, did that not get staged?
I didn't explain it accurately - I updated the old version of the README.md, it looked like this:
Inspecting and deleting all ACM Private CA in an AWS account
Inspecting and deleting all Auto scaling groups in an AWS account - except for ASGs tagged with Key=cloud-nuke-excluded Value=true
Inspecting and deleting all Elastic Load Balancers (v1 and v2) in an AWS account - except for ELBs tagged with Key=cloud-nuke-excluded Value=true
Inspecting and deleting all Transit Gateways in an AWS account
Inspecting and deleting all EBS Volumes in an AWS account - except for volumes tagged with Key=cloud-nuke-excluded Value=true
...
and I specifically added the except for <resource name> tagged with... section for each resource.
Now the section looks like this:
Cloud-nuke suppports 🔎 inspecting and 🔥💀 deleting the following AWS resources:
| Resource Family | Resource type
| --------------- | ----------
| EC2 | Auto scaling groups |
| EC2 | Elastic Load Balancers (v1 and v2) |
| EC2 | EBS Volumes |
| EC2 | Unprotected EC2 instances |
...
So I'm not sure where to make a note about excluding a resource based on an exclusion tag.
@merkata I see what you're saying. How about adding a new section header under the Excluding Resources by Age
header (or maybe right above it?)
@merkata I see what you're saying. How about adding a new section header under the
Excluding Resources by Age
header (or maybe right above it?)
@zackproser sorry I wasn't able to response earlier, I've added a short paragraph with the list of resources that would be supported, let me know what you think of it. Thanks!
This is looking very close @merkata, thanks for the great work! Left a couple NITs (one typo that was present in a couple of places, and some thoughts on the use of aws.StringValue
and related methods for dereferencing pointers. Otherwise we should be good to get this merged shortly :+1:
Hey @merkata, we haven't heard back from you for quite some time. Do you plan to continue making progress on this PR? We've recently made some refactoring changes in the code and it may require a bit of conflict resolution.
Moved this change to this PR - https://github.com/gruntwork-io/cloud-nuke/pull/573.