Skip to content

Commit 09eb912

Browse files
maltesanderclaude
andcommitted
refactor: resolve cluster-wide config into ValidatedClusterConfig
Mirrors trino-operator: ValidatedCluster now carries a ValidatedClusterConfig (name, namespace, dfs_replication, https/kerberos/ authentication/authorization flags, rack awareness, OPA authorization) resolved once during validation. The standalone hdfs_opa_config field is folded into it. The build steps no longer take the raw HdfsCluster: - hdfs_site/core_site builders and build_rolegroup_config_map consume &ValidatedClusterConfig / &ValidatedCluster. - The lower-level HdfsSiteConfigBuilder/CoreSiteConfigBuilder and their kerberos security_config impls take primitives (a KerberosConfig struct and bools) to keep the config layer free of controller types. Behavior-preserving: the flags are computed by the same HdfsCluster predicates as before, just resolved up-front. The discovery path keeps using the raw HdfsCluster as it does not go through validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 12bc737 commit 09eb912

9 files changed

Lines changed: 137 additions & 109 deletions

File tree

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use crate::{
1717
SERVICE_PORT_NAME_HTTPS, SERVICE_PORT_NAME_RPC,
1818
},
1919
storage::{DataNodeStorageConfig, DataNodeStorageConfigInnerType},
20-
v1alpha1,
2120
},
2221
};
2322

