Serve OpenAPI spec for registered CRDs by roycaihw · Pull Request #67205 · kubernetes/kubernetes

roycaihw · 2018-08-09T17:40:54Z

What this PR does / why we need it:
Generate and serve the OpenAPI spec for registered CRDs

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #58603

Special notes for your reviewer:

Release note:

reverted in #71158, original release note was:

kube-apiserver now serves OpenAPI specs for registered CRDs with defined validation schemata. Kubectl will validate client-side using those.

NONE

/sig api-machinery

cc @mbohlool @sttts

roycaihw · 2018-08-13T19:49:45Z

/retest

roycaihw · 2018-08-14T00:21:20Z

/cc @mbohlool @sttts

This PR is ready for review. Please take a look :)

Please see the gist https://gist.github.com/roycaihw/1bb6eb7973b5496cb501ee50c8ee1ee3 for an example of constructed openapi spec for CRD:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: foos.samplecontroller.k8s.io
spec:
  group: samplecontroller.k8s.io
  version: v1alpha1
  names:
    kind: Foo
    plural: foos
  scope: Namespaced
  validation:
    openAPIV3Schema:
        description: test

k8s-ci-robot · 2018-11-15T23:22:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: roycaihw, smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~hack/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/apiextensions-apiserver/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/apiserver/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/kube-aggregator/OWNERS~~ [smarterclayton]
~~test/OWNERS~~ [smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sttts · 2018-11-16T07:14:06Z

/lgtm

smarterclayton · 2018-11-16T15:00:04Z

I wasn't able to see this from a casual scan of the code, but do we have a way to pass description for fields and model objects back from this? If not, we definitely need that in order to make kubectl explain be able to help users (which is one of the key consumers of openapi).

sttts · 2018-11-16T16:16:35Z

@smarterclayton the description of a scheme prop is copied into the published schema. So from the code it looks like this should work.

@roycaihw can you add an e2e test which calls kubectl explain against such a CRD?

sttts · 2018-11-16T18:00:04Z

@smarterclayton this shows up in the openapi spec (but kubectl explain does not do anything, should it?)

   "samplecontroller.k8s.io.v1alpha1.Foo": {
    "properties": {
     "spec": {
      "properties": {
       "replicas": {
        "description": "replicas description",
        "type": "integer",
        "maximum": 10,
        "minimum": 1,
        "externalDocs": {
         "description": "external replicas description",
         "url": "http://k8s.io"
        },
        "example": 42
       }
      }
     }
    }
   },

liggitt · 2018-11-16T18:36:32Z

looks like the individual schema item does not include a x-kubernetes-group-version-kind stanza

kubectl create -f staging/src/k8s.io/csi-api/pkg/crd/manifests/csinodeinfo.yaml

kubectl get --raw /openapi/v2 | grep '"csi.storage.k8s.io.v1alpha1.CSINodeInfo":' -A 200
   "csi.storage.k8s.io.v1alpha1.CSINodeInfo": {
    "properties": {
     "spec": {
      "description": "Specification of CSINodeInfo",
      "properties": {
       "drivers": {
        "description": "List of CSI drivers running on the node and their specs.",
        "type": "array",
        "items": {
         "properties": {
          "name": {
           "description": "The CSI driver that this object refers to.",
           "type": "string"
          },
          "nodeID": {
           "description": "The node from the driver point of view.",
           "type": "string"
          },
          "topologyKeys": {
           "description": "List of keys supported by the driver.",
           "type": "array",
           "items": {
            "type": "string"
           }
          }
         }
        }
       }
      }
     },
     "status": {
      "description": "Status of CSINodeInfo",
      "properties": {
       "drivers": {
        "description": "List of CSI drivers running on the node and their statuses.",
        "type": "array",
        "items": {
         "properties": {
          "available": {
           "description": "Whether the CSI driver is installed.",
           "type": "boolean"
          },
          "name": {
           "description": "The CSI driver that this object refers to.",
           "type": "string"
          },
          "volumePluginMechanism": {
           "description": "Indicates to external components the required mechanism to use for any in-tree plugins replaced by this driver.",
           "type": "string",
           "pattern": "in-tree|csi"
          }
         }
        }
       }
      }
     }
    }
   },
   "csi.storage.k8s.io.v1alpha1.CSINodeInfoList": {
    "description": "CSINodeInfoList is a list of CSINodeInfo",
    "required": [
     "items"
    ],
    "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/api-conventions.md#resources",
      "type": "string"
     },
     "items": {
      "description": "List of csinodeinfos. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md",
      "type": "array",
      "items": {
       "$ref": "#/definitions/csi.storage.k8s.io.v1alpha1.CSINodeInfo"
      }
     },
     "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/api-conventions.md#types-kinds",
      "type": "string"
     },
     "metadata": {
      "description": "Standard list metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds",
      "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ListMeta"
     }
    },
    "x-kubernetes-group-version-kind": {
     "group": "csi.storage.k8s.io",
     "kind": "CSINodeInfoList",
     "version": "v1alpha1"
    }
   },
