diff --git a/Cargo.lock b/Cargo.lock index db3144c69..9997a3313 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3952,7 +3952,6 @@ dependencies = [ "schemars", "serde", "serde_json", - "serde_yaml", "snafu 0.9.2", "stackable-versioned-macros", ] diff --git a/crates/stackable-versioned-macros/src/attrs/item/mod.rs b/crates/stackable-versioned-macros/src/attrs/item/mod.rs index 4c8e2dfcf..fe63e28d3 100644 --- a/crates/stackable-versioned-macros/src/attrs/item/mod.rs +++ b/crates/stackable-versioned-macros/src/attrs/item/mod.rs @@ -8,7 +8,7 @@ use syn::{Attribute, Path, Type, spanned::Spanned}; use crate::{ codegen::{VersionDefinition, item::ItemStatus}, - utils::ItemIdents, + utils::{ItemIdents, doc_comments::DocComments as _}, }; mod field; @@ -223,9 +223,12 @@ impl CommonItemAttributes { let mut errors = Error::accumulator(); for change in &self.changes { - if change.from_name.is_none() && change.from_type.is_none() { + if change.from_name.is_none() + && change.from_type.is_none() + && change.from_docs.is_none() + { errors.push(Error::custom( - "both `from_name` and `from_type` are unset. Is this `changed()` action needed?" + "`from_name`, `from_type` and `from_docs` are unset. Is this `changed()` action needed?" ).with_span(&change.since.span())); } @@ -288,6 +291,18 @@ impl CommonItemAttributes { } impl CommonItemAttributes { + /// Returns the doc comments of the item before each change which provides `from_docs`, keyed + /// by the version of the change. + pub fn previous_docs(&self) -> BTreeMap> { + self.changes + .iter() + .filter_map(|change| { + let docs = change.from_docs.as_deref()?; + Some((*change.since, docs.as_str().into_doc_comments())) + }) + .collect() + } + #[expect(clippy::too_many_lines)] pub fn into_changeset( self, @@ -458,11 +473,13 @@ fn default_default_fn() -> SpannedValue { /// - `changed(since = "...", from_name = "...", from_type="...")` /// - `changed(since = "...", from_name = "...", from_type="...", upgrade_with = "...")` /// - `changed(since = "...", from_name = "...", from_type="...", downgrade_with = "...")` +/// - `changed(since = "...", from_docs = "...")` #[derive(Clone, Debug, FromMeta)] pub struct ChangedAttributes { pub since: SpannedValue, pub from_name: Option>, pub from_type: Option>, + pub from_docs: Option>, pub upgrade_with: Option>, pub downgrade_with: Option>, } 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/container/struct/conversion.rs b/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs index 92bb09c52..2f0657801 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs @@ -170,8 +170,11 @@ impl Struct { // Ideally we would integrate with the serde(rename) functionality to produce these field // names. let inserts = self.generate_tracking_inserts(direction, next_version, mod_gen_ctx); + let post_inserts = + self.generate_tracking_post_inserts(direction, next_version, mod_gen_ctx); let removals = self.generate_tracking_removals(direction, next_version, mod_gen_ctx); let json_paths = self.generate_json_paths(next_version, mod_gen_ctx); + let tracked_values = self.generate_tracked_values(direction, next_version, mod_gen_ctx); // TODO (@Techassi): Re-add support for generics // TODO (@Techassi): We know the status, so we can hard-code it, but hard to track across structs @@ -192,10 +195,18 @@ impl Struct { // the upgrade or downgrade section. Only then we can convert the spec. #inserts + // Fields which changed their type are consumed by the conversion, so their + // values need to be serialized before converting the spec. + #tracked_values + let mut spec = Self { #fields }; + // Fields which changed their type can only be inserted into the status after + // the spec is converted, because the downgraded value is tracked as well. + #post_inserts + // After the spec is converted, depending on the direction, we need to apply // changed values from either the upgrade or downgrade section. Afterwards // we can return the successfully converted spec and the status contains @@ -237,6 +248,12 @@ impl Struct { }) .collect(); + // This is the case if only fields which changed their type need to be tracked. + // These are inserted after the conversion, see generate_tracking_post_inserts. + if inserts.is_empty() { + return None; + } + Some(quote! { let upgrades = status .changes() @@ -250,6 +267,42 @@ impl Struct { } } + fn generate_tracking_post_inserts( + &self, + direction: Direction, + next_version: &VersionDefinition, + mod_gen_ctx: ModuleGenerationContext<'_>, + ) -> Option { + match direction { + Direction::Upgrade => None, + Direction::Downgrade => { + let next_version_string = next_version.inner.to_string(); + + let post_inserts: TokenStream = self + .fields + .iter() + .filter_map(|f| { + f.generate_for_status_post_insertion(direction, next_version, mod_gen_ctx) + }) + .collect(); + + if post_inserts.is_empty() { + return None; + } + + Some(quote! { + let upgrades = status + .changes() + .upgrades + .entry(#next_version_string.to_owned()) + .or_default(); + + #post_inserts + }) + } + } + } + fn generate_tracking_removals( &self, direction: Direction, @@ -263,7 +316,7 @@ impl Struct { let match_arms: TokenStream = self .fields .iter() - .filter_map(|f| f.generate_for_status_removal(direction, next_version)) + .filter_map(|f| f.generate_for_status_removal(direction, next_version, mod_gen_ctx)) .collect(); match direction { @@ -271,10 +324,24 @@ impl Struct { let next_version_string = next_version.inner.to_string(); let versioned_path = &*mod_gen_ctx.crates.versioned; + // The downgraded value is only needed by fields which changed their type. Binding + // it unconditionally would result in an unused variable otherwise. + let has_type_changes = self.fields.iter().any(|f| { + f.changes.as_ref().is_some_and(|c| { + c.value_is(&next_version.inner, ItemStatus::is_type_change) + }) + }); + + let downgraded_value = if has_type_changes { + quote! { downgraded_value } + } else { + quote! { .. } + }; + Some(quote! { // NOTE (@Techassi): This is an awkward thing to do. Can we possibly use &str for the keys here? if let Some(upgrades) = status.changes().upgrades.remove(&#next_version_string.to_owned()) { - for #versioned_path::ChangedValue { json_path, value } in upgrades { + for #versioned_path::ChangedValue { json_path, value, #downgraded_value } in upgrades { match json_path { #match_arms _ => unreachable!(), @@ -299,17 +366,37 @@ impl Struct { .collect() } + fn generate_tracked_values( + &self, + direction: Direction, + next_version: &VersionDefinition, + mod_gen_ctx: ModuleGenerationContext<'_>, + ) -> TokenStream { + let from_struct_ident = &self.common.idents.parameter; + + self.fields + .iter() + .filter_map(|f| { + f.generate_for_tracked_value( + direction, + next_version, + from_struct_ident, + mod_gen_ctx, + ) + }) + .collect() + } + pub(super) fn needs_tracking(&self, version: &VersionDefinition) -> bool { self.fields.iter().any(|f| { f.changes.as_ref().is_some_and(|c| { c.value_is(&version.inner, |s| { - // For now, only added fields need to be tracked. In the future, removals and - // type changes also need to be tracked + // For now, only added fields and fields which changed their type need to be + // tracked. In the future, removals also need to be tracked. match s { ItemStatus::Addition { .. } => true, - // TODO (@Techassi): Support tracking for changed fields - ItemStatus::Change { .. } - | ItemStatus::Deprecation { .. } + ItemStatus::Change { .. } => s.is_type_change(), + ItemStatus::Deprecation { .. } | ItemStatus::NoChange { .. } | ItemStatus::NotPresent => false, } diff --git a/crates/stackable-versioned-macros/src/codegen/item/field.rs b/crates/stackable-versioned-macros/src/codegen/item/field.rs index ea074aeea..dc49fb873 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/field.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/field.rs @@ -11,7 +11,7 @@ use crate::{ codegen::{ Direction, VersionDefinition, changes::{BTreeMapExt, ChangesetExt}, - item::ItemStatus, + item::{ItemStatus, generate_attributes}, module::ModuleGenerationContext, }, utils::{ItemIdentExt, ItemIdents}, @@ -20,6 +20,7 @@ use crate::{ #[derive(Debug)] pub struct VersionedField { pub original_attributes: Vec, + pub previous_docs: BTreeMap>, pub changes: Option>, pub idents: FieldIdents, pub hint: Option, @@ -45,6 +46,7 @@ impl VersionedField { })?; let idents = FieldIdents::from(ident); + let previous_docs = field_attributes.common.previous_docs(); let changes = field_attributes .common .into_changeset(&idents, field.ty.clone()); @@ -53,6 +55,7 @@ impl VersionedField { Ok(Self { original_attributes: field_attributes.attrs, hint: field_attributes.hint, + previous_docs, ty: field.ty, changes, idents, @@ -82,7 +85,11 @@ impl VersionedField { /// } /// ``` pub fn generate_for_container(&self, version: &VersionDefinition) -> Option { - let original_attributes = &self.original_attributes; + let attributes = generate_attributes( + &self.original_attributes, + &self.previous_docs, + &version.inner, + ); #[allow(clippy::single_match_else)] match &self.changes { @@ -106,13 +113,13 @@ impl VersionedField { ) }) { ItemStatus::Addition { ident, ty, .. } => Some(quote! { - #(#original_attributes)* + #attributes pub #ident: #ty, }), ItemStatus::Change { to_ident, to_type, .. } => Some(quote! { - #(#original_attributes)* + #attributes pub #to_ident: #to_type, }), ItemStatus::Deprecation { @@ -133,7 +140,7 @@ impl VersionedField { }; Some(quote! { - #(#original_attributes)* + #attributes #deprecated_attr pub #field_ident: #field_type, }) @@ -149,7 +156,7 @@ impl VersionedField { let deprecated_attr = previously_deprecated.then(|| quote! {#[deprecated]}); Some(quote! { - #(#original_attributes)* + #attributes #deprecated_attr pub #ident: #ty, }) @@ -163,7 +170,7 @@ impl VersionedField { let field_type = &self.ty; Some(quote! { - #(#original_attributes)* + #attributes pub #field_ident: #field_type, }) } @@ -299,7 +306,7 @@ impl VersionedField { Direction::Downgrade => { let next_change = changes.get_expect(&next_version.inner); - let serde_yaml_path = &*mod_gen_ctx.crates.serde_yaml; + let serde_json_path = &*mod_gen_ctx.crates.serde_json; let versioned_path = &*mod_gen_ctx.crates.versioned; match next_change { @@ -311,7 +318,90 @@ impl VersionedField { Some(quote! { upgrades.push(#versioned_path::ChangedValue { json_path: #json_path_ident, - value: #serde_yaml_path::to_value(&#from_struct_ident.#ident).unwrap(), + value: #serde_json_path::to_value(&#from_struct_ident.#ident).unwrap(), + downgraded_value: ::core::option::Option::None, + }); + }) + } + _ => None, + } + } + } + } + + /// Generates code which serializes the value of this field before it is converted. This is only + /// needed for fields which changed their type, because the conversion consumes the value. + /// + /// - When downgrading, this is the value of the newer type, which is tracked in the status. + /// - When upgrading, this is the value of the older type, which is compared against the tracked + /// downgraded value to detect if a user changed the field in the older version. + pub fn generate_for_tracked_value( + &self, + direction: Direction, + next_version: &VersionDefinition, + from_struct_ident: &IdentString, + mod_gen_ctx: ModuleGenerationContext<'_>, + ) -> Option { + let changes = self.changes.as_ref()?; + let next_change = changes.get_expect(&next_version.inner); + + match next_change { + ItemStatus::Change { + from_ident, + to_ident, + .. + } if next_change.is_type_change() => { + let serde_json_path = &*mod_gen_ctx.crates.serde_json; + let value_ident = to_ident.tracked_value_ident(); + + let field_ident = match direction { + Direction::Upgrade => from_ident, + Direction::Downgrade => to_ident, + }; + + Some(quote! { + let #value_ident = #serde_json_path::to_value(&#from_struct_ident.#field_ident).unwrap(); + }) + } + _ => None, + } + } + + /// Generates code needed when a tracked type change of this field needs to be inserted into the + /// status. In contrast to added fields, this can only be done after the conversion, because the + /// downgraded value is tracked as well. + pub fn generate_for_status_post_insertion( + &self, + direction: Direction, + next_version: &VersionDefinition, + mod_gen_ctx: ModuleGenerationContext<'_>, + ) -> Option { + let changes = self.changes.as_ref()?; + + match direction { + Direction::Upgrade => None, + Direction::Downgrade => { + let next_change = changes.get_expect(&next_version.inner); + + let serde_json_path = &*mod_gen_ctx.crates.serde_json; + let versioned_path = &*mod_gen_ctx.crates.versioned; + + match next_change { + ItemStatus::Change { + from_ident, + to_ident, + .. + } if next_change.is_type_change() => { + let json_path_ident = to_ident.json_path_ident(); + let value_ident = to_ident.tracked_value_ident(); + + Some(quote! { + upgrades.push(#versioned_path::ChangedValue { + json_path: #json_path_ident, + value: #value_ident, + downgraded_value: ::core::option::Option::Some( + #serde_json_path::to_value(&spec.#from_ident).unwrap() + ), }); }) } @@ -327,6 +417,7 @@ impl VersionedField { &self, direction: Direction, next_version: &VersionDefinition, + mod_gen_ctx: ModuleGenerationContext<'_>, ) -> Option { // If there are no changes for this field, there is also no need to generate a match arm // for applying a tracked value. @@ -335,16 +426,33 @@ impl VersionedField { match direction { Direction::Upgrade => { let next_change = changes.get_expect(&next_version.inner); + let serde_json_path = &*mod_gen_ctx.crates.serde_json; match next_change { - // NOTE (@Techassi): We currently only support tracking added fields. As such - // we only need to generate code if the next change is "Addition". ItemStatus::Addition { ident, .. } => { let json_path_ident = ident.json_path_ident(); Some(quote! { json_path if json_path == #json_path_ident => { - spec.#ident = serde_yaml::from_value(value).unwrap(); + spec.#ident = #serde_json_path::from_value(value).unwrap(); + }, + }) + } + // The tracked value is only applied if the field still contains the value it + // was downgraded to. Otherwise, a user changed the field in the older version + // and that change takes precedence over the tracked value. + // + // A downgraded value of null is serialized as `downgradedValue: null`, which is + // deserialized as None. As such, a missing downgraded value is treated as null. + ItemStatus::Change { to_ident, .. } if next_change.is_type_change() => { + let json_path_ident = to_ident.json_path_ident(); + let value_ident = to_ident.tracked_value_ident(); + + Some(quote! { + json_path if json_path == #json_path_ident => { + if downgraded_value.unwrap_or_default() == #value_ident { + spec.#to_ident = #serde_json_path::from_value(value).unwrap(); + } }, }) } @@ -383,17 +491,18 @@ impl VersionedField { (Some(changes), _) => { let next_change = changes.get_expect(&next_version.inner); - match next_change { - ItemStatus::Addition { ident, .. } => { - let field_ident = ident.json_path_ident(); - let child_string = ident.to_string(); + let ident = match next_change { + ItemStatus::Addition { ident, .. } => ident, + ItemStatus::Change { to_ident, .. } if next_change.is_type_change() => to_ident, + _ => return None, + }; - Some(quote! { - let #field_ident = #versioned_path::jthong_path(parent, #child_string); - }) - } - _ => None, - } + let field_ident = ident.json_path_ident(); + let child_string = ident.to_string(); + + Some(quote! { + let #field_ident = #versioned_path::jthong_path(parent, #child_string); + }) } } } diff --git a/crates/stackable-versioned-macros/src/codegen/item/mod.rs b/crates/stackable-versioned-macros/src/codegen/item/mod.rs index 46abd7df8..f3093dd9d 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/mod.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/mod.rs @@ -1,5 +1,12 @@ +use std::{collections::BTreeMap, ops::Bound}; + use darling::util::IdentString; -use syn::{Path, Type}; +use k8s_version::Version; +use proc_macro2::TokenStream; +use quote::quote; +use syn::{Attribute, Meta, Path, Type}; + +use crate::codegen::changes::Neighbors as _; mod field; pub use field::*; @@ -7,6 +14,31 @@ pub use field::*; mod variant; pub use variant::*; +/// Generates the attributes of an item (field or variant) for the provided `version`. +/// +/// If the docs of the item are changed in a later version (via `from_docs`), the original doc +/// comments are replaced by the docs which are valid in `version`. +pub fn generate_attributes( + original_attributes: &[Attribute], + previous_docs: &BTreeMap>, + version: &Version, +) -> TokenStream { + // The docs valid in this version are the previous docs of the closest change after this + // version. If there is no such change, the original docs are still valid. + let Some((_, docs)) = previous_docs.up_bound(Bound::Excluded(version)) else { + return quote! { #(#original_attributes)* }; + }; + + let attributes = original_attributes.iter().filter(|attribute| { + !matches!(&attribute.meta, Meta::NameValue(name_value) if name_value.path.is_ident("doc")) + }); + + quote! { + #(#[doc = #docs])* + #(#attributes)* + } +} + #[derive(Debug, PartialEq, Eq)] pub enum ItemStatus { Addition { @@ -49,4 +81,11 @@ impl ItemStatus { Self::NotPresent => unreachable!("ItemStatus::NotPresent does not have an ident"), } } + + /// Returns `true` if this status is a change which modified the type of the item. + /// + /// Changes which only rename the item are lossless and as such don't need to be tracked. + pub fn is_type_change(&self) -> bool { + matches!(self, Self::Change { from_type, to_type, .. } if from_type != to_type) + } } diff --git a/crates/stackable-versioned-macros/src/codegen/item/variant.rs b/crates/stackable-versioned-macros/src/codegen/item/variant.rs index 0175b43aa..11234ca9d 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/variant.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/variant.rs @@ -13,22 +13,30 @@ use crate::{ codegen::{ Direction, VersionDefinition, changes::{BTreeMapExt, ChangesetExt}, - item::ItemStatus, + item::{ItemStatus, generate_attributes}, + module::ModuleGenerationContext, }, utils::ItemIdents, }; pub struct VersionedVariant { pub original_attributes: Vec, + pub previous_docs: BTreeMap>, 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); @@ -39,13 +47,17 @@ impl VersionedVariant { attrs: Vec::new(), bang_token: Not([Span::call_site()]), }); + let previous_docs = variant_attributes.common.previous_docs(); + let nested = variant_attributes.nested.is_present(); let changes = variant_attributes.common.into_changeset(&idents, ty); Ok(Self { original_attributes: variant_attributes.attrs, + previous_docs, fields: variant.fields, idents, changes, + nested, }) } @@ -63,7 +75,11 @@ impl VersionedVariant { /// Generates tokens to be used in a container definition. pub fn generate_for_container(&self, version: &VersionDefinition) -> Option { - let original_attributes = &self.original_attributes; + let attributes = generate_attributes( + &self.original_attributes, + &self.previous_docs, + &version.inner, + ); let fields = &self.fields; #[allow(clippy::single_match_else)] @@ -80,11 +96,11 @@ impl VersionedVariant { ) }) { ItemStatus::Addition { ident, .. } => Some(quote! { - #(#original_attributes)* + #attributes #ident #fields, }), ItemStatus::Change { to_ident, .. } => Some(quote! { - #(#original_attributes)* + #attributes #to_ident #fields, }), ItemStatus::Deprecation { ident, note, .. } => { @@ -103,7 +119,7 @@ impl VersionedVariant { }; Some(quote! { - #(#original_attributes)* + #attributes #deprecated_attr #ident #fields, }) @@ -118,7 +134,7 @@ impl VersionedVariant { let deprecated_attr = previously_deprecated.then(|| quote! {#[deprecated]}); Some(quote! { - #(#original_attributes)* + #attributes #deprecated_attr #ident #fields, }) @@ -132,7 +148,7 @@ impl VersionedVariant { let ident = &self.idents.original; Some(quote! { - #(#original_attributes)* + #attributes #ident #fields, }) } @@ -145,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 { @@ -196,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) => { @@ -218,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 4e666a6b2..1757ac416 100644 --- a/crates/stackable-versioned-macros/src/lib.rs +++ b/crates/stackable-versioned-macros/src/lib.rs @@ -496,6 +496,7 @@ mod utils; /// - `since` to indicate since which version the item is changed. /// - `from_name` to indicate from which previous name the field is renamed. /// - `from_type` to indicate from which previous type the field is changed. +/// - `from_docs` to provide the previous doc comments of the item. /// - `upgrade_with` to provide a custom upgrade function. This argument can /// only be used in combination with the `from_type` argument. The expected /// function signature is: `fn (OLD_TYPE) -> NEW_TYPE`. This function must @@ -561,6 +562,59 @@ mod utils; /// ``` /// /// +/// #### Changed Docs +/// +/// The doc comments of an item can be changed using the `from_docs` argument. +/// The doc comments attached to the item are used since the version of the +/// change. All earlier versions use the doc comments provided via `from_docs` +/// instead. This is especially useful to adjust the description of a field in +/// the generated CRD schema without changing it for older versions. The +/// argument can be used on its own or in combination with any other argument. +/// +/// ``` +/// # use stackable_versioned_macros::versioned; +/// #[versioned(version(name = "v1alpha1"), version(name = "v1beta1"))] +/// mod versioned { +/// pub struct Foo { +/// /// The number of bars. +/// #[versioned(changed(since = "v1beta1", from_docs = "The bar."))] +/// bar: usize, +/// baz: bool, +/// } +/// } +/// ``` +/// +///
+/// Expand Generated Code +/// +/// 1. In version `v1alpha1` the field uses the doc comments provided via +/// `from_docs`. +/// 2. In the next version, `v1beta1`, the field uses the doc comments attached +/// to the field. +/// +/// ```ignore +/// pub mod v1alpha1 { +/// use super::*; +/// pub struct Foo { +/// /// The bar. // 1 +/// pub bar: usize, +/// pub baz: bool, +/// } +/// } +/// +/// // Snip +/// +/// pub mod v1beta1 { +/// use super::*; +/// pub struct Foo { +/// /// The number of bars. // 2 +/// pub bar: usize, +/// pub baz: bool, +/// } +/// } +/// ``` +///
+/// /// ### Deprecated Action /// /// This action indicates that an item is deprecated in a particular version. @@ -827,8 +881,8 @@ mod utils; /// ///
/// -/// Currently, only tracking of **added** fields is supported. This will be -/// expanded to removed fields, field type changes, and fields containing +/// Currently, only tracking of **added** fields and **field type changes** is +/// supported. This will be expanded to removed fields and fields containing /// collections in the future. /// ///
@@ -874,6 +928,29 @@ mod utils; /// The final upgrade to `v1` will apply the tracked value for the field `baz`. /// Again, it is removed from the status afterwards. /// +/// #### Tracking Type Changes +/// +/// Fields which changed their type via `changed(from_type = "...")` are tracked +/// as well, because converting to the older type might lose data. Renaming a +/// field without changing its type is lossless and is not tracked. In addition +/// to the original value, the value the field was downgraded to is tracked: +/// +/// ```yaml +/// status: +/// changedValues: +/// upgrades: +/// v1: +/// - jsonPath: "$.gender" +/// value: "It's complicated" +/// downgradedValue: "Unknown" +/// ``` +/// +/// Unlike added fields, the field also exists in the older version and can be +/// changed by users there. When upgrading again, the tracked value is only +/// applied if the field still contains the downgraded value. Otherwise, the +/// change made by the user in the older version takes precedence and the +/// tracked value is discarded. +/// /// ### Tracking Nested Changes /// /// To be able to automatically track values of changed fields in nested sub @@ -915,6 +992,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/src/utils/mod.rs b/crates/stackable-versioned-macros/src/utils/mod.rs index d69efaa73..87b2b7145 100644 --- a/crates/stackable-versioned-macros/src/utils/mod.rs +++ b/crates/stackable-versioned-macros/src/utils/mod.rs @@ -78,6 +78,7 @@ pub trait ItemIdents { pub trait ItemIdentExt { fn json_path_ident(&self) -> IdentString; + fn tracked_value_ident(&self) -> IdentString; } impl ItemIdentExt for IdentString { @@ -88,6 +89,14 @@ impl ItemIdentExt for IdentString { ) .into() } + + fn tracked_value_ident(&self) -> IdentString { + format_ident!( + "__sv_{lowercase_ident}_value", + lowercase_ident = self.as_str().to_lowercase() + ) + .into() + } } pub fn path_to_string(path: &Path) -> String { diff --git a/crates/stackable-versioned-macros/tests/inputs/fail/changed.stderr b/crates/stackable-versioned-macros/tests/inputs/fail/changed.stderr index 61d10138f..417ca1250 100644 --- a/crates/stackable-versioned-macros/tests/inputs/fail/changed.stderr +++ b/crates/stackable-versioned-macros/tests/inputs/fail/changed.stderr @@ -10,7 +10,7 @@ error: the previous name must not start with the deprecation prefix 13 | changed(since = "v1", from_name = "deprecated_baz"), | ^^^^^^^^^^^^^^^^ -error: both `from_name` and `from_type` are unset. Is this `changed()` action needed? +error: `from_name`, `from_type` and `from_docs` are unset. Is this `changed()` action needed? --> tests/inputs/fail/changed.rs:14:29 | 14 | changed(since = "v2") 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/inputs/pass/conversion_tracking_type_change.rs b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_type_change.rs new file mode 100644 index 000000000..5338623dc --- /dev/null +++ b/crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_type_change.rs @@ -0,0 +1,47 @@ +use kube::CustomResource; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use stackable_versioned::versioned; +// --- +#[versioned( + version(name = "v1alpha1"), + version(name = "v1beta1"), + version(name = "v1"), + 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 { + // Two consecutive type changes, the second one combined with a rename. Both are tracked. + #[versioned( + changed(since = "v1beta1", from_type = "u16", downgrade_with = u32_to_u16), + changed( + since = "v1", + from_name = "bah", + from_type = "u32", + downgrade_with = u64_to_u32 + ) + )] + bar: u64, + + // A rename without a type change is lossless and as such is not tracked. + #[versioned(changed(since = "v1", from_name = "qux"))] + baz: bool, + + // An added field in the same version as a type change. + #[versioned(added(since = "v1"))] + quux: String, + } +} +// --- +fn main() {} + +fn u32_to_u16(input: u32) -> u16 { + input.try_into().unwrap_or(u16::MAX) +} + +fn u64_to_u32(input: u64) -> u32 { + input.try_into().unwrap_or(u32::MAX) +} diff --git a/crates/stackable-versioned-macros/tests/inputs/pass/docs.rs b/crates/stackable-versioned-macros/tests/inputs/pass/docs.rs index 23888ab90..c00fd6d5b 100644 --- a/crates/stackable-versioned-macros/tests/inputs/pass/docs.rs +++ b/crates/stackable-versioned-macros/tests/inputs/pass/docs.rs @@ -35,6 +35,37 @@ mod versioned { #[versioned(changed(since = "v1beta1", from_name = "qoox"))] #[versioned(changed(since = "v1", from_name = "qaax"))] quux: String, + + /// The docs of this field changed in v1beta1 and v2. + #[versioned( + changed(since = "v1beta1", from_docs = "These are the docs in v1alpha1."), + changed( + since = "v2", + from_docs = r#" + These are the docs from v1beta1 until v1. + + Multi-line docs are also supported. + "# + ) + )] + #[doc(alias = "grault")] + corge: String, + + /// The docs of this field changed in v1, while it was renamed in v1beta1. + #[versioned( + changed(since = "v1beta1", from_name = "waldo"), + changed(since = "v1", from_docs = "These are the docs before v1.") + )] + fred: String, + } + + /// Test + #[derive(Default)] + enum Bar { + /// The docs of this variant changed in v1. + #[versioned(changed(since = "v1", from_docs = "These are the docs before v1."))] + #[default] + Baz, } } // --- diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap index 57f5cf4ae..68cee80a4 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap @@ -65,10 +65,10 @@ where baz: __sv_foospec.baz.into(), }; if let Some(upgrades) = status.changes().upgrades.remove(&"v1beta1".to_owned()) { - for ::stackable_versioned::ChangedValue { json_path, value } in upgrades { + for ::stackable_versioned::ChangedValue { json_path, value, .. } in upgrades { match json_path { json_path if json_path == __sv_bah_path => { - spec.bah = serde_yaml::from_value(value).unwrap(); + spec.bah = ::serde_json::from_value(value).unwrap(); } _ => unreachable!(), } @@ -97,7 +97,8 @@ where upgrades .push(::stackable_versioned::ChangedValue { json_path: __sv_bah_path, - value: ::serde_yaml::to_value(&__sv_foospec.bah).unwrap(), + value: ::serde_json::to_value(&__sv_foospec.bah).unwrap(), + downgraded_value: ::core::option::Option::None, }); let mut spec = Self { baz: __sv_foospec.baz.into(), 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..8eb2758f0 --- /dev/null +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap @@ -0,0 +1,577 @@ +--- +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_json::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_json::to_value(&__sv_icebergconnector.rest_catalog_uri) + .unwrap(), + downgraded_value: ::core::option::Option::None, + }); + 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@conversion_tracking_hints.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap index 96a70d907..4bf3ec1e1 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap @@ -61,10 +61,10 @@ where }; if let Some(upgrades) = status.changes().upgrades.remove(&"v1alpha2".to_owned()) { - for ::stackable_versioned::ChangedValue { json_path, value } in upgrades { + for ::stackable_versioned::ChangedValue { json_path, value, .. } in upgrades { match json_path { json_path if json_path == __sv_baz_baz_path => { - spec.baz_baz = serde_yaml::from_value(value).unwrap(); + spec.baz_baz = ::serde_json::from_value(value).unwrap(); } _ => unreachable!(), } @@ -89,7 +89,8 @@ where upgrades .push(::stackable_versioned::ChangedValue { json_path: __sv_baz_baz_path, - value: ::serde_yaml::to_value(&__sv_bar.baz_baz).unwrap(), + value: ::serde_json::to_value(&__sv_bar.baz_baz).unwrap(), + downgraded_value: ::core::option::Option::None, }); let mut spec = Self { bar_bar: __sv_bar.bar_bar.into(), diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_type_change.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_type_change.rs.snap new file mode 100644 index 000000000..e3e76e74e --- /dev/null +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_type_change.rs.snap @@ -0,0 +1,572 @@ +--- +source: crates/stackable-versioned-macros/src/lib.rs +expression: formatted +input_file: crates/stackable-versioned-macros/tests/inputs/pass/conversion_tracking_type_change.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 bah: u16, + pub qux: bool, + } +} +#[automatically_derived] +impl ::core::convert::From for v1beta1::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: v1beta1::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 v1beta1::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_bah_path = ::stackable_versioned::jthong_path(parent, "bah"); + let __sv_bah_value = ::serde_json::to_value(&__sv_foospec.bah).unwrap(); + let mut spec = Self { + bah: __sv_foospec.bah.into(), + qux: __sv_foospec.qux.into(), + }; + if let Some(upgrades) = status.changes().upgrades.remove(&"v1beta1".to_owned()) { + for ::stackable_versioned::ChangedValue { + json_path, + value, + downgraded_value, + } in upgrades { + match json_path { + json_path if json_path == __sv_bah_path => { + if downgraded_value.unwrap_or_default() == __sv_bah_value { + spec.bah = ::serde_json::from_value(value).unwrap(); + } + } + _ => unreachable!(), + } + } + } + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1alpha1::FooSpec +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_foospec: v1beta1::FooSpec, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_bah_path = ::stackable_versioned::jthong_path(parent, "bah"); + let __sv_bah_value = ::serde_json::to_value(&__sv_foospec.bah).unwrap(); + let mut spec = Self { + bah: u32_to_u16(__sv_foospec.bah), + qux: __sv_foospec.qux.into(), + }; + let upgrades = status + .changes() + .upgrades + .entry("v1beta1".to_owned()) + .or_default(); + upgrades + .push(::stackable_versioned::ChangedValue { + json_path: __sv_bah_path, + value: __sv_bah_value, + downgraded_value: ::core::option::Option::Some( + ::serde_json::to_value(&spec.bah).unwrap(), + ), + }); + spec + } +} +#[automatically_derived] +pub(crate) mod v1beta1 { + use super::*; + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, CustomResource)] + #[kube( + group = "stackable.tech", + version = "v1beta1", + kind = "Foo", + doc = "Test", + status = FooStatusWithChangedValues + )] + pub struct FooSpec { + pub bah: u32, + pub qux: bool, + } +} +#[automatically_derived] +impl ::core::convert::From for v1::Foo { + fn from(__sv_foo: v1beta1::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 v1beta1::Foo { + fn from(__sv_foo: v1::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 v1::FooSpec +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from( + __sv_foospec: v1beta1::FooSpec, + status: &mut S, + parent: &str, + ) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_bar_path = ::stackable_versioned::jthong_path(parent, "bar"); + let __sv_quux_path = ::stackable_versioned::jthong_path(parent, "quux"); + let __sv_bar_value = ::serde_json::to_value(&__sv_foospec.bah).unwrap(); + let mut spec = Self { + bar: __sv_foospec.bah.into(), + baz: __sv_foospec.qux.into(), + quux: ::std::default::Default::default(), + }; + if let Some(upgrades) = status.changes().upgrades.remove(&"v1".to_owned()) { + for ::stackable_versioned::ChangedValue { + json_path, + value, + downgraded_value, + } in upgrades { + match json_path { + json_path if json_path == __sv_bar_path => { + if downgraded_value.unwrap_or_default() == __sv_bar_value { + spec.bar = ::serde_json::from_value(value).unwrap(); + } + } + json_path if json_path == __sv_quux_path => { + spec.quux = ::serde_json::from_value(value).unwrap(); + } + _ => unreachable!(), + } + } + } + spec + } +} +#[automatically_derived] +impl ::stackable_versioned::TrackingFrom for v1beta1::FooSpec +where + S: ::stackable_versioned::TrackingStatus + ::core::default::Default, +{ + fn tracking_from(__sv_foospec: v1::FooSpec, status: &mut S, parent: &str) -> Self { + use ::stackable_versioned::TrackingInto as _; + let __sv_bar_path = ::stackable_versioned::jthong_path(parent, "bar"); + let __sv_quux_path = ::stackable_versioned::jthong_path(parent, "quux"); + let upgrades = status.changes().upgrades.entry("v1".to_owned()).or_default(); + upgrades + .push(::stackable_versioned::ChangedValue { + json_path: __sv_quux_path, + value: ::serde_json::to_value(&__sv_foospec.quux).unwrap(), + downgraded_value: ::core::option::Option::None, + }); + let __sv_bar_value = ::serde_json::to_value(&__sv_foospec.bar).unwrap(); + let mut spec = Self { + bah: u64_to_u32(__sv_foospec.bar), + qux: __sv_foospec.baz.into(), + }; + let upgrades = status.changes().upgrades.entry("v1".to_owned()).or_default(); + upgrades + .push(::stackable_versioned::ChangedValue { + json_path: __sv_bar_path, + value: __sv_bar_value, + downgraded_value: ::core::option::Option::Some( + ::serde_json::to_value(&spec.bah).unwrap(), + ), + }); + spec + } +} +#[automatically_derived] +pub(crate) mod v1 { + use super::*; + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, CustomResource)] + #[kube( + group = "stackable.tech", + version = "v1", + kind = "Foo", + doc = "Test", + status = FooStatusWithChangedValues + )] + pub struct FooSpec { + pub bar: u64, + pub baz: bool, + pub quux: String, + } +} +#[automatically_derived] +#[derive(::core::fmt::Debug)] +pub(crate) enum Foo { + V1Alpha1(v1alpha1::Foo), + V1Beta1(v1beta1::Foo), + V1(v1::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(), < + v1beta1::Foo as ::kube::core::CustomResourceExt > ::crd(), < v1::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::V1Beta1) => { + let converted: v1beta1::Foo = __sv_foo.into(); + let desired_object = Self::V1Beta1(converted); + let desired_object = desired_object + .into_json_value() + .map_err(|source| ::stackable_versioned::ConvertObjectError::Serialize { + source, + })?; + converted_objects.push(desired_object); + } + (Self::V1Alpha1(__sv_foo), FooVersion::V1) => { + let converted: v1beta1::Foo = __sv_foo.into(); + let converted: v1::Foo = converted.into(); + let desired_object = Self::V1(converted); + let desired_object = desired_object + .into_json_value() + .map_err(|source| ::stackable_versioned::ConvertObjectError::Serialize { + source, + })?; + converted_objects.push(desired_object); + } + (Self::V1Beta1(__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); + } + (Self::V1Beta1(__sv_foo), FooVersion::V1) => { + let converted: v1::Foo = __sv_foo.into(); + let desired_object = Self::V1(converted); + let desired_object = desired_object + .into_json_value() + .map_err(|source| ::stackable_versioned::ConvertObjectError::Serialize { + source, + })?; + converted_objects.push(desired_object); + } + (Self::V1(__sv_foo), FooVersion::V1Alpha1) => { + let converted: v1beta1::Foo = __sv_foo.into(); + let converted: v1alpha1::Foo = converted.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); + } + (Self::V1(__sv_foo), FooVersion::V1Beta1) => { + let converted: v1beta1::Foo = __sv_foo.into(); + let desired_object = Self::V1Beta1(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/v1beta1" => { + let object = ::serde_json::from_value(object_value) + .map_err(|source| ::stackable_versioned::ParseObjectError::Deserialize { + source, + })?; + Self::V1Beta1(object) + } + "stackable.tech/v1" => { + let object = ::serde_json::from_value(object_value) + .map_err(|source| ::stackable_versioned::ParseObjectError::Deserialize { + source, + })?; + Self::V1(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::V1Beta1(__sv_foo) => Ok(::serde_json::to_value(__sv_foo)?), + Self::V1(__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, + V1Beta1, + V1, +} +#[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::V1Beta1 => "v1beta1", + FooVersion::V1 => "v1", + } + } + pub fn as_api_version_str(&self) -> &str { + match self { + FooVersion::V1Alpha1 => "stackable.tech/v1alpha1", + FooVersion::V1Beta1 => "stackable.tech/v1beta1", + FooVersion::V1 => "stackable.tech/v1", + } + } + pub fn from_api_version( + api_version: &str, + ) -> Result { + match api_version { + "stackable.tech/v1alpha1" => Ok(FooVersion::V1Alpha1), + "stackable.tech/v1beta1" => Ok(FooVersion::V1Beta1), + "stackable.tech/v1" => Ok(FooVersion::V1), + _ => { + Err(::stackable_versioned::UnknownDesiredApiVersionError { + api_version: api_version.to_owned(), + }) + } + } + } +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1::FooSpec, + >(stringify!(Foo), "stackable.tech/v1", "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/v1", 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 d37c68374..b5ab1154b 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 @@ -15,6 +15,18 @@ mod v1alpha1 { pub bar: String, /// This is will keep changing over time. pub qoox: String, + ///These are the docs in v1alpha1. + #[doc(alias = "grault")] + pub corge: String, + ///These are the docs before v1. + pub waldo: String, + } + /// Test + #[derive(Default)] + pub enum Bar { + ///These are the docs before v1. + #[default] + Baz, } } #[automatically_derived] @@ -31,12 +43,14 @@ where deprecated_bar: __sv_foo.bar.into(), baz: ::std::default::Default::default(), qaax: __sv_foo.qoox.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.waldo.into(), }; if let Some(upgrades) = status.changes().upgrades.remove(&"v1beta1".to_owned()) { - for ::stackable_versioned::ChangedValue { json_path, value } in upgrades { + for ::stackable_versioned::ChangedValue { json_path, value, .. } in upgrades { match json_path { json_path if json_path == __sv_baz_path => { - spec.baz = serde_yaml::from_value(value).unwrap(); + spec.baz = ::serde_json::from_value(value).unwrap(); } _ => unreachable!(), } @@ -62,17 +76,44 @@ where upgrades .push(::stackable_versioned::ChangedValue { json_path: __sv_baz_path, - value: ::serde_yaml::to_value(&__sv_foo.baz).unwrap(), + value: ::serde_json::to_value(&__sv_foo.baz).unwrap(), + downgraded_value: ::core::option::Option::None, }); let mut spec = Self { foo: __sv_foo.foo.into(), bar: __sv_foo.deprecated_bar.into(), qoox: __sv_foo.qaax.into(), + corge: __sv_foo.corge.into(), + waldo: __sv_foo.fred.into(), }; spec } } #[automatically_derived] +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 ::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, + } + } +} +#[automatically_derived] mod v1beta1 { use super::*; ///Additional docs for this version which are purposefully long to @@ -90,6 +131,23 @@ mod v1beta1 { pub baz: String, /// This is will keep changing over time. pub qaax: String, + ///These are the docs from v1beta1 until v1. + /// + ///Multi-line docs are also supported. + #[doc(alias = "grault")] + pub corge: String, + ///These are the docs before v1. + pub fred: String, + } + ///Additional docs for this version which are purposefully long to + ///show how manual line wrapping works. \ + ///Multi-line docs are also supported, as per regular doc-comments. + /// Test + #[derive(Default)] + pub enum Bar { + ///These are the docs before v1. + #[default] + Baz, } } #[automatically_derived] @@ -105,6 +163,8 @@ where deprecated_bar: __sv_foo.deprecated_bar.into(), baz: __sv_foo.baz.into(), qaax: __sv_foo.qaax.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.fred.into(), }; spec } @@ -122,11 +182,37 @@ where deprecated_bar: __sv_foo.deprecated_bar.into(), baz: __sv_foo.baz.into(), qaax: __sv_foo.qaax.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.fred.into(), }; spec } } #[automatically_derived] +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 ::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, + } + } +} +#[automatically_derived] mod v1beta2 { use super::*; /// Test @@ -141,6 +227,20 @@ mod v1beta2 { pub baz: String, /// This is will keep changing over time. pub qaax: String, + ///These are the docs from v1beta1 until v1. + /// + ///Multi-line docs are also supported. + #[doc(alias = "grault")] + pub corge: String, + ///These are the docs before v1. + pub fred: String, + } + /// Test + #[derive(Default)] + pub enum Bar { + ///These are the docs before v1. + #[default] + Baz, } } #[automatically_derived] @@ -156,6 +256,8 @@ where deprecated_bar: __sv_foo.deprecated_bar.into(), baz: __sv_foo.baz.into(), quux: __sv_foo.qaax.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.fred.into(), }; spec } @@ -173,11 +275,37 @@ where deprecated_bar: __sv_foo.deprecated_bar.into(), baz: __sv_foo.baz.into(), qaax: __sv_foo.quux.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.fred.into(), }; spec } } #[automatically_derived] +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 ::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, + } + } +} +#[automatically_derived] mod v1 { use super::*; /// Test @@ -192,6 +320,20 @@ mod v1 { pub baz: String, /// This is will keep changing over time. pub quux: String, + ///These are the docs from v1beta1 until v1. + /// + ///Multi-line docs are also supported. + #[doc(alias = "grault")] + pub corge: String, + /// The docs of this field changed in v1, while it was renamed in v1beta1. + pub fred: String, + } + /// Test + #[derive(Default)] + pub enum Bar { + /// The docs of this variant changed in v1. + #[default] + Baz, } } #[automatically_derived] @@ -207,6 +349,8 @@ where deprecated_bar: __sv_foo.deprecated_bar.into(), baz: __sv_foo.baz.into(), quux: __sv_foo.quux.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.fred.into(), }; spec } @@ -224,11 +368,37 @@ where deprecated_bar: __sv_foo.deprecated_bar.into(), baz: __sv_foo.baz.into(), quux: __sv_foo.quux.into(), + corge: __sv_foo.corge.into(), + fred: __sv_foo.fred.into(), }; spec } } #[automatically_derived] +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 ::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, + } + } +} +#[automatically_derived] mod v2 { use super::*; /// Test @@ -243,5 +413,17 @@ mod v2 { pub baz: String, /// This is will keep changing over time. pub quux: String, + /// The docs of this field changed in v1beta1 and v2. + #[doc(alias = "grault")] + pub corge: String, + /// The docs of this field changed in v1, while it was renamed in v1beta1. + pub fred: String, + } + /// Test + #[derive(Default)] + pub enum Bar { + /// The docs of this variant changed in v1. + #[default] + Baz, } } diff --git a/crates/stackable-versioned/CHANGELOG.md b/crates/stackable-versioned/CHANGELOG.md index 3cf01678b..89b3ab879 100644 --- a/crates/stackable-versioned/CHANGELOG.md +++ b/crates/stackable-versioned/CHANGELOG.md @@ -8,9 +8,32 @@ All notable changes to this project will be documented in this file. - Add `#[versioned(hint(map))]` to provide a hint for map types during conversion ([#1285]). - Emit generics in plain `From` impl ([#1284]). +- Add `from_docs` argument to the `changed()` action to provide the previous doc comments of an item ([#XXXX]). +- 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]). +- Track fields which changed their type when `experimental_conversion_tracking` is enabled. The + tracked value is only applied on upgrade if the field was not changed in the older version + ([#ZZZZ]). + +### 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]). +- BREAKING: Store tracked values as `serde_json::Value` instead of `serde_yaml::Value`. Tracked + enum values were serialized as YAML tags, which failed to deserialize after a roundtrip through + the JSON status ([#ZZZZ]). [#1284]: https://github.com/stackabletech/operator-rs/pull/1284 [#1285]: https://github.com/stackabletech/operator-rs/pull/1285 +[#XXXX]: https://github.com/stackabletech/operator-rs/pull/XXXX +[#YYYY]: https://github.com/stackabletech/operator-rs/pull/YYYY +[#ZZZZ]: https://github.com/stackabletech/operator-rs/pull/ZZZZ ## [0.11.1] - 2026-07-06 diff --git a/crates/stackable-versioned/Cargo.toml b/crates/stackable-versioned/Cargo.toml index 069e43abc..792046924 100644 --- a/crates/stackable-versioned/Cargo.toml +++ b/crates/stackable-versioned/Cargo.toml @@ -17,7 +17,6 @@ kube.workspace = true schemars.workspace = true serde.workspace = true serde_json.workspace = true -serde_yaml.workspace = true snafu.workspace = true [dev-dependencies] diff --git a/crates/stackable-versioned/src/lib.rs b/crates/stackable-versioned/src/lib.rs index 6aa4bddc3..031b098c5 100644 --- a/crates/stackable-versioned/src/lib.rs +++ b/crates/stackable-versioned/src/lib.rs @@ -113,8 +113,21 @@ pub struct ChangedValue { pub json_path: String, /// The value to be used when upgrading or downgrading the custom resource. + // NOTE: This needs to be a JSON value, because the status is stored as JSON. YAML values + // represent enum variants as tags, which can't be deserialized again after a roundtrip + // through JSON. #[schemars(schema_with = "raw_object_schema")] - pub value: serde_yaml::Value, + pub value: serde_json::Value, + + /// The value the field was converted to during the downgrade. Only set for fields which + /// changed their type. + /// + /// When upgrading again, the tracked `value` is only applied if the field still contains this + /// value. Otherwise, the field was changed by a user in the older version and that change is + /// kept instead. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[schemars(schema_with = "raw_object_schema")] + pub downgraded_value: Option, } // TODO (@Techassi): Think about where this should live. Basically this already exists in diff --git a/crates/stackable-versioned/tests/enum_tracking.rs b/crates/stackable-versioned/tests/enum_tracking.rs new file mode 100644 index 000000000..62504fcb0 --- /dev/null +++ b/crates/stackable-versioned/tests/enum_tracking.rs @@ -0,0 +1,181 @@ +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 { + #[versioned(changed( + since = "v1alpha2", + from_name = "metastore", + from_type = "Option" + ))] + catalog: IcebergCatalog, + } +} + +#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] +pub struct TpchConnector {} + +#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] +#[serde(rename_all = "camelCase")] +pub enum IcebergCatalog { + Rest { uri: String }, + HiveMetastore { config_map: String }, + UserProvided {}, +} + +// Before v1alpha2, only Hive metastores were supported. Other catalogs had to be configured by +// users manually, so there is no way to represent a REST catalog in v1alpha1. +impl From for Option { + fn from(catalog: IcebergCatalog) -> Self { + match catalog { + IcebergCatalog::HiveMetastore { config_map } => Some(config_map), + IcebergCatalog::Rest { .. } | IcebergCatalog::UserProvided {} => None, + } + } +} + +impl From> for IcebergCatalog { + fn from(metastore: Option) -> Self { + match metastore { + Some(config_map) => Self::HiveMetastore { config_map }, + None => Self::UserProvided {}, + } + } +} + +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::Iceberg(v1alpha1::IcebergConnector { + metastore: None, + }), + }, + Self { + connector: v1alpha1::Connector::Tpch(TpchConnector {}), + }, + ] + } +} + +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha2::CatalogSpec { + fn roundtrip_test_data() -> Vec { + vec![ + // The REST catalog can not be represented in v1alpha1. It is tracked in the status + // and restored when upgrading again. + Self { + connector: v1alpha2::Connector::Iceberg(v1alpha2::IcebergConnector { + catalog: IcebergCatalog::Rest { + uri: "http://rest-catalog:8181".to_owned(), + }, + }), + }, + Self { + connector: v1alpha2::Connector::Iceberg(v1alpha2::IcebergConnector { + catalog: IcebergCatalog::HiveMetastore { + config_map: "hive".to_owned(), + }, + }), + }, + Self { + connector: v1alpha2::Connector::Iceberg(v1alpha2::IcebergConnector { + catalog: IcebergCatalog::UserProvided {}, + }), + }, + 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": { + "catalog": { + "rest": { + "uri": "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"]["metastore"], + serde_json::Value::Null + ); + assert_eq!( + object["status"]["changedValues"]["upgrades"]["v1alpha2"], + serde_json::json!([{ + "jsonPath": "$.connector.Iceberg.catalog", + "value": { + "rest": { + "uri": "http://rest-catalog:8181" + } + }, + "downgradedValue": null + }]) + ); +} diff --git a/crates/stackable-versioned/tests/person.rs b/crates/stackable-versioned/tests/person.rs index 5efa0f298..0deed7258 100644 --- a/crates/stackable-versioned/tests/person.rs +++ b/crates/stackable-versioned/tests/person.rs @@ -66,8 +66,6 @@ pub mod versioned { // We started out with a enum. As we *need* to provide a default, we have a Unknown variant. // Afterwards we figured let's be more flexible and accept any arbitrary String. - - // FIXME: The roundtrips are currently broken, see the `roundtrip_test_data` below. #[versioned(added(since = "v2"), changed(since = "v3", from_type = "Gender"))] gender: String, @@ -128,20 +126,18 @@ impl stackable_versioned::test_utils::RoundtripTestData for v3::PersonSpec { mastodon: "@jdoe@example.com".to_owned(), }, }, - // FIXME: The following test case fails. See the docs on the `versioned` macro, as of - // writing it only supports tracking `added` fields. - // Hence, the firstName, lastName, socials.mastodon work, while gender is broken. - // Self { - // username: "".to_owned(), - // first_name: "".to_owned(), - // last_name: "".to_owned(), - // // FIXME: Currently, the roundtrip results in "Unknown", although it should be "It's complicated" - // gender: "It's complicated".to_owned(), - // socials: v3::Socials { - // email: "".to_owned(), - // mastodon: "".to_owned(), - // }, - // }, + // Downgrading to v2 turns the gender into Gender::Unknown. The original value is + // tracked in the status and restored when upgrading again. + Self { + username: String::new(), + first_name: String::new(), + last_name: String::new(), + gender: "It's complicated".to_owned(), + socials: v3::Socials { + email: String::new(), + mastodon: String::new(), + }, + }, ] } } diff --git a/crates/stackable-versioned/tests/roundtrip.rs b/crates/stackable-versioned/tests/roundtrip.rs index b1703d81b..24e786c6b 100644 --- a/crates/stackable-versioned/tests/roundtrip.rs +++ b/crates/stackable-versioned/tests/roundtrip.rs @@ -1,5 +1,5 @@ use insta::glob; -use kube::core::response::StatusSummary; +use kube::core::{conversion::ConversionReview, response::StatusSummary}; use crate::person::{Person, PersonVersion}; @@ -52,3 +52,64 @@ fn person_v3_v1alpha1_v3() { assert_eq!(original_object, converted_object); }); } + +#[test] +fn person_v3_v2_v3_keeps_changes_made_in_v2() { + // "It's complicated" cannot be represented by the Gender enum used in v2, so it is downgraded + // to "Unknown". The original value is tracked in the status. + 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/v2", + "objects": [{ + "apiVersion": "test.stackable.tech/v3", + "kind": "Person", + "metadata": {}, + "spec": { + "username": "jdoe", + "firstName": "John", + "lastName": "Doe", + "gender": "It's complicated", + "socials": { + "email": "jdoe@example.com", + "mastodon": "@jdoe@example.com" + } + } + }] + } + })) + .expect("conversion review must be valid"); + + let response_v2 = Person::try_convert(review); + let mut roundtrip_review = person::roundtrip_conversion_review(response_v2, PersonVersion::V3); + + // A user changes the gender while working with the v2 object + let object = roundtrip_review + .request + .as_mut() + .expect("roundtrip review must have a request") + .objects + .first_mut() + .expect("there must be at least one object"); + + assert_eq!(object["spec"]["gender"], "Unknown"); + object["spec"]["gender"] = "Female".into(); + + let response_v3 = Person::try_convert(roundtrip_review); + let response = response_v3 + .response + .as_ref() + .expect("v3 review must have a response"); + + assert_eq!(response.result.status, Some(StatusSummary::Success)); + + // The change made in v2 takes precedence over the tracked value + let object = response + .converted_objects + .first() + .expect("there must be at least one object"); + + assert_eq!(object["spec"]["gender"], "Female"); +} diff --git a/crates/stackable-versioned/tests/snapshots/conversions__pass@persons_to_v1alpha1.json.snap b/crates/stackable-versioned/tests/snapshots/conversions__pass@persons_to_v1alpha1.json.snap index e469f60c7..cccb73fe4 100644 --- a/crates/stackable-versioned/tests/snapshots/conversions__pass@persons_to_v1alpha1.json.snap +++ b/crates/stackable-versioned/tests/snapshots/conversions__pass@persons_to_v1alpha1.json.snap @@ -168,6 +168,13 @@ input_file: crates/stackable-versioned/tests/inputs/conversions/pass/persons_to_ "jsonPath": "$.gender", "value": "Male" } + ], + "v3": [ + { + "downgradedValue": "Male", + "jsonPath": "$.gender", + "value": "Male" + } ] } }