Remove duplicate tags (#262)

* duplicatetags * remove duplicate tags * Use reflect and common function * Add changelog
giantswarm · Jun 25, 2024 · e9a9cf2 · e9a9cf2
1 parent 9c4c0aa
commit e9a9cf2
Show file tree

Hide file tree

Showing 7 changed files with 161 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Avoid duplicate AWS tags
+
 ## [0.27.4] - 2024-06-20
 
 ### Fixed

diff --git a/pkg/aws/services/acm/acm.go b/pkg/aws/services/acm/acm.go
@@ -64,6 +64,8 @@ func (s *Service) EnsureCertificate(domain string, customerTags map[string]strin
 		input.Tags = append(input.Tags, tag)
 	}
 
+	input.Tags = util.FilterUniqueTags(input.Tags)
+
 	s.scope.Logger().Info("Creating ACM certificate")
 
 	output, err := s.Client.RequestCertificate(input)

diff --git a/pkg/aws/services/cloudfront/cloudfront.go b/pkg/aws/services/cloudfront/cloudfront.go
@@ -146,12 +146,15 @@ func (s *Service) EnsureDistribution(config DistributionConfig) (*Distribution,
 		}
 
 		for k, v := range customerTags {
+			// Convert i.DistributionConfigWithTags.Tags.Items to a slice of strings
 			tag := &cloudfront.Tag{
 				Key:   aws.String(k),
 				Value: aws.String(v),
 			}
 			i.DistributionConfigWithTags.Tags.Items = append(i.DistributionConfigWithTags.Tags.Items, tag)
 		}
+
+		i.DistributionConfigWithTags.Tags.Items = util.FilterUniqueTags(i.DistributionConfigWithTags.Tags.Items)
 	}
 
 	if diff.NeedsCreate {

diff --git a/pkg/aws/services/iam/oidc.go b/pkg/aws/services/iam/oidc.go
@@ -63,12 +63,19 @@ func (s *Service) EnsureOIDCProviders(identityProviderURLs []string, clientID st
 				desiredTags = append(desiredTags, tag)
 			}
 
+			var tagKeys []string
+			for _, item := range desiredTags {
+				tagKeys = append(tagKeys, *item.Key)
+			}
+
 			for k, v := range customerTags {
-				tag := &iam.Tag{
-					Key:   aws.String(k),
-					Value: aws.String(v),
+				if !util.StringInSlice(k, tagKeys) {
+					tag := &iam.Tag{
+						Key:   aws.String(k),
+						Value: aws.String(v),
+					}
+					desiredTags = append(desiredTags, tag)
 				}
-				desiredTags = append(desiredTags, tag)
 			}
 
 			// Add a tag 'giantswarm.io/alias' that has value true for the provider having predictable URL and false for the cloudfront one
@@ -82,7 +89,7 @@ func (s *Service) EnsureOIDCProviders(identityProviderURLs []string, clientID st
 			})
 		}
 
-		desiredTags = removeDuplicates(desiredTags)
+		desiredTags = util.FilterUniqueTags(desiredTags)
 
 		// Check if one of the providers is already using the right URL.
 		found := false
@@ -330,16 +337,3 @@ func caThumbPrints(ep string) ([]string, error) {
 
 	return ret, nil
 }
-
-func removeDuplicates(tags []*iam.Tag) []*iam.Tag {
-	keys := make(map[string]bool)
-	list := []*iam.Tag{}
-
-	for _, entry := range tags {
-		if _, value := keys[*entry.Key]; !value {
-			keys[*entry.Key] = true
-			list = append(list, entry)
-		}
-	}
-	return list
-}
diff --git a/pkg/aws/services/s3/bucket.go b/pkg/aws/services/s3/bucket.go
@@ -73,6 +73,8 @@ func (s *Service) CreateTags(bucketName string, customerTags map[string]string)
 		i.Tagging.TagSet = append(i.Tagging.TagSet, &s3.Tag{Key: aws.String(k), Value: aws.String(v)})
 	}
 
+	i.Tagging.TagSet = util.FilterUniqueTags(i.Tagging.TagSet)
+
 	_, err := s.Client.PutBucketTagging(i)
 	if err != nil {
 		return err

diff --git a/pkg/util/util.go b/pkg/util/util.go
@@ -2,6 +2,7 @@ package util
 
 import (
 	"fmt"
+	"reflect"
 	"strings"
 )
 
@@ -35,3 +36,28 @@ func StringInSlice(a string, list []string) bool {
 	}
 	return false
 }
+
+func FilterUniqueTags[T any](tags []T) []T {
+	uniqueTags := make(map[string]string)
+	filteredTags := make([]T, 0)
+
+	for _, tag := range tags {
+		tagValue := reflect.ValueOf(tag)
+		keyField := tagValue.FieldByName("Key")
+		valueField := tagValue.FieldByName("Value")
+
+		if !keyField.IsValid() || !valueField.IsValid() {
+			continue
+		}
+
+		key := keyField.String()
+		value := valueField.String()
+
+		if _, exists := uniqueTags[key]; !exists {
+			uniqueTags[key] = value
+			filteredTags = append(filteredTags, tag)
+		}
+	}
+
+	return filteredTags
+}
diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go
@@ -1,6 +1,9 @@
 package util
 
-import "testing"
+import (
+	"reflect"
+	"testing"
+)
 
 func TestEnsureHTTPS(t *testing.T) {
 	tests := []struct {
@@ -32,3 +35,111 @@ func TestEnsureHTTPS(t *testing.T) {
 		})
 	}
 }
+
+func TestFilterTags(t *testing.T) {
+	type Tag struct {
+		Key   string
+		Value string
+	}
+
+	tests := []struct {
+		name string
+		tags []Tag
+		want []Tag
+	}{
+		{
+			name: "empty",
+			tags: []Tag{},
+			want: []Tag{},
+		},
+		{
+			name: "normal",
+			tags: []Tag{
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+			},
+			want: []Tag{
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+			},
+		},
+		{
+			name: "Duplicate Tag",
+			tags: []Tag{
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/cluster",
+					Value: "test1",
+				},
+			},
+			want: []Tag{
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/cluster",
+					Value: "test1",
+				},
+			},
+		},
+		{
+			name: "Multiple Duplicate Tag",
+			tags: []Tag{
+				{
+					Key:   "giantswart.io/installation",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/installation",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/cluster",
+					Value: "test1",
+				},
+			},
+			want: []Tag{
+				{
+					Key:   "giantswart.io/installation",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/organization",
+					Value: "test",
+				},
+				{
+					Key:   "giantswart.io/cluster",
+					Value: "test1",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			//compare the output of the function with the expected output
+			if got := FilterUniqueTags(tt.tags); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("FilterTags() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}