feat: Support setting TLS certificate lifetimes (#501)

razvan · sbernauer · web-flow · commit e3fe2ca66b76 · 2024-12-04T09:50:30.000Z
* feat: Support setting TLS certificate lifetimes

* chore: bump op-rs

* Update rust/crd/src/history.rs

Co-authored-by: Sebastian Bernauer &lt;sebastian.bernauer@stackable.de&gt;

* Update rust/crd/src/roles.rs

Co-authored-by: Sebastian Bernauer &lt;sebastian.bernauer@stackable.de&gt;

* Update rust/crd/src/roles.rs

Co-authored-by: Sebastian Bernauer &lt;sebastian.bernauer@stackable.de&gt;

* cargo update -p rustls

---------

Co-authored-by: Sebastian Bernauer &lt;sebastian.bernauer@stackable.de&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,13 @@ 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 ([#501]).
+
+[#501]: https://github.com/stackabletech/spark-k8s-operator/pull/501
+
 ## [24.11.0] - 2024-11-18
 
 ### Added
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -21,7 +21,7 @@ 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" }
 strum = { version = "0.26", features = ["derive"] }
 tokio = { version = "1.39", features = ["full"] }
 tracing = "0.1"
diff --git a/deploy/helm/spark-k8s-operator/crds/crds.yaml b/deploy/helm/spark-k8s-operator/crds/crds.yaml
@@ -193,6 +193,10 @@ spec:
                               nullable: true
                               type: boolean
                           type: object
+                        requestedSecretLifetime:
+                          description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. That this can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate.
+                          nullable: true
+                          type: string
                         resources:
                           default:
                             cpu:
@@ -463,6 +467,10 @@ spec:
                               nullable: true
                               type: boolean
                           type: object
+                        requestedSecretLifetime:
+                          description: Request secret (currently only autoTls certificates) lifetime from the secret operator, e.g. `7d`, or `30d`. That this can be shortened by the `maxCertificateLifetime` setting on the SecretClass issuing the TLS certificate.
+                          nullable: true
+                          type: string
                         resources:
                           default:
                             cpu:
@@ -549,6 +557,10 @@ spec:
                     config:
                       default: {}
                       properties:
+                        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:
@@ -1347,6 +1359,10 @@ spec:
                               nullable: true
                               type: boolean
                           type: object
+                        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:
@@ -1563,6 +1579,10 @@ spec:
                                     nullable: true
                                     type: boolean
                                 type: object
+                              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/history.rs b/rust/crd/src/history.rs
@@ -27,6 +27,7 @@ use stackable_operator::{
     product_logging::{self, spec::Logging},
     role_utils::{Role, RoleGroup, RoleGroupRef},
     schemars::{self, JsonSchema},
+    time::Duration,
 };
 use std::collections::{BTreeMap, HashMap};
 use strum::{Display, EnumIter};
@@ -400,9 +401,17 @@ pub struct HistoryConfig {
     pub logging: Logging<SparkHistoryServerContainer>,
     #[fragment_attrs(serde(default))]
     pub affinity: StackableAffinity,
+
+    /// 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<Duration>,
 }
 
 impl HistoryConfig {
+    // Auto TLS certificate lifetime
+    const DEFAULT_HISTORY_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
+
     fn default_config(cluster_name: &str) -> HistoryConfigFragment {
         HistoryConfigFragment {
             cleaner: None,
@@ -419,6 +428,7 @@ impl HistoryConfig {
             },
             logging: product_logging::spec::default_logging(),
             affinity: history_affinity(cluster_name),
+            requested_secret_lifetime: Some(Self::DEFAULT_HISTORY_SECRET_LIFETIME),
         }
     }
 }
diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs
@@ -14,6 +14,7 @@ use logdir::ResolvedLogDir;
 use product_config::{types::PropertyNameKind, ProductConfigManager};
 use serde::{Deserialize, Serialize};
 use snafu::{OptionExt, ResultExt, Snafu};
+use stackable_operator::time::Duration;
 use stackable_operator::{
     builder::pod::volume::{
         SecretFormat, SecretOperatorVolumeSourceBuilder, SecretOperatorVolumeSourceBuilderError,
@@ -281,6 +282,7 @@ impl SparkApplication {
         s3conn: &Option<S3ConnectionSpec>,
         logdir: &Option<ResolvedLogDir>,
         log_config_map: Option<&str>,
+        requested_secret_lifetime: &Duration,
     ) -> Result<Vec<Volume>, Error> {
         let mut result: Vec<Volume> = self.spec.volumes.clone();
 
@@ -356,6 +358,7 @@ impl SparkApplication {
                         .ephemeral(
                             SecretOperatorVolumeSourceBuilder::new(cert_secret)
                                 .with_format(SecretFormat::TlsPkcs12)
+                                .with_auto_tls_cert_lifetime(*requested_secret_lifetime)
                                 .build()
                                 .context(TlsCertSecretClassVolumeBuildSnafu)?,
                         )
@@ -1068,6 +1071,7 @@ mod tests {
 
     use indoc::indoc;
     use rstest::rstest;
+    use stackable_operator::time::Duration;
     use std::collections::{BTreeMap, HashMap};
 
     #[test]
@@ -1206,6 +1210,7 @@ mod tests {
             },
             volume_mounts: Default::default(),
             affinity: StackableAffinity::default(),
+            requested_secret_lifetime: Some(Duration::from_days_unchecked(1)),
         };
 
         let mut props = BTreeMap::new();
@@ -1250,6 +1255,7 @@ mod tests {
             },
             volume_mounts: Default::default(),
             affinity: StackableAffinity::default(),
+            requested_secret_lifetime: Some(Duration::from_days_unchecked(1)),
         };
 
         let mut props = BTreeMap::new();
diff --git a/rust/crd/src/logdir.rs b/rust/crd/src/logdir.rs
@@ -16,6 +16,7 @@ use stackable_operator::{
         secret_class::SecretClassVolume,
     },
     k8s_openapi::api::core::v1::{Volume, VolumeMount},
+    time::Duration,
 };
 use std::collections::BTreeMap;
 
@@ -103,9 +104,9 @@ impl ResolvedLogDir {
         }
     }
 
-    pub fn volumes(&self) -> Result<Vec<Volume>, Error> {
+    pub fn volumes(&self, requested_secret_lifetime: &Duration) -> Result<Vec<Volume>, Error> {
         match self {
-            ResolvedLogDir::S3(s3_log_dir) => s3_log_dir.volumes(),
+            ResolvedLogDir::S3(s3_log_dir) => s3_log_dir.volumes(requested_secret_lifetime),
             ResolvedLogDir::Custom(_) => Ok(vec![]),
         }
     }
@@ -248,7 +249,7 @@ impl S3LogDir {
         )
     }
 
-    pub fn volumes(&self) -> Result<Vec<Volume>, Error> {
+    pub fn volumes(&self, requested_secret_lifetime: &Duration) -> Result<Vec<Volume>, Error> {
         let mut volumes: Vec<Volume> = self.credentials_volume()?.into_iter().collect();
 
         if let Some(secret_name) = tlscerts::tls_secret_name(&self.bucket.connection) {
@@ -257,6 +258,7 @@ impl S3LogDir {
                     .ephemeral(
                         SecretOperatorVolumeSourceBuilder::new(secret_name)
                             .with_format(SecretFormat::TlsPkcs12)
+                            .with_auto_tls_cert_lifetime(*requested_secret_lifetime)
                             .build()
                             .context(TlsCertSecretClassVolumeBuildSnafu)?,
                     )
diff --git a/rust/crd/src/roles.rs b/rust/crd/src/roles.rs
@@ -36,6 +36,7 @@ use stackable_operator::{
     product_config_utils::Configuration,
     product_logging::{self, spec::Logging},
     schemars::{self, JsonSchema},
+    time::Duration,
     utils::crds::raw_object_list_schema,
 };
 use strum::{Display, EnumIter};
@@ -123,9 +124,17 @@ pub struct RoleConfig {
 
     #[fragment_attrs(serde(default))]
     pub affinity: StackableAffinity,
+
+    /// 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<Duration>,
 }
 
 impl RoleConfig {
+    // Auto TLS certificate lifetime
+    const DEFAULT_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
+
     pub fn default_config() -> RoleConfigFragment {
         RoleConfigFragment {
             resources: ResourcesFragment {
@@ -142,6 +151,7 @@ impl RoleConfig {
             logging: product_logging::spec::default_logging(),
             volume_mounts: Some(VolumeMounts::default()),
             affinity: Default::default(),
+            requested_secret_lifetime: Some(Self::DEFAULT_SECRET_LIFETIME),
         }
     }
     pub fn volume_mounts(
@@ -206,9 +216,17 @@ pub struct SubmitConfig {
     pub resources: Resources<SparkStorageConfig, NoRuntimeLimits>,
     #[fragment_attrs(serde(default, flatten))]
     pub volume_mounts: Option<VolumeMounts>,
+
+    /// 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<Duration>,
 }
 
 impl SubmitConfig {
+    // Auto TLS certificate lifetime
+    const DEFAULT_SECRET_LIFETIME: Duration = Duration::from_days_unchecked(7);
+
     pub fn default_config() -> SubmitConfigFragment {
         SubmitConfigFragment {
             resources: ResourcesFragment {
@@ -223,6 +241,7 @@ impl SubmitConfig {
                 storage: SparkStorageConfigFragment {},
             },
             volume_mounts: Some(VolumeMounts::default()),
+            requested_secret_lifetime: Some(Self::DEFAULT_SECRET_LIFETIME),
         }
     }
 }
diff --git a/rust/operator-binary/src/history/history_controller.rs b/rust/operator-binary/src/history/history_controller.rs
@@ -65,6 +65,9 @@ use strum::{EnumDiscriminants, IntoStaticStr};
 #[strum_discriminants(derive(IntoStaticStr))]
 #[allow(clippy::enum_variant_names)]
 pub enum Error {
+    #[snafu(display("missing secret lifetime"))]
+    MissingSecretLifetime,
+
     #[snafu(display("object has no namespace"))]
     ObjectHasNoNamespace,
 
@@ -444,15 +447,15 @@ fn build_stateful_set(
     resolved_product_image: &ResolvedProductImage,
     rolegroupref: &RoleGroupRef<SparkHistoryServer>,
     log_dir: &ResolvedLogDir,
-    config: &HistoryConfig,
+    merged_config: &HistoryConfig,
     serviceaccount: &ServiceAccount,
 ) -> Result<StatefulSet, Error> {
     let log_config_map = if let Some(ContainerLogConfig {
         choice:
             Some(ContainerLogConfigChoice::Custom(CustomContainerLogConfig {
                 custom: ConfigMapLogConfig { config_map },
             })),
-    }) = config
+    }) = merged_config
         .logging
         .containers
         .get(&SparkHistoryServerContainer::SparkHistory)
@@ -473,6 +476,9 @@ fn build_stateful_set(
 
     let mut pb = PodBuilder::new();
 
+    let requested_secret_lifetime = merged_config
+        .requested_secret_lifetime
+        .context(MissingSecretLifetimeSnafu)?;
     pb.service_account_name(serviceaccount.name_unchecked())
         .metadata(metadata)
         .image_pull_secrets_from_product_image(resolved_product_image)
@@ -497,7 +503,11 @@ fn build_stateful_set(
                 .build(),
         )
         .context(AddVolumeSnafu)?
-        .add_volumes(log_dir.volumes().context(CreateLogDirVolumesSpecSnafu)?)
+        .add_volumes(
+            log_dir
+                .volumes(&requested_secret_lifetime)
+                .context(CreateLogDirVolumesSpecSnafu)?,
+        )
         .context(AddVolumeSnafu)?
         .security_context(PodSecurityContext {
             run_as_user: Some(SPARK_UID),
@@ -516,7 +526,7 @@ fn build_stateful_set(
     let container = ContainerBuilder::new(container_name)
         .context(InvalidContainerNameSnafu)?
         .image_from_product_image(resolved_product_image)
-        .resources(config.resources.clone().into())
+        .resources(merged_config.resources.clone().into())
         .command(vec!["/bin/bash".to_string()])
         .args(command_args(log_dir))
         .add_container_port("http", 18080)
@@ -533,13 +543,13 @@ fn build_stateful_set(
         .build();
     pb.add_container(container);
 
-    if config.logging.enable_vector_agent {
+    if merged_config.logging.enable_vector_agent {
         pb.add_container(
             vector_container(
                 resolved_product_image,
                 VOLUME_MOUNT_NAME_CONFIG,
                 VOLUME_MOUNT_NAME_LOG,
-                config
+                merged_config
                     .logging
                     .containers
                     .get(&SparkHistoryServerContainer::Vector),
diff --git a/rust/operator-binary/src/spark_k8s_controller.rs b/rust/operator-binary/src/spark_k8s_controller.rs

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ use stackable_operator::{`
`16`	`16`	`secret_class::SecretClassVolume,`
`17`	`17`	`},`
`18`	`18`	`k8s_openapi::api::core::v1::{Volume, VolumeMount},`
	`19`	`+ time::Duration,`
`19`	`20`	`};`
`20`	`21`	`use std::collections::BTreeMap;`
`21`	`22`
`@@ -103,9 +104,9 @@ impl ResolvedLogDir {`
`103`	`104`	`}`
`104`	`105`	`}`
`105`	`106`
`106`		`- pub fn volumes(&self) -> Result<Vec<Volume>, Error> {`
	`107`	`+ pub fn volumes(&self, requested_secret_lifetime: &Duration) -> Result<Vec<Volume>, Error> {`
`107`	`108`	`match self {`
`108`		`- ResolvedLogDir::S3(s3_log_dir) => s3_log_dir.volumes(),`
	`109`	`+ ResolvedLogDir::S3(s3_log_dir) => s3_log_dir.volumes(requested_secret_lifetime),`
`109`	`110`	`ResolvedLogDir::Custom(_) => Ok(vec![]),`
`110`	`111`	`}`
`111`	`112`	`}`
`@@ -248,7 +249,7 @@ impl S3LogDir {`
`248`	`249`	`)`
`249`	`250`	`}`
`250`	`251`
`251`		`- pub fn volumes(&self) -> Result<Vec<Volume>, Error> {`
	`252`	`+ pub fn volumes(&self, requested_secret_lifetime: &Duration) -> Result<Vec<Volume>, Error> {`
`252`	`253`	`let mut volumes: Vec<Volume> = self.credentials_volume()?.into_iter().collect();`
`253`	`254`
`254`	`255`	`if let Some(secret_name) = tlscerts::tls_secret_name(&self.bucket.connection) {`
`@@ -257,6 +258,7 @@ impl S3LogDir {`
`257`	`258`	`.ephemeral(`
`258`	`259`	`SecretOperatorVolumeSourceBuilder::new(secret_name)`
`259`	`260`	`.with_format(SecretFormat::TlsPkcs12)`
	`261`	`+ .with_auto_tls_cert_lifetime(*requested_secret_lifetime)`
`260`	`262`	`.build()`
`261`	`263`	`.context(TlsCertSecretClassVolumeBuildSnafu)?,`
`262`	`264`	`)`