Merge pull request #68576 from jennybuckley/openapi-optimize

staging/src/k8s.io/apiserver/pkg/endpoints/BUILD

@@ -86,11 +86,7 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/endpoints/metrics:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server/filters:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/openapi:go_default_library",
         "//vendor/github.com/emicklei/go-restful:go_default_library",
-        "//vendor/k8s.io/kube-openapi/pkg/builder:go_default_library",
-        "//vendor/k8s.io/kube-openapi/pkg/common:go_default_library",
-        "//vendor/k8s.io/kube-openapi/pkg/util:go_default_library",
         "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library",
     ],
 )

staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go

@@ -31,7 +31,7 @@ import (
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/endpoints/discovery"
 	"k8s.io/apiserver/pkg/registry/rest"
-	openapicommon "k8s.io/kube-openapi/pkg/common"
+	openapiproto "k8s.io/kube-openapi/pkg/util/proto"
 )
 
 // APIGroupVersion is a helper for exposing rest.Storage objects as http.Handlers via go-restful
@@ -85,8 +85,8 @@ type APIGroupVersion struct {
 	// if the client requests it via Accept-Encoding
 	EnableAPIResponseCompression bool
 
-	// OpenAPIConfig lets the individual handlers build a subset of the OpenAPI schema before they are installed.
-	OpenAPIConfig *openapicommon.Config
+	// OpenAPIModels exposes the OpenAPI models to each individual handler.
+	OpenAPIModels openapiproto.Models
 }
 
 // InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container.

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -60,7 +60,7 @@ type RequestScope struct {
 	Trace           *utiltrace.Trace
 
 	TableConvertor rest.TableConvertor
-	OpenAPISchema  openapiproto.Schema
+	OpenAPIModels  openapiproto.Models
 
 	Resource    schema.GroupVersionResource
 	Kind        schema.GroupVersionKind

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -39,10 +39,6 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/metrics"
 	"k8s.io/apiserver/pkg/registry/rest"
 	genericfilters "k8s.io/apiserver/pkg/server/filters"
-	utilopenapi "k8s.io/apiserver/pkg/util/openapi"
-	openapibuilder "k8s.io/kube-openapi/pkg/builder"
-	openapiutil "k8s.io/kube-openapi/pkg/util"
-	openapiproto "k8s.io/kube-openapi/pkg/util/proto"
 )
 
 const (
@@ -135,17 +131,17 @@ func (a *APIInstaller) newWebService() *restful.WebService {
 	return ws
 }
 
-// getResourceKind returns the external group version kind registered for the given storage
+// GetResourceKind returns the external group version kind registered for the given storage
 // object. If the storage object is a subresource and has an override supplied for it, it returns
 // the group version kind supplied in the override.
-func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schema.GroupVersionKind, error) {
+func GetResourceKind(groupVersion schema.GroupVersion, storage rest.Storage, typer runtime.ObjectTyper) (schema.GroupVersionKind, error) {
 	// Let the storage tell us exactly what GVK it has
 	if gvkProvider, ok := storage.(rest.GroupVersionKindProvider); ok {
-		return gvkProvider.GroupVersionKind(a.group.GroupVersion), nil
+		return gvkProvider.GroupVersionKind(groupVersion), nil
 	}
 
 	object := storage.New()
-	fqKinds, _, err := a.group.Typer.ObjectKinds(object)
+	fqKinds, _, err := typer.ObjectKinds(object)
 	if err != nil {
 		return schema.GroupVersionKind{}, err
 	}
@@ -154,13 +150,13 @@ func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schem
 	// we're trying to register here
 	fqKindToRegister := schema.GroupVersionKind{}
 	for _, fqKind := range fqKinds {
-		if fqKind.Group == a.group.GroupVersion.Group {
-			fqKindToRegister = a.group.GroupVersion.WithKind(fqKind.Kind)
+		if fqKind.Group == groupVersion.Group {
+			fqKindToRegister = groupVersion.WithKind(fqKind.Kind)
 			break
 		}
 	}
 	if fqKindToRegister.Empty() {
-		return schema.GroupVersionKind{}, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion)
+		return schema.GroupVersionKind{}, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, groupVersion)
 	}
 
 	// group is guaranteed to match based on the check above
@@ -180,7 +176,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		return nil, err
 	}
 
