refactor: switch to v2 JavaCommonConfig & remove framework module

Author: maltesander

Cargo.lock

@@ -1,9 +1,7 @@
-use serde::Serialize;
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
     memory::{BinaryMultiple, MemoryQuantity},
-    role_utils::{self, GenericRoleConfig, JavaCommonConfig, JvmArgumentOverrides, Role},
-    schemars::JsonSchema,
+    v2::jvm_argument_overrides::JvmArgumentOverrides,
 };
 
 use crate::crd::{ConfigFileName, METRICS_PORT, STACKABLE_CONFIG_DIR, role::AnyConfig};
@@ -19,20 +17,14 @@ pub enum Error {
     InvalidMemoryConfig {
         source: stackable_operator::memory::Error,
     },
-
-    #[snafu(display("failed to merge jvm argument overrides"))]
-    MergeJvmArgumentOverrides { source: role_utils::Error },
 }
 
-/// All JVM arguments.
-fn construct_jvm_args<ConfigFragment, ConfigOverrides>(
+/// All JVM arguments: operator-generated base args with the already-merged
+/// (role <- role group) `jvmArgumentOverrides` applied on top.
+fn construct_jvm_args(
     merged_config: &AnyConfig,
-    role: &Role<ConfigFragment, ConfigOverrides, GenericRoleConfig, JavaCommonConfig>,
-    role_group: &str,
-) -> Result<Vec<String>, Error>
-where
-    ConfigOverrides: Default + JsonSchema + Serialize,
-{
+    jvm_argument_overrides: &JvmArgumentOverrides,
+) -> Result<Vec<String>, Error> {
     let heap_size = MemoryQuantity::try_from(
         merged_config
             .resources()
@@ -49,7 +41,6 @@ where
         .context(InvalidMemoryConfigSnafu)?;
 
     let jvm_args = vec![
-        // Heap settings
         format!("-Xmx{java_heap}"),
         format!("-Xms{java_heap}"),
         format!(
@@ -61,43 +52,26 @@ where
         ),
     ];
 
-    let operator_generated = JvmArgumentOverrides::new_with_only_additions(jvm_args);
-    let merged = role
-        .get_merged_jvm_argument_overrides(role_group, &operator_generated)
-        .context(MergeJvmArgumentOverridesSnafu)?;
-    Ok(merged
-        .effective_jvm_config_after_merging()
-        // Sorry for the clone, that's how operator-rs is currently modelled :P
-        .clone())
+    Ok(jvm_argument_overrides.apply_to(jvm_args))
 }
 
-/// Arguments that go into `EXTRA_ARGS`, so *not* the heap settings (which you can get using
-/// [`construct_heap_jvm_args`]).
-pub fn construct_non_heap_jvm_args<ConfigFragment, ConfigOverrides>(
+/// Arguments that go into `EXTRA_ARGS` (everything except heap settings).
+pub fn construct_non_heap_jvm_args(
     merged_config: &AnyConfig,
-    role: &Role<ConfigFragment, ConfigOverrides, GenericRoleConfig, JavaCommonConfig>,
-    role_group: &str,
-) -> Result<String, Error>
-where
-    ConfigOverrides: Default + JsonSchema + Serialize,
-{
-    let mut jvm_args = construct_jvm_args(merged_config, role, role_group)?;
+    jvm_argument_overrides: &JvmArgumentOverrides,
+) -> Result<String, Error> {
+    let mut jvm_args = construct_jvm_args(merged_config, jvm_argument_overrides)?;
     jvm_args.retain(|arg| !is_heap_jvm_argument(arg));
 
     Ok(jvm_args.join(" "))
 }
 
-/// Arguments that go into `KAFKA_HEAP_OPTS`.
-/// You can get the normal JVM arguments using [`construct_non_heap_jvm_args`].
-pub fn construct_heap_jvm_args<ConfigFragment, ConfigOverrides>(
+/// Arguments that go into `KAFKA_HEAP_OPTS` (only the heap settings).
+pub fn construct_heap_jvm_args(
     merged_config: &AnyConfig,
-    role: &Role<ConfigFragment, ConfigOverrides, GenericRoleConfig, JavaCommonConfig>,
-    role_group: &str,
-) -> Result<String, Error>
-where
-    ConfigOverrides: Default + JsonSchema + Serialize,
-{
-    let mut jvm_args = construct_jvm_args(merged_config, role, role_group)?;
+    jvm_argument_overrides: &JvmArgumentOverrides,
+) -> Result<String, Error> {
+    let mut jvm_args = construct_jvm_args(merged_config, jvm_argument_overrides)?;
     jvm_args.retain(|arg| is_heap_jvm_argument(arg));
 
     Ok(jvm_args.join(" "))
@@ -111,25 +85,34 @@ fn is_heap_jvm_argument(jvm_argument: &str) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use stackable_operator::kube::ResourceExt;
-
     use super::*;
     use crate::{
-        crd::{
-            BrokerRole,
-            role::{KafkaRole, broker::BrokerConfig},
-            v1alpha1,
-        },
-        framework::role_utils::with_validated_config,
+        controller::test_support::{minimal_kafka, validated_cluster},
+        crd::role::KafkaRole,
     };
 
+    /// Pulls the broker `default` role group's merged config + merged JVM overrides out of
+    /// a validated cluster built from the given YAML.
+    fn broker_default(yaml: &str) -> (AnyConfig, JvmArgumentOverrides) {
+        let kafka = minimal_kafka(yaml);
+        let validated = validated_cluster(&kafka);
+        let rg = validated
+            .role_group_configs
+            .get(&KafkaRole::Broker)
+            .and_then(|groups| groups.get("default"))
+            .expect("broker default role group should exist");
+        (rg.config.clone(), rg.jvm_argument_overrides.clone())
+    }
+
     #[test]
     fn test_construct_jvm_arguments_defaults() {
         let input = r#"
         apiVersion: kafka.stackable.tech/v1alpha1
         kind: KafkaCluster
         metadata:
           name: simple-kafka
+          namespace: default
+          uid: 12345678-1234-1234-1234-123456789012
         spec:
           image:
             productVersion: 3.9.2
@@ -140,10 +123,9 @@ mod tests {
               default:
                 replicas: 1
         "#;
-        let (kafka_role, role, merged_config) = construct_boilerplate(input);
-        let non_heap_jvm_args =
-            construct_non_heap_jvm_args(&kafka_role, &role, &merged_config).unwrap();
-        let heap_jvm_args = construct_heap_jvm_args(&kafka_role, &role, &merged_config).unwrap();
+        let (merged_config, jvm) = broker_default(input);
+        let non_heap_jvm_args = construct_non_heap_jvm_args(&merged_config, &jvm).unwrap();
+        let heap_jvm_args = construct_heap_jvm_args(&merged_config, &jvm).unwrap();
 
         assert_eq!(
             non_heap_jvm_args,
@@ -160,6 +142,8 @@ mod tests {
         kind: KafkaCluster
         metadata:
           name: simple-kafka
+          namespace: default
+          uid: 12345678-1234-1234-1234-123456789012
         spec:
           image:
             productVersion: 3.9.2
@@ -187,10 +171,9 @@ mod tests {
                     - -Xmx40000m
                     - -Dhttps.proxyPort=1234
         "#;
-        let (merged_config, role, role_group) = construct_boilerplate(input);
-        let non_heap_jvm_args =
-            construct_non_heap_jvm_args(&merged_config, &role, &role_group).unwrap();
-        let heap_jvm_args = construct_heap_jvm_args(&merged_config, &role, &role_group).unwrap();
+        let (merged_config, jvm) = broker_default(input);
+        let non_heap_jvm_args = construct_non_heap_jvm_args(&merged_config, &jvm).unwrap();
+        let heap_jvm_args = construct_heap_jvm_args(&merged_config, &jvm).unwrap();
 
         assert_eq!(
             non_heap_jvm_args,
@@ -202,18 +185,4 @@ mod tests {
         );
         assert_eq!(heap_jvm_args, "-Xms34406m -Xmx40000m");
     }
-
-    fn construct_boilerplate(kafka_cluster: &str) -> (AnyConfig, BrokerRole, String) {
-        let kafka: v1alpha1::KafkaCluster =
-            serde_yaml::from_str(kafka_cluster).expect("illegal test input");
-
-        let role = kafka.spec.brokers.clone().unwrap();
-        let role_group = role.role_groups.get("default").unwrap();
-        let default_config =
-            BrokerConfig::default_config(&kafka.name_any(), &KafkaRole::Broker.to_string());
-        let validated = with_validated_config(role_group, &role, &default_config).unwrap();
-        let merged_config = AnyConfig::Broker(validated.config);
-
-        (merged_config, role, "default".to_owned())
-    }
 }

Cargo.nix

@@ -20,15 +20,12 @@ pub(crate) mod build;
 pub(crate) mod dereference;
 pub(crate) mod validate;
 
-use crate::{
-    crd::{
-        KafkaPodDescriptor, MetadataManager,
-        authorization::KafkaAuthorizationConfig,
-        role::{AnyConfig, AnyConfigOverrides, KafkaRole},
-        security::KafkaTlsSecurity,
-        v1alpha1,
-    },
-    framework::role_utils::RoleGroupConfig,
+use crate::crd::{
+    KafkaPodDescriptor, MetadataManager,
+    authorization::KafkaAuthorizationConfig,
+    role::{AnyConfig, AnyConfigOverrides, KafkaRole},
+    security::KafkaTlsSecurity,
+    v1alpha1,
 };
 
 pub type RoleGroupName = String;
@@ -140,12 +137,62 @@ impl Resource for ValidatedCluster {
 /// A validated, merged Kafka role-group config.
 ///
 /// The merged config fragment is wrapped in [`AnyConfig`] and the merged
-/// `configOverrides` in [`AnyConfigOverrides`], so a single role-agnostic type
-/// carries both broker and controller role groups (their concrete config and
-/// override types differ). Produced via the local-`framework`
-/// [`with_validated_config`](crate::framework::role_utils::with_validated_config).
-pub type ValidatedRoleGroupConfig = RoleGroupConfig<
-    AnyConfig,
-    stackable_operator::role_utils::JavaCommonConfig,
-    AnyConfigOverrides,
->;
+/// `configOverrides` in [`AnyConfigOverrides`], so a single role-agnostic type carries
+/// both broker and controller role groups. Produced from the upstream
+/// [`stackable_operator::v2::role_utils::with_validated_config`] result in
+/// [`validate`](crate::controller::validate). `jvm_argument_overrides` is already merged
+/// (role <- role group) at validation time and applied as-is during build.
+#[derive(Clone, Debug, PartialEq)]
+pub struct ValidatedRoleGroupConfig {
+    pub replicas: u16,
+    pub config: AnyConfig,
+    pub config_overrides: AnyConfigOverrides,
+    pub env_overrides: stackable_operator::v2::builder::pod::container::EnvVarSet,
+    pub pod_overrides: stackable_operator::k8s_openapi::api::core::v1::PodTemplateSpec,
+    pub jvm_argument_overrides:
+        stackable_operator::v2::jvm_argument_overrides::JvmArgumentOverrides,
+}
+
+#[cfg(test)]
+pub(crate) mod test_support {
+    use stackable_operator::{
+        cli::OperatorEnvironmentOptions,
+        commons::networking::DomainName,
+        utils::{cluster_info::KubernetesClusterInfo, yaml_from_str_singleton_map},
+    };
+
+    use super::{ValidatedCluster, dereference::DereferencedObjects, validate::validate};
+    use crate::crd::{authentication::ResolvedAuthenticationClasses, v1alpha1};
+
+    pub fn minimal_kafka(yaml: &str) -> v1alpha1::KafkaCluster {
+        yaml_from_str_singleton_map(yaml).expect("invalid test KafkaCluster YAML")
+    }
+
+    fn cluster_info() -> KubernetesClusterInfo {
+        KubernetesClusterInfo {
+            cluster_domain: DomainName::try_from("cluster.local").expect("valid domain"),
+        }
+    }
+
+    fn operator_environment() -> OperatorEnvironmentOptions {
+        OperatorEnvironmentOptions {
+            operator_namespace: "stackable-operators".to_owned(),
+            operator_service_name: "kafka-operator".to_owned(),
+            image_repository: "oci.example.org".to_owned(),
+        }
+    }
+
+    /// Runs the real validate step against a minimal (auth/OPA-free) fixture.
+    pub fn validated_cluster(kafka: &v1alpha1::KafkaCluster) -> ValidatedCluster {
+        validate(
+            kafka,
+            DereferencedObjects {
+                authentication_classes: ResolvedAuthenticationClasses::new(Vec::new()),
+                authorization_config: None,
+                kubernetes_cluster_info: cluster_info(),
+            },
+            &operator_environment(),
+        )
+        .expect("validate should succeed for the test fixture")
+    }
+}