Re-register CRDs when CA changes, and use intermediate issuer

Author: alex-hunt-materialize

misc/helm-charts/operator/README.md

@@ -144,6 +144,9 @@ The following table lists the configurable parameters of the Materialize operato
 | `operator.args.enableInternalStatementLogging` |  | ``true`` |
 | `operator.args.enableLicenseKeyChecks` |  | ``false`` |
 | `operator.args.startupLogFilter` | Log filtering settings for startup logs | ``"INFO,mz_orchestratord=TRACE"`` |
+| `operator.args.webhookCertReloadInterval` | How often orchestratord reloads its webhook TLS certificate from disk and, when the CA changes, refreshes the conversion webhook's CA bundle. Must be shorter than the certificate's lifetime. Accepts a humantime duration (e.g. "1h", "30m"). Leave null to use the binary default. | ``nil`` |
+| `operator.certificate.caDuration` | Lifetime of the root CA that signs the webhook serving certificate, when `source` is "cert-manager". The serving certificate is signed by this CA, so the CA outlives individual serving-certificate rotations. | ``"87600h"`` |
+| `operator.certificate.caRenewBefore` | How long before the root CA expires to renew it. Must be less than `caDuration`. | ``"8760h"`` |
 | `operator.certificate.secretName` | Name of a secret in the operator's namespace containing ca.crt, tls.crt, and tls.key entries. Only used if `source` is "secret". | ``nil`` |
 | `operator.certificate.source` | Where to obtain the certificate for orchestratord. Valid values are 'cert-manager' and 'secret'. | ``"cert-manager"`` |
 | `operator.cloudProvider.providers.aws.accountID` | When using AWS, accountID is required | ``""`` |

misc/helm-charts/operator/templates/certificate.yaml

@@ -8,7 +8,15 @@
 # by the Apache License, Version 2.0.
 
 {{- if eq .Values.operator.certificate.source "cert-manager" -}}
+# We provision the webhook serving certificate from a stable root CA rather
+# than as a bare self-signed certificate. The serving certificate rotates
+# frequently, but it is always signed by the same long-lived CA, so the
+# `ca.crt` that orchestratord registers as the conversion webhook's caBundle
+# stays valid across serving-certificate rotations. orchestratord refreshes the
+# caBundle if the CA itself ever changes, e.g. on the rare root CA renewal, so a
+# routine serving-certificate rotation leaves the webhook undisturbed.
 ---
+# Bootstrap issuer used only to sign the root CA below.
 apiVersion: cert-manager.io/v1
 kind: Issuer
 metadata:
@@ -20,10 +28,49 @@ spec:
   selfSigned: {}
 
 ---
+# Long-lived root CA. Stays stable across serving-certificate rotations so the
+# conversion webhook's caBundle remains valid without changing every rotation.
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
-  name: {{ include "materialize-operator.fullname" . }}-self-signed
+  name: {{ include "materialize-operator.fullname" . }}-ca
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "materialize-operator.labels" . | nindent 4 }}
+spec:
+  isCA: true
+  commonName: {{ include "materialize-operator.fullname" . }}-ca
+  secretName: {{ include "materialize-operator.fullname" . }}-ca
+  duration: {{ .Values.operator.certificate.caDuration }}
+  renewBefore: {{ .Values.operator.certificate.caRenewBefore }}
+  privateKey:
+    algorithm: ECDSA
+    rotationPolicy: Always
+  issuerRef:
+    name: {{ include "materialize-operator.fullname" . }}-self-signed
+    kind: Issuer
+    group: cert-manager.io
+
+---
+# CA issuer that signs the serving certificate using the stable root CA.
+apiVersion: cert-manager.io/v1
+kind: Issuer
+metadata:
+  name: {{ include "materialize-operator.fullname" . }}-ca
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "materialize-operator.labels" . | nindent 4 }}
+spec:
+  ca:
+    secretName: {{ include "materialize-operator.fullname" . }}-ca
+
+---
+# Webhook serving certificate. Rotates frequently; its `ca.crt` is the stable
+# root CA above.
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: {{ include "materialize-operator.fullname" . }}-cert
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "materialize-operator.labels" . | nindent 4 }}
@@ -35,7 +82,7 @@ spec:
     algorithm: ECDSA
     rotationPolicy: Always
   issuerRef:
