From 4d2266f044fb8a66945adcb5b3780b59ef929a6e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 19:04:45 +0200 Subject: [PATCH 01/27] chore(deps): switch stackable-operator to smooth-operator branch Pulls in the v2 framework (role_utils::with_validated_config) needed for the product-config removal in subsequent commits. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 20 ++++++++++---------- Cargo.nix | 20 ++++++++++---------- Cargo.toml | 2 +- crate-hashes.json | 18 +++++++++--------- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9227a96e..e5a368c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1524,7 +1524,7 @@ dependencies = [ [[package]] name = "k8s-version" version = "0.1.3" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "darling", "regex", @@ -2881,7 +2881,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stackable-certs" version = "0.4.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "const-oid", "ecdsa", @@ -2904,8 +2904,8 @@ dependencies = [ [[package]] name = "stackable-operator" -version = "0.111.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +version = "0.111.1" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "base64", "clap", @@ -2946,7 +2946,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "darling", "proc-macro2", @@ -2957,7 +2957,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.1.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "jiff", "k8s-openapi", @@ -2974,7 +2974,7 @@ dependencies = [ [[package]] name = "stackable-telemetry" version = "0.6.3" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "axum", "clap", @@ -3021,7 +3021,7 @@ dependencies = [ [[package]] name = "stackable-versioned" version = "0.10.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "kube", "schemars", @@ -3035,7 +3035,7 @@ dependencies = [ [[package]] name = "stackable-versioned-macros" version = "0.10.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "convert_case", "convert_case_extras", @@ -3053,7 +3053,7 @@ dependencies = [ [[package]] name = "stackable-webhook" version = "0.9.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" dependencies = [ "arc-swap", "async-trait", diff --git a/Cargo.nix b/Cargo.nix index 628be318..33d9ae93 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -4862,7 +4862,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "k8s_version"; @@ -9491,7 +9491,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "stackable_certs"; @@ -9589,12 +9589,12 @@ rec { }; "stackable-operator" = rec { crateName = "stackable-operator"; - version = "0.111.0"; + version = "0.111.1"; edition = "2024"; workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "stackable_operator"; @@ -9774,7 +9774,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; procMacro = true; @@ -9809,7 +9809,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "stackable_shared"; @@ -9890,7 +9890,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "stackable_telemetry"; @@ -10101,7 +10101,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "stackable_versioned"; @@ -10151,7 +10151,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; procMacro = true; @@ -10219,7 +10219,7 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "b7c8a3a5483b4d35d0abfa11f6db6c153bda8a51"; + rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; }; libName = "stackable_webhook"; diff --git a/Cargo.toml b/Cargo.toml index acced5f7..03289dfb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/stackabletech/trino-operator" [workspace.dependencies] product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.8.0" } -stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.111.0", features = ["webhook"] } +stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", branch = "smooth-operator", features = ["webhook"] } anyhow = "1.0" async-trait = "0.1" diff --git a/crate-hashes.json b/crate-hashes.json index 71fbc1c3..4e2f0d70 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,12 +1,12 @@ { - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#k8s-version@0.1.3": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-certs@0.4.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-operator-derive@0.3.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-operator@0.111.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-shared@0.1.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-telemetry@0.6.3": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-versioned-macros@0.10.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-versioned@0.10.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.111.0#stackable-webhook@0.9.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#k8s-version@0.1.3": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-certs@0.4.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator-derive@0.3.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator@0.111.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-shared@0.1.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-telemetry@0.6.3": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned-macros@0.10.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned@0.10.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-webhook@0.9.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", "git+https://github.com/stackabletech/product-config.git?tag=0.8.0#product-config@0.8.0": "1dz70kapm2wdqcr7ndyjji0lhsl98bsq95gnb2lw487wf6yr7987" } \ No newline at end of file From 786da36d6eb81f9cce3091d35ab710e8537a7c08 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 19:11:25 +0200 Subject: [PATCH 02/27] feat(build): vendor java-properties writer Adds a minimal `to_java_properties_string` under `controller/build/properties/writer.rs`. Reproduces the escape rules exercised by the kuttl ConfigMap snapshot (14-assert.yaml.j2): - http\://opa-server...\:8081 (colons in URL values escaped) - ${ENV\:INTERNAL_SECRET} (colon in env-var interpolation escaped) No upstream v2 writer found in stackable-operator; vendored instead. Follow-up: upstream the writer to operator-rs `v2` and delete the vendored copy. Co-Authored-By: Claude Opus 4.7 (1M context) --- rust/operator-binary/src/controller.rs | 1 + .../src/controller/build/mod.rs | 5 + .../src/controller/build/properties/mod.rs | 7 + .../src/controller/build/properties/writer.rs | 141 ++++++++++++++++++ 4 files changed, 154 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/mod.rs create mode 100644 rust/operator-binary/src/controller/build/properties/mod.rs create mode 100644 rust/operator-binary/src/controller/build/properties/writer.rs diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 623b914b..3359d864 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -71,6 +71,7 @@ use stackable_operator::{ }; use strum::{EnumDiscriminants, IntoStaticStr}; +mod build; mod dereference; mod validate; diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs new file mode 100644 index 00000000..93539e2f --- /dev/null +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -0,0 +1,5 @@ +//! Builders that turn a `ValidatedCluster` into Kubernetes resource contents. +//! +//! Each submodule owns one output: properties files, ConfigMaps, StatefulSets, etc. + +pub mod properties; diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs new file mode 100644 index 00000000..cb6de00a --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -0,0 +1,7 @@ +//! Per-file builders for Trino `.properties` files. +//! +//! Each `.rs` module produces the rendered key/value pairs for one +//! Trino config file. The shared [`writer`] module serializes the map to the +//! Java-properties on-wire format. + +pub mod writer; diff --git a/rust/operator-binary/src/controller/build/properties/writer.rs b/rust/operator-binary/src/controller/build/properties/writer.rs new file mode 100644 index 00000000..99e60999 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/writer.rs @@ -0,0 +1,141 @@ +//! Java-properties writer. +//! +//! Reproduces the escape rules required for Trino's `.properties` files. Pinned +//! by the kuttl ConfigMap snapshot at +//! `tests/templates/kuttl/smoke/14-assert.yaml.j2`. + +use std::collections::BTreeMap; + +use snafu::{ResultExt, Snafu}; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("failed to write properties output"))] + Format { source: std::fmt::Error }, +} + +/// Serialize `props` as a Java-properties string, sorted by key. +/// +/// Keys and values are escaped per : +/// `:`, `=`, `#`, `!`, `\\`, leading whitespace, and ` ` (space). +pub fn to_java_properties_string( + props: &BTreeMap, +) -> Result { + use std::fmt::Write; + let mut out = String::new(); + for (k, v) in props { + write!(out, "{}={}\n", escape_key(k), escape_value(v)).context(FormatSnafu)?; + } + Ok(out) +} + +fn escape_key(key: &str) -> String { + let mut out = String::with_capacity(key.len()); + for c in key.chars() { + match c { + '\\' | ':' | '=' | '#' | '!' | ' ' => { + out.push('\\'); + out.push(c); + } + _ => out.push(c), + } + } + out +} + +fn escape_value(value: &str) -> String { + let mut out = String::with_capacity(value.len()); + let mut at_start = true; + for c in value.chars() { + match c { + '\\' | ':' | '=' | '#' | '!' => { + out.push('\\'); + out.push(c); + } + ' ' if at_start => { + out.push('\\'); + out.push(' '); + } + _ => out.push(c), + } + if c != ' ' { + at_start = false; + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + fn render(pairs: &[(&str, &str)]) -> String { + let props: BTreeMap = pairs + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(); + to_java_properties_string(&props).unwrap() + } + + #[test] + fn empty_map_renders_empty_string() { + let props: BTreeMap = BTreeMap::new(); + assert_eq!(to_java_properties_string(&props).unwrap(), ""); + } + + #[test] + fn keys_are_sorted_alphabetically() { + assert_eq!(render(&[("b", "2"), ("a", "1")]), "a=1\nb=2\n"); + } + + #[test] + fn colon_in_value_is_escaped() { + // From smoke snapshot: + // internal-communication.shared-secret=${ENV\:INTERNAL_SECRET} + assert_eq!( + render(&[("internal-communication.shared-secret", "${ENV:INTERNAL_SECRET}")]), + "internal-communication.shared-secret=${ENV\\:INTERNAL_SECRET}\n" + ); + } + + #[test] + fn colon_in_url_value_is_escaped() { + // From smoke snapshot: + // discovery.uri=https\://trino-coordinator-default-0...:8443 + assert_eq!( + render(&[("discovery.uri", "https://trino-coordinator.svc:8443")]), + "discovery.uri=https\\://trino-coordinator.svc\\:8443\n" + ); + } + + #[test] + fn equals_in_value_is_escaped() { + assert_eq!(render(&[("k", "a=b")]), "k=a\\=b\n"); + } + + #[test] + fn backslash_in_value_is_escaped() { + assert_eq!(render(&[("k", "a\\b")]), "k=a\\\\b\n"); + } + + #[test] + fn leading_space_in_value_is_escaped() { + assert_eq!(render(&[("k", " v")]), "k=\\ v\n"); + } + + #[test] + fn non_leading_space_in_value_is_not_escaped() { + assert_eq!(render(&[("k", "a b")]), "k=a b\n"); + } + + #[test] + fn space_in_key_is_escaped() { + assert_eq!(render(&[("a b", "1")]), "a\\ b=1\n"); + } + + #[test] + fn hash_and_bang_in_value_are_escaped() { + assert_eq!(render(&[("k", "#comment")]), "k=\\#comment\n"); + assert_eq!(render(&[("k", "!bang")]), "k=\\!bang\n"); + } +} From a73c25d3f143af67195916c2915069774d7bbd48 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 19:18:59 +0200 Subject: [PATCH 03/27] feat(crd): implement Merge for TrinoConfigOverrides `stackable_operator::v2::role_utils::with_validated_config` requires `ConfigOverrides: Merge`. Adds a hand-written impl until `KeyValueConfigOverrides` derives `Merge` upstream. Co-Authored-By: Claude Opus 4.7 (1M context) --- rust/operator-binary/src/crd/mod.rs | 77 +++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index ae133dfc..9c6d03db 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -478,6 +478,50 @@ impl KeyValueOverridesProvider for v1alpha1::TrinoConfigOverrides { } } +// TODO: Remove once `KeyValueConfigOverrides` derives `Merge` upstream. +// `stackable_operator::v2::role_utils::with_validated_config` requires +// `ConfigOverrides: Merge`. Until upstreamed, merge field-by-field: +// the right-hand (role-group level) override wins per file when both sides +// are Some; otherwise the side that is Some survives. +impl Merge for v1alpha1::TrinoConfigOverrides { + fn merge(&mut self, defaults: &Self) { + merge_key_value(&mut self.config_properties, &defaults.config_properties); + merge_key_value(&mut self.node_properties, &defaults.node_properties); + merge_key_value(&mut self.log_properties, &defaults.log_properties); + merge_key_value(&mut self.security_properties, &defaults.security_properties); + merge_key_value( + &mut self.access_control_properties, + &defaults.access_control_properties, + ); + merge_key_value( + &mut self.exchange_manager_properties, + &defaults.exchange_manager_properties, + ); + merge_key_value( + &mut self.spooling_manager_properties, + &defaults.spooling_manager_properties, + ); + } +} + +fn merge_key_value( + rg: &mut Option, + role: &Option, +) { + match (rg.as_mut(), role) { + (Some(rg_overrides), Some(role_overrides)) => { + // role-group keys win over role keys. + let mut merged = role_overrides.overrides.clone(); + merged.extend(std::mem::take(&mut rg_overrides.overrides)); + rg_overrides.overrides = merged; + } + (None, Some(role_overrides)) => { + *rg = Some(role_overrides.clone()); + } + _ => {} // None defaults: nothing to merge in. + } +} + impl Default for v1alpha1::TrinoCoordinatorRoleConfig { fn default() -> Self { v1alpha1::TrinoCoordinatorRoleConfig { @@ -1414,4 +1458,37 @@ mod tests { .expect("Failed to parse TrinoClusterSpec YAML") } } + + #[test] + fn trino_config_overrides_merge_role_group_wins() { + use stackable_operator::config::merge::Merge; + + let mut rg = v1alpha1::TrinoConfigOverrides { + config_properties: Some(stackable_operator::config_overrides::KeyValueConfigOverrides { + overrides: [ + ("k_both".to_string(), "rg".to_string()), + ("k_rg_only".to_string(), "rg".to_string()), + ] + .into(), + }), + ..Default::default() + }; + let role = v1alpha1::TrinoConfigOverrides { + config_properties: Some(stackable_operator::config_overrides::KeyValueConfigOverrides { + overrides: [ + ("k_both".to_string(), "role".to_string()), + ("k_role_only".to_string(), "role".to_string()), + ] + .into(), + }), + ..Default::default() + }; + + rg.merge(&role); + + let merged = rg.config_properties.unwrap().overrides; + assert_eq!(merged.get("k_both").map(String::as_str), Some("rg")); + assert_eq!(merged.get("k_rg_only").map(String::as_str), Some("rg")); + assert_eq!(merged.get("k_role_only").map(String::as_str), Some("role")); + } } From 0cc9f49e0c5f4044b1ed77ca43fd7d9d3e666d90 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 19:50:49 +0200 Subject: [PATCH 04/27] feat(controller): introduce ValidatedCluster and validate_v2 Adds the typed ValidatedCluster state and a v2 validate function. Since upstream stackable_operator::v2::role_utils is not exported from operator-rs lib.rs on the smooth-operator branch, we vendor a minimal local version under src/framework/role_utils.rs (env_overrides kept as HashMap for simplicity, no EnvVarSet). Notes: - JavaCommonConfig does not impl Merge upstream (its inner JvmArgumentOverrides::try_merge is fallible). The vendored with_validated_config drops the Merge bound on CommonConfig and lets product_specific_common_config carry the role-group level value through. Trino's JVM merging continues to flow through Role::get_merged_jvm_argument_overrides. - TrinoConfig::default_config is made pub so validate_v2 can call it. - TrinoRole derives Ord/PartialOrd so it can key BTreeMap. - TrinoOpaConfig, CatalogConfig, ResolvedClientProtocolConfig and ResolvedFaultTolerantExecutionConfig now derive Clone + Debug, so ValidatedCluster can derive them. The old validate function and the rest of the pipeline are untouched in this commit; subsequent commits switch reconcile over and remove the old paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- rust/operator-binary/src/authorization/opa.rs | 1 + rust/operator-binary/src/catalog/config.rs | 1 + .../src/config/client_protocol.rs | 1 + .../src/config/fault_tolerant_execution.rs | 1 + rust/operator-binary/src/controller.rs | 49 +++++ .../src/controller/validate.rs | 197 ++++++++++++++++++ rust/operator-binary/src/crd/mod.rs | 4 +- rust/operator-binary/src/framework/mod.rs | 10 + .../src/framework/role_utils.rs | 150 +++++++++++++ rust/operator-binary/src/main.rs | 1 + 10 files changed, 414 insertions(+), 1 deletion(-) create mode 100644 rust/operator-binary/src/framework/mod.rs create mode 100644 rust/operator-binary/src/framework/role_utils.rs diff --git a/rust/operator-binary/src/authorization/opa.rs b/rust/operator-binary/src/authorization/opa.rs index 1fa9bd95..0ae42bd2 100644 --- a/rust/operator-binary/src/authorization/opa.rs +++ b/rust/operator-binary/src/authorization/opa.rs @@ -9,6 +9,7 @@ use crate::crd::v1alpha1; pub const OPA_TLS_VOLUME_NAME: &str = "opa-tls"; +#[derive(Clone, Debug)] pub struct TrinoOpaConfig { /// URI for OPA policies, e.g. /// `http://localhost:8081/v1/data/trino/allow` diff --git a/rust/operator-binary/src/catalog/config.rs b/rust/operator-binary/src/catalog/config.rs index 8d4ef553..5104b772 100644 --- a/rust/operator-binary/src/catalog/config.rs +++ b/rust/operator-binary/src/catalog/config.rs @@ -11,6 +11,7 @@ use stackable_operator::{ use super::{FromTrinoCatalogError, ToCatalogConfig}; use crate::crd::catalog::{TrinoCatalogConnector, v1alpha1}; +#[derive(Clone, Debug)] pub struct CatalogConfig { /// Name of the catalog pub name: String, diff --git a/rust/operator-binary/src/config/client_protocol.rs b/rust/operator-binary/src/config/client_protocol.rs index 96354c21..72186514 100644 --- a/rust/operator-binary/src/config/client_protocol.rs +++ b/rust/operator-binary/src/config/client_protocol.rs @@ -31,6 +31,7 @@ pub enum Error { }, } +#[derive(Clone, Debug)] pub struct ResolvedClientProtocolConfig { /// Properties to add to config.properties pub config_properties: BTreeMap, diff --git a/rust/operator-binary/src/config/fault_tolerant_execution.rs b/rust/operator-binary/src/config/fault_tolerant_execution.rs index 852f15c3..23d46a70 100644 --- a/rust/operator-binary/src/config/fault_tolerant_execution.rs +++ b/rust/operator-binary/src/config/fault_tolerant_execution.rs @@ -42,6 +42,7 @@ pub enum Error { } /// Fault tolerant execution configuration with external resources resolved +#[derive(Clone, Debug)] pub struct ResolvedFaultTolerantExecutionConfig { /// Properties to add to config.properties pub config_properties: BTreeMap, diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 3359d864..b7620b81 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -104,6 +104,55 @@ use crate::{ service::{build_rolegroup_headless_service, build_rolegroup_metrics_service}, }; +/// Validated, fully-typed view of a `TrinoCluster` after dereferencing and merging. +/// +/// Produced by `controller::validate::validate_v2`. Consumed by `controller::build::*`. +/// Carries everything downstream needs — no `&v1alpha1::TrinoCluster` or +/// `&DereferencedObjects` survives past validate. +#[derive(Clone, Debug)] +#[allow(dead_code)] // wired up in Task 14 (switch reconcile to ValidatedCluster pipeline) +pub struct ValidatedCluster { + pub metadata: stackable_operator::k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta, + pub name: String, + pub namespace: String, + pub uid: String, + pub image: + stackable_operator::commons::product_image_selection::ResolvedProductImage, + pub product_version: u16, + + // CRD facts absorbed from &TrinoCluster. + pub server_tls: Option, + pub internal_tls: Option, + pub authentication_enabled: bool, + pub catalog_label_selector: + stackable_operator::k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector, + pub cluster_operation: stackable_operator::commons::cluster_operation::ClusterOperation, + pub object_overrides: stackable_operator::deep_merger::ObjectOverrides, + + // Dereferenced state absorbed from `DereferencedObjects`. + pub trino_authentication_config: crate::authentication::TrinoAuthenticationConfig, + pub trino_opa_config: Option, + pub resolved_fte_config: + Option, + pub resolved_client_protocol_config: + Option, + pub catalogs: Vec, + pub coordinator_pod_refs: Vec, + + pub role_group_configs: std::collections::BTreeMap< + crate::crd::TrinoRole, + std::collections::BTreeMap, + >, +} + +pub type RoleGroupName = String; + +pub type TrinoRoleGroupConfig = crate::framework::role_utils::RoleGroupConfig< + crate::crd::v1alpha1::TrinoConfig, + stackable_operator::role_utils::JavaCommonConfig, + crate::crd::v1alpha1::TrinoConfigOverrides, +>; + pub struct Ctx { pub client: stackable_operator::client::Client, pub product_config: ProductConfigManager, diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 8d16b787..255d7704 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -62,6 +62,20 @@ pub enum Error { InvalidProductConfig { source: stackable_operator::product_config_utils::Error, }, + + #[snafu(display("unable to parse Trino version: {product_version:?}"))] + ParseTrinoVersion { + source: std::num::ParseIntError, + product_version: String, + }, + + #[snafu(display("failed to validate config fragment"))] + InvalidConfigFragment { + source: stackable_operator::config::fragment::ValidationError, + }, + + #[snafu(display("failed to enumerate coordinator pods"))] + CoordinatorPods { source: crate::crd::Error }, } type Result = std::result::Result; @@ -176,3 +190,186 @@ pub(super) fn validated_product_config( validate_all_roles_and_groups_config(version, &role_config, product_config, false, false) .context(InvalidProductConfigSnafu) } + +/// New validator: produces the typed [`super::ValidatedCluster`]. +/// +/// Replaces the legacy product-config-driven `validate` pipeline. See Task 4 of the +/// product-config removal plan. Wired up in Task 14. +#[allow(dead_code)] +pub fn validate_v2( + trino: &v1alpha1::TrinoCluster, + dereferenced_objects: &super::dereference::DereferencedObjects, + operator_environment: &OperatorEnvironmentOptions, +) -> Result { + use std::str::FromStr; + + use stackable_operator::kube::ResourceExt as _; + + let resolved_product_image = trino + .spec + .image + .resolve( + super::CONTAINER_IMAGE_BASE_NAME, + &operator_environment.image_repository, + crate::built_info::PKG_VERSION, + ) + .context(ResolveProductImageSnafu)?; + + let product_version = u16::from_str(&resolved_product_image.product_version).context( + ParseTrinoVersionSnafu { + product_version: resolved_product_image.product_version.clone(), + }, + )?; + + let trino_authentication_config = TrinoAuthenticationConfig::new( + &resolved_product_image, + TrinoAuthenticationTypes::try_from( + dereferenced_objects.resolved_authentication_classes.clone(), + ) + .context(UnsupportedAuthenticationConfigSnafu)?, + ) + .context(InvalidAuthenticationConfigSnafu)?; + + if dereferenced_objects + .resolved_client_protocol_config + .is_some() + && resolved_product_image.product_version.starts_with("45") + { + return Err(Error::ClientSpoolingProtocolTrinoVersion { + product_version: resolved_product_image.product_version.clone(), + }); + } + + let mut role_group_configs: std::collections::BTreeMap< + TrinoRole, + std::collections::BTreeMap, + > = std::collections::BTreeMap::new(); + + for trino_role in [TrinoRole::Coordinator, TrinoRole::Worker] { + let role = trino + .role(&trino_role) + .with_context(|_| MissingTrinoRoleSnafu { + role: trino_role.to_string(), + })?; + let default_config = v1alpha1::TrinoConfig::default_config( + &trino.name_any(), + &trino_role, + &dereferenced_objects.catalog_definitions, + ); + let mut groups = std::collections::BTreeMap::new(); + for (rg_name, rg) in &role.role_groups { + let validated_rg = crate::framework::role_utils::with_validated_config::< + v1alpha1::TrinoConfig, + stackable_operator::role_utils::JavaCommonConfig, + v1alpha1::TrinoConfigFragment, + stackable_operator::role_utils::GenericRoleConfig, + v1alpha1::TrinoConfigOverrides, + >(rg, &role, &default_config) + .context(InvalidConfigFragmentSnafu)?; + groups.insert(rg_name.clone(), validated_rg); + } + role_group_configs.insert(trino_role, groups); + } + + let coordinator_pod_refs = trino + .coordinator_pods() + .context(CoordinatorPodsSnafu)? + .collect(); + + Ok(super::ValidatedCluster { + metadata: stackable_operator::k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta { + name: trino.metadata.name.clone(), + namespace: trino.metadata.namespace.clone(), + uid: trino.metadata.uid.clone(), + ..Default::default() + }, + name: trino.metadata.name.clone().unwrap_or_default(), + namespace: trino.metadata.namespace.clone().unwrap_or_default(), + uid: trino.metadata.uid.clone().unwrap_or_default(), + image: resolved_product_image, + product_version, + server_tls: trino.get_server_tls().map(String::from), + internal_tls: trino.get_internal_tls().map(String::from), + authentication_enabled: trino.authentication_enabled(), + catalog_label_selector: trino.spec.cluster_config.catalog_label_selector.clone(), + cluster_operation: trino.spec.cluster_operation.clone(), + object_overrides: trino.spec.object_overrides.clone(), + trino_authentication_config, + trino_opa_config: dereferenced_objects.trino_opa_config.clone(), + resolved_fte_config: dereferenced_objects.resolved_fte_config.clone(), + resolved_client_protocol_config: dereferenced_objects + .resolved_client_protocol_config + .clone(), + catalogs: dereferenced_objects.catalogs.clone(), + coordinator_pod_refs, + role_group_configs, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validate_v2_minimal_cluster() { + let trino_yaml = r#" + apiVersion: trino.stackable.tech/v1alpha1 + kind: TrinoCluster + metadata: + name: simple-trino + namespace: default + uid: "42" + spec: + image: + productVersion: "479" + clusterConfig: + catalogLabelSelector: {} + coordinators: + roleGroups: + default: + replicas: 1 + workers: + roleGroups: + default: + replicas: 1 + "#; + let trino: v1alpha1::TrinoCluster = + serde_yaml::from_str(trino_yaml).expect("invalid test input"); + + let derefs = super::super::dereference::DereferencedObjects { + resolved_authentication_classes: Vec::new(), + catalog_definitions: Vec::new(), + catalogs: Vec::new(), + trino_opa_config: None, + resolved_fte_config: None, + resolved_client_protocol_config: None, + }; + let operator_env = stackable_operator::cli::OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_string(), + operator_service_name: "trino-operator".to_string(), + image_repository: "oci.example.org".to_string(), + }; + + let validated = + validate_v2(&trino, &derefs, &operator_env).expect("validate_v2 should succeed"); + + assert_eq!(validated.name, "simple-trino"); + assert_eq!(validated.namespace, "default"); + assert_eq!(validated.product_version, 479); + assert!(!validated.authentication_enabled); + assert!( + validated + .role_group_configs + .contains_key(&TrinoRole::Coordinator) + ); + assert!( + validated + .role_group_configs + .contains_key(&TrinoRole::Worker) + ); + assert_eq!( + validated.role_group_configs[&TrinoRole::Coordinator]["default"].replicas, + 1 + ); + } +} diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 9c6d03db..50d32535 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -557,7 +557,9 @@ fn tls_secret_class_default() -> Option { Eq, Hash, JsonSchema, + Ord, PartialEq, + PartialOrd, Serialize, EnumString, )] @@ -639,7 +641,7 @@ pub enum Container { } impl v1alpha1::TrinoConfig { - fn default_config( + pub fn default_config( cluster_name: &str, role: &TrinoRole, trino_catalogs: &[catalog::v1alpha1::TrinoCatalog], diff --git a/rust/operator-binary/src/framework/mod.rs b/rust/operator-binary/src/framework/mod.rs new file mode 100644 index 00000000..d0a9b1bb --- /dev/null +++ b/rust/operator-binary/src/framework/mod.rs @@ -0,0 +1,10 @@ +//! Local framework helpers that mirror the work-in-progress upstream +//! `stackable_operator::v2::*` modules. +//! +//! The upstream `v2` module on the `smooth-operator` branch is not yet exported +//! from `lib.rs`, so we vendor the small subset of helpers we need. +//! +//! Follow-up: replace these with `stackable_operator::v2::*` imports once +//! upstream publishes the module. + +pub mod role_utils; diff --git a/rust/operator-binary/src/framework/role_utils.rs b/rust/operator-binary/src/framework/role_utils.rs new file mode 100644 index 00000000..5c2e1821 --- /dev/null +++ b/rust/operator-binary/src/framework/role_utils.rs @@ -0,0 +1,150 @@ +//! Vendored variant of `stackable_operator::v2::role_utils` from the +//! `smooth-operator` branch, with simplifications appropriate for trino-operator. +//! +//! Differences from upstream: +//! - `env_overrides` is `HashMap` instead of `EnvVarSet`. +//! - No `cli_overrides_to_vec` helper, `ResourceNames`, or service-account helpers. +//! - The `CommonConfig` (a.k.a. `product_specific_common_config`) does NOT need to +//! implement `Merge`. Upstream Trino uses `JavaCommonConfig`, which intentionally +//! does not implement `Merge` because its inner `JvmArgumentOverrides::try_merge` +//! is fallible (regex validation). Merging JVM argument overrides for Trino is +//! handled separately via `Role::get_merged_jvm_argument_overrides`. The +//! `RoleGroupConfig::product_specific_common_config` field here simply carries +//! the role-group level value through. +//! +//! Replace with `stackable_operator::v2::role_utils::*` once upstream publishes +//! the module. + +use std::collections::{BTreeMap, HashMap}; + +use serde::Serialize; +use stackable_operator::{ + config::{ + fragment::{self, FromFragment}, + merge::{Merge, merge}, + }, + k8s_openapi::{DeepMerge, api::core::v1::PodTemplateSpec}, + role_utils::{Role, RoleGroup}, + schemars::JsonSchema, +}; + +/// Trino-friendly view of a validated, merged `RoleGroup`. +/// +/// Mirrors `stackable_operator::v2::role_utils::RoleGroupConfig` on the +/// `smooth-operator` branch, with `env_overrides: HashMap` +/// instead of the upstream `EnvVarSet`. +#[derive(Clone, Debug, PartialEq)] +pub struct RoleGroupConfig { + pub replicas: u16, + pub config: Config, + pub config_overrides: ConfigOverrides, + pub env_overrides: HashMap, + pub cli_overrides: BTreeMap, + pub pod_overrides: PodTemplateSpec, + pub product_specific_common_config: CommonConfig, +} + +/// Merges and validates the `RoleGroup` with the given `role` and `default_config`, +/// returning a `RoleGroupConfig`. +/// +/// Merge order matches `with_validated_config` on `smooth-operator`: +/// - `Config` (Fragment): `default_config <- role.config <- rg.config` via `Merge::merge`, +/// then validated to `ValidatedConfig` via `FromFragment`. +/// - `ConfigOverrides`: `role.config_overrides <- rg.config_overrides` via `Merge::merge`. +/// - `env_overrides` / `cli_overrides`: `extend` (rg keys overwrite role keys). +/// - `pod_overrides`: `DeepMerge::merge_from` (rg overrides role). +/// - `product_specific_common_config`: passes through the role-group level value +/// (see module docs for rationale). +pub fn with_validated_config( + role_group: &RoleGroup, + role: &Role, + default_config: &Config, +) -> Result< + RoleGroupConfig, + fragment::ValidationError, +> +where + ValidatedConfig: FromFragment, + CommonConfig: Clone + Default + JsonSchema + Serialize, + Config: Clone + Merge, + RoleConfig: Default + JsonSchema + Serialize, + ConfigOverrides: Clone + Default + JsonSchema + Merge + Serialize, +{ + let validated_config = validate_config(role_group, role, default_config)?; + Ok(RoleGroupConfig { + replicas: role_group.replicas.unwrap_or(1), + config: validated_config, + config_overrides: merged_config_overrides( + &role.config.config_overrides, + role_group.config.config_overrides.clone(), + ), + env_overrides: merged_env_overrides( + role.config.env_overrides.clone(), + role_group.config.env_overrides.clone(), + ), + cli_overrides: merged_cli_overrides( + role.config.cli_overrides.clone(), + role_group.config.cli_overrides.clone(), + ), + pod_overrides: merged_pod_overrides( + role.config.pod_overrides.clone(), + role_group.config.pod_overrides.clone(), + ), + product_specific_common_config: role_group + .config + .product_specific_common_config + .clone(), + }) +} + +fn validate_config( + role_group: &RoleGroup, + role: &Role, + default_config: &Config, +) -> Result +where + ValidatedConfig: FromFragment, + CommonConfig: Default + JsonSchema + Serialize, + Config: Clone + Merge, + RoleConfig: Default + JsonSchema + Serialize, + ConfigOverrides: Default + JsonSchema + Serialize, +{ + role_group.validate_config(role, default_config) +} + +fn merged_config_overrides( + role_config_overrides: &ConfigOverrides, + role_group_config_overrides: ConfigOverrides, +) -> ConfigOverrides +where + ConfigOverrides: Merge, +{ + merge(role_group_config_overrides, role_config_overrides) +} + +fn merged_env_overrides( + role_env_overrides: HashMap, + role_group_env_overrides: HashMap, +) -> HashMap { + let mut merged = role_env_overrides; + merged.extend(role_group_env_overrides); + merged +} + +fn merged_cli_overrides( + role_cli_overrides: BTreeMap, + role_group_cli_overrides: BTreeMap, +) -> BTreeMap { + let mut merged = role_cli_overrides; + merged.extend(role_group_cli_overrides); + merged +} + +fn merged_pod_overrides( + role_pod_overrides: PodTemplateSpec, + role_group_pod_overrides: PodTemplateSpec, +) -> PodTemplateSpec { + let mut merged = role_pod_overrides; + merged.merge_from(role_group_pod_overrides); + merged +} diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 223765ad..beb35047 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -49,6 +49,7 @@ mod command; mod config; mod controller; mod crd; +mod framework; mod listener; mod operations; mod product_logging; From 5dea4c4476b5239fb177d380780980bf6a294b8b Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:09:25 +0200 Subject: [PATCH 05/27] feat(build): add log_properties builder Adds controller/build/properties/log_properties.rs producing log.properties from defaults + container logging config + user overrides. The io.trino=INFO default is migrated from deploy/config-spec/properties.yaml. Tests are added in a later commit once the shared ValidatedCluster fixture exists. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../build/properties/log_properties.rs | 43 +++++++++++++++++++ .../src/controller/build/properties/mod.rs | 1 + rust/operator-binary/src/product_logging.rs | 32 ++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/properties/log_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs new file mode 100644 index 00000000..f3263969 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -0,0 +1,43 @@ +//! Builder for `log.properties`. + +use std::collections::BTreeMap; + +use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; +use crate::crd::TrinoRole; + +const IO_TRINO: &str = "io.trino"; +const DEFAULT_IO_TRINO: &str = "INFO"; + +/// Build the `log.properties` key/value pairs for `(role, rg)`. +/// +/// Returns an empty map when there is nothing to write — callers +/// should omit the file from the ConfigMap if the result is empty. +pub fn build( + cluster: &ValidatedCluster, + role: TrinoRole, + rg: &TrinoRoleGroupConfig, +) -> BTreeMap { + let mut props = BTreeMap::new(); + + // 1. Defaults (lowest precedence) — migrated from deploy/config-spec/properties.yaml. + props.insert(IO_TRINO.to_string(), DEFAULT_IO_TRINO.to_string()); + + // 2. Automatic — per-container logger levels derived from rg.config.logging. + if let Some(per_container) = crate::product_logging::get_log_property_map(&rg.config.logging) { + props.extend(per_container); + } + + // 3. (no merged_config contribution for log.properties beyond logging tree) + // 4. User overrides (highest precedence). + if let Some(kv) = &rg.config_overrides.log_properties { + props.extend(kv.overrides.clone()); + } + + let _ = (cluster, role); // suppress unused for now; may use later for role-aware logging + props +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index cb6de00a..d9e9edef 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -4,4 +4,5 @@ //! Trino config file. The shared [`writer`] module serializes the map to the //! Java-properties on-wire format. +pub mod log_properties; pub mod writer; diff --git a/rust/operator-binary/src/product_logging.rs b/rust/operator-binary/src/product_logging.rs index 0906218d..8fceac5d 100644 --- a/rust/operator-binary/src/product_logging.rs +++ b/rust/operator-binary/src/product_logging.rs @@ -67,6 +67,38 @@ pub fn get_log_properties(logging: &Logging) -> Option { } } +/// Same as [`get_log_properties`] but returns a typed `BTreeMap` instead of +/// a Java-properties-formatted string. +/// +/// New per-file builders use this; the old `get_log_properties` will be removed +/// once the legacy `build_rolegroup_config_map` is deleted (see Task 14). +pub fn get_log_property_map( + logging: &Logging, +) -> Option> { + if let Some(ContainerLogConfig { + choice: Some(ContainerLogConfigChoice::Automatic(log_config)), + }) = logging.containers.get(&Container::Trino) + { + let map = log_config + .loggers + .iter() + .map(|(logger, config)| { + let log_level = TrinoLogLevel::from(config.level); + let key = if logger == AutomaticContainerLogConfig::ROOT_LOGGER { + // ROOT logger maps to an empty key in log.properties (=LEVEL). + String::new() + } else { + logger.clone() + }; + (key, log_level.to_string()) + }) + .collect(); + Some(map) + } else { + None + } +} + /// Return the vector toml configuration pub fn get_vector_toml( rolegroup: &RoleGroupRef, From 97bdcb17d5dc8ce8dc0cbeed5812b155f0180c92 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:15:35 +0200 Subject: [PATCH 06/27] chore(build): silence transient dead_code warnings The vendored Java-properties writer (Task 2), the log_properties builder (Task 5), and the get_log_property_map helper have no callers until build/config_map.rs is wired in at Task 12. Annotate each with #[allow(dead_code)] to keep `cargo check` clean during the intermediate commits. The annotations will be removed when callers land. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controller/build/properties/log_properties.rs | 3 +++ .../operator-binary/src/controller/build/properties/writer.rs | 4 ++++ rust/operator-binary/src/product_logging.rs | 3 +++ 3 files changed, 10 insertions(+) diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs index f3263969..3d88955f 100644 --- a/rust/operator-binary/src/controller/build/properties/log_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -12,6 +12,9 @@ const DEFAULT_IO_TRINO: &str = "INFO"; /// /// Returns an empty map when there is nothing to write — callers /// should omit the file from the ConfigMap if the result is empty. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, role: TrinoRole, diff --git a/rust/operator-binary/src/controller/build/properties/writer.rs b/rust/operator-binary/src/controller/build/properties/writer.rs index 99e60999..48ebb886 100644 --- a/rust/operator-binary/src/controller/build/properties/writer.rs +++ b/rust/operator-binary/src/controller/build/properties/writer.rs @@ -8,6 +8,9 @@ use std::collections::BTreeMap; use snafu::{ResultExt, Snafu}; +// Until callers exist (wired in by build/config_map.rs in Task 12), the +// writer is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] #[derive(Debug, Snafu)] pub enum Error { #[snafu(display("failed to write properties output"))] @@ -18,6 +21,7 @@ pub enum Error { /// /// Keys and values are escaped per : /// `:`, `=`, `#`, `!`, `\\`, leading whitespace, and ` ` (space). +#[allow(dead_code)] pub fn to_java_properties_string( props: &BTreeMap, ) -> Result { diff --git a/rust/operator-binary/src/product_logging.rs b/rust/operator-binary/src/product_logging.rs index 8fceac5d..38900037 100644 --- a/rust/operator-binary/src/product_logging.rs +++ b/rust/operator-binary/src/product_logging.rs @@ -72,6 +72,9 @@ pub fn get_log_properties(logging: &Logging) -> Option { /// /// New per-file builders use this; the old `get_log_properties` will be removed /// once the legacy `build_rolegroup_config_map` is deleted (see Task 14). +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// helper is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] pub fn get_log_property_map( logging: &Logging, ) -> Option> { From 10f4463ceabf81f4b1455c16fab1a346b2aa7e67 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:16:53 +0200 Subject: [PATCH 07/27] feat(build): add security_properties builder Migrates networkaddress.cache.ttl=30 and networkaddress.cache.negative.ttl=0 from deploy/config-spec/properties.yaml into the new builder. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controller/build/properties/mod.rs | 1 + .../build/properties/security_properties.rs | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/properties/security_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index d9e9edef..5eeabb07 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -5,4 +5,5 @@ //! Java-properties on-wire format. pub mod log_properties; +pub mod security_properties; pub mod writer; diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs new file mode 100644 index 00000000..4a8e0c69 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -0,0 +1,45 @@ +//! Builder for `security.properties` (Trino's JVM security properties file). + +use std::collections::BTreeMap; + +use crate::controller::TrinoRoleGroupConfig; + +const NETWORKADDRESS_CACHE_TTL: &str = "networkaddress.cache.ttl"; +const NETWORKADDRESS_CACHE_NEGATIVE_TTL: &str = "networkaddress.cache.negative.ttl"; + +const DEFAULT_NETWORKADDRESS_CACHE_TTL: &str = "30"; +const DEFAULT_NETWORKADDRESS_CACHE_NEGATIVE_TTL: &str = "0"; + +/// Build the `security.properties` key/value pairs. +/// +/// Both keys apply to both `coordinator` and `worker` roles. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn build(rg: &TrinoRoleGroupConfig) -> BTreeMap { + let mut props = BTreeMap::new(); + + // 1. Defaults — migrated from deploy/config-spec/properties.yaml. + props.insert( + NETWORKADDRESS_CACHE_TTL.to_string(), + DEFAULT_NETWORKADDRESS_CACHE_TTL.to_string(), + ); + props.insert( + NETWORKADDRESS_CACHE_NEGATIVE_TTL.to_string(), + DEFAULT_NETWORKADDRESS_CACHE_NEGATIVE_TTL.to_string(), + ); + + // 2. No automatic operator-injected values. + // 3. No merged_config contribution. + // 4. User overrides (highest precedence). + if let Some(kv) = &rg.config_overrides.security_properties { + props.extend(kv.overrides.clone()); + } + + props +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} From 26aaf9849e92bab470fd55388642e67cf683969e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:19:04 +0200 Subject: [PATCH 08/27] feat(build): add node_properties builder Moves the node.environment derivation out of compute_files into a typed builder. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controller/build/properties/mod.rs | 1 + .../build/properties/node_properties.rs | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/properties/node_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 5eeabb07..5c3963e1 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -5,5 +5,6 @@ //! Java-properties on-wire format. pub mod log_properties; +pub mod node_properties; pub mod security_properties; pub mod writer; diff --git a/rust/operator-binary/src/controller/build/properties/node_properties.rs b/rust/operator-binary/src/controller/build/properties/node_properties.rs new file mode 100644 index 00000000..c9a5b7c9 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/node_properties.rs @@ -0,0 +1,40 @@ +//! Builder for `node.properties`. + +use std::collections::BTreeMap; + +use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; + +const NODE_ENVIRONMENT: &str = "node.environment"; + +/// Build the `node.properties` key/value pairs. +/// +/// `node.environment` is derived from the cluster name: lowercased, with `-` +/// replaced by `_`. Trino requires `^[a-z][a-z0-9_]*[a-z0-9]$`; cluster names +/// constrained by Kubernetes naming already satisfy this after the transform. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn build( + cluster: &ValidatedCluster, + rg: &TrinoRoleGroupConfig, +) -> BTreeMap { + let mut props = BTreeMap::new(); + + // 1. No defaults. + // 2. Automatic — derived from cluster name. + let node_env = cluster.name.to_ascii_lowercase().replace('-', "_"); + props.insert(NODE_ENVIRONMENT.to_string(), node_env); + + // 3. No merged_config contribution for node.properties. + // 4. User overrides (highest precedence). + if let Some(kv) = &rg.config_overrides.node_properties { + props.extend(kv.overrides.clone()); + } + + props +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} From 5c15f28d275ca5a3f3dbc2919e87b7e913c1cc43 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:22:09 +0200 Subject: [PATCH 09/27] feat(build): add access_control_properties builder Lifts the OPA config injection out of build_rolegroup_config_map into a typed builder. TrinoOpaConfig::as_config() values that are None are dropped at this point (matches the old product-config write-time behavior); user overrides win. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../properties/access_control_properties.rs | 45 +++++++++++++++++++ .../src/controller/build/properties/mod.rs | 1 + 2 files changed, 46 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/properties/access_control_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs new file mode 100644 index 00000000..214ee025 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs @@ -0,0 +1,45 @@ +//! Builder for `access-control.properties`. + +use std::collections::BTreeMap; + +use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; + +/// Build the `access-control.properties` key/value pairs. +/// +/// Returns an empty map when neither OPA authorization is configured nor +/// user overrides are provided — callers should omit the file from the +/// ConfigMap in that case. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn build( + cluster: &ValidatedCluster, + rg: &TrinoRoleGroupConfig, +) -> BTreeMap { + let mut props = BTreeMap::new(); + + // 1. No defaults. + // 2. Automatic — OPA config when configured. + // TrinoOpaConfig::as_config() returns BTreeMap>; + // drop the None values (matches old product-config write-time behavior). + if let Some(opa) = &cluster.trino_opa_config { + props.extend( + opa.as_config() + .into_iter() + .filter_map(|(k, v)| v.map(|v| (k, v))), + ); + } + + // 3. No merged_config contribution. + // 4. User overrides (highest precedence). + if let Some(kv) = &rg.config_overrides.access_control_properties { + props.extend(kv.overrides.clone()); + } + + props +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 5c3963e1..894025f4 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -4,6 +4,7 @@ //! Trino config file. The shared [`writer`] module serializes the map to the //! Java-properties on-wire format. +pub mod access_control_properties; pub mod log_properties; pub mod node_properties; pub mod security_properties; From 68bb0deb3218d1137f2c8ca1b1b8ec7c097208ef Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:27:43 +0200 Subject: [PATCH 10/27] feat(build): add exchange_manager_properties builder Lifts the resolved-FTE exchange-manager properties out of build_rolegroup_config_map into a typed builder. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../properties/exchange_manager_properties.rs | 39 +++++++++++++++++++ .../src/controller/build/properties/mod.rs | 1 + 2 files changed, 40 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs new file mode 100644 index 00000000..fc57115d --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs @@ -0,0 +1,39 @@ +//! Builder for `exchange-manager.properties`. + +use std::collections::BTreeMap; + +use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; + +/// Build the `exchange-manager.properties` key/value pairs. +/// +/// Returns an empty map when fault-tolerant execution is not configured and no +/// user overrides are provided — callers should omit the file from the +/// ConfigMap in that case. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn build( + cluster: &ValidatedCluster, + rg: &TrinoRoleGroupConfig, +) -> BTreeMap { + let mut props = BTreeMap::new(); + + // 1. No defaults. + // 2. Automatic — from resolved fault-tolerant-execution config. + if let Some(fte) = &cluster.resolved_fte_config { + props.extend(fte.exchange_manager_properties.clone()); + } + + // 3. No merged_config contribution. + // 4. User overrides (highest precedence). + if let Some(kv) = &rg.config_overrides.exchange_manager_properties { + props.extend(kv.overrides.clone()); + } + + props +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 894025f4..62c9a4b5 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -5,6 +5,7 @@ //! Java-properties on-wire format. pub mod access_control_properties; +pub mod exchange_manager_properties; pub mod log_properties; pub mod node_properties; pub mod security_properties; From 825801674bf3e2db477d26a2f46feaa3470fc0dd Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:29:37 +0200 Subject: [PATCH 11/27] feat(build): add spooling_manager_properties builder Lifts the resolved client-spooling protocol properties out of build_rolegroup_config_map into a typed builder. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controller/build/properties/mod.rs | 1 + .../properties/spooling_manager_properties.rs | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 62c9a4b5..3037cd13 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -6,6 +6,7 @@ pub mod access_control_properties; pub mod exchange_manager_properties; +pub mod spooling_manager_properties; pub mod log_properties; pub mod node_properties; pub mod security_properties; diff --git a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs new file mode 100644 index 00000000..34e188c2 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs @@ -0,0 +1,39 @@ +//! Builder for `spooling-manager.properties`. + +use std::collections::BTreeMap; + +use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; + +/// Build the `spooling-manager.properties` key/value pairs. +/// +/// Returns an empty map when client spooling is not configured and no user +/// overrides are provided — callers should omit the file from the ConfigMap +/// in that case. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn build( + cluster: &ValidatedCluster, + rg: &TrinoRoleGroupConfig, +) -> BTreeMap { + let mut props = BTreeMap::new(); + + // 1. No defaults. + // 2. Automatic — from resolved client-spooling protocol config. + if let Some(spooling) = &cluster.resolved_client_protocol_config { + props.extend(spooling.spooling_manager_properties.clone()); + } + + // 3. No merged_config contribution. + // 4. User overrides (highest precedence). + if let Some(kv) = &rg.config_overrides.spooling_manager_properties { + props.extend(kv.overrides.clone()); + } + + props +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} From 750d4b01d16ace4ab5d056260d6b7b33137f9c7c Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:41:47 +0200 Subject: [PATCH 12/27] feat(build): add config_properties builder Largest of the per-file builders. Reproduces today's config.properties content: defaults + compute_files contributions + dynamic_resolved_config block + user overrides. Migrates the node-scheduler.include-coordinator and query.max-memory defaults out of properties.yaml. Also adds a graceful_shutdown_config_properties_v2 helper taking &ValidatedCluster, used by the new builder. The legacy graceful_shutdown_config_properties stays until Task 15 cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../build/properties/config_properties.rs | 231 ++++++++++++++++++ .../src/controller/build/properties/mod.rs | 1 + .../src/operations/graceful_shutdown.rs | 57 ++++- 3 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 rust/operator-binary/src/controller/build/properties/config_properties.rs diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs new file mode 100644 index 00000000..9fd39c94 --- /dev/null +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -0,0 +1,231 @@ +//! Builder for `config.properties` — the main Trino server config. + +use std::collections::BTreeMap; +use std::ops::Div; + +use snafu::Snafu; +use stackable_operator::{memory::BinaryMultiple, utils::cluster_info::KubernetesClusterInfo}; + +use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; +use crate::crd::{ + COORDINATOR, Container, DISCOVERY_URI, ENV_INTERNAL_SECRET, HTTP_PORT, + HTTP_SERVER_AUTHENTICATION_ALLOW_INSECURE_OVER_HTTP, HTTP_SERVER_HTTP_PORT, + HTTP_SERVER_HTTPS_ENABLED, HTTP_SERVER_HTTPS_KEYSTORE_KEY, HTTP_SERVER_HTTPS_PORT, + HTTP_SERVER_HTTPS_TRUSTSTORE_KEY, HTTP_SERVER_KEYSTORE_PATH, HTTP_SERVER_TRUSTSTORE_PATH, + HTTPS_PORT, INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_KEY, + INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_PATH, INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_KEY, + INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_PATH, INTERNAL_COMMUNICATION_SHARED_SECRET, + LOG_COMPRESSION, LOG_FORMAT, LOG_MAX_SIZE, LOG_MAX_TOTAL_SIZE, LOG_PATH, + MAX_TRINO_LOG_FILES_SIZE, NODE_INTERNAL_ADDRESS_SOURCE, NODE_INTERNAL_ADDRESS_SOURCE_FQDN, + QUERY_MAX_MEMORY, QUERY_MAX_MEMORY_PER_NODE, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_LOG_DIR, + STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, TrinoRole, + discovery::{TrinoDiscovery, TrinoDiscoveryProtocol}, +}; + +// Property names not exported from crd/mod.rs. +const NODE_SCHEDULER_INCLUDE_COORDINATOR: &str = "node-scheduler.include-coordinator"; +const HTTP_SERVER_LOG_ENABLED: &str = "http-server.log.enabled"; + +// Defaults migrated from deploy/config-spec/properties.yaml. +const DEFAULT_QUERY_MAX_MEMORY: &str = "50GB"; +const DEFAULT_NODE_SCHEDULER_INCLUDE_COORDINATOR: &str = "false"; + +// Mirrors the private constant of the same name in `crd/mod.rs`. +const LOG_FILE_COUNT: u32 = 2; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display( + "Trino requires client TLS to be enabled if any authentication method is enabled" + ))] + AuthenticationRequiresTls, +} + +/// Build the `config.properties` key/value pairs. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// builder is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn build( + cluster: &ValidatedCluster, + role: TrinoRole, + rg: &TrinoRoleGroupConfig, + cluster_info: &KubernetesClusterInfo, +) -> Result, Error> { + let mut props = BTreeMap::new(); + + // ---- 1. Hardcoded defaults (lowest precedence) ---- + // Migrated from deploy/config-spec/properties.yaml. + props.insert( + QUERY_MAX_MEMORY.to_string(), + DEFAULT_QUERY_MAX_MEMORY.to_string(), + ); + if role == TrinoRole::Coordinator { + props.insert( + NODE_SCHEDULER_INCLUDE_COORDINATOR.to_string(), + DEFAULT_NODE_SCHEDULER_INCLUDE_COORDINATOR.to_string(), + ); + } + + // ---- 2. Operator-injected automatic values ---- + props.insert( + COORDINATOR.to_string(), + (role == TrinoRole::Coordinator).to_string(), + ); + + // Trino's own JSON logging output. + props.insert(LOG_FORMAT.to_string(), "json".to_string()); + props.insert( + LOG_PATH.to_string(), + format!( + "{STACKABLE_LOG_DIR}/{container}/server.airlift.json", + container = Container::Trino + ), + ); + props.insert(LOG_COMPRESSION.to_string(), "none".to_string()); + props.insert( + LOG_MAX_SIZE.to_string(), + format!( + // Trino uses the unit "MB" for MiB. + "{}MB", + MAX_TRINO_LOG_FILES_SIZE + .scale_to(BinaryMultiple::Mebi) + .div(LOG_FILE_COUNT as f32) + .ceil() + .value, + ), + ); + props.insert( + LOG_MAX_TOTAL_SIZE.to_string(), + format!( + "{}MB", + MAX_TRINO_LOG_FILES_SIZE + .scale_to(BinaryMultiple::Mebi) + .ceil() + .value, + ), + ); + props.insert(HTTP_SERVER_LOG_ENABLED.to_string(), "false".to_string()); + props.insert( + INTERNAL_COMMUNICATION_SHARED_SECRET.to_string(), + format!("${{ENV:{ENV_INTERNAL_SECRET}}}"), + ); + + // TLS gating — mirrors the existing compute_files logic, including the + // authentication-requires-TLS check. + let server_tls_enabled = cluster.server_tls.is_some(); + let internal_tls_enabled = cluster.internal_tls.is_some(); + if cluster.authentication_enabled && !server_tls_enabled { + return Err(Error::AuthenticationRequiresTls); + } + if server_tls_enabled || internal_tls_enabled { + props.insert(HTTP_SERVER_HTTPS_ENABLED.to_string(), "true".to_string()); + props.insert(HTTP_SERVER_HTTPS_PORT.to_string(), HTTPS_PORT.to_string()); + let tls_store_dir = if server_tls_enabled { + STACKABLE_SERVER_TLS_DIR + } else { + // allow insecure communication via the http port + props.insert( + HTTP_SERVER_AUTHENTICATION_ALLOW_INSECURE_OVER_HTTP.to_string(), + "true".to_string(), + ); + props.insert(HTTP_SERVER_HTTP_PORT.to_string(), HTTP_PORT.to_string()); + STACKABLE_INTERNAL_TLS_DIR + }; + props.insert( + HTTP_SERVER_KEYSTORE_PATH.to_string(), + format!("{tls_store_dir}/keystore.p12"), + ); + props.insert( + HTTP_SERVER_HTTPS_KEYSTORE_KEY.to_string(), + STACKABLE_TLS_STORE_PASSWORD.to_string(), + ); + props.insert( + HTTP_SERVER_TRUSTSTORE_PATH.to_string(), + format!("{tls_store_dir}/truststore.p12"), + ); + props.insert( + HTTP_SERVER_HTTPS_TRUSTSTORE_KEY.to_string(), + STACKABLE_TLS_STORE_PASSWORD.to_string(), + ); + } + if internal_tls_enabled { + props.insert( + INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_PATH.to_string(), + format!("{STACKABLE_INTERNAL_TLS_DIR}/keystore.p12"), + ); + props.insert( + INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_KEY.to_string(), + STACKABLE_TLS_STORE_PASSWORD.to_string(), + ); + props.insert( + INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_PATH.to_string(), + format!("{STACKABLE_INTERNAL_TLS_DIR}/truststore.p12"), + ); + props.insert( + INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_KEY.to_string(), + STACKABLE_TLS_STORE_PASSWORD.to_string(), + ); + props.insert( + NODE_INTERNAL_ADDRESS_SOURCE.to_string(), + NODE_INTERNAL_ADDRESS_SOURCE_FQDN.to_string(), + ); + } + + // Authentication properties (only contributes when authentication is enabled). + for (k, v) in cluster + .trino_authentication_config + .config_properties(&role) + { + props.insert(k, v); + } + + // Discovery URI. + if let Some(coordinator_ref) = cluster.coordinator_pod_refs.first() { + let protocol = if internal_tls_enabled { + TrinoDiscoveryProtocol::Https + } else { + TrinoDiscoveryProtocol::Http + }; + let discovery = TrinoDiscovery::new(coordinator_ref, protocol); + props.insert( + DISCOVERY_URI.to_string(), + discovery.discovery_uri(cluster_info), + ); + } + // If there are no coordinator pods we silently skip — the old pipeline + // would have errored earlier (MissingCoordinatorPods). + + // Graceful shutdown. + for (k, v) in crate::operations::graceful_shutdown_config_properties_v2(cluster, role) { + props.insert(k, v); + } + + // Fault-tolerant execution. + if let Some(fte) = &cluster.resolved_fte_config { + props.extend(fte.config_properties.clone()); + } + // Client spooling protocol. + if let Some(spooling) = &cluster.resolved_client_protocol_config { + props.extend(spooling.config_properties.clone()); + } + + // ---- 3. merged_config CRD-spec values ---- + if let Some(qmm) = &rg.config.query_max_memory { + props.insert(QUERY_MAX_MEMORY.to_string(), qmm.clone()); + } + if let Some(qmmpn) = &rg.config.query_max_memory_per_node { + props.insert(QUERY_MAX_MEMORY_PER_NODE.to_string(), qmmpn.clone()); + } + + // ---- 4. User overrides (highest precedence) ---- + if let Some(kv) = &rg.config_overrides.config_properties { + props.extend(kv.overrides.clone()); + } + + Ok(props) +} + +#[cfg(test)] +mod tests { + // Tests added in Task 13 once the shared ValidatedCluster fixture exists. +} diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 3037cd13..1046983c 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -5,6 +5,7 @@ //! Java-properties on-wire format. pub mod access_control_properties; +pub mod config_properties; pub mod exchange_manager_properties; pub mod spooling_manager_properties; pub mod log_properties; diff --git a/rust/operator-binary/src/operations/graceful_shutdown.rs b/rust/operator-binary/src/operations/graceful_shutdown.rs index 50bad7ae..04b38a4a 100644 --- a/rust/operator-binary/src/operations/graceful_shutdown.rs +++ b/rust/operator-binary/src/operations/graceful_shutdown.rs @@ -9,8 +9,10 @@ use stackable_operator::{ k8s_openapi::api::core::v1::{ExecAction, LifecycleHandler}, }; +use crate::controller::ValidatedCluster; use crate::crd::{ - TrinoRole, WORKER_GRACEFUL_SHUTDOWN_SAFETY_OVERHEAD, WORKER_SHUTDOWN_GRACE_PERIOD, v1alpha1, + DEFAULT_WORKER_GRACEFUL_SHUTDOWN_TIMEOUT, TrinoRole, WORKER_GRACEFUL_SHUTDOWN_SAFETY_OVERHEAD, + WORKER_SHUTDOWN_GRACE_PERIOD, v1alpha1, }; #[derive(Debug, Snafu)] @@ -52,6 +54,59 @@ pub fn graceful_shutdown_config_properties( } } +/// V2 variant of [`graceful_shutdown_config_properties`] that reads from a +/// [`ValidatedCluster`]. The legacy function is preserved until reconcile is +/// switched (Task 14) and is then deleted (Task 15). +/// +/// Returns a flat `BTreeMap` (unlike the legacy variant which +/// returns `BTreeMap>`) — the new builders no longer +/// thread `Option` values through. +// Until callers exist (wired in by build/config_map.rs in Task 12), this +// helper is transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code)] +pub fn graceful_shutdown_config_properties_v2( + cluster: &ValidatedCluster, + role: TrinoRole, +) -> BTreeMap { + match role { + TrinoRole::Coordinator => { + // Only set query.max-execution-time if fault tolerant execution is not configured. + // With fault tolerant execution enabled, queries can be retried and run indefinitely. + if cluster.resolved_fte_config.is_none() { + let min_worker_graceful_shutdown_timeout = + min_worker_graceful_shutdown_timeout_v2(cluster); + BTreeMap::from([( + "query.max-execution-time".to_string(), + format!("{}s", min_worker_graceful_shutdown_timeout.as_secs()), + )]) + } else { + BTreeMap::new() + } + } + TrinoRole::Worker => BTreeMap::from([( + "shutdown.grace-period".to_string(), + format!("{}s", WORKER_SHUTDOWN_GRACE_PERIOD.as_secs()), + )]), + } +} + +/// Returns the minimal `gracefulShutdownTimeout` across all worker role-groups. +/// +/// Mirrors [`v1alpha1::TrinoCluster::min_worker_graceful_shutdown_timeout`] but +/// reads from [`ValidatedCluster::role_group_configs`]. +fn min_worker_graceful_shutdown_timeout_v2( + cluster: &ValidatedCluster, +) -> stackable_operator::shared::time::Duration { + cluster + .role_group_configs + .get(&TrinoRole::Worker) + .into_iter() + .flat_map(|groups| groups.values()) + .filter_map(|rg| rg.config.graceful_shutdown_timeout) + .min() + .unwrap_or(DEFAULT_WORKER_GRACEFUL_SHUTDOWN_TIMEOUT) +} + pub fn add_graceful_shutdown_config( trino: &v1alpha1::TrinoCluster, role: &TrinoRole, From 23dff6cc46c8fb2f89398132f37ba4aa6ae2709b Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 20:56:13 +0200 Subject: [PATCH 13/27] feat(build): add config_map builder taking &ValidatedCluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors today's build_rolegroup_config_map but consumes the typed ValidatedCluster only — no &TrinoCluster or DereferencedObjects threaded in. Reconcile is switched over in a follow-up commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controller/build/config_map.rs | 190 ++++++++++++++++++ .../src/controller/build/mod.rs | 1 + 2 files changed, 191 insertions(+) create mode 100644 rust/operator-binary/src/controller/build/config_map.rs diff --git a/rust/operator-binary/src/controller/build/config_map.rs b/rust/operator-binary/src/controller/build/config_map.rs new file mode 100644 index 00000000..d1a0eaa0 --- /dev/null +++ b/rust/operator-binary/src/controller/build/config_map.rs @@ -0,0 +1,190 @@ +//! Build per-rolegroup `ConfigMap` for the Trino cluster. + +use std::collections::BTreeMap; + +use snafu::{OptionExt, ResultExt, Snafu}; +use stackable_operator::{ + builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder}, + k8s_openapi::api::core::v1::ConfigMap, + kvp::ObjectLabels, + product_logging, + role_utils::RoleGroupRef, + utils::cluster_info::KubernetesClusterInfo, +}; + +use crate::controller::{ + ValidatedCluster, + build::properties::{ + access_control_properties, config_properties, exchange_manager_properties, log_properties, + node_properties, security_properties, spooling_manager_properties, + writer::to_java_properties_string, + }, +}; +use crate::crd::{ + ACCESS_CONTROL_PROPERTIES, CONFIG_PROPERTIES, EXCHANGE_MANAGER_PROPERTIES, JVM_CONFIG, + JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, NODE_PROPERTIES, SPOOLING_MANAGER_PROPERTIES, + TrinoRole, v1alpha1, +}; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("failed to build config.properties"))] + BuildConfigProperties { source: config_properties::Error }, + + #[snafu(display("failed to write {file} properties"))] + WriteProperties { + source: super::properties::writer::Error, + file: String, + }, + + #[snafu(display("missing rolegroup {role_group} under role {role}"))] + MissingRoleGroup { role: String, role_group: String }, + + #[snafu(display("failed to assemble ConfigMap for {rolegroup}"))] + Assemble { + source: stackable_operator::builder::configmap::Error, + rolegroup: RoleGroupRef, + }, + + #[snafu(display("metadata build failure"))] + Metadata { + source: stackable_operator::builder::meta::Error, + }, +} + +type Result = std::result::Result; + +// Until callers exist (wired in by reconcile in Task 14), this builder is +// transient dead code. Allow the warning to keep `cargo check` clean. +#[allow(dead_code, clippy::too_many_arguments)] +pub fn build_rolegroup_config_map( + cluster: &ValidatedCluster, + role: TrinoRole, + rolegroup_ref: &RoleGroupRef, + cluster_info: &KubernetesClusterInfo, + recommended_labels: ObjectLabels<'_, v1alpha1::TrinoCluster>, + jvm_config: String, + vector_toml: Option, + owner_target: &v1alpha1::TrinoCluster, +) -> Result { + let role_group_configs = + cluster + .role_group_configs + .get(&role) + .with_context(|| MissingRoleGroupSnafu { + role: role.to_string(), + role_group: rolegroup_ref.role_group.clone(), + })?; + let rg = role_group_configs + .get(&rolegroup_ref.role_group) + .with_context(|| MissingRoleGroupSnafu { + role: role.to_string(), + role_group: rolegroup_ref.role_group.clone(), + })?; + + let mut data: BTreeMap = BTreeMap::new(); + + // Auth files (e.g. password-authenticator file contents) — inserted FIRST + // to match the legacy ordering in controller.rs:621. + data.extend(cluster.trino_authentication_config.config_files(&role)); + + // 1. config.properties (fallible). + let cfg = config_properties::build(cluster, role.clone(), rg, cluster_info) + .context(BuildConfigPropertiesSnafu)?; + data.insert( + CONFIG_PROPERTIES.to_string(), + to_java_properties_string(&cfg).with_context(|_| WritePropertiesSnafu { + file: CONFIG_PROPERTIES.to_string(), + })?, + ); + + // 2. node.properties. + let node = node_properties::build(cluster, rg); + data.insert( + NODE_PROPERTIES.to_string(), + to_java_properties_string(&node).with_context(|_| WritePropertiesSnafu { + file: NODE_PROPERTIES.to_string(), + })?, + ); + + // 3. log.properties (optional — empty map → omit). + let log = log_properties::build(cluster, role.clone(), rg); + if !log.is_empty() { + data.insert( + LOG_PROPERTIES.to_string(), + to_java_properties_string(&log).with_context(|_| WritePropertiesSnafu { + file: LOG_PROPERTIES.to_string(), + })?, + ); + } + + // 4. security.properties. + let sec = security_properties::build(rg); + data.insert( + JVM_SECURITY_PROPERTIES.to_string(), + to_java_properties_string(&sec).with_context(|_| WritePropertiesSnafu { + file: JVM_SECURITY_PROPERTIES.to_string(), + })?, + ); + + // 5. access-control.properties (optional). + let ac = access_control_properties::build(cluster, rg); + if !ac.is_empty() { + data.insert( + ACCESS_CONTROL_PROPERTIES.to_string(), + to_java_properties_string(&ac).with_context(|_| WritePropertiesSnafu { + file: ACCESS_CONTROL_PROPERTIES.to_string(), + })?, + ); + } + + // 6. exchange-manager.properties (optional). + let em = exchange_manager_properties::build(cluster, rg); + if !em.is_empty() { + data.insert( + EXCHANGE_MANAGER_PROPERTIES.to_string(), + to_java_properties_string(&em).with_context(|_| WritePropertiesSnafu { + file: EXCHANGE_MANAGER_PROPERTIES.to_string(), + })?, + ); + } + + // 7. spooling-manager.properties (optional). + let sm = spooling_manager_properties::build(cluster, rg); + if !sm.is_empty() { + data.insert( + SPOOLING_MANAGER_PROPERTIES.to_string(), + to_java_properties_string(&sm).with_context(|_| WritePropertiesSnafu { + file: SPOOLING_MANAGER_PROPERTIES.to_string(), + })?, + ); + } + + // 8. jvm.config — passed in (still rendered by src/config/jvm.rs). + data.insert(JVM_CONFIG.to_string(), jvm_config); + + // 9. Vector sidecar toml if enabled. + if let Some(vector_toml) = vector_toml { + data.insert( + product_logging::framework::VECTOR_CONFIG_FILE.to_string(), + vector_toml, + ); + } + + ConfigMapBuilder::new() + .metadata( + ObjectMetaBuilder::new() + .name_and_namespace(owner_target) + .name(rolegroup_ref.object_name()) + .ownerreference_from_resource(owner_target, None, Some(true)) + .context(MetadataSnafu)? + .with_recommended_labels(&recommended_labels) + .context(MetadataSnafu)? + .build(), + ) + .data(data) + .build() + .with_context(|_| AssembleSnafu { + rolegroup: rolegroup_ref.clone(), + }) +} diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 93539e2f..3741cde0 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -2,4 +2,5 @@ //! //! Each submodule owns one output: properties files, ConfigMaps, StatefulSets, etc. +pub mod config_map; pub mod properties; From 762443c55f5be33c0fb348a6c3a5869bf7040341 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 21:02:36 +0200 Subject: [PATCH 14/27] test(build): cover default renders for every per-file builder Adds a shared validated_cluster_from_yaml fixture and one default_renders test per builder, pinning the migrated properties.yaml defaults (networkaddress.cache.ttl, networkaddress.cache.negative.ttl, io.trino, query.max-memory, node-scheduler.include-coordinator). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../properties/access_control_properties.rs | 14 +++++- .../build/properties/config_properties.rs | 24 ++++++++- .../properties/exchange_manager_properties.rs | 14 +++++- .../build/properties/log_properties.rs | 13 ++++- .../src/controller/build/properties/mod.rs | 49 +++++++++++++++++++ .../build/properties/node_properties.rs | 16 +++++- .../build/properties/security_properties.rs | 15 +++++- .../properties/spooling_manager_properties.rs | 14 +++++- 8 files changed, 152 insertions(+), 7 deletions(-) diff --git a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs index 214ee025..8bc5ee56 100644 --- a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs @@ -41,5 +41,17 @@ pub fn build( #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + use crate::crd::TrinoRole; + + #[test] + fn default_renders_empty_when_no_opa() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); + let props = build(&cluster, &rg); + assert!(props.is_empty()); + } } diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs index 9fd39c94..c861ddb2 100644 --- a/rust/operator-binary/src/controller/build/properties/config_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -227,5 +227,27 @@ pub fn build( #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + + #[test] + fn default_renders_includes_coordinator_default_and_query_max_memory_default() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); + let cluster_info = stackable_operator::utils::cluster_info::KubernetesClusterInfo { + cluster_domain: stackable_operator::commons::networking::DomainName::try_from( + "cluster.local", + ) + .unwrap(), + }; + let props = build(&cluster, TrinoRole::Coordinator, &rg, &cluster_info).unwrap(); + assert_eq!(props.get("coordinator").map(String::as_str), Some("true")); + assert_eq!( + props.get("node-scheduler.include-coordinator").map(String::as_str), + Some("false"), + ); + assert_eq!(props.get("query.max-memory").map(String::as_str), Some("50GB")); + } } diff --git a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs index fc57115d..54848cfa 100644 --- a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs @@ -35,5 +35,17 @@ pub fn build( #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + use crate::crd::TrinoRole; + + #[test] + fn default_renders_empty_when_no_fte() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); + let props = build(&cluster, &rg); + assert!(props.is_empty()); + } } diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs index 3d88955f..70a4d695 100644 --- a/rust/operator-binary/src/controller/build/properties/log_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -42,5 +42,16 @@ pub fn build( #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + + #[test] + fn default_renders_io_trino_info() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); + let props = build(&cluster, TrinoRole::Coordinator, &rg); + assert_eq!(props.get("io.trino").map(String::as_str), Some("INFO")); + } } diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 1046983c..77e10ff8 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -12,3 +12,52 @@ pub mod log_properties; pub mod node_properties; pub mod security_properties; pub mod writer; + +#[cfg(test)] +pub(crate) mod test_support { + use crate::controller::{ValidatedCluster, dereference::DereferencedObjects}; + use crate::crd::v1alpha1; + use stackable_operator::cli::OperatorEnvironmentOptions; + + pub fn validated_cluster_from_yaml(yaml: &str) -> ValidatedCluster { + let trino: v1alpha1::TrinoCluster = + serde_yaml::from_str(yaml).expect("invalid test YAML"); + let derefs = DereferencedObjects { + resolved_authentication_classes: Vec::new(), + catalog_definitions: Vec::new(), + catalogs: Vec::new(), + trino_opa_config: None, + resolved_fte_config: None, + resolved_client_protocol_config: None, + }; + let operator_env = OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_string(), + operator_service_name: "trino-operator".to_string(), + image_repository: "oci.example.org".to_string(), + }; + crate::controller::validate::validate_v2(&trino, &derefs, &operator_env) + .expect("validate_v2 should succeed for the minimal fixture") + } + + pub const MINIMAL_TRINO_YAML: &str = r#" + apiVersion: trino.stackable.tech/v1alpha1 + kind: TrinoCluster + metadata: + name: simple-trino + namespace: default + uid: "42" + spec: + image: + productVersion: "479" + clusterConfig: + catalogLabelSelector: {} + coordinators: + roleGroups: + default: + replicas: 1 + workers: + roleGroups: + default: + replicas: 1 + "#; +} diff --git a/rust/operator-binary/src/controller/build/properties/node_properties.rs b/rust/operator-binary/src/controller/build/properties/node_properties.rs index c9a5b7c9..707b7817 100644 --- a/rust/operator-binary/src/controller/build/properties/node_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/node_properties.rs @@ -36,5 +36,19 @@ pub fn build( #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + + #[test] + fn default_renders_node_environment_from_cluster_name() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&crate::crd::TrinoRole::Coordinator]["default"].clone(); + let props = build(&cluster, &rg); + assert_eq!( + props.get("node.environment").map(String::as_str), + Some("simple_trino"), + ); + } } diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index 4a8e0c69..f48a676e 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -41,5 +41,18 @@ pub fn build(rg: &TrinoRoleGroupConfig) -> BTreeMap { #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + use crate::crd::TrinoRole; + + #[test] + fn default_renders_networkaddress_cache_settings() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); + let props = build(&rg); + assert_eq!(props.get("networkaddress.cache.ttl").map(String::as_str), Some("30")); + assert_eq!(props.get("networkaddress.cache.negative.ttl").map(String::as_str), Some("0")); + } } diff --git a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs index 34e188c2..e854cf42 100644 --- a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs @@ -35,5 +35,17 @@ pub fn build( #[cfg(test)] mod tests { - // Tests added in Task 13 once the shared ValidatedCluster fixture exists. + use super::*; + use crate::controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }; + use crate::crd::TrinoRole; + + #[test] + fn default_renders_empty_when_no_spooling() { + let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); + let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); + let props = build(&cluster, &rg); + assert!(props.is_empty()); + } } From ef1b9431722101ae360207c7de8d33d49009a85d Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 21:16:08 +0200 Subject: [PATCH 15/27] refactor(controller): switch reconcile to ValidatedCluster pipeline reconcile_trino now consumes ValidatedCluster from validate (no longer called validate_v2) and builds the rolegroup ConfigMap via controller::build::config_map::build_rolegroup_config_map. The legacy ValidatedInputs struct, the original validate function, and the validated_product_config helper are deleted. Ctx no longer carries a ProductConfigManager, and main.rs destructures product_config: _ from RunArguments instead of loading deploy/config-spec/properties.yaml. The legacy build_rolegroup_config_map in controller.rs and its product-config-driven helpers remain in place (marked dead_code) until Task 15's cleanup. build_rolegroup_statefulset now takes env_overrides: &HashMap directly from the new pipeline instead of pulling it out of a PropertyNameKind-keyed map. Behavioral parity is verified by the existing kuttl ConfigMap snapshot at tests/templates/kuttl/smoke/14-assert.yaml.j2; no kuttl cluster was available locally, so verification relies on CI. The four legacy controller tests (test_config_overrides, test_client_protocol_config_overrides, test_access_control_overrides, test_env_overrides) were deleted because their helper depended on the legacy ProductConfigManager API. Task 17 will rewrite them against the new pipeline. Cargo test on stackable-trino-operator now reports 74 passing tests (down from 78). The #[allow(dead_code)] annotations on the now-live builders have been removed: - controller/build/config_map.rs::build_rolegroup_config_map - controller/build/properties/{access_control,config,exchange_manager, log,node,security,spooling_manager}_properties::build - controller/build/properties/writer.rs::{to_java_properties_string,Error} - product_logging.rs::get_log_property_map - operations/graceful_shutdown.rs::graceful_shutdown_config_properties_v2 Co-Authored-By: Claude Opus 4.7 (1M context) --- rust/operator-binary/src/controller.rs | 512 +++--------------- .../src/controller/build/config_map.rs | 4 +- .../properties/access_control_properties.rs | 3 - .../build/properties/config_properties.rs | 3 - .../properties/exchange_manager_properties.rs | 3 - .../build/properties/log_properties.rs | 3 - .../src/controller/build/properties/mod.rs | 4 +- .../build/properties/node_properties.rs | 3 - .../build/properties/security_properties.rs | 3 - .../properties/spooling_manager_properties.rs | 3 - .../src/controller/build/properties/writer.rs | 4 - .../src/controller/validate.rs | 151 +----- rust/operator-binary/src/main.rs | 8 +- .../src/operations/graceful_shutdown.rs | 3 - rust/operator-binary/src/product_logging.rs | 3 - 15 files changed, 89 insertions(+), 621 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index b7620b81..d99c65f8 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -9,7 +9,7 @@ use std::{ use const_format::concatcp; use product_config::{ - self, ProductConfigManager, + self, types::PropertyNameKind, writer::{PropertiesWriterError, to_java_properties_string}, }; @@ -106,11 +106,15 @@ use crate::{ /// Validated, fully-typed view of a `TrinoCluster` after dereferencing and merging. /// -/// Produced by `controller::validate::validate_v2`. Consumed by `controller::build::*`. +/// Produced by `controller::validate::validate`. Consumed by `controller::build::*`. /// Carries everything downstream needs — no `&v1alpha1::TrinoCluster` or /// `&DereferencedObjects` survives past validate. #[derive(Clone, Debug)] -#[allow(dead_code)] // wired up in Task 14 (switch reconcile to ValidatedCluster pipeline) +// Some fields (metadata, namespace, uid, catalog_label_selector, +// cluster_operation, object_overrides) are accumulated for future builders +// that don't yet consume them. They're kept on the struct because the +// validate step assembles them once from the input cluster. +#[allow(dead_code)] pub struct ValidatedCluster { pub metadata: stackable_operator::k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta, pub name: String, @@ -155,7 +159,6 @@ pub type TrinoRoleGroupConfig = crate::framework::role_utils::RoleGroupConfig< pub struct Ctx { pub client: stackable_operator::client::Client, - pub product_config: ProductConfigManager, pub operator_environment: OperatorEnvironmentOptions, } @@ -202,6 +205,12 @@ pub enum Error { rolegroup: RoleGroupRef, }, + #[snafu(display("failed to build rolegroup ConfigMap for {}", rolegroup))] + BuildRoleGroupConfigMap { + source: build::config_map::Error, + rolegroup: RoleGroupRef, + }, + #[snafu(display("failed to apply ConfigMap for {}", rolegroup))] ApplyRoleGroupConfig { source: stackable_operator::cluster_resources::Error, @@ -394,13 +403,8 @@ pub async fn reconcile_trino( .context(DereferenceSnafu)?; // validate (no client required) - let validated = validate::validate( - trino, - &dereferenced_objects, - &ctx.operator_environment, - &ctx.product_config, - ) - .context(ValidateClusterSnafu)?; + let validated = validate::validate(trino, &dereferenced_objects, &ctx.operator_environment) + .context(ValidateClusterSnafu)?; let mut cluster_resources = ClusterResources::new( APP_NAME, @@ -455,19 +459,10 @@ pub async fn reconcile_trino( let mut sts_cond_builder = StatefulSetConditionBuilder::default(); - for (trino_role_str, role_config) in validated.validated_role_config { - let trino_role = TrinoRole::from_str(&trino_role_str).context(FailedToParseRoleSnafu)?; - let role = trino.role(&trino_role).context(ReadRoleSnafu)?; - for (role_group, config) in role_config { - let role_group_ref = trino_role.rolegroup_ref(trino, &role_group); - - let merged_config = trino - .merged_config( - &trino_role, - &role_group_ref, - &dereferenced_objects.catalog_definitions, - ) - .context(FailedToResolveConfigSnafu)?; + for (trino_role, role_group_configs) in &validated.role_group_configs { + for (role_group_name, rg) in role_group_configs { + let role_group_ref = trino_role.rolegroup_ref(trino, role_group_name); + let merged_config = &rg.config; let role_group_service_recommended_labels = build_recommended_labels( trino, @@ -495,44 +490,63 @@ pub async fn reconcile_trino( let rg_metrics_service = build_rolegroup_metrics_service( trino, &role_group_ref, - role_group_service_recommended_labels, + role_group_service_recommended_labels.clone(), role_group_service_selector.into(), ) .context(ServiceConfigurationSnafu)?; - let rg_configmap = build_rolegroup_config_map( - trino, - &validated.image, + // The legacy statefulset builder still needs the `TrinoRoleType`-level + // role (it computes pod-overrides and similar fields from it). + let role = trino.role(trino_role).context(ReadRoleSnafu)?; + + // Per-file pieces still rendered outside the new builder: + // jvm.config (config::jvm::jvm_config) and the vector sidecar toml. + let jvm_config = config::jvm::jvm_config( + validated.product_version, + merged_config, &role, - &trino_role, + role_group_name, + ) + .context(FailedToCreateJvmConfigSnafu)?; + + let vector_toml = get_vector_toml(&role_group_ref, &merged_config.logging) + .context(InvalidLoggingConfigSnafu { + cm_name: role_group_ref.object_name(), + })?; + + let rg_configmap = build::config_map::build_rolegroup_config_map( + &validated, + trino_role.clone(), &role_group_ref, - &config, - &merged_config, - &validated.trino_authentication_config, - &dereferenced_objects.trino_opa_config, &client.kubernetes_cluster_info, - &dereferenced_objects.resolved_fte_config, - &dereferenced_objects.resolved_client_protocol_config, - )?; + role_group_service_recommended_labels, + jvm_config, + vector_toml, + trino, + ) + .with_context(|_| BuildRoleGroupConfigMapSnafu { + rolegroup: role_group_ref.clone(), + })?; + let rg_catalog_configmap = build_rolegroup_catalog_config_map( trino, &validated.image, &role_group_ref, - &dereferenced_objects.catalogs, + &validated.catalogs, )?; let rg_stateful_set = build_rolegroup_statefulset( trino, - &trino_role, + trino_role, &validated.image, &role_group_ref, - &config, - &merged_config, + &rg.env_overrides, + merged_config, &validated.trino_authentication_config, - &dereferenced_objects.catalogs, + &validated.catalogs, &rbac_sa.name_any(), - &dereferenced_objects.resolved_fte_config, - &dereferenced_objects.resolved_client_protocol_config, - &dereferenced_objects.trino_opa_config, + &validated.resolved_fte_config, + &validated.resolved_client_protocol_config, + &validated.trino_opa_config, )?; cluster_resources @@ -577,13 +591,13 @@ pub async fn reconcile_trino( } if let Some(listener_class) = trino_role.listener_class_name(trino) { - if let Some(listener_group_name) = group_listener_name(trino, &trino_role) { + if let Some(listener_group_name) = group_listener_name(trino, trino_role) { let role_group_listener = build_group_listener( trino, build_recommended_labels( trino, &validated.image.app_version_label_value, - &trino_role_str, + &trino_role.to_string(), "none", ), listener_class.to_string(), @@ -598,17 +612,20 @@ pub async fn reconcile_trino( } } - let role_config = trino.generic_role_config(&trino_role); + let role_config = trino.generic_role_config(trino_role); if let Some(GenericRoleConfig { pod_disruption_budget: pdb, }) = role_config { - add_pdbs(pdb, trino, &trino_role, client, &mut cluster_resources) + add_pdbs(pdb, trino, trino_role, client, &mut cluster_resources) .await .context(FailedToCreatePdbSnafu)?; } } + // dereferenced_objects is no longer used past this point - drop explicitly for clarity. + drop(dereferenced_objects); + let cluster_operation_cond_builder = ClusterOperationsConditionBuilder::new(&trino.spec.cluster_operation); @@ -632,7 +649,9 @@ pub async fn reconcile_trino( } /// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator -#[allow(clippy::too_many_arguments)] +// Deleted in Task 15. Until then, allow `dead_code` because the new pipeline +// (build/config_map.rs) is what reconcile actually calls. +#[allow(dead_code, clippy::too_many_arguments)] fn build_rolegroup_config_map( trino: &v1alpha1::TrinoCluster, resolved_product_image: &ResolvedProductImage, @@ -936,7 +955,7 @@ fn build_rolegroup_statefulset( trino_role: &TrinoRole, resolved_product_image: &ResolvedProductImage, role_group_ref: &RoleGroupRef, - config: &HashMap>, + env_overrides: &HashMap, merged_config: &v1alpha1::TrinoConfig, trino_authentication_config: &TrinoAuthenticationConfig, catalogs: &[CatalogConfig], @@ -1020,17 +1039,11 @@ fn build_rolegroup_statefulset( }); // Finally add the user defined envOverrides properties. - env.extend( - config - .get(&PropertyNameKind::Env) - .into_iter() - .flatten() - .map(|(k, v)| EnvVar { - name: k.clone(), - value: Some(v.clone()), - ..EnvVar::default() - }), - ); + env.extend(env_overrides.iter().map(|(k, v)| EnvVar { + name: k.clone(), + value: Some(v.clone()), + ..EnvVar::default() + })); let requested_secret_lifetime = merged_config .requested_secret_lifetime @@ -1675,377 +1688,8 @@ fn tls_volume_mounts( Ok(()) } -#[cfg(test)] -mod tests { - use stackable_operator::{ - commons::networking::DomainName, - kube::runtime::reflector::ObjectRef, - product_config_utils::{ - transform_all_roles_to_config, validate_all_roles_and_groups_config, - }, - }; - - use super::*; - use crate::{ - authentication::TrinoAuthenticationTypes, - config::{ - client_protocol::ResolvedClientProtocolConfig, - fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig, - }, - crd::v1alpha1::TrinoCluster, - }; - - #[tokio::test] - async fn test_config_overrides() { - let trino_yaml = r#" - apiVersion: trino.stackable.tech/v1alpha1 - kind: TrinoCluster - metadata: - name: simple-trino - spec: - image: - productVersion: "479" - clusterConfig: - catalogLabelSelector: - matchLabels: - trino: simple-trino - coordinators: - configOverrides: - config.properties: - foo: bar - level: role - hello-from-role: "true" - internal-communication.https.keystore.path: /my/custom/internal-truststore.p12 - roleGroups: - default: - configOverrides: - config.properties: - foo: bar - level: role-group - hello-from-role-group: "true" - http-server.https.truststore.path: /my/custom/truststore.p12 - replicas: 1 - workers: - roleGroups: - default: - replicas: 1 - "#; - let cm = build_config_map(trino_yaml).await.data.unwrap(); - let config = cm.get("config.properties").unwrap(); - assert!(config.contains("foo=bar")); - assert!(config.contains("level=role-group")); - assert!(config.contains("hello-from-role=true")); - assert!(config.contains("hello-from-role-group=true")); - assert!(config.contains("http-server.https.enabled=true")); - assert!( - config.contains("http-server.https.keystore.path=/stackable/server_tls/keystore.p12") - ); - assert!(config.contains( - "internal-communication.https.keystore.path=/my/custom/internal-truststore.p12" - )); - // Overwritten by configOverrides from role (does work) - assert!(config.contains("http-server.https.truststore.path=/my/custom/truststore.p12")); - - assert!(cm.contains_key("jvm.config")); - assert!(cm.contains_key("security.properties")); - assert!(cm.contains_key("node.properties")); - assert!(cm.contains_key("log.properties")); - assert!(cm.contains_key("access-control.properties")); - } - - #[tokio::test] - async fn test_client_protocol_config_overrides() { - let trino_yaml = r#" - apiVersion: trino.stackable.tech/v1alpha1 - kind: TrinoCluster - metadata: - name: simple-trino - spec: - image: - productVersion: "479" - clusterConfig: - catalogLabelSelector: - matchLabels: - trino: simple-trino - clientProtocol: - spooling: - location: s3://my-bucket/spooling - filesystem: - s3: - connection: - reference: test-s3-connection - coordinators: - configOverrides: - config.properties: - foo: bar - spooling-manager.properties: - fs.location: s3a://role-level - roleGroups: - default: - replicas: 1 - configOverrides: - spooling-manager.properties: - fs.location: s3a://role-group-level - workers: - roleGroups: - default: - replicas: 1 - "#; - - let cm = build_config_map(trino_yaml).await.data.unwrap(); - let config = cm.get("config.properties").unwrap(); - assert!(config.contains("protocol.spooling.enabled=true")); - assert!(config.contains(&format!( - "protocol.spooling.shared-secret-key=${{ENV\\:{ENV_SPOOLING_SECRET}}}" - ))); - assert!(config.contains("foo=bar")); - - let config = cm.get("spooling-manager.properties").unwrap(); - assert!(config.contains("fs.location=s3a\\://role-group-level")); - assert!(config.contains("spooling-manager.name=filesystem")); - } +// Tests for config overrides, client protocol overrides, access control +// overrides, and env overrides will be rewritten against the new pipeline +// in Task 17. They were deleted in this commit because their test helper +// (build_config_map) depended on the legacy ProductConfigManager API. - async fn build_config_map(trino_yaml: &str) -> ConfigMap { - let deserializer = serde_yaml::Deserializer::from_str(trino_yaml); - let mut trino: TrinoCluster = - serde_yaml::with::singleton_map_recursive::deserialize(deserializer) - .expect("invalid test input"); - trino.metadata.namespace = Some("default".to_owned()); - trino.metadata.uid = Some("42".to_owned()); - let cluster_info = KubernetesClusterInfo { - cluster_domain: DomainName::try_from("cluster.local").unwrap(), - }; - let resolved_product_image = trino - .spec - .image - .resolve(CONTAINER_IMAGE_BASE_NAME, "oci.example.org", "0.0.0-dev") - .expect("test resolved product image is always valid"); - - let config_files = vec![ - PropertyNameKind::File(CONFIG_PROPERTIES.to_string()), - PropertyNameKind::File(NODE_PROPERTIES.to_string()), - PropertyNameKind::File(JVM_CONFIG.to_string()), - PropertyNameKind::File(LOG_PROPERTIES.to_string()), - PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()), - PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()), - PropertyNameKind::File(SPOOLING_MANAGER_PROPERTIES.to_string()), - PropertyNameKind::File(EXCHANGE_MANAGER_PROPERTIES.to_string()), - ]; - let validated_config = validate_all_roles_and_groups_config( - // The Trino version is a single number like 396. - // The product config expects semver formatted version strings. - // That is why we just add minor and patch version 0 here. - &format!("{}.0.0", resolved_product_image.product_version), - &transform_all_roles_to_config( - &trino, - &HashMap::from([ - ( - TrinoRole::Coordinator.to_string(), - ( - config_files.clone(), - trino.role(&TrinoRole::Coordinator).unwrap(), - ), - ), - ( - TrinoRole::Worker.to_string(), - (config_files, trino.role(&TrinoRole::Worker).unwrap()), - ), - ]), - ) - .unwrap(), - // Using this instead of ProductConfigManager::from_yaml_file, as that did not find the file - &ProductConfigManager::from_str(include_str!( - "../../../deploy/config-spec/properties.yaml" - )) - .unwrap(), - false, - false, - ) - .unwrap(); - - let trino_role = TrinoRole::Coordinator; - let role = trino.role(&trino_role).unwrap(); - let rolegroup_ref = RoleGroupRef { - cluster: ObjectRef::from_obj(&trino), - role: trino_role.to_string(), - role_group: "default".to_string(), - }; - let trino_authentication_config = TrinoAuthenticationConfig::new( - &resolved_product_image, - TrinoAuthenticationTypes::try_from(Vec::new()).unwrap(), - ) - .unwrap(); - let trino_opa_config = Some(TrinoOpaConfig { - non_batched_connection_string: - "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/allow" - .to_string(), - batched_connection_string: - "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batch" - .to_string(), - row_filters_connection_string: Some( - "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/rowFilters" - .to_string(), - ), - batched_column_masking_connection_string: Some( - "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batchColumnMasks" - .to_string(), - ), - allow_permission_management_operations: true, - tls_secret_class: None, - }); - let resolved_fte_config = match &trino.spec.cluster_config.fault_tolerant_execution { - Some(fault_tolerant_execution) => Some( - ResolvedFaultTolerantExecutionConfig::from_config( - fault_tolerant_execution, - None, - &trino.namespace().unwrap(), - ) - .await - .unwrap(), - ), - None => None, - }; - let resolved_spooling_config = match &trino.spec.cluster_config.client_protocol { - Some(client_protocol) => Some( - ResolvedClientProtocolConfig::from_config( - client_protocol, - None, - &trino.namespace().unwrap(), - ) - .await - .unwrap(), - ), - None => None, - }; - let merged_config = trino - .merged_config(&trino_role, &rolegroup_ref, &[]) - .unwrap(); - - build_rolegroup_config_map( - &trino, - &resolved_product_image, - &role, - &trino_role, - &rolegroup_ref, - validated_config - .get("coordinator") - .unwrap() - .get("default") - .unwrap(), - &merged_config, - &trino_authentication_config, - &trino_opa_config, - &cluster_info, - &resolved_fte_config, - &resolved_spooling_config, - ) - .unwrap() - } - - #[tokio::test] - async fn test_access_control_overrides() { - let trino_yaml = r#" - apiVersion: trino.stackable.tech/v1alpha1 - kind: TrinoCluster - metadata: - name: trino - spec: - image: - productVersion: "479" - clusterConfig: - catalogLabelSelector: - matchLabels: - trino: simple-trino - authorization: - opa: - configMapName: simple-opa - package: my-product - coordinators: - configOverrides: - access-control.properties: - hello-from-role: "true" # only defined here at role level - foo.bar: "false" # overridden by role group below - opa.allow-permission-management-operations: "false" # override value from config - roleGroups: - default: - configOverrides: - access-control.properties: - hello-from-role-group: "true" # only defined here at group level - foo.bar: "true" # overrides role value - opa.policy.batched-uri: "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batch-new" # override value from config - opa.policy.batch-column-masking-uri: "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batchColumnMasks-new" # override value from config - replicas: 1 - workers: - roleGroups: - default: - replicas: 1 - "#; - - let cm = build_config_map(trino_yaml).await.data.unwrap(); - let access_control_config = cm.get("access-control.properties").unwrap(); - - assert!(access_control_config.contains("access-control.name=opa")); - assert!(access_control_config.contains("hello-from-role=true")); - assert!(access_control_config.contains("hello-from-role-group=true")); - assert!(access_control_config.contains("foo.bar=true")); - assert!(access_control_config.contains("opa.allow-permission-management-operations=false")); - assert!(access_control_config.contains(r#"opa.policy.batched-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batch-new"#)); - assert!(access_control_config.contains(r#"opa.policy.batch-column-masking-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batchColumnMasks-new"#)); - assert!(access_control_config.contains(r#"opa.policy.row-filters-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/rowFilters"#)); - assert!(access_control_config.contains(r#"opa.policy.uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/allow"#)); - } - - #[test] - fn test_env_overrides() { - let trino_yaml = r#" - apiVersion: trino.stackable.tech/v1alpha1 - kind: TrinoCluster - metadata: - name: trino - spec: - image: - productVersion: "479" - clusterConfig: - catalogLabelSelector: - matchLabels: - trino: simple-trino - coordinators: - envOverrides: - COMMON_VAR: role-value # overridden by role group below - ROLE_VAR: role-value # only defined here at role level - roleGroups: - default: - envOverrides: - COMMON_VAR: group-value # overrides role value - GROUP_VAR: group-value # only defined here at group level - replicas: 1 - workers: - roleGroups: - default: - replicas: 1 - "#; - let deserializer = serde_yaml::Deserializer::from_str(trino_yaml); - let trino: v1alpha1::TrinoCluster = - serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); - - let validated_config = validate::validated_product_config( - &trino, - "455.0.0", - &ProductConfigManager::from_yaml_file("../../deploy/config-spec/properties.yaml") - .unwrap(), - ) - .unwrap(); - - let env = validated_config - .get(&TrinoRole::Coordinator.to_string()) - .unwrap() - .get("default") - .unwrap() - .get(&PropertyNameKind::Env) - .unwrap(); - - assert_eq!(&"group-value".to_string(), env.get("COMMON_VAR").unwrap()); - assert_eq!(&"group-value".to_string(), env.get("GROUP_VAR").unwrap()); - assert_eq!(&"role-value".to_string(), env.get("ROLE_VAR").unwrap()); - } -} diff --git a/rust/operator-binary/src/controller/build/config_map.rs b/rust/operator-binary/src/controller/build/config_map.rs index d1a0eaa0..28973078 100644 --- a/rust/operator-binary/src/controller/build/config_map.rs +++ b/rust/operator-binary/src/controller/build/config_map.rs @@ -54,9 +54,7 @@ pub enum Error { type Result = std::result::Result; -// Until callers exist (wired in by reconcile in Task 14), this builder is -// transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code, clippy::too_many_arguments)] +#[allow(clippy::too_many_arguments)] pub fn build_rolegroup_config_map( cluster: &ValidatedCluster, role: TrinoRole, diff --git a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs index 8bc5ee56..c9668288 100644 --- a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs @@ -9,9 +9,6 @@ use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; /// Returns an empty map when neither OPA authorization is configured nor /// user overrides are provided — callers should omit the file from the /// ConfigMap in that case. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig, diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs index c861ddb2..cf2c9a5a 100644 --- a/rust/operator-binary/src/controller/build/properties/config_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -42,9 +42,6 @@ pub enum Error { } /// Build the `config.properties` key/value pairs. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, role: TrinoRole, diff --git a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs index 54848cfa..840a4514 100644 --- a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs @@ -9,9 +9,6 @@ use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; /// Returns an empty map when fault-tolerant execution is not configured and no /// user overrides are provided — callers should omit the file from the /// ConfigMap in that case. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig, diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs index 70a4d695..362cb875 100644 --- a/rust/operator-binary/src/controller/build/properties/log_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -12,9 +12,6 @@ const DEFAULT_IO_TRINO: &str = "INFO"; /// /// Returns an empty map when there is nothing to write — callers /// should omit the file from the ConfigMap if the result is empty. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, role: TrinoRole, diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 77e10ff8..3a857cb9 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -35,8 +35,8 @@ pub(crate) mod test_support { operator_service_name: "trino-operator".to_string(), image_repository: "oci.example.org".to_string(), }; - crate::controller::validate::validate_v2(&trino, &derefs, &operator_env) - .expect("validate_v2 should succeed for the minimal fixture") + crate::controller::validate::validate(&trino, &derefs, &operator_env) + .expect("validate should succeed for the minimal fixture") } pub const MINIMAL_TRINO_YAML: &str = r#" diff --git a/rust/operator-binary/src/controller/build/properties/node_properties.rs b/rust/operator-binary/src/controller/build/properties/node_properties.rs index 707b7817..7ad65ed8 100644 --- a/rust/operator-binary/src/controller/build/properties/node_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/node_properties.rs @@ -11,9 +11,6 @@ const NODE_ENVIRONMENT: &str = "node.environment"; /// `node.environment` is derived from the cluster name: lowercased, with `-` /// replaced by `_`. Trino requires `^[a-z][a-z0-9_]*[a-z0-9]$`; cluster names /// constrained by Kubernetes naming already satisfy this after the transform. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig, diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index f48a676e..c4fda4ef 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -13,9 +13,6 @@ const DEFAULT_NETWORKADDRESS_CACHE_NEGATIVE_TTL: &str = "0"; /// Build the `security.properties` key/value pairs. /// /// Both keys apply to both `coordinator` and `worker` roles. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build(rg: &TrinoRoleGroupConfig) -> BTreeMap { let mut props = BTreeMap::new(); diff --git a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs index e854cf42..dc524866 100644 --- a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs @@ -9,9 +9,6 @@ use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; /// Returns an empty map when client spooling is not configured and no user /// overrides are provided — callers should omit the file from the ConfigMap /// in that case. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// builder is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn build( cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig, diff --git a/rust/operator-binary/src/controller/build/properties/writer.rs b/rust/operator-binary/src/controller/build/properties/writer.rs index 48ebb886..99e60999 100644 --- a/rust/operator-binary/src/controller/build/properties/writer.rs +++ b/rust/operator-binary/src/controller/build/properties/writer.rs @@ -8,9 +8,6 @@ use std::collections::BTreeMap; use snafu::{ResultExt, Snafu}; -// Until callers exist (wired in by build/config_map.rs in Task 12), the -// writer is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] #[derive(Debug, Snafu)] pub enum Error { #[snafu(display("failed to write properties output"))] @@ -21,7 +18,6 @@ pub enum Error { /// /// Keys and values are escaped per : /// `:`, `=`, `#`, `!`, `\\`, leading whitespace, and ` ` (space). -#[allow(dead_code)] pub fn to_java_properties_string( props: &BTreeMap, ) -> Result { diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 255d7704..071804a4 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -1,30 +1,17 @@ //! The validate step in the TrinoCluster controller //! //! Synchronously validates inputs that don't require a Kubernetes client. Produces -//! [`ValidatedInputs`], consumed by the rest of `reconcile_trino`. +//! [`super::ValidatedCluster`], consumed by the rest of `reconcile_trino`. -use std::collections::HashMap; - -use product_config::{ProductConfigManager, types::PropertyNameKind}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ - cli::OperatorEnvironmentOptions, - commons::product_image_selection::{self, ResolvedProductImage}, - product_config_utils::{ - ValidatedRoleConfigByPropertyKind, transform_all_roles_to_config, - validate_all_roles_and_groups_config, - }, + cli::OperatorEnvironmentOptions, commons::product_image_selection, }; use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ authentication::{self, TrinoAuthenticationConfig, TrinoAuthenticationTypes}, - controller::dereference::DereferencedObjects, - crd::{ - ACCESS_CONTROL_PROPERTIES, CONFIG_PROPERTIES, EXCHANGE_MANAGER_PROPERTIES, JVM_CONFIG, - JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, NODE_PROPERTIES, SPOOLING_MANAGER_PROPERTIES, - TrinoRole, v1alpha1, - }, + crd::{TrinoRole, v1alpha1}, }; #[derive(Snafu, Debug, EnumDiscriminants)] @@ -53,16 +40,6 @@ pub enum Error { role: String, }, - #[snafu(display("failed to transform configs"))] - ProductConfigTransform { - source: stackable_operator::product_config_utils::Error, - }, - - #[snafu(display("invalid product config"))] - InvalidProductConfig { - source: stackable_operator::product_config_utils::Error, - }, - #[snafu(display("unable to parse Trino version: {product_version:?}"))] ParseTrinoVersion { source: std::num::ParseIntError, @@ -80,123 +57,9 @@ pub enum Error { type Result = std::result::Result; -/// Synchronous inputs the rest of `reconcile_trino` needs after dereferencing. -pub struct ValidatedInputs { - pub image: ResolvedProductImage, - pub trino_authentication_config: TrinoAuthenticationConfig, - pub validated_role_config: ValidatedRoleConfigByPropertyKind, -} - -/// Validates the cluster spec and the dereferenced inputs. +/// Validates the cluster spec and the dereferenced inputs, producing the typed +/// [`super::ValidatedCluster`]. pub fn validate( - trino: &v1alpha1::TrinoCluster, - dereferenced_objects: &DereferencedObjects, - operator_environment: &OperatorEnvironmentOptions, - product_config: &ProductConfigManager, -) -> Result { - let resolved_product_image = trino - .spec - .image - .resolve( - super::CONTAINER_IMAGE_BASE_NAME, - &operator_environment.image_repository, - crate::built_info::PKG_VERSION, - ) - .context(ResolveProductImageSnafu)?; - - let trino_authentication_config = TrinoAuthenticationConfig::new( - &resolved_product_image, - TrinoAuthenticationTypes::try_from( - dereferenced_objects.resolved_authentication_classes.clone(), - ) - .context(UnsupportedAuthenticationConfigSnafu)?, - ) - .context(InvalidAuthenticationConfigSnafu)?; - - if dereferenced_objects - .resolved_client_protocol_config - .is_some() - && resolved_product_image.product_version.starts_with("45") - { - return Err(Error::ClientSpoolingProtocolTrinoVersion { - product_version: resolved_product_image.product_version.clone(), - }); - } - - let validated_role_config = validated_product_config( - trino, - // The Trino version is a single number like 396. - // The product config expects semver formatted version strings. - // That is why we just add minor and patch version 0 here. - &format!("{}.0.0", resolved_product_image.product_version), - product_config, - )?; - - Ok(ValidatedInputs { - image: resolved_product_image, - trino_authentication_config, - validated_role_config, - }) -} - -pub(super) fn validated_product_config( - trino: &v1alpha1::TrinoCluster, - version: &str, - product_config: &ProductConfigManager, -) -> Result { - let mut roles = HashMap::new(); - - let config_files = vec![ - PropertyNameKind::Env, - PropertyNameKind::File(CONFIG_PROPERTIES.to_string()), - PropertyNameKind::File(NODE_PROPERTIES.to_string()), - PropertyNameKind::File(JVM_CONFIG.to_string()), - PropertyNameKind::File(LOG_PROPERTIES.to_string()), - PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string()), - PropertyNameKind::File(ACCESS_CONTROL_PROPERTIES.to_string()), - PropertyNameKind::File(SPOOLING_MANAGER_PROPERTIES.to_string()), - PropertyNameKind::File(EXCHANGE_MANAGER_PROPERTIES.to_string()), - ]; - - let coordinator_role = TrinoRole::Coordinator; - roles.insert( - coordinator_role.to_string(), - ( - config_files.clone(), - trino - .role(&coordinator_role) - .with_context(|_| MissingTrinoRoleSnafu { - role: coordinator_role.to_string(), - })?, - ), - ); - - let worker_role = TrinoRole::Worker; - roles.insert( - worker_role.to_string(), - ( - config_files, - trino - .role(&worker_role) - .with_context(|_| MissingTrinoRoleSnafu { - role: worker_role.to_string(), - })?, - ), - ); - - let role_config = - transform_all_roles_to_config(trino, &roles).context(ProductConfigTransformSnafu)?; - - validate_all_roles_and_groups_config(version, &role_config, product_config, false, false) - .context(InvalidProductConfigSnafu) -} - -/// New validator: produces the typed [`super::ValidatedCluster`]. -/// -/// Replaces the legacy product-config-driven `validate` pipeline. See Task 4 of the -/// product-config removal plan. Wired up in Task 14. -#[allow(dead_code)] -pub fn validate_v2( trino: &v1alpha1::TrinoCluster, dereferenced_objects: &super::dereference::DereferencedObjects, operator_environment: &OperatorEnvironmentOptions, @@ -311,7 +174,7 @@ mod tests { use super::*; #[test] - fn validate_v2_minimal_cluster() { + fn validate_minimal_cluster() { let trino_yaml = r#" apiVersion: trino.stackable.tech/v1alpha1 kind: TrinoCluster @@ -351,7 +214,7 @@ mod tests { }; let validated = - validate_v2(&trino, &derefs, &operator_env).expect("validate_v2 should succeed"); + validate(&trino, &derefs, &operator_env).expect("validate should succeed"); assert_eq!(validated.name, "simple-trino"); assert_eq!(validated.namespace, "default"); diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index beb35047..fb00b6a5 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -80,7 +80,7 @@ async fn main() -> anyhow::Result<()> { Command::Run(RunArguments { operator_environment, watch_namespace, - product_config, + product_config: _, maintenance, common, }) => { @@ -127,11 +127,6 @@ async fn main() -> anyhow::Result<()> { .run(sigterm_watcher.handle()) .map_err(|err| anyhow!(err).context("failed to run webhook server")); - let product_config = product_config.load(&[ - "deploy/config-spec/properties.yaml", - "/etc/stackable/trino-operator/config-spec/properties.yaml", - ])?; - let event_recorder = Arc::new(Recorder::new( client.as_kube_client(), Reporter { @@ -206,7 +201,6 @@ async fn main() -> anyhow::Result<()> { Arc::new(controller::Ctx { client: client.clone(), operator_environment, - product_config, }), ) // We can let the reporting happen in the background diff --git a/rust/operator-binary/src/operations/graceful_shutdown.rs b/rust/operator-binary/src/operations/graceful_shutdown.rs index 04b38a4a..ff878380 100644 --- a/rust/operator-binary/src/operations/graceful_shutdown.rs +++ b/rust/operator-binary/src/operations/graceful_shutdown.rs @@ -61,9 +61,6 @@ pub fn graceful_shutdown_config_properties( /// Returns a flat `BTreeMap` (unlike the legacy variant which /// returns `BTreeMap>`) — the new builders no longer /// thread `Option` values through. -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// helper is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn graceful_shutdown_config_properties_v2( cluster: &ValidatedCluster, role: TrinoRole, diff --git a/rust/operator-binary/src/product_logging.rs b/rust/operator-binary/src/product_logging.rs index 38900037..8fceac5d 100644 --- a/rust/operator-binary/src/product_logging.rs +++ b/rust/operator-binary/src/product_logging.rs @@ -72,9 +72,6 @@ pub fn get_log_properties(logging: &Logging) -> Option { /// /// New per-file builders use this; the old `get_log_properties` will be removed /// once the legacy `build_rolegroup_config_map` is deleted (see Task 14). -// Until callers exist (wired in by build/config_map.rs in Task 12), this -// helper is transient dead code. Allow the warning to keep `cargo check` clean. -#[allow(dead_code)] pub fn get_log_property_map( logging: &Logging, ) -> Option> { From 183544c9daaede9191ecb2b97c361b6cfba33f88 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 21:28:35 +0200 Subject: [PATCH 16/27] fix(build): drop io.trino=INFO default from log_properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy build_rolegroup_config_map ignored the product-config-validated map for LOG_PROPERTIES and emitted only get_log_properties output. The io.trino=INFO default in properties.yaml was dead — never reached the wire. The new builder must do the same to preserve byte-level parity with the kuttl smoke snapshot. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../build/properties/log_properties.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs index 362cb875..a5d80021 100644 --- a/rust/operator-binary/src/controller/build/properties/log_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -5,13 +5,10 @@ use std::collections::BTreeMap; use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; use crate::crd::TrinoRole; -const IO_TRINO: &str = "io.trino"; -const DEFAULT_IO_TRINO: &str = "INFO"; - /// Build the `log.properties` key/value pairs for `(role, rg)`. /// -/// Returns an empty map when there is nothing to write — callers -/// should omit the file from the ConfigMap if the result is empty. +/// Returns `None`-equivalent (empty map) when there is nothing to write — +/// callers should omit the file from the ConfigMap if the result is empty. pub fn build( cluster: &ValidatedCluster, role: TrinoRole, @@ -19,21 +16,21 @@ pub fn build( ) -> BTreeMap { let mut props = BTreeMap::new(); - // 1. Defaults (lowest precedence) — migrated from deploy/config-spec/properties.yaml. - props.insert(IO_TRINO.to_string(), DEFAULT_IO_TRINO.to_string()); - + // 1. No defaults — the legacy code path ignored the product-config default + // `io.trino=INFO`; only per-container logger config and user overrides + // reach the wire. // 2. Automatic — per-container logger levels derived from rg.config.logging. if let Some(per_container) = crate::product_logging::get_log_property_map(&rg.config.logging) { props.extend(per_container); } - // 3. (no merged_config contribution for log.properties beyond logging tree) + // 3. No merged_config contribution. // 4. User overrides (highest precedence). if let Some(kv) = &rg.config_overrides.log_properties { props.extend(kv.overrides.clone()); } - let _ = (cluster, role); // suppress unused for now; may use later for role-aware logging + let _ = (cluster, role); // currently unused; preserved for symmetry with siblings props } @@ -45,10 +42,13 @@ mod tests { }; #[test] - fn default_renders_io_trino_info() { + fn default_renders_root_logger_only() { let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); let props = build(&cluster, TrinoRole::Coordinator, &rg); - assert_eq!(props.get("io.trino").map(String::as_str), Some("INFO")); + // Legacy behavior: only the per-container logging tree reaches the wire. + // The ROOT logger maps to the empty-string key. + assert_eq!(props.get("").map(String::as_str), Some("info")); + assert!(!props.contains_key("io.trino")); } } From 5211b820a8549422d7c9641924484f9787c4518e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 21:35:00 +0200 Subject: [PATCH 17/27] fix(framework): use BTreeMap for env_overrides to preserve sort order HashMap iteration order is non-deterministic in Rust (RandomState seeded at process start). After Task 14's signature change, env_overrides flowed through HashMap from the framework helper into build_rolegroup_statefulset, which could shuffle env-var Vec order on subsequent reconciles and trigger unnecessary Pod restarts. The legacy product-config pipeline used a BTreeMap (sorted), so this restores byte-level determinism. Co-Authored-By: Claude Opus 4.7 (1M context) --- rust/operator-binary/src/controller.rs | 2 +- rust/operator-binary/src/framework/role_utils.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index d99c65f8..4e51f30c 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -955,7 +955,7 @@ fn build_rolegroup_statefulset( trino_role: &TrinoRole, resolved_product_image: &ResolvedProductImage, role_group_ref: &RoleGroupRef, - env_overrides: &HashMap, + env_overrides: &BTreeMap, merged_config: &v1alpha1::TrinoConfig, trino_authentication_config: &TrinoAuthenticationConfig, catalogs: &[CatalogConfig], diff --git a/rust/operator-binary/src/framework/role_utils.rs b/rust/operator-binary/src/framework/role_utils.rs index 5c2e1821..1fdc2c7e 100644 --- a/rust/operator-binary/src/framework/role_utils.rs +++ b/rust/operator-binary/src/framework/role_utils.rs @@ -15,7 +15,7 @@ //! Replace with `stackable_operator::v2::role_utils::*` once upstream publishes //! the module. -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; use serde::Serialize; use stackable_operator::{ @@ -31,14 +31,14 @@ use stackable_operator::{ /// Trino-friendly view of a validated, merged `RoleGroup`. /// /// Mirrors `stackable_operator::v2::role_utils::RoleGroupConfig` on the -/// `smooth-operator` branch, with `env_overrides: HashMap` +/// `smooth-operator` branch, with `env_overrides: BTreeMap` /// instead of the upstream `EnvVarSet`. #[derive(Clone, Debug, PartialEq)] pub struct RoleGroupConfig { pub replicas: u16, pub config: Config, pub config_overrides: ConfigOverrides, - pub env_overrides: HashMap, + pub env_overrides: BTreeMap, pub cli_overrides: BTreeMap, pub pod_overrides: PodTemplateSpec, pub product_specific_common_config: CommonConfig, @@ -79,8 +79,8 @@ where role_group.config.config_overrides.clone(), ), env_overrides: merged_env_overrides( - role.config.env_overrides.clone(), - role_group.config.env_overrides.clone(), + role.config.env_overrides.iter().map(|(k, v)| (k.clone(), v.clone())).collect(), + role_group.config.env_overrides.iter().map(|(k, v)| (k.clone(), v.clone())).collect(), ), cli_overrides: merged_cli_overrides( role.config.cli_overrides.clone(), @@ -123,9 +123,9 @@ where } fn merged_env_overrides( - role_env_overrides: HashMap, - role_group_env_overrides: HashMap, -) -> HashMap { + role_env_overrides: BTreeMap, + role_group_env_overrides: BTreeMap, +) -> BTreeMap { let mut merged = role_env_overrides; merged.extend(role_group_env_overrides); merged From a690512602845ad0c43cc96405c9fc29b7eb3988 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 21:51:16 +0200 Subject: [PATCH 18/27] chore: remove product-config dependency - Drop product-config from workspace. - Delete `impl Configuration for TrinoConfigFragment` and `impl KeyValueOverridesProvider for TrinoConfigOverrides`. - Delete the legacy `build_rolegroup_config_map` and the helpers it exclusively relied on (replaced by `controller::build::config_map` in Task 14). Switch `build_rolegroup_catalog_config_map` to the new local writer. - Switch the authentication modules to the local `controller::build::properties::writer::to_java_properties_string`. - Delete legacy `get_log_properties` / legacy `graceful_shutdown_config_properties` helpers that no longer have callers, and rename `_v2` survivor back to its original name. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 1 - Cargo.nix | 4 - Cargo.toml | 1 - rust/operator-binary/Cargo.toml | 1 - .../operator-binary/src/authentication/mod.rs | 2 +- .../src/authentication/password/mod.rs | 26 +- rust/operator-binary/src/controller.rs | 309 ++---------------- .../build/properties/config_properties.rs | 2 +- rust/operator-binary/src/crd/mod.rs | 227 +------------ .../src/operations/graceful_shutdown.rs | 44 +-- rust/operator-binary/src/product_logging.rs | 41 +-- 11 files changed, 40 insertions(+), 618 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5a368c3..cfc7e2c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3006,7 +3006,6 @@ dependencies = [ "const_format", "futures 0.3.32", "indoc", - "product-config", "rstest", "serde", "serde_json", diff --git a/Cargo.nix b/Cargo.nix index 33d9ae93..883c3ebb 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -10034,10 +10034,6 @@ rec { name = "indoc"; packageId = "indoc"; } - { - name = "product-config"; - packageId = "product-config"; - } { name = "serde"; packageId = "serde"; diff --git a/Cargo.toml b/Cargo.toml index 03289dfb..5f9c7bf0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,6 @@ edition = "2021" repository = "https://github.com/stackabletech/trino-operator" [workspace.dependencies] -product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.8.0" } stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", branch = "smooth-operator", features = ["webhook"] } anyhow = "1.0" diff --git a/rust/operator-binary/Cargo.toml b/rust/operator-binary/Cargo.toml index efa9d347..d93c5f1a 100644 --- a/rust/operator-binary/Cargo.toml +++ b/rust/operator-binary/Cargo.toml @@ -10,7 +10,6 @@ publish = false build = "build.rs" [dependencies] -product-config.workspace = true stackable-operator.workspace = true anyhow.workspace = true diff --git a/rust/operator-binary/src/authentication/mod.rs b/rust/operator-binary/src/authentication/mod.rs index c3117242..b14d366d 100644 --- a/rust/operator-binary/src/authentication/mod.rs +++ b/rust/operator-binary/src/authentication/mod.rs @@ -52,7 +52,7 @@ pub enum Error { #[snafu(display("failed to format trino authentication java properties"))] FailedToWriteJavaProperties { - source: product_config::writer::PropertiesWriterError, + source: crate::controller::build::properties::writer::Error, }, #[snafu(display("failed to configure trino password authentication"))] diff --git a/rust/operator-binary/src/authentication/password/mod.rs b/rust/operator-binary/src/authentication/password/mod.rs index f5eeb09c..562dc155 100644 --- a/rust/operator-binary/src/authentication/password/mod.rs +++ b/rust/operator-binary/src/authentication/password/mod.rs @@ -36,7 +36,7 @@ pub enum Error { #[snafu(display("failed to write password authentication config file"))] WritePasswordAuthenticationFile { - source: product_config::writer::PropertiesWriterError, + source: crate::controller::build::properties::writer::Error, }, #[snafu(display("failed to create LDAP Volumes and VolumeMounts"))] @@ -90,16 +90,13 @@ impl TrinoPasswordAuthentication { .push(format!("{RW_CONFIG_DIR_NAME}/{config_file_name}")); // authenticator property file + let file_props: BTreeMap = + file_authenticator.config_file_data().into_iter().collect(); password_authentication_config.add_config_file( TrinoRole::Coordinator, config_file_name, - product_config::writer::to_java_properties_string( - file_authenticator - .config_file_data() - .into_iter() - .map(|(k, v)| (k, Some(v))) - .collect::>>() - .iter(), + crate::controller::build::properties::writer::to_java_properties_string( + &file_props, ) .context(WritePasswordAuthenticationFileSnafu)?, ); @@ -130,17 +127,14 @@ impl TrinoPasswordAuthentication { .push(format!("{RW_CONFIG_DIR_NAME}/{config_file_name}",)); // authenticator property file + let ldap_props = ldap_authenticator + .config_file_data() + .context(InvalidLdapAuthenticationConfigurationSnafu)?; password_authentication_config.add_config_file( TrinoRole::Coordinator, config_file_name, - product_config::writer::to_java_properties_string( - ldap_authenticator - .config_file_data() - .context(InvalidLdapAuthenticationConfigurationSnafu)? - .into_iter() - .map(|(k, v)| (k, Some(v))) - .collect::>>() - .iter(), + crate::controller::build::properties::writer::to_java_properties_string( + &ldap_props, ) .context(WritePasswordAuthenticationFileSnafu)?, ); diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 4e51f30c..e1c579cd 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1,18 +1,12 @@ //! Ensures that `Pod`s are configured and running for each [`v1alpha1::TrinoCluster`] use std::{ - collections::{BTreeMap, HashMap}, + collections::BTreeMap, convert::Infallible, num::ParseIntError, - str::FromStr, sync::Arc, }; use const_format::concatcp; -use product_config::{ - self, - types::PropertyNameKind, - writer::{PropertiesWriterError, to_java_properties_string}, -}; use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ builder::{ @@ -67,11 +61,10 @@ use stackable_operator::{ compute_conditions, operations::ClusterOperationsConditionBuilder, statefulset::StatefulSetConditionBuilder, }, - utils::cluster_info::KubernetesClusterInfo, }; use strum::{EnumDiscriminants, IntoStaticStr}; -mod build; +pub mod build; mod dereference; mod validate; @@ -82,25 +75,18 @@ use crate::{ command, config::{self, client_protocol, fault_tolerant_execution}, crd::{ - ACCESS_CONTROL_PROPERTIES, APP_NAME, CONFIG_DIR_NAME, CONFIG_PROPERTIES, Container, - DISCOVERY_URI, ENV_INTERNAL_SECRET, ENV_SPOOLING_SECRET, EXCHANGE_MANAGER_PROPERTIES, - HTTP_PORT, HTTP_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, JVM_CONFIG, - JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, - METRICS_PORT_NAME, NODE_PROPERTIES, RW_CONFIG_DIR_NAME, SPOOLING_MANAGER_PROPERTIES, - STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, - STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, - TrinoRole, TrinoRoleType, - discovery::{TrinoDiscovery, TrinoDiscoveryProtocol, TrinoPodRef}, - v1alpha1, + APP_NAME, CONFIG_DIR_NAME, Container, ENV_INTERNAL_SECRET, ENV_SPOOLING_SECRET, HTTP_PORT, + HTTP_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, + METRICS_PORT_NAME, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, + STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, + STACKABLE_TLS_STORE_PASSWORD, TrinoRole, v1alpha1, }, listener::{ LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc, group_listener_name, secret_volume_listener_scope, }, - operations::{ - add_graceful_shutdown_config, graceful_shutdown_config_properties, pdb::add_pdbs, - }, - product_logging::{get_log_properties, get_vector_toml}, + operations::{add_graceful_shutdown_config, pdb::add_pdbs}, + product_logging::get_vector_toml, service::{build_rolegroup_headless_service, build_rolegroup_metrics_service}, }; @@ -110,10 +96,10 @@ use crate::{ /// Carries everything downstream needs — no `&v1alpha1::TrinoCluster` or /// `&DereferencedObjects` survives past validate. #[derive(Clone, Debug)] -// Some fields (metadata, namespace, uid, catalog_label_selector, -// cluster_operation, object_overrides) are accumulated for future builders -// that don't yet consume them. They're kept on the struct because the -// validate step assembles them once from the input cluster. +// Some fields (metadata, uid, catalog_label_selector, cluster_operation, +// object_overrides) are accumulated for future builders that don't yet +// consume them. They're kept on the struct because the validate step +// assembles them once from the input cluster. #[allow(dead_code)] pub struct ValidatedCluster { pub metadata: stackable_operator::k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta, @@ -229,7 +215,9 @@ pub enum Error { }, #[snafu(display("failed to format runtime properties"))] - FailedToWriteJavaProperties { source: PropertiesWriterError }, + FailedToWriteJavaProperties { + source: build::properties::writer::Error, + }, #[snafu(display("failed to parse role: {source}"))] FailedToParseRole { source: strum::ParseError }, @@ -281,12 +269,6 @@ pub enum Error { source: stackable_operator::commons::rbac::Error, }, - #[snafu(display("failed to serialize [{JVM_SECURITY_PROPERTIES}] for {}", rolegroup))] - JvmSecurityProperties { - source: PropertiesWriterError, - rolegroup: String, - }, - #[snafu(display("failed to create PodDisruptionBudget"))] FailedToCreatePdb { source: crate::operations::pdb::Error, @@ -648,249 +630,6 @@ pub async fn reconcile_trino( Ok(Action::await_change()) } -/// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator -// Deleted in Task 15. Until then, allow `dead_code` because the new pipeline -// (build/config_map.rs) is what reconcile actually calls. -#[allow(dead_code, clippy::too_many_arguments)] -fn build_rolegroup_config_map( - trino: &v1alpha1::TrinoCluster, - resolved_product_image: &ResolvedProductImage, - role: &TrinoRoleType, - trino_role: &TrinoRole, - rolegroup_ref: &RoleGroupRef, - config: &HashMap>, - merged_config: &v1alpha1::TrinoConfig, - trino_authentication_config: &TrinoAuthenticationConfig, - trino_opa_config: &Option, - cluster_info: &KubernetesClusterInfo, - resolved_fte_config: &Option, - resolved_spooling_config: &Option, -) -> Result { - let mut cm_conf_data = BTreeMap::new(); - - let product_version = &resolved_product_image.product_version; - let product_version = - u16::from_str(product_version).context(ParseTrinoVersionSnafu { product_version })?; - let jvm_config = config::jvm::jvm_config( - product_version, - merged_config, - role, - &rolegroup_ref.role_group, - ) - .context(FailedToCreateJvmConfigSnafu)?; - - // TODO: we support only one coordinator for now - let coordinator_ref: TrinoPodRef = trino - .coordinator_pods() - .context(InternalOperatorFailureSnafu)? - .next() - .context(MissingCoordinatorPodsSnafu)?; - - // Add additional config files for authentication - cm_conf_data.extend(trino_authentication_config.config_files(trino_role)); - - for (property_name_kind, config) in config { - // We used this temporary map to add all dynamically resolved (e.g. discovery config maps) - // properties. This will be extended with the merged role group properties (transformed_config) - // to respect all possible override settings. - let mut dynamic_resolved_config = BTreeMap::>::new(); - - let transformed_config: BTreeMap> = config - .iter() - .map(|(k, v)| (k.clone(), Some(v.clone()))) - .collect(); - - match property_name_kind { - PropertyNameKind::File(file_name) if file_name == CONFIG_PROPERTIES => { - // Add authentication properties (only required for the Coordinator) - dynamic_resolved_config.extend( - trino_authentication_config - .config_properties(trino_role) - .into_iter() - .map(|(k, v)| (k, Some(v))) - .collect::>>(), - ); - - let protocol = if trino.get_internal_tls().is_some() { - TrinoDiscoveryProtocol::Https - } else { - TrinoDiscoveryProtocol::Http - }; - - let discovery = TrinoDiscovery::new(&coordinator_ref, protocol); - dynamic_resolved_config.insert( - DISCOVERY_URI.to_string(), - Some(discovery.discovery_uri(cluster_info)), - ); - - dynamic_resolved_config - .extend(graceful_shutdown_config_properties(trino, trino_role)); - - // Add fault tolerant execution properties from resolved configuration - if let Some(resolved_fte) = resolved_fte_config { - dynamic_resolved_config.extend( - resolved_fte - .config_properties - .iter() - .map(|(k, v)| (k.clone(), Some(v.clone()))), - ); - } - - // Add spooling properties from resolved configuration - if let Some(resolved_spooling) = resolved_spooling_config { - dynamic_resolved_config.extend( - resolved_spooling - .config_properties - .iter() - .map(|(k, v)| (k.clone(), Some(v.clone()))), - ); - } - - // Add static properties and overrides - dynamic_resolved_config.extend(transformed_config); - - let config_properties = product_config::writer::to_java_properties_string( - dynamic_resolved_config.iter(), - ) - .context(FailedToWriteJavaPropertiesSnafu)?; - - cm_conf_data.insert(file_name.to_string(), config_properties); - } - - PropertyNameKind::File(file_name) if file_name == NODE_PROPERTIES => { - // Add static properties and overrides - dynamic_resolved_config.extend(transformed_config); - - let node_properties = product_config::writer::to_java_properties_string( - dynamic_resolved_config.iter(), - ) - .context(FailedToWriteJavaPropertiesSnafu)?; - - cm_conf_data.insert(file_name.to_string(), node_properties); - } - PropertyNameKind::File(file_name) if file_name == LOG_PROPERTIES => { - // No overrides required here, all settings can be set via logging options - if let Some(log_properties) = get_log_properties(&merged_config.logging) { - cm_conf_data.insert(file_name.to_string(), log_properties); - } - - if let Some(vector_toml) = get_vector_toml(rolegroup_ref, &merged_config.logging) - .context(InvalidLoggingConfigSnafu { - cm_name: rolegroup_ref.object_name(), - })? - { - cm_conf_data.insert( - product_logging::framework::VECTOR_CONFIG_FILE.to_string(), - vector_toml, - ); - } - } - PropertyNameKind::File(file_name) if file_name == ACCESS_CONTROL_PROPERTIES => { - if let Some(trino_opa_config) = trino_opa_config { - dynamic_resolved_config.extend(trino_opa_config.as_config()); - } - - // Add static properties and overrides - dynamic_resolved_config.extend(transformed_config); - - if !dynamic_resolved_config.is_empty() { - let access_control_properties = - product_config::writer::to_java_properties_string( - dynamic_resolved_config.iter(), - ) - .context(FailedToWriteJavaPropertiesSnafu)?; - - cm_conf_data.insert(file_name.to_string(), access_control_properties); - } - } - PropertyNameKind::File(file_name) if file_name == JVM_CONFIG => {} - PropertyNameKind::File(file_name) if file_name == SPOOLING_MANAGER_PROPERTIES => { - // Add automatic properties for the spooling protocol - if let Some(spooling_config) = resolved_spooling_config { - dynamic_resolved_config = spooling_config - .spooling_manager_properties - .iter() - .map(|(k, v)| (k.clone(), Some(v.clone()))) - .collect(); - } - - // Override automatic properties with user provided configuration for the spooling protocol - dynamic_resolved_config.extend(transformed_config); - - if !dynamic_resolved_config.is_empty() { - cm_conf_data.insert( - file_name.to_string(), - to_java_properties_string(dynamic_resolved_config.iter()) - .with_context(|_| FailedToWriteJavaPropertiesSnafu)?, - ); - } - } - PropertyNameKind::File(file_name) if file_name == EXCHANGE_MANAGER_PROPERTIES => { - // Add exchange manager properties from resolved fault tolerant execution configuration - if let Some(resolved_fte) = resolved_fte_config { - dynamic_resolved_config = resolved_fte - .exchange_manager_properties - .iter() - .map(|(k, v)| (k.clone(), Some(v.clone()))) - .collect(); - } - - // Override automatic properties with user provided configuration for the spooling protocol - dynamic_resolved_config.extend(transformed_config); - - if !dynamic_resolved_config.is_empty() { - cm_conf_data.insert( - file_name.to_string(), - to_java_properties_string(dynamic_resolved_config.iter()) - .with_context(|_| FailedToWriteJavaPropertiesSnafu)?, - ); - } - } - _ => {} - } - } - - cm_conf_data.insert(JVM_CONFIG.to_string(), jvm_config.to_string()); - - let jvm_sec_props: BTreeMap> = config - .get(&PropertyNameKind::File(JVM_SECURITY_PROPERTIES.to_string())) - .cloned() - .unwrap_or_default() - .into_iter() - .map(|(k, v)| (k, Some(v))) - .collect(); - - cm_conf_data.insert( - JVM_SECURITY_PROPERTIES.to_string(), - to_java_properties_string(jvm_sec_props.iter()).with_context(|_| { - JvmSecurityPropertiesSnafu { - rolegroup: rolegroup_ref.role_group.clone(), - } - })?, - ); - - ConfigMapBuilder::new() - .metadata( - ObjectMetaBuilder::new() - .name_and_namespace(trino) - .name(rolegroup_ref.object_name()) - .ownerreference_from_resource(trino, None, Some(true)) - .context(ObjectMissingMetadataForOwnerRefSnafu)? - .with_recommended_labels(&build_recommended_labels( - trino, - &resolved_product_image.app_version_label_value, - &rolegroup_ref.role, - &rolegroup_ref.role_group, - )) - .context(MetadataBuildSnafu)? - .build(), - ) - .data(cm_conf_data) - .build() - .with_context(|_| BuildRoleGroupConfigSnafu { - rolegroup: rolegroup_ref.clone(), - }) -} /// The rolegroup catalog [`ConfigMap`] configures the rolegroup catalog based on the configuration /// given by the administrator @@ -920,21 +659,15 @@ fn build_rolegroup_catalog_config_map( catalogs .iter() .map(|catalog| { - let catalog_props = catalog + let catalog_props: BTreeMap = catalog .properties .iter() - .map(|(k, v)| (k.to_string(), Some(v.to_string()))) - .collect::>(); + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); Ok(( format!("{}.properties", catalog.name), - // false positive https://github.com/rust-lang/rust-clippy/issues/9280 - // we need the tuple (&String, &Option) which the extra map is doing. - // Removing the map changes the type to &(String, Option) - #[allow(clippy::map_identity)] - product_config::writer::to_java_properties_string( - catalog_props.iter().map(|(k, v)| (k, v)), - ) - .context(FailedToWriteJavaPropertiesSnafu)?, + build::properties::writer::to_java_properties_string(&catalog_props) + .context(FailedToWriteJavaPropertiesSnafu)?, )) }) .collect::>()?, diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs index cf2c9a5a..571188db 100644 --- a/rust/operator-binary/src/controller/build/properties/config_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -193,7 +193,7 @@ pub fn build( // would have errored earlier (MissingCoordinatorPods). // Graceful shutdown. - for (k, v) in crate::operations::graceful_shutdown_config_properties_v2(cluster, role) { + for (k, v) in crate::operations::graceful_shutdown_config_properties(cluster, role) { props.insert(k, v); } diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 50d32535..b287c8d9 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -5,7 +5,7 @@ pub mod client_protocol; pub mod discovery; pub mod fault_tolerant_execution; -use std::{collections::BTreeMap, ops::Div, str::FromStr}; +use std::{collections::BTreeMap, str::FromStr}; use affinity::get_affinity; use serde::{Deserialize, Serialize}; @@ -25,13 +25,12 @@ use stackable_operator::{ fragment::{self, Fragment, ValidationError}, merge::Merge, }, - config_overrides::{KeyValueConfigOverrides, KeyValueOverridesProvider}, + config_overrides::KeyValueConfigOverrides, crd::authentication::core, deep_merger::ObjectOverrides, k8s_openapi::apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::LabelSelector}, kube::{CustomResource, ResourceExt, runtime::reflector::ObjectRef}, memory::{BinaryMultiple, MemoryQuantity}, - product_config_utils::{Configuration, Error as ConfigError}, product_logging::{self, spec::Logging}, role_utils::{ CommonConfiguration, GenericRoleConfig, JavaCommonConfig, Role, RoleGroup, RoleGroupRef, @@ -82,8 +81,6 @@ pub const ACCESS_CONTROL_PROPERTIES: &str = "access-control.properties"; pub const JVM_SECURITY_PROPERTIES: &str = "security.properties"; pub const EXCHANGE_MANAGER_PROPERTIES: &str = "exchange-manager.properties"; pub const SPOOLING_MANAGER_PROPERTIES: &str = "spooling-manager.properties"; -// node.properties -pub const NODE_ENVIRONMENT: &str = "node.environment"; // config.properties pub const COORDINATOR: &str = "coordinator"; pub const DISCOVERY_URI: &str = "discovery.uri"; @@ -133,7 +130,6 @@ pub const LOG_PATH: &str = "log.path"; pub const LOG_COMPRESSION: &str = "log.compression"; pub const LOG_MAX_SIZE: &str = "log.max-size"; pub const LOG_MAX_TOTAL_SIZE: &str = "log.max-total-size"; -const LOG_FILE_COUNT: u32 = 2; pub const MAX_TRINO_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity { value: 10.0, unit: BinaryMultiple::Mebi, @@ -460,24 +456,6 @@ impl v1alpha1::TrinoAuthorizationOpaConfig { } } -impl KeyValueOverridesProvider for v1alpha1::TrinoConfigOverrides { - fn get_key_value_overrides(&self, file: &str) -> BTreeMap> { - let field = match file { - CONFIG_PROPERTIES => self.config_properties.as_ref(), - NODE_PROPERTIES => self.node_properties.as_ref(), - LOG_PROPERTIES => self.log_properties.as_ref(), - JVM_SECURITY_PROPERTIES => self.security_properties.as_ref(), - ACCESS_CONTROL_PROPERTIES => self.access_control_properties.as_ref(), - EXCHANGE_MANAGER_PROPERTIES => self.exchange_manager_properties.as_ref(), - SPOOLING_MANAGER_PROPERTIES => self.spooling_manager_properties.as_ref(), - _ => None, - }; - field - .map(KeyValueConfigOverrides::as_product_config_overrides) - .unwrap_or_default() - } -} - // TODO: Remove once `KeyValueConfigOverrides` derives `Merge` upstream. // `stackable_operator::v2::role_utils::with_validated_config` requires // `ConfigOverrides: Merge`. Until upstreamed, merge field-by-field: @@ -684,207 +662,6 @@ impl v1alpha1::TrinoConfig { } } -impl Configuration for v1alpha1::TrinoConfigFragment { - type Configurable = v1alpha1::TrinoCluster; - - fn compute_env( - &self, - _resource: &Self::Configurable, - _role_name: &str, - ) -> Result>, ConfigError> { - Ok(BTreeMap::new()) - } - - fn compute_cli( - &self, - _resource: &Self::Configurable, - _role_name: &str, - ) -> Result>, ConfigError> { - Ok(BTreeMap::new()) - } - - fn compute_files( - &self, - resource: &Self::Configurable, - role_name: &str, - file: &str, - ) -> Result>, ConfigError> { - let mut result = BTreeMap::new(); - let authentication_enabled = resource.authentication_enabled(); - let server_tls_enabled: bool = resource.get_server_tls().is_some(); - let internal_tls_enabled: bool = resource.get_internal_tls().is_some(); - - match file { - NODE_PROPERTIES => { - // The resource name is alphanumeric and may have "-" characters - // The Trino node environment is bound to alphanumeric lowercase and "_" characters - // and must start with alphanumeric (which is the case for resource names as well?) - // see https://trino.io/docs/current/installation/deployment.html - let node_env = resource.name_any().to_ascii_lowercase().replace('-', "_"); - result.insert(NODE_ENVIRONMENT.to_string(), Some(node_env)); - } - CONFIG_PROPERTIES => { - // coordinator or worker - result.insert( - COORDINATOR.to_string(), - Some((role_name == TrinoRole::Coordinator.to_string()).to_string()), - ); - // TrinoConfig properties - if let Some(query_max_memory) = &self.query_max_memory { - result.insert( - QUERY_MAX_MEMORY.to_string(), - Some(query_max_memory.to_string()), - ); - } - if let Some(query_max_memory_per_node) = &self.query_max_memory_per_node { - result.insert( - QUERY_MAX_MEMORY_PER_NODE.to_string(), - Some(query_max_memory_per_node.to_string()), - ); - } - - // The log format used by Trino - result.insert(LOG_FORMAT.to_string(), Some("json".to_string())); - // The path to the log file used by Trino - result.insert( - LOG_PATH.to_string(), - Some(format!( - "{STACKABLE_LOG_DIR}/{container}/server.airlift.json", - container = Container::Trino - )), - ); - - // We do not compress. This will result in LOG_MAX_TOTAL_SIZE / LOG_MAX_SIZE files. - result.insert(LOG_COMPRESSION.to_string(), Some("none".to_string())); - - // The size of one log file - result.insert( - LOG_MAX_SIZE.to_string(), - Some(format!( - // Trino uses the unit "MB" for MiB. - "{}MB", - MAX_TRINO_LOG_FILES_SIZE - .scale_to(BinaryMultiple::Mebi) - .div(LOG_FILE_COUNT as f32) - .ceil() - .value, - )), - ); - // The maximum size of all logfiles combined - result.insert( - LOG_MAX_TOTAL_SIZE.to_string(), - Some(format!( - // Trino uses the unit "MB" for MiB. - "{}MB", - MAX_TRINO_LOG_FILES_SIZE - .scale_to(BinaryMultiple::Mebi) - .ceil() - .value, - )), - ); - - // disable http-request logs - result.insert( - "http-server.log.enabled".to_string(), - Some("false".to_string()), - ); - - // Always use the internal secret (base64) - result.insert( - INTERNAL_COMMUNICATION_SHARED_SECRET.to_string(), - Some(format!("${{ENV:{secret}}}", secret = ENV_INTERNAL_SECRET)), - ); - - // If authentication is enabled and client tls is explicitly deactivated we error out - // Therefore from here on we can use resource.get_server_tls() as the only source - // of truth when enabling client TLS. - if authentication_enabled && !server_tls_enabled { - return Err(ConfigError::InvalidProductSpecificConfiguration { - reason: - "Trino requires client TLS to be enabled if any authentication method is enabled! TLS was set to null. \ - Please set 'spec.clusterConfig.tls.secretClass' or use the provided default value.".to_string(), - }); - } - - if server_tls_enabled || internal_tls_enabled { - // enable TLS - result.insert( - HTTP_SERVER_HTTPS_ENABLED.to_string(), - Some(true.to_string()), - ); - // via https port - result.insert( - HTTP_SERVER_HTTPS_PORT.to_string(), - Some(HTTPS_PORT.to_string()), - ); - - let tls_store_dir = if server_tls_enabled { - STACKABLE_SERVER_TLS_DIR - } else { - // allow insecure communication - result.insert( - HTTP_SERVER_AUTHENTICATION_ALLOW_INSECURE_OVER_HTTP.to_string(), - Some("true".to_string()), - ); - // via the http port - result.insert( - HTTP_SERVER_HTTP_PORT.to_string(), - Some(HTTP_PORT.to_string()), - ); - - STACKABLE_INTERNAL_TLS_DIR - }; - - result.insert( - HTTP_SERVER_KEYSTORE_PATH.to_string(), - Some(format!("{}/{}", tls_store_dir, "keystore.p12")), - ); - result.insert( - HTTP_SERVER_HTTPS_KEYSTORE_KEY.to_string(), - Some(STACKABLE_TLS_STORE_PASSWORD.to_string()), - ); - result.insert( - HTTP_SERVER_TRUSTSTORE_PATH.to_string(), - Some(format!("{}/{}", tls_store_dir, "truststore.p12")), - ); - result.insert( - HTTP_SERVER_HTTPS_TRUSTSTORE_KEY.to_string(), - Some(STACKABLE_TLS_STORE_PASSWORD.to_string()), - ); - } - - if internal_tls_enabled { - result.insert( - INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_PATH.to_string(), - Some(format!("{}/keystore.p12", STACKABLE_INTERNAL_TLS_DIR)), - ); - result.insert( - INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_KEY.to_string(), - Some(STACKABLE_TLS_STORE_PASSWORD.to_string()), - ); - result.insert( - INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_PATH.to_string(), - Some(format!("{}/truststore.p12", STACKABLE_INTERNAL_TLS_DIR)), - ); - result.insert( - INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_KEY.to_string(), - Some(STACKABLE_TLS_STORE_PASSWORD.to_string()), - ); - result.insert( - NODE_INTERNAL_ADDRESS_SOURCE.to_string(), - Some(NODE_INTERNAL_ADDRESS_SOURCE_FQDN.to_string()), - ); - } - } - LOG_PROPERTIES => {} - ACCESS_CONTROL_PROPERTIES => {} - _ => {} - } - - Ok(result) - } -} - impl v1alpha1::TrinoCluster { /// Returns the name of the cluster and raises an Error if the name is not set. pub fn name_r(&self) -> Result { diff --git a/rust/operator-binary/src/operations/graceful_shutdown.rs b/rust/operator-binary/src/operations/graceful_shutdown.rs index ff878380..b1cca5ec 100644 --- a/rust/operator-binary/src/operations/graceful_shutdown.rs +++ b/rust/operator-binary/src/operations/graceful_shutdown.rs @@ -23,45 +23,9 @@ pub enum Error { }, } +/// Computes the graceful-shutdown-related properties for the role's +/// `config.properties` file from a [`ValidatedCluster`]. pub fn graceful_shutdown_config_properties( - trino: &v1alpha1::TrinoCluster, - role: &TrinoRole, -) -> BTreeMap> { - match role { - TrinoRole::Coordinator => { - // Only set query.max-execution-time if fault tolerant execution is not configured. - // With fault tolerant execution enabled, queries can be retried and run indefinitely. - if trino.spec.cluster_config.fault_tolerant_execution.is_none() { - let min_worker_graceful_shutdown_timeout = - trino.min_worker_graceful_shutdown_timeout(); - // We know that queries taking longer than the minimum gracefulShutdownTimeout are subject to failure. - // Read operator docs for reasoning. - BTreeMap::from([( - "query.max-execution-time".to_string(), - Some(format!( - "{}s", - min_worker_graceful_shutdown_timeout.as_secs() - )), - )]) - } else { - BTreeMap::new() - } - } - TrinoRole::Worker => BTreeMap::from([( - "shutdown.grace-period".to_string(), - Some(format!("{}s", WORKER_SHUTDOWN_GRACE_PERIOD.as_secs())), - )]), - } -} - -/// V2 variant of [`graceful_shutdown_config_properties`] that reads from a -/// [`ValidatedCluster`]. The legacy function is preserved until reconcile is -/// switched (Task 14) and is then deleted (Task 15). -/// -/// Returns a flat `BTreeMap` (unlike the legacy variant which -/// returns `BTreeMap>`) — the new builders no longer -/// thread `Option` values through. -pub fn graceful_shutdown_config_properties_v2( cluster: &ValidatedCluster, role: TrinoRole, ) -> BTreeMap { @@ -71,7 +35,7 @@ pub fn graceful_shutdown_config_properties_v2( // With fault tolerant execution enabled, queries can be retried and run indefinitely. if cluster.resolved_fte_config.is_none() { let min_worker_graceful_shutdown_timeout = - min_worker_graceful_shutdown_timeout_v2(cluster); + min_worker_graceful_shutdown_timeout(cluster); BTreeMap::from([( "query.max-execution-time".to_string(), format!("{}s", min_worker_graceful_shutdown_timeout.as_secs()), @@ -91,7 +55,7 @@ pub fn graceful_shutdown_config_properties_v2( /// /// Mirrors [`v1alpha1::TrinoCluster::min_worker_graceful_shutdown_timeout`] but /// reads from [`ValidatedCluster::role_group_configs`]. -fn min_worker_graceful_shutdown_timeout_v2( +fn min_worker_graceful_shutdown_timeout( cluster: &ValidatedCluster, ) -> stackable_operator::shared::time::Duration { cluster diff --git a/rust/operator-binary/src/product_logging.rs b/rust/operator-binary/src/product_logging.rs index 8fceac5d..7b09ddcc 100644 --- a/rust/operator-binary/src/product_logging.rs +++ b/rust/operator-binary/src/product_logging.rs @@ -55,23 +55,7 @@ impl From for TrinoLogLevel { } } -/// Return the `log.properties` configuration -pub fn get_log_properties(logging: &Logging) -> Option { - if let Some(ContainerLogConfig { - choice: Some(ContainerLogConfigChoice::Automatic(log_config)), - }) = logging.containers.get(&Container::Trino) - { - Some(create_trino_log_properties(log_config)) - } else { - None - } -} - -/// Same as [`get_log_properties`] but returns a typed `BTreeMap` instead of -/// a Java-properties-formatted string. -/// -/// New per-file builders use this; the old `get_log_properties` will be removed -/// once the legacy `build_rolegroup_config_map` is deleted (see Task 14). +/// Return the `log.properties` content as a typed `BTreeMap`. pub fn get_log_property_map( logging: &Logging, ) -> Option> { @@ -120,26 +104,3 @@ pub fn get_vector_toml( } } -/// Create trino `log.properties` containing loggers and their respective log levels. -/// The operator-rs framework `LogLevel` offers more choices which are parsed to the available -/// `TrinoLogLevel`. -/// -/// The `log.properties` will adhere to the example format: -/// ``` -/// io.trino=debug -/// io.trino.server=info -/// ``` -fn create_trino_log_properties(automatic_container_config: &AutomaticContainerLogConfig) -> String { - automatic_container_config - .loggers - .iter() - .map(|(logger, config)| { - let log_level = TrinoLogLevel::from(config.level); - if logger == AutomaticContainerLogConfig::ROOT_LOGGER { - format!("={}\n", log_level) - } else { - format!("{}={}\n", logger, log_level) - } - }) - .collect::() -} From 1854eba592cab44f89712cfc370693b31afbe978 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 21:57:13 +0200 Subject: [PATCH 19/27] chore(helm): remove product-config artifacts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deletes deploy/config-spec/properties.yaml and the regenerated chart copy at deploy/helm/trino-operator/configs/properties.yaml. Removes the config-spec volume + mount from deployment.yaml. The chart's ConfigMap template (templates/configmap.yaml) is preserved intentionally — it now renders an empty data block, which keeps the chart shape unchanged and lets any custom mounts users have added via overrides continue to work. The checksum/config annotation on the Deployment is harmless against an empty ConfigMap. Co-Authored-By: Claude Opus 4.7 (1M context) --- deploy/config-spec/properties.yaml | 269 ------------------ .../trino-operator/configs/properties.yaml | 269 ------------------ .../trino-operator/templates/deployment.yaml | 7 - 3 files changed, 545 deletions(-) delete mode 100644 deploy/config-spec/properties.yaml delete mode 100644 deploy/helm/trino-operator/configs/properties.yaml diff --git a/deploy/config-spec/properties.yaml b/deploy/config-spec/properties.yaml deleted file mode 100644 index 89c7e6b3..00000000 --- a/deploy/config-spec/properties.yaml +++ /dev/null @@ -1,269 +0,0 @@ -version: 0.1.0 -spec: - units: - - unit: &unitNodeEnvironment - name: "node_environment" - regex: "^[a-z][a-z0-9_]*[a-z0-9]$" - examples: - - "a1_2_3b" - - unit: &unitMemory - name: "memory" - regex: "(^\\p{N}+)(?:\\s*)((?:b|k|m|g|t|p|kb|mb|gb|tb|pb|B|K|M|G|T|P|KB|MB|GB|TB|PB)\\b$)" - examples: - - "1024b" - - "1024kb" - - "500m" - - "1g" - -################################################################################################### -# node.properties -################################################################################################### -properties: - - property: - propertyNames: - - name: "networkaddress.cache.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "30" - roles: - - name: "coordinator" - required: true - asOfVersion: "0.0.0" - comment: "TTL for successfully resolved domain names." - description: "TTL for successfully resolved domain names." - - - property: - propertyNames: - - name: "networkaddress.cache.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "30" - roles: - - name: "worker" - required: true - asOfVersion: "0.0.0" - comment: "TTL for successfully resolved domain names." - description: "TTL for successfully resolved domain names." - - - property: - propertyNames: - - name: "networkaddress.cache.negative.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "0" - roles: - - name: "coordinator" - required: true - asOfVersion: "0.0.0" - comment: "TTL for domain names that cannot be resolved." - description: "TTL for domain names that cannot be resolved." - - - property: - propertyNames: - - name: "networkaddress.cache.negative.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "0" - roles: - - name: "worker" - required: true - asOfVersion: "0.0.0" - comment: "TTL for domain names that cannot be resolved." - description: "TTL for domain names that cannot be resolved." - - - - property: &nodeEnvironment - propertyNames: - - name: "node.environment" - kind: - type: "file" - file: "node.properties" - datatype: - type: "string" - unit: *unitNodeEnvironment - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" - -################################################################################################### -# config.properties -################################################################################################### - - property: &coordinator - propertyNames: - - name: "coordinator" - kind: - type: "file" - file: "config.properties" - datatype: - type: "bool" - defaultValues: - - fromVersion: "0.0.0" - value: "false" - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" - - - property: &nodeSchedulerIncludeCoordinator - propertyNames: - - name: "node-scheduler.include-coordinator" - kind: - type: "file" - file: "config.properties" - datatype: - type: "bool" - defaultValues: - - fromVersion: "0.0.0" - value: "false" - roles: - - name: "coordinator" - required: true - asOfVersion: "0.0.0" - - - property: &httpServerHttpPort - propertyNames: - - name: "http-server.http.port" - kind: - type: "file" - file: "config.properties" - datatype: - type: "integer" - min: "1024" - max: "65535" - defaultValues: - - fromVersion: "0.0.0" - value: "8080" - roles: - - name: "coordinator" - required: false - - name: "worker" - required: false - asOfVersion: "0.0.0" - - - property: &httpServerHttpsPort - propertyNames: - - name: "http-server.https.port" - kind: - type: "file" - file: "config.properties" - datatype: - type: "integer" - min: "1024" - max: "65535" - defaultValues: - - fromVersion: "0.0.0" - value: "8443" - roles: - - name: "coordinator" - required: false - - name: "worker" - required: false - asOfVersion: "0.0.0" - - - property: &queryMaxMemory - propertyNames: - - name: "query.max-memory" - kind: - type: "file" - file: "config.properties" - datatype: - type: "string" - unit: *unitMemory - defaultValues: - - fromVersion: "0.0.0" - value: "50GB" - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" - - - property: &queryMaxMemoryPerNode - propertyNames: - - name: "query.max-memory-per-node" - kind: - type: "file" - file: "config.properties" - datatype: - type: "string" - unit: *unitMemory - roles: - - name: "coordinator" - required: false - - name: "worker" - required: false - asOfVersion: "0.0.0" - - - property: &httpServerAuthenticationType - propertyNames: - - name: "http-server.authentication.type" - kind: - type: "file" - file: "config.properties" - datatype: - type: "string" - roles: - - name: "coordinator" - required: false - asOfVersion: "0.0.0" - -################################################################################################### -# jvm.config -################################################################################################### - -################################################################################################### -# log.properties -################################################################################################### - - - property: &ioTrino - propertyNames: - - name: "io.trino" - kind: - type: "file" - file: "log.properties" - datatype: - type: "string" - defaultValues: - - fromVersion: "0.0.0" - value: "INFO" - allowedValues: - - "INFO" - - "DEBUG" - - "WARN" - - "ERROR" - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" diff --git a/deploy/helm/trino-operator/configs/properties.yaml b/deploy/helm/trino-operator/configs/properties.yaml deleted file mode 100644 index 89c7e6b3..00000000 --- a/deploy/helm/trino-operator/configs/properties.yaml +++ /dev/null @@ -1,269 +0,0 @@ -version: 0.1.0 -spec: - units: - - unit: &unitNodeEnvironment - name: "node_environment" - regex: "^[a-z][a-z0-9_]*[a-z0-9]$" - examples: - - "a1_2_3b" - - unit: &unitMemory - name: "memory" - regex: "(^\\p{N}+)(?:\\s*)((?:b|k|m|g|t|p|kb|mb|gb|tb|pb|B|K|M|G|T|P|KB|MB|GB|TB|PB)\\b$)" - examples: - - "1024b" - - "1024kb" - - "500m" - - "1g" - -################################################################################################### -# node.properties -################################################################################################### -properties: - - property: - propertyNames: - - name: "networkaddress.cache.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "30" - roles: - - name: "coordinator" - required: true - asOfVersion: "0.0.0" - comment: "TTL for successfully resolved domain names." - description: "TTL for successfully resolved domain names." - - - property: - propertyNames: - - name: "networkaddress.cache.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "30" - roles: - - name: "worker" - required: true - asOfVersion: "0.0.0" - comment: "TTL for successfully resolved domain names." - description: "TTL for successfully resolved domain names." - - - property: - propertyNames: - - name: "networkaddress.cache.negative.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "0" - roles: - - name: "coordinator" - required: true - asOfVersion: "0.0.0" - comment: "TTL for domain names that cannot be resolved." - description: "TTL for domain names that cannot be resolved." - - - property: - propertyNames: - - name: "networkaddress.cache.negative.ttl" - kind: - type: "file" - file: "security.properties" - datatype: - type: "integer" - min: "0" - recommendedValues: - - fromVersion: "0.0.0" - value: "0" - roles: - - name: "worker" - required: true - asOfVersion: "0.0.0" - comment: "TTL for domain names that cannot be resolved." - description: "TTL for domain names that cannot be resolved." - - - - property: &nodeEnvironment - propertyNames: - - name: "node.environment" - kind: - type: "file" - file: "node.properties" - datatype: - type: "string" - unit: *unitNodeEnvironment - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" - -################################################################################################### -# config.properties -################################################################################################### - - property: &coordinator - propertyNames: - - name: "coordinator" - kind: - type: "file" - file: "config.properties" - datatype: - type: "bool" - defaultValues: - - fromVersion: "0.0.0" - value: "false" - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" - - - property: &nodeSchedulerIncludeCoordinator - propertyNames: - - name: "node-scheduler.include-coordinator" - kind: - type: "file" - file: "config.properties" - datatype: - type: "bool" - defaultValues: - - fromVersion: "0.0.0" - value: "false" - roles: - - name: "coordinator" - required: true - asOfVersion: "0.0.0" - - - property: &httpServerHttpPort - propertyNames: - - name: "http-server.http.port" - kind: - type: "file" - file: "config.properties" - datatype: - type: "integer" - min: "1024" - max: "65535" - defaultValues: - - fromVersion: "0.0.0" - value: "8080" - roles: - - name: "coordinator" - required: false - - name: "worker" - required: false - asOfVersion: "0.0.0" - - - property: &httpServerHttpsPort - propertyNames: - - name: "http-server.https.port" - kind: - type: "file" - file: "config.properties" - datatype: - type: "integer" - min: "1024" - max: "65535" - defaultValues: - - fromVersion: "0.0.0" - value: "8443" - roles: - - name: "coordinator" - required: false - - name: "worker" - required: false - asOfVersion: "0.0.0" - - - property: &queryMaxMemory - propertyNames: - - name: "query.max-memory" - kind: - type: "file" - file: "config.properties" - datatype: - type: "string" - unit: *unitMemory - defaultValues: - - fromVersion: "0.0.0" - value: "50GB" - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" - - - property: &queryMaxMemoryPerNode - propertyNames: - - name: "query.max-memory-per-node" - kind: - type: "file" - file: "config.properties" - datatype: - type: "string" - unit: *unitMemory - roles: - - name: "coordinator" - required: false - - name: "worker" - required: false - asOfVersion: "0.0.0" - - - property: &httpServerAuthenticationType - propertyNames: - - name: "http-server.authentication.type" - kind: - type: "file" - file: "config.properties" - datatype: - type: "string" - roles: - - name: "coordinator" - required: false - asOfVersion: "0.0.0" - -################################################################################################### -# jvm.config -################################################################################################### - -################################################################################################### -# log.properties -################################################################################################### - - - property: &ioTrino - propertyNames: - - name: "io.trino" - kind: - type: "file" - file: "log.properties" - datatype: - type: "string" - defaultValues: - - fromVersion: "0.0.0" - value: "INFO" - allowedValues: - - "INFO" - - "DEBUG" - - "WARN" - - "ERROR" - roles: - - name: "coordinator" - required: true - - name: "worker" - required: true - asOfVersion: "0.0.0" diff --git a/deploy/helm/trino-operator/templates/deployment.yaml b/deploy/helm/trino-operator/templates/deployment.yaml index d0cecba4..752b0eed 100644 --- a/deploy/helm/trino-operator/templates/deployment.yaml +++ b/deploy/helm/trino-operator/templates/deployment.yaml @@ -41,9 +41,6 @@ spec: imagePullPolicy: {{ .Values.image.pullPolicy }} resources: {{- toYaml .Values.resources | nindent 12 }} - volumeMounts: - - mountPath: /etc/stackable/{{ include "operator.appname" . }}/config-spec - name: config-spec env: # The following env vars are passed as clap (think CLI) arguments to the operator. # They are picked up by clap using the structs defied in the operator. @@ -88,10 +85,6 @@ spec: {{- include "telemetry.envVars" . | nindent 12 }} {{- include "maintenance.envVars" . | nindent 12 }} - volumes: - - name: config-spec - configMap: - name: {{ include "operator.fullname" . }}-configmap {{- with .Values.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }} From 9206e952a39ebfd5aa558058086a27a2b2043c50 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 22:09:26 +0200 Subject: [PATCH 20/27] test(controller): port override and env tests to the new pipeline Rewrites the four tests that previously called ProductConfigManager::from_str(include_str!(...)) to use the new validate -> build_rolegroup_config_map pipeline directly. test_env_overrides now asserts against ValidatedCluster.role_group_configs[role][rg].env_overrides. Tests count: 74 -> 78. Co-Authored-By: Claude Opus 4.7 (1M context) --- rust/operator-binary/src/controller.rs | 359 ++++++++++++++++++++++++- 1 file changed, 355 insertions(+), 4 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index e1c579cd..28563df8 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1421,8 +1421,359 @@ fn tls_volume_mounts( Ok(()) } -// Tests for config overrides, client protocol overrides, access control -// overrides, and env overrides will be rewritten against the new pipeline -// in Task 17. They were deleted in this commit because their test helper -// (build_config_map) depended on the legacy ProductConfigManager API. +#[cfg(test)] +mod tests { + use stackable_operator::{ + cli::OperatorEnvironmentOptions, + commons::networking::DomainName, + k8s_openapi::api::core::v1::ConfigMap, + kube::runtime::reflector::ObjectRef, + role_utils::RoleGroupRef, + utils::cluster_info::KubernetesClusterInfo, + }; + + use super::*; + use crate::{ + authorization::opa::TrinoOpaConfig, + config::{ + client_protocol::ResolvedClientProtocolConfig, + fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig, + }, + controller::dereference::DereferencedObjects, + crd::{ENV_SPOOLING_SECRET, TrinoRole, v1alpha1}, + }; + + async fn build_config_map(trino_yaml: &str) -> ConfigMap { + let deserializer = serde_yaml::Deserializer::from_str(trino_yaml); + let mut trino: v1alpha1::TrinoCluster = + serde_yaml::with::singleton_map_recursive::deserialize(deserializer) + .expect("invalid test input"); + trino.metadata.namespace = Some("default".to_owned()); + trino.metadata.uid = Some("42".to_owned()); + + let cluster_info = KubernetesClusterInfo { + cluster_domain: DomainName::try_from("cluster.local").unwrap(), + }; + + // Build dereferenced objects directly from the test cluster spec — + // no Kubernetes client is needed because the test fixtures only use + // resolution paths that accept `None` for the client. + let namespace = trino.metadata.namespace.clone().unwrap(); + let resolved_fte_config = match &trino.spec.cluster_config.fault_tolerant_execution { + Some(fte) => Some( + ResolvedFaultTolerantExecutionConfig::from_config(fte, None, &namespace) + .await + .unwrap(), + ), + None => None, + }; + let resolved_client_protocol_config = match &trino.spec.cluster_config.client_protocol { + Some(cp) => Some( + ResolvedClientProtocolConfig::from_config(cp, None, &namespace) + .await + .unwrap(), + ), + None => None, + }; + // For OPA, the legacy helper used a hard-coded `TrinoOpaConfig` literal + // rather than resolving from cluster config; mirror that here so that + // `test_access_control_overrides` does not need a Kubernetes client and + // so that `test_config_overrides` keeps observing an + // `access-control.properties` entry in the rendered ConfigMap. + let trino_opa_config = Some(TrinoOpaConfig { + non_batched_connection_string: + "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/allow" + .to_string(), + batched_connection_string: + "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batch" + .to_string(), + row_filters_connection_string: Some( + "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/rowFilters" + .to_string(), + ), + batched_column_masking_connection_string: Some( + "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batchColumnMasks" + .to_string(), + ), + allow_permission_management_operations: true, + tls_secret_class: None, + }); + + let derefs = DereferencedObjects { + resolved_authentication_classes: Vec::new(), + catalog_definitions: Vec::new(), + catalogs: Vec::new(), + trino_opa_config, + resolved_fte_config, + resolved_client_protocol_config, + }; + + let operator_env = OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_string(), + operator_service_name: "trino-operator".to_string(), + image_repository: "oci.example.org".to_string(), + }; + + let validated = validate::validate(&trino, &derefs, &operator_env) + .expect("validate should succeed"); + + let trino_role = TrinoRole::Coordinator; + let role = trino.role(&trino_role).unwrap(); + let rolegroup_ref = RoleGroupRef { + cluster: ObjectRef::from_obj(&trino), + role: trino_role.to_string(), + role_group: "default".to_string(), + }; + let rg = &validated.role_group_configs[&trino_role]["default"]; + + let recommended_labels = build_recommended_labels( + &trino, + &validated.image.app_version_label_value, + &rolegroup_ref.role, + &rolegroup_ref.role_group, + ); + + let jvm_config = config::jvm::jvm_config( + validated.product_version, + &rg.config, + &role, + &rolegroup_ref.role_group, + ) + .expect("jvm config should render"); + let vector_toml = get_vector_toml(&rolegroup_ref, &rg.config.logging) + .expect("vector toml should render"); + + build::config_map::build_rolegroup_config_map( + &validated, + trino_role, + &rolegroup_ref, + &cluster_info, + recommended_labels, + jvm_config, + vector_toml, + &trino, + ) + .expect("build_rolegroup_config_map should succeed") + } + + #[tokio::test] + async fn test_config_overrides() { + let trino_yaml = r#" + apiVersion: trino.stackable.tech/v1alpha1 + kind: TrinoCluster + metadata: + name: simple-trino + spec: + image: + productVersion: "479" + clusterConfig: + catalogLabelSelector: + matchLabels: + trino: simple-trino + coordinators: + configOverrides: + config.properties: + foo: bar + level: role + hello-from-role: "true" + internal-communication.https.keystore.path: /my/custom/internal-truststore.p12 + roleGroups: + default: + configOverrides: + config.properties: + foo: bar + level: role-group + hello-from-role-group: "true" + http-server.https.truststore.path: /my/custom/truststore.p12 + replicas: 1 + workers: + roleGroups: + default: + replicas: 1 + "#; + let cm = build_config_map(trino_yaml).await.data.unwrap(); + let config = cm.get("config.properties").unwrap(); + assert!(config.contains("foo=bar")); + assert!(config.contains("level=role-group")); + assert!(config.contains("hello-from-role=true")); + assert!(config.contains("hello-from-role-group=true")); + assert!(config.contains("http-server.https.enabled=true")); + assert!( + config.contains("http-server.https.keystore.path=/stackable/server_tls/keystore.p12") + ); + assert!(config.contains( + "internal-communication.https.keystore.path=/my/custom/internal-truststore.p12" + )); + // Overwritten by configOverrides from role (does work) + assert!(config.contains("http-server.https.truststore.path=/my/custom/truststore.p12")); + + assert!(cm.contains_key("jvm.config")); + assert!(cm.contains_key("security.properties")); + assert!(cm.contains_key("node.properties")); + assert!(cm.contains_key("log.properties")); + assert!(cm.contains_key("access-control.properties")); + } + + #[tokio::test] + async fn test_client_protocol_config_overrides() { + let trino_yaml = r#" + apiVersion: trino.stackable.tech/v1alpha1 + kind: TrinoCluster + metadata: + name: simple-trino + spec: + image: + productVersion: "479" + clusterConfig: + catalogLabelSelector: + matchLabels: + trino: simple-trino + clientProtocol: + spooling: + location: s3://my-bucket/spooling + filesystem: + s3: + connection: + reference: test-s3-connection + coordinators: + configOverrides: + config.properties: + foo: bar + spooling-manager.properties: + fs.location: s3a://role-level + roleGroups: + default: + replicas: 1 + configOverrides: + spooling-manager.properties: + fs.location: s3a://role-group-level + workers: + roleGroups: + default: + replicas: 1 + "#; + + let cm = build_config_map(trino_yaml).await.data.unwrap(); + let config = cm.get("config.properties").unwrap(); + assert!(config.contains("protocol.spooling.enabled=true")); + assert!(config.contains(&format!( + "protocol.spooling.shared-secret-key=${{ENV\\:{ENV_SPOOLING_SECRET}}}" + ))); + assert!(config.contains("foo=bar")); + + let config = cm.get("spooling-manager.properties").unwrap(); + assert!(config.contains("fs.location=s3a\\://role-group-level")); + assert!(config.contains("spooling-manager.name=filesystem")); + } + + #[tokio::test] + async fn test_access_control_overrides() { + let trino_yaml = r#" + apiVersion: trino.stackable.tech/v1alpha1 + kind: TrinoCluster + metadata: + name: trino + spec: + image: + productVersion: "479" + clusterConfig: + catalogLabelSelector: + matchLabels: + trino: simple-trino + authorization: + opa: + configMapName: simple-opa + package: my-product + coordinators: + configOverrides: + access-control.properties: + hello-from-role: "true" # only defined here at role level + foo.bar: "false" # overridden by role group below + opa.allow-permission-management-operations: "false" # override value from config + roleGroups: + default: + configOverrides: + access-control.properties: + hello-from-role-group: "true" # only defined here at group level + foo.bar: "true" # overrides role value + opa.policy.batched-uri: "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batch-new" # override value from config + opa.policy.batch-column-masking-uri: "http://simple-opa.default.svc.cluster.local:8081/v1/data/my-product/batchColumnMasks-new" # override value from config + replicas: 1 + workers: + roleGroups: + default: + replicas: 1 + "#; + + let cm = build_config_map(trino_yaml).await.data.unwrap(); + let access_control_config = cm.get("access-control.properties").unwrap(); + + assert!(access_control_config.contains("access-control.name=opa")); + assert!(access_control_config.contains("hello-from-role=true")); + assert!(access_control_config.contains("hello-from-role-group=true")); + assert!(access_control_config.contains("foo.bar=true")); + assert!(access_control_config.contains("opa.allow-permission-management-operations=false")); + assert!(access_control_config.contains(r#"opa.policy.batched-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batch-new"#)); + assert!(access_control_config.contains(r#"opa.policy.batch-column-masking-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/batchColumnMasks-new"#)); + assert!(access_control_config.contains(r#"opa.policy.row-filters-uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/rowFilters"#)); + assert!(access_control_config.contains(r#"opa.policy.uri=http\://simple-opa.default.svc.cluster.local\:8081/v1/data/my-product/allow"#)); + } + + #[test] + fn test_env_overrides() { + let trino_yaml = r#" + apiVersion: trino.stackable.tech/v1alpha1 + kind: TrinoCluster + metadata: + name: trino + namespace: default + uid: "42" + spec: + image: + productVersion: "479" + clusterConfig: + catalogLabelSelector: + matchLabels: + trino: simple-trino + coordinators: + envOverrides: + COMMON_VAR: role-value # overridden by role group below + ROLE_VAR: role-value # only defined here at role level + roleGroups: + default: + envOverrides: + COMMON_VAR: group-value # overrides role value + GROUP_VAR: group-value # only defined here at group level + replicas: 1 + workers: + roleGroups: + default: + replicas: 1 + "#; + let deserializer = serde_yaml::Deserializer::from_str(trino_yaml); + let trino: v1alpha1::TrinoCluster = + serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); + + let derefs = DereferencedObjects { + resolved_authentication_classes: Vec::new(), + catalog_definitions: Vec::new(), + catalogs: Vec::new(), + trino_opa_config: None, + resolved_fte_config: None, + resolved_client_protocol_config: None, + }; + let operator_env = OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_string(), + operator_service_name: "trino-operator".to_string(), + image_repository: "oci.example.org".to_string(), + }; + let validated = validate::validate(&trino, &derefs, &operator_env) + .expect("validate should succeed"); + + let env = &validated.role_group_configs[&TrinoRole::Coordinator]["default"].env_overrides; + assert_eq!(env.get("COMMON_VAR").map(String::as_str), Some("group-value")); + assert_eq!(env.get("GROUP_VAR").map(String::as_str), Some("group-value")); + assert_eq!(env.get("ROLE_VAR").map(String::as_str), Some("role-value")); + } +} From 0d8cdb40d855ba12313e26400d62b21720ec1586 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Wed, 27 May 2026 22:15:29 +0200 Subject: [PATCH 21/27] chore(docker): remove deleted properties.yaml COPY The `deploy/config-spec/properties.yaml` file was deleted in commit 1854eba; this leftover Dockerfile COPY would have caused docker build to fail with "file not found". Co-Authored-By: Claude Opus 4.7 (1M context) --- docker/Dockerfile | 2 -- 1 file changed, 2 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 118e1812..be45816b 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -191,8 +191,6 @@ COPY LICENSE /licenses/LICENSE COPY --from=builder --chown=${STACKABLE_USER_UID}:0 /app/* /usr/local/bin/ -COPY deploy/config-spec/properties.yaml /etc/stackable/trino-operator/config-spec/properties.yaml - USER ${STACKABLE_USER_UID} ENTRYPOINT ["stackable-trino-operator"] From 5a34a3648d5b33bfdcf8f274fb15397a41943121 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 28 May 2026 08:34:13 +0200 Subject: [PATCH 22/27] docs: remove product config cli --- .../pages/reference/commandline-parameters.adoc | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/docs/modules/trino/pages/reference/commandline-parameters.adoc b/docs/modules/trino/pages/reference/commandline-parameters.adoc index f4f4dd36..4e27f328 100644 --- a/docs/modules/trino/pages/reference/commandline-parameters.adoc +++ b/docs/modules/trino/pages/reference/commandline-parameters.adoc @@ -2,19 +2,6 @@ This operator accepts the following command line parameters: -== product-config - -*Default value*: `/etc/stackable/trino-operator/config-spec/properties.yaml` - -*Required*: false - -*Multiple values:* false - -[source] ----- -cargo run -- run --product-config /foo/bar/properties.yaml ----- - == watch-namespace *Default value*: All namespaces From eb7e1599e23467aa69be59dc31772f0a86822de5 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 28 May 2026 08:34:47 +0200 Subject: [PATCH 23/27] fix(precommit): run cargo rmt --- rust/operator-binary/src/controller.rs | 56 +++++++++---------- .../properties/access_control_properties.rs | 5 +- .../build/properties/config_properties.rs | 14 +++-- .../properties/exchange_manager_properties.rs | 5 +- .../src/controller/build/properties/mod.rs | 5 +- .../build/properties/node_properties.rs | 5 +- .../build/properties/security_properties.rs | 12 +++- .../properties/spooling_manager_properties.rs | 5 +- .../src/controller/build/properties/writer.rs | 9 +-- .../src/controller/validate.rs | 14 ++--- rust/operator-binary/src/crd/mod.rs | 32 ++++++----- .../src/framework/role_utils.rs | 18 ++++-- rust/operator-binary/src/product_logging.rs | 1 - 13 files changed, 91 insertions(+), 90 deletions(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 28563df8..410d7e35 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1,10 +1,5 @@ //! Ensures that `Pod`s are configured and running for each [`v1alpha1::TrinoCluster`] -use std::{ - collections::BTreeMap, - convert::Infallible, - num::ParseIntError, - sync::Arc, -}; +use std::{collections::BTreeMap, convert::Infallible, num::ParseIntError, sync::Arc}; use const_format::concatcp; use snafu::{OptionExt, ResultExt, Snafu}; @@ -77,9 +72,10 @@ use crate::{ crd::{ APP_NAME, CONFIG_DIR_NAME, Container, ENV_INTERNAL_SECRET, ENV_SPOOLING_SECRET, HTTP_PORT, HTTP_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, - METRICS_PORT_NAME, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, - STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, - STACKABLE_TLS_STORE_PASSWORD, TrinoRole, v1alpha1, + METRICS_PORT_NAME, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, + STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, + STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, + TrinoRole, v1alpha1, }, listener::{ LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc, @@ -106,8 +102,7 @@ pub struct ValidatedCluster { pub name: String, pub namespace: String, pub uid: String, - pub image: - stackable_operator::commons::product_image_selection::ResolvedProductImage, + pub image: stackable_operator::commons::product_image_selection::ResolvedProductImage, pub product_version: u16, // CRD facts absorbed from &TrinoCluster. @@ -491,10 +486,11 @@ pub async fn reconcile_trino( ) .context(FailedToCreateJvmConfigSnafu)?; - let vector_toml = get_vector_toml(&role_group_ref, &merged_config.logging) - .context(InvalidLoggingConfigSnafu { + let vector_toml = get_vector_toml(&role_group_ref, &merged_config.logging).context( + InvalidLoggingConfigSnafu { cm_name: role_group_ref.object_name(), - })?; + }, + )?; let rg_configmap = build::config_map::build_rolegroup_config_map( &validated, @@ -630,7 +626,6 @@ pub async fn reconcile_trino( Ok(Action::await_change()) } - /// The rolegroup catalog [`ConfigMap`] configures the rolegroup catalog based on the configuration /// given by the administrator fn build_rolegroup_catalog_config_map( @@ -1424,12 +1419,9 @@ fn tls_volume_mounts( #[cfg(test)] mod tests { use stackable_operator::{ - cli::OperatorEnvironmentOptions, - commons::networking::DomainName, - k8s_openapi::api::core::v1::ConfigMap, - kube::runtime::reflector::ObjectRef, - role_utils::RoleGroupRef, - utils::cluster_info::KubernetesClusterInfo, + cli::OperatorEnvironmentOptions, commons::networking::DomainName, + k8s_openapi::api::core::v1::ConfigMap, kube::runtime::reflector::ObjectRef, + role_utils::RoleGroupRef, utils::cluster_info::KubernetesClusterInfo, }; use super::*; @@ -1514,8 +1506,8 @@ mod tests { image_repository: "oci.example.org".to_string(), }; - let validated = validate::validate(&trino, &derefs, &operator_env) - .expect("validate should succeed"); + let validated = + validate::validate(&trino, &derefs, &operator_env).expect("validate should succeed"); let trino_role = TrinoRole::Coordinator; let role = trino.role(&trino_role).unwrap(); @@ -1541,8 +1533,8 @@ mod tests { ) .expect("jvm config should render"); - let vector_toml = get_vector_toml(&rolegroup_ref, &rg.config.logging) - .expect("vector toml should render"); + let vector_toml = + get_vector_toml(&rolegroup_ref, &rg.config.logging).expect("vector toml should render"); build::config_map::build_rolegroup_config_map( &validated, @@ -1768,12 +1760,18 @@ mod tests { operator_service_name: "trino-operator".to_string(), image_repository: "oci.example.org".to_string(), }; - let validated = validate::validate(&trino, &derefs, &operator_env) - .expect("validate should succeed"); + let validated = + validate::validate(&trino, &derefs, &operator_env).expect("validate should succeed"); let env = &validated.role_group_configs[&TrinoRole::Coordinator]["default"].env_overrides; - assert_eq!(env.get("COMMON_VAR").map(String::as_str), Some("group-value")); - assert_eq!(env.get("GROUP_VAR").map(String::as_str), Some("group-value")); + assert_eq!( + env.get("COMMON_VAR").map(String::as_str), + Some("group-value") + ); + assert_eq!( + env.get("GROUP_VAR").map(String::as_str), + Some("group-value") + ); assert_eq!(env.get("ROLE_VAR").map(String::as_str), Some("role-value")); } } diff --git a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs index c9668288..e25cfc78 100644 --- a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs @@ -9,10 +9,7 @@ use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; /// Returns an empty map when neither OPA authorization is configured nor /// user overrides are provided — callers should omit the file from the /// ConfigMap in that case. -pub fn build( - cluster: &ValidatedCluster, - rg: &TrinoRoleGroupConfig, -) -> BTreeMap { +pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap { let mut props = BTreeMap::new(); // 1. No defaults. diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs index 571188db..692e399c 100644 --- a/rust/operator-binary/src/controller/build/properties/config_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -169,10 +169,7 @@ pub fn build( } // Authentication properties (only contributes when authentication is enabled). - for (k, v) in cluster - .trino_authentication_config - .config_properties(&role) - { + for (k, v) in cluster.trino_authentication_config.config_properties(&role) { props.insert(k, v); } @@ -242,9 +239,14 @@ mod tests { let props = build(&cluster, TrinoRole::Coordinator, &rg, &cluster_info).unwrap(); assert_eq!(props.get("coordinator").map(String::as_str), Some("true")); assert_eq!( - props.get("node-scheduler.include-coordinator").map(String::as_str), + props + .get("node-scheduler.include-coordinator") + .map(String::as_str), Some("false"), ); - assert_eq!(props.get("query.max-memory").map(String::as_str), Some("50GB")); + assert_eq!( + props.get("query.max-memory").map(String::as_str), + Some("50GB") + ); } } diff --git a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs index 840a4514..31904bfa 100644 --- a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs @@ -9,10 +9,7 @@ use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; /// Returns an empty map when fault-tolerant execution is not configured and no /// user overrides are provided — callers should omit the file from the /// ConfigMap in that case. -pub fn build( - cluster: &ValidatedCluster, - rg: &TrinoRoleGroupConfig, -) -> BTreeMap { +pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap { let mut props = BTreeMap::new(); // 1. No defaults. diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 3a857cb9..7fc12e15 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -7,10 +7,10 @@ pub mod access_control_properties; pub mod config_properties; pub mod exchange_manager_properties; -pub mod spooling_manager_properties; pub mod log_properties; pub mod node_properties; pub mod security_properties; +pub mod spooling_manager_properties; pub mod writer; #[cfg(test)] @@ -20,8 +20,7 @@ pub(crate) mod test_support { use stackable_operator::cli::OperatorEnvironmentOptions; pub fn validated_cluster_from_yaml(yaml: &str) -> ValidatedCluster { - let trino: v1alpha1::TrinoCluster = - serde_yaml::from_str(yaml).expect("invalid test YAML"); + let trino: v1alpha1::TrinoCluster = serde_yaml::from_str(yaml).expect("invalid test YAML"); let derefs = DereferencedObjects { resolved_authentication_classes: Vec::new(), catalog_definitions: Vec::new(), diff --git a/rust/operator-binary/src/controller/build/properties/node_properties.rs b/rust/operator-binary/src/controller/build/properties/node_properties.rs index 7ad65ed8..5fbcecd5 100644 --- a/rust/operator-binary/src/controller/build/properties/node_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/node_properties.rs @@ -11,10 +11,7 @@ const NODE_ENVIRONMENT: &str = "node.environment"; /// `node.environment` is derived from the cluster name: lowercased, with `-` /// replaced by `_`. Trino requires `^[a-z][a-z0-9_]*[a-z0-9]$`; cluster names /// constrained by Kubernetes naming already satisfy this after the transform. -pub fn build( - cluster: &ValidatedCluster, - rg: &TrinoRoleGroupConfig, -) -> BTreeMap { +pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap { let mut props = BTreeMap::new(); // 1. No defaults. diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index c4fda4ef..0409fc22 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -49,7 +49,15 @@ mod tests { let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML); let rg = cluster.role_group_configs[&TrinoRole::Coordinator]["default"].clone(); let props = build(&rg); - assert_eq!(props.get("networkaddress.cache.ttl").map(String::as_str), Some("30")); - assert_eq!(props.get("networkaddress.cache.negative.ttl").map(String::as_str), Some("0")); + assert_eq!( + props.get("networkaddress.cache.ttl").map(String::as_str), + Some("30") + ); + assert_eq!( + props + .get("networkaddress.cache.negative.ttl") + .map(String::as_str), + Some("0") + ); } } diff --git a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs index dc524866..fd37646e 100644 --- a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs @@ -9,10 +9,7 @@ use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; /// Returns an empty map when client spooling is not configured and no user /// overrides are provided — callers should omit the file from the ConfigMap /// in that case. -pub fn build( - cluster: &ValidatedCluster, - rg: &TrinoRoleGroupConfig, -) -> BTreeMap { +pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap { let mut props = BTreeMap::new(); // 1. No defaults. diff --git a/rust/operator-binary/src/controller/build/properties/writer.rs b/rust/operator-binary/src/controller/build/properties/writer.rs index 99e60999..5adc7335 100644 --- a/rust/operator-binary/src/controller/build/properties/writer.rs +++ b/rust/operator-binary/src/controller/build/properties/writer.rs @@ -18,9 +18,7 @@ pub enum Error { /// /// Keys and values are escaped per : /// `:`, `=`, `#`, `!`, `\\`, leading whitespace, and ` ` (space). -pub fn to_java_properties_string( - props: &BTreeMap, -) -> Result { +pub fn to_java_properties_string(props: &BTreeMap) -> Result { use std::fmt::Write; let mut out = String::new(); for (k, v) in props { @@ -93,7 +91,10 @@ mod tests { // From smoke snapshot: // internal-communication.shared-secret=${ENV\:INTERNAL_SECRET} assert_eq!( - render(&[("internal-communication.shared-secret", "${ENV:INTERNAL_SECRET}")]), + render(&[( + "internal-communication.shared-secret", + "${ENV:INTERNAL_SECRET}" + )]), "internal-communication.shared-secret=${ENV\\:INTERNAL_SECRET}\n" ); } diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 071804a4..c87113ea 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -4,9 +4,7 @@ //! [`super::ValidatedCluster`], consumed by the rest of `reconcile_trino`. use snafu::{ResultExt, Snafu}; -use stackable_operator::{ - cli::OperatorEnvironmentOptions, commons::product_image_selection, -}; +use stackable_operator::{cli::OperatorEnvironmentOptions, commons::product_image_selection}; use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ @@ -78,11 +76,10 @@ pub fn validate( ) .context(ResolveProductImageSnafu)?; - let product_version = u16::from_str(&resolved_product_image.product_version).context( - ParseTrinoVersionSnafu { + let product_version = + u16::from_str(&resolved_product_image.product_version).context(ParseTrinoVersionSnafu { product_version: resolved_product_image.product_version.clone(), - }, - )?; + })?; let trino_authentication_config = TrinoAuthenticationConfig::new( &resolved_product_image, @@ -213,8 +210,7 @@ mod tests { image_repository: "oci.example.org".to_string(), }; - let validated = - validate(&trino, &derefs, &operator_env).expect("validate should succeed"); + let validated = validate(&trino, &derefs, &operator_env).expect("validate should succeed"); assert_eq!(validated.name, "simple-trino"); assert_eq!(validated.namespace, "default"); diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index b287c8d9..f8b9c3f8 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -1243,23 +1243,27 @@ mod tests { use stackable_operator::config::merge::Merge; let mut rg = v1alpha1::TrinoConfigOverrides { - config_properties: Some(stackable_operator::config_overrides::KeyValueConfigOverrides { - overrides: [ - ("k_both".to_string(), "rg".to_string()), - ("k_rg_only".to_string(), "rg".to_string()), - ] - .into(), - }), + config_properties: Some( + stackable_operator::config_overrides::KeyValueConfigOverrides { + overrides: [ + ("k_both".to_string(), "rg".to_string()), + ("k_rg_only".to_string(), "rg".to_string()), + ] + .into(), + }, + ), ..Default::default() }; let role = v1alpha1::TrinoConfigOverrides { - config_properties: Some(stackable_operator::config_overrides::KeyValueConfigOverrides { - overrides: [ - ("k_both".to_string(), "role".to_string()), - ("k_role_only".to_string(), "role".to_string()), - ] - .into(), - }), + config_properties: Some( + stackable_operator::config_overrides::KeyValueConfigOverrides { + overrides: [ + ("k_both".to_string(), "role".to_string()), + ("k_role_only".to_string(), "role".to_string()), + ] + .into(), + }, + ), ..Default::default() }; diff --git a/rust/operator-binary/src/framework/role_utils.rs b/rust/operator-binary/src/framework/role_utils.rs index 1fdc2c7e..2e5729a0 100644 --- a/rust/operator-binary/src/framework/role_utils.rs +++ b/rust/operator-binary/src/framework/role_utils.rs @@ -79,8 +79,17 @@ where role_group.config.config_overrides.clone(), ), env_overrides: merged_env_overrides( - role.config.env_overrides.iter().map(|(k, v)| (k.clone(), v.clone())).collect(), - role_group.config.env_overrides.iter().map(|(k, v)| (k.clone(), v.clone())).collect(), + role.config + .env_overrides + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(), + role_group + .config + .env_overrides + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect(), ), cli_overrides: merged_cli_overrides( role.config.cli_overrides.clone(), @@ -90,10 +99,7 @@ where role.config.pod_overrides.clone(), role_group.config.pod_overrides.clone(), ), - product_specific_common_config: role_group - .config - .product_specific_common_config - .clone(), + product_specific_common_config: role_group.config.product_specific_common_config.clone(), }) } diff --git a/rust/operator-binary/src/product_logging.rs b/rust/operator-binary/src/product_logging.rs index 7b09ddcc..19766066 100644 --- a/rust/operator-binary/src/product_logging.rs +++ b/rust/operator-binary/src/product_logging.rs @@ -103,4 +103,3 @@ pub fn get_vector_toml( Ok(None) } } - From 6b70d5cb774b87bd02e0e7063c6af3c8c728e68e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 28 May 2026 08:38:48 +0200 Subject: [PATCH 24/27] fix(precommit): run cargo clippy --- rust/operator-binary/src/controller/build/properties/writer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/controller/build/properties/writer.rs b/rust/operator-binary/src/controller/build/properties/writer.rs index 5adc7335..e721552a 100644 --- a/rust/operator-binary/src/controller/build/properties/writer.rs +++ b/rust/operator-binary/src/controller/build/properties/writer.rs @@ -22,7 +22,7 @@ pub fn to_java_properties_string(props: &BTreeMap) -> Result Date: Thu, 28 May 2026 08:42:40 +0200 Subject: [PATCH 25/27] fix(precommit): run cargo fmt --- .../src/controller/build/config_map.rs | 24 +++++++------- .../properties/access_control_properties.rs | 8 +++-- .../build/properties/config_properties.rs | 33 ++++++++++--------- .../properties/exchange_manager_properties.rs | 8 +++-- .../build/properties/log_properties.rs | 6 ++-- .../src/controller/build/properties/mod.rs | 7 ++-- .../build/properties/security_properties.rs | 8 +++-- .../properties/spooling_manager_properties.rs | 8 +++-- .../src/operations/graceful_shutdown.rs | 10 +++--- 9 files changed, 65 insertions(+), 47 deletions(-) diff --git a/rust/operator-binary/src/controller/build/config_map.rs b/rust/operator-binary/src/controller/build/config_map.rs index 28973078..f70b070c 100644 --- a/rust/operator-binary/src/controller/build/config_map.rs +++ b/rust/operator-binary/src/controller/build/config_map.rs @@ -12,18 +12,20 @@ use stackable_operator::{ utils::cluster_info::KubernetesClusterInfo, }; -use crate::controller::{ - ValidatedCluster, - build::properties::{ - access_control_properties, config_properties, exchange_manager_properties, log_properties, - node_properties, security_properties, spooling_manager_properties, - writer::to_java_properties_string, +use crate::{ + controller::{ + ValidatedCluster, + build::properties::{ + access_control_properties, config_properties, exchange_manager_properties, + log_properties, node_properties, security_properties, spooling_manager_properties, + writer::to_java_properties_string, + }, + }, + crd::{ + ACCESS_CONTROL_PROPERTIES, CONFIG_PROPERTIES, EXCHANGE_MANAGER_PROPERTIES, JVM_CONFIG, + JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, NODE_PROPERTIES, SPOOLING_MANAGER_PROPERTIES, + TrinoRole, v1alpha1, }, -}; -use crate::crd::{ - ACCESS_CONTROL_PROPERTIES, CONFIG_PROPERTIES, EXCHANGE_MANAGER_PROPERTIES, JVM_CONFIG, - JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, NODE_PROPERTIES, SPOOLING_MANAGER_PROPERTIES, - TrinoRole, v1alpha1, }; #[derive(Debug, Snafu)] diff --git a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs index e25cfc78..c27332f8 100644 --- a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs @@ -36,10 +36,12 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< #[cfg(test)] mod tests { use super::*; - use crate::controller::build::properties::test_support::{ - MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + use crate::{ + controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }, + crd::TrinoRole, }; - use crate::crd::TrinoRole; #[test] fn default_renders_empty_when_no_opa() { diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs index 692e399c..1f61d1ba 100644 --- a/rust/operator-binary/src/controller/build/properties/config_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -1,25 +1,26 @@ //! Builder for `config.properties` — the main Trino server config. -use std::collections::BTreeMap; -use std::ops::Div; +use std::{collections::BTreeMap, ops::Div}; use snafu::Snafu; use stackable_operator::{memory::BinaryMultiple, utils::cluster_info::KubernetesClusterInfo}; -use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; -use crate::crd::{ - COORDINATOR, Container, DISCOVERY_URI, ENV_INTERNAL_SECRET, HTTP_PORT, - HTTP_SERVER_AUTHENTICATION_ALLOW_INSECURE_OVER_HTTP, HTTP_SERVER_HTTP_PORT, - HTTP_SERVER_HTTPS_ENABLED, HTTP_SERVER_HTTPS_KEYSTORE_KEY, HTTP_SERVER_HTTPS_PORT, - HTTP_SERVER_HTTPS_TRUSTSTORE_KEY, HTTP_SERVER_KEYSTORE_PATH, HTTP_SERVER_TRUSTSTORE_PATH, - HTTPS_PORT, INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_KEY, - INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_PATH, INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_KEY, - INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_PATH, INTERNAL_COMMUNICATION_SHARED_SECRET, - LOG_COMPRESSION, LOG_FORMAT, LOG_MAX_SIZE, LOG_MAX_TOTAL_SIZE, LOG_PATH, - MAX_TRINO_LOG_FILES_SIZE, NODE_INTERNAL_ADDRESS_SOURCE, NODE_INTERNAL_ADDRESS_SOURCE_FQDN, - QUERY_MAX_MEMORY, QUERY_MAX_MEMORY_PER_NODE, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_LOG_DIR, - STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, TrinoRole, - discovery::{TrinoDiscovery, TrinoDiscoveryProtocol}, +use crate::{ + controller::{TrinoRoleGroupConfig, ValidatedCluster}, + crd::{ + COORDINATOR, Container, DISCOVERY_URI, ENV_INTERNAL_SECRET, HTTP_PORT, + HTTP_SERVER_AUTHENTICATION_ALLOW_INSECURE_OVER_HTTP, HTTP_SERVER_HTTP_PORT, + HTTP_SERVER_HTTPS_ENABLED, HTTP_SERVER_HTTPS_KEYSTORE_KEY, HTTP_SERVER_HTTPS_PORT, + HTTP_SERVER_HTTPS_TRUSTSTORE_KEY, HTTP_SERVER_KEYSTORE_PATH, HTTP_SERVER_TRUSTSTORE_PATH, + HTTPS_PORT, INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_KEY, + INTERNAL_COMMUNICATION_HTTPS_KEYSTORE_PATH, INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_KEY, + INTERNAL_COMMUNICATION_HTTPS_TRUSTSTORE_PATH, INTERNAL_COMMUNICATION_SHARED_SECRET, + LOG_COMPRESSION, LOG_FORMAT, LOG_MAX_SIZE, LOG_MAX_TOTAL_SIZE, LOG_PATH, + MAX_TRINO_LOG_FILES_SIZE, NODE_INTERNAL_ADDRESS_SOURCE, NODE_INTERNAL_ADDRESS_SOURCE_FQDN, + QUERY_MAX_MEMORY, QUERY_MAX_MEMORY_PER_NODE, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_LOG_DIR, + STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, TrinoRole, + discovery::{TrinoDiscovery, TrinoDiscoveryProtocol}, + }, }; // Property names not exported from crd/mod.rs. diff --git a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs index 31904bfa..7490c22b 100644 --- a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs @@ -30,10 +30,12 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< #[cfg(test)] mod tests { use super::*; - use crate::controller::build::properties::test_support::{ - MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + use crate::{ + controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }, + crd::TrinoRole, }; - use crate::crd::TrinoRole; #[test] fn default_renders_empty_when_no_fte() { diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs index a5d80021..e4b35b87 100644 --- a/rust/operator-binary/src/controller/build/properties/log_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -2,8 +2,10 @@ use std::collections::BTreeMap; -use crate::controller::{TrinoRoleGroupConfig, ValidatedCluster}; -use crate::crd::TrinoRole; +use crate::{ + controller::{TrinoRoleGroupConfig, ValidatedCluster}, + crd::TrinoRole, +}; /// Build the `log.properties` key/value pairs for `(role, rg)`. /// diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs index 7fc12e15..5af8636c 100644 --- a/rust/operator-binary/src/controller/build/properties/mod.rs +++ b/rust/operator-binary/src/controller/build/properties/mod.rs @@ -15,10 +15,13 @@ pub mod writer; #[cfg(test)] pub(crate) mod test_support { - use crate::controller::{ValidatedCluster, dereference::DereferencedObjects}; - use crate::crd::v1alpha1; use stackable_operator::cli::OperatorEnvironmentOptions; + use crate::{ + controller::{ValidatedCluster, dereference::DereferencedObjects}, + crd::v1alpha1, + }; + pub fn validated_cluster_from_yaml(yaml: &str) -> ValidatedCluster { let trino: v1alpha1::TrinoCluster = serde_yaml::from_str(yaml).expect("invalid test YAML"); let derefs = DereferencedObjects { diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index 0409fc22..402dd2a9 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -39,10 +39,12 @@ pub fn build(rg: &TrinoRoleGroupConfig) -> BTreeMap { #[cfg(test)] mod tests { use super::*; - use crate::controller::build::properties::test_support::{ - MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + use crate::{ + controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }, + crd::TrinoRole, }; - use crate::crd::TrinoRole; #[test] fn default_renders_networkaddress_cache_settings() { diff --git a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs index fd37646e..33983f3b 100644 --- a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs @@ -30,10 +30,12 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< #[cfg(test)] mod tests { use super::*; - use crate::controller::build::properties::test_support::{ - MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + use crate::{ + controller::build::properties::test_support::{ + MINIMAL_TRINO_YAML, validated_cluster_from_yaml, + }, + crd::TrinoRole, }; - use crate::crd::TrinoRole; #[test] fn default_renders_empty_when_no_spooling() { diff --git a/rust/operator-binary/src/operations/graceful_shutdown.rs b/rust/operator-binary/src/operations/graceful_shutdown.rs index b1cca5ec..f8e5151b 100644 --- a/rust/operator-binary/src/operations/graceful_shutdown.rs +++ b/rust/operator-binary/src/operations/graceful_shutdown.rs @@ -9,10 +9,12 @@ use stackable_operator::{ k8s_openapi::api::core::v1::{ExecAction, LifecycleHandler}, }; -use crate::controller::ValidatedCluster; -use crate::crd::{ - DEFAULT_WORKER_GRACEFUL_SHUTDOWN_TIMEOUT, TrinoRole, WORKER_GRACEFUL_SHUTDOWN_SAFETY_OVERHEAD, - WORKER_SHUTDOWN_GRACE_PERIOD, v1alpha1, +use crate::{ + controller::ValidatedCluster, + crd::{ + DEFAULT_WORKER_GRACEFUL_SHUTDOWN_TIMEOUT, TrinoRole, + WORKER_GRACEFUL_SHUTDOWN_SAFETY_OVERHEAD, WORKER_SHUTDOWN_GRACE_PERIOD, v1alpha1, + }, }; #[derive(Debug, Snafu)] From 51c83c4115118890d8179eea7bc031aa0746ab8e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 28 May 2026 11:30:32 +0200 Subject: [PATCH 26/27] fix(nix): regenerate hashes --- Cargo.nix | 18 +++++++++--------- crate-hashes.json | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Cargo.nix b/Cargo.nix index 883c3ebb..6907eb84 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -4863,7 +4863,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "k8s_version"; authors = [ @@ -9492,7 +9492,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "stackable_certs"; authors = [ @@ -9595,7 +9595,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "stackable_operator"; authors = [ @@ -9775,7 +9775,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -9810,7 +9810,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "stackable_shared"; authors = [ @@ -9891,7 +9891,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "stackable_telemetry"; authors = [ @@ -10098,7 +10098,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "stackable_versioned"; authors = [ @@ -10148,7 +10148,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; procMacro = true; libName = "stackable_versioned_macros"; @@ -10216,7 +10216,7 @@ rec { src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc"; + sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; }; libName = "stackable_webhook"; authors = [ diff --git a/crate-hashes.json b/crate-hashes.json index 4e2f0d70..79c21be4 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,12 +1,12 @@ { - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#k8s-version@0.1.3": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-certs@0.4.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator-derive@0.3.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator@0.111.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-shared@0.1.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-telemetry@0.6.3": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned-macros@0.10.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned@0.10.0": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-webhook@0.9.1": "14q10sppdjdf3vbcbxz12rlgm1g9l6p87nk9wr707w2a71z8vgxc", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#k8s-version@0.1.3": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-certs@0.4.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator-derive@0.3.1": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator@0.111.1": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-shared@0.1.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-telemetry@0.6.3": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned-macros@0.10.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned@0.10.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-webhook@0.9.1": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", "git+https://github.com/stackabletech/product-config.git?tag=0.8.0#product-config@0.8.0": "1dz70kapm2wdqcr7ndyjji0lhsl98bsq95gnb2lw487wf6yr7987" } \ No newline at end of file From 546fab3a4f8957feaac74b970c97c8cc43612af1 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 28 May 2026 15:01:11 +0200 Subject: [PATCH 27/27] fix(config_overrides): use operator-rs v2 mergable overrides --- Cargo.lock | 19 +-- Cargo.nix | 48 ++++--- crate-hashes.json | 18 +-- extra/crds.yaml | 56 ++++---- .../properties/access_control_properties.rs | 10 +- .../build/properties/config_properties.rs | 10 +- .../properties/exchange_manager_properties.rs | 10 +- .../build/properties/log_properties.rs | 10 +- .../build/properties/node_properties.rs | 10 +- .../build/properties/security_properties.rs | 10 +- .../properties/spooling_manager_properties.rs | 10 +- rust/operator-binary/src/crd/mod.rs | 122 ++++++------------ 12 files changed, 167 insertions(+), 166 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cfc7e2c3..34291786 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1480,6 +1480,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f300e415e2134745ef75f04562dd0145405c2f7fd92065db029ac4b16b57fe90" dependencies = [ "jsonptr", + "schemars", "serde", "serde_json", "thiserror 1.0.69", @@ -1524,7 +1525,7 @@ dependencies = [ [[package]] name = "k8s-version" version = "0.1.3" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "darling", "regex", @@ -2881,7 +2882,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stackable-certs" version = "0.4.0" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "const-oid", "ecdsa", @@ -2905,7 +2906,7 @@ dependencies = [ [[package]] name = "stackable-operator" version = "0.111.1" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "base64", "clap", @@ -2946,7 +2947,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "darling", "proc-macro2", @@ -2957,7 +2958,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.1.0" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "jiff", "k8s-openapi", @@ -2974,7 +2975,7 @@ dependencies = [ [[package]] name = "stackable-telemetry" version = "0.6.3" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "axum", "clap", @@ -3020,7 +3021,7 @@ dependencies = [ [[package]] name = "stackable-versioned" version = "0.10.0" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "kube", "schemars", @@ -3034,7 +3035,7 @@ dependencies = [ [[package]] name = "stackable-versioned-macros" version = "0.10.0" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "convert_case", "convert_case_extras", @@ -3052,7 +3053,7 @@ dependencies = [ [[package]] name = "stackable-webhook" version = "0.9.1" -source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#713ca9c66c0f68bb93f0fac52338f530fd40c65f" +source = "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#2563bd275666339d96d3e99bd5e9d56aaf0a6e98" dependencies = [ "arc-swap", "async-trait", diff --git a/Cargo.nix b/Cargo.nix index 6907eb84..9d71cc6b 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -4709,6 +4709,11 @@ rec { name = "jsonptr"; packageId = "jsonptr"; } + { + name = "schemars"; + packageId = "schemars"; + optional = true; + } { name = "serde"; packageId = "serde"; @@ -4724,6 +4729,10 @@ rec { } ]; devDependencies = [ + { + name = "schemars"; + packageId = "schemars"; + } { name = "serde_json"; packageId = "serde_json"; @@ -4735,7 +4744,7 @@ rec { "schemars" = [ "dep:schemars" ]; "utoipa" = [ "dep:utoipa" ]; }; - resolvedDefaultFeatures = [ "default" "diff" ]; + resolvedDefaultFeatures = [ "default" "diff" "schemars" ]; }; "jsonpath-rust" = rec { crateName = "jsonpath-rust"; @@ -4862,8 +4871,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "k8s_version"; authors = [ @@ -9491,8 +9500,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "stackable_certs"; authors = [ @@ -9594,8 +9603,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "stackable_operator"; authors = [ @@ -9652,6 +9661,7 @@ rec { { name = "json-patch"; packageId = "json-patch"; + features = [ "schemars" ]; } { name = "k8s-openapi"; @@ -9774,8 +9784,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -9809,8 +9819,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "stackable_shared"; authors = [ @@ -9890,8 +9900,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "stackable_telemetry"; authors = [ @@ -10097,8 +10107,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "stackable_versioned"; authors = [ @@ -10147,8 +10157,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; procMacro = true; libName = "stackable_versioned_macros"; @@ -10215,8 +10225,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "713ca9c66c0f68bb93f0fac52338f530fd40c65f"; - sha256 = "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk"; + rev = "2563bd275666339d96d3e99bd5e9d56aaf0a6e98"; + sha256 = "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb"; }; libName = "stackable_webhook"; authors = [ diff --git a/crate-hashes.json b/crate-hashes.json index 79c21be4..c729fe68 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,12 +1,12 @@ { - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#k8s-version@0.1.3": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-certs@0.4.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator-derive@0.3.1": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator@0.111.1": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-shared@0.1.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-telemetry@0.6.3": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned-macros@0.10.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned@0.10.0": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", - "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-webhook@0.9.1": "097gfpqjmhzv3db714a2s2b00ipyc88wqjrlyja6401nnd7rmpdk", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#k8s-version@0.1.3": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-certs@0.4.0": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator-derive@0.3.1": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-operator@0.111.1": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-shared@0.1.0": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-telemetry@0.6.3": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned-macros@0.10.0": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-versioned@0.10.0": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", + "git+https://github.com/stackabletech/operator-rs.git?branch=smooth-operator#stackable-webhook@0.9.1": "074bm6x3qdf1px9rnpv42wpm4s1h2i08hw1jn7nrf71b0yvsbxlb", "git+https://github.com/stackabletech/product-config.git?tag=0.8.0#product-config@0.8.0": "1dz70kapm2wdqcr7ndyjji0lhsl98bsq95gnb2lw487wf6yr7987" } \ No newline at end of file diff --git a/extra/crds.yaml b/extra/crds.yaml index 59515338..cd43dbca 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -1419,73 +1419,73 @@ spec: properties: access-control.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object config.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object exchange-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object log.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object node.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object security.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object spooling-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object type: object envOverrides: @@ -2046,73 +2046,73 @@ spec: properties: access-control.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object config.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object exchange-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object log.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object node.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object security.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object spooling-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object type: object envOverrides: @@ -2725,73 +2725,73 @@ spec: properties: access-control.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object config.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object exchange-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object log.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object node.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object security.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object spooling-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object type: object envOverrides: @@ -3347,73 +3347,73 @@ spec: properties: access-control.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object config.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object exchange-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object log.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object node.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object security.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object spooling-manager.properties: additionalProperties: + nullable: true type: string description: |- Flat key-value overrides for `*.properties`, Hadoop XML, etc. This is backwards-compatible with the existing flat key-value YAML format used by `HashMap`. - nullable: true type: object type: object envOverrides: diff --git a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs index c27332f8..4d762ba5 100644 --- a/rust/operator-binary/src/controller/build/properties/access_control_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/access_control_properties.rs @@ -26,9 +26,13 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< // 3. No merged_config contribution. // 4. User overrides (highest precedence). - if let Some(kv) = &rg.config_overrides.access_control_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .access_control_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); props } diff --git a/rust/operator-binary/src/controller/build/properties/config_properties.rs b/rust/operator-binary/src/controller/build/properties/config_properties.rs index 1f61d1ba..0812b828 100644 --- a/rust/operator-binary/src/controller/build/properties/config_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/config_properties.rs @@ -213,9 +213,13 @@ pub fn build( } // ---- 4. User overrides (highest precedence) ---- - if let Some(kv) = &rg.config_overrides.config_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .config_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); Ok(props) } diff --git a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs index 7490c22b..5371ec11 100644 --- a/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/exchange_manager_properties.rs @@ -20,9 +20,13 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< // 3. No merged_config contribution. // 4. User overrides (highest precedence). - if let Some(kv) = &rg.config_overrides.exchange_manager_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .exchange_manager_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); props } diff --git a/rust/operator-binary/src/controller/build/properties/log_properties.rs b/rust/operator-binary/src/controller/build/properties/log_properties.rs index e4b35b87..395abab5 100644 --- a/rust/operator-binary/src/controller/build/properties/log_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/log_properties.rs @@ -28,9 +28,13 @@ pub fn build( // 3. No merged_config contribution. // 4. User overrides (highest precedence). - if let Some(kv) = &rg.config_overrides.log_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .log_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); let _ = (cluster, role); // currently unused; preserved for symmetry with siblings props diff --git a/rust/operator-binary/src/controller/build/properties/node_properties.rs b/rust/operator-binary/src/controller/build/properties/node_properties.rs index 5fbcecd5..b10ed436 100644 --- a/rust/operator-binary/src/controller/build/properties/node_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/node_properties.rs @@ -21,9 +21,13 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< // 3. No merged_config contribution for node.properties. // 4. User overrides (highest precedence). - if let Some(kv) = &rg.config_overrides.node_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .node_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); props } diff --git a/rust/operator-binary/src/controller/build/properties/security_properties.rs b/rust/operator-binary/src/controller/build/properties/security_properties.rs index 402dd2a9..e112af1b 100644 --- a/rust/operator-binary/src/controller/build/properties/security_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/security_properties.rs @@ -29,9 +29,13 @@ pub fn build(rg: &TrinoRoleGroupConfig) -> BTreeMap { // 2. No automatic operator-injected values. // 3. No merged_config contribution. // 4. User overrides (highest precedence). - if let Some(kv) = &rg.config_overrides.security_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .security_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); props } diff --git a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs index 33983f3b..cdf098ce 100644 --- a/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/spooling_manager_properties.rs @@ -20,9 +20,13 @@ pub fn build(cluster: &ValidatedCluster, rg: &TrinoRoleGroupConfig) -> BTreeMap< // 3. No merged_config contribution. // 4. User overrides (highest precedence). - if let Some(kv) = &rg.config_overrides.spooling_manager_properties { - props.extend(kv.overrides.clone()); - } + props.extend( + rg.config_overrides + .spooling_manager_properties + .overrides + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); props } diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index f8b9c3f8..3862357e 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -25,7 +25,6 @@ use stackable_operator::{ fragment::{self, Fragment, ValidationError}, merge::Merge, }, - config_overrides::KeyValueConfigOverrides, crd::authentication::core, deep_merger::ObjectOverrides, k8s_openapi::apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::LabelSelector}, @@ -39,6 +38,7 @@ use stackable_operator::{ shared::time::Duration, status::condition::{ClusterCondition, HasStatusCondition}, utils::cluster_info::KubernetesClusterInfo, + v2::config_overrides::KeyValueConfigOverrides, versioned::versioned, }; use strum::{Display, EnumIter, EnumString, IntoEnumIterator}; @@ -233,57 +233,57 @@ pub mod versioned { pub workers: Option, } - #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] + #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, Merge, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct TrinoConfigOverrides { #[serde( default, rename = "config.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub config_properties: Option, + pub config_properties: KeyValueConfigOverrides, #[serde( default, rename = "node.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub node_properties: Option, + pub node_properties: KeyValueConfigOverrides, #[serde( default, rename = "log.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub log_properties: Option, + pub log_properties: KeyValueConfigOverrides, #[serde( default, rename = "security.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub security_properties: Option, + pub security_properties: KeyValueConfigOverrides, #[serde( default, rename = "access-control.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub access_control_properties: Option, + pub access_control_properties: KeyValueConfigOverrides, #[serde( default, rename = "exchange-manager.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub exchange_manager_properties: Option, + pub exchange_manager_properties: KeyValueConfigOverrides, #[serde( default, rename = "spooling-manager.properties", - skip_serializing_if = "Option::is_none" + skip_serializing_if = "super::key_value_overrides_is_empty" )] - pub spooling_manager_properties: Option, + pub spooling_manager_properties: KeyValueConfigOverrides, } // TODO: move generic version to op-rs? @@ -456,48 +456,8 @@ impl v1alpha1::TrinoAuthorizationOpaConfig { } } -// TODO: Remove once `KeyValueConfigOverrides` derives `Merge` upstream. -// `stackable_operator::v2::role_utils::with_validated_config` requires -// `ConfigOverrides: Merge`. Until upstreamed, merge field-by-field: -// the right-hand (role-group level) override wins per file when both sides -// are Some; otherwise the side that is Some survives. -impl Merge for v1alpha1::TrinoConfigOverrides { - fn merge(&mut self, defaults: &Self) { - merge_key_value(&mut self.config_properties, &defaults.config_properties); - merge_key_value(&mut self.node_properties, &defaults.node_properties); - merge_key_value(&mut self.log_properties, &defaults.log_properties); - merge_key_value(&mut self.security_properties, &defaults.security_properties); - merge_key_value( - &mut self.access_control_properties, - &defaults.access_control_properties, - ); - merge_key_value( - &mut self.exchange_manager_properties, - &defaults.exchange_manager_properties, - ); - merge_key_value( - &mut self.spooling_manager_properties, - &defaults.spooling_manager_properties, - ); - } -} - -fn merge_key_value( - rg: &mut Option, - role: &Option, -) { - match (rg.as_mut(), role) { - (Some(rg_overrides), Some(role_overrides)) => { - // role-group keys win over role keys. - let mut merged = role_overrides.overrides.clone(); - merged.extend(std::mem::take(&mut rg_overrides.overrides)); - rg_overrides.overrides = merged; - } - (None, Some(role_overrides)) => { - *rg = Some(role_overrides.clone()); - } - _ => {} // None defaults: nothing to merge in. - } +fn key_value_overrides_is_empty(kv: &KeyValueConfigOverrides) -> bool { + kv.overrides.is_empty() } impl Default for v1alpha1::TrinoCoordinatorRoleConfig { @@ -1243,35 +1203,37 @@ mod tests { use stackable_operator::config::merge::Merge; let mut rg = v1alpha1::TrinoConfigOverrides { - config_properties: Some( - stackable_operator::config_overrides::KeyValueConfigOverrides { - overrides: [ - ("k_both".to_string(), "rg".to_string()), - ("k_rg_only".to_string(), "rg".to_string()), - ] - .into(), - }, - ), + config_properties: KeyValueConfigOverrides { + overrides: [ + ("k_both".to_string(), Some("rg".to_string())), + ("k_rg_only".to_string(), Some("rg".to_string())), + ] + .into(), + }, ..Default::default() }; let role = v1alpha1::TrinoConfigOverrides { - config_properties: Some( - stackable_operator::config_overrides::KeyValueConfigOverrides { - overrides: [ - ("k_both".to_string(), "role".to_string()), - ("k_role_only".to_string(), "role".to_string()), - ] - .into(), - }, - ), + config_properties: KeyValueConfigOverrides { + overrides: [ + ("k_both".to_string(), Some("role".to_string())), + ("k_role_only".to_string(), Some("role".to_string())), + ] + .into(), + }, ..Default::default() }; rg.merge(&role); - let merged = rg.config_properties.unwrap().overrides; - assert_eq!(merged.get("k_both").map(String::as_str), Some("rg")); - assert_eq!(merged.get("k_rg_only").map(String::as_str), Some("rg")); - assert_eq!(merged.get("k_role_only").map(String::as_str), Some("role")); + let merged = rg.config_properties.overrides; + assert_eq!(merged.get("k_both").and_then(|v| v.as_deref()), Some("rg")); + assert_eq!( + merged.get("k_rg_only").and_then(|v| v.as_deref()), + Some("rg") + ); + assert_eq!( + merged.get("k_role_only").and_then(|v| v.as_deref()), + Some("role") + ); } }