chore: Consolidate error enums and use expect where possible

CHANGELOG.md

@@ -33,6 +33,7 @@ All notable changes to this project will be documented in this file.
   a StatefulSet are immutable, StatefulSets created by older operator versions cannot be
   updated in place: after the operator upgrade, delete each node StatefulSet so that the
   operator immediately recreates it with the new labels ([#984]).
+- Make operations infallible where dependent on static inputs ([#990]).
 
 ### Fixed
 
@@ -51,6 +52,7 @@ All notable changes to this project will be documented in this file.
 [#982]: https://github.com/stackabletech/nifi-operator/pull/982
 [#984]: https://github.com/stackabletech/nifi-operator/pull/984
 [#985]: https://github.com/stackabletech/nifi-operator/pull/985
+[#990]: https://github.com/stackabletech/nifi-operator/pull/990
 
 ## [26.7.0] - 2026-07-21
 

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

@@ -11,7 +11,7 @@ use stackable_operator::{
         },
         types::{
             kubernetes::{ListenerClassName, ListenerName, PersistentVolumeClaimName},
-            operator::RoleName,
+            operator::{ClusterName, RoleName},
         },
     },
 };
@@ -24,11 +24,11 @@ use crate::{
     crd::NifiRole,
 };
 
-pub const LISTENER_VOLUME_NAME: &str = "listener";
 pub const LISTENER_VOLUME_DIR: &str = "/stackable/listener";
 
 // The listener volume is provisioned as a PVC by the listener-operator; this is its typed name.
-constant!(LISTENER_PVC_NAME: PersistentVolumeClaimName = "listener");
+// The volume mount referencing the PVC and the secret-operator listener scope use the same name.
+constant!(pub LISTENER_PVC_NAME: PersistentVolumeClaimName = "listener");
 
 pub fn build_group_listener(
     cluster: &ValidatedCluster,
@@ -68,23 +68,53 @@ pub fn build_group_listener_pvc(
     )
 }
 
+/// The returned ListenerName is a lowercase RFC 1035 label name (checked by a unit test).
 pub fn group_listener_name(cluster: &ValidatedCluster, role_name: &RoleName) -> 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;
+
     ListenerName::from_str(&format!(
         "{cluster_name}-{role_name}",
         cluster_name = cluster.name
     ))
-    .expect(
-        "the cluster name and role name form a valid Listener name, because both are length-bounded types whose combined length stays within the Listener name limit",
-    )
+    .expect("The role listener name is a valid Listener name.")
 }
 
 #[cfg(test)]
 mod tests {
+    use strum::IntoEnumIterator;
+
     use super::*;
+    use crate::controller::build::properties::test_support::minimal_validated_cluster;
 
     #[test]
     fn test_constants() {
         // Test that dereferencing the constants does not panic.
         let _ = *LISTENER_PVC_NAME;
     }
+
+    #[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 mut cluster = minimal_validated_cluster();
+        cluster.name = ClusterName::from_str(&"a".repeat(ClusterName::MAX_LENGTH))
+            .expect("is a valid ClusterName");
+
+        for role in NifiRole::iter() {
+            let group_listener_name = group_listener_name(&cluster, &role);
+            assert!(
+                stackable_operator::validation::is_lowercase_rfc_1035_label(
+                    group_listener_name.as_ref()
+                )
+                .is_ok()
+            );
+        }
+    }
 }

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

@@ -59,7 +59,7 @@ use crate::{
             recommended_labels_for_unversioned_role_group_resources,
             resource::{
                 listener::{
-                    LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener_pvc,
+                    LISTENER_PVC_NAME, LISTENER_VOLUME_DIR, build_group_listener_pvc,
                     group_listener_name,
                 },
                 probes::{
@@ -363,25 +363,25 @@ pub(crate) fn build_node_rolegroup_statefulset(
         )
         .context(AddVolumeMountSnafu)?
         .add_volume_mount(CONFIG_VOLUME_NAME.to_string(), CONFIG_VOLUME_MOUNT)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(
             KEYSTORE_VOLUME_NAME.to_string(),
             KEYSTORE_NIFI_CONTAINER_MOUNT,
         )
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(ACTIVE_CONFIG_VOLUME_NAME.to_string(), NIFI_CONFIG_DIRECTORY)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(
             SENSITIVE_PROPERTY_VOLUME_NAME.to_string(),
             SENSITIVE_PROPERTY_VOLUME_MOUNT,
         )
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(LOG_VOLUME_NAME.to_string(), STACKABLE_LOG_DIR)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(TRUSTSTORE_VOLUME_NAME.to_string(), STACKABLE_SERVER_TLS_DIR)
-        .context(AddVolumeMountSnafu)?
-        .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
+        .add_volume_mount(&*LISTENER_PVC_NAME, LISTENER_VOLUME_DIR)
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mounts(authorization_config.get_volume_mounts())
         .context(AddVolumeMountSnafu)?
         .resources(
@@ -425,25 +425,30 @@ pub(crate) fn build_node_rolegroup_statefulset(
             KEYSTORE_VOLUME_NAME.to_string(),
             KEYSTORE_NIFI_CONTAINER_MOUNT,
         )
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mounts(
             PERSISTENT_REPOSITORIES
                 .iter()
                 .map(NifiRepository::volume_mount),
         )
         .context(AddVolumeMountSnafu)?
         .add_volume_mount(ACTIVE_CONFIG_VOLUME_NAME.to_string(), NIFI_CONFIG_DIRECTORY)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(LOG_CONFIG_VOLUME_NAME.to_string(), 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.to_string(), STACKABLE_LOG_DIR)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mount(TRUSTSTORE_VOLUME_NAME.to_string(), STACKABLE_SERVER_TLS_DIR)
-        .context(AddVolumeMountSnafu)?
-        .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR)
-        .context(AddVolumeMountSnafu)?
+        .expect("The mount paths are statically defined and there should be no duplicates.")
+        .add_volume_mount(&*LISTENER_PVC_NAME, LISTENER_VOLUME_DIR)
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_volume_mounts(authorization_config.get_volume_mounts())
         .context(AddVolumeMountSnafu)?
+        .add_volume_mount(
+            PYTHON_WORKING_DIR_VOLUME_NAME.to_string(),
+            NIFI_PYTHON_WORKING_DIRECTORY,
+        )
+        .expect("The mount paths are statically defined and there should be no duplicates.")
         .add_container_port(HTTPS_PORT_NAME, HTTPS_PORT.into())
         .add_container_port(PROTOCOL_PORT_NAME, PROTOCOL_PORT.into())
         .add_container_port(BALANCE_PORT_NAME, BALANCE_PORT.into())
@@ -459,7 +464,14 @@ pub(crate) fn build_node_rolegroup_statefulset(
 
     add_graceful_shutdown_config(merged_config, &mut pod_builder).context(GracefulShutdownSnafu)?;
 
-    // Add user configured extra volumes if any are specified
+    pod_builder
+        .add_empty_dir_volume(PYTHON_WORKING_DIR_VOLUME_NAME.to_string(), None)
+        .expect("The volume names are statically defined and there should be no duplicates.");
+
+    // Mount the user configured extra volumes, if any are specified. The mounts come after every
+    // operator-managed mount of the NiFi container, so they can collide with those and stay
+    // fallible. The volumes themselves are added to the Pod at the very end, after every
+    // operator-managed volume.
     for volume in &cluster.cluster_config.extra_volumes {
         // Extract values into vars so we make it impossible to log something other than
         // what we actually use to create the mounts - maybe paranoid, but hey ..
@@ -472,24 +484,11 @@ pub(crate) fn build_node_rolegroup_statefulset(
             role = NifiRole::Node.as_ref(),
             "Adding user specified extra volume",
         );
-        pod_builder
-            .add_volume(volume.clone())
-            .context(AddVolumeSnafu)?;
         container_nifi
             .add_volume_mount(volume_name, mount_point)
             .context(AddVolumeMountSnafu)?;
     }
 
-    pod_builder
-        .add_empty_dir_volume(PYTHON_WORKING_DIR_VOLUME_NAME.to_string(), None)
-        .context(AddVolumeSnafu)?;
-    container_nifi
-        .add_volume_mount(
-            PYTHON_WORKING_DIR_VOLUME_NAME.to_string(),
-            NIFI_PYTHON_WORKING_DIRECTORY,
-        )
-        .context(AddVolumeMountSnafu)?;
-
     container_nifi
         .add_volume_mounts(git_sync_resources.git_content_volume_mounts.to_owned())
         .context(AddVolumeMountSnafu)?;
@@ -505,12 +504,6 @@ pub(crate) fn build_node_rolegroup_statefulset(
     for container in git_sync_resources.git_sync_init_containers.iter().cloned() {
         pod_builder.add_init_container(container);
     }
-    pod_builder
-        .add_volumes(git_sync_resources.git_content_volumes.to_owned())
-        .context(AddVolumeSnafu)?;
-    pod_builder
-        .add_volumes(git_sync_resources.git_ca_cert_volumes.to_owned())
-        .context(AddVolumeSnafu)?;
 
     // The NiFi `log-config` volume sources from the custom log ConfigMap when one is configured,
     // otherwise from this rolegroup's ConfigMap (which carries the operator-generated `logback.xml`).
@@ -547,22 +540,14 @@ pub(crate) fn build_node_rolegroup_statefulset(
         ));
     }
 
-    authentication_config
-        .add_volumes_and_mounts(&mut pod_builder, vec![&mut container_prepare])
-        .context(AddAuthVolumesSnafu)?;
-
-    let metadata = ObjectMetaBuilder::new()
-        .with_labels(recommended_object_labels)
-        .build();
-
     let requested_secret_lifetime = merged_config
         .requested_secret_lifetime
         .context(MissingSecretLifetimeSnafu)?;
+    // Operator-managed volumes first. Only the volume mounts with static paths are added
+    // infallibly, every volume add is fallible. The volumes derived from user input
+    // (authentication, git-sync) and the user's `extraVolumes` are added afterwards, so a name
+    // collision is reported on the user-derived side.
     pod_builder
-        .metadata(metadata)
-        .image_pull_secrets_from_product_image(resolved_product_image)
-        .add_init_container(container_prepare.build())
-        .affinity(&merged_config.affinity)
         // The rolegroup `ConfigMap` mounted as-is (it also carries `vector.yaml`); read by the
         // Vector sidecar via [`VECTOR_LOG_CONFIG_VOLUME_NAME`].
         .add_volume(Volume {
@@ -606,13 +591,13 @@ pub(crate) fn build_node_rolegroup_statefulset(
                     .to_string()],
                 SecretFormat::TlsPkcs12,
                 &requested_secret_lifetime,
-                Some(LISTENER_VOLUME_NAME),
+                Some(LISTENER_PVC_NAME.as_ref()),
             )
             .context(BuildTlsVolumeSnafu)?,
         )
         .context(AddVolumeSnafu)?
         .add_empty_dir_volume(TRUSTSTORE_VOLUME_NAME.to_string(), None)
-        .context(AddVolumeSnafu)?
+        .expect("The volume names are statically defined and there should be no duplicates.")
         .add_volumes(
             authorization_config
                 .get_volumes()
@@ -638,7 +623,30 @@ pub(crate) fn build_node_rolegroup_statefulset(
             name: ACTIVE_CONFIG_VOLUME_NAME.to_string(),
             ..Volume::default()
         })
-        .context(AddVolumeSnafu)?
+        .context(AddVolumeSnafu)?;
+
+    // Volumes derived from user input: the authentication volumes are named after the user's
+    // SecretClasses (the helper adds its own static `admin` volume first), the git-sync volumes
+    // are numbered per configured repository.
+    authentication_config
+        .add_volumes_and_mounts(&mut pod_builder, vec![&mut container_prepare])
+        .context(AddAuthVolumesSnafu)?;
+    pod_builder
+        .add_volumes(git_sync_resources.git_content_volumes.to_owned())
+        .context(AddVolumeSnafu)?;
+    pod_builder
+        .add_volumes(git_sync_resources.git_ca_cert_volumes.to_owned())
+        .context(AddVolumeSnafu)?;
+
+    let metadata = ObjectMetaBuilder::new()
+        .with_labels(recommended_object_labels)
+        .build();
+
+    pod_builder
+        .metadata(metadata)
+        .image_pull_secrets_from_product_image(resolved_product_image)
+        .add_init_container(container_prepare.build())
+        .affinity(&merged_config.affinity)
         .service_account_name(
             cluster
                 .cluster_resource_names()
@@ -651,6 +659,14 @@ pub(crate) fn build_node_rolegroup_statefulset(
                 .build(),
         );
 
+    // User configured extra volumes last: their names can collide with the operator-managed
+    // volumes above, so these adds stay fallible.
+    for volume in &cluster.cluster_config.extra_volumes {
+        pod_builder
+            .add_volume(volume.clone())
+            .context(AddVolumeSnafu)?;
+    }
+
     let mut pod_template = pod_builder.build_template();
     // `rg.pod_overrides` is already the merged role <- rolegroup overrides.
     pod_template.merge_from(rg.pod_overrides.clone());
@@ -840,4 +856,47 @@ mod tests {
             )]
         );
     }
+
+    /// A user-supplied extra volume whose name collides with an operator-managed volume must be
+    /// reported as an error (the operator's own volumes are added first, so the collision
+    /// surfaces on the user-supplied side).
+    #[test]
+    fn extra_volume_colliding_with_operator_volume_is_an_error() {
+        let cluster = validated_cluster_from_yaml(
+            r#"
+            apiVersion: nifi.stackable.tech/v1alpha1
+            kind: NifiCluster
+            metadata:
+              name: simple-nifi
+              namespace: default
+            spec:
+              image:
+                productVersion: 2.9.0
+              clusterConfig:
+                authentication:
+                  - authenticationClass: nifi-admin-credentials-simple
+                sensitiveProperties:
+                  keySecret: simple-nifi-sensitive-property-key
+                  autoGenerate: true
+                extraVolumes:
+                  - name: log
+                    emptyDir: {}
+              nodes:
+                roleGroups:
+                  default:
+                    replicas: 1
+            "#,
+        );
+        let role_group_name = RoleGroupName::from_str("default").expect("valid role group name");
+        let rg = default_rg(&cluster);
+
+        let Err(error) = build_node_rolegroup_statefulset(&cluster, &role_group_name, rg, None)
+        else {
+            panic!("the colliding extra volume must be rejected");
+        };
+        assert!(
+            matches!(error, Error::AddVolume { .. }),
+            "unexpected error: {error:?}"
+        );
+    }
 }