Merge pull request #114792 from apelisse/typeconverter-swagger

Hide OpenAPI details behind the NewTypeConverter

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -71,13 +71,11 @@ import (
 	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericfilters "k8s.io/apiserver/pkg/server/filters"
-	utilopenapi "k8s.io/apiserver/pkg/util/openapi"
 	"k8s.io/apiserver/pkg/warning"
 	"k8s.io/client-go/scale"
 	"k8s.io/client-go/scale/scheme/autoscalingv1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
-	"k8s.io/kube-openapi/pkg/util/proto"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 	"k8s.io/kube-openapi/pkg/validation/strfmt"
 	"k8s.io/kube-openapi/pkg/validation/validate"
@@ -1371,7 +1369,7 @@ func hasServedCRDVersion(spec *apiextensionsv1.CustomResourceDefinitionSpec, ver
 // buildOpenAPIModelsForApply constructs openapi models from any validation schemas specified in the custom resource,
 // and merges it with the models defined in the static OpenAPI spec.
 // Returns nil models ifthe static spec is nil, or an error is encountered.
-func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensionsv1.CustomResourceDefinition) (proto.Models, error) {
+func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensionsv1.CustomResourceDefinition) (*spec.Swagger, error) {
 	if staticOpenAPISpec == nil {
 		return nil, nil
 	}
@@ -1391,9 +1389,5 @@ func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensi
 	if err != nil {
 		return nil, err
 	}
-	models, err := utilopenapi.ToProtoModels(mergedOpenAPI)
+	return mergedOpenAPI, nil
-	if err != nil {
-		return nil, err
-	}
-	return models, nil
 }

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

@@ -34,7 +34,7 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/storageversion"
-	openapiproto "k8s.io/kube-openapi/pkg/util/proto"
+	"k8s.io/kube-openapi/pkg/validation/spec"
 )
 
 // ConvertabilityChecker indicates what versions a GroupKind is available in.
@@ -96,7 +96,7 @@ type APIGroupVersion struct {
 	MinRequestTimeout time.Duration
 
 	// OpenAPIModels exposes the OpenAPI models to each individual handler.
-	OpenAPIModels openapiproto.Models
+	OpenAPIModels *spec.Swagger
 
 	// The limit on the request body size that would be accepted and decoded in a write request.
 	// 0 means no limit.

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanagertest/testfieldmanager.go

@@ -17,8 +17,10 @@ limitations under the License.
 package fieldmanagertest
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"path/filepath"
 	"strings"
 
@@ -28,39 +30,34 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
-	"k8s.io/kube-openapi/pkg/util/proto"
+	"k8s.io/kube-openapi/pkg/validation/spec"
-	prototesting "k8s.io/kube-openapi/pkg/util/proto/testing"
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 	"sigs.k8s.io/structured-merge-diff/v4/merge"
 	"sigs.k8s.io/structured-merge-diff/v4/typed"
 )
 
