chore: Consolidate error enums and use expect where possible

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 dependent on static inputs ([#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
 

rust/operator-binary/src/controller/build/resource/deployment.rs

@@ -44,9 +44,6 @@ 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,
@@ -56,11 +53,6 @@ pub enum Error {
     AddVolume {
         source: stackable_operator::builder::pod::Error,
     },
-
-    #[snafu(display("failed to add needed volumeMount"))]
-    AddVolumeMount {
-        source: stackable_operator::builder::pod::container::Error,
-    },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
@@ -120,8 +112,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())

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 <args>`).
 pub(crate) fn bash_wrapper_command() -> Vec<String> {
@@ -264,24 +254,24 @@ pub(crate) fn build_superset_container_builder(
     validated: &ValidatedCluster,
     rolegroup_config: &SupersetRoleGroupConfig,
     role_specific_env_vars: EnvVarSet,
-) -> Result<ContainerBuilder, Error> {
+) -> 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

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,
@@ -84,11 +81,6 @@ pub enum Error {
     AddVolume {
         source: stackable_operator::builder::pod::Error,
     },
-
-    #[snafu(display("failed to add needed volumeMount"))]
-    AddVolumeMount {
-        source: stackable_operator::builder::pod::container::Error,
-    },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
@@ -135,14 +127,29 @@ 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. 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(),
+            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,
+    ))
+    .context(AddVolumeSnafu)?;
 
     add_authentication_volumes_and_volume_mounts(
         &validated.cluster_config.authentication_config,
@@ -221,24 +228,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) =

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)?;
 

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,41 @@ 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.
+    ///
+    /// 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<ListenerName> {
+        const _: () = assert!(
+            ClusterName::MAX_LENGTH + 1 /* dash */ + RoleName::MAX_LENGTH
+                <= ListenerName::MAX_LENGTH,
+            "The string `<cluster_name>-<role_name>` 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<SupersetRole> 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 +598,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<String> {
-        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<GenericRoleConfig> {
         self.get_role_config(role).map(|r| r.common.to_owned())
     }
@@ -621,29 +627,21 @@ impl v1alpha1::SupersetCluster {
 
 #[cfg(test)]
 mod tests {
-    use stackable_operator::{
-        v2::types::operator::RoleName, versioned::test_utils::RoundtripTestData,
-    };
+    use std::str::FromStr;
+
+    use stackable_operator::versioned::test_utils::RoundtripTestData;
     use strum::IntoEnumIterator;
 
     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, 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,
     };
 
-    /// Locks the invariant behind the `expect` in the `From<SupersetRole> 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;
@@ -658,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<Self> {
             stackable_operator::utils::yaml_from_str_singleton_map(indoc::indoc! {r#"