Skip to content

Commit fc27750

Browse files
committed
pass validate cluster and rg rather than multiple parameters
1 parent cfd0efd commit fc27750

6 files changed

Lines changed: 112 additions & 113 deletions

File tree

rust/operator-binary/src/controller.rs

Lines changed: 64 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
//! Ensures that `Pod`s are configured and running for each [`v1alpha1::KafkaCluster`].
22
3-
use std::sync::Arc;
3+
use std::{collections::BTreeMap, sync::Arc};
44

55
use const_format::concatcp;
66
use snafu::{ResultExt, Snafu};
77
use stackable_operator::{
88
cli::OperatorEnvironmentOptions,
99
cluster_resources::{ClusterResourceApplyStrategy, ClusterResources},
10-
commons::rbac::build_rbac_resources,
10+
commons::{product_image_selection::ResolvedProductImage, rbac::build_rbac_resources},
1111
crd::listener,
1212
kube::{
1313
Resource,
@@ -32,8 +32,10 @@ mod validate;
3232
use crate::{
3333
crd::{
3434
self, APP_NAME, KafkaClusterStatus, OPERATOR_NAME,
35+
authorization::KafkaAuthorizationConfig,
3536
listener::get_kafka_listener_config,
3637
role::{AnyConfig, KafkaRole},
38+
security::KafkaTlsSecurity,
3739
v1alpha1,
3840
},
3941
discovery::{self, build_discovery_configmap},
@@ -208,6 +210,35 @@ impl ReconcilerError for Error {
208210
}
209211
}
210212

213+
/// The validated cluster. Carries everything the build steps need, resolved once
214+
/// here so downstream code never re-derives it or touches the raw spec.
215+
pub struct ValidatedKafkaCluster {
216+
pub image: ResolvedProductImage,
217+
pub kafka_security: KafkaTlsSecurity,
218+
// DESIGN DECISION: the dereferenced authorization config is folded into the
219+
// validated cluster (read from here downstream). The other dereferenced input,
220+
// the authentication classes, is intentionally NOT stored: it is fully consumed
221+
// here to build `kafka_security`. Alternative: also store the resolved auth
222+
// classes — rejected because nothing downstream needs them beyond kafka_security.
223+
pub authorization_config: Option<KafkaAuthorizationConfig>,
224+
pub role_groups: BTreeMap<KafkaRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
225+
}
226+
227+
pub struct ValidatedRoleGroupConfig {
228+
pub merged_config: AnyConfig,
229+
// DESIGN DECISION: overrides are resolved into flat maps HERE rather than stored
230+
// as the typed KeyValueConfigOverrides and resolved in the per-file builders (the
231+
// hdfs-operator pattern). Reason: broker and controller use different override
232+
// struct types (KafkaBrokerConfigOverrides vs KafkaControllerConfigOverrides), so a
233+
// single typed field would require an enum. Resolving here keeps the build/properties
234+
// builders taking plain `BTreeMap<String,String>`. Alternative: an enum over the two
235+
// override types threaded to builders that call resolved_overrides() — more types for
236+
// no behavioural gain.
237+
pub config_file_overrides: BTreeMap<String, String>,
238+
pub jvm_security_overrides: BTreeMap<String, String>,
239+
pub env_overrides: BTreeMap<String, String>,
240+
}
241+
211242
pub async fn reconcile_kafka(
212243
kafka: Arc<DeserializeGuard<v1alpha1::KafkaCluster>>,
213244
ctx: Arc<Ctx>,
@@ -228,15 +259,12 @@ pub async fn reconcile_kafka(
228259
.context(DereferenceSnafu)?;
229260

230261
// validate (no client required)
231-
let validate::ValidatedKafkaCluster {
232-
authorization_config,
233-
image,
234-
kafka_security,
235-
role_groups,
236-
} = validate::validate(kafka, dereferenced_objects, &ctx.operator_environment)
237-
.context(ValidateClusterSnafu)?;
238-
239-
let opa_connect = authorization_config
262+
let validated_cluster =
263+
validate::validate(kafka, dereferenced_objects, &ctx.operator_environment)
264+
.context(ValidateClusterSnafu)?;
265+
266+
let opa_connect = validated_cluster
267+
.authorization_config
240268
.as_ref()
241269
.map(|auth_config| auth_config.opa_connect.clone());
242270

@@ -251,10 +279,10 @@ pub async fn reconcile_kafka(
251279
.context(CreateClusterResourcesSnafu)?;
252280

253281
tracing::debug!(
254-
kerberos_enabled = kafka_security.has_kerberos_enabled(),
255-
kerberos_secret_class = ?kafka_security.kerberos_secret_class(),
256-
tls_enabled = kafka_security.tls_enabled(),
257-
tls_client_authentication_class = ?kafka_security.tls_client_authentication_class(),
282+
kerberos_enabled = validated_cluster.kafka_security.has_kerberos_enabled(),
283+
kerberos_secret_class = ?validated_cluster.kafka_security.kerberos_secret_class(),
284+
tls_enabled = validated_cluster.kafka_security.tls_enabled(),
285+
tls_client_authentication_class = ?validated_cluster.kafka_security.tls_client_authentication_class(),
258286
"The following security settings are used"
259287
);
260288

@@ -280,20 +308,25 @@ pub async fn reconcile_kafka(
280308

281309
let mut bootstrap_listeners = Vec::<listener::v1alpha1::Listener>::new();
282310

283-
for (kafka_role, rg_map) in &role_groups {
311+
for (kafka_role, rg_map) in &validated_cluster.role_groups {
284312
for (rolegroup_name, validated_rg) in rg_map {
285313
let rolegroup_ref = kafka.rolegroup_ref(kafka_role, rolegroup_name);
286314

287-
let rg_headless_service =
288-
build_rolegroup_headless_service(kafka, &image, &rolegroup_ref, &kafka_security)
289-
.context(BuildServiceSnafu)?;
315+
let rg_headless_service = build_rolegroup_headless_service(
316+
kafka,
317+
&validated_cluster.image,
318+
&rolegroup_ref,
319+
&validated_cluster.kafka_security,
320+
)
321+
.context(BuildServiceSnafu)?;
290322

291-
let rg_metrics_service = build_rolegroup_metrics_service(kafka, &image, &rolegroup_ref)
292-
.context(BuildServiceSnafu)?;
323+
let rg_metrics_service =
324+
build_rolegroup_metrics_service(kafka, &validated_cluster.image, &rolegroup_ref)
325+
.context(BuildServiceSnafu)?;
293326

294327
let kafka_listeners = get_kafka_listener_config(
295328
kafka,
296-
&kafka_security,
329+
&validated_cluster.kafka_security,
297330
&rolegroup_ref,
298331
&client.kubernetes_cluster_info,
299332
)
@@ -303,18 +336,15 @@ pub async fn reconcile_kafka(
303336
.pod_descriptors(
304337
None,
305338
&client.kubernetes_cluster_info,
306-
kafka_security.client_port(),
339+
validated_cluster.kafka_security.client_port(),
307340
)
308341
.context(BuildPodDescriptorsSnafu)?;
309342

310343
let rg_configmap = build::config_map::build_rolegroup_config_map(
311344
kafka,
312-
&image,
313-
&kafka_security,
345+
&validated_cluster,
314346
&rolegroup_ref,
315-
validated_rg.config_file_overrides.clone(),
316-
validated_rg.jvm_security_overrides.clone(),
317-
&validated_rg.merged_config,
347+
validated_rg,
318348
&kafka_listeners,
319349
&pod_descriptors,
320350
opa_connect.as_deref(),
@@ -325,23 +355,19 @@ pub async fn reconcile_kafka(
325355
KafkaRole::Broker => build_broker_rolegroup_statefulset(
326356
kafka,
327357
kafka_role,
328-
&image,
358+
&validated_cluster,
329359
&rolegroup_ref,
330-
&validated_rg.env_overrides,
331-
&kafka_security,
332-
&validated_rg.merged_config,
360+
validated_rg,
333361
&rbac_sa,
334362
&client.kubernetes_cluster_info,
335363
)
336364
.context(BuildStatefulsetSnafu)?,
337365
KafkaRole::Controller => build_controller_rolegroup_statefulset(
338366
kafka,
339367
kafka_role,
340-
&image,
368+
&validated_cluster,
341369
&rolegroup_ref,
342-
&validated_rg.env_overrides,
343-
&kafka_security,
344-
&validated_rg.merged_config,
370+
validated_rg,
345371
&rbac_sa,
346372
&client.kubernetes_cluster_info,
347373
)
@@ -351,8 +377,7 @@ pub async fn reconcile_kafka(
351377
if let AnyConfig::Broker(broker_config) = &validated_rg.merged_config {
352378
let rg_bootstrap_listener = build_broker_rolegroup_bootstrap_listener(
353379
kafka,
354-
&image,
355-
&kafka_security,
380+
&validated_cluster,
356381
&rolegroup_ref,
357382
broker_config,
358383
)
@@ -409,7 +434,7 @@ pub async fn reconcile_kafka(
409434
}
410435

411436
let discovery_cm =
412-
build_discovery_configmap(kafka, kafka, &image, &kafka_security, &bootstrap_listeners)
437+
build_discovery_configmap(kafka, kafka, validated_cluster, &bootstrap_listeners)
413438
.context(BuildDiscoveryConfigSnafu)?;
414439

415440
cluster_resources

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,18 @@ use indoc::formatdoc;
44
use snafu::{ResultExt, Snafu};
55
use stackable_operator::{
66
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
7-
commons::product_image_selection::ResolvedProductImage,
87
k8s_openapi::api::core::v1::ConfigMap,
98
role_utils::RoleGroupRef,
109
v2::config_file_writer::{PropertiesWriterError, to_java_properties_string},
1110
};
1211

1312
use crate::{
14-
controller::KAFKA_CONTROLLER_NAME,
13+
controller::{KAFKA_CONTROLLER_NAME, ValidatedKafkaCluster, ValidatedRoleGroupConfig},
1514
crd::{
1615
JVM_SECURITY_PROPERTIES_FILE, KafkaPodDescriptor, MetadataManager,
1716
STACKABLE_LISTENER_BOOTSTRAP_DIR, STACKABLE_LISTENER_BROKER_DIR,
1817
listener::{KafkaListenerConfig, node_address_cmd},
1918
role::AnyConfig,
20-
security::KafkaTlsSecurity,
2119
v1alpha1,
2220
},
2321
product_logging::extend_role_group_config_map,
@@ -74,23 +72,23 @@ pub enum Error {
7472
#[allow(clippy::too_many_arguments)]
7573
pub fn build_rolegroup_config_map(
7674
kafka: &v1alpha1::KafkaCluster,
77-
resolved_product_image: &ResolvedProductImage,
78-
kafka_security: &KafkaTlsSecurity,
75+
validated_cluster: &ValidatedKafkaCluster,
7976
rolegroup: &RoleGroupRef<v1alpha1::KafkaCluster>,
80-
config_file_overrides: BTreeMap<String, String>,
81-
jvm_security_overrides: BTreeMap<String, String>,
82-
merged_config: &AnyConfig,
77+
validated_rg: &ValidatedRoleGroupConfig,
8378
listener_config: &KafkaListenerConfig,
8479
pod_descriptors: &[KafkaPodDescriptor],
8580
opa_connect_string: Option<&str>,
8681
) -> Result<ConfigMap, Error> {
87-
let kafka_config_file_name = merged_config.config_file_name();
82+
let kafka_security = &validated_cluster.kafka_security;
83+
let resolved_product_image = &validated_cluster.image;
84+
let kafka_config_file_name = validated_rg.merged_config.config_file_name();
85+
let config_overrides = validated_rg.config_file_overrides.clone();
8886

8987
let metadata_manager = kafka
9088
.effective_metadata_manager()
9189
.context(InvalidMetadataManagerSnafu)?;
9290

93-
let kafka_config = match merged_config {
91+
let kafka_config = match &validated_rg.merged_config {
9492
AnyConfig::Broker(_) => crate::controller::build::properties::broker_properties::build(
9593
kafka_security,
9694
listener_config,
@@ -102,15 +100,15 @@ pub fn build_rolegroup_config_map(
102100
.cluster_config
103101
.broker_id_pod_config_map_name
104102
.is_some(),
105-
config_file_overrides,
103+
config_overrides,
106104
),
107105
AnyConfig::Controller(_) => {
108106
crate::controller::build::properties::controller_properties::build(
109107
kafka_security,
110108
listener_config,
111109
pod_descriptors,
112110
metadata_manager == MetadataManager::KRaft,
113-
config_file_overrides,
111+
config_overrides,
114112
)
115113
}
116114
}
@@ -123,7 +121,9 @@ pub fn build_rolegroup_config_map(
123121
.map(|(k, v)| (k, Some(v)))
124122
.collect::<Vec<_>>();
125123

126-
let jvm_sec_props: BTreeMap<String, Option<String>> = jvm_security_overrides
124+
let jvm_sec_props: BTreeMap<String, Option<String>> = validated_rg
125+
.jvm_security_overrides
126+
.clone()
127127
.into_iter()
128128
.map(|(k, v)| (k, Some(v)))
129129
.collect();
@@ -189,7 +189,7 @@ pub fn build_rolegroup_config_map(
189189
extend_role_group_config_map(
190190
&resolved_product_image.product_version,
191191
rolegroup,
192-
merged_config,
192+
&validated_rg.merged_config,
193193
&mut cm_builder,
194194
);
195195

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

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,16 @@
66
use std::collections::BTreeMap;
77

88
use snafu::{ResultExt, Snafu};
9-
use stackable_operator::{
10-
cli::OperatorEnvironmentOptions,
11-
commons::product_image_selection::{self, ResolvedProductImage},
12-
};
9+
use stackable_operator::{cli::OperatorEnvironmentOptions, commons::product_image_selection};
1310

1411
use crate::{
15-
controller::dereference::DereferencedObjects,
12+
controller::{
13+
ValidatedKafkaCluster, ValidatedRoleGroupConfig, dereference::DereferencedObjects,
14+
},
1615
crd::{
1716
self, CONTAINER_IMAGE_BASE_NAME,
1817
authentication::{self},
19-
authorization::KafkaAuthorizationConfig,
20-
role::{AnyConfig, KafkaRole},
18+
role::KafkaRole,
2119
security::{self, KafkaTlsSecurity},
2220
v1alpha1,
2321
},
@@ -45,35 +43,6 @@ pub enum Error {
4543

4644
type Result<T, E = Error> = std::result::Result<T, E>;
4745

48-
/// The validated cluster. Carries everything the build steps need, resolved once
49-
/// here so downstream code never re-derives it or touches the raw spec.
50-
pub struct ValidatedKafkaCluster {
51-
pub image: ResolvedProductImage,
52-
pub kafka_security: KafkaTlsSecurity,
53-
// DESIGN DECISION: the dereferenced authorization config is folded into the
54-
// validated cluster (read from here downstream). The other dereferenced input,
55-
// the authentication classes, is intentionally NOT stored: it is fully consumed
56-
// here to build `kafka_security`. Alternative: also store the resolved auth
57-
// classes — rejected because nothing downstream needs them beyond kafka_security.
58-
pub authorization_config: Option<KafkaAuthorizationConfig>,
59-
pub role_groups: BTreeMap<KafkaRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
60-
}
61-
62-
pub struct ValidatedRoleGroupConfig {
63-
pub merged_config: AnyConfig,
64-
// DESIGN DECISION: overrides are resolved into flat maps HERE rather than stored
65-
// as the typed KeyValueConfigOverrides and resolved in the per-file builders (the
66-
// hdfs-operator pattern). Reason: broker and controller use different override
67-
// struct types (KafkaBrokerConfigOverrides vs KafkaControllerConfigOverrides), so a
68-
// single typed field would require an enum. Resolving here keeps the build/properties
69-
// builders taking plain `BTreeMap<String,String>`. Alternative: an enum over the two
70-
// override types threaded to builders that call resolved_overrides() — more types for
71-
// no behavioural gain.
72-
pub config_file_overrides: BTreeMap<String, String>,
73-
pub jvm_security_overrides: BTreeMap<String, String>,
74-
pub env_overrides: BTreeMap<String, String>,
75-
}
76-
7746
/// Validates the cluster spec and the dereferenced inputs.
7847
pub fn validate(
7948
kafka: &v1alpha1::KafkaCluster,

rust/operator-binary/src/discovery.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@ use std::num::TryFromIntError;
33
use snafu::{OptionExt, ResultExt, Snafu};
44
use stackable_operator::{
55
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
6-
commons::product_image_selection::ResolvedProductImage,
76
crd::listener,
87
k8s_openapi::api::core::v1::ConfigMap,
98
kube::{Resource, ResourceExt, runtime::reflector::ObjectRef},
109
};
1110

1211
use crate::{
13-
controller::KAFKA_CONTROLLER_NAME,
14-
crd::{role::KafkaRole, security::KafkaTlsSecurity, v1alpha1},
12+
controller::{KAFKA_CONTROLLER_NAME, ValidatedKafkaCluster},
13+
crd::{role::KafkaRole, v1alpha1},
1514
utils::build_recommended_labels,
1615
};
1716

@@ -48,10 +47,12 @@ pub enum Error {
4847
pub fn build_discovery_configmap(
4948
kafka: &v1alpha1::KafkaCluster,
5049
owner: &impl Resource<DynamicType = ()>,
51-
resolved_product_image: &ResolvedProductImage,
52-
kafka_security: &KafkaTlsSecurity,
50+
validated_cluster: ValidatedKafkaCluster,
5351
listeners: &[listener::v1alpha1::Listener],
5452
) -> Result<ConfigMap, Error> {
53+
let kafka_security = &validated_cluster.kafka_security;
54+
let resolved_product_image = &validated_cluster.image;
55+
5556
let port_name = if kafka_security.has_kerberos_enabled() {
5657
kafka_security.bootstrap_port_name()
5758
} else {

0 commit comments

Comments
 (0)