diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 6cd5f7448..21e698498 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -12,10 +12,17 @@ All notable changes to this project will be documented in this file. ### Changed - BREAKING: [v2] Improve functions for recommended labels in `v2::kvp::label` ([#1261]). +- BREAKING: [v2] `env_overrides` in `v2::role_utils::CommonConfiguration` is now the new + `v2::env_overrides::EnvOverrides` type (a `BTreeMap`) instead of a + `HashMap`, so environment variable names are validated on deserialization and + kept in a deterministic order ([#1262]). + `v2::role_utils` now defines its own `CommonConfiguration`, `Role` and `RoleGroup` instead of + re-exporting them from `crate::role_utils`. [#1259]: https://github.com/stackabletech/operator-rs/pull/1259 [#1260]: https://github.com/stackabletech/operator-rs/pull/1260 [#1261]: https://github.com/stackabletech/operator-rs/pull/1261 +[#1262]: https://github.com/stackabletech/operator-rs/pull/1262 ## [0.115.0] - 2026-08-04 diff --git a/crates/stackable-operator/src/v2/env_overrides.rs b/crates/stackable-operator/src/v2/env_overrides.rs new file mode 100644 index 000000000..f9e650b08 --- /dev/null +++ b/crates/stackable-operator/src/v2/env_overrides.rs @@ -0,0 +1,150 @@ +use std::collections::{BTreeMap, btree_map}; + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::v2::builder::pod::container::{EnvVarName, EnvVarSet}; + +/// A map from environment variable names to their values. +/// +/// This is a newtype around `BTreeMap` instead of a bare type alias because a +/// `BTreeMap` keyed by [`EnvVarName`] would generate a JSON schema using `patternProperties` (from +/// the [`EnvVarName`] pattern), which is not supported in CRDs. The custom [`JsonSchema`] +/// implementation therefore exposes the field as a plain `BTreeMap` in the CRD. +/// +/// As a consequence, the Kubernetes API server does not enforce the [`EnvVarName`] pattern: +/// invalid names are accepted on `apply` and only rejected later, when the operator deserializes +/// the resource. +/// +/// This uses a `BTreeMap` rather than an +/// [`EnvVarSet`](crate::v2::builder::pod::container::EnvVarSet), because for overrides only plain +/// values are supported at the moment. An `EnvVarSet` maps each name to a full `EnvVar`, which also +/// allows the other variants (such as `valueFrom`); those are intentionally not exposed here. +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +pub struct EnvOverrides(BTreeMap); + +impl EnvOverrides { + pub fn new() -> Self { + Self(BTreeMap::new()) + } + + pub fn insert(&mut self, env_var_name: EnvVarName, value: String) -> Option { + self.0.insert(env_var_name, value) + } + + pub fn iter(&self) -> btree_map::Iter<'_, EnvVarName, String> { + self.0.iter() + } + + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +impl FromIterator<(EnvVarName, String)> for EnvOverrides { + fn from_iter>(iter: T) -> Self { + Self(BTreeMap::from_iter(iter)) + } +} + +impl<'a> IntoIterator for &'a EnvOverrides { + type IntoIter = btree_map::Iter<'a, EnvVarName, String>; + type Item = (&'a EnvVarName, &'a String); + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl JsonSchema for EnvOverrides { + fn schema_name() -> std::borrow::Cow<'static, str> { + "EnvOverrides".into() + } + + fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::Schema { + BTreeMap::::json_schema(generator) + } +} + +impl IntoIterator for EnvOverrides { + type IntoIter = btree_map::IntoIter; + type Item = (EnvVarName, String); + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl Extend<(EnvVarName, String)> for EnvOverrides { + fn extend>(&mut self, iter: T) { + self.0.extend(iter); + } +} + +impl From for EnvVarSet { + fn from(value: EnvOverrides) -> Self { + Self::new().with_values(value) + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn deserialize_valid_names() { + let overrides: EnvOverrides = serde_json::from_value(json!({ + "FOO": "1", + "BAR": "2" + })) + .expect("should be valid EnvOverrides"); + + assert_eq!( + vec![ + (EnvVarName::from_str_unsafe("BAR"), "2".to_owned()), + (EnvVarName::from_str_unsafe("FOO"), "1".to_owned()) + ], + overrides.into_iter().collect::>() + ); + } + + #[test] + fn deserialize_rejects_invalid_names() { + // "=" is not allowed in environment variable names. + let result: Result = serde_json::from_value(json!({ + "FO=O": "1" + })); + + assert_eq!( + Err( + "no match for the regular expression \"^[ -<>-~]+$\" in the value \"FO=O\"" + .to_owned() + ), + result.map_err(|err| err.to_string()) + ); + } + + #[test] + fn json_schema_is_a_plain_string_map() { + let schema = serde_json::to_value(schemars::schema_for!(EnvOverrides)) + .expect("should produce a valid JSON schema"); + + assert_eq!( + json!({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "EnvOverrides", + "type": "object", + "additionalProperties": { + "type": "string" + } + }), + schema + ); + } +} diff --git a/crates/stackable-operator/src/v2/jvm_argument_overrides.rs b/crates/stackable-operator/src/v2/jvm_argument_overrides.rs index c6f0623b3..821a9d7e9 100644 --- a/crates/stackable-operator/src/v2/jvm_argument_overrides.rs +++ b/crates/stackable-operator/src/v2/jvm_argument_overrides.rs @@ -124,8 +124,8 @@ mod tests { use super::*; use crate::{ - role_utils::{GenericRoleConfig, Role, RoleGroup}, - v2::role_utils::{JavaCommonConfig, with_validated_config}, + role_utils::GenericRoleConfig, + v2::role_utils::{JavaCommonConfig, Role, RoleGroup, with_validated_config}, }; // #[derive( diff --git a/crates/stackable-operator/src/v2/macros/attributed_string_type.rs b/crates/stackable-operator/src/v2/macros/attributed_string_type.rs index e3cfbd689..3f17b5868 100644 --- a/crates/stackable-operator/src/v2/macros/attributed_string_type.rs +++ b/crates/stackable-operator/src/v2/macros/attributed_string_type.rs @@ -19,7 +19,7 @@ pub enum Error { #[snafu(display("invalid regular expression"))] InvalidRegex { source: regex::Error }, - #[snafu(display("regular expression not matched"))] + #[snafu(display("no match for the regular expression {regex:?} in the value {value:?}"))] RegexNotMatched { value: String, regex: &'static str }, #[snafu(display("not a valid label value"))] @@ -706,7 +706,9 @@ mod tests { .map_err(|err| err.to_string()) ); assert_eq!( - Err("regular expression not matched".to_owned()), + Err( + "no match for the regular expression \"^[est-]+$\" in the value \"abc\"".to_owned() + ), serde_json::from_value::(Value::String("abc".to_owned())) .map_err(|err| err.to_string()) ); diff --git a/crates/stackable-operator/src/v2/mod.rs b/crates/stackable-operator/src/v2/mod.rs index 82b2397cf..efc90c1a3 100644 --- a/crates/stackable-operator/src/v2/mod.rs +++ b/crates/stackable-operator/src/v2/mod.rs @@ -5,6 +5,7 @@ pub mod cluster_resources; pub mod config_file_writer; pub mod config_overrides; pub mod controller_utils; +pub mod env_overrides; pub mod flask_config_writer; pub mod jvm_argument_overrides; pub mod kvp; diff --git a/crates/stackable-operator/src/v2/role_utils.rs b/crates/stackable-operator/src/v2/role_utils.rs index 07a69fa8f..fd55eb765 100644 --- a/crates/stackable-operator/src/v2/role_utils.rs +++ b/crates/stackable-operator/src/v2/role_utils.rs @@ -19,15 +19,187 @@ use crate::{ merge::{self, Merge, merge}, }, k8s_openapi::{DeepMerge, api::core::v1::PodTemplateSpec}, - role_utils::{CommonConfiguration, Role, RoleGroup}, + role_utils::GenericRoleConfig, schemars::{self, JsonSchema}, + utils::crds::raw_object_schema, + v2::env_overrides::EnvOverrides, }; +// Variant of [`crate::role_utils::CommonConfiguration`] that uses [`EnvOverrides`] for `env_overrides` +#[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde( + rename_all = "camelCase", + bound( + deserialize = "Config: Default + Deserialize<'de>, CommonConfig: Default + Deserialize<'de>, ConfigOverrides: Default + Deserialize<'de>" + ) +)] +#[schemars( + bound = "Config: JsonSchema, CommonConfig: JsonSchema, ConfigOverrides: Default + JsonSchema" +)] +pub struct CommonConfiguration { + #[serde(default)] + // We can't depend on Config being `Default`, since that trait is not object-safe + // We only need to generate schemas for fully specified types, but schemars_derive + // does not support specifying custom bounds. + #[schemars(default = "Self::default_config")] + pub config: Config, + + /// The `configOverrides` can be used to configure properties in product config files + /// that are not exposed in the CRD. Read the + /// [config overrides documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/overrides#config-overrides) + /// and consult the operator specific usage guide documentation for details on the + /// available config files and settings for the specific product. + #[serde(default)] + pub config_overrides: ConfigOverrides, + + /// `envOverrides` configure environment variables to be set in the Pods. + /// It is a map from environment variable names to their values. The names are validated to be + /// valid environment variable names. + /// Read the + /// [environment variable overrides documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/overrides#env-overrides) + /// for more information and consult the operator specific usage guide to find out about + /// the product specific environment variables that are available. + #[serde(default)] + pub env_overrides: EnvOverrides, + + // BTreeMap to keep some order with the cli arguments. + // TODO add documentation. + #[serde(default)] + pub cli_overrides: BTreeMap, + + /// In the `podOverrides` property you can define a + /// [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#podtemplatespec-v1-core) + /// to override any property that can be set on a Kubernetes Pod. + /// Read the + /// [Pod overrides documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/overrides#pod-overrides) + /// for more information. + #[serde(default)] + #[schemars(schema_with = "raw_object_schema")] + pub pod_overrides: PodTemplateSpec, + + // No docs needed, as we flatten this struct. + // + // This field is product-specific and can contain e.g. jvmArgumentOverrides. + // + // Unlike [`crate::role_utils::CommonConfiguration`] (which needs + // [`crate::role_utils::Role::get_merged_jvm_argument_overrides`]), the role and roleGroup values + // here are merged generically via [`Merge`] in [`with_validated_config`], so read the + // already-merged field off its result instead of merging it yourself. + #[serde(flatten, default)] + pub product_specific_common_config: CommonConfig, +} + +impl + CommonConfiguration +{ + fn default_config() -> serde_json::Value { + serde_json::json!({}) + } +} + // Variant of [`crate::role_utils::GenericCommonConfig`] that implements [`Merge`] #[derive(Clone, Debug, Default, Deserialize, JsonSchema, Eq, Merge, PartialEq, Serialize)] #[merge(path_overrides(merge = "crate::config::merge"))] pub struct GenericCommonConfig {} +// Variant of [`crate::role_utils::Role`] with [`v2::CommonConfiguration`] +/// This struct represents a role - e.g. HDFS datanodes or Trino workers. It has a key-value-map containing +/// all the roleGroups that are part of this role. Additionally, there is a `config`, which is configurable +/// at the role *and* roleGroup level. Everything at roleGroup level is merged on top of what is configured +/// on role level. There is also a second form of config, which can only be configured +/// at role level, the `roleConfig`. +/// You can learn more about this in the +/// [Roles and role group concept documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/roles-and-role-groups). +// +// Everything below is only a "normal" comment, not rustdoc - so we don't bloat the CRD documentation +// with technical (Rust) details. +// +// `Config` here is the `config` shared between role and roleGroup. +// +// `RoleConfig` here is the `roleConfig` only available on the role. It defaults to [`GenericRoleConfig`], which is +// sufficient for most of the products. There are some exceptions, where e.g. [`EmptyRoleConfig`] is used. +// However, product-operators can define their own - custom - struct and use that here. +#[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct Role< + Config, + ConfigOverrides, + RoleConfig = GenericRoleConfig, + CommonConfig = GenericCommonConfig, +> where + // Don't remove this trait bounds!!! + // We don't know why, but if you remove either of them, the generated default value in the CRDs will + // be missing! + RoleConfig: Default + JsonSchema + Serialize, + CommonConfig: Default + JsonSchema + Serialize, + ConfigOverrides: Default + JsonSchema + Serialize, +{ + #[serde( + flatten, + bound( + deserialize = "Config: Default + Deserialize<'de>, CommonConfig: Deserialize<'de>, ConfigOverrides: Deserialize<'de>" + ) + )] + pub config: CommonConfiguration, + + #[serde(default)] + pub role_config: RoleConfig, + + /// The set of role groups for this role, keyed by their name. + /// + /// A role group is a subset of the replicas of a role that share the same configuration, + /// allowing finer-grained control than the role level. This is useful to e.g. schedule groups + /// onto different classes of nodes or into different regions, or to run them with different + /// settings. Configuration set on a role group is merged on top of the role-level `config`, + /// with the more specific role group values taking precedence. + /// + /// Every role needs at least one role group. A role with a single role group conventionally + /// names it `default`. + /// + /// Read the + /// [roles and role groups concept documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/roles-and-role-groups) + /// for more details. + pub role_groups: HashMap>, +} + +// Variant of [`crate::role_utils::RoleGroup`] with [`v2::CommonConfiguration`] +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde( + rename_all = "camelCase", + bound( + deserialize = "Config: Default + Deserialize<'de>, CommonConfig: Default + Deserialize<'de>, ConfigOverrides: Default + Deserialize<'de>" + ) +)] +#[schemars( + bound = "Config: JsonSchema, CommonConfig: JsonSchema, ConfigOverrides: Default + JsonSchema" +)] +pub struct RoleGroup { + #[serde(flatten)] + pub config: CommonConfiguration, + pub replicas: Option, +} + +impl RoleGroup { + pub fn validate_config( + &self, + role: &Role, + default_config: &Config, + ) -> Result + where + C: FromFragment, + Config: Merge + Clone, + RoleConfig: Default + JsonSchema + Serialize, + CommonConfig: Default + JsonSchema + Serialize, + ConfigOverrides: Default + JsonSchema + Serialize, + { + let mut role_config = role.config.config.clone(); + role_config.merge(default_config); + let mut rolegroup_config = self.config.config.clone(); + rolegroup_config.merge(&role_config); + fragment::validate(rolegroup_config) + } +} + // Variant of [`crate::role_utils::JavaCommonConfig`] that implements [`Merge`] #[derive(Clone, Debug, Default, Deserialize, JsonSchema, Merge, PartialEq, Eq, Serialize)] #[merge(path_overrides(merge = "crate::config::merge"))] @@ -45,7 +217,7 @@ pub struct JavaCommonConfig { /// /// Differences are: /// * `config` is flattened. -/// * The [`HashMap`] in `env_overrides` is replaced with an [`EnvVarSet`]. +/// * The [`EnvOverrides`] in `env_overrides` is replaced with an [`EnvVarSet`]. #[derive(Clone, Debug, PartialEq)] pub struct RoleGroupConfig { pub replicas: Option, @@ -120,9 +292,9 @@ where } fn merged_env_overrides( - role_env_overrides: HashMap, - role_group_env_overrides: HashMap, -) -> HashMap { + role_env_overrides: EnvOverrides, + role_group_env_overrides: EnvOverrides, +) -> EnvOverrides { let mut merged_env_overrides = role_env_overrides; merged_env_overrides.extend(role_group_env_overrides); merged_env_overrides @@ -205,19 +377,20 @@ impl ResourceNames { #[cfg(test)] mod tests { - use std::collections::{BTreeMap, HashMap}; + use std::collections::BTreeMap; use rstest::*; use serde::Serialize; - use super::ResourceNames; + use super::*; use crate::{ config::{fragment::Fragment, merge::Merge}, k8s_openapi::api::core::v1::PodTemplateSpec, kube::api::ObjectMeta, - role_utils::{CommonConfiguration, GenericRoleConfig, Role, RoleGroup}, + role_utils::GenericRoleConfig, schemars::{self, JsonSchema}, v2::{ + builder::pod::container::EnvVarName, config_overrides::KeyValueConfigOverrides, role_utils::with_validated_config, types::{ @@ -264,12 +437,12 @@ mod tests { override_value: Option<&str>, ) -> CommonConfiguration { let mut config_file_overrides = BTreeMap::new(); - let mut env_overrides = HashMap::new(); + let mut env_overrides = EnvOverrides::new(); let mut cli_overrides = BTreeMap::new(); if let Some(value) = override_value { config_file_overrides.insert("property".to_owned(), value.to_owned()); - env_overrides.insert("PROPERTY".to_owned(), value.to_owned()); + env_overrides.insert(EnvVarName::from_str_unsafe("PROPERTY"), value.to_owned()); cli_overrides.insert("--property".to_owned(), value.to_owned()); }