ekristen / aws-nuke

Remove all the resources from an AWS account
https://ekristen.github.io/aws-nuke/
MIT License
36 stars 7 forks source link

Resources without tags not filtered #212

Open iuliancristea opened 2 weeks ago

iuliancristea commented 2 weeks ago

Version: 3.3.2

When resource filtering is in place via tags, some resources aren't filtered out. For this preset:

presets:
  common:
    filters:
      global:
        - property: "tag:delete/.*"
          type: "regex"
          value: "true"
          invert: "true"

the following occur:

global - Route53ResourceRecordSet

global - Route53ResourceRecordSet - <NS_RECORD>. - [Name: "<NS_RECORD>.", Type: "NS"] - cannot delete NS record <<<<<----- EXPECTED NOT TO SHOW
global - Route53ResourceRecordSet - <SOA_RECORD>. - [Name: "<SOA_RECORD>.", Type: "SOA"] - cannot delete SOA record <<<<<----- EXPECTED NOT TO SHOW
global - Route53ResourceRecordSet - <A_RECORD>. - [Name: "<A_RECORD>.", Type: "A"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING TAGS
global - Route53ResourceRecordSet - <CNAME_RECORD>. - [Name: "<CNAME_RECORD>.", Type: "TXT"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING TAGS

global - IAMPolicy

global - IAMPolicy - arn:aws:iam::<aws_account>:policy/<custom_policy> - [ARN: "arn:aws:iam::<aws_account>:policy/<custom_policy>", Name: "<custom_policy>", Path: "/", PolicyID: "<policy_id>"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING TAGS

global - IAMRole

# most omitted for clarity, but this list includes SSO roles, etc. too

global - IAMRole - aws-controltower-AdministratorExecutionRole - [] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING TAGS
global - IAMRole - aws-controltower-ConfigRecorderRole - [] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING TAGS

global - IAMRolePolicy

global - IAMRolePolicy - <iam_role_policy>-ap-northeast-1 -> <iam_role_policy> - [PolicyName: "<iam_role_policy>", role:Path: "/", role:RoleID: "<role_id>", role:RoleName: "<iam_role_policy>-ap-northeast-1", tag:role:Availability: "3", tag:role:Environment: "production"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

global - IAMRolePolicyAttachment

global - IAMRolePolicyAttachment - AWSServiceRoleForIPAM -> AWSIPAMServiceRolePolicy - [PolicyArn: "arn:aws:iam::aws:policy/aws-service-role/AWSIPAMServiceRolePolicy", PolicyName: "AWSIPAMServiceRolePolicy", RoleCreateDate: "2024-02-15T15:23:09Z", RoleLastUsed: "2024-07-08T21:13:51Z", RoleName: "AWSServiceRoleForIPAM", RolePath: "/aws-service-role/ipam.amazonaws.com/"] - cannot detach from service roles <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
global - IAMRolePolicyAttachment - AWSServiceRoleForMarketplaceLicenseManagement -> AWSMarketplaceLicenseManagementServiceRolePolicy - [PolicyArn: "arn:aws:iam::aws:policy/aws-service-role/AWSMarketplaceLicenseManagementServiceRolePolicy", PolicyName: "AWSMarketplaceLicenseManagementServiceRolePolicy", RoleCreateDate: "2024-02-15T15:23:05Z", RoleLastUsed: "2024-02-15T15:23:05Z", RoleName: "AWSServiceRoleForMarketplaceLicenseManagement", RolePath: "/aws-service-role/license-management.marketplace.amazonaws.com/"] - cannot detach from service roles <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - KMSAlias

eu-central-1 - KMSAlias - alias/aws/acm - [Name: "alias/aws/acm"] - cannot delete AWS alias <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
eu-central-1 - KMSAlias - alias/aws/dynamodb - [Name: "alias/aws/dynamodb"] - cannot delete AWS alias <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - ElasticacheUserGroup

eu-central-1 - ElasticacheUserGroup - aasjpfhi - [ID: "aasjpfhi"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
eu-central-1 - ElasticacheUserGroup - asdwfasf - [ID: "asdwfasf"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - ElasticacheReplicationGroup

eu-central-1 - ElasticacheReplicationGroup - asjfhqei-gloo-redis - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
eu-central-1 - ElasticacheReplicationGroup - oqzrwbie-gloo-redis - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - ElasticacheCacheParameterGroup

eu-central-1 - ElasticacheCacheParameterGroup - default.memcached1.4 - [GroupFamily: "memcached1.4", GroupName: "default.memcached1.4"] - cannot delete default cache parameter group <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
eu-central-1 - ElasticacheCacheParameterGroup - default.memcached1.5 - [GroupFamily: "memcached1.5", GroupName: "default.memcached1.5"] - cannot delete default cache parameter group <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - ElasticacheUser

eu-central-1 - ElasticacheUser - aasjpfhi-default-custom - [ID: "aasjpfhi-default-custom", UserName: "default"] - cannot delete default user <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - S3Bucket

eu-central-1 - S3Bucket - s3://<bucket_name>-<aws_account>-eu-central-1 - [CreationDate: "2024-03-15T15:51:00Z", Name: "<bucket_name>-<aws_account>-eu-central-1", tag:Availability: "3", tag:Environment: "production"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
eu-central-1 - S3Bucket - s3://<bucket_name>-eu-central-1 - [CreationDate: "2024-04-10T16:29:48Z", Name: "<bucket_name>-eu-central-1"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - CloudWatchAlarm

eu-central-1 - CloudWatchAlarm - ProvisionerFaiureAlarm - [Name: "ProvisionerFaiureAlarm"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG
eu-central-1 - CloudWatchAlarm - <alarm>-ConfigChanges - [Name: "<alarm>-ConfigChanges", tag:exists: "true"] - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

regional - LifecycleHook

eu-central-1 - LifecycleHook - GracefulShutdown - would remove <<<<<----- EXPECTED NOT TO BE REMOVED DUE TO NOT HAVING EXPECTED TAG

Some errors which come from resources that wouldn't be deleted anyway are fine. The true problem lies where there's resources that are expected to be filtered but aren't because they do not have the right tag or do not have tags at all.

Should the default behaviour be "ignore" if a resource cannot be filtered?

ekristen commented 2 weeks ago
presets:
  common:
    filters:
      global:
        - property: "tag:delete/.*"
          type: "regex"
          value: "true"
          invert: "true"

Unfortunately this is not a valid filter configuration. Filters are based on ResourceType HOWEVER there is a __global__ resource type you can use with filters. Introduced by my rewrite using libnuke library.

Check out the filter documentation it shows how to use the __global__.

I think what you are trying to do would work as you hope, but I don't honestly know. The invert filter is very wonky. I don't really like it and have been thinking of a filter rewrite for v4, but I'm not sure yet.

iuliancristea commented 2 weeks ago

Ah! You're right... I've been messing with the config so much I couldn't see the forest from the trees.

The right syntax was __global__ not global 😮‍💨 .

However, it seems that while most of the resources get filtered out properly, there are still some that aren't filtered out because they "don't support custom properties":

WARN[0055] unable to get property: tag:kubernetes.io/cluster/.*  component=libnuke error="*resources.LifecycleHook does not support custom properties" handler=Filter item=GracefulShutdown type=LifecycleHook
eu-central-1 - LifecycleHook - GracefulShutdown - would remove
WARN[0055] unable to get property: tag:kubernetes.io/cluster/.*  component=libnuke error="*resources.CloudWatchEventsBus does not support custom properties" handler=Filter item=<event_bus> type=CloudWatchEventsBuses
eu-central-1 - CloudWatchEventsBuses - <event_bus> - would remove

Is this intended or should the default behaviour be that "what cannot be filtered shouldn't be removed as long as filtering is expected"?

ekristen commented 2 weeks ago

Well I would say you are definitely in a unique camp here. I don't think I've seen many if any take the approach of negating the filter to look for tag:delete/.* and then filter anything that doesn't have it out.

The original tool design was delete everything, filter what you want to keep instead of tag what you want to delete and keep everything else, so your config is a bit of an anti-pattern to a degree.

With respect to the LifecycleHook and the CloudWatchEventsBuses we could easily add the properties to those resources to avoid the error.

iuliancristea commented 2 weeks ago

I guess you're right that what I'm doing can be seen as an anti-pattern. The tag:delete/.* example is a bit untrue and was just there to exemplify. The way I'm currently using this is to filter the deletion of resources that have the tag:kubernetes.io/cluster/.* tag - this is useful for preview environments, etc.

Do you think it would be worth at least as a future feature to have a flag that just filters out what cannot be filtered? If not, we can close this. If yes, I'll create a new issue to keep the conversation going.

ekristen commented 1 week ago

I've given this some thought and the current way the library is written and the tooling is written I don't see a safe and logical way to "determine" if something can be or cannot be filtered. A lot of additional logic would have to be added to determine if filters were or were not successfully applied along with their state, and this likely posses to big of a risk of breaking something bad for someone.

Perhaps with more test coverage and/or a major update to libnuke something like this could be supported, but at the moment it's out of scope for me. Right now I'm trying to make updates faster, add reliability in where I can, and get updates churning out as needed and make preparations for AWS deprecating v1 of their library in 1 year from now.

danalstadt commented 1 week ago

We are using aws-nuke in a similar way - we have tagged resources that are part of preview environments, and are using aws-nuke as part of the cleanup with an inverted __global__ filter:

  1234567890:
    filters:
      __global__:
      - property: "tag:foo"
        value: bar
        invert: true

My current solution is to use excludes for AWS services that we use that don't support tagging, and to handle deletion of those resource types separately. I'm not familiar with the codebase yet, but to me it'd make sense for resources that throw the "unable to get property" error to be treated as not having the property.

Anyway, thanks for the for @ekristen - despite this issue, the global filtering has been quite useful.

ekristen commented 5 days ago

I've been thinking about this a lot, I suppose we can detect via libnuke if a property doesn't exist and automatically exclude it. That would definitely need to be a feature on libnuke and a feature-flag on aws-nuke set via config or run-time as it changes the behavior of the tool.

ekristen commented 5 days ago

@danalstadt glad to hear that global filtering has been useful!

gthomson31 commented 3 days ago

Great feature saved so many lines of code from our old config!