Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/stackable-operator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Added

- Add new configuration field `CommonConfiguration::min_secret_lifetime` and supporting `SecretOperatorVolumeSourceBuilder` code to use it ([#908]).

[#908]: https://github.com/stackabletech/operator-rs/pull/908

## [0.81.0] - 2024-11-05

### Added
Expand Down
15 changes: 15 additions & 0 deletions crates/stackable-operator/src/builder/pod/volume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use tracing::warn;
use crate::{
builder::meta::ObjectMetaBuilder,
kvp::{Annotation, AnnotationError, Annotations, LabelError, Labels},
time::Duration,
};

/// A builder to build [`Volume`] objects. May only contain one `volume_source`
Expand Down Expand Up @@ -280,6 +281,7 @@ pub struct SecretOperatorVolumeSourceBuilder {
format: Option<SecretFormat>,
kerberos_service_names: Vec<String>,
tls_pkcs12_password: Option<String>,
secret_lifetime: Option<Duration>,
}

impl SecretOperatorVolumeSourceBuilder {
Expand All @@ -290,9 +292,15 @@ impl SecretOperatorVolumeSourceBuilder {
format: None,
kerberos_service_names: Vec::new(),
tls_pkcs12_password: None,
secret_lifetime: None,
}
}

pub fn with_secret_lifetime(&mut self, lifetime: Option<Duration>) -> &mut Self {
self.secret_lifetime = lifetime;
self
}

pub fn with_node_scope(&mut self) -> &mut Self {
self.scopes.push(SecretOperatorVolumeScope::Node);
self
Expand Down Expand Up @@ -364,6 +372,13 @@ impl SecretOperatorVolumeSourceBuilder {
}
}

if let Some(lifetime) = &self.secret_lifetime {
annotations.insert(
Annotation::auto_tls_cert_lifetime(&lifetime.to_string())
.context(ParseAnnotationSnafu)?,
);
}

Ok(EphemeralVolumeSource {
volume_claim_template: Some(PersistentVolumeClaimTemplate {
metadata: Some(ObjectMetaBuilder::new().annotations(annotations).build()),
Expand Down
9 changes: 9 additions & 0 deletions crates/stackable-operator/src/kvp/annotation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,15 @@ impl Annotation {
))?;
Ok(Self(kvp))
}

/// Constructs a `secrets.stackable.tech/backend.autotls.cert.lifetime` annotation.
pub fn auto_tls_cert_lifetime(lifetime: &str) -> Result<Self, AnnotationError> {
let kvp = KeyValuePair::try_from((
"secrets.stackable.tech/backend.autotls.cert.lifetime",
lifetime,
))?;
Ok(Self(kvp))
}
}

/// A validated set/list of Kubernetes annotations.
Expand Down
2 changes: 2 additions & 0 deletions crates/stackable-operator/src/product_config_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ mod tests {

use super::*;
use crate::role_utils::{Role, RoleGroup};
use crate::time::Duration;
use k8s_openapi::api::core::v1::PodTemplateSpec;
use rstest::*;
use std::collections::HashMap;
Expand Down Expand Up @@ -617,6 +618,7 @@ mod tests {
env_overrides: env_overrides.unwrap_or_default(),
cli_overrides: cli_overrides.unwrap_or_default(),
pod_overrides: PodTemplateSpec::default(),
min_secret_lifetime: Duration::default(),
}
}

Expand Down
15 changes: 15 additions & 0 deletions crates/stackable-operator/src/role_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ use crate::{
merge::Merge,
},
product_config_utils::Configuration,
time::Duration,
utils::crds::raw_object_schema,
};
use derivative::Derivative;
Expand Down Expand Up @@ -144,6 +145,18 @@ pub struct CommonConfiguration<T> {
#[serde(default)]
#[schemars(schema_with = "raw_object_schema")]
pub pod_overrides: PodTemplateSpec,

/// The minimum lifetime of secrets generated by the secret operator.
/// Some secrets, such as self signed certificates are constrained to a maximum lifetime by the
/// [SecretClass](DOCS_BASE_URL_PLACEHOLDER/secret-operator/secretclass) object it's self.
/// Currently this property covers self signed certificates but in the future it may be extended to other
/// secret types such as Kerberos keytabs.
#[serde(default = "default_min_secret_lifetime")]
pub min_secret_lifetime: Duration,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Why do we use min_secret_lifetime here instead of just secret_lifetime to be inline with secrets.stackable.tech/backend.autotls.cert.lifetime?

question: Also, do we want it to be about all secrets? Or do we want to support scoped lifetimes for different types of (future) secrets? A more appropriate name would then be cert_lifetime for example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the corresponding proposal I try to explain the reasoning for naming. Other suggestions are welcome though.

As you mention, the idea is that Pod restarts due to secret expiration should be easily understood and transparent from a user's perspective regardless of the type of secrets.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you mention, the idea is that Pod restarts due to secret expiration should be easily understood and transparent from a user's perspective regardless of the type of secrets.

Yeah that makes sense. On the other hand, I can see users wanting different lifetimes for different secrets (if there eventually will be multiple ones for a single group). So a less generic name might be something we have to think about.

Also, why do we prefix the field with min_? It is the lifetime of the secret, not a minimal lifetime which could be longer. Pods will get restarted at exactly at the lifetimes interval.

Copy link
Member

@Techassi Techassi Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'll link this discussion in the proposal.

}

fn default_min_secret_lifetime() -> Duration {
Duration::from_hours_unchecked(24)
}

fn config_schema_default() -> serde_json::Value {
Expand Down Expand Up @@ -203,6 +216,7 @@ where
env_overrides: self.config.env_overrides,
cli_overrides: self.config.cli_overrides,
pod_overrides: self.config.pod_overrides,
min_secret_lifetime: self.config.min_secret_lifetime,
},
role_config: self.role_config,
role_groups: self
Expand All @@ -219,6 +233,7 @@ where
env_overrides: group.config.env_overrides,
cli_overrides: group.config.cli_overrides,
pod_overrides: group.config.pod_overrides,
min_secret_lifetime: group.config.min_secret_lifetime,
},
replicas: group.replicas,
},
Expand Down
8 changes: 8 additions & 0 deletions crates/stackable-operator/src/time/duration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,14 @@ impl Div<u32> for Duration {
}
}

/// This implementation is needed for `CommonConfiguration::min_secret_lifetime`.
/// The default value is arbitrary and should probably not be assumed at call sites.
impl Default for Duration {
fn default() -> Self {
Duration::from_hours_unchecked(1)
}
}

impl Duration {
/// Creates a new [`Duration`] containing the amount of time passed since
/// 1970-01-01 UTC. This can be used to calculate [`Duration`]s based on
Expand Down