From 7171c3e4a7c7a05eb784ae8676f12944d72830fb Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 10 Jul 2026 17:12:32 +0200 Subject: [PATCH 1/2] refactor: Make the DaemonSet builder take a ServiceAccount name --- .../controller/build/resource/daemonset.rs | 26 ++++--------------- rust/operator-binary/src/opa_controller.rs | 2 +- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/rust/operator-binary/src/controller/build/resource/daemonset.rs b/rust/operator-binary/src/controller/build/resource/daemonset.rs index f4324e7a..4451611a 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset.rs +++ b/rust/operator-binary/src/controller/build/resource/daemonset.rs @@ -30,12 +30,11 @@ use stackable_operator::{ apps::v1::{DaemonSet, DaemonSetSpec, DaemonSetUpdateStrategy, RollingUpdateDaemonSet}, core::v1::{ EmptyDirVolumeSource, EnvVarSource, HTTPGetAction, ObjectFieldSelector, Probe, - ResourceRequirements, SecretVolumeSource, ServiceAccount, + ResourceRequirements, SecretVolumeSource, }, }, apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString}, }, - kube::ResourceExt, memory::{BinaryMultiple, MemoryQuantity}, product_logging::{ self, @@ -245,7 +244,7 @@ pub fn build_server_rolegroup_daemonset( role_group: &OpaRoleGroupConfig, opa_bundle_builder_image: &str, user_info_fetcher_image: &str, - service_account: &ServiceAccount, + service_account_name: &str, cluster_info: &KubernetesClusterInfo, ) -> Result { let resolved_product_image = &cluster.image; @@ -405,7 +404,7 @@ pub fn build_server_rolegroup_daemonset( .build(), ) .context(AddVolumeSnafu)? - .service_account_name(service_account.name_any()) + .service_account_name(service_account_name) .security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); if let Some(tls) = &cluster.cluster_config.tls { @@ -847,12 +846,7 @@ fn build_prepare_start_command( #[cfg(test)] mod tests { use serde_json::json; - use stackable_operator::{ - commons::networking::DomainName, - k8s_openapi::{ - api::core::v1::ServiceAccount, apimachinery::pkg::apis::meta::v1::ObjectMeta, - }, - }; + use stackable_operator::commons::networking::DomainName; use super::*; use crate::{ @@ -865,16 +859,6 @@ mod tests { } } - fn service_account() -> ServiceAccount { - ServiceAccount { - metadata: ObjectMeta { - name: Some("test-opa-serviceaccount".to_owned()), - ..Default::default() - }, - ..Default::default() - } - } - fn build(cluster: &ValidatedCluster) -> DaemonSet { let (role_group_name, role_group) = cluster.role_group_configs[&OpaRole::Server] .iter() @@ -886,7 +870,7 @@ mod tests { role_group, "bundle-builder-image", "user-info-fetcher-image", - &service_account(), + "test-opa-serviceaccount", &cluster_info(), ) .expect("the daemonset should build") diff --git a/rust/operator-binary/src/opa_controller.rs b/rust/operator-binary/src/opa_controller.rs index 2e74f7c2..bd148883 100644 --- a/rust/operator-binary/src/opa_controller.rs +++ b/rust/operator-binary/src/opa_controller.rs @@ -228,7 +228,7 @@ pub async fn reconcile_opa( rolegroup, &ctx.opa_bundle_builder_image, &ctx.user_info_fetcher_image, - &rbac_sa, + &rbac_sa.name_any(), &ctx.cluster_info, ) .with_context(|_| BuildRoleGroupDaemonSetSnafu { From d77e833d56fe29ad89455e0cb5d2c58a5aa6c372 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 10 Jul 2026 17:28:15 +0200 Subject: [PATCH 2/2] refactor: Introduce build aggregator --- rust/operator-binary/src/controller/build.rs | 163 +++++++++++++++++- rust/operator-binary/src/controller/mod.rs | 16 ++ rust/operator-binary/src/opa_controller.rs | 171 +++++-------------- 3 files changed, 220 insertions(+), 130 deletions(-) diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index 11d58032..a87f1bfe 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -3,13 +3,104 @@ use std::str::FromStr; -use stackable_operator::v2::types::common::Port; +use snafu::{ResultExt, Snafu}; +use stackable_operator::{utils::cluster_info::KubernetesClusterInfo, v2::types::common::Port}; -use crate::controller::RoleGroupName; +use crate::controller::{ + KubernetesResources, RoleGroupName, ValidatedCluster, + build::resource::{ + config_map::build_rolegroup_config_map, + daemonset::build_server_rolegroup_daemonset, + discovery::build_discovery_config_map, + service::{ + build_rolegroup_headless_service, build_rolegroup_metrics_service, + build_server_role_service, + }, + }, +}; pub mod properties; pub mod resource; +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("failed to build ConfigMap for role group {role_group}"))] + ConfigMap { + source: resource::config_map::Error, + role_group: RoleGroupName, + }, + + #[snafu(display("failed to build DaemonSet for role group {role_group}"))] + DaemonSet { + source: resource::daemonset::Error, + role_group: RoleGroupName, + }, + + #[snafu(display("failed to build the discovery ConfigMap"))] + Discovery { source: resource::discovery::Error }, +} + +/// Builds every Kubernetes resource for the given validated cluster. +/// +/// Does not need a Kubernetes client: every reference to another Kubernetes resource is already +/// dereferenced and validated by this point, so the errors returned here are resource-assembly +/// failures only. +/// +/// `service_account_name` is the name of the RBAC `ServiceAccount` the Pods run under (the RBAC +/// resources are built and applied separately, in the reconcile step). `cluster_info` supplies the +/// Kubernetes cluster domain used in the discovery URL and the sidecar environment. +pub fn build( + cluster: &ValidatedCluster, + service_account_name: &str, + opa_bundle_builder_image: &str, + user_info_fetcher_image: &str, + cluster_info: &KubernetesClusterInfo, +) -> Result { + let mut daemon_sets = vec![]; + let mut services = vec![]; + let mut config_maps = vec![]; + + // The role-level load-balanced Service, which is not bound to a single role group. + services.push(build_server_role_service(cluster)); + + for role_group_configs in cluster.role_group_configs.values() { + for (role_group_name, role_group) in role_group_configs { + config_maps.push( + build_rolegroup_config_map(cluster, role_group_name, role_group).context( + ConfigMapSnafu { + role_group: role_group_name.clone(), + }, + )?, + ); + services.push(build_rolegroup_headless_service(cluster, role_group_name)); + services.push(build_rolegroup_metrics_service(cluster, role_group_name)); + daemon_sets.push( + build_server_rolegroup_daemonset( + cluster, + role_group_name, + role_group, + opa_bundle_builder_image, + user_info_fetcher_image, + service_account_name, + cluster_info, + ) + .context(DaemonSetSnafu { + role_group: role_group_name.clone(), + })?, + ); + } + } + + // The cluster-level discovery ConfigMap. + config_maps.push(build_discovery_config_map(cluster, cluster_info).context(DiscoverySnafu)?); + + Ok(KubernetesResources { + daemon_sets, + services, + config_maps, + }) +} + /// The port the bundle-builder sidecar listens on, and which OPA connects to for bundle polling. pub(crate) const BUNDLE_BUILDER_PORT: Port = Port(3030); @@ -21,3 +112,71 @@ stackable_operator::constant!(pub(crate) PLACEHOLDER_ROLE_LEVEL_ROLE_GROUP: Role // Placeholder role-group name for the recommended labels of the discovery `ConfigMap`, which is a // cluster-level object not bound to a single role group. stackable_operator::constant!(pub(crate) PLACEHOLDER_DISCOVERY_ROLE_GROUP: RoleGroupName = "discovery"); + +#[cfg(test)] +mod tests { + use serde_json::json; + use stackable_operator::{ + commons::networking::DomainName, kube::Resource, utils::cluster_info::KubernetesClusterInfo, + }; + + use super::build; + use crate::controller::{ + ValidatedCluster, build::properties::test_support::validated_cluster_from_spec, + }; + + fn cluster_info() -> KubernetesClusterInfo { + KubernetesClusterInfo { + cluster_domain: DomainName::try_from("cluster.local").unwrap(), + } + } + + fn sorted_names(resources: &[impl Resource]) -> Vec<&str> { + let mut names: Vec<&str> = resources + .iter() + .filter_map(|resource| resource.meta().name.as_deref()) + .collect(); + names.sort(); + names + } + + /// A validated single-role-group cluster with no TLS or extra sidecars. + fn cluster() -> ValidatedCluster { + validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + })) + } + + #[test] + fn build_produces_expected_resource_names() { + let resources = build( + &cluster(), + "test-opa-serviceaccount", + "bundle-builder-image", + "user-info-fetcher-image", + &cluster_info(), + ) + .expect("build succeeds"); + + // One DaemonSet per role group. + assert_eq!( + sorted_names(&resources.daemon_sets), + ["test-opa-server-default"] + ); + // The role-level Service plus a headless and a metrics Service per role group. + assert_eq!( + sorted_names(&resources.services), + [ + "test-opa-server", + "test-opa-server-default-headless", + "test-opa-server-default-metrics", + ] + ); + // The per-role-group ConfigMap plus the cluster-level discovery ConfigMap (`test-opa`). + assert_eq!( + sorted_names(&resources.config_maps), + ["test-opa", "test-opa-server-default"] + ); + } +} diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 31336579..c7e50986 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -11,6 +11,10 @@ use stackable_operator::{ product_image_selection::ResolvedProductImage, resources::{NoRuntimeLimits, Resources}, }, + k8s_openapi::api::{ + apps::v1::DaemonSet, + core::v1::{ConfigMap, Service}, + }, kube::{Resource as KubeResource, api::ObjectMeta}, kvp::Labels, shared::time::Duration, @@ -232,6 +236,18 @@ impl KubeResource for ValidatedCluster { } } +/// Every Kubernetes resource produced by the [`build`](build::build) step. +/// +/// OPA runs as a `DaemonSet` (one Pod per node), so there are no `StatefulSet`s, PDBs or +/// `Listener`s. `services` holds the role-level `Service` and the per-role-group headless and +/// metrics `Service`s; `config_maps` holds the per-role-group `ConfigMap`s and the cluster-level +/// discovery `ConfigMap`. +pub struct KubernetesResources { + pub daemon_sets: Vec, + pub services: Vec, + pub config_maps: Vec, +} + /// Cluster-wide settings resolved once during validation, so the build steps no longer need the /// raw `OpaCluster` to render config (except for owner references). pub struct ValidatedClusterConfig { diff --git a/rust/operator-binary/src/opa_controller.rs b/rust/operator-binary/src/opa_controller.rs index bd148883..92138745 100644 --- a/rust/operator-binary/src/opa_controller.rs +++ b/rust/operator-binary/src/opa_controller.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, sync::Arc}; +use std::sync::Arc; use const_format::concatcp; use serde_json::json; @@ -25,15 +25,8 @@ use stackable_operator::{ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ - controller::{ - RoleGroupName, build, - build::resource::service::{ - build_rolegroup_headless_service, build_rolegroup_metrics_service, - build_server_role_service, - }, - controller_name, operator_name, product_name, validate, - }, - crd::{APP_NAME, OPERATOR_NAME, OpaClusterStatus, OpaRole, v1alpha2}, + controller::{build, controller_name, operator_name, product_name, validate}, + crd::{APP_NAME, OPERATOR_NAME, OpaClusterStatus, v1alpha2}, }; pub const OPA_CONTROLLER_NAME: &str = "opacluster"; @@ -61,45 +54,21 @@ pub enum Error { source: Box, }, - #[snafu(display("failed to apply role Service"))] - ApplyRoleService { - source: stackable_operator::cluster_resources::Error, - }, - - #[snafu(display("failed to apply Service for [{rolegroup}]"))] - ApplyRoleGroupService { - source: stackable_operator::cluster_resources::Error, - rolegroup: RoleGroupName, - }, - - #[snafu(display("failed to build ConfigMap for [{rolegroup}]"))] - BuildRoleGroupConfig { - source: build::resource::config_map::Error, - rolegroup: RoleGroupName, - }, + #[snafu(display("failed to validate cluster"))] + ValidateCluster { source: validate::Error }, - #[snafu(display("failed to build DaemonSet for [{rolegroup}]"))] - BuildRoleGroupDaemonSet { - source: build::resource::daemonset::Error, - rolegroup: RoleGroupName, - }, + #[snafu(display("failed to build the Kubernetes resources"))] + BuildResources { source: build::Error }, - #[snafu(display("failed to apply ConfigMap for [{rolegroup}]"))] - ApplyRoleGroupConfig { + #[snafu(display("failed to apply Kubernetes resource"))] + ApplyResource { source: stackable_operator::cluster_resources::Error, - rolegroup: RoleGroupName, }, - #[snafu(display("failed to apply DaemonSet for [{rolegroup}]"))] - ApplyRoleGroupDaemonSet { - source: stackable_operator::cluster_resources::Error, - rolegroup: RoleGroupName, - }, - - #[snafu(display("failed to apply patch for DaemonSet for [{rolegroup}]"))] - ApplyPatchRoleGroupDaemonSet { + #[snafu(display("failed to apply legacy field-manager patch for DaemonSet {name}"))] + ApplyPatchDaemonSet { source: stackable_operator::client::Error, - rolegroup: RoleGroupName, + name: String, }, #[snafu(display("failed to patch service account"))] @@ -117,16 +86,6 @@ pub enum Error { source: stackable_operator::client::Error, }, - #[snafu(display("failed to build discovery ConfigMap"))] - BuildDiscoveryConfig { - source: build::resource::discovery::Error, - }, - - #[snafu(display("failed to apply discovery ConfigMap"))] - ApplyDiscoveryConfig { - source: stackable_operator::cluster_resources::Error, - }, - #[snafu(display("failed to delete orphaned resources"))] DeleteOrphans { source: stackable_operator::cluster_resources::Error, @@ -139,9 +98,6 @@ pub enum Error { #[snafu(display("failed to build label"))] BuildLabel { source: LabelError }, - - #[snafu(display("failed to validate cluster"))] - ValidateCluster { source: validate::Error }, } type Result = std::result::Result; @@ -167,8 +123,6 @@ pub async fn reconcile_opa( let validated_cluster = validate::validate(opa, &ctx.operator_environment).context(ValidateClusterSnafu)?; - let opa_role = OpaRole::Server; - let mut cluster_resources = cluster_resources_new( &product_name(), &operator_name(), @@ -180,18 +134,6 @@ pub async fn reconcile_opa( &opa.spec.object_overrides, ); - let empty_role_group_configs = BTreeMap::new(); - let role_group_configs = validated_cluster - .role_group_configs - .get(&opa_role) - .unwrap_or(&empty_role_group_configs); - - let server_role_service = build_server_role_service(&validated_cluster); - cluster_resources - .add(client, server_role_service) - .await - .context(ApplyRoleServiceSnafu)?; - let required_labels = cluster_resources .get_required_labels() .context(BuildLabelSnafu)?; @@ -199,7 +141,11 @@ pub async fn reconcile_opa( let (rbac_sa, rbac_rolebinding) = build_rbac_resources(opa, APP_NAME, required_labels).context(BuildRbacResourcesSnafu)?; - let rbac_sa = cluster_resources + // The ServiceAccount name is deterministic on the built object, so the build step does not + // depend on the applied ServiceAccount. + let service_account_name = rbac_sa.name_any(); + + cluster_resources .add(client, rbac_sa) .await .context(ApplyServiceAccountSnafu)?; @@ -208,58 +154,37 @@ pub async fn reconcile_opa( .await .context(ApplyRoleBindingSnafu)?; - let mut ds_cond_builder = DaemonSetConditionBuilder::default(); + let resources = build::build( + &validated_cluster, + &service_account_name, + &ctx.opa_bundle_builder_image, + &ctx.user_info_fetcher_image, + &ctx.cluster_info, + ) + .context(BuildResourcesSnafu)?; - for (rolegroup_name, rolegroup) in role_group_configs { - let rg_configmap = build::resource::config_map::build_rolegroup_config_map( - &validated_cluster, - rolegroup_name, - rolegroup, - ) - .with_context(|_| BuildRoleGroupConfigSnafu { - rolegroup: rolegroup_name.clone(), - })?; - let rg_service = build_rolegroup_headless_service(&validated_cluster, rolegroup_name); - let rg_metrics_service = - build_rolegroup_metrics_service(&validated_cluster, rolegroup_name); - let rg_daemonset = build::resource::daemonset::build_server_rolegroup_daemonset( - &validated_cluster, - rolegroup_name, - rolegroup, - &ctx.opa_bundle_builder_image, - &ctx.user_info_fetcher_image, - &rbac_sa.name_any(), - &ctx.cluster_info, - ) - .with_context(|_| BuildRoleGroupDaemonSetSnafu { - rolegroup: rolegroup_name.clone(), - })?; + let mut ds_cond_builder = DaemonSetConditionBuilder::default(); + // Apply order: DaemonSets last, so a changed mounted ConfigMap already exists before the Pods + // (that would otherwise restart) are updated (commons-operator#111). + for service in resources.services { cluster_resources - .add(client, rg_configmap) - .await - .with_context(|_| ApplyRoleGroupConfigSnafu { - rolegroup: rolegroup_name.clone(), - })?; - cluster_resources - .add(client, rg_service) + .add(client, service) .await - .with_context(|_| ApplyRoleGroupServiceSnafu { - rolegroup: rolegroup_name.clone(), - })?; + .context(ApplyResourceSnafu)?; + } + for config_map in resources.config_maps { cluster_resources - .add(client, rg_metrics_service) + .add(client, config_map) .await - .with_context(|_| ApplyRoleGroupServiceSnafu { - rolegroup: rolegroup_name.clone(), - })?; + .context(ApplyResourceSnafu)?; + } + for daemon_set in resources.daemon_sets { ds_cond_builder.add( cluster_resources - .add(client, rg_daemonset.clone()) + .add(client, daemon_set.clone()) .await - .with_context(|_| ApplyRoleGroupDaemonSetSnafu { - rolegroup: rolegroup_name.clone(), - })?, + .context(ApplyResourceSnafu)?, ); // Previous version of opa-operator used the field manager scope "opacluster" to write out a DaemonSet with the bundle-builder container called "opa-bundle-builder". @@ -271,31 +196,21 @@ pub async fn reconcile_opa( tracing::trace!( "Removing old field manager scope \"opacluster\" of DaemonSet {daemonset_name} to remove the \"opa-bundle-builder\" container. \ See https://github.com/stackabletech/opa-operator/issues/444 and https://github.com/stackabletech/issues/issues/390 for details.", - daemonset_name = rg_daemonset.name_any() + daemonset_name = daemon_set.name_any() ); client .apply_patch( "opacluster", - &rg_daemonset, + &daemon_set, // We can hardcode this here, as https://github.com/stackabletech/issues/issues/390 will solve the general problem and we always have created DaemonSets using the "apps/v1" version json!({"apiVersion": "apps/v1", "kind": "DaemonSet"}), ) .await - .context(ApplyPatchRoleGroupDaemonSetSnafu { - rolegroup: rolegroup_name.clone(), + .context(ApplyPatchDaemonSetSnafu { + name: daemon_set.name_any(), })?; } - let discovery_cm = build::resource::discovery::build_discovery_config_map( - &validated_cluster, - &client.kubernetes_cluster_info, - ) - .context(BuildDiscoveryConfigSnafu)?; - cluster_resources - .add(client, discovery_cm) - .await - .context(ApplyDiscoveryConfigSnafu)?; - let cluster_operation_cond_builder = ClusterOperationsConditionBuilder::new(&opa.spec.cluster_operation);