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.

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

Regression using schemars 0.8.11 with CRDs openAPIV3Schema that use enums #1047

Closed
jpmcb opened this issue Oct 10, 2022 · 1 comment · Fixed by #1051
Closed

Regression using schemars 0.8.11 with CRDs openAPIV3Schema that use enums #1047

jpmcb opened this issue Oct 10, 2022 · 1 comment · Fixed by #1051
Assignees
Labels
bug Something isn't working core generic apimachinery style work

Comments

@jpmcb
Copy link
Contributor

jpmcb commented Oct 10, 2022

Current and expected behavior

With a Cargo.toml file that looks like

[package]
name = "kube-enum-test"
version = "0.1.0"
edition = "2021"

[dependencies]
schemars = "0.8.10"
serde = { version = "1", features = [ "derive" ] }
serde_json = "1"
serde_yaml = "0.8"
k8s-openapi = { version = "0.16.0", default-features = false, features = ["v1_20"] }
kube = { version = "0.75.0", default-features = true, features = [ "derive", "runtime" ] }

(note the use of 0.8.10 for schemars)

And a small example rust program like so:

use kube::{CustomResource, CustomResourceExt};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use std::fs::File;
use std::path::PathBuf;

#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
pub enum RustStatusEnum {
    /// A status which indicates something
    SomeStatus,
    /// Some other status which indicates something else
    SomeOtherStatus,
}

impl Default for RustStatusEnum {
    fn default() -> Self {
        RustStatusEnum::SomeStatus
    }
}

#[derive(Clone, CustomResource, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)]
#[kube(
    derive = "Default",
    derive = "PartialEq",
    group = "mycrd.example.com",
    kind = "MyCrd",
    namespaced,
    plural = "mycrds",
    singular = "mycrd",
    status = "RustStatusEnum",
    version = "v1"
)]
pub struct MyCrdSpec {
    state: RustStatusEnum,
    version: Option<String>
}


fn main() {
    let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("mycrd.yaml");
    println!("{}", path.display());
    let f = File::create(&path).expect(&format!(
        "unable to open file '{}' for writing",
        path.display()
    ));

    serde_yaml::to_writer(&f, &MyCrd::crd()).expect("unable to write mycrd");
}

I am able to generate the follow valid yaml:

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: mycrds.mycrd.example.com
spec:
  group: mycrd.example.com
  names:
    categories: []
    kind: MyCrd
    plural: mycrds
    shortNames: []
    singular: mycrd
  scope: Namespaced
  versions:
    - additionalPrinterColumns: []
      name: v1
      schema:
        openAPIV3Schema:
          description: "Auto-generated derived type for MyCrdSpec via `CustomResource`"
          properties:
            spec:
              properties:
                state:
                  enum:
                    - SomeStatus
                    - SomeOtherStatus
                  type: string
                version:
                  nullable: true
                  type: string
              required:
                - state
              type: object
            status:
              enum:
                - SomeStatus
                - SomeOtherStatus
              nullable: true
              type: string
          required:
            - spec
          title: MyCrd
          type: object
      served: true
      storage: true
      subresources:
        status: {}

Applying this to a cluster works just fine:

❯ k apply -f mycrd.yaml
customresourcedefinition.apiextensions.k8s.io/mycrds.mycrd.example.com created

But, if I use the latest schemars version, with the following update to my Cargo.toml:

schemars = "0.8"
cargo update -p schemars --precise 0.8.11

