-
-
Notifications
You must be signed in to change notification settings - Fork 413
fix(derive): support container-level serde(default) in KubeSchema #1999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ use darling::{ | |||||
| util::{IdentString, parse_expr}, | ||||||
| }; | ||||||
| use proc_macro2::TokenStream; | ||||||
| use syn::{Attribute, DeriveInput, Expr, Ident, Path, parse_quote}; | ||||||
| use syn::{Attribute, DeriveInput, Expr, Ident, Meta, Path, Token, parse_quote, punctuated::Punctuated}; | ||||||
|
|
||||||
| #[derive(FromField)] | ||||||
| #[darling(attributes(x_kube))] | ||||||
|
|
@@ -97,6 +97,13 @@ pub(crate) fn derive_validated_schema(input: TokenStream) -> TokenStream { | |||||
| let attribute_whitelist = ["serde", "schemars", "doc", "validate"]; | ||||||
| ast.attrs = remove_attributes(&ast.attrs, &attribute_whitelist); | ||||||
|
|
||||||
| // A bare container-level `#[serde(default)]` makes the schemars derive call | ||||||
| // `<Self as Default>::default()` on the generated mirror structs, which have no `Default` | ||||||
| // impl of their own. Emit delegating impls alongside the mirrors in that case. | ||||||
| // (The `default = "path"` form needs none of this: schemars calls the function directly.) | ||||||
| let original_default = has_container_serde_default(&ast.attrs) | ||||||
| .then(|| quote! { <#ident as #std::default::Default>::default() }); | ||||||
|
|
||||||
| let struct_data = match ast.data { | ||||||
| syn::Data::Struct(ref mut struct_data) => struct_data, | ||||||
| _ => return quote! {}, | ||||||
|
|
@@ -105,7 +112,21 @@ pub(crate) fn derive_validated_schema(input: TokenStream) -> TokenStream { | |||||
| // Preserve all serde attributes, to allow #[serde(rename_all = "camelCase")] or similar | ||||||
| let struct_attrs: Vec<TokenStream> = ast.attrs.iter().map(|attr| quote! {#attr}).collect(); | ||||||
| let mut property_modifications = vec![]; | ||||||
| let mut validated_default_impl = quote! {}; | ||||||
| if let syn::Fields::Named(fields) = &mut struct_data.fields { | ||||||
| if let Some(original) = &original_default { | ||||||
| let field_idents: Vec<Ident> = fields.named.iter().filter_map(|f| f.ident.clone()).collect(); | ||||||
| let generated = struct_name.as_ident(); | ||||||
| validated_default_impl = quote! { | ||||||
| #[automatically_derived] | ||||||
| impl #std::default::Default for #generated { | ||||||
| fn default() -> Self { | ||||||
| let original = #original; | ||||||
| Self { #(#field_idents: original.#field_idents),* } | ||||||
| } | ||||||
| } | ||||||
| }; | ||||||
| } | ||||||
| for field in &mut fields.named { | ||||||
| let XKube { | ||||||
| validations, | ||||||
|
|
@@ -131,6 +152,19 @@ pub(crate) fn derive_validated_schema(input: TokenStream) -> TokenStream { | |||||
| let merge_strategy = merge_strategy | ||||||
| .map(|strategy| quote! {#kube_core::merge_strategy_property(merge, 0, #strategy).unwrap();}); | ||||||
|
|
||||||
| let field_default_impl = original_default.as_ref().map(|original| { | ||||||
| let field_ident = &field.ident; | ||||||
| quote! { | ||||||
| #[automatically_derived] | ||||||
| impl #std::default::Default for Validated { | ||||||
| fn default() -> Self { | ||||||
| let original = #original; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as in https://github.com/kube-rs/kube/pull/1999/changes#r3407680513,
Suggested change
|
||||||
| Self { #field_ident: original.#field_ident } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| }); | ||||||
|
|
||||||
| // We need to prepend derive macros, as they were consumed by this macro processing, being a derive by itself. | ||||||
| property_modifications.push(quote! { | ||||||
| { | ||||||
|
|
@@ -142,6 +176,8 @@ pub(crate) fn derive_validated_schema(input: TokenStream) -> TokenStream { | |||||
| #field | ||||||
| } | ||||||
|
|
||||||
| #field_default_impl | ||||||
|
|
||||||
| let merge = &mut Validated::json_schema(generate); | ||||||
| #(#rules)* | ||||||
| #merge_strategy | ||||||
|
|
@@ -170,6 +206,8 @@ pub(crate) fn derive_validated_schema(input: TokenStream) -> TokenStream { | |||||
| #[allow(missing_docs)] | ||||||
| #ast | ||||||
|
|
||||||
| #validated_default_impl | ||||||
|
|
||||||
| use #kube_core::{Rule, Message, Reason, ListMerge, MapMerge, StructMerge}; | ||||||
| let s = &mut #generated_struct_name::json_schema(generate); | ||||||
| #(#struct_rules)* | ||||||
|
|
@@ -180,6 +218,21 @@ pub(crate) fn derive_validated_schema(input: TokenStream) -> TokenStream { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Detect a bare container-level `#[serde(default)]`. | ||||||
| // A bare `default` parses as `Meta::Path`; `default = "path"` is a `Meta::NameValue` | ||||||
| // and intentionally does not match (schemars resolves that function directly). | ||||||
| fn has_container_serde_default(attrs: &[Attribute]) -> bool { | ||||||
| attrs | ||||||
| .iter() | ||||||
| .filter(|attr| attr.path().is_ident("serde")) | ||||||
| .filter_map(|attr| { | ||||||
| attr.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated) | ||||||
| .ok() | ||||||
| }) | ||||||
| .flatten() | ||||||
| .any(|meta| matches!(meta, Meta::Path(ref path) if path.is_ident("default"))) | ||||||
| } | ||||||
|
|
||||||
| // Remove all unknown attributes from the list | ||||||
| fn remove_attributes(attrs: &[Attribute], witelist: &[&str]) -> Vec<Attribute> { | ||||||
| attrs | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -578,3 +578,75 @@ fn test_optional_int_or_string_nullable() { | |
| assert_eq!(optional.x_kubernetes_int_or_string, Some(true)); | ||
| assert_eq!(optional.nullable, Some(true)); | ||
| } | ||
|
|
||
| // Test for container-level serde(default) handling (issue #1805) | ||
| #[derive(CustomResource, KubeSchema, Serialize, Deserialize, Debug, Clone)] | ||
| #[kube(group = "clux.dev", version = "v1", kind = "ContainerDefault", namespaced)] | ||
| #[serde(rename_all = "camelCase", default)] | ||
| struct ContainerDefaultSpec { | ||
| #[x_kube(validation = "self % 2 == 1")] | ||
| replica_count: i32, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add another field to the structure, so that test validates multiple defaulted values? |
||
| } | ||
|
|
||
| impl Default for ContainerDefaultSpec { | ||
| fn default() -> Self { | ||
| Self { replica_count: 3 } | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_container_level_serde_default() { | ||
| use kube::core::CustomResourceExt; | ||
| let crd = ContainerDefault::crd(); | ||
| let spec_schema = &crd.spec.versions[0] | ||
| .schema | ||
| .as_ref() | ||
| .unwrap() | ||
| .open_api_v3_schema | ||
| .as_ref() | ||
| .unwrap() | ||
| .properties | ||
| .as_ref() | ||
| .unwrap()["spec"]; | ||
|
|
||
| // Defaults from the container-level Default impl reach the schema | ||
| let replica_count = &spec_schema.properties.as_ref().unwrap()["replicaCount"]; | ||
| assert_eq!(replica_count.default.as_ref().unwrap().0, serde_json::json!(3)); | ||
|
|
||
| // The CEL rule on the defaulted field survives | ||
| let validations = replica_count.x_kubernetes_validations.as_ref().unwrap(); | ||
| assert_eq!(validations[0].rule, "self % 2 == 1"); | ||
| } | ||
|
|
||
| // The `#[serde(default = "path")]` container form resolves the function directly and needs | ||
| // no generated Default impls; pin that it keeps working alongside the bare form. | ||
| #[derive(CustomResource, KubeSchema, Serialize, Deserialize, Debug, Clone)] | ||
| #[kube(group = "clux.dev", version = "v1", kind = "ContainerPathDefault", namespaced)] | ||
| #[serde(default = "container_path_default")] | ||
| struct ContainerPathDefaultSpec { | ||
| #[x_kube(validation = "self != ''")] | ||
| name: String, | ||
| } | ||
|
|
||
| fn container_path_default() -> ContainerPathDefaultSpec { | ||
| ContainerPathDefaultSpec { name: "x".into() } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_container_level_serde_default_path() { | ||
| use kube::core::CustomResourceExt; | ||
| let crd = ContainerPathDefault::crd(); | ||
| let spec_schema = &crd.spec.versions[0] | ||
| .schema | ||
| .as_ref() | ||
| .unwrap() | ||
| .open_api_v3_schema | ||
| .as_ref() | ||
| .unwrap() | ||
| .properties | ||
| .as_ref() | ||
| .unwrap()["spec"]; | ||
|
|
||
| let name = &spec_schema.properties.as_ref().unwrap()["name"]; | ||
| assert_eq!(name.default.as_ref().unwrap().0, serde_json::json!("x")); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming is a bit confusing, since this is actually unwrapped
original_default, I think it is better to shadoworiginal_defaultwith 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.