-var kubernetesSwaggerSchema = prototesting.Fake{
+var builtinConverter = func() fieldmanager.TypeConverter {
-	Path: filepath.Join(
+	data, err := ioutil.ReadFile(filepath.Join(
 		strings.Repeat(".."+string(filepath.Separator), 8),
-		"api", "openapi-spec", "swagger.json"),
+		"api", "openapi-spec", "swagger.json"))
+	if err != nil {
+		panic(err)
 	}
-
+	spec := spec.Swagger{}
-// NewBuiltinTypeConverter creates a TypeConverter with all the built-in
+	if err := json.Unmarshal(data, &spec); err != nil {
-// types defined, given the committed kubernetes swagger.json.
+		panic(err)
-func NewBuiltinTypeConverter() fieldmanager.TypeConverter {
+	}
-	tc, err := fieldmanager.NewTypeConverter(newFakeOpenAPIModels(), false)
+	tc, err := fieldmanager.NewTypeConverter(&spec, false)
 	if err != nil {
 		panic(fmt.Errorf("Failed to build TypeConverter: %v", err))
 	}
 	return tc
-}
+}()
 
-func newFakeOpenAPIModels() proto.Models {
+// NewBuiltinTypeConverter creates a TypeConverter with all the built-in
-	d, err := kubernetesSwaggerSchema.OpenAPISchema()
+// types defined, given the committed kubernetes swagger.json.
-	if err != nil {
+func NewBuiltinTypeConverter() fieldmanager.TypeConverter {
-		panic(err)
+	return builtinConverter
-	}
-	m, err := proto.NewOpenAPIData(d)
-	if err != nil {
-		panic(err)
-	}
-	return m
 }
 
 type fakeObjectConvertor struct {

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/typeconverter.go

@@ -23,7 +23,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/managedfields"
-	"k8s.io/kube-openapi/pkg/util/proto"
+	utilopenapi "k8s.io/apiserver/pkg/util/openapi"
+	"k8s.io/kube-openapi/pkg/validation/spec"
 	"sigs.k8s.io/structured-merge-diff/v4/typed"
 	"sigs.k8s.io/structured-merge-diff/v4/value"
 )
@@ -70,10 +71,14 @@ type typeConverter struct {
 
 var _ TypeConverter = &typeConverter{}
 
-// NewTypeConverter builds a TypeConverter from a proto.Models. This
+// NewTypeConverter builds a TypeConverter from a spec.Swagger. This
 // will automatically find the proper version of the object, and the
 // corresponding schema information.
-func NewTypeConverter(models proto.Models, preserveUnknownFields bool) (TypeConverter, error) {
+func NewTypeConverter(openapiSpec *spec.Swagger, preserveUnknownFields bool) (TypeConverter, error) {
+	models, err := utilopenapi.ToProtoModels(openapiSpec)
+	if err != nil {
+		return nil, err
+	}
 	parser, err := managedfields.NewGVKParser(models, preserveUnknownFields)
 	if err != nil {
 		return nil, err

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/typeconverter_test.go

@@ -17,7 +17,9 @@ limitations under the License.
 package fieldmanager_test
 
 import (
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"path/filepath"
 	"reflect"
 	"testing"
@@ -27,25 +29,23 @@ import (
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
-	"k8s.io/kube-openapi/pkg/util/proto"
+	"k8s.io/kube-openapi/pkg/validation/spec"
-	prototesting "k8s.io/kube-openapi/pkg/util/proto/testing"
 )
 
-var testSchema = prototesting.Fake{
+var testSchema = func() *spec.Swagger {
-	Path: filepath.Join("testdata", "swagger.json"),
+	data, err := ioutil.ReadFile(filepath.Join("testdata", "swagger.json"))
+	if err != nil {
+		panic(err)
 	}
+	spec := spec.Swagger{}
+	if err := json.Unmarshal(data, &spec); err != nil {
+		panic(err)
+	}
+	return &spec
+}()
 
 func TestTypeConverter(t *testing.T) {
-	d, err := testSchema.OpenAPISchema()
+	tc, err := fieldmanager.NewTypeConverter(testSchema, false)
-	if err != nil {
-		t.Fatalf("Failed to parse OpenAPI schema: %v", err)
-	}
-	m, err := proto.NewOpenAPIData(d)
-	if err != nil {
-		t.Fatalf("Failed to build OpenAPI models: %v", err)
-	}
-
-	tc, err := fieldmanager.NewTypeConverter(m, false)
 	if err != nil {
 		t.Fatalf("Failed to build TypeConverter: %v", err)
 	}
@@ -177,16 +177,7 @@ spec:
 		b.Fatalf("Failed to parse yaml object: %v", err)
 	}
 
-	d, err := testSchema.OpenAPISchema()
+	tc, err := fieldmanager.NewTypeConverter(testSchema, false)
-	if err != nil {
-		b.Fatalf("Failed to parse OpenAPI schema: %v", err)
-	}
-	m, err := proto.NewOpenAPIData(d)
-	if err != nil {
-		b.Fatalf("Failed to build OpenAPI models: %v", err)
-	}
-
-	tc, err := fieldmanager.NewTypeConverter(m, false)
 	if err != nil {
 		b.Fatalf("Failed to build TypeConverter: %v", err)
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/versionconverter_test.go

@@ -17,7 +17,9 @@ limitations under the License.
 package fieldmanager
 
 import (
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"path/filepath"
 	"reflect"
 	"testing"
@@ -25,26 +27,25 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/kube-openapi/pkg/util/proto"
+	"k8s.io/kube-openapi/pkg/validation/spec"
-	prototesting "k8s.io/kube-openapi/pkg/util/proto/testing"
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
-var testSchema = prototesting.Fake{
+var testSchema = func() *spec.Swagger {
-	Path: filepath.Join("testdata", "swagger.json"),
+	data, err := ioutil.ReadFile(filepath.Join("testdata", "swagger.json"))
+	if err != nil {
+		panic(err)
 	}
+	spec := spec.Swagger{}
+	if err := json.Unmarshal(data, &spec); err != nil {
+		panic(err)
+	}
+	return &spec
+}()
 
 // TestVersionConverter tests the version converter
 func TestVersionConverter(t *testing.T) {
-	d, err := testSchema.OpenAPISchema()
+	tc, err := NewTypeConverter(testSchema, false)
-	if err != nil {
-		t.Fatalf("Failed to parse OpenAPI schema: %v", err)
-	}
-	m, err := proto.NewOpenAPIData(d)
-	if err != nil {
-		t.Fatalf("Failed to build OpenAPI models: %v", err)
-	}
-	tc, err := NewTypeConverter(m, false)
 	if err != nil {
 		t.Fatalf("Failed to build TypeConverter: %v", err)
 	}

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

@@ -49,7 +49,6 @@ import (
 	"k8s.io/apiserver/pkg/server/routes"
 	"k8s.io/apiserver/pkg/storageversion"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	utilopenapi "k8s.io/apiserver/pkg/util/openapi"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 	openapibuilder2 "k8s.io/kube-openapi/pkg/builder"
@@ -57,7 +56,6 @@ import (
 	"k8s.io/kube-openapi/pkg/handler"
 	"k8s.io/kube-openapi/pkg/handler3"
 	openapiutil "k8s.io/kube-openapi/pkg/util"
-	openapiproto "k8s.io/kube-openapi/pkg/util/proto"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 	"k8s.io/utils/clock"
 )
@@ -666,7 +664,7 @@ func (s preparedGenericAPIServer) NonBlockingRun(stopCh <-chan struct{}, shutdow
 }
 
 // installAPIResources is a private method for installing the REST storage backing each api groupversionresource
-func (s *GenericAPIServer) installAPIResources(apiPrefix string, apiGroupInfo *APIGroupInfo, openAPIModels openapiproto.Models) error {
+func (s *GenericAPIServer) installAPIResources(apiPrefix string, apiGroupInfo *APIGroupInfo, openAPIModels *spec.Swagger) error {
 	var resourceInfos []*storageversion.ResourceInfo
 	for _, groupVersion := range apiGroupInfo.PrioritizedVersions {
 		if len(apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version]) == 0 {
@@ -881,7 +879,7 @@ func NewDefaultAPIGroupInfo(group string, scheme *runtime.Scheme, parameterCodec
 }
 
 // getOpenAPIModels is a private method for getting the OpenAPI models
-func (s *GenericAPIServer) getOpenAPIModels(apiPrefix string, apiGroupInfos ...*APIGroupInfo) (openapiproto.Models, error) {
+func (s *GenericAPIServer) getOpenAPIModels(apiPrefix string, apiGroupInfos ...*APIGroupInfo) (*spec.Swagger, error) {
 	if s.openAPIConfig == nil {
 		return nil, nil
 	}
@@ -903,7 +901,7 @@ func (s *GenericAPIServer) getOpenAPIModels(apiPrefix string, apiGroupInfos ...*
 	for _, apiGroupInfo := range apiGroupInfos {
 		apiGroupInfo.StaticOpenAPISpec = openAPISpec
 	}
-	return utilopenapi.ToProtoModels(openAPISpec)
+	return openAPISpec, nil
 }
 
 // getResourceNamesForGroup is a private method for getting the canonical names for each resource to build in an api group