From 295c43199d999fc51faa3f2dbb0dd859a63cf48e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 8 Oct 2025 16:06:27 +0200 Subject: [PATCH 01/27] Add some tests for CRD generation for enums --- kube-derive/src/custom_resource.rs | 22 +- kube-derive/tests/crd_complex_enum_tests.rs | 519 ++++++++++++++++++ kube-derive/tests/crd_mixed_enum_test.rs | 234 ++++++++ ...num_test.rs => crd_top_level_enum_test.rs} | 0 kube-derive/tests/ui/union_fails.stderr | 4 +- 5 files changed, 767 insertions(+), 12 deletions(-) create mode 100644 kube-derive/tests/crd_complex_enum_tests.rs create mode 100644 kube-derive/tests/crd_mixed_enum_test.rs rename kube-derive/tests/{crd_enum_test.rs => crd_top_level_enum_test.rs} (100%) diff --git a/kube-derive/src/custom_resource.rs b/kube-derive/src/custom_resource.rs index 443653576..bc5a173a9 100644 --- a/kube-derive/src/custom_resource.rs +++ b/kube-derive/src/custom_resource.rs @@ -904,15 +904,17 @@ mod tests { #[test] fn test_derive_crd() { - let path = env::current_dir().unwrap().join("tests").join("crd_enum_test.rs"); - let file = fs::File::open(path).unwrap(); - runtime_macros::emulate_derive_macro_expansion(file, &[("CustomResource", derive)]).unwrap(); - - let path = env::current_dir() - .unwrap() - .join("tests") - .join("crd_schema_test.rs"); - let file = fs::File::open(path).unwrap(); - runtime_macros::emulate_derive_macro_expansion(file, &[("CustomResource", derive)]).unwrap(); + let files = [ + "crd_complex_enum_tests.rs", + "crd_mixed_enum_test.rs", + "crd_schema_test.rs", + "crd_top_level_enum_test.rs", + ]; + + for file in files { + let path = env::current_dir().unwrap().join("tests").join(file); + let file = fs::File::open(path).unwrap(); + runtime_macros::emulate_derive_macro_expansion(file, &[("CustomResource", derive)]).unwrap(); + } } } diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs new file mode 100644 index 000000000..111b39bf9 --- /dev/null +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -0,0 +1,519 @@ +#![allow(missing_docs)] +use assert_json_diff::assert_json_eq; +use kube::{CustomResource, CustomResourceExt}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use serde_json::json; + +/// A very simple enum with empty variants +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +enum NormalEnum { + /// First variant + A, + /// Second variant + B, +} + +/// An untagged enum with a nested enum inside +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[serde(untagged)] +enum UntaggedEnum { + /// Used in case the `one` field of tpye [`u32`] is present + A { one: String }, + /// Used in case the `two` field of type [`NormalEnum`] is present + B { two: NormalEnum }, + /// Used in case no fields are present + C {}, +} + +/// Put a [`UntaggedEnum`] behind `#[serde(flatten)]` +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +struct FlattenedUntaggedEnum { + #[serde(flatten)] + inner: UntaggedEnum, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "NormalEnumTest")] +struct NormalEnumTestSpec { + foo: NormalEnum, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "OptionalEnumTest")] +struct OptionalEnumTestSpec { + foo: Option, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "UntaggedEnumTest")] +struct UntaggedEnumTestSpec { + foo: UntaggedEnum, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "OptionalUntaggedEnumTest")] +struct OptionalUntaggedEnumTestSpec { + foo: Option, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "FlattenedUntaggedEnumTest")] +struct FlattenedUntaggedEnumTestSpec { + foo: FlattenedUntaggedEnum, +} + +/// Use `cargo test --package kube-derive print_crds -- --nocapture` to get the CRDs as YAML. +/// Afterwards you can use `kubectl apply -f -` to see if they are valid CRDs. +#[test] +fn print_crds() { + println!("{}", serde_yaml::to_string(&NormalEnumTest::crd()).unwrap()); + println!("---"); + println!("{}", serde_yaml::to_string(&OptionalEnumTest::crd()).unwrap()); + println!("---"); + println!("{}", serde_yaml::to_string(&UntaggedEnumTest::crd()).unwrap()); + println!("---"); + println!( + "{}", + serde_yaml::to_string(&OptionalUntaggedEnumTest::crd()).unwrap() + ); + println!("---"); + println!( + "{}", + serde_yaml::to_string(&FlattenedUntaggedEnumTest::crd()).unwrap() + ); +} + +#[test] +fn normal_enum() { + assert_json_eq!( + NormalEnumTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "normalenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "NormalEnumTest", + "plural": "normalenumtests", + "shortNames": [], + "singular": "normalenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for NormalEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "description": "A very simple enum with empty variants", + "enum": [ + "A", + "B" + ], + "type": "string" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "NormalEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[test] +fn optional_enum() { + assert_json_eq!( + OptionalEnumTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "optionalenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "OptionalEnumTest", + "plural": "optionalenumtests", + "shortNames": [], + "singular": "optionalenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for OptionalEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "description": "A very simple enum with empty variants", + "enum": [ + "A", + "B" + ], + "type": "string" + }, + { + "enum": [ + null + ], + "nullable": true + } + ] + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); + + // The CustomResourceDefinition "optionalenumtests.clux.dev" is invalid: + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].description: Forbidden: must be empty to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].type: Forbidden: must be empty to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[1].nullable: Forbidden: must be false to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].type: Required value: must not be empty for specified object fields + panic!("This CRD is currently not accepted by Kubernetes!"); +} + +#[test] +fn untagged_enum() { + assert_json_eq!( + UntaggedEnumTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "untaggedenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "UntaggedEnumTest", + "plural": "untaggedenumtests", + "shortNames": [], + "singular": "untaggedenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for UntaggedEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "required": [ + "one" + ] + }, + { + "required": [ + "two" + ] + }, + { + "description": "Used in case no fields are present", + "type": "object" + } + ], + "description": "An untagged enum with a nested enum inside", + "properties": { + "one": { + "description": "Used in case the `one` field of tpye [`u32`] is present", + "type": "string" + }, + "two": { + "description": "Used in case the `two` field of type [`NormalEnum`] is present", + "enum": [ + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "UntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); + + // The CustomResourceDefinition "untaggedenumtests.clux.dev" is invalid: + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].description: Forbidden: must be empty to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].type: Forbidden: must be empty to be structural + panic!("This CRD is currently not accepted by Kubernetes!"); +} + +#[test] +fn optional_untagged_enum() { + assert_json_eq!( + OptionalUntaggedEnumTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "optionaluntaggedenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "OptionalUntaggedEnumTest", + "plural": "optionaluntaggedenumtests", + "shortNames": [], + "singular": "optionaluntaggedenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for OptionalUntaggedEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "anyOf": [ + { + "required": [ + "one" + ] + }, + { + "required": [ + "two" + ] + }, + { + "description": "Used in case no fields are present", + "type": "object" + } + ] + }, + { + "enum": [ + null + ], + "nullable": true + } + ], + "properties": { + "one": { + "description": "Used in case the `one` field of tpye [`u32`] is present", + "type": "string" + }, + "two": { + "description": "Used in case the `two` field of type [`NormalEnum`] is present", + "enum": [ + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalUntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); + + // The CustomResourceDefinition "optionaluntaggedenumtests.clux.dev" is invalid: + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].anyOf[2].description: Forbidden: must be empty to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].anyOf[2].type: Forbidden: must be empty to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[1].nullable: Forbidden: must be false to be structural + panic!("This CRD is currently not accepted by Kubernetes!"); +} + +#[test] +fn flattened_untagged_enum() { + assert_json_eq!( + FlattenedUntaggedEnumTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "flatteneduntaggedenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "FlattenedUntaggedEnumTest", + "plural": "flatteneduntaggedenumtests", + "shortNames": [], + "singular": "flatteneduntaggedenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for FlattenedUntaggedEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "required": [ + "one" + ] + }, + { + "required": [ + "two" + ] + }, + { + "description": "Used in case no fields are present", + "type": "object" + } + ], + "description": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`,", + "properties": { + "one": { + "description": "Used in case the `one` field of tpye [`u32`] is present", + "type": "string" + }, + "two": { + "description": "Used in case the `two` field of type [`NormalEnum`] is present", + "enum": [ + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "FlattenedUntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); + + // The CustomResourceDefinition "flatteneduntaggedenumtests.clux.dev" is invalid: + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].description: Forbidden: must be empty to be structural + // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].type: Forbidden: must be empty to be structural + panic!("This CRD is currently not accepted by Kubernetes!"); +} diff --git a/kube-derive/tests/crd_mixed_enum_test.rs b/kube-derive/tests/crd_mixed_enum_test.rs new file mode 100644 index 000000000..08dfb8249 --- /dev/null +++ b/kube-derive/tests/crd_mixed_enum_test.rs @@ -0,0 +1,234 @@ +#![allow(missing_docs)] +use assert_json_diff::assert_json_eq; +use kube::{CustomResource, CustomResourceExt}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use serde_json::json; + +/// This enum is invalid, as "plain" (string) variants are mixed with object variants +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "InvalidEnum1")] +enum InvalidEnum1Spec { + /// Unit variant (represented as string) + A, + /// Takes an [`u32`] (represented as object) + B(u32), +} + +/// This enum is invalid, as "plain" (string) variants are mixed with object variants +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "InvalidEnum2")] +enum InvalidEnum2Spec { + /// Unit variant (represented as string) + A, + /// Takes a single field (represented as object) + B { inner: u32 }, +} + +/// This enum is valid, as all variants are objects +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "ValidEnum3")] +enum ValidEnum3Spec { + /// Takes an [`String`] (represented as object) + A(String), + /// Takes an [`u32`] (represented as object) + B(u32), +} + +// This enum intentionally has no documentation to increase test coverage! +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "ValidEnum4")] +enum ValidEnum4Spec { + A(String), + B { inner: u32 }, +} + +/// Use `cargo test --package kube-derive print_crds -- --nocapture` to get the CRDs as YAML. +/// Afterwards you can use `kubectl apply -f -` to see if they are valid CRDs. +#[test] +fn print_crds() { + println!("{}", serde_yaml::to_string(&ValidEnum3::crd()).unwrap()); + println!("---"); + println!("{}", serde_yaml::to_string(&ValidEnum4::crd()).unwrap()); +} + +#[test] +#[should_panic = "Enum variant set [String(\"A\")] has type Single(String) but was already defined as Some(Single(Object)). The instance type must be equal for all subschema variants."] +fn invalid_enum_1() { + InvalidEnum1::crd(); +} + +#[test] +#[should_panic = "Enum variant set [String(\"A\")] has type Single(String) but was already defined as Some(Single(Object)). The instance type must be equal for all subschema variants."] +fn invalid_enum_2() { + InvalidEnum2::crd(); +} + +#[test] +fn valid_enum_3() { + assert_json_eq!( + ValidEnum3::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "validenum3s.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "ValidEnum3", + "plural": "validenum3s", + "shortNames": [], + "singular": "validenum3" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for ValidEnum3Spec via `CustomResource`", + "properties": { + "spec": { + "description": "This enum is valid, as all variants are objects", + "oneOf": [ + { + "required": [ + "A" + ] + }, + { + "required": [ + "B" + ] + } + ], + "properties": { + "A": { + "description": "Takes an [`String`] (represented as object)", + "type": "string" + }, + "B": { + "description": "Takes an [`u32`] (represented as object)", + "format": "uint32", + "minimum": 0.0, + "type": "integer" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "ValidEnum3", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[test] +fn valid_enum_4() { + assert_json_eq!( + ValidEnum4::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "validenum4s.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "ValidEnum4", + "plural": "validenum4s", + "shortNames": [], + "singular": "validenum4" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for ValidEnum4Spec via `CustomResource`", + "properties": { + "spec": { + "oneOf": [ + { + "required": [ + "A" + ] + }, + { + "required": [ + "B" + ] + } + ], + "properties": { + "A": { + "type": "string" + }, + "B": { + "properties": { + "inner": { + "format": "uint32", + "minimum": 0.0, + "type": "integer" + } + }, + "required": [ + "inner" + ], + "type": "object" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "ValidEnum4", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[test] +#[should_panic = "Enum variant set [String(\"A\")] has type Single(String) but was already defined as Some(Single(Object)). The instance type must be equal for all subschema variants."] +fn struct_with_enum_1() { + #[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] + #[kube(group = "clux.dev", version = "v1", kind = "Foo")] + struct FooSpec { + foo: InvalidEnum1, + } + + Foo::crd(); +} diff --git a/kube-derive/tests/crd_enum_test.rs b/kube-derive/tests/crd_top_level_enum_test.rs similarity index 100% rename from kube-derive/tests/crd_enum_test.rs rename to kube-derive/tests/crd_top_level_enum_test.rs diff --git a/kube-derive/tests/ui/union_fails.stderr b/kube-derive/tests/ui/union_fails.stderr index 3bc077716..ae002ca36 100644 --- a/kube-derive/tests/ui/union_fails.stderr +++ b/kube-derive/tests/ui/union_fails.stderr @@ -7,7 +7,7 @@ error: Unions can not #[derive(CustomResource)] error: Serde does not support derive for unions --> tests/ui/union_fails.rs:8:1 | -8 | / union FooSpec { -9 | | int: u32, + 8 | / union FooSpec { + 9 | | int: u32, 10 | | } | |_^ From 13db30ccaa045095898bf69300830373b4b7d92a Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 9 Oct 2025 14:27:40 +0200 Subject: [PATCH 02/27] Update assertions to valid CRDs from kube 1.1.0 --- kube-derive/tests/crd_complex_enum_tests.rs | 87 +++++---------------- 1 file changed, 19 insertions(+), 68 deletions(-) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index 111b39bf9..834cd4246 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -180,22 +180,13 @@ fn optional_enum() { "spec": { "properties": { "foo": { - "anyOf": [ - { - "description": "A very simple enum with empty variants", - "enum": [ - "A", - "B" - ], - "type": "string" - }, - { - "enum": [ - null - ], - "nullable": true - } - ] + "description": "A very simple enum with empty variants", + "enum": [ + "A", + "B" + ], + "nullable": true, + "type": "string" } }, "type": "object" @@ -217,13 +208,6 @@ fn optional_enum() { } ) ); - - // The CustomResourceDefinition "optionalenumtests.clux.dev" is invalid: - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].description: Forbidden: must be empty to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].type: Forbidden: must be empty to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[1].nullable: Forbidden: must be false to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].type: Required value: must not be empty for specified object fields - panic!("This CRD is currently not accepted by Kubernetes!"); } #[test] @@ -269,10 +253,7 @@ fn untagged_enum() { "two" ] }, - { - "description": "Used in case no fields are present", - "type": "object" - } + {} ], "description": "An untagged enum with a nested enum inside", "properties": { @@ -314,11 +295,6 @@ fn untagged_enum() { } ) ); - - // The CustomResourceDefinition "untaggedenumtests.clux.dev" is invalid: - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].description: Forbidden: must be empty to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].type: Forbidden: must be empty to be structural - panic!("This CRD is currently not accepted by Kubernetes!"); } #[test] @@ -355,30 +331,19 @@ fn optional_untagged_enum() { "foo": { "anyOf": [ { - "anyOf": [ - { - "required": [ - "one" - ] - }, - { - "required": [ - "two" - ] - }, - { - "description": "Used in case no fields are present", - "type": "object" - } + "required": [ + "one" ] }, { - "enum": [ - null - ], - "nullable": true - } + "required": [ + "two" + ] + }, + {} ], + "description": "An untagged enum with a nested enum inside", + "nullable": true, "properties": { "one": { "description": "Used in case the `one` field of tpye [`u32`] is present", @@ -415,12 +380,6 @@ fn optional_untagged_enum() { } ) ); - - // The CustomResourceDefinition "optionaluntaggedenumtests.clux.dev" is invalid: - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].anyOf[2].description: Forbidden: must be empty to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[0].anyOf[2].type: Forbidden: must be empty to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[1].nullable: Forbidden: must be false to be structural - panic!("This CRD is currently not accepted by Kubernetes!"); } #[test] @@ -466,12 +425,9 @@ fn flattened_untagged_enum() { "two" ] }, - { - "description": "Used in case no fields are present", - "type": "object" - } + {} ], - "description": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`,", + "description": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`", "properties": { "one": { "description": "Used in case the `one` field of tpye [`u32`] is present", @@ -511,9 +467,4 @@ fn flattened_untagged_enum() { } ) ); - - // The CustomResourceDefinition "flatteneduntaggedenumtests.clux.dev" is invalid: - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].description: Forbidden: must be empty to be structural - // * spec.validation.openAPIV3Schema.properties[spec].properties[foo].anyOf[2].type: Forbidden: must be empty to be structural - panic!("This CRD is currently not accepted by Kubernetes!"); } From fb77413dee574962dbd67227613833d50748f8e8 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Thu, 9 Oct 2025 14:37:39 +0200 Subject: [PATCH 03/27] test: Add some un-documented variants to the NormalEnum --- kube-derive/tests/crd_complex_enum_tests.rs | 216 ++++++++++---------- 1 file changed, 112 insertions(+), 104 deletions(-) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index 834cd4246..4520fa6ef 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -12,6 +12,10 @@ enum NormalEnum { A, /// Second variant B, + + // No doc-comments on these variants + C, + D, } /// An untagged enum with a nested enum inside @@ -89,61 +93,63 @@ fn normal_enum() { assert_json_eq!( NormalEnumTest::crd(), json!( - { - "apiVersion": "apiextensions.k8s.io/v1", - "kind": "CustomResourceDefinition", - "metadata": { - "name": "normalenumtests.clux.dev" + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "normalenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "NormalEnumTest", + "plural": "normalenumtests", + "shortNames": [], + "singular": "normalenumtest" }, - "spec": { - "group": "clux.dev", - "names": { - "categories": [], - "kind": "NormalEnumTest", - "plural": "normalenumtests", - "shortNames": [], - "singular": "normalenumtest" - }, - "scope": "Cluster", - "versions": [ - { - "additionalPrinterColumns": [], - "name": "v1", - "schema": { - "openAPIV3Schema": { - "description": "Auto-generated derived type for NormalEnumTestSpec via `CustomResource`", - "properties": { - "spec": { - "properties": { - "foo": { - "description": "A very simple enum with empty variants", - "enum": [ - "A", - "B" - ], - "type": "string" - } - }, - "required": [ - "foo" - ], - "type": "object" - } - }, - "required": [ - "spec" - ], - "title": "NormalEnumTest", - "type": "object" - } - }, - "served": true, - "storage": true, - "subresources": {} - } - ] - } + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for NormalEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "description": "A very simple enum with empty variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "NormalEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] } + } ) ); } @@ -153,59 +159,61 @@ fn optional_enum() { assert_json_eq!( OptionalEnumTest::crd(), json!( - { - "apiVersion": "apiextensions.k8s.io/v1", - "kind": "CustomResourceDefinition", - "metadata": { - "name": "optionalenumtests.clux.dev" + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "optionalenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "OptionalEnumTest", + "plural": "optionalenumtests", + "shortNames": [], + "singular": "optionalenumtest" }, - "spec": { - "group": "clux.dev", - "names": { - "categories": [], - "kind": "OptionalEnumTest", - "plural": "optionalenumtests", - "shortNames": [], - "singular": "optionalenumtest" - }, - "scope": "Cluster", - "versions": [ - { - "additionalPrinterColumns": [], - "name": "v1", - "schema": { - "openAPIV3Schema": { - "description": "Auto-generated derived type for OptionalEnumTestSpec via `CustomResource`", - "properties": { - "spec": { - "properties": { - "foo": { - "description": "A very simple enum with empty variants", - "enum": [ - "A", - "B" - ], - "nullable": true, - "type": "string" - } - }, - "type": "object" - } - }, - "required": [ - "spec" - ], - "title": "OptionalEnumTest", - "type": "object" - } - }, - "served": true, - "storage": true, - "subresources": {} - } - ] - } + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for OptionalEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "description": "A very simple enum with empty variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "nullable": true, + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] } + } ) ); } From 0cb62f1da0064044975a2e662509ea4126b733c4 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Thu, 9 Oct 2025 15:32:23 +0200 Subject: [PATCH 04/27] test: Partial new impl for one_of hoisting --- kube-core/src/schema.rs | 109 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 1 deletion(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index 479918a8e..c91ef5d61 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -8,7 +8,10 @@ use schemars::{transform::Transform, JsonSchema}; use serde::{Deserialize, Serialize}; use serde_json::Value; -use std::collections::{btree_map::Entry, BTreeMap, BTreeSet}; +use std::{ + collections::{btree_map::Entry, BTreeMap, BTreeSet}, + ops::{Deref, Not}, +}; /// schemars [`Visitor`] that rewrites a [`Schema`] to conform to Kubernetes' "structural schema" rules /// @@ -246,10 +249,114 @@ enum SingleOrVec { Vec(Vec), } +#[cfg(test)] +mod test { + use assert_json_diff::assert_json_eq; + use schemars::{json_schema, schema_for, schema_for_value, JsonSchema}; + use serde::{de::Expected, Deserialize, Serialize}; + use serde_json::json; + + /// A very simple enum with empty variants + #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] + enum NormalEnum { + /// First variant + A, + /// Second variant + B, + + // No doc-comments on these variants + C, + D, + } + + #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] + enum B {} + + #[test] + fn hoisting_a_schema() { + let incoming = json_schema!( + { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": "A very simple enum with empty variants", + "oneOf": [ + { + "enum": [ + "C", + "D" + ], + "type": "string" + }, + { + "const": "A", + "description": "First variant", + "type": "string" + }, + { + "const": "B", + "description": "Second variant", + "type": "string" + } + ], + "title": "NormalEnum" + } + ); + + // Initial check that the text schema above is correct for NormalEnum + assert_json_eq!(schema_for!(NormalEnum), incoming); + + let expected = json_schema!( + { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": "A very simple enum with empty variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ], + "title": "NormalEnum" + } + ); + + // let actual = hoist + + // assert_json_eq!(expected, actual); + } +} + +fn hoist_one_of_enum(incoming: Schema) -> Schema { + let Schema::Object(SchemaObject { + subschemas: Some(subschemas), + .. + }) = &incoming + else { + return incoming; + }; + + let SubschemaValidation { + one_of: Some(one_of), .. + } = subschemas.deref() + else { + return incoming; + }; + + if one_of.is_empty() { + return incoming; + } + + // now the meat. Need to get the oneOf variants up into `enum` + // panic if the types differ + + + todo!("finish it") +} + impl Transform for StructuralSchemaRewriter { fn transform(&mut self, transform_schema: &mut schemars::Schema) { schemars::transform::transform_subschemas(self, transform_schema); + // TODO (@NickLarsenNZ): Replace with conversion function let mut schema: SchemaObject = match serde_json::from_value(transform_schema.clone().to_value()).ok() { Some(schema) => schema, From 85eaf36d089b15665727d9b63d82472cd3597059 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 9 Oct 2025 16:42:28 +0200 Subject: [PATCH 05/27] Finish enum oneOf hoisting --- kube-core/src/schema.rs | 158 +++++++++++++++++++++++----------------- 1 file changed, 90 insertions(+), 68 deletions(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index c91ef5d61..ab6c87e8b 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::{ collections::{btree_map::Entry, BTreeMap, BTreeSet}, - ops::{Deref, Not}, + }; /// schemars [`Visitor`] that rewrites a [`Schema`] to conform to Kubernetes' "structural schema" rules @@ -252,9 +252,10 @@ enum SingleOrVec { #[cfg(test)] mod test { use assert_json_diff::assert_json_eq; - use schemars::{json_schema, schema_for, schema_for_value, JsonSchema}; - use serde::{de::Expected, Deserialize, Serialize}; - use serde_json::json; + use schemars::{json_schema, schema_for, JsonSchema}; + use serde::{Deserialize, Serialize}; + + use super::*; /// A very simple enum with empty variants #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] @@ -274,82 +275,103 @@ mod test { #[test] fn hoisting_a_schema() { - let incoming = json_schema!( - { - "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with empty variants", - "oneOf": [ - { - "enum": [ - "C", - "D" + let schemars_schema = schema_for!(NormalEnum); + let mut kube_schema: crate::schema::Schema = + schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); + + assert_json_eq!( + schemars_schema, + json_schema!( + { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": "A very simple enum with empty variants", + "oneOf": [ + { + "enum": [ + "C", + "D" + ], + "type": "string" + }, + { + "const": "A", + "description": "First variant", + "type": "string" + }, + { + "const": "B", + "description": "Second variant", + "type": "string" + } ], - "type": "string" - }, - { - "const": "A", - "description": "First variant", - "type": "string" - }, - { - "const": "B", - "description": "Second variant", - "type": "string" + "title": "NormalEnum" } - ], - "title": "NormalEnum" - } + ) ); - - // Initial check that the text schema above is correct for NormalEnum - assert_json_eq!(schema_for!(NormalEnum), incoming); - - let expected = json_schema!( - { - "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with empty variants", - "type": "string", - "enum": [ - "C", - "D", - "A", - "B" - ], - "title": "NormalEnum" - } + hoist_one_of_enum(&mut kube_schema); + assert_json_eq!( + kube_schema, + json_schema!( + { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": "A very simple enum with empty variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ], + "title": "NormalEnum" + } + ) ); - - // let actual = hoist - - // assert_json_eq!(expected, actual); } } -fn hoist_one_of_enum(incoming: Schema) -> Schema { - let Schema::Object(SchemaObject { +#[cfg(test)] +fn schemars_schema_to_kube_schema(incoming: schemars::Schema) -> Result { + serde_json::from_value(incoming.to_value()) +} + +#[cfg(test)] +fn hoist_one_of_enum(schema: &mut Schema) { + if let Schema::Object(SchemaObject { subschemas: Some(subschemas), + instance_type: object_type, + enum_values: object_ebum, .. - }) = &incoming - else { - return incoming; - }; - - let SubschemaValidation { + }) = schema && let SubschemaValidation { one_of: Some(one_of), .. - } = subschemas.deref() - else { - return incoming; - }; - - if one_of.is_empty() { - return incoming; - } - - // now the meat. Need to get the oneOf variants up into `enum` - // panic if the types differ + } = &**subschemas + && !one_of.is_empty() + { + let mut types = one_of.iter().map(|obj| match obj { + Schema::Object(SchemaObject { instance_type: Some(type_), ..}) => type_, + Schema::Object(_) => panic!("oneOf variants need to define a type!: {obj:?}"), + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }); + let enums = one_of.iter().flat_map(|obj| match obj { + Schema::Object(SchemaObject {enum_values: Some(enum_), ..}) => enum_.clone(), + Schema::Object(SchemaObject {other, ..})=> match other.get("const") { + Some(const_) => vec![const_.clone()], + None => panic!("oneOf variant did not provide \"enum\" or \"const\": {obj:?}"), + }, + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }); + + let first_type = types + .next() + .expect("oneOf must have at least one variant - we already checked that"); + if types.any(|t| t != first_type) { + panic!("All oneOf variants must have the same type"); + } + *object_type = Some(first_type.clone()); + *object_ebum = Some(enums.collect()); + subschemas.one_of = None; - todo!("finish it") + } } impl Transform for StructuralSchemaRewriter { From 285ab646422f6dc0cc3d5ff8051b7c78a71dbb6e Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 10 Oct 2025 13:02:24 +0200 Subject: [PATCH 06/27] adjust and document the hoist_one_of_enum function Note: As described on the function, it is overly documented to express intent where it can be a little unclear. Ideally where would be some clearer official Kubernetes documentation on the differences between regular OpenAPI 3 schemas, and what Kubernetes accepts. --- kube-core/src/schema.rs | 109 +++++++++++++++++++++++++++++----------- 1 file changed, 81 insertions(+), 28 deletions(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index ab6c87e8b..8c2affd8d 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::{ collections::{btree_map::Entry, BTreeMap, BTreeSet}, - + ops::Deref as _, }; /// schemars [`Visitor`] that rewrites a [`Schema`] to conform to Kubernetes' "structural schema" rules @@ -252,7 +252,7 @@ enum SingleOrVec { #[cfg(test)] mod test { use assert_json_diff::assert_json_eq; - use schemars::{json_schema, schema_for, JsonSchema}; + use schemars::{json_schema, schema_for, JsonSchema}; use serde::{Deserialize, Serialize}; use super::*; @@ -276,8 +276,6 @@ mod test { #[test] fn hoisting_a_schema() { let schemars_schema = schema_for!(NormalEnum); - let mut kube_schema: crate::schema::Schema = - schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); assert_json_eq!( schemars_schema, @@ -308,9 +306,14 @@ mod test { } ) ); - hoist_one_of_enum(&mut kube_schema); + + + let kube_schema: crate::schema::Schema = + schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); + + let hoisted_kube_schema = hoist_one_of_enum(kube_schema); assert_json_eq!( - kube_schema, + hoisted_kube_schema, json_schema!( { "$schema": "https://json-schema.org/draft/2020-12/schema", @@ -334,44 +337,94 @@ fn schemars_schema_to_kube_schema(incoming: schemars::Schema) -> Result Schema { + // Run some initial checks in case there is nothing to do + let Schema::Object(SchemaObject { subschemas: Some(subschemas), - instance_type: object_type, - enum_values: object_ebum, .. - }) = schema && let SubschemaValidation { + }) = &incoming + else { + return incoming; + }; + + let SubschemaValidation { one_of: Some(one_of), .. - } = &**subschemas - && !one_of.is_empty() + } = subschemas.deref() + else { + return incoming; + }; + + if one_of.is_empty() { + return incoming; + } + + // At this point, we need to create a new Schema and hoist the `oneOf` + // variants' `enum`/`const` values up into a parent `enum`. + let mut new_schema = incoming.clone(); + if let Schema::Object(SchemaObject { + subschemas: Some(new_subschemas), + instance_type: new_instance_type, + enum_values: new_enum_values, + .. + }) = &mut new_schema { + // For each `oneOf`, get the `type`. + // Panic if it has no `type`, or if the entry is a boolean. let mut types = one_of.iter().map(|obj| match obj { - Schema::Object(SchemaObject { instance_type: Some(type_), ..}) => type_, + Schema::Object(SchemaObject { + instance_type: Some(r#type), + .. + }) => r#type, + // TODO (@NickLarsenNZ): Is it correct that JSON Schema oneOf must have a type? Schema::Object(_) => panic!("oneOf variants need to define a type!: {obj:?}"), Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), }); - let enums = one_of.iter().flat_map(|obj| match obj { - Schema::Object(SchemaObject {enum_values: Some(enum_), ..}) => enum_.clone(), - Schema::Object(SchemaObject {other, ..})=> match other.get("const") { - Some(const_) => vec![const_.clone()], - None => panic!("oneOf variant did not provide \"enum\" or \"const\": {obj:?}"), - }, - Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), - }); - let first_type = types + // Get the first `type` value, then panic if any subsequent `type` values differ. + let hoisted_instance_type = types .next() .expect("oneOf must have at least one variant - we already checked that"); - if types.any(|t| t != first_type) { + // TODO (@NickLarsenNZ): Didn't sbernauer say that the types + if types.any(|t| t != hoisted_instance_type) { panic!("All oneOf variants must have the same type"); } - *object_type = Some(first_type.clone()); - *object_ebum = Some(enums.collect()); - subschemas.one_of = None; + *new_instance_type = Some(hoisted_instance_type.clone()); + + // For each `oneOf` entry, iterate over the `enum` and `const` values. + // Panic on an entry that doesn't contain an `enum` or `const`. + let new_enums = one_of.iter().flat_map(|obj| match obj { + Schema::Object(SchemaObject { + enum_values: Some(r#enum), + .. + }) => r#enum.clone(), + // Warning: The `const` check below must come after the enum check above. + // Otherwise it will panic on a valid entry with an `enum`. + Schema::Object(SchemaObject { other, .. }) => match other.get("const") { + Some(r#const) => vec![r#const.clone()], + None => panic!("oneOf variant did not provide \"enum\" or \"const\": {obj:?}"), + }, + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }); + + // Just in case there were existing enum values, add to them. + // TODO (@NickLarsenNZ): Check if `oneOf` and `enum` are mutually exclusive for a valid spec. + new_enum_values.get_or_insert_default().extend(new_enums); + // We can clear out the existing oneOf's, since they will be hoisted below. + new_subschemas.one_of = None; } + + new_schema } impl Transform for StructuralSchemaRewriter { From e256f3ab553bd56b2013486de676121651c13757 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 10 Oct 2025 14:05:58 +0200 Subject: [PATCH 07/27] test: Add more basic testing to ensure hoisting is not performed when there are no oneOfs at the top level of the schema --- kube-core/src/schema.rs | 56 +++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index 8c2affd8d..b738323be 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -257,7 +257,14 @@ mod test { use super::*; - /// A very simple enum with empty variants + /// A very simple enum with unit variants, and no comments + #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] + enum NormalEnumNoComments { + A, + B, + } + + /// A very simple enum with unit variants, and comments #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] enum NormalEnum { /// First variant @@ -270,19 +277,49 @@ mod test { D, } - #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] - enum B {} + #[test] + fn schema_for_enum_without_comments() { + let schemars_schema = schema_for!(NormalEnumNoComments); + + assert_json_eq!( + schemars_schema, + // replace the json_schema with this to get the full output. + // serde_json::json!(42) + json_schema!( + { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "description": "A very simple enum with unit variants, and no comments", + "enum": [ + "A", + "B" + ], + "title": "NormalEnumNoComments", + "type": "string" + } + ) + ); + + let kube_schema: crate::schema::Schema = + schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); + + let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); + + // No hoisting needed + assert_json_eq!(hoisted_kube_schema, kube_schema); + } #[test] - fn hoisting_a_schema() { + fn schema_for_enum_with_comments() { let schemars_schema = schema_for!(NormalEnum); assert_json_eq!( schemars_schema, + // replace the json_schema with this to get the full output. + // serde_json::json!(42) json_schema!( { "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with empty variants", + "description": "A very simple enum with unit variants, and comments", "oneOf": [ { "enum": [ @@ -311,13 +348,18 @@ mod test { let kube_schema: crate::schema::Schema = schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); - let hoisted_kube_schema = hoist_one_of_enum(kube_schema); + let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); + + assert_ne!( + hoisted_kube_schema, kube_schema, + "Hoisting was performed, so hoisted_kube_schema != kube_schema" + ); assert_json_eq!( hoisted_kube_schema, json_schema!( { "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with empty variants", + "description": "A very simple enum with unit variants, and comments", "type": "string", "enum": [ "C", From 35034c5c9a35a3cddb1f960e8af305ec1d672179 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 10 Oct 2025 15:08:47 +0200 Subject: [PATCH 08/27] hoist anyOf optional enums --- kube-core/src/schema.rs | 338 ++++++++++++++++++++++++---------------- 1 file changed, 205 insertions(+), 133 deletions(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index b738323be..db0d534c1 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -7,7 +7,7 @@ use schemars::{transform::Transform, JsonSchema}; use serde::{Deserialize, Serialize}; -use serde_json::Value; +use serde_json::{json, Value}; use std::{ collections::{btree_map::Entry, BTreeMap, BTreeSet}, ops::Deref as _, @@ -249,130 +249,130 @@ enum SingleOrVec { Vec(Vec), } -#[cfg(test)] -mod test { - use assert_json_diff::assert_json_eq; - use schemars::{json_schema, schema_for, JsonSchema}; - use serde::{Deserialize, Serialize}; - - use super::*; - - /// A very simple enum with unit variants, and no comments - #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] - enum NormalEnumNoComments { - A, - B, - } - - /// A very simple enum with unit variants, and comments - #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] - enum NormalEnum { - /// First variant - A, - /// Second variant - B, - - // No doc-comments on these variants - C, - D, - } - - #[test] - fn schema_for_enum_without_comments() { - let schemars_schema = schema_for!(NormalEnumNoComments); - - assert_json_eq!( - schemars_schema, - // replace the json_schema with this to get the full output. - // serde_json::json!(42) - json_schema!( - { - "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with unit variants, and no comments", - "enum": [ - "A", - "B" - ], - "title": "NormalEnumNoComments", - "type": "string" - } - ) - ); - - let kube_schema: crate::schema::Schema = - schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); - - let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); - - // No hoisting needed - assert_json_eq!(hoisted_kube_schema, kube_schema); - } - - #[test] - fn schema_for_enum_with_comments() { - let schemars_schema = schema_for!(NormalEnum); - - assert_json_eq!( - schemars_schema, - // replace the json_schema with this to get the full output. - // serde_json::json!(42) - json_schema!( - { - "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with unit variants, and comments", - "oneOf": [ - { - "enum": [ - "C", - "D" - ], - "type": "string" - }, - { - "const": "A", - "description": "First variant", - "type": "string" - }, - { - "const": "B", - "description": "Second variant", - "type": "string" - } - ], - "title": "NormalEnum" - } - ) - ); - - - let kube_schema: crate::schema::Schema = - schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); - - let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); - - assert_ne!( - hoisted_kube_schema, kube_schema, - "Hoisting was performed, so hoisted_kube_schema != kube_schema" - ); - assert_json_eq!( - hoisted_kube_schema, - json_schema!( - { - "$schema": "https://json-schema.org/draft/2020-12/schema", - "description": "A very simple enum with unit variants, and comments", - "type": "string", - "enum": [ - "C", - "D", - "A", - "B" - ], - "title": "NormalEnum" - } - ) - ); - } -} +// #[cfg(test)] +// mod test { +// use assert_json_diff::assert_json_eq; +// use schemars::{json_schema, schema_for, JsonSchema}; +// use serde::{Deserialize, Serialize}; + +// use super::*; + +// /// A very simple enum with unit variants, and no comments +// #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +// enum NormalEnumNoComments { +// A, +// B, +// } + +// /// A very simple enum with unit variants, and comments +// #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +// enum NormalEnum { +// /// First variant +// A, +// /// Second variant +// B, + +// // No doc-comments on these variants +// C, +// D, +// } + +// #[test] +// fn schema_for_enum_without_comments() { +// let schemars_schema = schema_for!(NormalEnumNoComments); + +// assert_json_eq!( +// schemars_schema, +// // replace the json_schema with this to get the full output. +// // serde_json::json!(42) +// json_schema!( +// { +// "$schema": "https://json-schema.org/draft/2020-12/schema", +// "description": "A very simple enum with unit variants, and no comments", +// "enum": [ +// "A", +// "B" +// ], +// "title": "NormalEnumNoComments", +// "type": "string" +// } +// ) +// ); + +// let kube_schema: crate::schema::Schema = +// schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); + +// let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); + +// // No hoisting needed +// assert_json_eq!(hoisted_kube_schema, kube_schema); +// } + +// #[test] +// fn schema_for_enum_with_comments() { +// let schemars_schema = schema_for!(NormalEnum); + +// assert_json_eq!( +// schemars_schema, +// // replace the json_schema with this to get the full output. +// // serde_json::json!(42) +// json_schema!( +// { +// "$schema": "https://json-schema.org/draft/2020-12/schema", +// "description": "A very simple enum with unit variants, and comments", +// "oneOf": [ +// { +// "enum": [ +// "C", +// "D" +// ], +// "type": "string" +// }, +// { +// "const": "A", +// "description": "First variant", +// "type": "string" +// }, +// { +// "const": "B", +// "description": "Second variant", +// "type": "string" +// } +// ], +// "title": "NormalEnum" +// } +// ) +// ); + + +// let kube_schema: crate::schema::Schema = +// schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); + +// let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); + +// assert_ne!( +// hoisted_kube_schema, kube_schema, +// "Hoisting was performed, so hoisted_kube_schema != kube_schema" +// ); +// assert_json_eq!( +// hoisted_kube_schema, +// json_schema!( +// { +// "$schema": "https://json-schema.org/draft/2020-12/schema", +// "description": "A very simple enum with unit variants, and comments", +// "type": "string", +// "enum": [ +// "C", +// "D", +// "A", +// "B" +// ], +// "title": "NormalEnum" +// } +// ) +// ); +// } +// } #[cfg(test)] fn schemars_schema_to_kube_schema(incoming: schemars::Schema) -> Result { @@ -388,12 +388,12 @@ fn schemars_schema_to_kube_schema(incoming: schemars::Schema) -> Result Schema { +fn hoist_one_of_enum(incoming: SchemaObject) -> SchemaObject { // Run some initial checks in case there is nothing to do - let Schema::Object(SchemaObject { + let SchemaObject { subschemas: Some(subschemas), .. - }) = &incoming + } = &incoming else { return incoming; }; @@ -412,12 +412,12 @@ fn hoist_one_of_enum(incoming: Schema) -> Schema { // At this point, we need to create a new Schema and hoist the `oneOf` // variants' `enum`/`const` values up into a parent `enum`. let mut new_schema = incoming.clone(); - if let Schema::Object(SchemaObject { + if let SchemaObject { subschemas: Some(new_subschemas), instance_type: new_instance_type, enum_values: new_enum_values, .. - }) = &mut new_schema + } = &mut new_schema { // For each `oneOf`, get the `type`. // Panic if it has no `type`, or if the entry is a boolean. @@ -469,17 +469,89 @@ fn hoist_one_of_enum(incoming: Schema) -> Schema { new_schema } +// if anyOf with 2 entries, and one is nullable with enum that is [null], +// then hoist nullable, description, type, enum from the other entry. +// set anyOf to None +fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { + // Run some initial checks in case there is nothing to do + let SchemaObject { + subschemas: Some(subschemas), + .. + } = &incoming + else { + return incoming; + }; + + let SubschemaValidation { + any_of: Some(any_of), .. + } = subschemas.deref() + else { + return incoming; + }; + + if any_of.len() != 2 { + return incoming; + }; + + // This is the signature of an Optional enum that needs hoisting + let null = json!({ + "enum": [null], + "nullable": true + + }); + + // iter through any_of for matching null + let results: [bool; 2] = any_of + .iter() + .map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON")) + .map(|x| x == null) + .collect::>() + .try_into() + .expect("there should be exactly 2 elements. We checked earlier"); + + let to_hoist = match results { + [true, true] => panic!("Too many nulls, not enough drinks"), + [true, false] => &any_of[1], + [false, true] => &any_of[0], + [false, false] => return incoming, + }; + + // my goodness! + let Schema::Object(to_hoist) = to_hoist else { + panic!("Somehow we have stumbled across a bool schema"); + }; + + let mut new_schema = incoming.clone(); + + let mut new_metadata = incoming.metadata.clone().unwrap_or_default(); + new_metadata.description = to_hoist.metadata.as_ref().and_then(|m| m.description.clone()); + + new_schema.metadata = Some(new_metadata); + new_schema.instance_type = to_hoist.instance_type.clone(); + new_schema.enum_values = to_hoist.enum_values.clone(); + new_schema.other["nullable"] = true.into(); + + new_schema + .subschemas + .as_mut() + .expect("we have asserted that there is any_of") + .any_of = None; + + new_schema +} + + impl Transform for StructuralSchemaRewriter { fn transform(&mut self, transform_schema: &mut schemars::Schema) { schemars::transform::transform_subschemas(self, transform_schema); // TODO (@NickLarsenNZ): Replace with conversion function - let mut schema: SchemaObject = match serde_json::from_value(transform_schema.clone().to_value()).ok() - { + let schema: SchemaObject = match serde_json::from_value(transform_schema.clone().to_value()).ok() { Some(schema) => schema, None => return, }; - + let schema = hoist_one_of_enum(schema); + let mut schema = hoist_any_of_option_enum(schema); if let Some(subschemas) = &mut schema.subschemas { if let Some(one_of) = subschemas.one_of.as_mut() { // Tagged enums are serialized using `one_of` From 3850a1937347047f470883dfe80e4951fd8529fb Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 10 Oct 2025 15:12:21 +0200 Subject: [PATCH 09/27] left a todo for empty structural variants in untagged enums --- kube-core/src/schema.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index db0d534c1..7a1d1d155 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -551,7 +551,9 @@ impl Transform for StructuralSchemaRewriter { None => return, }; let schema = hoist_one_of_enum(schema); - let mut schema = hoist_any_of_option_enum(schema); + let schema = hoist_any_of_option_enum(schema); + // todo: let schema = strip_any_of_empty_object_entry(schema); + let mut schema = schema; if let Some(subschemas) = &mut schema.subschemas { if let Some(one_of) = subschemas.one_of.as_mut() { // Tagged enums are serialized using `one_of` From c2fbe7085f76b21fb0743f51c241a8de8a59fccd Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 10 Oct 2025 15:24:10 +0200 Subject: [PATCH 10/27] test: Add missing enum entries to expected schemas --- kube-derive/tests/crd_complex_enum_tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index 4520fa6ef..a25f01cad 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -272,6 +272,8 @@ fn untagged_enum() { "two": { "description": "Used in case the `two` field of type [`NormalEnum`] is present", "enum": [ + "C", + "D", "A", "B" ], @@ -360,6 +362,8 @@ fn optional_untagged_enum() { "two": { "description": "Used in case the `two` field of type [`NormalEnum`] is present", "enum": [ + "C", + "D", "A", "B" ], @@ -444,6 +448,8 @@ fn flattened_untagged_enum() { "two": { "description": "Used in case the `two` field of type [`NormalEnum`] is present", "enum": [ + "C", + "D", "A", "B" ], From 8abef367d4a6c436fc65101a2de8a96142e91da6 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 10 Oct 2025 15:26:05 +0200 Subject: [PATCH 11/27] test: Add empty object handling to existing hoisting function Note: At this point, there is some strange interaction with the new hoisting functions and the existing code whereby "properties" disappear. --- kube-core/src/schema.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index 7a1d1d155..c426da3b4 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -497,7 +497,6 @@ fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { let null = json!({ "enum": [null], "nullable": true - }); // iter through any_of for matching null @@ -692,6 +691,17 @@ fn hoist_subschema_properties( variant_obj.additional_properties = None; merge_metadata(instance_type, variant_type.take()); + } else if let Schema::Object(SchemaObject { + object: None, + instance_type: variant_type, + metadata, + .. + }) = variant + { + if *variant_type == Some(SingleOrVec::Single(Box::new(InstanceType::Object))) { + *variant_type = None; + *metadata = None; + } } } } From 8f578c2509c7046460dd6d6e602b6d8057dc0591 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 14 Oct 2025 16:13:23 +0200 Subject: [PATCH 12/27] fix: Change hoist_any_of_option_enum to cater to nested any_of as well as enum All tests in this PR are passing Note: The function should be renamed, but it needs some work to not only do optional things. --- kube-core/src/schema.rs | 85 ++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 22 deletions(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index c426da3b4..389ce6b17 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -442,6 +442,7 @@ fn hoist_one_of_enum(incoming: SchemaObject) -> SchemaObject { *new_instance_type = Some(hoisted_instance_type.clone()); + // Merge the enums: // For each `oneOf` entry, iterate over the `enum` and `const` values. // Panic on an entry that doesn't contain an `enum` or `const`. let new_enums = one_of.iter().flat_map(|obj| match obj { @@ -499,8 +500,9 @@ fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { "nullable": true }); - // iter through any_of for matching null - let results: [bool; 2] = any_of + // iter through any_of entries, converting them to Value, + // and build a truth table for null matches + let entry_is_null: [bool; 2] = any_of .iter() .map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON")) .map(|x| x == null) @@ -508,33 +510,67 @@ fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { .try_into() .expect("there should be exactly 2 elements. We checked earlier"); - let to_hoist = match results { - [true, true] => panic!("Too many nulls, not enough drinks"), + // Get the `any_of` entry that isn't the null entry + let non_null_any_of_entry = match entry_is_null { + [true, true] => return incoming, [true, false] => &any_of[1], [false, true] => &any_of[0], [false, false] => return incoming, }; // my goodness! - let Schema::Object(to_hoist) = to_hoist else { + let Schema::Object(to_hoist) = non_null_any_of_entry else { panic!("Somehow we have stumbled across a bool schema"); }; let mut new_schema = incoming.clone(); + // only do this if we still have `non_null_any_of_entry`. If the code changes, we need to revise this + new_schema.other["nullable"] = true.into(); let mut new_metadata = incoming.metadata.clone().unwrap_or_default(); new_metadata.description = to_hoist.metadata.as_ref().and_then(|m| m.description.clone()); new_schema.metadata = Some(new_metadata); new_schema.instance_type = to_hoist.instance_type.clone(); - new_schema.enum_values = to_hoist.enum_values.clone(); - new_schema.other["nullable"] = true.into(); - - new_schema + if to_hoist.enum_values.as_ref().is_some_and(|v| !v.is_empty()) { + // need to hoist enum up to parent + new_schema.enum_values = to_hoist.enum_values.clone(); + + // should we clear out subschemas? + // we do need to clear out anyOf, since that's where we got the enum that is now hoisted. + // but if this code changes to handle one_of too, then we need to ensure we don't drop data that we haven't hoisted yet. + new_schema + .subschemas + .as_mut() + .expect("we have asserted that there is any_of") + .any_of = None; + } else if to_hoist + .subschemas + .as_ref() + .is_some_and(|s| s.any_of.as_ref().is_some_and(|x| !x.is_empty())) + { + // Replace the parent any_of with the nested any_of + // TODO (@NickLarsenNZ): Might need to find the null object and clear it out. + new_schema + .subschemas + .as_mut() + .expect("We have asserted there is any_of") + .any_of = to_hoist + .subschemas + .as_ref() + .expect("We have asserted there is any_of") + .any_of + .clone(); + // Bring across the properties, etc... + // Is this ok? + new_schema.object = to_hoist.object.clone(); + } else if to_hoist .subschemas - .as_mut() - .expect("we have asserted that there is any_of") - .any_of = None; + .as_ref() + .is_some_and(|s| s.one_of.as_ref().is_some_and(|x| !x.is_empty())) + { + panic!("We have an unexpected one_of nested under an any_of. Not yet sure what and how to hoist") + } new_schema } @@ -542,6 +578,7 @@ fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { impl Transform for StructuralSchemaRewriter { fn transform(&mut self, transform_schema: &mut schemars::Schema) { + // Apply this (self) transform to all subschemas schemars::transform::transform_subschemas(self, transform_schema); // TODO (@NickLarsenNZ): Replace with conversion function @@ -551,20 +588,23 @@ impl Transform for StructuralSchemaRewriter { }; let schema = hoist_one_of_enum(schema); let schema = hoist_any_of_option_enum(schema); - // todo: let schema = strip_any_of_empty_object_entry(schema); + // todo: let schema = strip_any_of_empty_object_entry(schema); // this is currently done in an `else if` block in hoist_subschema_properties() let mut schema = schema; + if let Some(subschemas) = &mut schema.subschemas { - if let Some(one_of) = subschemas.one_of.as_mut() { - // Tagged enums are serialized using `one_of` - hoist_subschema_properties(one_of, &mut schema.object, &mut schema.instance_type); + // // if let Some(one_of) = subschemas.one_of.as_mut() { + // // // Tagged enums are serialized using `one_of` + // // // Maybe we also have to hoist properties? + // // hoist_subschema_properties(one_of, &mut schema.object, &mut schema.instance_type); - // "Plain" enums are serialized using `one_of` if they have doc tags - hoist_subschema_enum_values(one_of, &mut schema.enum_values, &mut schema.instance_type); + // // // "Plain" enums are serialized using `one_of` if they have doc tags + // // // This is now done by hoist_one_of_enum + // // hoist_subschema_enum_values(one_of, &mut schema.enum_values, &mut schema.instance_type); - if one_of.is_empty() { - subschemas.one_of = None; - } - } + // // if one_of.is_empty() { + // // subschemas.one_of = None; + // // } + // // } if let Some(any_of) = &mut subschemas.any_of { // Untagged enums are serialized using `any_of` @@ -594,6 +634,7 @@ impl Transform for StructuralSchemaRewriter { array.unique_items = None; } + // Convert back to schemars::Schema if let Ok(schema) = serde_json::to_value(schema) { if let Ok(transformed) = serde_json::from_value(schema) { *transform_schema = transformed; From c23aabc5e4435493e6176edd31bbfabcdc12cf78 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 14 Oct 2025 17:04:18 +0200 Subject: [PATCH 13/27] test: Add test for internal function, and add docs after seeing it's behavior --- kube-core/src/schema.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index 389ce6b17..67e18b0dc 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -739,6 +739,7 @@ fn hoist_subschema_properties( .. }) = variant { + // Clear out the description and type to represent the empty `{}` variant if *variant_type == Some(SingleOrVec::Single(Box::new(InstanceType::Object))) { *variant_type = None; *metadata = None; @@ -747,6 +748,9 @@ fn hoist_subschema_properties( } } +/// Get the single item from an [Iterator]. +/// +/// Return None if the [Iterator] is empty or has more than one entry. fn only_item(mut i: I) -> Option { let item = i.next()?; if i.next().is_some() { @@ -755,6 +759,13 @@ fn only_item(mut i: I) -> Option { Some(item) } +#[test] +fn only_item_t() { + assert_eq!(only_item::>([].iter()), None); + assert_eq!(only_item(["one"].iter()), Some(&"one")); + assert_eq!(only_item(["one", "two"].iter()), None); +} + fn merge_metadata( instance_type: &mut Option>, variant_type: Option>, From e91665ba7142a4d5f5e628753de2563e1a8d5a74 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 21 Oct 2025 22:23:26 +0200 Subject: [PATCH 14/27] feat: Add functions for specific hoisting tasks These include tests for a variety of schemas --- kube-core/src/schema/transforms.rs | 137 +++++++++ kube-core/src/schema/transformsb.rs | 161 ++++++++++ kube-core/src/schema/transformsc.rs | 460 ++++++++++++++++++++++++++++ 3 files changed, 758 insertions(+) create mode 100644 kube-core/src/schema/transforms.rs create mode 100644 kube-core/src/schema/transformsb.rs create mode 100644 kube-core/src/schema/transformsc.rs diff --git a/kube-core/src/schema/transforms.rs b/kube-core/src/schema/transforms.rs new file mode 100644 index 000000000..cd60ef04f --- /dev/null +++ b/kube-core/src/schema/transforms.rs @@ -0,0 +1,137 @@ +use std::ops::DerefMut; + +use crate::schema::{Schema, SchemaObject, SubschemaValidation}; + +#[cfg(test)] +#[test] +fn tagged_enum_with_unit_variants() { + let original_schema_object_value = serde_json::json!({ + "description": "A very simple enum with unit variants", + "oneOf": [ + { + "type": "string", + "enum": [ + "C", + "D" + ] + }, + { + "description": "First variant doc-comment", + "type": "string", + "enum": [ + "A" + ] + }, + { + "description": "Second variant doc-comment", + "type": "string", + "enum": [ + "B" + ] + }, + ] + }); + + let expected_converted_schema_object_value = serde_json::json!({ + "description": "A very simple enum with unit variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + }); + + + let original_schema_object: SchemaObject = + serde_json::from_value(original_schema_object_value).expect("valid JSON"); + let expected_converted_schema_object: SchemaObject = + serde_json::from_value(expected_converted_schema_object_value).expect("valid JSON"); + + let mut actual_converted_schema_object = original_schema_object.clone(); + hoist_one_of_enum_with_unit_variants(&mut actual_converted_schema_object); + + assert_json_diff::assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); +} + + +/// Replace a list of typed oneOf subschemas with a typed schema level enum +/// +/// Used for correcting the schema for tagged enums with unit variants. +/// NOTE: Subschema descriptions are lost when they are combined into a single enum of the same type. +/// +/// This will return early without modifications unless: +/// - There are `oneOf` subschemas (not empty) +/// - Each subschema contains an enum +/// - Each subschema is typed +/// - Each subschemas types is the same as the others +/// +/// NOTE: This should work regardless of whether other hoisting has been performed or not. +fn hoist_one_of_enum_with_unit_variants(kube_schema: &mut SchemaObject) { + // Run some initial checks in case there is nothing to do + let SchemaObject { + subschemas: Some(subschemas), + .. + } = kube_schema + else { + return; + }; + + let SubschemaValidation { + one_of: Some(one_of), .. + } = subschemas.deref_mut() + else { + return; + }; + + if one_of.is_empty() { + return; + } + + // At this point, we can be reasonably sure we need to hoist the oneOf + // subschema enums and types up to the schema level, and unset the oneOf field. + // From here, anything that looks wrong will panic instead of return. + // TODO (@NickLarsenNZ): Return errors instead of panicking, leave panicking up to the infallible schemars::Transform + + // Prepare to ensure each variant schema has a type + let mut types = one_of.iter().map(|schema| match schema { + Schema::Object(SchemaObject { + instance_type: Some(r#type), + .. + }) => r#type, + Schema::Object(untyped) => panic!("oneOf variants need to define a type: {untyped:#?}"), + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }); + + // Get the first type + let variant_type = types.next().expect("at this point, there must be a type"); + // Ensure all variant types match it + if types.any(|r#type| r#type != variant_type) { + panic!("oneOf variants must all have the same type"); + } + + // For each `oneOf` entry, iterate over the `enum` and `const` values. + // Panic on an entry that doesn't contain an `enum` or `const`. + let new_enums = one_of.iter().flat_map(|schema| match schema { + Schema::Object(SchemaObject { + enum_values: Some(r#enum), + .. + }) => r#enum.clone(), + // Warning: The `const` check below must come after the enum check above. + // Otherwise it will panic on a valid entry with an `enum`. + Schema::Object(SchemaObject { other, .. }) => match other.get("const") { + Some(r#const) => vec![r#const.clone()], + None => panic!("oneOf variant did not provide \"enum\" or \"const\": {schema:#?}"), + }, + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }); + // Merge the enums (extend just to be safe) + kube_schema.enum_values.get_or_insert_default().extend(new_enums); + + // Hoist the type + kube_schema.instance_type = Some(variant_type.clone()); + + // Clear the oneOf subschemas + subschemas.one_of = None; +} diff --git a/kube-core/src/schema/transformsb.rs b/kube-core/src/schema/transformsb.rs new file mode 100644 index 000000000..ad2567542 --- /dev/null +++ b/kube-core/src/schema/transformsb.rs @@ -0,0 +1,161 @@ +use std::ops::DerefMut; + +use crate::schema::{Schema, SchemaObject, SubschemaValidation}; + +#[cfg(test)] +#[test] +fn optional_tagged_enum_with_unit_variants() { + let original_schema_object_value = serde_json::json!({ + "anyOf": [ + { + "description": "A very simple enum with empty variants", + "oneOf": [ + { + "type": "string", + "enum": [ + "C", + "D" + ] + }, + { + "description": "First variant doc-comment", + "type": "string", + "enum": [ + "A" + ] + }, + { + "description": "Second variant doc-comment", + "type": "string", + "enum": [ + "B" + ] + } + ] + }, + { + "enum": [ + null + ], + "nullable": true + } + ] + }); + + let expected_converted_schema_object_value = serde_json::json!({ + "description": "A very simple enum with empty variants", + "nullable": true, + "oneOf": [ + { + "type": "string", + "enum": [ + "C", + "D" + ] + }, + { + "description": "First variant doc-comment", + "type": "string", + "enum": [ + "A" + ] + }, + { + "description": "Second variant doc-comment", + "type": "string", + "enum": [ + "B" + ] + } + ] + }); + + + let original_schema_object: SchemaObject = + serde_json::from_value(original_schema_object_value).expect("valid JSON"); + let expected_converted_schema_object: SchemaObject = + serde_json::from_value(expected_converted_schema_object_value).expect("valid JSON"); + + let mut actual_converted_schema_object = original_schema_object.clone(); + hoist_any_of_subschema_with_a_nullable_variant(&mut actual_converted_schema_object); + + assert_json_diff::assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); +} + + +/// Replace the schema with the non-null anyOf subschema when the only other subschema is the null schema. +/// +/// Used for correcting the schema for optional tagged unit enums. +/// The non-null subschema is hoisted, and nullable will be set to true. +/// +/// This will return early without modifications unless: +/// - There are exactly 2 `anyOf` subschemas +/// - One subschema represents the `null` (has an enum with a null entry, and nullable set to true) +/// +/// NOTE: This should work regardless of whether other hoisting has been performed or not. +fn hoist_any_of_subschema_with_a_nullable_variant(kube_schema: &mut SchemaObject) { + // Run some initial checks in case there is nothing to do + let SchemaObject { + subschemas: Some(subschemas), + .. + } = kube_schema + else { + return; + }; + + let SubschemaValidation { + any_of: Some(any_of), + one_of, + } = subschemas.deref_mut() + else { + return; + }; + + if any_of.len() != 2 { + return; + } + + // This is the signature for the null variant, indicating the "other" + // variant is the subschema that needs hoisting + let null = serde_json::json!({ + "enum": [null], + "nullable": true + }); + + // iter through any_of entries, converting them to Value, + // and build a truth table for null matches + let entry_is_null: [bool; 2] = any_of + .iter() + .map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON")) + .map(|x| x == null) + .collect::>() + .try_into() + .expect("there should be exactly 2 elements. We checked earlier"); + + // Get the `any_of` subschema that isn't the null entry + let subschema_to_hoist = match entry_is_null { + [true, false] => &any_of[1], + [false, true] => &any_of[0], + _ => return, + }; + + // At this point, we can be reasonably sure we need to hoist the non-null + // anyOf subschema up to the schema level, and unset the anyOf field. + // From here, anything that looks wrong will panic instead of return. + // TODO (@NickLarsenNZ): Return errors instead of panicking, leave panicking up to the infallible schemars::Transform + + let Schema::Object(to_hoist) = subschema_to_hoist else { + panic!("the non-null anyOf subschema is a bool. That is not expected here"); + }; + + // There should not be any oneOf's adjacent to the anyOf + if one_of.is_some() { + panic!("oneOf is set when there is already an anyOf: {one_of:#?}"); + } + + // Replace the schema with the non-null subschema + *kube_schema = to_hoist.clone(); + + // Set the schema to nullable (as we know we matched the null variant earlier) + kube_schema.extensions.insert("nullable".to_owned(), true.into()); +} diff --git a/kube-core/src/schema/transformsc.rs b/kube-core/src/schema/transformsc.rs new file mode 100644 index 000000000..e51df51a8 --- /dev/null +++ b/kube-core/src/schema/transformsc.rs @@ -0,0 +1,460 @@ +use std::ops::DerefMut; + +use crate::schema::{InstanceType, Schema, SchemaObject, SingleOrVec, SubschemaValidation}; + +#[cfg(test)] +#[test] +fn untagged_enum_with_empty_variant_before_one_of_hoisting() { + let original_schema_object_value = serde_json::json!({ + "description": "An untagged enum with a nested enum inside", + "anyOf": [ + { + "description": "Used in case the `one` field is present", + "type": "object", + "required": [ + "one" + ], + "properties": { + "one": { + "type": "string" + } + } + }, + { + "description": "Used in case the `two` field is present", + "type": "object", + "required": [ + "two" + ], + "properties": { + "two": { + "description": "A very simple enum with empty variants", + "oneOf": [ + { + "type": "string", + "enum": [ + "C", + "D" + ] + }, + { + "description": "First variant doc-comment", + "type": "string", + "enum": [ + "A" + ] + }, + { + "description": "Second variant doc-comment", + "type": "string", + "enum": [ + "B" + ] + } + ] + } + } + }, + { + "description": "Used in case no fields are present", + "type": "object" + } + ] + }); + + let expected_converted_schema_object_value = serde_json::json!({ + "description": "An untagged enum with a nested enum inside", + "type": "object", + "anyOf": [ + { + "required": [ + "one" + ] + }, + { + "required": [ + "two" + ] + }, + {} + ], + "properties": { + "one": { + "type": "string" + }, + "two": { + "description": "A very simple enum with empty variants", + "oneOf": [ + { + "type": "string", + "enum": [ + "C", + "D" + ] + }, + { + "description": "First variant doc-comment", + "type": "string", + "enum": [ + "A" + ] + }, + { + "description": "Second variant doc-comment", + "type": "string", + "enum": [ + "B" + ] + } + ] + } + } + }); + + let original_schema_object: SchemaObject = + serde_json::from_value(original_schema_object_value).expect("valid JSON"); + let expected_converted_schema_object: SchemaObject = + serde_json::from_value(expected_converted_schema_object_value).expect("valid JSON"); + + let mut actual_converted_schema_object = original_schema_object.clone(); + hoist_any_of_subschemas_with_properties_merged(&mut actual_converted_schema_object); + + assert_json_diff::assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); +} + +#[cfg(test)] +#[test] +fn untagged_enum_with_duplicate_field_of_same_shape() { + let original_schema_object_value = serde_json::json!({ + "description": "Comment for untagged enum ProductImageSelection", + "anyOf": [ + { + "description": "Comment for struct ProductImageCustom", + "properties": { + "custom": { + "description": "Comment for custom field", + "type": "string" + }, + "productVersion": { + "description": "Comment for product_version field (same on both structs)", + "type": "string" + } + }, + "required": [ + "productVersion", + "custom" + ], + "type": "object" + }, + { + "description": "Comment for struct ProductImageVersion", + "properties": { + "productVersion": { + "description": "Comment for product_version field (same on both structs)", + "type": "string" + }, + "repo": { + "description": "Comment for repo field", + "nullable": true, + "type": "string" + } + }, + "required": [ + "productVersion" + ], + "type": "object" + } + ] + }); + + let expected_converted_schema_object_value = serde_json::json!({ + "description": "Comment for untagged enum ProductImageSelection", + "type": "object", + "anyOf": [ + { + "required": [ + "custom", + "productVersion" + ] + }, + { + "required": [ + "productVersion" + ] + } + ], + "properties": { + "custom": { + "description": "Comment for custom field", + "type": "string" + }, + "productVersion": { + "description": "Comment for product_version field (same on both structs)", + "type": "string" + }, + "repo": { + "description": "Comment for repo field", + "nullable": true, + "type": "string" + } + } + + }); + + let original_schema_object: SchemaObject = + serde_json::from_value(original_schema_object_value).expect("valid JSON"); + let expected_converted_schema_object: SchemaObject = + serde_json::from_value(expected_converted_schema_object_value).expect("valid JSON"); + + let mut actual_converted_schema_object = original_schema_object.clone(); + hoist_any_of_subschemas_with_properties_merged(&mut actual_converted_schema_object); + + assert_json_diff::assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); +} + +#[cfg(test)] +#[test] +#[should_panic(expected = "Properties for \"two\" are defined multiple times with different shapes")] +fn invalid_untagged_enum_with_conflicting_variant_fields_before_one_of_hosting() { + let original_schema_object_value = serde_json::json!({ + "description": "An untagged enum with a nested enum inside", + "anyOf": [ + { + "description": "Used in case the `one` field is present", + "type": "object", + "required": [ + "one", + "two" + ], + "properties": { + "one": { + "type": "string" + }, + "two": { + "type": "integer" + } + } + }, + { + "description": "Used in case the `two` field is present", + "type": "object", + "required": [ + "two" + ], + "properties": { + "two": { + "description": "A very simple enum with empty variants", + "oneOf": [ + { + "type": "string", + "enum": [ + "C", + "D" + ] + }, + { + "description": "First variant doc-comment", + "type": "string", + "enum": [ + "A" + ] + }, + { + "description": "Second variant doc-comment", + "type": "string", + "enum": [ + "B" + ] + } + ] + } + } + }, + { + "description": "Used in case no fields are present", + "type": "object" + } + ] + }); + + + let original_schema_object: SchemaObject = + serde_json::from_value(original_schema_object_value).expect("valid JSON"); + + let mut actual_converted_schema_object = original_schema_object.clone(); + hoist_any_of_subschemas_with_properties_merged(&mut actual_converted_schema_object); +} + +#[cfg(test)] +#[test] +#[should_panic(expected = "Properties for \"two\" are defined multiple times with different shapes")] +fn invalid_untagged_enum_with_conflicting_variant_fields_after_one_of_hosting() { + // NOTE: the oneOf for the second variant has already been hoisted + let original_schema_object_value = serde_json::json!({ + "description": "An untagged enum with a nested enum inside", + "anyOf": [ + { + "description": "Used in case the `one` field is present", + "type": "object", + "required": [ + "one", + "two", + ], + "properties": { + "one": { + "type": "string" + }, + "two": { + "type": "string" + } + } + }, + { + "description": "Used in case the `two` field is present", + "type": "object", + "required": [ + "two" + ], + "properties": { + "two": { + "description": "A very simple enum with empty variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + } + } + }, + { + "description": "Used in case no fields are present", + "type": "object" + } + ] + }); + + let original_schema_object: SchemaObject = + serde_json::from_value(original_schema_object_value).expect("valid JSON"); + + let mut actual_converted_schema_object = original_schema_object.clone(); + hoist_properties_for_any_of_subschemas(&mut actual_converted_schema_object); +} + +/// Take subschema properties and insert them into the schema properties. +/// +/// Used for correcting the schema for serde untagged structural enums. +/// NOTE: Due to the nature of "untagging", enum variant doc-comments are not preserved. +/// +/// This will return early without modifications unless: +/// - There are `anyOf` subschemas +/// - Each subschema has the type "object" +/// +/// NOTE: This should work regardless of whether other hoisting has been performed or not. +fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObject) { + // Run some initial checks in case there is nothing to do + let SchemaObject { + subschemas: Some(subschemas), + object: parent_object, + .. + } = kube_schema + else { + return; + }; + + let SubschemaValidation { + any_of: Some(any_of), + one_of, + } = subschemas.deref_mut() + else { + return; + }; + + if any_of.is_empty() { + return; + } + + // Ensure we aren't looking at the one with a null + // TODO (@NickLarsenNZ): Combine the logic with the function that covers the nullable anyOf + if any_of.len() == 2 { + // This is the signature for the null variant, indicating the "other" + // variant is the subschema that needs hoisting + let null = serde_json::json!({ + "enum": [null], + "nullable": true + }); + + // Return if one of the two entries are nulls + for value in any_of + .iter() + .map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON")) + { + if value == null { + return; + } + } + } + + // At this point, we can be reasonably sure we need operate on the schema. + // TODO (@NickLarsenNZ): Return errors instead of panicking, leave panicking up to the infallible schemars::Transform + + // There should not be any oneOf's adjacent to the anyOf + if one_of.is_some() { + panic!("oneOf is set when there is already an anyOf: {one_of:#?}"); + } + + let subschemas = any_of + .into_iter() + .map(|schema| match schema { + Schema::Object(schema_object) => schema_object, + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }) + .collect::>(); + + for subschema in subschemas { + // Drop description/type + // This will clear out any objects that don't have required/properties fields (so that it appears as: {}). + subschema.metadata.take(); + subschema.instance_type.take(); + + // Set the schema type to object + kube_schema.instance_type = Some(SingleOrVec::Single(Box::new(InstanceType::Object))); + + if let Some(object) = subschema.object.as_deref_mut() { + // If properties are set, hoist them to the schema properties. + // This will panic if duplicate properties are encountered that do not have the same shape. + // That can happen when the untagged enum variants each refer to structs which contain the same field name. + // The developer needs to make them the same. + // TODO (@NickLarsenNZ): Add a case for a structural variant, and a tuple variant containing a structure where the same field name is used. + while let Some((property_name, Schema::Object(property_schema_object))) = + object.properties.pop_first() + { + // This would check that the variant property (that we want to now hoist) + // is exactly the same as what is already hoisted (in this function). + if let Some(existing_property) = parent_object + .get_or_insert_default() + .properties + .get(&property_name) + { + if existing_property != &Schema::Object(property_schema_object.clone()) { + // TODO (@NickLarsenNZ): Here we could do another check to see if it only differs by description. + // If the schema property description is not set, then we could overwrite it and not panic. + dbg!( + &property_name, + existing_property, + &Schema::Object(property_schema_object.clone()), + ); + panic!("Properties for {property_name:?} are defined multiple times with different shapes") + } + } else { + // Otherwise, insert the subschema properties into the schema properties + parent_object + .get_or_insert_default() + .properties + .insert(property_name.clone(), Schema::Object(property_schema_object)); + } + } + } + } +} From f13a751b9696b0f0097c2e3a624793832389a8d4 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 21 Oct 2025 22:26:09 +0200 Subject: [PATCH 15/27] chore: Add a crude fix to code that will be removed in a later commit --- kube-core/src/schema.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema.rs index 67e18b0dc..c90cad420 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema.rs @@ -562,8 +562,12 @@ fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { .any_of .clone(); // Bring across the properties, etc... - // Is this ok? - new_schema.object = to_hoist.object.clone(); + // Is this ok? Or should we just replace properties with sub properties? + // Or should we carefully copy the property entries over in case there are existing entries? + // new_schema.object = to_hoist.object.clone(); + if let Some(to_hoist_object) = &to_hoist.object { + new_schema.object.get_or_insert_default().properties = to_hoist_object.properties.clone(); + } } else if to_hoist .subschemas .as_ref() From 730e0dba492ac7ce062bc58ac296f7a8b74666a7 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 21 Oct 2025 22:40:09 +0200 Subject: [PATCH 16/27] fix: update changed function name in tests --- kube-core/src/schema/transformsc.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kube-core/src/schema/transformsc.rs b/kube-core/src/schema/transformsc.rs index e51df51a8..02f3de6b3 100644 --- a/kube-core/src/schema/transformsc.rs +++ b/kube-core/src/schema/transformsc.rs @@ -117,7 +117,7 @@ fn untagged_enum_with_empty_variant_before_one_of_hoisting() { serde_json::from_value(expected_converted_schema_object_value).expect("valid JSON"); let mut actual_converted_schema_object = original_schema_object.clone(); - hoist_any_of_subschemas_with_properties_merged(&mut actual_converted_schema_object); + hoist_properties_for_any_of_subschemas(&mut actual_converted_schema_object); assert_json_diff::assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); } @@ -207,7 +207,7 @@ fn untagged_enum_with_duplicate_field_of_same_shape() { serde_json::from_value(expected_converted_schema_object_value).expect("valid JSON"); let mut actual_converted_schema_object = original_schema_object.clone(); - hoist_any_of_subschemas_with_properties_merged(&mut actual_converted_schema_object); + hoist_properties_for_any_of_subschemas(&mut actual_converted_schema_object); assert_json_diff::assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); } @@ -282,7 +282,7 @@ fn invalid_untagged_enum_with_conflicting_variant_fields_before_one_of_hosting() serde_json::from_value(original_schema_object_value).expect("valid JSON"); let mut actual_converted_schema_object = original_schema_object.clone(); - hoist_any_of_subschemas_with_properties_merged(&mut actual_converted_schema_object); + hoist_properties_for_any_of_subschemas(&mut actual_converted_schema_object); } #[cfg(test)] From 7503fcc7a88a6f85adf5ce9502d3427c7eb08799 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 21 Oct 2025 23:12:06 +0200 Subject: [PATCH 17/27] chore: Make new hoisting functions pub(crate) --- kube-core/src/schema/transforms.rs | 2 +- kube-core/src/schema/transformsb.rs | 2 +- kube-core/src/schema/transformsc.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kube-core/src/schema/transforms.rs b/kube-core/src/schema/transforms.rs index cd60ef04f..736bac2fe 100644 --- a/kube-core/src/schema/transforms.rs +++ b/kube-core/src/schema/transforms.rs @@ -68,7 +68,7 @@ fn tagged_enum_with_unit_variants() { /// - Each subschemas types is the same as the others /// /// NOTE: This should work regardless of whether other hoisting has been performed or not. -fn hoist_one_of_enum_with_unit_variants(kube_schema: &mut SchemaObject) { +pub(crate) fn hoist_one_of_enum_with_unit_variants(kube_schema: &mut SchemaObject) { // Run some initial checks in case there is nothing to do let SchemaObject { subschemas: Some(subschemas), diff --git a/kube-core/src/schema/transformsb.rs b/kube-core/src/schema/transformsb.rs index ad2567542..42fdc83f2 100644 --- a/kube-core/src/schema/transformsb.rs +++ b/kube-core/src/schema/transformsb.rs @@ -93,7 +93,7 @@ fn optional_tagged_enum_with_unit_variants() { /// - One subschema represents the `null` (has an enum with a null entry, and nullable set to true) /// /// NOTE: This should work regardless of whether other hoisting has been performed or not. -fn hoist_any_of_subschema_with_a_nullable_variant(kube_schema: &mut SchemaObject) { +pub(crate) fn hoist_any_of_subschema_with_a_nullable_variant(kube_schema: &mut SchemaObject) { // Run some initial checks in case there is nothing to do let SchemaObject { subschemas: Some(subschemas), diff --git a/kube-core/src/schema/transformsc.rs b/kube-core/src/schema/transformsc.rs index 02f3de6b3..8df3c82fd 100644 --- a/kube-core/src/schema/transformsc.rs +++ b/kube-core/src/schema/transformsc.rs @@ -352,7 +352,7 @@ fn invalid_untagged_enum_with_conflicting_variant_fields_after_one_of_hosting() /// - Each subschema has the type "object" /// /// NOTE: This should work regardless of whether other hoisting has been performed or not. -fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObject) { +pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObject) { // Run some initial checks in case there is nothing to do let SchemaObject { subschemas: Some(subschemas), From 17e7a28c51db4b64d9210c18fdd8ecc3129708ec Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 21 Oct 2025 23:13:54 +0200 Subject: [PATCH 18/27] chore: Move schema.rs to schema/mod.rs --- kube-core/src/{schema.rs => schema/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename kube-core/src/{schema.rs => schema/mod.rs} (100%) diff --git a/kube-core/src/schema.rs b/kube-core/src/schema/mod.rs similarity index 100% rename from kube-core/src/schema.rs rename to kube-core/src/schema/mod.rs From cc6b20a808bdf19448b275adfe3598b346ad85ac Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Tue, 21 Oct 2025 23:24:23 +0200 Subject: [PATCH 19/27] feat: Implement new hoisting logic note: test expectaions changed as part of the implementation: Untagged variant doc-comments will not be preserved. Previously they were erroneously added as field descriptions. --- kube-core/src/schema/mod.rs | 517 +----------------- .../{transformsb.rs => transform_any_of.rs} | 0 .../{transforms.rs => transform_one_of.rs} | 0 ...transformsc.rs => transform_properties.rs} | 0 kube-derive/tests/crd_complex_enum_tests.rs | 9 +- 5 files changed, 23 insertions(+), 503 deletions(-) rename kube-core/src/schema/{transformsb.rs => transform_any_of.rs} (100%) rename kube-core/src/schema/{transforms.rs => transform_one_of.rs} (100%) rename kube-core/src/schema/{transformsc.rs => transform_properties.rs} (100%) diff --git a/kube-core/src/schema/mod.rs b/kube-core/src/schema/mod.rs index c90cad420..dc27fe20e 100644 --- a/kube-core/src/schema/mod.rs +++ b/kube-core/src/schema/mod.rs @@ -2,15 +2,22 @@ //! //! [`CustomResourceDefinition`]: `k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition` +mod transform_any_of; +mod transform_one_of; +mod transform_properties; + // Used in docs #[allow(unused_imports)] use schemars::generate::SchemaSettings; use schemars::{transform::Transform, JsonSchema}; use serde::{Deserialize, Serialize}; -use serde_json::{json, Value}; -use std::{ - collections::{btree_map::Entry, BTreeMap, BTreeSet}, - ops::Deref as _, +use serde_json::Value; +use std::collections::{BTreeMap, BTreeSet}; + +use crate::schema::{ + transform_any_of::hoist_any_of_subschema_with_a_nullable_variant, + transform_one_of::hoist_one_of_enum_with_unit_variants, + transform_properties::hoist_properties_for_any_of_subschemas, }; /// schemars [`Visitor`] that rewrites a [`Schema`] to conform to Kubernetes' "structural schema" rules @@ -249,372 +256,30 @@ enum SingleOrVec { Vec(Vec), } -// #[cfg(test)] -// mod test { -// use assert_json_diff::assert_json_eq; -// use schemars::{json_schema, schema_for, JsonSchema}; -// use serde::{Deserialize, Serialize}; - -// use super::*; - -// /// A very simple enum with unit variants, and no comments -// #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] -// enum NormalEnumNoComments { -// A, -// B, -// } - -// /// A very simple enum with unit variants, and comments -// #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] -// enum NormalEnum { -// /// First variant -// A, -// /// Second variant -// B, - -// // No doc-comments on these variants -// C, -// D, -// } - -// #[test] -// fn schema_for_enum_without_comments() { -// let schemars_schema = schema_for!(NormalEnumNoComments); - -// assert_json_eq!( -// schemars_schema, -// // replace the json_schema with this to get the full output. -// // serde_json::json!(42) -// json_schema!( -// { -// "$schema": "https://json-schema.org/draft/2020-12/schema", -// "description": "A very simple enum with unit variants, and no comments", -// "enum": [ -// "A", -// "B" -// ], -// "title": "NormalEnumNoComments", -// "type": "string" -// } -// ) -// ); - -// let kube_schema: crate::schema::Schema = -// schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); - -// let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); - -// // No hoisting needed -// assert_json_eq!(hoisted_kube_schema, kube_schema); -// } - -// #[test] -// fn schema_for_enum_with_comments() { -// let schemars_schema = schema_for!(NormalEnum); - -// assert_json_eq!( -// schemars_schema, -// // replace the json_schema with this to get the full output. -// // serde_json::json!(42) -// json_schema!( -// { -// "$schema": "https://json-schema.org/draft/2020-12/schema", -// "description": "A very simple enum with unit variants, and comments", -// "oneOf": [ -// { -// "enum": [ -// "C", -// "D" -// ], -// "type": "string" -// }, -// { -// "const": "A", -// "description": "First variant", -// "type": "string" -// }, -// { -// "const": "B", -// "description": "Second variant", -// "type": "string" -// } -// ], -// "title": "NormalEnum" -// } -// ) -// ); - - -// let kube_schema: crate::schema::Schema = -// schemars_schema_to_kube_schema(schemars_schema.clone()).unwrap(); - -// let hoisted_kube_schema = hoist_one_of_enum(kube_schema.clone()); - -// assert_ne!( -// hoisted_kube_schema, kube_schema, -// "Hoisting was performed, so hoisted_kube_schema != kube_schema" -// ); -// assert_json_eq!( -// hoisted_kube_schema, -// json_schema!( -// { -// "$schema": "https://json-schema.org/draft/2020-12/schema", -// "description": "A very simple enum with unit variants, and comments", -// "type": "string", -// "enum": [ -// "C", -// "D", -// "A", -// "B" -// ], -// "title": "NormalEnum" -// } -// ) -// ); -// } -// } - #[cfg(test)] fn schemars_schema_to_kube_schema(incoming: schemars::Schema) -> Result { serde_json::from_value(incoming.to_value()) } -/// Hoist `oneOf` into top level `enum`. -/// -/// This will move all `enum` variants and `const` values under `oneOf` into a single top level `enum` along with `type`. -/// It will panic if there are anomalies, like differences in `type` values, or lack of `enum` or `const` fields in the `oneOf` entries. -/// -/// Note: variant descriptions will be lost in the process, and the original `oneOf` will be erased. -/// -// Note: This function is heavily documented to express intent. It is intended to help developers -// make adjustments for future Schemars changes. -fn hoist_one_of_enum(incoming: SchemaObject) -> SchemaObject { - // Run some initial checks in case there is nothing to do - let SchemaObject { - subschemas: Some(subschemas), - .. - } = &incoming - else { - return incoming; - }; - - let SubschemaValidation { - one_of: Some(one_of), .. - } = subschemas.deref() - else { - return incoming; - }; - - if one_of.is_empty() { - return incoming; - } - - // At this point, we need to create a new Schema and hoist the `oneOf` - // variants' `enum`/`const` values up into a parent `enum`. - let mut new_schema = incoming.clone(); - if let SchemaObject { - subschemas: Some(new_subschemas), - instance_type: new_instance_type, - enum_values: new_enum_values, - .. - } = &mut new_schema - { - // For each `oneOf`, get the `type`. - // Panic if it has no `type`, or if the entry is a boolean. - let mut types = one_of.iter().map(|obj| match obj { - Schema::Object(SchemaObject { - instance_type: Some(r#type), - .. - }) => r#type, - // TODO (@NickLarsenNZ): Is it correct that JSON Schema oneOf must have a type? - Schema::Object(_) => panic!("oneOf variants need to define a type!: {obj:?}"), - Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), - }); - - // Get the first `type` value, then panic if any subsequent `type` values differ. - let hoisted_instance_type = types - .next() - .expect("oneOf must have at least one variant - we already checked that"); - // TODO (@NickLarsenNZ): Didn't sbernauer say that the types - if types.any(|t| t != hoisted_instance_type) { - panic!("All oneOf variants must have the same type"); - } - - *new_instance_type = Some(hoisted_instance_type.clone()); - - // Merge the enums: - // For each `oneOf` entry, iterate over the `enum` and `const` values. - // Panic on an entry that doesn't contain an `enum` or `const`. - let new_enums = one_of.iter().flat_map(|obj| match obj { - Schema::Object(SchemaObject { - enum_values: Some(r#enum), - .. - }) => r#enum.clone(), - // Warning: The `const` check below must come after the enum check above. - // Otherwise it will panic on a valid entry with an `enum`. - Schema::Object(SchemaObject { other, .. }) => match other.get("const") { - Some(r#const) => vec![r#const.clone()], - None => panic!("oneOf variant did not provide \"enum\" or \"const\": {obj:?}"), - }, - Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), - }); - - // Just in case there were existing enum values, add to them. - // TODO (@NickLarsenNZ): Check if `oneOf` and `enum` are mutually exclusive for a valid spec. - new_enum_values.get_or_insert_default().extend(new_enums); - - // We can clear out the existing oneOf's, since they will be hoisted below. - new_subschemas.one_of = None; - } - - new_schema -} - -// if anyOf with 2 entries, and one is nullable with enum that is [null], -// then hoist nullable, description, type, enum from the other entry. -// set anyOf to None -fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject { - // Run some initial checks in case there is nothing to do - let SchemaObject { - subschemas: Some(subschemas), - .. - } = &incoming - else { - return incoming; - }; - - let SubschemaValidation { - any_of: Some(any_of), .. - } = subschemas.deref() - else { - return incoming; - }; - - if any_of.len() != 2 { - return incoming; - }; - - // This is the signature of an Optional enum that needs hoisting - let null = json!({ - "enum": [null], - "nullable": true - }); - - // iter through any_of entries, converting them to Value, - // and build a truth table for null matches - let entry_is_null: [bool; 2] = any_of - .iter() - .map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON")) - .map(|x| x == null) - .collect::>() - .try_into() - .expect("there should be exactly 2 elements. We checked earlier"); - - // Get the `any_of` entry that isn't the null entry - let non_null_any_of_entry = match entry_is_null { - [true, true] => return incoming, - [true, false] => &any_of[1], - [false, true] => &any_of[0], - [false, false] => return incoming, - }; - - // my goodness! - let Schema::Object(to_hoist) = non_null_any_of_entry else { - panic!("Somehow we have stumbled across a bool schema"); - }; - - let mut new_schema = incoming.clone(); - // only do this if we still have `non_null_any_of_entry`. If the code changes, we need to revise this - new_schema.other["nullable"] = true.into(); - - let mut new_metadata = incoming.metadata.clone().unwrap_or_default(); - new_metadata.description = to_hoist.metadata.as_ref().and_then(|m| m.description.clone()); - - new_schema.metadata = Some(new_metadata); - new_schema.instance_type = to_hoist.instance_type.clone(); - if to_hoist.enum_values.as_ref().is_some_and(|v| !v.is_empty()) { - // need to hoist enum up to parent - new_schema.enum_values = to_hoist.enum_values.clone(); - - // should we clear out subschemas? - // we do need to clear out anyOf, since that's where we got the enum that is now hoisted. - // but if this code changes to handle one_of too, then we need to ensure we don't drop data that we haven't hoisted yet. - new_schema - .subschemas - .as_mut() - .expect("we have asserted that there is any_of") - .any_of = None; - } else if to_hoist - .subschemas - .as_ref() - .is_some_and(|s| s.any_of.as_ref().is_some_and(|x| !x.is_empty())) - { - // Replace the parent any_of with the nested any_of - // TODO (@NickLarsenNZ): Might need to find the null object and clear it out. - new_schema - .subschemas - .as_mut() - .expect("We have asserted there is any_of") - .any_of = to_hoist - .subschemas - .as_ref() - .expect("We have asserted there is any_of") - .any_of - .clone(); - // Bring across the properties, etc... - // Is this ok? Or should we just replace properties with sub properties? - // Or should we carefully copy the property entries over in case there are existing entries? - // new_schema.object = to_hoist.object.clone(); - if let Some(to_hoist_object) = &to_hoist.object { - new_schema.object.get_or_insert_default().properties = to_hoist_object.properties.clone(); - } - } else if to_hoist - .subschemas - .as_ref() - .is_some_and(|s| s.one_of.as_ref().is_some_and(|x| !x.is_empty())) - { - panic!("We have an unexpected one_of nested under an any_of. Not yet sure what and how to hoist") - } - - new_schema -} - - impl Transform for StructuralSchemaRewriter { fn transform(&mut self, transform_schema: &mut schemars::Schema) { // Apply this (self) transform to all subschemas schemars::transform::transform_subschemas(self, transform_schema); - // TODO (@NickLarsenNZ): Replace with conversion function - let schema: SchemaObject = match serde_json::from_value(transform_schema.clone().to_value()).ok() { + let mut schema: SchemaObject = match serde_json::from_value(transform_schema.clone().to_value()).ok() + { + // TODO (@NickLarsenNZ): At this point, we are seeing duplicate `title` when printing schema as json. + // This is because `title` is specified under both `extensions` and `other`. Some(schema) => schema, None => return, }; - let schema = hoist_one_of_enum(schema); - let schema = hoist_any_of_option_enum(schema); - // todo: let schema = strip_any_of_empty_object_entry(schema); // this is currently done in an `else if` block in hoist_subschema_properties() - let mut schema = schema; - - if let Some(subschemas) = &mut schema.subschemas { - // // if let Some(one_of) = subschemas.one_of.as_mut() { - // // // Tagged enums are serialized using `one_of` - // // // Maybe we also have to hoist properties? - // // hoist_subschema_properties(one_of, &mut schema.object, &mut schema.instance_type); - // // // "Plain" enums are serialized using `one_of` if they have doc tags - // // // This is now done by hoist_one_of_enum - // // hoist_subschema_enum_values(one_of, &mut schema.enum_values, &mut schema.instance_type); + dbg!(&schema); - // // if one_of.is_empty() { - // // subschemas.one_of = None; - // // } - // // } - - if let Some(any_of) = &mut subschemas.any_of { - // Untagged enums are serialized using `any_of` - hoist_subschema_properties(any_of, &mut schema.object, &mut schema.instance_type); - } - } + // Hoist parts of the schema to make it valid for k8s + hoist_one_of_enum_with_unit_variants(&mut schema); + hoist_any_of_subschema_with_a_nullable_variant(&mut schema); + hoist_properties_for_any_of_subschemas(&mut schema); // check for maps without with properties (i.e. flattened maps) // and allow these to persist dynamically @@ -646,145 +311,3 @@ impl Transform for StructuralSchemaRewriter { } } } - -/// Bring all plain enum values up to the root schema, -/// since Kubernetes doesn't allow subschemas to define enum options. -/// -/// (Enum here means a list of hard-coded values, not a tagged union.) -fn hoist_subschema_enum_values( - subschemas: &mut Vec, - common_enum_values: &mut Option>, - instance_type: &mut Option>, -) { - subschemas.retain(|variant| { - if let Schema::Object(SchemaObject { - instance_type: variant_type, - enum_values: Some(variant_enum_values), - .. - }) = variant - { - if let Some(variant_type) = variant_type { - match instance_type { - None => *instance_type = Some(variant_type.clone()), - Some(tpe) => { - if tpe != variant_type { - panic!("Enum variant set {variant_enum_values:?} has type {variant_type:?} but was already defined as {instance_type:?}. The instance type must be equal for all subschema variants.") - } - } - } - } - common_enum_values - .get_or_insert_with(Vec::new) - .extend(variant_enum_values.iter().cloned()); - false - } else { - true - } - }) -} - -/// Bring all property definitions from subschemas up to the root schema, -/// since Kubernetes doesn't allow subschemas to define properties. -fn hoist_subschema_properties( - subschemas: &mut Vec, - common_obj: &mut Option>, - instance_type: &mut Option>, -) { - for variant in subschemas { - if let Schema::Object(SchemaObject { - instance_type: variant_type, - object: Some(variant_obj), - metadata: variant_metadata, - .. - }) = variant - { - let common_obj = common_obj.get_or_insert_with(Box::::default); - - if let Some(variant_metadata) = variant_metadata { - // Move enum variant description from oneOf clause to its corresponding property - if let Some(description) = std::mem::take(&mut variant_metadata.description) { - if let Some(Schema::Object(variant_object)) = - only_item(variant_obj.properties.values_mut()) - { - let metadata = variant_object - .metadata - .get_or_insert_with(Box::::default); - metadata.description = Some(description); - } - } - } - - // Move all properties - let variant_properties = std::mem::take(&mut variant_obj.properties); - for (property_name, property) in variant_properties { - match common_obj.properties.entry(property_name) { - Entry::Vacant(entry) => { - entry.insert(property); - } - Entry::Occupied(entry) => { - if &property != entry.get() { - panic!("Property {:?} has the schema {:?} but was already defined as {:?} in another subschema. The schemas for a property used in multiple subschemas must be identical", - entry.key(), - &property, - entry.get()); - } - } - } - } - - // Kubernetes doesn't allow variants to set additionalProperties - variant_obj.additional_properties = None; - - merge_metadata(instance_type, variant_type.take()); - } else if let Schema::Object(SchemaObject { - object: None, - instance_type: variant_type, - metadata, - .. - }) = variant - { - // Clear out the description and type to represent the empty `{}` variant - if *variant_type == Some(SingleOrVec::Single(Box::new(InstanceType::Object))) { - *variant_type = None; - *metadata = None; - } - } - } -} - -/// Get the single item from an [Iterator]. -/// -/// Return None if the [Iterator] is empty or has more than one entry. -fn only_item(mut i: I) -> Option { - let item = i.next()?; - if i.next().is_some() { - return None; - } - Some(item) -} - -#[test] -fn only_item_t() { - assert_eq!(only_item::>([].iter()), None); - assert_eq!(only_item(["one"].iter()), Some(&"one")); - assert_eq!(only_item(["one", "two"].iter()), None); -} - -fn merge_metadata( - instance_type: &mut Option>, - variant_type: Option>, -) { - match (instance_type, variant_type) { - (_, None) => {} - (common_type @ None, variant_type) => { - *common_type = variant_type; - } - (Some(common_type), Some(variant_type)) => { - if *common_type != variant_type { - panic!( - "variant defined type {variant_type:?}, conflicting with existing type {common_type:?}" - ); - } - } - } -} diff --git a/kube-core/src/schema/transformsb.rs b/kube-core/src/schema/transform_any_of.rs similarity index 100% rename from kube-core/src/schema/transformsb.rs rename to kube-core/src/schema/transform_any_of.rs diff --git a/kube-core/src/schema/transforms.rs b/kube-core/src/schema/transform_one_of.rs similarity index 100% rename from kube-core/src/schema/transforms.rs rename to kube-core/src/schema/transform_one_of.rs diff --git a/kube-core/src/schema/transformsc.rs b/kube-core/src/schema/transform_properties.rs similarity index 100% rename from kube-core/src/schema/transformsc.rs rename to kube-core/src/schema/transform_properties.rs diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index a25f01cad..f335be6d3 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -266,11 +266,10 @@ fn untagged_enum() { "description": "An untagged enum with a nested enum inside", "properties": { "one": { - "description": "Used in case the `one` field of tpye [`u32`] is present", "type": "string" }, "two": { - "description": "Used in case the `two` field of type [`NormalEnum`] is present", + "description": "A very simple enum with empty variants", "enum": [ "C", "D", @@ -356,11 +355,10 @@ fn optional_untagged_enum() { "nullable": true, "properties": { "one": { - "description": "Used in case the `one` field of tpye [`u32`] is present", "type": "string" }, "two": { - "description": "Used in case the `two` field of type [`NormalEnum`] is present", + "description": "A very simple enum with empty variants", "enum": [ "C", "D", @@ -442,11 +440,10 @@ fn flattened_untagged_enum() { "description": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`", "properties": { "one": { - "description": "Used in case the `one` field of tpye [`u32`] is present", "type": "string" }, "two": { - "description": "Used in case the `two` field of type [`NormalEnum`] is present", + "description": "A very simple enum with empty variants", "enum": [ "C", "D", From 8e40d01f385b4e3f8ea780daaff4795ff6e45504 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 22 Oct 2025 12:14:20 +0200 Subject: [PATCH 20/27] fix: Handle property hoisting for tagged enums (oneOf subschemas) Also update function docs, and expected panic messages --- kube-core/src/schema/mod.rs | 2 - kube-core/src/schema/transform_any_of.rs | 8 ++- kube-core/src/schema/transform_one_of.rs | 47 ++++++++------ kube-core/src/schema/transform_properties.rs | 67 +++++++++++++------- kube-derive/tests/crd_mixed_enum_test.rs | 6 +- 5 files changed, 78 insertions(+), 52 deletions(-) diff --git a/kube-core/src/schema/mod.rs b/kube-core/src/schema/mod.rs index dc27fe20e..560bf076d 100644 --- a/kube-core/src/schema/mod.rs +++ b/kube-core/src/schema/mod.rs @@ -274,8 +274,6 @@ impl Transform for StructuralSchemaRewriter { None => return, }; - dbg!(&schema); - // Hoist parts of the schema to make it valid for k8s hoist_one_of_enum_with_unit_variants(&mut schema); hoist_any_of_subschema_with_a_nullable_variant(&mut schema); diff --git a/kube-core/src/schema/transform_any_of.rs b/kube-core/src/schema/transform_any_of.rs index 42fdc83f2..5259bc52a 100644 --- a/kube-core/src/schema/transform_any_of.rs +++ b/kube-core/src/schema/transform_any_of.rs @@ -83,14 +83,16 @@ fn optional_tagged_enum_with_unit_variants() { } -/// Replace the schema with the non-null anyOf subschema when the only other subschema is the null schema. +/// Replace the schema with the anyOf subschema and set to nullable when the +/// only other subschema is the nullable entry. /// /// Used for correcting the schema for optional tagged unit enums. /// The non-null subschema is hoisted, and nullable will be set to true. /// /// This will return early without modifications unless: -/// - There are exactly 2 `anyOf` subschemas -/// - One subschema represents the `null` (has an enum with a null entry, and nullable set to true) +/// - There are exactly 2 `anyOf` subschemas. +/// - One subschema represents the nullability (ie: it has an enum with a single +/// null entry, and nullable set to true). /// /// NOTE: This should work regardless of whether other hoisting has been performed or not. pub(crate) fn hoist_any_of_subschema_with_a_nullable_variant(kube_schema: &mut SchemaObject) { diff --git a/kube-core/src/schema/transform_one_of.rs b/kube-core/src/schema/transform_one_of.rs index 736bac2fe..81a6abc94 100644 --- a/kube-core/src/schema/transform_one_of.rs +++ b/kube-core/src/schema/transform_one_of.rs @@ -56,18 +56,21 @@ fn tagged_enum_with_unit_variants() { } -/// Replace a list of typed oneOf subschemas with a typed schema level enum +/// Merge oneOf subschema enums and consts into a schema level enum. /// /// Used for correcting the schema for tagged enums with unit variants. -/// NOTE: Subschema descriptions are lost when they are combined into a single enum of the same type. +/// +/// NOTE: Subschema descriptions are lost when they are combined into a single +/// enum of the same type. /// /// This will return early without modifications unless: -/// - There are `oneOf` subschemas (not empty) -/// - Each subschema contains an enum -/// - Each subschema is typed -/// - Each subschemas types is the same as the others +/// - There are `oneOf` subschemas (not empty). +/// - Each subschema contains an `enum` or `const`. +/// +/// Subschemas must define a type, and they must be the same for all. /// -/// NOTE: This should work regardless of whether other hoisting has been performed or not. +/// NOTE: This should work regardless of whether other hoisting has been +/// performed or not. pub(crate) fn hoist_one_of_enum_with_unit_variants(kube_schema: &mut SchemaObject) { // Run some initial checks in case there is nothing to do let SchemaObject { @@ -113,19 +116,23 @@ pub(crate) fn hoist_one_of_enum_with_unit_variants(kube_schema: &mut SchemaObjec // For each `oneOf` entry, iterate over the `enum` and `const` values. // Panic on an entry that doesn't contain an `enum` or `const`. - let new_enums = one_of.iter().flat_map(|schema| match schema { - Schema::Object(SchemaObject { - enum_values: Some(r#enum), - .. - }) => r#enum.clone(), - // Warning: The `const` check below must come after the enum check above. - // Otherwise it will panic on a valid entry with an `enum`. - Schema::Object(SchemaObject { other, .. }) => match other.get("const") { - Some(r#const) => vec![r#const.clone()], - None => panic!("oneOf variant did not provide \"enum\" or \"const\": {schema:#?}"), - }, - Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), - }); + let new_enums = one_of + .iter() + .flat_map(|schema| match schema { + Schema::Object(SchemaObject { + enum_values: Some(r#enum), + .. + }) => r#enum.clone(), + Schema::Object(SchemaObject { other, .. }) => other.get("const").cloned().into_iter().collect(), + Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + }) + .collect::>(); + + // If there are no enums in the oneOf subschemas, there is nothing more to do here. + if new_enums.is_empty() { + return; + } + // Merge the enums (extend just to be safe) kube_schema.enum_values.get_or_insert_default().extend(new_enums); diff --git a/kube-core/src/schema/transform_properties.rs b/kube-core/src/schema/transform_properties.rs index 8df3c82fd..ecec74f94 100644 --- a/kube-core/src/schema/transform_properties.rs +++ b/kube-core/src/schema/transform_properties.rs @@ -1,6 +1,4 @@ -use std::ops::DerefMut; - -use crate::schema::{InstanceType, Schema, SchemaObject, SingleOrVec, SubschemaValidation}; +use crate::schema::{InstanceType, Schema, SchemaObject, SingleOrVec}; #[cfg(test)] #[test] @@ -342,13 +340,14 @@ fn invalid_untagged_enum_with_conflicting_variant_fields_after_one_of_hosting() hoist_properties_for_any_of_subschemas(&mut actual_converted_schema_object); } -/// Take subschema properties and insert them into the schema properties. +/// Take oneOf or anyOf subschema properties and move them them into the schema +/// properties. /// /// Used for correcting the schema for serde untagged structural enums. -/// NOTE: Due to the nature of "untagging", enum variant doc-comments are not preserved. +/// NOTE: Doc-comments are not preserved for untagged enums. /// /// This will return early without modifications unless: -/// - There are `anyOf` subschemas +/// - There are `oneOf` or `anyOf` subschemas /// - Each subschema has the type "object" /// /// NOTE: This should work regardless of whether other hoisting has been performed or not. @@ -363,21 +362,23 @@ pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObj return; }; - let SubschemaValidation { - any_of: Some(any_of), - one_of, - } = subschemas.deref_mut() - else { - return; + // Deal with both tagged and untagged enums. + // Untagged enum descriptions will not be preserved. + let (subschemas, preserve_description) = match (subschemas.any_of.as_mut(), subschemas.one_of.as_mut()) { + (None, None) => return, + (None, Some(one_of)) => (one_of, true), + (Some(any_of), None) => (any_of, false), + (Some(_), Some(_)) => panic!("oneOf and anyOf are mutually exclusive"), }; - if any_of.is_empty() { + + if subschemas.is_empty() { return; } // Ensure we aren't looking at the one with a null // TODO (@NickLarsenNZ): Combine the logic with the function that covers the nullable anyOf - if any_of.len() == 2 { + if subschemas.len() == 2 { // This is the signature for the null variant, indicating the "other" // variant is the subschema that needs hoisting let null = serde_json::json!({ @@ -386,7 +387,7 @@ pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObj }); // Return if one of the two entries are nulls - for value in any_of + for value in subschemas .iter() .map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON")) { @@ -399,29 +400,47 @@ pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObj // At this point, we can be reasonably sure we need operate on the schema. // TODO (@NickLarsenNZ): Return errors instead of panicking, leave panicking up to the infallible schemars::Transform - // There should not be any oneOf's adjacent to the anyOf - if one_of.is_some() { - panic!("oneOf is set when there is already an anyOf: {one_of:#?}"); - } - - let subschemas = any_of + let subschemas = subschemas .into_iter() .map(|schema| match schema { Schema::Object(schema_object) => schema_object, - Schema::Bool(_) => panic!("oneOf variants can not be of type boolean"), + Schema::Bool(_) => panic!("oneOf and anyOf variants cannot be of type boolean"), }) .collect::>(); for subschema in subschemas { - // Drop description/type // This will clear out any objects that don't have required/properties fields (so that it appears as: {}). - subschema.metadata.take(); + let metadata = subschema.metadata.take(); subschema.instance_type.take(); // Set the schema type to object kube_schema.instance_type = Some(SingleOrVec::Single(Box::new(InstanceType::Object))); if let Some(object) = subschema.object.as_deref_mut() { + // Kubernetes doesn't allow variants to set additionalProperties + object.additional_properties.take(); + + // For a tagged enum, we need to preserve the variant description + if preserve_description { + if object.properties.len() != 1 { + // TODO (@NickLarsenNZ): Use assert_eq with error message (and in the other place) + panic!("Expecting a subschema for a tagged enum variant, so there should only be one property"); + } + + if let Schema::Object(x) = object + .properties + .values_mut() + .next() + .expect("it's there, trust sbernauer") + { + // I hope the new metadata doesn't have default set + // I also hope that we didn't destroy some existing metadata. + // Surely it would only exist in one or the other + // See: https://github.com/kube-rs/kube/blob/98bfbe3d7923321a16ccde9e690fd2ce8c7efc32/kube-core/src/schema.rs#L409-L426 + x.metadata = metadata + }; + } + // If properties are set, hoist them to the schema properties. // This will panic if duplicate properties are encountered that do not have the same shape. // That can happen when the untagged enum variants each refer to structs which contain the same field name. diff --git a/kube-derive/tests/crd_mixed_enum_test.rs b/kube-derive/tests/crd_mixed_enum_test.rs index 08dfb8249..7bb7679a2 100644 --- a/kube-derive/tests/crd_mixed_enum_test.rs +++ b/kube-derive/tests/crd_mixed_enum_test.rs @@ -53,13 +53,13 @@ fn print_crds() { } #[test] -#[should_panic = "Enum variant set [String(\"A\")] has type Single(String) but was already defined as Some(Single(Object)). The instance type must be equal for all subschema variants."] +#[should_panic = "oneOf variants must all have the same type"] fn invalid_enum_1() { InvalidEnum1::crd(); } #[test] -#[should_panic = "Enum variant set [String(\"A\")] has type Single(String) but was already defined as Some(Single(Object)). The instance type must be equal for all subschema variants."] +#[should_panic = "oneOf variants must all have the same type"] fn invalid_enum_2() { InvalidEnum2::crd(); } @@ -222,7 +222,7 @@ fn valid_enum_4() { } #[test] -#[should_panic = "Enum variant set [String(\"A\")] has type Single(String) but was already defined as Some(Single(Object)). The instance type must be equal for all subschema variants."] +#[should_panic = "oneOf variants must all have the same type"] fn struct_with_enum_1() { #[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] #[kube(group = "clux.dev", version = "v1", kind = "Foo")] From e3113616e6e6e366dc31016e7382f2f51c480dc8 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 22 Oct 2025 12:24:36 +0200 Subject: [PATCH 21/27] lint: commit whitespace differences --- kube-derive/tests/crd_complex_enum_tests.rs | 452 ++++++++++---------- 1 file changed, 226 insertions(+), 226 deletions(-) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index f335be6d3..fe61314c3 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -223,85 +223,85 @@ fn untagged_enum() { assert_json_eq!( UntaggedEnumTest::crd(), json!( - { - "apiVersion": "apiextensions.k8s.io/v1", - "kind": "CustomResourceDefinition", - "metadata": { - "name": "untaggedenumtests.clux.dev" + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "untaggedenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "UntaggedEnumTest", + "plural": "untaggedenumtests", + "shortNames": [], + "singular": "untaggedenumtest" }, - "spec": { - "group": "clux.dev", - "names": { - "categories": [], - "kind": "UntaggedEnumTest", - "plural": "untaggedenumtests", - "shortNames": [], - "singular": "untaggedenumtest" - }, - "scope": "Cluster", - "versions": [ - { - "additionalPrinterColumns": [], - "name": "v1", - "schema": { - "openAPIV3Schema": { - "description": "Auto-generated derived type for UntaggedEnumTestSpec via `CustomResource`", - "properties": { - "spec": { - "properties": { - "foo": { - "anyOf": [ - { - "required": [ - "one" - ] - }, - { - "required": [ - "two" - ] - }, - {} - ], - "description": "An untagged enum with a nested enum inside", - "properties": { - "one": { - "type": "string" - }, - "two": { - "description": "A very simple enum with empty variants", - "enum": [ - "C", - "D", - "A", - "B" - ], - "type": "string" - } + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for UntaggedEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "required": [ + "one" + ] }, - "type": "object" - } - }, - "required": [ - "foo" - ], - "type": "object" - } - }, - "required": [ - "spec" - ], - "title": "UntaggedEnumTest", - "type": "object" - } - }, - "served": true, - "storage": true, - "subresources": {} - } - ] - } + { + "required": [ + "two" + ] + }, + {} + ], + "description": "An untagged enum with a nested enum inside", + "properties": { + "one": { + "type": "string" + }, + "two": { + "description": "A very simple enum with empty variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "UntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] } + } ) ); } @@ -311,83 +311,83 @@ fn optional_untagged_enum() { assert_json_eq!( OptionalUntaggedEnumTest::crd(), json!( - { - "apiVersion": "apiextensions.k8s.io/v1", - "kind": "CustomResourceDefinition", - "metadata": { - "name": "optionaluntaggedenumtests.clux.dev" + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "optionaluntaggedenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "OptionalUntaggedEnumTest", + "plural": "optionaluntaggedenumtests", + "shortNames": [], + "singular": "optionaluntaggedenumtest" }, - "spec": { - "group": "clux.dev", - "names": { - "categories": [], - "kind": "OptionalUntaggedEnumTest", - "plural": "optionaluntaggedenumtests", - "shortNames": [], - "singular": "optionaluntaggedenumtest" - }, - "scope": "Cluster", - "versions": [ - { - "additionalPrinterColumns": [], - "name": "v1", - "schema": { - "openAPIV3Schema": { - "description": "Auto-generated derived type for OptionalUntaggedEnumTestSpec via `CustomResource`", - "properties": { - "spec": { - "properties": { - "foo": { - "anyOf": [ - { - "required": [ - "one" - ] - }, - { - "required": [ - "two" - ] - }, - {} - ], - "description": "An untagged enum with a nested enum inside", - "nullable": true, - "properties": { - "one": { - "type": "string" - }, - "two": { - "description": "A very simple enum with empty variants", - "enum": [ - "C", - "D", - "A", - "B" - ], - "type": "string" - } + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for OptionalUntaggedEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "required": [ + "one" + ] }, - "type": "object" - } - }, - "type": "object" - } - }, - "required": [ - "spec" - ], - "title": "OptionalUntaggedEnumTest", - "type": "object" - } - }, - "served": true, - "storage": true, - "subresources": {} - } - ] - } + { + "required": [ + "two" + ] + }, + {} + ], + "description": "An untagged enum with a nested enum inside", + "nullable": true, + "properties": { + "one": { + "type": "string" + }, + "two": { + "description": "A very simple enum with empty variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalUntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] } + } ) ); } @@ -397,85 +397,85 @@ fn flattened_untagged_enum() { assert_json_eq!( FlattenedUntaggedEnumTest::crd(), json!( - { - "apiVersion": "apiextensions.k8s.io/v1", - "kind": "CustomResourceDefinition", - "metadata": { - "name": "flatteneduntaggedenumtests.clux.dev" + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "flatteneduntaggedenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "FlattenedUntaggedEnumTest", + "plural": "flatteneduntaggedenumtests", + "shortNames": [], + "singular": "flatteneduntaggedenumtest" }, - "spec": { - "group": "clux.dev", - "names": { - "categories": [], - "kind": "FlattenedUntaggedEnumTest", - "plural": "flatteneduntaggedenumtests", - "shortNames": [], - "singular": "flatteneduntaggedenumtest" - }, - "scope": "Cluster", - "versions": [ - { - "additionalPrinterColumns": [], - "name": "v1", - "schema": { - "openAPIV3Schema": { - "description": "Auto-generated derived type for FlattenedUntaggedEnumTestSpec via `CustomResource`", - "properties": { - "spec": { - "properties": { - "foo": { - "anyOf": [ - { - "required": [ - "one" - ] - }, - { - "required": [ - "two" - ] - }, - {} - ], - "description": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`", - "properties": { - "one": { - "type": "string" - }, - "two": { - "description": "A very simple enum with empty variants", - "enum": [ - "C", - "D", - "A", - "B" - ], - "type": "string" - } + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for FlattenedUntaggedEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "anyOf": [ + { + "required": [ + "one" + ] + }, + { + "required": [ + "two" + ] + }, + {} + ], + "description": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`", + "properties": { + "one": { + "type": "string" }, - "type": "object" - } - }, - "required": [ - "foo" - ], - "type": "object" - } - }, - "required": [ - "spec" - ], - "title": "FlattenedUntaggedEnumTest", - "type": "object" - } - }, - "served": true, - "storage": true, - "subresources": {} - } - ] - } + "two": { + "description": "A very simple enum with empty variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "FlattenedUntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] } + } ) ); } From 307e828598aa9b2b0c6545fe149027d83eb611df Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 22 Oct 2025 15:52:40 +0200 Subject: [PATCH 22/27] chore: use assert/assert_eq with message instead of panic --- kube-core/src/schema/transform_any_of.rs | 1 + kube-core/src/schema/transform_properties.rs | 45 ++++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/kube-core/src/schema/transform_any_of.rs b/kube-core/src/schema/transform_any_of.rs index 5259bc52a..f33079107 100644 --- a/kube-core/src/schema/transform_any_of.rs +++ b/kube-core/src/schema/transform_any_of.rs @@ -159,5 +159,6 @@ pub(crate) fn hoist_any_of_subschema_with_a_nullable_variant(kube_schema: &mut S *kube_schema = to_hoist.clone(); // Set the schema to nullable (as we know we matched the null variant earlier) + // TODO (@NickLarsenNZ): Do we need to insert `nullable` into `other` too? kube_schema.extensions.insert("nullable".to_owned(), true.into()); } diff --git a/kube-core/src/schema/transform_properties.rs b/kube-core/src/schema/transform_properties.rs index ecec74f94..a1b9a9534 100644 --- a/kube-core/src/schema/transform_properties.rs +++ b/kube-core/src/schema/transform_properties.rs @@ -377,7 +377,6 @@ pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObj } // Ensure we aren't looking at the one with a null - // TODO (@NickLarsenNZ): Combine the logic with the function that covers the nullable anyOf if subschemas.len() == 2 { // This is the signature for the null variant, indicating the "other" // variant is the subschema that needs hoisting @@ -422,28 +421,31 @@ pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObj // For a tagged enum, we need to preserve the variant description if preserve_description { - if object.properties.len() != 1 { - // TODO (@NickLarsenNZ): Use assert_eq with error message (and in the other place) - panic!("Expecting a subschema for a tagged enum variant, so there should only be one property"); - } + assert_eq!( + object.properties.len(), + 1, + "Expecting only a single property defined for the tagged enum variant schema" + ); - if let Schema::Object(x) = object + if let Schema::Object(subschema) = object .properties .values_mut() .next() - .expect("it's there, trust sbernauer") + .expect("asserted that one and only one property exists") { - // I hope the new metadata doesn't have default set - // I also hope that we didn't destroy some existing metadata. - // Surely it would only exist in one or the other - // See: https://github.com/kube-rs/kube/blob/98bfbe3d7923321a16ccde9e690fd2ce8c7efc32/kube-core/src/schema.rs#L409-L426 - x.metadata = metadata + assert!( + // While possible, it is unexpected if the subschema metadata is already set for the property + subschema.metadata.is_none(), + "subschema metadata for property should be empty" + ); + // Move the variant description down to the properties (before they get hoisted) + subschema.metadata = metadata }; } // If properties are set, hoist them to the schema properties. // This will panic if duplicate properties are encountered that do not have the same shape. - // That can happen when the untagged enum variants each refer to structs which contain the same field name. + // That can happen when the untagged enum variants each refer to structs which contain the same field name but with different types or doc-comments. // The developer needs to make them the same. // TODO (@NickLarsenNZ): Add a case for a structural variant, and a tuple variant containing a structure where the same field name is used. while let Some((property_name, Schema::Object(property_schema_object))) = @@ -456,16 +458,13 @@ pub(crate) fn hoist_properties_for_any_of_subschemas(kube_schema: &mut SchemaObj .properties .get(&property_name) { - if existing_property != &Schema::Object(property_schema_object.clone()) { - // TODO (@NickLarsenNZ): Here we could do another check to see if it only differs by description. - // If the schema property description is not set, then we could overwrite it and not panic. - dbg!( - &property_name, - existing_property, - &Schema::Object(property_schema_object.clone()), - ); - panic!("Properties for {property_name:?} are defined multiple times with different shapes") - } + // TODO (@NickLarsenNZ): Here we could do another check to see if it only differs by description. + // If the schema property description is not set, then we could overwrite it and not panic. + assert_eq!( + existing_property, + &Schema::Object(property_schema_object.clone()), + "Properties for {property_name:?} are defined multiple times with different shapes" + ); } else { // Otherwise, insert the subschema properties into the schema properties parent_object From 25e10dbb45d3e5704c77666104c64485504afbcf Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 23 Oct 2025 14:24:31 +0200 Subject: [PATCH 23/27] Add integration test for (optional) complex enums --- kube-derive/tests/crd_complex_enum_tests.rs | 42 +++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index fe61314c3..ad3522af4 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -5,6 +5,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::json; +// Enum definitions + /// A very simple enum with empty variants #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] enum NormalEnum { @@ -18,6 +20,13 @@ enum NormalEnum { D, } +/// A complex enum with unit and struct variants +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +enum ComplexEnum { + Normal(NormalEnum), + Hardcore { hard: String, core: NormalEnum }, +} + /// An untagged enum with a nested enum inside #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] #[serde(untagged)] @@ -37,6 +46,8 @@ struct FlattenedUntaggedEnum { inner: UntaggedEnum, } +// CRD definitions + #[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] #[kube(group = "clux.dev", version = "v1", kind = "NormalEnumTest")] struct NormalEnumTestSpec { @@ -49,6 +60,18 @@ struct OptionalEnumTestSpec { foo: Option, } +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "ComplexEnumTest")] +struct ComplexEnumTestSpec { + foo: ComplexEnum, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "OptionalComplexEnumTest")] +struct OptionalComplexEnumTestSpec { + foo: Option, +} + #[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] #[kube(group = "clux.dev", version = "v1", kind = "UntaggedEnumTest")] struct UntaggedEnumTestSpec { @@ -75,6 +98,13 @@ fn print_crds() { println!("---"); println!("{}", serde_yaml::to_string(&OptionalEnumTest::crd()).unwrap()); println!("---"); + println!("{}", serde_yaml::to_string(&ComplexEnumTest::crd()).unwrap()); + println!("---"); + println!( + "{}", + serde_yaml::to_string(&OptionalComplexEnumTest::crd()).unwrap() + ); + println!("---"); println!("{}", serde_yaml::to_string(&UntaggedEnumTest::crd()).unwrap()); println!("---"); println!( @@ -218,6 +248,18 @@ fn optional_enum() { ); } + +#[test] +fn complex_enum() { + assert_json_eq!(ComplexEnumTest::crd(), json!(42)); +} + + +#[test] +fn optional_complex_enum() { + assert_json_eq!(OptionalComplexEnumTest::crd(), json!(42)); +} + #[test] fn untagged_enum() { assert_json_eq!( From 764c418961b17e3448905c6bca6997a433832015 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 23 Oct 2025 15:03:01 +0200 Subject: [PATCH 24/27] Add test case for Option>> --- kube-derive/tests/crd_schema_test.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/kube-derive/tests/crd_schema_test.rs b/kube-derive/tests/crd_schema_test.rs index c00009ff6..77c733e29 100644 --- a/kube-derive/tests/crd_schema_test.rs +++ b/kube-derive/tests/crd_schema_test.rs @@ -49,6 +49,7 @@ struct FooSpec { #[serde(skip_serializing_if = "Option::is_none")] nullable_skipped: Option, nullable: Option, + nullnullnullable: Option>>, #[serde(skip_serializing_if = "Option::is_none")] #[serde(default = "default_nullable")] @@ -175,6 +176,7 @@ fn test_serialized_matches_expected() { non_nullable_with_default: "asdf".to_string(), nullable_skipped: None, nullable: None, + nullnullnullable: None, nullable_skipped_with_default: None, nullable_with_default: None, timestamp: DateTime::from_timestamp(0, 0).unwrap(), @@ -207,6 +209,7 @@ fn test_serialized_matches_expected() { "nonNullable": "asdf", "nonNullableWithDefault": "asdf", "nullable": null, + "nullnullnullable": null, "nullableWithDefault": null, "timestamp": "1970-01-01T00:00:00Z", "complexEnum": { @@ -299,6 +302,10 @@ fn test_crd_schema_matches_expected() { "nullable": true, "type": "string" }, + "nullnullnullable": { + "nullable": true, + "type": "string" + }, "nullableSkippedWithDefault": { "default": "default_nullable", "nullable": true, From d52c2c68d11714b7bfe106b4b2b894fcfbed29be Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 23 Oct 2025 15:03:40 +0200 Subject: [PATCH 25/27] Remove leftover newline --- kube-derive/tests/crd_schema_test.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/kube-derive/tests/crd_schema_test.rs b/kube-derive/tests/crd_schema_test.rs index 77c733e29..6a7670484 100644 --- a/kube-derive/tests/crd_schema_test.rs +++ b/kube-derive/tests/crd_schema_test.rs @@ -293,7 +293,6 @@ fn test_crd_schema_matches_expected() { "default": "default_value", "type": "string" }, - "nullableSkipped": { "nullable": true, "type": "string" From 6d156a04db242d8c0606d282d638a49e9be43da6 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 23 Oct 2025 15:27:32 +0200 Subject: [PATCH 26/27] Add (failing) test for enums without descriptions --- kube-derive/tests/crd_complex_enum_tests.rs | 173 +++++++++++++++++++- 1 file changed, 172 insertions(+), 1 deletion(-) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index ad3522af4..468bdcd02 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -20,11 +20,23 @@ enum NormalEnum { D, } +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +pub enum NormalEnumWithoutDescriptions { + A, + B, + C, + D, +} + /// A complex enum with unit and struct variants #[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] enum ComplexEnum { Normal(NormalEnum), - Hardcore { hard: String, core: NormalEnum }, + Hardcore { + hard: String, + core: NormalEnum, + without_description: NormalEnumWithoutDescriptions, + }, } /// An untagged enum with a nested enum inside @@ -60,6 +72,26 @@ struct OptionalEnumTestSpec { foo: Option, } +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube( + group = "clux.dev", + version = "v1", + kind = "NormalEnumWithoutDescriptionsTest" +)] +struct NormalEnumWithoutDescriptionsTestSpec { + foo: NormalEnumWithoutDescriptions, +} + +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube( + group = "clux.dev", + version = "v1", + kind = "OptionalEnumWithoutDescriptionsTest" +)] +struct OptionalEnumWithoutDescriptionsTestSpec { + foo: Option, +} + #[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] #[kube(group = "clux.dev", version = "v1", kind = "ComplexEnumTest")] struct ComplexEnumTestSpec { @@ -98,6 +130,16 @@ fn print_crds() { println!("---"); println!("{}", serde_yaml::to_string(&OptionalEnumTest::crd()).unwrap()); println!("---"); + println!( + "{}", + serde_yaml::to_string(&NormalEnumWithoutDescriptionsTest::crd()).unwrap() + ); + println!("---"); + println!( + "{}", + serde_yaml::to_string(&OptionalEnumWithoutDescriptionsTest::crd()).unwrap() + ); + println!("---"); println!("{}", serde_yaml::to_string(&ComplexEnumTest::crd()).unwrap()); println!("---"); println!( @@ -249,6 +291,135 @@ fn optional_enum() { } +#[test] +fn normal_enum_without_descriptions() { + assert_json_eq!( + NormalEnumWithoutDescriptionsTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "normalenumwithoutdescriptionstests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "NormalEnumWithoutDescriptionsTest", + "plural": "normalenumwithoutdescriptionstests", + "shortNames": [], + "singular": "normalenumwithoutdescriptionstest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for NormalEnumWithoutDescriptionsTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "enum": [ + "A", + "B", + "C", + "D" + ], + "type": "string" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "NormalEnumWithoutDescriptionsTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[test] +fn optional_enum_without_descriptions() { + assert_json_eq!( + OptionalEnumWithoutDescriptionsTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "optionalenumwithoutdescriptionstests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "OptionalEnumWithoutDescriptionsTest", + "plural": "optionalenumwithoutdescriptionstests", + "shortNames": [], + "singular": "optionalenumwithoutdescriptionstest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for OptionalEnumWithoutDescriptionsTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "enum": [ + "A", + "B", + "C", + "D", + // Note there is *no* null list entry here + ], + "nullable": true, + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalEnumWithoutDescriptionsTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + #[test] fn complex_enum() { assert_json_eq!(ComplexEnumTest::crd(), json!(42)); From e3172d597bd9f955c7ecc3a6ad6dfe44e547c7bc Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Thu, 23 Oct 2025 15:30:36 +0200 Subject: [PATCH 27/27] Improve test comment --- kube-derive/tests/crd_complex_enum_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kube-derive/tests/crd_complex_enum_tests.rs b/kube-derive/tests/crd_complex_enum_tests.rs index 468bdcd02..e8cb97a32 100644 --- a/kube-derive/tests/crd_complex_enum_tests.rs +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -393,7 +393,7 @@ fn optional_enum_without_descriptions() { "B", "C", "D", - // Note there is *no* null list entry here + // Note there should be *no* null list entry here ], "nullable": true, "type": "string"