refactor: cleanup build_statefulset and tls_volume parameters

maltesander · maltesander · commit 4e8723c4d3ef · 2026-06-15T21:08:10.000+02:00
diff --git a/rust/operator-binary/src/command.rs b/rust/operator-binary/src/command.rs
@@ -10,12 +10,12 @@ use crate::{
     authentication::TrinoAuthenticationConfig,
     catalog::config::CatalogConfig,
     config::{client_protocol, fault_tolerant_execution},
-    controller::ValidatedTrinoConfig,
+    controller::{ValidatedCluster, ValidatedTrinoConfig},
     crd::{
         CONFIG_DIR_NAME, Container, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR,
         STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
         STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD,
-        TrinoRole, v1alpha1,
+        TrinoRole,
     },
     trino_controller::{STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR},
 };
@@ -27,7 +27,7 @@ const EXCHANGE_MANAGER_PROPERTIES: &str = "exchange-manager.properties";
 const SPOOLING_MANAGER_PROPERTIES: &str = "spooling-manager.properties";
 
 pub fn container_prepare_args(
-    trino: &v1alpha1::TrinoCluster,
+    cluster: &ValidatedCluster,
     catalogs: &[CatalogConfig],
     merged_config: &ValidatedTrinoConfig,
     resolved_fte_config: &Option<fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig>,
@@ -54,15 +54,15 @@ pub fn container_prepare_args(
     // S3, LDAP, OIDC, FTE or whatnot.
     args.push(format!("cert-tools generate-pkcs12-truststore --pem /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem --out {STACKABLE_CLIENT_TLS_DIR}/truststore.p12 --out-password {STACKABLE_TLS_STORE_PASSWORD}"));
 
-    if trino.tls_enabled() {
+    if cluster.tls_enabled() {
         args.push(format!("cp {STACKABLE_MOUNT_SERVER_TLS_DIR}/truststore.p12 {STACKABLE_SERVER_TLS_DIR}/truststore.p12"));
         args.push(format!("cp {STACKABLE_MOUNT_SERVER_TLS_DIR}/keystore.p12 {STACKABLE_SERVER_TLS_DIR}/keystore.p12"));
     }
 
-    if trino.get_internal_tls().is_some() {
+    if cluster.get_internal_tls().is_some() {
         args.push(format!("cp {STACKABLE_MOUNT_INTERNAL_TLS_DIR}/truststore.p12 {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12"));
         args.push(format!("cp {STACKABLE_MOUNT_INTERNAL_TLS_DIR}/keystore.p12 {STACKABLE_INTERNAL_TLS_DIR}/keystore.p12"));
-        if trino.tls_enabled() {
+        if cluster.tls_enabled() {
             args.push(format!("cert-tools generate-pkcs12-truststore --pkcs12 {STACKABLE_MOUNT_SERVER_TLS_DIR}/truststore.p12:{STACKABLE_TLS_STORE_PASSWORD} --pkcs12 {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12:{STACKABLE_TLS_STORE_PASSWORD} --out {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12 --out-password {STACKABLE_TLS_STORE_PASSWORD}"));
         }
     }
diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs
@@ -6,15 +6,11 @@ use stackable_operator::{
     },
     crd::listener::v1alpha1::{Listener, ListenerPort, ListenerSpec},
     k8s_openapi::api::core::v1::PersistentVolumeClaim,
-    kube::ResourceExt,
     kvp::Labels,
     v2::builder::meta::ownerreference_from_resource,
 };
 
-use crate::{
-    controller::ValidatedCluster,
-    crd::{TrinoRole, v1alpha1},
-};
+use crate::{controller::ValidatedCluster, crd::TrinoRole};
 
 pub const LISTENER_VOLUME_NAME: &str = "listener";
 pub const LISTENER_VOLUME_DIR: &str = "/stackable/listener";
@@ -64,11 +60,11 @@ pub fn build_group_listener_pvc(
 /// The name of the group-listener provided for a specific role-group.
 /// Coordinator(s) will use this group listener so that only one load balancer
 /// is needed (per role group).
-pub fn group_listener_name(trino: &v1alpha1::TrinoCluster, role: &TrinoRole) -> Option<String> {
+pub fn group_listener_name(cluster: &ValidatedCluster, role: &TrinoRole) -> Option<String> {
     match role {
         TrinoRole::Coordinator => Some(format!(
             "{cluster_name}-{role}",
-            cluster_name = trino.name_any()
+            cluster_name = cluster.name
         )),
         TrinoRole::Worker => None,
     }
diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs
@@ -41,10 +41,8 @@ use stackable_operator::{
 };
 
 use crate::{
-    authorization::opa::{OPA_TLS_VOLUME_NAME, TrinoOpaConfig},
-    catalog::config::CatalogConfig,
+    authorization::opa::OPA_TLS_VOLUME_NAME,
     command,
-    config::{client_protocol, fault_tolerant_execution},
     controller::{
         TrinoRoleGroupConfig, ValidatedCluster, build,
         build::resource::listener::{
@@ -58,7 +56,7 @@ use crate::{
         METRICS_PORT_NAME, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR,
         STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
         STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD,
-        TrinoRole, v1alpha1,
+        TrinoRole,
     },
     trino_controller::{
         MAX_PREPARE_LOG_FILE_SIZE, STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR,
@@ -77,9 +75,6 @@ pub enum Error {
     #[snafu(display("missing secret lifetime"))]
     MissingSecretLifetime,
 
-    #[snafu(display("internal operator failure: {source}"))]
-    InternalOperatorFailure { source: crate::crd::Error },
-
     #[snafu(display("illegal container name: [{container_name}]"))]
     IllegalContainerName {
         source: stackable_operator::builder::pod::container::Error,
@@ -140,7 +135,6 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 /// corresponding [`stackable_operator::k8s_openapi::api::core::v1::Service`] (from
 /// [`build_rolegroup_headless_service`](super::service::build_rolegroup_headless_service)).
 pub fn build_rolegroup_statefulset(
-    trino: &v1alpha1::TrinoCluster,
     cluster: &ValidatedCluster,
     trino_role: &TrinoRole,
     role_group_name: &str,
@@ -158,13 +152,6 @@ pub fn build_rolegroup_statefulset(
     let env_overrides = &role_group_config.env_overrides;
     let merged_config = &role_group_config.config;
 
-    let role = trino
-        .role(trino_role)
-        .context(InternalOperatorFailureSnafu)?;
-    let rolegroup = trino
-        .rolegroup(trino_role, role_group_name)
-        .context(InternalOperatorFailureSnafu)?;
-
     let resource_names = cluster.resource_names(trino_role, role_group_name);
     let config_map_name = resource_names.role_group_config_map().to_string();
 
@@ -187,14 +174,14 @@ pub fn build_rolegroup_statefulset(
     // additional authentication env vars
     let mut env = trino_authentication_config.env_vars(trino_role, &Container::Trino);
 
-    let internal_secret_name = shared_internal_secret_name(trino);
+    let internal_secret_name = shared_internal_secret_name(&cluster.name);
     env.push(env_var_from_secret(
         &internal_secret_name,
         None,
         ENV_INTERNAL_SECRET,
     ));
 
-    let spooling_secret_name = shared_spooling_secret_name(trino);
+    let spooling_secret_name = shared_spooling_secret_name(&cluster.name);
     env.push(env_var_from_secret(
         &spooling_secret_name,
         None,
@@ -244,16 +231,12 @@ pub fn build_rolegroup_statefulset(
 
     // add volume mounts depending on the client tls, internal tls, catalogs and authentication
     tls_volume_mounts(
-        trino,
+        cluster,
         trino_role,
         &mut pod_builder,
         &mut cb_prepare,
         &mut cb_trino,
-        catalogs,
         &requested_secret_lifetime,
-        resolved_fte_config,
-        resolved_spooling_config,
-        trino_opa_config,
     )?;
 
     let mut prepare_args = vec![];
@@ -268,7 +251,7 @@ pub fn build_rolegroup_statefulset(
     }
 
     prepare_args.extend(command::container_prepare_args(
-        trino,
+        cluster,
         catalogs,
         merged_config,
         resolved_fte_config,
@@ -317,7 +300,7 @@ pub fn build_rolegroup_statefulset(
 
     let mut persistent_volume_claims = vec![];
     // Add listener
-    if let Some(group_listener_name) = group_listener_name(trino, trino_role) {
+    if let Some(group_listener_name) = group_listener_name(cluster, trino_role) {
         cb_trino
             .add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR)
             .context(AddVolumeMountSnafu)?;
@@ -442,8 +425,10 @@ pub fn build_rolegroup_statefulset(
         .security_context(PodSecurityContextBuilder::new().fs_group(1000).build());
 
     let mut pod_template = pod_builder.build_template();
-    pod_template.merge_from(role.config.pod_overrides.clone());
-    pod_template.merge_from(rolegroup.config.pod_overrides.clone());
+    // `pod_overrides` already carries the merged role + role-group overrides (see
+    // `with_validated_config`), so a single merge here is equivalent to the previous
+    // role-then-role-group sequence.
+    pod_template.merge_from(role_group_config.pod_overrides.clone());
 
     let annotations = restarter_ignore_secret_annotations(
         trino_authentication_config
@@ -454,7 +439,7 @@ pub fn build_rolegroup_statefulset(
 
     Ok(StatefulSet {
         metadata: ObjectMetaBuilder::new()
-            .name_and_namespace(trino)
+            .name_and_namespace(cluster)
             .name(resource_names.stateful_set_name().to_string())
             .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
             .with_labels(cluster.recommended_labels(trino_role, role_group_name))
@@ -463,7 +448,7 @@ pub fn build_rolegroup_statefulset(
             .build(),
         spec: Some(StatefulSetSpec {
             pod_management_policy: Some("Parallel".to_string()),
-            replicas: rolegroup.replicas.map(i32::from),
+            replicas: Some(i32::from(role_group_config.replicas)),
             selector: LabelSelector {
                 match_labels: Some(
                     cluster
@@ -635,20 +620,20 @@ fn create_tls_volume(
         .build())
 }
 
-#[allow(clippy::too_many_arguments)]
 fn tls_volume_mounts(
-    trino: &v1alpha1::TrinoCluster,
+    cluster: &ValidatedCluster,
     trino_role: &TrinoRole,
     pod_builder: &mut PodBuilder,
     cb_prepare: &mut ContainerBuilder,
     cb_trino: &mut ContainerBuilder,
-    catalogs: &[CatalogConfig],
     requested_secret_lifetime: &Duration,
-    resolved_fte_config: &Option<fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig>,
-    resolved_spooling_config: &Option<client_protocol::ResolvedClientProtocolConfig>,
-    trino_opa_config: &Option<TrinoOpaConfig>,
 ) -> Result<()> {
-    if let Some(server_tls) = trino.get_server_tls() {
+    let catalogs = cluster.cluster_config.catalogs.as_slice();
+    let resolved_fte_config = &cluster.cluster_config.fault_tolerant_execution;
+    let resolved_spooling_config = &cluster.cluster_config.client_protocol;
+    let trino_opa_config = &cluster.cluster_config.authorization;
+
+    if let Some(server_tls) = cluster.get_server_tls() {
         cb_prepare
             .add_volume_mount("server-tls-mount", STACKABLE_MOUNT_SERVER_TLS_DIR)
             .context(AddVolumeMountSnafu)?;
@@ -686,7 +671,7 @@ fn tls_volume_mounts(
         .add_empty_dir_volume("client-tls", None)
         .context(AddVolumeSnafu)?;
 
-    if let Some(internal_tls) = trino.get_internal_tls() {
+    if let Some(internal_tls) = cluster.get_internal_tls() {
         cb_prepare
             .add_volume_mount("internal-tls-mount", STACKABLE_MOUNT_INTERNAL_TLS_DIR)
             .context(AddVolumeMountSnafu)?;
diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs
@@ -149,6 +149,24 @@ impl ValidatedCluster {
         self.cluster_config.tls.server.is_some()
     }
 
+    /// The user-provided server TLS SecretClass, if any.
+    ///
+    /// Mirrors [`v1alpha1::TrinoCluster::get_server_tls`] but reads from the validated config so
+    /// build steps don't need the raw cluster.
+    pub fn get_server_tls(&self) -> Option<&str> {
+        self.cluster_config.tls.server.as_deref()
+    }
+
+    /// The user-provided internal TLS SecretClass, if any.
+    pub fn get_internal_tls(&self) -> Option<&str> {
+        self.cluster_config.tls.internal.as_deref()
+    }
+
+    /// Whether client TLS should be set, depending on authentication and server TLS settings.
+    pub fn tls_enabled(&self) -> bool {
+        self.cluster_config.authentication_enabled || self.server_tls_enabled()
+    }
+
     /// The client-facing port Trino exposes: HTTPS when server TLS is enabled, otherwise HTTP.
     ///
     /// Replaces `v1alpha1::TrinoCluster::exposed_port`, derived here from the validated TLS config
diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs
@@ -241,10 +241,11 @@ pub fn validate(
         role_group_configs.insert(trino_role, groups);
     }
 
+    let tls = &trino.spec.cluster_config.tls;
     let cluster_config = ValidatedClusterConfig {
         tls: ValidatedTls {
-            server: trino.get_server_tls().map(String::from),
-            internal: trino.get_internal_tls().map(String::from),
+            server: tls.server_secret_class.as_ref().map(ToString::to_string),
+            internal: tls.internal_secret_class.as_ref().map(ToString::to_string),
         },
         authentication,
         authentication_enabled: trino.authentication_enabled(),
diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs
diff --git a/rust/operator-binary/src/trino_controller.rs b/rust/operator-binary/src/trino_controller.rs
