Arnavion / k8s-openapi

Rust definitions of the resource types in the Kubernetes client API
Apache License 2.0
379 stars 41 forks source link

Interop with schemars crate - impl schemars::JsonSchema for resource types #86

Closed nikhiljha closed 3 years ago

nikhiljha commented 3 years ago

Sometimes it's helpful to include other structs inside a CRD spec, like this example...

#[derive(CustomResource, Debug, Clone, Deserialize, Serialize, JsonSchema)]
#[kube(group = "example.com", version = "v1", kind = "Example")]
#[kube(shortname = "ex", namespaced)]
pub struct ExampleSpec {
    pub pvcspec: Option<PersistentVolumeClaimSpec>,
}

Unfortunately, PersistentVolumeClaimSpec doesn't derive JsonSchema, so this doesn't work if I want to automatically generate the CRD object in Rust. The cleanest way around this is probably to derive JsonSchema on all generated structs in this library.

Arnavion commented 3 years ago

I'm not sure what JsonSpec and JsonSchema are, but they're not macros in this crate. You may have meant to file on https://github.com/clux/kube-rs

nikhiljha commented 3 years ago

JsonSchema is from schemars. It's a derive macro that allows you to generate a Json Schema (like you need for a Kubernetes CRD), so it would be helpful to include that derive in this library.

I happen to be using kube-rs, but this is a change that would need to happen here

Arnavion commented 3 years ago

The schema generated by schemars::JsonSchema would be wrong. The upstream schema that the types were originally created from is more correct.

Arnavion commented 3 years ago

So here are two ways we can go forward with this:

  1. k8s-openapi attaches a serde_json::Value to every resource type that contains the original spec. You (the user) can then construct a schemars::schema::Schema from such a blob via its serde::Deserialize impl. You'll need to ask the schemars maintainer to add a way to consume serde_json::Value specs for "foreign types" in their proc macro. Eg

    // k8s-openapi
    struct PodSpec { ... }
    impl PodSpec { fn schema() -> serde_json::Value { ... } }
    // Your crate
    #[derive(schemars::JsonSchema)]
    struct MySpec {
        // Indicates schema is obtainable from `schemars::schema::Schema::deserialize(PodSpec::schema())`
        // instead of `<PodSpec as schemars::JsonSchema>::json_schema()`
        #[json_schema(foreign_schema = "schema")]
        inner: k8s_openapi::PodSpec,
    }
  2. k8s-openapi attaches a serde_json::Value to every resource type that contains the original spec, and implements the schemars::JsonSchema trait on the type. The problem with this is that the doc for JsonSchema::json_schema says:

    If the returned schema depends on any referenceable schemas, then this method will add them to the SchemaGenerator's schema definitions.

    ... but there is no way for an implementation from k8s-openapi to satisfy this, because every resource is isolated. So this method isn't possible.

MikailBag commented 3 years ago

The third option (very similar to 1, but probably faster to compile) is to attach schema as a string instead of serde_json::Value

Arnavion commented 3 years ago

Actually I'm not sure (1) (or MikailBag's (3) ) will work either for schemars's sake (though they'll be useful info to have for other use cases, of course). I can't imagine how a hypothetical #[json_schema(foreign_schema)] could satisfy the requirement of resolving $refs in the foreign schema either.

clux commented 3 years ago

I think the first option would at the very least work because you can impl JsonSchema yourself, see https://github.com/clux/kube-rs/pull/390 ) It's not as elegant as a proc-macro attr, but it at least works.

Resolving $refs sounds potentially more hairy. Is that something the spec produces?

Arnavion commented 3 years ago
    "io.k8s.api.core.v1.Pod": {
      "description": "Pod is a collection of containers that can run on a host. This resource is created by clients and scheduled onto hosts.",
      "properties": {
        "apiVersion": {
          "description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
          "type": "string"
        },
        "kind": {
          "description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
          "type": "string"
        },
        "metadata": {
          "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
          "description": "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata"
        },
        "spec": {
          "$ref": "#/definitions/io.k8s.api.core.v1.PodSpec",
          "description": "Specification of the desired behavior of the pod. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status"
        },
        "status": {
          "$ref": "#/definitions/io.k8s.api.core.v1.PodStatus",
          "description": "Most recently observed status of the pod. This data may not be up to date. Populated by the system. Read-only. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status"
        }
      },
      "type": "object",
      "x-kubernetes-group-version-kind": [
        {
          "group": "",
          "kind": "Pod",
          "version": "v1"
        }
      ]
    },

Notice the spec field references PodSpec by $ref (for obvious reasons).

And as I said, the contract of JsonSchema::json_schema requires that k8s_openapi::Pod's impl of that trait method cannot return a Schema::Object(SchemaObject { reference: Some(...) }) for the spec field. Instead it apparently must call schema_generator.subschema_for::<k8s_openapi::PodSpec>() (which in turn will delegate to <PodSpec as JsonSchema>::json_schema and so on).

kazk commented 3 years ago

How about this. In k8s_openapi, add a method schema that returns serde_json::Value based on the original spec like 1, but $ref replaced with "properties": T::schema()["properties"] and "type": "object". k8s_openapi doesn't need to depend on schemars.

For example, for Pod from above:

// api/core/v1/pod.rs
#[cfg(feature = "schema")]
impl Pod {
pub fn schema() -> serde_json::Value {
    serde_json::json!({
      "description": "...",
      "properties": {
        "apiVersion": {
          "description": "...",
          "type": "string"
        },
        "kind": {
          "description": "...",
          "type": "string"
        },
        "metadata": {
          // "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
          "properties": crate::apimachinery::pkg::apis::meta::v1::ObjectMeta::schema()["properties"],
          "type": "object",
          "description": "..."
        },
        "spec": {
          // "$ref": "#/definitions/io.k8s.api.core.v1.PodSpec",
          "properties": crate::api::core::v1::PodSpec::schema()["properties"],
          "type": "object",
          "description": "..."
        },
        "status": {
          // "$ref": "#/definitions/io.k8s.api.core.v1.PodStatus",
          "properties": crate::api::core::v1::PodStatus::schema()["properties"],
          "type": "object",
          "description": "..."
        }
      },
      "type": "object"
    })
}
}

This way, it can include the fields not present in the Rust struct like apiVersion and kind. Also, users don't need a wrapper:

#[derive(CustomResource, Debug, Clone, Deserialize, Serialize, JsonSchema)]
#[kube(group = "example.com", version = "v1", kind = "Example")]
#[kube(shortname = "ex", namespaced)]
pub struct ExampleSpec {
    // specify a function to generate schema for this field
    #[schemars(schema_with = "pod_schema")]
    pub pod: Pod,
}

fn pod_schema(_: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema {
    serde_json::from_value(Pod::schema()).unwrap()
}

schema_with can specify a custom function to use for a field.

kazk commented 3 years ago

I got https://github.com/Arnavion/k8s-openapi/issues/86#issuecomment-777912231 mostly working.

println!("{}", serde_json::to_string_pretty(&Pod::schema()).unwrap());

https://gist.github.com/kazk/e4a56b7ff9431416a1c29a1c602a7c8a

property: null is a bug. T::schema()["properties"] is null when $ref is not type: object, so it needs to be adjusted.

Also, these definitions don't have type (at least in v1.20):

    "io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaPropsOrArray": {
      "description": "JSONSchemaPropsOrArray represents a value that can either be a JSONSchemaProps or an array of JSONSchemaProps. Mainly here for serialization purposes."
    },
    "io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaPropsOrBool": {
      "description": "JSONSchemaPropsOrBool represents JSONSchemaProps or a boolean value. Defaults to true for the boolean property."
    },
    "io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.JSONSchemaPropsOrStringArray": {
      "description": "JSONSchemaPropsOrStringArray represents a JSONSchemaProps or a string array."
    },

#[cfg(feature = "schema")]
impl Pod {
    pub fn schema() -> serde_json::Value {
        serde_json::json!({
          "description": "Pod is a collection of containers that can run on a host. This resource is created by clients and scheduled onto hosts.",
          "x-kubernetes-group-version-kind": [
            {
              "group": "",
              "kind": "Pod",
              "version": "v1"
            }
          ],
          "properties": {
            "apiVersion": {
              "description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
              "type": "string"
            },
            "kind": {
              "description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
              "type": "string"
            },
            "metadata": {
              "description": "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
              "type": "object", "properties": crate::apimachinery::pkg::apis::meta::v1::ObjectMeta::schema()["properties"]
            },
            "spec": {
              "description": "Specification of the desired behavior of the pod. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status",
              "type": "object", "properties": crate::api::core::v1::PodSpec::schema()["properties"]
            },
            "status": {
              "description": "Most recently observed status of the pod. This data may not be up to date. Populated by the system. Read-only. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status",
              "type": "object", "properties": crate::api::core::v1::PodStatus::schema()["properties"]
            }
          },
          "required": [
            "metadata"
          ],
          "type": "object"
        })
    }
}
kazk commented 3 years ago

104 should be ready for review. I ended up replacing objects with $ref with T::schema() and overriding the description if specified.

Pod::schema:

#[cfg(feature = "schema")]
impl Pod {
    pub fn schema() -> serde_json::Value {
        serde_json::json!({
          "description": "Pod is a collection of containers that can run on a host. This resource is created by clients and scheduled onto hosts.",
          "x-kubernetes-group-version-kind": [
            {
              "group": "",
              "kind": "Pod",
              "version": "v1"
            }
          ],
          "properties": {
            "apiVersion": {
              "description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
              "type": "string"
            },
            "kind": {
              "description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
              "type": "string"
            },
            "metadata": crate::schema_ref_with_description(crate::apimachinery::pkg::apis::meta::v1::ObjectMeta::schema(), "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata"),
            "spec": crate::schema_ref_with_description(crate::api::core::v1::PodSpec::schema(), "Specification of the desired behavior of the pod. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status"),
            "status": crate::schema_ref_with_description(crate::api::core::v1::PodStatus::schema(), "Most recently observed status of the pod. This data may not be up to date. Populated by the system. Read-only. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status")
          },
          "required": [
            "metadata"
          ],
          "type": "object"
        })
    }
}

Output: https://gist.github.com/kazk/e4a56b7ff9431416a1c29a1c602a7c8a

See https://github.com/Arnavion/k8s-openapi/pull/104/commits/02c9a0fffbc611b8e994fd91b8f6bd731f5d77b6 for how it's done.

TODO