Skip to content

Commit 34f354c

Browse files
adwk67maltesander
andauthored
refactor: Build RBAC with recommended labels (#966)
* refactor: Source cluster domain from ValidatedCluster * refactor: Introduce build aggregator * factor: add infallible rbac functions * rename resource name functions * update lock file * changelog * added role/From impl for single nifi role plus parsing test * use the role parameter via its From impl in build_pdb * moved tests and made them more thorough * move object_meta to build step * make the nodes role required in the CRD * add validate happy-path test covering all derived values * improved comment * move swap-guard comment to the fixture that defines the cluster name --------- Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
1 parent 3624112 commit 34f354c

17 files changed

Lines changed: 623 additions & 206 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,13 @@ All notable changes to this project will be documented in this file.
99
- Internal operator refactoring: introduce a build() step in the reconciler that
1010
assembles all relevant Kubernetes resources before anything is applied ([#961]).
1111
- Bump stackable-operator to 0.114.0 ([#970])
12+
- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
13+
functions and carry the full set of recommended labels ([#966]).
14+
- BREAKING: The `nodes` role is now required by the CRD; a NifiCluster without it was
15+
previously accepted by the API server but failed reconciliation ([#966]).
1216

1317
[#961]: https://github.com/stackabletech/nifi-operator/pull/961
18+
[#966]: https://github.com/stackabletech/nifi-operator/pull/966
1419
[#970]: https://github.com/stackabletech/nifi-operator/pull/970
1520

1621
## [26.7.0] - 2026-07-21

extra/crds.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,6 @@ spec:
489489
at role level, the `roleConfig`.
490490
You can learn more about this in the
491491
[Roles and role group concept documentation](https://docs.stackable.tech/home/nightly/concepts/roles-and-role-groups).
492-
nullable: true
493492
properties:
494493
cliOverrides:
495494
additionalProperties:
@@ -2209,6 +2208,7 @@ spec:
22092208
required:
22102209
- clusterConfig
22112210
- image
2211+
- nodes
22122212
type: object
22132213
status:
22142214
nullable: true

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

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,14 @@
44
55
use std::str::FromStr;
66

7-
use snafu::{OptionExt, ResultExt, Snafu};
8-
use stackable_operator::v2::types::{common::Port, operator::RoleGroupName};
7+
use snafu::{ResultExt, Snafu};
8+
use stackable_operator::{
9+
builder::meta::ObjectMetaBuilder,
10+
v2::{
11+
builder::meta::ownerreference_from_resource,
12+
types::{common::Port, operator::RoleGroupName},
13+
},
14+
};
915

1016
use crate::{
1117
controller::{
@@ -14,6 +20,7 @@ use crate::{
1420
config_map::build_rolegroup_config_map,
1521
listener::{build_group_listener, group_listener_name},
1622
pdb::build_pdb,
23+
rbac::{build_role_binding, build_service_account},
1724
service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
1825
statefulset::build_node_rolegroup_statefulset,
1926
},
@@ -45,9 +52,6 @@ pub const NIFI_PYTHON_WORKING_DIRECTORY: &str = "/nifi-python-working-directory"
4552

4653
#[derive(Snafu, Debug)]
4754
pub enum Error {
48-
#[snafu(display("NifiCluster has no nodes role defined"))]
49-
NoNodesDefined,
50-
5155
#[snafu(display("failed to build ConfigMap for role group {role_group}"))]
5256
ConfigMap {
5357
source: resource::config_map::Error,
@@ -66,13 +70,7 @@ pub enum Error {
6670
/// Does not need a Kubernetes client: every reference to another Kubernetes resource is already
6771
/// dereferenced and validated by this point, so the errors returned here are resource-assembly
6872
/// failures only.
69-
///
70-
/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under
71-
/// (RBAC resources are built and applied separately, in the reconcile step).
72-
pub fn build(
73-
cluster: &ValidatedCluster,
74-
service_account_name: &str,
75-
) -> Result<KubernetesResources, Error> {
73+
pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources, Error> {
7674
let mut stateful_sets = vec![];
7775
let mut services = vec![];
7876
let mut listeners = vec![];
@@ -84,7 +82,7 @@ pub fn build(
8482
let node_role_group_configs = cluster
8583
.role_group_configs
8684
.get(&nifi_role)
87-
.context(NoNodesDefinedSnafu)?;
85+
.expect("the nodes role is required by the CRD and validate always inserts it");
8886

8987
// Role-level resources (one per role): the PodDisruptionBudget and the group Listener.
9088
let role_config = &cluster.role_config;
@@ -109,16 +107,10 @@ pub fn build(
109107

110108
let effective_replicas = rg.replicas.map(i32::from);
111109
stateful_sets.push(
112-
build_node_rolegroup_statefulset(
113-
cluster,
114-
role_group_name,
115-
rg,
116-
effective_replicas,
117-
service_account_name,
118-
)
119-
.context(StatefulSetSnafu {
120-
role_group: role_group_name.clone(),
121-
})?,
110+
build_node_rolegroup_statefulset(cluster, role_group_name, rg, effective_replicas)
111+
.context(StatefulSetSnafu {
112+
role_group: role_group_name.clone(),
113+
})?,
122114
);
123115
}
124116

@@ -128,9 +120,33 @@ pub fn build(
128120
listeners,
129121
config_maps,
130122
pod_disruption_budgets,
123+
service_accounts: vec![build_service_account(cluster)],
124+
role_bindings: vec![build_role_binding(cluster)],
131125
})
132126
}
133127

128+
/// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to
129+
/// the cluster, and the recommended labels for a resource named `name` in `role_group_name`.
130+
///
131+
/// Consolidates the metadata chain repeated by the child-resource builders. Call sites that
132+
/// need extra labels/annotations chain them onto the returned builder. Role-level resources
133+
/// (e.g. the per-role [`Listener`](stackable_operator::crd::listener::v1alpha1::Listener)) pass
134+
/// the placeholder role-group `none`, preserving the historical
135+
/// `app.kubernetes.io/role-group: none` label.
136+
pub(crate) fn object_meta(
137+
cluster: &ValidatedCluster,
138+
name: impl Into<String>,
139+
role_group_name: &RoleGroupName,
140+
) -> ObjectMetaBuilder {
141+
let mut builder = ObjectMetaBuilder::new();
142+
builder
143+
.name_and_namespace(cluster)
144+
.name(name)
145+
.ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
146+
.with_labels(cluster.recommended_labels(role_group_name));
147+
builder
148+
}
149+
134150
#[cfg(test)]
135151
mod tests {
136152
use stackable_operator::kube::Resource;
@@ -149,7 +165,7 @@ mod tests {
149165
#[test]
150166
fn build_produces_expected_resources() {
151167
let cluster = minimal_validated_cluster();
152-
let resources = build(&cluster, "simple-nifi-serviceaccount").expect("build succeeds");
168+
let resources = build(&cluster).expect("build succeeds");
153169

154170
// The minimal fixture has a single `default` role group for the `node` role.
155171
assert_eq!(
@@ -168,5 +184,14 @@ mod tests {
168184
sorted_names(&resources.pod_disruption_budgets),
169185
["simple-nifi-node"]
170186
);
187+
// The cluster-shared RBAC pair.
188+
assert_eq!(
189+
sorted_names(&resources.service_accounts),
190+
["simple-nifi-serviceaccount"]
191+
);
192+
assert_eq!(
193+
sorted_names(&resources.role_bindings),
194+
["simple-nifi-rolebinding"]
195+
);
171196
}
172197
}

rust/operator-binary/src/controller/build/properties.rs

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,23 @@ pub(crate) mod test_support {
104104
},
105105
};
106106

107+
/// The expected `app.kubernetes.io/version` label value for the given product version.
108+
///
109+
/// The `-stackable` suffix carries the operator's own version, which is `0.0.0-dev` on main
110+
/// but rewritten by the release process — so tests must derive it rather than hardcode it,
111+
/// or they fail on release branches.
112+
pub fn app_version_label(product_version: &str) -> String {
113+
format!(
114+
"{product_version}-stackable{}",
115+
crate::built_info::PKG_VERSION
116+
)
117+
}
118+
107119
/// A minimal NiFi cluster YAML. Mirrors the fixture used by bootstrap_conf tests,
108120
/// stripped down to the mandatory fields only (Kubernetes clustering backend, SingleUser auth).
121+
///
122+
/// The cluster name (`simple-nifi`) deliberately differs from the product name (`nifi`), so
123+
/// tests asserting recommended labels catch swapped `name`/`instance` values.
109124
pub const MINIMAL_NIFI_YAML: &str = r#"
110125
apiVersion: nifi.stackable.tech/v1alpha1
111126
kind: NifiCluster
@@ -140,24 +155,24 @@ pub(crate) mod test_support {
140155
let nifi: v1alpha1::NifiCluster =
141156
serde_yaml::from_str(MINIMAL_NIFI_YAML).expect("invalid test YAML");
142157

158+
// Mirrors what `image.resolve()` produces in production, so label-asserting tests see
159+
// realistic values.
143160
let image = ResolvedProductImage {
144161
product_version: "2.9.0".to_string(),
145-
app_version_label_value: "2.9.0".parse::<LabelValue>().unwrap(),
146-
image: "oci.stackable.tech/sdp/nifi:2.9.0-stackable0.0.0-dev".to_string(),
162+
app_version_label_value: app_version_label("2.9.0").parse::<LabelValue>().unwrap(),
163+
image: format!("oci.stackable.tech/sdp/nifi:{}", app_version_label("2.9.0")),
147164
image_pull_policy: "IfNotPresent".to_string(),
148165
pull_secrets: None,
149166
};
150167

151168
let role_group_configs = build_role_group_configs(&nifi, &image, &None)
152169
.expect("role group configs should merge for minimal fixture");
153170

154-
let role_config = nifi
155-
.role_config(&NifiRole::Node)
156-
.map(|role_config| ValidatedRoleConfig {
157-
pdb: role_config.common.pod_disruption_budget.clone(),
158-
listener_class: role_config.listener_class.clone(),
159-
})
160-
.expect("the minimal fixture defines the nodes role");
171+
let node_role_config = nifi.role_config(&NifiRole::Node);
172+
let role_config = ValidatedRoleConfig {
173+
pdb: node_role_config.common.pod_disruption_budget.clone(),
174+
listener_class: node_role_config.listener_class.clone(),
175+
};
161176

162177
let name = ClusterName::from_str("simple-nifi").expect("valid cluster name");
163178
let namespace = NamespaceName::from_str("default").expect("valid namespace");

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use stackable_operator::{
99
use crate::controller::{
1010
NifiRoleGroupConfig, ValidatedCluster,
1111
build::{
12+
object_meta,
1213
properties::{
1314
ConfigFileName, authorizers, bootstrap_conf, login_identity_providers, nifi_properties,
1415
product_logging, security_properties, state_management_xml,
@@ -66,15 +67,15 @@ pub fn build_rolegroup_config_map(
6667

6768
cm_builder
6869
.metadata(
69-
cluster
70-
.object_meta(
71-
cluster
72-
.resource_names(role_group_name)
73-
.role_group_config_map()
74-
.to_string(),
75-
role_group_name,
76-
)
77-
.build(),
70+
object_meta(
71+
cluster,
72+
cluster
73+
.role_group_resource_names(role_group_name)
74+
.role_group_config_map()
75+
.to_string(),
76+
role_group_name,
77+
)
78+
.build(),
7879
)
7980
.add_data(
8081
ConfigFileName::BootstrapConf.to_string(),

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use stackable_operator::{
1414

1515
use crate::controller::{
1616
ValidatedCluster,
17-
build::{HTTPS_PORT, HTTPS_PORT_NAME, PLACEHOLDER_LISTENER_ROLE_GROUP},
17+
build::{HTTPS_PORT, HTTPS_PORT_NAME, PLACEHOLDER_LISTENER_ROLE_GROUP, object_meta},
1818
};
1919

2020
pub const LISTENER_VOLUME_NAME: &str = "listener";
@@ -29,12 +29,12 @@ pub fn build_group_listener(
2929
listener_group_name: ListenerName,
3030
) -> Listener {
3131
Listener {
32-
metadata: cluster
33-
.object_meta(
34-
listener_group_name.to_string(),
35-
&PLACEHOLDER_LISTENER_ROLE_GROUP,
36-
)
37-
.build(),
32+
metadata: object_meta(
33+
cluster,
34+
listener_group_name.to_string(),
35+
&PLACEHOLDER_LISTENER_ROLE_GROUP,
36+
)
37+
.build(),
3838
spec: ListenerSpec {
3939
class_name: Some(listener_class.to_string()),
4040
ports: Some(vec![ListenerPort {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@
55
pub mod config_map;
66
pub mod listener;
77
pub mod pdb;
8+
pub mod rbac;
89
pub mod service;
910
pub mod statefulset;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub fn build_pdb(
2323
let pdb = pod_disruption_budget_builder_with_role(
2424
cluster,
2525
&product_name(),
26-
&ValidatedCluster::role_name(),
26+
&role.into(),
2727
&operator_name(),
2828
&controller_name(),
2929
)

0 commit comments

Comments
 (0)