From 206695b9f77639c88364a3de248d0c2b31a1ddde Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 7 Sep 2026 17:17:47 +0200 Subject: [PATCH 1/4] use expect where possible and remove unecessary enums/Results --- .../controller/build/resource/config_map.rs | 12 +-- .../controller/build/resource/deployment.rs | 18 +--- .../src/controller/build/resource/mod.rs | 20 ++--- .../controller/build/resource/statefulset.rs | 47 ++++------- .../src/controller/validate.rs | 14 +--- rust/operator-binary/src/crd/mod.rs | 82 +++++++++---------- .../src/druid_connection_controller/mod.rs | 17 ++-- 7 files changed, 78 insertions(+), 132 deletions(-) diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index e2343f6e..276624c9 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -22,12 +22,6 @@ pub enum Error { source: superset_config::Error, role_group_name: RoleGroupName, }, - - #[snafu(display("failed to build ConfigMap for role group {role_group_name}"))] - RoleGroupConfig { - source: stackable_operator::builder::configmap::Error, - role_group_name: RoleGroupName, - }, } type Result = std::result::Result; @@ -72,9 +66,9 @@ pub fn build_rolegroup_config_map( cm_builder.add_data(VECTOR_CONFIG_FILE, vector_config); } - cm_builder.build().with_context(|_| RoleGroupConfigSnafu { - role_group_name: role_group_name.clone(), - }) + Ok(cm_builder + .build() + .expect("The ConfigMap metadata is set in this function.")) } #[cfg(test)] diff --git a/rust/operator-binary/src/controller/build/resource/deployment.rs b/rust/operator-binary/src/controller/build/resource/deployment.rs index d2f0e745..bca99bad 100644 --- a/rust/operator-binary/src/controller/build/resource/deployment.rs +++ b/rust/operator-binary/src/controller/build/resource/deployment.rs @@ -44,23 +44,10 @@ const CELERY_APP_INVOCATION: &str = "celery --app=superset.tasks.celery_app:app" #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build container"))] - BuildContainer { source: super::Error }, - #[snafu(display("failed to set termination grace period for graceful shutdown"))] GracefulShutdown { source: stackable_operator::builder::pod::Error, }, - - #[snafu(display("failed to add needed volume"))] - AddVolume { - source: stackable_operator::builder::pod::Error, - }, - - #[snafu(display("failed to add needed volumeMount"))] - AddVolumeMount { - source: stackable_operator::builder::pod::container::Error, - }, } type Result = std::result::Result; @@ -120,8 +107,7 @@ pub fn build_rolegroup_deployment( // The Celery roles set no role-specific env vars, so an empty set is passed. let mut superset_cb = - super::build_superset_container_builder(validated, rolegroup_config, EnvVarSet::new()) - .context(BuildContainerSnafu)?; + super::build_superset_container_builder(validated, rolegroup_config, EnvVarSet::new()); superset_cb .command(super::bash_wrapper_command()) @@ -160,7 +146,7 @@ pub fn build_rolegroup_deployment( resource_names.role_group_config_map().as_ref(), &rolegroup_config.config.logging.superset_container, )) - .context(AddVolumeSnafu)?; + .expect("The volume names are statically defined and there should be no duplicates."); pb.add_container(super::build_metrics_container(&validated.image)); if let Some(vector_container) = diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index 8b8727c6..d186bbce 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -1,7 +1,6 @@ use std::str::FromStr; use indoc::formatdoc; -use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::pod::{ container::ContainerBuilder, resources::ResourceRequirementsBuilder, volume::VolumeBuilder, @@ -93,15 +92,6 @@ pub(crate) const PROTOCOL_TCP: &str = "TCP"; /// The `fsGroup` the Pods run as, required by secret-operator-provided volumes. pub(crate) const SECRET_OPERATOR_FS_GROUP: i64 = 1000; -/// Errors shared by the container builders below. -#[derive(Snafu, Debug)] -pub enum Error { - #[snafu(display("failed to add needed volumeMount"))] - AddVolumeMount { - source: stackable_operator::builder::pod::container::Error, - }, -} - /// The shell wrapper used to launch the long-running product containers /// (`/bin/bash -x -euo pipefail -c `). pub(crate) fn bash_wrapper_command() -> Vec { @@ -264,24 +254,24 @@ pub(crate) fn build_superset_container_builder( validated: &ValidatedCluster, rolegroup_config: &SupersetRoleGroupConfig, role_specific_env_vars: EnvVarSet, -) -> Result { +) -> ContainerBuilder { let mut superset_cb = new_container_builder(&Container::Superset.to_container_name()); superset_cb .image_from_product_image(&validated.image) .add_volume_mount(CONFIG_VOLUME_NAME.as_ref(), STACKABLE_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(LOG_CONFIG_VOLUME_NAME.as_ref(), STACKABLE_LOG_CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_env_vars(build_env_vars( validated, rolegroup_config, role_specific_env_vars, )); - Ok(superset_cb) + superset_cb } /// Builds the `metrics` (statsd exporter) sidecar container, shared by the StatefulSet and diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 6f3e75c1..a0c132f2 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -62,9 +62,6 @@ const POD_MANAGEMENT_POLICY_ORDERED_READY: &str = "OrderedReady"; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build container"))] - BuildContainer { source: super::Error }, - #[snafu(display("failed to set termination grace period for graceful shutdown"))] GracefulShutdown { source: stackable_operator::builder::pod::Error, @@ -79,16 +76,6 @@ pub enum Error { AddTlsVolumesAndVolumeMounts { source: stackable_operator::commons::tls_verification::TlsClientDetailsError, }, - - #[snafu(display("failed to add needed volume"))] - AddVolume { - source: stackable_operator::builder::pod::Error, - }, - - #[snafu(display("failed to add needed volumeMount"))] - AddVolumeMount { - source: stackable_operator::builder::pod::container::Error, - }, } type Result = std::result::Result; @@ -135,14 +122,28 @@ pub fn build_node_rolegroup_statefulset( // The `Node` role serves the Superset web UI, so it additionally passes the authentication // env vars into the shared container builder (which merges the user `envOverrides` in last, - // so they keep the highest precedence) and mounts the authentication volumes. These mounts - // are added after the common config volume mounts (volume mount order is not significant). + // so they keep the highest precedence) and mounts the authentication volumes. let mut superset_cb = super::build_superset_container_builder( validated, rolegroup_config, authentication_env_vars(&validated.cluster_config.authentication_config), - ) - .context(BuildContainerSnafu)?; + ); + + // Operator-managed volumes and volume mounts with static names and paths first: their adds + // are infallible. The authentication volumes and mounts below are named after the user's + // SecretClasses, so they are added afterwards and stay fallible, as they can collide with + // the operator-managed ones. + superset_cb + .add_volume_mount( + super::LISTENER_VOLUME_NAME_PVC.as_ref(), + LISTENER_VOLUME_DIR, + ) + .expect("The mount paths are statically defined and there should be no duplicates."); + pb.add_volumes(super::create_volumes( + resource_names.role_group_config_map().as_ref(), + &rolegroup_config.config.logging.superset_container, + )) + .expect("The volume names are statically defined and there should be no duplicates."); add_authentication_volumes_and_volume_mounts( &validated.cluster_config.authentication_config, @@ -221,24 +222,12 @@ pub fn build_node_rolegroup_statefulset( None }; - superset_cb - .add_volume_mount( - super::LISTENER_VOLUME_NAME_PVC.as_ref(), - LISTENER_VOLUME_DIR, - ) - .context(AddVolumeMountSnafu)?; - pb.add_container(superset_cb.build()); if let Some(termination_grace_period) = merged_config.graceful_shutdown_timeout { pb.termination_grace_period(&termination_grace_period) .context(GracefulShutdownSnafu)?; } - pb.add_volumes(super::create_volumes( - resource_names.role_group_config_map().as_ref(), - &rolegroup_config.config.logging.superset_container, - )) - .context(AddVolumeSnafu)?; pb.add_container(super::build_metrics_container(&validated.image)); if let Some(vector_container) = diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 558d5e45..9dd8fb6f 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -70,12 +70,6 @@ pub enum Error { role_group: RoleGroupName, }, - #[snafu(display("invalid environment variable override name in role group {role_group}"))] - ParseEnvVarName { - source: stackable_operator::v2::macros::attributed_string_type::Error, - role_group: RoleGroupName, - }, - #[snafu(display("invalid role group name {role_group}"))] ParseRoleGroupName { source: stackable_operator::v2::macros::attributed_string_type::Error, @@ -155,6 +149,8 @@ pub fn validate_cluster( .vector_aggregator_config_map_name .clone(); + let cluster_name = get_cluster_name(superset).context(ResolveClusterNameSnafu)?; + let mut role_groups = BTreeMap::new(); let mut role_configs = BTreeMap::new(); @@ -172,10 +168,7 @@ pub fn validate_cluster( }| pod_disruption_budget, ), listener_class: role.listener_class_name(superset), - group_listener_name: superset.group_listener_name(&role).map(|name| { - name.parse() - .expect("the group listener name is a valid ListenerName") - }), + group_listener_name: role.group_listener_name(&cluster_name), }, ); @@ -203,7 +196,6 @@ pub fn validate_cluster( let cluster_config = &superset.spec.cluster_config; - let cluster_name = get_cluster_name(superset).context(ResolveClusterNameSnafu)?; let namespace = get_namespace(superset).context(ResolveNamespaceSnafu)?; let uid = get_uid(superset).context(ResolveUidSnafu)?; diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index e7ee1e46..0584320d 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -31,8 +31,10 @@ use stackable_operator::{ role_utils::{GenericCommonConfig, Role, RoleGroup}, types::{ common::Port, - kubernetes::{ConfigMapName, ContainerName, ListenerClassName, SecretKey}, - operator::RoleName, + kubernetes::{ + ConfigMapName, ContainerName, ListenerClassName, ListenerName, SecretKey, + }, + operator::{ClusterName, RoleName}, }, }, versioned::versioned, @@ -46,13 +48,13 @@ use crate::crd::{ v1alpha1::SupersetRoleConfig, }; -/// Default [`ListenerClassName`] value used by the rolegroup listener. -pub const DEFAULT_LISTENER_CLASS: &str = "cluster-internal"; +// Default listener class used by the rolegroup listener. +constant!(pub DEFAULT_LISTENER_CLASS: ListenerClassName = "cluster-internal"); -/// Default listener class used by the rolegroup listener. +/// Default listener class used by the rolegroup listener (the serde default of +/// `SupersetRoleConfig::listener_class`). fn default_listener_class() -> ListenerClassName { - ListenerClassName::from_str(DEFAULT_LISTENER_CLASS) - .expect("the default listener class is a valid listener class name") + DEFAULT_LISTENER_CLASS.clone() } pub mod affinity; @@ -420,23 +422,39 @@ impl SupersetRole { Self::Worker | Self::Beat => None, } } + + /// The name of the group listener provided for the role, if the role serves the web UI. + /// Nodes will use this group listener so that only one load balancer is needed for that role. + pub fn group_listener_name(&self, cluster_name: &ClusterName) -> Option { + const _: () = assert!( + ClusterName::MAX_LENGTH + 1 /* dash */ + RoleName::MAX_LENGTH + <= ListenerName::MAX_LENGTH, + "The string `-` must not exceed the limit of Listener names." + ); + // Both halves are RFC 1123 labels joined by a dash, which is a valid RFC 1123 subdomain. + let _ = ClusterName::IS_RFC_1123_SUBDOMAIN_NAME; + let _ = RoleName::IS_RFC_1123_LABEL_NAME; + + let role_name: &RoleName = self; + match self { + Self::Node => Some( + ListenerName::from_str(&format!("{cluster_name}-{role_name}")) + .expect("The role listener name is a valid Listener name."), + ), + Self::Worker | Self::Beat => None, + } + } } impl From for RoleName { fn from(value: SupersetRole) -> Self { - value - .to_string() - .parse() - .expect("a SupersetRole serialises to a valid RoleName") + RoleName::clone(&value) } } impl From<&SupersetRole> for RoleName { fn from(value: &SupersetRole) -> Self { - value - .to_string() - .parse() - .expect("a SupersetRole serialises to a valid RoleName") + RoleName::clone(value) } } @@ -578,20 +596,6 @@ impl v1alpha1::SupersetCluster { &self.spec.cluster_config.metadata_database } - /// The name of the group-listener provided for a specific role. - /// Nodes will use this group listener so that only one load balancer - /// is needed for that role. - pub fn group_listener_name(&self, role: &SupersetRole) -> Option { - match role { - SupersetRole::Node => Some(format!( - "{cluster_name}-{role}", - role = role.as_ref(), - cluster_name = self.name_any() - )), - SupersetRole::Worker | SupersetRole::Beat => None, - } - } - pub fn generic_role_config(&self, role: &SupersetRole) -> Option { self.get_role_config(role).map(|r| r.common.to_owned()) } @@ -621,29 +625,17 @@ impl v1alpha1::SupersetCluster { #[cfg(test)] mod tests { - use stackable_operator::{ - v2::types::operator::RoleName, versioned::test_utils::RoundtripTestData, - }; - use strum::IntoEnumIterator; + use stackable_operator::versioned::test_utils::RoundtripTestData; use super::{ - BEAT_ROLE_NAME, INTERNAL_SECRET_SECRET_KEY, MAPBOX_API_KEY_ENV, MAPBOX_API_KEY_SECRET_KEY, - NODE_ROLE_NAME, SECRET_KEY_ENV, SupersetRole, WORKER_ROLE_NAME, v1alpha1, + BEAT_ROLE_NAME, DEFAULT_LISTENER_CLASS, INTERNAL_SECRET_SECRET_KEY, MAPBOX_API_KEY_ENV, + MAPBOX_API_KEY_SECRET_KEY, NODE_ROLE_NAME, SECRET_KEY_ENV, WORKER_ROLE_NAME, v1alpha1, }; - /// Locks the invariant behind the `expect` in the `From for RoleName` impls: - /// every `SupersetRole` variant (present and future) must serialise to a valid `RoleName`. - #[test] - fn every_superset_role_serialises_to_a_valid_role_name() { - for role in SupersetRole::iter() { - let _: RoleName = (&role).into(); - let _: RoleName = role.into(); - } - } - #[test] fn test_constants() { // Test that dereferencing the constants does not panic. + let _ = *DEFAULT_LISTENER_CLASS; let _ = *NODE_ROLE_NAME; let _ = *WORKER_ROLE_NAME; let _ = *BEAT_ROLE_NAME; diff --git a/rust/operator-binary/src/druid_connection_controller/mod.rs b/rust/operator-binary/src/druid_connection_controller/mod.rs index 0519deac..91f4796e 100644 --- a/rust/operator-binary/src/druid_connection_controller/mod.rs +++ b/rust/operator-binary/src/druid_connection_controller/mod.rs @@ -3,10 +3,7 @@ use std::{str::FromStr, sync::Arc}; use const_format::concatcp; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ - builder::{ - meta::ObjectMetaBuilder, - pod::{container::ContainerBuilder, security::PodSecurityContextBuilder}, - }, + builder::{meta::ObjectMetaBuilder, pod::security::PodSecurityContextBuilder}, cli::OperatorEnvironmentOptions, client::Client, commons::product_image_selection::{self, ResolvedProductImage}, @@ -24,7 +21,10 @@ use stackable_operator::{ logging::controller::ReconcilerError, shared::time::Duration, status::condition::{ClusterConditionStatus, ClusterConditionType}, - v2::builder::pod::container::{EnvVarName, EnvVarSet}, + v2::{ + builder::pod::container::{EnvVarName, EnvVarSet, new_container_builder}, + types::kubernetes::ContainerName, + }, }; use strum::{EnumDiscriminants, IntoStaticStr}; @@ -337,6 +337,9 @@ constant!(SQLALCHEMY_DATABASE_URI_ENV: EnvVarName = "SQLALCHEMY_DATABASE_URI"); // Name of the env var holding the Flask `SECRET_KEY` for the import job. constant!(SUPERSET_SECRET_KEY_ENV: EnvVarName = "SUPERSET_SECRET_KEY"); +// Name of the import job's only container. +constant!(IMPORT_JOB_CONTAINER_NAME: ContainerName = "superset-import-druid-connection"); + /// Builds the import job. When run it will import the druid connection into the database. async fn build_import_job( superset_cluster: &v1alpha1::SupersetCluster, @@ -405,8 +408,7 @@ async fn build_import_job( ); } - let mut container_builder = ContainerBuilder::new("superset-import-druid-connection") - .expect("ContainerBuilder not created"); + let mut container_builder = new_container_builder(&IMPORT_JOB_CONTAINER_NAME); container_builder .image_from_product_image(resolved_product_image) .command(bash_wrapper_command()) @@ -522,6 +524,7 @@ spec: {} #[test] fn test_constants() { // Test that dereferencing the constants does not panic. + let _ = *IMPORT_JOB_CONTAINER_NAME; let _ = *SQLALCHEMY_DATABASE_URI_ENV; let _ = *SUPERSET_SECRET_KEY_ENV; } From fae828d0ef60017c521133f687d205c435b435e2 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 7 Sep 2026 17:29:51 +0200 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e8d6e84..12d29129 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Since `volumeClaimTemplates` cannot be updated in place, StatefulSets created by older operator versions cannot be updated after the upgrade: delete the `node` StatefulSet(s) so that the operator immediately recreates them with the new labels ([#779]). +- Make operations infallible where appropriate ([#785]). ### Removed @@ -47,6 +48,7 @@ [#773]: https://github.com/stackabletech/superset-operator/pull/773 [#779]: https://github.com/stackabletech/superset-operator/pull/779 [#781]: https://github.com/stackabletech/superset-operator/pull/781 +[#785]: https://github.com/stackabletech/superset-operator/pull/785 ## [26.7.0] - 2026-07-21 From 8c51ae3b78e257f471a3163b506950b74a6f4d7f Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 8 Sep 2026 17:50:42 +0200 Subject: [PATCH 3/4] revert expects where checked data is not static/explicit --- CHANGELOG.md | 2 +- .../src/controller/build/resource/config_map.rs | 12 +++++++++--- .../src/controller/build/resource/deployment.rs | 7 ++++++- .../src/controller/build/resource/statefulset.rs | 16 +++++++++++----- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12d29129..9bae9ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ Since `volumeClaimTemplates` cannot be updated in place, StatefulSets created by older operator versions cannot be updated after the upgrade: delete the `node` StatefulSet(s) so that the operator immediately recreates them with the new labels ([#779]). -- Make operations infallible where appropriate ([#785]). +- Make operations infallible where dependent on static inputs ([#785]). ### Removed diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index 276624c9..e2343f6e 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -22,6 +22,12 @@ pub enum Error { source: superset_config::Error, role_group_name: RoleGroupName, }, + + #[snafu(display("failed to build ConfigMap for role group {role_group_name}"))] + RoleGroupConfig { + source: stackable_operator::builder::configmap::Error, + role_group_name: RoleGroupName, + }, } type Result = std::result::Result; @@ -66,9 +72,9 @@ pub fn build_rolegroup_config_map( cm_builder.add_data(VECTOR_CONFIG_FILE, vector_config); } - Ok(cm_builder - .build() - .expect("The ConfigMap metadata is set in this function.")) + cm_builder.build().with_context(|_| RoleGroupConfigSnafu { + role_group_name: role_group_name.clone(), + }) } #[cfg(test)] diff --git a/rust/operator-binary/src/controller/build/resource/deployment.rs b/rust/operator-binary/src/controller/build/resource/deployment.rs index bca99bad..4843fbda 100644 --- a/rust/operator-binary/src/controller/build/resource/deployment.rs +++ b/rust/operator-binary/src/controller/build/resource/deployment.rs @@ -48,6 +48,11 @@ pub enum Error { GracefulShutdown { source: stackable_operator::builder::pod::Error, }, + + #[snafu(display("failed to add needed volume"))] + AddVolume { + source: stackable_operator::builder::pod::Error, + }, } type Result = std::result::Result; @@ -146,7 +151,7 @@ pub fn build_rolegroup_deployment( resource_names.role_group_config_map().as_ref(), &rolegroup_config.config.logging.superset_container, )) - .expect("The volume names are statically defined and there should be no duplicates."); + .context(AddVolumeSnafu)?; pb.add_container(super::build_metrics_container(&validated.image)); if let Some(vector_container) = diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index a0c132f2..1cc17d5e 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -76,6 +76,11 @@ pub enum Error { AddTlsVolumesAndVolumeMounts { source: stackable_operator::commons::tls_verification::TlsClientDetailsError, }, + + #[snafu(display("failed to add needed volume"))] + AddVolume { + source: stackable_operator::builder::pod::Error, + }, } type Result = std::result::Result; @@ -129,10 +134,11 @@ pub fn build_node_rolegroup_statefulset( authentication_env_vars(&validated.cluster_config.authentication_config), ); - // Operator-managed volumes and volume mounts with static names and paths first: their adds - // are infallible. The authentication volumes and mounts below are named after the user's - // SecretClasses, so they are added afterwards and stay fallible, as they can collide with - // the operator-managed ones. + // Operator-managed volumes and volume mounts with static names and paths first. The mount + // add is infallible because both its arguments are constants; the volume add is fallible + // because the volumes are built by a helper. The authentication volumes and mounts below + // are named after the user's SecretClasses, so they are added afterwards and stay fallible, + // as they can collide with the operator-managed ones. superset_cb .add_volume_mount( super::LISTENER_VOLUME_NAME_PVC.as_ref(), @@ -143,7 +149,7 @@ pub fn build_node_rolegroup_statefulset( resource_names.role_group_config_map().as_ref(), &rolegroup_config.config.logging.superset_container, )) - .expect("The volume names are statically defined and there should be no duplicates."); + .context(AddVolumeSnafu)?; add_authentication_volumes_and_volume_mounts( &validated.cluster_config.authentication_config, From a7aeb5e52f0aea0fe742f053795277d3b1b03c83 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 9 Sep 2026 16:20:46 +0200 Subject: [PATCH 4/4] add comment/test for group_listener_name --- rust/operator-binary/src/crd/mod.rs | 30 +++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 0584320d..84c13b91 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -425,6 +425,8 @@ impl SupersetRole { /// The name of the group listener provided for the role, if the role serves the web UI. /// Nodes will use this group listener so that only one load balancer is needed for that role. + /// + /// The returned ListenerName is a lowercase RFC 1035 label name (checked by a unit test). pub fn group_listener_name(&self, cluster_name: &ClusterName) -> Option { const _: () = assert!( ClusterName::MAX_LENGTH + 1 /* dash */ + RoleName::MAX_LENGTH @@ -625,11 +627,15 @@ impl v1alpha1::SupersetCluster { #[cfg(test)] mod tests { + use std::str::FromStr; + use stackable_operator::versioned::test_utils::RoundtripTestData; + use strum::IntoEnumIterator; use super::{ - BEAT_ROLE_NAME, DEFAULT_LISTENER_CLASS, INTERNAL_SECRET_SECRET_KEY, MAPBOX_API_KEY_ENV, - MAPBOX_API_KEY_SECRET_KEY, NODE_ROLE_NAME, SECRET_KEY_ENV, WORKER_ROLE_NAME, v1alpha1, + BEAT_ROLE_NAME, ClusterName, DEFAULT_LISTENER_CLASS, INTERNAL_SECRET_SECRET_KEY, + MAPBOX_API_KEY_ENV, MAPBOX_API_KEY_SECRET_KEY, NODE_ROLE_NAME, SECRET_KEY_ENV, + SupersetRole, WORKER_ROLE_NAME, v1alpha1, }; #[test] @@ -650,6 +656,26 @@ mod tests { assert_eq!(secret_key_env, internal_secret_secret_key); } + #[test] + fn group_listener_name_is_rfc_1035_label_name() { + // Every ClusterName is a valid RFC 1035 label name, so we use just some string with maximum + // length. + let _ = ClusterName::IS_RFC_1035_LABEL_NAME; + let cluster_name = ClusterName::from_str(&"a".repeat(ClusterName::MAX_LENGTH)) + .expect("is a valid ClusterName"); + + for role in SupersetRole::iter() { + if let Some(group_listener_name) = role.group_listener_name(&cluster_name) { + assert!( + stackable_operator::validation::is_lowercase_rfc_1035_label( + group_listener_name.as_ref() + ) + .is_ok() + ); + } + } + } + impl RoundtripTestData for v1alpha1::SupersetClusterSpec { fn roundtrip_test_data() -> Vec { stackable_operator::utils::yaml_from_str_singleton_map(indoc::indoc! {r#"