Repository: gruntwork-io/cloud-nuke
Pull request #607 (2023-10-17 21:26:28)

Fix the bug of overwriting config value generated from yml file
<!-- Prepend '[WIP]' to the title if this PR is still a work-in-progress. Remove it when it is ready for review! -->

## Description

Fixes https://github.com/gruntwork-io/cloud-nuke/issues/579. 
Added unit tests to validate the expected behaviour. 
Also tested things locally. 

When we were reading config file + setting time filter together, we had a bug where the configuration file content would have been overwritten by time filter, instead of merging the two. 

<!-- Description of the changes introduced by this PR. -->

## TODOs

Read the [Gruntwork contribution guidelines](https://gruntwork.notion.site/Gruntwork-Coding-Methodology-02fdcd6e4b004e818553684760bf691e).

- [ ] Update the docs.
- [ ] Run the relevant tests successfully, including pre-commit checks.
- [ ] Ensure any 3rd party code adheres with our [license policy](https://www.notion.so/gruntwork/Gruntwork-licenses-and-open-source-usage-policy-f7dece1f780341c7b69c1763f22b1378) or delete this line if its not applicable.
- [ ] Include release notes. If this PR is backward incompatible, include a migration guide.
- [ ] Attention Grunts - if this PR adds support for a new resource, ensure the `nuke_sandbox` and `nuke_phxdevops` jobs in `.circleci/config.yml` have been updated with appropriate exclusions (either directly in the job or via the `.circleci/nuke_config.yml` file) to prevent nuking IAM roles, groups, resources, etc that are important for the test accounts.


## Release Notes (draft)

<!-- One-line description of the PR that can be included in the final release notes. -->
Added / Removed / Updated [X].

### Migration Guide

<!-- Important: If you made any backward incompatible changes, then you must write a migration guide! -->




Patch:
diff --git a/config/config.go b/config/config.go
index 75d4fd2..5b343a3 100644
--- a/config/config.go
+++ b/config/config.go
@@ -86,15 +86,15 @@ func (c *Config) addTimeAfterFilter(timeFilter *time.Time, fieldName string) {
 	}
 
 	v := reflect.ValueOf(c).Elem()
-	filterRule := FilterRule{TimeAfter: timeFilter}
 	for i := 0; i < v.NumField(); i++ {
 		field := v.Field(i)
-		if field.Kind() == reflect.Struct {
-			ruleField := field.FieldByName(fieldName)
-			if ruleField.CanSet() {
-				ruleField.Set(reflect.ValueOf(filterRule))
-			}
+		if field.Kind() != reflect.Struct {
+			continue
 		}
+
+		ruleField := field.FieldByName(fieldName)
+		filterRule := ruleField.Addr().Interface().(*FilterRule)
+		filterRule.TimeAfter = timeFilter
 	}
 }
 


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index 3a77281..b4b159a 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -1074,3 +1074,29 @@ func TestShouldInclude_NameAndTimeFilter(t *testing.T) {
 		Time: aws.Time(now.Add(-1)),
 	}))
 }
+
+func TestAddIncludeAndExcludeAfterTime(t *testing.T) {
+	now := aws.Time(time.Now())
+
+	exclude, err := regexp.Compile(`test.*`)
+	require.NoError(t, err)
+	excludeREs := []Expression{{RE: *exclude}}
+
+	testConfig := &Config{}
+	testConfig.ACM = ResourceType{
+		ExcludeRule: FilterRule{
+			NamesRegExp: excludeREs,
+			TimeAfter:   now,
+		},
+	}
+
+	testConfig.AddExcludeAfterTime(now)
+	assert.Equal(t, testConfig.ACM.ExcludeRule.NamesRegExp, excludeREs)
+	assert.Equal(t, testConfig.ACM.ExcludeRule.TimeAfter, now)
+	assert.Nil(t, testConfig.ACM.IncludeRule.TimeAfter)
+
+	testConfig.AddIncludeAfterTime(now)
+	assert.Equal(t, testConfig.ACM.ExcludeRule.NamesRegExp, excludeREs)
+	assert.Equal(t, testConfig.ACM.ExcludeRule.TimeAfter, now)
+	assert.Equal(t, testConfig.ACM.IncludeRule.TimeAfter, now)
+}