-	fqKindToRegister, err := a.getResourceKind(path, storage)
+	fqKindToRegister, err := GetResourceKind(a.group.GroupVersion, storage, a.group.Typer)
 	if err != nil {
 		return nil, err
 	}
@@ -513,10 +509,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	if a.group.MetaGroupVersion != nil {
 		reqScope.MetaGroupVersion = *a.group.MetaGroupVersion
 	}
-	reqScope.OpenAPISchema, err = a.getOpenAPISchema(ws.RootPath(), resource, fqKindToRegister, defaultVersionedObject)
-	if err != nil {
-		return nil, fmt.Errorf("unable to get openapi schema for %v: %v", fqKindToRegister, err)
-	}
+	reqScope.OpenAPIModels = a.group.OpenAPIModels
 	for _, action := range actions {
 		producedObject := storageMeta.ProducesObject(action.Verb)
 		if producedObject == nil {
@@ -558,7 +551,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 				return nil, fmt.Errorf("missing parent storage: %q", resource)
 			}
 
-			fqParentKind, err := a.getResourceKind(resource, parentStorage)
+			fqParentKind, err := GetResourceKind(a.group.GroupVersion, parentStorage, a.group.Typer)
 			if err != nil {
 				return nil, err
 			}
@@ -873,24 +866,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	return &apiResource, nil
 }
 
-// getOpenAPISchema builds the openapi schema for a single resource model to be given to each handler. It will
-// return nil if the apiserver doesn't have openapi enabled, or if the specific path should be ignored by openapi.
-func (a *APIInstaller) getOpenAPISchema(rootPath, resource string, kind schema.GroupVersionKind, sampleObject interface{}) (openapiproto.Schema, error) {
-	path := gpath.Join(rootPath, resource)
-	if a.group.OpenAPIConfig == nil {
-		return nil, nil
-	}
-	pathsToIgnore := openapiutil.NewTrie(a.group.OpenAPIConfig.IgnorePrefixes)
-	if pathsToIgnore.HasPrefix(path) {
-		return nil, nil
-	}
-	openAPIDefinitions, err := openapibuilder.BuildOpenAPIDefinitionsForResource(sampleObject, a.group.OpenAPIConfig)
-	if err != nil {
-		return nil, err
-	}
-	return utilopenapi.ToProtoSchema(openAPIDefinitions, kind)
-}
-
 // indirectArbitraryPointer returns *ptrToObject for an arbitrary pointer
 func indirectArbitraryPointer(ptrToObject interface{}) interface{} {
 	return reflect.Indirect(reflect.ValueOf(ptrToObject)).Interface()

staging/src/k8s.io/apiserver/pkg/server/BUILD

@@ -103,6 +103,7 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/server/storage:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/logs:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/openapi:go_default_library",
         "//staging/src/k8s.io/client-go/informers:go_default_library",
         "//staging/src/k8s.io/client-go/rest:go_default_library",
         "//staging/src/k8s.io/client-go/util/cert:go_default_library",
@@ -113,7 +114,10 @@ go_library(
         "//vendor/github.com/pborman/uuid:go_default_library",
         "//vendor/golang.org/x/net/http2:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
+        "//vendor/k8s.io/kube-openapi/pkg/builder:go_default_library",
         "//vendor/k8s.io/kube-openapi/pkg/common:go_default_library",
+        "//vendor/k8s.io/kube-openapi/pkg/util:go_default_library",
+        "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library",
     ],
 )
 

staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go

