Skip to content

Commit 6c23361

Browse files
committed
refactor: introduce ValidatedClusterConfig
1 parent 662e9a1 commit 6c23361

8 files changed

Lines changed: 75 additions & 75 deletions

File tree

rust/operator-binary/src/controller.rs

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -209,44 +209,34 @@ impl ReconcilerError for Error {
209209
}
210210
}
211211

212+
pub type RoleGroupName = String;
213+
212214
/// The validated cluster. Carries everything the build steps need, resolved once
213215
/// here so downstream code never re-derives it or touches the raw spec.
214216
///
215217
/// The cluster identity (`name`, `namespace`, `uid`) is captured here so that owner
216218
/// references for child objects can be built straight from this struct (via its
217219
/// [`Resource`] impl) without threading the raw [`v1alpha1::KafkaCluster`] around.
218220
/// This mirrors the hive-/opensearch-operator's `ValidatedCluster`.
219-
pub struct ValidatedKafkaCluster {
221+
pub struct ValidatedCluster {
220222
/// `ObjectMeta` carrying `name`, `namespace` and `uid`, so this struct can act as the
221223
/// owner [`Resource`] for child objects.
222224
metadata: ObjectMeta,
223225
pub name: ClusterName,
224226
pub namespace: NamespaceName,
225227
pub image: ResolvedProductImage,
226-
pub kafka_security: KafkaTlsSecurity,
227-
// DESIGN DECISION: the dereferenced authorization config is folded into the
228-
// validated cluster (read from here downstream). The other dereferenced input,
229-
// the authentication classes, is intentionally NOT stored: it is fully consumed
230-
// here to build `kafka_security`. Alternative: also store the resolved auth
231-
// classes — rejected because nothing downstream needs them beyond kafka_security.
232-
pub authorization_config: Option<KafkaAuthorizationConfig>,
233-
pub role_groups: BTreeMap<KafkaRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
234-
pub pod_descriptors: Vec<KafkaPodDescriptor>,
235-
pub metadata_manager: MetadataManager,
228+
pub cluster_config: ValidatedClusterConfig,
229+
pub role_group_configs: BTreeMap<KafkaRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
236230
}
237231

238-
impl ValidatedKafkaCluster {
239-
#[allow(clippy::too_many_arguments)]
232+
impl ValidatedCluster {
240233
pub fn new(
241234
name: ClusterName,
242235
namespace: NamespaceName,
243236
uid: Uid,
244237
image: ResolvedProductImage,
245-
kafka_security: KafkaTlsSecurity,
246-
authorization_config: Option<KafkaAuthorizationConfig>,
247-
role_groups: BTreeMap<KafkaRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
248-
pod_descriptors: Vec<KafkaPodDescriptor>,
249-
metadata_manager: MetadataManager,
238+
cluster_config: ValidatedClusterConfig,
239+
role_group_configs: BTreeMap<KafkaRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
250240
) -> Self {
251241
Self {
252242
metadata: ObjectMeta {
@@ -258,18 +248,26 @@ impl ValidatedKafkaCluster {
258248
name,
259249
namespace,
260250
image,
261-
kafka_security,
262-
authorization_config,
263-
role_groups,
264-
pod_descriptors,
265-
metadata_manager,
251+
cluster_config,
252+
role_group_configs,
266253
}
267254
}
268255
}
269256

270-
/// Lets [`ValidatedKafkaCluster`] act as the owner [`Resource`] for child objects, so owner
257+
/// Cluster-wide settings resolved during validation and dereferencing.
258+
///
259+
/// Everything the build steps need is resolved here so they never have to read the
260+
/// raw [`v1alpha1::KafkaCluster`] spec.
261+
pub struct ValidatedClusterConfig {
262+
pub kafka_security: KafkaTlsSecurity,
263+
pub authorization_config: Option<KafkaAuthorizationConfig>,
264+
pub pod_descriptors: Vec<KafkaPodDescriptor>,
265+
pub metadata_manager: MetadataManager,
266+
}
267+
268+
/// Lets [`ValidatedCluster`] act as the owner [`Resource`] for child objects, so owner
271269
/// references are built from it (via the captured `metadata`) rather than the raw CR.
272-
impl Resource for ValidatedKafkaCluster {
270+
impl Resource for ValidatedCluster {
273271
type DynamicType = <v1alpha1::KafkaCluster as Resource>::DynamicType;
274272
type Scope = <v1alpha1::KafkaCluster as Resource>::Scope;
275273

@@ -348,10 +346,10 @@ pub async fn reconcile_kafka(
348346
.context(CreateClusterResourcesSnafu)?;
349347

350348
tracing::debug!(
351-
kerberos_enabled = validated_cluster.kafka_security.has_kerberos_enabled(),
352-
kerberos_secret_class = ?validated_cluster.kafka_security.kerberos_secret_class(),
353-
tls_enabled = validated_cluster.kafka_security.tls_enabled(),
354-
tls_client_authentication_class = ?validated_cluster.kafka_security.tls_client_authentication_class(),
349+
kerberos_enabled = validated_cluster.cluster_config.kafka_security.has_kerberos_enabled(),
350+
kerberos_secret_class = ?validated_cluster.cluster_config.kafka_security.kerberos_secret_class(),
351+
tls_enabled = validated_cluster.cluster_config.kafka_security.tls_enabled(),
352+
tls_client_authentication_class = ?validated_cluster.cluster_config.kafka_security.tls_client_authentication_class(),
355353
"The following security settings are used"
356354
);
357355

@@ -377,15 +375,15 @@ pub async fn reconcile_kafka(
377375

378376
let mut bootstrap_listeners = Vec::<listener::v1alpha1::Listener>::new();
379377

380-
for (kafka_role, rg_map) in &validated_cluster.role_groups {
378+
for (kafka_role, rg_map) in &validated_cluster.role_group_configs {
381379
for (rolegroup_name, validated_rg) in rg_map {
382380
let rolegroup_ref = kafka.rolegroup_ref(kafka_role, rolegroup_name);
383381

384382
let rg_headless_service = build_rolegroup_headless_service(
385383
&validated_cluster,
386384
&validated_cluster.image,
387385
&rolegroup_ref,
388-
&validated_cluster.kafka_security,
386+
&validated_cluster.cluster_config.kafka_security,
389387
)
390388
.context(BuildServiceSnafu)?;
391389

@@ -398,7 +396,7 @@ pub async fn reconcile_kafka(
398396

399397
let kafka_listeners = get_kafka_listener_config(
400398
kafka,
401-
&validated_cluster.kafka_security,
399+
&validated_cluster.cluster_config.kafka_security,
402400
&rolegroup_ref,
403401
&client.kubernetes_cluster_info,
404402
)

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use stackable_operator::{
88
};
99

1010
use crate::{
11-
controller::{KAFKA_CONTROLLER_NAME, ValidatedKafkaCluster, ValidatedRoleGroupConfig},
11+
controller::{KAFKA_CONTROLLER_NAME, ValidatedCluster, ValidatedRoleGroupConfig},
1212
crd::{
1313
JVM_SECURITY_PROPERTIES_FILE, MetadataManager, STACKABLE_LISTENER_BOOTSTRAP_DIR,
1414
STACKABLE_LISTENER_BROKER_DIR,
@@ -69,32 +69,33 @@ pub enum Error {
6969
/// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator
7070
pub fn build_rolegroup_config_map(
7171
kafka: &v1alpha1::KafkaCluster,
72-
validated_cluster: &ValidatedKafkaCluster,
72+
validated_cluster: &ValidatedCluster,
7373
rolegroup: &RoleGroupRef<v1alpha1::KafkaCluster>,
7474
validated_rg: &ValidatedRoleGroupConfig,
7575
listener_config: &KafkaListenerConfig,
7676
) -> Result<ConfigMap, Error> {
77-
let kafka_security = &validated_cluster.kafka_security;
77+
let kafka_security = &validated_cluster.cluster_config.kafka_security;
7878
let resolved_product_image = &validated_cluster.image;
7979
let kafka_config_file_name = validated_rg.merged_config.config_file_name();
8080
let config_overrides = validated_rg.config_file_overrides.clone();
8181

8282
let opa_connect = validated_cluster
83+
.cluster_config
8384
.authorization_config
8485
.as_ref()
8586
.map(|auth_config| auth_config.opa_connect.clone());
8687

87-
let kraft_mode = validated_cluster.metadata_manager == MetadataManager::KRaft;
88+
let kraft_mode = validated_cluster.cluster_config.metadata_manager == MetadataManager::KRaft;
8889

89-
if kraft_mode && validated_cluster.pod_descriptors.is_empty() {
90+
if kraft_mode && validated_cluster.cluster_config.pod_descriptors.is_empty() {
9091
return NoKraftControllersFoundSnafu.fail();
9192
}
9293

9394
let kafka_config = match &validated_rg.merged_config {
9495
AnyConfig::Broker(_) => crate::controller::build::properties::broker_properties::build(
9596
kafka_security,
9697
listener_config,
97-
&validated_cluster.pod_descriptors,
98+
&validated_cluster.cluster_config.pod_descriptors,
9899
opa_connect.as_deref(),
99100
kraft_mode,
100101
kafka
@@ -108,7 +109,7 @@ pub fn build_rolegroup_config_map(
108109
crate::controller::build::properties::controller_properties::build(
109110
kafka_security,
110111
listener_config,
111-
&validated_cluster.pod_descriptors,
112+
&validated_cluster.cluster_config.pod_descriptors,
112113
kraft_mode,
113114
config_overrides,
114115
)

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use stackable_operator::{
99
};
1010

1111
use crate::{
12-
controller::{KAFKA_CONTROLLER_NAME, ValidatedKafkaCluster},
12+
controller::{KAFKA_CONTROLLER_NAME, ValidatedCluster},
1313
crd::{role::KafkaRole, v1alpha1},
1414
utils::build_recommended_labels,
1515
};
@@ -42,10 +42,10 @@ pub enum Error {
4242
/// Build a discovery [`ConfigMap`] containing information about how to connect to a certain
4343
/// [`v1alpha1::KafkaCluster`].
4444
pub fn build_discovery_configmap(
45-
validated_cluster: &ValidatedKafkaCluster,
45+
validated_cluster: &ValidatedCluster,
4646
listeners: &[listener::v1alpha1::Listener],
4747
) -> Result<ConfigMap, Error> {
48-
let kafka_security = &validated_cluster.kafka_security;
48+
let kafka_security = &validated_cluster.cluster_config.kafka_security;
4949
let resolved_product_image = &validated_cluster.image;
5050

5151
let port_name = if kafka_security.has_kerberos_enabled() {
@@ -87,7 +87,7 @@ pub fn build_discovery_configmap(
8787

8888
/// An [`ObjectRef`] to the owning cluster, built from the validated identity — used only for
8989
/// error context.
90-
fn cluster_object_ref(cluster: &ValidatedKafkaCluster) -> ObjectRef<v1alpha1::KafkaCluster> {
90+
fn cluster_object_ref(cluster: &ValidatedCluster) -> ObjectRef<v1alpha1::KafkaCluster> {
9191
ObjectRef::new(cluster.name.as_ref()).within(cluster.namespace.as_ref())
9292
}
9393

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

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! The validate step in the KafkaCluster controller.
22
//!
33
//! Synchronously validates inputs that don't require a Kubernetes client. Produces
4-
//! [`ValidatedKafkaCluster`], consumed by the rest of `reconcile_kafka`.
4+
//! [`ValidatedCluster`], consumed by the rest of `reconcile_kafka`.
55
66
use std::{collections::BTreeMap, str::FromStr};
77

@@ -22,7 +22,8 @@ use stackable_operator::{
2222

2323
use crate::{
2424
controller::{
25-
ValidatedKafkaCluster, ValidatedRoleGroupConfig, dereference::DereferencedObjects,
25+
RoleGroupName, ValidatedCluster, ValidatedClusterConfig, ValidatedRoleGroupConfig,
26+
dereference::DereferencedObjects,
2627
},
2728
crd::{
2829
self, CONTAINER_IMAGE_BASE_NAME,
@@ -87,7 +88,7 @@ pub fn validate(
8788
kafka: &v1alpha1::KafkaCluster,
8889
dereferenced_objects: DereferencedObjects,
8990
operator_environment: &OperatorEnvironmentOptions,
90-
) -> Result<ValidatedKafkaCluster> {
91+
) -> Result<ValidatedCluster> {
9192
let image = kafka
9293
.spec
9394
.image
@@ -115,13 +116,10 @@ pub fn validate(
115116
.validate_authentication_methods()
116117
.context(FailedToValidateAuthenticationMethodSnafu)?;
117118

118-
// DESIGN DECISION: build the per-rolegroup config (merged config + resolved overrides)
119-
// here, so reconcile reads a fully-typed ValidatedKafkaCluster instead of re-deriving
120-
// merged_config in the loop and threading a product-config HashMap. Alternative: keep
121-
// deriving merged_config in the reconcile loop — rejected; validation is the right place
122-
// to prove every rolegroup resolves before any resource is built.
123-
let mut role_groups: BTreeMap<KafkaRole, BTreeMap<String, ValidatedRoleGroupConfig>> =
124-
BTreeMap::new();
119+
let mut role_group_configs: BTreeMap<
120+
KafkaRole,
121+
BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>,
122+
> = BTreeMap::new();
125123

126124
// Brokers always exist.
127125
let broker_role = kafka
@@ -131,7 +129,7 @@ pub fn validate(
131129
role: KafkaRole::Broker,
132130
})?;
133131

134-
let mut broker_groups: BTreeMap<String, ValidatedRoleGroupConfig> = BTreeMap::new();
132+
let mut broker_groups: BTreeMap<RoleGroupName, ValidatedRoleGroupConfig> = BTreeMap::new();
135133
for rolegroup_name in broker_role.role_groups.keys() {
136134
let merged_config = KafkaRole::Broker
137135
.merged_config(kafka, rolegroup_name)
@@ -148,7 +146,7 @@ pub fn validate(
148146
},
149147
);
150148
}
151-
role_groups.insert(KafkaRole::Broker, broker_groups);
149+
role_group_configs.insert(KafkaRole::Broker, broker_groups);
152150

153151
// We need this guard because controller_role() returns an error if controllers is None,
154152
// which would stop reconciliation for ZooKeeper-mode clusters.
@@ -160,7 +158,8 @@ pub fn validate(
160158
role: KafkaRole::Controller,
161159
})?;
162160

163-
let mut controller_groups: BTreeMap<String, ValidatedRoleGroupConfig> = BTreeMap::new();
161+
let mut controller_groups: BTreeMap<RoleGroupName, ValidatedRoleGroupConfig> =
162+
BTreeMap::new();
164163
for rolegroup_name in controller_role.role_groups.keys() {
165164
let merged_config = KafkaRole::Controller
166165
.merged_config(kafka, rolegroup_name)
@@ -177,7 +176,7 @@ pub fn validate(
177176
},
178177
);
179178
}
180-
role_groups.insert(KafkaRole::Controller, controller_groups);
179+
role_group_configs.insert(KafkaRole::Controller, controller_groups);
181180
}
182181

183182
let pod_descriptors = kafka
@@ -197,16 +196,18 @@ pub fn validate(
197196
.context(InvalidNamespaceSnafu)?;
198197
let uid = Uid::from_str(&kafka.uid().context(ObjectHasNoUidSnafu)?).context(InvalidUidSnafu)?;
199198

200-
Ok(ValidatedKafkaCluster::new(
199+
Ok(ValidatedCluster::new(
201200
name,
202201
namespace,
203202
uid,
204203
image,
205-
kafka_security,
206-
dereferenced_objects.authorization_config,
207-
role_groups,
208-
pod_descriptors,
209-
metadata_manager,
204+
ValidatedClusterConfig {
205+
kafka_security,
206+
authorization_config: dereferenced_objects.authorization_config,
207+
pod_descriptors,
208+
metadata_manager,
209+
},
210+
role_group_configs,
210211
))
211212
}
212213

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use stackable_operator::{
44
};
55

66
use crate::{
7-
controller::{KAFKA_CONTROLLER_NAME, ValidatedKafkaCluster},
7+
controller::{KAFKA_CONTROLLER_NAME, ValidatedCluster},
88
crd::{role::broker::BrokerConfig, security::KafkaTlsSecurity, v1alpha1},
99
utils::build_recommended_labels,
1010
};
@@ -27,11 +27,11 @@ pub enum Error {
2727
// TODO (@NickLarsenNZ): Move shared functionality to stackable-operator
2828
pub fn build_broker_rolegroup_bootstrap_listener(
2929
kafka: &v1alpha1::KafkaCluster,
30-
validated_cluster: &ValidatedKafkaCluster,
30+
validated_cluster: &ValidatedCluster,
3131
rolegroup: &RoleGroupRef<v1alpha1::KafkaCluster>,
3232
merged_config: &BrokerConfig,
3333
) -> Result<listener::v1alpha1::Listener, Error> {
34-
let kafka_security = &validated_cluster.kafka_security;
34+
let kafka_security = &validated_cluster.cluster_config.kafka_security;
3535
let resolved_product_image = &validated_cluster.image;
3636

3737
Ok(listener::v1alpha1::Listener {

rust/operator-binary/src/resource/service.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use stackable_operator::{
88
};
99

1010
use crate::{
11-
controller::{KAFKA_CONTROLLER_NAME, ValidatedKafkaCluster},
11+
controller::{KAFKA_CONTROLLER_NAME, ValidatedCluster},
1212
crd::{APP_NAME, METRICS_PORT, METRICS_PORT_NAME, security::KafkaTlsSecurity, v1alpha1},
1313
utils::build_recommended_labels,
1414
};
@@ -35,7 +35,7 @@ pub enum Error {
3535
///
3636
/// This is mostly useful for internal communication between peers, or for clients that perform client-side load balancing.
3737
pub fn build_rolegroup_headless_service(
38-
validated_cluster: &ValidatedKafkaCluster,
38+
validated_cluster: &ValidatedCluster,
3939
resolved_product_image: &ResolvedProductImage,
4040
rolegroup: &RoleGroupRef<v1alpha1::KafkaCluster>,
4141
kafka_security: &KafkaTlsSecurity,
@@ -77,7 +77,7 @@ pub fn build_rolegroup_headless_service(
7777

7878
/// The rolegroup metrics [`Service`] is a service that exposes metrics and a prometheus scraping label
7979
pub fn build_rolegroup_metrics_service(
80-
validated_cluster: &ValidatedKafkaCluster,
80+
validated_cluster: &ValidatedCluster,
8181
resolved_product_image: &ResolvedProductImage,
8282
rolegroup: &RoleGroupRef<v1alpha1::KafkaCluster>,
8383
) -> Result<Service, Error> {

0 commit comments

Comments
 (0)