Skip to content

Commit bc3e8e2

Browse files
committed
refactor: add framework module and merging
1 parent 6c23361 commit bc3e8e2

10 files changed

Lines changed: 383 additions & 336 deletions

File tree

rust/operator-binary/src/config/jvm.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,17 @@ fn is_heap_jvm_argument(jvm_argument: &str) -> bool {
110110

111111
#[cfg(test)]
112112
mod tests {
113+
use stackable_operator::kube::ResourceExt;
114+
113115
use super::*;
114-
use crate::crd::{BrokerRole, role::KafkaRole, v1alpha1};
116+
use crate::{
117+
crd::{
118+
BrokerRole,
119+
role::{KafkaRole, broker::BrokerConfig},
120+
v1alpha1,
121+
},
122+
framework::role_utils::with_validated_config,
123+
};
115124

116125
#[test]
117126
fn test_construct_jvm_arguments_defaults() {
@@ -197,12 +206,12 @@ mod tests {
197206
let kafka: v1alpha1::KafkaCluster =
198207
serde_yaml::from_str(kafka_cluster).expect("illegal test input");
199208

200-
let kafka_role = KafkaRole::Broker;
201-
let rolegroup_ref = kafka.rolegroup_ref(&kafka_role, "default");
202-
let merged_config = kafka_role
203-
.merged_config(&kafka, &rolegroup_ref.role_group)
204-
.unwrap();
205-
let role = kafka.spec.brokers.unwrap();
209+
let role = kafka.spec.brokers.clone().unwrap();
210+
let role_group = role.role_groups.get("default").unwrap();
211+
let default_config =
212+
BrokerConfig::default_config(&kafka.name_any(), &KafkaRole::Broker.to_string());
213+
let validated = with_validated_config(role_group, &role, &default_config).unwrap();
214+
let merged_config = AnyConfig::Broker(validated.config);
206215

207216
(merged_config, role, "default".to_owned())
208217
}

rust/operator-binary/src/controller.rs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use crate::{
3838
self, APP_NAME, KafkaClusterStatus, KafkaPodDescriptor, MetadataManager, OPERATOR_NAME,
3939
authorization::KafkaAuthorizationConfig,
4040
listener::get_kafka_listener_config,
41-
role::{AnyConfig, KafkaRole},
41+
role::{AnyConfig, AnyConfigOverrides, KafkaRole},
4242
security::KafkaTlsSecurity,
4343
v1alpha1,
4444
},
@@ -296,20 +296,18 @@ impl Resource for ValidatedCluster {
296296
}
297297
}
298298

299-
pub struct ValidatedRoleGroupConfig {
300-
pub merged_config: AnyConfig,
301-
// DESIGN DECISION: overrides are resolved into flat maps HERE rather than stored
302-
// as the typed KeyValueConfigOverrides and resolved in the per-file builders (the
303-
// hdfs-operator pattern). Reason: broker and controller use different override
304-
// struct types (KafkaBrokerConfigOverrides vs KafkaControllerConfigOverrides), so a
305-
// single typed field would require an enum. Resolving here keeps the build/properties
306-
// builders taking plain `BTreeMap<String,String>`. Alternative: an enum over the two
307-
// override types threaded to builders that call resolved_overrides() — more types for
308-
// no behavioural gain.
309-
pub config_file_overrides: BTreeMap<String, String>,
310-
pub jvm_security_overrides: BTreeMap<String, String>,
311-
pub env_overrides: BTreeMap<String, String>,
312-
}
299+
/// A validated, merged Kafka role-group config.
300+
///
301+
/// The merged config fragment is wrapped in [`AnyConfig`] and the merged
302+
/// `configOverrides` in [`AnyConfigOverrides`], so a single role-agnostic type
303+
/// carries both broker and controller role groups (their concrete config and
304+
/// override types differ). Produced via the local-`framework`
305+
/// [`with_validated_config`](crate::framework::role_utils::with_validated_config).
306+
pub type ValidatedRoleGroupConfig = crate::framework::role_utils::RoleGroupConfig<
307+
AnyConfig,
308+
stackable_operator::role_utils::JavaCommonConfig,
309+
AnyConfigOverrides,
310+
>;
313311

314312
pub async fn reconcile_kafka(
315313
kafka: Arc<DeserializeGuard<v1alpha1::KafkaCluster>>,
@@ -434,7 +432,7 @@ pub async fn reconcile_kafka(
434432
.context(BuildStatefulsetSnafu)?,
435433
};
436434

437-
if let AnyConfig::Broker(broker_config) = &validated_rg.merged_config {
435+
if let AnyConfig::Broker(broker_config) = &validated_rg.config {
438436
let rg_bootstrap_listener = build_broker_rolegroup_bootstrap_listener(
439437
kafka,
440438
&validated_cluster,

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,12 @@ pub fn build_rolegroup_config_map(
7676
) -> Result<ConfigMap, Error> {
7777
let kafka_security = &validated_cluster.cluster_config.kafka_security;
7878
let resolved_product_image = &validated_cluster.image;
79-
let kafka_config_file_name = validated_rg.merged_config.config_file_name();
80-
let config_overrides = validated_rg.config_file_overrides.clone();
79+
let kafka_config_file_name = validated_rg.config.config_file_name();
80+
let config_overrides = validated_rg
81+
.config_overrides
82+
.config_file_overrides()
83+
.overrides
84+
.clone();
8185

8286
let opa_connect = validated_cluster
8387
.cluster_config
@@ -91,7 +95,7 @@ pub fn build_rolegroup_config_map(
9195
return NoKraftControllersFoundSnafu.fail();
9296
}
9397

94-
let kafka_config = match &validated_rg.merged_config {
98+
let kafka_config = match &validated_rg.config {
9599
AnyConfig::Broker(_) => crate::controller::build::properties::broker_properties::build(
96100
kafka_security,
97101
listener_config,
@@ -116,7 +120,10 @@ pub fn build_rolegroup_config_map(
116120
}
117121
};
118122

119-
let jvm_sec_props = &validated_rg.jvm_security_overrides;
123+
let jvm_sec_props = &validated_rg
124+
.config_overrides
125+
.security_properties()
126+
.overrides;
120127

121128
let mut cm_builder = ConfigMapBuilder::new();
122129
cm_builder
@@ -179,7 +186,7 @@ pub fn build_rolegroup_config_map(
179186
let config_data = role_group_config_map_data(
180187
&resolved_product_image.product_version,
181188
rolegroup,
182-
&validated_rg.merged_config,
189+
&validated_rg.config,
183190
);
184191
for (file_name, data) in config_data {
185192
if let Some(data) = data {

0 commit comments

Comments
 (0)