Fix using matchTags and matchNoTags together

Makefile

@@ -33,7 +33,7 @@ test: ## Run unit tests
 .PHONY: test-all
 test-all: ## Run tests (including acceptance and integration tests)
 	go clean -testcache ${PKG_LIST}
-	./bin/go-acc ${PKG_LIST} -- -v -$(TESTARGS) -p 1 -race -timeout 30m
+	./bin/go-acc ${PKG_LIST} -- -v $(TESTARGS) -p 1 -race -timeout 30m
 
 .PHONY: build
 build: ## Build binary

README.md

@@ -46,7 +46,7 @@ Delete resources via a filter declared in a YAML file.
 The following filter deletes all EC2 instances that ID matches `^foo.*` and that have been created between
  `2018-06-28 12:28:39` and `2018-10-14` UTC (instance filter part 1); additionally, EC2 instances having a tag 
  `foo: bar` *AND* not a tag key `owner` with arbitrary value are deleted (instance filter part 2); last but not least,
- ALL security groups are deleted.
+ ALL security groups are deleted:
 
     aws_instance:
 
@@ -66,8 +66,6 @@ The following filter deletes all EC2 instances that ID matches `^foo.*` and that
 The general filter syntax is as follows:
 
     <resource type>:
-
-      # filter 1
       - id: <regex to filter by id> | NOT(<regex to filter by id>)
         tagged: bool (optional)
         tags:
@@ -76,10 +74,8 @@ The general filter syntax is as follows:
         created:
           before: <timestamp> (optional)
           after: <timestamp> (optional)
-
-      # filter 2
+      # OR
       - ...
-
     <resource type>:
       ...
 

resource/filter.go

@@ -14,7 +14,7 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-// Filter represents the content of a yaml file that is used filter resources for deletion.
+// Filter represents the content of a yaml file that is used to filter resources for deletion.
 type Filter map[TerraformResourceType][]TypeFilter
 
 // TypeFilter represents an entry in the yaml file to filter the resources of a particular resource type.
@@ -101,6 +101,7 @@ func (f TypeFilter) matchID(id string) bool {
 	return false
 }
 
