Add support for CA/certificate rotation

Mounted secrets are automatically updated into pods, but...
* It doesn't work with `subPath` mountings
* When `subPath` is not used, then a bunch of directories are mounted
* And one of those directories is a symlink, so `IsDir()` returns false
* And a watch is needed to notice the change

So, update the certificate volume patch, which requires a change in how
we look for certificates in the CA cert directory.

Add a watch, so when the certs do change, we can restart (because that's
the easiest thing to do).

Also look at validity dates of certificates, and error on expired certs.
And add a restart when those certs *do* expire (hopefully they will be
rotated before).

The default cert-manager certificates have 90 days validities.

Signed-off-by: Todd Short <[email protected]>

Author: tmshort

.github/workflows/e2e.yaml

@@ -59,3 +59,15 @@ jobs:
 
     - name: Run the upgrade e2e test
       run: make test-upgrade-e2e
+
+  cert-e2e:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+
+    - uses: actions/setup-go@v5
+      with:
+        go-version-file: go.mod
+
+    - name: Run the cert e2e test
+      run: make test-cert-e2e

Makefile

@@ -188,6 +188,15 @@ test-upgrade-e2e: export TEST_CLUSTER_CATALOG_NAME := test-catalog
 test-upgrade-e2e: export TEST_CLUSTER_EXTENSION_NAME := test-package
 test-upgrade-e2e: kind-cluster run-latest-release image-registry build-push-e2e-catalog registry-load-bundles pre-upgrade-setup docker-build kind-load kind-deploy post-upgrade-checks kind-clean #HELP Run upgrade e2e tests on a local kind cluster
 
+.PHONY: run-cert-e2e
+run-cert-e2e:
+	./hack/test/cert-e2e.sh
+
+.PHONY: test-cert-e2e
+test-cert-e2e: KIND_CLUSTER_NAME := operator-controller-cert-e2e
+test-cert-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/cert-manager
+test-cert-e2e: run run-cert-e2e kind-clean #HELP Run certificate e2e tests on a local kind cluster
+
 .PHONY: e2e-coverage
 e2e-coverage:
 	COVERAGE_OUTPUT=./coverage/e2e.out ./hack/e2e-coverage.sh

cmd/manager/main.go

@@ -176,11 +176,15 @@ func main() {
 		os.Exit(1)
 	}
 
-	certPool, err := httputil.NewCertPool(caCertDir)
+	certPool, err := httputil.NewCertPool(caCertDir, setupLog, func() {
+		setupLog.WithName("cert-pool").Info("restarting to reload CA certificates")
+		os.Exit(1)
+	})
 	if err != nil {
 		setupLog.Error(err, "unable to create CA certificate pool")
 		os.Exit(1)
 	}
