Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for untagged enums in CRDs #1028

Merged
merged 13 commits into from Oct 7, 2022
Merged

Add support for untagged enums in CRDs #1028

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
440f60b
Add support for untagged enums in CRDs
sbernauer Sep 23, 2022
ff57eb5
Merge branch 'main' into support-untagged-enums
sbernauer Sep 27, 2022
e1d5406
Refactor out to fn convert_to_structural
sbernauer Sep 27, 2022
8edddb9
Merge branch 'main' into support-untagged-enums
sbernauer Sep 27, 2022
2d530dc
Update kube-core/src/schema.rs
sbernauer Sep 28, 2022
5375c30
Update kube-core/src/schema.rs
sbernauer Sep 28, 2022
3e1c94b
Update kube-core/src/schema.rs
sbernauer Sep 28, 2022
1467dd4
Update kube-core/src/schema.rs
sbernauer Sep 28, 2022
7ddae51
Move allow_repeated_property to allow_if_equal
sbernauer Sep 28, 2022
211b156
trigger checks
sbernauer Sep 28, 2022
9840511
Don't mention enums in panic message
sbernauer Oct 3, 2022
7d6ee75
Remove allow_if_equal flag
sbernauer Oct 7, 2022
9ad4b36
Merge branch 'main' into support-untagged-enums
sbernauer Oct 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
154 changes: 94 additions & 60 deletions kube-core/src/schema.rs
View file
Open in desktop
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,24 @@ 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;
convert_to_structural(one_of, &mut schema.object, &mut schema.instance_type, false);
sbernauer marked this conversation as resolved.
Show resolved Hide resolved
}

// 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())
{
convert_to_structural(any_of, &mut schema.object, &mut schema.instance_type, true);
sbernauer marked this conversation as resolved.
Show resolved Hide resolved
}
// 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 +64,91 @@ impl Visitor for StructuralSchemaRewriter {
}
}

fn convert_to_structural(
sbernauer marked this conversation as resolved.
Show resolved Hide resolved
subschemas: &mut Vec<Schema>,
common_obj: &mut Option<Box<ObjectValidation>>,
instance_type: &mut Option<SingleOrVec<InstanceType>>,
allow_repeated_property: bool,
) {
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::Occupied(entry) => {
if !allow_repeated_property {
nightkr marked this conversation as resolved.
Show resolved Hide resolved
panic!(
"property {:?} is already defined in another enum variant",
entry.key()
)
} else if &property != entry.get() {
panic!("Property {:?} has the schema {:?} but was already defined as {:?} in another untagged enum variant. The schemas for a property used in multiple untagged enum variants must be identical",
sbernauer marked this conversation as resolved.
Show resolved Hide resolved
entry.key(),
&property,
entry.get());
}
}
Entry::Vacant(entry) => {
entry.insert(property);
}
}
}

// 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
View file
Open in desktop
@@ -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