From 0f2c0a33ecd48064082221737f25d820351a9ff4 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Mon, 6 Jul 2026 15:15:23 +0200 Subject: [PATCH 01/13] feat: Take dependencies into account when sorting environment variables --- crates/stackable-operator/CHANGELOG.md | 7 + .../src/v2/builder/pod/container.rs | 437 +++++++++++++++++- 2 files changed, 441 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 8f50b4a44..ee81f4b5d 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- [v2] BREAKING: Converting an `EnvVarSet` into a `Vec` takes dependencies between the + environment variables into account ([#1249]). + +[#1249]: https://github.com/stackabletech/operator-rs/pull/1249 + ## [0.113.4] - 2026-07-09 ### Changed diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index ff2e529c8..ac5d94bc9 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -1,5 +1,9 @@ -use std::collections::{BTreeMap, btree_map}; +use std::{ + collections::{BTreeMap, BTreeSet, btree_map}, + str::FromStr, +}; +use regex::Regex; use snafu::Snafu; use strum::{EnumDiscriminants, IntoStaticStr}; @@ -7,7 +11,10 @@ use crate::{ attributed_string_type, builder::pod::container::{ContainerBuilder, FieldPathEnvVar}, k8s_openapi::api::core::v1::{ConfigMapKeySelector, EnvVar, EnvVarSource, ObjectFieldSelector}, - v2::types::kubernetes::{ConfigMapKey, ConfigMapName, ContainerName}, + v2::{ + macros::attributed_string_type, + types::kubernetes::{ConfigMapKey, ConfigMapName, ContainerName}, + }, }; #[derive(Snafu, Debug, EnumDiscriminants)] @@ -60,6 +67,15 @@ impl EnvVarSet { self } + /// Adds the given [`EnvVar`] to this set + /// + /// An [`EnvVar`] with the same name is overridden. + pub fn with_env_var(mut self, env_var: EnvVar) -> Result { + self.0.insert(EnvVarName::from_str(&env_var.name)?, env_var); + + Ok(self) + } + /// Adds the given [`EnvVar`]s to this set /// /// [`EnvVar`]s with the same name are overridden. @@ -144,7 +160,335 @@ impl EnvVarSet { impl From for Vec { fn from(value: EnvVarSet) -> Self { - value.0.values().cloned().collect() + let mut env_var_closure = EnvVarDependencyResolver::new(&value, 10); + + let mut vec: Self = value.0.values().cloned().collect(); + vec.sort_by_key(|env_var| env_var_closure.sort_key(env_var)); + vec + } +} + +/// Resolves dependencies between environment variables and provides sort keys which take these +/// dependencies into account +pub struct EnvVarDependencyResolver<'a> { + /// [EnvVarSet] with possibly dependent environment variables + env_vars: &'a EnvVarSet, + + /// Maximum recursion depth + /// + /// Long dependency chains could slow down the operator. + max_recursion_depth: usize, + + /// Pattern for an escaped environment variable reference, e.g. `$$(ESCAPED_REFERENCE)` + escaped_env_vars_pattern: Regex, + + /// Pattern for a referenced environment variable, e.g. `$(ENV_VAR)` + referenced_env_vars_pattern: Regex, +} + +impl<'a> EnvVarDependencyResolver<'a> { + pub fn new(env_vars: &'a EnvVarSet, max_recursion_depth: usize) -> Self { + Self { + env_vars, + max_recursion_depth, + escaped_env_vars_pattern: Regex::new(r"\$\$\([^\)]*\)") + .expect("should be a valid regular expression"), + referenced_env_vars_pattern: Regex::new(r"\$\(([^\)]+)\)") + .expect("should be a valid regular expression"), + } + } + + /// Returns a sort key for the given environment variable which considers dependencies to other + /// environment variables + /// + /// # Example + /// + /// ```rust + /// # use std::{ + /// # collections::BTreeSet, + /// # str::FromStr, + /// # }; + /// # use stackable_operator::{ + /// # k8s_openapi::api::core::v1::{ + /// # EnvVar, EnvVarSource, ObjectFieldSelector + /// # }, + /// # v2::builder::pod::container::{ + /// # EnvVarDependencyResolver, EnvVarName, EnvVarSet + /// # }, + /// # }; + /// + /// let env_var1 = EnvVar { + /// name: "ENV1".to_owned(), + /// value: Some("references to $(ENV2) and $(ENV4)".to_owned()), + /// value_from: None, + /// }; + /// let env_var2 = EnvVar { + /// name: "ENV2".to_owned(), + /// value: Some("reference to $(ENV4)".to_owned()), + /// value_from: None, + /// }; + /// let env_var3 = EnvVar { + /// name: "ENV3".to_owned(), + /// value: Some("reference to $(ENV4)".to_owned()), + /// value_from: None, + /// }; + /// let env_var4 = EnvVar { + /// name: "ENV4".to_owned(), + /// value: None, + /// value_from: Some(EnvVarSource { + /// field_ref: Some(ObjectFieldSelector { + /// field_path: "metadata.name".to_owned(), + /// ..ObjectFieldSelector::default() + /// }), + /// ..EnvVarSource::default() + /// }), + /// }; + /// let env_var5 = EnvVar { + /// name: "ENV5".to_owned(), + /// value: Some("self reference to $(ENV5)".to_owned()), + /// value_from: None, + /// }; + /// + /// let env_vars = EnvVarSet::new() + /// .with_env_var(env_var1.clone()) + /// .unwrap() + /// .with_env_var(env_var2.clone()) + /// .unwrap() + /// .with_env_var(env_var3.clone()) + /// .unwrap() + /// .with_env_var(env_var4.clone()) + /// .unwrap() + /// .with_env_var(env_var5.clone()) + /// .unwrap(); + /// + /// let mut resolver = EnvVarDependencyResolver::new(&env_vars, 2); + /// assert_eq!( + /// vec!["ENV4".to_owned(), "ENV2".to_owned(), "ENV1".to_owned()], + /// resolver.sort_key(&env_var1) + /// ); + /// assert_eq!( + /// vec!["ENV4".to_owned(), "ENV2".to_owned()], + /// resolver.sort_key(&env_var2) + /// ); + /// assert_eq!( + /// vec!["ENV4".to_owned(), "ENV3".to_owned()], + /// resolver.sort_key(&env_var3) + /// ); + /// assert_eq!(vec!["ENV4".to_owned()], resolver.sort_key(&env_var4)); + /// assert_eq!(vec!["ENV5".to_owned()], resolver.sort_key(&env_var5)); + /// ``` + pub fn sort_key(&mut self, env_var: &EnvVar) -> Vec { + if let Some(mut closure) = self.calculate_closure(env_var) { + // Add the name of the variable to its closure to make the set unique for every + // variable. + closure.insert(env_var.name.clone()); + + closure.into_iter().rev().collect() + } else { + vec![env_var.name.clone()] + } + } + + /// Calculates the transitive closure of referenced environment variables + /// + /// If the given environment variable is part of a reference cycle or a reference chain longer + /// than the maximum recursion depth, then `None` is returned. + /// + /// # Example + /// + /// ```rust + /// # use std::{ + /// # collections::BTreeSet, + /// # str::FromStr, + /// # }; + /// # use stackable_operator::{ + /// # k8s_openapi::api::core::v1::{ + /// # EnvVar, EnvVarSource, ObjectFieldSelector + /// # }, + /// # v2::builder::pod::container::{ + /// # EnvVarDependencyResolver, EnvVarName, EnvVarSet + /// # }, + /// # }; + /// + /// let env_var1 = EnvVar { + /// name: "ENV1".to_owned(), + /// value: Some("references to $(ENV2) and $(ENV4)".to_owned()), + /// value_from: None, + /// }; + /// let env_var2 = EnvVar { + /// name: "ENV2".to_owned(), + /// value: Some("reference to $(ENV4)".to_owned()), + /// value_from: None, + /// }; + /// let env_var3 = EnvVar { + /// name: "ENV3".to_owned(), + /// value: Some("reference to $(ENV4)".to_owned()), + /// value_from: None, + /// }; + /// let env_var4 = EnvVar { + /// name: "ENV4".to_owned(), + /// value: None, + /// value_from: Some(EnvVarSource { + /// field_ref: Some(ObjectFieldSelector { + /// field_path: "metadata.name".to_owned(), + /// ..ObjectFieldSelector::default() + /// }), + /// ..EnvVarSource::default() + /// }), + /// }; + /// let env_var5 = EnvVar { + /// name: "ENV5".to_owned(), + /// value: Some("self reference to $(ENV5)".to_owned()), + /// value_from: None, + /// }; + /// let env_var6 = EnvVar { + /// name: "ENV6".to_owned(), + /// value: Some("cyclic reference to $(ENV7)".to_owned()), + /// value_from: None, + /// }; + /// let env_var7 = EnvVar { + /// name: "ENV7".to_owned(), + /// value: Some("cyclic reference to $(ENV6)".to_owned()), + /// value_from: None, + /// }; + /// let env_var8 = EnvVar { + /// name: "ENV8".to_owned(), + /// value: Some("long reference chain to $(ENV1)".to_owned()), + /// value_from: None, + /// }; + /// + /// let env_vars = EnvVarSet::new() + /// .with_env_var(env_var1.clone()) + /// .unwrap() + /// .with_env_var(env_var2.clone()) + /// .unwrap() + /// .with_env_var(env_var3.clone()) + /// .unwrap() + /// .with_env_var(env_var4.clone()) + /// .unwrap() + /// .with_env_var(env_var5.clone()) + /// .unwrap() + /// .with_env_var(env_var6.clone()) + /// .unwrap() + /// .with_env_var(env_var7.clone()) + /// .unwrap() + /// .with_env_var(env_var8.clone()) + /// .unwrap(); + /// + /// let mut resolver = EnvVarDependencyResolver::new(&env_vars, 2); + /// assert_eq!( + /// Some(BTreeSet::from(["ENV2".to_owned(), "ENV4".to_owned()])), + /// resolver.calculate_closure(&env_var1) + /// ); + /// assert_eq!( + /// Some(BTreeSet::from(["ENV4".to_owned()])), + /// resolver.calculate_closure(&env_var2) + /// ); + /// assert_eq!( + /// Some(BTreeSet::from(["ENV4".to_owned()])), + /// resolver.calculate_closure(&env_var3) + /// ); + /// assert_eq!(Some(BTreeSet::new()), resolver.calculate_closure(&env_var4)); + /// assert_eq!(None, resolver.calculate_closure(&env_var5)); + /// assert_eq!(None, resolver.calculate_closure(&env_var6)); + /// assert_eq!(None, resolver.calculate_closure(&env_var7)); + /// assert_eq!(None, resolver.calculate_closure(&env_var8)); + /// ``` + pub fn calculate_closure(&mut self, env_var: &EnvVar) -> Option> { + self.calculate_closure_rec(env_var, self.max_recursion_depth) + } + + fn calculate_closure_rec( + &mut self, + env_var: &EnvVar, + remaining_recursion_depth: usize, + ) -> Option> { + if env_var.value.is_none() { + Some(BTreeSet::new()) + } else if let Some(value) = &env_var.value + && remaining_recursion_depth > 0 + { + let mut closure = BTreeSet::new(); + + for referenced_env_var in self.referenced_env_vars(value) { + closure.insert(referenced_env_var.name.clone()); + closure.extend( + self.calculate_closure_rec(referenced_env_var, remaining_recursion_depth - 1)?, + ); + } + + Some(closure) + } else { + None + } + } + + /// Returns the directly referenced environment variables + /// + /// # Example + /// + /// ```rust + /// # use std::str::FromStr; + /// # use stackable_operator::{ + /// # k8s_openapi::api::core::v1::EnvVar, + /// # v2::builder::pod::container::{ + /// # EnvVarDependencyResolver, EnvVarName, EnvVarSet + /// # }, + /// # }; + /// + /// let env_vars = EnvVarSet::new().with_values([ + /// (EnvVarName::from_str("ENV1").unwrap(), "value 1"), + /// (EnvVarName::from_str("ENV2").unwrap(), "value 2"), + /// (EnvVarName::from_str("ENV3").unwrap(), "value 3"), + /// ]); + /// + /// let resolver = EnvVarDependencyResolver::new(&env_vars, 10); + /// + /// assert_eq!( + /// Vec::<&EnvVar>::new(), + /// resolver.referenced_env_vars("no references") + /// ); + /// assert_eq!( + /// vec![ + /// &EnvVar { + /// name: "ENV2".to_owned(), + /// value: Some("value 2".to_owned()), + /// value_from: None + /// }, + /// &EnvVar { + /// name: "ENV3".to_owned(), + /// value: Some("value 3".to_owned()), + /// value_from: None + /// }, + /// ], + /// resolver.referenced_env_vars("references to $(ENV2) and $(ENV3)") + /// ); + /// assert_eq!( + /// vec![&EnvVar { + /// name: "ENV1".to_owned(), + /// value: Some("value 1".to_owned()), + /// value_from: None + /// }], + /// resolver.referenced_env_vars("reference to $(ENV1) and escaped reference to $$(ENV2)") + /// ); + /// assert_eq!( + /// vec![&EnvVar { + /// name: "ENV1".to_owned(), + /// value: Some("value 1".to_owned()), + /// value_from: None + /// }], + /// resolver.referenced_env_vars("reference to $(ENV1) and invalid reference to $(ENV4)") + /// ); + /// ``` + pub fn referenced_env_vars(&self, value: &str) -> Vec<&'a EnvVar> { + let value_without_escapes = self.escaped_env_vars_pattern.replace_all(value, ""); + + self.referenced_env_vars_pattern + .captures_iter(&value_without_escapes) + .filter_map(|capture| capture.get(1)) + .filter_map(|regex_match| EnvVarName::from_str(regex_match.as_str()).ok()) + .filter_map(|env_var_name| self.env_vars.0.get(&env_var_name)) + .collect() } } @@ -346,4 +690,91 @@ mod tests { env_var_set.get(&EnvVarName::from_str_unsafe("ENV")) ); } + + #[test] + fn test_envvarset_with_references() { + let env_var_set = EnvVarSet::new() + .with_value(&EnvVarName::from_str_unsafe("ENV1"), "value1") + // valid reference to a later variable + .with_value(&EnvVarName::from_str_unsafe("ENV2"), "$(ENV3)") + // valid reference to a later variable + .with_value(&EnvVarName::from_str_unsafe("ENV3"), "$(ENV4)") + // valid reference to an earlier variable + .with_value(&EnvVarName::from_str_unsafe("ENV4"), "$(ENV1)") + // invalid reference + .with_value(&EnvVarName::from_str_unsafe("ENV5"), "$(ENV?)") + // Same keys are allowed in Kubernetes, but not in `EnvVarSet`. + // The existing ENV1 is overridden. + .with_value(&EnvVarName::from_str_unsafe("ENV6"), "value6") + .with_value(&EnvVarName::from_str_unsafe("ENV6"), "$(ENV6)") + // multiple references + .with_value( + &EnvVarName::from_str_unsafe("ENV7"), + "$(ENV5) $(ENV8) $(ENV2)", + ) + // multiple references with escaped and invalid references + .with_value( + &EnvVarName::from_str_unsafe("ENV8"), + "$(ENV1) $$(ENV9) $() $(ENV2)", + ) + // No value + .with_field_path(&EnvVarName::from_str_unsafe("ENV9"), &FieldPathEnvVar::Name); + + assert_eq!( + vec![ + EnvVar { + name: "ENV1".to_owned(), + value: Some("value1".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV4".to_owned(), + value: Some("$(ENV1)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV3".to_owned(), + value: Some("$(ENV4)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV2".to_owned(), + value: Some("$(ENV3)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV5".to_owned(), + value: Some("$(ENV?)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV6".to_owned(), + value: Some("$(ENV6)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV8".to_owned(), + value: Some("$(ENV1) $$(ENV9) $() $(ENV2)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV7".to_owned(), + value: Some("$(ENV5) $(ENV8) $(ENV2)".to_owned()), + value_from: None + }, + EnvVar { + name: "ENV9".to_owned(), + value: None, + value_from: Some(EnvVarSource { + field_ref: Some(ObjectFieldSelector { + field_path: FieldPathEnvVar::Name.to_string(), + ..ObjectFieldSelector::default() + }), + ..EnvVarSource::default() + }), + }, + ], + Vec::from(env_var_set) + ); + } } From 209d08192f3df7d8a7772a532f78445fbdbcc794 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 10:00:56 +0200 Subject: [PATCH 02/13] feat: Use sorting algorithm for IntoIterator for EnvVarSet --- .../src/v2/builder/pod/container.rs | 133 ++++++++---------- 1 file changed, 56 insertions(+), 77 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index ac5d94bc9..fe5fa12e3 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -1,6 +1,7 @@ use std::{ - collections::{BTreeMap, BTreeSet, btree_map}, + collections::{BTreeMap, BTreeSet}, str::FromStr, + vec, }; use regex::Regex; @@ -168,6 +169,15 @@ impl From for Vec { } } +impl IntoIterator for EnvVarSet { + type IntoIter = vec::IntoIter; + type Item = EnvVar; + + fn into_iter(self) -> Self::IntoIter { + Vec::from(self).into_iter() + } +} + /// Resolves dependencies between environment variables and provides sort keys which take these /// dependencies into account pub struct EnvVarDependencyResolver<'a> { @@ -492,15 +502,6 @@ impl<'a> EnvVarDependencyResolver<'a> { } } -impl IntoIterator for EnvVarSet { - type IntoIter = btree_map::IntoValues; - type Item = EnvVar; - - fn into_iter(self) -> Self::IntoIter { - self.0.into_values() - } -} - #[cfg(test)] mod tests { use std::str::FromStr; @@ -692,89 +693,67 @@ mod tests { } #[test] - fn test_envvarset_with_references() { + fn test_vec_envvar_from_envvarset() { let env_var_set = EnvVarSet::new() - .with_value(&EnvVarName::from_str_unsafe("ENV1"), "value1") - // valid reference to a later variable - .with_value(&EnvVarName::from_str_unsafe("ENV2"), "$(ENV3)") - // valid reference to a later variable - .with_value(&EnvVarName::from_str_unsafe("ENV3"), "$(ENV4)") - // valid reference to an earlier variable - .with_value(&EnvVarName::from_str_unsafe("ENV4"), "$(ENV1)") - // invalid reference - .with_value(&EnvVarName::from_str_unsafe("ENV5"), "$(ENV?)") - // Same keys are allowed in Kubernetes, but not in `EnvVarSet`. - // The existing ENV1 is overridden. - .with_value(&EnvVarName::from_str_unsafe("ENV6"), "value6") - .with_value(&EnvVarName::from_str_unsafe("ENV6"), "$(ENV6)") - // multiple references - .with_value( - &EnvVarName::from_str_unsafe("ENV7"), - "$(ENV5) $(ENV8) $(ENV2)", - ) - // multiple references with escaped and invalid references - .with_value( - &EnvVarName::from_str_unsafe("ENV8"), - "$(ENV1) $$(ENV9) $() $(ENV2)", - ) - // No value - .with_field_path(&EnvVarName::from_str_unsafe("ENV9"), &FieldPathEnvVar::Name); + .with_value(&EnvVarName::from_str_unsafe("ENV1"), "$(ENV2)") + .with_value(&EnvVarName::from_str_unsafe("ENV2"), "value 2") + .with_value(&EnvVarName::from_str_unsafe("ENV3"), "value 3"); assert_eq!( vec![ - EnvVar { - name: "ENV1".to_owned(), - value: Some("value1".to_owned()), - value_from: None - }, - EnvVar { - name: "ENV4".to_owned(), - value: Some("$(ENV1)".to_owned()), - value_from: None - }, - EnvVar { - name: "ENV3".to_owned(), - value: Some("$(ENV4)".to_owned()), - value_from: None - }, EnvVar { name: "ENV2".to_owned(), - value: Some("$(ENV3)".to_owned()), - value_from: None - }, - EnvVar { - name: "ENV5".to_owned(), - value: Some("$(ENV?)".to_owned()), + value: Some("value 2".to_owned()), value_from: None }, EnvVar { - name: "ENV6".to_owned(), - value: Some("$(ENV6)".to_owned()), - value_from: None - }, - EnvVar { - name: "ENV8".to_owned(), - value: Some("$(ENV1) $$(ENV9) $() $(ENV2)".to_owned()), + name: "ENV1".to_owned(), + value: Some("$(ENV2)".to_owned()), value_from: None }, EnvVar { - name: "ENV7".to_owned(), - value: Some("$(ENV5) $(ENV8) $(ENV2)".to_owned()), + name: "ENV3".to_owned(), + value: Some("value 3".to_owned()), value_from: None }, - EnvVar { - name: "ENV9".to_owned(), - value: None, - value_from: Some(EnvVarSource { - field_ref: Some(ObjectFieldSelector { - field_path: FieldPathEnvVar::Name.to_string(), - ..ObjectFieldSelector::default() - }), - ..EnvVarSource::default() - }), - }, ], Vec::from(env_var_set) ); } + + #[test] + fn test_envvarset_intoiterator() { + let env_var_set = EnvVarSet::new() + .with_value(&EnvVarName::from_str_unsafe("ENV1"), "$(ENV2)") + .with_value(&EnvVarName::from_str_unsafe("ENV2"), "value 2") + .with_value(&EnvVarName::from_str_unsafe("ENV3"), "value 3"); + + let mut iter = env_var_set.into_iter(); + + assert_eq!( + Some(EnvVar { + name: "ENV2".to_owned(), + value: Some("value 2".to_owned()), + value_from: None + }), + iter.next() + ); + assert_eq!( + Some(EnvVar { + name: "ENV1".to_owned(), + value: Some("$(ENV2)".to_owned()), + value_from: None + }), + iter.next() + ); + assert_eq!( + Some(EnvVar { + name: "ENV3".to_owned(), + value: Some("value 3".to_owned()), + value_from: None + }), + iter.next() + ); + assert_eq!(None, iter.next()); + } } From 0f21c80cfbee2feae60ba4538d15e42e59adad6d Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 10:01:59 +0200 Subject: [PATCH 03/13] chore: Upgrade spin --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c6c504fd2..56e176500 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3632,9 +3632,9 @@ dependencies = [ [[package]] name = "spin" -version = "0.9.8" +version = "0.9.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" +checksum = "3763264f6b73151db08c50ff20d7d8a0b8796e021cdea7ceedad07b80155fa0e" [[package]] name = "spki" From e4c5cf8b6798d09cfebe9455931104e1e826c4cf Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 10:19:22 +0200 Subject: [PATCH 04/13] chore: Use LazyLock for regexes --- .../src/v2/builder/pod/container.rs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index fe5fa12e3..0d8778312 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -1,6 +1,7 @@ use std::{ collections::{BTreeMap, BTreeSet}, str::FromStr, + sync::LazyLock, vec, }; @@ -18,6 +19,14 @@ use crate::{ }, }; +/// Pattern for an escaped environment variable reference, e.g. `$$(ESCAPED_REFERENCE)` +static ESCAPED_ENV_VARS_PATTERN: LazyLock = + LazyLock::new(|| Regex::new(r"\$\$\([^\)]*\)").expect("should be a valid regular expression")); + +/// Pattern for a referenced environment variable, e.g. `$(ENV_VAR)` +static REFERENCED_ENV_VARS_PATTERN: LazyLock = + LazyLock::new(|| Regex::new(r"\$\(([^\)]+)\)").expect("should be a valid regular expression")); + #[derive(Snafu, Debug, EnumDiscriminants)] #[strum_discriminants(derive(IntoStaticStr))] pub enum Error { @@ -188,12 +197,6 @@ pub struct EnvVarDependencyResolver<'a> { /// /// Long dependency chains could slow down the operator. max_recursion_depth: usize, - - /// Pattern for an escaped environment variable reference, e.g. `$$(ESCAPED_REFERENCE)` - escaped_env_vars_pattern: Regex, - - /// Pattern for a referenced environment variable, e.g. `$(ENV_VAR)` - referenced_env_vars_pattern: Regex, } impl<'a> EnvVarDependencyResolver<'a> { @@ -201,10 +204,6 @@ impl<'a> EnvVarDependencyResolver<'a> { Self { env_vars, max_recursion_depth, - escaped_env_vars_pattern: Regex::new(r"\$\$\([^\)]*\)") - .expect("should be a valid regular expression"), - referenced_env_vars_pattern: Regex::new(r"\$\(([^\)]+)\)") - .expect("should be a valid regular expression"), } } @@ -491,9 +490,9 @@ impl<'a> EnvVarDependencyResolver<'a> { /// ); /// ``` pub fn referenced_env_vars(&self, value: &str) -> Vec<&'a EnvVar> { - let value_without_escapes = self.escaped_env_vars_pattern.replace_all(value, ""); + let value_without_escapes = ESCAPED_ENV_VARS_PATTERN.replace_all(value, ""); - self.referenced_env_vars_pattern + REFERENCED_ENV_VARS_PATTERN .captures_iter(&value_without_escapes) .filter_map(|capture| capture.get(1)) .filter_map(|regex_match| EnvVarName::from_str(regex_match.as_str()).ok()) From b5e94bc240059f6978e4c381faeeba2dd2de179b Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 10:22:03 +0200 Subject: [PATCH 05/13] chore: Use sort_by_cached_key --- crates/stackable-operator/src/v2/builder/pod/container.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index 0d8778312..5270f37c3 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -173,7 +173,7 @@ impl From for Vec { let mut env_var_closure = EnvVarDependencyResolver::new(&value, 10); let mut vec: Self = value.0.values().cloned().collect(); - vec.sort_by_key(|env_var| env_var_closure.sort_key(env_var)); + vec.sort_by_cached_key(|env_var| env_var_closure.sort_key(env_var)); vec } } From b4779a29d23a9f88e8f568d89375bbe49ccfe7ec Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 10:42:00 +0200 Subject: [PATCH 06/13] fix: escaped dollar signs --- .../src/v2/builder/pod/container.rs | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index 5270f37c3..afa8fa7af 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -19,9 +19,9 @@ use crate::{ }, }; -/// Pattern for an escaped environment variable reference, e.g. `$$(ESCAPED_REFERENCE)` -static ESCAPED_ENV_VARS_PATTERN: LazyLock = - LazyLock::new(|| Regex::new(r"\$\$\([^\)]*\)").expect("should be a valid regular expression")); +/// Pattern for an escaped dollar sign, e.g. `$$` +static ESCAPED_DOLLAR_SIGN_PATTERN: LazyLock = + LazyLock::new(|| Regex::new(r"\$\$").expect("should be a valid regular expression")); /// Pattern for a referenced environment variable, e.g. `$(ENV_VAR)` static REFERENCED_ENV_VARS_PATTERN: LazyLock = @@ -170,10 +170,10 @@ impl EnvVarSet { impl From for Vec { fn from(value: EnvVarSet) -> Self { - let mut env_var_closure = EnvVarDependencyResolver::new(&value, 10); + let env_var_closure = EnvVarDependencyResolver::new(&value, 10); let mut vec: Self = value.0.values().cloned().collect(); - vec.sort_by_cached_key(|env_var| env_var_closure.sort_key(env_var)); + vec.sort_by_key(|env_var| env_var_closure.sort_key(env_var)); vec } } @@ -222,7 +222,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// # EnvVar, EnvVarSource, ObjectFieldSelector /// # }, /// # v2::builder::pod::container::{ - /// # EnvVarDependencyResolver, EnvVarName, EnvVarSet + /// # EnvVarDependencyResolver, EnvVarSet /// # }, /// # }; /// @@ -270,7 +270,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// .with_env_var(env_var5.clone()) /// .unwrap(); /// - /// let mut resolver = EnvVarDependencyResolver::new(&env_vars, 2); + /// let resolver = EnvVarDependencyResolver::new(&env_vars, 2); /// assert_eq!( /// vec!["ENV4".to_owned(), "ENV2".to_owned(), "ENV1".to_owned()], /// resolver.sort_key(&env_var1) @@ -286,7 +286,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// assert_eq!(vec!["ENV4".to_owned()], resolver.sort_key(&env_var4)); /// assert_eq!(vec!["ENV5".to_owned()], resolver.sort_key(&env_var5)); /// ``` - pub fn sort_key(&mut self, env_var: &EnvVar) -> Vec { + pub fn sort_key(&self, env_var: &EnvVar) -> Vec { if let Some(mut closure) = self.calculate_closure(env_var) { // Add the name of the variable to its closure to make the set unique for every // variable. @@ -315,7 +315,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// # EnvVar, EnvVarSource, ObjectFieldSelector /// # }, /// # v2::builder::pod::container::{ - /// # EnvVarDependencyResolver, EnvVarName, EnvVarSet + /// # EnvVarDependencyResolver, EnvVarSet /// # }, /// # }; /// @@ -384,7 +384,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// .with_env_var(env_var8.clone()) /// .unwrap(); /// - /// let mut resolver = EnvVarDependencyResolver::new(&env_vars, 2); + /// let resolver = EnvVarDependencyResolver::new(&env_vars, 2); /// assert_eq!( /// Some(BTreeSet::from(["ENV2".to_owned(), "ENV4".to_owned()])), /// resolver.calculate_closure(&env_var1) @@ -403,12 +403,12 @@ impl<'a> EnvVarDependencyResolver<'a> { /// assert_eq!(None, resolver.calculate_closure(&env_var7)); /// assert_eq!(None, resolver.calculate_closure(&env_var8)); /// ``` - pub fn calculate_closure(&mut self, env_var: &EnvVar) -> Option> { + pub fn calculate_closure(&self, env_var: &EnvVar) -> Option> { self.calculate_closure_rec(env_var, self.max_recursion_depth) } fn calculate_closure_rec( - &mut self, + &self, env_var: &EnvVar, remaining_recursion_depth: usize, ) -> Option> { @@ -449,6 +449,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// (EnvVarName::from_str("ENV1").unwrap(), "value 1"), /// (EnvVarName::from_str("ENV2").unwrap(), "value 2"), /// (EnvVarName::from_str("ENV3").unwrap(), "value 3"), + /// (EnvVarName::from_str("ENV4").unwrap(), "value 4"), /// ]); /// /// let resolver = EnvVarDependencyResolver::new(&env_vars, 10); @@ -473,12 +474,21 @@ impl<'a> EnvVarDependencyResolver<'a> { /// resolver.referenced_env_vars("references to $(ENV2) and $(ENV3)") /// ); /// assert_eq!( - /// vec![&EnvVar { - /// name: "ENV1".to_owned(), - /// value: Some("value 1".to_owned()), - /// value_from: None - /// }], - /// resolver.referenced_env_vars("reference to $(ENV1) and escaped reference to $$(ENV2)") + /// vec![ + /// &EnvVar { + /// name: "ENV1".to_owned(), + /// value: Some("value 1".to_owned()), + /// value_from: None + /// }, + /// &EnvVar { + /// name: "ENV2".to_owned(), + /// value: Some("value 2".to_owned()), + /// value_from: None + /// }, + /// ], + /// resolver.referenced_env_vars( + /// "references to $(ENV1) and $$$(ENV2) and escaped references to $$(ENV3) and $$$$(ENV4)" + /// ) /// ); /// assert_eq!( /// vec![&EnvVar { @@ -486,11 +496,11 @@ impl<'a> EnvVarDependencyResolver<'a> { /// value: Some("value 1".to_owned()), /// value_from: None /// }], - /// resolver.referenced_env_vars("reference to $(ENV1) and invalid reference to $(ENV4)") + /// resolver.referenced_env_vars("reference to $(ENV1) and invalid reference to $(ENV5)") /// ); /// ``` pub fn referenced_env_vars(&self, value: &str) -> Vec<&'a EnvVar> { - let value_without_escapes = ESCAPED_ENV_VARS_PATTERN.replace_all(value, ""); + let value_without_escapes = ESCAPED_DOLLAR_SIGN_PATTERN.replace_all(value, ""); REFERENCED_ENV_VARS_PATTERN .captures_iter(&value_without_escapes) From 95be0fab103af987cb208330f40c9ec4f1ca6885 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 11:30:25 +0200 Subject: [PATCH 07/13] chore: Use local error type --- .../src/v2/builder/pod/container.rs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index afa8fa7af..39c38c6ce 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -6,17 +6,14 @@ use std::{ }; use regex::Regex; -use snafu::Snafu; +use snafu::{ResultExt, Snafu}; use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ attributed_string_type, builder::pod::container::{ContainerBuilder, FieldPathEnvVar}, k8s_openapi::api::core::v1::{ConfigMapKeySelector, EnvVar, EnvVarSource, ObjectFieldSelector}, - v2::{ - macros::attributed_string_type, - types::kubernetes::{ConfigMapKey, ConfigMapName, ContainerName}, - }, + v2::types::kubernetes::{ConfigMapKey, ConfigMapName, ContainerName}, }; /// Pattern for an escaped dollar sign, e.g. `$$` @@ -27,6 +24,11 @@ static ESCAPED_DOLLAR_SIGN_PATTERN: LazyLock = static REFERENCED_ENV_VARS_PATTERN: LazyLock = LazyLock::new(|| Regex::new(r"\$\(([^\)]+)\)").expect("should be a valid regular expression")); +/// Maximum recursion depth until references in environment variables are followed +const ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH: usize = 10; + +type Result = std::result::Result; + #[derive(Snafu, Debug, EnumDiscriminants)] #[strum_discriminants(derive(IntoStaticStr))] pub enum Error { @@ -34,7 +36,9 @@ pub enum Error { "invalid environment variable name: a valid environment variable name must not be empty \ and must consist only of printable ASCII characters other than '='" ))] - ParseEnvVarName { env_var_name: String }, + ParseEnvVarName { + source: crate::v2::macros::attributed_string_type::Error, + }, } /// Infallible variant of [`crate::builder::pod::container::ContainerBuilder::new`] @@ -80,8 +84,11 @@ impl EnvVarSet { /// Adds the given [`EnvVar`] to this set /// /// An [`EnvVar`] with the same name is overridden. - pub fn with_env_var(mut self, env_var: EnvVar) -> Result { - self.0.insert(EnvVarName::from_str(&env_var.name)?, env_var); + pub fn with_env_var(mut self, env_var: EnvVar) -> Result { + self.0.insert( + EnvVarName::from_str(&env_var.name).context(ParseEnvVarNameSnafu)?, + env_var, + ); Ok(self) } @@ -170,7 +177,8 @@ impl EnvVarSet { impl From for Vec { fn from(value: EnvVarSet) -> Self { - let env_var_closure = EnvVarDependencyResolver::new(&value, 10); + let env_var_closure = + EnvVarDependencyResolver::new(&value, ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH); let mut vec: Self = value.0.values().cloned().collect(); vec.sort_by_key(|env_var| env_var_closure.sort_key(env_var)); From 4e1e353254d95d11dfe48baf3acce86e3a295226 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 11:36:01 +0200 Subject: [PATCH 08/13] chore: Re-add sort_by_cached_key --- crates/stackable-operator/src/v2/builder/pod/container.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index 39c38c6ce..4de460615 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -181,7 +181,7 @@ impl From for Vec { EnvVarDependencyResolver::new(&value, ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH); let mut vec: Self = value.0.values().cloned().collect(); - vec.sort_by_key(|env_var| env_var_closure.sort_key(env_var)); + vec.sort_by_cached_key(|env_var| env_var_closure.sort_key(env_var)); vec } } From 631e9e0a84b8a553422f6a47256b3b1432861b18 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 12:00:19 +0200 Subject: [PATCH 09/13] chore: Improve comment wording --- crates/stackable-operator/CHANGELOG.md | 6 ++++- .../src/v2/builder/pod/container.rs | 27 ++++++++++--------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index ee81f4b5d..47e5df715 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -6,7 +6,11 @@ All notable changes to this project will be documented in this file. ### Added -- [v2] BREAKING: Converting an `EnvVarSet` into a `Vec` takes dependencies between the +- [v2] Add `EnvVarSet::with_env_var` to add a given `EnvVar` to the set ([#1249]). + +### Changed + +- [v2] BREAKING: Converting an `EnvVarSet` into a `Vec` takes dependencies between environment variables into account ([#1249]). [#1249]: https://github.com/stackabletech/operator-rs/pull/1249 diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index 4de460615..b07360fbe 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -16,7 +16,7 @@ use crate::{ v2::types::kubernetes::{ConfigMapKey, ConfigMapName, ContainerName}, }; -/// Pattern for an escaped dollar sign, e.g. `$$` +/// Pattern for an escaped dollar sign (`$$`) static ESCAPED_DOLLAR_SIGN_PATTERN: LazyLock = LazyLock::new(|| Regex::new(r"\$\$").expect("should be a valid regular expression")); @@ -24,7 +24,7 @@ static ESCAPED_DOLLAR_SIGN_PATTERN: LazyLock = static REFERENCED_ENV_VARS_PATTERN: LazyLock = LazyLock::new(|| Regex::new(r"\$\(([^\)]+)\)").expect("should be a valid regular expression")); -/// Maximum recursion depth until references in environment variables are followed +/// Maximum depth to which references between environment variables are followed const ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH: usize = 10; type Result = std::result::Result; @@ -177,11 +177,11 @@ impl EnvVarSet { impl From for Vec { fn from(value: EnvVarSet) -> Self { - let env_var_closure = + let dependency_resolver = EnvVarDependencyResolver::new(&value, ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH); let mut vec: Self = value.0.values().cloned().collect(); - vec.sort_by_cached_key(|env_var| env_var_closure.sort_key(env_var)); + vec.sort_by_cached_key(|env_var| dependency_resolver.sort_key(env_var)); vec } } @@ -198,12 +198,13 @@ impl IntoIterator for EnvVarSet { /// Resolves dependencies between environment variables and provides sort keys which take these /// dependencies into account pub struct EnvVarDependencyResolver<'a> { - /// [EnvVarSet] with possibly dependent environment variables + /// [`EnvVarSet`] with possibly dependent environment variables env_vars: &'a EnvVarSet, - /// Maximum recursion depth + /// Maximum depth to which references between environment variables are followed /// - /// Long dependency chains could slow down the operator. + /// The recursion depth is limited because long dependency chains could slow down the + /// operator. max_recursion_depth: usize, } @@ -215,8 +216,8 @@ impl<'a> EnvVarDependencyResolver<'a> { } } - /// Returns a sort key for the given environment variable which considers dependencies to other - /// environment variables + /// Returns a sort key for the given environment variable, taking dependencies on other + /// environment variables into account /// /// # Example /// @@ -262,7 +263,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// }; /// let env_var5 = EnvVar { /// name: "ENV5".to_owned(), - /// value: Some("self reference to $(ENV5)".to_owned()), + /// value: Some("self-reference to $(ENV5)".to_owned()), /// value_from: None, /// }; /// @@ -296,8 +297,8 @@ impl<'a> EnvVarDependencyResolver<'a> { /// ``` pub fn sort_key(&self, env_var: &EnvVar) -> Vec { if let Some(mut closure) = self.calculate_closure(env_var) { - // Add the name of the variable to its closure to make the set unique for every - // variable. + // Add the name of the variable to its closure so that every variable gets a unique + // sort key. closure.insert(env_var.name.clone()); closure.into_iter().rev().collect() @@ -355,7 +356,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// }; /// let env_var5 = EnvVar { /// name: "ENV5".to_owned(), - /// value: Some("self reference to $(ENV5)".to_owned()), + /// value: Some("self-reference to $(ENV5)".to_owned()), /// value_from: None, /// }; /// let env_var6 = EnvVar { From 9a3111d076eef844589418b81efdece841e9700d Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 15 Jul 2026 17:02:41 +0200 Subject: [PATCH 10/13] chore: Adapt wording --- .../stackable-operator/src/v2/builder/pod/container.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index b07360fbe..dada971b7 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -17,12 +17,14 @@ use crate::{ }; /// Pattern for an escaped dollar sign (`$$`) -static ESCAPED_DOLLAR_SIGN_PATTERN: LazyLock = - LazyLock::new(|| Regex::new(r"\$\$").expect("should be a valid regular expression")); +static ESCAPED_DOLLAR_SIGN_PATTERN: LazyLock = LazyLock::new(|| { + Regex::new(r"\$\$").expect("static string must be a valid regular expression") +}); /// Pattern for a referenced environment variable, e.g. `$(ENV_VAR)` -static REFERENCED_ENV_VARS_PATTERN: LazyLock = - LazyLock::new(|| Regex::new(r"\$\(([^\)]+)\)").expect("should be a valid regular expression")); +static REFERENCED_ENV_VARS_PATTERN: LazyLock = LazyLock::new(|| { + Regex::new(r"\$\(([^\)]+)\)").expect("static string must be a valid regular expression") +}); /// Maximum depth to which references between environment variables are followed const ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH: usize = 10; From 05660fae204b96d53cc2c3536c07733bf95a0554 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Thu, 16 Jul 2026 12:18:16 +0200 Subject: [PATCH 11/13] chore: Use references when sorting environment variables --- .../src/v2/builder/pod/container.rs | 97 +++++++++++-------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index dada971b7..c5217439f 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -74,6 +74,14 @@ impl EnvVarSet { self.0.get(env_var_name) } + /// Returns an iterator over the [`EnvVar`]s in this set + /// + /// The [`EnvVar`]s are sorted so that variables referencing other variables come after the + /// referenced ones. + pub fn iter(&self) -> vec::IntoIter<&EnvVar> { + self.into_iter() + } + /// Moves all [`EnvVar`]s from the given set into this one. /// /// [`EnvVar`]s with the same name are overridden. @@ -177,14 +185,24 @@ impl EnvVarSet { } } -impl From for Vec { - fn from(value: EnvVarSet) -> Self { +impl<'a> From<&'a EnvVarSet> for Vec<&'a EnvVar> { + fn from(value: &'a EnvVarSet) -> Self { + value.into_iter().collect() + } +} + +impl<'a> IntoIterator for &'a EnvVarSet { + type IntoIter = vec::IntoIter; + type Item = &'a EnvVar; + + fn into_iter(self) -> Self::IntoIter { let dependency_resolver = - EnvVarDependencyResolver::new(&value, ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH); + EnvVarDependencyResolver::new(self, ENV_VAR_DEPENDENCY_RESOLVER_MAX_RECURSION_DEPTH); - let mut vec: Self = value.0.values().cloned().collect(); - vec.sort_by_cached_key(|env_var| dependency_resolver.sort_key(env_var)); - vec + let mut env_vars: Vec<&EnvVar> = self.0.values().collect(); + env_vars.sort_by_cached_key(|env_var| dependency_resolver.sort_key(env_var)); + + env_vars.into_iter() } } @@ -193,7 +211,11 @@ impl IntoIterator for EnvVarSet { type Item = EnvVar; fn into_iter(self) -> Self::IntoIter { - Vec::from(self).into_iter() + Vec::from(&self) + .into_iter() + .cloned() + .collect::>() + .into_iter() } } @@ -282,30 +304,21 @@ impl<'a> EnvVarDependencyResolver<'a> { /// .unwrap(); /// /// let resolver = EnvVarDependencyResolver::new(&env_vars, 2); - /// assert_eq!( - /// vec!["ENV4".to_owned(), "ENV2".to_owned(), "ENV1".to_owned()], - /// resolver.sort_key(&env_var1) - /// ); - /// assert_eq!( - /// vec!["ENV4".to_owned(), "ENV2".to_owned()], - /// resolver.sort_key(&env_var2) - /// ); - /// assert_eq!( - /// vec!["ENV4".to_owned(), "ENV3".to_owned()], - /// resolver.sort_key(&env_var3) - /// ); - /// assert_eq!(vec!["ENV4".to_owned()], resolver.sort_key(&env_var4)); - /// assert_eq!(vec!["ENV5".to_owned()], resolver.sort_key(&env_var5)); + /// assert_eq!(vec!["ENV4", "ENV2", "ENV1"], resolver.sort_key(&env_var1)); + /// assert_eq!(vec!["ENV4", "ENV2"], resolver.sort_key(&env_var2)); + /// assert_eq!(vec!["ENV4", "ENV3"], resolver.sort_key(&env_var3)); + /// assert_eq!(vec!["ENV4"], resolver.sort_key(&env_var4)); + /// assert_eq!(vec!["ENV5"], resolver.sort_key(&env_var5)); /// ``` - pub fn sort_key(&self, env_var: &EnvVar) -> Vec { + pub fn sort_key(&self, env_var: &'a EnvVar) -> Vec<&'a String> { if let Some(mut closure) = self.calculate_closure(env_var) { // Add the name of the variable to its closure so that every variable gets a unique // sort key. - closure.insert(env_var.name.clone()); + closure.insert(&env_var.name); closure.into_iter().rev().collect() } else { - vec![env_var.name.clone()] + vec![&env_var.name] } } @@ -397,15 +410,15 @@ impl<'a> EnvVarDependencyResolver<'a> { /// /// let resolver = EnvVarDependencyResolver::new(&env_vars, 2); /// assert_eq!( - /// Some(BTreeSet::from(["ENV2".to_owned(), "ENV4".to_owned()])), + /// Some(BTreeSet::from([&env_var2.name, &env_var4.name])), /// resolver.calculate_closure(&env_var1) /// ); /// assert_eq!( - /// Some(BTreeSet::from(["ENV4".to_owned()])), + /// Some(BTreeSet::from([&env_var4.name])), /// resolver.calculate_closure(&env_var2) /// ); /// assert_eq!( - /// Some(BTreeSet::from(["ENV4".to_owned()])), + /// Some(BTreeSet::from([&env_var4.name])), /// resolver.calculate_closure(&env_var3) /// ); /// assert_eq!(Some(BTreeSet::new()), resolver.calculate_closure(&env_var4)); @@ -414,7 +427,7 @@ impl<'a> EnvVarDependencyResolver<'a> { /// assert_eq!(None, resolver.calculate_closure(&env_var7)); /// assert_eq!(None, resolver.calculate_closure(&env_var8)); /// ``` - pub fn calculate_closure(&self, env_var: &EnvVar) -> Option> { + pub fn calculate_closure(&self, env_var: &EnvVar) -> Option> { self.calculate_closure_rec(env_var, self.max_recursion_depth) } @@ -422,7 +435,7 @@ impl<'a> EnvVarDependencyResolver<'a> { &self, env_var: &EnvVar, remaining_recursion_depth: usize, - ) -> Option> { + ) -> Option> { if env_var.value.is_none() { Some(BTreeSet::new()) } else if let Some(value) = &env_var.value @@ -431,7 +444,7 @@ impl<'a> EnvVarDependencyResolver<'a> { let mut closure = BTreeSet::new(); for referenced_env_var in self.referenced_env_vars(value) { - closure.insert(referenced_env_var.name.clone()); + closure.insert(&referenced_env_var.name); closure.extend( self.calculate_closure_rec(referenced_env_var, remaining_recursion_depth - 1)?, ); @@ -597,17 +610,17 @@ mod tests { assert_eq!( vec![ - EnvVar { + &EnvVar { name: "ENV1".to_owned(), value: Some("value1 from env_var_set1".to_owned()), value_from: None }, - EnvVar { + &EnvVar { name: "ENV2".to_owned(), value: Some("value2 from env_var_set2".to_owned()), value_from: None }, - EnvVar { + &EnvVar { name: "ENV3".to_owned(), value: None, value_from: Some(EnvVarSource { @@ -618,13 +631,13 @@ mod tests { ..EnvVarSource::default() }), }, - EnvVar { + &EnvVar { name: "ENV4".to_owned(), value: Some("value4 from env_var_set2".to_owned()), value_from: None } ], - Vec::from(merged_env_var_set) + Vec::from(&merged_env_var_set) ); } @@ -637,18 +650,18 @@ mod tests { assert_eq!( vec![ - EnvVar { + &EnvVar { name: "ENV1".to_owned(), value: Some("value1".to_owned()), value_from: None }, - EnvVar { + &EnvVar { name: "ENV2".to_owned(), value: Some("value2".to_owned()), value_from: None } ], - Vec::from(env_var_set) + Vec::from(&env_var_set) ); } @@ -721,23 +734,23 @@ mod tests { assert_eq!( vec![ - EnvVar { + &EnvVar { name: "ENV2".to_owned(), value: Some("value 2".to_owned()), value_from: None }, - EnvVar { + &EnvVar { name: "ENV1".to_owned(), value: Some("$(ENV2)".to_owned()), value_from: None }, - EnvVar { + &EnvVar { name: "ENV3".to_owned(), value: Some("value 3".to_owned()), value_from: None }, ], - Vec::from(env_var_set) + Vec::from(&env_var_set) ); } From fc4197ed71982b99eb65c6408ae287277a5632a1 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Mon, 20 Jul 2026 14:54:33 +0200 Subject: [PATCH 12/13] Apply suggestions from code review Co-authored-by: Techassi --- .../src/v2/builder/pod/container.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index c5217439f..3070d3158 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -190,7 +190,11 @@ impl<'a> From<&'a EnvVarSet> for Vec<&'a EnvVar> { value.into_iter().collect() } } - +impl From for Vec { + fn from(value: EnvVarSet) -> Self { + value.into_iter().collect() + } +} impl<'a> IntoIterator for &'a EnvVarSet { type IntoIter = vec::IntoIter; type Item = &'a EnvVar; @@ -211,11 +215,7 @@ impl IntoIterator for EnvVarSet { type Item = EnvVar; fn into_iter(self) -> Self::IntoIter { - Vec::from(&self) - .into_iter() - .cloned() - .collect::>() - .into_iter() + Vec::from(self).into_iter() } } From de636c0d1c02b569be31c2280c0bb3cc4fa67a9b Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Mon, 20 Jul 2026 15:32:40 +0200 Subject: [PATCH 13/13] Fix cyclic call in IntoIterator for EnvVarSet --- crates/stackable-operator/src/v2/builder/pod/container.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/v2/builder/pod/container.rs b/crates/stackable-operator/src/v2/builder/pod/container.rs index 3070d3158..24623baf7 100644 --- a/crates/stackable-operator/src/v2/builder/pod/container.rs +++ b/crates/stackable-operator/src/v2/builder/pod/container.rs @@ -190,11 +190,13 @@ impl<'a> From<&'a EnvVarSet> for Vec<&'a EnvVar> { value.into_iter().collect() } } + impl From for Vec { fn from(value: EnvVarSet) -> Self { value.into_iter().collect() } } + impl<'a> IntoIterator for &'a EnvVarSet { type IntoIter = vec::IntoIter; type Item = &'a EnvVar; @@ -215,7 +217,7 @@ impl IntoIterator for EnvVarSet { type Item = EnvVar; fn into_iter(self) -> Self::IntoIter { - Vec::from(self).into_iter() + (&self).into_iter().cloned().collect::>().into_iter() } }