cloud-nuke icon indicating copy to clipboard operation
cloud-nuke copied to clipboard

Add ability to exclude more resources via the cloud-nuke-excuded tag

Open merkata opened this issue 3 years ago • 1 comments

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.

merkata avatar Sep 28 '21 19:09 merkata

Nice work @merkata! What is preventing this from getting merged in @brikis98 @yorinasub17

harmrai-pol avatar Aug 04 '22 15:08 harmrai-pol

It would be great if this feature can be reviewed / merged 💚

gmembre-zenika avatar Oct 26 '22 08:10 gmembre-zenika

Bump.

This PR is related to this issue 166

tommy31 avatar Nov 24 '22 13:11 tommy31

Hi 👋 Any updates on this ?

gforien avatar Jan 18 '23 10:01 gforien

@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?

ellisonc avatar Jan 18 '23 15:01 ellisonc

@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 avatar Jan 22 '23 12:01 merkata

@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?

zackproser avatar Jan 31 '23 16:01 zackproser

@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 avatar Feb 03 '23 07:02 merkata

@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?

ellisonc avatar Apr 06 '23 14:04 ellisonc

@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 avatar Apr 12 '23 11:04 merkata

@merkata I also don't see the readme update you mentioned, did that not get staged?

ellisonc avatar Apr 12 '23 18:04 ellisonc

@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 avatar Apr 13 '23 11:04 merkata

@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 avatar Apr 24 '23 18:04 zackproser

@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!

merkata avatar May 17 '23 12:05 merkata

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:

zackproser avatar May 30 '23 13:05 zackproser

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.

james03160927 avatar Aug 22 '23 23:08 james03160927

Moved this change to this PR - https://github.com/gruntwork-io/cloud-nuke/pull/573.

james03160927 avatar Aug 29 '23 22:08 james03160927