fix(derive): support container-level serde(default) in KubeSchema#1999
fix(derive): support container-level serde(default) in KubeSchema#1999mrthinger wants to merge 1 commit into
Conversation
af73f3b to
a9e3f18
Compare
A bare container-level #[serde(default)] makes the schemars derive call <Self as Default>::default() on the mirror structs that KubeSchema generates to compute its schema (StructNameValidated and the per-field Validated wrappers). Those mirrors have no Default impls, so combining x_kube validations with a container serde default failed to compile with "no function or associated item named `default`". Detect the bare container default and emit delegating Default impls for the generated mirrors: each calls the original type's Default once and moves the resulting values into the mirror. The default = "path" form is unaffected (schemars resolves the function directly); a test pins that it keeps working. Fixes kube-rs#1805 Signed-off-by: Evan Pierce <evan@mrthinger.com>
71d078e to
64c0332
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1999 +/- ##
=======================================
+ Coverage 76.8% 76.9% +0.1%
=======================================
Files 89 89
Lines 8813 8855 +42
=======================================
+ Hits 6764 6801 +37
- Misses 2049 2054 +5
🚀 New features to boost your workflow:
|
Danil-Grigorev
left a comment
There was a problem hiding this comment.
Hey! Looks good overall, nice change!
Just one request for test coverage, and naming for these macros. It would help longer term to orient in these changes.
| #[serde(rename_all = "camelCase", default)] | ||
| struct ContainerDefaultSpec { | ||
| #[x_kube(validation = "self % 2 == 1")] | ||
| replica_count: i32, |
There was a problem hiding this comment.
Can you please add another field to the structure, so that test validates multiple defaulted values?
| #[automatically_derived] | ||
| impl #std::default::Default for #generated { | ||
| fn default() -> Self { | ||
| let original = #original; |
There was a problem hiding this comment.
The naming is a bit confusing, since this is actually unwrapped original_default, I think it is better to shadow original_default with unwrapped value in this scope, or give it different name, like #default. Macros are harder to read overall, but this should improve it a bit.
| let original = #original; | |
| let original_default = #default; |
| #[automatically_derived] | ||
| impl #std::default::Default for Validated { | ||
| fn default() -> Self { | ||
| let original = #original; |
There was a problem hiding this comment.
Same as in https://github.com/kube-rs/kube/pull/1999/changes#r3407680513,
| let original = #original; | |
| let original_default = #default; |
Motivation
fix #1805
Combining
KubeSchemawith a bare container-level#[serde(default)]fails to compile. The derive generates mirror structs ({Name}Validated, plus a per-fieldValidatedwrapper) to compute the schema, and copies the container serde attributes onto them. schemars turns the containerdefaultinto<Self as Default>::default()calls on those mirrors, which don't implementDefault, so the build fails withno function or associated item named `default` found for struct `MyCrdSpecValidated`.Solution
Detect a bare container-level
serde(default)and emit delegatingDefaultimpls for the generated mirrors. Each impl constructs the original type's default and moves the values into the mirror.Since the per-field wrappers each construct the full original default and take one field from it, the original
Defaultruns once perx_kubefield (plus once for the full mirror) while generating the schema. This matches how serde sources missing fields from a container default, and only happens when the CRD is generated.The
default = "path"form already works, since schemars resolves the given function directly. A new test pins that behavior.