From 18a7e93d52b9935834debd4f4eaae8ec9989f6f3 Mon Sep 17 00:00:00 2001 From: David Simansky Date: Wed, 26 Aug 2026 21:45:39 +0200 Subject: [PATCH 1/3] fix: reload serving certificates at runtime The backend previously loaded its serving certificate only during startup, so a rotated Secret was not reflected until the pod restarted. Reload the mounted pair periodically and retain the last valid pair when an update is incomplete or invalid, allowing new TLS connections to use the rotated certificate without interrupting existing ones. --- backend/main.go | 13 +++-- backend/tlsreload/reloader.go | 54 ++++++++++++++++++++ backend/tlsreload/reloader_test.go | 81 ++++++++++++++++++++++++++++++ backend/tlsreload/suite_test.go | 13 +++++ 4 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 backend/tlsreload/reloader.go create mode 100644 backend/tlsreload/reloader_test.go create mode 100644 backend/tlsreload/suite_test.go diff --git a/backend/main.go b/backend/main.go index 4b95497..05fcfab 100644 --- a/backend/main.go +++ b/backend/main.go @@ -16,6 +16,7 @@ import ( "github.com/openshift/faas-console-plugin/backend/handler" "github.com/openshift/faas-console-plugin/backend/scm" "github.com/openshift/faas-console-plugin/backend/scm/github" + "github.com/openshift/faas-console-plugin/backend/tlsreload" ) const defaultCAPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" @@ -72,6 +73,12 @@ func main() { _, certErr := os.Stat(*certFile) _, keyErr := os.Stat(*keyFile) if certErr == nil && keyErr == nil { + reloader, err := tlsreload.New(*certFile, *keyFile) + if err != nil { + log.Fatalf("Failed to load TLS certificate: %v", err) + } + go reloader.Run() + go func() { ln, err := net.Listen("tcp", fmt.Sprintf(":%d", *httpPort)) if err != nil { @@ -81,16 +88,12 @@ func main() { log.Fatal(http.Serve(ln, muxHandler)) }() - cert, err := tls.LoadX509KeyPair(*certFile, *keyFile) - if err != nil { - log.Fatalf("Failed to load TLS certificate: %v", err) - } ln, err := net.Listen("tcp", fmt.Sprintf(":%d", *httpsPort)) if err != nil { log.Fatal(err) } tlsLn := tls.NewListener(ln, &tls.Config{ - Certificates: []tls.Certificate{cert}, + GetCertificate: reloader.GetCertificate, }) log.Printf("Listening on https://%s", ln.Addr()) log.Fatal(http.Serve(tlsLn, muxHandler)) diff --git a/backend/tlsreload/reloader.go b/backend/tlsreload/reloader.go new file mode 100644 index 0000000..c444e0c --- /dev/null +++ b/backend/tlsreload/reloader.go @@ -0,0 +1,54 @@ +package tlsreload + +import ( + "crypto/tls" + "errors" + "fmt" + "log/slog" + "sync/atomic" + "time" +) + +const certificateReloadInterval = 30 * time.Second + +type Reloader struct { + certFile string + keyFile string + current atomic.Pointer[tls.Certificate] +} + +func New(certFile, keyFile string) (*Reloader, error) { + reloader := &Reloader{certFile: certFile, keyFile: keyFile} + if err := reloader.reload(); err != nil { + return nil, err + } + return reloader, nil +} + +func (r *Reloader) reload() error { + cert, err := tls.LoadX509KeyPair(r.certFile, r.keyFile) + if err != nil { + return fmt.Errorf("load TLS certificate: %w", err) + } + r.current.Store(&cert) + return nil +} + +func (r *Reloader) Run() { + ticker := time.NewTicker(certificateReloadInterval) + defer ticker.Stop() + + for range ticker.C { + if err := r.reload(); err != nil { + slog.Error("failed to reload TLS certificate", "err", err) + } + } +} + +func (r *Reloader) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { + cert := r.current.Load() + if cert == nil { + return nil, errors.New("TLS certificate is not loaded") + } + return cert, nil +} diff --git a/backend/tlsreload/reloader_test.go b/backend/tlsreload/reloader_test.go new file mode 100644 index 0000000..b838baa --- /dev/null +++ b/backend/tlsreload/reloader_test.go @@ -0,0 +1,81 @@ +package tlsreload + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "os" + "path/filepath" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Reloader", func() { + It("uses the certificate loaded after a rotation", func() { + dir := GinkgoT().TempDir() + certFile := filepath.Join(dir, "tls.crt") + keyFile := filepath.Join(dir, "tls.key") + + writeCertificatePair(certFile, keyFile, "old.example.com") + reloader, err := New(certFile, keyFile) + Expect(err).NotTo(HaveOccurred()) + + writeCertificatePair(certFile, keyFile, "new.example.com") + Expect(reloader.reload()).To(Succeed()) + + cert, err := reloader.GetCertificate(nil) + Expect(err).NotTo(HaveOccurred()) + Expect(certificateCommonName(cert)).To(Equal("new.example.com")) + }) + + It("keeps the current certificate when the replacement is invalid", func() { + dir := GinkgoT().TempDir() + certFile := filepath.Join(dir, "tls.crt") + keyFile := filepath.Join(dir, "tls.key") + + writeCertificatePair(certFile, keyFile, "current.example.com") + reloader, err := New(certFile, keyFile) + Expect(err).NotTo(HaveOccurred()) + + Expect(os.WriteFile(certFile, []byte("invalid certificate"), 0600)).To(Succeed()) + Expect(reloader.reload()).To(MatchError(ContainSubstring("load TLS certificate"))) + + cert, err := reloader.GetCertificate(nil) + Expect(err).NotTo(HaveOccurred()) + Expect(certificateCommonName(cert)).To(Equal("current.example.com")) + }) +}) + +func writeCertificatePair(certFile, keyFile, commonName string) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + Expect(err).NotTo(HaveOccurred()) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: commonName}, + DNSNames: []string{commonName}, + NotBefore: time.Now().Add(-time.Minute), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + Expect(err).NotTo(HaveOccurred()) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + Expect(os.WriteFile(certFile, certPEM, 0600)).To(Succeed()) + Expect(os.WriteFile(keyFile, keyPEM, 0600)).To(Succeed()) +} + +func certificateCommonName(cert *tls.Certificate) string { + parsed, err := x509.ParseCertificate(cert.Certificate[0]) + Expect(err).NotTo(HaveOccurred()) + return parsed.Subject.CommonName +} diff --git a/backend/tlsreload/suite_test.go b/backend/tlsreload/suite_test.go new file mode 100644 index 0000000..26cbf3d --- /dev/null +++ b/backend/tlsreload/suite_test.go @@ -0,0 +1,13 @@ +package tlsreload + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestTLSReload(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "TLS Reload Suite") +} From 62c5b59d9b1248242348ea7dbff086f7c504b82c Mon Sep 17 00:00:00 2001 From: David Simansky Date: Tue, 1 Sep 2026 11:55:37 +0200 Subject: [PATCH 2/3] refactor: harden TLS certificate reloader The reloader polled every 30s, re-reading and re-parsing the cert/key on every tick and swapping the cached certificate even when nothing changed. It logged only failures, so a real rotation was invisible, and kept an unreachable nil guard in GetCertificate that New already makes impossible. Poll every 5 minutes and skip the parse and atomic swap when a content hash shows the on-disk pair is unchanged, so steady state costs only two small reads. Log successful loads with the cert expiry so rotations are observable, and drop the dead nil guard. Polling is kept (rather than inotify) because Kubernetes rotates mounted secrets via an atomic ..data symlink swap that file watches miss, and it avoids the k8s.io/apiserver dependency tree. The decision is recorded in docs/ARCHITECTURE.md. Issue SRVOCF-1052 Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/tlsreload/reloader.go | 40 ++++++++++++++++++++++++++++------- docs/ARCHITECTURE.md | 4 ++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/backend/tlsreload/reloader.go b/backend/tlsreload/reloader.go index c444e0c..969d6d5 100644 --- a/backend/tlsreload/reloader.go +++ b/backend/tlsreload/reloader.go @@ -1,20 +1,22 @@ package tlsreload import ( + "crypto/sha256" "crypto/tls" - "errors" "fmt" "log/slog" + "os" "sync/atomic" "time" ) -const certificateReloadInterval = 30 * time.Second +const certificateReloadInterval = 5 * time.Minute type Reloader struct { certFile string keyFile string current atomic.Pointer[tls.Certificate] + lastHash [sha256.Size]byte } func New(certFile, keyFile string) (*Reloader, error) { @@ -26,11 +28,37 @@ func New(certFile, keyFile string) (*Reloader, error) { } func (r *Reloader) reload() error { - cert, err := tls.LoadX509KeyPair(r.certFile, r.keyFile) + certPEM, err := os.ReadFile(r.certFile) + if err != nil { + return fmt.Errorf("read TLS certificate: %w", err) + } + keyPEM, err := os.ReadFile(r.keyFile) + if err != nil { + return fmt.Errorf("read TLS key: %w", err) + } + + // Skip the parse and swap when the on-disk pair is unchanged. + digest := sha256.New() + digest.Write(certPEM) + digest.Write(keyPEM) + var hash [sha256.Size]byte + digest.Sum(hash[:0]) + if r.current.Load() != nil && hash == r.lastHash { + return nil + } + + cert, err := tls.X509KeyPair(certPEM, keyPEM) if err != nil { return fmt.Errorf("load TLS certificate: %w", err) } r.current.Store(&cert) + r.lastHash = hash + + attrs := []any{"certFile", r.certFile} + if cert.Leaf != nil { + attrs = append(attrs, "notAfter", cert.Leaf.NotAfter) + } + slog.Info("loaded TLS certificate", attrs...) return nil } @@ -46,9 +74,5 @@ func (r *Reloader) Run() { } func (r *Reloader) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { - cert := r.current.Load() - if cert == nil { - return nil, errors.New("TLS certificate is not loaded") - } - return cert, nil + return r.current.Load(), nil } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cb1d6d3..12f8992 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -115,6 +115,7 @@ Go + `net/http` standard library. Key dependencies: | `scm` | SCM abstraction types (`Platform`, `Registry`, `Client`) and filesystem helpers | | `scm/github` | go-github implementation of `scm.Client` | | `config` | Package-level wiring vars (`SCMRegistry`, constants) | +| `tlsreload` | Periodic reload of the serving cert/key from disk, swapping an atomic `*tls.Certificate` via `GetCertificate` so rotated certs are served without a restart | ### Dependency Rules @@ -145,6 +146,9 @@ Everything that wraps `knative.dev/func` lives in this one package, so there is **External API URL resolved at Helm install time** The URL embedded in generated kubeconfigs (`externalAPIServerURL`) comes from the Infrastructure CR (`config.openshift.io/v1/Infrastructure/cluster`) via Helm `lookup` at install time, injected as `--external-api-server-url`. It is not fetched at runtime. This eliminates the need for a `ClusterRole` to query the Infrastructure CR from within the pod. +**TLS serving certificate reloaded at runtime by stdlib polling** +The OCP service CA operator rotates the serving cert/key automatically. `tlsreload.Reloader` polls the mounted pair every 5 minutes and atomically swaps the cached `*tls.Certificate` served via `tls.Config.GetCertificate`, so a rotation is picked up without restarting the pod. Stdlib polling (Option B from SRVOCF-1002) was chosen over `k8s.io/apiserver/dynamiccertificates` to avoid pulling in its large transitive dependency tree, and is robust to the atomic `..data` symlink swap Kubernetes uses for mounted secrets (which naive inotify file watches miss). Reloads are content-hashed to skip re-parsing when the pair is unchanged, and the last valid pair is retained when an update is incomplete or invalid. + **SCM is abstracted behind a registry** `scm.Registry` maps `scm.Platform` → `scm.ClientFactory`. The active registry lives at `config.SCMRegistry`, a package-level var that tests swap out via `withSCMMock`. Handlers never reference a concrete SCM client type. The platform is currently resolved statically (`scm.DefaultPlatform = GitHub`), but the registry is designed to support dynamic platform selection — the handler can later derive the platform from the request body or header without changes to the registry or client implementations. From 379b55789b26c9d23efc57199bc0508b74d5d3a3 Mon Sep 17 00:00:00 2001 From: David Simansky Date: Tue, 1 Sep 2026 12:06:20 +0200 Subject: [PATCH 3/3] test: cover TLS reloader content-hash short-circuit Assert that reload() returns the same cached *tls.Certificate pointer when the on-disk cert/key pair is unchanged, guarding the skip-reparse optimization against a future regression. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/tlsreload/reloader_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/backend/tlsreload/reloader_test.go b/backend/tlsreload/reloader_test.go index b838baa..93d1518 100644 --- a/backend/tlsreload/reloader_test.go +++ b/backend/tlsreload/reloader_test.go @@ -50,6 +50,25 @@ var _ = Describe("Reloader", func() { Expect(err).NotTo(HaveOccurred()) Expect(certificateCommonName(cert)).To(Equal("current.example.com")) }) + + It("skips reparsing when the certificate pair is unchanged", func() { + dir := GinkgoT().TempDir() + certFile := filepath.Join(dir, "tls.crt") + keyFile := filepath.Join(dir, "tls.key") + + writeCertificatePair(certFile, keyFile, "stable.example.com") + reloader, err := New(certFile, keyFile) + Expect(err).NotTo(HaveOccurred()) + + before, err := reloader.GetCertificate(nil) + Expect(err).NotTo(HaveOccurred()) + + Expect(reloader.reload()).To(Succeed()) + + after, err := reloader.GetCertificate(nil) + Expect(err).NotTo(HaveOccurred()) + Expect(after).To(BeIdenticalTo(before)) + }) }) func writeCertificatePair(certFile, keyFile, commonName string) {