From a196b41f8adac6980fbf06f222a89778f1e1ef06 Mon Sep 17 00:00:00 2001 From: michaelhtm <98621731+michaelhtm@users.noreply.github.com> Date: Sat, 22 Nov 2025 21:28:36 -0800 Subject: [PATCH] fix: ignore tags passed on startup With this change, ACK now ignores tags that are passed on startup. On top of that it also valdidates ResourceTags on startup, instead of doing so during reconcile. --- mocks/pkg/types/aws_resource_manager.go | 6 +++--- pkg/config/config.go | 10 ++++++++++ pkg/runtime/reconciler.go | 2 +- pkg/runtime/reconciler_test.go | 7 +++++-- pkg/types/aws_resource_manager.go | 2 +- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/mocks/pkg/types/aws_resource_manager.go b/mocks/pkg/types/aws_resource_manager.go index d36d9af..0ee82e5 100644 --- a/mocks/pkg/types/aws_resource_manager.go +++ b/mocks/pkg/types/aws_resource_manager.go @@ -134,9 +134,9 @@ func (_m *AWSResourceManager) EnsureTags(_a0 context.Context, _a1 types.AWSResou return r0 } -// FilterSystemTags provides a mock function with given fields: _a0 -func (_m *AWSResourceManager) FilterSystemTags(_a0 types.AWSResource) { - _m.Called(_a0) +// FilterSystemTags provides a mock function with given fields: _a0, _a1 +func (_m *AWSResourceManager) FilterSystemTags(_a0 types.AWSResource, _a1 []string) { + _m.Called(_a0, _a1) } // IsSynced provides a mock function with given fields: _a0, _a1 diff --git a/pkg/config/config.go b/pkg/config/config.go index 7ddef6f..5767090 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -98,6 +98,7 @@ type Config struct { AllowUnsafeEndpointURL bool LogLevel string ResourceTags []string + ResourceTagKeys []string WatchNamespace string WatchSelectors string EnableWebhookServer bool @@ -307,6 +308,15 @@ func (cfg *Config) SetAWSAccountID(ctx context.Context) error { // Validate ensures the options are valid func (cfg *Config) Validate(ctx context.Context, options ...Option) error { + cfg.ResourceTagKeys = []string{} + // parse resource tags + for _, tag := range cfg.ResourceTags { + split := strings.Split(tag, "=") + if len(split) != 2 { + return fmt.Errorf("invalid resource tag: %s", tag) + } + cfg.ResourceTagKeys = append(cfg.ResourceTagKeys, split[0]) + } merged := mergeOptions(options) if len(merged.gvks) > 0 { err := cfg.validateReconcileConfigResources(merged.gvks) diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 8dbd6b0..2abf836 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -591,7 +591,7 @@ func (r *resourceReconciler) Sync( return latest, err } } else if adoptionPolicy == AdoptionPolicy_Adopt { - rm.FilterSystemTags(latest) + rm.FilterSystemTags(latest, r.cfg.ResourceTagKeys) if err = r.setResourceManagedAndAdopted(ctx, rm, latest); err != nil { return latest, err } diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index 36db708..9dd9cac 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -120,6 +120,7 @@ func reconcilerMocks( featuregate.ReadOnlyResources: {Enabled: true}, featuregate.ResourceAdoption: {Enabled: true}, }, + ResourceTagKeys: []string{}, } metrics := ackmetrics.NewMetrics("bookstore") @@ -393,7 +394,7 @@ func TestReconcilerAdoptResource(t *testing.T) { rd.On("Delta", latest, latest).Return(ackcompare.NewDelta()) r, kc, scmd := reconcilerMocks(rmf) - rm.On("FilterSystemTags", latest).Return() + rm.On("FilterSystemTags", latest, []string{}) rd.On("MarkAdopted", latest).Return() rm.On("EnsureTags", ctx, desired, scmd).Return(nil) statusWriter := &ctrlrtclientmock.SubResourceWriter{} @@ -783,7 +784,7 @@ func TestReconcilerUpdate(t *testing.T) { rm.On("ReadOne", ctx, desired).Return( latest, nil, ) - rm.On("FilterSystemTags", latest) + rm.On("FilterSystemTags", latest, []string{}) rm.On("Update", ctx, desired, latest, delta).Return( latest, nil, ) @@ -1851,6 +1852,7 @@ func TestReconcile_AccountDrifted(t *testing.T) { // Create caches with the k8s client caches := ackrtcache.New(fakeLogger, ackrtcache.Config{}, featuregate.FeatureGates{}) + irscaches := iamroleselector.NewCache(fakeLogger) // Run the caches stopCh := make(chan struct{}) @@ -1891,6 +1893,7 @@ func TestReconcile_AccountDrifted(t *testing.T) { log: fakeLogger, cfg: ackcfg.Config{AccountID: "333333333333"}, carmCache: caches, + irsCache: irscaches, metrics: ackmetrics.NewMetrics("test"), }, rmf: rmf, diff --git a/pkg/types/aws_resource_manager.go b/pkg/types/aws_resource_manager.go index 4ca5740..6ca943e 100644 --- a/pkg/types/aws_resource_manager.go +++ b/pkg/types/aws_resource_manager.go @@ -90,7 +90,7 @@ type AWSResourceManager interface { // and this function will remove them before adoption. // Eg. resources created with cloudformation have tags that cannot be //removed by an ACK controller - FilterSystemTags(AWSResource) + FilterSystemTags(AWSResource, []string) } // AWSResourceManagerFactory returns an AWSResourceManager that can be used to