diff --git a/pkg/cloud/tags/tags.go b/pkg/cloud/tags/tags.go index 42c8bfd843..47654eb4a4 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" @@ -32,6 +33,10 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" ) +const ( + AwsInternalTagPrefix = "aws:" // AwsInternalTagPrefix is the prefix for AWS internal tags, which are reserved for internal AWS use. +) + var ( // ErrBuildParamsRequired defines an error for when no build params are supplied. ErrBuildParamsRequired = errors.New("no build params supplied") @@ -99,7 +104,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, AwsInternalTagPrefix) { + sortedKeys = append(sortedKeys, k) + } } sort.Strings(sortedKeys) @@ -127,10 +135,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, AwsInternalTagPrefix) { + 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)) }