+
 	unpacker := &source.ImageRegistry{
 		BaseCachePath: filepath.Join(cachePath, "unpack"),
 		// TODO: This needs to be derived per extension via ext.Spec.InstallNamespace

config/components/tls/patches/manager_deployment_cert.yaml

@@ -3,7 +3,7 @@
   value: {"name":"olmv1-certificate", "secret":{"secretName":"olmv1-cert", "optional": false, "items": [{"key": "ca.crt", "path": "olm-ca.crt"}]}}
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
-  value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/olm-ca.crt", "subPath":"olm-ca.crt"}
+  value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"}
 - op: add
   path: /spec/template/spec/containers/0/args/-
   value: "--ca-certs-dir=/var/certs"

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/Masterminds/semver/v3 v3.2.1
 	github.com/blang/semver/v4 v4.0.0
 	github.com/containerd/containerd v1.7.19
+	github.com/fsnotify/fsnotify v1.7.0
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-cmp v0.6.0
 	github.com/google/go-containerregistry v0.20.0
@@ -113,7 +114,6 @@ require (
 	github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect
 	github.com/fatih/color v1.15.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
-	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-errors/errors v1.4.2 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
 	github.com/go-git/go-billy/v5 v5.5.0 // indirect

hack/test/cert-e2e.sh

@@ -0,0 +1,25 @@
+#!/bin/bash
+
+set -e
+
+# patch the certificates to renew every minute
+# (if set to 2 minutes, then the big timeout wait is about 5 minutes)
+kubectl patch certificate.cert-manager.io -n cert-manager olmv1-ca --type='merge' -p '{"spec":{"duration":"1h", "renewBefore":"59m"}}'
+kubectl patch certificate.cert-manager.io -n olmv1-system olmv1-cert --type='merge' -p '{"spec":{"duration":"1h", "renewBefore":"59m"}}'
+
+# delete the old secrets, so new secrets are generated
+kubectl delete secret -n cert-manager olmv1-ca
+kubectl delete secret -n olmv1-system olmv1-cert
+
+# delete the existing operator-controller, to force it to get a new secret
+# (deleting the secret itself isn't enough)
+kubectl delete pod -n olmv1-system -l control-plane=controller-manager
+kubectl wait --for=condition=Available --namespace=olmv1-system "deployment/operator-controller-controller-manager" --timeout="60s"
+
+# Now we need to wait for a while, for the pod to restart...
+kubectl wait --for=condition=Available=false --namespace=olmv1-system "deployment/operator-controller-controller-manager" --timeout="10m"
+# ...and wait for it to come back
+kubectl wait --for=condition=Available --namespace=olmv1-system "deployment/operator-controller-controller-manager" --timeout="60s"
+
+# and then search through the previous logs
+kubectl logs -p -c manager -n olmv1-system -l control-plane=controller-manager | tail -1 | grep "restarting to reload CA certificates"

internal/httputil/certutil.go

@@ -8,6 +8,10 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"time"
+
+	"github.com/fsnotify/fsnotify"
+	"github.com/go-logr/logr"
 )
 
 var pemStart = []byte("\n-----BEGIN ")
@@ -157,7 +161,7 @@ func pemDecode(data []byte) (*pem.Block, []byte) {
 }
 
 // This version of (*x509.CertPool).AppendCertsFromPEM() will error out if parsing fails
-func appendCertsFromPEM(s *x509.CertPool, pemCerts []byte) error {
+func appendCertsFromPEM(s *x509.CertPool, pemCerts []byte, firstExpiration *time.Time) error {
 	n := 1
 	for len(pemCerts) > 0 {
 		var block *pem.Block
@@ -179,16 +183,27 @@ func appendCertsFromPEM(s *x509.CertPool, pemCerts []byte) error {
 		if err != nil {
 			return fmt.Errorf("unable to parse cert %d: %w", n, err)
 		}
-		// no return values - panics or always succeeds
-		s.AddCert(cert)
+		if firstExpiration.IsZero() || firstExpiration.After(cert.NotAfter) {
+			*firstExpiration = cert.NotAfter
+		}
+		now := time.Now()
+		if now.Before(cert.NotBefore) {
+			return fmt.Errorf("not yet valid cert %d: %q", n, cert.NotBefore.Format(time.RFC3339))
+		} else if now.After(cert.NotAfter) {
+			return fmt.Errorf("expired cert %d: %q", n, cert.NotAfter.Format(time.RFC3339))
+		} else {
+			// no return values - panics or always succeeds
+			s.AddCert(cert)
+		}
 		n++
 	}
 
 	return nil
 }
 
-func NewCertPool(caDir string) (*x509.CertPool, error) {
+func NewCertPool(caDir string, log logr.Logger, reload func()) (*x509.CertPool, error) {
 	caCertPool, err := x509.SystemCertPool()
+	log = log.WithName("cert-pool")
 	if err != nil {
 		return nil, err
 	}
@@ -200,19 +215,71 @@ func NewCertPool(caDir string) (*x509.CertPool, error) {
 	if err != nil {
 		return nil, err
 	}
+	count := 0
+	firstExpiration := time.Time{}
+
+	if reload != nil {
+		watcher, err := fsnotify.NewWatcher()
+		if err != nil {
+			return nil, err
+		}
+		log.Info("adding watch", "directory", caDir)
+		if err = watcher.Add(caDir); err != nil {
+			return nil, err
+		}
+		go func() {
+			for {
+				select {
+				case <-watcher.Events:
+					watcher.Close()
+					log.Info("directory updated")
+					reload()
+				case <-watcher.Errors:
+					watcher.Close()
+					log.Info("directory error")
+					reload()
+				}
+			}
+		}()
+	}
+
 	for _, e := range dirEntries {
-		if e.IsDir() {
+		file := filepath.Join(caDir, e.Name())
+		// These might be symlinks pointing to directories, so use Stat() to resolve
+		fi, err := os.Stat(file)
+		if err != nil {
+			return nil, err
+		}
+		if fi.IsDir() {
+			log.Info("skip directory", "name", e.Name())
 			continue
 		}
-		file := filepath.Join(caDir, e.Name())
+		log.Info("load certificate", "name", e.Name())
 		data, err := os.ReadFile(file)
 		if err != nil {
 			return nil, fmt.Errorf("error reading cert file %q: %w", file, err)
 		}
-		err = appendCertsFromPEM(caCertPool, data)
+		err = appendCertsFromPEM(caCertPool, data, &firstExpiration)
 		if err != nil {
 			return nil, fmt.Errorf("error adding cert file %q: %w", file, err)
 		}
+		count++
+	}
+
+	// Found no certs!
+	if count == 0 {
+		return nil, fmt.Errorf("no certificates found in %q", caDir)
+	}
+
+	if reload != nil {
+		now := time.Now()
+		log.Info("first expiration", "time", firstExpiration.Format(time.RFC3339))
+		if now.Before(firstExpiration) {
+			time.AfterFunc(firstExpiration.Sub(now), func() {
+				log.Info("certificate expiration")
+				reload()
+			})
+		}
 	}
 
 	return caCertPool, nil

internal/httputil/certutil_test.go

@@ -1,8 +1,10 @@
 package httputil_test
 
 import (
+	"context"
 	"testing"
 
+	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/require"
 
 	"github.com/operator-framework/operator-controller/internal/httputil"
@@ -21,17 +23,20 @@ func TestNewCertPool(t *testing.T) {
 		dir string
 		msg string
 	}{
+		{"../../testdata/certs/", `no certificates found in "../../testdata/certs/"`},
 		{"../../testdata/certs/good", ""},
 		{"../../testdata/certs/bad", `error adding cert file "../../testdata/certs/bad/Amazon_Root_CA_2.pem": unable to PEM decode cert 1`},
 		{"../../testdata/certs/ugly", `error adding cert file "../../testdata/certs/ugly/Amazon_Root_CA.pem": unable to PEM decode cert 2`},
 		{"../../testdata/certs/ugly2", `error adding cert file "../../testdata/certs/ugly2/Amazon_Root_CA_1.pem": unable to PEM decode cert 1`},
 		{"../../testdata/certs/ugly3", `error adding cert file "../../testdata/certs/ugly3/not_a_cert.pem": unable to PEM decode cert 1`},
 		{"../../testdata/certs/empty", `error adding cert file "../../testdata/certs/empty/empty.pem": unable to parse cert 1: x509: malformed certificate`},
+		{"../../testdata/certs/expired", `error adding cert file "../../testdata/certs/expired/expired.pem": expired cert 1: "2024-01-02T15:00:00Z"`},
 	}
 
+	log, _ := logr.FromContext(context.Background())
 	for _, caDir := range caDirs {
 		t.Logf("Loading certs from %q", caDir.dir)
-		pool, err := httputil.NewCertPool(caDir.dir)
+		pool, err := httputil.NewCertPool(caDir.dir, log, nil)
 		if caDir.msg == "" {
 			require.NoError(t, err)
 			require.NotNil(t, pool)

testdata/certs/expired/expired.pem

@@ -0,0 +1,31 @@
+-----BEGIN CERTIFICATE-----
+MIIFXzCCA0egAwIBAgIUN5r8l1RrpH53+9e6pfj6CXoyqP0wDQYJKoZIhvcNAQEL
+BQAwPzELMAkGA1UEBhMCVVMxEDAOBgNVBAoMB1JlZCBIYXQxDDAKBgNVBAsMA09M
+TTEQMA4GA1UEAwwHZXhwaXJlZDAeFw0yNDAxMDExNTAwMDBaFw0yNDAxMDIxNTAw
+MDBaMD8xCzAJBgNVBAYTAlVTMRAwDgYDVQQKDAdSZWQgSGF0MQwwCgYDVQQLDANP
+TE0xEDAOBgNVBAMMB2V4cGlyZWQwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIK
+AoICAQDFalyjEXz0cMGs3pt360Cz0uD0CDnnAqQFHxXPchfCMZnW/VRGrJQq29rZ
+UgU5PnxPgqadrw20BodfR2RS9xIacMP+092GY7Ep96xWokwXcsGPj2e5VMlEYVM1
+0MGqIbEv52ZnoEaZDHl4yprYeTs+b/7NGvdG1+N/YNAjkpk8cCBKUXo4ZhkgAZoW
+jbv3DkAdkpQHipUYkQZNRws1ebyfTbKaEPxw7abEh9TJrHD1EI9hbmYOGJWLfe1e
+zeBQjFioQA31FcQR3/v+aNEDX390+qi3p0LXe7GMabgcoFYcGXO7XvX0DdUBvdZZ
+dyHA7cJvyfWfcbucI7xQ9xvAnu/4Ih4D8mHnJXjZK5ReQn06FPM/ZCgZ5LrHAKcZ
+0mrOts/8noY9dMmBreSJmLCP8EqzY7yKJFFHVCeKo+bU6/KOyNhJGGSCHVJ/pZGK
+ZpOQcNwVvHciLH+MfpW12xJXPEs8Wv24KufDdBCDliSFnVTYH3kZaq4Ozb7+3A5j
+wUQ2aDg8nrq4oNORMSCafvia8MYH3NXbpUq1SAyD5DTKtMcWY3gcVnJgrBai1hPn
+TPhrMb2NMDFnMnj7/l8jdu9xHrsgOmOrv7Zj0ytmpT6ITJgWNGXsiq7Dp+HH1c6N
+ggG6g0zqoyoaxcPVN7PMrWTvfKUD3LHfIsesPc4+lT+TSlBQYwIDAQABo1MwUTAd
+BgNVHQ4EFgQU8mBHR/00anEl8Io/A2c0LQlGF5MwHwYDVR0jBBgwFoAU8mBHR/00
+anEl8Io/A2c0LQlGF5MwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC
+AgEABZYGEeJY2dgyi4W0LNVgN8mKuZuapIcisQ66foe46WuWGAjVONIHlb0Ciy75
+aaClLC8fiiIh+FUFZ5aIZkfhKH97QvehFO5O7mqCjM7ipvtEm+Vs1IVtXWDONUxo
+SfgbjEPBV8+eflgvKQ6jJqiSqs8EnqdbGAfhxVG/3RN1b5xSFtKz6kzHQE+Gy6QT
+DGCVhYvDq8j6G2LCePsqE8piOnSaXuRwD4/YEOaYhx4jjgOnaM0m/dM/Cx9wy2xg
+LMRBjBwxFf6palgiFUvyqvturIPONQICkM/lZkpmHbeM4FCat/CD5VW+JgpYiEtW
+2oFslTEbawUjmEYnzdo9iw9KPLJQqtasFEWzkWWnJrfm7AVGxcgAHVqGZhUMgq0k
+MccM2zYZN2fCSZUUueDB7VCxFq5jK2oLzE14ngXdR7ZbxT3qai/zvGg1kl9y1bIF
+WVTK0WZnHqZwVnHQVBH0Duv0uyRUzb6yRRziuLN5aBGQpy/Jm7MS0jLidCbqoCXC
+dYqGMFlImzU+6CwPyTJo+X+v6L+FATIxZRpBBeEhHqEU6wz51ms68Sjx4bpW33b+
+WFt0JKEmIxB1puJK1qQvKu/MxJyy52GNqiRg7HXkJH9MMYWoAkF2jKMLFoerUPun
+7GaV8SIUTFO/5pbnpxZ97a2FuB2RvDKs7GSdspEmC3wbAPU=
+-----END CERTIFICATE-----