-    name: {{ include "materialize-operator.fullname" . }}-self-signed
+    name: {{ include "materialize-operator.fullname" . }}-ca
     kind: Issuer
     group: cert-manager.io
 {{- end -}}

misc/helm-charts/operator/templates/deployment.yaml

@@ -66,6 +66,9 @@ spec:
         {{- if not .Values.operator.args.enableLicenseKeyChecks }}
         - "--disable-license-key-checks"
         {{- end }}
+        {{- if .Values.operator.args.webhookCertReloadInterval }}
+        - "--webhook-cert-reload-interval={{ .Values.operator.args.webhookCertReloadInterval }}"
+        {{- end }}
 
         {{/* AWS Configuration */}}
         {{- if eq .Values.operator.cloudProvider.type "aws" }}

misc/helm-charts/operator/values.yaml

@@ -24,6 +24,12 @@ operator:
     enableInternalStatementLogging: true
     # Newer versions ignore this setting and always enforce license key checks.
     enableLicenseKeyChecks: false
+    # -- (string) How often orchestratord reloads its webhook TLS certificate
+    # from disk and, when the CA changes, refreshes the conversion webhook's CA
+    # bundle. Must be shorter than the certificate's lifetime. Accepts a
+    # humantime duration (e.g. "1h", "30m"). Leave null to use the binary
+    # default.
+    webhookCertReloadInterval: null
 
   # -- Additional columns to display when printing the Materialize CRD in table format.
   additionalMaterializeCRDColumns: {}
@@ -39,6 +45,14 @@ operator:
     source: cert-manager
     # -- (string) Name of a secret in the operator's namespace containing ca.crt, tls.crt, and tls.key entries. Only used if `source` is "secret".
     secretName: null
+    # -- (string) Lifetime of the root CA that signs the webhook serving
+    # certificate, when `source` is "cert-manager". The serving certificate is
+    # signed by this CA, so the CA outlives individual serving-certificate
+    # rotations.
+    caDuration: 87600h
+    # -- (string) How long before the root CA expires to renew it. Must be less
+    # than `caDuration`.
+    caRenewBefore: 8760h
 
 
   # Cloud provider configuration

src/orchestratord/src/bin/orchestratord.rs

