Add tests and address comments

Author: Jefftree

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/getkin/kin-openapi v0.76.0
 	github.com/go-openapi/jsonreference v0.19.3
 	github.com/go-openapi/swag v0.19.5
+	github.com/go-test/deep v1.0.8
 	github.com/golang/protobuf v1.5.2
 	github.com/google/gnostic v0.5.7-v3refs
 	github.com/google/go-cmp v0.5.5

go.sum

@@ -32,6 +32,8 @@ github.com/go-openapi/jsonreference v0.19.3 h1:5cxNfTy0UVC3X8JL5ymxzyoUZmo8iZb+j
 github.com/go-openapi/jsonreference v0.19.3/go.mod h1:rjx6GuL8TTa9VaixXglHmQmIL98+wF9xc8zWvFonSJ8=
 github.com/go-openapi/swag v0.19.5 h1:lTz6Ys4CmqqCQmZPBlbQENR1/GucA2bzYTE12Pw4tFY=
 github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk=
+github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM=
+github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=

pkg/builder3/openapi.go

@@ -39,7 +39,6 @@ type openAPI struct {
 	definitions map[string]common.OpenAPIDefinition
 }
 
-
 func groupRoutesByPath(routes []common.Route) map[string][]common.Route {
 	pathToRoutes := make(map[string][]common.Route)
 	for _, r := range routes {
@@ -52,7 +51,6 @@ func (o *openAPI) buildResponse(model interface{}, description string, content [
 	response := &spec3.Response{
 		ResponseProps: spec3.ResponseProps{
 			Description: description,
-			Headers:     make(map[string]*spec3.Header),
 			Content:     make(map[string]*spec3.MediaType),
 		},
 	}
@@ -83,6 +81,15 @@ func (o *openAPI) buildOperations(route common.Route, inPathCommonParamsMap map[
 			},
 		},
 	}
+	for k, v := range route.Metadata() {
+		if strings.HasPrefix(k, common.ExtensionPrefix) {
+			if ret.Extensions == nil {
+				ret.Extensions = spec.Extensions{}
+			}
+			ret.Extensions.Add(k, v)
+		}
+	}
+
 	var err error
 	if ret.OperationId, ret.Tags, err = o.config.GetOperationIDAndTagsFromRoute(route); err != nil {
 		return ret, err
@@ -115,7 +122,6 @@ func (o *openAPI) buildOperations(route common.Route, inPathCommonParamsMap map[
 		ret.Responses.Default = o.config.DefaultResponse
 	}
 
-	ret.Parameters = make([]*spec3.Parameter, 0)
 	params := route.Parameters()
 	for _, param := range params {
 		_, isCommon := inPathCommonParamsMap[mapKeyFromParam(param)]
@@ -128,7 +134,7 @@ func (o *openAPI) buildOperations(route common.Route, inPathCommonParamsMap map[
 		}
 	}
 
-	body, err := o.buildRequestBody(params, route.RequestPayloadSample())
+	body, err := o.buildRequestBody(params, route.Consumes(), route.RequestPayloadSample())
 	if err != nil {
 		return nil, err
 	}
@@ -139,7 +145,7 @@ func (o *openAPI) buildOperations(route common.Route, inPathCommonParamsMap map[
 	return ret, nil
 }
 
-func (o *openAPI) buildRequestBody(parameters []common.Parameter, bodySample interface{}) (*spec3.RequestBody, error) {
+func (o *openAPI) buildRequestBody(parameters []common.Parameter, consumes []string, bodySample interface{}) (*spec3.RequestBody, error) {
 	for _, param := range parameters {
 		if param.Kind() == common.BodyParameterKind && bodySample != nil {
 			schema, err := o.toSchema(util.GetCanonicalTypeName(bodySample))
@@ -148,15 +154,16 @@ func (o *openAPI) buildRequestBody(parameters []common.Parameter, bodySample int
 			}
 			r := &spec3.RequestBody{
 				RequestBodyProps: spec3.RequestBodyProps{
-					Content: map[string]*spec3.MediaType{
-						"application/json": &spec3.MediaType{
-							MediaTypeProps: spec3.MediaTypeProps{
-								Schema: schema,
-							},
-						},
-					},
+					Content: map[string]*spec3.MediaType{},
 				},
 			}
+			for _, consume := range consumes {
+				r.Content[consume] = &spec3.MediaType{
+					MediaTypeProps: spec3.MediaTypeProps{
+						Schema: schema,
+					},
+				}
+			}
 			return r, nil
 		}
 	}
@@ -174,15 +181,25 @@ func newOpenAPI(config *common.Config) openAPI {
 			},
 			Components: &spec3.Components{
 				Schemas:         map[string]*spec.Schema{},
-				SecuritySchemes: make(spec3.SecuritySchemes),
-				Responses: make(map[string]*spec3.Response),
 			},
 		},
 	}
+	if len(o.config.ResponseDefinitions) > 0 {
+		o.spec.Components.Responses = make(map[string]*spec3.Response)
+
+	}
 	for k, response := range o.config.ResponseDefinitions {
 		o.spec.Components.Responses[k] = response
 	}
 
+	if len(o.config.SecuritySchemes) > 0 {
+		o.spec.Components.SecuritySchemes = make(spec3.SecuritySchemes)
+
+	}
+	for k, securityScheme := range o.config.SecuritySchemes {
+		o.spec.Components.SecuritySchemes[k] = securityScheme
+	}
+
 	if o.config.GetOperationIDAndTagsFromRoute == nil {
 		// Map the deprecated handler to the common interface, if provided.
 		if o.config.GetOperationIDAndTags != nil {
@@ -249,9 +266,7 @@ func (o *openAPI) buildOpenAPISpec(webServices []common.RouteContainer) error {
 			}
 
 			pathItem = &spec3.Path{
-				PathProps: spec3.PathProps{
-					Parameters: make([]*spec3.Parameter, 0),
-				},
+				PathProps: spec3.PathProps{},
 			}
 
 			// add web services's parameters as well as any parameters appears in all ops, as common parameters
@@ -263,6 +278,7 @@ func (o *openAPI) buildOpenAPISpec(webServices []common.RouteContainer) error {
 
 			for _, route := range routes {
 				op, _ := o.buildOperations(route, inPathCommonParamsMap)
+				sortParameters(op.Parameters)
 
 				switch strings.ToUpper(route.Method()) {
 				case "GET":

pkg/openapiconv/convert.go

@@ -54,18 +54,23 @@ func ConvertExternalDocumentation(v2ED *spec.ExternalDocumentation) *spec3.Exter
 }
 
 func ConvertComponents(v2SecurityDefinitions spec.SecurityDefinitions, v2Definitions spec.Definitions, v2Responses map[string]spec.Response, produces []string) *spec3.Components {
-	components := &spec3.Components{
-		Schemas:         make(map[string]*spec.Schema),
-		SecuritySchemes: make(spec3.SecuritySchemes),
-		Responses:       make(map[string]*spec3.Response),
+	components := &spec3.Components{}
+
+	if len(v2Definitions) > 0 {
+		components.Schemas = make(map[string]*spec.Schema)
 	}
 	for s, schema := range v2Definitions {
 		components.Schemas[s] = ConvertSchema(&schema)
 	}
-
+	if len(v2SecurityDefinitions) > 0 {
+		components.SecuritySchemes = make(spec3.SecuritySchemes)
+	}
 	for s, securityScheme := range v2SecurityDefinitions {
 		components.SecuritySchemes[s] = ConvertSecurityScheme(securityScheme)
 	}
+	if len(v2Responses) > 0 {
+		components.Responses = make(map[string]*spec3.Response)
+	}
 	for r, response := range v2Responses {
 		components.Responses[r] = ConvertResponse(&response, produces)
 	}
@@ -144,9 +149,11 @@ func ConvertPaths(v2Paths *spec.Paths) *spec3.Paths {
 	}
 	paths := &spec3.Paths{
 		VendorExtensible: v2Paths.VendorExtensible,
-		Paths:            make(map[string]*spec3.Path),
 	}
 
+	if len(v2Paths.Paths) > 0 {
+		paths.Paths = make(map[string]*spec3.Path)
+	}
 	for k, v := range v2Paths.Paths {
 		paths.Paths[k] = ConvertPathItem(v)
 	}
@@ -157,14 +164,13 @@ func ConvertPathItem(v2pathItem spec.PathItem) *spec3.Path {
 	path := &spec3.Path{
 		Refable: v2pathItem.Refable,
 		PathProps: spec3.PathProps{
-			Get:        ConvertOperation(v2pathItem.Get),
-			Put:        ConvertOperation(v2pathItem.Put),
-			Post:       ConvertOperation(v2pathItem.Post),
-			Delete:     ConvertOperation(v2pathItem.Delete),
-			Options:    ConvertOperation(v2pathItem.Options),
-			Head:       ConvertOperation(v2pathItem.Head),
-			Patch:      ConvertOperation(v2pathItem.Patch),
-			Parameters: []*spec3.Parameter{},
+			Get:     ConvertOperation(v2pathItem.Get),
+			Put:     ConvertOperation(v2pathItem.Put),
+			Post:    ConvertOperation(v2pathItem.Post),
+			Delete:  ConvertOperation(v2pathItem.Delete),
+			Options: ConvertOperation(v2pathItem.Options),
+			Head:    ConvertOperation(v2pathItem.Head),
+			Patch:   ConvertOperation(v2pathItem.Patch),
 		},
 		VendorExtensible: v2pathItem.VendorExtensible,
 	}
@@ -187,16 +193,16 @@ func ConvertOperation(v2Operation *spec.Operation) *spec3.Operation {
 			Summary:      v2Operation.Summary,
 			Deprecated:   v2Operation.Deprecated,
 			OperationId:  v2Operation.ID,
-			Parameters:   []*spec3.Parameter{},
 		},
 	}
 
 	for _, param := range v2Operation.Parameters {
 		if param.ParamProps.Name == "body" && param.ParamProps.Schema != nil {
 			operation.OperationProps.RequestBody = &spec3.RequestBody{
-				RequestBodyProps: spec3.RequestBodyProps{
-					Content: make(map[string]*spec3.MediaType),
-				},
+				RequestBodyProps: spec3.RequestBodyProps{},
+			}
+			if len(v2Operation.Consumes) > 0 {
+				operation.RequestBody.Content = make(map[string]*spec3.MediaType)
 			}
 			for _, consumer := range v2Operation.Consumes {
 				operation.RequestBody.Content[consumer] = &spec3.MediaType{
@@ -205,17 +211,20 @@ func ConvertOperation(v2Operation *spec.Operation) *spec3.Operation {
 					},
 				}
 			}
+		} else {
+			operation.Parameters = append(operation.Parameters, ConvertParameter(param))
 		}
-		operation.Parameters = append(operation.Parameters, ConvertParameter(param))
 	}
 
 	operation.Responses = &spec3.Responses{ResponsesProps: spec3.ResponsesProps{
 		Default:             ConvertResponse(v2Operation.Responses.Default, v2Operation.Produces),
-		StatusCodeResponses: make(map[int]*spec3.Response),
 	},
-		VendorExtensible: v2Operation.VendorExtensible,
+		VendorExtensible: v2Operation.Responses.VendorExtensible,
 	}
 
+	if len(v2Operation.Responses.StatusCodeResponses) > 0 {
+		operation.Responses.StatusCodeResponses = make(map[int]*spec3.Response)
+	}
 	for k, v := range v2Operation.Responses.StatusCodeResponses {
 		operation.Responses.StatusCodeResponses[k] = ConvertResponse(&v, v2Operation.Produces)
 	}
@@ -231,12 +240,13 @@ func ConvertResponse(v2Response *spec.Response, produces []string) *spec3.Respon
 		VendorExtensible: v2Response.VendorExtensible,
 		ResponseProps: spec3.ResponseProps{
 			Description: v2Response.Description,
-			Headers:     make(map[string]*spec3.Header),
-			Content:     make(map[string]*spec3.MediaType),
 		},
 	}
 
 	if v2Response.Schema != nil {
+		if len(produces) > 0 {
+			response.Content = make(map[string]*spec3.MediaType)
+		}
 		for _, producer := range produces {
 			response.ResponseProps.Content[producer] = &spec3.MediaType{
 				MediaTypeProps: spec3.MediaTypeProps{
@@ -261,6 +271,17 @@ func ConvertParameter(v2Param spec.Parameter) *spec3.Parameter {
 			AllowEmptyValue: v2Param.AllowEmptyValue,
 		},
 	}
+	// Convert SimpleSchema into Schema
+	if param.Schema == nil {
+		param.Schema = &spec.Schema{
+			SchemaProps: spec.SchemaProps{
+				Type:        []string{v2Param.Type},
+				Format:      v2Param.Format,
+				UniqueItems: v2Param.UniqueItems,
+			},
+		}
+	}
+
 	return param
 }
 

pkg/openapiconv/convert_test.go

@@ -0,0 +1,69 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package openapiconv
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"path/filepath"
+	"reflect"
+	"testing"
+
+	"k8s.io/kube-openapi/pkg/spec3"
+	"k8s.io/kube-openapi/pkg/validation/spec"
+)
+
+func TestConvert(t *testing.T) {
+
+	tcs := []struct {
+		groupVersion string
+	}{{
+		"batch.v1",
+	}, {
+		"api.v1",
+	}, {
+		"apiextensions.k8s.io.v1",
+	}}
+
+	for _, tc := range tcs {
+
+		spec2JSON, err := ioutil.ReadFile(filepath.Join("testdata/v2_" + tc.groupVersion + ".json"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		var swaggerSpec spec.Swagger
+		err = json.Unmarshal(spec2JSON, &swaggerSpec)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		convertedV3Spec := ConvertV2ToV3(&swaggerSpec)
+		if err != nil {
+			t.Fatal(err)
+		}
+		spec3JSON, err := ioutil.ReadFile(filepath.Join("testdata/v3_" + tc.groupVersion + ".json"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		var V3Spec spec3.OpenAPI
+		json.Unmarshal(spec3JSON, &V3Spec)
+
+		if !reflect.DeepEqual(V3Spec, *convertedV3Spec) {
+			t.Error("Expected specs to be equal")
+		}
+	}
+}