From 43e8d474b32d4aca3f42494625ac09c534880111 Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Tue, 27 Jan 2026 15:53:13 +0100 Subject: [PATCH] use renew trigger window instead of static renew time Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- internal/certificate/renew.go | 28 ++++++-- internal/certificate/renew_test.go | 84 ++++++++++++++++++++++ pkg/authority/ca_secret_controller.go | 8 ++- pkg/authority/ca_secret_controller_test.go | 10 +-- pkg/authority/leaf_cert_controller.go | 3 +- 5 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 internal/certificate/renew_test.go diff --git a/internal/certificate/renew.go b/internal/certificate/renew.go index 4cbeeca..2fe933b 100644 --- a/internal/certificate/renew.go +++ b/internal/certificate/renew.go @@ -18,12 +18,32 @@ package certificate import ( "crypto/x509" + "math/rand/v2" "time" ) -// RenewAfter returns the duration until the certificate should be renewed. -func RenewAfter(cert *x509.Certificate) time.Duration { +type TriggerWindow struct { + Start time.Time + End time.Time +} + +func (tw TriggerWindow) duration() time.Duration { + return tw.End.Sub(tw.Start) +} + +// Random returns a random time within the TriggerWindow. +// This value lies within [Start, End). +func (tw TriggerWindow) Random() time.Time { + randomRenewalPoint := time.Duration(float64(tw.duration()) * rand.Float64()) // #nosec G404 + return tw.Start.Add(randomRenewalPoint) +} + +// RenewTriggerWindow returns the period during which a certificate renewal should +// be scheduled (between 6/10 and 7/10 of its lifetime). +func RenewTriggerWindow(cert *x509.Certificate) TriggerWindow { lifetime := cert.NotAfter.Sub(cert.NotBefore) - renewTime := cert.NotBefore.Add(lifetime * 2 / 3) - return time.Until(renewTime) + return TriggerWindow{ + Start: cert.NotBefore.Add(lifetime * 6 / 10), + End: cert.NotBefore.Add(lifetime * 7 / 10), + } } diff --git a/internal/certificate/renew_test.go b/internal/certificate/renew_test.go new file mode 100644 index 0000000..6e7451d --- /dev/null +++ b/internal/certificate/renew_test.go @@ -0,0 +1,84 @@ +/* +Copyright 2025 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package certificate + +import ( + "crypto/x509" + "testing" + "time" +) + +func TestRenewTriggerWindow_TableDriven(t *testing.T) { + cases := []struct { + name string + notBefore time.Time + lifetime time.Duration + }{ + { + name: "bounds", + notBefore: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + lifetime: 100 * time.Hour, + }, + { + name: "random_within_bounds", + notBefore: time.Now().UTC().Truncate(time.Second), + lifetime: 10 * time.Hour, + }, + { + name: "very_short_lifetime", + notBefore: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + lifetime: 1 * time.Nanosecond, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cert := &x509.Certificate{ + NotBefore: c.notBefore, + NotAfter: c.notBefore.Add(c.lifetime), + } + tw := RenewTriggerWindow(cert) + + wantStart := c.notBefore.Add(c.lifetime * 6 / 10) + wantEnd := c.notBefore.Add(c.lifetime * 7 / 10) + + if !tw.Start.Equal(wantStart) { + t.Fatalf("%s: Start = %v, want %v", c.name, tw.Start, wantStart) + } + if !tw.End.Equal(wantEnd) { + t.Fatalf("%s: End = %v, want %v", c.name, tw.End, wantEnd) + } + + // If the window collapsed to a point, Start == End and Random() should equal Start. + if tw.Start.Equal(tw.End) { + r := tw.Random() + if !r.Equal(tw.Start) { + t.Fatalf("%s: Random returned %v, want %v", c.name, r, tw.Start) + } + return + } + + // Otherwise ensure Random returns values within [Start, End). + for range 200 { + r := tw.Random() + if r.Before(tw.Start) || !r.Before(tw.End) { + t.Fatalf("%s: Random returned %v outside window [%v, %v)", c.name, r, tw.Start, tw.End) + } + } + }) + } +} diff --git a/pkg/authority/ca_secret_controller.go b/pkg/authority/ca_secret_controller.go index 8f1d55c..09f15bb 100644 --- a/pkg/authority/ca_secret_controller.go +++ b/pkg/authority/ca_secret_controller.go @@ -21,6 +21,7 @@ import ( "crypto" "crypto/tls" "crypto/x509" + "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -74,7 +75,10 @@ func (r *CASecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } caCert, err := r.reconcileSecret(ctx, secret) - return ctrl.Result{RequeueAfter: certificate.RenewAfter(caCert)}, err + if err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: time.Until(certificate.RenewTriggerWindow(caCert).Random())}, nil } func (r *CASecretReconciler) reconcileSecret(ctx context.Context, secret *corev1.Secret) (caCert *x509.Certificate, err error) { @@ -168,7 +172,7 @@ func caRequiresRegeneration(s *corev1.Secret) (bool, string) { if !x509Cert.IsCA { return true, "Stored certificate is not marked as a CA." } - if certificate.RenewAfter(x509Cert) < 0 { + if !time.Now().Before(certificate.RenewTriggerWindow(x509Cert).Start) { return true, "CA certificate is nearing expiry." } diff --git a/pkg/authority/ca_secret_controller_test.go b/pkg/authority/ca_secret_controller_test.go index 0bdb311..73ac861 100644 --- a/pkg/authority/ca_secret_controller_test.go +++ b/pkg/authority/ca_secret_controller_test.go @@ -124,8 +124,9 @@ func Test__caRequiresRegeneration(t *testing.T) { secret: &corev1.Secret{ Data: generateSecretData( func(cert *x509.Certificate) { - cert.NotBefore = time.Now().Add(-2*time.Hour - 1*time.Minute) - cert.NotAfter = cert.NotBefore.Add(3 * time.Hour) + lifetime := 10 * time.Hour + cert.NotBefore = time.Now().Add(-6*lifetime/10 - 1*time.Minute) + cert.NotAfter = cert.NotBefore.Add(lifetime) }, ), }, @@ -137,8 +138,9 @@ func Test__caRequiresRegeneration(t *testing.T) { secret: &corev1.Secret{ Data: generateSecretData( func(cert *x509.Certificate) { - cert.NotBefore = time.Now().Add(-2*time.Hour + 1*time.Minute) - cert.NotAfter = cert.NotBefore.Add(3 * time.Hour) + lifetime := 10 * time.Hour + cert.NotBefore = time.Now().Add(-6*lifetime/10 + 1*time.Minute) + cert.NotAfter = cert.NotBefore.Add(lifetime) }, ), }, diff --git a/pkg/authority/leaf_cert_controller.go b/pkg/authority/leaf_cert_controller.go index ff8218d..a2b42dc 100644 --- a/pkg/authority/leaf_cert_controller.go +++ b/pkg/authority/leaf_cert_controller.go @@ -20,6 +20,7 @@ import ( "context" "crypto" "crypto/x509" + "time" corev1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" @@ -64,7 +65,7 @@ func (r *LeafCertReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } r.CertificateHolder.SetCertificate(&tlsCert) - return ctrl.Result{RequeueAfter: certificate.RenewAfter(cert)}, nil + return ctrl.Result{RequeueAfter: time.Until(certificate.RenewTriggerWindow(cert).Random())}, nil } func (r *LeafCertReconciler) generateCertificate(caSecret *corev1.Secret) (cert *x509.Certificate, pk crypto.Signer, err error) {