From 5a3449e0da99a9d5b869569dffc0f109f703dc6c Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:38:51 +0100 Subject: [PATCH 1/6] feat: Support setting TLS certificate lifetimes --- CHANGELOG.md | 6 +++++ Cargo.lock | 19 ++++------------ Cargo.toml | 4 ++-- deploy/helm/trino-operator/crds/crds.yaml | 16 ++++++++++++++ rust/crd/src/lib.rs | 6 +++++ rust/operator-binary/src/controller.rs | 27 ++++++++++++++++++++--- 6 files changed, 58 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e282207..d993a39f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- The lifetime of auto generated TLS certificates is now configurable with the role and roleGroup + config property `requestedSecretLifetime`. This helps reducing frequent Pod restarts ([#676]). + ### Fixed - Fix OIDC endpoint construction in case the `rootPath` does have a trailing slash ([#673]). @@ -13,6 +18,7 @@ All notable changes to this project will be documented in this file. [#672]: https://github.com/stackabletech/trino-operator/pull/672 [#673]: https://github.com/stackabletech/trino-operator/pull/673 +[#676]: https://github.com/stackabletech/trino-operator/pull/676 ## [24.11.0] - 2024-11-18 diff --git a/Cargo.lock b/Cargo.lock index 9eccdf12..5d7d4562 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -478,17 +478,6 @@ dependencies = [ "powerfmt", ] -[[package]] -name = "derivative" -version = "2.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcc3dd5e9e9c0b295d6e1e4d811fb6f157d5ffd784b8d202fc62eac8035a770b" -dependencies = [ - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "digest" version = "0.10.7" @@ -2226,14 +2215,14 @@ checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" [[package]] name = "stackable-operator" version = "0.82.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.82.0#415bbd031bd52e9c0c5392060235030e9930b46b" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#7939b3516f01483c0d9f601d57ee70003420f7e5" dependencies = [ "chrono", "clap", "const_format", "delegate", - "derivative", "dockerfile-parser", + "educe", "either", "futures 0.3.31", "indexmap", @@ -2264,7 +2253,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.82.0#415bbd031bd52e9c0c5392060235030e9930b46b" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#7939b3516f01483c0d9f601d57ee70003420f7e5" dependencies = [ "darling", "proc-macro2", @@ -2275,7 +2264,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.0.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.82.0#415bbd031bd52e9c0c5392060235030e9930b46b" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#7939b3516f01483c0d9f601d57ee70003420f7e5" dependencies = [ "kube", "semver", diff --git a/Cargo.toml b/Cargo.toml index b9c57ba9..5a6cf1ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,6 @@ strum = { version = "0.26", features = ["derive"] } tokio = { version = "1.40", features = ["full"] } tracing = "0.1" -# [patch."https://github.com/stackabletech/operator-rs.git"] -# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } +[patch."https://github.com/stackabletech/operator-rs.git"] +stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } # stackable-operator = { path = "../operator-rs/crates/stackable-operator" } diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml index fa37005e..bf803700 100644 --- a/deploy/helm/trino-operator/crds/crds.yaml +++ b/deploy/helm/trino-operator/crds/crds.yaml @@ -297,6 +297,10 @@ spec: queryMaxMemoryPerNode: nullable: true type: string + requestedSecretLifetime: + description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + nullable: true + type: string resources: default: cpu: @@ -566,6 +570,10 @@ spec: queryMaxMemoryPerNode: nullable: true type: string + requestedSecretLifetime: + description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + nullable: true + type: string resources: default: cpu: @@ -864,6 +872,10 @@ spec: queryMaxMemoryPerNode: nullable: true type: string + requestedSecretLifetime: + description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + nullable: true + type: string resources: default: cpu: @@ -1133,6 +1145,10 @@ spec: queryMaxMemoryPerNode: nullable: true type: string + requestedSecretLifetime: + description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + nullable: true + type: string resources: default: cpu: diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 7e6859f8..ee4796ae 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -432,9 +432,14 @@ pub struct TrinoConfig { /// Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. #[fragment_attrs(serde(default))] pub graceful_shutdown_timeout: Option, + /// Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. + /// This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. + #[fragment_attrs(serde(default))] + pub requested_secret_lifetime: Option, } impl TrinoConfig { + const DEFAULT_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7); fn default_config( cluster_name: &str, role: &TrinoRole, @@ -472,6 +477,7 @@ impl TrinoConfig { query_max_memory: None, query_max_memory_per_node: None, graceful_shutdown_timeout: Some(graceful_shutdown_timeout), + requested_secret_lifetime: Some(Self::DEFAULT_SECRET_LIFETIME), } } } diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 6ca16439..e71b883e 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -124,6 +124,9 @@ const DOCKER_IMAGE_BASE_NAME: &str = "trino"; #[strum_discriminants(derive(IntoStaticStr))] #[allow(clippy::enum_variant_names)] pub enum Error { + #[snafu(display("missing secret lifetime"))] + MissingSecretLifetime, + #[snafu(display("object defines no namespace"))] ObjectHasNoNamespace, @@ -944,6 +947,9 @@ fn build_rolegroup_statefulset( }), ); + let requested_secret_lifetime = merged_config + .requested_secret_lifetime + .context(MissingSecretLifetimeSnafu)?; // add volume mounts depending on the client tls, internal tls, catalogs and authentication tls_volume_mounts( trino, @@ -951,6 +957,7 @@ fn build_rolegroup_statefulset( &mut cb_prepare, &mut cb_trino, catalogs, + &requested_secret_lifetime, )?; let mut prepare_args = vec![]; @@ -1486,13 +1493,18 @@ fn liveness_probe(trino: &TrinoCluster) -> Probe { } } -fn create_tls_volume(volume_name: &str, tls_secret_class: &str) -> Result { +fn create_tls_volume( + volume_name: &str, + tls_secret_class: &str, + requested_secret_lifetime: &Duration, +) -> Result { Ok(VolumeBuilder::new(volume_name) .ephemeral( SecretOperatorVolumeSourceBuilder::new(tls_secret_class) .with_pod_scope() .with_node_scope() .with_format(SecretFormat::TlsPkcs12) + .with_auto_tls_cert_lifetime(*requested_secret_lifetime) .build() .context(TlsCertSecretClassVolumeBuildSnafu)?, ) @@ -1505,6 +1517,7 @@ fn tls_volume_mounts( cb_prepare: &mut ContainerBuilder, cb_trino: &mut ContainerBuilder, catalogs: &[CatalogConfig], + requested_secret_lifetime: &Duration, ) -> Result<()> { if let Some(server_tls) = trino.get_server_tls() { cb_prepare @@ -1514,7 +1527,11 @@ fn tls_volume_mounts( .add_volume_mount("server-tls-mount", STACKABLE_MOUNT_SERVER_TLS_DIR) .context(AddVolumeMountSnafu)?; pod_builder - .add_volume(create_tls_volume("server-tls-mount", server_tls)?) + .add_volume(create_tls_volume( + "server-tls-mount", + server_tls, + requested_secret_lifetime, + )?) .context(AddVolumeSnafu)?; } @@ -1546,7 +1563,11 @@ fn tls_volume_mounts( .add_volume_mount("internal-tls-mount", STACKABLE_MOUNT_INTERNAL_TLS_DIR) .context(AddVolumeMountSnafu)?; pod_builder - .add_volume(create_tls_volume("internal-tls-mount", internal_tls)?) + .add_volume(create_tls_volume( + "internal-tls-mount", + internal_tls, + requested_secret_lifetime, + )?) .context(AddVolumeSnafu)?; cb_prepare From 091a63d756fb5a2441c1aaf9665b30b834cb0bde Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Mon, 2 Dec 2024 18:08:15 +0100 Subject: [PATCH 2/6] typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d993a39f..53d5ff44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Added - The lifetime of auto generated TLS certificates is now configurable with the role and roleGroup - config property `requestedSecretLifetime`. This helps reducing frequent Pod restarts ([#676]). + config property `requestedSecretLifetime`. This helps reduce frequent Pod restarts ([#676]). ### Fixed From ae8061d51901379d846058b0e6d2a8bd7336aab9 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Tue, 3 Dec 2024 16:40:38 +0100 Subject: [PATCH 3/6] chore: bump op-rs --- Cargo.lock | 8 ++++---- Cargo.toml | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d7d4562..12009994 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2214,8 +2214,8 @@ checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" [[package]] name = "stackable-operator" -version = "0.82.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#7939b3516f01483c0d9f601d57ee70003420f7e5" +version = "0.83.0" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.83.0#3ce7bcbdb58097cde0c0f19488a104c96f69dbc3" dependencies = [ "chrono", "clap", @@ -2253,7 +2253,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#7939b3516f01483c0d9f601d57ee70003420f7e5" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.83.0#3ce7bcbdb58097cde0c0f19488a104c96f69dbc3" dependencies = [ "darling", "proc-macro2", @@ -2264,7 +2264,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.0.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#7939b3516f01483c0d9f601d57ee70003420f7e5" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.83.0#3ce7bcbdb58097cde0c0f19488a104c96f69dbc3" dependencies = [ "kube", "semver", diff --git a/Cargo.toml b/Cargo.toml index 5a6cf1ed..c630202d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,12 +24,12 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" serde_yaml = "0.9" snafu = "0.8" -stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.82.0" } +stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.83.0" } product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.7.0" } strum = { version = "0.26", features = ["derive"] } tokio = { version = "1.40", features = ["full"] } tracing = "0.1" -[patch."https://github.com/stackabletech/operator-rs.git"] -stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } +#[patch."https://github.com/stackabletech/operator-rs.git"] +#stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } # stackable-operator = { path = "../operator-rs/crates/stackable-operator" } From 814df335a56015278c053bba1f1c7637101d3779 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Wed, 4 Dec 2024 09:13:28 +0100 Subject: [PATCH 4/6] cargo update -p rustls --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12009994..72d87946 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1872,9 +1872,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.15" +version = "0.23.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fbb44d7acc4e873d613422379f69f237a1b141928c02f6bc6ccfddddc2d7993" +checksum = "934b404430bb06b3fae2cba809eb45a1ab1aecd64491213d7c3301b88393f8d1" dependencies = [ "log", "once_cell", From ce04a18ff58351e2c55fe977fb447f6521c63a5f Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Wed, 4 Dec 2024 11:00:54 +0100 Subject: [PATCH 5/6] Update rust/crd/src/lib.rs Co-authored-by: Sebastian Bernauer --- rust/crd/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index ee4796ae..087621a5 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -432,6 +432,7 @@ pub struct TrinoConfig { /// Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. #[fragment_attrs(serde(default))] pub graceful_shutdown_timeout: Option, + /// Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. /// This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. #[fragment_attrs(serde(default))] From 1e74dab3dd1b791747ba8b53409dae1e75883792 Mon Sep 17 00:00:00 2001 From: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com> Date: Wed, 4 Dec 2024 11:05:58 +0100 Subject: [PATCH 6/6] rustfmt --- rust/crd/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 087621a5..863fdf42 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -432,7 +432,7 @@ pub struct TrinoConfig { /// Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. #[fragment_attrs(serde(default))] pub graceful_shutdown_timeout: Option, - + /// Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. /// This can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate. #[fragment_attrs(serde(default))]