Hoist enum values from subschemas

Fixes kube-rs#1047

Signed-off-by: Teo Klestrup Röijezon <teo@nullable.se>

kube-core/src/schema.rs

@@ -33,22 +33,23 @@ impl Visitor for StructuralSchemaRewriter {
     fn visit_schema_object(&mut self, schema: &mut schemars::schema::SchemaObject) {
         schemars::visit::visit_schema_object(self, schema);
 
-        if let Some(one_of) = schema
-            .subschemas
-            .as_mut()
-            .and_then(|subschemas| 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(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(any_of) = schema
-            .subschemas
-            .as_mut()
-            .and_then(|subschemas| subschemas.any_of.as_mut())
-        {
-            // Untagged enums are serialized using `any_of`
-            hoist_subschema_properties(any_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);
+
+                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);
+            }
         }
 
         // check for maps without with properties (i.e. flattened maps)
@@ -66,15 +67,49 @@ impl Visitor 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<Schema>,
+    common_enum_values: &mut Option<Vec<serde_json::Value>>,
+    instance_type: &mut Option<SingleOrVec<InstanceType>>,
+) {
+    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<Schema>,
     common_obj: &mut Option<Box<ObjectValidation>>,
     instance_type: &mut Option<SingleOrVec<InstanceType>>,
 ) {
-    let common_obj = common_obj.get_or_insert_with(|| Box::new(ObjectValidation::default()));
-
     for variant in subschemas {
         if let Schema::Object(SchemaObject {
             instance_type: variant_type,
@@ -83,6 +118,8 @@ fn hoist_subschema_properties(
             ..
         }) = variant
         {
+            let common_obj = common_obj.get_or_insert_with(|| Box::new(ObjectValidation::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) {

kube-derive/Cargo.toml

@@ -31,3 +31,4 @@ schemars = { version = "0.8.6", features = ["chrono"] }
 validator = { version = "0.16.0", features = ["derive"] }
 chrono = { version = "0.4.19", default-features = false }
 trybuild = "1.0.48"
+assert-json-diff = "2.0.2"

kube-derive/tests/crd_schema_test.rs

@@ -1,5 +1,6 @@
 #![recursion_limit = "256"]
 
+use assert_json_diff::assert_json_eq;
 use chrono::{DateTime, NaiveDateTime, Utc};
 use kube_derive::CustomResource;
 use schemars::JsonSchema;
@@ -105,6 +106,7 @@ struct SexAndDateOfBirth {
 enum Sex {
     Female,
     Male,
+    /// This variant has a comment!
     Other,
 }
 
@@ -122,7 +124,7 @@ fn test_shortnames() {
 
 #[test]
 fn test_serialized_matches_expected() {
-    assert_eq!(
+    assert_json_eq!(
         serde_json::to_value(Foo::new("bar", FooSpec {
             non_nullable: "asdf".to_string(),
             non_nullable_with_default: "asdf".to_string(),
@@ -167,9 +169,9 @@ fn test_serialized_matches_expected() {
 #[test]
 fn test_crd_schema_matches_expected() {
     use kube::core::CustomResourceExt;
-    assert_eq!(
+    assert_json_eq!(
         Foo::crd(),
-        serde_json::from_value(serde_json::json!({
+        serde_json::json!({
             "apiVersion": "apiextensions.k8s.io/v1",
             "kind": "CustomResourceDefinition",
             "metadata": {
@@ -318,8 +320,7 @@ fn test_crd_schema_matches_expected() {
                     }
                 ]
             }
-        }))
-        .unwrap()
+        })
     );
 }