From cbe15edcf7a4fdb73bc132562deed841519cca50 Mon Sep 17 00:00:00 2001 From: Yamunadevi Shanmugam Date: Thu, 21 May 2026 08:45:44 +0530 Subject: [PATCH] fix(pki): requeue certificate revocation to prevent sync timeout Update logic of CertificateRevocationController to request a requeue when encountering stale total-client-ca bundle cache. --- .../certificaterevocationcontroller.go | 4 +- .../certificaterevocationcontroller_test.go | 93 +++++++++++++++++-- 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller.go b/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller.go index 7ebe9cd1ae65..c1c093dd22a1 100644 --- a/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller.go +++ b/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller.go @@ -733,7 +733,7 @@ func (c *CertificateRevocationController) ensureNewSignerCertificatePropagated(c // that, though, and it's always valid to first check that our certificates have propagated as far // as we can tell in the system before asking the KAS, since that's expensive if len(trustedCertificates(totalClientTrustBundle, []*certificateSecret{{cert: signers[0]}}, now)) == 0 { - return true, nil, false, nil + return true, nil, true, nil } // if the updated trust bundle has propagated as far as we can tell, let's go ahead and ask @@ -1033,7 +1033,7 @@ func (c *CertificateRevocationController) ensureOldSignerCertificateRevoked(ctx // that, though, and it's always valid to first check that our certificates have propagated as far // as we can tell in the system before asking the KAS, since that's expensive if len(trustedCertificates(totalClientTrustBundle, []*certificateSecret{{cert: oldCerts[0]}}, now)) != 0 { - return true, nil, false, nil + return true, nil, true, nil } // if the updated trust bundle has propagated as far as we can tell, let's go ahead and ask diff --git a/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller_test.go b/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller_test.go index d8c7b96f010b..ae4c47c70867 100644 --- a/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller_test.go +++ b/control-plane-pki-operator/certificaterevocationcontroller/certificaterevocationcontroller_test.go @@ -463,10 +463,11 @@ func TestCertificateRevocationController_processCertificateRevocationRequest(t * }, }, { - name: "not yet propagated, nothing to do", - now: postRevocationClock.Now, - crrNamespace: "crr-ns", - crrName: "crr-name", + name: "not yet propagated, nothing to do", + now: postRevocationClock.Now, + crrNamespace: "crr-ns", + crrName: "crr-name", + expectedRequeue: true, // New cert not yet in total bundle, requeue to wait for TargetConfigController crr: &certificatesv1alpha1.CertificateRevocationRequest{ ObjectMeta: metav1.ObjectMeta{Namespace: "crr-ns", Name: "crr-name"}, Spec: certificatesv1alpha1.CertificateRevocationRequestSpec{SignerClass: string(certificates.CustomerBreakGlassSigner)}, @@ -847,10 +848,11 @@ func TestCertificateRevocationController_processCertificateRevocationRequest(t * }, }, { - name: "validating, previous still valid", - now: postRevocationClock.Now, - crrNamespace: "crr-ns", - crrName: "crr-name", + name: "validating, previous still valid", + now: postRevocationClock.Now, + crrNamespace: "crr-ns", + crrName: "crr-name", + expectedRequeue: true, // Old cert still in total bundle, requeue to wait for TargetConfigController crr: &certificatesv1alpha1.CertificateRevocationRequest{ ObjectMeta: metav1.ObjectMeta{Namespace: "crr-ns", Name: "crr-name"}, Spec: certificatesv1alpha1.CertificateRevocationRequestSpec{SignerClass: string(certificates.CustomerBreakGlassSigner)}, @@ -1033,6 +1035,81 @@ func TestCertificateRevocationController_processCertificateRevocationRequest(t * }, }, }, + { + name: "SRE signer: validating, previous still valid (requeue path)", + now: postRevocationClock.Now, + crrNamespace: "crr-ns", + crrName: "crr-name-sre", + expectedRequeue: true, // Old cert still in total bundle for SRE signer, must requeue + crr: &certificatesv1alpha1.CertificateRevocationRequest{ + ObjectMeta: metav1.ObjectMeta{Namespace: "crr-ns", Name: "crr-name-sre"}, + Spec: certificatesv1alpha1.CertificateRevocationRequestSpec{SignerClass: string(certificates.SREBreakGlassSigner)}, + Status: certificatesv1alpha1.CertificateRevocationRequestStatus{ + RevocationTimestamp: ptr.To(metav1.NewTime(revocationClock.Now())), + PreviousSigner: &corev1.LocalObjectReference{Name: "1pfcydcz358pa1glirkmc72sdkf5zw21uam4jbnj03pw"}, + Conditions: []metav1.Condition{{ + Type: certificatesv1alpha1.LeafCertificatesRegeneratedType, + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(postRevocationClock.Now()), + Reason: hypershiftv1beta1.AsExpectedReason, + Message: `All leaf certificates are re-generated.`, + }, { + Type: certificatesv1alpha1.RootCertificatesRegeneratedType, + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(postRevocationClock.Now()), + Reason: hypershiftv1beta1.AsExpectedReason, + Message: `Signer certificate crr-ns/sre-system-admin-signer regenerated.`, + }, { + Type: certificatesv1alpha1.NewCertificatesTrustedType, + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.NewTime(postRevocationClock.Now()), + Reason: hypershiftv1beta1.AsExpectedReason, + Message: `New signer certificate crr-ns/sre-system-admin-signer trusted.`, + }}, + }, + }, + secrets: []*corev1.Secret{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "crr-ns", + Name: manifests.SRESystemAdminSigner("").Name, + Annotations: map[string]string{certrotation.CertificateIssuer: "crr-ns_sre-break-glass-signer@1234"}, + }, + Data: map[string][]byte{ + corev1.TLSCertKey: data.future.raw.signerCert, + corev1.TLSPrivateKeyKey: data.future.raw.signerKey, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "crr-ns", + Name: "1pfcydcz358pa1glirkmc72sdkf5zw21uam4jbnj03pw", + }, + Data: map[string][]byte{ + corev1.TLSCertKey: data.original.raw.signerCert, + corev1.TLSPrivateKeyKey: data.original.raw.signerKey, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "crr-ns", + Name: manifests.SRESystemAdminClientCertSecret("").Name, + Annotations: map[string]string{certrotation.CertificateIssuer: "crr-ns_sre-break-glass-signer@1234"}, + }, + Data: map[string][]byte{ + corev1.TLSCertKey: data.future.raw.signedCert, + corev1.TLSPrivateKeyKey: data.future.raw.clientKey, + }, + }}, + cms: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Namespace: "crr-ns", Name: manifests.SRESystemAdminSignerCA("").Name}, + Data: map[string]string{ + "ca-bundle.crt": string(data.future.raw.signerCert), + }, + }, { + ObjectMeta: metav1.ObjectMeta{Namespace: "crr-ns", Name: manifests.TotalKASClientCABundle("").Name}, + Data: map[string]string{ + "ca-bundle.crt": string(data.original.raw.signerCert) + string(data.future.raw.signerCert), + }, + }}, + }, } { t.Run(testCase.name, func(t *testing.T) { c := &CertificateRevocationController{