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

Tags are not respected? #110

Closed
YuriGal opened this issue Mar 7, 2024 · 17 comments · Fixed by #111 or #114
Closed

Tags are not respected? #110

YuriGal opened this issue Mar 7, 2024 · 17 comments · Fixed by #111 or #114

Comments

@YuriGal
Copy link

YuriGal commented Mar 7, 2024

We tag our resources that we want to keep from nuking with key do-not-nuke and value true, then we add this to filter section - and for majority of resources it works. But for some resources it's ignored. For example we tagged a Sagemaker domain:

Screenshot 2024-03-07 at 4 29 54 PM

And using filter similar to this one:

accounts:
  '123456789':
    filters:
      SageMakerDomain:
      - property: "tag:do-not-nuke"
        value: "true"

Similar setup works for majority of other resources, yet here we're getting output

us-west-2 - SageMakerDomain - d-lkdfjldfadsf - [DomainID: "d-lkdfjldfadsf"] - would remove

is it a bug, or some omission on our side? How do we make it respect the tags?

@ekristen
Copy link
Owner

ekristen commented Mar 7, 2024

@YuriGal it looks like SageMakerDomain doesn't have tag support, should be trivial for me to add, I'll take a look tonight.

@YuriGal
Copy link
Author

YuriGal commented Mar 7, 2024

That would be awesome! Would it be possible to add this to other similar resources? I know some resources can't be tagged at all in AWS, but if all of those that can could be filtered by tags it would be great.

@ekristen
Copy link
Owner

ekristen commented Mar 7, 2024

Yes, as long as the resource supports tags, the resource in this project just needs to be identified as needing it's tags exposed and then filtering will work.

@YuriGal
Copy link
Author

YuriGal commented Mar 8, 2024

Not sure if I am doing something wrong, but with v3.0.0-beta.32 SageMakerDomain is always excluded, no matter tagged or not.

@ekristen
Copy link
Owner

ekristen commented Mar 8, 2024

What happens exactly? Does it show up as "filtered by config"? Can you please provide the output and the config?

@ekristen
Copy link
Owner

ekristen commented Mar 8, 2024

@YuriGal let me know when you get a chance. I'll see what I can do to replicate or test.

@YuriGal
Copy link
Author

YuriGal commented Mar 8, 2024

Ok, so to simplify testing I am using a basic config like this

regions:
  - global
  - us-west-2
  - us-east-1
  - eu-central-1
  - af-south-1
account-blocklist:
  - "123456789"
accounts:
  '987654321':
    filters:
      SageMakerDomain:
      - property: "tag:role:do-not-nuke"
        value: "true"
resource-types:
  targets:
    - SageMakerDomain

There is a sagemaker domain in us-west-2, but no matter whether it's tagged or not I am getting

Scan complete: 0 total, 0 nukeable, 0 filtered.

@ekristen
Copy link
Owner

ekristen commented Mar 8, 2024

@YuriGal I broke it when I added tests. Fix inbound.

@YuriGal
Copy link
Author

YuriGal commented Mar 8, 2024

Thanks! And thank you for such a fast response time.

@ekristen
Copy link
Owner

ekristen commented Mar 8, 2024

🎉 This issue has been resolved in version 3.0.0-beta.33 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@YuriGal
Copy link
Author

YuriGal commented Mar 8, 2024

Looks like it's works properly for SageMakerDomain, thanks!

Would it be possible to do the same for other resources that support tags, but tags aren't exposed?

@ekristen
Copy link
Owner

ekristen commented Mar 8, 2024

Ideally just need an issue with a list of Resources that are missing tags that support tags and I can look into it. It depends on the resource and the API as to how much effort it takes.

@YuriGal
Copy link
Author

YuriGal commented Mar 8, 2024

I think I can provide the list, just have to collect it.

Meanwhile I am afraid I found another bug, will open a separate issue.

@YuriGal
Copy link
Author

YuriGal commented Mar 11, 2024

Ok, so teams provided me with following list of resources that were tagged, but got nuked anyway

SageMakerUserProfiles
ECSCluster
ServiceDiscoveryNamespace
SQSQueue
CognitoUserPoolDomain
CognitoUserPoolClient
CognitoUserPool
CognitoIdentityProvider
ElasticacheSubnetGroup
ElasticacheCacheCluster

I know for sure that this is true for ECSCluster and SQSQueue, but didn't get a chance to verify other ones.

@YuriGal
Copy link
Author

YuriGal commented Mar 13, 2024

btw, (unrelated) question: does this nuke supports --max-wait-retries option?

@ekristen
Copy link
Owner

It's suppose to, but looking at the options, it appears to be missing. The goal is to be 99% backwards compatible with the original, minus the nuke subcommand.

I'll fix this oversight, mind opening an issue? You'll get notified that way when it get's fixed.

@ekristen
Copy link
Owner

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants