Skip to content

Commit 46cd3f9

Browse files
adwk67claudemaltesander
authored
refactor: Add config file writers to v2 (#1217)
* feat(v2): add shared config-file writers (java-properties/Hadoop-XML + Flask) Adds v2::config_file_writer (to_java_properties_string + to_hadoop_xml, backed by the java-properties and xml crates) and v2::flask_config_writer (the Flask App Builder Python config writer), both originally from the product-config crate's writer modules and until now vendored separately into the operators (hdfs/hbase/hive byte-identical full copies; kafka/nifi/zookeeper java-only subsets; airflow/superset identical Flask copies). Unit tests moved along with the code; minor lint-driven cleanups only (use_self, format_push_string, identical match arms, no unwrap in Result-returning tests) — rendered output is unchanged and pinned by the tests. Operators will migrate to these in follow-up commits, deleting their vendored copies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: fix rustdoc warnings (broken intra-doc links, bare URLs) stackable-operator: intra-doc links within the crate must use `crate::` rather than the package name `stackable_operator::`, which is not in scope and produced `broken_intra_doc_links` warnings. Also wrap two bare URLs in the scaler docs in angle brackets so rustdoc renders them as hyperlinks. stackable-telemetry: `SettingsBuilder` is not imported in `tracing/mod.rs` (only `Settings` is), so the four doc links to it did not resolve. Point them at `settings::SettingsBuilder` via the child module, keeping the rendered link text unchanged. `cargo doc --document-private-items` is now warning-free across the workspace. * Update crates/stackable-operator/src/v2/config_file_writer.rs Co-authored-by: maltesander <malte.sander.it@gmail.com> * Apply suggestions from code review Co-authored-by: maltesander <malte.sander.it@gmail.com> * remove error suffix and clippy --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: maltesander <malte.sander.it@gmail.com>
1 parent 831e1ba commit 46cd3f9

16 files changed

Lines changed: 488 additions & 22 deletions

File tree

Cargo.lock

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

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ insta = { version = "1.40", features = ["glob"] }
3838
hyper = { version = "1.4.1", features = ["full"] }
3939
hyper-util = "0.1.8"
4040
itertools = "0.14.0"
41+
java-properties = "2.0"
4142
json-patch = "4.0.0"
4243
k8s-openapi = { version = "0.27.0", default-features = false, features = ["schemars", "v1_35"] }
4344
# We use rustls instead of openssl for easier portability, e.g. so that we can build stackablectl without the need to vendor (build from source) openssl
@@ -89,6 +90,7 @@ url = { version = "2.5.2", features = ["serde"] }
8990
uuid = "1.23"
9091
winnow = "1.0.3"
9192
x509-cert = { version = "0.2.5", features = ["builder"] }
93+
xml = "1.3"
9294
zeroize = "1.8.1"
9395

9496
[workspace.lints.clippy]

crates/stackable-operator/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ educe.workspace = true
3636
futures.workspace = true
3737
http.workspace = true
3838
indexmap.workspace = true
39+
java-properties.workspace = true
3940
jiff.workspace = true
4041
json-patch = { workspace = true, features = ["schemars"] }
4142
k8s-openapi.workspace = true
@@ -57,6 +58,7 @@ tracing-subscriber.workspace = true
5758
url.workspace = true
5859
uuid.workspace = true
5960
winnow = { workspace = true, optional = true }
61+
xml.workspace = true
6062

6163
[dev-dependencies]
6264
indoc.workspace = true

crates/stackable-operator/src/crd/scaler/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ pub mod versioned {
3131
///
3232
/// Upstream issues:
3333
///
34-
/// - https://github.com/kubernetes/kubernetes/issues/105533
35-
/// - https://github.com/Arnavion/k8s-openapi/issues/136
34+
/// - <https://github.com/kubernetes/kubernetes/issues/105533>
35+
/// - <https://github.com/Arnavion/k8s-openapi/issues/136>
3636
pub replicas: u16,
3737
}
3838
}

crates/stackable-operator/src/v2/builder/meta.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
};
77

88
/// Infallible variant of
9-
/// [`stackable_operator::builder::meta::ObjectMetaBuilder::ownerreference_from_resource`]
9+
/// [`crate::builder::meta::ObjectMetaBuilder::ownerreference_from_resource`]
1010
pub fn ownerreference_from_resource(
1111
resource: &(impl Resource<DynamicType = ()> + HasName + HasUid),
1212
block_owner_deletion: Option<bool>,

crates/stackable-operator/src/v2/builder/pdb.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
};
1010

1111
/// Infallible variant of
12-
/// [`stackable_operator::builder::pdb::PodDisruptionBudgetBuilder::new_with_role`]
12+
/// [`crate::builder::pdb::PodDisruptionBudgetBuilder::new_with_role`]
1313
pub fn pod_disruption_budget_builder_with_role(
1414
owner: &(impl Resource<DynamicType = ()> + HasName + NameIsValidLabelValue + HasUid),
1515
product_name: &ProductName,

crates/stackable-operator/src/v2/builder/pod/container.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub enum Error {
2323
ParseEnvVarName { env_var_name: String },
2424
}
2525

26-
/// Infallible variant of [`stackable_operator::builder::pod::container::ContainerBuilder::new`]
26+
/// Infallible variant of [`crate::builder::pod::container::ContainerBuilder::new`]
2727
pub fn new_container_builder(container_name: &ContainerName) -> ContainerBuilder {
2828
ContainerBuilder::new(container_name.as_ref()).expect("should be a valid container name")
2929
}

crates/stackable-operator/src/v2/builder/pod/volume.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
v2::types::kubernetes::{ListenerClassName, ListenerName, PersistentVolumeClaimName},
66
};
77

8-
/// Infallible variant of [`stackable_operator::builder::pod::volume::ListenerReference`]
8+
/// Infallible variant of [`crate::builder::pod::volume::ListenerReference`]
99
#[derive(Clone, Debug, Eq, PartialEq)]
1010
pub enum ListenerReference {
1111
ListenerClass(ListenerClassName),
@@ -26,7 +26,7 @@ impl From<&ListenerReference> for crate::builder::pod::volume::ListenerReference
2626
}
2727

2828
/// Infallible variant of
29-
/// [`stackable_operator::builder::pod::volume::ListenerOperatorVolumeSourceBuilder::build_pvc`]
29+
/// [`crate::builder::pod::volume::ListenerOperatorVolumeSourceBuilder::build_pvc`]
3030
pub fn listener_operator_volume_source_builder_build_pvc(
3131
listener_reference: &ListenerReference,
3232
labels: &Labels,

crates/stackable-operator/src/v2/cluster_resources.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
v2::{NameIsValidLabelValue, macros::attributed_string_type::MAX_LABEL_VALUE_LENGTH},
1010
};
1111