And re-run my rust program, I get the following invalid yaml:

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: mycrds.mycrd.example.com
spec:
  group: mycrd.example.com
  names:
    categories: []
    kind: MyCrd
    plural: mycrds
    shortNames: []
    singular: mycrd
  scope: Namespaced
  versions:
    - additionalPrinterColumns: []
      name: v1
      schema:
        openAPIV3Schema:
          description: "Auto-generated derived type for MyCrdSpec via `CustomResource`"
          properties:
            spec:
              properties:
                state:
                  oneOf:
                    - description: A status which indicates something
                      enum:
                        - SomeStatus
                      type: string
                    - description: Some other status which indicates something else
                      enum:
                        - SomeOtherStatus
                      type: string
                version:
                  nullable: true
                  type: string
              required:
                - state
              type: object
            status:
              nullable: true
              oneOf:
                - description: A status which indicates something
                  enum:
                    - SomeStatus
                  type: string
                - description: Some other status which indicates something else
                  enum:
                    - SomeOtherStatus
                  type: string
          required:
            - spec
          title: MyCrd
          type: object
      served: true
      storage: true
      subresources:
        status: {}

Attempting to apply this yaml to a cluster gives schema structural errors:

❯ k apply -f mycrd.yaml
The CustomResourceDefinition "mycrds.mycrd.example.com" is invalid:
* spec.validation.openAPIV3Schema.properties[spec].properties[state].oneOf[0].description: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[state].oneOf[0].type: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[state].oneOf[1].description: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[state].oneOf[1].type: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[state].type: Required value: must not be empty for specified object fields
* spec.validation.openAPIV3Schema.properties[status].oneOf[0].description: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[status].oneOf[0].type: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[status].oneOf[1].description: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[status].oneOf[1].type: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[status].type: Required value: must not be empty for specified object fields

Possible solution

I've found an odd workaround: remove the comments (which remove the description on each of the enum types):

#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
pub enum RustStatusEnum {
    SomeStatus,
    SomeOtherStatus,
}

Which then, if I once again generate the yaml, I get the following, back to the original state:

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: mycrds.mycrd.example.com
spec:
  group: mycrd.example.com
  names:
    categories: []
    kind: MyCrd
    plural: mycrds
    shortNames: []
    singular: mycrd
  scope: Namespaced
  versions:
    - additionalPrinterColumns: []
      name: v1
      schema:
        openAPIV3Schema:
          description: "Auto-generated derived type for MyCrdSpec via `CustomResource`"
          properties:
            spec:
              properties:
                state:
                  enum:
                    - SomeStatus
                    - SomeOtherStatus
                  type: string
                version:
                  nullable: true
                  type: string
              required:
                - state
              type: object
            status:
              enum:
                - SomeStatus
                - SomeOtherStatus
              nullable: true
              type: string
          required:
            - spec
          title: MyCrd
          type: object
      served: true
      storage: true
      subresources:
        status: {}

Additional context

Honestly, I'm a bit confused what the interlinking between schemars and kube are, but it seems that the 0.8.11 release regressed this behavior, and specifically this PR (which made "Derived JsonSchema now respects attributes on unit enum variants): GREsau/schemars#152 - If I had to guess, schemars now respects those comments as descriptions which breaks the API spec.

Apologies if this is the wrong place for this issue, but since most of this behavior seems to be specific to kubernetes accepting a correct spec, this seemed like the right place. And it looks like there was previous discussion on this with a fix that may no longer be fully valid: #595 and #648

Environment

❯ k version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.2", GitCommit:"5835544ca568b757a8ecae5c153f317e5736700e", GitTreeState:"clean", BuildDate:"2022-09-21T14:33:49Z", GoVersion:"go1.19.1", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.2", GitCommit:"5835544ca568b757a8ecae5c153f317e5736700e", GitTreeState:"clean", BuildDate:"2022-09-22T05:25:21Z", GoVersion:"go1.19.1", Compiler:"gc", Platform:"linux/amd64"}

(this is just using the default kind cluster. I've seen the same behavior on different EKS versions as well)

Configuration and features

No response

Affected crates

No response

Would you like to work on fixing this bug?

No response

@nightkr
Copy link
Member

nightkr commented Oct 14, 2022

Honestly, I'm a bit confused what the interlinking between schemars and kube are

We leave most of the generation up to schemars, but try to post-process the schema to comply with K8s' structural CRD rules. This is definitely an area that should already be affected by our oneOf property hoisting, need to look into why that's not happening...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core generic apimachinery style work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants