From fb36ae988efcbb5ee8559f70075a4cf86af7577e Mon Sep 17 00:00:00 2001 From: dervoeti Date: Fri, 7 Feb 2025 11:41:47 +0100 Subject: [PATCH 1/4] feat: trim optional trailing dot of cluster domain to always get the non-FQDN variant --- rust/operator-binary/src/backend/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/rust/operator-binary/src/backend/mod.rs b/rust/operator-binary/src/backend/mod.rs index 91de3932..25062529 100644 --- a/rust/operator-binary/src/backend/mod.rs +++ b/rust/operator-binary/src/backend/mod.rs @@ -179,7 +179,8 @@ impl SecretVolumeSelector { scope: &scope::SecretScope, ) -> Result, ScopeAddressesError> { use scope_addresses_error::*; - let cluster_domain = &pod_info.kubernetes_cluster_domain; + // Turn FQDNs into bare domain names by removing the trailing dot + let cluster_domain = pod_info.kubernetes_cluster_domain.trim_end_matches("."); let namespace = &self.namespace; Ok(match scope { scope::SecretScope::Node => { @@ -208,7 +209,13 @@ impl SecretVolumeSelector { .listener_addresses .get(name) .context(NoListenerAddressesSnafu { listener: name })? - .to_vec(), + .iter() + .map(|addr| match addr { + // Turn FQDNs into bare domain names by removing the trailing dot + Address::Dns(dns) => Address::Dns(dns.trim_end_matches(".").to_string()), + _ => addr.clone(), + }) + .collect(), }) } From 44739b54c1f7b86971fcfc3b7f805945d4270769 Mon Sep 17 00:00:00 2001 From: dervoeti Date: Fri, 14 Feb 2025 14:05:09 +0100 Subject: [PATCH 2/4] chore: add tests / changelog entry --- CHANGELOG.md | 2 + docs/modules/secret-operator/pages/scope.adoc | 4 +- rust/operator-binary/src/backend/mod.rs | 115 +++++++++++++++++- rust/operator-binary/src/backend/pod_info.rs | 2 +- 4 files changed, 118 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 076a5a40..bff261c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,13 @@ All notable changes to this project will be documented in this file. ### Changed - Default to OCI for image metadata ([#544]). +- [BREAKING] When using a fully qualified domain name, only the variant without the trailing dot is added to the SANs. This should only improve the behavior in scenarios where FQDNs are used and not affect anything else ([#564]). [#528]: https://github.com/stackabletech/secret-operator/pull/528 [#548]: https://github.com/stackabletech/secret-operator/pull/548 [#552]: https://github.com/stackabletech/secret-operator/pull/552 [#544]: https://github.com/stackabletech/secret-operator/pull/544 +[#564]: https://github.com/stackabletech/secret-operator/pull/564 ## [24.11.1] - 2025-01-10 diff --git a/docs/modules/secret-operator/pages/scope.adoc b/docs/modules/secret-operator/pages/scope.adoc index 70ff91f0..cf189854 100644 --- a/docs/modules/secret-operator/pages/scope.adoc +++ b/docs/modules/secret-operator/pages/scope.adoc @@ -59,5 +59,5 @@ For example, a TLS certificate provisioned by the xref:secretclass.adoc#backend- xref:#node[] and xref:#pod[] would contain the following values in its `subjectAlternateName` (SAN) extension field: * The node's IP address -* The node's fully qualified domain name (`my-node.example.com`) -* The pod's fully qualified domain name (`my-pod.my-service.my-namespace.svc.cluster.local`) +* The node's fully qualified domain name (`my-node.example.com`, trailing dots are removed) +* The pod's fully qualified domain name (`my-pod.my-service.my-namespace.svc.cluster.local`, trailing dots are removed) diff --git a/rust/operator-binary/src/backend/mod.rs b/rust/operator-binary/src/backend/mod.rs index 25062529..48d8ecdd 100644 --- a/rust/operator-binary/src/backend/mod.rs +++ b/rust/operator-binary/src/backend/mod.rs @@ -179,7 +179,7 @@ impl SecretVolumeSelector { scope: &scope::SecretScope, ) -> Result, ScopeAddressesError> { use scope_addresses_error::*; - // Turn FQDNs into bare domain names by removing the trailing dot + // Turn FQDNs into bare domain names by removing the trailing dots let cluster_domain = pod_info.kubernetes_cluster_domain.trim_end_matches("."); let namespace = &self.namespace; Ok(match scope { @@ -211,7 +211,7 @@ impl SecretVolumeSelector { .context(NoListenerAddressesSnafu { listener: name })? .iter() .map(|addr| match addr { - // Turn FQDNs into bare domain names by removing the trailing dot + // Turn FQDNs into bare domain names by removing the trailing dots Address::Dns(dns) => Address::Dns(dns.trim_end_matches(".").to_string()), _ => addr.clone(), }) @@ -304,3 +304,114 @@ impl SecretBackendError for Infallible { match *self {} } } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use pod_info::PodInfo; + + use super::*; + + #[test] + fn test_scope_addresses_without_trailing_dot() { + let pod_info = construct_pod_info("cluster.local"); + + assert_eq!( + calculate_scope(&pod_info, &SecretScope::Pod), + vec![ + dns("my-sts.default.svc.cluster.local"), + dns("my-sts-0.my-sts.default.svc.cluster.local"), + ip("10.0.0.42"), + ] + ); + + assert_eq!( + calculate_scope( + &pod_info, + &SecretScope::Service { + name: "my-service".to_owned() + } + ), + vec![dns("my-service.default.svc.cluster.local"),] + ); + + assert_eq!( + calculate_scope(&pod_info, &SecretScope::Node), + vec![dns("my-node"), ip("192.168.0.1"),] + ); + } + + #[test] + fn test_scope_addresses_with_trailing_dot() { + let pod_info = construct_pod_info("custom.cluster.local."); + + assert_eq!( + calculate_scope(&pod_info, &SecretScope::Pod), + vec![ + dns("my-sts.default.svc.custom.cluster.local"), + dns("my-sts-0.my-sts.default.svc.custom.cluster.local"), + ip("10.0.0.42"), + ] + ); + + assert_eq!( + calculate_scope( + &pod_info, + &SecretScope::Service { + name: "my-service".to_owned() + } + ), + vec![ + dns("my-service.default.svc.custom.cluster.local") + ] + ); + + assert_eq!( + calculate_scope(&pod_info, &SecretScope::Node), + vec![dns("my-node"), ip("192.168.0.1"),] + ); + } + + fn construct_pod_info(cluster_domain: &str) -> PodInfo { + PodInfo { + pod_ips: vec!["10.0.0.42".parse().unwrap()], + service_name: Some("my-sts".to_owned()), + node_name: "my-node".to_owned(), + node_ips: vec!["192.168.0.1".parse().unwrap()], + listener_addresses: HashMap::from([]), + kubernetes_cluster_domain: cluster_domain.parse().unwrap(), + scheduling: SchedulingPodInfo { + namespace: "default".to_owned(), + volume_listener_names: HashMap::new(), + has_node_scope: false, + }, + } + } + + fn calculate_scope(pod_info: &PodInfo, scope: &SecretScope) -> Vec
{ + let secret_volume_selector = construct_secret_volume_selector(); + secret_volume_selector + .scope_addresses(pod_info, scope) + .unwrap() + } + + fn dns(dns: &str) -> Address { + Address::Dns(dns.to_owned()) + } + + fn ip(ip: &str) -> Address { + Address::Ip(ip.parse().unwrap()) + } + + fn construct_secret_volume_selector() -> SecretVolumeSelector { + serde_yaml::from_str( + r#" +secrets.stackable.tech/class: tls +csi.storage.k8s.io/pod.name: my-sts-0 +csi.storage.k8s.io/pod.namespace: default + "#, + ) + .unwrap() + } +} diff --git a/rust/operator-binary/src/backend/pod_info.rs b/rust/operator-binary/src/backend/pod_info.rs index 8969e2ba..03417c07 100644 --- a/rust/operator-binary/src/backend/pod_info.rs +++ b/rust/operator-binary/src/backend/pod_info.rs @@ -175,7 +175,7 @@ impl PodInfo { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum Address { Dns(String), Ip(IpAddr), From b5d59f4abc7c6eea0e5ab23f211fba7d999d330a Mon Sep 17 00:00:00 2001 From: dervoeti Date: Fri, 14 Feb 2025 14:58:45 +0100 Subject: [PATCH 3/4] fix: remove trailing dots from node names --- rust/operator-binary/src/backend/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/backend/mod.rs b/rust/operator-binary/src/backend/mod.rs index 48d8ecdd..d66b4c84 100644 --- a/rust/operator-binary/src/backend/mod.rs +++ b/rust/operator-binary/src/backend/mod.rs @@ -184,7 +184,7 @@ impl SecretVolumeSelector { let namespace = &self.namespace; Ok(match scope { scope::SecretScope::Node => { - let mut addrs = vec![Address::Dns(pod_info.node_name.clone())]; + let mut addrs = vec![Address::Dns(pod_info.node_name.trim_end_matches(".").to_owned())]; addrs.extend(pod_info.node_ips.iter().copied().map(Address::Ip)); addrs } From 114343bff07e9027bc39c3a884163e37f79ce317 Mon Sep 17 00:00:00 2001 From: dervoeti Date: Fri, 14 Feb 2025 18:18:56 +0100 Subject: [PATCH 4/4] fix: move logic to tls module --- docs/modules/secret-operator/pages/scope.adoc | 4 +- rust/operator-binary/src/backend/mod.rs | 124 +----------------- rust/operator-binary/src/backend/pod_info.rs | 2 +- rust/operator-binary/src/backend/tls/mod.rs | 8 ++ 4 files changed, 14 insertions(+), 124 deletions(-) diff --git a/docs/modules/secret-operator/pages/scope.adoc b/docs/modules/secret-operator/pages/scope.adoc index cf189854..e1d75380 100644 --- a/docs/modules/secret-operator/pages/scope.adoc +++ b/docs/modules/secret-operator/pages/scope.adoc @@ -59,5 +59,5 @@ For example, a TLS certificate provisioned by the xref:secretclass.adoc#backend- xref:#node[] and xref:#pod[] would contain the following values in its `subjectAlternateName` (SAN) extension field: * The node's IP address -* The node's fully qualified domain name (`my-node.example.com`, trailing dots are removed) -* The pod's fully qualified domain name (`my-pod.my-service.my-namespace.svc.cluster.local`, trailing dots are removed) +* The node's fully qualified domain name (`my-node.example.com`, without a trailing dot) +* The pod's fully qualified domain name (`my-pod.my-service.my-namespace.svc.cluster.local`, without a trailing dot) diff --git a/rust/operator-binary/src/backend/mod.rs b/rust/operator-binary/src/backend/mod.rs index d66b4c84..91de3932 100644 --- a/rust/operator-binary/src/backend/mod.rs +++ b/rust/operator-binary/src/backend/mod.rs @@ -179,12 +179,11 @@ impl SecretVolumeSelector { scope: &scope::SecretScope, ) -> Result, ScopeAddressesError> { use scope_addresses_error::*; - // Turn FQDNs into bare domain names by removing the trailing dots - let cluster_domain = pod_info.kubernetes_cluster_domain.trim_end_matches("."); + let cluster_domain = &pod_info.kubernetes_cluster_domain; let namespace = &self.namespace; Ok(match scope { scope::SecretScope::Node => { - let mut addrs = vec![Address::Dns(pod_info.node_name.trim_end_matches(".").to_owned())]; + let mut addrs = vec![Address::Dns(pod_info.node_name.clone())]; addrs.extend(pod_info.node_ips.iter().copied().map(Address::Ip)); addrs } @@ -209,13 +208,7 @@ impl SecretVolumeSelector { .listener_addresses .get(name) .context(NoListenerAddressesSnafu { listener: name })? - .iter() - .map(|addr| match addr { - // Turn FQDNs into bare domain names by removing the trailing dots - Address::Dns(dns) => Address::Dns(dns.trim_end_matches(".").to_string()), - _ => addr.clone(), - }) - .collect(), + .to_vec(), }) } @@ -304,114 +297,3 @@ impl SecretBackendError for Infallible { match *self {} } } - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use pod_info::PodInfo; - - use super::*; - - #[test] - fn test_scope_addresses_without_trailing_dot() { - let pod_info = construct_pod_info("cluster.local"); - - assert_eq!( - calculate_scope(&pod_info, &SecretScope::Pod), - vec![ - dns("my-sts.default.svc.cluster.local"), - dns("my-sts-0.my-sts.default.svc.cluster.local"), - ip("10.0.0.42"), - ] - ); - - assert_eq!( - calculate_scope( - &pod_info, - &SecretScope::Service { - name: "my-service".to_owned() - } - ), - vec![dns("my-service.default.svc.cluster.local"),] - ); - - assert_eq!( - calculate_scope(&pod_info, &SecretScope::Node), - vec![dns("my-node"), ip("192.168.0.1"),] - ); - } - - #[test] - fn test_scope_addresses_with_trailing_dot() { - let pod_info = construct_pod_info("custom.cluster.local."); - - assert_eq!( - calculate_scope(&pod_info, &SecretScope::Pod), - vec![ - dns("my-sts.default.svc.custom.cluster.local"), - dns("my-sts-0.my-sts.default.svc.custom.cluster.local"), - ip("10.0.0.42"), - ] - ); - - assert_eq!( - calculate_scope( - &pod_info, - &SecretScope::Service { - name: "my-service".to_owned() - } - ), - vec![ - dns("my-service.default.svc.custom.cluster.local") - ] - ); - - assert_eq!( - calculate_scope(&pod_info, &SecretScope::Node), - vec![dns("my-node"), ip("192.168.0.1"),] - ); - } - - fn construct_pod_info(cluster_domain: &str) -> PodInfo { - PodInfo { - pod_ips: vec!["10.0.0.42".parse().unwrap()], - service_name: Some("my-sts".to_owned()), - node_name: "my-node".to_owned(), - node_ips: vec!["192.168.0.1".parse().unwrap()], - listener_addresses: HashMap::from([]), - kubernetes_cluster_domain: cluster_domain.parse().unwrap(), - scheduling: SchedulingPodInfo { - namespace: "default".to_owned(), - volume_listener_names: HashMap::new(), - has_node_scope: false, - }, - } - } - - fn calculate_scope(pod_info: &PodInfo, scope: &SecretScope) -> Vec
{ - let secret_volume_selector = construct_secret_volume_selector(); - secret_volume_selector - .scope_addresses(pod_info, scope) - .unwrap() - } - - fn dns(dns: &str) -> Address { - Address::Dns(dns.to_owned()) - } - - fn ip(ip: &str) -> Address { - Address::Ip(ip.parse().unwrap()) - } - - fn construct_secret_volume_selector() -> SecretVolumeSelector { - serde_yaml::from_str( - r#" -secrets.stackable.tech/class: tls -csi.storage.k8s.io/pod.name: my-sts-0 -csi.storage.k8s.io/pod.namespace: default - "#, - ) - .unwrap() - } -} diff --git a/rust/operator-binary/src/backend/pod_info.rs b/rust/operator-binary/src/backend/pod_info.rs index 03417c07..8969e2ba 100644 --- a/rust/operator-binary/src/backend/pod_info.rs +++ b/rust/operator-binary/src/backend/pod_info.rs @@ -175,7 +175,7 @@ impl PodInfo { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone)] pub enum Address { Dns(String), Ip(IpAddr), diff --git a/rust/operator-binary/src/backend/tls/mod.rs b/rust/operator-binary/src/backend/tls/mod.rs index f479c09f..9ca2bc72 100644 --- a/rust/operator-binary/src/backend/tls/mod.rs +++ b/rust/operator-binary/src/backend/tls/mod.rs @@ -252,6 +252,14 @@ impl SecretBackend for TlsGenerate { .context(ScopeAddressesSnafu { scope })?, ); } + for address in &mut addresses { + if let Address::Dns(dns) = address { + // Turn FQDNs into bare domain names by removing the trailing dot + if dns.ends_with('.') { + dns.pop(); + } + } + } let ca = self .ca_manager .find_certificate_authority_for_signing(not_after)