@@ -168,11 +167,11 @@ impl HdfsSiteConfigBuilder {
168167

169168
pub fn dfs_namenode_http_address_ha(
170169
&mut self,
171-
hdfs: &v1alpha1::HdfsCluster,
170+
https_enabled: bool,
172171
cluster_info: &KubernetesClusterInfo,
173172
namenode_podrefs: &[HdfsPodRef],
174173
) -> &mut Self {
175-
if hdfs.has_https_enabled() {
174+
if https_enabled {
176175
self.dfs_namenode_address_ha(
177176
cluster_info,
178177
namenode_podrefs,

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

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use snafu::{OptionExt, ResultExt, Snafu};
66
use stackable_operator::{
77
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
88
k8s_openapi::api::core::v1::ConfigMap,
9-
kube::runtime::reflector::ObjectRef,
109
role_utils::RoleGroupRef,
1110
utils::cluster_info::KubernetesClusterInfo,
1211
};
@@ -24,11 +23,6 @@ use crate::{
2423

2524
#[derive(Snafu, Debug)]
2625
pub enum Error {
27-
#[snafu(display("object has no name"))]
28-
ObjectHasNoName {
29-
obj_ref: ObjectRef<v1alpha1::HdfsCluster>,
30-
},
31-
3226
#[snafu(display("could not parse HDFS role [{role}]"))]
3327
UnidentifiedHdfsRole {
3428
source: strum::ParseError,
@@ -63,10 +57,8 @@ pub enum Error {
6357

6458
type Result<T, E = Error> = std::result::Result<T, E>;
6559

66-
#[allow(clippy::too_many_arguments)]
6760
pub fn build_rolegroup_config_map(
6861
cluster: &ValidatedCluster,
69-
hdfs: &v1alpha1::HdfsCluster,
7062
cluster_info: &KubernetesClusterInfo,
7163
metadata: &ObjectMetaBuilder,
7264
rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
@@ -90,42 +82,30 @@ pub fn build_rolegroup_config_map(
9082
})?;
9183
let merged_config = &rolegroup_config.merged_config;
9284
let config_overrides = &rolegroup_config.config_overrides;
93-
let hdfs_opa_config = cluster.hdfs_opa_config.as_ref();
94-
95-
let hdfs_name = hdfs
96-
.metadata
97-
.name
98-
.as_deref()
99-
.with_context(|| ObjectHasNoNameSnafu {
100-
obj_ref: ObjectRef::from_obj(hdfs),
101-
})?;
85+
let cluster_config = &cluster.cluster_config;
10286

10387
let hdfs_site_xml = hdfs_site::build(
104-
hdfs,
105-
hdfs_name,
88+
cluster_config,
10689
cluster_info,
10790
merged_config,
10891
namenode_podrefs,
10992
journalnode_podrefs,
110-
hdfs_opa_config,
11193
config_overrides.hdfs_site_xml.clone(),
11294
);
11395
let core_site_xml = core_site::build(
114-
hdfs,
115-
hdfs_name,
96+
cluster_config,
11697
role,
11798
cluster_info,
118-
hdfs_opa_config,
11999
config_overrides.core_site_xml.clone(),
120100
)
121101
.context(BuildCoreSiteXmlSnafu)?;
122102
let hadoop_policy_xml = hadoop_policy::build(config_overrides.hadoop_policy_xml.clone());
123103
let ssl_server_xml = ssl_server::build(
124-
hdfs.has_https_enabled(),
104+
cluster_config.https_enabled,
125105
config_overrides.ssl_server_xml.clone(),
126106
);
127107
let ssl_client_xml = ssl_client::build(
128-
hdfs.has_https_enabled(),
108+
cluster_config.https_enabled,
129109
config_overrides.ssl_client_xml.clone(),
130110
);
131111

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

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ use stackable_operator::{
1010
use crate::{
1111
config::CoreSiteConfigBuilder,
1212
controller::build::properties::resolved_overrides,
13-
crd::{HdfsNodeRole, v1alpha1},
14-
security::{kerberos, opa::HdfsOpaConfig},
13+
crd::HdfsNodeRole,
14+
hdfs_controller::ValidatedClusterConfig,
15+
security::kerberos::{self, KerberosConfig},
1516
};
1617

1718
#[derive(Debug, Snafu)]
@@ -23,32 +24,38 @@ pub enum Error {
2324
/// Renders `core-site.xml`: operator defaults + kerberos/OPA security config,
2425
/// with user `configOverrides` applied last.
2526
pub fn build(
26-
hdfs: &v1alpha1::HdfsCluster,
27-
hdfs_name: &str,
27+
cluster_config: &ValidatedClusterConfig,
2828
role: HdfsNodeRole,
2929
cluster_info: &KubernetesClusterInfo,
30-
opa_config: Option<&HdfsOpaConfig>,
3130
overrides: KeyValueConfigOverrides,
3231
) -> Result<String, Error> {
33-
let mut core_site = CoreSiteConfigBuilder::new(hdfs_name.to_string());
32+
let kerberos = KerberosConfig {
33+
cluster_name: &cluster_config.name,
34+
cluster_namespace: cluster_config.namespace.as_deref(),
35+
authentication_enabled: cluster_config.authentication_enabled,
36+
kerberos_enabled: cluster_config.kerberos_enabled,
37+
authorization_enabled: cluster_config.authorization_enabled,
38+
};
39+
40+
let mut core_site = CoreSiteConfigBuilder::new(cluster_config.name.clone());
3441
core_site
3542
.fs_default_fs()
3643
.ha_zookeeper_quorum()
37-
.security_config(hdfs, cluster_info)
44+
.security_config(&kerberos, cluster_info)
3845
.context(BuildSecurityConfigSnafu)?
3946
.enable_prometheus_endpoint()
4047
// The default (4096) hasn't changed since 2009.
4148
// Increase to 128k to allow for faster transfers.
4249
.add("io.file.buffer.size", "131072");
4350
// Rack awareness topology provider, namenode only. Previously injected via
4451
// the product-config `Configuration::compute_files`.
45-
if role == HdfsNodeRole::Name && hdfs.rackawareness_config().is_some() {
52+
if role == HdfsNodeRole::Name && cluster_config.rack_awareness.is_some() {
4653
core_site.add(
4754
"net.topology.node.switch.mapping.impl",
4855
"tech.stackable.hadoop.StackableTopologyProvider",
4956
);
5057
}
51-
if let Some(opa_config) = opa_config {
58+
if let Some(opa_config) = &cluster_config.authorization {
5259
opa_config.add_core_site_config(&mut core_site);
5360
}
5461
// the extend with config must come last in order to have overrides working!!!
@@ -60,18 +67,15 @@ pub fn build(
6067
mod tests {
6168
use super::*;
6269
use crate::controller::build::properties::test_support::{
63-
cluster_info, config_overrides, minimal_hdfs,
70+
cluster_info, config_overrides, validated_cluster_config,
6471
};
6572

6673
#[test]
6774
fn renders_operator_defaults() {
68-
let hdfs = minimal_hdfs();
6975
let xml = build(
70-
&hdfs,
71-
"hdfs",
76+
&validated_cluster_config(),
7277
HdfsNodeRole::Name,
7378
&cluster_info(),
74-
None,
7579
config_overrides(&[]),
7680
)
7781
.unwrap();
@@ -93,13 +97,10 @@ mod tests {
9397

9498
#[test]
9599
fn user_overrides_win_over_defaults() {
96-
let hdfs = minimal_hdfs();
97100
let xml = build(
98-
&hdfs,
99-
"hdfs",
101+
&validated_cluster_config(),
100102
HdfsNodeRole::Name,
101103
&cluster_info(),
102-
None,
103104
config_overrides(&[("io.file.buffer.size", "65536")]),
104105
)
105106
.unwrap();

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

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,18 @@ use stackable_operator::{
99
use crate::{
1010
config::HdfsSiteConfigBuilder,
1111
controller::build::properties::resolved_overrides,
12-
crd::{AnyNodeConfig, HdfsPodRef, v1alpha1},
13-
security::opa::HdfsOpaConfig,
12+
crd::{AnyNodeConfig, HdfsPodRef},
13+
hdfs_controller::ValidatedClusterConfig,
1414
};
1515

1616
/// Renders `hdfs-site.xml`: operator defaults, HA wiring derived from the pod
1717
/// refs, kerberos/OPA security config, with user `configOverrides` applied last.
18-
#[allow(clippy::too_many_arguments)]
1918
pub fn build(
20-
hdfs: &v1alpha1::HdfsCluster,
21-
hdfs_name: &str,
19+
cluster_config: &ValidatedClusterConfig,
2220
cluster_info: &KubernetesClusterInfo,
2321
merged_config: &AnyNodeConfig,
2422
namenode_podrefs: &[HdfsPodRef],
2523
journalnode_podrefs: &[HdfsPodRef],
26-
opa_config: Option<&HdfsOpaConfig>,
2724
overrides: KeyValueConfigOverrides,
2825
) -> String {
2926
// IMPORTANT: these folders must be under the volume mount point, otherwise they will not
@@ -37,7 +34,7 @@ pub fn build(
3734
// https://hadoop.apache.org/docs/stable/hadoop-project-dist/hadoop-hdfs/HDFSHighAvailabilityWithNFS.html
3835
// This caused a deadlock with no namenode becoming active during a startup after
3936
// HDFS was completely down for a while.
40-
let mut hdfs_site = HdfsSiteConfigBuilder::new(hdfs_name.to_string());
37+
let mut hdfs_site = HdfsSiteConfigBuilder::new(cluster_config.name.clone());
4138
hdfs_site
4239
.dfs_namenode_name_dir()
4340
.dfs_datanode_data_dir(
@@ -46,15 +43,15 @@ pub fn build(
4643
.map(|node| node.resources.storage.clone()),
4744
)
4845
.dfs_journalnode_edits_dir()
49-
.dfs_replication(hdfs.spec.cluster_config.dfs_replication)
46+
.dfs_replication(cluster_config.dfs_replication)
5047
.dfs_name_services()
5148
.dfs_ha_namenodes(namenode_podrefs)
5249
.dfs_namenode_shared_edits_dir(cluster_info, journalnode_podrefs)
5350
.dfs_namenode_name_dir_ha(namenode_podrefs)
5451
.dfs_namenode_rpc_address_ha(cluster_info, namenode_podrefs)
55-
.dfs_namenode_http_address_ha(hdfs, cluster_info, namenode_podrefs)
52+
.dfs_namenode_http_address_ha(cluster_config.https_enabled, cluster_info, namenode_podrefs)
5653
.dfs_client_failover_proxy_provider()
57-
.security_config(hdfs)
54+
.security_config(cluster_config.kerberos_enabled)
5855
.add("dfs.ha.fencing.methods", "shell(/bin/true)")
5956
.add("dfs.ha.automatic-failover.enabled", "true")
6057
.add("dfs.ha.namenode.id", "${env.POD_NAME}")
@@ -99,12 +96,12 @@ pub fn build(
9996
// But today's Java and IO should be able to handle more, so bump it to 8192 for
10097
// better performance/concurrency.
10198
.add("dfs.datanode.max.transfer.threads", "8192");
102-
if hdfs.has_https_enabled() {
99+
if cluster_config.https_enabled {
103100
hdfs_site.add("dfs.datanode.registered.https.port", "${env.HTTPS_PORT}");
104101
} else {
105102
hdfs_site.add("dfs.datanode.registered.http.port", "${env.HTTP_PORT}");
106103
}
107-
if let Some(opa_config) = opa_config {
104+
if let Some(opa_config) = &cluster_config.authorization {
108105
opa_config.add_hdfs_site_config(&mut hdfs_site);
109106
}
110107
// the extend with config must come last in order to have overrides working!!!
@@ -117,9 +114,9 @@ mod tests {
117114
use super::*;
118115
use crate::{
119116
controller::build::properties::test_support::{
120-
cluster_info, config_overrides, minimal_hdfs,
117+
cluster_info, config_overrides, minimal_hdfs, validated_cluster_config,
121118
},
122-
crd::HdfsNodeRole,
119+
crd::{HdfsNodeRole, v1alpha1},
123120
};
124121

125122
fn namenode_merged_config(hdfs: &v1alpha1::HdfsCluster) -> AnyNodeConfig {
@@ -130,16 +127,13 @@ mod tests {
130127

131128
#[test]
132129
fn renders_operator_defaults() {
133-
let hdfs = minimal_hdfs();
134-
let merged = namenode_merged_config(&hdfs);
130+
let merged = namenode_merged_config(&minimal_hdfs());
135131
let xml = build(
136-
&hdfs,
137-
"hdfs",
132+
&validated_cluster_config(),
138133
&cluster_info(),
139134
&merged,
140135
&[],
141136
&[],
142-
None,
143137
config_overrides(&[]),
144138
);
145139
assert!(
@@ -154,16 +148,13 @@ mod tests {
154148

155149
#[test]
156150
fn user_overrides_win_over_defaults() {
157-
let hdfs = minimal_hdfs();
158-
let merged = namenode_merged_config(&hdfs);
151+
let merged = namenode_merged_config(&minimal_hdfs());
159152
let xml = build(
160-
&hdfs,
161-
"hdfs",
153+
&validated_cluster_config(),
162154
&cluster_info(),
163155
&merged,
164156
&[],
165157
&[],
166-
None,
167158
config_overrides(&[("dfs.replication", "5")]),
168159
);
169160
assert!(

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ pub(crate) mod test_support {
7373
v2::config_overrides::KeyValueConfigOverrides,
7474
};
7575

76-
use crate::crd::v1alpha1;
76+
use crate::{crd::v1alpha1, hdfs_controller::ValidatedClusterConfig};
7777

7878
/// Builds a [`KeyValueConfigOverrides`] from `(key, value)` pairs for tests.
7979
pub fn config_overrides(pairs: &[(&str, &str)]) -> KeyValueConfigOverrides {
@@ -120,4 +120,8 @@ spec:
120120
cluster_domain: DomainName::try_from("cluster.local").unwrap(),
121121
}
122122
}
123+
124+
pub fn validated_cluster_config() -> ValidatedClusterConfig {
125+
ValidatedClusterConfig::resolve(&minimal_hdfs(), None)
126+
}
123127
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ use strum::IntoEnumIterator;
2020
use crate::{
2121
crd::{HdfsNodeRole, v1alpha1},
2222
hdfs_controller::{
23-
CONTAINER_IMAGE_BASE_NAME, ValidatedCluster, ValidatedRoleConfig, ValidatedRoleGroupConfig,
23+
CONTAINER_IMAGE_BASE_NAME, ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig,
24+
ValidatedRoleGroupConfig,
2425
},
2526
security::opa::HdfsOpaConfig,
2627
};
@@ -97,9 +98,9 @@ pub fn validate_cluster(
9798

9899
Ok(ValidatedCluster {
99100
image: resolved_product_image,
101+
cluster_config: ValidatedClusterConfig::resolve(hdfs, hdfs_opa_config),
100102
role_groups,
101103
role_configs,
102-
hdfs_opa_config,
103104
})
104105
}
105106

rust/operator-binary/src/discovery.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{
1212
config::{CoreSiteConfigBuilder, HdfsSiteConfigBuilder},
1313
controller::build::properties::ConfigFileName,
1414
crd::{HdfsNodeRole, HdfsPodRef, v1alpha1},
15-
security::kerberos,
15+
security::kerberos::{self, KerberosConfig},
1616
};
1717

1818
type Result<T, E = Error> = std::result::Result<T, E>;
@@ -89,9 +89,9 @@ fn build_discovery_hdfs_site_xml(
8989
.dfs_name_services()
9090
.dfs_ha_namenodes(namenode_podrefs)
9191
.dfs_namenode_rpc_address_ha(cluster_info, namenode_podrefs)
92-
.dfs_namenode_http_address_ha(hdfs, cluster_info, namenode_podrefs)
92+
.dfs_namenode_http_address_ha(hdfs.has_https_enabled(), cluster_info, namenode_podrefs)
9393
.dfs_client_failover_proxy_provider()
94-
.security_discovery_config(hdfs)
94+
.security_discovery_config(hdfs.has_kerberos_enabled())
9595
.build_as_xml()
9696
}
9797

@@ -100,9 +100,18 @@ fn build_discovery_core_site_xml(
100100
cluster_info: &KubernetesClusterInfo,
101101
logical_name: String,
102102
) -> Result<String> {
103+
let cluster_name = hdfs.name_any();
104+
let cluster_namespace = hdfs.namespace();
105+
let kerberos = KerberosConfig {
106+
cluster_name: &cluster_name,
107+
cluster_namespace: cluster_namespace.as_deref(),
108+
authentication_enabled: hdfs.authentication_config().is_some(),
109+
kerberos_enabled: hdfs.has_kerberos_enabled(),
110+
authorization_enabled: hdfs.has_authorization_enabled(),
111+
};
103112
Ok(CoreSiteConfigBuilder::new(logical_name)
104113
.fs_default_fs()
105-
.security_discovery_config(hdfs, cluster_info)
114+
.security_discovery_config(&kerberos, cluster_info)
106115
.context(BuildSecurityDiscoveryConfigMapSnafu)?
107116
.build_as_xml())
108117
}

0 commit comments

Comments
 (0)