Skip to content

Commit a2408a4

Browse files
adwk67claude
andauthored
refactor: extract dereference and validate steps into controller modules (#783)
* refactor: extract dereference and validate steps into controller modules Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * changelog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * formatting * extend assert step to increase test coverage by checking most generated resources Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * added resources assert and re-org as per hive review feedback * extend STS definition for the assert and remove duplicate envOverrides test * Expand smoke test assertions with full STS specs and ConfigMap snapshots 30-assert now includes args, imagePullPolicy, defaultMode, and all container specs for the three HDFS StatefulSets. 31-assert is refactored to diff ConfigMap data against external snapshot files using envsubst and yq normalisation, replacing the previous inline envOverrides checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * linting * more linting * Use jq to normalise trailing newlines in ConfigMap snapshot diffs Replaces variable-based comparison with pipe-to-file approach. Both sides are normalised via jq (collapsing trailing \n+ to \n) so YAML block scalar style differences (|, |+) don't cause false failures. Temp files use $NAMESPACE prefix for concurrent test safety. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0bd066f commit a2408a4

16 files changed

Lines changed: 2270 additions & 242 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ All notable changes to this project will be documented in this file.
1616
`security.properties`).
1717
Previously, arbitrary file names were silently accepted and ignored ([#777]).
1818
- Bump `stackable-operator` to 0.111.1 ([#777], [#778]).
19+
- Internal operator refactoring: introduce dereference() and validate() steps in the reconciler ([#783]).
1920

2021
[#770]: https://github.com/stackabletech/hdfs-operator/pull/770
2122
[#777]: https://github.com/stackabletech/hdfs-operator/pull/777
2223
[#778]: https://github.com/stackabletech/hdfs-operator/pull/778
24+
[#783]: https://github.com/stackabletech/hdfs-operator/pull/783
2325

2426
## [26.3.0] - 2026-03-16
2527

Cargo.nix

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate-hashes.json

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
use snafu::{ResultExt, Snafu};
2+
3+
use crate::{crd::v1alpha1, security::opa::HdfsOpaConfig};
4+
5+
#[derive(Snafu, Debug)]
6+
pub enum Error {
7+
#[snafu(display("invalid OPA configuration"))]
8+
InvalidOpaConfig { source: crate::security::opa::Error },
9+
}
10+
11+
/// External references resolved during the dereference step.
12+
pub struct DereferencedObjects {
13+
pub hdfs_opa_config: Option<HdfsOpaConfig>,
14+
}
15+
16+
pub async fn dereference(
17+
client: &stackable_operator::client::Client,
18+
hdfs: &v1alpha1::HdfsCluster,
19+
) -> Result<DereferencedObjects, Error> {
20+
let hdfs_opa_config = match &hdfs.spec.cluster_config.authorization {
21+
Some(opa_config) => Some(
22+
HdfsOpaConfig::from_opa_config(client, hdfs, opa_config)
23+
.await
24+
.context(InvalidOpaConfigSnafu)?,
25+
),
26+
None => None,
27+
};
28+
29+
Ok(DereferencedObjects { hdfs_opa_config })
30+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
pub mod dereference;
2+
pub mod validate;
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
use std::{collections::BTreeMap, str::FromStr};
2+
3+
use product_config::ProductConfigManager;
4+
use snafu::{ResultExt, Snafu};
5+
use stackable_operator::{
6+
commons::product_image_selection,
7+
product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config},
8+
role_utils::GenericRoleConfig,
9+
};
10+
11+
use crate::{
12+
crd::{HdfsNodeRole, v1alpha1},
13+
hdfs_controller::{
14+
CONTAINER_IMAGE_BASE_NAME, ValidatedCluster, ValidatedRoleConfig, ValidatedRoleGroupConfig,
15+
},
16+
security::opa::HdfsOpaConfig,
17+
};
18+
19+
#[derive(Snafu, Debug)]
20+
pub enum Error {
21+
#[snafu(display("failed to resolve product image"))]
22+
ResolveProductImage {
23+
source: product_image_selection::Error,
24+
},
25+
26+
#[snafu(display("invalid role properties"))]
27+
RoleProperties { source: crate::crd::Error },
28+
29+
#[snafu(display("failed to generate product config"))]
30+
GenerateProductConfig {
31+
source: stackable_operator::product_config_utils::Error,
32+
},
33+
34+
#[snafu(display("invalid product configuration"))]
35+
InvalidProductConfig {
36+
source: stackable_operator::product_config_utils::Error,
37+
},
38+
39+
#[snafu(display("could not parse HDFS role [{role}]"))]
40+
UnidentifiedHdfsRole {
41+
source: strum::ParseError,
42+
role: String,
43+
},
44+
45+
#[snafu(display("failed to resolve and merge config for role and role group"))]
46+
FailedToResolveConfig { source: crate::crd::Error },
47+
}
48+
49+
pub fn validate_cluster(
50+
hdfs: &v1alpha1::HdfsCluster,
51+
image_repository: &str,
52+
product_config_manager: &ProductConfigManager,
53+
hdfs_opa_config: Option<HdfsOpaConfig>,
54+
) -> Result<ValidatedCluster, Error> {
55+
let resolved_product_image = hdfs
56+
.spec
57+
.image
58+
.resolve(
59+
CONTAINER_IMAGE_BASE_NAME,
60+
image_repository,
61+
crate::built_info::PKG_VERSION,
62+
)
63+
.context(ResolveProductImageSnafu)?;
64+
65+
let roles = hdfs.build_role_properties().context(RolePropertiesSnafu)?;
66+
67+
let validated_config = validate_all_roles_and_groups_config(
68+
&resolved_product_image.product_version,
69+
&transform_all_roles_to_config(hdfs, &roles).context(GenerateProductConfigSnafu)?,
70+
product_config_manager,
71+
false,
72+
false,
73+
)
74+
.context(InvalidProductConfigSnafu)?;
75+
76+
let mut role_groups = BTreeMap::new();
77+
let mut role_configs = BTreeMap::new();
78+
79+
for (role_name, group_config) in validated_config.iter() {
80+
let hdfs_role = HdfsNodeRole::from_str(role_name).context(UnidentifiedHdfsRoleSnafu {
81+
role: role_name.to_string(),
82+
})?;
83+
84+
if let Some(GenericRoleConfig {
85+
pod_disruption_budget: pdb,
86+
}) = hdfs.role_config(&hdfs_role)
87+
{
88+
role_configs.insert(hdfs_role, ValidatedRoleConfig { pdb: pdb.clone() });
89+
}
90+
91+
let mut group_configs = BTreeMap::new();
92+
for (rolegroup_name, rolegroup_config) in group_config.iter() {
93+
let merged_config = hdfs_role
94+
.merged_config(hdfs, rolegroup_name)
95+
.context(FailedToResolveConfigSnafu)?;
96+
97+
group_configs.insert(
98+
rolegroup_name.clone(),
99+
ValidatedRoleGroupConfig {
100+
merged_config,
101+
product_config_properties: rolegroup_config.clone(),
102+
},
103+
);
104+
}
105+
106+
role_groups.insert(hdfs_role, group_configs);
107+
}
108+
109+
Ok(ValidatedCluster {
110+
image: resolved_product_image,
111+
role_groups,
112+
role_configs,
113+
hdfs_opa_config,
114+
})
115+
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ pub struct CommonNodeConfig {
993993
}
994994

995995
/// Configuration for a rolegroup of an unknown type.
996-
#[derive(Debug)]
996+
#[derive(Clone, Debug)]
997997
pub enum AnyNodeConfig {
998998
Name(NameNodeConfig),
999999
Data(DataNodeConfig),
@@ -1087,7 +1087,9 @@ impl AnyNodeConfig {
10871087
Eq,
10881088
Hash,
10891089
JsonSchema,
1090+
Ord,
10901091
PartialEq,
1092+
PartialOrd,
10911093
Serialize,
10921094
)]
10931095
pub enum HdfsNodeRole {

0 commit comments

Comments
 (0)