From 1e5f504753f328388171aaeac824f5f8ab9cfe82 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Thu, 25 Sep 2025 11:05:44 -0400 Subject: [PATCH 1/4] OCPBUGS-58145: reload serving cert on rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Compute TLS secret hash (tls.crt, tls.key, optional ca.crt) • Annotate pod template with canary-serving-cert hash • Watch canary serving cert Secret to trigger reconcile • Emit event when cert hash changes (traceability) • Update desiredCanaryDaemonSet(canaryImage, certHash) • Reconcile logic compares/propagates hash annotation • Add unit tests for hash compute and DaemonSet change detection Signed-off-by: Brett Tofel --- pkg/operator/controller/canary/controller.go | 15 +++- pkg/operator/controller/canary/daemonset.go | 74 ++++++++++++++++++- .../controller/canary/daemonset_test.go | 14 +++- pkg/operator/controller/canary/secret_hash.go | 47 ++++++++++++ .../controller/canary/secret_hash_test.go | 66 +++++++++++++++++ 5 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 pkg/operator/controller/canary/secret_hash.go create mode 100644 pkg/operator/controller/canary/secret_hash_test.go diff --git a/pkg/operator/controller/canary/controller.go b/pkg/operator/controller/canary/controller.go index a9096f2e7f..6eae0f8ac4 100644 --- a/pkg/operator/controller/canary/controller.go +++ b/pkg/operator/controller/canary/controller.go @@ -35,6 +35,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + + "k8s.io/client-go/tools/record" ) const ( @@ -80,6 +82,7 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) { reconciler := &reconciler{ config: config, client: mgr.GetClient(), + recorder: mgr.GetEventRecorderFor(canaryControllerName), enableCanaryRouteRotation: false, } c, err := controller.New(canaryControllerName, mgr, controller.Options{Reconciler: reconciler}) @@ -161,6 +164,15 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) { return nil, err } + // Watch the canary serving cert secret and enqueue the default ingress controller so + // that changes to the serving cert cause the canary daemonset to be reconciled. + canarySecretPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool { + return o.GetNamespace() == operatorcontroller.DefaultCanaryNamespace && o.GetName() == "canary-serving-cert" + }) + if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.Secret{}, enqueueRequestForDefaultIngressController(config.Namespace), canarySecretPredicate)); err != nil { + return nil, err + } + return c, nil } @@ -252,7 +264,8 @@ type Config struct { type reconciler struct { config Config - client client.Client + client client.Client + recorder record.EventRecorder // Use a mutex so enableCanaryRotation is // go-routine safe. diff --git a/pkg/operator/controller/canary/daemonset.go b/pkg/operator/controller/canary/daemonset.go index ea477b4b13..c75e4e5676 100644 --- a/pkg/operator/controller/canary/daemonset.go +++ b/pkg/operator/controller/canary/daemonset.go @@ -13,11 +13,31 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" ) // ensureCanaryDaemonSet ensures the canary daemonset exists func (r *reconciler) ensureCanaryDaemonSet() (bool, *appsv1.DaemonSet, error) { - desired := desiredCanaryDaemonSet(r.config.CanaryImage) + // Attempt to read the canary serving cert secret and compute a content hash. + // If the secret is missing or incomplete, proceed without the annotation but + // surface a log entry so operators can investigate. + var certHash string + secret := &corev1.Secret{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: controller.DefaultCanaryNamespace, Name: "canary-serving-cert"}, secret); err != nil { + if errors.IsNotFound(err) { + log.Info("canary serving cert secret not found; skipping canary-serving-cert-hash annotation") + } else { + return false, nil, fmt.Errorf("failed to get canary serving cert secret: %v", err) + } + } else { + if h, err := ComputeTLSSecretHash(secret); err != nil { + log.Info("canary serving cert secret is incomplete; skipping canary-serving-cert-hash annotation", "error", err) + } else { + certHash = h + } + } + + desired := desiredCanaryDaemonSet(r.config.CanaryImage, certHash) haveDs, current, err := r.currentCanaryDaemonSet() if err != nil { return false, nil, err @@ -70,17 +90,40 @@ func (r *reconciler) updateCanaryDaemonSet(current, desired *appsv1.DaemonSet) ( return false, nil } + // Capture annotation change for events. + var oldHash, newHash string + if current.Spec.Template.Annotations != nil { + oldHash = current.Spec.Template.Annotations[CanaryServingCertHashAnnotation] + } + if updated.Spec.Template.Annotations != nil { + newHash = updated.Spec.Template.Annotations[CanaryServingCertHashAnnotation] + } + diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { return false, fmt.Errorf("failed to update canary daemonset %s/%s: %v", updated.Namespace, updated.Name, err) } + log.Info("updated canary daemonset", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) + + // If the only meaningful change (or one of the changes) was the canary cert + // annotation, emit an event for traceability. + if newHash != "" && newHash != oldHash { + short := newHash + if len(short) > 8 { + short = short[:8] + } + if r.recorder != nil { + r.recorder.Eventf(updated, "Normal", "CanaryCertRotated", "Canary serving cert rotated, updated pod template annotation hash: %s", short) + } + } + return true, nil } // desiredCanaryDaemonSet returns the desired canary daemonset read in // from manifests -func desiredCanaryDaemonSet(canaryImage string) *appsv1.DaemonSet { +func desiredCanaryDaemonSet(canaryImage string, certHash string) *appsv1.DaemonSet { daemonset := manifests.CanaryDaemonSet() name := controller.CanaryDaemonSetName() daemonset.Name = name.Name @@ -97,6 +140,13 @@ func desiredCanaryDaemonSet(canaryImage string) *appsv1.DaemonSet { daemonset.Spec.Template.Spec.Containers[0].Image = canaryImage daemonset.Spec.Template.Spec.Containers[0].Command = []string{"ingress-operator", CanaryHealthcheckCommand} + if certHash != "" { + if daemonset.Spec.Template.Annotations == nil { + daemonset.Spec.Template.Annotations = map[string]string{} + } + daemonset.Spec.Template.Annotations[CanaryServingCertHashAnnotation] = certHash + } + return daemonset } @@ -163,6 +213,26 @@ func canaryDaemonSetChanged(current, expected *appsv1.DaemonSet) (bool, *appsv1. changed = true } + // Update when the canary-serving-cert hash annotation changes on the pod template. + var currentHash, expectedHash string + if current.Spec.Template.Annotations != nil { + currentHash = current.Spec.Template.Annotations[CanaryServingCertHashAnnotation] + } + if expected.Spec.Template.Annotations != nil { + expectedHash = expected.Spec.Template.Annotations[CanaryServingCertHashAnnotation] + } + if currentHash != expectedHash { + if updated.Spec.Template.Annotations == nil { + updated.Spec.Template.Annotations = map[string]string{} + } + if expectedHash == "" { + delete(updated.Spec.Template.Annotations, CanaryServingCertHashAnnotation) + } else { + updated.Spec.Template.Annotations[CanaryServingCertHashAnnotation] = expectedHash + } + changed = true + } + if !changed { return false, nil } diff --git a/pkg/operator/controller/canary/daemonset_test.go b/pkg/operator/controller/canary/daemonset_test.go index d29bc5afca..9c73599fff 100644 --- a/pkg/operator/controller/canary/daemonset_test.go +++ b/pkg/operator/controller/canary/daemonset_test.go @@ -16,7 +16,7 @@ import ( func Test_desiredCanaryDaemonSet(t *testing.T) { // canaryImageName is the ingress-operator image canaryImageName := "openshift/origin-cluster-ingress-operator:latest" - daemonset := desiredCanaryDaemonSet(canaryImageName) + daemonset := desiredCanaryDaemonSet(canaryImageName, "") expectedDaemonSetName := controller.CanaryDaemonSetName() @@ -225,11 +225,21 @@ func Test_canaryDaemonsetChanged(t *testing.T) { }, expect: true, }, + { + description: "if canary serving cert annotation changes", + mutate: func(ds *appsv1.DaemonSet) { + if ds.Spec.Template.Annotations == nil { + ds.Spec.Template.Annotations = map[string]string{} + } + ds.Spec.Template.Annotations[CanaryServingCertHashAnnotation] = "d34db33f" + }, + expect: true, + }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - original := desiredCanaryDaemonSet("") + original := desiredCanaryDaemonSet("", "") mutated := original.DeepCopy() tc.mutate(mutated) if changed, updated := canaryDaemonSetChanged(original, mutated); changed != tc.expect { diff --git a/pkg/operator/controller/canary/secret_hash.go b/pkg/operator/controller/canary/secret_hash.go new file mode 100644 index 0000000000..1b336f8c15 --- /dev/null +++ b/pkg/operator/controller/canary/secret_hash.go @@ -0,0 +1,47 @@ +package canary + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + + corev1 "k8s.io/api/core/v1" +) + +const ( + // CanaryServingCertHashAnnotation is the annotation key used on the + // canary DaemonSet PodTemplate to force a rollout when the canary + // serving cert secret changes. + CanaryServingCertHashAnnotation = "ingress.operator.openshift.io/canary-serving-cert-hash" +) + +// ComputeTLSSecretHash computes a stable sha256 hex string over the +// relevant keys in a TLS secret. Required keys are `tls.crt` and +// `tls.key`. `ca.crt` is included if present. +func ComputeTLSSecretHash(secret *corev1.Secret) (string, error) { + if secret == nil { + return "", fmt.Errorf("secret is nil") + } + + if secret.Data == nil { + return "", fmt.Errorf("secret has no data") + } + + crt, okCrt := secret.Data["tls.crt"] + key, okKey := secret.Data["tls.key"] + if !okCrt || !okKey { + return "", fmt.Errorf("secret missing tls.crt or tls.key") + } + + hasher := sha256.New() + hasher.Write(crt) + hasher.Write([]byte("|")) + hasher.Write(key) + if ca, ok := secret.Data["ca.crt"]; ok { + hasher.Write([]byte("|")) + hasher.Write(ca) + } + + sum := hasher.Sum(nil) + return hex.EncodeToString(sum), nil +} diff --git a/pkg/operator/controller/canary/secret_hash_test.go b/pkg/operator/controller/canary/secret_hash_test.go new file mode 100644 index 0000000000..50c563a0f1 --- /dev/null +++ b/pkg/operator/controller/canary/secret_hash_test.go @@ -0,0 +1,66 @@ +package canary + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func Test_ComputeTLSSecretHash(t *testing.T) { + base := &corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("cert-data"), + "tls.key": []byte("key-data"), + }, + } + + same := &corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("cert-data"), + "tls.key": []byte("key-data"), + }, + } + + different := &corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("other-cert"), + "tls.key": []byte("key-data"), + }, + } + + withCA := &corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": []byte("cert-data"), + "tls.key": []byte("key-data"), + "ca.crt": []byte("ca-data"), + }, + } + + h1, err := ComputeTLSSecretHash(base) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + h2, err := ComputeTLSSecretHash(same) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if h1 != h2 { + t.Fatalf("expected same hash for identical secrets: %s != %s", h1, h2) + } + + hd, err := ComputeTLSSecretHash(different) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if h1 == hd { + t.Fatalf("expected different hash for different secret data") + } + + hca, err := ComputeTLSSecretHash(withCA) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if h1 == hca { + t.Fatalf("expected different hash when ca.crt is present") + } +} From 9df597d64968c251a364e63224ebd1a8b2c3f3d1 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Thu, 5 Feb 2026 17:29:11 -0500 Subject: [PATCH 2/4] Address PR review comments: use CanaryCertificateName and propagate context Signed-off-by: Brett Tofel --- pkg/operator/controller/canary/controller.go | 24 ++++++++------- pkg/operator/controller/canary/daemonset.go | 27 ++++++++--------- pkg/operator/controller/canary/namespace.go | 24 +++++++-------- pkg/operator/controller/canary/route.go | 32 ++++++++++---------- pkg/operator/controller/canary/service.go | 22 +++++++------- 5 files changed, 65 insertions(+), 64 deletions(-) diff --git a/pkg/operator/controller/canary/controller.go b/pkg/operator/controller/canary/controller.go index 6eae0f8ac4..9e4708679a 100644 --- a/pkg/operator/controller/canary/controller.go +++ b/pkg/operator/controller/canary/controller.go @@ -167,7 +167,8 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) { // Watch the canary serving cert secret and enqueue the default ingress controller so // that changes to the serving cert cause the canary daemonset to be reconciled. canarySecretPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool { - return o.GetNamespace() == operatorcontroller.DefaultCanaryNamespace && o.GetName() == "canary-serving-cert" + name := operatorcontroller.CanaryCertificateName() + return o.GetNamespace() == name.Namespace && o.GetName() == name.Name }) if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.Secret{}, enqueueRequestForDefaultIngressController(config.Namespace), canarySecretPredicate)); err != nil { return nil, err @@ -195,13 +196,13 @@ func enqueueRequestForDefaultIngressController(namespace string) handler.EventHa func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { result := reconcile.Result{} - if _, _, err := r.ensureCanaryNamespace(); err != nil { + if _, _, err := r.ensureCanaryNamespace(ctx); err != nil { // Return if the canary namespace cannot be created since // resource creation in a namespace that does not exist will fail. return result, fmt.Errorf("failed to ensure canary namespace: %v", err) } - haveDs, daemonset, err := r.ensureCanaryDaemonSet() + haveDs, daemonset, err := r.ensureCanaryDaemonSet(ctx) if err != nil { return result, fmt.Errorf("failed to ensure canary daemonset: %v", err) } else if !haveDs { @@ -217,14 +218,14 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( Controller: &trueVar, } - haveService, service, err := r.ensureCanaryService(daemonsetRef) + haveService, service, err := r.ensureCanaryService(ctx, daemonsetRef) if err != nil { return result, fmt.Errorf("failed to ensure canary service: %v", err) } else if !haveService { return result, fmt.Errorf("failed to get canary service: %v", err) } - haveRoute, _, err := r.ensureCanaryRoute(service) + haveRoute, _, err := r.ensureCanaryRoute(ctx, service) if err != nil { return result, fmt.Errorf("failed to ensure canary route: %v", err) } else if !haveRoute { @@ -234,7 +235,7 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( // Get the canary route rotation annotation value // from the default ingress controller. ic := &operatorv1.IngressController{} - if err := r.client.Get(context.TODO(), request.NamespacedName, ic); err != nil { + if err := r.client.Get(ctx, request.NamespacedName, ic); err != nil { return result, fmt.Errorf("failed to get ingress controller %s: %v", request.NamespacedName.Name, err) } @@ -302,8 +303,9 @@ func (r *reconciler) startCanaryRoutePolling(stop <-chan struct{}) error { // using wait.NonSlidingUntil so that the canary runs every canaryCheckFrequency, regardless of how long the function takes go wait.NonSlidingUntil(func() { + ctx := context.TODO() // Get the current canary route every iteration in case it has been modified - haveRoute, route, err := r.currentCanaryRoute() + haveRoute, route, err := r.currentCanaryRoute(ctx) if err != nil { log.Error(err, "failed to get current canary route for canary check") return @@ -351,7 +353,7 @@ func (r *reconciler) startCanaryRoutePolling(stop <-chan struct{}) error { if rotationEnabled { checkCount++ if checkCount >= canaryCheckCycleCount { - haveService, service, err := r.currentCanaryService() + haveService, service, err := r.currentCanaryService(ctx) if err != nil { log.Error(err, "failed to get canary service") return @@ -359,7 +361,7 @@ func (r *reconciler) startCanaryRoutePolling(stop <-chan struct{}) error { log.Info("canary check service does not exist") return } - route, err = r.rotateRouteEndpoint(service, route) + route, err = r.rotateRouteEndpoint(ctx, service, route) if err != nil { log.Error(err, "failed to rotate canary route endpoint") return @@ -493,13 +495,13 @@ func (r *reconciler) setCanaryStatusCondition(cond operatorv1.OperatorCondition) // Switch the current RoutePort that the route points to. // Use this function to periodically update the canary route endpoint // to verify if the router has wedged. -func (r *reconciler) rotateRouteEndpoint(service *corev1.Service, current *routev1.Route) (*routev1.Route, error) { +func (r *reconciler) rotateRouteEndpoint(ctx context.Context, service *corev1.Service, current *routev1.Route) (*routev1.Route, error) { updated, err := cycleServicePort(service, current) if err != nil { return nil, fmt.Errorf("failed to rotate route port: %v", err) } - if changed, err := r.updateCanaryRoute(current, updated); err != nil { + if changed, err := r.updateCanaryRoute(ctx, current, updated); err != nil { return current, err } else if !changed { return current, fmt.Errorf("expected canary route to be updated: No relevant changes detected") diff --git a/pkg/operator/controller/canary/daemonset.go b/pkg/operator/controller/canary/daemonset.go index c75e4e5676..07174610e1 100644 --- a/pkg/operator/controller/canary/daemonset.go +++ b/pkg/operator/controller/canary/daemonset.go @@ -13,17 +13,16 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" ) // ensureCanaryDaemonSet ensures the canary daemonset exists -func (r *reconciler) ensureCanaryDaemonSet() (bool, *appsv1.DaemonSet, error) { +func (r *reconciler) ensureCanaryDaemonSet(ctx context.Context) (bool, *appsv1.DaemonSet, error) { // Attempt to read the canary serving cert secret and compute a content hash. // If the secret is missing or incomplete, proceed without the annotation but // surface a log entry so operators can investigate. var certHash string secret := &corev1.Secret{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: controller.DefaultCanaryNamespace, Name: "canary-serving-cert"}, secret); err != nil { + if err := r.client.Get(ctx, controller.CanaryCertificateName(), secret); err != nil { if errors.IsNotFound(err) { log.Info("canary serving cert secret not found; skipping canary-serving-cert-hash annotation") } else { @@ -38,22 +37,22 @@ func (r *reconciler) ensureCanaryDaemonSet() (bool, *appsv1.DaemonSet, error) { } desired := desiredCanaryDaemonSet(r.config.CanaryImage, certHash) - haveDs, current, err := r.currentCanaryDaemonSet() + haveDs, current, err := r.currentCanaryDaemonSet(ctx) if err != nil { return false, nil, err } switch { case !haveDs: - if err := r.createCanaryDaemonSet(desired); err != nil { + if err := r.createCanaryDaemonSet(ctx, desired); err != nil { return false, nil, err } - return r.currentCanaryDaemonSet() + return r.currentCanaryDaemonSet(ctx) case haveDs: - if updated, err := r.updateCanaryDaemonSet(current, desired); err != nil { + if updated, err := r.updateCanaryDaemonSet(ctx, current, desired); err != nil { return true, current, err } else if updated { - return r.currentCanaryDaemonSet() + return r.currentCanaryDaemonSet(ctx) } } @@ -61,9 +60,9 @@ func (r *reconciler) ensureCanaryDaemonSet() (bool, *appsv1.DaemonSet, error) { } // currentCanaryDaemonSet returns the current canary daemonset -func (r *reconciler) currentCanaryDaemonSet() (bool, *appsv1.DaemonSet, error) { +func (r *reconciler) currentCanaryDaemonSet(ctx context.Context) (bool, *appsv1.DaemonSet, error) { daemonset := &appsv1.DaemonSet{} - if err := r.client.Get(context.TODO(), controller.CanaryDaemonSetName(), daemonset); err != nil { + if err := r.client.Get(ctx, controller.CanaryDaemonSetName(), daemonset); err != nil { if errors.IsNotFound(err) { return false, nil, nil } @@ -73,8 +72,8 @@ func (r *reconciler) currentCanaryDaemonSet() (bool, *appsv1.DaemonSet, error) { } // createCanaryDaemonSet creates the given daemonset resource -func (r *reconciler) createCanaryDaemonSet(daemonset *appsv1.DaemonSet) error { - if err := r.client.Create(context.TODO(), daemonset); err != nil { +func (r *reconciler) createCanaryDaemonSet(ctx context.Context, daemonset *appsv1.DaemonSet) error { + if err := r.client.Create(ctx, daemonset); err != nil { return fmt.Errorf("failed to create canary daemonset %s/%s: %v", daemonset.Namespace, daemonset.Name, err) } @@ -84,7 +83,7 @@ func (r *reconciler) createCanaryDaemonSet(daemonset *appsv1.DaemonSet) error { // updateCanaryDaemonSet updates the canary daemonset if an appropriate change // has been detected -func (r *reconciler) updateCanaryDaemonSet(current, desired *appsv1.DaemonSet) (bool, error) { +func (r *reconciler) updateCanaryDaemonSet(ctx context.Context, current, desired *appsv1.DaemonSet) (bool, error) { changed, updated := canaryDaemonSetChanged(current, desired) if !changed { return false, nil @@ -100,7 +99,7 @@ func (r *reconciler) updateCanaryDaemonSet(current, desired *appsv1.DaemonSet) ( } diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) - if err := r.client.Update(context.TODO(), updated); err != nil { + if err := r.client.Update(ctx, updated); err != nil { return false, fmt.Errorf("failed to update canary daemonset %s/%s: %v", updated.Namespace, updated.Name, err) } diff --git a/pkg/operator/controller/canary/namespace.go b/pkg/operator/controller/canary/namespace.go index a287ac071e..48ebae1c02 100644 --- a/pkg/operator/controller/canary/namespace.go +++ b/pkg/operator/controller/canary/namespace.go @@ -15,25 +15,25 @@ import ( ) // ensureCanaryNamespace ensures that the ingress-canary namespace exists -func (r *reconciler) ensureCanaryNamespace() (bool, *corev1.Namespace, error) { +func (r *reconciler) ensureCanaryNamespace(ctx context.Context) (bool, *corev1.Namespace, error) { desired := manifests.CanaryNamespace() - haveNamespace, current, err := r.currentCanaryNamespace() + haveNamespace, current, err := r.currentCanaryNamespace(ctx) if err != nil { return false, nil, err } switch { case !haveNamespace: - if err := r.createCanaryNamespace(desired); err != nil { + if err := r.createCanaryNamespace(ctx, desired); err != nil { return false, nil, err } - return r.currentCanaryNamespace() + return r.currentCanaryNamespace(ctx) case haveNamespace: - if updated, err := r.updateCanaryNamespace(current, desired); err != nil { + if updated, err := r.updateCanaryNamespace(ctx, current, desired); err != nil { return true, current, err } else if updated { - return r.currentCanaryNamespace() + return r.currentCanaryNamespace(ctx) } } @@ -41,9 +41,9 @@ func (r *reconciler) ensureCanaryNamespace() (bool, *corev1.Namespace, error) { } // currentCanaryNamespace gets the current canary namespace resource -func (r *reconciler) currentCanaryNamespace() (bool, *corev1.Namespace, error) { +func (r *reconciler) currentCanaryNamespace(ctx context.Context) (bool, *corev1.Namespace, error) { ns := &corev1.Namespace{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: controller.DefaultCanaryNamespace}, ns); err != nil { + if err := r.client.Get(ctx, types.NamespacedName{Name: controller.DefaultCanaryNamespace}, ns); err != nil { if errors.IsNotFound(err) { return false, nil, nil } @@ -53,8 +53,8 @@ func (r *reconciler) currentCanaryNamespace() (bool, *corev1.Namespace, error) { } // createCanaryNamespace creates the given namespace -func (r *reconciler) createCanaryNamespace(ns *corev1.Namespace) error { - if err := r.client.Create(context.TODO(), ns); err != nil { +func (r *reconciler) createCanaryNamespace(ctx context.Context, ns *corev1.Namespace) error { + if err := r.client.Create(ctx, ns); err != nil { return fmt.Errorf("failed to create canary namespace %s: %v", ns.Name, err) } @@ -64,13 +64,13 @@ func (r *reconciler) createCanaryNamespace(ns *corev1.Namespace) error { // updateCanaryNamespace updates the canary namespace if an appropriate change // has been detected -func (r *reconciler) updateCanaryNamespace(current, desired *corev1.Namespace) (bool, error) { +func (r *reconciler) updateCanaryNamespace(ctx context.Context, current, desired *corev1.Namespace) (bool, error) { changed, updated := canaryNamespaceChanged(current, desired) if !changed { return false, nil } - if err := r.client.Update(context.TODO(), updated); err != nil { + if err := r.client.Update(ctx, updated); err != nil { return false, fmt.Errorf("failed to update canary namespace %s: %v", updated.Name, err) } log.Info("updated canary namespace", "namespace", updated.Name) diff --git a/pkg/operator/controller/canary/route.go b/pkg/operator/controller/canary/route.go index e1bf40e99d..4e3c0910bf 100644 --- a/pkg/operator/controller/canary/route.go +++ b/pkg/operator/controller/canary/route.go @@ -19,28 +19,28 @@ import ( ) // ensureCanaryRoute ensures the canary route exists -func (r *reconciler) ensureCanaryRoute(service *corev1.Service) (bool, *routev1.Route, error) { +func (r *reconciler) ensureCanaryRoute(ctx context.Context, service *corev1.Service) (bool, *routev1.Route, error) { desired, err := desiredCanaryRoute(service) if err != nil { return false, nil, fmt.Errorf("failed to build canary route: %v", err) } - haveRoute, current, err := r.currentCanaryRoute() + haveRoute, current, err := r.currentCanaryRoute(ctx) if err != nil { return false, nil, err } switch { case !haveRoute: - if err := r.createCanaryRoute(desired); err != nil { + if err := r.createCanaryRoute(ctx, desired); err != nil { return false, nil, err } - return r.currentCanaryRoute() + return r.currentCanaryRoute(ctx) case haveRoute: - if updated, err := r.updateCanaryRoute(current, desired); err != nil { + if updated, err := r.updateCanaryRoute(ctx, current, desired); err != nil { return true, current, err } else if updated { - return r.currentCanaryRoute() + return r.currentCanaryRoute(ctx) } } @@ -48,9 +48,9 @@ func (r *reconciler) ensureCanaryRoute(service *corev1.Service) (bool, *routev1. } // currentCanaryRoute gets the current canary route resource -func (r *reconciler) currentCanaryRoute() (bool, *routev1.Route, error) { +func (r *reconciler) currentCanaryRoute(ctx context.Context) (bool, *routev1.Route, error) { route := &routev1.Route{} - if err := r.client.Get(context.TODO(), controller.CanaryRouteName(), route); err != nil { + if err := r.client.Get(ctx, controller.CanaryRouteName(), route); err != nil { if errors.IsNotFound(err) { return false, nil, nil } @@ -60,8 +60,8 @@ func (r *reconciler) currentCanaryRoute() (bool, *routev1.Route, error) { } // createCanaryRoute creates the given route -func (r *reconciler) createCanaryRoute(route *routev1.Route) error { - if err := r.client.Create(context.TODO(), route); err != nil { +func (r *reconciler) createCanaryRoute(ctx context.Context, route *routev1.Route) error { + if err := r.client.Create(ctx, route); err != nil { return fmt.Errorf("failed to create canary route %s/%s: %v", route.Namespace, route.Name, err) } @@ -71,7 +71,7 @@ func (r *reconciler) createCanaryRoute(route *routev1.Route) error { // updateCanaryRoute updates the canary route if an appropriate change // has been detected -func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, error) { +func (r *reconciler) updateCanaryRoute(ctx context.Context, current, desired *routev1.Route) (bool, error) { changed, updated := canaryRouteChanged(current, desired) if !changed { return false, nil @@ -83,10 +83,10 @@ func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, e log.Info("deleting and recreating the canary route to clear spec.host", "namespace", current.Namespace, "name", current.Name, "old spec.host", current.Spec.Host) foreground := metav1.DeletePropagationForeground deleteOptions := crclient.DeleteOptions{PropagationPolicy: &foreground} - if _, err := r.deleteCanaryRoute(current, &deleteOptions); err != nil { + if _, err := r.deleteCanaryRoute(ctx, current, &deleteOptions); err != nil { return false, err } - if err := r.createCanaryRoute(desired); err != nil { + if err := r.createCanaryRoute(ctx, desired); err != nil { return false, err } return true, nil @@ -94,7 +94,7 @@ func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, e // Diff before updating because the client may mutate the object. diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) - if err := r.client.Update(context.TODO(), updated); err != nil { + if err := r.client.Update(ctx, updated); err != nil { return false, fmt.Errorf("failed to update canary route %s/%s: %v", updated.Namespace, updated.Name, err) } log.Info("updated canary route", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) @@ -102,9 +102,9 @@ func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, e } // deleteCanaryRoute deletes a given route -func (r *reconciler) deleteCanaryRoute(route *routev1.Route, options *crclient.DeleteOptions) (bool, error) { +func (r *reconciler) deleteCanaryRoute(ctx context.Context, route *routev1.Route, options *crclient.DeleteOptions) (bool, error) { - if err := r.client.Delete(context.TODO(), route, options); err != nil { + if err := r.client.Delete(ctx, route, options); err != nil { return false, fmt.Errorf("failed to delete canary route %s/%s: %v", route.Namespace, route.Name, err) } diff --git a/pkg/operator/controller/canary/service.go b/pkg/operator/controller/canary/service.go index 74d00532e9..2fecf1a1ed 100644 --- a/pkg/operator/controller/canary/service.go +++ b/pkg/operator/controller/canary/service.go @@ -18,21 +18,21 @@ import ( var CanaryPorts = []int32{8443, 8888} // ensureCanaryService ensures the ingress canary service exists. -func (r *reconciler) ensureCanaryService(daemonsetRef metav1.OwnerReference) (bool, *corev1.Service, error) { +func (r *reconciler) ensureCanaryService(ctx context.Context, daemonsetRef metav1.OwnerReference) (bool, *corev1.Service, error) { desired := desiredCanaryService(daemonsetRef) - haveService, current, err := r.currentCanaryService() + haveService, current, err := r.currentCanaryService(ctx) if err != nil { return false, nil, err } switch { case haveService: - if updated, err := r.updateCanaryService(current, desired); err != nil { + if updated, err := r.updateCanaryService(ctx, current, desired); err != nil { return true, current, err } else if updated { - return r.currentCanaryService() + return r.currentCanaryService(ctx) } case !haveService: - if err := r.createCanaryService(desired); err != nil { + if err := r.createCanaryService(ctx, desired); err != nil { return false, nil, err } return true, desired, nil @@ -41,7 +41,7 @@ func (r *reconciler) ensureCanaryService(daemonsetRef metav1.OwnerReference) (bo } // updateCanaryService updates the canary service if an appropriate change is detected. -func (r *reconciler) updateCanaryService(current, desired *corev1.Service) (bool, error) { +func (r *reconciler) updateCanaryService(ctx context.Context, current, desired *corev1.Service) (bool, error) { changed, updated := canaryServiceChanged(current, desired) if !changed { return false, nil @@ -49,7 +49,7 @@ func (r *reconciler) updateCanaryService(current, desired *corev1.Service) (bool // Diff before updating because the client may mutate the object. diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) - if err := r.client.Update(context.TODO(), updated); err != nil { + if err := r.client.Update(ctx, updated); err != nil { return false, fmt.Errorf("failed to update canary service %s/%s: %w", updated.Namespace, updated.Name, err) } log.Info("updated canary service", "namespace", updated.Namespace, "name", updated.Name, "diff", diff) @@ -79,9 +79,9 @@ func canaryServiceChanged(current, desired *corev1.Service) (bool, *corev1.Servi } // currentCanaryService gets the current ingress canary service resource. -func (r *reconciler) currentCanaryService() (bool, *corev1.Service, error) { +func (r *reconciler) currentCanaryService(ctx context.Context) (bool, *corev1.Service, error) { current := &corev1.Service{} - err := r.client.Get(context.TODO(), controller.CanaryServiceName(), current) + err := r.client.Get(ctx, controller.CanaryServiceName(), current) if err != nil { if errors.IsNotFound(err) { return false, nil, nil @@ -92,8 +92,8 @@ func (r *reconciler) currentCanaryService() (bool, *corev1.Service, error) { } // createCanaryService creates the given service resource. -func (r *reconciler) createCanaryService(service *corev1.Service) error { - if err := r.client.Create(context.TODO(), service); err != nil { +func (r *reconciler) createCanaryService(ctx context.Context, service *corev1.Service) error { + if err := r.client.Create(ctx, service); err != nil { return fmt.Errorf("failed to create canary service %s/%s: %w", service.Namespace, service.Name, err) } From ff08e125788a03eda84caf7033fb754234948e4f Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Thu, 5 Feb 2026 17:30:47 -0500 Subject: [PATCH 3/4] Move CanaryServingCertHashAnnotation to daemonset.go Signed-off-by: Brett Tofel --- pkg/operator/controller/canary/daemonset.go | 7 +++++++ pkg/operator/controller/canary/secret_hash.go | 7 +------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/operator/controller/canary/daemonset.go b/pkg/operator/controller/canary/daemonset.go index 07174610e1..a132f4237a 100644 --- a/pkg/operator/controller/canary/daemonset.go +++ b/pkg/operator/controller/canary/daemonset.go @@ -15,6 +15,13 @@ import ( "k8s.io/apimachinery/pkg/api/errors" ) +const ( + // CanaryServingCertHashAnnotation is the annotation key used on the + // canary DaemonSet PodTemplate to force a rollout when the canary + // serving cert secret changes. + CanaryServingCertHashAnnotation = "ingress.operator.openshift.io/canary-serving-cert-hash" +) + // ensureCanaryDaemonSet ensures the canary daemonset exists func (r *reconciler) ensureCanaryDaemonSet(ctx context.Context) (bool, *appsv1.DaemonSet, error) { // Attempt to read the canary serving cert secret and compute a content hash. diff --git a/pkg/operator/controller/canary/secret_hash.go b/pkg/operator/controller/canary/secret_hash.go index 1b336f8c15..3fa3049aef 100644 --- a/pkg/operator/controller/canary/secret_hash.go +++ b/pkg/operator/controller/canary/secret_hash.go @@ -8,12 +8,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -const ( - // CanaryServingCertHashAnnotation is the annotation key used on the - // canary DaemonSet PodTemplate to force a rollout when the canary - // serving cert secret changes. - CanaryServingCertHashAnnotation = "ingress.operator.openshift.io/canary-serving-cert-hash" -) + // ComputeTLSSecretHash computes a stable sha256 hex string over the // relevant keys in a TLS secret. Required keys are `tls.crt` and From 15e4cae5499af26f267ed50315da21bd90876833 Mon Sep 17 00:00:00 2001 From: Brett Tofel Date: Fri, 6 Feb 2026 10:35:49 -0500 Subject: [PATCH 4/4] Fix formatting in secret_hash.go Signed-off-by: Brett Tofel --- pkg/operator/controller/canary/secret_hash.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/operator/controller/canary/secret_hash.go b/pkg/operator/controller/canary/secret_hash.go index 3fa3049aef..0f5d8ad18e 100644 --- a/pkg/operator/controller/canary/secret_hash.go +++ b/pkg/operator/controller/canary/secret_hash.go @@ -8,8 +8,6 @@ import ( corev1 "k8s.io/api/core/v1" ) - - // ComputeTLSSecretHash computes a stable sha256 hex string over the // relevant keys in a TLS secret. Required keys are `tls.crt` and // `tls.key`. `ca.crt` is included if present.