Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
- Internal operator refactoring: introduce a build() step in the reconciler that
assembles all relevant Kubernetes resources before anything is applied ([#756]).
- Bump stackable-operator to 0.114.0 ([#765]).
- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
functions and carry the full set of recommended labels ([#761]).

[#756]: https://github.com/stackabletech/superset-operator/pull/756
[#761]: https://github.com/stackabletech/superset-operator/pull/761
[#765]: https://github.com/stackabletech/superset-operator/pull/765

## [26.7.0] - 2026-07-21
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ tokio = { version = "1.52", features = ["full"] }
tracing = "0.1"

[patch."https://github.com/stackabletech/operator-rs.git"]
# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" }
# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "feat/smooth-operator/build-rbac" }
# stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
82 changes: 33 additions & 49 deletions rust/operator-binary/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ use stackable_operator::{
affinity::StackableAffinity,
product_image_selection::ResolvedProductImage,
random_secret_creation::{self, create_random_secret_if_not_exists},
rbac::build_rbac_resources,
resources::{NoRuntimeLimits, Resources},
},
crd::listener,
k8s_openapi::api::{
apps::v1::{Deployment, StatefulSet},
core::v1::{ConfigMap, Secret, Service},
core::v1::{ConfigMap, Secret, Service, ServiceAccount},
policy::v1::PodDisruptionBudget,
rbac::v1::RoleBinding,
},
kube::{
Resource, ResourceExt,
Resource,
api::ObjectMeta,
core::{DeserializeGuard, error_boundary},
runtime::controller::Action,
Expand All @@ -44,7 +44,7 @@ use stackable_operator::{
kvp::label::{recommended_labels, role_group_selector},
product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig},
role_group_utils::ResourceNames,
role_utils::{GenericCommonConfig, RoleGroupConfig},
role_utils::{self, GenericCommonConfig, RoleGroupConfig},
types::{
kubernetes::{ListenerClassName, ListenerName, NamespaceName, Uid},
operator::{
Expand Down Expand Up @@ -94,6 +94,8 @@ pub struct KubernetesResources {
pub listeners: Vec<listener::v1alpha1::Listener>,
pub config_maps: Vec<ConfigMap>,
pub pod_disruption_budgets: Vec<PodDisruptionBudget>,
pub service_accounts: Vec<ServiceAccount>,
pub role_bindings: Vec<RoleBinding>,
}

/// Per-role configuration extracted during validation.
Expand Down Expand Up @@ -226,24 +228,33 @@ impl ValidatedCluster {
}
}

pub fn resource_names(
pub fn role_group_resource_names(
&self,
role: &SupersetRole,
role_group_name: &RoleGroupName,
) -> ResourceNames {
ResourceNames {
cluster_name: self.name.clone(),
role_name: role.role_name(),
role_name: role.into(),
role_group_name: role_group_name.clone(),
}
}

/// Type-safe names for the per-cluster RBAC resources: the ServiceAccount shared by all
/// Pods, its (namespaced) RoleBinding, and the operator-deployed ClusterRole it binds.
pub fn cluster_resource_names(&self) -> role_utils::ResourceNames {
role_utils::ResourceNames {
cluster_name: self.name.clone(),
product_name: product_name(),
}
}

pub fn recommended_labels(
&self,
role: &SupersetRole,
role_group_name: &RoleGroupName,
) -> Labels {
self.recommended_labels_for(&role.role_name(), role_group_name)
self.recommended_labels_for(&role.into(), role_group_name)
}

pub fn recommended_labels_for(
Expand All @@ -263,7 +274,7 @@ impl ValidatedCluster {
) -> Labels {
self.recommended_labels_with(
&build::UNVERSIONED_PRODUCT_VERSION,
&role.role_name(),
&role.into(),
role_group_name,
)
}
Expand All @@ -290,7 +301,7 @@ impl ValidatedCluster {
role: &SupersetRole,
role_group_name: &RoleGroupName,
) -> Labels {
role_group_selector(self, &product_name(), &role.role_name(), role_group_name)
role_group_selector(self, &product_name(), &role.into(), role_group_name)
}

/// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to
Expand Down Expand Up @@ -409,27 +420,6 @@ pub enum Error {
source: stackable_operator::client::Error,
},

#[snafu(display("failed to patch service account"))]
ApplyServiceAccount {
source: stackable_operator::cluster_resources::Error,
},

#[snafu(display("failed to patch role binding"))]
ApplyRoleBinding {
source: stackable_operator::cluster_resources::Error,
},

#[snafu(display("failed to build RBAC objects"))]
BuildRBACObjects {
source: stackable_operator::commons::rbac::Error,
},

#[snafu(display("failed to get required Labels"))]
GetRequiredLabels {
source:
stackable_operator::kvp::KeyValuePairError<stackable_operator::kvp::LabelValueError>,
},

#[snafu(display("SupersetCluster object is invalid"))]
InvalidSupersetCluster {
source: error_boundary::InvalidObject,
Expand Down Expand Up @@ -504,24 +494,6 @@ pub async fn reconcile_superset(
&superset.spec.object_overrides,
);

let (rbac_sa, rbac_rolebinding) = build_rbac_resources(
superset,
APP_NAME,
cluster_resources
.get_required_labels()
.context(GetRequiredLabelsSnafu)?,
)
.context(BuildRBACObjectsSnafu)?;

let rbac_sa = cluster_resources
.add(client, rbac_sa)
.await
.context(ApplyServiceAccountSnafu)?;
cluster_resources
.add(client, rbac_rolebinding)
.await
.context(ApplyRoleBindingSnafu)?;

// TODO: Can be removed after SDP 26.7 is released (it's only a migration from 26.3 - 26.7)
// (don't forget about the snafu Error variants).
// Removal is tracked in https://github.com/stackabletech/superset-operator/issues/755
Expand All @@ -536,14 +508,26 @@ pub async fn reconcile_superset(
.await
.context(CreateSecretKeySecretSnafu)?;

let resources = build::build(&validated, &rbac_sa.name_any()).context(BuildResourcesSnafu)?;
let resources = build::build(&validated).context(BuildResourcesSnafu)?;

let mut statefulset_cond_builder = StatefulSetConditionBuilder::default();
let mut deployment_cond_builder = DeploymentConditionBuilder::default();

// The StatefulSets/Deployments are applied last, so every ConfigMap and Secret they mount
// already exists — otherwise a changed mount would restart the Pods.
// See https://github.com/stackabletech/commons-operator/issues/111 for details.
for service_account in resources.service_accounts {
cluster_resources
.add(client, service_account)
.await
.context(ApplyResourceSnafu)?;
}
for role_binding in resources.role_bindings {
cluster_resources
.add(client, role_binding)
.await
.context(ApplyResourceSnafu)?;
}
for service in resources.services {
cluster_resources
.add(client, service)
Expand Down
68 changes: 57 additions & 11 deletions rust/operator-binary/src/controller/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::{
deployment::build_rolegroup_deployment,
listener::build_group_listener,
pdb::build_pdb,
rbac::{build_role_binding, build_service_account},
service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
statefulset::build_node_rolegroup_statefulset,
},
Expand All @@ -26,7 +27,7 @@ pub mod resource;

// Placeholder role-group name used for the recommended labels of the role-level `Listener`
// (which is not tied to a single role group).
stackable_operator::constant!(pub(crate) PLACEHOLDER_LISTENER_ROLE_GROUP: RoleGroupName = "none");
stackable_operator::constant!(pub(crate) NONE_ROLE_GROUP_NAME: RoleGroupName = "none");

// Product version used for the recommended labels of PVC templates, which cannot be modified after
// deployment. A constant `none` keeps those labels stable across version upgrades.
Expand Down Expand Up @@ -54,13 +55,7 @@ pub enum Error {
}

/// Builds every Kubernetes resource for the given validated cluster.
///
/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under.
/// The RBAC resources themselves are built and applied separately in the reconcile step.
pub fn build(
cluster: &ValidatedCluster,
service_account_name: &str,
) -> Result<KubernetesResources, Error> {
pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources, Error> {
let mut stateful_sets = vec![];
let mut deployments = vec![];
let mut services = vec![];
Expand Down Expand Up @@ -110,7 +105,6 @@ pub fn build(
superset_role,
role_group_name,
rolegroup_config,
service_account_name,
)
.context(StatefulSetSnafu {
role_group: role_group_name.clone(),
Expand All @@ -124,7 +118,6 @@ pub fn build(
superset_role,
role_group_name,
rolegroup_config,
service_account_name,
)
.context(DeploymentSnafu {
role_group: role_group_name.clone(),
Expand Down Expand Up @@ -162,11 +155,15 @@ pub fn build(
listeners,
config_maps,
pod_disruption_budgets,
service_accounts: vec![build_service_account(cluster)],
role_bindings: vec![build_role_binding(cluster)],
})
}

#[cfg(test)]
mod tests {
use std::collections::BTreeMap;

use stackable_operator::{kube::Resource, utils::yaml_from_str_singleton_map};

use super::build;
Expand Down Expand Up @@ -230,7 +227,7 @@ mod tests {
#[test]
fn build_produces_expected_resource_names() {
let cluster = validated_cluster();
let resources = build(&cluster, "simple-superset-serviceaccount").expect("build succeeds");
let resources = build(&cluster).expect("build succeeds");

assert_eq!(
sorted_names(&resources.stateful_sets),
Expand Down Expand Up @@ -263,4 +260,53 @@ mod tests {
]
);
}

/// Locks the RBAC resource names, the roleRef, and the recommended label set against
/// accidental drift. The fixture's cluster name deliberately differs from the product name so
/// that swapped `name`/`instance` label values cannot pass unnoticed.
#[test]
fn build_produces_rbac() {
let cluster = validated_cluster();
let resources = build(&cluster).expect("build succeeds");

assert_eq!(
sorted_names(&resources.service_accounts),
["simple-superset-serviceaccount"]
);
assert_eq!(
sorted_names(&resources.role_bindings),
["simple-superset-rolebinding"]
);

let expected_labels = BTreeMap::from(
[
("app.kubernetes.io/component", "none"),
("app.kubernetes.io/instance", "simple-superset"),
(
"app.kubernetes.io/managed-by",
"superset.stackable.tech_supersetcluster",
),
("app.kubernetes.io/name", "superset"),
("app.kubernetes.io/role-group", "none"),
("app.kubernetes.io/version", "4.1.4-stackable0.0.0-dev"),
("stackable.tech/vendor", "Stackable"),
]
.map(|(key, value)| (key.to_string(), value.to_string())),
);
let service_account = resources
.service_accounts
.first()
.expect("a ServiceAccount is built");
assert_eq!(
service_account.metadata.labels,
Some(expected_labels.clone())
);

let role_binding = resources
.role_bindings
.first()
.expect("a RoleBinding is built");
assert_eq!(role_binding.metadata.labels, Some(expected_labels));
assert_eq!(role_binding.role_ref.name, "superset-clusterrole");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn build_rolegroup_config_map(
validated
.object_meta(
validated
.resource_names(role, role_group_name)
.role_group_resource_names(role, role_group_name)
.role_group_config_map()
.to_string(),
role,
Expand Down
10 changes: 7 additions & 3 deletions rust/operator-binary/src/controller/build/resource/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,10 @@ pub fn build_rolegroup_deployment(
superset_role: &SupersetRole,
role_group_name: &RoleGroupName,
rolegroup_config: &SupersetRoleGroupConfig,
sa_name: &str,
) -> Result<Deployment> {
let merged_config = &rolegroup_config.config;

let resource_names = validated.resource_names(superset_role, role_group_name);
let resource_names = validated.role_group_resource_names(superset_role, role_group_name);
let recommended_object_labels = validated.recommended_labels(superset_role, role_group_name);

// The Celery process command, liveness probe and replica policy are the only differences
Expand Down Expand Up @@ -102,7 +101,12 @@ pub fn build_rolegroup_deployment(
.build(),
)
.affinity(&merged_config.affinity)
.service_account_name(sa_name);
.service_account_name(
validated
.cluster_resource_names()
.service_account_name()
.to_string(),
);

let mut superset_cb = super::build_superset_container_builder(validated, rolegroup_config)
.context(BuildContainerSnafu)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use stackable_operator::{crd::listener, v2::types::kubernetes::ListenerClassName};

use crate::{
controller::{ValidatedCluster, build::PLACEHOLDER_LISTENER_ROLE_GROUP},
controller::{ValidatedCluster, build::NONE_ROLE_GROUP_NAME},
crd::{APP_PORT, APP_PORT_NAME, SupersetRole},
};

Expand All @@ -16,7 +16,7 @@ pub fn build_group_listener(
// The group listener is a role-level object, so the constant `none` placeholder role-group is
// used for the recommended labels.
let metadata = validated
.object_meta(listener_group_name, role, &PLACEHOLDER_LISTENER_ROLE_GROUP)
.object_meta(listener_group_name, role, &NONE_ROLE_GROUP_NAME)
.build();

let spec = listener::v1alpha1::ListenerSpec {
Expand Down
3 changes: 2 additions & 1 deletion rust/operator-binary/src/controller/build/resource/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub mod config_map;
pub mod deployment;
pub mod listener;
pub mod pdb;
pub mod rbac;
pub mod service;
pub mod statefulset;

Expand Down Expand Up @@ -252,7 +253,7 @@ pub(crate) fn build_vector_container(
&Container::Vector.to_container_name(),
&validated.image,
vector_log_config,
&validated.resource_names(superset_role, role_group_name),
&validated.role_group_resource_names(superset_role, role_group_name),
&CONFIG_VOLUME_NAME,
&LOG_VOLUME_NAME,
EnvVarSet::new(),
Expand Down
Loading
Loading