diff --git a/Cargo.lock b/Cargo.lock index 1cdc26c2..222e62c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3277,14 +3277,14 @@ dependencies = [ [[package]] name = "stackable-versioned" -version = "0.4.1" +version = "0.5.0" dependencies = [ "stackable-versioned-macros", ] [[package]] name = "stackable-versioned-macros" -version = "0.4.1" +version = "0.5.0" dependencies = [ "convert_case", "darling", diff --git a/crates/stackable-operator/src/product_config_utils.rs b/crates/stackable-operator/src/product_config_utils.rs index 1d1a68c6..24095775 100644 --- a/crates/stackable-operator/src/product_config_utils.rs +++ b/crates/stackable-operator/src/product_config_utils.rs @@ -167,13 +167,21 @@ pub fn config_for_role_and_group<'a>( /// - `resource` - Not used directly. It's passed on to the `Configuration::compute_*` calls. /// - `roles` - A map keyed by role names. The value is a tuple of a vector of `PropertyNameKind` /// like (Cli, Env or Files) and [`crate::role_utils::Role`] with a boxed [`Configuration`]. -pub fn transform_all_roles_to_config( +#[allow(clippy::type_complexity)] +pub fn transform_all_roles_to_config( resource: &T::Configurable, - roles: HashMap, Role)>, + roles: HashMap< + String, + ( + Vec, + Role, + ), + >, ) -> Result where T: Configuration, U: Default + JsonSchema + Serialize, + ProductSpecificCommonConfig: Default + JsonSchema + Serialize, { let mut result = HashMap::new(); @@ -359,15 +367,16 @@ fn process_validation_result( /// - `role_name` - The name of the role. /// - `role` - The role for which to transform the configuration parameters. /// - `property_kinds` - Used as "buckets" to partition the configuration properties by. -fn transform_role_to_config( +fn transform_role_to_config( resource: &T::Configurable, role_name: &str, - role: &Role, + role: &Role, property_kinds: &[PropertyNameKind], ) -> Result where T: Configuration, U: Default + JsonSchema + Serialize, + ProductSpecificCommonConfig: Default + JsonSchema + Serialize, { let mut result = HashMap::new(); @@ -422,10 +431,10 @@ where /// - `role_name` - Not used directly but passed on to the `Configuration::compute_*` calls. /// - `config` - The configuration properties to partition. /// - `property_kinds` - The "buckets" used to partition the configuration properties. -fn parse_role_config( +fn parse_role_config( resource: &::Configurable, role_name: &str, - config: &CommonConfiguration, + config: &CommonConfiguration, property_kinds: &[PropertyNameKind], ) -> Result>>> where @@ -452,8 +461,8 @@ where Ok(result) } -fn parse_role_overrides( - config: &CommonConfiguration, +fn parse_role_overrides( + config: &CommonConfiguration, property_kinds: &[PropertyNameKind], ) -> Result>>> where @@ -489,8 +498,8 @@ where Ok(result) } -fn parse_file_overrides( - config: &CommonConfiguration, +fn parse_file_overrides( + config: &CommonConfiguration, file: &str, ) -> Result>> where @@ -522,7 +531,7 @@ mod tests { } use super::*; - use crate::role_utils::{Role, RoleGroup}; + use crate::role_utils::{GenericProductSpecificCommonConfig, Role, RoleGroup}; use k8s_openapi::api::core::v1::PodTemplateSpec; use rstest::*; use std::collections::HashMap; @@ -610,13 +619,14 @@ mod tests { config_overrides: Option>>, env_overrides: Option>, cli_overrides: Option>, - ) -> CommonConfiguration> { + ) -> CommonConfiguration, GenericProductSpecificCommonConfig> { CommonConfiguration { config: test_config.unwrap_or_default(), config_overrides: config_overrides.unwrap_or_default(), env_overrides: env_overrides.unwrap_or_default(), cli_overrides: cli_overrides.unwrap_or_default(), pod_overrides: PodTemplateSpec::default(), + product_specific_common_config: GenericProductSpecificCommonConfig::default(), } } diff --git a/crates/stackable-operator/src/role_utils.rs b/crates/stackable-operator/src/role_utils.rs index 04ecd259..1ac5d563 100644 --- a/crates/stackable-operator/src/role_utils.rs +++ b/crates/stackable-operator/src/role_utils.rs @@ -99,18 +99,27 @@ use k8s_openapi::api::core::v1::PodTemplateSpec; use kube::{runtime::reflector::ObjectRef, Resource}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use snafu::{OptionExt, Snafu}; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("missing roleGroup {role_group:?}"))] + MissingRoleGroup { role_group: String }, +} #[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde( rename_all = "camelCase", - bound(deserialize = "T: Default + Deserialize<'de>") + bound( + deserialize = "T: Default + Deserialize<'de>, ProductSpecificCommonConfig: Default + Deserialize<'de>" + ) )] -pub struct CommonConfiguration { +pub struct CommonConfiguration { #[serde(default)] // We can't depend on T 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 = "config_schema_default")] + #[schemars(default = "Self::default_config")] pub config: T, /// The `configOverrides` can be used to configure properties in product config files @@ -144,10 +153,70 @@ pub struct CommonConfiguration { #[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. + // It is not accessible by operators, please use to read the value + #[serde(flatten, default)] + pub(crate) product_specific_common_config: ProductSpecificCommonConfig, +} + +impl CommonConfiguration { + fn default_config() -> serde_json::Value { + serde_json::json!({}) + } +} + +#[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)] +pub struct GenericProductSpecificCommonConfig {} + +#[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize, Merge)] +#[merge(path_overrides(merge = "crate::config::merge"))] +#[serde(rename_all = "camelCase")] +pub struct JavaCommonConfig { + /// Allows overriding JVM arguments. + /// + // TODO: Docs + // Use [`JavaCommonConfig::effective_jvm_config`] to retrieve the effective JVM arguments! + #[serde(default)] + jvm_argument_overrides: BTreeMap, +} + +impl JavaCommonConfig { + pub fn new(jvm_argument_overrides: BTreeMap) -> Self { + Self { + jvm_argument_overrides, + } + } + + /// Returns all arguments that should be passed to the JVM. + /// + /// Please note that the values of the [`BTreeMap`] are [`Option`]. A value of [`None`] + /// expresses that the given argument is just a flag without any argument. + pub fn effective_jvm_config(&self) -> BTreeMap> { + self.jvm_argument_overrides + .iter() + .filter_map(|(k, v)| match v { + JvmArgument::Argument(argument) => Some((k.to_owned(), Some(argument.to_owned()))), + JvmArgument::Flag {} => Some((k.to_owned(), None)), + JvmArgument::Remove {} => None, + }) + .collect() + } } -fn config_schema_default() -> serde_json::Value { - serde_json::json!({}) +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub enum JvmArgument { + Argument(String), + Flag {}, + Remove {}, +} +impl Merge for JvmArgument { + fn merge(&mut self, _defaults: &Self) { + // We ignore whatever was in there before, later values override earlier ones + } } /// This struct represents a role - e.g. HDFS datanodes or Trino workers. It has a key-value-map containing @@ -168,33 +237,46 @@ fn config_schema_default() -> serde_json::Value { // 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 -where +pub struct Role< + T, + U = GenericRoleConfig, + ProductSpecificCommonConfig = GenericProductSpecificCommonConfig, +> 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! U: Default + JsonSchema + Serialize, + ProductSpecificCommonConfig: Default + JsonSchema + Serialize, { - #[serde(flatten, bound(deserialize = "T: Default + Deserialize<'de>"))] - pub config: CommonConfiguration, + #[serde( + flatten, + bound( + deserialize = "T: Default + Deserialize<'de>, ProductSpecificCommonConfig: Deserialize<'de>" + ) + )] + pub config: CommonConfiguration, #[serde(default)] pub role_config: U, - pub role_groups: HashMap>, + pub role_groups: HashMap>, } -impl Role +impl Role where T: Configuration + 'static, U: Default + JsonSchema + Serialize, + ProductSpecificCommonConfig: Default + JsonSchema + Serialize + Clone + Merge, { /// This casts a generic struct implementing [`crate::product_config_utils::Configuration`] /// and used in [`Role`] into a Box of a dynamically dispatched /// [`crate::product_config_utils::Configuration`] Trait. This is required to use the generic /// [`Role`] with more than a single generic struct. For example different roles most likely /// have different structs implementing Configuration. - pub fn erase(self) -> Role>, U> { + pub fn erase( + self, + ) -> Role>, U, ProductSpecificCommonConfig> + { Role { config: CommonConfiguration { config: Box::new(self.config.config) @@ -203,6 +285,7 @@ where env_overrides: self.config.env_overrides, cli_overrides: self.config.cli_overrides, pod_overrides: self.config.pod_overrides, + product_specific_common_config: self.config.product_specific_common_config, }, role_config: self.role_config, role_groups: self @@ -219,6 +302,9 @@ where env_overrides: group.config.env_overrides, cli_overrides: group.config.cli_overrides, pod_overrides: group.config.pod_overrides, + product_specific_common_config: group + .config + .product_specific_common_config, }, replicas: group.replicas, }, @@ -227,6 +313,23 @@ where .collect(), } } + + pub fn merged_product_specific_common_config( + &self, + role_group: &str, + ) -> Result { + let from_role = &self.config.product_specific_common_config; + let mut merged = self + .role_groups + .get(role_group) + .with_context(|| MissingRoleGroupSnafu { role_group })? + .config + .product_specific_common_config + .clone(); + merged.merge(from_role); + + Ok(merged) + } } /// This is a product-agnostic RoleConfig, which is sufficient for most of the products. @@ -246,15 +349,17 @@ pub struct EmptyRoleConfig {} #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde( rename_all = "camelCase", - bound(deserialize = "T: Default + Deserialize<'de>") + bound( + deserialize = "T: Default + Deserialize<'de>, ProductSpecificCommonConfig: Default + Deserialize<'de>" + ) )] -pub struct RoleGroup { +pub struct RoleGroup { #[serde(flatten)] - pub config: CommonConfiguration, + pub config: CommonConfiguration, pub replicas: Option, } -impl RoleGroup { +impl RoleGroup { pub fn validate_config( &self, role: &Role, @@ -296,3 +401,192 @@ impl Display for RoleGroupRef { )) } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use crate::{config::merge::Merge, role_utils::JvmArgument}; + + use super::JavaCommonConfig; + + #[test] + fn test_parse_java_common_config() { + let input = r#" + jvmArgumentOverrides: + -XX:+UseG1GC: + flag: {} + -Dhttps.proxyHost: + argument: proxy.my.corp + -XX:+ExitOnOutOfMemoryError: + remove: {} + "#; + let deserializer = serde_yaml::Deserializer::from_str(input); + let java_common_config: JavaCommonConfig = + serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); + + assert_eq!( + java_common_config, + JavaCommonConfig { + jvm_argument_overrides: BTreeMap::from([ + ("-XX:+UseG1GC".to_owned(), JvmArgument::Flag {}), + ( + "-Dhttps.proxyHost".to_owned(), + JvmArgument::Argument("proxy.my.corp".to_owned()) + ), + ( + "-XX:+ExitOnOutOfMemoryError".to_owned(), + JvmArgument::Remove {} + ) + ]) + } + ); + } + + #[test] + fn test_merge_java_common_config() { + // The operator generates some JVM arguments + let operator_generated = JavaCommonConfig { + jvm_argument_overrides: BTreeMap::from([ + // Some flags + ("-Xms34406m".to_owned(), JvmArgument::Flag {}), + ("-Xmx34406m".to_owned(), JvmArgument::Flag {}), + ("-XX:+UseG1GC".to_owned(), JvmArgument::Flag {}), + ( + "-XX:+ExitOnOutOfMemoryError".to_owned(), + JvmArgument::Flag {}, + ), + // And some arguments + ( + "-Djava.protocol.handler.pkgs".to_owned(), + JvmArgument::Argument("sun.net.www.protocol".to_owned()), + ), + ( + "-Dsun.net.http.allowRestrictedHeaders".to_owned(), + JvmArgument::Argument(true.to_string()), + ), + ( + "-Djava.security.properties".to_owned(), + JvmArgument::Argument("/stackable/nifi/conf/security.properties".to_owned()), + ), + ]), + }; + + // Let's say we want to set some additional HTTP Proxy and IPv4 settings + // And we don't like the garbage collector for some reason... + let role = serde_yaml::Deserializer::from_str( + r#" + jvmArgumentOverrides: + -XX:+UseG1GC: + remove: {} + -Dhttps.proxyHost: + argument: proxy.my.corp + -Dhttps.proxyPort: + argument: "8080" + -Djava.net.preferIPv4Stack: + argument: "true" + "#, + ); + let role: JavaCommonConfig = + serde_yaml::with::singleton_map_recursive::deserialize(role).unwrap(); + + // For the roleGroup, let's say we need a different memory config. + // For that to work we first remove the flags generated by the operator and add our own. + // Also we override the proxy port to test that the roleGroup config takes precedence over the role config. + let role_group = serde_yaml::Deserializer::from_str( + r#" + jvmArgumentOverrides: + # We need more memory! + -Xmx34406m: + remove: {} + -Xmx40000m: + flag: {} + -Dhttps.proxyPort: + argument: "1234" + "#, + ); + let role_group: JavaCommonConfig = + serde_yaml::with::singleton_map_recursive::deserialize(role_group).unwrap(); + + let mut merged = role_group; + merged.merge(&role); + merged.merge(&operator_generated); + + assert_eq!( + merged, + JavaCommonConfig { + jvm_argument_overrides: BTreeMap::from([ + // Flags + ("-Xms34406m".to_owned(), JvmArgument::Flag {}), + // Note the different memory config from the roleGroup! + ("-Xmx34406m".to_owned(), JvmArgument::Remove {}), + ("-Xmx40000m".to_owned(), JvmArgument::Flag {}), + // Note that the "-XX:+UseG1GC" flag is removed! + ("-XX:+UseG1GC".to_owned(), JvmArgument::Remove {}), + ( + "-XX:+ExitOnOutOfMemoryError".to_owned(), + JvmArgument::Flag {}, + ), + // Arguments + ( + "-Djava.protocol.handler.pkgs".to_owned(), + JvmArgument::Argument("sun.net.www.protocol".to_owned()), + ), + ( + "-Dsun.net.http.allowRestrictedHeaders".to_owned(), + JvmArgument::Argument(true.to_string()), + ), + ( + "-Djava.security.properties".to_owned(), + JvmArgument::Argument( + "/stackable/nifi/conf/security.properties".to_owned() + ), + ), + ( + "-Dhttps.proxyHost".to_owned(), + JvmArgument::Argument("proxy.my.corp".to_owned()), + ), + ( + "-Dhttps.proxyPort".to_owned(), + // Note: This is overridden by the roleGroup + JvmArgument::Argument("1234".to_owned()), + ), + ( + "-Djava.net.preferIPv4Stack".to_owned(), + JvmArgument::Argument("true".to_owned()), + ), + ]) + } + ); + + assert_eq!( + merged.effective_jvm_config(), + BTreeMap::from([ + ("-Xms34406m".to_owned(), None), + ("-Xmx40000m".to_owned(), None), + ("-XX:+ExitOnOutOfMemoryError".to_owned(), None), + ( + "-Djava.protocol.handler.pkgs".to_owned(), + Some("sun.net.www.protocol".to_owned()) + ), + ( + "-Dsun.net.http.allowRestrictedHeaders".to_owned(), + Some("true".to_owned()) + ), + ( + "-Djava.security.properties".to_owned(), + Some("/stackable/nifi/conf/security.properties".to_owned()) + ), + ( + "-Dhttps.proxyHost".to_owned(), + Some("proxy.my.corp".to_owned()) + ), + ("-Dhttps.proxyPort".to_owned(), Some("1234".to_owned())), + ( + "-Djava.net.preferIPv4Stack".to_owned(), + Some("true".to_owned()) + ), + ]) + ); + } +}