From 147b0888b99a095b8a8b28284112ed7746224b68 Mon Sep 17 00:00:00 2001 From: James Kwon <96548424+hongil0316@users.noreply.github.com> Date: Fri, 14 Jul 2023 14:38:33 -0400 Subject: [PATCH] Refactor Config to Capture all FilterRules and Apply to ApiGateway as prototype (#499) * Refactor Config to Capture all FilterRules and Apply to ApiGateway as prototype * fmt --- aws/apigateway.go | 28 ++----- aws/apigateway_test.go | 20 ++++- aws/aws.go | 2 +- aws/kms_customer_key_test.go | 28 +++---- commands/cli.go | 17 ++++- config/config.go | 143 ++++++++++++++++++++++------------- config/config_test.go | 74 +++++++++++++++++- 7 files changed, 216 insertions(+), 96 deletions(-) diff --git a/aws/apigateway.go b/aws/apigateway.go index d29b6667..961eb1df 100644 --- a/aws/apigateway.go +++ b/aws/apigateway.go @@ -2,7 +2,6 @@ package aws import ( "sync" - "time" commonTelemetry "github.com/gruntwork-io/go-commons/telemetry" @@ -16,8 +15,7 @@ import ( "github.com/hashicorp/go-multierror" ) -func (gateway ApiGateway) getAll( - excludeAfter time.Time, configObj config.Config) ([]*string, error) { +func (gateway ApiGateway) getAll(configObj config.Config) ([]*string, error) { result, err := gateway.Client.GetRestApis(&apigateway.GetRestApisInput{}) if err != nil { return []*string{}, errors.WithStackTrace(err) @@ -25,7 +23,10 @@ func (gateway ApiGateway) getAll( var IDs []*string for _, api := range result.Items { - if gateway.shouldInclude(api, excludeAfter, configObj) { + if configObj.APIGateway.ShouldInclude(config.ResourceValue{ + Name: api.Name, + Time: api.CreatedDate, + }) { IDs = append(IDs, api.Id) } } @@ -33,25 +34,6 @@ func (gateway ApiGateway) getAll( return IDs, nil } -func (gateway ApiGateway) shouldInclude( - apigw *apigateway.RestApi, excludeAfter time.Time, configObj config.Config) bool { - if apigw == nil { - return false - } - - if apigw.CreatedDate != nil { - if excludeAfter.Before(aws.TimeValue(apigw.CreatedDate)) { - return false - } - } - - return config.ShouldInclude( - aws.StringValue(apigw.Name), - configObj.APIGateway.IncludeRule.NamesRegExp, - configObj.APIGateway.ExcludeRule.NamesRegExp, - ) -} - func (gateway ApiGateway) nukeAll(identifiers []*string) error { if len(identifiers) == 0 { logging.Logger.Debugf("No API Gateways (v1) to nuke in region %s", gateway.Region) diff --git a/aws/apigateway_test.go b/aws/apigateway_test.go index 4776be41..a40240d4 100644 --- a/aws/apigateway_test.go +++ b/aws/apigateway_test.go @@ -47,7 +47,7 @@ func TestAPIGatewayGetAllAndNukeAll(t *testing.T) { }, } - apis, err := apiGateway.getAll(time.Now(), config.Config{}) + apis, err := apiGateway.getAll(config.Config{}) require.NoError(t, err) require.Contains(t, awsgo.StringValueSlice(apis), testApiID) @@ -73,12 +73,24 @@ func TestAPIGatewayGetAllTimeFilter(t *testing.T) { } // test API is not excluded from the filter - IDs, err := apiGateway.getAll(now.Add(1), config.Config{}) + IDs, err := apiGateway.getAll(config.Config{ + APIGateway: config.ResourceType{ + ExcludeRule: config.FilterRule{ + TimeAfter: aws.Time(now.Add(1)), + }, + }, + }) require.NoError(t, err) assert.Contains(t, aws.StringValueSlice(IDs), testApiID) // test API being excluded from the filter - apiGwIdsOlder, err := apiGateway.getAll(now.Add(-1), config.Config{}) + apiGwIdsOlder, err := apiGateway.getAll(config.Config{ + APIGateway: config.ResourceType{ + ExcludeRule: config.FilterRule{ + TimeAfter: aws.Time(now.Add(-1)), + }, + }, + }) require.NoError(t, err) assert.NotContains(t, aws.StringValueSlice(apiGwIdsOlder), testApiID) } @@ -101,7 +113,7 @@ func TestNukeAPIGatewayMoreThanOne(t *testing.T) { }, } - apis, err := apiGateway.getAll(time.Now(), config.Config{}) + apis, err := apiGateway.getAll(config.Config{}) require.NoError(t, err) require.Contains(t, awsgo.StringValueSlice(apis), testApiID1) require.Contains(t, awsgo.StringValueSlice(apis), testApiID2) diff --git a/aws/aws.go b/aws/aws.go index 0b062916..fc38db76 100644 --- a/aws/aws.go +++ b/aws/aws.go @@ -1470,7 +1470,7 @@ func GetAllResources(targetRegions []string, excludeAfter time.Time, resourceTyp } if IsNukeable(apiGateways.ResourceName(), resourceTypes) { start := time.Now() - gatewayIds, err := apiGateways.getAll(excludeAfter, configObj) + gatewayIds, err := apiGateways.getAll(configObj) if err != nil { ge := report.GeneralError{ Error: err, diff --git a/aws/kms_customer_key_test.go b/aws/kms_customer_key_test.go index d90bd478..5bd8d36c 100644 --- a/aws/kms_customer_key_test.go +++ b/aws/kms_customer_key_test.go @@ -6,14 +6,12 @@ import ( "testing" "time" - "github.com/gruntwork-io/cloud-nuke/telemetry" - - "github.com/gruntwork-io/cloud-nuke/config" - "github.com/gruntwork-io/cloud-nuke/util" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/kms" + "github.com/gruntwork-io/cloud-nuke/config" + "github.com/gruntwork-io/cloud-nuke/telemetry" + "github.com/gruntwork-io/cloud-nuke/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -48,10 +46,12 @@ func TestListKmsUserKeys(t *testing.T) { // test if matching by regexp works keys, aliases, err = getAllKmsUserKeys(session, KmsCustomerKeys{}.MaxBatchSize(), time.Now(), config.Config{ - KMSCustomerKeys: config.ResourceType{ - IncludeRule: config.FilterRule{ - NamesRegExp: []config.Expression{ - {RE: *regexp.MustCompile(fmt.Sprintf("^%s", keyAlias))}, + KMSCustomerKeys: config.KMSCustomerKeyResourceType{ + ResourceType: config.ResourceType{ + IncludeRule: config.FilterRule{ + NamesRegExp: []config.Expression{ + {RE: *regexp.MustCompile(fmt.Sprintf("^%s", keyAlias))}, + }, }, }, }, @@ -63,10 +63,12 @@ func TestListKmsUserKeys(t *testing.T) { // test if exclusion by regexp works keys, aliases, err = getAllKmsUserKeys(session, KmsCustomerKeys{}.MaxBatchSize(), time.Now(), config.Config{ - KMSCustomerKeys: config.ResourceType{ - ExcludeRule: config.FilterRule{ - NamesRegExp: []config.Expression{ - {RE: *regexp.MustCompile(fmt.Sprintf("^%s", keyAlias))}, + KMSCustomerKeys: config.KMSCustomerKeyResourceType{ + ResourceType: config.ResourceType{ + ExcludeRule: config.FilterRule{ + NamesRegExp: []config.Expression{ + {RE: *regexp.MustCompile(fmt.Sprintf("^%s", keyAlias))}, + }, }, }, }, diff --git a/commands/cli.go b/commands/cli.go index 512917dc..ae81f2bb 100644 --- a/commands/cli.go +++ b/commands/cli.go @@ -3,6 +3,7 @@ package commands import ( "fmt" "os" + "reflect" "strings" "time" @@ -288,6 +289,20 @@ func awsNuke(c *cli.Context) error { return errors.WithStackTrace(err) } + // exclude after filter has been applied to all resources via `older-than` flag, we are + // setting this rule across all resource types. + // + // TODO: after refactoring all the code, we can remove having excludeAfter in config and + // passing in as additional argument to GetAllResources. + v := reflect.ValueOf(configObj) + for i := 0; i < v.NumField(); i++ { + excludeRule := v.Field(i).FieldByName("ExcludeRule").Interface().(config.FilterRule) + excludeRule.TimeAfter = excludeAfter + } + + deleteUnaliasedKmsKeys := c.Bool("delete-unaliased-kms-keys") + configObj.KMSCustomerKeys.DeleteUnaliasedKeys = deleteUnaliasedKmsKeys + spinnerMsg := fmt.Sprintf("Retrieving active AWS resources in [%s]", strings.Join(targetRegions[:], ", ")) // Start a simple spinner to track progress reading all relevant AWS resources @@ -299,7 +314,7 @@ func awsNuke(c *cli.Context) error { return errors.WithStackTrace(spinnerErr) } - account, err := aws.GetAllResources(targetRegions, *excludeAfter, resourceTypes, configObj, c.Bool("delete-unaliased-kms-keys")) + account, err := aws.GetAllResources(targetRegions, *excludeAfter, resourceTypes, configObj, deleteUnaliasedKmsKeys) // Stop the spinner spinnerSuccess.Stop() if err != nil { diff --git a/config/config.go b/config/config.go index 417558fe..1682151e 100644 --- a/config/config.go +++ b/config/config.go @@ -4,61 +4,68 @@ import ( "io/ioutil" "path/filepath" "regexp" + "time" "gopkg.in/yaml.v2" ) // Config - the config object we pass around type Config struct { - S3 ResourceType `yaml:"s3"` - IAMUsers ResourceType `yaml:"IAMUsers"` - IAMGroups ResourceType `yaml:"IAMGroups"` - IAMPolicies ResourceType `yaml:"IAMPolicies"` - IAMServiceLinkedRoles ResourceType `yaml:"IAMServiceLinkedRoles"` - IAMRoles ResourceType `yaml:"IAMRoles"` - SecretsManagerSecrets ResourceType `yaml:"SecretsManager"` - NatGateway ResourceType `yaml:"NatGateway"` - AccessAnalyzer ResourceType `yaml:"AccessAnalyzer"` - CloudWatchDashboard ResourceType `yaml:"CloudWatchDashboard"` - OpenSearchDomain ResourceType `yaml:"OpenSearchDomain"` - DynamoDB ResourceType `yaml:"DynamoDB"` - EBSVolume ResourceType `yaml:"EBSVolume"` - LambdaFunction ResourceType `yaml:"LambdaFunction"` - ELBv2 ResourceType `yaml:"ELBv2"` - ECSService ResourceType `yaml:"ECSService"` - ECSCluster ResourceType `yaml:"ECSCluster"` - Elasticache ResourceType `yaml:"Elasticache"` - ElasticacheParameterGroups ResourceType `yaml:"ElasticacheParameterGroups"` - ElasticacheSubnetGroups ResourceType `yaml:"ElasticacheSubnetGroups"` - VPC ResourceType `yaml:"VPC"` - OIDCProvider ResourceType `yaml:"OIDCProvider"` - AutoScalingGroup ResourceType `yaml:"AutoScalingGroup"` - LaunchConfiguration ResourceType `yaml:"LaunchConfiguration"` - ElasticIP ResourceType `yaml:"ElasticIP"` - EC2 ResourceType `yaml:"EC2"` - EC2KeyPairs ResourceType `yaml:"EC2KeyPairs"` - EC2DedicatedHosts ResourceType `yaml:"EC2DedicatedHosts"` - CloudWatchLogGroup ResourceType `yaml:"CloudWatchLogGroup"` - KMSCustomerKeys ResourceType `yaml:"KMSCustomerKeys"` - EKSCluster ResourceType `yaml:"EKSCluster"` - SageMakerNotebook ResourceType `yaml:"SageMakerNotebook"` - KinesisStream ResourceType `yaml:"KinesisStream"` - APIGateway ResourceType `yaml:"APIGateway"` - APIGatewayV2 ResourceType `yaml:"APIGatewayV2"` - ElasticFileSystem ResourceType `yaml:"ElasticFileSystem"` - CloudtrailTrail ResourceType `yaml:"CloudtrailTrail"` - ECRRepository ResourceType `yaml:"ECRRepository"` - DBInstances ResourceType `yaml:"DBInstances"` - DBSubnetGroups ResourceType `yaml:"DBSubnetGroups"` - LaunchTemplate ResourceType `yaml:"LaunchTemplate"` - ConfigServiceRule ResourceType `yaml:"ConfigServiceRule"` - ConfigServiceRecorder ResourceType `yaml:"ConfigServiceRecorder"` - CloudWatchAlarm ResourceType `yaml:"CloudWatchAlarm"` - Redshift ResourceType `yaml:"Redshift"` - CodeDeployApplications ResourceType `yaml:"CodeDeployApplications"` - ACM ResourceType `yaml:"ACM"` - SNS ResourceType `yaml:"SNS"` - BackupVault ResourceType `yaml:"BackupVault"` + S3 ResourceType `yaml:"s3"` + IAMUsers ResourceType `yaml:"IAMUsers"` + IAMGroups ResourceType `yaml:"IAMGroups"` + IAMPolicies ResourceType `yaml:"IAMPolicies"` + IAMServiceLinkedRoles ResourceType `yaml:"IAMServiceLinkedRoles"` + IAMRoles ResourceType `yaml:"IAMRoles"` + SecretsManagerSecrets ResourceType `yaml:"SecretsManager"` + NatGateway ResourceType `yaml:"NatGateway"` + AccessAnalyzer ResourceType `yaml:"AccessAnalyzer"` + CloudWatchDashboard ResourceType `yaml:"CloudWatchDashboard"` + OpenSearchDomain ResourceType `yaml:"OpenSearchDomain"` + DynamoDB ResourceType `yaml:"DynamoDB"` + EBSVolume ResourceType `yaml:"EBSVolume"` + LambdaFunction ResourceType `yaml:"LambdaFunction"` + ELBv2 ResourceType `yaml:"ELBv2"` + ECSService ResourceType `yaml:"ECSService"` + ECSCluster ResourceType `yaml:"ECSCluster"` + Elasticache ResourceType `yaml:"Elasticache"` + ElasticacheParameterGroups ResourceType `yaml:"ElasticacheParameterGroups"` + ElasticacheSubnetGroups ResourceType `yaml:"ElasticacheSubnetGroups"` + VPC ResourceType `yaml:"VPC"` + OIDCProvider ResourceType `yaml:"OIDCProvider"` + AutoScalingGroup ResourceType `yaml:"AutoScalingGroup"` + LaunchConfiguration ResourceType `yaml:"LaunchConfiguration"` + ElasticIP ResourceType `yaml:"ElasticIP"` + EC2 ResourceType `yaml:"EC2"` + EC2KeyPairs ResourceType `yaml:"EC2KeyPairs"` + EC2DedicatedHosts ResourceType `yaml:"EC2DedicatedHosts"` + CloudWatchLogGroup ResourceType `yaml:"CloudWatchLogGroup"` + KMSCustomerKeys KMSCustomerKeyResourceType `yaml:"KMSCustomerKeys"` + EKSCluster ResourceType `yaml:"EKSCluster"` + SageMakerNotebook ResourceType `yaml:"SageMakerNotebook"` + KinesisStream ResourceType `yaml:"KinesisStream"` + APIGateway ResourceType `yaml:"APIGateway"` + APIGatewayV2 ResourceType `yaml:"APIGatewayV2"` + ElasticFileSystem ResourceType `yaml:"ElasticFileSystem"` + CloudtrailTrail ResourceType `yaml:"CloudtrailTrail"` + ECRRepository ResourceType `yaml:"ECRRepository"` + DBInstances ResourceType `yaml:"DBInstances"` + DBSubnetGroups ResourceType `yaml:"DBSubnetGroups"` + LaunchTemplate ResourceType `yaml:"LaunchTemplate"` + ConfigServiceRule ResourceType `yaml:"ConfigServiceRule"` + ConfigServiceRecorder ResourceType `yaml:"ConfigServiceRecorder"` + CloudWatchAlarm ResourceType `yaml:"CloudWatchAlarm"` + Redshift ResourceType `yaml:"Redshift"` + CodeDeployApplications ResourceType `yaml:"CodeDeployApplications"` + ACM ResourceType `yaml:"ACM"` + SNS ResourceType `yaml:"SNS"` + BackupVault ResourceType `yaml:"BackupVault"` +} + +type KMSCustomerKeyResourceType struct { + DeleteUnaliasedKeys bool `yaml:"delete_unaliased_keys"` + + ResourceType } type ResourceType struct { @@ -68,7 +75,8 @@ type ResourceType struct { type FilterRule struct { NamesRegExp []Expression `yaml:"names_regex"` - // TODO(james): consider adding time filter rule here instead of passing in as function argument in other place. + TimeAfter *time.Time `yaml:"time_after"` + TimeBefore *time.Time `yaml:"time_before"` } type Expression struct { @@ -124,7 +132,7 @@ func matches(name string, regexps []Expression) bool { return false } -// ShouldInclude - Checks if a resource's name should be included according to the inclusion and exclusion rules +// 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 { if len(includeREs) == 0 && len(excludeREs) == 0 { // If no rules are defined, should always include @@ -133,10 +141,39 @@ func ShouldInclude(name string, includeREs []Expression, excludeREs []Expression // If a rule that exclude matches, should not include return false } else if len(includeREs) == 0 { - // Given the 'name' is not in the 'exclude' list, should include if there is no 'include' list + // Given the 'Name' is not in the 'exclude' list, should include if there is no 'include' list return true } else { - // Given there is a 'include' list, and 'name' is there, should include + // Given there is a 'include' list, and 'Name' is there, should include return matches(name, includeREs) } } + +type ResourceValue struct { + Name *string + Time *time.Time +} + +func (r ResourceType) ShouldIncludeBasedOnTime(time time.Time) bool { + if r.ExcludeRule.TimeAfter != nil && time.After(*r.ExcludeRule.TimeAfter) { + return false + } else if r.ExcludeRule.TimeBefore != nil && time.Before(*r.ExcludeRule.TimeBefore) { + return false + } else if r.IncludeRule.TimeAfter != nil && time.Before(*r.IncludeRule.TimeAfter) { + return false + } else if r.IncludeRule.TimeBefore != nil && time.After(*r.IncludeRule.TimeBefore) { + return false + } + + return true +} + +func (r ResourceType) ShouldInclude(value ResourceValue) bool { + if value.Name != nil && !ShouldInclude(*value.Name, r.IncludeRule.NamesRegExp, r.ExcludeRule.NamesRegExp) { + return false + } else if value.Time != nil && !r.ShouldIncludeBasedOnTime(*value.Time) { + return false + } + + return true +} diff --git a/config/config_test.go b/config/config_test.go index 9d20af76..7371770d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -4,7 +4,9 @@ import ( "reflect" "regexp" "testing" + "time" + "github.com/aws/aws-sdk-go/aws" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -40,7 +42,7 @@ func emptyConfig() *Config { ResourceType{FilterRule{}, FilterRule{}}, ResourceType{FilterRule{}, FilterRule{}}, ResourceType{FilterRule{}, FilterRule{}}, - ResourceType{FilterRule{}, FilterRule{}}, + KMSCustomerKeyResourceType{false, ResourceType{FilterRule{}, FilterRule{}}}, ResourceType{FilterRule{}, FilterRule{}}, ResourceType{FilterRule{}, FilterRule{}}, ResourceType{FilterRule{}, FilterRule{}}, @@ -989,3 +991,73 @@ func TestShouldInclude_WhenMatchesIncludeAndExclude(t *testing.T) { assert.False(t, ShouldInclude("terraform-tf-state", includeREs, excludeREs), "Should not include when doesn't matches 'include' list") } + +func TestShouldIncludeBasedOnTime_IncludeTimeBefore(t *testing.T) { + now := time.Now() + + r := ResourceType{ + IncludeRule: FilterRule{TimeBefore: &now}, + } + assert.True(t, r.ShouldIncludeBasedOnTime(now.Add(-1))) + assert.False(t, r.ShouldIncludeBasedOnTime(now.Add(1))) +} + +func TestShouldIncludeBasedOnTime_IncludeTimeAfter(t *testing.T) { + now := time.Now() + + r := ResourceType{ + IncludeRule: FilterRule{TimeAfter: &now}, + } + assert.False(t, r.ShouldIncludeBasedOnTime(now.Add(-1))) + assert.True(t, r.ShouldIncludeBasedOnTime(now.Add(1))) +} + +func TestShouldIncludeBasedOnTime_ExcludeTimeBefore(t *testing.T) { + now := time.Now() + + r := ResourceType{ + ExcludeRule: FilterRule{TimeBefore: &now}, + } + assert.False(t, r.ShouldIncludeBasedOnTime(now.Add(-1))) + assert.True(t, r.ShouldIncludeBasedOnTime(now.Add(1))) +} + +func TestShouldIncludeBasedOnTime_ExcludeTimeAfter(t *testing.T) { + now := time.Now() + + r := ResourceType{ + ExcludeRule: FilterRule{TimeAfter: &now}, + } + assert.False(t, r.ShouldIncludeBasedOnTime(now.Add(1))) + assert.True(t, r.ShouldIncludeBasedOnTime(now.Add(-1))) +} + +func TestShouldInclude_NameAndTimeFilter(t *testing.T) { + now := time.Now() + + exclude, err := regexp.Compile(`test.*`) + require.NoError(t, err) + excludeREs := []Expression{{RE: *exclude}} + r := ResourceType{ + ExcludeRule: FilterRule{ + NamesRegExp: excludeREs, + TimeAfter: &now, + }, + } + + // Filter by Time + assert.False(t, r.ShouldInclude(ResourceValue{ + Name: aws.String("hello_world"), + Time: aws.Time(now.Add(1)), + })) + // Filter by Name + assert.False(t, r.ShouldInclude(ResourceValue{ + Name: aws.String("test_hello_world"), + Time: aws.Time(now.Add(1)), + })) + // Pass filters + assert.True(t, r.ShouldInclude(ResourceValue{ + Name: aws.String("hello_world"), + Time: aws.Time(now.Add(-1)), + })) +}