From 18177e2bdeafbddeb3d66fec0b8cb88794cd69ff Mon Sep 17 00:00:00 2001 From: deads2k Date: Tue, 23 May 2017 15:16:45 -0400 Subject: [PATCH] move CRD behind TPR --- cmd/kube-apiserver/app/server.go | 28 +++--- .../cmd/federation-apiserver/app/server.go | 4 +- pkg/master/master.go | 4 +- pkg/master/master_openapi_test.go | 2 +- .../src/k8s.io/apiserver/pkg/server/config.go | 19 ++-- .../apiserver/pkg/server/config_test.go | 10 +- .../apiserver/pkg/server/genericapiserver.go | 5 +- .../pkg/server/genericapiserver_test.go | 14 +-- .../k8s.io/apiserver/pkg/server/handler.go | 93 ++++++++++++++++--- .../k8s.io/apiserver/pkg/server/healthz.go | 2 +- .../src/k8s.io/apiserver/pkg/server/mux/BUILD | 1 + .../apiserver/pkg/server/mux/pathrecorder.go | 15 ++- .../pkg/server/mux/pathrecorder_test.go | 6 +- .../pkg/server/options/serving_test.go | 2 +- .../pkg/apiserver/apiserver.go | 18 ++-- .../pkg/apiserver/apiserver.go | 28 +++++- .../pkg/apiserver/apiserver.go | 2 +- 17 files changed, 175 insertions(+), 78 deletions(-) diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index c8f3e5c326c..1a087b736cc 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -105,8 +105,18 @@ func Run(runOptions *options.ServerRunOptions, stopCh <-chan struct{}) error { return err } - // kubeAPIServer is at the base for now. This ensures that CustomResourceDefinitions trump TPRs - kubeAPIServer, err := CreateKubeAPIServer(kubeAPIServerConfig, genericapiserver.EmptyDelegate, sharedInformers) + // TPRs are enabled and not yet beta, since this these are the successor, they fall under the same enablement rule + // If additional API servers are added, they should be gated. + apiExtensionsConfig, err := createAPIExtensionsConfig(*kubeAPIServerConfig.GenericConfig, runOptions) + if err != nil { + return err + } + apiExtensionsServer, err := createAPIExtensionsServer(apiExtensionsConfig, genericapiserver.EmptyDelegate) + if err != nil { + return err + } + + kubeAPIServer, err := CreateKubeAPIServer(kubeAPIServerConfig, apiExtensionsServer.GenericAPIServer, sharedInformers) if err != nil { return err } @@ -128,24 +138,12 @@ func Run(runOptions *options.ServerRunOptions, stopCh <-chan struct{}) error { // this wires up openapi kubeAPIServer.GenericAPIServer.PrepareRun() - // TPRs are enabled and not yet beta, since this these are the successor, they fall under the same enablement rule - // Subsequent API servers in between here and kube-apiserver will need to be gated. - // These come first so that if someone registers both a TPR and a CRD, the CRD is preferred. - apiExtensionsConfig, err := createAPIExtensionsConfig(*kubeAPIServerConfig.GenericConfig, runOptions) - if err != nil { - return err - } - apiExtensionsServer, err := createAPIExtensionsServer(apiExtensionsConfig, kubeAPIServer.GenericAPIServer) - if err != nil { - return err - } - // aggregator comes last in the chain aggregatorConfig, err := createAggregatorConfig(*kubeAPIServerConfig.GenericConfig, runOptions) if err != nil { return err } - aggregatorServer, err := createAggregatorServer(aggregatorConfig, apiExtensionsServer.GenericAPIServer, sharedInformers, apiExtensionsServer.Informers) + aggregatorServer, err := createAggregatorServer(aggregatorConfig, kubeAPIServer.GenericAPIServer, sharedInformers, apiExtensionsServer.Informers) if err != nil { // we don't need special handling for innerStopCh because the aggregator server doesn't create any go routines return err diff --git a/federation/cmd/federation-apiserver/app/server.go b/federation/cmd/federation-apiserver/app/server.go index dbc7456b93d..80f60191829 100644 --- a/federation/cmd/federation-apiserver/app/server.go +++ b/federation/cmd/federation-apiserver/app/server.go @@ -227,12 +227,12 @@ func NonBlockingRun(s *options.ServerRunOptions, stopCh <-chan struct{}) error { cachesize.SetWatchCacheSizes(s.GenericServerRunOptions.WatchCacheSizes) } - m, err := genericConfig.Complete().New(genericapiserver.EmptyDelegate) + m, err := genericConfig.Complete().New("federation", genericapiserver.EmptyDelegate) if err != nil { return err } - routes.UIRedirect{}.Install(m.Handler.PostGoRestfulMux) + routes.UIRedirect{}.Install(m.Handler.NonGoRestfulMux) routes.Logs{}.Install(m.Handler.GoRestfulContainer) apiResourceConfigSource := storageFactory.APIResourceConfigSource diff --git a/pkg/master/master.go b/pkg/master/master.go index 26d114ca29f..4932ff5f4cc 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -211,13 +211,13 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) return nil, fmt.Errorf("Master.New() called with empty config.KubeletClientConfig") } - s, err := c.Config.GenericConfig.SkipComplete().New(delegationTarget) // completion is done in Complete, no need for a second time + s, err := c.Config.GenericConfig.SkipComplete().New("kube-apiserver", delegationTarget) // completion is done in Complete, no need for a second time if err != nil { return nil, err } if c.EnableUISupport { - routes.UIRedirect{}.Install(s.Handler.PostGoRestfulMux) + routes.UIRedirect{}.Install(s.Handler.NonGoRestfulMux) } if c.EnableLogsSupport { routes.Logs{}.Install(s.Handler.GoRestfulContainer) diff --git a/pkg/master/master_openapi_test.go b/pkg/master/master_openapi_test.go index 39ddb686c53..4629e4159eb 100644 --- a/pkg/master/master_openapi_test.go +++ b/pkg/master/master_openapi_test.go @@ -60,7 +60,7 @@ func TestValidOpenAPISpec(t *testing.T) { } // make sure swagger.json is not registered before calling PrepareRun. - server := httptest.NewServer(apirequest.WithRequestContext(master.GenericAPIServer.Handler.GoRestfulContainer.ServeMux, master.GenericAPIServer.RequestContextMapper())) + server := httptest.NewServer(apirequest.WithRequestContext(master.GenericAPIServer.Handler.Director, master.GenericAPIServer.RequestContextMapper())) defer server.Close() resp, err := http.Get(server.URL + "/swagger.json") if !assert.NoError(err) { diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 30208ba1bac..9708277f37a 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -251,7 +251,7 @@ func DefaultOpenAPIConfig(getDefinitions openapicommon.GetOpenAPIDefinitions, sc // WebServices set. func DefaultSwaggerConfig() *swagger.Config { return &swagger.Config{ - ApiPath: "/swaggerapi/", + ApiPath: "/swaggerapi", SwaggerPath: "/swaggerui/", SwaggerFilePath: "/swagger-ui/", SchemaFormatHandler: func(typeName string) string { @@ -369,7 +369,8 @@ func (c *Config) SkipComplete() completedConfig { } // New creates a new server which logically combines the handling chain with the passed server. -func (c completedConfig) New(delegationTarget DelegationTarget) (*GenericAPIServer, error) { +// name is used to differentiate for logging. The handler chain in particular can be difficult as it starts delgating. +func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*GenericAPIServer, error) { // The delegationTarget and the config must agree on the RequestContextMapper if c.Serializer == nil { @@ -382,7 +383,7 @@ func (c completedConfig) New(delegationTarget DelegationTarget) (*GenericAPIServ handlerChainBuilder := func(handler http.Handler) http.Handler { return c.BuildHandlerChainFunc(handler, c.Config) } - apiServerHandler := NewAPIServerHandler(c.RequestContextMapper, c.Serializer, handlerChainBuilder, delegationTarget.UnprotectedHandler()) + apiServerHandler := NewAPIServerHandler(name, c.RequestContextMapper, c.Serializer, handlerChainBuilder, delegationTarget.UnprotectedHandler()) s := &GenericAPIServer{ discoveryAddresses: c.DiscoveryAddresses, @@ -449,7 +450,7 @@ func (c completedConfig) New(delegationTarget DelegationTarget) (*GenericAPIServ // use the UnprotectedHandler from the delegation target to ensure that we don't attempt to double authenticator, authorize, // or some other part of the filter chain in delegation cases. if delegationTarget.UnprotectedHandler() == nil && c.EnableIndex { - s.Handler.PostGoRestfulMux.NotFoundHandler(routes.IndexLister{ + s.Handler.NonGoRestfulMux.NotFoundHandler(routes.IndexLister{ StatusCode: http.StatusNotFound, PathProvider: s.listedPathProvider, }) @@ -478,22 +479,22 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler { func installAPI(s *GenericAPIServer, c *Config) { if c.EnableIndex { - routes.Index{}.Install(s.listedPathProvider, s.Handler.PostGoRestfulMux) + routes.Index{}.Install(s.listedPathProvider, s.Handler.NonGoRestfulMux) } if c.SwaggerConfig != nil && c.EnableSwaggerUI { - routes.SwaggerUI{}.Install(s.Handler.PostGoRestfulMux) + routes.SwaggerUI{}.Install(s.Handler.NonGoRestfulMux) } if c.EnableProfiling { - routes.Profiling{}.Install(s.Handler.PostGoRestfulMux) + routes.Profiling{}.Install(s.Handler.NonGoRestfulMux) if c.EnableContentionProfiling { goruntime.SetBlockProfileRate(1) } } if c.EnableMetrics { if c.EnableProfiling { - routes.MetricsWithReset{}.Install(s.Handler.PostGoRestfulMux) + routes.MetricsWithReset{}.Install(s.Handler.NonGoRestfulMux) } else { - routes.DefaultMetrics{}.Install(s.Handler.PostGoRestfulMux) + routes.DefaultMetrics{}.Install(s.Handler.NonGoRestfulMux) } } routes.Version{Version: c.Version}.Install(s.Handler.GoRestfulContainer) diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index 8597ec541eb..c94046d0b8e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -52,11 +52,11 @@ func TestNewWithDelegate(t *testing.T) { return fmt.Errorf("delegate failed healthcheck") })) - delegateServer, err := delegateConfig.SkipComplete().New(EmptyDelegate) + delegateServer, err := delegateConfig.SkipComplete().New("test", EmptyDelegate) if err != nil { t.Fatal(err) } - delegateServer.Handler.PostGoRestfulMux.HandleFunc("/foo", func(w http.ResponseWriter, _ *http.Request) { + delegateServer.Handler.NonGoRestfulMux.HandleFunc("/foo", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusForbidden) }) @@ -81,11 +81,11 @@ func TestNewWithDelegate(t *testing.T) { return fmt.Errorf("wrapping failed healthcheck") })) - wrappingServer, err := wrappingConfig.Complete().New(delegateServer) + wrappingServer, err := wrappingConfig.Complete().New("test", delegateServer) if err != nil { t.Fatal(err) } - wrappingServer.Handler.PostGoRestfulMux.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) { + wrappingServer.Handler.NonGoRestfulMux.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) }) @@ -113,7 +113,7 @@ func TestNewWithDelegate(t *testing.T) { "/healthz/poststarthook/generic-apiserver-start-informers", "/healthz/poststarthook/wrapping-post-start-hook", "/healthz/wrapping-health", - "/swaggerapi/" + "/swaggerapi" ] }`, t) checkPath(server.URL+"/healthz", http.StatusInternalServerError, `[+]ping ok diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 192180fe0f3..29a6fed127f 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -168,7 +168,8 @@ type DelegationTarget interface { } func (s *GenericAPIServer) UnprotectedHandler() http.Handler { - return s.Handler.GoRestfulContainer.ServeMux + // when we delegate, we need the server we're delegating to choose whether or not to use gorestful + return s.Handler.Director } func (s *GenericAPIServer) PostStartHooks() map[string]postStartHookEntry { return s.postStartHooks @@ -235,7 +236,7 @@ func (s *GenericAPIServer) PrepareRun() preparedGenericAPIServer { if s.openAPIConfig != nil { routes.OpenAPI{ Config: s.openAPIConfig, - }.Install(s.Handler.GoRestfulContainer, s.Handler.PostGoRestfulMux) + }.Install(s.Handler.GoRestfulContainer, s.Handler.NonGoRestfulMux) } s.installHealthz() diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go index c1f355f0ed4..b8aed64c1f8 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go @@ -114,7 +114,7 @@ func setUp(t *testing.T) (*etcdtesting.EtcdTestServer, Config, *assert.Assertion func newMaster(t *testing.T) (*GenericAPIServer, *etcdtesting.EtcdTestServer, Config, *assert.Assertions) { etcdserver, config, assert := setUp(t) - s, err := config.Complete().New(EmptyDelegate) + s, err := config.Complete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } @@ -146,7 +146,7 @@ func TestInstallAPIGroups(t *testing.T) { config.LegacyAPIGroupPrefixes = sets.NewString("/apiPrefix") config.DiscoveryAddresses = discovery.DefaultAddresses{DefaultAddress: "ExternalAddress"} - s, err := config.SkipComplete().New(EmptyDelegate) + s, err := config.SkipComplete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } @@ -309,7 +309,7 @@ func TestPrepareRun(t *testing.T) { assert.NotNil(config.SwaggerConfig) - server := httptest.NewServer(s.Handler.GoRestfulContainer.ServeMux) + server := httptest.NewServer(s.Handler.Director) defer server.Close() done := make(chan struct{}) @@ -347,13 +347,13 @@ func TestCustomHandlerChain(t *testing.T) { called = true }) - s, err := config.SkipComplete().New(EmptyDelegate) + s, err := config.SkipComplete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } - s.Handler.PostGoRestfulMux.Handle("/nonswagger", handler) - s.Handler.PostGoRestfulMux.Handle("/secret", handler) + s.Handler.NonGoRestfulMux.Handle("/nonswagger", handler) + s.Handler.NonGoRestfulMux.Handle("/secret", handler) type Test struct { handler http.Handler @@ -402,7 +402,7 @@ func TestNotRestRoutesHaveAuth(t *testing.T) { kubeVersion := fakeVersion() config.Version = &kubeVersion - s, err := config.SkipComplete().New(EmptyDelegate) + s, err := config.SkipComplete().New("test", EmptyDelegate) if err != nil { t.Fatalf("Error in bringing up the server: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/server/handler.go b/staging/src/k8s.io/apiserver/pkg/server/handler.go index 148f8a020a9..48d1408a45c 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/handler.go +++ b/staging/src/k8s.io/apiserver/pkg/server/handler.go @@ -23,6 +23,7 @@ import ( "net/http" rt "runtime" "sort" + "strings" "github.com/emicklei/go-restful" "github.com/golang/glog" @@ -37,27 +38,45 @@ import ( ) // APIServerHandlers holds the different http.Handlers used by the API server. -// This includes the full handler chain, the gorestful handler (used for the API) which falls through to the postGoRestful handler -// and the postGoRestful handler (which can contain a fallthrough of its own) +// This includes the full handler chain, the director (which chooses between gorestful and nonGoRestful, +// the gorestful handler (used for the API) which falls through to the nonGoRestful handler on unregistered paths, +// and the nonGoRestful handler (which can contain a fallthrough of its own) +// FullHandlerChain -> Director -> {GoRestfulContainer,NonGoRestfulMux} based on inspection of registered web services type APIServerHandler struct { // FullHandlerChain is the one that is eventually served with. It should include the full filter - // chain and then call the GoRestfulContainer. + // chain and then call the Director. FullHandlerChain http.Handler - // The registered APIs + // The registered APIs. InstallAPIs uses this. Other servers probably shouldn't access this directly. GoRestfulContainer *restful.Container - // PostGoRestfulMux is the final HTTP handler in the chain. + // NonGoRestfulMux is the final HTTP handler in the chain. // It comes after all filters and the API handling - PostGoRestfulMux *mux.PathRecorderMux + // This is where other servers can attach handler to various parts of the chain. + NonGoRestfulMux *mux.PathRecorderMux + + // Director is here so that we can properly handle fall through and proxy cases. + // This looks a bit bonkers, but here's what's happening. We need to have /apis handling registered in gorestful in order to have + // swagger generated for compatibility. Doing that with `/apis` as a webservice, means that it forcibly 404s (no defaulting allowed) + // all requests which are not /apis or /apis/. We need those calls to fall through behind goresful for proper delegation. Trying to + // register for a pattern which includes everything behind it doesn't work because gorestful negotiates for verbs and content encoding + // and all those things go crazy when gorestful really just needs to pass through. In addition, openapi enforces unique verb constraints + // which we don't fit into and it still muddies up swagger. Trying to switch the webservices into a route doesn't work because the + // containing webservice faces all the same problems listed above. + // This leads to the crazy thing done here. Our mux does what we need, so we'll place it in front of gorestful. It will introspect to + // decide if the the route is likely to be handled by goresful and route there if needed. Otherwise, it goes to PostGoRestful mux in + // order to handle "normal" paths and delegation. Hopefully no API consumers will ever have to deal with this level of detail. I think + // we should consider completely removing gorestful. + // Other servers should only use this opaquely to delegate to an API server. + Director http.Handler } // HandlerChainBuilderFn is used to wrap the GoRestfulContainer handler using the provided handler chain. // It is normally used to apply filtering like authentication and authorization type HandlerChainBuilderFn func(apiHandler http.Handler) http.Handler -func NewAPIServerHandler(contextMapper request.RequestContextMapper, s runtime.NegotiatedSerializer, handlerChainBuilder HandlerChainBuilderFn, notFoundHandler http.Handler) *APIServerHandler { - postGoRestfulMux := genericmux.NewPathRecorderMux() +func NewAPIServerHandler(name string, contextMapper request.RequestContextMapper, s runtime.NegotiatedSerializer, handlerChainBuilder HandlerChainBuilderFn, notFoundHandler http.Handler) *APIServerHandler { + nonGoRestfulMux := genericmux.NewPathRecorderMux(name) if notFoundHandler != nil { - postGoRestfulMux.NotFoundHandler(notFoundHandler) + nonGoRestfulMux.NotFoundHandler(notFoundHandler) } gorestfulContainer := restful.NewContainer() @@ -74,14 +93,17 @@ func NewAPIServerHandler(contextMapper request.RequestContextMapper, s runtime.N serviceErrorHandler(ctx, s, serviceErr, request, response) }) - // register the defaultHandler for everything. This will allow an unhandled request to fall through to another handler instead of - // ending up with a forced 404 - gorestfulContainer.Handle("/", postGoRestfulMux) + director := director{ + name: name, + goRestfulContainer: gorestfulContainer, + nonGoRestfulMux: nonGoRestfulMux, + } return &APIServerHandler{ - FullHandlerChain: handlerChainBuilder(gorestfulContainer.ServeMux), + FullHandlerChain: handlerChainBuilder(director), GoRestfulContainer: gorestfulContainer, - PostGoRestfulMux: postGoRestfulMux, + NonGoRestfulMux: nonGoRestfulMux, + Director: director, } } @@ -92,12 +114,53 @@ func (a *APIServerHandler) ListedPaths() []string { for _, ws := range a.GoRestfulContainer.RegisteredWebServices() { handledPaths = append(handledPaths, ws.RootPath()) } - handledPaths = append(handledPaths, a.PostGoRestfulMux.ListedPaths()...) + handledPaths = append(handledPaths, a.NonGoRestfulMux.ListedPaths()...) sort.Strings(handledPaths) return handledPaths } +type director struct { + name string + goRestfulContainer *restful.Container + nonGoRestfulMux *mux.PathRecorderMux +} + +func (d director) ServeHTTP(w http.ResponseWriter, req *http.Request) { + path := req.URL.Path + + // check to see if our webservices want to claim this path + for _, ws := range d.goRestfulContainer.RegisteredWebServices() { + switch { + case ws.RootPath() == "/apis": + // if we are exactly /apis or /apis/, then we need special handling in loop. + // normally these are passed to the nonGoRestfulMux, but if discovery is enabled, it will go directly. + // We can't rely on a prefix match since /apis matches everything (see the big comment on Director above) + if path == "/apis" || path == "/apis/" { + glog.V(5).Infof("%v: %v %q satisfied by gorestful with webservice %v", d.name, req.Method, path, ws.RootPath()) + // don't use servemux here because gorestful servemuxes get messed up when removing webservices + // TODO fix gorestful, remove TPRs, or stop using gorestful + d.goRestfulContainer.Dispatch(w, req) + return + } + + case strings.HasPrefix(path, ws.RootPath()): + // ensure an exact match or a path boundary match + if len(path) == len(ws.RootPath()) || path[len(ws.RootPath())] == '/' { + glog.V(5).Infof("%v: %v %q satisfied by gorestful with webservice %v", d.name, req.Method, path, ws.RootPath()) + // don't use servemux here because gorestful servemuxes get messed up when removing webservices + // TODO fix gorestful, remove TPRs, or stop using gorestful + d.goRestfulContainer.Dispatch(w, req) + return + } + } + } + + // if we didn't find a match, then we just skip gorestful altogether + glog.V(5).Infof("%v: %v %q satisfied by nonGoRestful", d.name, req.Method, path) + d.nonGoRestfulMux.ServeHTTP(w, req) +} + //TODO: Unify with RecoverPanics? func logStackOnRecover(s runtime.NegotiatedSerializer, panicReason interface{}, w http.ResponseWriter) { var buffer bytes.Buffer diff --git a/staging/src/k8s.io/apiserver/pkg/server/healthz.go b/staging/src/k8s.io/apiserver/pkg/server/healthz.go index 527917478d3..43e102b5cc7 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/healthz.go +++ b/staging/src/k8s.io/apiserver/pkg/server/healthz.go @@ -41,5 +41,5 @@ func (s *GenericAPIServer) installHealthz() { defer s.healthzLock.Unlock() s.healthzCreated = true - healthz.InstallHandler(s.Handler.PostGoRestfulMux, s.healthzChecks...) + healthz.InstallHandler(s.Handler.NonGoRestfulMux, s.healthzChecks...) } diff --git a/staging/src/k8s.io/apiserver/pkg/server/mux/BUILD b/staging/src/k8s.io/apiserver/pkg/server/mux/BUILD index 16b6684df21..de2309ec3f5 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/mux/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/server/mux/BUILD @@ -24,6 +24,7 @@ go_library( ], tags = ["automanaged"], deps = [ + "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], diff --git a/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder.go b/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder.go index 4583a876cb5..2f0eb7aa5b2 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder.go +++ b/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder.go @@ -25,12 +25,17 @@ import ( "sync" "sync/atomic" + "github.com/golang/glog" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" ) // PathRecorderMux wraps a mux object and records the registered exposedPaths. type PathRecorderMux struct { + // name is used for logging so you can trace requests through + name string + lock sync.Mutex notFoundHandler http.Handler pathToHandler map[string]http.Handler @@ -53,6 +58,9 @@ type PathRecorderMux struct { // pathHandler is an http.Handler that will satify requests first by exact match, then by prefix, // then by notFoundHandler type pathHandler struct { + // muxName is used for logging so you can trace requests through + muxName string + // pathToHandler is a map of exactly matching request to its handler pathToHandler map[string]http.Handler @@ -72,8 +80,9 @@ type prefixHandler struct { } // NewPathRecorderMux creates a new PathRecorderMux -func NewPathRecorderMux() *PathRecorderMux { +func NewPathRecorderMux(name string) *PathRecorderMux { ret := &PathRecorderMux{ + name: name, pathToHandler: map[string]http.Handler{}, prefixToHandler: map[string]http.Handler{}, mux: atomic.Value{}, @@ -104,6 +113,7 @@ func (m *PathRecorderMux) trackCallers(path string) { // not be consistent func (m *PathRecorderMux) refreshMuxLocked() { newMux := &pathHandler{ + muxName: m.name, pathToHandler: map[string]http.Handler{}, prefixHandlers: []prefixHandler{}, notFoundHandler: http.NotFoundHandler(), @@ -227,17 +237,20 @@ func (m *PathRecorderMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { // ServeHTTP makes it an http.Handler func (h *pathHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if exactHandler, ok := h.pathToHandler[r.URL.Path]; ok { + glog.V(5).Infof("%v: %q satisfied by exact match", h.muxName, r.URL.Path) exactHandler.ServeHTTP(w, r) return } for _, prefixHandler := range h.prefixHandlers { if strings.HasPrefix(r.URL.Path, prefixHandler.prefix) { + glog.V(5).Infof("%v: %q satisfied by prefix %v", h.muxName, r.URL.Path, prefixHandler.prefix) prefixHandler.handler.ServeHTTP(w, r) return } } + glog.V(5).Infof("%v: %q satisfied by NotFoundHandler", h.muxName, r.URL.Path) h.notFoundHandler.ServeHTTP(w, r) } diff --git a/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder_test.go b/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder_test.go index 6d15b47e24c..1ff767915e3 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/mux/pathrecorder_test.go @@ -25,7 +25,7 @@ import ( ) func TestSecretHandlers(t *testing.T) { - c := NewPathRecorderMux() + c := NewPathRecorderMux("test") c.UnlistedHandleFunc("/secret", func(http.ResponseWriter, *http.Request) {}) c.HandleFunc("/nonswagger", func(http.ResponseWriter, *http.Request) {}) assert.NotContains(t, c.ListedPaths(), "/secret") @@ -36,7 +36,7 @@ func TestUnregisterHandlers(t *testing.T) { first := 0 second := 0 - c := NewPathRecorderMux() + c := NewPathRecorderMux("test") s := httptest.NewServer(c) defer s.Close() @@ -69,7 +69,7 @@ func TestUnregisterHandlers(t *testing.T) { } func TestPrefixHandlers(t *testing.T) { - c := NewPathRecorderMux() + c := NewPathRecorderMux("test") s := httptest.NewServer(c) defer s.Close() diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/serving_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/serving_test.go index ec4d8e8edca..c1869dd1bd5 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/serving_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/serving_test.go @@ -475,7 +475,7 @@ NextTest: return } - s, err := config.Complete().New(server.EmptyDelegate) + s, err := config.Complete().New("test", server.EmptyDelegate) if err != nil { t.Errorf("%q - failed creating the server: %v", title, err) return diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go index 87e0d3e695a..8c11209ffc2 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go @@ -130,7 +130,7 @@ func (c *Config) SkipComplete() completedConfig { // New returns a new instance of APIAggregator from the given config. func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.DelegationTarget) (*APIAggregator, error) { - genericServer, err := c.Config.GenericConfig.SkipComplete().New(delegationTarget) // completion is done in Complete, no need for a second time + genericServer, err := c.Config.GenericConfig.SkipComplete().New("kube-aggregator", delegationTarget) // completion is done in Complete, no need for a second time if err != nil { return nil, err } @@ -174,8 +174,8 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg lister: s.lister, mapper: s.contextMapper, } - s.GenericAPIServer.Handler.PostGoRestfulMux.Handle("/apis", apisHandler) - s.GenericAPIServer.Handler.PostGoRestfulMux.UnlistedHandle("/apis/", apisHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.Handle("/apis", apisHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.UnlistedHandle("/apis/", apisHandler) apiserviceRegistrationController := NewAPIServiceRegistrationController(informerFactory.Apiregistration().InternalVersion().APIServices(), kubeInformers.Core().V1().Services(), s) availableController := statuscontrollers.NewAvailableConditionController( @@ -227,8 +227,8 @@ func (s *APIAggregator) AddAPIService(apiService *apiregistration.APIService, de } proxyHandler.updateAPIService(apiService, destinationHost) s.proxyHandlers[apiService.Name] = proxyHandler - s.GenericAPIServer.Handler.PostGoRestfulMux.Handle(proxyPath, proxyHandler) - s.GenericAPIServer.Handler.PostGoRestfulMux.UnlistedHandlePrefix(proxyPath+"/", proxyHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.Handle(proxyPath, proxyHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.UnlistedHandlePrefix(proxyPath+"/", proxyHandler) // if we're dealing with the legacy group, we're done here if apiService.Name == legacyAPIServiceName { @@ -250,8 +250,8 @@ func (s *APIAggregator) AddAPIService(apiService *apiregistration.APIService, de contextMapper: s.contextMapper, } // aggregation is protected - s.GenericAPIServer.Handler.PostGoRestfulMux.Handle(groupPath, groupDiscoveryHandler) - s.GenericAPIServer.Handler.PostGoRestfulMux.UnlistedHandle(groupPath+"/", groupDiscoveryHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.Handle(groupPath, groupDiscoveryHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.UnlistedHandle(groupPath+"/", groupDiscoveryHandler) s.handledGroups.Insert(apiService.Spec.Group) } @@ -265,8 +265,8 @@ func (s *APIAggregator) RemoveAPIService(apiServiceName string) { if apiServiceName == legacyAPIServiceName { proxyPath = "/api" } - s.GenericAPIServer.Handler.PostGoRestfulMux.Unregister(proxyPath) - s.GenericAPIServer.Handler.PostGoRestfulMux.Unregister(proxyPath + "/") + s.GenericAPIServer.Handler.NonGoRestfulMux.Unregister(proxyPath) + s.GenericAPIServer.Handler.NonGoRestfulMux.Unregister(proxyPath + "/") delete(s.proxyHandlers, apiServiceName) // TODO unregister group level discovery when there are no more versions for the group diff --git a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go index dec369eb56f..56a6ec4954b 100644 --- a/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/kube-apiextensions-server/pkg/apiserver/apiserver.go @@ -17,9 +17,13 @@ limitations under the License. package apiserver import ( + "fmt" "net/http" + "os" "time" + "github.com/golang/glog" + "k8s.io/apimachinery/pkg/apimachinery/announced" "k8s.io/apimachinery/pkg/apimachinery/registered" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -107,7 +111,7 @@ func (c *Config) SkipComplete() completedConfig { // New returns a new instance of CustomResourceDefinitions from the given config. func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) (*CustomResourceDefinitions, error) { - genericServer, err := c.Config.GenericConfig.SkipComplete().New(delegationTarget) // completion is done in Complete, no need for a second time + genericServer, err := c.Config.GenericConfig.SkipComplete().New("kube-apiextensions-server", delegationTarget) // completion is done in Complete, no need for a second time if err != nil { return nil, err } @@ -130,7 +134,18 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) crdClient, err := internalclientset.NewForConfig(s.GenericAPIServer.LoopbackClientConfig) if err != nil { - return nil, err + // it's really bad that this is leaking here, but until we can fix the test (which I'm pretty sure isn't even testing what it wants to test), + // we need to be able to move forward + kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS") + if len(kubeAPIVersions) == 0 { + return nil, fmt.Errorf("failed to create clientset: %v", err) + } + + // KUBE_API_VERSIONS is used in test-update-storage-objects.sh, disabling a number of API + // groups. This leads to a nil client above and undefined behaviour further down. + // + // TODO: get rid of KUBE_API_VERSIONS or define sane behaviour if set + glog.Errorf("Failed to create clientset with KUBE_API_VERSIONS=%q. KUBE_API_VERSIONS is only for testing. Things will break.", kubeAPIVersions) } s.Informers = internalinformers.NewSharedInformerFactory(crdClient, 5*time.Minute) @@ -156,8 +171,8 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) c.CRDRESTOptionsGetter, c.GenericConfig.AdmissionControl, ) - s.GenericAPIServer.Handler.PostGoRestfulMux.Handle("/apis", crdHandler) - s.GenericAPIServer.Handler.PostGoRestfulMux.HandlePrefix("/apis/", crdHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.Handle("/apis", crdHandler) + s.GenericAPIServer.Handler.NonGoRestfulMux.HandlePrefix("/apis/", crdHandler) crdController := NewDiscoveryController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), versionDiscoveryHandler, groupDiscoveryHandler, c.GenericConfig.RequestContextMapper) namingController := status.NewNamingConditionController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient) @@ -167,6 +182,11 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) crdHandler, ) + // this only happens when KUBE_API_VERSIONS is set. We must return without adding poststarthooks which would affect healthz + if crdClient == nil { + return s, nil + } + s.GenericAPIServer.AddPostStartHook("start-apiextensions-informers", func(context genericapiserver.PostStartHookContext) error { s.Informers.Start(context.StopCh) return nil diff --git a/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go b/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go index f014e7f92b9..619aece7f0b 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/apiserver/apiserver.go @@ -90,7 +90,7 @@ func (c *Config) SkipComplete() completedConfig { // New returns a new instance of WardleServer from the given config. func (c completedConfig) New() (*WardleServer, error) { - genericServer, err := c.Config.GenericConfig.SkipComplete().New(genericapiserver.EmptyDelegate) // completion is done in Complete, no need for a second time + genericServer, err := c.Config.GenericConfig.SkipComplete().New("sample-apiserver", genericapiserver.EmptyDelegate) // completion is done in Complete, no need for a second time if err != nil { return nil, err }