diff --git a/kube-core/src/schema.rs b/kube-core/src/schema/mod.rs similarity index 70% rename from kube-core/src/schema.rs rename to kube-core/src/schema/mod.rs index 479918a8e..976e0d263 100644 --- a/kube-core/src/schema.rs +++ b/kube-core/src/schema/mod.rs @@ -2,13 +2,35 @@ //! //! [`CustomResourceDefinition`]: `k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition` +mod transform_any_of; +mod transform_one_of; +mod transform_optional_enum_with_null; +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::Value; -use std::collections::{btree_map::Entry, BTreeMap, BTreeSet}; +use std::{ + collections::{BTreeMap, BTreeSet}, + sync::LazyLock, +}; + +pub use crate::schema::{ + transform_any_of::AnyOfSchemaRewriter, transform_one_of::OneOfSchemaRewriter, + transform_optional_enum_with_null::OptionalEnumSchemaRewriter, + transform_properties::PropertiesSchemaRewriter, +}; + +/// This is the signature for the `null` variant produced by schemars. +static NULL_SCHEMA: LazyLock = LazyLock::new(|| { + serde_json::json!({ + "enum": [null], + "nullable": true + }) +}); /// schemars [`Visitor`] that rewrites a [`Schema`] to conform to Kubernetes' "structural schema" rules /// @@ -24,6 +46,7 @@ use std::collections::{btree_map::Entry, BTreeMap, BTreeSet}; /// /// The [`Visitor`] functions may panic if the transform could not be applied. For example, /// there must not be any overlapping properties between `oneOf` branches. +// TODO (@NickLarsenNZ): Rename this transformer and update the docs above #[derive(Debug, Clone)] pub struct StructuralSchemaRewriter; @@ -248,33 +271,17 @@ enum SingleOrVec { 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); 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, }; - 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); - - // "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) // and allow these to persist dynamically if let Some(object) = &mut schema.object { @@ -297,6 +304,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; @@ -304,123 +312,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()); - } - } -} - -fn only_item(mut i: I) -> Option { - let item = i.next()?; - if i.next().is_some() { - return None; - } - Some(item) -} - -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/transform_any_of.rs b/kube-core/src/schema/transform_any_of.rs new file mode 100644 index 000000000..8d26694ac --- /dev/null +++ b/kube-core/src/schema/transform_any_of.rs @@ -0,0 +1,238 @@ +use std::ops::DerefMut; + +use schemars::transform::Transform; + +use crate::schema::{Schema, SchemaObject, SubschemaValidation, 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 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. +#[derive(Debug, Clone)] +pub struct AnyOfSchemaRewriter; + +impl Transform for AnyOfSchemaRewriter { + fn transform(&mut self, transform_schema: &mut schemars::Schema) { + // Apply this (self) transform to all subschemas + schemars::transform::transform_subschemas(self, transform_schema); + + 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, + }; + + hoist_any_of_subschema_with_a_nullable_variant(&mut schema); + + // 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; + } + } + } +} + +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: None, + } = subschemas.deref_mut() + else { + return; + }; + + if any_of.len() != 2 { + return; + } + + let entry_is_null: [bool; 2] = any_of + .iter() + .map(|schema| serde_json::to_value(schema).expect("schema should be able to convert to JSON")) + .map(|value| value == *NULL_SCHEMA) + .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"); + }; + + let mut to_hoist = to_hoist.clone(); + let kube_schema_metadata = kube_schema.metadata.take(); + + if to_hoist.metadata.is_none() { + to_hoist.metadata = kube_schema_metadata; + } + + // Replace the schema with the non-null subschema + *kube_schema = to_hoist; + + // Set the schema to nullable (as we know we matched the null variant earlier) + kube_schema.extensions.insert("nullable".to_owned(), true.into()); +} + +#[cfg(test)] +mod tests { + use assert_json_diff::assert_json_eq; + + use super::*; + + #[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_eq!(actual_converted_schema_object, expected_converted_schema_object); + } + + #[test] + fn optional_tagged_enum_with_unit_variants_but_also_an_existing_description() { + let original_schema_object_value = serde_json::json!({ + "description": "This comment will be lost", + "anyOf": [ + { + "description": "A very simple enum with empty variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + }, + { + "enum": [ + null + ], + "nullable": true + } + ] + }); + + let expected_converted_schema_object_value = serde_json::json!({ + "description": "A very simple enum with empty variants", + "nullable": true, + "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_any_of_subschema_with_a_nullable_variant(&mut actual_converted_schema_object); + + assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); + } +} diff --git a/kube-core/src/schema/transform_one_of.rs b/kube-core/src/schema/transform_one_of.rs new file mode 100644 index 000000000..0450fce22 --- /dev/null +++ b/kube-core/src/schema/transform_one_of.rs @@ -0,0 +1,181 @@ +use std::ops::DerefMut; + +use schemars::transform::Transform; + +use crate::schema::{Schema, SchemaObject, SubschemaValidation}; + +/// 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. +/// +/// This will return early without modifications unless: +/// - 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. +#[derive(Debug, Clone)] +pub struct OneOfSchemaRewriter; + +impl Transform for OneOfSchemaRewriter { + fn transform(&mut self, transform_schema: &mut schemars::Schema) { + // Apply this (self) transform to all subschemas + schemars::transform::transform_subschemas(self, transform_schema); + + 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, + }; + + hoist_one_of_enum_with_unit_variants(&mut schema); + + // 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; + } + } + } +} + +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 { + any_of: None, + 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(), + 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 (or consts converted to enums) in the oneOf subschemas, there is nothing more to do here. + // For example, when the schema has `properties` and `required`, so we leave that for the properties hoister. + 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); + + // Hoist the type + kube_schema.instance_type = Some(variant_type.clone()); + + // Clear the oneOf subschemas + subschemas.one_of = None; +} + +#[cfg(test)] +mod tests { + use assert_json_diff::assert_json_eq; + + use super::*; + + #[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_eq!(actual_converted_schema_object, expected_converted_schema_object); + } +} diff --git a/kube-core/src/schema/transform_optional_enum_with_null.rs b/kube-core/src/schema/transform_optional_enum_with_null.rs new file mode 100644 index 000000000..4e3c80bb1 --- /dev/null +++ b/kube-core/src/schema/transform_optional_enum_with_null.rs @@ -0,0 +1,101 @@ +use schemars::transform::Transform; +use serde_json::Value; + +use super::SchemaObject; + +/// Drop trailing null on optional enums. +/// +/// The nullability is already indicated when "nullable" is set to true. +/// +/// NOTE: The trailing null is removed because it's not needed by Kubernetes +/// and makes the CRD more compact by removing redundant information. +#[derive(Debug, Clone)] +pub struct OptionalEnumSchemaRewriter; + +impl Transform for OptionalEnumSchemaRewriter { + fn transform(&mut self, transform_schema: &mut schemars::Schema) { + // Apply this (self) transform to all subschemas + schemars::transform::transform_subschemas(self, transform_schema); + + 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, + }; + + remove_optional_enum_null_variant(&mut schema); + + // 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; + } + } + } +} + +fn remove_optional_enum_null_variant(kube_schema: &mut SchemaObject) { + let SchemaObject { + enum_values: Some(enum_values), + extensions, + .. + } = kube_schema + else { + return; + }; + + // For added safety, check nullability. It should always be true when there is a null variant. + if let Some(Value::Bool(true)) = extensions.get("nullable") { + // Don't drop the null entry if it is the only thing in the enum. + // This is because other hoisting code depends on `kube::core::NULL_SCHEMA` to detect null + // variants. + if enum_values.len() > 1 { + enum_values.retain(|enum_value| enum_value != &Value::Null); + } + } +} + +#[cfg(test)] +mod tests { + use assert_json_diff::assert_json_eq; + + use super::*; + + #[test] + fn optional_enum_with_null() { + let original_schema_object_value = serde_json::json!({ + "description": "A very simple enum with unit variants without descriptions", + "enum": [ + "A", + "B", + "C", + "D", + null + ], + "nullable": true + }); + + let expected_converted_schema_object_value = serde_json::json!({ + "description": "A very simple enum with unit variants without descriptions", + "enum": [ + "A", + "B", + "C", + "D" + ], + "nullable": true + }); + + 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(); + remove_optional_enum_null_variant(&mut actual_converted_schema_object); + + assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); + } +} diff --git a/kube-core/src/schema/transform_properties.rs b/kube-core/src/schema/transform_properties.rs new file mode 100644 index 000000000..b720a0302 --- /dev/null +++ b/kube-core/src/schema/transform_properties.rs @@ -0,0 +1,806 @@ +use schemars::transform::Transform; + +use crate::schema::{InstanceType, Metadata, Schema, SchemaObject, SingleOrVec, NULL_SCHEMA}; + +/// 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: Doc-comments are not preserved for untagged enums. +/// +/// This will return early without modifications unless: +/// - 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. +#[derive(Debug, Clone)] +pub struct PropertiesSchemaRewriter; + +impl Transform for PropertiesSchemaRewriter { + fn transform(&mut self, transform_schema: &mut schemars::Schema) { + // Apply this (self) transform to all subschemas + schemars::transform::transform_subschemas(self, transform_schema); + + 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, + }; + + hoist_properties_for_any_of_subschemas(&mut schema); + + // 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; + } + } + } +} + +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; + }; + + // 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 subschemas.is_empty() { + return; + } + + // Ensure we aren't looking at the subschema with a null, as that is hoisted by another + // transformer. + if subschemas.len() == 2 { + // Return if there is a null entry + if subschemas + .iter() + .map(|schema| serde_json::to_value(schema).expect("schema should be able to convert to JSON")) + .any(|value| value == *NULL_SCHEMA) + { + return; + } + } + + // At this point, we can be reasonably sure we need to operate on the schema. + // TODO (@NickLarsenNZ): Return errors instead of panicking, leave panicking up to the + // infallible schemars::Transform + + let subschemas = subschemas + .iter_mut() + .map(|schema| match schema { + Schema::Object(schema_object) => schema_object, + Schema::Bool(_) => panic!("oneOf and anyOf variants cannot be of type boolean"), + }) + .collect::>(); + + for subschema in subschemas { + // Drop the "type" field on subschema. It needs to be set to "object" on the schema. + subschema.instance_type.take(); + kube_schema.instance_type = Some(SingleOrVec::Single(Box::new(InstanceType::Object))); + + // Take the description (which will be preserved for tagged enums further down). + // This (along with the dropping of the "type" above) will allow for empty variants ({}). + let subschema_metadata = subschema.metadata.take(); + + 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 (oneOf), we need to preserve the variant description + if preserve_description { + if let Some(Schema::Object(property_schema)) = object.properties.values_mut().next() { + if let Some(Metadata { + description: Some(_), .. + }) = subschema_metadata.as_deref() + { + property_schema.metadata = subschema_metadata + } + }; + } + + // If subschema 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 but with different types or doc-comments. + // The developer needs to make them the same. + while let Some((property_name, Schema::Object(property_schema_object))) = + object.properties.pop_first() + { + let parent_object = parent_object + // get the `ObjectValidation`, or an empty one without any properties set + .get_or_insert_default(); + + // 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.properties.get(&property_name) { + // 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 + .properties + .insert(property_name.clone(), Schema::Object(property_schema_object)); + } + } + } + } +} + +#[cfg(test)] +mod tests { + use assert_json_diff::assert_json_eq; + + use super::*; + + #[test] + fn tagged_enum_with_stuct_and_tuple_variants_before_one_of_hoisting() { + let original_schema_object_value = serde_json::json!({ + "description": "A complex tagged enum with unit and struct variants", + "oneOf": [ + { + "additionalProperties": false, + "description": "Override documentation on the Normal variant", + "properties": { + "Normal": { + "description": "A very simple enum with unit variants", + "oneOf": [ + { + "enum": [ + "C", + "D" + ], + "type": "string" + }, + { + "description": "First variant", + "enum": [ + "A" + ], + "type": "string" + }, + { + "description": "Second variant", + "enum": [ + "B" + ], + "type": "string" + } + ] + } + }, + "required": [ + "Normal" + ], + "type": "object" + }, + { + "additionalProperties": false, + "description": "Documentation on the Hardcore variant", + "properties": { + "Hardcore": { + "properties": { + "core": { + "description": "A very simple enum with unit variants", + "oneOf": [ + { + "enum": [ + "C", + "D" + ], + "type": "string" + }, + { + "description": "First variant", + "enum": [ + "A" + ], + "type": "string" + }, + { + "description": "Second variant", + "enum": [ + "B" + ], + "type": "string" + } + ] + }, + "hard": { + "type": "string" + } + }, + "required": [ + "hard", + "core" + ], + "type": "object" + } + }, + "required": [ + "Hardcore" + ], + "type": "object" + } + ] + }); + + let expected_converted_schema_object_value = serde_json::json!( + { + "description": "A complex tagged enum with unit and struct variants", + "oneOf": [ + { + "required": [ + "Normal" + ] + }, + { + "required": [ + "Hardcore" + ] + } + ], + "properties": { + "Hardcore": { + "description": "Documentation on the Hardcore variant", + "properties": { + "core": { + "description": "A very simple enum with unit variants", + "oneOf": [ + { + "enum": [ + "C", + "D" + ], + "type": "string" + }, + { + "description": "First variant", + "enum": [ + "A" + ], + "type": "string" + }, + { + "description": "Second variant", + "enum": [ + "B" + ], + "type": "string" + } + ] + }, + "hard": { + "type": "string" + } + }, + "required": [ + "core", + "hard" + ], + "type": "object" + }, + "Normal": { + "description": "Override documentation on the Normal variant", + "oneOf": [ + { + "enum": [ + "C", + "D" + ], + "type": "string" + }, + { + "description": "First variant", + "enum": [ + "A" + ], + "type": "string" + }, + { + "description": "Second variant", + "enum": [ + "B" + ], + "type": "string" + } + ] + } + }, + "type": "object" + } + ); + + 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_properties_for_any_of_subschemas(&mut actual_converted_schema_object); + + assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); + } + + #[test] + fn tagged_enum_with_stuct_and_tuple_variants_after_one_of_hoisting() { + let original_schema_object_value = serde_json::json!({ + "description": "A complex tagged enum with unit and struct variants", + "oneOf": [ + { + "additionalProperties": false, + "description": "Override documentation on the Normal variant", + "properties": { + "Normal": { + "description": "A very simple enum with unit variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + } + }, + "required": [ + "Normal" + ], + "type": "object" + }, + { + "additionalProperties": false, + "description": "Documentation on the Hardcore variant", + "properties": { + "Hardcore": { + "properties": { + "core": { + "description": "A very simple enum with unit variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + }, + "hard": { + "type": "string" + } + }, + "required": [ + "hard", + "core" + ], + "type": "object" + } + }, + "required": [ + "Hardcore" + ], + "type": "object" + } + ] + }); + + let expected_converted_schema_object_value = serde_json::json!( + { + "description": "A complex tagged enum with unit and struct variants", + "oneOf": [ + { + "required": [ + "Normal" + ] + }, + { + "required": [ + "Hardcore" + ] + } + ], + "properties": { + "Hardcore": { + "description": "Documentation on the Hardcore variant", + "properties": { + "core": { + "description": "A very simple enum with unit variants", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + }, + "hard": { + "type": "string" + } + }, + "required": [ + "core", + "hard" + ], + "type": "object" + }, + "Normal": { + "description": "Override documentation on the Normal variant", + "type": "string", + "enum": [ + "C", + "D", + "A", + "B" + ] + } + }, + "type": "object" + } + ); + + 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_properties_for_any_of_subschemas(&mut actual_converted_schema_object); + + assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); + } + + #[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 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" + ] + } + ] + } + } + }, + { + "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 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 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_properties_for_any_of_subschemas(&mut actual_converted_schema_object); + + assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); + } + + #[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_properties_for_any_of_subschemas(&mut actual_converted_schema_object); + + assert_json_eq!(actual_converted_schema_object, expected_converted_schema_object); + } + + #[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 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" + ] + } + ] + } + } + }, + { + "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); + } + + #[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 unit 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); + } +} diff --git a/kube-derive/Cargo.toml b/kube-derive/Cargo.toml index 384884b38..faddd9fe9 100644 --- a/kube-derive/Cargo.toml +++ b/kube-derive/Cargo.toml @@ -36,3 +36,4 @@ trybuild.workspace = true assert-json-diff.workspace = true runtime-macros.workspace = true prettyplease.workspace = true +tokio = { workspace = true, features = ["rt", "macros"] } diff --git a/kube-derive/src/custom_resource.rs b/kube-derive/src/custom_resource.rs index 977ca712b..90c156531 100644 --- a/kube-derive/src/custom_resource.rs +++ b/kube-derive/src/custom_resource.rs @@ -676,6 +676,10 @@ pub(crate) fn derive(input: proc_macro2::TokenStream) -> proc_macro2::TokenStrea s.meta_schema = None; }) .with_transform(#schemars::transform::AddNullable::default()) + .with_transform(#kube_core::schema::OneOfSchemaRewriter) + .with_transform(#kube_core::schema::AnyOfSchemaRewriter) + .with_transform(#kube_core::schema::PropertiesSchemaRewriter) + .with_transform(#kube_core::schema::OptionalEnumSchemaRewriter) .with_transform(#kube_core::schema::StructuralSchemaRewriter) .into_generator(); let schema = generate.into_root_schema_for::(); @@ -976,15 +980,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..8914d10a8 --- /dev/null +++ b/kube-derive/tests/crd_complex_enum_tests.rs @@ -0,0 +1,924 @@ +#![allow(missing_docs)] +use std::time::Duration; + +use assert_json_diff::assert_json_eq; +use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::{ + CustomResourceConversion, CustomResourceDefinition, +}; +use kube::{ + api::{DeleteParams, PostParams}, + Api, Client, CustomResource, CustomResourceExt, ResourceExt, +}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use serde_json::json; + +// Enum definitions + +/// A very simple enum with unit 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)] +pub enum NormalEnumWithoutDescriptions { + A, + B, + C, + D, +} + +/// A complex enum with tuple and struct variants +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] +enum ComplexEnum { + /// Override documentation on the Normal variant + Normal(NormalEnum), + + /// Documentation on the Hardcore variant + Hardcore { + hard: String, + core: NormalEnum, + without_description: NormalEnumWithoutDescriptions, + }, +} + +/// 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 type [`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, +} + +// CRD definitions + +#[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 = "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 { + 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 { + 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, +} + +#[tokio::test] +#[ignore = "needs apiserver to validate CRDs"] +async fn check_are_valid_crds() { + let crds = [ + NormalEnumTest::crd(), + OptionalEnumTest::crd(), + NormalEnumWithoutDescriptionsTest::crd(), + OptionalEnumWithoutDescriptionsTest::crd(), + ComplexEnumTest::crd(), + OptionalComplexEnumTest::crd(), + UntaggedEnumTest::crd(), + OptionalUntaggedEnumTest::crd(), + FlattenedUntaggedEnumTest::crd(), + ]; + + let client = Client::try_default() + .await + .expect("failed to create Kubernetes client"); + let crd_api: Api = Api::all(client); + for crd in crds { + // Clean up existing CRDs. As these are only test CRDs and this test is not run by default + // this is fine. + let _ = crd_api.delete(&crd.name_any(), &DeleteParams::default()).await; + + // Prevent "object is being deleted: customresourcedefinition already exists + tokio::time::sleep(Duration::from_millis(100)).await; + + crd_api + .create(&PostParams::default(), &crd) + .await + .expect("failed to create CRD"); + } +} + + +#[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 unit variants", + "enum": [ + "C", + "D", + "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": { + "description": "A very simple enum with unit variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "nullable": true, + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + + +#[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 should be *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!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "complexenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "ComplexEnumTest", + "plural": "complexenumtests", + "shortNames": [], + "singular": "complexenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for ComplexEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "description": "A complex enum with tuple and struct variants", + "oneOf": [ + { + "required": [ + "Normal" + ] + }, + { + "required": [ + "Hardcore" + ] + } + ], + "properties": { + "Hardcore": { + "description": "Documentation on the Hardcore variant", + "properties": { + "core": { + "description": "A very simple enum with unit variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + }, + "hard": { + "type": "string" + }, + "without_description": { + "enum": [ + "A", + "B", + "C", + "D" + ], + "type": "string" + } + }, + "required": [ + "core", + "hard", + "without_description" + ], + "type": "object" + }, + "Normal": { + "description": "Override documentation on the Normal variant", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "required": [ + "foo" + ], + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "ComplexEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + + +#[test] +fn optional_complex_enum() { + assert_json_eq!( + OptionalComplexEnumTest::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "optionalcomplexenumtests.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "OptionalComplexEnumTest", + "plural": "optionalcomplexenumtests", + "shortNames": [], + "singular": "optionalcomplexenumtest" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for OptionalComplexEnumTestSpec via `CustomResource`", + "properties": { + "spec": { + "properties": { + "foo": { + "description": "A complex enum with tuple and struct variants", + "nullable": true, + "oneOf": [ + { + "required": [ + "Normal" + ] + }, + { + "required": [ + "Hardcore" + ] + } + ], + "properties": { + "Hardcore": { + "description": "Documentation on the Hardcore variant", + "properties": { + "core": { + "description": "A very simple enum with unit variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + }, + "hard": { + "type": "string" + }, + "without_description": { + "enum": [ + "A", + "B", + "C", + "D" + ], + "type": "string" + } + }, + "required": [ + "core", + "hard", + "without_description" + ], + "type": "object" + }, + "Normal": { + "description": "Override documentation on the Normal variant", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalComplexEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[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": "An untagged enum with a nested enum inside", + "properties": { + "one": { + "type": "string" + }, + "two": { + "description": "A very simple enum with unit 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": {} + } + ] + } + } + ) + ); +} + +#[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": [ + { + "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 unit variants", + "enum": [ + "C", + "D", + "A", + "B" + ], + "type": "string" + } + }, + "type": "object" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "OptionalUntaggedEnumTest", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[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": "Put a [`UntaggedEnum`] behind `#[serde(flatten)]`", + "properties": { + "one": { + "type": "string" + }, + "two": { + "description": "A very simple enum with unit 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": {} + } + ] + } + } + ) + ); +} 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..732f764c3 --- /dev/null +++ b/kube-derive/tests/crd_mixed_enum_test.rs @@ -0,0 +1,396 @@ +#![allow(missing_docs)] +use std::time::Duration; + +use assert_json_diff::assert_json_eq; +use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition; +use kube::{ + api::{DeleteParams, PostParams}, + Api, Client, CustomResource, CustomResourceExt, ResourceExt, +}; +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 }, +} + +/// This enum is invalid, as the types of the `inner` fields differ. +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "InvalidEnum5")] +#[serde(untagged)] +enum InvalidEnum5Spec { + A { inner: String }, + B { inner: u32 }, +} + +/// This enum is valid, as the `inner` fields are the same. +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "ValidEnum6")] +#[serde(untagged)] +enum ValidEnum6Spec { + A { + /// The inner fields need to have the same schema (so also same description) + inner: u32, + /// An additional field + additional: String, + }, + B { + /// The inner fields need to have the same schema (so also same description) + inner: u32, + }, +} + +/// This enum is invalid, as the typed of `inner` fields are the same, *but* the description (which +/// is part of the schema differs). +#[derive(CustomResource, Serialize, Deserialize, Debug, Clone, JsonSchema)] +#[kube(group = "clux.dev", version = "v1", kind = "InvalidEnum7")] +#[serde(untagged)] +enum InvalidEnum7Spec { + A { + /// The inner fields need to have the same schema (so also same description) + inner: u32, + additional: String, + }, + B { + /// This description differs! + inner: u32, + }, +} + +#[tokio::test] +#[ignore = "needs apiserver to validate CRDs"] +async fn check_are_valid_crds() { + let crds = [ValidEnum3::crd(), ValidEnum4::crd(), ValidEnum6::crd()]; + + let client = Client::try_default() + .await + .expect("failed to create Kubernetes client"); + let crd_api: Api = Api::all(client); + for crd in crds { + // Clean up existing CRDs. As these are only test CRDs and this test is not run by default + // this is fine. + let _ = crd_api.delete(&crd.name_any(), &DeleteParams::default()).await; + + // Prevent "object is being deleted: customresourcedefinition already exists + tokio::time::sleep(Duration::from_millis(100)).await; + + crd_api + .create(&PostParams::default(), &crd) + .await + .expect("failed to create CRD"); + } +} + +/// 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 = "oneOf variants must all have the same type"] +fn invalid_enum_1() { + InvalidEnum1::crd(); +} + +#[test] +#[should_panic = "oneOf variants must all have the same type"] +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 = "Properties for \"inner\" are defined multiple times with different shapes"] +fn invalid_enum_5() { + InvalidEnum5::crd(); +} + +#[test] +fn valid_enum_6() { + assert_json_eq!( + ValidEnum6::crd(), + json!( + { + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "validenum6s.clux.dev" + }, + "spec": { + "group": "clux.dev", + "names": { + "categories": [], + "kind": "ValidEnum6", + "plural": "validenum6s", + "shortNames": [], + "singular": "validenum6" + }, + "scope": "Cluster", + "versions": [ + { + "additionalPrinterColumns": [], + "name": "v1", + "schema": { + "openAPIV3Schema": { + "description": "Auto-generated derived type for ValidEnum6Spec via `CustomResource`", + "properties": { + "spec": { + "anyOf": [ + { + "required": [ + "additional", + "inner" + ] + }, + { + "required": [ + "inner" + ] + } + ], + "description": "This enum is valid, as the `inner` fields are the same.", + "properties": { + "additional": { + "description": "An additional field", + "type": "string" + }, + "inner": { + "description": "The inner fields need to have the same schema (so also same description)", + "format": "uint32", + "minimum": 0.0, + "type": "integer" + } + }, + "type": "object" + } + }, + "required": [ + "spec" + ], + "title": "ValidEnum6", + "type": "object" + } + }, + "served": true, + "storage": true, + "subresources": {} + } + ] + } + } + ) + ); +} + +#[test] +#[should_panic = "Properties for \"inner\" are defined multiple times with different shapes"] +fn invalid_enum_7() { + InvalidEnum7::crd(); +} + +#[test] +#[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")] + struct FooSpec { + foo: InvalidEnum1, + } + + Foo::crd(); +} diff --git a/kube-derive/tests/crd_schema_test.rs b/kube-derive/tests/crd_schema_test.rs index c00009ff6..6a7670484 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": { @@ -290,7 +293,6 @@ fn test_crd_schema_matches_expected() { "default": "default_value", "type": "string" }, - "nullableSkipped": { "nullable": true, "type": "string" @@ -299,6 +301,10 @@ fn test_crd_schema_matches_expected() { "nullable": true, "type": "string" }, + "nullnullnullable": { + "nullable": true, + "type": "string" + }, "nullableSkippedWithDefault": { "default": "default_nullable", "nullable": true, 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