Skip to content

Commit a22eb38

Browse files
refactor: Build RBAC with recommended labels (#821)
* add rbac labels, use infallible functions * reference PR branch * changelog * added test to verify parsing of all roles * rename constant * renamed resource names functions for clarity * bump to op-rs 0.114 * linting * Update rust/operator-binary/src/controller/mod.rs Co-authored-by: Siegfried Weber <mail@siegfriedweber.net> --------- Co-authored-by: Siegfried Weber <mail@siegfriedweber.net>
1 parent ba154b3 commit a22eb38

13 files changed

Lines changed: 232 additions & 102 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66

77
- Internal operator refactoring: introduce a build() step in the reconciler that
88
assembles all relevant Kubernetes resources before anything is applied ([#814]).
9+
- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
10+
functions and carry the full set of recommended labels ([#821]).
911
- Bump stackable-operator to 0.114.0 ([#827]).
1012

1113
[#814]: https://github.com/stackabletech/airflow-operator/pull/814
14+
[#821]: https://github.com/stackabletech/airflow-operator/pull/821
1215
[#827]: https://github.com/stackabletech/airflow-operator/pull/827
1316

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

rust/operator-binary/src/airflow_controller.rs

Lines changed: 17 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,12 @@ use snafu::{ResultExt, Snafu};
99
use stackable_operator::{
1010
cli::OperatorEnvironmentOptions,
1111
cluster_resources::ClusterResourceApplyStrategy,
12-
commons::{random_secret_creation, rbac::build_rbac_resources},
12+
commons::random_secret_creation,
1313
k8s_openapi::api::core::v1::EnvVar,
1414
kube::{
15-
ResourceExt,
1615
core::{DeserializeGuard, error_boundary},
1716
runtime::controller::Action,
1817
},
19-
kvp::LabelError,
2018
logging::controller::ReconcilerError,
2119
shared::time::Duration,
2220
status::condition::{
@@ -30,7 +28,7 @@ use strum::{EnumDiscriminants, IntoStaticStr};
3028
use crate::{
3129
controller::{ValidatedCluster, build, controller_name, operator_name, product_name},
3230
crd::{
33-
APP_NAME, AirflowClusterStatus, OPERATOR_NAME,
31+
AirflowClusterStatus, OPERATOR_NAME,
3432
internal_secret::{
3533
FERNET_KEY_SECRET_KEY, INTERNAL_SECRET_SECRET_KEY, JWT_SECRET_SECRET_KEY,
3634
},
@@ -58,21 +56,6 @@ pub enum Error {
5856
source: stackable_operator::cluster_resources::Error,
5957
},
6058

61-
#[snafu(display("failed to patch service account"))]
62-
ApplyServiceAccount {
63-
source: stackable_operator::cluster_resources::Error,
64-
},
65-
66-
#[snafu(display("failed to patch role binding: {source}"))]
67-
ApplyRoleBinding {
68-
source: stackable_operator::cluster_resources::Error,
69-
},
70-
71-
#[snafu(display("failed to build RBAC objects"))]
72-
BuildRBACObjects {
73-
source: stackable_operator::commons::rbac::Error,
74-
},
75-
7659
#[snafu(display("failed to build the Kubernetes resources"))]
7760
BuildResources { source: build::Error },
7861

@@ -101,9 +84,6 @@ pub enum Error {
10184
source: crate::controller::validate::Error,
10285
},
10386

104-
#[snafu(display("failed to build label"))]
105-
BuildLabel { source: LabelError },
106-
10787
#[snafu(display("AirflowCluster object is invalid"))]
10888
InvalidAirflowCluster {
10989
source: error_boundary::InvalidObject,
@@ -159,33 +139,25 @@ pub async fn reconcile_airflow(
159139
&airflow.spec.object_overrides,
160140
);
161141

162-
let required_labels = cluster_resources
163-
.get_required_labels()
164-
.context(BuildLabelSnafu)?;
165-
166-
let (rbac_sa, rbac_rolebinding) =
167-
build_rbac_resources(airflow, APP_NAME, required_labels).context(BuildRBACObjectsSnafu)?;
168-
169-
// The ServiceAccount name is deterministic on the built object, so the build step does not
170-
// depend on the applied ServiceAccount.
171-
let service_account_name = rbac_sa.name_any();
172-
173-
cluster_resources
174-
.add(client, rbac_sa)
175-
.await
176-
.context(ApplyServiceAccountSnafu)?;
177-
cluster_resources
178-
.add(client, rbac_rolebinding)
179-
.await
180-
.context(ApplyRoleBindingSnafu)?;
181-
182-
let resources =
183-
build::build(&validated_cluster, &service_account_name).context(BuildResourcesSnafu)?;
142+
let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?;
184143

185144
let mut ss_cond_builder = StatefulSetConditionBuilder::default();
186145

187146
// Apply order is: StatefulSets last (a changed mounted ConfigMap/Secret
188-
// must exist first, else Pods restart -- commons-operator#111).
147+
// must exist first, else Pods restart -- commons-operator#111). The ServiceAccount comes
148+
// first because the Pods reference it at creation time.
149+
for service_account in resources.service_accounts {
150+
cluster_resources
151+
.add(client, service_account)
152+
.await
153+
.context(ApplyResourceSnafu)?;
154+
}
155+
for role_binding in resources.role_bindings {
156+
cluster_resources
157+
.add(client, role_binding)
158+
.await
159+
.context(ApplyResourceSnafu)?;
160+
}
189161
for service in resources.services {
190162
cluster_resources
191163
.add(client, service)

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

Lines changed: 77 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::{
1111
executor::build_executor_template_config_map,
1212
listener::build_group_listener,
1313
pdb::build_pdb,
14+
rbac::{build_role_binding, build_service_account},
1415
service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
1516
statefulset::build_server_rolegroup_statefulset,
1617
},
@@ -47,14 +48,7 @@ pub enum Error {
4748
/// Does not need a Kubernetes client: every reference to another Kubernetes resource is already
4849
/// dereferenced and validated by this point. Cluster configuration is likewise already validated,
4950
/// so the errors returned here are resource-assembly failures only.
50-
///
51-
/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods and the
52-
/// Kubernetes-executor pod template run under (RBAC resources are built and applied separately,
53-
/// in the reconcile step).
54-
pub fn build(
55-
cluster: &ValidatedCluster,
56-
service_account_name: &str,
57-
) -> Result<KubernetesResources, Error> {
51+
pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources, Error> {
5852
let mut stateful_sets = vec![];
5953
let mut services = vec![];
6054
let mut listeners = vec![];
@@ -81,7 +75,6 @@ pub fn build(
8175

8276
let executor_template_config_map = build_executor_template_config_map(
8377
cluster,
84-
service_account_name,
8578
&executor_template.config,
8679
&executor_template.env_overrides,
8780
&executor_template.pod_overrides,
@@ -123,7 +116,7 @@ pub fn build(
123116
config_maps.push(
124117
config_map::build_rolegroup_config_map(
125118
cluster,
126-
&role.role_name(),
119+
&ValidatedCluster::role_name(role),
127120
role_group_name,
128121
&rg_config.config_overrides,
129122
logging,
@@ -140,7 +133,6 @@ pub fn build(
140133
role_group_name,
141134
rg_config,
142135
logging,
143-
service_account_name,
144136
)
145137
.context(StatefulSetSnafu {
146138
role_group: role_group_name.clone(),
@@ -155,11 +147,15 @@ pub fn build(
155147
listeners,
156148
config_maps,
157149
pod_disruption_budgets,
150+
service_accounts: vec![build_service_account(cluster)],
151+
role_bindings: vec![build_role_binding(cluster)],
158152
})
159153
}
160154

161155
#[cfg(test)]
162156
mod tests {
157+
use std::collections::BTreeMap;
158+
163159
use stackable_operator::kube::Resource;
164160

165161
use super::build;
@@ -184,7 +180,7 @@ mod tests {
184180
apiVersion: airflow.stackable.tech/v1alpha2
185181
kind: AirflowCluster
186182
metadata:
187-
name: airflow
183+
name: my-airflow
188184
namespace: default
189185
uid: e6ac237d-a6d4-43a1-8135-f36506110912
190186
spec:
@@ -263,25 +259,84 @@ mod tests {
263259
#[test]
264260
fn build_produces_expected_resource_names() {
265261
let cluster = celery_executor_cluster();
266-
let resources = build(&cluster, "airflow-serviceaccount").expect("build succeeds");
262+
let resources = build(&cluster).expect("build succeeds");
267263

268264
assert_eq!(
269265
sorted_names(&resources.stateful_sets),
270-
["airflow-scheduler-default", "airflow-webserver-default"]
266+
[
267+
"my-airflow-scheduler-default",
268+
"my-airflow-webserver-default"
269+
]
271270
);
272271
// One headless and one metrics Service per role group.
273272
assert_eq!(resources.services.len(), 4);
274273
assert_eq!(
275274
sorted_names(&resources.config_maps),
276-
["airflow-scheduler-default", "airflow-webserver-default"]
275+
[
276+
"my-airflow-scheduler-default",
277+
"my-airflow-webserver-default"
278+
]
277279
);
278280
// The webserver is the only role with a group Listener.
279-
assert_eq!(sorted_names(&resources.listeners), ["airflow-webserver"]);
281+
assert_eq!(sorted_names(&resources.listeners), ["my-airflow-webserver"]);
280282
// A default PDB per role (the Celery worker included).
281283
assert_eq!(
282284
sorted_names(&resources.pod_disruption_budgets),
283-
["airflow-scheduler", "airflow-webserver", "airflow-worker"]
285+
[
286+
"my-airflow-scheduler",
287+
"my-airflow-webserver",
288+
"my-airflow-worker"
289+
]
290+
);
291+
}
292+
293+
/// Locks the RBAC resource names, the roleRef, and the recommended label set against
294+
/// accidental drift. The fixture's cluster name deliberately differs from the product name so
295+
/// that swapped `name`/`instance` label values cannot pass unnoticed.
296+
#[test]
297+
fn build_produces_rbac() {
298+
let cluster = celery_executor_cluster();
299+
let resources = build(&cluster).expect("build succeeds");
300+
301+
assert_eq!(
302+
sorted_names(&resources.service_accounts),
303+
["my-airflow-serviceaccount"]
304+
);
305+
assert_eq!(
306+
sorted_names(&resources.role_bindings),
307+
["my-airflow-rolebinding"]
308+
);
309+
310+
let expected_labels = BTreeMap::from(
311+
[
312+
("app.kubernetes.io/component", "none"),
313+
("app.kubernetes.io/instance", "my-airflow"),
314+
(
315+
"app.kubernetes.io/managed-by",
316+
"airflow.stackable.tech_airflowcluster",
317+
),
318+
("app.kubernetes.io/name", "airflow"),
319+
("app.kubernetes.io/role-group", "none"),
320+
("app.kubernetes.io/version", "3.1.6-stackable0.0.0-dev"),
321+
("stackable.tech/vendor", "Stackable"),
322+
]
323+
.map(|(key, value)| (key.to_string(), value.to_string())),
284324
);
325+
let service_account = resources
326+
.service_accounts
327+
.first()
328+
.expect("a ServiceAccount is built");
329+
assert_eq!(
330+
service_account.metadata.labels,
331+
Some(expected_labels.clone())
332+
);
333+
334+
let role_binding = resources
335+
.role_bindings
336+
.first()
337+
.expect("a RoleBinding is built");
338+
assert_eq!(role_binding.metadata.labels, Some(expected_labels));
339+
assert_eq!(role_binding.role_ref.name, "airflow-clusterrole");
285340
}
286341

287342
/// The Kubernetes-executor branch of `build()` (moved here from `reconcile`) additionally emits
@@ -290,15 +345,15 @@ mod tests {
290345
#[test]
291346
fn build_kubernetes_executor_adds_pod_template_config_maps() {
292347
let cluster = kubernetes_executor_cluster();
293-
let resources = build(&cluster, "airflow-serviceaccount").expect("build succeeds");
348+
let resources = build(&cluster).expect("build succeeds");
294349

295350
assert_eq!(
296351
sorted_names(&resources.config_maps),
297352
[
298-
"airflow-executor-kubernetes",
299-
"airflow-executor-pod-template",
300-
"airflow-scheduler-default",
301-
"airflow-webserver-default",
353+
"my-airflow-executor-kubernetes",
354+
"my-airflow-executor-pod-template",
355+
"my-airflow-scheduler-default",
356+
"my-airflow-webserver-default",
302357
]
303358
);
304359
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ pub fn build_rolegroup_config_map(
6666
validated_cluster
6767
.object_meta(
6868
validated_cluster
69-
.resource_names(role_name, role_group_name)
69+
.role_group_resource_names(role_name, role_group_name)
7070
.role_group_config_map()
7171
.to_string(),
7272
validated_cluster.recommended_labels_for(role_name, role_group_name),

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ type Result<T, E = Error> = std::result::Result<T, E>;
7676

7777
pub fn build_executor_template_config_map(
7878
cluster: &ValidatedCluster,
79-
sa_name: &str,
8079
executor_config: &ValidatedAirflowConfig,
8180
env_overrides: &HashMap<String, String>,
8281
pod_overrides: &PodTemplateSpec,
@@ -99,7 +98,12 @@ pub fn build_executor_template_config_map(
9998
pb.metadata(pb_metadata)
10099
.image_pull_secrets_from_product_image(resolved_product_image)
101100
.affinity(&executor_config.affinity)
102-
.service_account_name(sa_name)
101+
.service_account_name(
102+
cluster
103+
.cluster_resource_names()
104+
.service_account_name()
105+
.to_string(),
106+
)
103107
.restart_policy("Never")
104108
.security_context(PodSecurityContextBuilder::new().fs_group(1000).build());
105109

@@ -152,7 +156,7 @@ pub fn build_executor_template_config_map(
152156
.context(AddVolumeSnafu)?;
153157
pb.add_volumes(volumes::create_volumes(
154158
cluster
155-
.resource_names(&executor_role_name(), &executor_role_group_name())
159+
.role_group_resource_names(&executor_role_name(), &executor_role_group_name())
156160
.role_group_config_map()
157161
.as_ref(),
158162
&executor_config.logging.product_container,
@@ -163,7 +167,10 @@ pub fn build_executor_template_config_map(
163167
pb.add_container(build_logging_container(
164168
resolved_product_image,
165169
vector_log_config,
166-
&cluster.resource_names(&executor_role_name(), &executor_template_role_group_name()),
170+
&cluster.role_group_resource_names(
171+
&executor_role_name(),
172+
&executor_template_role_group_name(),
173+
),
167174
));
168175
}
169176

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
1+
use std::str::FromStr;
2+
13
use stackable_operator::{
24
crd::listener,
3-
v2::types::kubernetes::{ListenerClassName, ListenerName},
5+
v2::types::{
6+
kubernetes::{ListenerClassName, ListenerName},
7+
operator::RoleGroupName,
8+
},
49
};
510

611
use crate::{
712
controller::ValidatedCluster,
813
crd::{AirflowRole, HTTP_PORT, HTTP_PORT_NAME},
914
};
1015

16+
// The group listener is a role-level object, so a constant `none` role-group is used as the
17+
// role-group label value.
18+
stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none");
19+
1120
pub fn build_group_listener(
1221
cluster: &ValidatedCluster,
1322
role: &AirflowRole,
@@ -18,11 +27,9 @@ pub fn build_group_listener(
1827
metadata: cluster
1928
.object_meta(
2029
listener_group_name,
21-
// The group listener is a role-level object, so a constant `none` role-group is
22-
// used as the role-group label value.
2330
cluster.recommended_labels_for(
24-
&role.role_name(),
25-
&"none".parse().expect("'none' is a valid role group name"),
31+
&ValidatedCluster::role_name(role),
32+
&NONE_ROLE_GROUP_NAME,
2633
),
2734
)
2835
.build(),

0 commit comments

Comments
 (0)