diff --git a/CHANGELOG.md b/CHANGELOG.md index b1ed39a4..7cc5e959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,11 @@ All notable changes to this project will be documented in this file. - Internal operator refactoring: introduce a build() step in the reconciler that assembles all relevant Kubernetes resources before anything is applied ([#852]). - Bump `stackable-operator` to 0.114.0 ([#867]). +- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac` + functions and carry the full set of recommended labels ([#861]). [#852]: https://github.com/stackabletech/opa-operator/pull/852 +[#861]: https://github.com/stackabletech/opa-operator/pull/861 [#867]: https://github.com/stackabletech/opa-operator/pull/867 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index a87f1bfe..acc5568d 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -4,7 +4,11 @@ use std::str::FromStr; use snafu::{ResultExt, Snafu}; -use stackable_operator::{utils::cluster_info::KubernetesClusterInfo, v2::types::common::Port}; +use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + utils::cluster_info::KubernetesClusterInfo, + v2::{builder::meta::ownerreference_from_resource, types::common::Port}, +}; use crate::controller::{ KubernetesResources, RoleGroupName, ValidatedCluster, @@ -12,6 +16,7 @@ use crate::controller::{ config_map::build_rolegroup_config_map, daemonset::build_server_rolegroup_daemonset, discovery::build_discovery_config_map, + rbac::{build_role_binding, build_service_account}, service::{ build_rolegroup_headless_service, build_rolegroup_metrics_service, build_server_role_service, @@ -51,7 +56,6 @@ pub enum Error { /// 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, @@ -81,7 +85,6 @@ pub fn build( role_group, opa_bundle_builder_image, user_info_fetcher_image, - service_account_name, cluster_info, ) .context(DaemonSetSnafu { @@ -98,6 +101,8 @@ pub fn build( daemon_sets, services, config_maps, + service_accounts: vec![build_service_account(cluster)], + role_bindings: vec![build_role_binding(cluster)], }) } @@ -113,6 +118,25 @@ stackable_operator::constant!(pub(crate) PLACEHOLDER_ROLE_LEVEL_ROLE_GROUP: Role // cluster-level object not bound to a single role group. stackable_operator::constant!(pub(crate) PLACEHOLDER_DISCOVERY_ROLE_GROUP: RoleGroupName = "discovery"); +/// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to +/// the cluster, and the recommended labels for a resource named `name` in `role_group_name`. +/// +/// Consolidates the metadata chain repeated by the child-resource builders. Call sites that +/// need extra labels/annotations chain them onto the returned builder before calling `build()`. +pub(crate) fn object_meta( + cluster: &ValidatedCluster, + name: impl Into, + role_group_name: &RoleGroupName, +) -> ObjectMetaBuilder { + let mut builder = ObjectMetaBuilder::new(); + builder + .name_and_namespace(cluster) + .name(name) + .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) + .with_labels(cluster.recommended_labels(role_group_name)); + builder +} + #[cfg(test)] mod tests { use serde_json::json; @@ -152,7 +176,6 @@ mod tests { fn build_produces_expected_resource_names() { let resources = build( &cluster(), - "test-opa-serviceaccount", "bundle-builder-image", "user-info-fetcher-image", &cluster_info(), @@ -178,5 +201,14 @@ mod tests { sorted_names(&resources.config_maps), ["test-opa", "test-opa-server-default"] ); + // The cluster-shared RBAC pair. + assert_eq!( + sorted_names(&resources.service_accounts), + ["test-opa-serviceaccount"] + ); + assert_eq!( + sorted_names(&resources.role_bindings), + ["test-opa-rolebinding"] + ); } } diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 44d83b46..fb4497d8 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -26,8 +26,23 @@ pub(crate) mod test_support { crd::v1alpha2, }; + /// The expected `app.kubernetes.io/version` label value for the given product version. + /// + /// The `-stackable` suffix carries the operator's own version, which is `0.0.0-dev` on main + /// but rewritten by the release process — so tests must derive it rather than hardcode it, + /// or they fail on release branches. + pub fn app_version_label(product_version: &str) -> String { + format!( + "{product_version}-stackable{}", + crate::built_info::PKG_VERSION + ) + } + /// Builds an `OpaCluster` from the given `spec` JSON and runs the validate step, returning the /// resulting [`ValidatedCluster`]. + /// + /// The cluster name (`test-opa`) deliberately differs from the product name (`opa`), so tests + /// asserting recommended labels catch swapped `name`/`instance` values. pub fn validated_cluster_from_spec(spec: Value) -> ValidatedCluster { let opa: v1alpha2::OpaCluster = serde_json::from_value(json!({ "apiVersion": "opa.stackable.tech/v1alpha2", diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index bc9aee78..7895dde1 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -9,7 +9,10 @@ use stackable_operator::{ use crate::controller::{ OpaRoleGroupConfig, RoleGroupName, ValidatedCluster, - build::properties::{ConfigFileName, config_json, product_logging, user_info_fetcher}, + build::{ + object_meta, + properties::{ConfigFileName, config_json, product_logging, user_info_fetcher}, + }, }; #[derive(Snafu, Debug)] @@ -41,15 +44,15 @@ pub fn build_rolegroup_config_map( ) -> Result { let mut cm_builder = ConfigMapBuilder::new(); - let metadata = cluster - .object_meta( - cluster - .resource_names(role_group_name) - .role_group_config_map() - .to_string(), - role_group_name, - ) - .build(); + let metadata = object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .role_group_config_map() + .to_string(), + role_group_name, + ) + .build(); cm_builder.metadata(metadata).add_data( ConfigFileName::ConfigJson.to_string(), diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs b/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs index 2846e488..19fc1040 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs @@ -221,7 +221,6 @@ pub fn build_server_rolegroup_daemonset( role_group: &OpaRoleGroupConfig, opa_bundle_builder_image: &str, user_info_fetcher_image: &str, - service_account_name: &str, cluster_info: &KubernetesClusterInfo, ) -> Result { let resolved_product_image = &cluster.image; @@ -353,7 +352,7 @@ pub fn build_server_rolegroup_daemonset( VolumeBuilder::new(CONFIG_VOLUME_NAME.as_ref()) .with_config_map( cluster - .resource_names(role_group_name) + .role_group_resource_names(role_group_name) .role_group_config_map() .to_string(), ) @@ -381,7 +380,12 @@ pub fn build_server_rolegroup_daemonset( .build(), ) .context(AddVolumeSnafu)? - .service_account_name(service_account_name) + .service_account_name( + cluster + .cluster_resource_names() + .service_account_name() + .to_string(), + ) .security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); if let Some(tls) = &cluster.cluster_config.tls { @@ -396,13 +400,13 @@ pub fn build_server_rolegroup_daemonset( .with_service_scope(cluster.server_role_service_name()) .with_service_scope( cluster - .resource_names(role_group_name) + .role_group_resource_names(role_group_name) .headless_service_name() .to_string(), ) .with_service_scope( cluster - .resource_names(role_group_name) + .role_group_resource_names(role_group_name) .metrics_service_name() .to_string(), ) @@ -430,7 +434,7 @@ pub fn build_server_rolegroup_daemonset( &container_name(&Container::Vector), resolved_product_image, vector_log_config, - &cluster.resource_names(role_group_name), + &cluster.role_group_resource_names(role_group_name), &CONFIG_VOLUME_NAME, &LOG_VOLUME_NAME, EnvVarSet::new(), @@ -442,15 +446,15 @@ pub fn build_server_rolegroup_daemonset( let mut pod_template = pb.build_template(); pod_template.merge_from(rolegroup_config.pod_overrides.clone()); - let metadata = cluster - .object_meta( - cluster - .resource_names(role_group_name) - .daemon_set_name() - .to_string(), - role_group_name, - ) - .build(); + let metadata = build::object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .daemon_set_name() + .to_string(), + role_group_name, + ) + .build(); let daemonset_spec = DaemonSetSpec { selector: LabelSelector { @@ -734,7 +738,6 @@ mod tests { role_group, "bundle-builder-image", "user-info-fetcher-image", - "test-opa-serviceaccount", &cluster_info(), ) .expect("the daemonset should build") diff --git a/rust/operator-binary/src/controller/build/resource/discovery.rs b/rust/operator-binary/src/controller/build/resource/discovery.rs index 1c19cc4e..4f596f7b 100644 --- a/rust/operator-binary/src/controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/controller/build/resource/discovery.rs @@ -6,7 +6,10 @@ use stackable_operator::{ }; use super::service::{APP_PORT, APP_TLS_PORT}; -use crate::controller::{ValidatedCluster, build::PLACEHOLDER_DISCOVERY_ROLE_GROUP}; +use crate::controller::{ + ValidatedCluster, + build::{PLACEHOLDER_DISCOVERY_ROLE_GROUP, object_meta}, +}; #[derive(Snafu, Debug)] pub enum Error { @@ -39,9 +42,12 @@ pub fn build_discovery_config_map( // Discovery is a cluster-level object (named after the cluster); `discovery` is used as a // placeholder role-group name for the recommended labels. - let metadata = cluster - .object_meta(cluster.name.to_string(), &PLACEHOLDER_DISCOVERY_ROLE_GROUP) - .build(); + let metadata = object_meta( + cluster, + cluster.name.to_string(), + &PLACEHOLDER_DISCOVERY_ROLE_GROUP, + ) + .build(); let mut cm_builder = ConfigMapBuilder::new(); cm_builder.metadata(metadata).add_data("OPA", url); diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index 6cb12f6c..a921b543 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -4,4 +4,5 @@ pub mod config_map; pub mod daemonset; pub mod discovery; +pub mod rbac; pub mod service; diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/rust/operator-binary/src/controller/build/resource/rbac.rs new file mode 100644 index 00000000..fdeffe4e --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -0,0 +1,143 @@ +//! Builds the RBAC resources (ServiceAccount + RoleBinding) shared by all role groups. + +use std::str::FromStr; + +use stackable_operator::{ + k8s_openapi::api::{core::v1::ServiceAccount, rbac::v1::RoleBinding}, + kvp::Labels, + v2::{ + rbac, + types::operator::{RoleGroupName, RoleName}, + }, +}; + +use crate::controller::ValidatedCluster; + +stackable_operator::constant!(NONE_ROLE_NAME: RoleName = "none"); +stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none"); + +/// Builds the [`ServiceAccount`] that the role-group Pods run under. +pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount { + rbac::build_service_account( + cluster, + &cluster.cluster_resource_names(), + rbac_labels(cluster), + ) +} + +/// Builds the [`RoleBinding`] that binds the [`ServiceAccount`] from [`build_service_account`] to +/// the operator-deployed ClusterRole. +pub fn build_role_binding(cluster: &ValidatedCluster) -> RoleBinding { + rbac::build_role_binding( + cluster, + &cluster.cluster_resource_names(), + rbac_labels(cluster), + ) +} + +/// Both resources are shared by the whole cluster rather than tied to a role or role group, so +/// the recommended labels carry `none` for both values. +fn rbac_labels(cluster: &ValidatedCluster) -> Labels { + cluster.recommended_labels_for(&NONE_ROLE_NAME, &NONE_ROLE_GROUP_NAME) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + use crate::controller::build::properties::test_support::{ + app_version_label, validated_cluster_from_spec, + }; + + // `test-opa` vs `opa`: see the swap-guard note on `validated_cluster_from_spec`. + fn cluster() -> ValidatedCluster { + validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + })) + } + + #[test] + fn test_service_account() { + let service_account = build_service_account(&cluster()); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + // The RBAC resources are cluster-shared, so role and role group are `none`. + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "test-opa", + "app.kubernetes.io/managed-by": "opa.stackable.tech_opacluster", + "app.kubernetes.io/name": "opa", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("1.2.3"), + "stackable.tech/vendor": "Stackable" + }, + "name": "test-opa-serviceaccount", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "opa.stackable.tech/v1alpha2", + "controller": true, + "kind": "OpaCluster", + "name": "test-opa", + "uid": "c27b3971-ca72-42c1-80a4-abdfc1db0ddd" + } + ] + } + }), + serde_json::to_value(service_account).expect("must be serializable") + ); + } + + #[test] + fn test_role_binding() { + let role_binding = build_role_binding(&cluster()); + + assert_eq!( + json!({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "RoleBinding", + "metadata": { + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "test-opa", + "app.kubernetes.io/managed-by": "opa.stackable.tech_opacluster", + "app.kubernetes.io/name": "opa", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("1.2.3"), + "stackable.tech/vendor": "Stackable" + }, + "name": "test-opa-rolebinding", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "opa.stackable.tech/v1alpha2", + "controller": true, + "kind": "OpaCluster", + "name": "test-opa", + "uid": "c27b3971-ca72-42c1-80a4-abdfc1db0ddd" + } + ] + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "opa-clusterrole" + }, + "subjects": [ + { + "kind": "ServiceAccount", + "name": "test-opa-serviceaccount", + "namespace": "default" + } + ] + }), + serde_json::to_value(role_binding).expect("must be serializable") + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 2f8ce83a..6e49120e 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -9,7 +9,8 @@ use stackable_operator::{ }; use crate::controller::{ - RoleGroupName, ValidatedCluster, build::PLACEHOLDER_ROLE_LEVEL_ROLE_GROUP, + RoleGroupName, ValidatedCluster, + build::{PLACEHOLDER_ROLE_LEVEL_ROLE_GROUP, object_meta}, }; pub const APP_PORT: Port = Port(8081); @@ -21,12 +22,12 @@ pub const METRICS_PORT_NAME: &str = "metrics"; /// The server-role service is the primary endpoint that should be used by clients that do not perform internal load balancing, /// including targets outside of the cluster. pub(crate) fn build_server_role_service(cluster: &ValidatedCluster) -> Service { - let metadata = cluster - .object_meta( - cluster.server_role_service_name(), - &PLACEHOLDER_ROLE_LEVEL_ROLE_GROUP, - ) - .build(); + let metadata = object_meta( + cluster, + cluster.server_role_service_name(), + &PLACEHOLDER_ROLE_LEVEL_ROLE_GROUP, + ) + .build(); let service_spec = ServiceSpec { type_: Some(cluster.cluster_config.listener_class.k8s_service_type()), @@ -56,15 +57,15 @@ pub(crate) fn build_rolegroup_headless_service( cluster: &ValidatedCluster, role_group_name: &RoleGroupName, ) -> Service { - let metadata = cluster - .object_meta( - cluster - .resource_names(role_group_name) - .headless_service_name() - .to_string(), - role_group_name, - ) - .build(); + let metadata = object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .headless_service_name() + .to_string(), + role_group_name, + ) + .build(); // Currently we don't offer listener-exposition of OPA mostly due to security concerns. // OPA is currently public within the Kubernetes (without authentication). @@ -114,23 +115,23 @@ pub(crate) fn build_rolegroup_metrics_service( } else { (Scheme::Http, APP_PORT) }; - let metadata = cluster - .object_meta( - cluster - .resource_names(role_group_name) - .metrics_service_name() - .to_string(), - role_group_name, - ) - .with_labels(prometheus_labels(&Scraping::Enabled)) - // The metrics are served on the same port as the HTTP/HTTPS traffic, under `/metrics`. - .with_annotations(prometheus_annotations( - &Scraping::Enabled, - &scheme, - "/metrics", - &port, - )) - .build(); + let metadata = object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .metrics_service_name() + .to_string(), + role_group_name, + ) + .with_labels(prometheus_labels(&Scraping::Enabled)) + // The metrics are served on the same port as the HTTP/HTTPS traffic, under `/metrics`. + .with_annotations(prometheus_annotations( + &Scraping::Enabled, + &scheme, + "/metrics", + &port, + )) + .build(); let service_spec = headless_cluster_ip_service_spec( vec![metrics_service_port(tls_enabled)], diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index c7e50986..f9dc9023 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -13,7 +13,8 @@ use stackable_operator::{ }, k8s_openapi::api::{ apps::v1::DaemonSet, - core::v1::{ConfigMap, Service}, + core::v1::{ConfigMap, Service, ServiceAccount}, + rbac::v1::RoleBinding, }, kube::{Resource as KubeResource, api::ObjectMeta}, kvp::Labels, @@ -22,7 +23,7 @@ use stackable_operator::{ HasName, HasUid, NameIsValidLabelValue, kvp::label::{recommended_labels, role_group_selector, role_selector}, role_group_utils::ResourceNames, - role_utils::{GenericCommonConfig, RoleGroupConfig}, + role_utils::{self, GenericCommonConfig, RoleGroupConfig}, types::{ kubernetes::{NamespaceName, Uid}, operator::{ @@ -55,6 +56,9 @@ pub struct ValidatedCluster { pub name: ClusterName, pub namespace: NamespaceName, pub uid: Uid, + /// The product version as a valid label value, for the recommended `app.kubernetes.io/version` + /// label. Derived from the resolved image's app-version label value. + pub product_version: ProductVersion, pub image: ResolvedProductImage, pub cluster_config: ValidatedClusterConfig, pub role_group_configs: BTreeMap>, @@ -69,6 +73,9 @@ impl ValidatedCluster { cluster_config: ValidatedClusterConfig, role_group_configs: BTreeMap>, ) -> Self { + let product_version = ProductVersion::from_str(&image.app_version_label_value) + .expect("the app version label value is a valid product version"); + let metadata = ObjectMeta { name: Some(name.to_string()), namespace: Some(namespace.to_string()), @@ -80,6 +87,7 @@ impl ValidatedCluster { name, namespace, uid, + product_version, image, cluster_config, role_group_configs, @@ -96,80 +104,71 @@ impl ValidatedCluster { format!("{name}-{role}", name = self.name, role = OpaRole::Server) } - /// The single OPA role name (`server`). - pub fn role_name() -> RoleName { - RoleName::from_str(&OpaRole::Server.to_string()) - .expect("the server role name is a valid role name") + /// 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(), + } } /// Type-safe names for the resources of a given role group. - pub(crate) fn resource_names(&self, role_group_name: &RoleGroupName) -> ResourceNames { + pub(crate) fn role_group_resource_names( + &self, + role_group_name: &RoleGroupName, + ) -> ResourceNames { ResourceNames { cluster_name: self.name.clone(), - role_name: Self::role_name(), + role_name: OpaRole::Server.into(), role_group_name: role_group_name.clone(), } } - /// The product version as a type-safe label value. - /// - /// `app_version_label_value` is constructed to be a valid label value, so it is also a valid - /// [`ProductVersion`]. - fn product_version(&self) -> ProductVersion { - ProductVersion::from_str(&self.image.app_version_label_value) - .expect("the app version label value is a valid product version") + pub fn recommended_labels(&self, role_group_name: &RoleGroupName) -> Labels { + self.recommended_labels_for(&OpaRole::Server.into(), role_group_name) } - /// Recommended labels for a role-group resource. - /// - /// For role-level or cluster-level resources (e.g. the role `Service` or the discovery - /// `ConfigMap`) pass a placeholder role-group name such as `global` or `discovery`. - pub fn recommended_labels(&self, role_group_name: &RoleGroupName) -> Labels { + /// Recommended labels for a resource that is not tied to a concrete role, + /// using a free-form role/role-group label value. + pub fn recommended_labels_for( + &self, + role_name: &RoleName, + role_group_name: &RoleGroupName, + ) -> Labels { + self.recommended_labels_with(&self.product_version, role_name, role_group_name) + } + + fn recommended_labels_with( + &self, + product_version: &ProductVersion, + role_name: &RoleName, + role_group_name: &RoleGroupName, + ) -> Labels { recommended_labels( self, &product_name(), - &self.product_version(), + product_version, &operator_name(), &controller_name(), - &Self::role_name(), + role_name, role_group_name, ) } /// Selector labels matching the pods of a role group. pub fn role_group_selector(&self, role_group_name: &RoleGroupName) -> Labels { - role_group_selector(self, &product_name(), &Self::role_name(), role_group_name) + role_group_selector( + self, + &product_name(), + &OpaRole::Server.into(), + role_group_name, + ) } /// Selector labels matching all pods of the (single) OPA role. pub fn role_selector(&self) -> Labels { - role_selector(self, &product_name(), &Self::role_name()) - } - - /// Returns an [`ObjectMetaBuilder`](stackable_operator::builder::meta::ObjectMetaBuilder) - /// pre-filled with the namespace, an owner reference back to this cluster, and the recommended - /// labels for a resource named `name` in `role_group_name`. - /// - /// Consolidates the metadata chain repeated by the child-resource builders. Call sites that - /// need extra labels/annotations chain them onto the returned builder before calling `build()`. - pub(crate) fn object_meta( - &self, - name: impl Into, - role_group_name: &RoleGroupName, - ) -> stackable_operator::builder::meta::ObjectMetaBuilder { - let mut builder = stackable_operator::builder::meta::ObjectMetaBuilder::new(); - builder - .name_and_namespace(self) - .name(name) - .ownerreference( - stackable_operator::v2::builder::meta::ownerreference_from_resource( - self, - None, - Some(true), - ), - ) - .with_labels(self.recommended_labels(role_group_name)); - builder + role_selector(self, &product_name(), &OpaRole::Server.into()) } } @@ -246,6 +245,8 @@ pub struct KubernetesResources { pub daemon_sets: Vec, pub services: Vec, pub config_maps: Vec, + pub service_accounts: Vec, + pub role_bindings: Vec, } /// Cluster-wide settings resolved once during validation, so the build steps no longer need the @@ -279,3 +280,21 @@ impl ValidatedOpaConfig { } } } + +#[cfg(test)] +mod tests { + use stackable_operator::v2::types::operator::RoleName; + use strum::IntoEnumIterator; + + use crate::crd::OpaRole; + + /// Locks the invariant behind the `expect` in the `From for RoleName` impls: + /// every `OpaRole` variant (present and future) must serialise to a valid `RoleName`. + #[test] + fn every_opa_role_serialises_to_a_valid_role_name() { + for role in OpaRole::iter() { + let _: RoleName = (&role).into(); + let _: RoleName = role.into(); + } + } +} diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index aef9c278..ffd36132 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -240,11 +240,81 @@ pub fn validate( #[cfg(test)] mod tests { + use serde_json::json; use stackable_operator::product_logging::spec::{ AutomaticContainerLogConfig, ContainerLogConfig, ContainerLogConfigChoice, }; use super::*; + use crate::controller::build::properties::test_support::app_version_label; + + /// Locks every value the validate step itself derives from the minimal fixture — so a + /// validation regression fails here, with a validate-shaped message, instead of surfacing as + /// a confusing build-test failure downstream. + /// + /// The merged per-role-group config (resources, affinity, logging defaults, …) is produced by + /// the config merge machinery, whose contracts are tested in operator-rs and the properties + /// tests; only the values this module derives on top are re-asserted here. + #[test] + fn validate_ok_derives_expected_values() { + let opa: v1alpha2::OpaCluster = serde_json::from_value(json!({ + "apiVersion": "opa.stackable.tech/v1alpha2", + "kind": "OpaCluster", + "metadata": { + "name": "test-opa", + "namespace": "default", + "uid": "c27b3971-ca72-42c1-80a4-abdfc1db0ddd", + }, + "spec": { + "image": { "productVersion": "1.2.3" }, + "servers": { "roleGroups": { "default": {} } }, + }, + })) + .expect("valid test input"); + let operator_environment = OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_string(), + operator_service_name: "opa-operator".to_string(), + image_repository: "oci.example.org".to_string(), + }; + + let cluster = validate(&opa, &operator_environment).expect("the minimal fixture validates"); + + assert_eq!(cluster.name.to_string(), "test-opa"); + assert_eq!(cluster.namespace.to_string(), "default"); + assert_eq!( + cluster.uid.to_string(), + "c27b3971-ca72-42c1-80a4-abdfc1db0ddd" + ); + assert_eq!( + cluster.image.image, + format!("oci.example.org/opa:{}", app_version_label("1.2.3")) + ); + assert_eq!(cluster.image.product_version, "1.2.3"); + assert_eq!( + cluster.product_version.to_string(), + app_version_label("1.2.3") + ); + + // The minimal fixture configures no user-info fetcher and no TLS; the listener class + // falls back to its default. + assert_eq!(cluster.cluster_config.user_info, None); + assert_eq!(cluster.cluster_config.tls, None); + assert_eq!( + cluster.cluster_config.listener_class, + v1alpha2::CurrentlySupportedListenerClasses::ClusterInternal + ); + + // A single `server` role with the single `default` role group; the Vector agent is off. + assert_eq!(cluster.role_group_configs.len(), 1); + let role_groups = &cluster.role_group_configs[&OpaRole::Server]; + let role_group_names: Vec = role_groups.keys().map(ToString::to_string).collect(); + assert_eq!(role_group_names, ["default"]); + let role_group = role_groups + .values() + .next() + .expect("the default role group exists"); + assert_eq!(role_group.config.logging.vector_container, None); + } /// A [`Logging`] with an automatic log config for every container, as the (defaulted) merged /// config provides at runtime. `validate_logging` validates all containers, so all must be diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 1c090312..4545aebc 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -1,3 +1,5 @@ +use std::str::FromStr; + use serde::{Deserialize, Serialize}; use stackable_operator::{ commons::{ @@ -21,7 +23,10 @@ use stackable_operator::{ v2::{ config_overrides::JsonConfigOverrides, role_utils::GenericCommonConfig, - types::kubernetes::{ConfigMapName, SecretClassName}, + types::{ + kubernetes::{ConfigMapName, SecretClassName}, + operator::RoleName, + }, }, versioned::versioned, }; @@ -254,6 +259,18 @@ pub enum OpaRole { Server, } +impl From for RoleName { + fn from(value: OpaRole) -> Self { + RoleName::from_str(&value.to_string()).expect("an OpaRole is a valid role name") + } +} + +impl From<&OpaRole> for RoleName { + fn from(value: &OpaRole) -> Self { + RoleName::from_str(&value.to_string()).expect("an OpaRole is a valid role name") + } +} + // TODO (@Techassi): Support versioned status #[derive(Clone, Default, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] diff --git a/rust/operator-binary/src/opa_controller.rs b/rust/operator-binary/src/opa_controller.rs index 92138745..6c4a916c 100644 --- a/rust/operator-binary/src/opa_controller.rs +++ b/rust/operator-binary/src/opa_controller.rs @@ -6,13 +6,11 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ cli::OperatorEnvironmentOptions, cluster_resources::ClusterResourceApplyStrategy, - commons::rbac::build_rbac_resources, kube::{ ResourceExt, core::{DeserializeGuard, error_boundary}, runtime::controller::Action, }, - kvp::LabelError, logging::controller::ReconcilerError, shared::time::Duration, status::condition::{ @@ -26,7 +24,7 @@ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ controller::{build, controller_name, operator_name, product_name, validate}, - crd::{APP_NAME, OPERATOR_NAME, OpaClusterStatus, v1alpha2}, + crd::{OPERATOR_NAME, OpaClusterStatus, v1alpha2}, }; pub const OPA_CONTROLLER_NAME: &str = "opacluster"; @@ -71,16 +69,6 @@ pub enum Error { name: String, }, - #[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 update status"))] ApplyStatus { source: stackable_operator::client::Error, @@ -90,14 +78,6 @@ pub enum Error { DeleteOrphans { source: stackable_operator::cluster_resources::Error, }, - - #[snafu(display("failed to build RBAC resources"))] - BuildRbacResources { - source: stackable_operator::commons::rbac::Error, - }, - - #[snafu(display("failed to build label"))] - BuildLabel { source: LabelError }, } type Result = std::result::Result; @@ -134,29 +114,8 @@ pub async fn reconcile_opa( &opa.spec.object_overrides, ); - let required_labels = cluster_resources - .get_required_labels() - .context(BuildLabelSnafu)?; - - let (rbac_sa, rbac_rolebinding) = - build_rbac_resources(opa, APP_NAME, required_labels).context(BuildRbacResourcesSnafu)?; - - // 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)?; - cluster_resources - .add(client, rbac_rolebinding) - .await - .context(ApplyRoleBindingSnafu)?; - let resources = build::build( &validated_cluster, - &service_account_name, &ctx.opa_bundle_builder_image, &ctx.user_info_fetcher_image, &ctx.cluster_info, @@ -167,6 +126,18 @@ pub async fn reconcile_opa( // 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_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)