Merge pull request #38690 from sttts/sttts-swagger-postbuildhandler

Automatic merge from submit-queue

genericapiserver: unify swagger and openapi in config

- make swagger config customizable
- remove superfluous `Config.Enable*` flags for OpenAPI and Swagger.

This is necessary for downstream projects to tweak the swagger spec.

cmd/kube-apiserver/app/server.go

@@ -283,11 +283,11 @@ func Run(s *options.ServerRunOptions) error {
 	genericConfig.Authenticator = apiAuthenticator
 	genericConfig.Authorizer = apiAuthorizer
 	genericConfig.AdmissionControl = admissionController
-	genericConfig.OpenAPIConfig.Info.Title = "Kubernetes"
+	genericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(generatedopenapi.OpenAPIDefinitions)
-	genericConfig.OpenAPIConfig.Definitions = generatedopenapi.OpenAPIDefinitions
-	genericConfig.EnableOpenAPISupport = true
-	genericConfig.EnableMetrics = true
 	genericConfig.OpenAPIConfig.SecurityDefinitions = securityDefinitions
+	genericConfig.OpenAPIConfig.Info.Title = "Kubernetes"
+	genericConfig.SwaggerConfig = genericapiserver.DefaultSwaggerConfig()
+	genericConfig.EnableMetrics = true
 	genericConfig.LongRunningFunc = filters.BasicLongRunningRequestCheck(
 		sets.NewString("watch", "proxy"),
 		sets.NewString("attach", "exec", "proxy", "log", "portforward"),

examples/apiserver/apiserver.go

@@ -111,6 +111,8 @@ func (serverOptions *ServerRunOptions) Run(stopCh <-chan struct{}) error {
 	}
 
 	config.Authorizer = authorizer.NewAlwaysAllowAuthorizer()
+	config.SwaggerConfig = genericapiserver.DefaultSwaggerConfig()
+
 	s, err := config.Complete().New()
 	if err != nil {
 		return fmt.Errorf("Error in bringing up the server: %v", err)

federation/cmd/federation-apiserver/app/server.go

@@ -165,9 +165,9 @@ func Run(s *options.ServerRunOptions) error {
 	genericConfig.Authenticator = apiAuthenticator
 	genericConfig.Authorizer = apiAuthorizer
 	genericConfig.AdmissionControl = admissionController
-	genericConfig.OpenAPIConfig.Definitions = openapi.OpenAPIDefinitions
+	genericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(openapi.OpenAPIDefinitions)
-	genericConfig.EnableOpenAPISupport = true
 	genericConfig.OpenAPIConfig.SecurityDefinitions = securityDefinitions
+	genericConfig.SwaggerConfig = genericapiserver.DefaultSwaggerConfig()
 	genericConfig.LongRunningFunc = filters.BasicLongRunningRequestCheck(
 		sets.NewString("watch", "proxy"),
 		sets.NewString("attach", "exec", "proxy", "log", "portforward"),

pkg/genericapiserver/BUILD

@@ -72,6 +72,7 @@ go_library(
         "//plugin/pkg/auth/authenticator/request/union:go_default_library",
         "//vendor:github.com/coreos/go-systemd/daemon",
         "//vendor:github.com/emicklei/go-restful",
+        "//vendor:github.com/emicklei/go-restful/swagger",
         "//vendor:github.com/go-openapi/spec",
         "//vendor:github.com/golang/glog",
         "//vendor:github.com/pborman/uuid",

pkg/genericapiserver/config.go

@@ -32,6 +32,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/emicklei/go-restful/swagger"
 	"github.com/go-openapi/spec"
 	"github.com/golang/glog"
 	"github.com/pborman/uuid"
@@ -55,7 +56,7 @@ import (
 	apiserverauthorizer "k8s.io/kubernetes/pkg/genericapiserver/authorizer"
 	genericfilters "k8s.io/kubernetes/pkg/genericapiserver/filters"
 	"k8s.io/kubernetes/pkg/genericapiserver/mux"
-	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
+	openapicommon "k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
 	"k8s.io/kubernetes/pkg/genericapiserver/options"
 	"k8s.io/kubernetes/pkg/genericapiserver/routes"
 	genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation"
@@ -94,15 +95,13 @@ type Config struct {
 	AdmissionControl      admission.Interface
 	CorsAllowedOriginList []string
 
-	EnableSwaggerSupport bool
+	EnableSwaggerUI bool
-	EnableSwaggerUI      bool
+	EnableIndex     bool
-	EnableIndex          bool
+	EnableProfiling bool
-	EnableProfiling      bool
 	// Requires generic profiling enabled
 	EnableContentionProfiling bool
 	EnableGarbageCollection   bool
 	EnableMetrics             bool
-	EnableOpenAPISupport      bool
 
 	// Version will enable the /version endpoint if non-nil
 	Version *version.Info
@@ -136,8 +135,11 @@ type Config struct {
 	// Serializer is required and provides the interface for serializing and converting objects to and from the wire
 	// The default (api.Codecs) usually works fine.
 	Serializer runtime.NegotiatedSerializer
-	// OpenAPIConfig will be used in generating OpenAPI spec.  This has "working" defaults.
+	// OpenAPIConfig will be used in generating OpenAPI spec. This is nil by default. Use DefaultOpenAPIConfig for "working" defaults.
-	OpenAPIConfig *common.Config
+	OpenAPIConfig *openapicommon.Config
+	// SwaggerConfig will be used in generating Swagger spec. This is nil by default. Use DefaultSwaggerConfig for "working" defaults.
+	SwaggerConfig *swagger.Config
+
 	// If specified, requests will be allocated a random timeout between this value, and twice this value.
 	// Note that it is up to the request handlers to ignore or honor this timeout. In seconds.
 	MinRequestTimeout int
@@ -202,25 +204,7 @@ func NewConfig() *Config {
 		BuildHandlerChainsFunc: DefaultBuildHandlerChain,
 		LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix),
 		HealthzChecks:          []healthz.HealthzChecker{healthz.PingHealthz},
-
+		EnableIndex:            true,
-		EnableIndex:          true,
-		EnableSwaggerSupport: true,
-		OpenAPIConfig: &common.Config{
-			ProtocolList:   []string{"https"},
-			IgnorePrefixes: []string{"/swaggerapi"},
-			Info: &spec.Info{
-				InfoProps: spec.InfoProps{
-					Title:   "Generic API Server",
-					Version: "unversioned",
-				},
-			},
-			DefaultResponse: &spec.Response{
-				ResponseProps: spec.ResponseProps{
-					Description: "Default Response.",
-				},
-			},
-			GetOperationIDAndTags: apiserveropenapi.GetOperationIDAndTags,
-		},
 
 		// Default to treating watch as a long-running operation
 		// Generic API servers have no inherent long-running subresources
@@ -235,6 +219,43 @@ func NewConfig() *Config {
 	return config.ApplyOptions(defaultOptions)
 }
 
+func DefaultOpenAPIConfig(definitions *openapicommon.OpenAPIDefinitions) *openapicommon.Config {
+	return &openapicommon.Config{
+		ProtocolList:   []string{"https"},
+		IgnorePrefixes: []string{"/swaggerapi"},
+		Info: &spec.Info{
+			InfoProps: spec.InfoProps{
+				Title:   "Generic API Server",
+				Version: "unversioned",
+			},
+		},
+		DefaultResponse: &spec.Response{
+			ResponseProps: spec.ResponseProps{
+				Description: "Default Response.",
+			},
+		},
+		GetOperationIDAndTags: apiserveropenapi.GetOperationIDAndTags,
+		Definitions:           definitions,
+	}
+}
+
+// DefaultSwaggerConfig returns a default configuration without WebServiceURL and
+// WebServices set.
+func DefaultSwaggerConfig() *swagger.Config {
+	return &swagger.Config{
+		ApiPath:         "/swaggerapi/",
+		SwaggerPath:     "/swaggerui/",
+		SwaggerFilePath: "/swagger-ui/",
+		SchemaFormatHandler: func(typeName string) string {
+			switch typeName {
+			case "metav1.Time", "*metav1.Time":
+				return "date-time"
+			}
+			return ""
+		},
+	}
+}
+
 func (c *Config) ApplySecureServingOptions(secureServing *options.SecureServingOptions) (*Config, error) {
 	if secureServing == nil || secureServing.ServingOptions.BindPort <= 0 {
 		return c, nil
@@ -442,8 +463,8 @@ func (c *Config) Complete() completedConfig {
 		}
 		c.ExternalAddress = hostAndPort
 	}
-	// All APIs will have the same authentication for now.
 	if c.OpenAPIConfig != nil && c.OpenAPIConfig.SecurityDefinitions != nil {
+		// Setup OpenAPI security: all APIs will have the same authentication for now.
 		c.OpenAPIConfig.DefaultSecurity = []map[string][]string{}
 		keys := []string{}
 		for k := range *c.OpenAPIConfig.SecurityDefinitions {
@@ -464,6 +485,13 @@ func (c *Config) Complete() completedConfig {
 			}
 		}
 	}
+	if c.SwaggerConfig != nil && len(c.SwaggerConfig.WebServicesUrl) == 0 {
+		if c.SecureServingInfo != nil {
+			c.SwaggerConfig.WebServicesUrl = "https://" + c.ExternalAddress
+		} else {
+			c.SwaggerConfig.WebServicesUrl = "http://" + c.ExternalAddress
+		}
+	}
 	if c.DiscoveryAddresses == nil {
 		c.DiscoveryAddresses = DefaultDiscoveryAddresses{DefaultAddress: c.ExternalAddress}
 	}
@@ -530,8 +558,7 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 		requestContextMapper:   c.RequestContextMapper,
 		Serializer:             c.Serializer,
 
-		minRequestTimeout:    time.Duration(c.MinRequestTimeout) * time.Second,
+		minRequestTimeout: time.Duration(c.MinRequestTimeout) * time.Second,
-		enableSwaggerSupport: c.EnableSwaggerSupport,
 
 		SecureServingInfo:   c.SecureServingInfo,
 		InsecureServingInfo: c.InsecureServingInfo,
@@ -539,8 +566,8 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 
 		apiGroupsForDiscovery: map[string]metav1.APIGroup{},
 
-		enableOpenAPISupport: c.EnableOpenAPISupport,
+		swaggerConfig: c.SwaggerConfig,
-		openAPIConfig:        c.OpenAPIConfig,
+		openAPIConfig: c.OpenAPIConfig,
 
 		postStartHooks: map[string]postStartHookEntry{},
 		healthzChecks:  c.HealthzChecks,
@@ -583,7 +610,7 @@ func (s *GenericAPIServer) installAPI(c *Config) {
 	if c.EnableIndex {
 		routes.Index{}.Install(s.HandlerContainer)
 	}
-	if c.EnableSwaggerSupport && c.EnableSwaggerUI {
+	if c.SwaggerConfig != nil && c.EnableSwaggerUI {
 		routes.SwaggerUI{}.Install(s.HandlerContainer)
 	}
 	if c.EnableProfiling {

pkg/genericapiserver/genericapiserver.go

@@ -27,6 +27,7 @@ import (
 
 	systemd "github.com/coreos/go-systemd/daemon"
 	"github.com/emicklei/go-restful"
+	"github.com/emicklei/go-restful/swagger"
 	"github.com/golang/glog"
 
 	"k8s.io/kubernetes/pkg/admission"
@@ -38,7 +39,7 @@ import (
 	"k8s.io/kubernetes/pkg/apiserver"
 	"k8s.io/kubernetes/pkg/client/restclient"
 	genericmux "k8s.io/kubernetes/pkg/genericapiserver/mux"
-	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
+	openapicommon "k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
 	"k8s.io/kubernetes/pkg/genericapiserver/routes"
 	"k8s.io/kubernetes/pkg/healthz"
 	"k8s.io/kubernetes/pkg/runtime"
@@ -86,12 +87,6 @@ type GenericAPIServer struct {
 	// minRequestTimeout is how short the request timeout can be.  This is used to build the RESTHandler
 	minRequestTimeout time.Duration
 
-	// enableSwaggerSupport indicates that swagger should be served.  This is currently separate because
-	// the API group routes are created *after* initialization and you can't generate the swagger routes until
-	// after those are available.
-	// TODO eventually we should be able to factor this out to take place during initialization.
-	enableSwaggerSupport bool
-
 	// legacyAPIGroupPrefixes is used to set up URL parsing for authorization and for validating requests
 	// to InstallLegacyAPIGroup
 	legacyAPIGroupPrefixes sets.String
@@ -131,10 +126,9 @@ type GenericAPIServer struct {
 	apiGroupsForDiscoveryLock sync.RWMutex
 	apiGroupsForDiscovery     map[string]metav1.APIGroup
 
-	// See Config.$name for documentation of these flags
+	// Enable swagger and/or OpenAPI if these configs are non-nil.
-
+	swaggerConfig *swagger.Config
-	enableOpenAPISupport bool
+	openAPIConfig *openapicommon.Config
-	openAPIConfig        *common.Config
 
 	// PostStartHooks are each called after the server has started listening, in a separate go func for each
 	// with no guaranteee of ordering between them.  The map key is a name used for error reporting.
@@ -174,10 +168,10 @@ type preparedGenericAPIServer struct {
 
 // PrepareRun does post API installation setup steps.
 func (s *GenericAPIServer) PrepareRun() preparedGenericAPIServer {
-	if s.enableSwaggerSupport {
+	if s.swaggerConfig != nil {
-		routes.Swagger{ExternalAddress: s.ExternalAddress}.Install(s.HandlerContainer)
+		routes.Swagger{Config: s.swaggerConfig}.Install(s.HandlerContainer)
 	}
-	if s.enableOpenAPISupport {
+	if s.openAPIConfig != nil {
 		routes.OpenAPI{
 			Config: s.openAPIConfig,
 		}.Install(s.HandlerContainer)

pkg/genericapiserver/genericapiserver_test.go

@@ -58,15 +58,14 @@ func setUp(t *testing.T) (*etcdtesting.EtcdTestServer, Config, *assert.Assertion
 	config.RequestContextMapper = api.NewRequestContextMapper()
 	config.LegacyAPIGroupPrefixes = sets.NewString("/api")
 
-	config.EnableOpenAPISupport = true
+	config.OpenAPIConfig = DefaultOpenAPIConfig(openapigen.OpenAPIDefinitions)
-	config.EnableSwaggerSupport = true
-	config.OpenAPIConfig.Definitions = openapigen.OpenAPIDefinitions
 	config.OpenAPIConfig.Info = &spec.Info{
 		InfoProps: spec.InfoProps{
 			Title:   "Kubernetes",
 			Version: "unversioned",
 		},
 	}
+	config.SwaggerConfig = DefaultSwaggerConfig()
 
 	return etcdServer, *config, assert.New(t)
 }
@@ -88,13 +87,14 @@ func TestNew(t *testing.T) {
 	defer etcdserver.Terminate(t)
 
 	// Verify many of the variables match their config counterparts
-	assert.Equal(s.enableSwaggerSupport, config.EnableSwaggerSupport)
 	assert.Equal(s.legacyAPIGroupPrefixes, config.LegacyAPIGroupPrefixes)
 	assert.Equal(s.admissionControl, config.AdmissionControl)
 	assert.Equal(s.RequestContextMapper(), config.RequestContextMapper)
 
 	// these values get defaulted
-	assert.Equal(s.ExternalAddress, net.JoinHostPort(config.PublicAddress.String(), "6443"))
+	assert.Equal(net.JoinHostPort(config.PublicAddress.String(), "6443"), s.ExternalAddress)
+	assert.NotNil(s.swaggerConfig)
+	assert.Equal("http://"+s.ExternalAddress, s.swaggerConfig.WebServicesUrl)
 }
 
 // Verifies that AddGroupVersions works as expected.
@@ -270,8 +270,8 @@ func TestPrepareRun(t *testing.T) {
 	s, etcdserver, config, assert := newMaster(t)
 	defer etcdserver.Terminate(t)
 
-	assert.True(config.EnableSwaggerSupport)
+	assert.NotNil(config.SwaggerConfig)
-	assert.True(config.EnableOpenAPISupport)
+	assert.NotNil(config.OpenAPIConfig)
 
 	server := httptest.NewServer(s.HandlerContainer.ServeMux)
 	defer server.Close()
@@ -369,7 +369,7 @@ func TestNotRestRoutesHaveAuth(t *testing.T) {
 	config.EnableSwaggerUI = true
 	config.EnableIndex = true
 	config.EnableProfiling = true
-	config.EnableSwaggerSupport = true
+	config.SwaggerConfig = DefaultSwaggerConfig()
 
 	kubeVersion := version.Get()
 	config.Version = &kubeVersion

pkg/genericapiserver/routes/BUILD

@@ -5,7 +5,6 @@ licenses(["notice"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
-    "go_test",
 )
 
 go_library(
@@ -38,14 +37,3 @@ go_library(
         "//vendor:github.com/prometheus/client_golang/prometheus",
     ],
 )
-
-go_test(
-    name = "go_default_test",
-    srcs = ["swagger_test.go"],
-    library = "go_default_library",
-    tags = ["automanaged"],
-    deps = [
-        "//pkg/genericapiserver/mux:go_default_library",
-        "//vendor:github.com/stretchr/testify/assert",
-    ],
-)

pkg/genericapiserver/routes/swagger.go

@@ -27,23 +27,11 @@ import (
 // register their own web services into the Kubernetes mux prior to initialization
 // of swagger, so that other resource types show up in the documentation.
 type Swagger struct {
-	ExternalAddress string
+	Config *swagger.Config
 }
 
 // Install adds the SwaggerUI webservice to the given mux.
 func (s Swagger) Install(c *mux.APIContainer) {
-	swagger.RegisterSwaggerService(swagger.Config{
+	s.Config.WebServices = c.RegisteredWebServices()
-		WebServicesUrl:  "https://" + s.ExternalAddress,
+	swagger.RegisterSwaggerService(*s.Config, c.Container)
-		WebServices:     c.RegisteredWebServices(),
-		ApiPath:         "/swaggerapi/",
-		SwaggerPath:     "/swaggerui/",
-		SwaggerFilePath: "/swagger-ui/",
-		SchemaFormatHandler: func(typeName string) string {
-			switch typeName {
-			case "metav1.Time", "*metav1.Time":
-				return "date-time"
-			}
-			return ""
-		},
-	}, c.Container)
 }

pkg/genericapiserver/routes/swagger_test.go

@@ -1,48 +0,0 @@
-/*
-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.
-*/
-
-package routes
-
-import (
-	"net/http"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-
-	genericmux "k8s.io/kubernetes/pkg/genericapiserver/mux"
-)
-
-// TestInstallSwaggerAPI verifies that the swagger api is added
-// at the proper endpoint.
-func TestInstallSwaggerAPI(t *testing.T) {
-	assert := assert.New(t)
-
-	// Install swagger and test
-	c := genericmux.NewAPIContainer(http.NewServeMux(), nil)
-	Swagger{ExternalAddress: "1.2.3.4"}.Install(c)
-	ws := c.RegisteredWebServices()
-	if assert.NotEqual(0, len(ws), "SwaggerAPI not installed.") {
-		assert.Equal("/swaggerapi/", ws[0].RootPath(), "SwaggerAPI did not install to the proper path. %s != /swaggerapi", ws[0].RootPath())
-	}
-
-	// Empty externalHost verification
-	c = genericmux.NewAPIContainer(http.NewServeMux(), nil)
-	Swagger{ExternalAddress: ""}.Install(c)
-	ws = c.RegisteredWebServices()
-	if assert.NotEqual(0, len(ws), "SwaggerAPI not installed.") {
-		assert.Equal("/swaggerapi/", ws[0].RootPath(), "SwaggerAPI did not install to the proper path. %s != /swaggerapi", ws[0].RootPath())
-	}
-}

pkg/master/master_test.go

@@ -314,15 +314,16 @@ func TestValidOpenAPISpec(t *testing.T) {
 	_, etcdserver, config, assert := setUp(t)
 	defer etcdserver.Terminate(t)
 
-	config.GenericConfig.OpenAPIConfig.Definitions = openapigen.OpenAPIDefinitions
-	config.GenericConfig.EnableOpenAPISupport = true
 	config.GenericConfig.EnableIndex = true
+	config.GenericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(openapigen.OpenAPIDefinitions)
 	config.GenericConfig.OpenAPIConfig.Info = &spec.Info{
 		InfoProps: spec.InfoProps{
 			Title:   "Kubernetes",
 			Version: "unversioned",
 		},
 	}
+	config.GenericConfig.SwaggerConfig = genericapiserver.DefaultSwaggerConfig()
+
 	master, err := config.Complete().New()
 	if err != nil {
 		t.Fatalf("Error in bringing up the master: %v", err)

test/integration/framework/master_utils.go

@@ -184,8 +184,7 @@ func startMasterOrDie(masterConfig *master.Config, incomingServer *httptest.Serv
 		masterConfig = NewMasterConfig()
 		masterConfig.GenericConfig.EnableProfiling = true
 		masterConfig.GenericConfig.EnableMetrics = true
-		masterConfig.GenericConfig.EnableSwaggerSupport = true
+		masterConfig.GenericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(openapi.OpenAPIDefinitions)
-		masterConfig.GenericConfig.EnableOpenAPISupport = true
 		masterConfig.GenericConfig.OpenAPIConfig.Info = &spec.Info{
 			InfoProps: spec.InfoProps{
 				Title:   "Kubernetes",
@@ -198,6 +197,7 @@ func startMasterOrDie(masterConfig *master.Config, incomingServer *httptest.Serv
 			},
 		}
 		masterConfig.GenericConfig.OpenAPIConfig.Definitions = openapi.OpenAPIDefinitions
+		masterConfig.GenericConfig.SwaggerConfig = genericapiserver.DefaultSwaggerConfig()
 	}
 
 	// set the loopback client config

test/integration/openshift/openshift_test.go

@@ -28,8 +28,7 @@ import (
 func TestMasterExportsSymbols(t *testing.T) {
 	_ = &master.Config{
 		GenericConfig: &genericapiserver.Config{
-			EnableSwaggerSupport: false,
+			EnableMetrics: true,
-			EnableMetrics:        true,
 		},
 		EnableCoreControllers: false,
 		EnableUISupport:       false,