mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-05 19:47:37 +09:00
refactor(kube-ovn): reload webhook cert via a plain GetCertificate closure
Collapse the certReloader (mtime tracking, serialized refresh, retry backoff,
rate-limited logging, last-good-certificate fallback) into a GetCertificate
callback that simply re-reads the key pair from disk on every handshake. The
signature is unchanged, so main.go is untouched.
The extra machinery guarded scenarios this deployment cannot produce:
- Torn reads are unreachable: the cert/key files come from a plain Secret
volume with no subPath, and Kubernetes' atomic writer swaps the whole ..data
directory via a single symlink flip, so a reader always sees a complete old
or complete new generation of both files.
- The per-handshake LoadX509KeyPair cost is negligible next to the asymmetric
crypto the handshake already performs, so no cache is needed.
- The webhook configures no mTLS, so GetCertificate not refreshing client CA
pools does not apply.
If the mounted files genuinely become unreadable (an operator error, not a
renewal) the handshake now fails loudly instead of silently serving a stale
certificate. The core regression test is retained and remains non-vacuous: it
fails against a static tls.Config.Certificates and passes with the reloading
callback.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
(cherry picked from commit 8baa449f53)
This commit is contained in:
IvanHunters authored and github-actions[bot] committed
1 parent
60b42e219d
commit
d2dc85f712
2 files changed
+36
-252
No files matched your search
@@ -1,155 +1,45 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"crypto/tls"
|
||||
"fmt"
|
||||
"log"
|
||||
"os"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
import "crypto/tls"
|
||||
|
||||
// certReloader keeps the webhook serving certificate in sync with the key pair
|
||||
// files written by cert-manager.
|
||||
// newReloadingTLSConfig returns a tls.Config that reloads the certificate and
|
||||
// key from disk on every TLS handshake, so a cert-manager renewal of the mounted
|
||||
// Secret is picked up by the running process without a restart.
|
||||
//
|
||||
// The certificate is served through tls.Config.GetCertificate, so a cert-manager
|
||||
// renewal of the backing Secret is picked up by the running process without a
|
||||
// restart. Without this, the key pair is read exactly once at startup and cached
|
||||
// for the lifetime of the process: after the certificate expires (roughly one year
|
||||
// after install) the pod keeps presenting the expired certificate, the
|
||||
// kube-apiserver's TLS call to the webhook fails, and because the
|
||||
// MutatingWebhookConfiguration uses failurePolicy: Fail, every pod creation in
|
||||
// tenant namespaces is rejected.
|
||||
type certReloader struct {
|
||||
certFile string
|
||||
keyFile string
|
||||
|
||||
// retryInterval bounds how often a failed reload (or a repeated stat failure) is
|
||||
// retried, so a persistently broken file does not read/parse/log on every handshake.
|
||||
retryInterval time.Duration
|
||||
|
||||
mu sync.RWMutex
|
||||
cert *tls.Certificate
|
||||
loadedModTime time.Time
|
||||
reloading bool
|
||||
nextRetry time.Time
|
||||
nextLog time.Time
|
||||
}
|
||||
|
||||
const defaultReloadRetryInterval = 30 * time.Second
|
||||
|
||||
// newCertReloader loads the initial key pair and returns a reloader for it.
|
||||
func newCertReloader(certFile, keyFile string) (*certReloader, error) {
|
||||
cr := &certReloader{
|
||||
certFile: certFile,
|
||||
keyFile: keyFile,
|
||||
retryInterval: defaultReloadRetryInterval,
|
||||
}
|
||||
modTime, err := fileModTime(certFile)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := cr.load(modTime); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return cr, nil
|
||||
}
|
||||
|
||||
// fileModTime returns the modification time of the given file.
|
||||
func fileModTime(name string) (time.Time, error) {
|
||||
fi, err := os.Stat(name)
|
||||
if err != nil {
|
||||
return time.Time{}, err
|
||||
}
|
||||
return fi.ModTime(), nil
|
||||
}
|
||||
|
||||
// load reads the key pair and, on success, caches it tagged with modTime. On failure
|
||||
// the cached certificate and its recorded mtime are left unchanged, so a transient
|
||||
// failure on a valid file is retried rather than the renewal being abandoned.
|
||||
func (cr *certReloader) load(modTime time.Time) error {
|
||||
cert, err := tls.LoadX509KeyPair(cr.certFile, cr.keyFile)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
cr.mu.Lock()
|
||||
cr.cert = &cert
|
||||
cr.loadedModTime = modTime
|
||||
cr.mu.Unlock()
|
||||
return nil
|
||||
}
|
||||
|
||||
// refresh reloads the key pair when the certificate file's modification time changes.
|
||||
// Without this, the key pair is read exactly once at startup and cached for the
|
||||
// lifetime of the process: after the certificate expires (roughly one year after
|
||||
// install) the pod keeps presenting the expired certificate, the kube-apiserver's
|
||||
// TLS call to the webhook fails, and because the MutatingWebhookConfiguration uses
|
||||
// failurePolicy: Fail, every pod creation in tenant namespaces is rejected.
|
||||
//
|
||||
// A failed reload keeps the last good certificate and does NOT advance the recorded
|
||||
// mtime, so a transient error (fd exhaustion, a torn read) is retried, at most once per
|
||||
// retryInterval, until it succeeds: a transient failure must never permanently pin an
|
||||
// expiring certificate. A single reload is kept in flight so a burst of handshakes does
|
||||
// not trigger a thundering herd of reads.
|
||||
func (cr *certReloader) refresh() {
|
||||
modTime, err := fileModTime(cr.certFile)
|
||||
if err != nil {
|
||||
cr.logRateLimited("certificate stat failed, keeping previous certificate: %v", err)
|
||||
return
|
||||
}
|
||||
|
||||
cr.mu.Lock()
|
||||
if modTime.Equal(cr.loadedModTime) || cr.reloading || time.Now().Before(cr.nextRetry) {
|
||||
cr.mu.Unlock()
|
||||
return
|
||||
}
|
||||
cr.reloading = true
|
||||
cr.mu.Unlock()
|
||||
|
||||
loadErr := cr.load(modTime)
|
||||
|
||||
cr.mu.Lock()
|
||||
cr.reloading = false
|
||||
if loadErr == nil {
|
||||
cr.nextRetry = time.Time{}
|
||||
} else {
|
||||
cr.nextRetry = time.Now().Add(cr.retryInterval)
|
||||
}
|
||||
cr.mu.Unlock()
|
||||
|
||||
if loadErr != nil {
|
||||
cr.logRateLimited("certificate reload failed, keeping previous certificate: %v", loadErr)
|
||||
}
|
||||
}
|
||||
|
||||
// logRateLimited emits a log line at most once per retryInterval, so a persistently
|
||||
// broken mount does not spam the log on every TLS handshake.
|
||||
func (cr *certReloader) logRateLimited(format string, args ...any) {
|
||||
cr.mu.Lock()
|
||||
if time.Now().Before(cr.nextLog) {
|
||||
cr.mu.Unlock()
|
||||
return
|
||||
}
|
||||
cr.nextLog = time.Now().Add(cr.retryInterval)
|
||||
cr.mu.Unlock()
|
||||
log.Printf(format, args...)
|
||||
}
|
||||
|
||||
// GetCertificate is a tls.Config.GetCertificate callback. It refreshes the key pair
|
||||
// when the certificate file changes and keeps serving the last good certificate if a
|
||||
// reload fails.
|
||||
func (cr *certReloader) GetCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) {
|
||||
cr.refresh()
|
||||
|
||||
cr.mu.RLock()
|
||||
defer cr.mu.RUnlock()
|
||||
if cr.cert == nil {
|
||||
return nil, fmt.Errorf("no certificate loaded")
|
||||
}
|
||||
return cr.cert, nil
|
||||
}
|
||||
|
||||
// newReloadingTLSConfig builds a tls.Config that serves the key pair via a
|
||||
// certReloader, so cert-manager renewals are honoured without a restart.
|
||||
// tls.Config.GetCertificate is invoked on every handshake whenever Certificates is
|
||||
// left unset, so re-reading the files there is enough; no watcher, mtime tracking
|
||||
// or cache is needed:
|
||||
// - The cert/key files are mounted from a plain Secret volume with no subPath.
|
||||
// Kubernetes' atomic writer swaps the whole ..data directory via a single
|
||||
// symlink flip, so a reader always sees a complete old or complete new
|
||||
// generation of both files, never a torn mid-write mixture. A per-handshake
|
||||
// LoadX509KeyPair therefore cannot observe a partial renewal.
|
||||
// - The per-handshake cost (a few KB of file I/O plus a PEM/key parse) is
|
||||
// negligible next to the asymmetric crypto the handshake already performs.
|
||||
// - The webhook configures no mTLS (no ClientCAs), so GetCertificate's
|
||||
// limitation of not refreshing client CA pools does not apply.
|
||||
//
|
||||
// If the mounted files genuinely become unreadable (Secret deleted, permissions
|
||||
// broken: an operator error, not a normal renewal) the handshake fails loudly
|
||||
// rather than silently serving a stale certificate.
|
||||
func newReloadingTLSConfig(certFile, keyFile string) (*tls.Config, error) {
|
||||
cr, err := newCertReloader(certFile, keyFile)
|
||||
if err != nil {
|
||||
// Fail fast at startup if the initial key pair is missing or malformed.
|
||||
if _, err := tls.LoadX509KeyPair(certFile, keyFile); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &tls.Config{GetCertificate: cr.GetCertificate}, nil
|
||||
return &tls.Config{
|
||||
GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) {
|
||||
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &cert, nil
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
@@ -71,27 +71,6 @@ func setModTime(t *testing.T, name string, mtime time.Time) {
|
||||
}
|
||||
}
|
||||
|
||||
// certSerial returns the serial number of a *tls.Certificate leaf without mutating the
|
||||
// shared value returned by GetCertificate (it parses into a local when Leaf is unset).
|
||||
func certSerial(t *testing.T, c *tls.Certificate) int64 {
|
||||
t.Helper()
|
||||
if c == nil {
|
||||
t.Fatal("nil certificate")
|
||||
}
|
||||
leaf := c.Leaf
|
||||
if leaf == nil {
|
||||
if len(c.Certificate) == 0 {
|
||||
t.Fatal("certificate has no DER data")
|
||||
}
|
||||
parsed, err := x509.ParseCertificate(c.Certificate[0])
|
||||
if err != nil {
|
||||
t.Fatalf("parse served cert: %v", err)
|
||||
}
|
||||
leaf = parsed
|
||||
}
|
||||
return leaf.SerialNumber.Int64()
|
||||
}
|
||||
|
||||
// servedSerial completes a TLS handshake against addr and returns the serial number
|
||||
// of the leaf certificate the server actually presented.
|
||||
func servedSerial(t *testing.T, addr string) int64 {
|
||||
@@ -158,88 +137,3 @@ func TestReloadingTLSConfigServesRenewedCertificate(t *testing.T) {
|
||||
t.Fatalf("after renewal: expected renewed serial 2, got %d (certificate was not reloaded)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCertReloaderKeepsLastGoodCertificate verifies that a failed reload (e.g. a torn
|
||||
// read while the volume is updated) does not take the webhook down: it keeps serving
|
||||
// the previously loaded certificate instead of erroring the handshake.
|
||||
func TestCertReloaderKeepsLastGoodCertificate(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
certFile := filepath.Join(dir, "tls.crt")
|
||||
keyFile := filepath.Join(dir, "tls.key")
|
||||
|
||||
certA, keyA := genSelfSigned(t, 1)
|
||||
writeKeyPair(t, certFile, keyFile, certA, keyA, time.Now().Add(-2*time.Second))
|
||||
|
||||
cr, err := newCertReloader(certFile, keyFile)
|
||||
if err != nil {
|
||||
t.Fatalf("newCertReloader: %v", err)
|
||||
}
|
||||
|
||||
// Corrupt the cert file and advance its mtime so a reload is attempted and fails.
|
||||
if err := os.WriteFile(certFile, []byte("not a certificate"), 0o600); err != nil {
|
||||
t.Fatalf("corrupt cert: %v", err)
|
||||
}
|
||||
setModTime(t, certFile, time.Now().Add(2*time.Second))
|
||||
|
||||
got, err := cr.GetCertificate(&tls.ClientHelloInfo{})
|
||||
if err != nil {
|
||||
t.Fatalf("GetCertificate returned error instead of serving last good cert: %v", err)
|
||||
}
|
||||
if certSerial(t, got) != 1 {
|
||||
t.Fatalf("expected to keep serving serial 1 after failed reload, got %d", certSerial(t, got))
|
||||
}
|
||||
}
|
||||
|
||||
// TestCertReloaderRetriesAfterTransientFailure is the regression test for the reloader
|
||||
// abandoning a renewal on a transient error. A failed reload must NOT advance the
|
||||
// recorded mtime, so once the (unchanged-mtime) file becomes readable the renewed
|
||||
// certificate is still picked up. Against the code that advanced the mtime on failure,
|
||||
// the renewal is permanently missed and this test fails.
|
||||
func TestCertReloaderRetriesAfterTransientFailure(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
certFile := filepath.Join(dir, "tls.crt")
|
||||
keyFile := filepath.Join(dir, "tls.key")
|
||||
|
||||
certA, keyA := genSelfSigned(t, 1)
|
||||
writeKeyPair(t, certFile, keyFile, certA, keyA, time.Now().Add(-2*time.Second))
|
||||
|
||||
cr, err := newCertReloader(certFile, keyFile)
|
||||
if err != nil {
|
||||
t.Fatalf("newCertReloader: %v", err)
|
||||
}
|
||||
cr.retryInterval = 0 // retry immediately, no backoff wait in the test
|
||||
|
||||
// Renewal advances the mtime, but the first load fails (simulating a transient,
|
||||
// content-independent error) by writing an unreadable cert at the new mtime.
|
||||
renewMod := time.Now().Add(2 * time.Second)
|
||||
if err := os.WriteFile(certFile, []byte("transiently unreadable"), 0o600); err != nil {
|
||||
t.Fatalf("write bad cert: %v", err)
|
||||
}
|
||||
setModTime(t, certFile, renewMod)
|
||||
|
||||
if got, gerr := cr.GetCertificate(&tls.ClientHelloInfo{}); gerr != nil {
|
||||
t.Fatalf("GetCertificate errored during transient failure: %v", gerr)
|
||||
} else if certSerial(t, got) != 1 {
|
||||
t.Fatalf("during transient failure: expected to keep serial 1, got %d", certSerial(t, got))
|
||||
}
|
||||
|
||||
// The renewed, valid certificate is now readable at the SAME mtime as the failed
|
||||
// attempt. The reloader must still pick it up on retry.
|
||||
certB, keyB := genSelfSigned(t, 2)
|
||||
if err := os.WriteFile(certFile, certB, 0o600); err != nil {
|
||||
t.Fatalf("write renewed cert: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(keyFile, keyB, 0o600); err != nil {
|
||||
t.Fatalf("write renewed key: %v", err)
|
||||
}
|
||||
setModTime(t, certFile, renewMod)
|
||||
setModTime(t, keyFile, renewMod)
|
||||
|
||||
got, gerr := cr.GetCertificate(&tls.ClientHelloInfo{})
|
||||
if gerr != nil {
|
||||
t.Fatalf("GetCertificate errored after retry: %v", gerr)
|
||||
}
|
||||
if certSerial(t, got) != 2 {
|
||||
t.Fatalf("after transient failure: expected renewed serial 2 on retry, got %d (mtime advanced on failure, renewal permanently missed)", certSerial(t, got))
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user