Skip to content

Commit 8f6ee27

Browse files
maltesanderclaude
andcommitted
refactor: Decouple config-file builders from the HiveCluster spec
Resolve the JDBC driver, Kerberos hive-site entries and the core-site decision into ValidatedClusterConfig during validation, so hive_site/core_site builders consume only ValidatedClusterConfig. config_map keeps an explicit `owner: &HiveCluster` solely for the ObjectMeta owner reference. Rename the reconcile binding `validated` -> `validated_cluster`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1f10eca commit 8f6ee27

6 files changed

Lines changed: 146 additions & 218 deletions

File tree

rust/operator-binary/src/controller.rs

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,20 @@ pub struct ValidatedCluster {
322322
}
323323

324324
/// Cluster-wide settings resolved during validation and dereferencing.
325+
///
326+
/// Everything the config-file builders need is resolved here so they never have to
327+
/// read the raw [`v1alpha1::HiveCluster`] spec.
325328
pub struct ValidatedClusterConfig {
326329
pub metadata_database_connection_details: JdbcDatabaseConnectionDetails,
330+
/// The resolved JDBC driver class (Derby version special-casing already applied).
331+
pub connection_driver: String,
327332
pub s3_connection_spec: Option<s3::v1alpha1::ConnectionSpec>,
328333
pub hive_opa_config: Option<HiveOpaConfig>,
334+
/// Kerberos-related `hive-site.xml` entries (empty when Kerberos is disabled).
335+
pub kerberos_config: BTreeMap<String, String>,
336+
/// Whether a `core-site.xml` with `hadoop.security.authentication=kerberos` is
337+
/// required (Kerberos enabled and no HDFS backend).
338+
pub needs_kerberos_core_site: bool,
329339
}
330340

