Skip to content

Commit e922bb8

Browse files
authored
Merge pull request #638 from michaelhtm/fix/ignoretagspassedonstart
fix: remove global var for System tags, pass systemTags from runtime
2 parents 7a7f688 + dbadfc5 commit e922bb8

File tree

5 files changed

+31
-14
lines changed

5 files changed

+31
-14
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.24.0
44

55
require (
66
github.com/aws-controllers-k8s/pkg v0.0.19
7-
github.com/aws-controllers-k8s/runtime v0.54.0
7+
github.com/aws-controllers-k8s/runtime v0.54.1
88
github.com/aws/aws-sdk-go v1.49.0
99
github.com/aws/aws-sdk-go-v2 v1.32.7
1010
github.com/dlclark/regexp2 v1.10.0 // indirect

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,12 @@ github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:l
7373
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
7474
github.com/aws-controllers-k8s/pkg v0.0.19 h1:q/NhMvj6fCkHAJTyasrOWNdYME9/eCdeA5tStz4hHb8=
7575
github.com/aws-controllers-k8s/pkg v0.0.19/go.mod h1:VvdjLWmR6IJ3KU8KByKiq/lJE8M+ur2piXysXKTGUS0=
76+
github.com/aws-controllers-k8s/runtime v0.53.1 h1:l9MkR1KfZW8H8icT5rrRK3pdnVVA4io/eINVe5aspWs=
77+
github.com/aws-controllers-k8s/runtime v0.53.1/go.mod h1:OkUJN+Ds799JLYZsMJrO2vDJ4snxUeHK2MgrQHbU+Qc=
7678
github.com/aws-controllers-k8s/runtime v0.54.0 h1:RWv5cVb428styiRTHugRF8bMWjXkgsDc5E6Q/BoD8gA=
7779
github.com/aws-controllers-k8s/runtime v0.54.0/go.mod h1:OkUJN+Ds799JLYZsMJrO2vDJ4snxUeHK2MgrQHbU+Qc=
80+
github.com/aws-controllers-k8s/runtime v0.54.1 h1:0mbCJELz3t7jbG4abNecF0yeRd8YeFZQPr7nnzr8DC8=
81+
github.com/aws-controllers-k8s/runtime v0.54.1/go.mod h1:OkUJN+Ds799JLYZsMJrO2vDJ4snxUeHK2MgrQHbU+Qc=
7882
github.com/aws/aws-sdk-go v1.49.0 h1:g9BkW1fo9GqKfwg2+zCD+TW/D36Ux+vtfJ8guF4AYmY=
7983
github.com/aws/aws-sdk-go v1.49.0/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk=
8084
github.com/aws/aws-sdk-go-v2 v1.32.7 h1:ky5o35oENWi0JYWUZkB7WYvVPP+bcRF5/Iq7JWSb5Rw=

pkg/generate/ack/runtime_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func (frm *fakeRM) EnsureTags(
147147
return nil
148148
}
149149

150-
func (frm *fakeRM) FilterSystemTags(acktypes.AWSResource) {}
150+
func (frm *fakeRM) FilterSystemTags(acktypes.AWSResource, []string) {}
151151

152152
// This test is mostly just a hack to introduce a Go module dependency between
153153
// the ACK runtime library and the code generator. The code generator doesn't

templates/pkg/resource/manager.go.tpl

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -312,13 +312,26 @@ func (rm *resourceManager) EnsureTags(
312312
{{- end }}
313313
}
314314
315-
// FilterAWSTags ignores tags that have keys that start with "aws:"
316-
// is needed to ensure the controller does not attempt to remove
317-
// tags set by AWS. This function needs to be called after each Read
318-
// operation.
319-
// Eg. resources created with cloudformation have tags that cannot be
320-
//removed by an ACK controller
321-
func (rm *resourceManager) FilterSystemTags(res acktypes.AWSResource) {
315+
// FilterSystemTags removes system-managed tags from the resource's tag collection
316+
// to prevent the controller from attempting to manage them. This includes:
317+
// - Tags with keys starting with "aws:" (AWS-managed system tags)
318+
// - Tags specified via the --resource-tags startup flag (controller-level tags)
319+
// - Tags injected by AWS services (e.g., CloudFormation, EKS, etc.)
320+
//
321+
// This filtering is essential because:
322+
// 1. AWS services automatically add system tags that cannot be modified by users
323+
// 2. Attempting to remove these tags would result in API errors
324+
// 3. The controller should only manage user-defined tags, not system tags
325+
//
326+
// Must be called after each Read operation to ensure the resource state
327+
// reflects only manageable tags. This prevents unnecessary update attempts
328+
// and maintains consistency between desired and actual resource state.
329+
//
330+
// Example system tags that are filtered:
331+
// - aws:cloudformation:stack-name (CloudFormation)
332+
// - aws:eks:cluster-name (EKS)
333+
// - services.k8s.aws/* (Kubernetes-managed)
334+
func (rm *resourceManager) FilterSystemTags(res acktypes.AWSResource, systemTags []string) {
322335
{{- if $hookCode := Hook .CRD "filter_tags" }}
323336
{{ $hookCode }}
324337
{{ else }}
@@ -342,7 +355,7 @@ func (rm *resourceManager) FilterSystemTags(res acktypes.AWSResource) {
342355
{{ end -}}
343356
existingTags = r.ko.Spec.{{ $tagField.Path }}
344357
resourceTags, tagKeyOrder := convertToOrderedACKTags(existingTags)
345-
ignoreSystemTags(resourceTags)
358+
ignoreSystemTags(resourceTags, systemTags)
346359
{{ GoCodeInitializeNestedStructField .CRD "r.ko" $tagField "svcapitypes" 1 -}}
347360
r.ko.Spec.{{ $tagField.Path }} = fromACKTags(resourceTags, tagKeyOrder)
348361
{{- end }}

templates/pkg/resource/tags.go.tpl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import(
1111
var (
1212
_ = svcapitypes.{{ .CRD.Kind }}{}
1313
_ = acktags.NewTags()
14-
ACKSystemTags = []string{"services.k8s.aws/namespace", "services.k8s.aws/controller-version"}
1514
)
1615

1716
{{- if $hookCode := Hook .CRD "convert_tags" }}
@@ -59,13 +58,14 @@ func fromACKTags(tags acktags.Tags, keyOrder []string) {{ $tagFieldGoType }} {
5958
{{ end }}
6059

6160
// ignoreSystemTags ignores tags that have keys that start with "aws:"
62-
// and ACKSystemTags, to avoid patching them to the resourceSpec.
61+
// and systemTags defined on startup via the --resource-tags flag,
62+
// to avoid patching them to the resourceSpec.
6363
// Eg. resources created with cloudformation have tags that cannot be
6464
// removed by an ACK controller
65-
func ignoreSystemTags(tags acktags.Tags) {
65+
func ignoreSystemTags(tags acktags.Tags, systemTags []string) {
6666
for k := range tags {
6767
if strings.HasPrefix(k, "aws:") ||
68-
slices.Contains(ACKSystemTags, k) {
68+
slices.Contains(systemTags, k) {
6969
delete(tags, k)
7070
}
7171
}

0 commit comments

Comments
 (0)