From 3ef7e0f469a6a00974c319d163e6335ea7099488 Mon Sep 17 00:00:00 2001 From: Jose Armesto Date: Tue, 22 Oct 2024 12:51:00 +0200 Subject: [PATCH] Filter out AWS internal tags when reconciling --- pkg/cloud/tags/tags.go | 12 +++-- pkg/cloud/tags/tags_test.go | 89 ++++++++++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 25 deletions(-) diff --git a/pkg/cloud/tags/tags.go b/pkg/cloud/tags/tags.go index 42c8bfd843..6fba38435c 100644 --- a/pkg/cloud/tags/tags.go +++ b/pkg/cloud/tags/tags.go @@ -21,6 +21,7 @@ import ( "context" "fmt" "sort" + "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -99,7 +100,10 @@ func WithEC2(ec2client ec2iface.EC2API) BuilderOption { // For testing, we need sorted keys sortedKeys := make([]string, 0, len(tags)) for k := range tags { - sortedKeys = append(sortedKeys, k) + // We want to filter out the tag keys that start with `aws:` as they are reserved for internal AWS use. + if !strings.HasPrefix(k, "aws:") { + sortedKeys = append(sortedKeys, k) + } } sort.Strings(sortedKeys) @@ -127,10 +131,12 @@ func WithEKS(eksclient eksiface.EKSAPI) BuilderOption { return func(b *Builder) { b.applyFunc = func(params *infrav1.BuildParams) error { tags := infrav1.Build(*params) - eksTags := make(map[string]*string, len(tags)) for k, v := range tags { - eksTags[k] = aws.String(v) + // We want to filter out the tag keys that start with `aws:` as they are reserved for internal AWS use. + if !strings.HasPrefix(k, "aws:") { + eksTags[k] = aws.String(v) + } } tagResourcesInput := &eks.TagResourceInput{ diff --git a/pkg/cloud/tags/tags_test.go b/pkg/cloud/tags/tags_test.go index 536db71c61..7ea0006d86 100644 --- a/pkg/cloud/tags/tags_test.go +++ b/pkg/cloud/tags/tags_test.go @@ -34,14 +34,7 @@ import ( ) var ( - bp = infrav1.BuildParams{ - Lifecycle: infrav1.ResourceLifecycleOwned, - ClusterName: "testcluster", - Name: aws.String("test"), - Role: aws.String("testrole"), - Additional: map[string]string{"k1": "v1"}, - } - tags = []*ec2.Tag{ + expectedTags = []*ec2.Tag{ { Key: aws.String("Name"), Value: aws.String("test"), @@ -140,30 +133,64 @@ func TestTagsEnsureWithEC2(t *testing.T) { expect func(m *mocks.MockEC2APIMockRecorder) }{ { - name: "Should return error when create tag fails", - builder: Builder{params: &bp}, + name: "Should return error when create tag fails", + builder: Builder{params: &infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1"}, + }}, expect: func(m *mocks.MockEC2APIMockRecorder) { m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{ Resources: aws.StringSlice([]string{""}), - Tags: tags, + Tags: expectedTags, })).Return(nil, errors.New("failed to create tag")) }, }, { - name: "Should return error when optional configuration for builder is nil", - builder: Builder{params: &bp, applyFunc: nil}, + name: "Should return error when optional configuration for builder is nil", + builder: Builder{params: &infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1"}, + }, applyFunc: nil}, }, { name: "Should return error when build params is nil", builder: Builder{params: nil}, }, { - name: "Should ensure tags successfully", - builder: Builder{params: &bp}, + name: "Should ensure tags successfully", + builder: Builder{params: &infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1"}, + }}, + expect: func(m *mocks.MockEC2APIMockRecorder) { + m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{ + Resources: aws.StringSlice([]string{""}), + Tags: expectedTags, + })).Return(nil, nil) + }, + }, + { + name: "Should filter internal aws tags", + builder: Builder{params: &infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1", "aws:cloudformation:stack-name": "cloudformation-stack-name"}, + }}, expect: func(m *mocks.MockEC2APIMockRecorder) { m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{ Resources: aws.StringSlice([]string{""}), - Tags: tags, + Tags: expectedTags, })).Return(nil, nil) }, }, @@ -198,8 +225,14 @@ func TestTagsEnsureWithEKS(t *testing.T) { expect func(m *mock_eksiface.MockEKSAPIMockRecorder) }{ { - name: "Should return error when tag resources fails", - builder: Builder{params: &bp}, + name: "Should return error when tag resources fails", + builder: Builder{params: &infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1"}, + }}, expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { m.TagResource(gomock.Eq(&eks.TagResourceInput{ ResourceArn: aws.String(""), @@ -208,8 +241,14 @@ func TestTagsEnsureWithEKS(t *testing.T) { }, }, { - name: "Should ensure tags successfully", - builder: Builder{params: &bp}, + name: "Should ensure tags successfully", + builder: Builder{params: &infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1"}, + }}, expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { m.TagResource(gomock.Eq(&eks.TagResourceInput{ ResourceArn: aws.String(""), @@ -243,10 +282,16 @@ func TestTagsEnsureWithEKS(t *testing.T) { func TestTagsBuildParamsToTagSpecification(t *testing.T) { g := NewWithT(t) - tagSpec := BuildParamsToTagSpecification("test-resource", bp) + tagSpec := BuildParamsToTagSpecification("test-resource", infrav1.BuildParams{ + Lifecycle: infrav1.ResourceLifecycleOwned, + ClusterName: "testcluster", + Name: aws.String("test"), + Role: aws.String("testrole"), + Additional: map[string]string{"k1": "v1"}, + }) expectedTagSpec := &ec2.TagSpecification{ ResourceType: aws.String("test-resource"), - Tags: tags, + Tags: expectedTags, } g.Expect(expectedTagSpec).To(Equal(tagSpec)) }