From 985e356a0a3276fc80ceb98c1ddd6fc434827f4e Mon Sep 17 00:00:00 2001 From: Jeremiah Stuever Date: Tue, 10 Mar 2026 14:21:58 -0700 Subject: [PATCH] Fix infrastructure resource name filtering in watch predicate OCPBUGS-74627: CCO should only process the infrastructure resource named "cluster", not other infrastructure resources like "cloud-provider-config" that may exist in the cluster. This change updates the infraResourcePredicate to check that the infrastructure resource name is "cluster" before processing create and update events. This prevents CCO from incorrectly processing infrastructure resources with other names. Also adds comprehensive tests to verify the predicate correctly filters infrastructure resources by name. Assisted-by: Claude Sonnet 4.5 --- .../credentialsrequest_controller.go | 8 +- .../credentialsrequest_controller_test.go | 209 ++++++++++++++++++ 2 files changed, 214 insertions(+), 3 deletions(-) diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller.go b/pkg/operator/credentialsrequest/credentialsrequest_controller.go index 1dc5c34aa4..5cdfc27a4d 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller.go +++ b/pkg/operator/credentialsrequest/credentialsrequest_controller.go @@ -254,16 +254,18 @@ func add(mgr, adminMgr manager.Manager, r reconcile.Reconciler) error { return requests }) - // predicate functions to filter the events based on the AWS resourceTags presence. + // predicate functions to filter events for the "cluster" infrastructure resource based on AWS resourceTags presence. + // Only the infrastructure resource named "cluster" should be processed; other infrastructure resources like + // "cloud-provider-config" must be ignored to prevent incorrect behavior. infraResourcePredicate := predicate.TypedFuncs[*configv1.Infrastructure]{ CreateFunc: func(e event.TypedCreateEvent[*configv1.Infrastructure]) bool { - return hasResourceTags(e.Object) + return e.Object.GetName() == "cluster" && hasResourceTags(e.Object) }, DeleteFunc: func(e event.TypedDeleteEvent[*configv1.Infrastructure]) bool { return false }, UpdateFunc: func(e event.TypedUpdateEvent[*configv1.Infrastructure]) bool { - return areTagsUpdated(e.ObjectOld, e.ObjectNew) + return e.ObjectNew.GetName() == "cluster" && areTagsUpdated(e.ObjectOld, e.ObjectNew) }, } // Watch for the changes happening to Infrastructure Resource diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go b/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go index 09953918ea..5bc3ced64f 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go +++ b/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go @@ -2074,3 +2074,212 @@ func testOperatorConfig(mode operatorv1.CloudCredentialsMode) *operatorv1.CloudC return conf } + +func TestHasResourceTags(t *testing.T) { + tests := []struct { + name string + infra *configv1.Infrastructure + expected bool + }{ + { + name: "infrastructure with resource tags", + infra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "infrastructure without resource tags", + infra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{}, + }, + }, + }, + expected: false, + }, + { + name: "infrastructure with nil status", + infra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + expected: false, + }, + { + name: "nil infrastructure", + infra: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasResourceTags(tt.infra) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestAreTagsUpdated(t *testing.T) { + tests := []struct { + name string + oldInfra *configv1.Infrastructure + newInfra *configv1.Infrastructure + expected bool + }{ + { + name: "tags updated", + oldInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + }, + }, + }, + }, + }, + newInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value2"}, + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "tags not updated", + oldInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + }, + }, + }, + }, + }, + newInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + }, + }, + }, + }, + }, + expected: false, + }, + { + name: "new tags added", + oldInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{}, + }, + }, + }, + newInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "new infra without tags", + oldInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + }, + }, + }, + }, + }, + newInfra: &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{}, + }, + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := areTagsUpdated(tt.oldInfra, tt.newInfra) + assert.Equal(t, tt.expected, result) + }) + } +}