Skip to content

Commit

Permalink
Merge pull request #1028 from sbernauer/support-untagged-enums
Browse files Browse the repository at this point in the history
Add support for untagged enums in CRDs
  • Loading branch information
nightkr committed Oct 7, 2022
2 parents a26e7f3 + 9ad4b36 commit 3033e0f
Show file tree
Hide file tree
Showing 2 changed files with 171 additions and 63 deletions.
152 changes: 92 additions & 60 deletions kube-core/src/schema.rs
Expand Up @@ -8,14 +8,15 @@ use std::collections::btree_map::Entry;
#[allow(unused_imports)] use schemars::gen::SchemaSettings;

use schemars::{
schema::{Metadata, ObjectValidation, Schema, SchemaObject},
schema::{InstanceType, Metadata, ObjectValidation, Schema, SchemaObject, SingleOrVec},
visit::Visitor,
};

/// schemars [`Visitor`] that rewrites a [`Schema`] to conform to Kubernetes' "structural schema" rules
///
/// The following two transformations are applied
/// * Rewrite enums from `oneOf` to `object`s with multiple variants ([schemars#84](https://github.com/GREsau/schemars/issues/84))
/// * Rewrite untagged enums from `anyOf` to `object`s with multiple variants ([kube#1028](https://github.com/kube-rs/kube/pull/1028))
/// * Rewrite `additionalProperties` from `#[serde(flatten)]` to `x-kubernetes-preserve-unknown-fields` ([kube#844](https://github.com/kube-rs/kube/issues/844))
///
/// This is used automatically by `kube::derive`'s `#[derive(CustomResource)]`,
Expand All @@ -31,72 +32,26 @@ pub struct StructuralSchemaRewriter;
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())
{
let common_obj = schema
.object
.get_or_insert_with(|| Box::new(ObjectValidation::default()));
for variant in one_of {
if let Schema::Object(SchemaObject {
instance_type: variant_type,
object: Some(variant_obj),
metadata: variant_metadata,
..
}) = variant
{
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::new(Metadata::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::Occupied(entry) => panic!(
"property {:?} is already defined in another enum variant",
entry.key()
),
Entry::Vacant(entry) => {
entry.insert(property);
}
}
}

// Kubernetes doesn't allow variants to set additionalProperties
variant_obj.additional_properties = None;
// Tagged enums are serialized using `one_of`
hoist_subschema_properties(one_of, &mut schema.object, &mut schema.instance_type);
}

// Try to merge metadata
match (&mut schema.instance_type, variant_type.take()) {
(_, 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 {:?}, conflicting with existing type {:?}",
variant_type, common_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);
}
// check for maps without with properties (i.e. flattnened maps)

// check for maps without with properties (i.e. flattened maps)
// and allow these to persist dynamically
if let Some(object) = &mut schema.object {
if !object.properties.is_empty()
Expand All @@ -111,10 +66,87 @@ impl Visitor for StructuralSchemaRewriter {
}
}

/// 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,
object: Some(variant_obj),
metadata: variant_metadata,
..
}) = variant
{
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::new(Metadata::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<I: Iterator>(mut i: I) -> Option<I::Item> {
let item = i.next()?;
if i.next().is_some() {
return None;
}
Some(item)
}

fn merge_metadata(
instance_type: &mut Option<SingleOrVec<InstanceType>>,
variant_type: Option<SingleOrVec<InstanceType>>,
) {
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 {:?}, conflicting with existing type {:?}",
variant_type, common_type
);
}
}
}
}
82 changes: 79 additions & 3 deletions kube-derive/tests/crd_schema_test.rs
@@ -1,3 +1,5 @@
#![recursion_limit = "256"]

use chrono::{DateTime, NaiveDateTime, Utc};
use kube_derive::CustomResource;
use schemars::JsonSchema;
Expand Down Expand Up @@ -37,8 +39,11 @@ struct FooSpec {
// Using feature `chrono`
timestamp: DateTime<Utc>,

/// This is a complex enum
/// This is a complex enum with a description
complex_enum: ComplexEnum,

/// This is a untagged enum with a description
untagged_enum_person: UntaggedEnumPerson,
}

fn default_value() -> String {
Expand Down Expand Up @@ -69,6 +74,40 @@ enum ComplexEnum {
VariantThree {},
}

#[derive(Serialize, Deserialize, Debug, PartialEq, Clone, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(untagged)]
enum UntaggedEnumPerson {
SexAndAge(SexAndAge),
SexAndDateOfBirth(SexAndDateOfBirth),
}

#[derive(Serialize, Deserialize, Debug, PartialEq, Clone, JsonSchema)]
#[serde(rename_all = "camelCase")]
struct SexAndAge {
/// Sex of the person
sex: Sex,
/// Age of the person in years
age: i32,
}

#[derive(Serialize, Deserialize, Debug, PartialEq, Clone, JsonSchema)]
#[serde(rename_all = "camelCase")]
struct SexAndDateOfBirth {
/// Sex of the person
sex: Sex,
/// Date of birth of the person as ISO 8601 date
date_of_birth: String,
}

#[derive(Serialize, Deserialize, Debug, PartialEq, Clone, JsonSchema)]
#[serde(rename_all = "PascalCase")]
enum Sex {
Female,
Male,
Other,
}

#[test]
fn test_crd_name() {
use kube::core::CustomResourceExt;
Expand All @@ -93,6 +132,10 @@ fn test_serialized_matches_expected() {
nullable_with_default: None,
timestamp: DateTime::from_utc(NaiveDateTime::from_timestamp(0, 0), Utc),
complex_enum: ComplexEnum::VariantOne { int: 23 },
untagged_enum_person: UntaggedEnumPerson::SexAndAge(SexAndAge {
age: 42,
sex: Sex::Male,
})
}))
.unwrap(),
serde_json::json!({
Expand All @@ -111,6 +154,10 @@ fn test_serialized_matches_expected() {
"variantOne": {
"int": 23
}
},
"untaggedEnumPerson": {
"age": 42,
"sex": "Male"
}
}
})
Expand Down Expand Up @@ -220,13 +267,42 @@ fn test_crd_schema_matches_expected() {
"required": ["variantThree"]
}
],
"description": "This is a complex enum"
"description": "This is a complex enum with a description"
},
"untaggedEnumPerson": {
"type": "object",
"properties": {
"age": {
"type": "integer",
"format": "int32",
"description": "Age of the person in years"
},
"dateOfBirth": {
"type": "string",
"description": "Date of birth of the person as ISO 8601 date"
},
"sex": {
"type": "string",
"enum": ["Female", "Male", "Other"],
"description": "Sex of the person"
}
},
"anyOf": [
{
"required": ["age", "sex"]
},
{
"required": ["dateOfBirth", "sex"]
}
],
"description": "This is a untagged enum with a description"
}
},
"required": [
"complexEnum",
"nonNullable",
"timestamp"
"timestamp",
"untaggedEnumPerson"
],
"type": "object"
}
Expand Down

0 comments on commit 3033e0f

Please sign in to comment.