@@ -19,6 +19,7 @@ package server
 import (
 	"fmt"
 	"net/http"
+	gpath "path"
 	"strings"
 	"sync"
 	"time"
@@ -42,8 +43,12 @@ import (
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/server/healthz"
 	"k8s.io/apiserver/pkg/server/routes"
+	utilopenapi "k8s.io/apiserver/pkg/util/openapi"
 	restclient "k8s.io/client-go/rest"
+	openapibuilder "k8s.io/kube-openapi/pkg/builder"
 	openapicommon "k8s.io/kube-openapi/pkg/common"
+	openapiutil "k8s.io/kube-openapi/pkg/util"
+	openapiproto "k8s.io/kube-openapi/pkg/util/proto"
 )
 
 // Info about an API group.
@@ -320,6 +325,10 @@ func (s preparedGenericAPIServer) NonBlockingRun(stopCh <-chan struct{}) error {
 
 // installAPIResources is a private method for installing the REST storage backing each api groupversionresource
 func (s *GenericAPIServer) installAPIResources(apiPrefix string, apiGroupInfo *APIGroupInfo) error {
+	openAPIGroupModels, err := s.getOpenAPIModelsForGroup(apiPrefix, apiGroupInfo)
+	if err != nil {
+		return fmt.Errorf("unable to get openapi models for group %v: %v", apiPrefix, err)
+	}
 	for _, groupVersion := range apiGroupInfo.PrioritizedVersions {
 		if len(apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version]) == 0 {
 			klog.Warningf("Skipping API %v because it has no resources.", groupVersion)
@@ -330,6 +339,7 @@ func (s *GenericAPIServer) installAPIResources(apiPrefix string, apiGroupInfo *A
 		if apiGroupInfo.OptionsExternalVersion != nil {
 			apiGroupVersion.OptionsExternalVersion = apiGroupInfo.OptionsExternalVersion
 		}
+		apiGroupVersion.OpenAPIModels = openAPIGroupModels
 
 		if err := apiGroupVersion.InstallREST(s.Handler.GoRestfulContainer); err != nil {
 			return fmt.Errorf("unable to setup API %v: %v", apiGroupInfo, err)
@@ -427,7 +437,6 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV
 		Admit:                        s.admissionControl,
 		MinRequestTimeout:            s.minRequestTimeout,
 		EnableAPIResponseCompression: s.enableAPIResponseCompression,
-		OpenAPIConfig:                s.openAPIConfig,
 		Authorizer:                   s.Authorizer,
 	}
 }
@@ -445,3 +454,37 @@ func NewDefaultAPIGroupInfo(group string, scheme *runtime.Scheme, parameterCodec
 		NegotiatedSerializer:   codecs,
 	}
 }
+
+// getOpenAPIModelsForGroup is a private method for getting the OpenAPI Schemas for each  api group
+func (s *GenericAPIServer) getOpenAPIModelsForGroup(apiPrefix string, apiGroupInfo *APIGroupInfo) (openapiproto.Models, error) {
+	if s.openAPIConfig == nil {
+		return nil, nil
+	}
+	pathsToIgnore := openapiutil.NewTrie(s.openAPIConfig.IgnorePrefixes)
+	// Get the canonical names of every resource we need to build in this api group
+	resourceNames := make([]string, 0)
+	for _, groupVersion := range apiGroupInfo.PrioritizedVersions {
+		for resource, storage := range apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version] {
+			path := gpath.Join(apiPrefix, groupVersion.Group, groupVersion.Version, resource)
+			if !pathsToIgnore.HasPrefix(path) {
+				kind, err := genericapi.GetResourceKind(groupVersion, storage, apiGroupInfo.Scheme)
+				if err != nil {
+					return nil, err
+				}
+				sampleObject, err := apiGroupInfo.Scheme.New(kind)
+				if err != nil {
+					return nil, err
+				}
+				name := openapiutil.GetCanonicalTypeName(sampleObject)
+				resourceNames = append(resourceNames, name)
+			}
+		}
+	}
+
+	// Build the openapi definitions for those resources and convert it to proto models
+	openAPISpec, err := openapibuilder.BuildOpenAPIDefinitionsForResources(s.openAPIConfig, resourceNames...)
+	if err != nil {
+		return nil, err
+	}
+	return utilopenapi.ToProtoModels(openAPISpec)
+}

staging/src/k8s.io/apiserver/pkg/util/openapi/BUILD

@@ -7,7 +7,6 @@ go_library(
     importpath = "k8s.io/apiserver/pkg/util/openapi",
     visibility = ["//visibility:public"],
     deps = [
-        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//vendor/github.com/go-openapi/spec:go_default_library",
         "//vendor/github.com/googleapis/gnostic/OpenAPIv2:go_default_library",
         "//vendor/github.com/googleapis/gnostic/compiler:go_default_library",
@@ -35,7 +34,6 @@ go_test(
     srcs = ["proto_test.go"],
     embed = [":go_default_library"],
     deps = [
-        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//vendor/github.com/go-openapi/spec:go_default_library",
         "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library",
     ],

staging/src/k8s.io/apiserver/pkg/util/openapi/proto.go

@@ -18,29 +18,17 @@ package openapi
 
 import (
 	"encoding/json"
-	"fmt"
 
 	"github.com/go-openapi/spec"
 	openapi_v2 "github.com/googleapis/gnostic/OpenAPIv2"
 	"github.com/googleapis/gnostic/compiler"
 	yaml "gopkg.in/yaml.v2"
 
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/kube-openapi/pkg/util/proto"
 )
 
-const (
-	// groupVersionKindExtensionKey is the key used to lookup the
-	// GroupVersionKind value for an object definition from the
-	// definition's "extensions" map.
-	groupVersionKindExtensionKey = "x-kubernetes-group-version-kind"
-)
-
-// ToProtoSchema builds the proto formatted schema from an OpenAPI spec
-func ToProtoSchema(openAPIDefinitions *spec.Definitions, gvk schema.GroupVersionKind) (proto.Schema, error) {
-	openAPISpec := newMinimalValidOpenAPISpec()
-	openAPISpec.Definitions = *openAPIDefinitions
-
+// ToProtoModels builds the proto formatted models from OpenAPI spec
+func ToProtoModels(openAPISpec *spec.Swagger) (proto.Models, error) {
 	specBytes, err := json.MarshalIndent(openAPISpec, " ", " ")
 	if err != nil {
 		return nil, err
@@ -62,81 +50,5 @@ func ToProtoSchema(openAPIDefinitions *spec.Definitions, gvk schema.GroupVersion
 		return nil, err
 	}
 
-	for _, modelName := range models.ListModels() {
-		model := models.LookupModel(modelName)
-		if model == nil {
-			return nil, fmt.Errorf("the ListModels function returned a model that can't be looked-up")
-		}
-		gvkList := parseGroupVersionKind(model)
-		for _, modelGVK := range gvkList {
-			if modelGVK == gvk {
-				return model, nil
-			}
-		}
-	}
-
-	return nil, fmt.Errorf("no model found with a %v tag matching %v", groupVersionKindExtensionKey, gvk)
-}
-
-// newMinimalValidOpenAPISpec creates a minimal openapi spec with only the required fields filled in
-func newMinimalValidOpenAPISpec() *spec.Swagger {
-	return &spec.Swagger{
-		SwaggerProps: spec.SwaggerProps{
-			Swagger: "2.0",
-			Info: &spec.Info{
-				InfoProps: spec.InfoProps{
-					Title:   "Kubernetes",
-					Version: "0.0.0",
-				},
-			},
-		},
-	}
-}
-
-// parseGroupVersionKind gets and parses GroupVersionKind from the extension. Returns empty if it doesn't have one.
-func parseGroupVersionKind(s proto.Schema) []schema.GroupVersionKind {
-	extensions := s.GetExtensions()
-
-	gvkListResult := []schema.GroupVersionKind{}
-
-	// Get the extensions
-	gvkExtension, ok := extensions[groupVersionKindExtensionKey]
-	if !ok {
-		return []schema.GroupVersionKind{}
-	}
-
-	// gvk extension must be a list of at least 1 element.
-	gvkList, ok := gvkExtension.([]interface{})
-	if !ok {
-		return []schema.GroupVersionKind{}
-	}
-
-	for _, gvk := range gvkList {
-		// gvk extension list must be a map with group, version, and
-		// kind fields
-		gvkMap, ok := gvk.(map[interface{}]interface{})
-		if !ok {
-			continue
-		}
-		group, ok := gvkMap["group"].(string)
-		if !ok {
-			continue
-		}
-		version, ok := gvkMap["version"].(string)
-		if !ok {
-			continue
-		}
-		kind, ok := gvkMap["kind"].(string)
-		if !ok {
-			continue
-		}
-
-		gvkListResult = append(gvkListResult, schema.GroupVersionKind{
-			Group:   group,
-			Version: version,
-			Kind:    kind,
-		})
-	}
-
-	return gvkListResult
+	return models, nil
 }

staging/src/k8s.io/apiserver/pkg/util/openapi/proto_test.go

@@ -22,36 +22,41 @@ import (
 
 	"github.com/go-openapi/spec"
 
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/kube-openapi/pkg/util/proto"
 )
 
 // TestOpenAPIDefinitionsToProtoSchema tests the openapi parser
-func TestOpenAPIDefinitionsToProtoSchema(t *testing.T) {
-	openAPIDefinitions := &spec.Definitions{
-		"io.k8s.api.testgroup.v1.Foo": spec.Schema{
-			SchemaProps: spec.SchemaProps{
-				Description: "Description of Foos",
-				Properties:  map[string]spec.Schema{},
+func TestOpenAPIDefinitionsToProtoModels(t *testing.T) {
+	openAPISpec := &spec.Swagger{
+		SwaggerProps: spec.SwaggerProps{
+			Swagger: "2.0",
+			Info: &spec.Info{
+				InfoProps: spec.InfoProps{
+					Title:   "Kubernetes",
+					Version: "0.0.0",
+				},
 			},
-			VendorExtensible: spec.VendorExtensible{
-				Extensions: spec.Extensions{
-					"x-kubernetes-group-version-kind": []map[string]string{
-						{
-							"group":   "testgroup.k8s.io",
-							"version": "v1",
-							"kind":    "Foo",
+			Definitions: spec.Definitions{
+				"io.k8s.api.testgroup.v1.Foo": spec.Schema{
+					SchemaProps: spec.SchemaProps{
+						Description: "Description of Foos",
+						Properties:  map[string]spec.Schema{},
+					},
+					VendorExtensible: spec.VendorExtensible{
+						Extensions: spec.Extensions{
+							"x-kubernetes-group-version-kind": []map[string]string{
+								{
+									"group":   "testgroup.k8s.io",
+									"version": "v1",
+									"kind":    "Foo",
+								},
+							},
 						},
 					},
 				},
 			},
 		},
 	}
-	gvk := schema.GroupVersionKind{
-		Group:   "testgroup.k8s.io",
-		Version: "v1",
-		Kind:    "Foo",
-	}
 	expectedSchema := &proto.Arbitrary{
 		BaseSchema: proto.BaseSchema{
 			Description: "Description of Foos",
@@ -67,10 +72,11 @@ func TestOpenAPIDefinitionsToProtoSchema(t *testing.T) {
 			Path: proto.NewPath("io.k8s.api.testgroup.v1.Foo"),
 		},
 	}
-	actualSchema, err := ToProtoSchema(openAPIDefinitions, gvk)
+	protoModels, err := ToProtoModels(openAPISpec)
 	if err != nil {
-		t.Fatalf("expected ToProtoSchema not to return an error")
+		t.Fatalf("expected ToProtoModels not to return an error")
 	}
+	actualSchema := protoModels.LookupModel("io.k8s.api.testgroup.v1.Foo")
 	if !reflect.DeepEqual(expectedSchema, actualSchema) {
 		t.Fatalf("expected schema:\n%v\nbut got:\n%v", expectedSchema, actualSchema)
 	}