@@ -78,6 +78,13 @@ pub struct Args {
     tls_cert: String,
     #[clap(long, default_value = "/etc/tls/tls.key")]
     tls_key: String,
+    /// How often to reload the webhook TLS serving certificate from disk and,
+    /// when the CA changes, refresh the conversion webhook's CA bundle. The
+    /// certificate is rotated out-of-band (e.g. by cert-manager), so this must
+    /// be short enough that rotations are picked up before the old certificate
+    /// expires.
+    #[clap(long, default_value = "1h", value_parser = humantime::parse_duration)]
+    webhook_cert_reload_interval: Duration,
 
     #[clap(long)]
     cloud_provider: CloudProvider,
@@ -283,9 +290,10 @@ async fn run(args: Args) -> Result<(), anyhow::Error> {
 
     let metrics = Arc::new(Metrics::register_into(&metrics_registry));
 
-    {
-        let tls_cert = args.tls_cert;
-        let tls_key = args.tls_key;
+    let tls_cert = args.tls_cert;
+    let tls_key = args.tls_key;
+    let tls_ca = args.tls_ca;
+    let reload_config = {
         let config = OpenSSLConfig::from_pem_file(&tls_cert, &tls_key).unwrap();
         let reload_config = config.clone();
         let webhook_listen_address = args.webhook_listen_address;
@@ -299,28 +307,89 @@ async fn run(args: Args) -> Result<(), anyhow::Error> {
             }
         });
 
+        reload_config
+    };
+
+    let (client, namespace) = create_client(args.kubernetes_context.clone()).await?;
+    let additional_crd_columns = args.additional_crd_columns.unwrap_or_default();
+    let webhook_service_name = args.webhook_service_name;
+    let webhook_service_namespace = args.webhook_service_namespace;
+    let webhook_service_port = args.webhook_service_port;
+    register_crds(
+        client.clone(),
+        additional_crd_columns.clone(),
+        webhook_service_name.clone(),
+        webhook_service_namespace.clone(),
+        webhook_service_port,
+        tls_ca.clone(),
+    )
+    .await?;
+
+    // Periodically reload the webhook serving certificate from disk, and
+    // refresh the conversion webhook's CA bundle whenever the CA changes.
+    //
+    // The certificate is rotated out-of-band (e.g. by cert-manager). The
+    // serving certificate is signed by a stable root CA, so routine rotations
+    // reuse the same CA and the CA bundle registered into the CRD at startup
+    // keeps working. But if the CA itself rotates (e.g. on root CA renewal),
+    // that startup CA bundle would not trust the served certificate, and the
+    // Kubernetes API server would reject every conversion request. Refreshing
+    // the CA bundle when the CA changes keeps the webhook working across CA
+    // rotations.
+    {
+        let reload_interval = args.webhook_cert_reload_interval;
+        let client = client.clone();
         mz_ore::task::spawn(|| "webhook certificate reload", async move {
-            let mut interval = tokio::time::interval(Duration::from_secs(60 * 60));
+            let mut last_ca = tokio::fs::read(&tls_ca).await.ok();
+            let mut interval = tokio::time::interval(reload_interval);
+            // The first tick completes immediately; skip it so we don't
+            // re-register the CRDs we just registered above.
+            interval.tick().await;
             loop {
                 interval.tick().await;
                 if let Err(err) = reload_config.reload_from_pem_file(&tls_cert, &tls_key) {
                     tracing::error!("failed to reload webhook TLS certificate: {err}");
+                    continue;
+                }
+                let current_ca = match tokio::fs::read(&tls_ca).await {
+                    Ok(ca) => ca,
+                    Err(err) => {
+                        tracing::error!("failed to read webhook CA certificate: {err}");
+                        continue;
+                    }
+                };
+                if last_ca.as_deref() == Some(current_ca.as_slice()) {
+                    continue;
+                }
+                // The CA changed, meaning the certificate was rotated. Re-register
+                // the CRDs so the conversion webhook's caBundle matches the
+                // newly-served certificate.
+                match register_crds(
+                    client.clone(),
+                    additional_crd_columns.clone(),
+                    webhook_service_name.clone(),
+                    webhook_service_namespace.clone(),
+                    webhook_service_port,
+                    tls_ca.clone(),
+                )
+                .await
+                {
+                    Ok(()) => {
+                        tracing::info!(
+                            "refreshed conversion webhook CA bundle after certificate rotation"
+                        );
+                        last_ca = Some(current_ca);
+                    }
+                    Err(err) => {
+                        tracing::error!(
+                            "failed to refresh conversion webhook CA bundle after rotation: {err}"
+                        );
+                    }
                 }
             }
         });
     }
 
-    let (client, namespace) = create_client(args.kubernetes_context.clone()).await?;
-    register_crds(
-        client.clone(),
-        args.additional_crd_columns.unwrap_or_default(),
-        args.webhook_service_name,
-        args.webhook_service_namespace,
-        args.webhook_service_port,
-        args.tls_ca,
-    )
-    .await?;
-
     let crd_api: Api<CustomResourceDefinition> = Api::all(client.clone());
     let crds = crd_api.list(&ListParams::default()).await?;
     let has_cert_manager = crds