From fb0e3b9312a9963f680c0817184399251c9840ae Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Wed, 24 Sep 2025 17:10:13 +0200 Subject: [PATCH 1/4] Introduce refreshWhenExpiredOnly mode in resourcesync controller Sync resources only when the destination secret/configmap has expired --- pkg/cmd/recoverycontroller/csrcontroller.go | 1 + .../resourcesynccontroller.go | 1 + .../resourcesync_controller.go | 41 ++++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/recoverycontroller/csrcontroller.go b/pkg/cmd/recoverycontroller/csrcontroller.go index b11109c8c..9abf8c3b9 100644 --- a/pkg/cmd/recoverycontroller/csrcontroller.go +++ b/pkg/cmd/recoverycontroller/csrcontroller.go @@ -84,6 +84,7 @@ func NewCSRController( v1helpers.CachedSecretGetter(kubeClient.CoreV1(), kubeInformersForNamespaces), v1helpers.CachedConfigMapGetter(kubeClient.CoreV1(), kubeInformersForNamespaces), c.eventRecorder, + true, ) err := operatorresourcesync.AddSyncCSRControllerCA(c.resourceSyncController) if err != nil { diff --git a/pkg/operator/resourcesynccontroller/resourcesynccontroller.go b/pkg/operator/resourcesynccontroller/resourcesynccontroller.go index 2ff5fc53a..d59512486 100644 --- a/pkg/operator/resourcesynccontroller/resourcesynccontroller.go +++ b/pkg/operator/resourcesynccontroller/resourcesynccontroller.go @@ -38,6 +38,7 @@ func NewResourceSyncController( v1helpers.CachedSecretGetter(secretsGetter, kubeInformersForNamespaces), v1helpers.CachedConfigMapGetter(configMapsGetter, kubeInformersForNamespaces), eventRecorder, + false, ) if err := AddSyncCSRControllerCA(resourceSyncController); err != nil { return nil, err diff --git a/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go b/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go index cbfc2dd5f..315985e1b 100644 --- a/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go +++ b/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go @@ -4,13 +4,14 @@ import ( "context" "encoding/json" "fmt" - applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" "net/http" "sort" "strings" "sync" "time" + applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -19,6 +20,7 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/certrotation" "github.com/openshift/library-go/pkg/operator/condition" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/management" @@ -36,9 +38,10 @@ type ResourceSyncController struct { configMapSyncRules syncRules // secretSyncRules is a map from destination location to source location secretSyncRules syncRules - // knownNamespaces is the list of namespaces we are watching. knownNamespaces sets.Set[string] + // RefreshOnlyWhenExpired is a flag that indicates whether the controller should only refresh resources when they expire. + refreshWhenExpiredOnly bool configMapGetter corev1client.ConfigMapsGetter secretGetter corev1client.SecretsGetter @@ -60,6 +63,7 @@ func NewResourceSyncController( secretsGetter corev1client.SecretsGetter, configMapsGetter corev1client.ConfigMapsGetter, eventRecorder events.Recorder, + refreshWhenExpiredOnly bool, ) *ResourceSyncController { c := &ResourceSyncController{ controllerInstanceName: factory.ControllerInstanceName(instanceName, "ResourceSync"), @@ -69,6 +73,7 @@ func NewResourceSyncController( secretSyncRules: syncRules{}, kubeInformersForNamespaces: kubeInformersForNamespaces, knownNamespaces: kubeInformersForNamespaces.Namespaces(), + refreshWhenExpiredOnly: refreshWhenExpiredOnly, configMapGetter: v1helpers.CachedConfigMapGetter(configMapsGetter, kubeInformersForNamespaces), secretGetter: v1helpers.CachedSecretGetter(secretsGetter, kubeInformersForNamespaces), @@ -225,6 +230,15 @@ func (c *ResourceSyncController) Sync(ctx context.Context, syncCtx factory.SyncC continue } + // Don't run the sync if the destination secret is not expired in RefreshWhenExpiredOnly mode + if c.refreshWhenExpiredOnly { + cm, err := c.configMapGetter.ConfigMaps(destination.Namespace).Get(ctx, destination.Name, metav1.GetOptions{}) + if err == nil && !isExpired(&cm.ObjectMeta) { + klog.V(2).Infof("DEBUG Skipping sync of configmap %s/%s because it is not expired", destination.Namespace, destination.Name) + continue + } + } + _, _, err := resourceapply.SyncPartialConfigMap(ctx, c.configMapGetter, syncCtx.Recorder(), source.Namespace, source.Name, destination.Namespace, destination.Name, source.syncedKeys, []metav1.OwnerReference{}) if err != nil { errors = append(errors, errorWithProvider(source.Provider, err)) @@ -253,6 +267,14 @@ func (c *ResourceSyncController) Sync(ctx context.Context, syncCtx factory.SyncC continue } + // Don't run the sync if the destination secret is not expired in RefreshOnlyWhenExpired mode + if c.refreshWhenExpiredOnly { + secret, err := c.secretGetter.Secrets(destination.Namespace).Get(ctx, destination.Name, metav1.GetOptions{}) + if err == nil && !isExpired(&secret.ObjectMeta) { + continue + } + } + _, _, err := resourceapply.SyncPartialSecret(ctx, c.secretGetter, syncCtx.Recorder(), source.Namespace, source.Name, destination.Namespace, destination.Name, source.syncedKeys, []metav1.OwnerReference{}) if err != nil { errors = append(errors, errorWithProvider(source.Provider, err)) @@ -284,6 +306,21 @@ func (c *ResourceSyncController) Sync(ctx context.Context, syncCtx factory.SyncC return nil } +func isExpired(object *metav1.ObjectMeta) bool { + if object == nil { + return false + } + expirationTime := object.Annotations[certrotation.CertificateNotAfterAnnotation] + if expirationTime == "" { + return false + } + expiration, err := time.Parse(time.RFC3339, expirationTime) + if err != nil { + return false + } + return time.Now().After(expiration) +} + func NewDebugHandler(controller *ResourceSyncController) http.Handler { return &debugHTTPHandler{controller: controller} } From 68abd92c6b229cc6901f54db220cc72c39c57add Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Wed, 24 Sep 2025 17:15:07 +0200 Subject: [PATCH 2/4] CombineCABundleConfigMapsOptimistically: set CA bundle not after to the latest certificate found --- .../pkg/operator/resourcesynccontroller/core.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/core.go b/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/core.go index bbd2ab58f..67d55f5df 100644 --- a/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/core.go +++ b/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/core.go @@ -4,6 +4,7 @@ import ( "crypto/x509" "fmt" "reflect" + "time" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -78,6 +79,7 @@ func CombineCABundleConfigMapsOptimistically(destinationConfigMap *corev1.Config } else { cm = destinationConfigMap.DeepCopy() } + certificates := []*x509.Certificate{} for _, input := range inputConfigMaps { inputConfigMap, err := lister.ConfigMaps(input.Namespace).Get(input.Name) @@ -121,6 +123,20 @@ func CombineCABundleConfigMapsOptimistically(destinationConfigMap *corev1.Config return nil, false, err } + // Set NotBefore/NotAfter annotations, marking the time range when the certificates are valid + oldestNotAfter := time.Time{} + youngestNotBefore := time.Time{} + for _, cert := range finalCertificates { + if cert.NotAfter.After(oldestNotAfter) { + oldestNotAfter = cert.NotAfter + } + if cert.NotBefore.Before(youngestNotBefore) || youngestNotBefore.IsZero() { + youngestNotBefore = cert.NotBefore + } + } + additionalAnnotations.NotAfter = oldestNotAfter.Format(time.RFC3339) + additionalAnnotations.NotBefore = youngestNotBefore.Format(time.RFC3339) + modified := additionalAnnotations.EnsureTLSMetadataUpdate(&cm.ObjectMeta) newCMData := map[string]string{ "ca-bundle.crt": string(caBytes), From f835f28b2d0b6888abde2a44a8f96ea7a6bf95bd Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Wed, 24 Sep 2025 18:32:00 +0200 Subject: [PATCH 3/4] Support RefreshOnlyWhenExpired mode in ManageCSRCABundle --- pkg/cmd/recoverycontroller/csrcontroller.go | 2 +- .../targetconfigcontroller/targetconfigcontroller.go | 6 +++--- .../targetconfigcontroller/targetconfigcontroller_test.go | 2 +- .../resourcesynccontroller/resourcesync_controller.go | 2 ++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/recoverycontroller/csrcontroller.go b/pkg/cmd/recoverycontroller/csrcontroller.go index 9abf8c3b9..8372443a8 100644 --- a/pkg/cmd/recoverycontroller/csrcontroller.go +++ b/pkg/cmd/recoverycontroller/csrcontroller.go @@ -172,7 +172,7 @@ func (c *CSRController) sync(ctx context.Context) error { klog.Info("Refreshed CSRIntermediateCABundle.") } - _, changed, err = targetconfigcontroller.ManageCSRCABundle(ctx, c.configMapLister, c.kubeClient.CoreV1(), c.eventRecorder) + _, changed, err = targetconfigcontroller.ManageCSRCABundle(ctx, c.configMapLister, c.kubeClient.CoreV1(), c.eventRecorder, true) if err != nil { return err } diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go index 918efd00b..aba617576 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go @@ -222,7 +222,7 @@ func createTargetConfigController(ctx context.Context, syncCtx factory.SyncConte if err != nil { errors = append(errors, fmt.Errorf("%q: %v", "configmap/csr-intermediate-ca", err)) } - _, _, err = ManageCSRCABundle(ctx, c.configMapLister, c.kubeClient.CoreV1(), syncCtx.Recorder()) + _, _, err = ManageCSRCABundle(ctx, c.configMapLister, c.kubeClient.CoreV1(), syncCtx.Recorder(), false) if err != nil { errors = append(errors, fmt.Errorf("%q: %v", "configmap/csr-controller-ca", err)) } @@ -744,7 +744,7 @@ func manageServiceAccountCABundle(ctx context.Context, lister corev1listers.Conf return caBundleConfigMap, false, nil } -func ManageCSRCABundle(ctx context.Context, lister corev1listers.ConfigMapLister, client corev1client.ConfigMapsGetter, recorder events.Recorder) (*corev1.ConfigMap, bool, error) { +func ManageCSRCABundle(ctx context.Context, lister corev1listers.ConfigMapLister, client corev1client.ConfigMapsGetter, recorder events.Recorder, refreshOnlyWhenExpired bool) (*corev1.ConfigMap, bool, error) { additionalAnnotations := certrotation.AdditionalAnnotations{ JiraComponent: "kube-controller-manager", Description: "CA to recognize the CSRs (both serving and client) signed by the kube-controller-manager.", @@ -788,7 +788,7 @@ func ManageCSRCABundle(ctx context.Context, lister corev1listers.ConfigMapLister } klog.V(2).Infof("Created CSR CA bundle configmap %s/%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name) return caBundleConfigMap, true, nil - } else if updateRequired { + } else if updateRequired && !refreshOnlyWhenExpired { caBundleConfigMap, err = client.ConfigMaps(operatorclient.OperatorNamespace).Update(ctx, requiredConfigMap, metav1.UpdateOptions{}) resourcehelper.ReportUpdateEvent(recorder, caBundleConfigMap, err) if err != nil { diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go index 3715cdd93..10d23d3f0 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go @@ -1049,7 +1049,7 @@ func TestManageCSRCABundle(t *testing.T) { recorder := events.NewInMemoryRecorder("test", clock.RealClock{}) // Call the function under test - resultConfigMap, changed, err := ManageCSRCABundle(context.Background(), lister, client.CoreV1(), recorder) + resultConfigMap, changed, err := ManageCSRCABundle(context.Background(), lister, client.CoreV1(), recorder, false) // Assert error expectations require.NoError(t, err) diff --git a/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go b/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go index 315985e1b..f689f0fa2 100644 --- a/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go +++ b/vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/resourcesync_controller.go @@ -10,6 +10,8 @@ import ( "sync" "time" + "k8s.io/klog/v2" + applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" From e5cb74557e07fd40cc8c8753cb1a769adcde62ed Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Thu, 25 Sep 2025 08:30:31 +0200 Subject: [PATCH 4/4] DEBUG Update csr-controller-ca's component to trigger events bug --- pkg/operator/targetconfigcontroller/targetconfigcontroller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go index aba617576..5848a14c8 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go @@ -746,7 +746,7 @@ func manageServiceAccountCABundle(ctx context.Context, lister corev1listers.Conf func ManageCSRCABundle(ctx context.Context, lister corev1listers.ConfigMapLister, client corev1client.ConfigMapsGetter, recorder events.Recorder, refreshOnlyWhenExpired bool) (*corev1.ConfigMap, bool, error) { additionalAnnotations := certrotation.AdditionalAnnotations{ - JiraComponent: "kube-controller-manager", + JiraComponent: "kube-controller-manager-foobar", Description: "CA to recognize the CSRs (both serving and client) signed by the kube-controller-manager.", } caBundleConfigMapName := "csr-controller-ca"