From 1abf414ac9bb584bd2288b1f9a7e3d5a1d302958 Mon Sep 17 00:00:00 2001 From: James Kwon <96548424+hongil0316@users.noreply.github.com> Date: Thu, 8 Aug 2024 18:02:04 -0400 Subject: [PATCH] fix: no name tag resources are listing for nuke with include filter --- README.md | 2 -- aws/resource_registry.go | 1 - aws/resources/kms_customer_key.go | 2 +- aws/resources/sns.go | 2 +- aws/resources/transit_gateway.go | 18 ++---------------- aws/resources/transit_gateway_test.go | 11 ----------- config/config.go | 15 ++++++++++----- config/config_test.go | 17 ++++++++--------- 8 files changed, 22 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index ba50318d..fee6f8dc 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,6 @@ Cloud-nuke suppports 🔎 inspecting and 🔥💀 deleting the following AWS res | EC2 | Endpoint | | EC2 | Security Group | | EC2 | Network Interface | -| EC2 | Placement Group | | Certificate Manager | ACM Private CA | | Direct Connect | Transit Gateways | | Elasticache | Clusters | @@ -578,7 +577,6 @@ of the file that are supported are listed here. | ec2-ipam-pool | EC2IPAMPool | ✅ (IPAM Pool name) | ✅ (Creation Time) | ✅ | ✅ | | ec2-ipam-resource-discovery | EC2IPAMResourceDiscovery | ✅ (IPAM Discovery Name) | ✅ (Creation Time) | ✅ | ✅ | | ec2-ipam-scope | EC2IPAMScope | ✅ (IPAM Scope Name) | ✅ (Creation Time) | ✅ | ✅ | -| ec2-placement-groups | EC2PlacementGroups | ✅ (Placement Group Name) | ✅ (First Seen Tag Time) | ✅ | ✅ | | ec2-subnet | EC2Subnet | ✅ (Subnet Name) | ✅ (Creation Time) | ✅ | ❌ | | ec2-endpoint | EC2Endpoint | ✅ (Endpoint Name) | ✅ (Creation Time) | ✅ | ✅ | | ecr | ECRRepository | ✅ (Repository Name) | ✅ (Creation Time) | ❌ | ✅ | diff --git a/aws/resource_registry.go b/aws/resource_registry.go index de4475ff..fe1c4f48 100644 --- a/aws/resource_registry.go +++ b/aws/resource_registry.go @@ -75,7 +75,6 @@ func getRegisteredRegionalResources() []AwsResource { &resources.EC2Instances{}, &resources.EC2DedicatedHosts{}, &resources.EC2KeyPairs{}, - &resources.EC2PlacementGroups{}, &resources.TransitGateways{}, &resources.TransitGatewaysRouteTables{}, // Note: nuking transitgateway vpc attachement before nuking the vpc since vpc could be associated with it. diff --git a/aws/resources/kms_customer_key.go b/aws/resources/kms_customer_key.go index 2288815d..22fc1fc4 100644 --- a/aws/resources/kms_customer_key.go +++ b/aws/resources/kms_customer_key.go @@ -106,7 +106,7 @@ func (kck *KmsCustomerKeys) shouldInclude( includedByName := false // verify if key aliases matches configurations for _, alias := range aliases { - if config.ShouldInclude(alias, configObj.KMSCustomerKeys.IncludeRule.NamesRegExp, + if config.ShouldInclude(&alias, configObj.KMSCustomerKeys.IncludeRule.NamesRegExp, configObj.KMSCustomerKeys.ExcludeRule.NamesRegExp) { includedByName = true } diff --git a/aws/resources/sns.go b/aws/resources/sns.go index e4f704cb..38a32024 100644 --- a/aws/resources/sns.go +++ b/aws/resources/sns.go @@ -127,7 +127,7 @@ func shouldIncludeSNS(topicArn string, excludeAfter, firstSeenTime time.Time, co return false } - return config.ShouldInclude(topicName, configObj.SNS.IncludeRule.NamesRegExp, configObj.SNS.ExcludeRule.NamesRegExp) + return config.ShouldInclude(&topicName, configObj.SNS.IncludeRule.NamesRegExp, configObj.SNS.ExcludeRule.NamesRegExp) } func (s *SNSTopic) nukeAll(identifiers []*string) error { diff --git a/aws/resources/transit_gateway.go b/aws/resources/transit_gateway.go index 2d6798de..addff8d2 100644 --- a/aws/resources/transit_gateway.go +++ b/aws/resources/transit_gateway.go @@ -17,8 +17,6 @@ import ( const ( TransitGatewayAttachmentTypePeering = "peering" - TransitGatewayAttachmentTypeVPC = "vpc" - TransitGatewayAttachmentTypeConnect = "connect" ) // [Note 1] : NOTE on the Apporach used:-Using the `dry run` approach on verifying the nuking permission in case of a scoped IAM role. @@ -121,22 +119,9 @@ func (tgw *TransitGateways) nukeAttachments(id *string) error { switch attachmentType { case TransitGatewayAttachmentTypePeering: logging.Debugf("[Execution] deleting the attachments of type %v for %v ", attachmentType, awsgo.StringValue(id)) - // Delete the Transit Gateway Peering Attachment _, err = tgw.Client.DeleteTransitGatewayPeeringAttachmentWithContext(tgw.Context, &ec2.DeleteTransitGatewayPeeringAttachmentInput{ TransitGatewayAttachmentId: attachments.TransitGatewayAttachmentId, }) - case TransitGatewayAttachmentTypeVPC: - logging.Debugf("[Execution] deleting the attachments of type %v for %v ", attachmentType, awsgo.StringValue(id)) - // Delete the Transit Gateway VPC Attachment - _, err = tgw.Client.DeleteTransitGatewayVpcAttachmentWithContext(tgw.Context, &ec2.DeleteTransitGatewayVpcAttachmentInput{ - TransitGatewayAttachmentId: attachments.TransitGatewayAttachmentId, - }) - case TransitGatewayAttachmentTypeConnect: - logging.Debugf("[Execution] deleting the attachments of type %v for %v ", attachmentType, awsgo.StringValue(id)) - // Delete the Transit Gateway Connect Attachment - _, err = tgw.Client.DeleteTransitGatewayConnectWithContext(tgw.Context, &ec2.DeleteTransitGatewayConnectInput{ - TransitGatewayAttachmentId: attachments.TransitGatewayAttachmentId, - }) default: err = fmt.Errorf("%v typed transit gateway attachment nuking not handled", attachmentType) } @@ -144,8 +129,9 @@ func (tgw *TransitGateways) nukeAttachments(id *string) error { logging.Errorf("[Failed] unable to delete the transit gateway peernig attachment for %v : %s", awsgo.StringValue(id), err) return err } + if err := tgw.WaitUntilTransitGatewayAttachmentDeleted(id, attachmentType); err != nil { - logging.Errorf("[Failed] unable to wait until nuking the transit gateway attachment with type %v for %v : %s", attachmentType,awsgo.StringValue(id), err) + logging.Errorf("[Failed] unable to wait until nuking the transit gateway attachment with type %v for %v : %s", attachmentType, awsgo.StringValue(id), err) return errors.WithStackTrace(err) } diff --git a/aws/resources/transit_gateway_test.go b/aws/resources/transit_gateway_test.go index 5d345872..7b481999 100644 --- a/aws/resources/transit_gateway_test.go +++ b/aws/resources/transit_gateway_test.go @@ -20,10 +20,6 @@ type mockedTransitGateway struct { DeleteTransitGatewayOutput ec2.DeleteTransitGatewayOutput DescribeTransitGatewayAttachmentsOutput ec2.DescribeTransitGatewayAttachmentsOutput DeleteTransitGatewayPeeringAttachmentOutput ec2.DeleteTransitGatewayPeeringAttachmentOutput - DeleteTransitGatewayVpcAttachmentOutput ec2.DeleteTransitGatewayVpcAttachmentOutput - DeleteVpnConnectionOutput ec2.DeleteVpnConnectionOutput - DeleteTransitGatewayConnectOutput ec2.DeleteTransitGatewayConnectOutput - } func (m mockedTransitGateway) DescribeTransitGatewaysWithContext(_ awsgo.Context, _ *ec2.DescribeTransitGatewaysInput, _ ...request.Option) (*ec2.DescribeTransitGatewaysOutput, error) { @@ -41,13 +37,6 @@ func (m mockedTransitGateway) DescribeTransitGatewayAttachmentsWithContext(awsgo func (m mockedTransitGateway) DeleteTransitGatewayPeeringAttachmentWithContext(aws.Context, *ec2.DeleteTransitGatewayPeeringAttachmentInput, ...request.Option) (*ec2.DeleteTransitGatewayPeeringAttachmentOutput, error) { return &m.DeleteTransitGatewayPeeringAttachmentOutput, nil } -func (m mockedTransitGateway) DeleteTransitGatewayVpcAttachmentWithContext(_ awsgo.Context, _ *ec2.DeleteTransitGatewayVpcAttachmentInput, _ ...request.Option) (*ec2.DeleteTransitGatewayVpcAttachmentOutput, error) { - return &m.DeleteTransitGatewayVpcAttachmentOutput, nil -} - -func (m mockedTransitGateway) DeleteTransitGatewayConnectWithContext(_ awsgo.Context, _ *ec2.DeleteTransitGatewayConnectInput, _ ...request.Option) (*ec2.DeleteTransitGatewayConnectOutput, error) { - return &m.DeleteTransitGatewayConnectOutput, nil -} func (m mockedTransitGateway) WaitUntilTransitGatewayAttachmentDeleted(*string, string) error { return nil } diff --git a/config/config.go b/config/config.go index f745e175..f4ca1de1 100644 --- a/config/config.go +++ b/config/config.go @@ -56,7 +56,6 @@ type Config struct { EC2IPAMResourceDiscovery ResourceType `yaml:"EC2IPAMResourceDiscovery"` EC2IPAMScope ResourceType `yaml:"EC2IPAMScope"` EC2Endpoint ResourceType `yaml:"EC2Endpoint"` - EC2PlacementGroups ResourceType `yaml:"EC2PlacementGroups"` EC2Subnet EC2ResourceType `yaml:"EC2Subnet"` EgressOnlyInternetGateway ResourceType `yaml:"EgressOnlyInternetGateway"` ECRRepository ResourceType `yaml:"ECRRepository"` @@ -320,11 +319,17 @@ func matches(name string, regexps []Expression) bool { } // ShouldInclude - Checks if a resource's Name should be included according to the inclusion and exclusion rules -func ShouldInclude(name string, includeREs []Expression, excludeREs []Expression) bool { +func ShouldInclude(name *string, includeREs []Expression, excludeREs []Expression) bool { + var resourceName string + if name != nil { + resourceName = *name + } + + if len(includeREs) == 0 && len(excludeREs) == 0 { // If no rules are defined, should always include return true - } else if matches(name, excludeREs) { + } else if matches(resourceName, excludeREs) { // If a rule that exclude matches, should not include return false } else if len(includeREs) == 0 { @@ -332,7 +337,7 @@ func ShouldInclude(name string, includeREs []Expression, excludeREs []Expression return true } else { // Given there is a 'include' list, and 'Name' is there, should include - return matches(name, includeREs) + return matches(resourceName, includeREs) } } @@ -403,7 +408,7 @@ func (r ResourceType) ShouldIncludeBasedOnTag(tags map[string]string) bool { } func (r ResourceType) ShouldInclude(value ResourceValue) bool { - if value.Name != nil && !ShouldInclude(*value.Name, r.IncludeRule.NamesRegExp, r.ExcludeRule.NamesRegExp) { + if !ShouldInclude(value.Name, r.IncludeRule.NamesRegExp, r.ExcludeRule.NamesRegExp) { return false } else if value.Time != nil && !r.ShouldIncludeBasedOnTime(*value.Time) { return false diff --git a/config/config_test.go b/config/config_test.go index cc31eb2d..1abbd7ab 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -48,7 +48,6 @@ func emptyConfig() *Config { EC2IPAMResourceDiscovery: ResourceType{FilterRule{}, FilterRule{}, "",false}, EC2IPAMScope: ResourceType{FilterRule{}, FilterRule{}, "",false}, EC2Endpoint: ResourceType{FilterRule{}, FilterRule{}, "",false}, - EC2PlacementGroups: ResourceType{FilterRule{}, FilterRule{}, "",false}, EC2Subnet: EC2ResourceType{false, ResourceType{FilterRule{}, FilterRule{}, "",false}}, EgressOnlyInternetGateway: ResourceType{FilterRule{}, FilterRule{}, "",false}, ECRRepository: ResourceType{FilterRule{}, FilterRule{}, "",false}, @@ -161,7 +160,7 @@ func TestShouldInclude_AllowWhenEmpty(t *testing.T) { var includeREs []Expression var excludeREs []Expression - assert.True(t, ShouldInclude("test-open-vpn", includeREs, excludeREs), + assert.True(t, ShouldInclude(aws.String("test-open-vpn"), includeREs, excludeREs), "Should include when both lists are empty") } @@ -172,9 +171,9 @@ func TestShouldInclude_ExcludeWhenMatches(t *testing.T) { require.NoError(t, err) excludeREs := []Expression{{RE: *exclude}} - assert.False(t, ShouldInclude("test-openvpn-123", includeREs, excludeREs), + assert.False(t, ShouldInclude(aws.String("test-openvpn-123"), includeREs, excludeREs), "Should not include when matches from the 'exclude' list") - assert.True(t, ShouldInclude("tf-state-bucket", includeREs, excludeREs), + assert.True(t, ShouldInclude(aws.String("tf-state-bucket"), includeREs, excludeREs), "Should include when doesn't matches from the 'exclude' list") } @@ -185,9 +184,9 @@ func TestShouldInclude_IncludeWhenMatches(t *testing.T) { var excludeREs []Expression - assert.True(t, ShouldInclude("test-openvpn-123", includeREs, excludeREs), + assert.True(t, ShouldInclude(aws.String("test-openvpn-123"), includeREs, excludeREs), "Should include when matches the 'include' list") - assert.False(t, ShouldInclude("test-vpc-123", includeREs, excludeREs), + assert.False(t, ShouldInclude(aws.String("test-vpc-123"), includeREs, excludeREs), "Should not include when doesn't matches the 'include' list") } @@ -200,11 +199,11 @@ func TestShouldInclude_WhenMatchesIncludeAndExclude(t *testing.T) { require.NoError(t, err) excludeREs := []Expression{{RE: *exclude}} - assert.True(t, ShouldInclude("test-eks-cluster-123", includeREs, excludeREs), + assert.True(t, ShouldInclude(aws.String("test-eks-cluster-123"), includeREs, excludeREs), "Should include when matches the 'include' list but not matches the 'exclude' list") - assert.False(t, ShouldInclude("test-openvpn-123", includeREs, excludeREs), + assert.False(t, ShouldInclude(aws.String("test-openvpn-123"), includeREs, excludeREs), "Should not include when matches 'exclude' list") - assert.False(t, ShouldInclude("terraform-tf-state", includeREs, excludeREs), + assert.False(t, ShouldInclude(aws.String("terraform-tf-state"), includeREs, excludeREs), "Should not include when doesn't matches 'include' list") }