12-
/// Infallible variant of [`stackable_operator::cluster_resources::ClusterResources::new`]
12+
/// Infallible variant of [`crate::cluster_resources::ClusterResources::new`]
1313
#[allow(clippy::too_many_arguments)]
1414
pub fn cluster_resources_new<'a>(
1515
product_name: &ProductName,
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
//! Writers for Hadoop XML config files and Java `.properties` files.
2+
3+
use std::{fmt::Write as _, io::Write};
4+
5+
use java_properties::{PropertiesError, PropertiesWriter};
6+
use snafu::{ResultExt, Snafu};
7+
use xml::escape::escape_str_attribute;
8+
9+
#[derive(Debug, Snafu)]
10+
pub enum PropertiesWriterError {
11+
#[snafu(display("failed to create properties file"))]
12+
Properties { source: PropertiesError },
13+
14+
#[snafu(display("failed to convert properties file byte array to UTF-8"))]
15+
FromUtf8 { source: std::string::FromUtf8Error },
16+
}
17+
18+
/// Creates a common Java properties file string in the format:
19+
/// `property_1=value_1\nproperty_2=value_2\n`.
20+
pub fn to_java_properties_string<'a, T>(properties: T) -> Result<String, PropertiesWriterError>
21+
where
22+
T: Iterator<Item = (&'a String, &'a Option<String>)>,
23+
{
24+
let mut output = Vec::new();
25+
write_java_properties(&mut output, properties)?;
26+
String::from_utf8(output).context(FromUtf8Snafu)
27+
}
28+
29+
/// Writes Java properties to the given writer. A `None` value is written as an
30+
/// empty value (`key=`).
31+
fn write_java_properties<'a, W, T>(writer: W, properties: T) -> Result<(), PropertiesWriterError>
32+
where
33+
W: Write,
34+
T: Iterator<Item = (&'a String, &'a Option<String>)>,
35+
{
36+
let mut writer = PropertiesWriter::new(writer);
37+
for (k, v) in properties {
38+
let property_value = v.as_deref().unwrap_or_default();
39+
writer.write(k, property_value).context(PropertiesSnafu)?;
40+
}
41+
writer.flush().context(PropertiesSnafu)?;
42+
Ok(())
43+
}
44+
45+
/// Converts properties into a Hadoop configuration XML, including the wrapping
46+
/// `<configuration>...</configuration>` elements. Properties with a `None` value
47+
/// are skipped. Keys and values are XML-escaped.
48+
pub fn to_hadoop_xml<'a, T>(properties: T) -> String
49+
where
50+
T: Iterator<Item = (&'a String, &'a Option<String>)>,
51+
{
52+
let mut snippet = String::new();
53+
for (k, v) in properties {
54+
let escaped_value = match v {
55+
Some(value) => escape_str_attribute(value),
56+
None => continue,
57+
};
58+
let escaped_key = escape_str_attribute(k);
59+
write!(
60+
snippet,
61+
" <property>\n <name>{escaped_key}</name>\n <value>{escaped_value}</value>\n </property>\n"
62+
)
63+
.expect("writing to a String is infallible");
64+
}
65+
format!("<?xml version=\"1.0\"?>\n<configuration>\n{snippet}</configuration>")
66+
}
67+
68+
#[cfg(test)]
69+
mod tests {
70+
use std::collections::BTreeMap;
71+
72+
use super::*;
73+
74+
fn xml(pairs: &[(&str, Option<&str>)]) -> String {
75+
let map: BTreeMap<String, Option<String>> = pairs
76+
.iter()
77+
.map(|(k, v)| (k.to_string(), v.map(str::to_string)))
78+
.collect();
79+
to_hadoop_xml(map.iter())
80+
}
81+
82+
fn props(pairs: &[(&str, Option<&str>)]) -> String {
83+
let map: BTreeMap<String, Option<String>> = pairs
84+
.iter()
85+
.map(|(k, v)| (k.to_string(), v.map(str::to_string)))
86+
.collect();
87+
to_java_properties_string(map.iter()).unwrap()
88+
}
89+
90+
#[test]
91+
fn hadoop_xml_wraps_empty_configuration() {
92+
assert_eq!(
93+
xml(&[]),
94+
"<?xml version=\"1.0\"?>\n<configuration>\n</configuration>"
95+
);
96+
}
97+
98+
#[test]
99+
fn hadoop_xml_renders_single_property() {
100+
assert_eq!(
101+
xml(&[("fs.defaultFS", Some("hdfs://hdfs/"))]),
102+
"<?xml version=\"1.0\"?>\n<configuration>\n \
103+
<property>\n <name>fs.defaultFS</name>\n \
104+
<value>hdfs://hdfs/</value>\n </property>\n</configuration>"
105+
);
106+
}
107+
108+
#[test]
109+
fn hadoop_xml_skips_none_values() {
110+
assert_eq!(
111+
xml(&[("kept", Some("1")), ("dropped", None)]),
112+
"<?xml version=\"1.0\"?>\n<configuration>\n \
113+
<property>\n <name>kept</name>\n \
114+
<value>1</value>\n </property>\n</configuration>"
115+
);
116+
}
117+
118+
#[test]
119+
fn hadoop_xml_escapes_special_characters() {
120+
let rendered = xml(&[("k", Some("<a>&b"))]);
121+
assert!(
122+
rendered.contains("<value>&lt;a&gt;&amp;b</value>"),
123+
"{rendered}"
124+
);
125+
}
126+
127+
#[test]
128+
fn java_properties_renders_key_value() {
129+
assert_eq!(props(&[("a", Some("1")), ("b", Some("2"))]), "a=1\nb=2\n");
130+
}
131+
132+
#[test]
133+
fn java_properties_renders_none_as_empty() {
134+
assert_eq!(props(&[("none", None)]), "none=\n");
135+
}
136+
137+
#[test]
138+
fn java_properties_escapes_colon_in_value() {
139+
assert_eq!(
140+
props(&[("url", Some("file://this/location/file.abc"))]),
141+
"url=file\\://this/location/file.abc\n"
142+
);
143+
}
144+
}

0 commit comments

Comments
 (0)