Skip to content

Commit 101f50a

Browse files
committed
eks: skip aws:-prefixed tags in nodegroup reconciliation
When reconciling tags for EKS managed nodegroups and Fargate profiles, ignore `aws:`-prefixed keys when building the list of keys to remove, using the same reserved-prefix logic as `pkg/cloud/tags` for tag application. Those tags are not part of CAPA’s desired tag map since the validating webhooks forbid them in `additionalTags`, but they can exist on nodegroups created outside of CAPA (e.g., via CloudFormation). Including them in `UntagResource` triggers an API validation error, failing reconciliation and adoption of pre-existing nodegroups. Additionally, this changelist also replaces manual copying of variables (`kCopy`/`vCopy`) with `ptr.To()` so the loop-variable aliasing workaround is not required. This has been tested on an eksctl-provisioned cluster which creates nodegroups using CloudFormation. The AWS-reserved tags on the nodegroup (`aws:cloudformation:stack-name`, `aws:cloudformation:logical-id` and `aws:cloudformation:stack-id`) were correctly skipped and reconciliation passed. Signed-off-by: cpu1 <patwal.chetan@gmail.com>
1 parent 0da8c3d commit 101f50a

2 files changed

Lines changed: 26 additions & 28 deletions

File tree

pkg/cloud/services/eks/tags.go

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package eks
1919
import (
2020
"context"
2121
"fmt"
22+
"slices"
23+
"strings"
2224

2325
"github.com/aws/aws-sdk-go-v2/aws"
2426
"github.com/aws/aws-sdk-go-v2/service/autoscaling"
@@ -63,23 +65,23 @@ func (s *Service) getEKSTagParams(id string) *infrav1.BuildParams {
6365
}
6466
}
6567

66-
func getTagUpdates(currentTags map[string]string, tags map[string]string) (untagKeys []string, newTags map[string]string) {
68+
func getTagUpdates(currentTags map[string]string, desiredTags map[string]string) (untagKeys []string, newTags map[string]string) {
6769
untagKeys = []string{}
6870
newTags = make(map[string]string)
6971
for key := range currentTags {
70-
if _, ok := tags[key]; !ok {
72+
if _, ok := desiredTags[key]; !ok && !strings.HasPrefix(key, tags.AwsInternalTagPrefix) {
7173
untagKeys = append(untagKeys, key)
7274
}
7375
}
74-
for key, value := range tags {
76+
for key, value := range desiredTags {
7577
if currentV, ok := currentTags[key]; !ok || value != currentV {
7678
newTags[key] = value
7779
}
7880
}
7981
return untagKeys, newTags
8082
}
8183

82-
func getASGTagUpdates(clusterName string, currentTags map[string]string, tags map[string]string) (tagsToDelete map[string]string, tagsToAdd map[string]string) {
84+
func getASGTagUpdates(clusterName string, currentTags, desiredTags map[string]string) (tagsToDelete, tagsToAdd map[string]string) {
8385
officialASGTagsByEKS := []string{
8486
eksClusterNameTag,
8587
eksNodeGroupNameTag,
@@ -90,20 +92,11 @@ func getASGTagUpdates(clusterName string, currentTags map[string]string, tags ma
9092
tagsToDelete = make(map[string]string)
9193
tagsToAdd = make(map[string]string)
9294
for k, v := range currentTags {
93-
if _, ok := tags[k]; !ok {
94-
isOfficialTag := false
95-
for _, tag := range officialASGTagsByEKS {
96-
if tag == k {
97-
isOfficialTag = true
98-
break
99-
}
100-
}
101-
if !isOfficialTag {
102-
tagsToDelete[k] = v
103-
}
95+
if _, ok := desiredTags[k]; !ok && !slices.Contains(officialASGTagsByEKS, k) {
96+
tagsToDelete[k] = v
10497
}
10598
}
106-
for key, value := range tags {
99+
for key, value := range desiredTags {
107100
if currentV, ok := currentTags[key]; !ok || value != currentV {
108101
tagsToAdd[key] = value
109102
}
@@ -137,16 +130,12 @@ func (s *NodegroupService) reconcileASGTags(ctx context.Context, ng *ekstypes.No
137130
if len(tagsToAdd) > 0 {
138131
input := &autoscaling.CreateOrUpdateTagsInput{}
139132
for k, v := range tagsToAdd {
140-
// The k/vCopy is used to address the "Implicit memory aliasing in for loop" issue
141-
// https://stackoverflow.com/questions/62446118/implicit-memory-aliasing-in-for-loop
142-
kCopy := k
143-
vCopy := v
144133
input.Tags = append(input.Tags, autoscalingtypes.Tag{
145-
Key: &kCopy,
134+
Key: ptr.To(k),
146135
PropagateAtLaunch: aws.Bool(true),
147136
ResourceId: asg.AutoScalingGroupName,
148-
ResourceType: ptr.To[string]("auto-scaling-group"),
149-
Value: &vCopy,
137+
ResourceType: ptr.To("auto-scaling-group"),
138+
Value: ptr.To(v),
150139
})
151140
}
152141
_, err = s.AutoscalingClient.CreateOrUpdateTags(ctx, input)
@@ -158,13 +147,10 @@ func (s *NodegroupService) reconcileASGTags(ctx context.Context, ng *ekstypes.No
158147
if len(tagsToDelete) > 0 {
159148
input := &autoscaling.DeleteTagsInput{}
160149
for k := range tagsToDelete {
161-
// The k/vCopy is used to address the "Implicit memory aliasing in for loop" issue
162-
// https://stackoverflow.com/questions/62446118/implicit-memory-aliasing-in-for-loop
163-
kCopy := k
164150
input.Tags = append(input.Tags, autoscalingtypes.Tag{
165-
Key: &kCopy,
151+
Key: ptr.To(k),
166152
ResourceId: asg.AutoScalingGroupName,
167-
ResourceType: ptr.To[string]("auto-scaling-group"),
153+
ResourceType: ptr.To("auto-scaling-group"),
168154
})
169155
}
170156
_, err = s.AutoscalingClient.DeleteTags(ctx, input)

pkg/cloud/services/eks/tags_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ func TestGetTagUpdates(t *testing.T) {
4242
"x": "2",
4343
},
4444
},
45+
{
46+
current: map[string]string{
47+
"kubernetes.io/cluster/foo": "owned",
48+
"aws:cloudformation:stack-name": "eks-stack",
49+
"nodegroup-name": "ng1",
50+
},
51+
next: map[string]string{
52+
"kubernetes.io/cluster/foo": "owned",
53+
},
54+
expectUntag: []string{"nodegroup-name"},
55+
expectTag: map[string]string{},
56+
},
4557
}
4658
for i, tc := range testCases {
4759
t.Run(strconv.Itoa(i), func(t *testing.T) {

0 commit comments

Comments
 (0)