From 828935a4d8d17e16043089ce8cac645bcdea05d0 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 21 Jan 2026 17:50:33 +0100 Subject: [PATCH] chore: Move controller utility functions to separate module --- .../src/controller/validate.rs | 77 ++----- rust/operator-binary/src/framework.rs | 1 + .../src/framework/controller_utils.rs | 211 ++++++++++++++++++ 3 files changed, 229 insertions(+), 60 deletions(-) create mode 100644 rust/operator-binary/src/framework/controller_utils.rs diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index ccacd25..717aae5 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -1,13 +1,10 @@ //! The validate step in the OpenSearchCluster controller -use std::{collections::BTreeMap, num::TryFromIntError, str::FromStr}; +use std::{collections::BTreeMap, str::FromStr}; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ - kube::{Resource, ResourceExt}, - product_logging::spec::Logging, - role_utils::RoleGroup, - shared::time::Duration, + product_logging::spec::Logging, role_utils::RoleGroup, shared::time::Duration, }; use strum::{EnumDiscriminants, IntoStaticStr}; @@ -19,14 +16,12 @@ use crate::{ crd::v1alpha1::{self}, framework::{ builder::pod::container::{EnvVarName, EnvVarSet}, + controller_utils::{get_cluster_name, get_namespace, get_uid}, product_logging::framework::{ VectorContainerLogConfig, validate_logging_configuration_for_container, }, role_utils::{GenericProductSpecificCommonConfig, RoleGroupConfig, with_validated_config}, - types::{ - kubernetes::{ConfigMapName, NamespaceName, Uid}, - operator::ClusterName, - }, + types::{kubernetes::ConfigMapName, operator::ClusterName}, }, }; @@ -34,34 +29,25 @@ use crate::{ #[strum_discriminants(derive(IntoStaticStr))] pub enum Error { #[snafu(display("failed to get the cluster name"))] - GetClusterName {}, + GetClusterName { + source: crate::framework::controller_utils::Error, + }, #[snafu(display("failed to get the cluster namespace"))] - GetClusterNamespace {}, + GetClusterNamespace { + source: crate::framework::controller_utils::Error, + }, #[snafu(display("failed to get the cluster UID"))] - GetClusterUid {}, + GetClusterUid { + source: crate::framework::controller_utils::Error, + }, #[snafu(display( "failed to get vectorAggregatorConfigMapName; It must be set if enableVectorAgent is true." ))] GetVectorAggregatorConfigMapName {}, - #[snafu(display("failed to set cluster name"))] - ParseClusterName { - source: crate::framework::macros::attributed_string_type::Error, - }, - - #[snafu(display("failed to set cluster namespace"))] - ParseClusterNamespace { - source: crate::framework::macros::attributed_string_type::Error, - }, - - #[snafu(display("failed to set UID"))] - ParseClusterUid { - source: crate::framework::macros::attributed_string_type::Error, - }, - #[snafu(display("failed to parse environment variable"))] ParseEnvironmentVariable { source: crate::framework::builder::pod::container::Error, @@ -94,7 +80,7 @@ pub enum Error { #[snafu(display("termination grace period is too long (got {duration}, maximum allowed is {max})", max = Duration::from_secs(i64::MAX as u64)))] TerminationGracePeriodTooLong { - source: TryFromIntError, + source: std::num::TryFromIntError, duration: Duration, }, } @@ -114,14 +100,9 @@ pub fn validate( context_names: &ContextNames, cluster: &v1alpha1::OpenSearchCluster, ) -> Result { - let raw_cluster_name = cluster.meta().name.clone().context(GetClusterNameSnafu)?; - let cluster_name = ClusterName::from_str(&raw_cluster_name).context(ParseClusterNameSnafu)?; - - let raw_namespace = cluster.namespace().context(GetClusterNamespaceSnafu)?; - let namespace = NamespaceName::from_str(&raw_namespace).context(ParseClusterNamespaceSnafu)?; - - let raw_uid = cluster.uid().context(GetClusterUidSnafu)?; - let uid = Uid::from_str(&raw_uid).context(ParseClusterUidSnafu)?; + let cluster_name = get_cluster_name(cluster).context(GetClusterNameSnafu)?; + let namespace = get_namespace(cluster).context(GetClusterNamespaceSnafu)?; + let uid = get_uid(cluster).context(GetClusterUidSnafu)?; let product_image = cluster .spec @@ -541,14 +522,6 @@ mod tests { ); } - #[test] - fn test_validate_err_parse_cluster_name() { - test_validate_err( - |cluster| cluster.metadata.name = Some("invalid cluster name".to_owned()), - ErrorDiscriminants::ParseClusterName, - ); - } - #[test] fn test_validate_err_get_cluster_namespace() { test_validate_err( @@ -557,14 +530,6 @@ mod tests { ); } - #[test] - fn test_validate_err_parse_cluster_namespace() { - test_validate_err( - |cluster| cluster.metadata.namespace = Some("invalid cluster namespace".to_owned()), - ErrorDiscriminants::ParseClusterNamespace, - ); - } - #[test] fn test_validate_err_get_cluster_uid() { test_validate_err( @@ -573,14 +538,6 @@ mod tests { ); } - #[test] - fn test_validate_err_parse_cluster_uid() { - test_validate_err( - |cluster| cluster.metadata.uid = Some("invalid cluster UID".to_owned()), - ErrorDiscriminants::ParseClusterUid, - ); - } - #[test] fn test_validate_err_resolve_product_image() { test_validate_err( diff --git a/rust/operator-binary/src/framework.rs b/rust/operator-binary/src/framework.rs index 2e42c3c..e6f58e9 100644 --- a/rust/operator-binary/src/framework.rs +++ b/rust/operator-binary/src/framework.rs @@ -24,6 +24,7 @@ use types::kubernetes::Uid; pub mod builder; pub mod cluster_resources; +pub mod controller_utils; pub mod kvp; pub mod macros; pub mod product_logging; diff --git a/rust/operator-binary/src/framework/controller_utils.rs b/rust/operator-binary/src/framework/controller_utils.rs new file mode 100644 index 0000000..d15e53f --- /dev/null +++ b/rust/operator-binary/src/framework/controller_utils.rs @@ -0,0 +1,211 @@ +//! Helper functions which are not tied to a specific controller step + +use std::str::FromStr; + +use snafu::{OptionExt, ResultExt, Snafu}; +use stackable_operator::kube::runtime::reflector::Lookup; +use strum::{EnumDiscriminants, IntoStaticStr}; + +use crate::framework::types::{ + kubernetes::{NamespaceName, Uid}, + operator::ClusterName, +}; + +#[derive(Snafu, Debug, EnumDiscriminants)] +#[strum_discriminants(derive(IntoStaticStr))] +pub enum Error { + #[snafu(display("failed to get the cluster name"))] + GetClusterName {}, + + #[snafu(display("failed to get the namespace"))] + GetNamespace {}, + + #[snafu(display("failed to get the UID"))] + GetUid {}, + + #[snafu(display("failed to set the cluster name"))] + ParseClusterName { + source: crate::framework::macros::attributed_string_type::Error, + }, + + #[snafu(display("failed to set the namespace"))] + ParseNamespace { + source: crate::framework::macros::attributed_string_type::Error, + }, + + #[snafu(display("failed to set the UID"))] + ParseUid { + source: crate::framework::macros::attributed_string_type::Error, + }, +} + +type Result = std::result::Result; + +/// Get the cluster name from the given resource +pub fn get_cluster_name(cluster: &impl Lookup) -> Result { + let raw_cluster_name = cluster.name().context(GetClusterNameSnafu)?; + let cluster_name = ClusterName::from_str(&raw_cluster_name).context(ParseClusterNameSnafu)?; + + Ok(cluster_name) +} + +/// Get the namespace from the given resource +pub fn get_namespace(resource: &impl Lookup) -> Result { + let raw_namespace = resource.namespace().context(GetNamespaceSnafu)?; + let namespace = NamespaceName::from_str(&raw_namespace).context(ParseNamespaceSnafu)?; + + Ok(namespace) +} + +/// Get the UID from the given resource +pub fn get_uid(resource: &impl Lookup) -> Result { + let raw_uid = resource.uid().context(GetUidSnafu)?; + let uid = Uid::from_str(&raw_uid).context(ParseUidSnafu)?; + + Ok(uid) +} + +#[cfg(test)] +mod tests { + use stackable_operator::kube::runtime::reflector::Lookup; + use uuid::uuid; + + use super::{ErrorDiscriminants, get_cluster_name, get_namespace, get_uid}; + use crate::framework::types::{ + kubernetes::{NamespaceName, Uid}, + operator::ClusterName, + }; + + #[derive(Debug, Default)] + struct TestResource { + name: Option<&'static str>, + namespace: Option<&'static str>, + uid: Option<&'static str>, + } + + impl Lookup for TestResource { + type DynamicType = (); + + fn kind(_dyntype: &Self::DynamicType) -> std::borrow::Cow<'_, str> { + "TestResource".into() + } + + fn group(_dyntype: &Self::DynamicType) -> std::borrow::Cow<'_, str> { + "stackable.tech".into() + } + + fn version(_dyntype: &Self::DynamicType) -> std::borrow::Cow<'_, str> { + "v1".into() + } + + fn plural(_dyntype: &Self::DynamicType) -> std::borrow::Cow<'_, str> { + "testresources".into() + } + + fn name(&self) -> Option> { + self.name.map(std::borrow::Cow::Borrowed) + } + + fn namespace(&self) -> Option> { + self.namespace.map(std::borrow::Cow::Borrowed) + } + + fn resource_version(&self) -> Option> { + Some("1".into()) + } + + fn uid(&self) -> Option> { + self.uid.map(std::borrow::Cow::Borrowed) + } + } + + #[test] + fn test_get_cluster_name() { + assert_eq!( + ClusterName::from_str_unsafe("test-cluster"), + get_cluster_name(&TestResource { + name: Some("test-cluster"), + ..TestResource::default() + }) + .expect("should contain a valid cluster name") + ); + + assert_eq!( + Err(ErrorDiscriminants::GetClusterName), + get_cluster_name(&TestResource { + name: None, + ..TestResource::default() + }) + .map_err(ErrorDiscriminants::from) + ); + + assert_eq!( + Err(ErrorDiscriminants::ParseClusterName), + get_cluster_name(&TestResource { + name: Some("invalid cluster name"), + ..TestResource::default() + }) + .map_err(ErrorDiscriminants::from) + ); + } + + #[test] + fn test_get_namespace() { + assert_eq!( + NamespaceName::from_str_unsafe("test-namespace"), + get_namespace(&TestResource { + namespace: Some("test-namespace"), + ..TestResource::default() + }) + .expect("should contain a valid namespace") + ); + + assert_eq!( + Err(ErrorDiscriminants::GetNamespace), + get_namespace(&TestResource { + namespace: None, + ..TestResource::default() + }) + .map_err(ErrorDiscriminants::from) + ); + + assert_eq!( + Err(ErrorDiscriminants::ParseNamespace), + get_namespace(&TestResource { + namespace: Some("invalid namespace"), + ..TestResource::default() + }) + .map_err(ErrorDiscriminants::from) + ); + } + + #[test] + fn test_get_uid() { + assert_eq!( + Uid::from(uuid!("e6ac237d-a6d4-43a1-8135-f36506110912")), + get_uid(&TestResource { + uid: Some("e6ac237d-a6d4-43a1-8135-f36506110912"), + ..TestResource::default() + }) + .expect("should contain a valid UID") + ); + + assert_eq!( + Err(ErrorDiscriminants::GetUid), + get_uid(&TestResource { + uid: None, + ..TestResource::default() + }) + .map_err(ErrorDiscriminants::from) + ); + + assert_eq!( + Err(ErrorDiscriminants::ParseUid), + get_uid(&TestResource { + uid: Some("invalid UID"), + ..TestResource::default() + }) + .map_err(ErrorDiscriminants::from) + ); + } +}