331341
/// Per-role configuration extracted during validation.
@@ -352,9 +362,11 @@ pub async fn reconcile_hive(
352362
.await
353363
.context(DereferenceSnafu)?;
354364

355-
let validated = validate::validate_cluster(
365+
let validated_cluster = validate::validate_cluster(
356366
hive,
357367
&ctx.operator_environment.image_repository,
368+
&hive_namespace,
369+
&client.kubernetes_cluster_info,
358370
dereferenced_objects,
359371
)
360372
.context(ValidateSnafu)?;
@@ -390,24 +402,22 @@ pub async fn reconcile_hive(
390402

391403
let mut ss_cond_builder = StatefulSetConditionBuilder::default();
392404

393-
for (hive_role, role_group_configs) in &validated.role_group_configs {
405+
for (hive_role, role_group_configs) in &validated_cluster.role_group_configs {
394406
for (rolegroup_name, rg) in role_group_configs {
395407
let rolegroup = hive.metastore_rolegroup_ref(rolegroup_name);
396408

397409
let rg_metrics_service =
398-
build_rolegroup_metrics_service(hive, &validated.image, &rolegroup)
410+
build_rolegroup_metrics_service(hive, &validated_cluster.image, &rolegroup)
399411
.context(ServiceConfigurationSnafu)?;
400412

401413
let rg_headless_service =
402-
build_rolegroup_headless_service(hive, &validated.image, &rolegroup)
414+
build_rolegroup_headless_service(hive, &validated_cluster.image, &rolegroup)
403415
.context(ServiceConfigurationSnafu)?;
404416

405417
let rg_configmap = build::config_map::build_metastore_rolegroup_config_map(
406418
hive,
407-
&hive_namespace,
408-
&validated,
419+
&validated_cluster,
409420
&rolegroup,
410-
&client.kubernetes_cluster_info,
411421
)
412422
.with_context(|_| BuildRoleGroupConfigMapSnafu {
413423
rolegroup: rolegroup.clone(),
@@ -416,7 +426,7 @@ pub async fn reconcile_hive(
416426
let rg_statefulset = build_metastore_rolegroup_statefulset(
417427
hive,
418428
hive_role,
419-
&validated,
429+
&validated_cluster,
420430
&rolegroup,
421431
rg,
422432
&rbac_sa.name_any(),
@@ -460,7 +470,7 @@ pub async fn reconcile_hive(
460470
// We don't /need/ stability, but it's still nice to avoid spurious changes where possible.
461471
let mut discovery_hash = FnvHasher::with_key(0);
462472

463-
if let Some(role_config) = validated.role_config {
473+
if let Some(role_config) = validated_cluster.role_config {
464474
add_pdbs(
465475
&role_config.pdb,
466476
hive,
@@ -473,7 +483,7 @@ pub async fn reconcile_hive(
473483

474484
let role_listener: Listener = build_role_listener(
475485
hive,
476-
&validated.image,
486+
&validated_cluster.image,
477487
&HiveRole::MetaStore,
478488
&role_config.listener_class,
479489
)
@@ -488,7 +498,7 @@ pub async fn reconcile_hive(
488498
hive,
489499
hive,
490500
HiveRole::MetaStore,
491-
&validated.image,
501+
&validated_cluster.image,
492502
None,
493503
listener,
494504
)

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

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use stackable_operator::{
55
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
66
k8s_openapi::api::core::v1::ConfigMap,
77
role_utils::RoleGroupRef,
8-
utils::cluster_info::KubernetesClusterInfo,
98
};
109

1110
use crate::{
@@ -60,11 +59,9 @@ type Result<T, E = Error> = std::result::Result<T, E>;
6059
/// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the
6160
/// administrator.
6261
pub fn build_metastore_rolegroup_config_map(
63-
hive: &v1alpha1::HiveCluster,
64-
hive_namespace: &str,
62+
owner: &v1alpha1::HiveCluster,
6563
cluster: &ValidatedCluster,
6664
rolegroup: &RoleGroupRef<v1alpha1::HiveCluster>,
67-
cluster_info: &KubernetesClusterInfo,
6865
) -> Result<ConfigMap> {
6966
let rg = cluster
7067
.role_group_configs
@@ -77,14 +74,9 @@ pub fn build_metastore_rolegroup_config_map(
7774
// hive-site.xml
7875
let hive_site_overrides = resolved_overrides(rg.config_overrides.hive_site_xml.clone());
7976
let hive_site_data = hive_site::build(
80-
hive,
81-
hive_namespace,
77+
&cluster.cluster_config,
8278
&cluster.image.product_version,
8379
&rg.config,
84-
&cluster.cluster_config.metadata_database_connection_details,
85-
cluster.cluster_config.s3_connection_spec.as_ref(),
86-
cluster.cluster_config.hive_opa_config.as_ref(),
87-
cluster_info,
8880
hive_site_overrides,
8981
)
9082
.context(BuildHiveSiteSnafu)?;
@@ -97,12 +89,12 @@ pub fn build_metastore_rolegroup_config_map(
9789
cm_builder
9890
.metadata(
9991
ObjectMetaBuilder::new()
100-
.name_and_namespace(hive)
92+
.name_and_namespace(owner)
10193
.name(rolegroup.object_name())
102-
.ownerreference_from_resource(hive, None, Some(true))
94+
.ownerreference_from_resource(owner, None, Some(true))
10395
.context(ObjectMissingMetadataForOwnerRefSnafu)?
10496
.with_recommended_labels(&build_recommended_labels(
105-
hive,
97+
owner,
10698
&cluster.image.app_version_label_value,
10799
&rolegroup.role,
108100
&rolegroup.role_group,
@@ -118,7 +110,7 @@ pub fn build_metastore_rolegroup_config_map(
118110
);
119111

120112
// core-site.xml is only required when Kerberos is enabled without an HDFS backend.
121-
if let Some(core_site_data) = core_site::build(hive) {
113+
if let Some(core_site_data) = core_site::build(&cluster.cluster_config) {
122114
cm_builder.add_data(CORE_SITE_XML, to_hadoop_xml(core_site_data.iter()));
123115
}
124116

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,59 @@ pub(crate) fn resolved_overrides(overrides: KeyValueConfigOverrides) -> BTreeMap
1717
.filter_map(|(key, value)| value.map(|value| (key, value)))
1818
.collect()
1919
}
20+
21+
#[cfg(test)]
22+
pub(crate) mod test_support {
23+
use std::collections::BTreeMap;
24+
25+
use crate::{
26+
controller::ValidatedClusterConfig,
27+
crd::{
28+
databases::{MetadataDatabaseConnection, derby_driver_class},
29+
v1alpha1,
30+
},
31+
};
32+
33+
pub const DERBY_YAML: &str = r#"
34+
apiVersion: hive.stackable.tech/v1alpha1
35+
kind: HiveCluster
36+
metadata:
37+
name: simple-hive
38+
namespace: default
39+
spec:
40+
image:
41+
productVersion: "4.0.0"
42+
clusterConfig:
43+
metadataDatabase:
44+
derby: {}
45+
metastore:
46+
roleGroups:
47+
default:
48+
replicas: 1
49+
"#;
50+
51+
/// Build a minimal Derby-backed [`ValidatedClusterConfig`] for builder tests.
52+
pub fn derby_cluster_config() -> ValidatedClusterConfig {
53+
let hive: v1alpha1::HiveCluster =
54+
stackable_operator::utils::yaml_from_str_singleton_map(DERBY_YAML)
55+
.expect("valid HiveCluster YAML");
56+
let metadata_database_connection_details = hive
57+
.spec
58+
.cluster_config
59+
.metadata_database
60+
.jdbc_connection_details("METADATA")
61+
.expect("derby connection details");
62+
let connection_driver = match &hive.spec.cluster_config.metadata_database {
63+
MetadataDatabaseConnection::Derby(_) => derby_driver_class("4.0.0").to_owned(),
64+
_ => metadata_database_connection_details.driver.clone(),
65+
};
66+
ValidatedClusterConfig {
67+
metadata_database_connection_details,
68+
connection_driver,
69+
s3_connection_spec: None,
70+
hive_opa_config: None,
71+
kerberos_config: BTreeMap::new(),
72+
needs_kerberos_core_site: false,
73+
}
74+
}
75+
}
Lines changed: 14 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
//! Builder for `core-site.xml`.
22
//!
3-
//! Only emitted when Kerberos is enabled and there is no HDFS backend (i.e. S3),
4-
//! in which case `hadoop.security.authentication=kerberos` is required.
3+
//! Only emitted when Kerberos is enabled without an HDFS backend (resolved during
4+
//! validation as [`ValidatedClusterConfig::needs_kerberos_core_site`]), in which case
5+
//! `hadoop.security.authentication=kerberos` is required.
56
67
use std::collections::BTreeMap;
78

8-
use crate::crd::v1alpha1;
9+
use crate::controller::ValidatedClusterConfig;
910

1011
const HADOOP_SECURITY_AUTHENTICATION: &str = "hadoop.security.authentication";
1112

1213
/// Returns the `core-site.xml` properties, or `None` if the file should be omitted.
13-
pub fn build(hive: &v1alpha1::HiveCluster) -> Option<BTreeMap<String, Option<String>>> {
14-
if hive.has_kerberos_enabled() && hive.spec.cluster_config.hdfs.is_none() {
14+
pub fn build(cluster_config: &ValidatedClusterConfig) -> Option<BTreeMap<String, Option<String>>> {
15+
if cluster_config.needs_kerberos_core_site {
1516
let mut data = BTreeMap::new();
1617
data.insert(
1718
HADOOP_SECURITY_AUTHENTICATION.to_string(),
@@ -26,93 +27,22 @@ pub fn build(hive: &v1alpha1::HiveCluster) -> Option<BTreeMap<String, Option<Str
2627
#[cfg(test)]
2728
mod tests {
2829
use super::*;
29-
30-
fn hive_cluster(yaml: &str) -> v1alpha1::HiveCluster {
31-
stackable_operator::utils::yaml_from_str_singleton_map(yaml)
32-
.expect("valid HiveCluster YAML")
33-
}
34-
35-
const NO_KERBEROS_YAML: &str = r#"
36-
apiVersion: hive.stackable.tech/v1alpha1
37-
kind: HiveCluster
38-
metadata:
39-
name: simple-hive
40-
namespace: default
41-
spec:
42-
image:
43-
productVersion: "4.0.0"
44-
clusterConfig:
45-
metadataDatabase:
46-
derby: {}
47-
metastore:
48-
roleGroups:
49-
default:
50-
replicas: 1
51-
"#;
52-
53-
const KERBEROS_S3_YAML: &str = r#"
54-
apiVersion: hive.stackable.tech/v1alpha1
55-
kind: HiveCluster
56-
metadata:
57-
name: simple-hive
58-
namespace: default
59-
spec:
60-
image:
61-
productVersion: "4.0.0"
62-
clusterConfig:
63-
authentication:
64-
kerberos:
65-
secretClass: kerberos
66-
metadataDatabase:
67-
derby: {}
68-
metastore:
69-
roleGroups:
70-
default:
71-
replicas: 1
72-
"#;
73-
74-
const KERBEROS_HDFS_YAML: &str = r#"
75-
apiVersion: hive.stackable.tech/v1alpha1
76-
kind: HiveCluster
77-
metadata:
78-
name: simple-hive
79-
namespace: default
80-
spec:
81-
image:
82-
productVersion: "4.0.0"
83-
clusterConfig:
84-
authentication:
85-
kerberos:
86-
secretClass: kerberos
87-
metadataDatabase:
88-
derby: {}
89-
hdfs:
90-
configMap: hdfs
91-
metastore:
92-
roleGroups:
93-
default:
94-
replicas: 1
95-
"#;
30+
use crate::controller::build::properties::test_support::derby_cluster_config;
9631

9732
#[test]
98-
fn omitted_without_kerberos() {
99-
let hive = hive_cluster(NO_KERBEROS_YAML);
100-
assert!(build(&hive).is_none());
33+
fn omitted_when_not_required() {
34+
let cluster_config = derby_cluster_config();
35+
assert!(build(&cluster_config).is_none());
10136
}
10237

10338
#[test]
104-
fn emitted_with_kerberos_and_no_hdfs() {
105-
let hive = hive_cluster(KERBEROS_S3_YAML);
106-
let data = build(&hive).expect("core-site present");
39+
fn emitted_when_required() {
40+
let mut cluster_config = derby_cluster_config();
41+
cluster_config.needs_kerberos_core_site = true;
42+
let data = build(&cluster_config).expect("core-site present");
10743
assert_eq!(
10844
data.get("hadoop.security.authentication"),
10945
Some(&Some("kerberos".to_string()))
11046
);
11147
}
112-
113-
#[test]
114-
fn omitted_with_kerberos_but_hdfs_backend() {
115-
let hive = hive_cluster(KERBEROS_HDFS_YAML);
116-
assert!(build(&hive).is_none());
117-
}
11848
}

0 commit comments

Comments
 (0)