+// MatchTagged filters resources with a non-empty or empty tag set.
 func (f TypeFilter) MatchTagged(tags map[string]string) bool {
 	if f.Tagged == nil {
 		return true
@@ -117,19 +118,20 @@ func (f TypeFilter) MatchTagged(tags map[string]string) bool {
 	return false
 }
 
-// MatchesTags checks whether a resource's tags match the filter.
-//
-//The keys must match exactly, whereas the tag value is checked against a regex.
+// MatchesTags checks whether a resource's tag set matches the filter.
 func (f TypeFilter) MatchTags(tags map[string]string) bool {
-	if f.Tags == nil {
+	return f.matchIncludedTags(tags) && f.matchExcludedTags(tags)
+}
+
+// matchIncludedTags checks for tags that must be included in a resource's tag set.
+func (f TypeFilter) matchIncludedTags(tags map[string]string) bool {
+	tagFilters := notNegatedTagFilterExpr(f.Tags)
+
+	if len(tagFilters) == 0 {
 		return true
 	}
 
-	for key, valueFilter := range f.Tags {
-		if isNegated(key) {
-			continue
-		}
-
+	for key, valueFilter := range tagFilters {
 		value, ok := tags[key]
 		if !ok {
 			return false
@@ -147,25 +149,15 @@ func (f TypeFilter) MatchTags(tags map[string]string) bool {
 	return true
 }
 
-func isNegated(s string) bool {
-	if strings.HasPrefix(s, "NOT(") && strings.HasSuffix(s, ")") {
-		return true
-	}
-
-	return false
-}
+// matchExcludedTags checks for tags that must not exist in a resource's tag set.
+func (f TypeFilter) matchExcludedTags(tags map[string]string) bool {
+	tagFilters := negatedTagFilterExpr(f.Tags)
 
-func (f TypeFilter) MatchNoTags(tags map[string]string) bool {
-	if f.Tags == nil {
+	if len(tagFilters) == 0 {
 		return true
 	}
 
-	for key, valueFilter := range f.Tags {
-		if !isNegated(key) {
-			continue
-		}
-		key = strings.TrimSuffix(strings.TrimPrefix(key, "NOT("), ")")
-
+	for key, valueFilter := range tagFilters {
 		value, ok := tags[key]
 		if !ok {
 			return true
@@ -183,6 +175,39 @@ func (f TypeFilter) MatchNoTags(tags map[string]string) bool {
 	return false
 }
 
+// notNegatedTagFilterExpr returns tag filter expressions where keys are not surrounded by NOT(...).
+func notNegatedTagFilterExpr(tags map[string]StringFilter) map[string]StringFilter {
+	result := map[string]StringFilter{}
+
+	for key, value := range tags {
+		if !isNegatedTagKey(key) {
+			result[key] = value
+		}
+	}
+
+	return result
+}
+
+// notNegatedTagFilterExpr returns tag filter expressions where keys are surrounded by NOT(...).
+func negatedTagFilterExpr(tags map[string]StringFilter) map[string]StringFilter {
+	result := map[string]StringFilter{}
+
+	for key, value := range tags {
+		if isNegatedTagKey(key) {
+			key = strings.TrimPrefix(key, "NOT(")
+			key = strings.TrimSuffix(key, ")")
+
+			result[key] = value
+		}
+	}
+
+	return result
+}
+
+func isNegatedTagKey(key string) bool {
+	return strings.HasPrefix(key, "NOT(") && strings.HasSuffix(key, ")")
+}
+
 func (f TypeFilter) matchCreated(creationTime *time.Time) bool {
 	if f.Created == nil {
 		return true
@@ -205,8 +230,8 @@ func (f TypeFilter) matchCreated(creationTime *time.Time) bool {
 	return createdAfter && createdBefore
 }
 
-// matches checks whether a resource matches the filter criteria.
-func (f Filter) matches(r *Resource) bool {
+// Match checks whether a resource matches the filter criteria.
+func (f Filter) Match(r *Resource) bool {
 	resTypeFilters, found := f[r.Type]
 	if !found {
 		return false
@@ -219,7 +244,6 @@ func (f Filter) matches(r *Resource) bool {
 	for _, rtf := range resTypeFilters {
 		if rtf.MatchTagged(r.Tags) &&
 			rtf.MatchTags(r.Tags) &&
-			rtf.MatchNoTags(r.Tags) &&
 			rtf.matchID(r.ID) &&
 			rtf.matchCreated(r.Created) {
 			return true

resource/filter_test.go

@@ -170,7 +170,7 @@ func TestTypeFilter_MatchTagged(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			if got := tt.filter.MatchTagged(tt.tags); got != tt.want {
-				t.Errorf("MatchTags() = %v, want %v", got, tt.want)
+				t.Errorf("MatchTagged() = %v, want %v", got, tt.want)
 			}
 		})
 	}
@@ -389,7 +389,7 @@ func TestTypeFilter_MatchNoTags(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := tt.filter.MatchNoTags(tt.tags); got != tt.want {
+			if got := tt.filter.MatchTags(tt.tags); got != tt.want {
 				t.Errorf("MatchTags() = %v, want %v", got, tt.want)
 			}
 		})

resource/select.go

@@ -37,7 +37,7 @@ func (f Filter) defaultFilter(res Resources) []Resources {
 	result := Resources{}
 
 	for _, r := range res {
-		if f.matches(r) {
+		if f.Match(r) {
 			result = append(result, r)
 		}
 	}
@@ -49,7 +49,7 @@ func (f Filter) efsFileSystemFilter(res Resources, raw interface{}, c *AWS) []Re
 	resultMt := Resources{}
 
 	for _, r := range res {
-		if f.matches(&Resource{Type: r.Type, ID: *raw.([]*efs.FileSystemDescription)[0].Name}) {
+		if f.Match(&Resource{Type: r.Type, ID: *raw.([]*efs.FileSystemDescription)[0].Name}) {
 			res, err := c.DescribeMountTargets(&efs.DescribeMountTargetsInput{
 				FileSystemId: &r.ID,
 			})
@@ -74,7 +74,7 @@ func (f Filter) iamUserFilter(res Resources, c *AWS) []Resources {
 	resultUserPol := Resources{}
 
 	for _, r := range res {
-		if f.matches(r) {
+		if f.Match(r) {
 			// list inline policies, delete with "aws_iam_user_policy" delete routine
 			ups, err := c.ListUserPolicies(&iam.ListUserPoliciesInput{
 				UserName: &r.ID,
@@ -116,7 +116,7 @@ func (f Filter) iamPolicyFilter(res Resources, raw interface{}, c *AWS) []Resour
 	resultAtt := Resources{}
 
 	for i, r := range res {
-		if f.matches(r) {
+		if f.Match(r) {
 			es, err := c.ListEntitiesForPolicy(&iam.ListEntitiesForPolicyInput{
 				PolicyArn: &r.ID,
 			})
@@ -161,7 +161,7 @@ func (f Filter) kmsKeysFilter(res Resources, c *AWS) []Resources {
 	result := Resources{}
 
 	for _, r := range res {
-		if f.matches(r) {
+		if f.Match(r) {
 			req, res := c.DescribeKeyRequest(&kms.DescribeKeyInput{
 				KeyId: aws.String(r.ID),
 			})
@@ -184,7 +184,7 @@ func (f Filter) kmsKeyAliasFilter(res Resources) []Resources {
 	result := Resources{}
 
 	for _, r := range res {
-		if f.matches(r) && !strings.HasPrefix(r.ID, "alias/aws/") {
+		if f.Match(r) && !strings.HasPrefix(r.ID, "alias/aws/") {
 			result = append(result, r)
 		}
 	}