From 1532fca2017157ed082b107576cdc603d6869166 Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Wed, 14 Jan 2026 15:54:34 +0100 Subject: [PATCH 1/2] OCPBUGS-63502: Redeploy console pods upon cert rotation --- pkg/console/operator/operator.go | 2 +- pkg/console/operator/sync_v400.go | 9 +++ .../subresource/deployment/deployment.go | 6 ++ .../subresource/deployment/deployment_test.go | 70 ++++++++++++------- 4 files changed, 60 insertions(+), 27 deletions(-) diff --git a/pkg/console/operator/operator.go b/pkg/console/operator/operator.go index b981c3811..d9d2ffd2b 100644 --- a/pkg/console/operator/operator.go +++ b/pkg/console/operator/operator.go @@ -247,7 +247,7 @@ func NewConsoleOperator( factory.NamesFilter(api.OAuthClientName), oauthClientSwitchedInformer.Informer(), ).WithFilteredEventsInformers( - util.IncludeNamesFilter(deployment.ConsoleOauthConfigName), + util.IncludeNamesFilter(deployment.ConsoleOauthConfigName, api.ConsoleServingCertName), secretsInformer.Informer(), ).WithFilteredEventsInformers( util.IncludeNamesFilter(telemetry.TelemetryConfigMapName), diff --git a/pkg/console/operator/sync_v400.go b/pkg/console/operator/sync_v400.go index beddeaef4..be03e77c3 100644 --- a/pkg/console/operator/sync_v400.go +++ b/pkg/console/operator/sync_v400.go @@ -176,6 +176,12 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact return statusHandler.FlushAndReturn(secErr) } + consoleServingCertSecret, servingCertErr := co.secretsLister.Secrets(api.TargetNamespace).Get(api.ConsoleServingCertName) + statusHandler.AddConditions(status.HandleProgressingOrDegraded("ConsoleServingCertSecretGet", "FailedGet", servingCertErr)) + if servingCertErr != nil { + return statusHandler.FlushAndReturn(servingCertErr) + } + actualDeployment, depChanged, depErrReason, depErr := co.SyncDeployment( ctx, set.Operator, @@ -186,6 +192,7 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact trustedCAConfigMap, clientSecret, sessionSecret, + consoleServingCertSecret, set.Proxy, set.Infrastructure, controllerContext.Recorder(), @@ -289,6 +296,7 @@ func (co *consoleOperator) SyncDeployment( trustedCAConfigMap *corev1.ConfigMap, sec *corev1.Secret, sessionSecret *corev1.Secret, + consoleServingCertSecret *corev1.Secret, proxyConfig *configv1.Proxy, infrastructureConfig *configv1.Infrastructure, recorder events.Recorder, @@ -303,6 +311,7 @@ func (co *consoleOperator) SyncDeployment( trustedCAConfigMap, sec, sessionSecret, + consoleServingCertSecret, proxyConfig, infrastructureConfig, ) diff --git a/pkg/console/subresource/deployment/deployment.go b/pkg/console/subresource/deployment/deployment.go index 542df12a4..8c485ae8b 100644 --- a/pkg/console/subresource/deployment/deployment.go +++ b/pkg/console/subresource/deployment/deployment.go @@ -39,6 +39,7 @@ const ( authnConfigVersionAnnotation = "console.openshift.io/authentication-config-version" authnCATrustConfigMapResourceVersionAnnotation = "console.openshift.io/authn-ca-trust-config-version" sessionSecretRVAnnotation = "console.openshift.io/session-secret-version" + servingCertSecretResourceVersionAnnotation = "console.openshift.io/serving-cert-secret-version" ) var ( @@ -51,6 +52,7 @@ var ( trustedCAConfigMapResourceVersionAnnotation, secretResourceVersionAnnotation, consoleImageAnnotation, + servingCertSecretResourceVersionAnnotation, } ) @@ -73,6 +75,7 @@ func DefaultDeployment( trustedCAConfigMap *corev1.ConfigMap, oAuthClientSecret *corev1.Secret, sessionSecret *corev1.Secret, + consoleServingCertSecret *corev1.Secret, proxyConfig *configv1.Proxy, infrastructureConfig *configv1.Infrastructure, ) *appsv1.Deployment { @@ -93,6 +96,7 @@ func DefaultDeployment( trustedCAConfigMap, oAuthClientSecret, sessionSecret, + consoleServingCertSecret, proxyConfig, infrastructureConfig, ) @@ -200,6 +204,7 @@ func withConsoleAnnotations( trustedCAConfigMap *corev1.ConfigMap, oAuthClientSecret *corev1.Secret, sessionSecret *corev1.Secret, + consoleServingCertSecret *corev1.Secret, proxyConfig *configv1.Proxy, infrastructureConfig *configv1.Infrastructure, ) { @@ -211,6 +216,7 @@ func withConsoleAnnotations( infrastructureConfigResourceVersionAnnotation: infrastructureConfig.GetResourceVersion(), secretResourceVersionAnnotation: oAuthClientSecret.GetResourceVersion(), consoleImageAnnotation: util.GetImageEnv("CONSOLE_IMAGE"), + servingCertSecretResourceVersionAnnotation: consoleServingCertSecret.GetResourceVersion(), } if authServerCAConfigMap != nil { diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index 7a97bda9a..37fa2e201 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -46,6 +46,7 @@ func TestDefaultDeployment(t *testing.T) { trustedCAConfigMap *corev1.ConfigMap oAuthClientSecret *corev1.Secret sessionSecret *corev1.Secret + consoleServingCertSecret *corev1.Secret proxyConfig *configv1.Proxy infrastructureConfig *configv1.Infrastructure } @@ -82,6 +83,7 @@ func TestDefaultDeployment(t *testing.T) { proxyConfigResourceVersionAnnotation: "", infrastructureConfigResourceVersionAnnotation: "", consoleImageAnnotation: "", + servingCertSecretResourceVersionAnnotation: "", }, OwnerReferences: []metav1.OwnerReference{{ APIVersion: "operator.openshift.io/v1", @@ -136,6 +138,7 @@ func TestDefaultDeployment(t *testing.T) { proxyConfigResourceVersionAnnotation: "", infrastructureConfigResourceVersionAnnotation: "", consoleImageAnnotation: "", + servingCertSecretResourceVersionAnnotation: "", workloadManagementAnnotation: workloadManagementAnnotationValue, requiredSCCAnnotation: "restricted-v2", } @@ -213,8 +216,9 @@ func TestDefaultDeployment(t *testing.T) { StringData: nil, Type: "", }, - proxyConfig: proxyConfig, - infrastructureConfig: infrastructureConfigHighlyAvailable, + consoleServingCertSecret: &corev1.Secret{}, + proxyConfig: proxyConfig, + infrastructureConfig: infrastructureConfigHighlyAvailable, }, want: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -292,8 +296,9 @@ func TestDefaultDeployment(t *testing.T) { StringData: nil, Type: "", }, - proxyConfig: proxyConfig, - infrastructureConfig: infrastructureConfigHighlyAvailable, + consoleServingCertSecret: &corev1.Secret{}, + proxyConfig: proxyConfig, + infrastructureConfig: infrastructureConfigHighlyAvailable, }, want: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -370,8 +375,9 @@ func TestDefaultDeployment(t *testing.T) { StringData: nil, Type: "", }, - proxyConfig: proxyConfig, - infrastructureConfig: infrastructureConfigSingleReplica, + consoleServingCertSecret: &corev1.Secret{}, + proxyConfig: proxyConfig, + infrastructureConfig: infrastructureConfigSingleReplica, }, want: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -441,8 +447,9 @@ func TestDefaultDeployment(t *testing.T) { StringData: nil, Type: "", }, - proxyConfig: proxyConfig, - infrastructureConfig: infrastructureConfigExternalTopologyMode, + consoleServingCertSecret: &corev1.Secret{}, + proxyConfig: proxyConfig, + infrastructureConfig: infrastructureConfigExternalTopologyMode, }, want: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -514,6 +521,7 @@ func TestDefaultDeployment(t *testing.T) { tt.args.trustedCAConfigMap, tt.args.oAuthClientSecret, tt.args.sessionSecret, + tt.args.consoleServingCertSecret, tt.args.proxyConfig, tt.args.infrastructureConfig, ), tt.want); diff != nil { @@ -525,16 +533,17 @@ func TestDefaultDeployment(t *testing.T) { func TestWithConsoleAnnotations(t *testing.T) { type args struct { - deployment *appsv1.Deployment - consoleConfigMap *corev1.ConfigMap - serviceCAConfigMap *corev1.ConfigMap - authServerCAConfigMap *corev1.ConfigMap - trustedCAConfigMap *corev1.ConfigMap - oAuthClientSecret *corev1.Secret - sessionSecret *corev1.Secret - proxyConfig *configv1.Proxy - infrastructureConfig *configv1.Infrastructure - authnConfig *configv1.Authentication + deployment *appsv1.Deployment + consoleConfigMap *corev1.ConfigMap + serviceCAConfigMap *corev1.ConfigMap + authServerCAConfigMap *corev1.ConfigMap + trustedCAConfigMap *corev1.ConfigMap + oAuthClientSecret *corev1.Secret + sessionSecret *corev1.Secret + consoleServingCertSecret *corev1.Secret + proxyConfig *configv1.Proxy + infrastructureConfig *configv1.Infrastructure + authnConfig *configv1.Authentication } consoleConfigMap := &corev1.ConfigMap{ @@ -584,6 +593,12 @@ func TestWithConsoleAnnotations(t *testing.T) { }, } + consoleServingCertSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + ResourceVersion: "202020", + }, + } + tests := []struct { name string args args @@ -606,13 +621,14 @@ func TestWithConsoleAnnotations(t *testing.T) { }, }, }, - consoleConfigMap: consoleConfigMap, - serviceCAConfigMap: serviceCAConfigMap, - authServerCAConfigMap: oauthServingCertConfigMap, - trustedCAConfigMap: trustedCAConfigMap, - oAuthClientSecret: oAuthClientSecret, - proxyConfig: proxyConfig, - infrastructureConfig: infrastructureConfig, + consoleConfigMap: consoleConfigMap, + serviceCAConfigMap: serviceCAConfigMap, + authServerCAConfigMap: oauthServingCertConfigMap, + trustedCAConfigMap: trustedCAConfigMap, + oAuthClientSecret: oAuthClientSecret, + consoleServingCertSecret: consoleServingCertSecret, + proxyConfig: proxyConfig, + infrastructureConfig: infrastructureConfig, }, want: &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -625,6 +641,7 @@ func TestWithConsoleAnnotations(t *testing.T) { infrastructureConfigResourceVersionAnnotation: infrastructureConfig.GetResourceVersion(), secretResourceVersionAnnotation: oAuthClientSecret.GetResourceVersion(), consoleImageAnnotation: util.GetImageEnv("CONSOLE_IMAGE"), + servingCertSecretResourceVersionAnnotation: consoleServingCertSecret.GetResourceVersion(), }, }, Spec: appsv1.DeploymentSpec{ @@ -640,6 +657,7 @@ func TestWithConsoleAnnotations(t *testing.T) { infrastructureConfigResourceVersionAnnotation: infrastructureConfig.GetResourceVersion(), secretResourceVersionAnnotation: oAuthClientSecret.GetResourceVersion(), consoleImageAnnotation: util.GetImageEnv("CONSOLE_IMAGE"), + servingCertSecretResourceVersionAnnotation: consoleServingCertSecret.GetResourceVersion(), }, }, }, @@ -649,7 +667,7 @@ func TestWithConsoleAnnotations(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - withConsoleAnnotations(tt.args.deployment, tt.args.consoleConfigMap, tt.args.serviceCAConfigMap, tt.args.authServerCAConfigMap, tt.args.trustedCAConfigMap, tt.args.oAuthClientSecret, tt.args.sessionSecret, tt.args.proxyConfig, tt.args.infrastructureConfig) + withConsoleAnnotations(tt.args.deployment, tt.args.consoleConfigMap, tt.args.serviceCAConfigMap, tt.args.authServerCAConfigMap, tt.args.trustedCAConfigMap, tt.args.oAuthClientSecret, tt.args.sessionSecret, tt.args.consoleServingCertSecret, tt.args.proxyConfig, tt.args.infrastructureConfig) if diff := deep.Equal(tt.args.deployment, tt.want); diff != nil { t.Error(diff) } From 0a408ade45421ec09ed9fbb049cd656d691d953d Mon Sep 17 00:00:00 2001 From: Jakub Hadvig Date: Mon, 18 May 2026 22:30:47 +0200 Subject: [PATCH 2/2] OCPBUGS-84254: unit & e2e test --- .../subresource/deployment/deployment_test.go | 77 +++++++++++++ test/e2e/cert_rotation_test.go | 106 ++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 test/e2e/cert_rotation_test.go diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index 37fa2e201..d936dad57 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -675,6 +675,83 @@ func TestWithConsoleAnnotations(t *testing.T) { } } +func TestServingCertAnnotationChangesOnRotation(t *testing.T) { + consoleConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"}, + } + serviceCAConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "2"}, + } + trustedCAConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "3"}, + } + oAuthClientSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "4"}, + } + proxyConfig := &configv1.Proxy{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "5"}, + } + infrastructureConfig := &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "6"}, + } + + makeDeployment := func() *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}, + }, + }, + } + } + + oldCert := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "111111"}, + } + newCert := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ResourceVersion: "222222"}, + } + + depBefore := makeDeployment() + withConsoleAnnotations(depBefore, consoleConfigMap, serviceCAConfigMap, nil, trustedCAConfigMap, oAuthClientSecret, nil, oldCert, proxyConfig, infrastructureConfig) + + depAfter := makeDeployment() + withConsoleAnnotations(depAfter, consoleConfigMap, serviceCAConfigMap, nil, trustedCAConfigMap, oAuthClientSecret, nil, newCert, proxyConfig, infrastructureConfig) + + oldVal := depBefore.ObjectMeta.Annotations[servingCertSecretResourceVersionAnnotation] + newVal := depAfter.ObjectMeta.Annotations[servingCertSecretResourceVersionAnnotation] + + if oldVal != "111111" { + t.Errorf("expected annotation value '111111' for old cert, got %q", oldVal) + } + if newVal != "222222" { + t.Errorf("expected annotation value '222222' for new cert, got %q", newVal) + } + if oldVal == newVal { + t.Error("annotation value did not change after cert rotation") + } + + oldPodVal := depBefore.Spec.Template.ObjectMeta.Annotations[servingCertSecretResourceVersionAnnotation] + newPodVal := depAfter.Spec.Template.ObjectMeta.Annotations[servingCertSecretResourceVersionAnnotation] + if oldPodVal == newPodVal { + t.Error("pod template annotation value did not change after cert rotation — rollout would not be triggered") + } +} + +func TestServingCertAnnotationInResourceAnnotations(t *testing.T) { + found := false + for _, a := range resourceAnnotations { + if a == servingCertSecretResourceVersionAnnotation { + found = true + break + } + } + if !found { + t.Errorf("servingCertSecretResourceVersionAnnotation (%q) is not in resourceAnnotations — LogDeploymentAnnotationChanges will not detect cert rotation", servingCertSecretResourceVersionAnnotation) + } +} + func TestWithReplicas(t *testing.T) { var ( singleNodeReplicaCount int32 = SingleNodeConsoleReplicas diff --git a/test/e2e/cert_rotation_test.go b/test/e2e/cert_rotation_test.go new file mode 100644 index 000000000..4bb6c44dc --- /dev/null +++ b/test/e2e/cert_rotation_test.go @@ -0,0 +1,106 @@ +package e2e + +import ( + "context" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/openshift/console-operator/pkg/api" + "github.com/openshift/console-operator/test/e2e/framework" +) + +const servingCertAnnotation = "console.openshift.io/serving-cert-secret-version" + +// TestCertRotationTriggersRollout verifies that deleting the console-serving-cert +// secret (simulating a service-CA cert rotation) causes the operator to detect +// the new secret and roll out a new console deployment with the updated +// resource version annotation. +func TestCertRotationTriggersRollout(t *testing.T) { + client, _ := framework.StandardSetup(t) + defer framework.StandardCleanup(t, client) + + // 1. Record the current state of the deployment and secret. + deployment, err := framework.GetConsoleDeployment(client) + if err != nil { + t.Fatalf("failed to get console deployment: %v", err) + } + oldAnnotation := deployment.Spec.Template.ObjectMeta.Annotations[servingCertAnnotation] + oldGeneration := deployment.ObjectMeta.Generation + t.Logf("before rotation: annotation=%q, generation=%d", oldAnnotation, oldGeneration) + + oldSecret, err := client.Core.Secrets(api.TargetNamespace).Get(context.TODO(), api.ConsoleServingCertName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get console-serving-cert secret: %v", err) + } + oldSecretRV := oldSecret.ResourceVersion + t.Logf("before rotation: secret resourceVersion=%q", oldSecretRV) + + // 2. Delete the secret to trigger service-CA to regenerate it with a new resourceVersion. + t.Log("deleting console-serving-cert secret to simulate cert rotation...") + err = client.Core.Secrets(api.TargetNamespace).Delete(context.TODO(), api.ConsoleServingCertName, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("failed to delete console-serving-cert secret: %v", err) + } + + // 3. Wait for the secret to be recreated by service-CA with a new resourceVersion. + t.Log("waiting for service-CA to recreate the secret...") + var newSecretRV string + err = wait.PollImmediate(2*time.Second, framework.AsyncOperationTimeout, func() (bool, error) { + secret, err := client.Core.Secrets(api.TargetNamespace).Get(context.TODO(), api.ConsoleServingCertName, metav1.GetOptions{}) + if err != nil { + return false, nil + } + if secret.ResourceVersion != oldSecretRV { + newSecretRV = secret.ResourceVersion + return true, nil + } + return false, nil + }) + if err != nil { + t.Fatalf("timed out waiting for console-serving-cert secret to be recreated: %v", err) + } + t.Logf("secret recreated with new resourceVersion=%q", newSecretRV) + + // 4. Wait for the operator to reconcile and update the deployment annotation. + t.Log("waiting for operator to update deployment annotation...") + err = wait.PollImmediate(2*time.Second, framework.AsyncOperationTimeout, func() (bool, error) { + dep, err := framework.GetConsoleDeployment(client) + if err != nil { + return false, nil + } + currentAnnotation := dep.Spec.Template.ObjectMeta.Annotations[servingCertAnnotation] + return currentAnnotation != oldAnnotation && currentAnnotation != "", nil + }) + if err != nil { + t.Fatalf("timed out waiting for deployment annotation to update after cert rotation: %v", err) + } + + // 5. Verify the new annotation matches the new secret's resourceVersion. + updatedDeployment, err := framework.GetConsoleDeployment(client) + if err != nil { + t.Fatalf("failed to get updated console deployment: %v", err) + } + newAnnotation := updatedDeployment.Spec.Template.ObjectMeta.Annotations[servingCertAnnotation] + t.Logf("after rotation: annotation=%q, generation=%d", newAnnotation, updatedDeployment.ObjectMeta.Generation) + + if newAnnotation != newSecretRV { + t.Errorf("expected deployment annotation %q to match new secret resourceVersion %q", newAnnotation, newSecretRV) + } + + if updatedDeployment.ObjectMeta.Generation <= oldGeneration { + t.Errorf("expected deployment generation to increase after cert rotation: old=%d, new=%d", oldGeneration, updatedDeployment.ObjectMeta.Generation) + } + + // 6. Wait for operator to settle after the rollout. + t.Log("waiting for operator to reach settled state...") + settled, err := framework.WaitForSettledState(t, client, "cert-rotation") + if err != nil { + t.Fatalf("operator did not settle after cert rotation: %v", err) + } + if !settled { + t.Error("operator did not reach settled state after cert rotation") + } +}