...

liggitt · 2018-11-16T18:39:58Z

interestingly, it looks like the <CustomKind>List object schema does include the gvk

smarterclayton · 2018-11-16T18:59:25Z

The end use case is that a CRD is able to show something like the following for any CRD property: explain pods.spec.serviceAccountName KIND: Pod VERSION: v1 FIELD: serviceAccountName <string> DESCRIPTION: ServiceAccountName is the name of the ServiceAccount to use to run this pod. More info: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/

liggitt · 2018-11-16T19:13:07Z

this adds the GVK to the openapi spec:

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/openapi/construction.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/openapi/construction.go
index 804681da1f..0327e6eacb 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/openapi/construction.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/openapi/construction.go
@@ -96,6 +96,11 @@ func (c *SwaggerConstructor) ConstructCRDOpenAPISpec() *spec.Swagger {
        var schema spec.Schema
        if c.schema != nil {
                schema = *c.schema
+               schema.AddExtension("x-kubernetes-group-version-kind", map[string]string{
+                       "group":   c.group,
+                       "kind":    c.kind,
+                       "version": c.version,
+               })
        }

but explain is still confused:

kubectl explain csinodeinfos
error: Couldn't find resource for "csi.storage.k8s.io/v1alpha1, Kind=CSINodeInfo"

and kubectl validation doesn't pick up the schema and complain about unknown fields:

echo '{"kind":"CSINodeInfo","apiVersion":"csi.storage.k8s.io/v1alpha1","metadata":{"name":"foo"},"spec":{"foo":true}}' \
  | kubectl create -f -
csinodeinfo.csi.storage.k8s.io/foo created

roycaihw · 2018-11-16T20:56:54Z

@liggitt The format of x-kubernetes-group-version-kind in my code was wrong. It should be a list of map, instead of a single map. Fixing

roycaihw · 2018-11-16T21:16:09Z

kubectl explain and validation works now. Will send a PR with an e2e test

$ kubectl explain csinodeinfos
KIND:     CSINodeInfo
VERSION:  csi.storage.k8s.io/v1alpha1

DESCRIPTION:

FIELDS:
   spec	<Object>
     Specification of CSINodeInfo

   status	<Object>
     Status of CSINodeInfo

$ echo '{"kind":"CSINodeInfo","apiVersion":"csi.storage.k8s.io/v1alpha1","metadata":{"name":"foo"},"spec":{"foo":true}}' \
  | kubectl create -f -
error: error validating "STDIN": error validating data: [ValidationError(CSINodeInfo): unknown field "apiVersion" in csi.storage.k8s.io.v1alpha1.CSINodeInfo, ValidationError(CSINodeInfo): unknown field "kind" in csi.storage.k8s.io.v1alpha1.CSINodeInfo, ValidationError(CSINodeInfo): unknown field "metadata" in csi.storage.k8s.io.v1alpha1.CSINodeInfo, ValidationError(CSINodeInfo.spec): unknown field "foo" in csi.storage.k8s.io.v1alpha1.CSINodeInfo.spec]; if you choose to ignore these errors, turn validation off with --validate=false

liggitt · 2018-11-16T21:22:06Z

ValidationError(CSINodeInfo): unknown field "apiVersion" in csi.storage.k8s.io.v1alpha1.CSINodeInfo, ValidationError(CSINodeInfo): unknown field "kind" in csi.storage.k8s.io.v1alpha1.CSINodeInfo, ValidationError(CSINodeInfo): unknown field "metadata" in csi.storage.k8s.io.v1alpha1.CSINodeInfo, ValidationError(CSINodeInfo.spec): unknown field "foo" in csi.storage.k8s.io.v1alpha1.CSINodeInfo.spec]; if you choose to ignore these errors, turn validation off with --validate=false

that actually illustrates a problem... the published schema does not contain standard object fields kind/apiVersion/metadata

this was not caught by the tests because of the delay in publishing the schema and the missing group version kind.

at this point, I think it is too late to rush to fix these issues. I think we need to roll back this PR and queue it up for 1.14 (or file for an exception while the issues are worked on out of master)

…-spec" This reverts commit 54ee58b, reversing changes made to 9e2820e.

k8s-ci-robot requested review from CaoShuFeng and janetkuo August 9, 2018 17:40

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 9, 2018

roycaihw force-pushed the crd-openapi-spec branch 8 times, most recently from e3ada48 to e1e5526 Compare August 11, 2018 00:49

This was referenced Aug 11, 2018

Generate OpenAPI spec for registered CRDs #58603

Closed

Umbrella issue for CRDs moving to GA #58682

Closed

roycaihw force-pushed the crd-openapi-spec branch from e1e5526 to b384394 Compare August 12, 2018 22:37

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 12, 2018

roycaihw changed the title ~~[WIP] Generate OpenAPI spec for registered CRDs~~ Serve OpenAPI spec for registered CRDs Aug 12, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 12, 2018

roycaihw force-pushed the crd-openapi-spec branch 2 times, most recently from a5b4f86 to ef38608 Compare August 12, 2018 23:10

roycaihw force-pushed the crd-openapi-spec branch from 0a0e490 to af368cf Compare August 13, 2018 23:01

k8s-ci-robot requested review from mbohlool and sttts August 14, 2018 00:21

roycaihw force-pushed the crd-openapi-spec branch from 103ae5e to 45c0e83 Compare August 15, 2018 21:28

k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Nov 15, 2018

roycaihw force-pushed the crd-openapi-spec branch from de18cc7 to 12f796b Compare November 15, 2018 23:26

add basic e2e test and integration for CRD openapi

4625bf7

roycaihw force-pushed the crd-openapi-spec branch from 12f796b to 4625bf7 Compare November 16, 2018 00:40

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2018

k8s-ci-robot merged commit 54ee58b into kubernetes:master Nov 16, 2018

This was referenced Nov 16, 2018

Published CRD openapi does not inform kubectl explain or validation #71142

Closed

Published CRD openapi schema takes ~60 seconds to appear, increased test time significantly #71143

Closed

liggitt mentioned this pull request Nov 16, 2018

Revert openapi publish #71158

Merged

liggitt added a commit to liggitt/kubernetes that referenced this pull request Nov 16, 2018

Revert "Merge pull request kubernetes#67205 from roycaihw/crd-openapi…

8799eb4

…-spec" This reverts commit 54ee58b, reversing changes made to 9e2820e.

liggitt mentioned this pull request Nov 16, 2018

Published CRD openapi schema does not include kind, apiVersion, metadata fields #71159

Closed

mingfang mentioned this pull request Nov 18, 2018

Using kube-openapi + 'kubectl explain' on Custom Resources? kubernetes/kube-openapi#97

Closed

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Nov 18, 2018

roycaihw mentioned this pull request Nov 19, 2018

Publish CRD openapi #71192

Merged

3 tasks

itowlson mentioned this pull request Jan 23, 2019

Intellisense and validation for CRD instance YAML vscode-kubernetes-tools/vscode-kubernetes-tools#447

Closed

roycaihw mentioned this pull request Jul 11, 2019

sig-api-machinery: Add scale targets to CRDs to GA KEP kubernetes/enhancements#1015

Merged

Conversation

roycaihw commented Aug 9, 2018 • edited by liggitt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roycaihw commented Aug 13, 2018

Uh oh!

roycaihw commented Aug 14, 2018

Uh oh!

k8s-ci-robot commented Nov 15, 2018

Uh oh!

sttts commented Nov 16, 2018

Uh oh!

smarterclayton commented Nov 16, 2018

Uh oh!

sttts commented Nov 16, 2018

Uh oh!

sttts commented Nov 16, 2018

Uh oh!

liggitt commented Nov 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liggitt commented Nov 16, 2018

Uh oh!

smarterclayton commented Nov 16, 2018 via email

Uh oh!

liggitt commented Nov 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roycaihw commented Nov 16, 2018

Uh oh!

roycaihw commented Nov 16, 2018

Uh oh!

liggitt commented Nov 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

roycaihw commented Aug 9, 2018 •

edited by liggitt

Loading

liggitt commented Nov 16, 2018 •

edited

Loading

liggitt commented Nov 16, 2018 •

edited

Loading

liggitt commented Nov 16, 2018 •

edited

Loading