Skip to content

Commit 8d7cb44

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #5181 from giantswarm/filter-out-aws-internal-tags
🐛 Filter out AWS internal tags when reconciling
2 parents 0cc6e45 + 4c61f2b commit 8d7cb44

File tree

2 files changed

+80
-25
lines changed

2 files changed

+80
-25
lines changed

‎pkg/cloud/tags/tags.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"context"
2222
"fmt"
2323
"sort"
24+
"strings"
2425

2526
"github.com/aws/aws-sdk-go/aws"
2627
"github.com/aws/aws-sdk-go/service/ec2"
@@ -32,6 +33,10 @@ import (
3233
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
3334
)
3435

36+
const (
37+
AwsInternalTagPrefix = "aws:" // AwsInternalTagPrefix is the prefix for AWS internal tags, which are reserved for internal AWS use.
38+
)
39+
3540
var (
3641
// ErrBuildParamsRequired defines an error for when no build params are supplied.
3742
ErrBuildParamsRequired = errors.New("no build params supplied")
@@ -99,7 +104,10 @@ func WithEC2(ec2client ec2iface.EC2API) BuilderOption {
99104
// For testing, we need sorted keys
100105
sortedKeys := make([]string, 0, len(tags))
101106
for k := range tags {
102-
sortedKeys = append(sortedKeys, k)
107+
// We want to filter out the tag keys that start with `aws:` as they are reserved for internal AWS use.
108+
if !strings.HasPrefix(k, AwsInternalTagPrefix) {
109+
sortedKeys = append(sortedKeys, k)
110+
}
103111
}
104112
sort.Strings(sortedKeys)
105113

@@ -127,10 +135,12 @@ func WithEKS(eksclient eksiface.EKSAPI) BuilderOption {
127135
return func(b *Builder) {
128136
b.applyFunc = func(params *infrav1.BuildParams) error {
129137
tags := infrav1.Build(*params)
130-
131138
eksTags := make(map[string]*string, len(tags))
132139
for k, v := range tags {
133-
eksTags[k] = aws.String(v)
140+
// We want to filter out the tag keys that start with `aws:` as they are reserved for internal AWS use.
141+
if !strings.HasPrefix(k, AwsInternalTagPrefix) {
142+
eksTags[k] = aws.String(v)
143+
}
134144
}
135145

136146
tagResourcesInput := &eks.TagResourceInput{

‎pkg/cloud/tags/tags_test.go

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,7 @@ import (
3434
)
3535

3636
var (
37-
bp = infrav1.BuildParams{
38-
Lifecycle: infrav1.ResourceLifecycleOwned,
39-
ClusterName: "testcluster",
40-
Name: aws.String("test"),
41-
Role: aws.String("testrole"),
42-
Additional: map[string]string{"k1": "v1"},
43-
}
44-
tags = []*ec2.Tag{
37+
expectedTags = []*ec2.Tag{
4538
{
4639
Key: aws.String("Name"),
4740
Value: aws.String("test"),
@@ -140,30 +133,64 @@ func TestTagsEnsureWithEC2(t *testing.T) {
140133
expect func(m *mocks.MockEC2APIMockRecorder)
141134
}{
142135
{
143-
name: "Should return error when create tag fails",
144-
builder: Builder{params: &bp},
136+
name: "Should return error when create tag fails",
137+
builder: Builder{params: &infrav1.BuildParams{
138+
Lifecycle: infrav1.ResourceLifecycleOwned,
139+
ClusterName: "testcluster",
140+
Name: aws.String("test"),
141+
Role: aws.String("testrole"),
142+
Additional: map[string]string{"k1": "v1"},
143+
}},
145144
expect: func(m *mocks.MockEC2APIMockRecorder) {
146145
m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{
147146
Resources: aws.StringSlice([]string{""}),
148-
Tags: tags,
147+
Tags: expectedTags,
149148
})).Return(nil, errors.New("failed to create tag"))
150149
},
151150
},
152151
{
153-
name: "Should return error when optional configuration for builder is nil",
154-
builder: Builder{params: &bp, applyFunc: nil},
152+
name: "Should return error when optional configuration for builder is nil",
153+
builder: Builder{params: &infrav1.BuildParams{
154+
Lifecycle: infrav1.ResourceLifecycleOwned,
155+
ClusterName: "testcluster",
156+
Name: aws.String("test"),
157+
Role: aws.String("testrole"),
158+
Additional: map[string]string{"k1": "v1"},
159+
}, applyFunc: nil},
155160
},
156161
{
157162
name: "Should return error when build params is nil",
158163
builder: Builder{params: nil},
159164
},
160165
{
161-
name: "Should ensure tags successfully",
162-
builder: Builder{params: &bp},
166+
name: "Should ensure tags successfully",
167+
builder: Builder{params: &infrav1.BuildParams{
168+
Lifecycle: infrav1.ResourceLifecycleOwned,
169+
ClusterName: "testcluster",
170+
Name: aws.String("test"),
171+
Role: aws.String("testrole"),
172+
Additional: map[string]string{"k1": "v1"},
173+
}},
174+
expect: func(m *mocks.MockEC2APIMockRecorder) {
175+
m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{
176+
Resources: aws.StringSlice([]string{""}),
177+
Tags: expectedTags,
178+
})).Return(nil, nil)
179+
},
180+
},
181+
{
182+
name: "Should filter internal aws tags",
183+
builder: Builder{params: &infrav1.BuildParams{
184+
Lifecycle: infrav1.ResourceLifecycleOwned,
185+
ClusterName: "testcluster",
186+
Name: aws.String("test"),
187+
Role: aws.String("testrole"),
188+
Additional: map[string]string{"k1": "v1", "aws:cloudformation:stack-name": "cloudformation-stack-name"},
189+
}},
163190
expect: func(m *mocks.MockEC2APIMockRecorder) {
164191
m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{
165192
Resources: aws.StringSlice([]string{""}),
166-
Tags: tags,
193+
Tags: expectedTags,
167194
})).Return(nil, nil)
168195
},
169196
},
@@ -198,8 +225,14 @@ func TestTagsEnsureWithEKS(t *testing.T) {
198225
expect func(m *mock_eksiface.MockEKSAPIMockRecorder)
199226
}{
200227
{
201-
name: "Should return error when tag resources fails",
202-
builder: Builder{params: &bp},
228+
name: "Should return error when tag resources fails",
229+
builder: Builder{params: &infrav1.BuildParams{
230+
Lifecycle: infrav1.ResourceLifecycleOwned,
231+
ClusterName: "testcluster",
232+
Name: aws.String("test"),
233+
Role: aws.String("testrole"),
234+
Additional: map[string]string{"k1": "v1"},
235+
}},
203236
expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {
204237
m.TagResource(gomock.Eq(&eks.TagResourceInput{
205238
ResourceArn: aws.String(""),
@@ -208,8 +241,14 @@ func TestTagsEnsureWithEKS(t *testing.T) {
208241
},
209242
},
210243
{
211-
name: "Should ensure tags successfully",
212-
builder: Builder{params: &bp},
244+
name: "Should ensure tags successfully",
245+
builder: Builder{params: &infrav1.BuildParams{
246+
Lifecycle: infrav1.ResourceLifecycleOwned,
247+
ClusterName: "testcluster",
248+
Name: aws.String("test"),
249+
Role: aws.String("testrole"),
250+
Additional: map[string]string{"k1": "v1"},
251+
}},
213252
expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {
214253
m.TagResource(gomock.Eq(&eks.TagResourceInput{
215254
ResourceArn: aws.String(""),
@@ -243,10 +282,16 @@ func TestTagsEnsureWithEKS(t *testing.T) {
243282

244283
func TestTagsBuildParamsToTagSpecification(t *testing.T) {
245284
g := NewWithT(t)
246-
tagSpec := BuildParamsToTagSpecification("test-resource", bp)
285+
tagSpec := BuildParamsToTagSpecification("test-resource", infrav1.BuildParams{
286+
Lifecycle: infrav1.ResourceLifecycleOwned,
287+
ClusterName: "testcluster",
288+
Name: aws.String("test"),
289+
Role: aws.String("testrole"),
290+
Additional: map[string]string{"k1": "v1"},
291+
})
247292
expectedTagSpec := &ec2.TagSpecification{
248293
ResourceType: aws.String("test-resource"),
249-
Tags: tags,
294+
Tags: expectedTags,
250295
}
251296
g.Expect(expectedTagSpec).To(Equal(tagSpec))
252297
}

0 commit comments

Comments
 (0)