diff --git a/CHANGELOG.md b/CHANGELOG.md index 6633fee1..12117d31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,11 +16,17 @@ All notable changes to this project will be documented in this file. `serviceAccount.create=false` now requires `serviceAccount.name`; it used to fall back to the namespace default ServiceAccount, which lacks the operator ClusterRole ([#736]). - `CreateVolume` no longer returns gRPC codes that make external-provisioner retry indefinitely ([#743]). +- Report the certificate expiry for the `certManager` backend, so that Pods are restarted before + their certificate expires and pick up the renewed one. Previously no expiry was reported at all, + so a Pod kept the certificate it was given at startup and eventually ran on an expired one + indefinitely, even though cert-manager had long since renewed it in the Secret. The restart is + scheduled halfway between cert-manager's own `status.renewalTime` and the expiry ([#752]). [#730]: https://github.com/stackabletech/secret-operator/pull/730 [#735]: https://github.com/stackabletech/secret-operator/pull/735 [#736]: https://github.com/stackabletech/secret-operator/pull/736 [#743]: https://github.com/stackabletech/secret-operator/pull/743 +[#752]: https://github.com/stackabletech/secret-operator/pull/752 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/backend/auto_tls/mod.rs b/rust/operator-binary/src/backend/auto_tls/mod.rs index 34ef5d8d..cc936239 100644 --- a/rust/operator-binary/src/backend/auto_tls/mod.rs +++ b/rust/operator-binary/src/backend/auto_tls/mod.rs @@ -3,7 +3,6 @@ use std::{cmp::min, ops::Range}; use async_trait::async_trait; -use chrono::{FixedOffset, TimeZone}; use openssl::{ asn1::{Asn1Integer, Asn1Time}, bn::{BigNum, MsbOption}, @@ -21,7 +20,7 @@ use openssl::{ }, }; use rand::RngExt as _; -use snafu::{OptionExt, ResultExt, Snafu, ensure}; +use snafu::{ResultExt, Snafu, ensure}; use stackable_operator::{kube::runtime::reflector::ObjectRef, shared::time::Duration}; use time::OffsetDateTime; @@ -34,7 +33,7 @@ use crate::{ }, crd::v1alpha2, format::{SecretData, WellKnownSecretData, well_known}, - utils::iterator_try_concat_bytes, + utils::{DateTimeOutOfBoundsError, iterator_try_concat_bytes, time_datetime_to_chrono}, }; mod ca; @@ -496,41 +495,3 @@ impl SecretBackend for TlsGenerate { ))) } } - -#[derive(Snafu, Debug)] -#[snafu(module)] -pub enum DateTimeOutOfBoundsError { - #[snafu(display("datetime is invalid"))] - DateTime, - - #[snafu(display("time zone is out of bounds"))] - TimeZone, -} -fn time_datetime_to_chrono( - dt: time::OffsetDateTime, -) -> Result, DateTimeOutOfBoundsError> { - let tz = chrono::FixedOffset::east_opt(dt.offset().whole_seconds()) - .context(date_time_out_of_bounds_error::TimeZoneSnafu)?; - tz.timestamp_opt(dt.unix_timestamp(), dt.nanosecond()) - .earliest() - .context(date_time_out_of_bounds_error::DateTimeSnafu) -} - -#[cfg(test)] -mod tests { - use time::format_description::well_known::Rfc3339; - - use super::time_datetime_to_chrono; - - #[test] - fn datetime_conversion() { - // Conversion should preserve timezone and fractional seconds - assert_eq!( - time_datetime_to_chrono( - time::OffsetDateTime::parse("2021-02-04T05:23:00.123+01:00", &Rfc3339).unwrap() - ) - .unwrap(), - chrono::DateTime::parse_from_rfc3339("2021-02-04T06:23:00.123+02:00").unwrap() - ); - } -} diff --git a/rust/operator-binary/src/backend/cert_manager.rs b/rust/operator-binary/src/backend/cert_manager.rs index 7768379e..5b9553ff 100644 --- a/rust/operator-binary/src/backend/cert_manager.rs +++ b/rust/operator-binary/src/backend/cert_manager.rs @@ -5,9 +5,11 @@ use std::collections::HashSet; use async_trait::async_trait; +use chrono::{DateTime, FixedOffset, TimeDelta, Utc}; +use openssl::x509::X509; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ - k8s_openapi::{ByteString, api::core::v1::Secret}, + k8s_openapi::{ByteString, api::core::v1::Secret, apimachinery::pkg::apis::meta::v1::Time}, kube::{api::ObjectMeta, runtime::reflector::ObjectRef}, shared::time::Duration, }; @@ -22,8 +24,11 @@ use super::{ use crate::{ crd::v1alpha2, external_crd::{self, cert_manager::CertificatePrivateKey}, - format::SecretData, - utils::Unloggable, + format::{SecretData, SecretFiles, well_known::FILE_PEM_CERT_CERT}, + utils::{ + Asn1TimeParseError, DateTimeOutOfBoundsError, Unloggable, asn1time_to_offsetdatetime, + time_datetime_to_chrono, + }, }; /// Default lifetime of certs when no annotations are set on the Volume. @@ -63,10 +68,127 @@ pub enum Error { certificate: ObjectRef, }, + #[snafu(display("failed to read the expiry of {secret} (provisioned by {certificate})"))] + InvalidProvisionedCertificate { + source: CertificateExpiryError, + secret: ObjectRef, + certificate: ObjectRef, + }, + #[snafu(display("the certManager backend does not currently support TrustStore exports"))] TrustExportUnsupported, } +#[derive(Debug, Snafu)] +#[snafu(module)] +pub enum CertificateExpiryError { + #[snafu(display("Secret has no {FILE_PEM_CERT_CERT:?} entry"))] + NoCertificate, + + #[snafu(display("failed to parse {FILE_PEM_CERT_CERT:?} as a PEM certificate"))] + ParseCertificate { source: openssl::error::ErrorStack }, + + #[snafu(display("failed to read the certificate's validity period"))] + ReadValidity { source: Asn1TimeParseError }, + + #[snafu(display("the certificate's validity period is out of range"))] + ValidityOutOfBounds { source: DateTimeOutOfBoundsError }, + + #[snafu(display("cert-manager reported a renewal time that is out of range: {seconds}s"))] + RenewalTimeOutOfRange { seconds: i64 }, + + #[snafu(display( + "the provisioned certificate expired at {not_after} and cert-manager has not replaced it (renewal was due at {renewal})" + ))] + CertificateAlreadyExpired { + not_after: DateTime, + renewal: DateTime, + }, +} + +/// Returns when the Pod holding the certificate provisioned into `secret_data` should be restarted. +/// +/// Restarting *at* the certificate's expiry is too late: eviction respects +/// `terminationGracePeriodSeconds` and is serialised by any PodDisruptionBudget, so every replica +/// after the first would keep serving an expired certificate for as long as the rollout takes. +/// So aim for halfway between cert-manager's renewal and the expiry, +/// which also scales with the certificate's lifetime. +fn expire_pod_after( + secret_data: &SecretFiles, + renewal_time: Option<&Time>, + now: DateTime, +) -> Result, CertificateExpiryError> { + use certificate_expiry_error::*; + + let cert_pem = secret_data + .get(FILE_PEM_CERT_CERT) + .context(NoCertificateSnafu)?; + // Reads the first certificate in the file, which for cert-manager is the leaf. + // Any intermediates that follow are ignored. + let cert = X509::from_pem(cert_pem).context(ParseCertificateSnafu)?; + let not_before = asn1_time_to_chrono(cert.not_before())?; + let not_after = asn1_time_to_chrono(cert.not_after())?; + + // Prefer cert-manager's own `status.renewalTime`: it accounts for a `renewBefore` that might + // differ from our requested duration for all kinds of reasons (config etc.). + // It is empty between our apply and cert-manager's next reconcile, so fall back to its default + // of renewing two thirds through the certificate's validity. + let renewal = match renewal_time { + Some(renewal_time) => { + let seconds = renewal_time.0.as_second(); + DateTime::from_timestamp(seconds, 0) + .context(RenewalTimeOutOfRangeSnafu { seconds })? + .fixed_offset() + } + None => { + let validity: TimeDelta = not_after - not_before; + let renewal = not_before + validity * 2 / 3; + tracing::info!( + certificate.not_before = %not_before, + certificate.not_after = %not_after, + certificate.renewal_time = %renewal, + "Certificate has no status.renewalTime, assuming cert-manager's default of two thirds through the validity period" + ); + renewal + } + }; + + // Halfway from renewal to expiry. + // With the two-thirds fallback that lands on "not_before + 5/6 * validity". + let remaining: TimeDelta = not_after - renewal; + let expire_pod_after = renewal + remaining / 2; + + // Reporting an expiry that has already passed would have the restarter evict the Pod at once, + // and the replacement would be handed this same certificate and evicted again. There is nothing + // useful to hand out, so fail and let the kubelet retry until cert-manager catches up. + if not_after <= now { + return CertificateAlreadyExpiredSnafu { not_after, renewal }.fail(); + } + + if expire_pod_after <= now { + // cert-manager has not renewed even though it said it would by now, so a restart would hand + // the Pod back the same certificate and evict it again. Wait for the expiry instead, which + // the check above guarantees is still in the future. + tracing::warn!( + certificate.not_after = %not_after, + certificate.renewal_time = %renewal, + "cert-manager has not renewed this certificate yet, falling back to restarting the Pod at its expiry" + ); + return Ok(not_after); + } + + Ok(expire_pod_after) +} + +fn asn1_time_to_chrono( + time: &openssl::asn1::Asn1TimeRef, +) -> Result, CertificateExpiryError> { + use certificate_expiry_error::*; + + let time = asn1time_to_offsetdatetime(time).context(ReadValiditySnafu)?; + time_datetime_to_chrono(time).context(ValidityOutOfBoundsSnafu) +} + impl SecretBackendError for Error { fn grpc_code(&self) -> tonic::Code { match self { @@ -75,6 +197,7 @@ impl SecretBackendError for Error { Error::GetSecret { .. } => tonic::Code::Unavailable, Error::GetCertManagerCertificate { .. } => tonic::Code::Unavailable, Error::ApplyCertManagerCertificate { .. } => tonic::Code::Unavailable, + Error::InvalidProvisionedCertificate { .. } => tonic::Code::Unavailable, Error::TrustExportUnsupported => tonic::Code::FailedPrecondition, } } @@ -90,6 +213,7 @@ impl SecretBackendError for Error { Error::GetCertManagerCertificate { certificate, .. } => { Some(certificate.clone().erase()) } + Error::InvalidProvisionedCertificate { secret, .. } => Some(secret.clone().erase()), Error::TrustExportUnsupported => None, } } @@ -144,6 +268,7 @@ impl SecretBackend for CertManager { ), ..Default::default() }, + status: None, spec: external_crd::cert_manager::CertificateSpec { secret_name: cert_name.clone(), duration: Some(format!( @@ -175,23 +300,38 @@ impl SecretBackend for CertManager { certificate: ObjectRef::from_obj(&cert), })?; + let secret_ref = + ObjectRef::::new(&cert.spec.secret_name).within(&selector.namespace); let secret = self .client .get::(&cert.spec.secret_name, &selector.namespace) .await .with_context(|_| GetSecretSnafu { certificate: ObjectRef::from_obj(&cert), - secret: ObjectRef::::new(&cert.spec.secret_name) - .within(&selector.namespace), + secret: secret_ref.clone(), })?; - Ok(SecretContents::new(SecretData::Unknown( - secret - .data - .unwrap_or_default() - .into_iter() - .map(|(k, ByteString(v))| (k, v)) - .collect(), - ))) + let secret_data = secret + .data + .unwrap_or_default() + .into_iter() + .map(|(k, ByteString(v))| (k, v)) + .collect::(); + + // cert-manager renews the certificate in the Secret on its own schedule. + // We copy the material into the pod and never touch it again. + // This reports an expiry so that the Pod gets restarted (and so picks up the renewed + // certificate) via the commons-operator restarter mechanism. + let renewal_time = cert + .status + .as_ref() + .and_then(|status| status.renewal_time.as_ref()); + let expires_after = expire_pod_after(&secret_data, renewal_time, Utc::now().fixed_offset()) + .with_context(|_| InvalidProvisionedCertificateSnafu { + certificate: ObjectRef::from_obj(&cert), + secret: secret_ref.clone(), + })?; + + Ok(SecretContents::new(SecretData::Unknown(secret_data)).expires_after(expires_after)) } async fn get_trust_data( @@ -230,3 +370,129 @@ impl SecretBackend for CertManager { } } } + +#[cfg(test)] +mod tests { + use openssl::{asn1::Asn1Time, pkey::PKey, rsa::Rsa, x509::X509Builder}; + + use super::*; + + /// The certificates below are all one day long, starting at the epoch. + const HOUR: i64 = 3600; + const NOT_BEFORE: i64 = 0; + const NOT_AFTER: i64 = 24 * HOUR; + + fn certificate_valid_between(not_before: i64, not_after: i64) -> Vec { + let pkey = PKey::try_from(Rsa::generate(2048).unwrap()).unwrap(); + let mut builder = X509Builder::new().unwrap(); + builder + .set_not_before(Asn1Time::from_unix(not_before).unwrap().as_ref()) + .unwrap(); + builder + .set_not_after(Asn1Time::from_unix(not_after).unwrap().as_ref()) + .unwrap(); + builder.set_pubkey(&pkey).unwrap(); + builder + .sign(&pkey, openssl::hash::MessageDigest::sha256()) + .unwrap(); + builder.build().to_pem().unwrap() + } + + fn secret_with(cert_pem: Vec) -> SecretFiles { + SecretFiles::from([(FILE_PEM_CERT_CERT.to_owned(), cert_pem)]) + } + + fn at(timestamp: i64) -> DateTime { + DateTime::from_timestamp(timestamp, 0) + .unwrap() + .fixed_offset() + } + + fn time_at(timestamp: i64) -> Time { + Time(stackable_operator::k8s_openapi::jiff::Timestamp::from_second(timestamp).unwrap()) + } + + #[test] + fn pod_expires_halfway_between_cert_managers_renewal_and_the_expiry() { + let secret_data = secret_with(certificate_valid_between(NOT_BEFORE, NOT_AFTER)); + + // Renewal at 12h of a 24h certificate, so the Pod should be restarted at 18h. + assert_eq!( + expire_pod_after(&secret_data, Some(&time_at(12 * HOUR)), at(HOUR)) + .unwrap() + .timestamp(), + 18 * HOUR + ); + } + + #[test] + fn without_a_renewal_time_the_default_of_two_thirds_is_assumed() { + let secret_data = secret_with(certificate_valid_between(NOT_BEFORE, NOT_AFTER)); + + // cert-manager renews two thirds in (16h), so halfway to the expiry is 5/6 in (20h). + assert_eq!( + expire_pod_after(&secret_data, None, at(HOUR)) + .unwrap() + .timestamp(), + 20 * HOUR + ); + } + + #[test] + fn an_overdue_renewal_falls_back_to_the_expiry() { + let secret_data = secret_with(certificate_valid_between(NOT_BEFORE, NOT_AFTER)); + + // cert-manager said it would renew at 12h and has not, so restarting now would hand the Pod + // the same certificate back. + assert_eq!( + expire_pod_after(&secret_data, Some(&time_at(12 * HOUR)), at(23 * HOUR)) + .unwrap() + .timestamp(), + NOT_AFTER + ); + } + + #[test] + fn only_the_leaf_of_a_certificate_chain_is_read() { + let mut chain = certificate_valid_between(NOT_BEFORE, NOT_AFTER); + // An intermediate outliving the leaf, as a real chain would have. + chain.extend(certificate_valid_between(NOT_BEFORE, 365 * 24 * HOUR)); + + assert_eq!( + expire_pod_after(&secret_with(chain), None, at(HOUR)) + .unwrap() + .timestamp(), + 20 * HOUR + ); + } + + #[test] + fn an_already_expired_certificate_is_an_error() { + let secret_data = secret_with(certificate_valid_between(NOT_BEFORE, NOT_AFTER)); + + // Reporting an expiry in the past would have the restarter evict the Pod immediately, and + // the replacement would be handed this same certificate. + assert!(matches!( + expire_pod_after(&secret_data, Some(&time_at(16 * HOUR)), at(30 * HOUR)), + Err(CertificateExpiryError::CertificateAlreadyExpired { .. }) + )); + } + + #[test] + fn expiry_of_secret_without_certificate_is_an_error() { + assert!(matches!( + expire_pod_after(&SecretFiles::new(), None, at(HOUR)), + Err(CertificateExpiryError::NoCertificate) + )); + } + + #[test] + fn expiry_of_unparseable_certificate_is_an_error() { + let secret_data = secret_with(b"not a certificate".to_vec()); + + assert!(matches!( + expire_pod_after(&secret_data, None, at(HOUR)), + Err(CertificateExpiryError::ParseCertificate { .. }) + )); + } +} diff --git a/rust/operator-binary/src/external_crd/cert_manager.rs b/rust/operator-binary/src/external_crd/cert_manager.rs index 42beec6f..26dcbffc 100644 --- a/rust/operator-binary/src/external_crd/cert_manager.rs +++ b/rust/operator-binary/src/external_crd/cert_manager.rs @@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize}; use stackable_operator::{ + k8s_openapi::apimachinery::pkg::apis::meta::v1::Time, kube::CustomResource, schemars::{self, JsonSchema}, }; @@ -13,6 +14,7 @@ use stackable_operator::{ version = "v1", kind = "Certificate", namespaced, + status = "CertificateStatus", crates( kube_core = "stackable_operator::kube::core", k8s_openapi = "stackable_operator::k8s_openapi", @@ -31,6 +33,14 @@ pub struct CertificateSpec { pub private_key: CertificatePrivateKey, } +/// See . +#[derive(Serialize, Deserialize, Clone, Debug, Default, PartialEq, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct CertificateStatus { + /// When cert-manager will next attempt to renew the certificate. + pub renewal_time: Option