From faee4759ba1e364cff1de954f8cb1238372f5dd2 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 9 Oct 2026 15:49:10 +0200 Subject: [PATCH 1/3] feat(versioned): Support conversion tracking through enums With experimental_conversion_tracking enabled, versioned enums now implement TrackingFrom instead of From, in line with structs. Variants marked with as an additional path segment (eg. `$.connector.Iceberg.field`). Also fix the generated From impls for variants with multiple unnamed fields, which were missing separating commas. --- .../src/attrs/item/variant.rs | 18 +- .../src/codegen/container/enum.rs | 45 +- .../src/codegen/item/variant.rs | 57 +- .../src/codegen/module.rs | 20 +- crates/stackable-versioned-macros/src/lib.rs | 44 ++ .../inputs/pass/conversion_tracking_enum.rs | 56 ++ ...ots__pass@conversion_tracking_enum.rs.snap | 576 ++++++++++++++++++ ...ioned_macros__snapshots__pass@docs.rs.snap | 64 +- crates/stackable-versioned/CHANGELOG.md | 14 + .../tests/enum_tracking.rs | 131 ++++ 10 files changed, 989 insertions(+), 36 deletions(-) create mode 100644 crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_enum.rs create mode 100644 crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap create mode 100644 crates/stackable-versioned/tests/enum_tracking.rs diff --git a/crates/stackable-versioned-macros/src/attrs/item/variant.rs b/crates/stackable-versioned-macros/src/attrs/item/variant.rs index 46d1c69fe..06cd4b3ed 100644 --- a/crates/stackable-versioned-macros/src/attrs/item/variant.rs +++ b/crates/stackable-versioned-macros/src/attrs/item/variant.rs @@ -1,5 +1,5 @@ use convert_case::{Case, Casing}; -use darling::{Error, FromVariant, Result}; +use darling::{Error, FromVariant, Result, util::Flag}; use syn::{Attribute, Ident}; use crate::{ @@ -40,6 +40,11 @@ pub struct VariantAttributes { // FromMeta. /// The original attributes for the field. pub attrs: Vec, + + /// Indicates that the data of this variant contains nested sub structs or enums. The + /// indicator is needed to let the macro know to generate conversion code with support for + /// tracking across container boundaries. + pub nested: Flag, } impl VariantAttributes { @@ -73,6 +78,17 @@ impl VariantAttributes { pub fn validate_versions(&self, versions: &[VersionDefinition]) -> Result<()> { self.common.validate_versions(versions) } + + pub fn validate_nested_flag(&self, experimental_conversion_tracking: bool) -> Result<()> { + if self.nested.is_present() && !experimental_conversion_tracking { + return Err( + Error::custom("the `nested` argument can only be used if the module-level `experimental_conversion_tracking` flag is set") + .with_span(&self.nested.span()) + ); + } + + Ok(()) + } } // We don't want to take a dependency on the (unmaintained[1]) `convert_case_extras` crate just for diff --git a/crates/stackable-versioned-macros/src/codegen/container/enum.rs b/crates/stackable-versioned-macros/src/codegen/container/enum.rs index fbe5b40b7..1730b3207 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/enum.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/enum.rs @@ -3,7 +3,7 @@ use std::ops::Not; use darling::{FromAttributes, Result}; use proc_macro2::TokenStream; use quote::quote; -use syn::{Generics, ItemEnum}; +use syn::{Generics, ItemEnum, parse_quote}; use crate::{ attrs::container::ContainerAttributes, @@ -19,12 +19,17 @@ use crate::{ }; impl Container { - pub fn new_enum(item_enum: ItemEnum, versions: &[VersionDefinition]) -> Result { + pub fn new_enum( + item_enum: ItemEnum, + versions: &[VersionDefinition], + experimental_conversion_tracking: bool, + ) -> Result { let attributes = ContainerAttributes::from_attributes(&item_enum.attrs)?; let mut versioned_variants = Vec::new(); for variant in item_enum.variants { - let mut versioned_variant = VersionedVariant::new(variant, versions)?; + let mut versioned_variant = + VersionedVariant::new(variant, versions, experimental_conversion_tracking)?; versioned_variant.insert_container_versions(versions); versioned_variants.push(versioned_variant); } @@ -157,7 +162,7 @@ impl Enum { self.variants .iter() .filter_map(|v| { - v.generate_for_from_impl(direction, version, next_version, enum_ident) + v.generate_for_from_impl(direction, version, next_version, enum_ident, gen_ctx) }) .collect() }; @@ -177,6 +182,38 @@ impl Enum { }, }; + // With conversion tracking enabled, the enum needs to forward the status and the + // current path to the data of nested variants. As such, only a TrackingFrom impl is + // generated, which is in line with the impls generated for structs. + if gen_ctx.kubernetes_options.experimental_conversion_tracking.is_present() { + let versioned_path = &*gen_ctx.crates.versioned; + + let mut tracking_generics = self.generics.clone(); + tracking_generics.params.push(parse_quote! { S }); + tracking_generics + .make_where_clause() + .predicates + .push(parse_quote! { S: #versioned_path::TrackingStatus + ::core::default::Default }); + let (impl_generics, _, where_clause) = tracking_generics.split_for_impl(); + + return quote! { + #automatically_derived + #allow_attribute + impl #impl_generics #versioned_path::TrackingFrom<#from_module_ident::#enum_ident #type_generics, S> for #for_module_ident::#enum_ident #type_generics + #where_clause + { + fn tracking_from(#from_enum_ident: #from_module_ident::#enum_ident #type_generics, status: &mut S, parent: &str) -> Self { + // TODO (@Techassi): Only emit this if any of the variants below need it + use #versioned_path::TrackingInto as _; + + match #from_enum_ident { + #variants + } + } + } + }; + } + quote! { #automatically_derived #allow_attribute diff --git a/crates/stackable-versioned-macros/src/codegen/item/variant.rs b/crates/stackable-versioned-macros/src/codegen/item/variant.rs index d97e10c1b..b2661c624 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/variant.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/variant.rs @@ -14,6 +14,7 @@ use crate::{ Direction, VersionDefinition, changes::{BTreeMapExt, ChangesetExt}, item::{ItemStatus, generate_attributes}, + module::ModuleGenerationContext, }, utils::ItemIdents, }; @@ -24,12 +25,18 @@ pub struct VersionedVariant { pub changes: Option>, pub idents: VariantIdents, pub fields: Fields, + pub nested: bool, } impl VersionedVariant { - pub fn new(variant: Variant, versions: &[VersionDefinition]) -> Result { + pub fn new( + variant: Variant, + versions: &[VersionDefinition], + experimental_conversion_tracking: bool, + ) -> Result { let variant_attributes = VariantAttributes::from_variant(&variant)?; variant_attributes.validate_versions(versions)?; + variant_attributes.validate_nested_flag(experimental_conversion_tracking)?; let idents = VariantIdents::from(variant.ident); @@ -41,6 +48,7 @@ impl VersionedVariant { bang_token: Not([Span::call_site()]), }); let changed_docs = variant_attributes.common.changed_docs(); + let nested = variant_attributes.nested.is_present(); let changes = variant_attributes.common.into_changeset(&idents, ty); Ok(Self { @@ -49,6 +57,7 @@ impl VersionedVariant { fields: variant.fields, idents, changes, + nested, }) } @@ -152,9 +161,10 @@ impl VersionedVariant { version: &VersionDefinition, next_version: &VersionDefinition, enum_ident: &IdentString, + mod_gen_ctx: ModuleGenerationContext<'_>, ) -> Option { let from_fields = self.generate_from_fields(); - let for_fields = self.generate_for_fields(); + let for_fields = self.generate_for_fields(mod_gen_ctx); #[allow(clippy::single_match_else)] match &self.changes { @@ -203,20 +213,55 @@ impl VersionedVariant { } } - fn generate_for_fields(&self) -> Option { + fn generate_for_fields(&self, mod_gen_ctx: ModuleGenerationContext<'_>) -> Option { match &self.fields { Fields::Named(fields_named) => { let fields = Self::named_field_idents(fields_named); - Some(quote! { { #(#fields: #fields.into(),)* } }) + let conversions = fields.iter().map(|field| { + self.generate_conversion_function(Some(&field.to_string()), mod_gen_ctx) + }); + + Some(quote! { { #(#fields: #fields.#conversions,)* } }) } Fields::Unnamed(fields_unnamed) => { let fields = Self::unnamed_field_ident(fields_unnamed); - Some(quote! { ( #(#fields.into())* ) }) + + // Newtype variants (which are the most common variants with data) don't need an + // additional path segment, as the variant only contains a single field. + let conversions = (0..fields.len()).map(|index| { + let child = (fields.len() > 1).then(|| index.to_string()); + self.generate_conversion_function(child.as_deref(), mod_gen_ctx) + }); + + Some(quote! { ( #(#fields.#conversions),* ) }) } Fields::Unit => None, } } + /// Generates the conversion function for a single field of the variant data. + /// + /// The data of variants marked as nested is converted with support for tracking. The path + /// passed down consists of the variant name and the provided `child`, if any. + fn generate_conversion_function( + &self, + child: Option<&str>, + mod_gen_ctx: ModuleGenerationContext<'_>, + ) -> TokenStream { + if !self.nested { + return quote! { into() }; + } + + let versioned_path = &*mod_gen_ctx.crates.versioned; + let variant = &self.idents.original; + let child_string = match child { + Some(child) => format!("{variant}.{child}"), + None => variant.to_string(), + }; + + quote! { tracking_into(status, &#versioned_path::jthong_path(parent, #child_string)) } + } + fn generate_from_fields(&self) -> Option { match &self.fields { Fields::Named(fields_named) => { @@ -225,7 +270,7 @@ impl VersionedVariant { } Fields::Unnamed(fields_unnamed) => { let fields = Self::unnamed_field_ident(fields_unnamed); - Some(quote! { ( #(#fields)* ) }) + Some(quote! { ( #(#fields),* ) }) } Fields::Unit => None, } diff --git a/crates/stackable-versioned-macros/src/codegen/module.rs b/crates/stackable-versioned-macros/src/codegen/module.rs index 681321850..c76e85e42 100644 --- a/crates/stackable-versioned-macros/src/codegen/module.rs +++ b/crates/stackable-versioned-macros/src/codegen/module.rs @@ -50,22 +50,24 @@ impl Module { let mut submodules = HashMap::new(); let mut containers = Vec::new(); + let experimental_conversion_tracking = module_attributes + .options + .kubernetes + .experimental_conversion_tracking + .is_present(); + for item in items { match item { Item::Enum(item_enum) => { - if let Some(container) = - errors.handle(Container::new_enum(item_enum, &versions)) - { + if let Some(container) = errors.handle(Container::new_enum( + item_enum, + &versions, + experimental_conversion_tracking, + )) { containers.push(container); } } Item::Struct(item_struct) => { - let experimental_conversion_tracking = module_attributes - .options - .kubernetes - .experimental_conversion_tracking - .is_present(); - if let Some(container) = errors.handle(Container::new_struct( item_struct, &versions, diff --git a/crates/stackable-versioned-macros/src/lib.rs b/crates/stackable-versioned-macros/src/lib.rs index 6813c61d2..4410b365e 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -969,6 +969,50 @@ mod utils; /// # fn main() {} /// ``` /// +/// Changes can also be tracked through enums. With conversion tracking enabled, +/// versioned structs and enums only implement `TrackingFrom` instead of `From`. +/// As such, every field and every enum variant containing a versioned struct +/// or enum needs to be marked with `#[versioned(nested)]`. All data of a nested +/// variant is converted with tracking support. The path of tracked values +/// contains the variant name, eg. `$.connector.Baz.quox`. +/// +/// ``` +/// # use stackable_versioned_macros::versioned; +/// # use kube::CustomResource; +/// # use schemars::JsonSchema; +/// # use serde::{Deserialize, Serialize}; +/// #[versioned( +/// version(name = "v1alpha1"), +/// version(name = "v1beta1"), +/// options(k8s(experimental_conversion_tracking)) +/// )] +/// mod versioned { +/// #[versioned(crd(group = "example.com", doc = "My custom resource."))] +/// #[derive(Clone, Debug, Deserialize, Serialize, CustomResource, JsonSchema)] +/// struct FooSpec { +/// #[versioned(nested)] +/// connector: Connector, +/// } +/// +/// #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +/// enum Connector { +/// #[versioned(nested)] +/// Baz(Baz), +/// +/// Unit, +/// } +/// +/// #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +/// struct Baz { +/// quax: String, +/// +/// #[versioned(added(since = "v1beta1"))] +/// quox: bool, +/// } +/// } +/// # fn main() {} +/// ``` +/// /// # OpenTelemetry Semantic Conventions /// /// If tracing is enabled, various traces and events are emitted. The fields of diff --git a/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_enum.rs b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_enum.rs new file mode 100644 index 000000000..428ee98d4 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_enum.rs @@ -0,0 +1,56 @@ +use kube::CustomResource; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use stackable_versioned::versioned; +// --- +#[versioned( + version(name = "v1alpha1"), + version(name = "v1alpha2"), + options(k8s(experimental_conversion_tracking)) +)] +// --- +pub(crate) mod versioned { + #[versioned(crd(group = "stackable.tech", doc = "Test"))] + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, CustomResource)] + pub(crate) struct FooSpec { + // With conversion tracking enabled, enums only implement TrackingFrom. As such, fields + // using a versioned enum need to be marked as nested. + #[versioned(nested)] + connector: Connector, + } + + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] + pub(crate) enum Connector { + // The data of this variant is a versioned struct, which tracks changes. + #[versioned(nested)] + Iceberg(IcebergConnector), + + // Nested variants with named fields are supported as well. + #[versioned(nested)] + Hive { connector: IcebergConnector }, + + // Nested tuple variants with multiple fields use the index as an additional path segment. + #[versioned(nested)] + Both(IcebergConnector, IcebergConnector), + + // The data of this variant is not versioned and is converted using From. + Tpch(TpchConnector), + + Tpcds(TpchConnector, TpchConnector), + + Unit, + } + + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] + pub(crate) struct IcebergConnector { + metastore: Option, + + #[versioned(added(since = "v1alpha2"))] + rest_catalog_uri: Option, + } +} +// --- +fn main() {} + +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct TpchConnector {} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap new file mode 100644 index 000000000..40f7abe7f --- /dev/null +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap @@ -0,0 +1,576 @@ +--- +source: crates/stackable-versioned-macros/src/lib.rs +expression: formatted +input_file: crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_enum.rs +--- +#[automatically_derived] +pub(crate) mod v1alpha1 { + use super::*; + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, CustomResource)] + #[kube( + group = "stackable.tech", + version = "v1alpha1", + kind = "Foo", + doc = "Test", + status = FooStatusWithChangedValues + )] + pub struct FooSpec { + pub connector: Connector, + } + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] + pub enum Connector { + Iceberg(IcebergConnector), + Hive { connector: IcebergConnector }, + Both(IcebergConnector, IcebergConnector), + Tpch(TpchConnector), + Tpcds(TpchConnector, TpchConnector), + Unit, + } + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] + pub struct IcebergConnector { + pub metastore: Option, + } +} +#[automatically_derived] +impl ::core::convert::From for v1alpha2::Foo { + fn from(__sv_foo: v1alpha1::Foo) -> Self { + let mut status = __sv_foo.status.unwrap_or_default(); + let spec = >::tracking_from(__sv_foo.spec, &mut status, "$"); + Self { + metadata: __sv_foo.metadata, + status: Some(status), + spec, + } + } +} +#[automatically_derived] +impl ::core::convert::From for v1alpha1::Foo { + fn from(__sv_foo: v1alpha2::Foo) -> Self { + let mut status = __sv_foo.status.unwrap_or_default(); + let spec = >::tracking_from(__sv_foo.spec, &mut status, "$"); + Self { + metadata: __sv_foo.metadata, + status: Some(status), + spec, + } + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha2::FooSpec +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_foospec: v1alpha1::FooSpec, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_connector_path = ::stackable_versioned::jthong_path( + parent, + "connector", + ); + let mut spec = Self { + connector: __sv_foospec.connector.tracking_into(status, &__sv_connector_path), + }; + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha1::FooSpec +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_foospec: v1alpha2::FooSpec, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_connector_path = ::stackable_versioned::jthong_path( + parent, + "connector", + ); + let mut spec = Self { + connector: __sv_foospec.connector.tracking_into(status, &__sv_connector_path), + }; + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom +for v1alpha2::Connector +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_connector: v1alpha1::Connector, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + match __sv_connector { + v1alpha1::Connector::Iceberg(__sv_0) => { + v1alpha2::Connector::Iceberg( + __sv_0 + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Iceberg"), + ), + ) + } + v1alpha1::Connector::Hive { connector } => { + v1alpha2::Connector::Hive { + connector: connector + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Hive.connector"), + ), + } + } + v1alpha1::Connector::Both(__sv_0, __sv_1) => { + v1alpha2::Connector::Both( + __sv_0 + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Both.0"), + ), + __sv_1 + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Both.1"), + ), + ) + } + v1alpha1::Connector::Tpch(__sv_0) => v1alpha2::Connector::Tpch(__sv_0.into()), + v1alpha1::Connector::Tpcds(__sv_0, __sv_1) => { + v1alpha2::Connector::Tpcds(__sv_0.into(), __sv_1.into()) + } + v1alpha1::Connector::Unit => v1alpha2::Connector::Unit, + } + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom +for v1alpha1::Connector +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_connector: v1alpha2::Connector, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + match __sv_connector { + v1alpha2::Connector::Iceberg(__sv_0) => { + v1alpha1::Connector::Iceberg( + __sv_0 + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Iceberg"), + ), + ) + } + v1alpha2::Connector::Hive { connector } => { + v1alpha1::Connector::Hive { + connector: connector + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Hive.connector"), + ), + } + } + v1alpha2::Connector::Both(__sv_0, __sv_1) => { + v1alpha1::Connector::Both( + __sv_0 + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Both.0"), + ), + __sv_1 + .tracking_into( + status, + &::stackable_versioned::jthong_path(parent, "Both.1"), + ), + ) + } + v1alpha2::Connector::Tpch(__sv_0) => v1alpha1::Connector::Tpch(__sv_0.into()), + v1alpha2::Connector::Tpcds(__sv_0, __sv_1) => { + v1alpha1::Connector::Tpcds(__sv_0.into(), __sv_1.into()) + } + v1alpha2::Connector::Unit => v1alpha1::Connector::Unit, + } + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom +for v1alpha2::IcebergConnector +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_icebergconnector: v1alpha1::IcebergConnector, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_rest_catalog_uri_path = ::stackable_versioned::jthong_path( + parent, + "rest_catalog_uri", + ); + let mut spec = Self { + metastore: __sv_icebergconnector.metastore.into(), + rest_catalog_uri: ::std::default::Default::default(), + }; + if let Some(upgrades) = status.changes().upgrades.remove(&"v1alpha2".to_owned()) + { + for ::stackable_versioned::ChangedValue { json_path, value } in upgrades { + match json_path { + json_path if json_path == __sv_rest_catalog_uri_path => { + spec.rest_catalog_uri = serde_yaml::from_value(value).unwrap(); + } + _ => unreachable!(), + } + } + } + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom +for v1alpha1::IcebergConnector +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_icebergconnector: v1alpha2::IcebergConnector, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_rest_catalog_uri_path = ::stackable_versioned::jthong_path( + parent, + "rest_catalog_uri", + ); + let upgrades = status + .changes() + .upgrades + .entry("v1alpha2".to_owned()) + .or_default(); + upgrades + .push(::stackable_versioned::ChangedValue { + json_path: __sv_rest_catalog_uri_path, + value: ::serde_yaml::to_value(&__sv_icebergconnector.rest_catalog_uri) + .unwrap(), + }); + let mut spec = Self { + metastore: __sv_icebergconnector.metastore.into(), + }; + spec + } +} +#[automatically_derived] +pub(crate) mod v1alpha2 { + use super::*; + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, CustomResource)] + #[kube( + group = "stackable.tech", + version = "v1alpha2", + kind = "Foo", + doc = "Test", + status = FooStatusWithChangedValues + )] + pub struct FooSpec { + pub connector: Connector, + } + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] + pub enum Connector { + Iceberg(IcebergConnector), + Hive { connector: IcebergConnector }, + Both(IcebergConnector, IcebergConnector), + Tpch(TpchConnector), + Tpcds(TpchConnector, TpchConnector), + Unit, + } + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] + pub struct IcebergConnector { + pub metastore: Option, + pub rest_catalog_uri: Option, + } +} +#[automatically_derived] +#[derive(::core::fmt::Debug)] +pub(crate) enum Foo { + V1Alpha1(v1alpha1::Foo), + V1Alpha2(v1alpha2::Foo), +} +#[automatically_derived] +impl Foo { + /// Generates a merged CRD containing all versions and marking `stored_apiversion` as stored. + pub fn merged_crd( + stored_apiversion: FooVersion, + ) -> ::std::result::Result< + ::k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition, + ::kube::core::crd::MergeError, + > { + ::kube::core::crd::merge_crds( + vec![ + < v1alpha1::Foo as ::kube::core::CustomResourceExt > ::crd(), < + v1alpha2::Foo as ::kube::core::CustomResourceExt > ::crd() + ], + stored_apiversion.as_version_str(), + ) + } + ///Tries to convert a list of objects of kind [`Foo`] to the desired API version + ///specified in the [`ConversionReview`][cr]. + /// + ///The returned [`ConversionReview`][cr] either indicates a success or a failure, which + ///is handed back to the Kubernetes API server. + /// + ///[cr]: ::kube::core::conversion::ConversionReview + pub fn try_convert( + review: ::kube::core::conversion::ConversionReview, + ) -> ::kube::core::conversion::ConversionReview { + let request = match ::kube::core::conversion::ConversionRequest::from_review( + review, + ) { + ::std::result::Result::Ok(request) => request, + ::std::result::Result::Err(err) => { + return ::kube::core::conversion::ConversionResponse::invalid(::kube::core::Status { + status: Some(::kube::core::response::StatusSummary::Failure), + message: err.to_string(), + metadata: None, + reason: err.to_string(), + details: None, + code: 400, + }) + .into_review(); + } + }; + let response = match Self::convert_objects( + request.objects, + &request.desired_api_version, + ) { + ::std::result::Result::Ok(converted_objects) => { + ::kube::core::conversion::ConversionResponse { + result: ::kube::core::Status::success(), + types: request.types, + uid: request.uid, + converted_objects, + } + } + ::std::result::Result::Err(err) => { + let code = err.http_status_code(); + let message = err.join_errors(); + ::kube::core::conversion::ConversionResponse { + result: ::kube::core::Status { + status: Some(::kube::core::response::StatusSummary::Failure), + message: message.clone(), + metadata: None, + reason: message, + details: None, + code, + }, + types: request.types, + uid: request.uid, + converted_objects: vec![], + } + } + }; + response.into_review() + } + fn convert_objects( + objects: ::std::vec::Vec<::serde_json::Value>, + desired_api_version: &str, + ) -> ::std::result::Result< + ::std::vec::Vec<::serde_json::Value>, + ::stackable_versioned::ConvertObjectError, + > { + let desired_api_version = FooVersion::from_api_version(desired_api_version) + .map_err(|source| ::stackable_versioned::ConvertObjectError::ParseDesiredApiVersion { + source, + })?; + let mut converted_objects = ::std::vec::Vec::with_capacity(objects.len()); + for object in objects { + let current_object = Self::from_json_object(object.clone()) + .map_err(|source| ::stackable_versioned::ConvertObjectError::Parse { + source, + })?; + match (current_object, desired_api_version) { + (Self::V1Alpha1(__sv_foo), FooVersion::V1Alpha2) => { + let converted: v1alpha2::Foo = __sv_foo.into(); + let desired_object = Self::V1Alpha2(converted); + let desired_object = desired_object + .into_json_value() + .map_err(|source| ::stackable_versioned::ConvertObjectError::Serialize { + source, + })?; + converted_objects.push(desired_object); + } + (Self::V1Alpha2(__sv_foo), FooVersion::V1Alpha1) => { + let converted: v1alpha1::Foo = __sv_foo.into(); + let desired_object = Self::V1Alpha1(converted); + let desired_object = desired_object + .into_json_value() + .map_err(|source| ::stackable_versioned::ConvertObjectError::Serialize { + source, + })?; + converted_objects.push(desired_object); + } + _ => converted_objects.push(object), + } + } + ::std::result::Result::Ok(converted_objects) + } + fn from_json_object( + object_value: ::serde_json::Value, + ) -> ::std::result::Result { + let kind = object_value + .get("kind") + .ok_or_else(|| ::stackable_versioned::ParseObjectError::FieldNotPresent { + field: "kind".to_owned(), + })? + .as_str() + .ok_or_else(|| ::stackable_versioned::ParseObjectError::FieldNotStr { + field: "kind".to_owned(), + })?; + if kind != "Foo" { + return Err(::stackable_versioned::ParseObjectError::UnexpectedKind { + kind: kind.to_owned(), + expected: "Foo".to_owned(), + }); + } + let api_version = object_value + .get("apiVersion") + .ok_or_else(|| ::stackable_versioned::ParseObjectError::FieldNotPresent { + field: "apiVersion".to_owned(), + })? + .as_str() + .ok_or_else(|| ::stackable_versioned::ParseObjectError::FieldNotStr { + field: "apiVersion".to_owned(), + })?; + let object = match api_version { + "stackable.tech/v1alpha1" => { + let object = ::serde_json::from_value(object_value) + .map_err(|source| ::stackable_versioned::ParseObjectError::Deserialize { + source, + })?; + Self::V1Alpha1(object) + } + "stackable.tech/v1alpha2" => { + let object = ::serde_json::from_value(object_value) + .map_err(|source| ::stackable_versioned::ParseObjectError::Deserialize { + source, + })?; + Self::V1Alpha2(object) + } + unknown_api_version => { + return ::std::result::Result::Err(::stackable_versioned::ParseObjectError::UnknownApiVersion { + api_version: unknown_api_version.to_owned(), + }); + } + }; + ::std::result::Result::Ok(object) + } + fn into_json_value( + self, + ) -> ::std::result::Result<::serde_json::Value, ::serde_json::Error> { + match self { + Self::V1Alpha1(__sv_foo) => Ok(::serde_json::to_value(__sv_foo)?), + Self::V1Alpha2(__sv_foo) => Ok(::serde_json::to_value(__sv_foo)?), + } + } +} +#[automatically_derived] +#[derive(::core::marker::Copy, ::core::clone::Clone, ::core::fmt::Debug)] +pub(crate) enum FooVersion { + V1Alpha1, + V1Alpha2, +} +#[automatically_derived] +impl ::core::fmt::Display for FooVersion { + fn fmt( + &self, + f: &mut ::core::fmt::Formatter<'_>, + ) -> ::std::result::Result<(), ::std::fmt::Error> { + f.write_str(self.as_version_str()) + } +} +#[automatically_derived] +impl FooVersion { + pub fn as_version_str(&self) -> &str { + match self { + FooVersion::V1Alpha1 => "v1alpha1", + FooVersion::V1Alpha2 => "v1alpha2", + } + } + pub fn as_api_version_str(&self) -> &str { + match self { + FooVersion::V1Alpha1 => "stackable.tech/v1alpha1", + FooVersion::V1Alpha2 => "stackable.tech/v1alpha2", + } + } + pub fn from_api_version( + api_version: &str, + ) -> Result { + match api_version { + "stackable.tech/v1alpha1" => Ok(FooVersion::V1Alpha1), + "stackable.tech/v1alpha2" => Ok(FooVersion::V1Alpha2), + _ => { + Err(::stackable_versioned::UnknownDesiredApiVersionError { + api_version: api_version.to_owned(), + }) + } + } + } +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha2::FooSpec, + >( + stringify!(Foo), + "stackable.tech/v1alpha2", + "stackable.tech/v1alpha1", + Foo::try_convert, + ); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(Foo), + "stackable.tech/v1alpha1", + "stackable.tech/v1alpha2", + Foo::try_convert, + ); +} +#[automatically_derived] +#[derive( + ::core::clone::Clone, + ::core::default::Default, + ::core::fmt::Debug, + ::serde::Deserialize, + ::serde::Serialize, + ::schemars::JsonSchema +)] +#[serde(rename_all = "camelCase")] +pub struct FooStatusWithChangedValues { + pub changed_values: ::stackable_versioned::ChangedValues, +} +#[automatically_derived] +impl ::stackable_versioned::TrackingStatus for FooStatusWithChangedValues { + fn changes(&mut self) -> &mut ::stackable_versioned::ChangedValues { + &mut self.changed_values + } +} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@docs.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@docs.rs.snap index e79b8d287..34713f34b 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@docs.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@docs.rs.snap @@ -90,16 +90,24 @@ where } } #[automatically_derived] -impl ::core::convert::From for v1beta1::Bar { - fn from(__sv_bar: v1alpha1::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1beta1::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1alpha1::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1alpha1::Bar::Baz => v1beta1::Bar::Baz, } } } #[automatically_derived] -impl ::core::convert::From for v1alpha1::Bar { - fn from(__sv_bar: v1beta1::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1alpha1::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1beta1::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1beta1::Bar::Baz => v1alpha1::Bar::Baz, } @@ -182,16 +190,24 @@ where } } #[automatically_derived] -impl ::core::convert::From for v1beta2::Bar { - fn from(__sv_bar: v1beta1::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1beta2::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1beta1::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1beta1::Bar::Baz => v1beta2::Bar::Baz, } } } #[automatically_derived] -impl ::core::convert::From for v1beta1::Bar { - fn from(__sv_bar: v1beta2::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1beta1::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1beta2::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1beta2::Bar::Baz => v1beta1::Bar::Baz, } @@ -268,16 +284,24 @@ where } } #[automatically_derived] -impl ::core::convert::From for v1::Bar { - fn from(__sv_bar: v1beta2::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1beta2::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1beta2::Bar::Baz => v1::Bar::Baz, } } } #[automatically_derived] -impl ::core::convert::From for v1beta2::Bar { - fn from(__sv_bar: v1::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1beta2::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1::Bar::Baz => v1beta2::Bar::Baz, } @@ -354,16 +378,24 @@ where } } #[automatically_derived] -impl ::core::convert::From for v2::Bar { - fn from(__sv_bar: v1::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v2::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v1::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v1::Bar::Baz => v2::Bar::Baz, } } } #[automatically_derived] -impl ::core::convert::From for v1::Bar { - fn from(__sv_bar: v2::Bar) -> Self { +impl ::stackable_versioned::TrackingFrom for v1::Bar +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_bar: v2::Bar, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; match __sv_bar { v2::Bar::Baz => v1::Bar::Baz, } diff --git a/crates/stackable-versioned/CHANGELOG.md b/crates/stackable-versioned/CHANGELOG.md index 4dba782cd..272b3cb11 100644 --- a/crates/stackable-versioned/CHANGELOG.md +++ b/crates/stackable-versioned/CHANGELOG.md @@ -11,10 +11,24 @@ All notable changes to this project will be documented in this file. - Add `from_docs` argument to the `changed()` action to provide the previous doc comments of an item ([#1293]). - Add `#[versioned(hint(map))]` to provide a hint for map types during conversion ([#1285]). - Emit generics in plain `From` impl ([#1284]). +- Support tracking changes through enums when `experimental_conversion_tracking` is enabled. + Variants containing versioned structs or enums need to be marked with `#[versioned(nested)]` + ([#YYYY]). + +### Changed + +- BREAKING: With `experimental_conversion_tracking` enabled, versioned enums only implement + `TrackingFrom` instead of `From`, in line with structs. Fields using versioned enums need to be + marked with `#[versioned(nested)]` ([#YYYY]). + +### Fixed + +- Fix `From` impls of enum variants with multiple unnamed fields ([#YYYY]). [#1284]: https://github.com/stackabletech/operator-rs/pull/1284 [#1285]: https://github.com/stackabletech/operator-rs/pull/1285 [#1293]: https://github.com/stackabletech/operator-rs/pull/1293 +[#YYYY]: https://github.com/stackabletech/operator-rs/pull/YYYY ## [0.11.1] - 2026-07-06 diff --git a/crates/stackable-versioned/tests/enum_tracking.rs b/crates/stackable-versioned/tests/enum_tracking.rs new file mode 100644 index 000000000..d5846f53a --- /dev/null +++ b/crates/stackable-versioned/tests/enum_tracking.rs @@ -0,0 +1,131 @@ +use kube::{ + CustomResource, + core::{conversion::ConversionReview, response::StatusSummary}, +}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use stackable_versioned::versioned; + +#[versioned( + version(name = "v1alpha1"), + version(name = "v1alpha2"), + options(k8s(experimental_conversion_tracking)) +)] +pub mod versioned { + #[versioned(crd(group = "test.stackable.tech", doc = "Test"))] + #[derive(Clone, Debug, CustomResource, Deserialize, JsonSchema, Serialize)] + #[serde(rename_all = "camelCase")] + pub struct CatalogSpec { + #[versioned(nested)] + connector: Connector, + } + + #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] + #[serde(rename_all = "camelCase")] + pub enum Connector { + #[versioned(nested)] + Iceberg(IcebergConnector), + + Tpch(TpchConnector), + } + + #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] + #[serde(rename_all = "camelCase")] + pub struct IcebergConnector { + metastore: Option, + + #[versioned(added(since = "v1alpha2"))] + rest_catalog_uri: Option, + } +} + +#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] +pub struct TpchConnector {} + +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::CatalogSpec { + fn roundtrip_test_data() -> Vec { + vec![ + Self { + connector: v1alpha1::Connector::Iceberg(v1alpha1::IcebergConnector { + metastore: Some("hive".to_owned()), + }), + }, + Self { + connector: v1alpha1::Connector::Tpch(TpchConnector {}), + }, + ] + } +} + +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha2::CatalogSpec { + fn roundtrip_test_data() -> Vec { + vec![ + // The REST catalog URI doesn't exist in v1alpha1. It is tracked in the status and + // restored when upgrading again. + Self { + connector: v1alpha2::Connector::Iceberg(v1alpha2::IcebergConnector { + metastore: None, + rest_catalog_uri: Some("http://rest-catalog:8181".to_owned()), + }), + }, + Self { + connector: v1alpha2::Connector::Iceberg(v1alpha2::IcebergConnector { + metastore: Some("hive".to_owned()), + rest_catalog_uri: None, + }), + }, + Self { + connector: v1alpha2::Connector::Tpch(TpchConnector {}), + }, + ] + } +} + +#[test] +fn tracks_values_through_enum_variants() { + let review: ConversionReview = serde_json::from_value(serde_json::json!({ + "kind": "ConversionReview", + "apiVersion": "apiextensions.k8s.io/v1", + "request": { + "uid": "c4e55572-ee1f-4e94-9097-28936985d45f", + "desiredAPIVersion": "test.stackable.tech/v1alpha1", + "objects": [{ + "apiVersion": "test.stackable.tech/v1alpha2", + "kind": "Catalog", + "metadata": {}, + "spec": { + "connector": { + "iceberg": { + "metastore": null, + "restCatalogUri": "http://rest-catalog:8181" + } + } + } + }] + } + })) + .expect("conversion review must be valid"); + + let response = Catalog::try_convert(review) + .response + .expect("v1alpha1 review must have a response"); + + assert_eq!(response.result.status, Some(StatusSummary::Success)); + + let object = response + .converted_objects + .first() + .expect("there must be at least one object"); + + assert_eq!( + object["spec"]["connector"]["iceberg"], + serde_json::json!({ "metastore": null }) + ); + assert_eq!( + object["status"]["changedValues"]["upgrades"]["v1alpha2"], + serde_json::json!([{ + "jsonPath": "$.connector.Iceberg.rest_catalog_uri", + "value": "http://rest-catalog:8181" + }]) + ); +} From a6513532122bc1055c8c21c235dbe42b5e083849 Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 16 Oct 2026 10:30:01 +0200 Subject: [PATCH 2/3] refactor(versioned): Cleanup and improve code, leave TODO comments --- .../src/codegen/container/enum.rs | 94 +++++++++++------- .../src/codegen/container/struct/mod.rs | 2 + .../src/codegen/item/field.rs | 4 + .../src/codegen/item/variant.rs | 95 ++++++++++++------- ...ots__pass@conversion_tracking_enum.rs.snap | 18 ++-- .../tests/trybuild.rs | 1 + .../tests/enum_tracking.rs | 2 +- 7 files changed, 141 insertions(+), 75 deletions(-) diff --git a/crates/stackable-versioned-macros/src/codegen/container/enum.rs b/crates/stackable-versioned-macros/src/codegen/container/enum.rs index 1730b3207..28d4eb9aa 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/enum.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/enum.rs @@ -1,4 +1,4 @@ -use std::ops::Not; +use std::{cell::OnceCell, ops::Not}; use darling::{FromAttributes, Result}; use proc_macro2::TokenStream; @@ -50,6 +50,7 @@ impl Container { }; Ok(Self::Enum(Enum { + tracking_generics: OnceCell::new(), generics: item_enum.generics, variants: versioned_variants, common, @@ -68,6 +69,17 @@ pub struct Enum { /// Generic types of the enum pub generics: Generics, + + /// Generic types of the enum which additionally also includes the tracking state generic type + /// and where clause. + /// + /// We use a [`OnceCell`] here to avoid outer mutability and instead rely on interior mutability + /// to lazily initialize this field. This is done to avoid the macro the re-clone and + /// re-construct these generics over and over again. + /// + /// SAFETY: Using [`OnceCell`] instead of `OnceLock` is fine here, because proc-macros run in a + /// single thread. + tracking_generics: OnceCell, } // Common token generation @@ -75,7 +87,7 @@ impl Enum { pub fn generate_tokens<'a>( &'a self, versions: &'a [VersionDefinition], - gen_ctx: ModuleGenerationContext<'a>, + mod_gen_ctx: ModuleGenerationContext<'a>, ) -> ContainerTokens<'a> { let mut versions = versions.iter().peekable(); let mut container_tokens = ContainerTokens::default(); @@ -85,8 +97,9 @@ impl Enum { let ver_ctx = VersionContext::new(version, next_version); let enum_definition = self.generate_definition(ver_ctx); - let upgrade_from = self.generate_from_impl(Direction::Upgrade, ver_ctx, gen_ctx); - let downgrade_from = self.generate_from_impl(Direction::Downgrade, ver_ctx, gen_ctx); + let upgrade_from = self.generate_from_impl(Direction::Upgrade, ver_ctx, mod_gen_ctx); + let downgrade_from = + self.generate_from_impl(Direction::Downgrade, ver_ctx, mod_gen_ctx); container_tokens .extend_inner(&version.inner, enum_definition) @@ -121,11 +134,11 @@ impl Enum { } // TODO (@Techassi): Add doc comments - pub fn generate_from_impl( + fn generate_from_impl( &self, direction: Direction, ver_ctx: VersionContext<'_>, - gen_ctx: ModuleGenerationContext<'_>, + mod_gen_ctx: ModuleGenerationContext<'_>, ) -> Option { if ver_ctx.version.skip_from || self.common.options.skip_from { return None; @@ -154,7 +167,7 @@ impl Enum { // Only add the #[automatically_derived] attribute only if this impl is used // outside of a module (in standalone mode). - let automatically_derived = gen_ctx.add_attributes + let automatically_derived = mod_gen_ctx.add_attributes .not() .then(|| quote! {#[automatically_derived]}); @@ -162,7 +175,7 @@ impl Enum { self.variants .iter() .filter_map(|v| { - v.generate_for_from_impl(direction, version, next_version, enum_ident, gen_ctx) + v.generate_for_from_impl(direction, version, next_version, enum_ident, mod_gen_ctx) }) .collect() }; @@ -185,18 +198,14 @@ impl Enum { // With conversion tracking enabled, the enum needs to forward the status and the // current path to the data of nested variants. As such, only a TrackingFrom impl is // generated, which is in line with the impls generated for structs. - if gen_ctx.kubernetes_options.experimental_conversion_tracking.is_present() { - let versioned_path = &*gen_ctx.crates.versioned; - - let mut tracking_generics = self.generics.clone(); - tracking_generics.params.push(parse_quote! { S }); - tracking_generics - .make_where_clause() - .predicates - .push(parse_quote! { S: #versioned_path::TrackingStatus + ::core::default::Default }); - let (impl_generics, _, where_clause) = tracking_generics.split_for_impl(); - - return quote! { + // TODO (@Techassi): This currently does NOT support tracking changes to the enum itself, + // only changes in nested structs are tracked. + if mod_gen_ctx.kubernetes_options.experimental_conversion_tracking.is_present() { + let versioned_path = &*mod_gen_ctx.crates.versioned; + + let (impl_generics, _, where_clause) = self.get_tracking_generics(mod_gen_ctx).split_for_impl(); + + quote! { #automatically_derived #allow_attribute impl #impl_generics #versioned_path::TrackingFrom<#from_module_ident::#enum_ident #type_generics, S> for #for_module_ident::#enum_ident #type_generics @@ -211,18 +220,18 @@ impl Enum { } } } - }; - } - - quote! { - #automatically_derived - #allow_attribute - impl #impl_generics ::core::convert::From<#from_module_ident::#enum_ident #type_generics> for #for_module_ident::#enum_ident #type_generics - #where_clause - { - fn from(#from_enum_ident: #from_module_ident::#enum_ident #type_generics) -> Self { - match #from_enum_ident { - #variants + } + } else { + quote! { + #automatically_derived + #allow_attribute + impl #impl_generics ::core::convert::From<#from_module_ident::#enum_ident #type_generics> for #for_module_ident::#enum_ident #type_generics + #where_clause + { + fn from(#from_enum_ident: #from_module_ident::#enum_ident #type_generics) -> Self { + match #from_enum_ident { + #variants + } } } } @@ -230,6 +239,27 @@ impl Enum { }) } + /// Construct (if needed) and return the [`Enum`]'s generics extended with the tracking status + /// generic type and where clause. + /// + /// This function lazily initializes the underlying field in [`Enum`]. Afterwards, the value is + /// re-used without constructing the value from scratch over and over again. This is achieved + /// through interior mutability via [`OnceCell`]. See [`Enum`]'s `tracking_generics` field for + /// more details. + pub fn get_tracking_generics(&self, mod_gen_ctx: ModuleGenerationContext<'_>) -> &Generics { + self.tracking_generics.get_or_init(|| { + let versioned_path = &*mod_gen_ctx.crates.versioned; + + let mut tracking_generics = self.generics.clone(); + tracking_generics.params.push(parse_quote! { S }); + tracking_generics.make_where_clause().predicates.push( + parse_quote! { S: #versioned_path::TrackingStatus + ::core::default::Default }, + ); + + tracking_generics + }) + } + /// Returns whether any variant is deprecated in the provided `version`. fn is_any_variant_deprecated(&self, version: &VersionDefinition) -> bool { // First, iterate over all variants. The `any` function will return true diff --git a/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs b/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs index 61f435e73..922e2d3f6 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs @@ -478,6 +478,8 @@ impl Struct { ) -> TokenStream { // TODO (@Techassi): A bunch this stuff is duplicated in self.generate_tracking_from_impl. // Ideally we remove that duplication. + // The enum's codegen code removes a whole bunch of the duplication. Implement something + // similar here. let (impl_generics, type_generics, where_clause) = self.generics.split_for_impl(); let from_struct_ident = &self.common.idents.parameter; diff --git a/crates/stackable-versioned-macros/src/codegen/item/field.rs b/crates/stackable-versioned-macros/src/codegen/item/field.rs index 38c2010a7..c14fcd92d 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/field.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/field.rs @@ -426,6 +426,10 @@ impl VersionedField { custom_conversion_function: Option<&Path>, ) -> TokenStream { match custom_conversion_function { + // TODO (@Techassi): We should also support tracking here. Currently, the nested hint is + // ignored and the generated code doesn't force the custom conversion function to accept + // a tracking status. + // // The user specified a custom conversion function which will be used here instead of the // default conversion call which utilizes From impls. Some(convert_fn) => quote! { diff --git a/crates/stackable-versioned-macros/src/codegen/item/variant.rs b/crates/stackable-versioned-macros/src/codegen/item/variant.rs index b2661c624..093252ef9 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/variant.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/variant.rs @@ -155,6 +155,11 @@ impl VersionedVariant { } } + // TODO (@Techassi): Make this section of code more similar to the VersionStruct/VersionedField + // code. Also see if we can change VersionedVariant's fields field contain VersionedFields + // instead. This would allow us to hook into already existing machinery. The biggest blocker for + // this is currently that VersionedField assumes there is always an ident. This is not the case + // unnamed variant fields. pub fn generate_for_from_impl( &self, direction: Direction, @@ -213,27 +218,63 @@ impl VersionedVariant { } } + fn generate_from_fields(&self) -> Option { + match &self.fields { + Fields::Named(fields_named) => { + let fields = Self::named_field_idents(fields_named); + Some(quote! { { #(#fields,)* } }) + } + Fields::Unnamed(fields_unnamed) => { + let fields = Self::unnamed_field_ident(fields_unnamed); + Some(quote! { ( #(#fields),* ) }) + } + Fields::Unit => None, + } + } + fn generate_for_fields(&self, mod_gen_ctx: ModuleGenerationContext<'_>) -> Option { match &self.fields { Fields::Named(fields_named) => { let fields = Self::named_field_idents(fields_named); - let conversions = fields.iter().map(|field| { - self.generate_conversion_function(Some(&field.to_string()), mod_gen_ctx) - }); - Some(quote! { { #(#fields: #fields.#conversions,)* } }) + if self.nested { + let funcs = fields.iter().map(|field| { + self.generate_tracking_conversion_function_for_field( + // NOTE (@Techassi): Switching to VersionedField would also have the + // benefit that we can use IdentString here, which has as_str(). + &field.to_string(), + mod_gen_ctx, + ) + }); + + Some(quote! { { #(#fields: #fields.#funcs,)* } }) + } else { + let funcs = fields + .iter() + .map(|_| self.generate_conversion_function_for_field()); + + Some(quote! { { #(#fields: #fields.#funcs,)* } }) + } } Fields::Unnamed(fields_unnamed) => { let fields = Self::unnamed_field_ident(fields_unnamed); - // Newtype variants (which are the most common variants with data) don't need an - // additional path segment, as the variant only contains a single field. - let conversions = (0..fields.len()).map(|index| { - let child = (fields.len() > 1).then(|| index.to_string()); - self.generate_conversion_function(child.as_deref(), mod_gen_ctx) - }); - - Some(quote! { ( #(#fields.#conversions),* ) }) + if self.nested { + let funcs = (0..fields.len()).map(|index| { + self.generate_tracking_conversion_function_for_field( + &index.to_string(), + mod_gen_ctx, + ) + }); + + Some(quote! { ( #(#fields.#funcs),* ) }) + } else { + let funcs = fields + .iter() + .map(|_| self.generate_conversion_function_for_field()); + + Some(quote! { ( #(#fields.#funcs),* ) }) + } } Fields::Unit => None, } @@ -243,37 +284,25 @@ impl VersionedVariant { /// /// The data of variants marked as nested is converted with support for tracking. The path /// passed down consists of the variant name and the provided `child`, if any. - fn generate_conversion_function( + fn generate_tracking_conversion_function_for_field( &self, - child: Option<&str>, + field_ident_or_index: &str, mod_gen_ctx: ModuleGenerationContext<'_>, ) -> TokenStream { - if !self.nested { - return quote! { into() }; - } + // TODO (@Techassi): Add support for hints let versioned_path = &*mod_gen_ctx.crates.versioned; let variant = &self.idents.original; - let child_string = match child { - Some(child) => format!("{variant}.{child}"), - None => variant.to_string(), - }; + let child_string = format!("{variant}.{field_ident_or_index}"); quote! { tracking_into(status, &#versioned_path::jthong_path(parent, #child_string)) } } - fn generate_from_fields(&self) -> Option { - match &self.fields { - Fields::Named(fields_named) => { - let fields = Self::named_field_idents(fields_named); - Some(quote! { { #(#fields,)* } }) - } - Fields::Unnamed(fields_unnamed) => { - let fields = Self::unnamed_field_ident(fields_unnamed); - Some(quote! { ( #(#fields),* ) }) - } - Fields::Unit => None, - } + // NOTE (@Techassi): We will need self in the future to support hints. + #[allow(clippy::unused_self)] + fn generate_conversion_function_for_field(&self) -> TokenStream { + // TODO (@Techassi): Add support for hints + quote! { into() } } fn named_field_idents(fields_named: &FieldsNamed) -> Vec<&Ident> { diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap index 40f7abe7f..22bab38fa 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap @@ -121,7 +121,7 @@ where __sv_0 .tracking_into( status, - &::stackable_versioned::jthong_path(parent, "Iceberg"), + &::stackable_versioned::jthong_path(parent, "Iceberg.0"), ), ) } @@ -174,7 +174,7 @@ where __sv_0 .tracking_into( status, - &::stackable_versioned::jthong_path(parent, "Iceberg"), + &::stackable_versioned::jthong_path(parent, "Iceberg.0"), ), ) } @@ -234,7 +234,7 @@ where for ::stackable_versioned::ChangedValue { json_path, value } in upgrades { match json_path { json_path if json_path == __sv_rest_catalog_uri_path => { - spec.rest_catalog_uri = serde_yaml::from_value(value).unwrap(); + spec.rest_catalog_uri = ::yaml_serde::from_value(value).unwrap(); } _ => unreachable!(), } @@ -267,7 +267,7 @@ where upgrades .push(::stackable_versioned::ChangedValue { json_path: __sv_rest_catalog_uri_path, - value: ::serde_yaml::to_value(&__sv_icebergconnector.rest_catalog_uri) + value: ::yaml_serde::to_value(&__sv_icebergconnector.rest_catalog_uri) .unwrap(), }); let mut spec = Self { @@ -328,13 +328,13 @@ impl Foo { stored_apiversion.as_version_str(), ) } - ///Tries to convert a list of objects of kind [`Foo`] to the desired API version - ///specified in the [`ConversionReview`][cr]. + /// Tries to convert a list of objects of kind [`Foo`] to the desired API version + /// specified in the [`ConversionReview`][cr]. /// - ///The returned [`ConversionReview`][cr] either indicates a success or a failure, which - ///is handed back to the Kubernetes API server. + /// The returned [`ConversionReview`][cr] either indicates a success or a failure, which + /// is handed back to the Kubernetes API server. /// - ///[cr]: ::kube::core::conversion::ConversionReview + /// [cr]: ::kube::core::conversion::ConversionReview pub fn try_convert( review: ::kube::core::conversion::ConversionReview, ) -> ::kube::core::conversion::ConversionReview { diff --git a/crates/stackable-versioned-macros/tests/trybuild.rs b/crates/stackable-versioned-macros/tests/trybuild.rs index 25e9e9b4e..ce9bbd298 100644 --- a/crates/stackable-versioned-macros/tests/trybuild.rs +++ b/crates/stackable-versioned-macros/tests/trybuild.rs @@ -21,6 +21,7 @@ mod inputs { // mod basic; // mod conversion_hints; // mod conversion_tracking_hints; + // mod conversion_tracking_enum; // mod conversion_tracking; // mod crate_overrides; // mod crate_overrides_only_kube; diff --git a/crates/stackable-versioned/tests/enum_tracking.rs b/crates/stackable-versioned/tests/enum_tracking.rs index d5846f53a..67ad914e2 100644 --- a/crates/stackable-versioned/tests/enum_tracking.rs +++ b/crates/stackable-versioned/tests/enum_tracking.rs @@ -124,7 +124,7 @@ fn tracks_values_through_enum_variants() { assert_eq!( object["status"]["changedValues"]["upgrades"]["v1alpha2"], serde_json::json!([{ - "jsonPath": "$.connector.Iceberg.rest_catalog_uri", + "jsonPath": "$.connector.Iceberg.0.rest_catalog_uri", "value": "http://rest-catalog:8181" }]) ); From dcd17beb51e363912c68ec49531580074ce1dabb Mon Sep 17 00:00:00 2001 From: Techassi Date: Fri, 16 Oct 2026 10:39:49 +0200 Subject: [PATCH 3/3] fix: Bump rustls to 0.23.46 to negate RUSTSEC-2026-0346 and RUSTSEC-2026-0347 --- Cargo.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cca347c60..b6bc7b791 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3316,9 +3316,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.45" +version = "0.23.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" +checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" dependencies = [ "log", "once_cell", @@ -4159,7 +4159,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom 0.3.4", "once_cell", "rustix", "windows-sys 0.61.2",