diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fd40a4b0..1b5817e55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,9 @@ All notable changes to this project will be documented in this file. operator upgrade, delete each broker StatefulSet so that the operator immediately recreates it with the new labels ([#1011]). - Make operations infallible where dependent on static inputs ([#1017]). +- Internal operator refactoring: build each role group through per-role modules (brokers, + controllers) and a shared role group builder, with the broker and controller configuration held + in typed fields of the validated cluster ([#1041]). - Remove `serde_yaml` ([#1043]). - All pods now set `securityContext.seccompProfile.type` to `RuntimeDefault` ([#1043]). - Support Vector 0.58.0 ([#1039], [#1044], [#1046]). @@ -93,6 +96,7 @@ All notable changes to this project will be documented in this file. [#1034]: https://github.com/stackabletech/kafka-operator/pull/1034 [#1035]: https://github.com/stackabletech/kafka-operator/pull/1035 [#1039]: https://github.com/stackabletech/kafka-operator/pull/1039 +[#1041]: https://github.com/stackabletech/kafka-operator/pull/1041 [#1042]: https://github.com/stackabletech/kafka-operator/pull/1042 [#1043]: https://github.com/stackabletech/kafka-operator/pull/1043 [#1044]: https://github.com/stackabletech/kafka-operator/pull/1044 diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index a4a539a40..dd3176ecf 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -18,7 +18,12 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ cli::OperatorEnvironmentOptions, cluster_resources::ClusterResourceApplyStrategy, - commons::{networking::DomainName, product_image_selection::ResolvedProductImage}, + commons::{ + affinity::StackableAffinity, + networking::DomainName, + product_image_selection::ResolvedProductImage, + resources::{NoRuntimeLimits, Resources}, + }, constant, crd::listener, k8s_openapi::api::{ @@ -40,7 +45,7 @@ use stackable_operator::{ role_group_utils::ResourceNames, role_utils, types::{ - kubernetes::{ConfigMapName, ListenerName, NamespaceName, Uid}, + kubernetes::{ConfigMapName, ListenerClassName, ListenerName, NamespaceName, Uid}, operator::{ClusterName, ControllerName, OperatorName, ProductName, ProductVersion}, }, }, @@ -67,7 +72,7 @@ use crate::{ crd::{ APP_NAME, KAFKA_OPERATOR_NAME, KafkaPodDescriptor, MetadataManager, authorization::KafkaAuthorizationConfig, - role::{AnyConfig, AnyConfigOverrides, KafkaRole}, + role::{KafkaRole, commons::Storage}, v1alpha1, }, }; @@ -143,9 +148,15 @@ pub struct ValidatedCluster { /// value. pub product_version: ProductVersion, pub cluster_config: ValidatedClusterConfig, - /// Per-role configuration (e.g. the Pod disruption budget), keyed by role. - pub role_configs: BTreeMap, - pub role_group_configs: BTreeMap>, + /// The broker role-level config. The CRD requires the broker role. + pub broker_config: ValidatedRoleConfig, + /// The validated config of every broker role group, keyed by role group name. + pub broker_role_group_configs: BTreeMap, + /// The controller role-level config, or `None` if the role is absent (ZooKeeper mode). + pub controller_config: Option, + /// The validated config of every controller role group, keyed by role group name; empty if + /// the role is absent. + pub controller_role_group_configs: BTreeMap, /// The broker role groups' bootstrap `Listener`s as currently stored in the cluster (fetched /// in the dereference step), from which the discovery `ConfigMap` is built. Missing or still /// address-less around the first reconcile runs; the listener-operator populates the ingress @@ -153,41 +164,41 @@ pub struct ValidatedCluster { pub bootstrap_listeners: Vec, } +/// The number of pods a role group runs when its `replicas` is unset. +/// +/// An unset count stays unset on the StatefulSet, and Kubernetes then runs one pod. This default +/// applies only where the operator needs a concrete count: to predict the pods (their names and +/// node IDs) and to sum a role's replicas. +pub(crate) const DEFAULT_REPLICAS: u16 = 1; + +/// The role group's replica count, or [`DEFAULT_REPLICAS`] if it is unset. +pub(crate) fn replicas_or_default(replicas: Option) -> u16 { + replicas.unwrap_or(DEFAULT_REPLICAS) +} + impl ValidatedCluster { - #[allow(clippy::too_many_arguments)] - pub fn new( - name: ClusterName, - namespace: NamespaceName, - uid: Uid, - cluster_domain: DomainName, - image: ResolvedProductImage, - cluster_config: ValidatedClusterConfig, - role_configs: BTreeMap, - role_group_configs: BTreeMap>, - bootstrap_listeners: Vec, - ) -> Self { + /// The `ObjectMeta` that lets [`ValidatedCluster`] act as the owner [`Resource`] for child + /// objects: just `name`, `namespace` and `uid`. + pub(crate) fn object_meta( + name: &ClusterName, + namespace: &NamespaceName, + uid: &Uid, + ) -> ObjectMeta { + ObjectMeta { + name: Some(name.to_string()), + namespace: Some(namespace.to_string()), + uid: Some(uid.to_string()), + ..ObjectMeta::default() + } + } + + /// The product version as a valid label value, for the recommended `app.kubernetes.io/version` + /// label. + pub(crate) fn product_version(image: &ResolvedProductImage) -> ProductVersion { // `app_version_label_value` is constructed to be a valid label value, so it is also a // valid `ProductVersion`. - let product_version = ProductVersion::from_str(&image.app_version_label_value) - .expect("the app version label value is a valid product version"); - Self { - metadata: ObjectMeta { - name: Some(name.to_string()), - namespace: Some(namespace.to_string()), - uid: Some(uid.to_string()), - ..ObjectMeta::default() - }, - name, - namespace, - uid, - cluster_domain, - image, - product_version, - cluster_config, - role_configs, - role_group_configs, - bootstrap_listeners, - } + ProductVersion::from_str(&image.app_version_label_value) + .expect("the app version label value is a valid product version") } /// Predicts the pods of this cluster (or just `requested_kafka_role`'s pods, if given). @@ -206,8 +217,25 @@ impl ValidatedCluster { let mut pod_descriptors = Vec::new(); let mut seen_hashes = HashMap::::new(); - for (role, role_groups) in &self.role_group_configs { - for (role_group_name, validated_rg) in role_groups { + // In `KafkaRole` variant order, which decides the reported side of a hash collision. + let broker_role_groups: Vec<(&RoleGroupName, Option)> = self + .broker_role_group_configs + .iter() + .map(|(role_group_name, rg)| (role_group_name, rg.replicas)) + .collect(); + let controller_role_groups: Vec<(&RoleGroupName, Option)> = self + .controller_role_group_configs + .iter() + .map(|(role_group_name, rg)| (role_group_name, rg.replicas)) + .collect(); + let role_groups = [ + (KafkaRole::Broker, broker_role_groups), + (KafkaRole::Controller, controller_role_groups), + ]; + + for (role, role_groups) in &role_groups { + for (role_group_name, replicas) in role_groups { + let role_group_name = *role_group_name; let node_id_hash_offset = node_id_hash32_offset(role, role_group_name.as_ref()); // The node id hash offset must be unique across the cluster. @@ -229,8 +257,8 @@ impl ValidatedCluster { let role_group_statefulset_name = resource_names.stateful_set_name(); let role_group_service_name = resource_names.headless_service_name(); // Pods must be predicted from a concrete count (e.g. for KRaft quorum - // voters), so an unset replica count falls back to 1. - for replica in 0..validated_rg.replicas.unwrap_or(1) { + // voters), so an unset replica count falls back to the default. + for replica in 0..replicas_or_default(*replicas) { pod_descriptors.push(KafkaPodDescriptor { namespace: self.namespace.clone(), role_group_service_name: role_group_service_name.clone(), @@ -379,19 +407,44 @@ impl Resource for ValidatedCluster { } } -/// The validated, merged per-role-group config. +/// A broker role group's validated config: the fields of the merged +/// [`BrokerConfig`](crate::crd::role::broker::BrokerConfig) that the build step reads, with the +/// logging validated and the values the merge always sets unwrapped. #[derive(Clone, Debug, PartialEq)] -pub struct ValidatedKafkaConfig { - pub config: AnyConfig, - /// Validated logging configuration (derived from `config.logging` during validation). +pub struct ValidatedBrokerConfig { + pub resources: Resources, pub logging: validate::ValidatedLogging, + pub affinity: StackableAffinity, + pub graceful_shutdown_timeout: Duration, + pub requested_secret_lifetime: Duration, + pub bootstrap_listener_class: ListenerClassName, + pub broker_listener_class: ListenerClassName, } -/// A validated, merged Kafka role-group config. -pub type ValidatedRoleGroupConfig = stackable_operator::v2::role_utils::RoleGroupConfig< - ValidatedKafkaConfig, - stackable_operator::v2::role_utils::JavaCommonConfig, - AnyConfigOverrides, +/// A controller role group's validated config: the fields of the merged +/// [`ControllerConfig`](crate::crd::role::controller::ControllerConfig) that the build step +/// reads, with the logging validated and the values the merge always sets unwrapped. +#[derive(Clone, Debug, PartialEq)] +pub struct ValidatedControllerConfig { + pub resources: Resources, + pub logging: validate::ValidatedLogging, + pub affinity: StackableAffinity, + pub graceful_shutdown_timeout: Duration, + pub requested_secret_lifetime: Duration, +} + +/// The [`RoleGroupConfig`](role_utils::RoleGroupConfig) of one broker role group. +pub type BrokerRoleGroupConfig = role_utils::RoleGroupConfig< + ValidatedBrokerConfig, + role_utils::JavaCommonConfig, + v1alpha1::KafkaBrokerConfigOverrides, +>; + +/// The [`RoleGroupConfig`](role_utils::RoleGroupConfig) of one controller role group. +pub type ControllerRoleGroupConfig = role_utils::RoleGroupConfig< + ValidatedControllerConfig, + role_utils::JavaCommonConfig, + v1alpha1::KafkaControllerConfigOverrides, >; pub struct Ctx { @@ -519,12 +572,18 @@ pub(crate) mod test_support { use stackable_operator::{ cli::OperatorEnvironmentOptions, commons::networking::DomainName, - crd::listener, + crd::{authentication::core, listener}, utils::{cluster_info::KubernetesClusterInfo, yaml_from_str_singleton_map}, }; - use super::{ValidatedCluster, dereference::DereferencedObjects, validate::validate}; - use crate::crd::{authentication::ResolvedAuthenticationClasses, v1alpha1}; + use super::{ + BrokerRoleGroupConfig, ControllerRoleGroupConfig, ValidatedCluster, + dereference::DereferencedObjects, validate::validate, + }; + use crate::crd::{ + authentication::ResolvedAuthenticationClasses, authorization::KafkaAuthorizationConfig, + v1alpha1, + }; /// The expected `app.kubernetes.io/version` label value for the given product version. /// @@ -614,6 +673,48 @@ pub(crate) mod test_support { validate_err(kafka).expect("validate should succeed for the test fixture") } + /// The validated config of the named broker role group. + pub fn broker_role_group_config<'a>( + cluster: &'a ValidatedCluster, + role_group: &str, + ) -> &'a BrokerRoleGroupConfig { + cluster + .broker_role_group_configs + .get(&role_group.parse().expect("valid role group name")) + .expect("the broker role group exists") + } + + /// The validated config of the named controller role group. + pub fn controller_role_group_config<'a>( + cluster: &'a ValidatedCluster, + role_group: &str, + ) -> &'a ControllerRoleGroupConfig { + cluster + .controller_role_group_configs + .get(&role_group.parse().expect("valid role group name")) + .expect("the controller role group exists") + } + + /// Runs the real validate step with the given dereferenced `AuthenticationClass`es and OPA + /// config, for fixtures that exercise the Kerberos, TLS-authentication or OPA paths. + pub fn validated_cluster_with_security( + kafka: &v1alpha1::KafkaCluster, + authentication_classes: Vec, + authorization_config: Option, + ) -> ValidatedCluster { + validate( + kafka, + DereferencedObjects { + authentication_classes: ResolvedAuthenticationClasses::new(authentication_classes), + authorization_config, + kubernetes_cluster_info: cluster_info(), + bootstrap_listeners: Vec::new(), + }, + &operator_environment(), + ) + .expect("validate should succeed for the test fixture") + } + /// Runs the real validate step against a minimal (auth/OPA-free) fixture, without unwrapping /// the result. Used for tests asserting on a specific validation failure. pub fn validate_err( @@ -703,6 +804,65 @@ mod tests { } } + /// The predicted pods follow the StatefulSet's replica count: a role group scaled to zero + /// predicts no pods, and one without a replica count predicts `DEFAULT_REPLICAS` pods, as + /// Kubernetes runs for `replicas: null`. Each descriptor carries the role group's StatefulSet + /// name and the pod's index, from which its pod name (`-`, the name + /// Kubernetes gives the pod) is rendered. + #[test] + fn pod_descriptors_follow_the_replica_count() { + let kafka = minimal_kafka( + r#" + apiVersion: kafka.stackable.tech/v1alpha1 + kind: KafkaCluster + metadata: + name: simple-kafka + namespace: default + uid: 12345678-1234-1234-1234-123456789012 + spec: + image: + productVersion: 3.9.2 + clusterConfig: + zookeeperConfigMapName: xyz + brokers: + roleGroups: + unset: {} + stopped: + replicas: 0 + three: + replicas: 3 + "#, + ); + let validated = validated_cluster(&kafka); + let descriptors = validated + .pod_descriptors(None) + .expect("non-colliding role groups must not error"); + + for (role_group, expected_replicas) in [ + ("unset", super::DEFAULT_REPLICAS), + ("stopped", 0), + ("three", 3), + ] { + let stateful_set_name = validated + .role_group_resource_names( + &KafkaRole::Broker, + &role_group.parse().expect("valid role group name"), + ) + .stateful_set_name() + .to_string(); + let replicas: Vec = descriptors + .iter() + .filter(|d| d.role_group_statefulset_name.to_string() == stateful_set_name) + .map(|d| d.replica) + .collect(); + assert_eq!( + replicas, + (0..expected_replicas).collect::>(), + "{role_group}: one descriptor per pod, indexed from 0" + ); + } + } + /// Non-colliding role groups expand to one descriptor per replica, each with a /// unique `node_id`. #[test] diff --git a/rust/operator-binary/src/controller/build/command.rs b/rust/operator-binary/src/controller/build/command.rs index ebe59764d..04c1f5e07 100644 --- a/rust/operator-binary/src/controller/build/command.rs +++ b/rust/operator-binary/src/controller/build/command.rs @@ -318,15 +318,11 @@ const PRE_STOP_MAX_DEADLINE_SECONDS: u64 = 120; /// The wall-clock budget (seconds) [`controller_remove_self_pre_stop_command`] retries voter removal /// for, derived from the pod's actual `gracefulShutdownTimeout` rather than a single fixed /// constant. -fn pre_stop_deadline_seconds(graceful_shutdown_timeout: Option) -> u64 { - graceful_shutdown_timeout - .map(|timeout| { - let secs = timeout.as_secs(); - secs.saturating_sub(PRE_STOP_RESERVED_FOR_KAFKA_SHUTDOWN_SECONDS) - .clamp(PRE_STOP_MIN_DEADLINE_SECONDS, PRE_STOP_MAX_DEADLINE_SECONDS) - .min(secs) // never outlive the grace period itself - }) - .unwrap_or(PRE_STOP_MIN_DEADLINE_SECONDS) +fn pre_stop_deadline_seconds(graceful_shutdown_timeout: Duration) -> u64 { + let secs = graceful_shutdown_timeout.as_secs(); + secs.saturating_sub(PRE_STOP_RESERVED_FOR_KAFKA_SHUTDOWN_SECONDS) + .clamp(PRE_STOP_MIN_DEADLINE_SECONDS, PRE_STOP_MAX_DEADLINE_SECONDS) + .min(secs) // never outlive the grace period itself } /// Pause (seconds) between two voter-removal attempts inside the `preStop` script's retry @@ -364,9 +360,7 @@ fn strip_shell_comments(script: &str) -> String { /// [`CONTROLLER_REMOVE_SELF_PRE_STOP_SCRIPT`] its inputs; the logic, and the reasoning behind /// it, lives in that script (whose maintenance comments [`strip_shell_comments`] drops on /// the way in). -pub fn controller_remove_self_pre_stop_command( - graceful_shutdown_timeout: Option, -) -> String { +pub fn controller_remove_self_pre_stop_command(graceful_shutdown_timeout: Duration) -> String { formatdoc! {" set -uo pipefail {derive_pod_index} @@ -786,7 +780,7 @@ mod tests { /// assigned by the generated command. #[test] fn generated_pre_stop_command_defines_every_input_the_script_requires() { - let command = controller_remove_self_pre_stop_command(None); + let command = controller_remove_self_pre_stop_command(Duration::from_minutes_unchecked(30)); let required_inputs: Vec<&str> = CONTROLLER_REMOVE_SELF_PRE_STOP_SCRIPT .lines() .filter_map(|line| line.trim().strip_prefix(r#": "${"#)) @@ -811,32 +805,25 @@ mod tests { /// documented floor/cap regardless of how short or long that timeout is. #[test] fn pre_stop_deadline_seconds_is_derived_from_graceful_shutdown_timeout_within_floor_and_cap() { - // No configured timeout (shouldn't happen in practice) falls back to the floor. - assert_eq!( - pre_stop_deadline_seconds(None), - PRE_STOP_MIN_DEADLINE_SECONDS - ); - // A short timeout (shorter than the reserved buffer) still gets at least the floor. - assert_eq!(pre_stop_deadline_seconds(Some(Duration::from_secs(10))), 10); + assert_eq!(pre_stop_deadline_seconds(Duration::from_secs(10)), 10); // A generous timeout (the operator's own 30-minute default) is capped, not handed the // entire budget minus the reserve. assert_eq!( - pre_stop_deadline_seconds(Some(Duration::from_minutes_unchecked(30))), + pre_stop_deadline_seconds(Duration::from_minutes_unchecked(30)), PRE_STOP_MAX_DEADLINE_SECONDS ); // A timeout comfortably between the floor and the cap (once the reserve is subtracted) // is used as-is. assert_eq!( - pre_stop_deadline_seconds(Some(Duration::from_secs(90))), + pre_stop_deadline_seconds(Duration::from_secs(90)), 90 - PRE_STOP_RESERVED_FOR_KAFKA_SHUTDOWN_SECONDS ); // ...and the generated script is handed that value, rather than a literal. - let command = - controller_remove_self_pre_stop_command(Some(Duration::from_minutes_unchecked(30))); + let command = controller_remove_self_pre_stop_command(Duration::from_minutes_unchecked(30)); assert!(command.contains(&format!( "REMOVAL_DEADLINE_SECONDS={PRE_STOP_MAX_DEADLINE_SECONDS}" ))); diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs new file mode 100644 index 000000000..0c109658c --- /dev/null +++ b/rust/operator-binary/src/controller/build/container.rs @@ -0,0 +1,176 @@ +//! The parts of the `kafka` container that broker and controller role groups share. +//! +//! The role modules in [`super::role_group`] build their own `kafka` container; they take the +//! environment variables, ports and log config volume every role has from here. + +use std::str::FromStr; + +use stackable_operator::{ + builder::pod::{container::FieldPathEnvVar, volume::VolumeBuilder}, + commons::{ + product_image_selection::ResolvedProductImage, + resources::{NoRuntimeLimits, Resources}, + }, + k8s_openapi::api::core::v1::{ContainerPort, Volume}, + v2::{ + builder::pod::container::{EnvVarName, EnvVarSet}, + jvm_argument_overrides::JvmArgumentOverrides, + product_logging::framework::{STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice}, + role_group_utils::ResourceNames, + types::kubernetes::ConfigMapKey, + }, +}; + +use crate::{ + controller::{ + RoleGroupName, ValidatedCluster, + build::{ + command::{KAFKA_LOG4J_OPTS, kafka_log_opts}, + jvm, + }, + node_id_hasher::node_id_hash32_offset, + security::ValidatedKafkaSecurity, + validate::ValidatedLogging, + }, + crd::{ + KAFKA_HEAP_OPTS, METRICS_PORT, METRICS_PORT_NAME, STACKABLE_LOG_CONFIG_DIR_NAME, + role::{KAFKA_NODE_ID_OFFSET, KafkaRole, commons::Storage}, + }, +}; + +// Env vars the operator sets on the Kafka containers. +stackable_operator::constant!(pub(crate) POD_NAME: EnvVarName = "POD_NAME"); +stackable_operator::constant!(KAFKA_CLIENT_PORT: EnvVarName = "KAFKA_CLIENT_PORT"); +stackable_operator::constant!(EXTRA_ARGS: EnvVarName = "EXTRA_ARGS"); +// Needed for the `containerdebug` process to log its tracing information to. +stackable_operator::constant!(pub(crate) CONTAINERDEBUG_LOG_DIRECTORY: EnvVarName = "CONTAINERDEBUG_LOG_DIRECTORY"); + +// The env var and the ZooKeeper discovery ConfigMap key holding the ZooKeeper connection +// string (the same string, as the env var name is used as the ConfigMap key). +stackable_operator::constant!(ZOOKEEPER: EnvVarName = "ZOOKEEPER"); +stackable_operator::constant!(ZOOKEEPER_CONFIG_MAP_KEY: ConfigMapKey = "ZOOKEEPER"); + +/// Environment variables the operator sets on the Kafka container that are common to broker and +/// controller role groups. +/// +/// These form the base; the role modules merge the user's `envOverrides` on top, so a user +/// override wins on a name collision. Using an [`EnvVarSet`] (a name-keyed map) makes that +/// precedence explicit and de-duplicates by name, rather than relying on append order. +pub(crate) fn common_operator_env_vars( + validated_cluster: &ValidatedCluster, + kafka_security: &ValidatedKafkaSecurity, +) -> EnvVarSet { + let mut env = EnvVarSet::new() + .with_field_path(&POD_NAME, &FieldPathEnvVar::Name) + .with_value(&KAFKA_CLIENT_PORT, kafka_security.client_port().to_string()); + + // Present in ZooKeeper mode only: brokers use it to connect, controllers for migration. + if let Some(zookeeper_config_map_name) = + &validated_cluster.cluster_config.zookeeper_config_map_name + { + env = env.with_config_map_key_ref( + &ZOOKEEPER, + zookeeper_config_map_name, + &ZOOKEEPER_CONFIG_MAP_KEY, + ); + } + + env +} + +/// Environment variables the operator sets on the Kafka container of both roles, on top of +/// [`common_operator_env_vars`]. +/// +/// Returned as an [`EnvVarSet`] so the callers can merge the user's `envOverrides` on top, +/// letting an override win on a name collision. +pub(crate) fn common_kafka_env( + resources: &Resources, + jvm_argument_overrides: &JvmArgumentOverrides, + resolved_product_image: &ResolvedProductImage, + kafka_role: &KafkaRole, + role_group_name: &RoleGroupName, +) -> Result { + Ok(EnvVarSet::new() + .with_value( + &EXTRA_ARGS, + jvm::construct_non_heap_jvm_args(resources, jvm_argument_overrides)?, + ) + .with_value( + &KAFKA_HEAP_OPTS, + jvm::construct_heap_jvm_args(resources, jvm_argument_overrides)?, + ) + .with_value( + &KAFKA_LOG4J_OPTS, + kafka_log_opts(&resolved_product_image.product_version), + ) + .with_value( + &CONTAINERDEBUG_LOG_DIRECTORY, + format!("{STACKABLE_LOG_DIR}/containerdebug"), + ) + .with_value( + &KAFKA_NODE_ID_OFFSET, + node_id_hash32_offset(kafka_role, role_group_name.as_ref()).to_string(), + )) +} + +/// The ports the `kafka` container exposes: metrics and the client port, plus the bootstrap port +/// when Kerberos is enabled. +pub(crate) fn container_ports(kafka_security: &ValidatedKafkaSecurity) -> Vec { + let mut ports = vec![ + ContainerPort { + name: Some(METRICS_PORT_NAME.to_string()), + container_port: METRICS_PORT.into(), + protocol: Some("TCP".to_string()), + ..ContainerPort::default() + }, + ContainerPort { + name: Some(kafka_security.client_port_name().to_string()), + container_port: kafka_security.client_port().into(), + protocol: Some("TCP".to_string()), + ..ContainerPort::default() + }, + ]; + if kafka_security.has_kerberos_enabled() { + ports.push(ContainerPort { + name: Some(kafka_security.bootstrap_port_name().to_string()), + container_port: kafka_security.bootstrap_port().into(), + protocol: Some("TCP".to_string()), + ..ContainerPort::default() + }); + } + ports +} + +/// The `log-config` volume, sourced either from the user-supplied custom log config `ConfigMap` +/// or the rolegroup `ConfigMap` (which carries the operator-generated config). +/// Branches on the *validated* Kafka-container logging choice. +pub(crate) fn log_config_volume( + logging: &ValidatedLogging, + resource_names: &ResourceNames, +) -> Volume { + let config_map = match &logging.kafka_container { + ValidatedContainerLogConfigChoice::Custom(config_map_name) => config_map_name.to_string(), + ValidatedContainerLogConfigChoice::Automatic(_) => { + resource_names.role_group_config_map().to_string() + } + }; + VolumeBuilder::new(&*STACKABLE_LOG_CONFIG_DIR_NAME) + .with_config_map(config_map) + .build() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *POD_NAME; + let _ = *KAFKA_CLIENT_PORT; + let _ = *EXTRA_ARGS; + let _ = *CONTAINERDEBUG_LOG_DIRECTORY; + let _ = *ZOOKEEPER; + let _ = *ZOOKEEPER_CONFIG_MAP_KEY; + } +} diff --git a/rust/operator-binary/src/controller/build/graceful_shutdown.rs b/rust/operator-binary/src/controller/build/graceful_shutdown.rs index b03187f4b..b7e6d9ea1 100644 --- a/rust/operator-binary/src/controller/build/graceful_shutdown.rs +++ b/rust/operator-binary/src/controller/build/graceful_shutdown.rs @@ -1,9 +1,7 @@ use std::collections::BTreeMap; use snafu::{ResultExt, Snafu}; -use stackable_operator::builder::pod::PodBuilder; - -use crate::crd::role::AnyConfig; +use stackable_operator::{builder::pod::PodBuilder, shared::time::Duration}; #[derive(Debug, Snafu)] pub enum Error { @@ -20,16 +18,12 @@ pub fn graceful_shutdown_config_properties() -> BTreeMap { } pub fn add_graceful_shutdown_config( - merged_config: &AnyConfig, + graceful_shutdown_timeout: &Duration, pod_builder: &mut PodBuilder, ) -> Result<(), Error> { - // This must be always set by the merge mechanism, as we provide a default value, - // users can not disable graceful shutdown. - if let Some(graceful_shutdown_timeout) = &merged_config.graceful_shutdown_timeout { - pod_builder - .termination_grace_period(graceful_shutdown_timeout) - .context(SetTerminationGracePeriodSnafu)?; - } + pod_builder + .termination_grace_period(graceful_shutdown_timeout) + .context(SetTerminationGracePeriodSnafu)?; Ok(()) } diff --git a/rust/operator-binary/src/controller/build/jvm.rs b/rust/operator-binary/src/controller/build/jvm.rs index 0a26c38cc..9bd4b8d14 100644 --- a/rust/operator-binary/src/controller/build/jvm.rs +++ b/rust/operator-binary/src/controller/build/jvm.rs @@ -1,11 +1,12 @@ use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ + commons::resources::{NoRuntimeLimits, Resources}, memory::{BinaryMultiple, MemoryQuantity}, v2::jvm_argument_overrides::JvmArgumentOverrides, }; use super::properties::ConfigFileName; -use crate::crd::{METRICS_PORT, STACKABLE_CONFIG_DIR, role::AnyConfig}; +use crate::crd::{METRICS_PORT, STACKABLE_CONFIG_DIR, role::commons::Storage}; const JAVA_HEAP_FACTOR: f32 = 0.8; @@ -23,12 +24,11 @@ pub enum Error { /// All JVM arguments: operator-generated base args with the already-merged /// (role <- role group) `jvmArgumentOverrides` applied on top. fn construct_jvm_args( - merged_config: &AnyConfig, + resources: &Resources, jvm_argument_overrides: &JvmArgumentOverrides, ) -> Result, Error> { let heap_size = MemoryQuantity::try_from( - merged_config - .resources() + resources .memory .limit .as_ref() @@ -58,10 +58,10 @@ fn construct_jvm_args( /// Arguments that go into `EXTRA_ARGS` (everything except heap settings). pub fn construct_non_heap_jvm_args( - merged_config: &AnyConfig, + resources: &Resources, jvm_argument_overrides: &JvmArgumentOverrides, ) -> Result { - let mut jvm_args = construct_jvm_args(merged_config, jvm_argument_overrides)?; + let mut jvm_args = construct_jvm_args(resources, jvm_argument_overrides)?; jvm_args.retain(|arg| !is_heap_jvm_argument(arg)); Ok(jvm_args.join(" ")) @@ -69,10 +69,10 @@ pub fn construct_non_heap_jvm_args( /// Arguments that go into `KAFKA_HEAP_OPTS` (only the heap settings). pub fn construct_heap_jvm_args( - merged_config: &AnyConfig, + resources: &Resources, jvm_argument_overrides: &JvmArgumentOverrides, ) -> Result { - let mut jvm_args = construct_jvm_args(merged_config, jvm_argument_overrides)?; + let mut jvm_args = construct_jvm_args(resources, jvm_argument_overrides)?; jvm_args.retain(|arg| is_heap_jvm_argument(arg)); Ok(jvm_args.join(" ")) @@ -87,23 +87,18 @@ fn is_heap_jvm_argument(jvm_argument: &str) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::{ - controller::test_support::{minimal_kafka, validated_cluster}, - crd::role::KafkaRole, + use crate::controller::test_support::{ + broker_role_group_config, minimal_kafka, validated_cluster, }; - /// Pulls the broker `default` role group's merged config + merged JVM overrides out of + /// Pulls the broker `default` role group's merged resources + merged JVM overrides out of /// a validated cluster built from the given YAML. - fn broker_default(yaml: &str) -> (AnyConfig, JvmArgumentOverrides) { + fn broker_default(yaml: &str) -> (Resources, JvmArgumentOverrides) { let kafka = minimal_kafka(yaml); let validated = validated_cluster(&kafka); - let rg = validated - .role_group_configs - .get(&KafkaRole::Broker) - .and_then(|groups| groups.get(&"default".parse().unwrap())) - .expect("broker default role group should exist"); + let rg = broker_role_group_config(&validated, "default"); ( - rg.config.config.clone(), + rg.config.resources.clone(), rg.product_specific_common_config .jvm_argument_overrides .clone(), @@ -129,9 +124,9 @@ mod tests { default: replicas: 1 "#; - let (merged_config, jvm) = broker_default(input); - let non_heap_jvm_args = construct_non_heap_jvm_args(&merged_config, &jvm).unwrap(); - let heap_jvm_args = construct_heap_jvm_args(&merged_config, &jvm).unwrap(); + let (resources, jvm) = broker_default(input); + let non_heap_jvm_args = construct_non_heap_jvm_args(&resources, &jvm).unwrap(); + let heap_jvm_args = construct_heap_jvm_args(&resources, &jvm).unwrap(); assert_eq!( non_heap_jvm_args, @@ -177,9 +172,9 @@ mod tests { - -Xmx40000m - -Dhttps.proxyPort=1234 "#; - let (merged_config, jvm) = broker_default(input); - let non_heap_jvm_args = construct_non_heap_jvm_args(&merged_config, &jvm).unwrap(); - let heap_jvm_args = construct_heap_jvm_args(&merged_config, &jvm).unwrap(); + let (resources, jvm) = broker_default(input); + let non_heap_jvm_args = construct_non_heap_jvm_args(&resources, &jvm).unwrap(); + let heap_jvm_args = construct_heap_jvm_args(&resources, &jvm).unwrap(); assert_eq!( non_heap_jvm_args, diff --git a/rust/operator-binary/src/controller/build/kerberos.rs b/rust/operator-binary/src/controller/build/kerberos.rs index 0335a4fd8..0f7af858f 100644 --- a/rust/operator-binary/src/controller/build/kerberos.rs +++ b/rust/operator-binary/src/controller/build/kerberos.rs @@ -2,19 +2,16 @@ use std::str::FromStr; use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::{ - self, - pod::{ - PodBuilder, - container::ContainerBuilder, - volume::{ - SecretOperatorVolumeSourceBuilder, SecretOperatorVolumeSourceBuilderError, - VolumeBuilder, - }, + builder::pod::{ + container::ContainerBuilder, + volume::{ + SecretOperatorVolumeSourceBuilder, SecretOperatorVolumeSourceBuilderError, + VolumeBuilder, }, }, commons::secret_class::SecretClassVolumeProvisionParts, constant, + k8s_openapi::api::core::v1::Volume, v2::{ builder::pod::container::{EnvVarName, EnvVarSet}, types::kubernetes::VolumeName, @@ -37,64 +34,94 @@ pub enum Error { KerberosSecretVolume { source: SecretOperatorVolumeSourceBuilderError, }, +} - #[snafu(display("failed to add needed volume"))] - AddVolume { source: builder::pod::Error }, +/// Mounts the Kerberos keytab and `krb5.conf` volume into a broker's `kafka` container and +/// returns the pod volume, when Kerberos is enabled; `None` otherwise. +/// +/// Brokers are exposed through listener-operator `Listener` volumes (the broker and bootstrap +/// listeners), so the keytab's principals must cover both. +/// +/// # Panics +/// +/// Panics if the volume mount cannot be added to the container builder. Only call this on a +/// container builder whose mount paths are still distinct from the ones added here. +pub fn mount_broker_kerberos_volume( + kafka_security: &ValidatedKafkaSecurity, + cb_kafka: &mut ContainerBuilder, +) -> Result, Error> { + let Some(kerberos_secret_class) = kafka_security.kerberos_secret_class() else { + return Ok(None); + }; + let mut keytab = keytab_volume_source(&kerberos_secret_class); + keytab + .with_listener_volume_scope(&*LISTENER_BROKER_VOLUME_NAME) + .with_listener_volume_scope(&*LISTENER_BOOTSTRAP_VOLUME_NAME); + mount_keytab_volume( + &mut keytab, + KafkaRole::Broker.kerberos_service_name(), + cb_kafka, + ) + .map(Some) } -/// Adds the Kerberos keytab and `krb5.conf` volume to the pod builder and mounts it into the -/// Kafka and kcat-prober containers, when Kerberos is enabled. +/// Mounts the Kerberos keytab and `krb5.conf` volume into a controller's `kafka` container and +/// returns the pod volume, when Kerberos is enabled; `None` otherwise. +/// +/// KRaft controllers have no listener-operator `Listener` volume: they are only reachable through +/// their own StatefulSet pod DNS name, so the keytab must be pod-scoped, matching how the +/// controller's internal TLS certificate is provisioned in +/// [`mount_controller_tls_volume`](super::security::mount_controller_tls_volume). /// /// # Panics /// -/// Panics if the volume mounts cannot be added to the container builders. Only call this on -/// container builders whose mount paths are still distinct from the ones added here. -pub fn add_kerberos_pod_config( +/// Panics if the volume mount cannot be added to the container builder. Only call this on a +/// container builder whose mount paths are still distinct from the ones added here. +pub fn mount_controller_kerberos_volume( kafka_security: &ValidatedKafkaSecurity, - role: &KafkaRole, cb_kafka: &mut ContainerBuilder, - pb: &mut PodBuilder, -) -> Result<(), Error> { - if let Some(kerberos_secret_class) = kafka_security.kerberos_secret_class() { - // Mount keytab - let mut volume_builder = SecretOperatorVolumeSourceBuilder::new( - kerberos_secret_class, - // We need both public (krb5.conf) and private (keytab) parts. - SecretClassVolumeProvisionParts::PublicPrivate, - ); - match role { - // Brokers are exposed through listener-operator `Listener` volumes (the broker - // and bootstrap listeners), so the keytab principal must cover both. - KafkaRole::Broker => { - volume_builder - .with_listener_volume_scope(&*LISTENER_BROKER_VOLUME_NAME) - .with_listener_volume_scope(&*LISTENER_BOOTSTRAP_VOLUME_NAME); - } - // KRaft controllers have no listener-operator `Listener` volume: they are only - // reachable through their own StatefulSet pod DNS name, so the keytab must be - // pod-scoped, matching how the controller's internal TLS cert is provisioned in - // `add_controller_volume_and_volume_mounts`. - KafkaRole::Controller => { - volume_builder.with_pod_scope(); - } - } - let kerberos_secret_operator_volume = volume_builder - .with_kerberos_service_name(role.kerberos_service_name()) - .build() - .context(KerberosSecretVolumeSnafu)?; - pb.add_volume( - VolumeBuilder::new(&*KERBEROS_VOLUME_NAME) - .ephemeral(kerberos_secret_operator_volume) - .build(), - ) - .context(AddVolumeSnafu)?; +) -> Result, Error> { + let Some(kerberos_secret_class) = kafka_security.kerberos_secret_class() else { + return Ok(None); + }; + let mut keytab = keytab_volume_source(&kerberos_secret_class); + keytab.with_pod_scope(); + mount_keytab_volume( + &mut keytab, + KafkaRole::Controller.kerberos_service_name(), + cb_kafka, + ) + .map(Some) +} - cb_kafka - .add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR) - .expect("The mount paths are statically defined and there should be no duplicates."); - } +/// The secret-operator volume source for the keytab and `krb5.conf`, before it is scoped to the +/// addresses the role's principals must cover. +fn keytab_volume_source(kerberos_secret_class: &str) -> SecretOperatorVolumeSourceBuilder { + SecretOperatorVolumeSourceBuilder::new( + kerberos_secret_class, + // We need both public (krb5.conf) and private (keytab) parts. + SecretClassVolumeProvisionParts::PublicPrivate, + ) +} + +/// Builds the scoped `keytab` volume source into the pod volume and mounts it into the `kafka` +/// container. +fn mount_keytab_volume( + keytab: &mut SecretOperatorVolumeSourceBuilder, + kerberos_service_name: &str, + cb_kafka: &mut ContainerBuilder, +) -> Result { + let kerberos_secret_operator_volume = keytab + .with_kerberos_service_name(kerberos_service_name) + .build() + .context(KerberosSecretVolumeSnafu)?; + cb_kafka + .add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR) + .expect("The mount paths are statically defined and there should be no duplicates."); - Ok(()) + Ok(VolumeBuilder::new(&*KERBEROS_VOLUME_NAME) + .ephemeral(kerberos_secret_operator_volume) + .build()) } constant!(pub KRB5_CONFIG: EnvVarName = "KRB5_CONFIG"); diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 002182910..33775c002 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -3,51 +3,63 @@ use std::marker::PhantomData; use snafu::{ResultExt, Snafu}; -use stackable_operator::{kvp::Labels, v2::kvp::label}; +use stackable_operator::{ + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Service}, + policy::v1::PodDisruptionBudget, + }, + kvp::Labels, + v2::kvp::label, +}; +use strum::IntoEnumIterator; use crate::{ controller::{ CONTROLLER_NAME, KubernetesResources, OPERATOR_NAME, PRODUCT_NAME, Prepared, RoleGroupName, RoleName, ValidatedCluster, build::{ - properties::{ - listener::get_kafka_listener_config, product_logging::vector_config_file_content, - }, resource::{ - config_map::build_rolegroup_config_map, discovery::build_discovery_configmap, - listener::build_broker_rolegroup_bootstrap_listener, pdb::build_pdb, rbac::{build_role_binding, build_service_account}, - service::{build_rolegroup_headless_service, build_rolegroup_metrics_service}, - statefulset::{ - build_broker_rolegroup_statefulset, build_controller_rolegroup_statefulset, - }, }, + role_group::RoleGroupBuilder, }, }, - crd::role::{AnyConfig, KafkaRole}, + crd::role::KafkaRole, }; pub mod command; +pub mod container; pub mod graceful_shutdown; pub mod jvm; pub mod kerberos; pub mod properties; pub mod resource; +pub mod role_group; pub mod security; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build ConfigMap for role group {role_group}"))] - ConfigMap { - source: resource::config_map::Error, + #[snafu(display("failed to build broker role group {role_group}"))] + Broker { + source: role_group::broker::Error, + role_group: RoleGroupName, + }, + + #[snafu(display("failed to build controller role group {role_group}"))] + Controller { + source: role_group::controller::Error, role_group: RoleGroupName, }, - #[snafu(display("failed to build StatefulSet for role group {role_group}"))] - StatefulSet { - source: resource::statefulset::Error, + #[snafu(display( + "failed to build the Kubernetes objects of role {role} role group {role_group}" + ))] + RoleGroup { + source: role_group::Error, + role: KafkaRole, role_group: RoleGroupName, }, @@ -55,98 +67,80 @@ pub enum Error { DiscoveryConfigMap { source: resource::discovery::Error }, } +/// The resources of every role group, accumulated one role group at a time by [`build`]. +#[derive(Default)] +struct RoleGroupResources { + services: Vec, + config_maps: Vec, + stateful_sets: Vec, +} + +impl RoleGroupResources { + /// Builds the Services, ConfigMap and StatefulSet of one role group and adds them to the + /// collections. + fn add(&mut self, builder: &RoleGroupBuilder) -> Result<(), role_group::Error> { + self.services.extend(builder.build_services()); + self.config_maps.push(builder.build_config_map()?); + self.stateful_sets.push(builder.build_statefulset()?); + Ok(()) + } +} + /// Builds every Kubernetes resource for the given validated cluster. /// /// Does not need a Kubernetes client: every external reference is already dereferenced and /// validated by this point, so the only errors are resource-assembly failures. /// +/// Each loop hands its role group's typed config to that role's builder, which is where +/// everything specific to the role lives. The loops run in [`KafkaRole`] variant order, so each +/// collection lists the brokers' resources before the controllers'. +/// /// This includes the discovery `ConfigMap`, built from the bootstrap `Listener`s fetched in the /// dereference step; see /// [`build_discovery_configmap`] for how its /// content depends on their ingress addresses. pub fn build(cluster: &ValidatedCluster) -> Result, Error> { - let mut stateful_sets = vec![]; - let mut services = vec![]; + let mut built = RoleGroupResources::default(); let mut listeners = vec![]; - let mut config_maps = vec![]; - let mut pod_disruption_budgets = vec![]; - - for (role, role_group_configs) in &cluster.role_group_configs { - // Kafka's `GenericRoleConfig` only carries the PodDisruptionBudget. - if let Some(role_config) = cluster.role_configs.get(role) { - pod_disruption_budgets.extend(build_pdb(&role_config.pdb, cluster, role)); - } - - for (role_group_name, validated_rg) in role_group_configs { - // The Vector agent config is the static `vector.yaml`, added to the rolegroup - // ConfigMap only when the Vector agent is enabled (resolved during validation). - let vector_config = validated_rg - .config - .logging - .vector_container - .is_some() - .then(vector_config_file_content); - - services.push(build_rolegroup_headless_service( - cluster, - role, - role_group_name, - &cluster.cluster_config.kafka_security, - )); - services.push(build_rolegroup_metrics_service( - cluster, - role, - role_group_name, - )); - - let kafka_listeners = get_kafka_listener_config( - cluster, - &cluster.cluster_config.kafka_security, - role, - role_group_name, - ); - config_maps.push( - build_rolegroup_config_map( - cluster, - role_group_name, - validated_rg, - &kafka_listeners, - vector_config, - ) - .context(ConfigMapSnafu { + for (role_group_name, rg_config) in &cluster.broker_role_group_configs { + let (builder, bootstrap_listener) = + role_group::broker::build(cluster, role_group_name, rg_config).with_context(|_| { + BrokerSnafu { role_group: role_group_name.clone(), - })?, - ); - - let stateful_set = match role { - KafkaRole::Broker => { - build_broker_rolegroup_statefulset(role, role_group_name, cluster, validated_rg) } - KafkaRole::Controller => build_controller_rolegroup_statefulset( - role, - role_group_name, - cluster, - validated_rg, - ), - } - .context(StatefulSetSnafu { + })?; + + built.add(&builder).with_context(|_| RoleGroupSnafu { + role: KafkaRole::Broker, + role_group: role_group_name.clone(), + })?; + // Only broker role groups get a bootstrap Listener. + listeners.push(bootstrap_listener); + } + + for (role_group_name, rg_config) in &cluster.controller_role_group_configs { + let builder = role_group::controller::build(cluster, role_group_name, rg_config) + .with_context(|_| ControllerSnafu { role_group: role_group_name.clone(), })?; - stateful_sets.push(stateful_set); - - // Only broker role groups get a bootstrap Listener. - if let AnyConfig::Broker(broker_config) = &validated_rg.config.config { - listeners.push(build_broker_rolegroup_bootstrap_listener( - cluster, - role, - role_group_name, - broker_config, - )); - } - } + + built.add(&builder).with_context(|_| RoleGroupSnafu { + role: KafkaRole::Controller, + role_group: role_group_name.clone(), + })?; } + let pod_disruption_budgets: Vec = KafkaRole::iter() + .filter_map(|role| build_pdb(cluster, &role)) + .collect(); + + let RoleGroupResources { + services, + mut config_maps, + stateful_sets, + } = built; + config_maps.push(build_discovery_configmap(cluster).context(DiscoveryConfigMapSnafu)?); Ok(KubernetesResources { @@ -227,18 +221,40 @@ pub(crate) fn role_group_selector( #[cfg(test)] mod tests { - use stackable_operator::kube::Resource; - - use super::{build, security::STACKABLE_TLS_KAFKA_INTERNAL_DIR}; + use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + crd::authentication::{core, kerberos}, + k8s_openapi::api::{apps::v1::StatefulSetSpec, core::v1::PodSpec}, + kube::Resource, + v2::role_group_utils::ResourceNames, + }; + use strum::IntoEnumIterator; + + use super::{ + build, + kerberos::KERBEROS_VOLUME_NAME, + properties::ConfigFileName, + role_group::controller::QUORUM_MANAGER_CONTAINER_NAME, + security::{ + OPA_TLS_VOLUME_NAME, STACKABLE_TLS_KAFKA_INTERNAL_DIR, + STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME, + STACKABLE_TLS_KCAT_VOLUME_NAME, + }, + }; use crate::{ controller::{ - ValidatedCluster, + KubernetesResources, Prepared, ValidatedCluster, test_support::{ bootstrap_listener, ingress_address, minimal_kafka, validated_cluster, - zookeeper_mode_cluster, + validated_cluster_with_security, zookeeper_mode_cluster, }, }, - crd::{STACKABLE_CONFIG_DIR, STACKABLE_DATA_DIR}, + crd::{ + BROKER_ID_POD_MAP_DIR_NAME, LISTENER_BOOTSTRAP_VOLUME_NAME, + LISTENER_BROKER_VOLUME_NAME, STACKABLE_CONFIG_DIR, STACKABLE_CONFIG_DIR_NAME, + STACKABLE_DATA_DIR, STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_DIR_NAME, + authorization::KafkaAuthorizationConfig, role::KafkaRole, + }, }; /// Sorted `metadata.name`s of the given resources, for order-independent assertions. @@ -251,6 +267,77 @@ mod tests { names } + /// The resource names of the role's `default` role group (the one role group of + /// [`kraft_mode_cluster`]), as the builder derives them. + fn default_resource_names(cluster: &ValidatedCluster, role: &KafkaRole) -> ResourceNames { + cluster.role_group_resource_names(role, &"default".parse().expect("valid role group name")) + } + + fn stateful_set_spec<'a>( + resources: &'a KubernetesResources, + cluster: &ValidatedCluster, + role: &KafkaRole, + ) -> &'a StatefulSetSpec { + let name = default_resource_names(cluster, role) + .stateful_set_name() + .to_string(); + resources + .stateful_sets + .iter() + .find(|sts| sts.metadata.name.as_deref() == Some(name.as_str())) + .and_then(|sts| sts.spec.as_ref()) + .expect("the role group's StatefulSet is built with a spec") + } + + fn pod_spec(spec: &StatefulSetSpec) -> &PodSpec { + spec.template + .spec + .as_ref() + .expect("the pod template has a spec") + } + + fn config_map_keys( + resources: &KubernetesResources, + cluster: &ValidatedCluster, + role: &KafkaRole, + ) -> Vec { + let name = default_resource_names(cluster, role) + .role_group_config_map() + .to_string(); + resources + .config_maps + .iter() + .find(|cm| cm.metadata.name.as_deref() == Some(name.as_str())) + .and_then(|cm| cm.data.as_ref()) + .expect("the role group's ConfigMap is built with data") + .keys() + .cloned() + .collect() + } + + fn has_claim_template(spec: &StatefulSetSpec, name: &str) -> bool { + spec.volume_claim_templates + .iter() + .flatten() + .any(|pvc| pvc.metadata.name.as_deref() == Some(name)) + } + + fn has_pod_volume(spec: &StatefulSetSpec, name: &str) -> bool { + pod_spec(spec) + .volumes + .iter() + .flatten() + .any(|volume| volume.name == name) + } + + fn container_names(spec: &StatefulSetSpec) -> Vec<&str> { + pod_spec(spec) + .containers + .iter() + .map(|container| container.name.as_str()) + .collect() + } + /// A KRaft cluster with one `broker` and one `controller` role group, resolved through the real /// validate step (mirroring the other build fixtures), since [`ValidatedCluster`] carries /// several resolved types that are impractical to construct by hand. @@ -281,6 +368,90 @@ mod tests { validated_cluster(&kafka) } + /// A KRaft cluster that takes every role-specific build path at once: Kerberos with server + /// and internal TLS, OPA with a TLS SecretClass, the Vector agent, a custom log ConfigMap, + /// a broker-id ConfigMap, config and env overrides for both roles, and two broker role groups. + fn rich_kraft_mode_cluster() -> ValidatedCluster { + let kafka = minimal_kafka( + r#" + apiVersion: kafka.stackable.tech/v1alpha1 + kind: KafkaCluster + metadata: + name: simple-kafka + namespace: default + uid: 12345678-1234-1234-1234-123456789012 + spec: + image: + productVersion: 3.9.2 + clusterConfig: + metadataManager: kraft + authentication: + - authenticationClass: kerberos-auth + authorization: + opa: + configMapName: opa + package: kafka + tls: + serverSecretClass: tls + vectorAggregatorConfigMapName: vector-aggregator-discovery + brokerIdPodConfigMapName: broker-ids + controllers: + config: + logging: + enableVectorAgent: true + configOverrides: + controller.properties: + controller.quorum.election.backoff.max.ms: "2000" + security.properties: + networkaddress.cache.ttl: "10" + envOverrides: + CONTROLLER_ENV: controller + roleGroups: + default: + replicas: 3 + brokers: + config: + logging: + enableVectorAgent: true + containers: + kafka: + custom: + configMap: kafka-log-config + configOverrides: + broker.properties: + compression.type: snappy + security.properties: + networkaddress.cache.ttl: "20" + envOverrides: + BROKER_ENV: broker + roleGroups: + default: + replicas: 3 + secondary: + replicas: 2 + "#, + ); + let kerberos_auth_class = core::v1alpha1::AuthenticationClass { + metadata: ObjectMetaBuilder::new().name("kerberos-auth").build(), + spec: core::v1alpha1::AuthenticationClassSpec { + provider: core::v1alpha1::AuthenticationClassProvider::Kerberos( + kerberos::v1alpha1::AuthenticationProvider { + kerberos_secret_class: "kerberos".to_owned(), + }, + ), + }, + }; + let authorization_config = KafkaAuthorizationConfig { + opa_connect: "http://opa.default.svc.cluster.local:8081/v1/data/kafka".to_owned(), + secret_class: Some("opa-tls".parse().expect("valid SecretClass name")), + }; + validated_cluster_with_security( + &kafka, + vec![kerberos_auth_class], + Some(authorization_config), + ) + } + #[test] fn build_succeeds_when_every_kraft_role_group_is_scaled_to_zero() { let kafka = minimal_kafka( @@ -400,24 +571,16 @@ mod tests { let cluster = kraft_mode_cluster(); let resources = build(&cluster).expect("build succeeds"); - let controller_sts = resources - .stateful_sets - .iter() - .find(|sts| sts.metadata.name.as_deref() == Some("simple-kafka-controller-default")) - .expect("the controller StatefulSet should be built"); - let pod_spec = controller_sts - .spec - .as_ref() - .expect("the StatefulSet should have a spec") - .template - .spec - .as_ref() - .expect("the pod template should have a spec"); - let quorum_manager = pod_spec - .containers - .iter() - .find(|c| c.name == "quorum-manager") - .expect("the controller pod should have a quorum-manager sidecar"); + let quorum_manager_name = QUORUM_MANAGER_CONTAINER_NAME.to_string(); + let quorum_manager = pod_spec(stateful_set_spec( + &resources, + &cluster, + &KafkaRole::Controller, + )) + .containers + .iter() + .find(|c| c.name == quorum_manager_name) + .expect("the controller pod should have a quorum-manager sidecar"); let mount_paths: Vec<&str> = quorum_manager .volume_mounts @@ -445,6 +608,109 @@ mod tests { ); } + /// Each role gets exactly its own properties file, and only controllers get the admin-client + /// file their quorum-manager sidecar reads. + #[test] + fn each_role_gets_only_its_own_properties_files() { + let cluster = kraft_mode_cluster(); + let resources = build(&cluster).expect("build succeeds"); + for role in KafkaRole::iter() { + let keys = config_map_keys(&resources, &cluster, &role); + let is_controller = matches!(role, KafkaRole::Controller); + for (file, expected) in [ + (ConfigFileName::BrokerProperties, !is_controller), + (ConfigFileName::ControllerProperties, is_controller), + (ConfigFileName::AdminClient, is_controller), + ] { + assert_eq!( + keys.contains(&file.to_string()), + expected, + "{role:?} {file}" + ); + } + } + } + + /// Brokers get the bootstrap listener as a claim template and the broker listener as an + /// ephemeral pod volume; controllers have neither. + #[test] + fn listeners_are_broker_only() { + let cluster = kraft_mode_cluster(); + let resources = build(&cluster).expect("build succeeds"); + for role in KafkaRole::iter() { + let spec = stateful_set_spec(&resources, &cluster, &role); + let is_broker = matches!(role, KafkaRole::Broker); + assert_eq!( + has_claim_template(spec, &LISTENER_BOOTSTRAP_VOLUME_NAME.to_string()), + is_broker, + "{role:?} bootstrap listener claim template" + ); + assert_eq!( + has_pod_volume(spec, &LISTENER_BROKER_VOLUME_NAME.to_string()), + is_broker, + "{role:?} broker listener volume" + ); + } + } + + /// Only controllers run the quorum-manager sidecar. + #[test] + fn only_controllers_run_the_quorum_manager() { + let cluster = kraft_mode_cluster(); + let resources = build(&cluster).expect("build succeeds"); + let quorum_manager_name = QUORUM_MANAGER_CONTAINER_NAME.to_string(); + for role in KafkaRole::iter() { + assert_eq!( + container_names(stateful_set_spec(&resources, &cluster, &role)) + .contains(&quorum_manager_name.as_str()), + matches!(role, KafkaRole::Controller), + "{role:?} quorum-manager" + ); + } + } + + /// A pod's `volumes` are an ordered list: reordering them changes the pod template of every + /// running StatefulSet and so restarts its pods. Pinned on the fixture that has every + /// optional volume, the role's own volumes first and the ones every role has last. + #[test] + fn pod_volumes_keep_their_order() { + let cluster = rich_kraft_mode_cluster(); + let resources = build(&cluster).expect("build succeeds"); + for role in KafkaRole::iter() { + let expected: Vec = match role { + KafkaRole::Broker => vec![ + &*STACKABLE_TLS_KCAT_VOLUME_NAME, + &*STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME, + &*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, + &*OPA_TLS_VOLUME_NAME, + &*KERBEROS_VOLUME_NAME, + &*STACKABLE_LOG_CONFIG_DIR_NAME, + &*LISTENER_BROKER_VOLUME_NAME, + &*BROKER_ID_POD_MAP_DIR_NAME, + &*STACKABLE_CONFIG_DIR_NAME, + &*STACKABLE_LOG_DIR_NAME, + ], + KafkaRole::Controller => vec![ + &*KERBEROS_VOLUME_NAME, + &*STACKABLE_LOG_CONFIG_DIR_NAME, + &*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, + &*STACKABLE_CONFIG_DIR_NAME, + &*STACKABLE_LOG_DIR_NAME, + ], + } + .into_iter() + .map(ToString::to_string) + .collect(); + let actual: Vec<&str> = pod_spec(stateful_set_spec(&resources, &cluster, &role)) + .volumes + .iter() + .flatten() + .map(|volume| volume.name.as_str()) + .collect(); + assert_eq!(actual, expected, "{role:?} pod volumes"); + } + } + /// ZooKeeper mode has no `controller` role, so `build()` emits no controller resources while /// still producing the broker's bootstrap Listener. #[test] diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index e751613aa..8cd11fc51 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -8,10 +8,7 @@ pub mod security_properties; use std::collections::BTreeSet; -use crate::crd::{ - KafkaPodDescriptor, - role::{AnyConfig, KafkaRole}, -}; +use crate::crd::{KafkaPodDescriptor, role::KafkaRole}; /// The names of the config files assembled into the rolegroup `ConfigMap`. /// @@ -44,15 +41,6 @@ pub enum ConfigFileName { Log4j2, } -/// The config file name for a role group, derived from its role (`broker.properties` for brokers, -/// `controller.properties` for controllers). -pub fn config_file_name(config: &AnyConfig) -> ConfigFileName { - match config { - AnyConfig::Broker(_) => ConfigFileName::BrokerProperties, - AnyConfig::Controller(_) => ConfigFileName::ControllerProperties, - } -} - /// Whether the given Kafka version uses the legacy log4j logging framework. /// /// Kafka 3.x uses log4j ([`ConfigFileName::Log4j`]); Kafka 4.0 and later use log4j2 diff --git a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs index 8375f0e2c..7e428dd0a 100644 --- a/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/product_logging/mod.rs @@ -1,19 +1,15 @@ //! Renders the logging config files (`log4j.properties` / `log4j2.properties` and the //! Vector agent config) assembled into the rolegroup `ConfigMap`. -use std::{borrow::Cow, collections::BTreeMap, fmt::Display}; +use std::{collections::BTreeMap, fmt::Display}; use stackable_operator::{ memory::{BinaryMultiple, MemoryQuantity}, - product_logging::{ - self, - spec::{ContainerLogConfig, ContainerLogConfigChoice}, - }, - v2::product_logging::framework::STACKABLE_LOG_DIR, + product_logging, + v2::product_logging::framework::{STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice}, }; use super::ConfigFileName; -use crate::crd::role::{AnyConfig, broker::BrokerContainer, controller::ControllerContainer}; /// The maximum size of a single Kafka log file before it is rotated. pub const MAX_KAFKA_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity { @@ -37,17 +33,14 @@ const CONSOLE_CONVERSION_PATTERN_LOG4J2: &str = "%d{ISO8601} %p [%t] %c - %m%n"; /// Get the role group ConfigMap data with the log4j/log4j2 logging configuration. /// -/// The Vector agent config (`vector.yaml`) is provided separately via -/// [`vector_config_file_content`] and added by the caller. +/// `container_name` and `kafka_logging` are the role's `kafka` container and its validated log +/// config; a custom log config `ConfigMap` gets no entry. The Vector agent config (`vector.yaml`) +/// is provided separately via [`vector_config_file_content`] and added by the caller. pub fn role_group_config_map_data( product_version: &str, - merged_config: &AnyConfig, + container_name: &str, + kafka_logging: &ValidatedContainerLogConfigChoice, ) -> BTreeMap> { - let container_name = match merged_config { - AnyConfig::Broker(_) => BrokerContainer::Kafka.to_string(), - AnyConfig::Controller(_) => ControllerContainer::Kafka.to_string(), - }; - let mut configs: BTreeMap> = BTreeMap::new(); // Starting with Kafka 4.0, log4j2 is used instead of log4j. @@ -56,7 +49,7 @@ pub fn role_group_config_map_data( configs.insert( ConfigFileName::Log4j.to_string(), log4j_config_if_automatic( - Some(merged_config.kafka_logging()), + kafka_logging, container_name, KAFKA_LOG4J_FILE, MAX_KAFKA_LOG_FILES_SIZE, @@ -67,7 +60,7 @@ pub fn role_group_config_map_data( configs.insert( ConfigFileName::Log4j2.to_string(), log4j2_config_if_automatic( - Some(merged_config.kafka_logging()), + kafka_logging, container_name, KAFKA_LOG4J2_FILE, MAX_KAFKA_LOG_FILES_SIZE, @@ -80,15 +73,12 @@ pub fn role_group_config_map_data( } fn log4j_config_if_automatic( - log_config: Option>, + log_config: &ValidatedContainerLogConfigChoice, container_name: impl Display, log_file: &str, max_log_file_size: MemoryQuantity, ) -> Option { - if let Some(ContainerLogConfig { - choice: Some(ContainerLogConfigChoice::Automatic(log_config)), - }) = log_config.as_deref() - { + if let ValidatedContainerLogConfigChoice::Automatic(log_config) = log_config { Some(product_logging::framework::create_log4j_config( &format!("{STACKABLE_LOG_DIR}/{container_name}"), log_file, @@ -105,15 +95,12 @@ fn log4j_config_if_automatic( } fn log4j2_config_if_automatic( - log_config: Option>, + log_config: &ValidatedContainerLogConfigChoice, container_name: impl Display, log_file: &str, max_log_file_size: MemoryQuantity, ) -> Option { - if let Some(ContainerLogConfig { - choice: Some(ContainerLogConfigChoice::Automatic(log_config)), - }) = log_config.as_deref() - { + if let ValidatedContainerLogConfigChoice::Automatic(log_config) = log_config { Some(product_logging::framework::create_log4j2_config( &format!("{STACKABLE_LOG_DIR}/{container_name}",), log_file, 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 8d8057e65..0a61fc072 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -1,4 +1,5 @@ -use indoc::formatdoc; +use std::collections::BTreeMap; + use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder}, @@ -7,303 +8,124 @@ use stackable_operator::{ v2::{ builder::meta::ownerreference_from_resource, config_file_writer::{PropertiesWriterError, to_java_properties_string}, + types::kubernetes::ContainerName, }, }; -use crate::{ - controller::{ - RoleGroupName, ValidatedCluster, ValidatedRoleGroupConfig, - build::{ - properties::{ - ConfigFileName, config_file_name, product_logging::role_group_config_map_data, - }, - recommended_labels_for_role_group_resources, - security::{client_properties, controller_admin_client_properties}, +use crate::controller::{ + ValidatedCluster, + build::{ + properties::{ + ConfigFileName, + product_logging::{role_group_config_map_data, vector_config_file_content}, }, + recommended_labels_for_role_group_resources, + role_group::RoleGroupBuilder, + security::client_properties, }, - crd::{ - CONTROLLER_POD_FQDN_TEMPLATE, STACKABLE_KERBEROS_KEYTAB_PATH, - STACKABLE_LISTENER_BOOTSTRAP_DIR, STACKABLE_LISTENER_BROKER_DIR, - listener::{KafkaListenerConfig, node_address_cmd}, - role::{AnyConfig, KafkaRole}, - }, + validate::ValidatedLogging, }; #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to build ConfigMap for role group {role_group}"))] + #[snafu(display("failed to build ConfigMap"))] BuildRoleGroupConfig { source: stackable_operator::builder::configmap::Error, - role_group: RoleGroupName, }, - #[snafu(display( - "failed to serialize [{}] for role group {role_group}", - ConfigFileName::Security - ))] - JvmSecurityProperties { - source: PropertiesWriterError, - role_group: RoleGroupName, - }, - - #[snafu(display("failed to serialize config for role group {role_group}"))] - SerializeConfig { - source: PropertiesWriterError, - role_group: RoleGroupName, - }, + #[snafu(display("failed to serialize [{}]", ConfigFileName::Security))] + JvmSecurityProperties { source: PropertiesWriterError }, - #[snafu(display( - "failed to serialize client-side connection properties ([{}] or [{}]) for role group {role_group}", - ConfigFileName::Client, - ConfigFileName::AdminClient - ))] - ClientProperties { - source: PropertiesWriterError, - role_group: RoleGroupName, - }, - - #[snafu(display("failed to build pod descriptors"))] - BuildPodDescriptors { - source: crate::controller::PodDescriptorsError, - }, + #[snafu(display("failed to serialize [{}]", ConfigFileName::Client))] + ClientProperties { source: PropertiesWriterError }, } -/// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator. -/// -/// `vector_config` is the static Vector agent config (`vector.yaml`) added by the caller; it is -/// `None` when the Vector agent is disabled. Resource naming and labels use the role (derived -/// from `validated_rg.config`) and the typed `role_group_name`. -pub fn build_rolegroup_config_map( +/// The rolegroup [`ConfigMap`] entries every role has: `security.properties` with the role +/// group's `security_overrides`, `client.properties`, the log config file of the role's `kafka` +/// container (named `kafka_container_name`) unless it uses a custom log config, and `vector.yaml` +/// if the Vector agent is enabled. +pub(crate) fn common_config_map_data( validated_cluster: &ValidatedCluster, - role_group_name: &RoleGroupName, - validated_rg: &ValidatedRoleGroupConfig, - listener_config: &KafkaListenerConfig, - vector_config: Option, -) -> Result { - let role = validated_rg.config.config.kafka_role(); - let cluster_config = &validated_cluster.cluster_config; - let kafka_security = &cluster_config.kafka_security; - let resolved_product_image = &validated_cluster.image; - let kafka_config_file_name = config_file_name(&validated_rg.config.config).to_string(); - let config_overrides = validated_rg - .config_overrides - .config_file_overrides() - .overrides - .clone(); - - let pod_descriptors = validated_cluster - .pod_descriptors(None) - .context(BuildPodDescriptorsSnafu)?; - - let kafka_config = match &validated_rg.config.config { - AnyConfig::Broker(_) => crate::controller::build::properties::broker_properties::build( - cluster_config, - listener_config, - &pod_descriptors, - config_overrides, - ), - AnyConfig::Controller(_) => { - crate::controller::build::properties::controller_properties::build( - cluster_config, - listener_config, - &pod_descriptors, - config_overrides, - ) - } - }; + kafka_container_name: &ContainerName, + logging: &ValidatedLogging, + security_overrides: BTreeMap, +) -> Result, Error> { + let kafka_security = &validated_cluster.cluster_config.kafka_security; // The `networkaddress.cache.*` defaults are always emitted (with user `security.properties` // overrides winning); see `security_properties::build`. - let jvm_sec_props = crate::controller::build::properties::security_properties::build( - validated_rg - .config_overrides - .security_properties() - .overrides - .clone(), - ); + let jvm_sec_props = + crate::controller::build::properties::security_properties::build(security_overrides); - let mut cm_builder = ConfigMapBuilder::new(); - cm_builder - .metadata( - ObjectMetaBuilder::new() - .name_and_namespace(validated_cluster) - .name( - validated_cluster - .role_group_resource_names(&role, role_group_name) - .role_group_config_map() - .to_string(), - ) - .ownerreference(ownerreference_from_resource( - validated_cluster, - None, - Some(true), - )) - .with_labels(recommended_labels_for_role_group_resources( - validated_cluster, - &role, - role_group_name, - )) - .build(), - ) - .add_data( - kafka_config_file_name, - to_java_properties_string(kafka_config.iter()).with_context(|_| { - SerializeConfigSnafu { - role_group: role_group_name.clone(), - } - })?, - ) - .add_data( + let mut data = vec![ + ( ConfigFileName::Security.to_string(), - to_java_properties_string(jvm_sec_props.iter()).with_context(|_| { - JvmSecurityPropertiesSnafu { - role_group: role_group_name.clone(), - } - })?, - ) - .add_data( + to_java_properties_string(jvm_sec_props.iter()).context(JvmSecurityPropertiesSnafu)?, + ), + ( ConfigFileName::Client.to_string(), to_java_properties_string( client_properties(kafka_security) .iter() .filter_map(|(k, v)| v.as_ref().map(|v| (k, v))), ) - .with_context(|_| ClientPropertiesSnafu { - role_group: role_group_name.clone(), - })?, - ) - // This file contains the JAAS configuration for Kerberos authentication - // It has the ".properties" extension but is not a Java properties file. - // It is processed by `config-utils` to substitute "env:" and "file:" variables - // and this tool currently doesn't support the JAAS login configuration format. - .add_data( - ConfigFileName::Jaas.to_string(), - jaas_config_file(kafka_security.has_kerberos_enabled(), &role), - ); - - // `admin-client.properties` is only needed by the controller-side sidecar running - // `kafka-metadata-quorum.sh` against the CONTROLLER listener; brokers don't need it. - if let AnyConfig::Controller(_) = &validated_rg.config.config { - cm_builder.add_data( - ConfigFileName::AdminClient.to_string(), - to_java_properties_string( - controller_admin_client_properties(kafka_security) - .iter() - .filter_map(|(k, v)| v.as_ref().map(|v| (k, v))), - ) - .with_context(|_| ClientPropertiesSnafu { - role_group: role_group_name.clone(), - })?, - ); - } + .context(ClientPropertiesSnafu)?, + ), + ]; - tracing::debug!(?kafka_config, "Applied kafka config"); tracing::debug!(?jvm_sec_props, "Applied JVM config"); - let config_data = role_group_config_map_data( - &resolved_product_image.product_version, - &validated_rg.config.config, + let log_config_data = role_group_config_map_data( + &validated_cluster.image.product_version, + kafka_container_name.as_ref(), + &logging.kafka_container, ); - for (file_name, data) in config_data { - if let Some(data) = data { - cm_builder.add_data(file_name, data); + for (file_name, data_entry) in log_config_data { + if let Some(data_entry) = data_entry { + data.push((file_name, data_entry)); } } - if let Some(vector_config) = vector_config { - cm_builder.add_data(VECTOR_CONFIG_FILE, vector_config); + // The Vector agent config is the static `vector.yaml`, added only when the Vector agent is + // enabled (resolved during validation). + if logging.vector_container.is_some() { + data.push((VECTOR_CONFIG_FILE.to_string(), vector_config_file_content())); } - cm_builder - .build() - .with_context(|_| BuildRoleGroupConfigSnafu { - role_group: role_group_name.clone(), - }) + Ok(data) } -// Generate JAAS configuration file for Kerberos authentication -// or an empty string if Kerberos is not enabled. -// See https://docs.oracle.com/javase/8/docs/technotes/guides/security/jgss/tutorials/LoginConfigFile.html -fn jaas_config_file(is_kerberos_enabled: bool, role: &KafkaRole) -> String { - if !is_kerberos_enabled { - return String::new(); - } - - // Broker pods reach the CONTROLLER listener as SASL clients; the only principals in - // their keytab (see `add_kerberos_pod_config`) are for the broker and bootstrap listener - // addresses, so their CONTROLLER section must reuse the broker address. - // Controller pods have no listener-operator `Listener` volume; their keytab is - // pod-scoped, so their CONTROLLER section uses their own pod FQDN — the same expression - // already used for `KAFKA_LISTENERS` in `controller_properties.rs`. - let controller_principal_address = match role { - KafkaRole::Broker => node_address_cmd(STACKABLE_LISTENER_BROKER_DIR), - KafkaRole::Controller => CONTROLLER_POD_FQDN_TEMPLATE.to_string(), - }; - - // Unlike the bootstrap and client sections below, this context is used for BOTH sides of - // every CONTROLLER-listener connection: brokers connect out to controllers, and - // controllers connect to each other for Raft. This is the only listener in this operator - // where the process must act as a GSSAPI initiator as well as an acceptor, so - // `isInitiator` is intentionally left at its default (`true`). - let controller_section = formatdoc! {" - controller.KafkaServer {{ - com.sun.security.auth.module.Krb5LoginModule required - useKeyTab=true - storeKey=true - keyTab=\"{STACKABLE_KERBEROS_KEYTAB_PATH}\" - principal=\"kafka/{controller_principal_address}@${{env:KERBEROS_REALM}}\"; - }}; - "}; - - match role { - KafkaRole::Controller => controller_section, - KafkaRole::Broker => formatdoc! {" - bootstrap.KafkaServer {{ - com.sun.security.auth.module.Krb5LoginModule required - useKeyTab=true - storeKey=true - isInitiator=false - keyTab=\"{STACKABLE_KERBEROS_KEYTAB_PATH}\" - principal=\"kafka/{bootstrap_address}@${{env:KERBEROS_REALM}}\"; - }}; +/// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the +/// administrator; its entries are already rendered ([`RoleGroupBuilder::config_map_data`]). +pub fn build_config_map(builder: &RoleGroupBuilder) -> Result { + let validated_cluster = builder.cluster; - client.KafkaServer {{ - com.sun.security.auth.module.Krb5LoginModule required - useKeyTab=true - storeKey=true - isInitiator=false - keyTab=\"{STACKABLE_KERBEROS_KEYTAB_PATH}\" - principal=\"kafka/{broker_address}@${{env:KERBEROS_REALM}}\"; - }}; - - {controller_section} - ", - bootstrap_address = node_address_cmd(STACKABLE_LISTENER_BOOTSTRAP_DIR), - broker_address = node_address_cmd(STACKABLE_LISTENER_BROKER_DIR), - }, - } -} - -#[cfg(test)] -mod tests { - use super::jaas_config_file; - use crate::crd::role::KafkaRole; + let mut cm_builder = ConfigMapBuilder::new(); + cm_builder.metadata( + ObjectMetaBuilder::new() + .name_and_namespace(validated_cluster) + .name( + validated_cluster + .role_group_resource_names(&builder.role, &builder.role_group_name) + .role_group_config_map() + .to_string(), + ) + .ownerreference(ownerreference_from_resource( + validated_cluster, + None, + Some(true), + )) + .with_labels(recommended_labels_for_role_group_resources( + validated_cluster, + &builder.role, + &builder.role_group_name, + )) + .build(), + ); - #[test] - fn jaas_config_file_empty_without_kerberos() { - assert_eq!(jaas_config_file(false, &KafkaRole::Broker), ""); - assert_eq!(jaas_config_file(false, &KafkaRole::Controller), ""); + for (file_name, data) in &builder.config_map_data { + cm_builder.add_data(file_name, data); } - #[test] - fn jaas_config_file_renders_bootstrap_and_client_sections_with_kerberos() { - let jaas = jaas_config_file(true, &KafkaRole::Broker); - assert!(jaas.contains("bootstrap.KafkaServer")); - assert!(jaas.contains("client.KafkaServer")); - assert!(jaas.contains("Krb5LoginModule")); - assert!(jaas.contains("${env:KERBEROS_REALM}")); - // The bootstrap and client principals embed distinct listener addresses. - assert!(jaas.contains("/stackable/listener-bootstrap")); - assert!(jaas.contains("/stackable/listener-broker")); - } + cm_builder.build().context(BuildRoleGroupConfigSnafu) } diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index a7a9be6fc..060730596 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -13,10 +13,10 @@ use stackable_operator::{ use crate::{ controller::{ - RoleGroupName, ValidatedCluster, build::recommended_labels_for_role_group_resources, - security::ValidatedKafkaSecurity, + RoleGroupName, ValidatedBrokerConfig, ValidatedCluster, + build::recommended_labels_for_role_group_resources, security::ValidatedKafkaSecurity, }, - crd::role::{KafkaRole, broker::BrokerConfig}, + crd::role::KafkaRole, }; /// The name of a broker role group's bootstrap [`Listener`](listener::v1alpha1::Listener), @@ -79,7 +79,7 @@ pub fn build_broker_rolegroup_bootstrap_listener( validated_cluster: &ValidatedCluster, role: &KafkaRole, role_group_name: &RoleGroupName, - merged_config: &BrokerConfig, + merged_config: &ValidatedBrokerConfig, ) -> listener::v1alpha1::Listener { let kafka_security = &validated_cluster.cluster_config.kafka_security; diff --git a/rust/operator-binary/src/controller/build/resource/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs index af647fa63..d1c67246a 100644 --- a/rust/operator-binary/src/controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/controller/build/resource/pdb.rs @@ -1,5 +1,5 @@ use stackable_operator::{ - commons::pdb::PdbConfig, k8s_openapi::api::policy::v1::PodDisruptionBudget, + k8s_openapi::api::policy::v1::PodDisruptionBudget, v2::builder::pdb::pod_disruption_budget_builder_with_role, }; @@ -8,19 +8,27 @@ use crate::{ crd::role::KafkaRole, }; -/// Builds the [`PodDisruptionBudget`] for the given `role`, or `None` if PDBs are disabled. +/// Builds the [`PodDisruptionBudget`] for the given `role`, or `None` if the role is absent or +/// its PDB is disabled. pub fn build_pdb( - pdb: &PdbConfig, validated_cluster: &ValidatedCluster, role: &KafkaRole, ) -> Option { + // Kafka's `GenericRoleConfig` only carries the PodDisruptionBudget. + let (pdb, default_max_unavailable) = match role { + KafkaRole::Broker => ( + &validated_cluster.broker_config.pdb, + max_unavailable_brokers(), + ), + KafkaRole::Controller => ( + &validated_cluster.controller_config.as_ref()?.pdb, + max_unavailable_controllers(), + ), + }; if !pdb.enabled { return None; } - let max_unavailable = pdb.max_unavailable.unwrap_or(match role { - KafkaRole::Broker => max_unavailable_brokers(), - KafkaRole::Controller => max_unavailable_controllers(), - }); + let max_unavailable = pdb.max_unavailable.unwrap_or(default_max_unavailable); let pdb = pod_disruption_budget_builder_with_role( validated_cluster, &PRODUCT_NAME, diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 4ea3b886f..7dc85c99a 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -1,84 +1,41 @@ -use std::{ops::Deref, str::FromStr}; +use std::str::FromStr; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::{ meta::ObjectMetaBuilder, - pod::{ - PodBuilder, container::FieldPathEnvVar, resources::ResourceRequirementsBuilder, - security::PodSecurityContextBuilder, volume::VolumeBuilder, - }, + pod::{PodBuilder, security::PodSecurityContextBuilder}, }, commons::product_image_selection::ResolvedProductImage, constants::RESTART_CONTROLLER_ENABLED_LABEL, k8s_openapi::{ DeepMerge, api::{ - apps::v1::{StatefulSet, StatefulSetSpec, StatefulSetUpdateStrategy}, - core::v1::{ - ConfigMapVolumeSource, ContainerPort, EnvVar, EnvVarSource, ExecAction, - LifecycleHandler, ObjectFieldSelector, Volume, - }, + apps::v1::{StatefulSet, StatefulSetSpec}, + core::v1::{ConfigMapVolumeSource, Volume}, }, apimachinery::pkg::apis::meta::v1::LabelSelector, }, product_logging, v2::{ - builder::{ - meta::ownerreference_from_resource, - pod::{ - container::{EnvVarName, EnvVarSet, new_container_builder}, - volume::{ListenerReference, listener_operator_volume_source_builder_build_pvc}, - }, - }, - jvm_argument_overrides::JvmArgumentOverrides, - product_logging::framework::{ - STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container, - }, + builder::{meta::ownerreference_from_resource, pod::container::EnvVarSet}, + product_logging::framework::vector_container, role_group_utils::ResourceNames, - types::kubernetes::{ConfigMapKey, ContainerName, VolumeName}, + types::kubernetes::{ContainerName, VolumeName}, }, }; -use super::probes; use crate::{ controller::{ - RoleGroupName, ValidatedCluster, ValidatedRoleGroupConfig, build::{ - command::{ - KAFKA_LOG4J_OPTS, broker_kafka_container_commands, - controller_kafka_container_command, controller_remove_self_pre_stop_command, - kafka_log_opts, quorum_manager_container_command, - }, graceful_shutdown::add_graceful_shutdown_config, - kerberos::{ - KAFKA_OPTS, KERBEROS_VOLUME_NAME, KRB5_CONFIG, add_kerberos_pod_config, - kerberos_env_vars, - }, properties::product_logging::MAX_KAFKA_LOG_FILES_SIZE, - recommended_labels_for_role_group_resources, - recommended_labels_for_unversioned_role_group_resources, role_group_selector, - security::{ - STACKABLE_TLS_KAFKA_INTERNAL_DIR, STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, - add_broker_volume_and_volume_mounts, add_controller_volume_and_volume_mounts, - }, + recommended_labels_for_role_group_resources, role_group::RoleGroupBuilder, + role_group_selector, }, - node_id_hasher::node_id_hash32_offset, - security::ValidatedKafkaSecurity, validate::ValidatedLogging, }, - crd::{ - BROKER_ID_POD_MAP_DIR, BROKER_ID_POD_MAP_DIR_NAME, KAFKA_HEAP_OPTS, - LISTENER_BOOTSTRAP_VOLUME_NAME, LISTENER_BROKER_VOLUME_NAME, LOG_DIRS_VOLUME_NAME, - METRICS_PORT, METRICS_PORT_NAME, STACKABLE_CONFIG_DIR, STACKABLE_CONFIG_DIR_NAME, - STACKABLE_DATA_DIR, STACKABLE_KERBEROS_DIR, STACKABLE_KERBEROS_KRB5_PATH, - STACKABLE_LISTENER_BOOTSTRAP_DIR, STACKABLE_LISTENER_BROKER_DIR, STACKABLE_LOG_CONFIG_DIR, - STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_DIR_NAME, - role::{ - AnyConfig, KAFKA_NODE_ID_OFFSET, KafkaRole, broker::BrokerContainer, - controller::ControllerContainer, - }, - }, + crd::{STACKABLE_CONFIG_DIR_NAME, STACKABLE_LOG_DIR_NAME}, }; // The Vector container reads its `vector.yaml` from the `config` volume (the rolegroup @@ -86,499 +43,54 @@ use crate::{ stackable_operator::constant!(VECTOR_CONFIG_VOLUME_NAME: VolumeName = "config"); stackable_operator::constant!(VECTOR_LOG_VOLUME_NAME: VolumeName = "log"); -// Env vars the operator sets on the Kafka containers. -stackable_operator::constant!(POD_NAME: EnvVarName = "POD_NAME"); -stackable_operator::constant!(POD_IP: EnvVarName = "POD_IP"); -stackable_operator::constant!(KAFKA_CLIENT_PORT: EnvVarName = "KAFKA_CLIENT_PORT"); -stackable_operator::constant!(NAMESPACE: EnvVarName = "NAMESPACE"); -stackable_operator::constant!(ROLEGROUP_HEADLESS_SERVICE_NAME: EnvVarName = "ROLEGROUP_HEADLESS_SERVICE_NAME"); -stackable_operator::constant!(CLUSTER_DOMAIN: EnvVarName = "CLUSTER_DOMAIN"); -stackable_operator::constant!(EXTRA_ARGS: EnvVarName = "EXTRA_ARGS"); -// Needed for the `containerdebug` process to log its tracing information to. -stackable_operator::constant!(CONTAINERDEBUG_LOG_DIRECTORY: EnvVarName = "CONTAINERDEBUG_LOG_DIRECTORY"); - -// The env var and the ZooKeeper discovery ConfigMap key holding the ZooKeeper connection -// string (the same string, as the env var name is used as the ConfigMap key). -stackable_operator::constant!(ZOOKEEPER: EnvVarName = "ZOOKEEPER"); -stackable_operator::constant!(ZOOKEEPER_CONFIG_MAP_KEY: ConfigMapKey = "ZOOKEEPER"); - -/// Environment variables the operator sets on the Kafka container that are common to broker and -/// controller role groups. -/// -/// These form the base; the caller merges the user's `envOverrides` on top (see -/// [`build_broker_rolegroup_statefulset`] and [`build_controller_rolegroup_statefulset`]), so a -/// user override wins on a name collision. Using an [`EnvVarSet`] (a name-keyed map) makes that -/// precedence explicit and de-duplicates by name, rather than relying on append order. -fn common_operator_env_vars( - validated_cluster: &ValidatedCluster, - kafka_security: &ValidatedKafkaSecurity, -) -> EnvVarSet { - let mut env = EnvVarSet::new() - .with_field_path(&POD_NAME, &FieldPathEnvVar::Name) - .with_value(&KAFKA_CLIENT_PORT, kafka_security.client_port().to_string()); - - // Present in ZooKeeper mode only: brokers use it to connect, controllers for migration. - if let Some(zookeeper_config_map_name) = - &validated_cluster.cluster_config.zookeeper_config_map_name - { - env = env.with_config_map_key_ref( - &ZOOKEEPER, - zookeeper_config_map_name, - &ZOOKEEPER_CONFIG_MAP_KEY, - ); - } - - env -} - -/// Environment variables the operator sets that are common to the `kafka` and `quorum-manager` -/// containers. -fn controller_pod_shared_env_vars( - validated_cluster: &ValidatedCluster, - kafka_security: &ValidatedKafkaSecurity, - resource_names: &ResourceNames, -) -> EnvVarSet { - // TODO: for op-rs - // `FieldPathEnvVar` has no `status.podIP` variant, so this one is built by hand rather - // than through `with_field_path`. - let pod_ip_env_var = EnvVar { - name: POD_IP.to_string(), - value: None, - value_from: Some(EnvVarSource { - field_ref: Some(ObjectFieldSelector { - field_path: "status.podIP".to_string(), - ..ObjectFieldSelector::default() - }), - ..EnvVarSource::default() - }), - }; - - common_operator_env_vars(validated_cluster, kafka_security) - .with_field_path(&NAMESPACE, &FieldPathEnvVar::Namespace) - .with_value( - &ROLEGROUP_HEADLESS_SERVICE_NAME, - resource_names.headless_service_name().to_string(), - ) - .with_value( - &CLUSTER_DOMAIN, - validated_cluster.cluster_domain.to_string(), - ) - .with_env_var(pod_ip_env_var) - .expect("the env var name is a valid EnvVarName") -} - -const POD_MANAGEMENT_POLICY_PARALLEL: &str = "Parallel"; -const POD_MANAGEMENT_POLICY_ORDERED_READY: &str = "OrderedReady"; - #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("failed to add kerberos config"))] - AddKerberosConfig { - source: crate::controller::build::kerberos::Error, - }, - - #[snafu(display("failed to add listener volume"))] - AddListenerVolume { - source: stackable_operator::builder::pod::Error, - }, - - #[snafu(display("failed to add Secret Volumes and VolumeMounts"))] - AddVolumesAndVolumeMounts { - source: crate::controller::build::security::Error, - }, - #[snafu(display("failed to add needed volume"))] AddVolume { source: stackable_operator::builder::pod::Error, }, - #[snafu(display("failed to build pod descriptors"))] - BuildPodDescriptors { - source: crate::controller::PodDescriptorsError, - }, - - #[snafu(display("failed to build container probe"))] - BuildProbe { source: probes::Error }, - - #[snafu(display("failed to construct JVM arguments"))] - ConstructJvmArguments { - source: crate::controller::build::jvm::Error, - }, - #[snafu(display("failed to configure graceful shutdown"))] GracefulShutdown { source: crate::controller::build::graceful_shutdown::Error, }, - - #[snafu(display("missing secret lifetime"))] - MissingSecretLifetime, } -/// The broker rolegroup [`StatefulSet`] runs the rolegroup, as configured by the administrator. +/// The rolegroup [`StatefulSet`] runs the rolegroup, as configured by the administrator. +/// +/// Starts from the containers and volumes the role's module built +/// ([`RoleGroupBuilder::role_containers`], [`RoleGroupBuilder::role_pod_volumes`]) and adds what +/// every role's pod has: the pod metadata, image pull secrets and affinity, the `config` and +/// `log` volumes, the service account and security context, the Vector sidecar and the graceful +/// shutdown period. Pod overrides are merged last. /// /// The [`Pod`](`stackable_operator::k8s_openapi::api::core::v1::Pod`)s are accessible through the corresponding /// [`Service`](`stackable_operator::k8s_openapi::api::core::v1::Service`) from [`build_rolegroup_headless_service`](`crate::controller::build::resource::service::build_rolegroup_headless_service`). -pub fn build_broker_rolegroup_statefulset( - kafka_role: &KafkaRole, - role_group_name: &RoleGroupName, - validated_cluster: &ValidatedCluster, - validated_rg: &ValidatedRoleGroupConfig, -) -> Result { - let kafka_security = &validated_cluster.cluster_config.kafka_security; +pub fn build_statefulset(builder: &RoleGroupBuilder) -> Result { + let validated_cluster = builder.cluster; + let kafka_role = &builder.role; + let role_group_name = &builder.role_group_name; let resolved_product_image = &validated_cluster.image; - let merged_config = &validated_rg.config.config; let resource_names = validated_cluster.role_group_resource_names(kafka_role, role_group_name); let recommended_labels = recommended_labels_for_role_group_resources(validated_cluster, kafka_role, role_group_name); - // Used for PVC templates, which cannot be modified once they are deployed. The version label - // is omitted so the labels stay stable across version upgrades. - let unversioned_recommended_labels = recommended_labels_for_unversioned_role_group_resources( - validated_cluster, - kafka_role, - role_group_name, - ); - - let mut cb_kafka = new_container_builder(BrokerContainer::Kafka.name()); let mut pod_builder = PodBuilder::new(); - - // Add TLS related volumes and volume mounts - let requested_secret_lifetime = merged_config - .deref() - .requested_secret_lifetime - .context(MissingSecretLifetimeSnafu)?; - add_broker_volume_and_volume_mounts( - kafka_security, - &mut pod_builder, - &mut cb_kafka, - &requested_secret_lifetime, - ) - .context(AddVolumesAndVolumeMountsSnafu)?; - - let mut pvcs = merged_config.resources().storage.build_pvcs(); - - // bootstrap listener should be persistent, - // main broker listener is an ephemeral PVC instead - let bootstrap_listener_name = - validated_cluster.bootstrap_listener_name(kafka_role, role_group_name); - pvcs.push(listener_operator_volume_source_builder_build_pvc( - &ListenerReference::Listener(bootstrap_listener_name), - &unversioned_recommended_labels, - &LISTENER_BOOTSTRAP_VOLUME_NAME, - )); - - if kafka_security.has_kerberos_enabled() { - add_kerberos_pod_config(kafka_security, kafka_role, &mut cb_kafka, &mut pod_builder) - .context(AddKerberosConfigSnafu)?; - } - - // Operator-set env vars first; the user's `envOverrides` are merged on top last and win. - let env: Vec = common_operator_env_vars(validated_cluster, kafka_security) - .merge(common_kafka_env( - merged_config, - &validated_rg - .product_specific_common_config - .jvm_argument_overrides, - resolved_product_image, - kafka_role, - role_group_name, - )?) - .merge(kerberos_env_vars(kafka_security)) - .merge(validated_rg.env_overrides.clone()) - .into(); - - // The client port can accept connections before the broker has replayed its log and - // reached the JMX `RUNNING` state, so the startupProbe waits for both, giving it up to - // 5 minutes (60 * 5s) before the livenessProbe is allowed to start counting failures. - let broker_startup_probe = - probes::broker_running_probe(kafka_security.client_port(), METRICS_PORT, 5, 5, 60) - .context(BuildProbeSnafu)?; - let broker_liveness_probe = - probes::broker_running_probe(kafka_security.client_port(), METRICS_PORT, 10, 30, 20) - .context(BuildProbeSnafu)?; - let broker_readiness_probe = - probes::broker_kcat_readiness_probe(kafka_security).context(BuildProbeSnafu)?; - - cb_kafka - .image_from_product_image(resolved_product_image) - .command(vec![ - "/bin/bash".to_string(), - "-x".to_string(), - "-euo".to_string(), - "pipefail".to_string(), - "-c".to_string(), - ]) - .args(vec![broker_kafka_container_commands( - validated_cluster.cluster_config.is_kraft_mode(), - kafka_security, - )]); - - cb_kafka - .add_env_vars(env) - .add_container_ports(container_ports(kafka_security)) - .add_volume_mount(&*LOG_DIRS_VOLUME_NAME, STACKABLE_DATA_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount( - &*LISTENER_BOOTSTRAP_VOLUME_NAME, - STACKABLE_LISTENER_BOOTSTRAP_DIR, - ) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*LISTENER_BROKER_VOLUME_NAME, STACKABLE_LISTENER_BROKER_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_CONFIG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*STACKABLE_LOG_DIR_NAME, STACKABLE_LOG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .resources(merged_config.resources().clone().into()) - .startup_probe(broker_startup_probe) - .liveness_probe(broker_liveness_probe) - .readiness_probe(broker_readiness_probe); - - add_log_config_volume( - &mut pod_builder, - &validated_rg.config.logging, - &resource_names, - )?; - - let metadata = ObjectMetaBuilder::new() - .with_labels(recommended_labels.clone()) - .build(); - - if let Some(listener_class) = merged_config.listener_class() { - pod_builder - .add_listener_volume_by_listener_class( - LISTENER_BROKER_VOLUME_NAME.as_ref(), - listener_class.as_ref(), - &recommended_labels, - ) - .context(AddListenerVolumeSnafu)?; - } - - if let Some(broker_id_config_map_name) = &validated_cluster - .cluster_config - .broker_id_pod_config_map_name - { - pod_builder - .add_volume( - VolumeBuilder::new(&*BROKER_ID_POD_MAP_DIR_NAME) - .with_config_map(broker_id_config_map_name) - .build(), - ) - .context(AddVolumeSnafu)?; - cb_kafka - .add_volume_mount(&*BROKER_ID_POD_MAP_DIR_NAME, BROKER_ID_POD_MAP_DIR) - .expect("The mount paths are statically defined and there should be no duplicates."); - } - pod_builder - .metadata(metadata) - .image_pull_secrets_from_product_image(resolved_product_image) - .add_container(cb_kafka.build()) - .affinity(&merged_config.affinity); - - add_common_pod_config( - &mut pod_builder, - &resource_names, - validated_cluster - .cluster_resource_names() - .service_account_name() - .as_ref(), - )?; - - add_vector_container( - &mut pod_builder, - BrokerContainer::Vector.name(), - &validated_rg.config.logging, - resolved_product_image, - &resource_names, - ); - - add_graceful_shutdown_config(merged_config, &mut pod_builder).context(GracefulShutdownSnafu)?; - - let mut pod_template = pod_builder.build_template(); - - // Pod overrides were already merged (role <- role group) during validation. - pod_template.merge_from(validated_rg.pod_overrides.clone()); - - Ok(StatefulSet { - metadata: ObjectMetaBuilder::new() - .name_and_namespace(validated_cluster) - .name(resource_names.stateful_set_name().to_string()) - .ownerreference(ownerreference_from_resource( - validated_cluster, - None, - Some(true), - )) - .with_labels(recommended_labels.clone()) - .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) - .build(), - spec: Some(StatefulSetSpec { - pod_management_policy: Some(POD_MANAGEMENT_POLICY_PARALLEL.to_string()), - replicas: validated_rg.replicas.map(i32::from), - selector: LabelSelector { - match_labels: Some( - role_group_selector(validated_cluster, kafka_role, role_group_name).into(), - ), - ..LabelSelector::default() - }, - service_name: Some(resource_names.headless_service_name().to_string()), - template: pod_template, - volume_claim_templates: Some(pvcs), - ..StatefulSetSpec::default() - }), - status: None, - }) -} - -/// The controller rolegroup [`StatefulSet`] runs the rolegroup, as configured by the administrator. -pub fn build_controller_rolegroup_statefulset( - kafka_role: &KafkaRole, - role_group_name: &RoleGroupName, - validated_cluster: &ValidatedCluster, - validated_rg: &ValidatedRoleGroupConfig, -) -> Result { - let kafka_security = &validated_cluster.cluster_config.kafka_security; - let resolved_product_image = &validated_cluster.image; - let merged_config = &validated_rg.config.config; - let resource_names = validated_cluster.role_group_resource_names(kafka_role, role_group_name); - let recommended_labels = - recommended_labels_for_role_group_resources(validated_cluster, kafka_role, role_group_name); - - let mut cb_kafka = new_container_builder(ControllerContainer::Kafka.name()); - - let mut pod_builder = PodBuilder::new(); - - add_kerberos_pod_config(kafka_security, kafka_role, &mut cb_kafka, &mut pod_builder) - .context(AddKerberosConfigSnafu)?; - - let node_id_offset = node_id_hash32_offset(kafka_role, role_group_name.as_ref()).to_string(); - - // Operator-set env vars first (common + controller-specific); the user's `envOverrides` - // are merged on top and win. Shared between the `kafka` container and the - // `quorum-manager` sidecar (see `controller_pod_shared_env_vars`) so they can't drift - // apart; each container then layers its own additions on top. - let controller_shared_env = - controller_pod_shared_env_vars(validated_cluster, kafka_security, &resource_names); - - let env: Vec = controller_shared_env - .clone() - .merge(common_kafka_env( - merged_config, - &validated_rg - .product_specific_common_config - .jvm_argument_overrides, - resolved_product_image, - kafka_role, - role_group_name, - )?) - // Kerberos env goes on the `kafka` container only. `controller_shared_env` is also - // the `quorum-manager` sidecar's base, and `KAFKA_OPTS` points the JVM at - // `/tmp/jaas.properties`, which only the `kafka` container renders. - .merge(kerberos_env_vars(kafka_security)) - .merge(validated_rg.env_overrides.clone()) - .into(); - - let quorum_manager_env = controller_shared_env - .with_value(&KAFKA_NODE_ID_OFFSET, &node_id_offset) - .merge(validated_rg.env_overrides.clone()); - - let controller_pod_descriptors = validated_cluster - .pod_descriptors(Some(kafka_role)) - .context(BuildPodDescriptorsSnafu)?; - - let controller_startup_probe = - probes::controller_tcp_probe(kafka_security.client_port(), 5, 5, 60) - .context(BuildProbeSnafu)?; - - let controller_liveness_probe = probes::controller_stuck_unattached_liveness_probe( - kafka_security.client_port(), - METRICS_PORT, - 10, - 30, - 20, - ) - .context(BuildProbeSnafu)?; - let controller_readiness_probe = - probes::controller_raft_state_probe(METRICS_PORT, 10, 10, 6).context(BuildProbeSnafu)?; - - cb_kafka - .image_from_product_image(resolved_product_image) - .command(vec![ - "/bin/bash".to_string(), - "-x".to_string(), - "-euo".to_string(), - "pipefail".to_string(), - "-c".to_string(), - ]) - .args(vec![controller_kafka_container_command( - kafka_security, - controller_pod_descriptors, - )]); - - cb_kafka - .add_env_vars(env) - .add_container_ports(container_ports(kafka_security)) - .add_volume_mount(&*LOG_DIRS_VOLUME_NAME, STACKABLE_DATA_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_CONFIG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .add_volume_mount(&*STACKABLE_LOG_DIR_NAME, STACKABLE_LOG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - .resources(merged_config.resources().clone().into()) - .startup_probe(controller_startup_probe) - .liveness_probe(controller_liveness_probe) - .readiness_probe(controller_readiness_probe); - cb_kafka.lifecycle_pre_stop(LifecycleHandler { - exec: Some(ExecAction { - command: Some(vec![ - "/bin/bash".to_string(), - "-c".to_string(), - controller_remove_self_pre_stop_command(merged_config.graceful_shutdown_timeout), - ]), - }), - ..LifecycleHandler::default() - }); - - add_log_config_volume( - &mut pod_builder, - &validated_rg.config.logging, - &resource_names, - )?; + .add_volumes(builder.role_pod_volumes.clone()) + .context(AddVolumeSnafu)?; + for container in &builder.role_containers { + pod_builder.add_container(container.clone()); + } let metadata = ObjectMetaBuilder::new() .with_labels(recommended_labels.clone()) .build(); - // Add TLS related volumes and volume mounts - let requested_secret_lifetime = merged_config - .deref() - .requested_secret_lifetime - .context(MissingSecretLifetimeSnafu)?; - add_controller_volume_and_volume_mounts( - kafka_security, - &mut pod_builder, - &mut cb_kafka, - &requested_secret_lifetime, - ) - .context(AddVolumesAndVolumeMountsSnafu)?; - - let kafka_container = cb_kafka.build(); - pod_builder .metadata(metadata) .image_pull_secrets_from_product_image(resolved_product_image) - .add_container(kafka_container) - .affinity(&merged_config.affinity); - - pod_builder.add_container(build_quorum_manager_container( - resolved_product_image, - kafka_security, - quorum_manager_env, - )); + .affinity(&builder.affinity); add_common_pod_config( &mut pod_builder, @@ -591,18 +103,19 @@ pub fn build_controller_rolegroup_statefulset( add_vector_container( &mut pod_builder, - ControllerContainer::Vector.name(), - &validated_rg.config.logging, + builder.vector_container_name, + &builder.logging, resolved_product_image, &resource_names, ); - add_graceful_shutdown_config(merged_config, &mut pod_builder).context(GracefulShutdownSnafu)?; + add_graceful_shutdown_config(&builder.graceful_shutdown_timeout, &mut pod_builder) + .context(GracefulShutdownSnafu)?; let mut pod_template = pod_builder.build_template(); // Pod overrides were already merged (role <- role group) during validation. - pod_template.merge_from(validated_rg.pod_overrides.clone()); + pod_template.merge_from(builder.pod_overrides.clone()); Ok(StatefulSet { metadata: ObjectMetaBuilder::new() @@ -617,12 +130,8 @@ pub fn build_controller_rolegroup_statefulset( .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) .build(), spec: Some(StatefulSetSpec { - pod_management_policy: Some(POD_MANAGEMENT_POLICY_ORDERED_READY.to_string()), - update_strategy: Some(StatefulSetUpdateStrategy { - type_: Some("RollingUpdate".to_string()), - ..StatefulSetUpdateStrategy::default() - }), - replicas: validated_rg.replicas.map(i32::from), + pod_management_policy: Some(builder.pod_management_policy.to_string()), + replicas: builder.replicas.map(i32::from), selector: LabelSelector { match_labels: Some( role_group_selector(validated_cluster, kafka_role, role_group_name).into(), @@ -631,107 +140,13 @@ pub fn build_controller_rolegroup_statefulset( }, service_name: Some(resource_names.headless_service_name().to_string()), template: pod_template, - volume_claim_templates: Some(merged_config.resources().storage.build_pvcs()), + volume_claim_templates: Some(builder.volume_claim_templates.clone()), ..StatefulSetSpec::default() }), status: None, }) } -/// We only expose client HTTP / HTTPS and Metrics ports. -fn container_ports(kafka_security: &ValidatedKafkaSecurity) -> Vec { - let mut ports = vec![ - ContainerPort { - name: Some(METRICS_PORT_NAME.to_string()), - container_port: METRICS_PORT.into(), - protocol: Some("TCP".to_string()), - ..ContainerPort::default() - }, - ContainerPort { - name: Some(kafka_security.client_port_name().to_string()), - container_port: kafka_security.client_port().into(), - protocol: Some("TCP".to_string()), - ..ContainerPort::default() - }, - ]; - if kafka_security.has_kerberos_enabled() { - ports.push(ContainerPort { - name: Some(kafka_security.bootstrap_port_name().to_string()), - container_port: kafka_security.bootstrap_port().into(), - protocol: Some("TCP".to_string()), - ..ContainerPort::default() - }); - } - ports -} - -/// Environment variables the operator sets on the Kafka container of both roles, on top of -/// [`common_operator_env_vars`]. -/// -/// Returned as an [`EnvVarSet`] so the callers can merge the user's `envOverrides` on top, -/// letting an override win on a name collision. -fn common_kafka_env( - merged_config: &AnyConfig, - jvm_argument_overrides: &JvmArgumentOverrides, - resolved_product_image: &ResolvedProductImage, - kafka_role: &KafkaRole, - role_group_name: &RoleGroupName, -) -> Result { - Ok(EnvVarSet::new() - .with_value( - &EXTRA_ARGS, - crate::controller::build::jvm::construct_non_heap_jvm_args( - merged_config, - jvm_argument_overrides, - ) - .context(ConstructJvmArgumentsSnafu)?, - ) - .with_value( - &KAFKA_HEAP_OPTS, - crate::controller::build::jvm::construct_heap_jvm_args( - merged_config, - jvm_argument_overrides, - ) - .context(ConstructJvmArgumentsSnafu)?, - ) - .with_value( - &KAFKA_LOG4J_OPTS, - kafka_log_opts(&resolved_product_image.product_version), - ) - .with_value( - &CONTAINERDEBUG_LOG_DIRECTORY, - format!("{STACKABLE_LOG_DIR}/containerdebug"), - ) - .with_value( - &KAFKA_NODE_ID_OFFSET, - node_id_hash32_offset(kafka_role, role_group_name.as_ref()).to_string(), - )) -} - -/// Adds the `log-config` volume, sourced either from the user-supplied custom log config -/// `ConfigMap` or the rolegroup `ConfigMap` (which carries the operator-generated config). -/// Branches on the *validated* Kafka-container logging choice. -fn add_log_config_volume( - pod_builder: &mut PodBuilder, - logging: &ValidatedLogging, - resource_names: &ResourceNames, -) -> Result<(), Error> { - let config_map = match &logging.kafka_container { - ValidatedContainerLogConfigChoice::Custom(config_map_name) => config_map_name.to_string(), - ValidatedContainerLogConfigChoice::Automatic(_) => { - resource_names.role_group_config_map().to_string() - } - }; - pod_builder - .add_volume( - VolumeBuilder::new(&*STACKABLE_LOG_CONFIG_DIR_NAME) - .with_config_map(config_map) - .build(), - ) - .context(AddVolumeSnafu)?; - Ok(()) -} - /// Adds the `config` volume, the `log` emptyDir, the service account and the pod security /// context that the broker and controller pods share. fn add_common_pod_config( @@ -765,83 +180,6 @@ fn add_common_pod_config( Ok(()) } -// Name of the controller's `quorum-manager` sidecar container. -stackable_operator::constant!(QUORUM_MANAGER_CONTAINER_NAME: ContainerName = "quorum-manager"); - -/// Builds the `quorum-manager` sidecar for a controller pod. -fn build_quorum_manager_container( - resolved_product_image: &ResolvedProductImage, - kafka_security: &ValidatedKafkaSecurity, - env: EnvVarSet, -) -> stackable_operator::k8s_openapi::api::core::v1::Container { - let mut cb = new_container_builder(&QUORUM_MANAGER_CONTAINER_NAME); - - let mut local_env = EnvVarSet::new() - // `kafka-metadata-quorum.sh` goes through `kafka-run-class.sh`, which defaults - // `KAFKA_HEAP_OPTS` to `-Xmx256M` when unset. Set an explicit, modest heap so the - // JVM's max heap plus its base/metaspace/SSL-buffer overhead stays comfortably - // under the container's memory limit below. - .with_value(&KAFKA_HEAP_OPTS, "-Xmx128M"); - - cb.image_from_product_image(resolved_product_image) - .command(vec![ - "/bin/bash".to_string(), - "-c".to_string(), - quorum_manager_container_command(kafka_security), - ]) - .resources( - ResourceRequirementsBuilder::new() - .with_cpu_request("100m") - // A JVM cold start plus an SSL handshake and an admin-client round-trip all - // need to happen inside this sidecar's existing budgets - .with_cpu_limit("500m") - .with_memory_request("512Mi") - .with_memory_limit("512Mi") - .build(), - ) - .add_volume_mount(&*STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR) - .expect("The mount paths are statically defined and there should be no duplicates.") - // `controller_admin_client_properties` always points its keystore/truststore - // at this directory, so the sidecar's admin-client calls need it mounted - // here too, not just on the `kafka` container. - .add_volume_mount( - &*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, - STACKABLE_TLS_KAFKA_INTERNAL_DIR, - ) - .expect("The mount paths are statically defined and there should be no duplicates.") - // `add-controller` reads this controller's own on-disk `meta.properties` from `log.dirs`. - .add_volume_mount(&*LOG_DIRS_VOLUME_NAME, STACKABLE_DATA_DIR) - .expect("The mount paths are statically defined and there should be no duplicates."); - - if kafka_security.has_kerberos_enabled() { - // `controller_admin_client_properties` authenticates with the pod-scoped keytab - // mounted by `add_kerberos_pod_config`; the volume is already on the pod, this - // container just needs its own mount and `KRB5_CONFIG`. It deliberately does *not* - // get `KAFKA_OPTS`: that points the JVM at `/tmp/jaas.properties`, which only the - // `kafka` container renders. - cb.add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR) - .expect("The mount paths are statically defined and there should be no duplicates."); - - local_env = local_env - .with_value(&KRB5_CONFIG, STACKABLE_KERBEROS_KRB5_PATH) - // `KRB5_CONFIG` only reaches native MIT tools; the JVM reads the - // `java.security.krb5.conf` system property, without which the admin client fails - // with "Unable to locate KDC for realm". Unlike the `kafka` container's `KAFKA_OPTS` - // this deliberately omits `java.security.auth.login.config`: that points at - // `/tmp/jaas.properties`, which only the `kafka` container renders. This container - // authenticates with the inline `sasl.jaas.config` in `admin-client.properties`. - .with_value( - &KAFKA_OPTS, - format!("-Djava.security.krb5.conf={STACKABLE_KERBEROS_KRB5_PATH}"), - ); - } - - // This allows to override explicit env vars set in this function. - cb.add_env_vars(local_env.merge(env)); - - cb.build() -} - /// Adds the Vector log-aggregation sidecar container, when the Vector agent is enabled. /// /// Whether Vector is enabled, the per-container log config and the (validated) aggregator @@ -872,145 +210,30 @@ fn add_vector_container( #[cfg(test)] mod tests { - use stackable_operator::k8s_openapi::apimachinery::pkg::util::intstr::IntOrString; + use stackable_operator::k8s_openapi::apimachinery::pkg::{ + api::resource::Quantity, util::intstr::IntOrString, + }; use super::*; - use crate::controller::test_support::{minimal_kafka, validated_cluster}; + use crate::{ + controller::{ + build::{ + resource::config_map::build_config_map, + role_group::{self, controller::QUORUM_MANAGER_CONTAINER_NAME}, + }, + test_support::{ + broker_role_group_config, controller_role_group_config, minimal_kafka, + validated_cluster, + }, + }, + crd::role::KAFKA_NODE_ID_OFFSET, + }; #[test] fn test_constants() { // Test that dereferencing the constants does not panic. let _ = *VECTOR_CONFIG_VOLUME_NAME; let _ = *VECTOR_LOG_VOLUME_NAME; - let _ = *POD_NAME; - let _ = *POD_IP; - let _ = *KAFKA_CLIENT_PORT; - let _ = *NAMESPACE; - let _ = *ROLEGROUP_HEADLESS_SERVICE_NAME; - let _ = *CLUSTER_DOMAIN; - let _ = *EXTRA_ARGS; - let _ = *CONTAINERDEBUG_LOG_DIRECTORY; - let _ = *ZOOKEEPER; - let _ = *ZOOKEEPER_CONFIG_MAP_KEY; - } - - /// The user-supplied `envOverrides` must be merged in after all operator-set environment - /// variables, so that they can override any of them. `CONTAINERDEBUG_LOG_DIRECTORY` is used - /// as the example here because it is set unconditionally by the operator. The controller - /// builder merges the same [`EnvVarSet`] the same way. - #[test] - fn env_overrides_override_operator_set_env_vars() { - let kafka = minimal_kafka( - r#" - apiVersion: kafka.stackable.tech/v1alpha1 - kind: KafkaCluster - metadata: - name: simple-kafka - namespace: default - uid: 12345678-1234-1234-1234-123456789012 - spec: - image: - productVersion: 3.9.2 - clusterConfig: - zookeeperConfigMapName: xyz - brokers: - roleGroups: - default: - replicas: 1 - "#, - ); - let cluster = validated_cluster(&kafka); - let role_group_name = RoleGroupName::from_str("default").expect("valid role group name"); - let mut validated_rg = - cluster.role_group_configs[&KafkaRole::Broker][&role_group_name].clone(); - validated_rg.env_overrides = validated_rg - .env_overrides - .with_value(&CONTAINERDEBUG_LOG_DIRECTORY, "/custom/log/dir"); - - let stateful_set = build_broker_rolegroup_statefulset( - &KafkaRole::Broker, - &role_group_name, - &cluster, - &validated_rg, - ) - .expect("the StatefulSet builds"); - - let env = stateful_set - .spec - .expect("the StatefulSet has a spec") - .template - .spec - .expect("the pod template has a spec") - .containers - .into_iter() - .find(|container| container.name == "kafka") - .expect("the kafka container exists") - .env - .expect("the kafka container has env vars"); - - let containerdebug: Vec<_> = env - .iter() - .filter(|env_var| env_var.name == "CONTAINERDEBUG_LOG_DIRECTORY") - .collect(); - assert_eq!( - containerdebug.len(), - 1, - "the override must replace the operator-set value, not duplicate it" - ); - assert_eq!(containerdebug[0].value.as_deref(), Some("/custom/log/dir")); - } - - /// A [`ResolvedProductImage`] good enough for tests that only care about the container - /// spec produced from it, not the actual image contents. - fn test_resolved_product_image() -> ResolvedProductImage { - ResolvedProductImage { - product_version: "3.9.2".to_string(), - app_version_label_value: "3.9.2-stackable0.0.0-dev" - .parse() - .expect("valid label value"), - image: "oci.stackable.tech/sdp/kafka:3.9.2-stackable0.0.0-dev".to_string(), - image_pull_policy: "Always".to_string(), - pull_secrets: None, - } - } - - /// The `quorum-manager` sidecar sets `KAFKA_HEAP_OPTS` unconditionally, and - /// `KRB5_CONFIG`/`KAFKA_OPTS` when Kerberos is enabled. All three must still be - /// overridable by the caller's `env` (in practice, the rolegroup's `envOverrides`), the - /// same way [`env_overrides_override_operator_set_env_vars`] proves it for the `kafka` - /// container's env. - #[test] - fn quorum_manager_container_env_can_be_overridden() { - let resolved_product_image = test_resolved_product_image(); - let kafka_security = crate::controller::build::security::tests::kerberos(); - assert!( - kafka_security.has_kerberos_enabled(), - "sanity check failed: the fixture must have Kerberos enabled so that KRB5_CONFIG \ - and KAFKA_OPTS are set in the first place" - ); - - let overrides = EnvVarSet::new() - .with_value(&KAFKA_HEAP_OPTS, "-Xmx999M") - .with_value(&KRB5_CONFIG, "/custom/krb5.conf") - .with_value(&KAFKA_OPTS, "-Doverridden=true"); - - let container = - build_quorum_manager_container(&resolved_product_image, &kafka_security, overrides); - let env = container.env.expect("the sidecar has env vars"); - - for (name, expected_value) in [ - ("KAFKA_HEAP_OPTS", "-Xmx999M"), - ("KRB5_CONFIG", "/custom/krb5.conf"), - ("KAFKA_OPTS", "-Doverridden=true"), - ] { - let matching: Vec<_> = env.iter().filter(|env_var| env_var.name == name).collect(); - assert_eq!( - matching.len(), - 1, - "the override must replace the operator-set {name}, not duplicate it" - ); - assert_eq!(matching[0].value.as_deref(), Some(expected_value)); - } } /// A minimal KRaft cluster with one controller role group, resolved through the real @@ -1044,20 +267,36 @@ mod tests { validated_cluster(&kafka) } + /// The [`RoleGroupBuilder`] of the `default` controller role group. + fn controller_builder(cluster: &crate::controller::ValidatedCluster) -> RoleGroupBuilder<'_> { + role_group::controller::build( + cluster, + &"default".parse().expect("valid role group name"), + controller_role_group_config(cluster, "default"), + ) + .expect("the role group builds") + } + + /// The [`RoleGroupBuilder`] of the `default` broker role group. + fn broker_builder(cluster: &crate::controller::ValidatedCluster) -> RoleGroupBuilder<'_> { + let (builder, _) = role_group::broker::build( + cluster, + &"default".parse().expect("valid role group name"), + broker_role_group_config(cluster, "default"), + ) + .expect("the role group builds"); + builder + } + #[test] fn statefulsets_use_ordered_ready_pod_management_for_controllers_only() { let cluster = kraft_mode_cluster(); - let resources = crate::controller::build::build(&cluster).expect("build succeeds"); - for (name, expected_policy) in [ - ("simple-kafka-controller-default", "OrderedReady"), - ("simple-kafka-broker-default", "Parallel"), + for (builder, expected_policy) in [ + (controller_builder(&cluster), "OrderedReady"), + (broker_builder(&cluster), "Parallel"), ] { - let sts = resources - .stateful_sets - .iter() - .find(|sts| sts.metadata.name.as_deref() == Some(name)) - .unwrap_or_else(|| panic!("the {name} StatefulSet is built")); + let sts = build_statefulset(&builder).expect("the StatefulSet builds"); assert_eq!( sts.spec .as_ref() @@ -1072,13 +311,9 @@ mod tests { fn controller_containers( cluster: &crate::controller::ValidatedCluster, ) -> Vec { - let resources = crate::controller::build::build(cluster).expect("build succeeds"); - let sts = resources - .stateful_sets - .into_iter() - .find(|sts| sts.metadata.name.as_deref() == Some("simple-kafka-controller-default")) - .expect("the controller StatefulSet is built"); - sts.spec + build_statefulset(&controller_builder(cluster)) + .expect("the StatefulSet builds") + .spec .expect("the StatefulSet has a spec") .template .spec @@ -1147,13 +382,9 @@ mod tests { #[test] fn quorum_manager_sidecar_has_every_env_var_controller_properties_rendering_references() { let cluster = kraft_mode_cluster(); - let resources = crate::controller::build::build(&cluster).expect("build succeeds"); - let controller_properties = resources - .config_maps - .iter() - .find(|cm| cm.metadata.name.as_deref() == Some("simple-kafka-controller-default")) - .expect("the controller rolegroup ConfigMap is built") + let controller_properties = build_config_map(&controller_builder(&cluster)) + .expect("the ConfigMap builds") .data .as_ref() .expect("the ConfigMap carries data") @@ -1224,13 +455,9 @@ mod tests { fn broker_kafka_container( cluster: &crate::controller::ValidatedCluster, ) -> stackable_operator::k8s_openapi::api::core::v1::Container { - let resources = crate::controller::build::build(cluster).expect("build succeeds"); - let sts = resources - .stateful_sets - .into_iter() - .find(|sts| sts.metadata.name.as_deref() == Some("simple-kafka-broker-default")) - .expect("the broker StatefulSet is built"); - sts.spec + build_statefulset(&broker_builder(cluster)) + .expect("the StatefulSet builds") + .spec .expect("the StatefulSet has a spec") .template .spec @@ -1313,8 +540,6 @@ mod tests { expected_memory_request: &str, expected_memory_limit: &str, ) { - use stackable_operator::k8s_openapi::apimachinery::pkg::api::resource::Quantity; - let requests = resources .requests .as_ref() @@ -1347,11 +572,11 @@ mod tests { } /// Unlike the `kafka` container, the `quorum-manager` sidecar's resources are hardcoded in - /// [`build_quorum_manager_container`] and not exposed through `config.resources` (which only - /// applies to the `kafka` container, see + /// the controller's `build_quorum_manager_container` and not exposed through + /// `config.resources` (which only applies to the `kafka` container, see /// [`kafka_container_resources_match_configured_role_resources`]). `podOverrides` is the only /// way to change them, applied via a strategic merge on `spec.containers` keyed by container - /// `name` (see the `merge_from` call at the end of `build_controller_rolegroup_statefulset`). + /// `name` (see the `merge_from` call at the end of [`build_statefulset`]). #[test] fn quorum_manager_resources_can_be_changed_by_pod_overrides() { let kafka = minimal_kafka( diff --git a/rust/operator-binary/src/controller/build/role_group/broker.rs b/rust/operator-binary/src/controller/build/role_group/broker.rs new file mode 100644 index 000000000..10ea1abb2 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/broker.rs @@ -0,0 +1,488 @@ +//! Building one broker role group. + +use indoc::formatdoc; +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + builder::pod::volume::{ + ListenerOperatorVolumeSourceBuilder, ListenerOperatorVolumeSourceBuilderError, + ListenerReference as ListenerOperatorReference, VolumeBuilder, + }, + k8s_openapi::api::core::v1::{Container, EnvVar, PersistentVolumeClaim, Volume}, + v2::{ + builder::pod::{ + container::new_container_builder, + volume::{ListenerReference, listener_operator_volume_source_builder_build_pvc}, + }, + config_file_writer::{PropertiesWriterError, to_java_properties_string}, + product_logging::framework::STACKABLE_LOG_DIR, + }, +}; + +use super::{RoleGroupBuilder, controller_jaas_section}; +use crate::{ + controller::{ + BrokerRoleGroupConfig, PodDescriptorsError, RoleGroupName, ValidatedBrokerConfig, + ValidatedCluster, + build::{ + command::broker_kafka_container_commands, + container::{ + common_kafka_env, common_operator_env_vars, container_ports, log_config_volume, + }, + jvm, + kerberos::{self, kerberos_env_vars, mount_broker_kerberos_volume}, + properties::{ConfigFileName, broker_properties, listener::get_kafka_listener_config}, + recommended_labels_for_role_group_resources, + recommended_labels_for_unversioned_role_group_resources, + resource::{ + config_map::{self, common_config_map_data}, + listener::build_broker_rolegroup_bootstrap_listener, + probes, + }, + security::{self, mount_broker_tls_volumes}, + }, + }, + crd::{ + BROKER_ID_POD_MAP_DIR, BROKER_ID_POD_MAP_DIR_NAME, LISTENER_BOOTSTRAP_VOLUME_NAME, + LISTENER_BROKER_VOLUME_NAME, LOG_DIRS_VOLUME_NAME, METRICS_PORT, STACKABLE_CONFIG_DIR, + STACKABLE_CONFIG_DIR_NAME, STACKABLE_DATA_DIR, STACKABLE_KERBEROS_KEYTAB_PATH, + STACKABLE_LISTENER_BOOTSTRAP_DIR, STACKABLE_LISTENER_BROKER_DIR, STACKABLE_LOG_CONFIG_DIR, + STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_DIR_NAME, + listener::node_address_cmd, + role::{KafkaRole, broker::BrokerContainer}, + }, + listener, +}; + +const ROLE: KafkaRole = KafkaRole::Broker; + +const POD_MANAGEMENT_POLICY_PARALLEL: &str = "Parallel"; + +/// Why gathering one broker role group's inputs failed. The caller adds which role group it was. +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("failed to render the ConfigMap entries every role has"))] + CommonConfigMapData { source: config_map::Error }, + + #[snafu(display("failed to serialize [{file}]"))] + SerializeConfigFile { + source: PropertiesWriterError, + file: ConfigFileName, + }, + + #[snafu(display("failed to build pod descriptors"))] + BuildPodDescriptors { source: PodDescriptorsError }, + + #[snafu(display("failed to build the TLS volumes"))] + BuildTlsVolumes { source: security::Error }, + + #[snafu(display("failed to build the Kerberos volume"))] + BuildKerberosVolume { source: kerberos::Error }, + + #[snafu(display("failed to build the broker listener volume"))] + BuildListenerVolume { + source: ListenerOperatorVolumeSourceBuilderError, + }, + + #[snafu(display("failed to build container probe"))] + BuildProbe { source: probes::Error }, + + #[snafu(display("failed to construct JVM arguments"))] + ConstructJvmArguments { source: jvm::Error }, +} + +/// Builds one broker role group: the [`RoleGroupBuilder`] for its Services, ConfigMap and +/// StatefulSet, and its bootstrap `Listener`. +/// +/// Brokers are the only role with a bootstrap `Listener` and the only role exposed through +/// listener-operator volumes: the bootstrap listener as a persistent claim template and the +/// broker listener as an ephemeral pod volume. +pub(crate) fn build<'a>( + cluster: &'a ValidatedCluster, + role_group_name: &RoleGroupName, + rg_config: &BrokerRoleGroupConfig, +) -> Result<(RoleGroupBuilder<'a>, listener::v1alpha1::Listener), Error> { + let config = &rg_config.config; + let logging = &rg_config.config.logging; + + let mut config_map_data = role_config_map_data(cluster, role_group_name, rg_config)?; + config_map_data.extend( + common_config_map_data( + cluster, + BrokerContainer::Kafka.name(), + logging, + rg_config + .config_overrides + .security_properties + .overrides + .clone(), + ) + .context(CommonConfigMapDataSnafu)?, + ); + + let (role_containers, role_pod_volumes) = role_pod(cluster, role_group_name, rg_config)?; + + let builder = RoleGroupBuilder { + cluster, + role: ROLE, + role_group_name: role_group_name.clone(), + affinity: config.affinity.clone(), + graceful_shutdown_timeout: config.graceful_shutdown_timeout, + logging: logging.clone(), + vector_container_name: BrokerContainer::Vector.name(), + role_containers, + role_pod_volumes, + volume_claim_templates: volume_claim_templates(cluster, role_group_name, config), + pod_management_policy: POD_MANAGEMENT_POLICY_PARALLEL, + replicas: rg_config.replicas, + pod_overrides: rg_config.pod_overrides.clone(), + config_map_data, + }; + + let bootstrap_listener = + build_broker_rolegroup_bootstrap_listener(cluster, &ROLE, role_group_name, config); + + Ok((builder, bootstrap_listener)) +} + +/// `broker.properties` rendered with the role group's overrides, and the broker's JAAS config. +fn role_config_map_data( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + rg_config: &BrokerRoleGroupConfig, +) -> Result, Error> { + let kafka_security = &cluster.cluster_config.kafka_security; + + let listener_config = + get_kafka_listener_config(cluster, kafka_security, &ROLE, role_group_name); + + let pod_descriptors = cluster + .pod_descriptors(None) + .context(BuildPodDescriptorsSnafu)?; + + let kafka_config = broker_properties::build( + &cluster.cluster_config, + &listener_config, + &pod_descriptors, + rg_config + .config_overrides + .broker_properties + .overrides + .clone(), + ); + + tracing::debug!(?kafka_config, "Applied kafka config"); + + Ok(vec![ + ( + ConfigFileName::BrokerProperties.to_string(), + to_java_properties_string(kafka_config.iter()).context(SerializeConfigFileSnafu { + file: ConfigFileName::BrokerProperties, + })?, + ), + // This file contains the JAAS configuration for Kerberos authentication + // It has the ".properties" extension but is not a Java properties file. + // It is processed by `config-utils` to substitute "env:" and "file:" variables + // and this tool currently doesn't support the JAAS login configuration format. + ( + ConfigFileName::Jaas.to_string(), + jaas_config_file(kafka_security.has_kerberos_enabled()), + ), + ]) +} + +/// The `StatefulSet`'s volume claim templates: the storage volumes and the bootstrap listener. +fn volume_claim_templates( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + config: &ValidatedBrokerConfig, +) -> Vec { + // Used for PVC templates, which cannot be modified once they are deployed. The version label + // is omitted so the labels stay stable across version upgrades. + let unversioned_recommended_labels = + recommended_labels_for_unversioned_role_group_resources(cluster, &ROLE, role_group_name); + + let mut pvcs = config.resources.storage.build_pvcs(); + + // bootstrap listener should be persistent, + // main broker listener is an ephemeral PVC instead + let bootstrap_listener_name = cluster.bootstrap_listener_name(&ROLE, role_group_name); + pvcs.push(listener_operator_volume_source_builder_build_pvc( + &ListenerReference::Listener(bootstrap_listener_name), + &unversioned_recommended_labels, + &LISTENER_BOOTSTRAP_VOLUME_NAME, + )); + + pvcs +} + +/// The broker pod's `kafka` container and the volumes only brokers have, in pod order. +fn role_pod( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + rg_config: &BrokerRoleGroupConfig, +) -> Result<(Vec, Vec), Error> { + let config = &rg_config.config; + let logging = &rg_config.config.logging; + let kafka_security = &cluster.cluster_config.kafka_security; + let image = &cluster.image; + let resource_names = cluster.role_group_resource_names(&ROLE, role_group_name); + + let mut cb_kafka = new_container_builder(BrokerContainer::Kafka.name()); + + // The push order below (TLS, Kerberos, `log-config`, the broker listener, the broker-id + // ConfigMap) is part of the pod template: a pod's `volumes` is an ordered list, so changing + // the order changes the template of every deployed StatefulSet and restarts its pods. The + // order is pinned by `pod_volumes_keep_their_order` in `build/mod.rs`. + + // The TLS certificates are requested with the role group's secret lifetime. + let mut pod_volumes = mount_broker_tls_volumes( + kafka_security, + &mut cb_kafka, + &config.requested_secret_lifetime, + ) + .context(BuildTlsVolumesSnafu)?; + + if let Some(kerberos_volume) = mount_broker_kerberos_volume(kafka_security, &mut cb_kafka) + .context(BuildKerberosVolumeSnafu)? + { + pod_volumes.push(kerberos_volume); + } + + // Operator-set env vars first; the user's `envOverrides` are merged on top last and win. + let env: Vec = common_operator_env_vars(cluster, kafka_security) + .merge( + common_kafka_env( + &config.resources, + &rg_config + .product_specific_common_config + .jvm_argument_overrides, + image, + &ROLE, + role_group_name, + ) + .context(ConstructJvmArgumentsSnafu)?, + ) + .merge(kerberos_env_vars(kafka_security)) + .merge(rg_config.env_overrides.clone()) + .into(); + + // The client port can accept connections before the broker has replayed its log and + // reached the JMX `RUNNING` state, so the startupProbe waits for both, giving it up to + // 5 minutes (60 * 5s) before the livenessProbe is allowed to start counting failures. + let startup_probe = + probes::broker_running_probe(kafka_security.client_port(), METRICS_PORT, 5, 5, 60) + .context(BuildProbeSnafu)?; + let liveness_probe = + probes::broker_running_probe(kafka_security.client_port(), METRICS_PORT, 10, 30, 20) + .context(BuildProbeSnafu)?; + let readiness_probe = + probes::broker_kcat_readiness_probe(kafka_security).context(BuildProbeSnafu)?; + + cb_kafka + .image_from_product_image(image) + .command(vec![ + "/bin/bash".to_string(), + "-x".to_string(), + "-euo".to_string(), + "pipefail".to_string(), + "-c".to_string(), + ]) + .args(vec![broker_kafka_container_commands( + cluster.cluster_config.is_kraft_mode(), + kafka_security, + )]); + + cb_kafka + .add_env_vars(env) + .add_container_ports(container_ports(kafka_security)) + .add_volume_mount(&*LOG_DIRS_VOLUME_NAME, STACKABLE_DATA_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount( + &*LISTENER_BOOTSTRAP_VOLUME_NAME, + STACKABLE_LISTENER_BOOTSTRAP_DIR, + ) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*LISTENER_BROKER_VOLUME_NAME, STACKABLE_LISTENER_BROKER_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_CONFIG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*STACKABLE_LOG_DIR_NAME, STACKABLE_LOG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .resources(config.resources.clone().into()) + .startup_probe(startup_probe) + .liveness_probe(liveness_probe) + .readiness_probe(readiness_probe); + + pod_volumes.push(log_config_volume(logging, &resource_names)); + pod_volumes.push(broker_listener_volume(cluster, role_group_name, config)?); + + if let Some(broker_id_config_map_name) = &cluster.cluster_config.broker_id_pod_config_map_name { + pod_volumes.push( + VolumeBuilder::new(&*BROKER_ID_POD_MAP_DIR_NAME) + .with_config_map(broker_id_config_map_name) + .build(), + ); + cb_kafka + .add_volume_mount(&*BROKER_ID_POD_MAP_DIR_NAME, BROKER_ID_POD_MAP_DIR) + .expect("The mount paths are statically defined and there should be no duplicates."); + } + + Ok((vec![cb_kafka.build()], pod_volumes)) +} + +/// The ephemeral listener volume through which the broker is reached directly, from the role +/// group's `brokerListenerClass`. Unlike the bootstrap listener it is not persistent, so it is a +/// pod volume rather than a claim template. +/// +/// Built with the v1 [`ListenerOperatorVolumeSourceBuilder`], whose reference type is imported as +/// `ListenerOperatorReference` to keep it apart from the v2 `ListenerReference` the bootstrap +/// claim template uses: operator-rs has no v2 helper for the ephemeral form, only for the claim. +fn broker_listener_volume( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + config: &ValidatedBrokerConfig, +) -> Result { + let recommended_labels = + recommended_labels_for_role_group_resources(cluster, &ROLE, role_group_name); + let listener_volume_source = ListenerOperatorVolumeSourceBuilder::new( + &ListenerOperatorReference::ListenerClass(config.broker_listener_class.to_string()), + &recommended_labels, + ) + .build_ephemeral() + .context(BuildListenerVolumeSnafu)?; + + Ok(Volume { + name: LISTENER_BROKER_VOLUME_NAME.to_string(), + ephemeral: Some(listener_volume_source), + ..Volume::default() + }) +} + +// Generate JAAS configuration file for Kerberos authentication +// or an empty string if Kerberos is not enabled. +// See https://docs.oracle.com/javase/8/docs/technotes/guides/security/jgss/tutorials/LoginConfigFile.html +fn jaas_config_file(is_kerberos_enabled: bool) -> String { + if !is_kerberos_enabled { + return String::new(); + } + + // Broker pods reach the CONTROLLER listener as SASL clients; the only principals in + // their keytab (see `mount_broker_kerberos_volume`) are for the broker and bootstrap listener + // addresses, so their CONTROLLER section must reuse the broker address. + let controller_section = + controller_jaas_section(&node_address_cmd(STACKABLE_LISTENER_BROKER_DIR)); + + formatdoc! {" + bootstrap.KafkaServer {{ + com.sun.security.auth.module.Krb5LoginModule required + useKeyTab=true + storeKey=true + isInitiator=false + keyTab=\"{STACKABLE_KERBEROS_KEYTAB_PATH}\" + principal=\"kafka/{bootstrap_address}@${{env:KERBEROS_REALM}}\"; + }}; + + client.KafkaServer {{ + com.sun.security.auth.module.Krb5LoginModule required + useKeyTab=true + storeKey=true + isInitiator=false + keyTab=\"{STACKABLE_KERBEROS_KEYTAB_PATH}\" + principal=\"kafka/{broker_address}@${{env:KERBEROS_REALM}}\"; + }}; + + {controller_section} + ", + bootstrap_address = node_address_cmd(STACKABLE_LISTENER_BOOTSTRAP_DIR), + broker_address = node_address_cmd(STACKABLE_LISTENER_BROKER_DIR), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::controller::{ + build::container::CONTAINERDEBUG_LOG_DIRECTORY, + test_support::{broker_role_group_config, minimal_kafka, validated_cluster}, + }; + + /// The user-supplied `envOverrides` must be merged in after all operator-set environment + /// variables, so that they can override any of them. `CONTAINERDEBUG_LOG_DIRECTORY` is used + /// as the example here because it is set unconditionally by the operator. The controller + /// builder merges the same [`EnvVarSet`] the same way. + #[test] + fn env_overrides_override_operator_set_env_vars() { + let kafka = minimal_kafka( + r#" + apiVersion: kafka.stackable.tech/v1alpha1 + kind: KafkaCluster + metadata: + name: simple-kafka + namespace: default + uid: 12345678-1234-1234-1234-123456789012 + spec: + image: + productVersion: 3.9.2 + clusterConfig: + zookeeperConfigMapName: xyz + brokers: + roleGroups: + default: + replicas: 1 + "#, + ); + let cluster = validated_cluster(&kafka); + let role_group_name: RoleGroupName = "default".parse().expect("valid role group name"); + let mut validated_rg = broker_role_group_config(&cluster, "default").clone(); + validated_rg.env_overrides = validated_rg + .env_overrides + .with_value(&CONTAINERDEBUG_LOG_DIRECTORY, "/custom/log/dir"); + + let (builder, _) = + build(&cluster, &role_group_name, &validated_rg).expect("the role group builds"); + let stateful_set = builder.build_statefulset().expect("the StatefulSet builds"); + + let env = stateful_set + .spec + .expect("the StatefulSet has a spec") + .template + .spec + .expect("the pod template has a spec") + .containers + .into_iter() + .find(|container| container.name == "kafka") + .expect("the kafka container exists") + .env + .expect("the kafka container has env vars"); + + let containerdebug: Vec<_> = env + .iter() + .filter(|env_var| env_var.name == "CONTAINERDEBUG_LOG_DIRECTORY") + .collect(); + assert_eq!( + containerdebug.len(), + 1, + "the override must replace the operator-set value, not duplicate it" + ); + assert_eq!(containerdebug[0].value.as_deref(), Some("/custom/log/dir")); + } + + #[test] + fn jaas_config_file_empty_without_kerberos() { + assert_eq!(jaas_config_file(false), ""); + } + + #[test] + fn jaas_config_file_renders_bootstrap_and_client_sections_with_kerberos() { + let jaas = jaas_config_file(true); + assert!(jaas.contains("bootstrap.KafkaServer")); + assert!(jaas.contains("client.KafkaServer")); + assert!(jaas.contains("Krb5LoginModule")); + assert!(jaas.contains("${env:KERBEROS_REALM}")); + // The bootstrap and client principals embed distinct listener addresses. + assert!(jaas.contains("/stackable/listener-bootstrap")); + assert!(jaas.contains("/stackable/listener-broker")); + } +} diff --git a/rust/operator-binary/src/controller/build/role_group/controller.rs b/rust/operator-binary/src/controller/build/role_group/controller.rs new file mode 100644 index 000000000..99d24dd99 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/controller.rs @@ -0,0 +1,547 @@ +//! Building one controller role group. + +use std::str::FromStr; + +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + builder::pod::{container::FieldPathEnvVar, resources::ResourceRequirementsBuilder}, + commons::product_image_selection::ResolvedProductImage, + k8s_openapi::api::core::v1::{ + Container, EnvVar, EnvVarSource, ExecAction, LifecycleHandler, ObjectFieldSelector, Volume, + }, + v2::{ + builder::pod::container::{EnvVarName, EnvVarSet, new_container_builder}, + config_file_writer::{PropertiesWriterError, to_java_properties_string}, + product_logging::framework::STACKABLE_LOG_DIR, + role_group_utils::ResourceNames, + types::kubernetes::ContainerName, + }, +}; + +use super::{RoleGroupBuilder, controller_jaas_section}; +use crate::{ + controller::{ + ControllerRoleGroupConfig, PodDescriptorsError, RoleGroupName, ValidatedCluster, + build::{ + command::{ + controller_kafka_container_command, controller_remove_self_pre_stop_command, + quorum_manager_container_command, + }, + container::{ + common_kafka_env, common_operator_env_vars, container_ports, log_config_volume, + }, + jvm, + kerberos::{ + self, KAFKA_OPTS, KERBEROS_VOLUME_NAME, KRB5_CONFIG, kerberos_env_vars, + mount_controller_kerberos_volume, + }, + properties::{ + ConfigFileName, controller_properties, listener::get_kafka_listener_config, + }, + resource::{ + config_map::{self, common_config_map_data}, + probes, + }, + security::{ + self, STACKABLE_TLS_KAFKA_INTERNAL_DIR, STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, + controller_admin_client_properties, mount_controller_tls_volume, + }, + }, + node_id_hasher::node_id_hash32_offset, + security::ValidatedKafkaSecurity, + }, + crd::{ + CONTROLLER_POD_FQDN_TEMPLATE, KAFKA_HEAP_OPTS, LOG_DIRS_VOLUME_NAME, METRICS_PORT, + STACKABLE_CONFIG_DIR, STACKABLE_CONFIG_DIR_NAME, STACKABLE_DATA_DIR, + STACKABLE_KERBEROS_DIR, STACKABLE_KERBEROS_KRB5_PATH, STACKABLE_LOG_CONFIG_DIR, + STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_DIR_NAME, + role::{KAFKA_NODE_ID_OFFSET, KafkaRole, controller::ControllerContainer}, + }, +}; + +const ROLE: KafkaRole = KafkaRole::Controller; + +const POD_MANAGEMENT_POLICY_ORDERED_READY: &str = "OrderedReady"; + +/// Why gathering one controller role group's inputs failed. The caller adds which role group it +/// was. +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("failed to render the ConfigMap entries every role has"))] + CommonConfigMapData { source: config_map::Error }, + + #[snafu(display("failed to serialize [{file}]"))] + SerializeConfigFile { + source: PropertiesWriterError, + file: ConfigFileName, + }, + + #[snafu(display("failed to build pod descriptors"))] + BuildPodDescriptors { source: PodDescriptorsError }, + + #[snafu(display("failed to build the TLS volume"))] + BuildTlsVolumes { source: security::Error }, + + #[snafu(display("failed to build the Kerberos volume"))] + BuildKerberosVolume { source: kerberos::Error }, + + #[snafu(display("failed to build container probe"))] + BuildProbe { source: probes::Error }, + + #[snafu(display("failed to construct JVM arguments"))] + ConstructJvmArguments { source: jvm::Error }, +} + +// Env vars the operator sets on the controller's `kafka` and `quorum-manager` containers. +stackable_operator::constant!(POD_IP: EnvVarName = "POD_IP"); +stackable_operator::constant!(NAMESPACE: EnvVarName = "NAMESPACE"); +stackable_operator::constant!(ROLEGROUP_HEADLESS_SERVICE_NAME: EnvVarName = "ROLEGROUP_HEADLESS_SERVICE_NAME"); +stackable_operator::constant!(CLUSTER_DOMAIN: EnvVarName = "CLUSTER_DOMAIN"); + +// Name of the controller's `quorum-manager` sidecar container. +stackable_operator::constant!(pub(crate) QUORUM_MANAGER_CONTAINER_NAME: ContainerName = "quorum-manager"); + +/// Builds one controller role group: the [`RoleGroupBuilder`] for its Services, ConfigMap and +/// StatefulSet. +/// +/// Controllers run the `quorum-manager` sidecar next to their `kafka` container, and only they +/// get `admin-client.properties`, which that sidecar reads. They have no listener-operator +/// volumes: they are only reachable through their own StatefulSet pod DNS names. +pub(crate) fn build<'a>( + cluster: &'a ValidatedCluster, + role_group_name: &RoleGroupName, + rg_config: &ControllerRoleGroupConfig, +) -> Result, Error> { + let config = &rg_config.config; + let logging = &rg_config.config.logging; + + let mut config_map_data = role_config_map_data(cluster, role_group_name, rg_config)?; + config_map_data.extend( + common_config_map_data( + cluster, + ControllerContainer::Kafka.name(), + logging, + rg_config + .config_overrides + .security_properties + .overrides + .clone(), + ) + .context(CommonConfigMapDataSnafu)?, + ); + + let (role_containers, role_pod_volumes) = role_pod(cluster, role_group_name, rg_config)?; + + Ok(RoleGroupBuilder { + cluster, + role: ROLE, + role_group_name: role_group_name.clone(), + affinity: config.affinity.clone(), + graceful_shutdown_timeout: config.graceful_shutdown_timeout, + logging: logging.clone(), + vector_container_name: ControllerContainer::Vector.name(), + role_containers, + role_pod_volumes, + volume_claim_templates: config.resources.storage.build_pvcs(), + pod_management_policy: POD_MANAGEMENT_POLICY_ORDERED_READY, + replicas: rg_config.replicas, + pod_overrides: rg_config.pod_overrides.clone(), + config_map_data, + }) +} + +/// `controller.properties` rendered with the role group's overrides, the controller's JAAS +/// config, and `admin-client.properties`. +fn role_config_map_data( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + rg_config: &ControllerRoleGroupConfig, +) -> Result, Error> { + let kafka_security = &cluster.cluster_config.kafka_security; + + let listener_config = + get_kafka_listener_config(cluster, kafka_security, &ROLE, role_group_name); + + let pod_descriptors = cluster + .pod_descriptors(None) + .context(BuildPodDescriptorsSnafu)?; + + let kafka_config = controller_properties::build( + &cluster.cluster_config, + &listener_config, + &pod_descriptors, + rg_config + .config_overrides + .controller_properties + .overrides + .clone(), + ); + + tracing::debug!(?kafka_config, "Applied kafka config"); + + Ok(vec![ + ( + ConfigFileName::ControllerProperties.to_string(), + to_java_properties_string(kafka_config.iter()).context(SerializeConfigFileSnafu { + file: ConfigFileName::ControllerProperties, + })?, + ), + // This file contains the JAAS configuration for Kerberos authentication + // It has the ".properties" extension but is not a Java properties file. + // It is processed by `config-utils` to substitute "env:" and "file:" variables + // and this tool currently doesn't support the JAAS login configuration format. + ( + ConfigFileName::Jaas.to_string(), + jaas_config_file(kafka_security.has_kerberos_enabled()), + ), + // `admin-client.properties` is only needed by the controller-side sidecar running + // `kafka-metadata-quorum.sh` against the CONTROLLER listener; brokers don't need it. + ( + ConfigFileName::AdminClient.to_string(), + to_java_properties_string( + controller_admin_client_properties(kafka_security) + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k, v))), + ) + .context(SerializeConfigFileSnafu { + file: ConfigFileName::AdminClient, + })?, + ), + ]) +} + +/// The controller pod's `kafka` container and `quorum-manager` sidecar, and the volumes only +/// controllers have, in pod order. +fn role_pod( + cluster: &ValidatedCluster, + role_group_name: &RoleGroupName, + rg_config: &ControllerRoleGroupConfig, +) -> Result<(Vec, Vec), Error> { + let config = &rg_config.config; + let logging = &rg_config.config.logging; + let kafka_security = &cluster.cluster_config.kafka_security; + let image = &cluster.image; + let resource_names = cluster.role_group_resource_names(&ROLE, role_group_name); + + let mut cb_kafka = new_container_builder(ControllerContainer::Kafka.name()); + + // The push order below (Kerberos, `log-config`, then the internal TLS volume, which is not + // next to Kerberos as it is for brokers) is part of the pod template: a pod's `volumes` is + // an ordered list, so changing the order changes the template of every deployed StatefulSet + // and restarts its pods. The order is pinned by `pod_volumes_keep_their_order` in + // `build/mod.rs`. + let mut pod_volumes: Vec = Vec::new(); + + if let Some(kerberos_volume) = mount_controller_kerberos_volume(kafka_security, &mut cb_kafka) + .context(BuildKerberosVolumeSnafu)? + { + pod_volumes.push(kerberos_volume); + } + + let node_id_offset = node_id_hash32_offset(&ROLE, role_group_name.as_ref()).to_string(); + + // Operator-set env vars first (common + controller-specific); the user's `envOverrides` + // are merged on top and win. Shared between the `kafka` container and the + // `quorum-manager` sidecar (see `controller_pod_shared_env_vars`) so they can't drift + // apart; each container then layers its own additions on top. + let controller_shared_env = + controller_pod_shared_env_vars(cluster, kafka_security, &resource_names); + + let env: Vec = controller_shared_env + .clone() + .merge( + common_kafka_env( + &config.resources, + &rg_config + .product_specific_common_config + .jvm_argument_overrides, + image, + &ROLE, + role_group_name, + ) + .context(ConstructJvmArgumentsSnafu)?, + ) + // Kerberos env goes on the `kafka` container only. `controller_shared_env` is also + // the `quorum-manager` sidecar's base, and `KAFKA_OPTS` points the JVM at + // `/tmp/jaas.properties`, which only the `kafka` container renders. + .merge(kerberos_env_vars(kafka_security)) + .merge(rg_config.env_overrides.clone()) + .into(); + + let quorum_manager_env = controller_shared_env + .with_value(&KAFKA_NODE_ID_OFFSET, &node_id_offset) + .merge(rg_config.env_overrides.clone()); + + let controller_pod_descriptors = cluster + .pod_descriptors(Some(&ROLE)) + .context(BuildPodDescriptorsSnafu)?; + + let startup_probe = probes::controller_tcp_probe(kafka_security.client_port(), 5, 5, 60) + .context(BuildProbeSnafu)?; + let liveness_probe = probes::controller_stuck_unattached_liveness_probe( + kafka_security.client_port(), + METRICS_PORT, + 10, + 30, + 20, + ) + .context(BuildProbeSnafu)?; + let readiness_probe = + probes::controller_raft_state_probe(METRICS_PORT, 10, 10, 6).context(BuildProbeSnafu)?; + + cb_kafka + .image_from_product_image(image) + .command(vec![ + "/bin/bash".to_string(), + "-x".to_string(), + "-euo".to_string(), + "pipefail".to_string(), + "-c".to_string(), + ]) + .args(vec![controller_kafka_container_command( + kafka_security, + controller_pod_descriptors, + )]); + + cb_kafka + .add_env_vars(env) + .add_container_ports(container_ports(kafka_security)) + .add_volume_mount(&*LOG_DIRS_VOLUME_NAME, STACKABLE_DATA_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*STACKABLE_LOG_CONFIG_DIR_NAME, STACKABLE_LOG_CONFIG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .add_volume_mount(&*STACKABLE_LOG_DIR_NAME, STACKABLE_LOG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + .resources(config.resources.clone().into()) + .startup_probe(startup_probe) + .liveness_probe(liveness_probe) + .readiness_probe(readiness_probe); + cb_kafka.lifecycle_pre_stop(LifecycleHandler { + exec: Some(ExecAction { + command: Some(vec![ + "/bin/bash".to_string(), + "-c".to_string(), + controller_remove_self_pre_stop_command(config.graceful_shutdown_timeout), + ]), + }), + ..LifecycleHandler::default() + }); + + pod_volumes.push(log_config_volume(logging, &resource_names)); + + // The TLS certificate is requested with the role group's secret lifetime. + pod_volumes.push( + mount_controller_tls_volume( + kafka_security, + &mut cb_kafka, + &config.requested_secret_lifetime, + ) + .context(BuildTlsVolumesSnafu)?, + ); + + let containers = vec![ + cb_kafka.build(), + build_quorum_manager_container(image, kafka_security, quorum_manager_env), + ]; + + Ok((containers, pod_volumes)) +} + +/// Environment variables the operator sets that are common to the `kafka` and `quorum-manager` +/// containers. +fn controller_pod_shared_env_vars( + validated_cluster: &ValidatedCluster, + kafka_security: &ValidatedKafkaSecurity, + resource_names: &ResourceNames, +) -> EnvVarSet { + // TODO: for op-rs + // `FieldPathEnvVar` has no `status.podIP` variant, so this one is built by hand rather + // than through `with_field_path`. + let pod_ip_env_var = EnvVar { + name: POD_IP.to_string(), + value: None, + value_from: Some(EnvVarSource { + field_ref: Some(ObjectFieldSelector { + field_path: "status.podIP".to_string(), + ..ObjectFieldSelector::default() + }), + ..EnvVarSource::default() + }), + }; + + common_operator_env_vars(validated_cluster, kafka_security) + .with_field_path(&NAMESPACE, &FieldPathEnvVar::Namespace) + .with_value( + &ROLEGROUP_HEADLESS_SERVICE_NAME, + resource_names.headless_service_name().to_string(), + ) + .with_value( + &CLUSTER_DOMAIN, + validated_cluster.cluster_domain.to_string(), + ) + .with_env_var(pod_ip_env_var) + .expect("the env var name is a valid EnvVarName") +} + +/// Builds the `quorum-manager` sidecar for a controller pod. +fn build_quorum_manager_container( + resolved_product_image: &ResolvedProductImage, + kafka_security: &ValidatedKafkaSecurity, + env: EnvVarSet, +) -> Container { + let mut cb = new_container_builder(&QUORUM_MANAGER_CONTAINER_NAME); + + let mut local_env = EnvVarSet::new() + // `kafka-metadata-quorum.sh` goes through `kafka-run-class.sh`, which defaults + // `KAFKA_HEAP_OPTS` to `-Xmx256M` when unset. Set an explicit, modest heap so the + // JVM's max heap plus its base/metaspace/SSL-buffer overhead stays comfortably + // under the container's memory limit below. + .with_value(&KAFKA_HEAP_OPTS, "-Xmx128M"); + + cb.image_from_product_image(resolved_product_image) + .command(vec![ + "/bin/bash".to_string(), + "-c".to_string(), + quorum_manager_container_command(kafka_security), + ]) + .resources( + ResourceRequirementsBuilder::new() + .with_cpu_request("100m") + // A JVM cold start plus an SSL handshake and an admin-client round-trip all + // need to happen inside this sidecar's existing budgets + .with_cpu_limit("500m") + .with_memory_request("512Mi") + .with_memory_limit("512Mi") + .build(), + ) + .add_volume_mount(&*STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR) + .expect("The mount paths are statically defined and there should be no duplicates.") + // `controller_admin_client_properties` always points its keystore/truststore + // at this directory, so the sidecar's admin-client calls need it mounted + // here too, not just on the `kafka` container. + .add_volume_mount( + &*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, + STACKABLE_TLS_KAFKA_INTERNAL_DIR, + ) + .expect("The mount paths are statically defined and there should be no duplicates.") + // `add-controller` reads this controller's own on-disk `meta.properties` from `log.dirs`. + .add_volume_mount(&*LOG_DIRS_VOLUME_NAME, STACKABLE_DATA_DIR) + .expect("The mount paths are statically defined and there should be no duplicates."); + + if kafka_security.has_kerberos_enabled() { + // `controller_admin_client_properties` authenticates with the pod-scoped keytab + // mounted by `mount_controller_kerberos_volume`; the volume is already on the pod, this + // container just needs its own mount and `KRB5_CONFIG`. It deliberately does *not* + // get `KAFKA_OPTS`: that points the JVM at `/tmp/jaas.properties`, which only the + // `kafka` container renders. + cb.add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR) + .expect("The mount paths are statically defined and there should be no duplicates."); + + local_env = local_env + .with_value(&KRB5_CONFIG, STACKABLE_KERBEROS_KRB5_PATH) + // `KRB5_CONFIG` only reaches native MIT tools; the JVM reads the + // `java.security.krb5.conf` system property, without which the admin client fails + // with "Unable to locate KDC for realm". Unlike the `kafka` container's `KAFKA_OPTS` + // this deliberately omits `java.security.auth.login.config`: that points at + // `/tmp/jaas.properties`, which only the `kafka` container renders. This container + // authenticates with the inline `sasl.jaas.config` in `admin-client.properties`. + .with_value( + &KAFKA_OPTS, + format!("-Djava.security.krb5.conf={STACKABLE_KERBEROS_KRB5_PATH}"), + ); + } + + // This allows to override explicit env vars set in this function. + cb.add_env_vars(local_env.merge(env)); + + cb.build() +} + +// Generate JAAS configuration file for Kerberos authentication +// or an empty string if Kerberos is not enabled. +// See https://docs.oracle.com/javase/8/docs/technotes/guides/security/jgss/tutorials/LoginConfigFile.html +fn jaas_config_file(is_kerberos_enabled: bool) -> String { + if !is_kerberos_enabled { + return String::new(); + } + + // Controller pods have no listener-operator `Listener` volume; their keytab is + // pod-scoped, so their CONTROLLER section uses their own pod FQDN — the same expression + // already used for `KAFKA_LISTENERS` in `controller_properties.rs`. + controller_jaas_section(CONTROLLER_POD_FQDN_TEMPLATE) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A [`ResolvedProductImage`] good enough for tests that only care about the container + /// spec produced from it, not the actual image contents. + fn test_resolved_product_image() -> ResolvedProductImage { + ResolvedProductImage { + product_version: "3.9.2".to_string(), + app_version_label_value: "3.9.2-stackable0.0.0-dev" + .parse() + .expect("valid label value"), + image: "oci.stackable.tech/sdp/kafka:3.9.2-stackable0.0.0-dev".to_string(), + image_pull_policy: "Always".to_string(), + pull_secrets: None, + } + } + + /// The `quorum-manager` sidecar sets `KAFKA_HEAP_OPTS` unconditionally, and + /// `KRB5_CONFIG`/`KAFKA_OPTS` when Kerberos is enabled. All three must still be + /// overridable by the caller's `env` (in practice, the rolegroup's `envOverrides`), the + /// same way [`env_overrides_override_operator_set_env_vars`] proves it for the `kafka` + /// container's env. + #[test] + fn quorum_manager_container_env_can_be_overridden() { + let resolved_product_image = test_resolved_product_image(); + let kafka_security = crate::controller::build::security::tests::kerberos(); + assert!( + kafka_security.has_kerberos_enabled(), + "sanity check failed: the fixture must have Kerberos enabled so that KRB5_CONFIG \ + and KAFKA_OPTS are set in the first place" + ); + + let overrides = EnvVarSet::new() + .with_value(&KAFKA_HEAP_OPTS, "-Xmx999M") + .with_value(&KRB5_CONFIG, "/custom/krb5.conf") + .with_value(&KAFKA_OPTS, "-Doverridden=true"); + + let container = + build_quorum_manager_container(&resolved_product_image, &kafka_security, overrides); + let env = container.env.expect("the sidecar has env vars"); + + for (name, expected_value) in [ + ("KAFKA_HEAP_OPTS", "-Xmx999M"), + ("KRB5_CONFIG", "/custom/krb5.conf"), + ("KAFKA_OPTS", "-Doverridden=true"), + ] { + let matching: Vec<_> = env.iter().filter(|env_var| env_var.name == name).collect(); + assert_eq!( + matching.len(), + 1, + "the override must replace the operator-set {name}, not duplicate it" + ); + assert_eq!(matching[0].value.as_deref(), Some(expected_value)); + } + } + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *POD_IP; + let _ = *NAMESPACE; + let _ = *ROLEGROUP_HEADLESS_SERVICE_NAME; + let _ = *CLUSTER_DOMAIN; + let _ = *QUORUM_MANAGER_CONTAINER_NAME; + } + + #[test] + fn jaas_config_file_empty_without_kerberos() { + assert_eq!(jaas_config_file(false), ""); + } +} diff --git a/rust/operator-binary/src/controller/build/role_group/mod.rs b/rust/operator-binary/src/controller/build/role_group/mod.rs new file mode 100644 index 000000000..5c07d0a48 --- /dev/null +++ b/rust/operator-binary/src/controller/build/role_group/mod.rs @@ -0,0 +1,131 @@ +//! Building the Kubernetes resources of one role group. +//! +//! Each role's module ([`broker`], [`controller`]) fills in a [`RoleGroupBuilder`] for one role +//! group. It builds the containers only that role runs (its `kafka` container, and for +//! controllers the `quorum-manager` sidecar) together with the pod volumes they need, and renders +//! the role group's `ConfigMap` entries. Kafka's roles differ in their main container (probes, +//! `preStop`, command and environment) and in the sidecars they run, which is why the role +//! modules build those containers themselves. +//! +//! [`RoleGroupBuilder`]'s `build_*` methods then emit the Kubernetes objects. They add what every +//! role's pod has (the `config` and `log` volumes, the Vector sidecar and the pod-level settings) +//! and never look at which role they are building. + +pub(crate) mod broker; +pub(crate) mod controller; + +use indoc::formatdoc; +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + commons::affinity::StackableAffinity, + k8s_openapi::api::{ + apps::v1::StatefulSet, + core::v1::{ConfigMap, Container, PersistentVolumeClaim, PodTemplateSpec, Service, Volume}, + }, + shared::time::Duration, + v2::types::kubernetes::ContainerName, +}; + +use super::resource; +use crate::{ + controller::{RoleGroupName, ValidatedCluster, validate::ValidatedLogging}, + crd::{STACKABLE_KERBEROS_KEYTAB_PATH, role::KafkaRole}, +}; + +/// Why emitting one role group's Kubernetes objects failed. The caller adds which role and role +/// group it was. Errors from gathering a role's inputs are the role module's own ([`broker::Error`], +/// [`controller::Error`]). +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("failed to build ConfigMap"))] + ConfigMap { source: resource::config_map::Error }, + + #[snafu(display("failed to build StatefulSet"))] + StatefulSet { + source: resource::statefulset::Error, + }, +} + +/// Everything one role group's Kubernetes objects are built from. +/// +/// The role's module fills this in with the role's containers already built and the `ConfigMap` +/// entries already rendered, so the `build_*` methods read straight through it. +pub(crate) struct RoleGroupBuilder<'a> { + pub(crate) cluster: &'a ValidatedCluster, + /// Used for the names and labels of the role group's objects. + pub(crate) role: KafkaRole, + pub(crate) role_group_name: RoleGroupName, + /// Read for the pod's affinity. + pub(crate) affinity: StackableAffinity, + /// Read for the pod's termination grace period. + pub(crate) graceful_shutdown_timeout: Duration, + /// The role group's validated logging; read for the Vector sidecar, which runs only when the + /// Vector agent is enabled. + pub(crate) logging: ValidatedLogging, + /// The name of the Vector sidecar, from the role's container enum. + pub(crate) vector_container_name: &'static ContainerName, + /// The containers only this role runs, in pod order: the `kafka` container, and for + /// controllers the `quorum-manager` sidecar. The Vector sidecar comes after them. + pub(crate) role_containers: Vec, + /// The pod volumes only this role has, in the order the pod lists them, including the + /// `log-config` volume, which sits between them. The `config` and `log` volumes come after + /// them. + pub(crate) role_pod_volumes: Vec, + /// The `StatefulSet`'s persistent volume claim templates. For brokers these include the + /// bootstrap listener claim template. + pub(crate) volume_claim_templates: Vec, + pub(crate) pod_management_policy: &'static str, + /// The role group's replica count, passed to the StatefulSet as is; `None` makes Kubernetes + /// run one pod. + pub(crate) replicas: Option, + pub(crate) pod_overrides: PodTemplateSpec, + /// Every `ConfigMap` entry, as (file name, content). + pub(crate) config_map_data: Vec<(String, String)>, +} + +impl RoleGroupBuilder<'_> { + /// The headless and metrics `Service`s. + pub(crate) fn build_services(&self) -> Vec { + vec![ + resource::service::build_rolegroup_headless_service( + self.cluster, + &self.role, + &self.role_group_name, + &self.cluster.cluster_config.kafka_security, + ), + resource::service::build_rolegroup_metrics_service( + self.cluster, + &self.role, + &self.role_group_name, + ), + ] + } + + pub(crate) fn build_config_map(&self) -> Result { + resource::config_map::build_config_map(self).context(ConfigMapSnafu) + } + + pub(crate) fn build_statefulset(&self) -> Result { + resource::statefulset::build_statefulset(self).context(StatefulSetSnafu) + } +} + +/// The JAAS section for the CONTROLLER listener, which both roles render, authenticating as +/// `kafka/`. +/// +/// Unlike the broker's bootstrap and client sections, this context is used for BOTH sides of +/// every CONTROLLER-listener connection: brokers connect out to controllers, and controllers +/// connect to each other for Raft. This is the only listener in this operator where the process +/// must act as a GSSAPI initiator as well as an acceptor, so `isInitiator` is intentionally left +/// at its default (`true`). +fn controller_jaas_section(controller_principal_address: &str) -> String { + formatdoc! {" + controller.KafkaServer {{ + com.sun.security.auth.module.Krb5LoginModule required + useKeyTab=true + storeKey=true + keyTab=\"{STACKABLE_KERBEROS_KEYTAB_PATH}\" + principal=\"kafka/{controller_principal_address}@${{env:KERBEROS_REALM}}\"; + }}; + "} +} diff --git a/rust/operator-binary/src/controller/build/security.rs b/rust/operator-binary/src/controller/build/security.rs index 0906bf7ef..bffc7fc51 100644 --- a/rust/operator-binary/src/controller/build/security.rs +++ b/rust/operator-binary/src/controller/build/security.rs @@ -7,13 +7,9 @@ use std::{collections::BTreeMap, str::FromStr}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::{ - self, - pod::{ - PodBuilder, - container::ContainerBuilder, - volume::{SecretFormat, SecretOperatorVolumeSourceBuilder, VolumeBuilder}, - }, + builder::pod::{ + container::ContainerBuilder, + volume::{SecretFormat, SecretOperatorVolumeSourceBuilder, VolumeBuilder}, }, commons::secret_class::SecretClassVolumeProvisionParts, constant, @@ -41,7 +37,7 @@ const INTER_BROKER_LISTENER_NAME: &str = "inter.broker.listener.name"; const KEYSTORE_P12_FILE_NAME: &str = "keystore.p12"; const OPA_TLS_MOUNT_PATH: &str = "/stackable/tls-opa"; // opa -constant!(OPA_TLS_VOLUME_NAME: VolumeName = "tls-opa"); +constant!(pub(crate) OPA_TLS_VOLUME_NAME: VolumeName = "tls-opa"); const SSL_STORE_PASSWORD: &str = ""; const SSL_STORE_TYPE_PKCS12: &str = "PKCS12"; const SSL_CLIENT_AUTH_REQUIRED: &str = "required"; @@ -58,10 +54,10 @@ const PROPERTY_SASL_JAAS_CONFIG: &str = "sasl.jaas.config"; pub(crate) const STACKABLE_TLS_KAFKA_INTERNAL_DIR: &str = "/stackable/tls-kafka-internal"; constant!(pub(crate) STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME: VolumeName = "tls-kafka-internal"); const STACKABLE_TLS_KAFKA_SERVER_DIR: &str = "/stackable/tls-kafka-server"; -constant!(STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME: VolumeName = "tls-kafka-server"); +constant!(pub(crate) STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME: VolumeName = "tls-kafka-server"); // directories const STACKABLE_TLS_KCAT_DIR: &str = "/stackable/tls-kcat"; -constant!(STACKABLE_TLS_KCAT_VOLUME_NAME: VolumeName = "tls-kcat"); +constant!(pub(crate) STACKABLE_TLS_KCAT_VOLUME_NAME: VolumeName = "tls-kcat"); const TRUSTSTORE_P12_FILE_NAME: &str = "truststore.p12"; #[derive(Snafu, Debug)] @@ -71,9 +67,6 @@ pub enum Error { source: stackable_operator::builder::pod::volume::SecretOperatorVolumeSourceBuilderError, }, - #[snafu(display("failed to add needed volume"))] - AddVolume { source: builder::pod::Error }, - #[snafu(display("failed to build OPA TLS certificate volume"))] OpaTlsCertSecretClassVolumeBuild { source: stackable_operator::builder::pod::volume::SecretOperatorVolumeSourceBuilderError, @@ -208,17 +201,17 @@ pub fn client_properties(security: &ValidatedKafkaSecurity) -> Vec<(String, Opti /// Client-side (unprefixed `security.protocol`/`ssl.*`) properties for an admin CLI tool /// (e.g. `kafka-metadata-quorum.sh`) talking to the CONTROLLER listener from *inside* a /// controller pod, over the `tls-kafka-internal` volume mounted by -/// `add_controller_volume_and_volume_mounts`. +/// `mount_controller_tls_volume`. /// When Kerberos is enabled the CONTROLLER listener is `SASL_SSL` (see /// [`get_kafka_listener_config`][glc]), so these calls must authenticate with GSSAPI. They do /// so as the controller's *own* pod principal, from the pod-scoped keytab mounted by -/// [`add_kerberos_pod_config`][akpc] — the correct identity for a voter registering itself. +/// [`mount_controller_kerberos_volume`] — the correct identity for a voter registering itself. /// /// The principal contains `${env:…}` placeholders, so the rendered file must be passed /// through `config-utils template` before use; see [`quorum_manager_container_command`][qmcc]. /// /// [glc]: crate::controller::build::properties::listener::get_kafka_listener_config -/// [akpc]: crate::controller::build::kerberos::add_kerberos_pod_config +/// [`mount_controller_kerberos_volume`]: crate::controller::build::kerberos::mount_controller_kerberos_volume /// [qmcc]: crate::controller::build::command::quorum_manager_container_command pub fn controller_admin_client_properties( security: &ValidatedKafkaSecurity, @@ -263,41 +256,38 @@ pub fn controller_admin_client_properties( properties } -/// Adds required volumes and volume mounts to the broker pod and container builders -/// depending on the tls and authentication settings. +/// Adds the volume mounts the tls and authentication settings require to the broker's container +/// builder and returns the pod volumes they mount, in pod order. /// /// # Panics /// /// Panics if the volume mounts cannot be added to the container builders. Only call this on /// container builders whose mount paths are still distinct from the ones added here. -pub fn add_broker_volume_and_volume_mounts( +pub fn mount_broker_tls_volumes( security: &ValidatedKafkaSecurity, - pod_builder: &mut PodBuilder, cb_kafka: &mut ContainerBuilder, requested_secret_lifetime: &Duration, -) -> Result<(), Error> { +) -> Result, Error> { + let mut volumes = Vec::new(); + // add tls (server or client authentication volumes) if required if let Some(tls_server_secret_class) = tls_secret_class(security) { // We have to mount tls pem files for kcat's readiness-probe command (the mount can be // used directly) - pod_builder - .add_volume(create_kcat_tls_volume( - &STACKABLE_TLS_KCAT_VOLUME_NAME, - tls_server_secret_class, - requested_secret_lifetime, - )?) - .context(AddVolumeSnafu)?; + volumes.push(create_kcat_tls_volume( + &STACKABLE_TLS_KCAT_VOLUME_NAME, + tls_server_secret_class, + requested_secret_lifetime, + )?); cb_kafka .add_volume_mount(&*STACKABLE_TLS_KCAT_VOLUME_NAME, STACKABLE_TLS_KCAT_DIR) .expect("The mount paths are statically defined and there should be no duplicates."); // Keystores fore the kafka container - pod_builder - .add_volume(create_tls_keystore_volume( - &STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME, - tls_server_secret_class, - requested_secret_lifetime, - )?) - .context(AddVolumeSnafu)?; + volumes.push(create_tls_keystore_volume( + &STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME, + tls_server_secret_class, + requested_secret_lifetime, + )?); cb_kafka .add_volume_mount( &*STACKABLE_TLS_KAFKA_SERVER_VOLUME_NAME, @@ -306,13 +296,11 @@ pub fn add_broker_volume_and_volume_mounts( .expect("The mount paths are statically defined and there should be no duplicates."); } - pod_builder - .add_volume(create_tls_keystore_volume( - &STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, - security.tls_internal_secret_class(), - requested_secret_lifetime, - )?) - .context(AddVolumeSnafu)?; + volumes.push(create_tls_keystore_volume( + &STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, + security.tls_internal_secret_class(), + requested_secret_lifetime, + )?); cb_kafka .add_volume_mount( &*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, @@ -325,59 +313,52 @@ pub fn add_broker_volume_and_volume_mounts( .add_volume_mount(&*OPA_TLS_VOLUME_NAME, OPA_TLS_MOUNT_PATH) .expect("The mount paths are statically defined and there should be no duplicates."); - pod_builder - .add_volume( - VolumeBuilder::new(&*OPA_TLS_VOLUME_NAME) - .ephemeral( - SecretOperatorVolumeSourceBuilder::new( - secret_class, - // Only the truststore is required to connect to OPA. - SecretClassVolumeProvisionParts::Public, - ) - .build() - .context(OpaTlsCertSecretClassVolumeBuildSnafu)?, + volumes.push( + VolumeBuilder::new(&*OPA_TLS_VOLUME_NAME) + .ephemeral( + SecretOperatorVolumeSourceBuilder::new( + secret_class, + // Only the truststore is required to connect to OPA. + SecretClassVolumeProvisionParts::Public, ) - .build(), - ) - .context(AddVolumeSnafu)?; + .build() + .context(OpaTlsCertSecretClassVolumeBuildSnafu)?, + ) + .build(), + ); } - Ok(()) + Ok(volumes) } -/// Adds required volumes and volume mounts to the controller pod and container builders -/// depending on the tls and authentication settings. +/// Adds the internal TLS volume mount to the controller's container builder and returns the pod +/// volume it mounts. /// /// # Panics /// /// Panics if the volume mounts cannot be added to the container builder. Only call this on a /// container builder whose mount paths are still distinct from the ones added here. -pub fn add_controller_volume_and_volume_mounts( +pub fn mount_controller_tls_volume( security: &ValidatedKafkaSecurity, - pod_builder: &mut PodBuilder, cb_kafka: &mut ContainerBuilder, requested_secret_lifetime: &Duration, -) -> Result<(), Error> { - pod_builder - .add_volume( - VolumeBuilder::new(&*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME) - .ephemeral( - SecretOperatorVolumeSourceBuilder::new( - security.tls_internal_secret_class(), - // Kafka needs both the public certificate and the private key for - // the internal communication. - SecretClassVolumeProvisionParts::PublicPrivate, - ) - .with_pod_scope() - .with_format(SecretFormat::TlsPkcs12) - .with_auto_tls_cert_lifetime(*requested_secret_lifetime) - .with_auto_tls_cert_domain_components_in_subject_dn(true) - .build() - .context(SecretVolumeBuildSnafu)?, - ) - .build(), +) -> Result { + let volume = VolumeBuilder::new(&*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME) + .ephemeral( + SecretOperatorVolumeSourceBuilder::new( + security.tls_internal_secret_class(), + // Kafka needs both the public certificate and the private key for + // the internal communication. + SecretClassVolumeProvisionParts::PublicPrivate, + ) + .with_pod_scope() + .with_format(SecretFormat::TlsPkcs12) + .with_auto_tls_cert_lifetime(*requested_secret_lifetime) + .with_auto_tls_cert_domain_components_in_subject_dn(true) + .build() + .context(SecretVolumeBuildSnafu)?, ) - .context(AddVolumeSnafu)?; + .build(); cb_kafka .add_volume_mount( &*STACKABLE_TLS_KAFKA_INTERNAL_VOLUME_NAME, @@ -385,7 +366,7 @@ pub fn add_controller_volume_and_volume_mounts( ) .expect("The mount paths are statically defined and there should be no duplicates."); - Ok(()) + Ok(volume) } /// Inserts the `listener..ssl.{keystore,truststore}.{location,password,type}` @@ -1073,7 +1054,7 @@ pub(crate) mod tests { #[test] fn controller_admin_client_properties_always_uses_tls_even_without_external_client_tls() { // Internal (broker/controller) TLS is mandatory (`tls_internal_secret_class()` always - // returns a SecretClass, defaulting to "tls"), and `add_controller_volume_and_volume_mounts` + // returns a SecretClass, defaulting to "tls"), and `mount_controller_tls_volume` // unconditionally mounts both the keystore and truststore on controller pods, independent // of the external client TLS/authentication settings. So even the "plaintext" fixture // (no external client TLS, no client-cert auth) still needs SSL to reach the CONTROLLER diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 3b06fadaf..3f0f2322d 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -23,25 +23,27 @@ use stackable_operator::{ ValidatedContainerLogConfigChoice, VectorContainerLogConfig, validate_logging_configuration_for_container, }, - role_utils::{JavaCommonConfig, Role, with_validated_config}, + role_utils::{JavaCommonConfig, Role, RoleGroup, RoleGroupConfig, with_validated_config}, types::kubernetes::ConfigMapName, }, }; use crate::{ controller::{ - RoleGroupName, ValidatedCluster, ValidatedClusterConfig, ValidatedKafkaConfig, - ValidatedRoleConfig, ValidatedRoleGroupConfig, + BrokerRoleGroupConfig, ControllerRoleGroupConfig, RoleGroupName, ValidatedBrokerConfig, + ValidatedCluster, ValidatedClusterConfig, ValidatedControllerConfig, ValidatedRoleConfig, dereference::DereferencedObjects, + replicas_or_default, security::{self, ValidatedKafkaSecurity}, }, crd::{ - CONTAINER_IMAGE_BASE_NAME, + BrokerRole, CONTAINER_IMAGE_BASE_NAME, ControllerRole, authentication::{self}, role::{ - AnyConfig, AnyConfigOverrides, KafkaRole, - broker::{BrokerConfig, BrokerContainer}, - controller::{ControllerConfig, ControllerContainer}, + KafkaRole, + broker::{BrokerConfig, BrokerConfigFragment, BrokerContainer}, + commons::CommonConfig, + controller::{ControllerConfig, ControllerConfigFragment, ControllerContainer}, }, tls, v1alpha1, }, @@ -102,6 +104,20 @@ pub enum Error { ))] MissingVectorAggregatorConfigMapName, + #[snafu(display( + "the graceful shutdown timeout of {role} role group {role_group_name:?} is missing" + ))] + MissingGracefulShutdownTimeout { + role: KafkaRole, + role_group_name: String, + }, + + #[snafu(display("the secret lifetime of {role} role group {role_group_name:?} is missing"))] + MissingSecretLifetime { + role: KafkaRole, + role_group_name: String, + }, + #[snafu(display( "at least one KRaft controller replica is required while any broker replicas are \ configured; a KRaft cluster with zero controllers has no metadata quorum for its \ @@ -168,11 +184,11 @@ where /// Validates a broker role group's logging configuration. fn validate_broker_logging( - config: &BrokerConfig, + logging: &Logging, vector_aggregator_config_map_name: &Option, ) -> Result { validate_logging( - &config.logging, + logging, BrokerContainer::Kafka, BrokerContainer::Vector, vector_aggregator_config_map_name, @@ -181,11 +197,11 @@ fn validate_broker_logging( /// Validates a controller role group's logging configuration. fn validate_controller_logging( - config: &ControllerConfig, + logging: &Logging, vector_aggregator_config_map_name: &Option, ) -> Result { validate_logging( - &config.logging, + logging, ControllerContainer::Kafka, ControllerContainer::Vector, vector_aggregator_config_map_name, @@ -250,15 +266,12 @@ pub fn validate( .vector_aggregator_config_map_name .clone(); - let mut role_configs: BTreeMap = BTreeMap::new(); - let mut role_group_configs: BTreeMap< - KafkaRole, - BTreeMap, - > = BTreeMap::new(); + // The first failure propagates, so this order decides which misconfiguration the user is told + // about when more than one role is wrong. It is `KafkaRole`'s variant order. // The broker role is required by the CRD. let broker_role = &kafka.spec.brokers; - let broker_groups = validate_role_group_configs( + let broker_role_group_configs = validate_broker_role_group_configs( broker_role, BrokerConfig::default_config( &kafka.name_any(), @@ -266,18 +279,9 @@ pub fn validate( kafka.spec.cluster_config.authorization.opa.as_ref(), ), cluster_id, - AnyConfig::Broker, - AnyConfigOverrides::Broker, - validate_broker_logging, &vector_aggregator_config_map_name, )?; - role_configs.insert( - KafkaRole::Broker, - ValidatedRoleConfig { - pdb: broker_role.role_config.pod_disruption_budget.clone(), - }, - ); - role_group_configs.insert(KafkaRole::Broker, broker_groups); + let broker_config = validated_role_config(&broker_role.role_config); let metadata_manager = kafka .effective_metadata_manager() @@ -289,45 +293,45 @@ pub fn validate( return MissingKraftControllerRoleSnafu.fail(); } - if let Some(controller_role) = kafka.spec.controllers.as_ref() { - let controller_groups = validate_role_group_configs( - controller_role, - ControllerConfig::default_config(&kafka.name_any(), &KafkaRole::Controller.to_string()), - cluster_id, - AnyConfig::Controller, - AnyConfigOverrides::Controller, - validate_controller_logging, - &vector_aggregator_config_map_name, - )?; - - // A KRaft cluster with zero controller replicas *and running brokers* is a broken - // half-state. Reject that combination here. - // - // Controllers *and* brokers at zero together is not rejected: that is exactly what - // `clusterOperation.stopped` already does today. - let controller_replicas: u16 = controller_groups - .values() - .map(|rg| rg.replicas.unwrap_or(1)) - .sum(); - let broker_replicas: u16 = role_group_configs[&KafkaRole::Broker] - .values() - .map(|rg| rg.replicas.unwrap_or(1)) - .sum(); - if metadata_manager == crate::crd::MetadataManager::KRaft - && controller_replicas == 0 - && broker_replicas > 0 - { - return NoKraftControllerReplicasSnafu.fail(); + let (controller_config, controller_role_group_configs) = match kafka.spec.controllers.as_ref() { + Some(controller_role) => { + let controller_role_group_configs = validate_controller_role_group_configs( + controller_role, + ControllerConfig::default_config( + &kafka.name_any(), + &KafkaRole::Controller.to_string(), + ), + cluster_id, + &vector_aggregator_config_map_name, + )?; + + // A KRaft cluster with zero controller replicas *and running brokers* is a broken + // half-state. Reject that combination here. + // + // Controllers *and* brokers at zero together is not rejected: that is exactly what + // `clusterOperation.stopped` already does today. + let controller_replicas: u16 = controller_role_group_configs + .values() + .map(|rg| replicas_or_default(rg.replicas)) + .sum(); + let broker_replicas: u16 = broker_role_group_configs + .values() + .map(|rg| replicas_or_default(rg.replicas)) + .sum(); + if metadata_manager == crate::crd::MetadataManager::KRaft + && controller_replicas == 0 + && broker_replicas > 0 + { + return NoKraftControllerReplicasSnafu.fail(); + } + + ( + Some(validated_role_config(&controller_role.role_config)), + controller_role_group_configs, + ) } - - role_configs.insert( - KafkaRole::Controller, - ValidatedRoleConfig { - pdb: controller_role.role_config.pod_disruption_budget.clone(), - }, - ); - role_group_configs.insert(KafkaRole::Controller, controller_groups); - } + None => (None, BTreeMap::new()), + }; let name = get_cluster_name(kafka).context(ResolveClusterNameSnafu)?; let namespace = get_namespace(kafka).context(ResolveNamespaceSnafu)?; @@ -337,13 +341,15 @@ pub fn validate( .cluster_domain .clone(); - Ok(ValidatedCluster::new( + Ok(ValidatedCluster { + metadata: ValidatedCluster::object_meta(&name, &namespace, &uid), name, namespace, uid, cluster_domain, + product_version: ValidatedCluster::product_version(&image), image, - ValidatedClusterConfig { + cluster_config: ValidatedClusterConfig { kafka_security, authorization_config: dereferenced_objects.authorization_config, metadata_manager, @@ -354,81 +360,188 @@ pub fn validate( .broker_id_pod_config_map_name .clone(), }, - role_configs, - role_group_configs, - dereferenced_objects.bootstrap_listeners, - )) + broker_config, + broker_role_group_configs, + controller_config, + controller_role_group_configs, + bootstrap_listeners: dereferenced_objects.bootstrap_listeners, + }) } -/// Validates every role group of a role into a map keyed by role group name. -/// -/// Each role group is merged and validated via -/// [`with_validated_config`], which folds the config fragment (default <- role <- -/// role group) plus the `configOverrides`, `envOverrides`, `podOverrides` and -/// `jvmArgumentOverrides` (role group wins) into a single -/// [`ValidatedRoleGroupConfig`]. The concrete per-role validated config and overrides -/// are wrapped into the role-agnostic [`AnyConfig`]/[`AnyConfigOverrides`] via -/// `wrap_config`/`wrap_overrides`, and the operator-managed `KAFKA_CLUSTER_ID` is -/// injected into the env overrides. -fn validate_role_group_configs( - role: &Role, - default_config: Config, +/// The role-level config, resolved from the role's `roleConfig`. +fn validated_role_config(role_config: &GenericRoleConfig) -> ValidatedRoleConfig { + // Destructured without `..`, so a field added upstream fails to compile here instead of + // being silently dropped. + let GenericRoleConfig { + pod_disruption_budget, + } = role_config; + ValidatedRoleConfig { + pdb: pod_disruption_budget.clone(), + } +} + +/// Validates every broker role group into a map keyed by role group name: merges it (see +/// [`validate_role_group_config`]), then validates its logging, then its name. +fn validate_broker_role_group_configs( + role: &BrokerRole, + default_config: BrokerConfigFragment, cluster_id: Option<&str>, - wrap_config: fn(ValidatedConfig) -> AnyConfig, - wrap_overrides: fn(ConfigOverrides) -> AnyConfigOverrides, - validate_logging: fn(&ValidatedConfig, &Option) -> Result, vector_aggregator_config_map_name: &Option, -) -> Result> -where - Config: Clone + Merge, - ValidatedConfig: FromFragment, - ConfigOverrides: Clone + Default + JsonSchema + Merge + Serialize, -{ +) -> Result> { + role.role_groups + .iter() + .map(|(role_group_name, role_group)| { + let merged = validate_role_group_config(role, role_group, &default_config, cluster_id)?; + let BrokerConfig { + common_config: + CommonConfig { + affinity, + graceful_shutdown_timeout, + requested_secret_lifetime, + }, + bootstrap_listener_class, + broker_listener_class, + logging, + resources, + } = merged.config; + let validated = BrokerRoleGroupConfig { + replicas: merged.replicas, + config: ValidatedBrokerConfig { + resources, + logging: validate_broker_logging(&logging, vector_aggregator_config_map_name)?, + affinity, + graceful_shutdown_timeout: graceful_shutdown_timeout.with_context(|| { + MissingGracefulShutdownTimeoutSnafu { + role: KafkaRole::Broker, + role_group_name: role_group_name.to_owned(), + } + })?, + requested_secret_lifetime: requested_secret_lifetime.with_context(|| { + MissingSecretLifetimeSnafu { + role: KafkaRole::Broker, + role_group_name: role_group_name.to_owned(), + } + })?, + bootstrap_listener_class, + broker_listener_class, + }, + config_overrides: merged.config_overrides, + env_overrides: merged.env_overrides, + cli_overrides: merged.cli_overrides, + pod_overrides: merged.pod_overrides, + product_specific_common_config: merged.product_specific_common_config, + }; + Ok((parse_role_group_name(role_group_name)?, validated)) + }) + .collect() +} + +/// Validates every controller role group into a map keyed by role group name: merges it (see +/// [`validate_role_group_config`]), then validates its logging, then its name. +fn validate_controller_role_group_configs( + role: &ControllerRole, + default_config: ControllerConfigFragment, + cluster_id: Option<&str>, + vector_aggregator_config_map_name: &Option, +) -> Result> { role.role_groups .iter() .map(|(role_group_name, role_group)| { - let merged = with_validated_config::< - ValidatedConfig, - JavaCommonConfig, - Config, - GenericRoleConfig, - ConfigOverrides, - >(role_group, role, &default_config) - .context(ValidateRoleGroupConfigSnafu)?; - - // The env override names are validated on deserialization; inject the - // operator-managed KAFKA_CLUSTER_ID (unless the user overrides it). - let env_overrides = inject_cluster_id(merged.config.env_overrides.into(), cluster_id); - - let logging = - validate_logging(&merged.config.config, vector_aggregator_config_map_name)?; - - let validated = ValidatedRoleGroupConfig { - // Passed through as-is (including `None`) so an unset replica count lets a - // horizontal autoscaler own the StatefulSet's `.spec.replicas`. + let merged = validate_role_group_config(role, role_group, &default_config, cluster_id)?; + let ControllerConfig { + common_config: + CommonConfig { + affinity, + graceful_shutdown_timeout, + requested_secret_lifetime, + }, + logging, + resources, + } = merged.config; + let validated = ControllerRoleGroupConfig { replicas: merged.replicas, - config: ValidatedKafkaConfig { - config: wrap_config(merged.config.config), - logging, + config: ValidatedControllerConfig { + resources, + logging: validate_controller_logging( + &logging, + vector_aggregator_config_map_name, + )?, + affinity, + graceful_shutdown_timeout: graceful_shutdown_timeout.with_context(|| { + MissingGracefulShutdownTimeoutSnafu { + role: KafkaRole::Controller, + role_group_name: role_group_name.to_owned(), + } + })?, + requested_secret_lifetime: requested_secret_lifetime.with_context(|| { + MissingSecretLifetimeSnafu { + role: KafkaRole::Controller, + role_group_name: role_group_name.to_owned(), + } + })?, }, - config_overrides: wrap_overrides(merged.config.config_overrides), - env_overrides, - // Kafka does not use CLI overrides; the field is carried (and merged upstream) - // but unused. - cli_overrides: merged.config.cli_overrides, - pod_overrides: merged.config.pod_overrides, - product_specific_common_config: merged.config.product_specific_common_config, + config_overrides: merged.config_overrides, + env_overrides: merged.env_overrides, + cli_overrides: merged.cli_overrides, + pod_overrides: merged.pod_overrides, + product_specific_common_config: merged.product_specific_common_config, }; - let role_group_name = RoleGroupName::from_str(role_group_name).with_context(|_| { - ParseRoleGroupNameSnafu { - role_group_name: role_group_name.clone(), - } - })?; - Ok((role_group_name, validated)) + Ok((parse_role_group_name(role_group_name)?, validated)) }) .collect() } +/// Merges and validates one role group via [`with_validated_config`], which folds the config +/// fragment (default <- role <- role group) plus the `configOverrides`, `envOverrides`, +/// `podOverrides` and `jvmArgumentOverrides` (role group wins) into a single +/// [`RoleGroupConfig`], and injects the operator-managed `KAFKA_CLUSTER_ID` into the env +/// overrides. +fn validate_role_group_config( + role: &Role, + role_group: &RoleGroup, + default_config: &Config, + cluster_id: Option<&str>, +) -> Result> +where + Config: Clone + Merge, + ValidatedConfig: FromFragment, + ConfigOverrides: Clone + Default + JsonSchema + Merge + Serialize, +{ + let merged = with_validated_config::< + ValidatedConfig, + JavaCommonConfig, + Config, + GenericRoleConfig, + ConfigOverrides, + >(role_group, role, default_config) + .context(ValidateRoleGroupConfigSnafu)?; + + // The env override names are validated on deserialization; inject the + // operator-managed KAFKA_CLUSTER_ID (unless the user overrides it). + let env_overrides = inject_cluster_id(merged.config.env_overrides.into(), cluster_id); + + Ok(RoleGroupConfig { + // Passed through as is, including `None`: the StatefulSet then leaves `.spec.replicas` + // unset, and the pod predictions count it as `DEFAULT_REPLICAS`. + replicas: merged.replicas, + config: merged.config.config, + config_overrides: merged.config.config_overrides, + env_overrides, + // Kafka does not use CLI overrides; the field is carried (and merged upstream) + // but unused. + cli_overrides: merged.config.cli_overrides, + pod_overrides: merged.config.pod_overrides, + product_specific_common_config: merged.config.product_specific_common_config, + }) +} + +/// Parses a role group name from the CRD into its type-safe form. +fn parse_role_group_name(role_group_name: &str) -> Result { + RoleGroupName::from_str(role_group_name).with_context(|_| ParseRoleGroupNameSnafu { + role_group_name: role_group_name.to_owned(), + }) +} + /// Injects the operator-managed `KAFKA_CLUSTER_ID` into the merged env overrides, /// but only when the user has not already set it via `envOverrides` (user value /// wins). @@ -446,16 +559,12 @@ fn inject_cluster_id(env_overrides: EnvVarSet, cluster_id: Option<&str>) -> EnvV #[cfg(test)] mod tests { - use std::str::FromStr; - - use stackable_operator::v2::{ - builder::pod::container::EnvVarSet, types::operator::RoleGroupName, - }; + use stackable_operator::v2::builder::pod::container::EnvVarSet; use super::{Error, KAFKA_CLUSTER_ID, inject_cluster_id}; - use crate::{ - controller::test_support::{app_version_label, minimal_kafka, validated_cluster}, - crd::role::KafkaRole, + use crate::controller::test_support::{ + app_version_label, broker_role_group_config, controller_role_group_config, minimal_kafka, + validated_cluster, }; #[test] @@ -535,24 +644,37 @@ mod tests { assert_eq!(security.opa_secret_class(), None); // Both roles are present, with default (enabled) PDB configs. - let roles: Vec<_> = cluster.role_configs.keys().collect(); - assert_eq!(roles, [&KafkaRole::Broker, &KafkaRole::Controller]); - for role_config in cluster.role_configs.values() { + let controller_config = cluster + .controller_config + .as_ref() + .expect("the controller role is present"); + for role_config in [&cluster.broker_config, controller_config] { assert!(role_config.pdb.enabled); assert_eq!(role_config.pdb.max_unavailable, None); } // One `default` role group per role. The KRaft cluster id (derived from the cluster // name) is injected into every role group's env overrides. - let default_rg = RoleGroupName::from_str("default").expect("valid role group name"); - for role in [KafkaRole::Broker, KafkaRole::Controller] { - let role_group = &cluster.role_group_configs[&role][&default_rg]; - assert_eq!(role_group.replicas, Some(3)); + let broker_role_group = broker_role_group_config(&cluster, "default"); + let controller_role_group = controller_role_group_config(&cluster, "default"); + for (replicas, env_overrides, logging) in [ + ( + broker_role_group.replicas, + &broker_role_group.env_overrides, + &broker_role_group.config.logging, + ), + ( + controller_role_group.replicas, + &controller_role_group.env_overrides, + &controller_role_group.config.logging, + ), + ] { + assert_eq!(replicas, Some(3)); assert_eq!( - cluster_id_value(&role_group.env_overrides), + cluster_id_value(env_overrides), Some("simple-kafka".to_string()) ); - assert_eq!(role_group.config.logging.vector_container, None); + assert_eq!(logging.vector_container, None); } } diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index 838ed8dc2..e46b71ecc 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -57,7 +57,10 @@ mod tests { }; use crate::{ - controller::test_support::{minimal_kafka, validated_cluster}, + controller::test_support::{ + broker_role_group_config, controller_role_group_config, minimal_kafka, + validated_cluster, + }, crd::KafkaRole, }; @@ -93,15 +96,21 @@ mod tests { let kafka = minimal_kafka(input); let validated = validated_cluster(&kafka); - let merged_config = validated - .role_group_configs - .get(&role) - .and_then(|groups| groups.get(&"default".parse().unwrap())) - .map(|rg| &rg.config.config) - .expect("role group should exist"); + let affinity = match role { + KafkaRole::Broker => { + &broker_role_group_config(&validated, "default") + .config + .affinity + } + KafkaRole::Controller => { + &controller_role_group_config(&validated, "default") + .config + .affinity + } + }; assert_eq!( - merged_config.affinity, + *affinity, StackableAffinity { pod_affinity: match role { // Only brokers are configured with the OPA authorizer. diff --git a/rust/operator-binary/src/crd/role/mod.rs b/rust/operator-binary/src/crd/role/mod.rs index 7c4e3168b..8d3e9507a 100644 --- a/rust/operator-binary/src/crd/role/mod.rs +++ b/rust/operator-binary/src/crd/role/mod.rs @@ -2,28 +2,13 @@ pub mod broker; pub mod commons; pub mod controller; -use std::{borrow::Cow, ops::Deref, str::FromStr}; +use std::{fmt, ops::Deref, str::FromStr}; use stackable_operator::{ - commons::resources::{NoRuntimeLimits, Resources}, constant, - product_logging::spec::ContainerLogConfig, - v2::{ - builder::pod::container::EnvVarName, - config_overrides::KeyValueConfigOverrides, - types::{kubernetes::ListenerClassName, operator::RoleName}, - }, -}; -use strum::{EnumIter, IntoEnumIterator}; - -use crate::{ - crd::role::{ - broker::BrokerConfig, - commons::{CommonConfig, Storage}, - controller::ControllerConfig, - }, - v1alpha1, + v2::{builder::pod::container::EnvVarName, types::operator::RoleName}, }; +use strum::EnumIter; constant!(pub KAFKA_NODE_ID_OFFSET: EnvVarName = "NODE_ID_OFFSET"); @@ -74,6 +59,16 @@ pub enum KafkaRole { Controller, } +impl fmt::Display for KafkaRole { + /// The role's name, e.g. `broker`. + /// + /// `to_string()` already worked through the [`Deref`] to [`RoleName`]; this impl exists so a + /// role can be a `{role}` field in snafu `display` strings, which require `Display` itself. + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.deref().fmt(f) + } +} + impl Deref for KafkaRole { type Target = RoleName; @@ -86,15 +81,6 @@ impl Deref for KafkaRole { } impl KafkaRole { - /// Return all available roles - pub fn roles() -> Vec { - let mut roles = vec![]; - for role in Self::iter() { - roles.push(role) - } - roles - } - /// A Kerberos principal has three parts, with the form username/fully.qualified.domain.name@YOUR-REALM.COM. /// but is similar to HBase). pub fn kerberos_service_name(&self) -> &'static str { @@ -109,89 +95,6 @@ impl KafkaRole { } } -/// Configuration for a role and rolegroup of an unknown type. -#[derive(Clone, Debug, PartialEq)] -pub enum AnyConfig { - Broker(BrokerConfig), - Controller(ControllerConfig), -} - -impl Deref for AnyConfig { - type Target = CommonConfig; - - fn deref(&self) -> &Self::Target { - match self { - AnyConfig::Broker(broker_config) => &broker_config.common_config, - AnyConfig::Controller(controller_config) => &controller_config.common_config, - } - } -} - -impl AnyConfig { - /// The [`KafkaRole`] this config belongs to. - pub fn kafka_role(&self) -> KafkaRole { - match self { - AnyConfig::Broker(_) => KafkaRole::Broker, - AnyConfig::Controller(_) => KafkaRole::Controller, - } - } - - pub fn resources(&self) -> &Resources { - match self { - AnyConfig::Broker(broker_config) => &broker_config.resources, - AnyConfig::Controller(controller_config) => &controller_config.resources, - } - } - - // Logging config is distinct between each role, due to the different enum types, - // so provide helpers for containers that are common between all roles. - pub fn kafka_logging(&'_ self) -> Cow<'_, ContainerLogConfig> { - match self { - AnyConfig::Broker(node) => node.logging.for_container(&broker::BrokerContainer::Kafka), - AnyConfig::Controller(node) => node - .logging - .for_container(&controller::ControllerContainer::Kafka), - } - } - - pub fn listener_class(&self) -> Option<&ListenerClassName> { - match self { - AnyConfig::Broker(broker_config) => Some(&broker_config.broker_listener_class), - AnyConfig::Controller(_) => None, - } - } -} - -/// Merged role/role-group `configOverrides` for a role group of an unknown type. -/// -/// Mirrors [`AnyConfig`] for the override side: broker and controller use distinct -/// override structs, so this enum lets the build layer carry the typed, merged -/// overrides through a single role-agnostic `RoleGroupConfig`. -#[derive(Clone, Debug, PartialEq)] -pub enum AnyConfigOverrides { - Broker(v1alpha1::KafkaBrokerConfigOverrides), - Controller(v1alpha1::KafkaControllerConfigOverrides), -} - -impl AnyConfigOverrides { - /// The merged config file overrides (`broker.properties` for brokers, `controller.properties` - /// for controllers). - pub fn config_file_overrides(&self) -> &KeyValueConfigOverrides { - match self { - AnyConfigOverrides::Broker(o) => &o.broker_properties, - AnyConfigOverrides::Controller(o) => &o.controller_properties, - } - } - - /// The merged `security.properties` overrides (shared by both roles). - pub fn security_properties(&self) -> &KeyValueConfigOverrides { - match self { - AnyConfigOverrides::Broker(o) => &o.security_properties, - AnyConfigOverrides::Controller(o) => &o.security_properties, - } - } -} - #[cfg(test)] mod tests { use super::*;