From 38b2567d8b2af28f0191a20fdaa33d82bd824c38 Mon Sep 17 00:00:00 2001 From: mbohlool Date: Tue, 13 Sep 2016 17:11:36 -0700 Subject: [PATCH] Move generated openAPI specs out of genericapiserver and make it configurable --- cmd/kube-apiserver/app/server.go | 3 ++ .../go2idl/openapi-gen/generators/openapi.go | 40 +++++++++---------- .../cmd/federation-apiserver/app/server.go | 3 ++ pkg/generated/doc.go | 19 +++++++++ pkg/generated/openapi/doc.go | 19 +++++++++ pkg/genericapiserver/config.go | 7 +++- pkg/genericapiserver/genericapiserver.go | 28 +++++++------ pkg/genericapiserver/openapi/openapi.go | 21 ++++------ pkg/genericapiserver/openapi/openapi_test.go | 8 ++-- pkg/master/master_test.go | 2 + test/integration/framework/master_utils.go | 3 ++ 11 files changed, 102 insertions(+), 51 deletions(-) create mode 100644 pkg/generated/doc.go create mode 100644 pkg/generated/openapi/doc.go diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 907d3c9752..7a3581aa01 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -45,6 +45,7 @@ import ( "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/controller/informers" serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount" + "k8s.io/kubernetes/pkg/generated/openapi" "k8s.io/kubernetes/pkg/genericapiserver" "k8s.io/kubernetes/pkg/genericapiserver/authorizer" genericoptions "k8s.io/kubernetes/pkg/genericapiserver/options" @@ -282,6 +283,8 @@ func Run(s *options.APIServer) error { genericConfig.ProxyTLSClientConfig = proxyTLSClientConfig genericConfig.Serializer = api.Codecs genericConfig.OpenAPIInfo.Title = "Kubernetes" + genericConfig.OpenAPIDefinitions = openapi.OpenAPIDefinitions + genericConfig.EnableOpenAPISupport = true config := &master.Config{ Config: genericConfig, diff --git a/cmd/libs/go2idl/openapi-gen/generators/openapi.go b/cmd/libs/go2idl/openapi-gen/generators/openapi.go index 21181a4335..2f6de50ecd 100644 --- a/cmd/libs/go2idl/openapi-gen/generators/openapi.go +++ b/cmd/libs/go2idl/openapi-gen/generators/openapi.go @@ -85,7 +85,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat `)...) - targets := []*types.Type{} + targets := []*types.Package{} for i := range inputs { glog.V(5).Infof("considering pkg %q", i) pkg, ok := context.Universe[i] @@ -93,11 +93,9 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat // If the input had no Go files, for example. continue } - for _, t := range pkg.Types { - if hasOpenAPITagValue(t.CommentLines, tagTargetType) { - glog.V(5).Infof("target type : %q", t) - targets = append(targets, t) - } + if hasOpenAPITagValue(pkg.Comments, tagTargetType) || hasOpenAPITagValue(pkg.DocComments, tagTargetType) { + glog.V(5).Infof("target package : %q", pkg) + targets = append(targets, pkg) } } switch len(targets) { @@ -106,7 +104,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat // and build excluded the target package. return generator.Packages{} case 1: - pkg := context.Universe[targets[0].Name.Package] + pkg := targets[0] return generator.Packages{&generator.DefaultPackage{ PackageName: strings.Split(filepath.Base(pkg.Path), ".")[0], PackagePath: pkg.Path, @@ -144,27 +142,27 @@ const ( // openApiGen produces a file with auto-generated OpenAPI functions. type openAPIGen struct { generator.DefaultGen - // TargetType is the type that will get OpenAPIDefinitions method returning all definitions. - targetType *types.Type - imports namer.ImportTracker - context *generator.Context + // TargetPackage is the package that will get OpenAPIDefinitions variable contains all open API definitions. + targetPackage *types.Package + imports namer.ImportTracker + context *generator.Context } -func NewOpenAPIGen(sanitizedName string, targetType *types.Type, context *generator.Context) generator.Generator { +func NewOpenAPIGen(sanitizedName string, targetPackage *types.Package, context *generator.Context) generator.Generator { return &openAPIGen{ DefaultGen: generator.DefaultGen{ OptionalName: sanitizedName, }, - imports: generator.NewImportTracker(), - targetType: targetType, - context: context, + imports: generator.NewImportTracker(), + targetPackage: targetPackage, + context: context, } } func (g *openAPIGen) Namers(c *generator.Context) namer.NameSystems { // Have the raw namer for this file track what it imports. return namer.NameSystems{ - "raw": namer.NewRawNamer(g.targetType.Name.Package, g.imports), + "raw": namer.NewRawNamer(g.targetPackage.Path, g.imports), } } @@ -177,10 +175,10 @@ func (g *openAPIGen) Filter(c *generator.Context, t *types.Type) bool { } func (g *openAPIGen) isOtherPackage(pkg string) bool { - if pkg == g.targetType.Name.Package { + if pkg == g.targetPackage.Path { return false } - if strings.HasSuffix(pkg, "\""+g.targetType.Name.Package+"\"") { + if strings.HasSuffix(pkg, "\""+g.targetPackage.Path+"\"") { return false } return true @@ -205,14 +203,14 @@ func argsFromType(t *types.Type) generator.Args { func (g *openAPIGen) Init(c *generator.Context, w io.Writer) error { sw := generator.NewSnippetWriter(w, c, "$", "$") - sw.Do("func (_ $.type|raw$) OpenAPIDefinitions() *$.OpenAPIDefinitions|raw$ {\n", argsFromType(g.targetType)) - sw.Do("return &$.OpenAPIDefinitions|raw${\n", argsFromType(nil)) + sw.Do("var OpenAPIDefinitions *$.OpenAPIDefinitions|raw$ = ", argsFromType(nil)) + sw.Do("&$.OpenAPIDefinitions|raw${\n", argsFromType(nil)) return sw.Error() } func (g *openAPIGen) Finalize(c *generator.Context, w io.Writer) error { sw := generator.NewSnippetWriter(w, c, "$", "$") - sw.Do("}\n}\n", nil) + sw.Do("}\n", nil) return sw.Error() } diff --git a/federation/cmd/federation-apiserver/app/server.go b/federation/cmd/federation-apiserver/app/server.go index 279838ae38..dc18e82572 100644 --- a/federation/cmd/federation-apiserver/app/server.go +++ b/federation/cmd/federation-apiserver/app/server.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/apiserver/authenticator" "k8s.io/kubernetes/pkg/controller/informers" + "k8s.io/kubernetes/pkg/generated/openapi" "k8s.io/kubernetes/pkg/genericapiserver" "k8s.io/kubernetes/pkg/genericapiserver/authorizer" genericoptions "k8s.io/kubernetes/pkg/genericapiserver/options" @@ -185,6 +186,8 @@ func Run(s *options.ServerRunOptions) error { genericConfig.APIResourceConfigSource = storageFactory.APIResourceConfigSource genericConfig.MasterServiceNamespace = s.MasterServiceNamespace genericConfig.Serializer = api.Codecs + genericConfig.OpenAPIDefinitions = openapi.OpenAPIDefinitions + genericConfig.EnableOpenAPISupport = true // TODO: Move this to generic api server (Need to move the command line flag). if s.EnableWatchCache { diff --git a/pkg/generated/doc.go b/pkg/generated/doc.go new file mode 100644 index 0000000000..f17a557129 --- /dev/null +++ b/pkg/generated/doc.go @@ -0,0 +1,19 @@ +/* +Copyright 2016 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. +*/ + +// generated package is the destination for all generated files. Not all generated files are currently use this package +// but the plan is to move as much of them as possible to this package. +package generated diff --git a/pkg/generated/openapi/doc.go b/pkg/generated/openapi/doc.go new file mode 100644 index 0000000000..cd40eb2533 --- /dev/null +++ b/pkg/generated/openapi/doc.go @@ -0,0 +1,19 @@ +/* +Copyright 2016 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. +*/ + +// openapi generated definitions. +// +k8s:openapi-gen=target +package openapi diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index 6f919c6213..a1ab057c6c 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -32,6 +32,7 @@ import ( "github.com/golang/glog" "gopkg.in/natefinch/lumberjack.v2" + "k8s.io/kubernetes/cmd/libs/go2idl/openapi-gen/generators/common" "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/unversioned" @@ -157,6 +158,10 @@ type Config struct { // OpenAPIDefaultResponse will be used if an web service operation does not have any responses listed. OpenAPIDefaultResponse spec.Response + + // OpenAPIDefinitions is a map of type to OpenAPI spec for all types used in this API server. Failure to provide + // this map or any of the models used by the server APIs will result in spec generation failure. + OpenAPIDefinitions *common.OpenAPIDefinitions } func NewConfig(options *options.ServerRunOptions) *Config { @@ -183,7 +188,6 @@ func NewConfig(options *options.ServerRunOptions) *Config { ReadWritePort: options.SecurePort, ServiceClusterIPRange: &options.ServiceClusterIPRange, ServiceNodePortRange: options.ServiceNodePortRange, - EnableOpenAPISupport: true, OpenAPIDefaultResponse: spec.Response{ ResponseProps: spec.ResponseProps{ Description: "Default Response."}}, @@ -308,6 +312,7 @@ func (c Config) New() (*GenericAPIServer, error) { enableOpenAPISupport: c.EnableOpenAPISupport, openAPIInfo: c.OpenAPIInfo, openAPIDefaultResponse: c.OpenAPIDefaultResponse, + openAPIDefinitions: c.OpenAPIDefinitions, } if c.EnableWatchCache { diff --git a/pkg/genericapiserver/genericapiserver.go b/pkg/genericapiserver/genericapiserver.go index 399e4fcac8..962d73dfae 100644 --- a/pkg/genericapiserver/genericapiserver.go +++ b/pkg/genericapiserver/genericapiserver.go @@ -35,6 +35,7 @@ import ( "github.com/golang/glog" "github.com/go-openapi/spec" + "k8s.io/kubernetes/cmd/libs/go2idl/openapi-gen/generators/common" "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/rest" @@ -171,6 +172,7 @@ type GenericAPIServer struct { PostStartHooks map[string]PostStartHookFunc postStartHookLock sync.Mutex postStartHooksCalled bool + openAPIDefinitions *common.OpenAPIDefinitions } func (s *GenericAPIServer) StorageDecorator() generic.StorageDecorator { @@ -565,24 +567,26 @@ func (s *GenericAPIServer) InstallOpenAPI() { info := s.openAPIInfo info.Title = info.Title + " " + w.RootPath() err := openapi.RegisterOpenAPIService(&openapi.Config{ - OpenAPIServePath: w.RootPath() + "/swagger.json", - WebServices: []*restful.WebService{w}, - ProtocolList: []string{"https"}, - IgnorePrefixes: []string{"/swaggerapi"}, - Info: &info, - DefaultResponse: &s.openAPIDefaultResponse, + OpenAPIServePath: w.RootPath() + "/swagger.json", + WebServices: []*restful.WebService{w}, + ProtocolList: []string{"https"}, + IgnorePrefixes: []string{"/swaggerapi"}, + Info: &info, + DefaultResponse: &s.openAPIDefaultResponse, + OpenAPIDefinitions: s.openAPIDefinitions, }, s.HandlerContainer) if err != nil { glog.Fatalf("Failed to register open api spec for %v: %v", w.RootPath(), err) } } err := openapi.RegisterOpenAPIService(&openapi.Config{ - OpenAPIServePath: "/swagger.json", - WebServices: s.HandlerContainer.RegisteredWebServices(), - ProtocolList: []string{"https"}, - IgnorePrefixes: []string{"/swaggerapi"}, - Info: &s.openAPIInfo, - DefaultResponse: &s.openAPIDefaultResponse, + OpenAPIServePath: "/swagger.json", + WebServices: s.HandlerContainer.RegisteredWebServices(), + ProtocolList: []string{"https"}, + IgnorePrefixes: []string{"/swaggerapi"}, + Info: &s.openAPIInfo, + DefaultResponse: &s.openAPIDefaultResponse, + OpenAPIDefinitions: s.openAPIDefinitions, }, s.HandlerContainer) if err != nil { glog.Fatalf("Failed to register open api spec for root: %v", err) diff --git a/pkg/genericapiserver/openapi/openapi.go b/pkg/genericapiserver/openapi/openapi.go index e064c2c28c..00ef9a5077 100644 --- a/pkg/genericapiserver/openapi/openapi.go +++ b/pkg/genericapiserver/openapi/openapi.go @@ -50,14 +50,16 @@ type Config struct { 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 } -// +k8s:openapi-gen=target type openAPI struct { - config *Config - swagger *spec.Swagger - protocolList []string - openAPIDefinitions *common.OpenAPIDefinitions + config *Config + swagger *spec.Swagger + protocolList []string } // RegisterOpenAPIService registers a handler to provides standard OpenAPI specification. @@ -91,17 +93,10 @@ func RegisterOpenAPIService(config *Config, containers *restful.Container) (err } func (o *openAPI) init() error { - if o.openAPIDefinitions == nil { - // Compilation error here means the code generator need to run first. - o.openAPIDefinitions = o.OpenAPIDefinitions() - } err := o.buildPaths() if err != nil { return err } - // no need to the keep type list in memory - o.openAPIDefinitions = nil - return nil } @@ -109,7 +104,7 @@ func (o *openAPI) buildDefinitionRecursively(name string) error { if _, ok := o.swagger.Definitions[name]; ok { return nil } - if item, ok := (*o.openAPIDefinitions)[name]; ok { + if item, ok := (*o.config.OpenAPIDefinitions)[name]; ok { o.swagger.Definitions[name] = item.Schema for _, v := range item.Dependencies { if err := o.buildDefinitionRecursively(v); err != nil { diff --git a/pkg/genericapiserver/openapi/openapi_test.go b/pkg/genericapiserver/openapi/openapi_test.go index c6696fd835..0eba12cfe7 100644 --- a/pkg/genericapiserver/openapi/openapi_test.go +++ b/pkg/genericapiserver/openapi/openapi_test.go @@ -42,10 +42,6 @@ func setUp(t *testing.T, fullMethods bool) (openAPI, *assert.Assertions) { Info: config.Info, }, }, - openAPIDefinitions: &common.OpenAPIDefinitions{ - "openapi.TestInput": *TestInput{}.OpenAPIDefinition(), - "openapi.TestOutput": *TestOutput{}.OpenAPIDefinition(), - }, }, assert } @@ -193,6 +189,10 @@ func getConfig(fullMethods bool) *Config { Description: "Test API", }, }, + OpenAPIDefinitions: &common.OpenAPIDefinitions{ + "openapi.TestInput": *TestInput{}.OpenAPIDefinition(), + "openapi.TestOutput": *TestOutput{}.OpenAPIDefinition(), + }, } } diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index 08690c48da..a3163ea7ef 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -71,6 +71,7 @@ import ( "github.com/go-openapi/validate" "github.com/stretchr/testify/assert" "golang.org/x/net/context" + "k8s.io/kubernetes/pkg/generated/openapi" ) // setUp is a convience function for setting up for (most) tests. @@ -1251,6 +1252,7 @@ func TestValidOpenAPISpec(t *testing.T) { _, etcdserver, config, assert := setUp(t) defer etcdserver.Terminate(t) + config.OpenAPIDefinitions = openapi.OpenAPIDefinitions config.EnableOpenAPISupport = true config.EnableIndex = true config.OpenAPIInfo = spec.Info{ diff --git a/test/integration/framework/master_utils.go b/test/integration/framework/master_utils.go index 9f4e8de6c8..c6f1e5285e 100644 --- a/test/integration/framework/master_utils.go +++ b/test/integration/framework/master_utils.go @@ -58,6 +58,7 @@ import ( "github.com/go-openapi/spec" "github.com/pborman/uuid" + "k8s.io/kubernetes/pkg/generated/openapi" ) const ( @@ -235,6 +236,8 @@ func NewMasterConfig() *master.Config { ServiceClusterIPRange: parseCIDROrDie("10.0.0.0/24"), ServiceNodePortRange: utilnet.PortRange{Base: 30000, Size: 2768}, EnableVersion: true, + OpenAPIDefinitions: openapi.OpenAPIDefinitions, + EnableOpenAPISupport: true, }, KubeletClient: kubeletclient.FakeKubeletClient{}, }