Skip to content

Commit b9cbd8c

Browse files
sbernauerclaude
andcommitted
fix(versioned): Track values as JSON
Tracked values were stored as serde_yaml::Value, which represents enum variants as YAML tags. After a roundtrip through the JSON status, these could not be deserialized again, which panicked during the upgrade. The status is stored as JSON anyway, so values are now tracked as serde_json::Value. Also fix restoring type changes whose downgraded value is null: it is serialized as `downgradedValue: null` and deserialized as None, so the comparison never matched. A missing downgraded value is now treated as null. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 023c0be commit b9cbd8c

11 files changed

Lines changed: 107 additions & 55 deletions

‎Cargo.lock‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/stackable-versioned-macros/src/codegen/item/field.rs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ impl VersionedField {
306306
Direction::Downgrade => {
307307
let next_change = changes.get_expect(&next_version.inner);
308308

309-
let serde_yaml_path = &*mod_gen_ctx.crates.serde_yaml;
309+
let serde_json_path = &*mod_gen_ctx.crates.serde_json;
310310
let versioned_path = &*mod_gen_ctx.crates.versioned;
311311

312312
match next_change {
@@ -318,7 +318,7 @@ impl VersionedField {
318318
Some(quote! {
319319
upgrades.push(#versioned_path::ChangedValue {
320320
json_path: #json_path_ident,
321-
value: #serde_yaml_path::to_value(&#from_struct_ident.#ident).unwrap(),
321+
value: #serde_json_path::to_value(&#from_struct_ident.#ident).unwrap(),
322322
downgraded_value: ::core::option::Option::None,
323323
});
324324
})
@@ -351,7 +351,7 @@ impl VersionedField {
351351
to_ident,
352352
..
353353
} if next_change.is_type_change() => {
354-
let serde_yaml_path = &*mod_gen_ctx.crates.serde_yaml;
354+
let serde_json_path = &*mod_gen_ctx.crates.serde_json;
355355
let value_ident = to_ident.tracked_value_ident();
356356

357357
let field_ident = match direction {
@@ -360,7 +360,7 @@ impl VersionedField {
360360
};
361361

362362
Some(quote! {
363-
let #value_ident = #serde_yaml_path::to_value(&#from_struct_ident.#field_ident).unwrap();
363+
let #value_ident = #serde_json_path::to_value(&#from_struct_ident.#field_ident).unwrap();
364364
})
365365
}
366366
_ => None,
@@ -383,7 +383,7 @@ impl VersionedField {
383383
Direction::Downgrade => {
384384
let next_change = changes.get_expect(&next_version.inner);
385385

386-
let serde_yaml_path = &*mod_gen_ctx.crates.serde_yaml;
386+
let serde_json_path = &*mod_gen_ctx.crates.serde_json;
387387
let versioned_path = &*mod_gen_ctx.crates.versioned;
388388

389389
match next_change {
@@ -400,7 +400,7 @@ impl VersionedField {
400400
json_path: #json_path_ident,
401401
value: #value_ident,
402402
downgraded_value: ::core::option::Option::Some(
403-
#serde_yaml_path::to_value(&spec.#from_ident).unwrap()
403+
#serde_json_path::to_value(&spec.#from_ident).unwrap()
404404
),
405405
});
406406
})
@@ -426,29 +426,32 @@ impl VersionedField {
426426
match direction {
427427
Direction::Upgrade => {
428428
let next_change = changes.get_expect(&next_version.inner);
429-
let serde_yaml_path = &*mod_gen_ctx.crates.serde_yaml;
429+
let serde_json_path = &*mod_gen_ctx.crates.serde_json;
430430

431431
match next_change {
432432
ItemStatus::Addition { ident, .. } => {
433433
let json_path_ident = ident.json_path_ident();
434434

435435
Some(quote! {
436436
json_path if json_path == #json_path_ident => {
437-
spec.#ident = #serde_yaml_path::from_value(value).unwrap();
437+
spec.#ident = #serde_json_path::from_value(value).unwrap();
438438
},
439439
})
440440
}
441441
// The tracked value is only applied if the field still contains the value it
442442
// was downgraded to. Otherwise, a user changed the field in the older version
443443
// and that change takes precedence over the tracked value.
444+
//
445+
// A downgraded value of null is serialized as `downgradedValue: null`, which is
446+
// deserialized as None. As such, a missing downgraded value is treated as null.
444447
ItemStatus::Change { to_ident, .. } if next_change.is_type_change() => {
445448
let json_path_ident = to_ident.json_path_ident();
446449
let value_ident = to_ident.tracked_value_ident();
447450

448451
Some(quote! {
449452
json_path if json_path == #json_path_ident => {
450-
if downgraded_value.as_ref() == ::core::option::Option::Some(&#value_ident) {
451-
spec.#to_ident = #serde_yaml_path::from_value(value).unwrap();
453+
if downgraded_value.unwrap_or_default() == #value_ident {
454+
spec.#to_ident = #serde_json_path::from_value(value).unwrap();
452455
}
453456
},
454457
})

‎crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_enum.rs.snap‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_hints.rs.snap‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking_type_change.rs.snap‎

Lines changed: 12 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@docs.rs.snap‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/stackable-versioned/CHANGELOG.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ All notable changes to this project will be documented in this file.
2525
### Fixed
2626

2727
- Fix `From` impls of enum variants with multiple unnamed fields ([#YYYY]).
28-
- Use the configured `serde_yaml` crate path when applying tracked values ([#ZZZZ]).
28+
- BREAKING: Store tracked values as `serde_json::Value` instead of `serde_yaml::Value`. Tracked
29+
enum values were serialized as YAML tags, which failed to deserialize after a roundtrip through
30+
the JSON status ([#ZZZZ]).
2931

3032
[#1284]: https://github.com/stackabletech/operator-rs/pull/1284
3133
[#1285]: https://github.com/stackabletech/operator-rs/pull/1285

‎crates/stackable-versioned/Cargo.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ kube.workspace = true
1717
schemars.workspace = true
1818
serde.workspace = true
1919
serde_json.workspace = true
20-
serde_yaml.workspace = true
2120
snafu.workspace = true
2221

2322
[dev-dependencies]

‎crates/stackable-versioned/src/lib.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,11 @@ pub struct ChangedValue {
113113
pub json_path: String,
114114

115115
/// The value to be used when upgrading or downgrading the custom resource.
116+
// NOTE: This needs to be a JSON value, because the status is stored as JSON. YAML values
117+
// represent enum variants as tags, which can't be deserialized again after a roundtrip
118+
// through JSON.
116119
#[schemars(schema_with = "raw_object_schema")]
117-
pub value: serde_yaml::Value,
120+
pub value: serde_json::Value,
118121

119122
/// The value the field was converted to during the downgrade. Only set for fields which
120123
/// changed their type.
@@ -124,7 +127,7 @@ pub struct ChangedValue {
124127
/// kept instead.
125128
#[serde(default, skip_serializing_if = "Option::is_none")]
126129
#[schemars(schema_with = "raw_object_schema")]
127-
pub downgraded_value: Option<serde_yaml::Value>,
130+
pub downgraded_value: Option<serde_json::Value>,
128131
}
129132

130133
// TODO (@Techassi): Think about where this should live. Basically this already exists in

0 commit comments

Comments
 (0)