Make Kubernetes OpenAPI operation IDs unique

This commit is contained in:
mbohlool
2016-10-08 02:36:37 -07:00
parent 221a620a14
commit 5ba06cf2bc
12 changed files with 244 additions and 153 deletions

View File

@@ -16,7 +16,10 @@ limitations under the License.
package common
import "github.com/go-openapi/spec"
import (
"github.com/emicklei/go-restful"
"github.com/go-openapi/spec"
)
// OpenAPIDefinition describes single type. Normally these definitions are auto-generated using gen-openapi.
type OpenAPIDefinition struct {
@@ -35,6 +38,27 @@ type OpenAPIDefinitionGetter interface {
OpenAPIDefinition() *OpenAPIDefinition
}
// Config is set of configuration for openAPI spec generation.
type Config struct {
// List of supported protocols such as https, http, etc.
ProtocolList []string
// Info is general information about the API.
Info *spec.Info
// DefaultResponse will be used if an operation does not have any responses listed. It
// will show up as ... "responses" : {"default" : $DefaultResponse} in the spec.
DefaultResponse *spec.Response
// List of webservice's path prefixes to ignore
IgnorePrefixes []string
// OpenAPIDefinitions should provide definition for all models used by routes. Failure to provide this map
// or any of the models will result in spec generation failure.
Definitions *OpenAPIDefinitions
// GetOperationID returns operation id for a restful route. It is an optional function to customize operation IDs.
GetOperationID func(servePath string, r *restful.Route) (string, error)
}
// This function is a reference for converting go (or any custom type) to a simple open API type,format pair. There are
// two ways to customize spec for a type. If you add it here, a type will be converted to a simple type and the type
// comment (the comment that is added before type definition) will be lost. The spec will still have the property

View File

@@ -26,47 +26,26 @@ import (
"github.com/go-openapi/spec"
"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
"k8s.io/kubernetes/pkg/util/json"
"k8s.io/kubernetes/pkg/util"
"k8s.io/kubernetes/pkg/util/json"
)
const (
OpenAPIVersion = "2.0"
)
// Config is set of configuration for openAPI spec generation.
type Config struct {
// Path to the spec file. by convention, it should name [.*/]*/swagger.json
OpenAPIServePath string
// List of web services for this API spec
WebServices []*restful.WebService
// List of supported protocols such as https, http, etc.
ProtocolList []string
// Info is general information about the API.
Info *spec.Info
// DefaultResponse will be used if an operation does not have any responses listed. It
// will show up as ... "responses" : {"default" : $DefaultResponse} in the spec.
DefaultResponse *spec.Response
// List of webservice's path prefixes to ignore
IgnorePrefixes []string
// OpenAPIDefinitions should provide definition for all models used by routes. Failure to provide this map
// or any of the models will result in spec generation failure.
OpenAPIDefinitions *common.OpenAPIDefinitions
}
type openAPI struct {
config *Config
config *common.Config
swagger *spec.Swagger
protocolList []string
servePath string
}
// RegisterOpenAPIService registers a handler to provides standard OpenAPI specification.
func RegisterOpenAPIService(config *Config, containers *restful.Container) (err error) {
func RegisterOpenAPIService(servePath string, webServices []*restful.WebService, config *common.Config, containers *restful.Container) (err error) {
o := openAPI{
config: config,
config: config,
servePath: servePath,
swagger: &spec.Swagger{
SwaggerProps: spec.SwaggerProps{
Swagger: OpenAPIVersion,
@@ -77,14 +56,14 @@ func RegisterOpenAPIService(config *Config, containers *restful.Container) (err
},
}
err = o.init()
err = o.init(webServices)
if err != nil {
return err
}
containers.ServeMux.HandleFunc(config.OpenAPIServePath, func(w http.ResponseWriter, r *http.Request) {
containers.ServeMux.HandleFunc(servePath, func(w http.ResponseWriter, r *http.Request) {
resp := restful.NewResponse(w)
if r.URL.Path != config.OpenAPIServePath {
if r.URL.Path != servePath {
resp.WriteErrorString(http.StatusNotFound, "Path not found!")
}
// TODO: we can cache json string and return it here.
@@ -93,8 +72,13 @@ func RegisterOpenAPIService(config *Config, containers *restful.Container) (err
return nil
}
func (o *openAPI) init() error {
err := o.buildPaths()
func (o *openAPI) init(webServices []*restful.WebService) error {
if o.config.GetOperationID == nil {
o.config.GetOperationID = func(_ string, r *restful.Route) (string, error) {
return r.Operation, nil
}
}
err := o.buildPaths(webServices)
if err != nil {
return err
}
@@ -105,7 +89,7 @@ func (o *openAPI) buildDefinitionRecursively(name string) error {
if _, ok := o.swagger.Definitions[name]; ok {
return nil
}
if item, ok := (*o.config.OpenAPIDefinitions)[name]; ok {
if item, ok := (*o.config.Definitions)[name]; ok {
o.swagger.Definitions[name] = item.Schema
for _, v := range item.Dependencies {
if err := o.buildDefinitionRecursively(v); err != nil {
@@ -134,20 +118,10 @@ func (o *openAPI) buildDefinitionForType(sample interface{}) (string, error) {
}
// buildPaths builds OpenAPI paths using go-restful's web services.
func (o *openAPI) buildPaths() error {
func (o *openAPI) buildPaths(webServices []*restful.WebService) error {
pathsToIgnore := util.CreateTrie(o.config.IgnorePrefixes)
duplicateOpId := make(map[string]bool)
// Find duplicate operation IDs.
for _, service := range o.config.WebServices {
if pathsToIgnore.HasPrefix(service.RootPath()) {
continue
}
for _, route := range service.Routes() {
_, exists := duplicateOpId[route.Operation]
duplicateOpId[route.Operation] = exists
}
}
for _, w := range o.config.WebServices {
duplicateOpId := make(map[string]string)
for _, w := range webServices {
rootPath := w.RootPath()
if pathsToIgnore.HasPrefix(rootPath) {
continue
@@ -191,11 +165,11 @@ func (o *openAPI) buildPaths() error {
if err != nil {
return err
}
if duplicateOpId[op.ID] {
// Repeated Operation IDs are not allowed in OpenAPI spec but if
// an OperationID is empty, client generators will infer the ID
// from the path and method of operation.
op.ID = ""
dpath, exists := duplicateOpId[op.ID]
if exists {
return fmt.Errorf("Duplicate Operation ID %v for path %v and %v.", op.ID, dpath, path)
} else {
duplicateOpId[op.ID] = path
}
switch strings.ToUpper(route.Method) {
case "GET":
@@ -227,7 +201,6 @@ func (o *openAPI) buildOperations(route restful.Route, inPathCommonParamsMap map
Description: route.Doc,
Consumes: route.Consumes,
Produces: route.Produces,
ID: route.Operation,
Schemes: o.config.ProtocolList,
Responses: &spec.Responses{
ResponsesProps: spec.ResponsesProps{
@@ -236,6 +209,9 @@ func (o *openAPI) buildOperations(route restful.Route, inPathCommonParamsMap map
},
},
}
if ret.ID, err = o.config.GetOperationID(o.servePath, &route); err != nil {
return ret, err
}
// Build responses
for _, resp := range route.ResponseErrors {

View File

@@ -28,9 +28,9 @@ import (
)
// setUp is a convenience function for setting up for (most) tests.
func setUp(t *testing.T, fullMethods bool) (openAPI, *assert.Assertions) {
func setUp(t *testing.T, fullMethods bool) (openAPI, *restful.Container, *assert.Assertions) {
assert := assert.New(t)
config := getConfig(fullMethods)
config, container := getConfig(fullMethods)
return openAPI{
config: config,
swagger: &spec.Swagger{
@@ -41,7 +41,7 @@ func setUp(t *testing.T, fullMethods bool) (openAPI, *assert.Assertions) {
Info: config.Info,
},
},
}, assert
}, container, assert
}
func noOp(request *restful.Request, response *restful.Response) {}
@@ -130,11 +130,11 @@ func (_ TestOutput) OpenAPIDefinition() *common.OpenAPIDefinition {
var _ common.OpenAPIDefinitionGetter = TestInput{}
var _ common.OpenAPIDefinitionGetter = TestOutput{}
func getTestRoute(ws *restful.WebService, method string, additionalParams bool) *restful.RouteBuilder {
func getTestRoute(ws *restful.WebService, method string, additionalParams bool, opPrefix string) *restful.RouteBuilder {
ret := ws.Method(method).
Path("/test/{path:*}").
Doc(fmt.Sprintf("%s test input", method)).
Operation(fmt.Sprintf("%sTestInput", method)).
Operation(fmt.Sprintf("%s%sTestInput", method, opPrefix)).
Produces(restful.MIME_JSON).
Consumes(restful.MIME_JSON).
Param(ws.PathParameter("path", "path to the resource").DataType("string")).
@@ -150,52 +150,50 @@ func getTestRoute(ws *restful.WebService, method string, additionalParams bool)
return ret
}
func getConfig(fullMethods bool) *Config {
func getConfig(fullMethods bool) (*common.Config, *restful.Container) {
mux := http.NewServeMux()
container := restful.NewContainer()
container.ServeMux = mux
ws := new(restful.WebService)
ws.Path("/foo")
ws.Route(getTestRoute(ws, "get", true))
ws.Route(getTestRoute(ws, "get", true, "foo"))
if fullMethods {
ws.Route(getTestRoute(ws, "post", false)).
Route(getTestRoute(ws, "put", false)).
Route(getTestRoute(ws, "head", false)).
Route(getTestRoute(ws, "patch", false)).
Route(getTestRoute(ws, "options", false)).
Route(getTestRoute(ws, "delete", false))
ws.Route(getTestRoute(ws, "post", false, "foo")).
Route(getTestRoute(ws, "put", false, "foo")).
Route(getTestRoute(ws, "head", false, "foo")).
Route(getTestRoute(ws, "patch", false, "foo")).
Route(getTestRoute(ws, "options", false, "foo")).
Route(getTestRoute(ws, "delete", false, "foo"))
}
ws.Path("/bar")
ws.Route(getTestRoute(ws, "get", true))
ws.Route(getTestRoute(ws, "get", true, "bar"))
if fullMethods {
ws.Route(getTestRoute(ws, "post", false)).
Route(getTestRoute(ws, "put", false)).
Route(getTestRoute(ws, "head", false)).
Route(getTestRoute(ws, "patch", false)).
Route(getTestRoute(ws, "options", false)).
Route(getTestRoute(ws, "delete", false))
ws.Route(getTestRoute(ws, "post", false, "bar")).
Route(getTestRoute(ws, "put", false, "bar")).
Route(getTestRoute(ws, "head", false, "bar")).
Route(getTestRoute(ws, "patch", false, "bar")).
Route(getTestRoute(ws, "options", false, "bar")).
Route(getTestRoute(ws, "delete", false, "bar"))
}
container.Add(ws)
return &Config{
WebServices: container.RegisteredWebServices(),
ProtocolList: []string{"https"},
OpenAPIServePath: "/swagger.json",
return &common.Config{
ProtocolList: []string{"https"},
Info: &spec.Info{
InfoProps: spec.InfoProps{
Title: "TestAPI",
Description: "Test API",
},
},
OpenAPIDefinitions: &common.OpenAPIDefinitions{
Definitions: &common.OpenAPIDefinitions{
"openapi.TestInput": *TestInput{}.OpenAPIDefinition(),
"openapi.TestOutput": *TestOutput{}.OpenAPIDefinition(),
},
}
}, container
}
func getTestOperation(method string) *spec.Operation {
func getTestOperation(method string, opPrefix string) *spec.Operation {
return &spec.Operation{
OperationProps: spec.OperationProps{
Description: fmt.Sprintf("%s test input", method),
@@ -204,25 +202,26 @@ func getTestOperation(method string) *spec.Operation {
Schemes: []string{"https"},
Parameters: []spec.Parameter{},
Responses: getTestResponses(),
ID: fmt.Sprintf("%s%sTestInput", method, opPrefix),
},
}
}
func getTestPathItem(allMethods bool) spec.PathItem {
func getTestPathItem(allMethods bool, opPrefix string) spec.PathItem {
ret := spec.PathItem{
PathItemProps: spec.PathItemProps{
Get: getTestOperation("get"),
Get: getTestOperation("get", opPrefix),
Parameters: getTestCommonParameters(),
},
}
ret.Get.Parameters = getAdditionalTestParameters()
if allMethods {
ret.PathItemProps.Put = getTestOperation("put")
ret.PathItemProps.Post = getTestOperation("post")
ret.PathItemProps.Head = getTestOperation("head")
ret.PathItemProps.Patch = getTestOperation("patch")
ret.PathItemProps.Delete = getTestOperation("delete")
ret.PathItemProps.Options = getTestOperation("options")
ret.PathItemProps.Put = getTestOperation("put", opPrefix)
ret.PathItemProps.Post = getTestOperation("post", opPrefix)
ret.PathItemProps.Head = getTestOperation("head", opPrefix)
ret.PathItemProps.Patch = getTestOperation("patch", opPrefix)
ret.PathItemProps.Delete = getTestOperation("delete", opPrefix)
ret.PathItemProps.Options = getTestOperation("options", opPrefix)
}
return ret
}
@@ -380,7 +379,7 @@ func getTestOutputDefinition() spec.Schema {
}
func TestBuildSwaggerSpec(t *testing.T) {
o, assert := setUp(t, true)
o, container, assert := setUp(t, true)
expected := &spec.Swagger{
SwaggerProps: spec.SwaggerProps{
Info: &spec.Info{
@@ -392,8 +391,8 @@ func TestBuildSwaggerSpec(t *testing.T) {
Swagger: "2.0",
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/foo/test/{path}": getTestPathItem(true),
"/bar/test/{path}": getTestPathItem(true),
"/foo/test/{path}": getTestPathItem(true, "foo"),
"/bar/test/{path}": getTestPathItem(true, "bar"),
},
},
Definitions: spec.Definitions{
@@ -402,7 +401,7 @@ func TestBuildSwaggerSpec(t *testing.T) {
},
},
}
err := o.init()
err := o.init(container.RegisteredWebServices())
if assert.NoError(err) {
assert.Equal(expected, o.swagger)
}