From c2874941620c264813b6bfa42913f95fd0cc2317 Mon Sep 17 00:00:00 2001 From: deads2k Date: Wed, 2 Aug 2017 08:55:34 -0400 Subject: [PATCH] cleanup dead installer code --- .../src/k8s.io/apiserver/pkg/endpoints/BUILD | 1 - .../apiserver/pkg/endpoints/apiserver_test.go | 85 ------------------- .../apiserver/pkg/endpoints/groupversion.go | 56 ++---------- .../apiserver/pkg/endpoints/installer.go | 12 +-- .../apiserver/pkg/server/genericapiserver.go | 3 - 5 files changed, 13 insertions(+), 144 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD index 465cbc9b13b..c8da7be8586 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/BUILD @@ -70,7 +70,6 @@ go_library( tags = ["automanaged"], deps = [ "//vendor/github.com/emicklei/go-restful:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index b28dbc1a1a1..19980d543d1 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -3228,91 +3228,6 @@ func TestCreateChecksDecode(t *testing.T) { } } -// TestUpdateREST tests that you can add new rest implementations to a pre-existing -// web service. -func TestUpdateREST(t *testing.T) { - makeGroup := func(storage map[string]rest.Storage) *APIGroupVersion { - return &APIGroupVersion{ - Storage: storage, - Root: "/" + prefix, - Creater: scheme, - Convertor: scheme, - Copier: scheme, - Defaulter: scheme, - Typer: scheme, - Linker: selfLinker, - - Admit: admissionControl, - Context: requestContextMapper, - Mapper: namespaceMapper, - - GroupVersion: newGroupVersion, - OptionsExternalVersion: &newGroupVersion, - - Serializer: codecs, - ParameterCodec: parameterCodec, - } - } - - makeStorage := func(paths ...string) map[string]rest.Storage { - storage := map[string]rest.Storage{} - for _, s := range paths { - storage[s] = &SimpleRESTStorage{} - } - return storage - } - - testREST := func(t *testing.T, container *restful.Container, barCode int) { - handler := genericapifilters.WithRequestInfo(container, newTestRequestInfoResolver(), requestContextMapper) - handler = request.WithRequestContext(handler, requestContextMapper) - - w := httptest.NewRecorder() - handler.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/foo/test"}}) - if w.Code != http.StatusOK { - t.Fatalf("expected OK: %#v", w) - } - - w = httptest.NewRecorder() - handler.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/namespaces/test/bar/test"}}) - if w.Code != barCode { - t.Errorf("expected response code %d for GET to bar but received %d", barCode, w.Code) - } - } - - storage1 := makeStorage("foo") - group1 := makeGroup(storage1) - - storage2 := makeStorage("bar") - group2 := makeGroup(storage2) - - container := restful.NewContainer() - - // install group1. Ensure that - // 1. Foo storage is accessible - // 2. Bar storage is not accessible - if err := group1.InstallREST(container); err != nil { - t.Fatal(err) - } - testREST(t, container, http.StatusNotFound) - - // update with group2. Ensure that - // 1. Foo storage is still accessible - // 2. Bar storage is now accessible - if err := group2.UpdateREST(container); err != nil { - t.Fatal(err) - } - testREST(t, container, http.StatusOK) - - // try to update a group that does not have an existing webservice with a matching prefix - // should not affect the existing registered webservice - invalidGroup := makeGroup(storage1) - invalidGroup.Root = "bad" - if err := invalidGroup.UpdateREST(container); err == nil { - t.Fatal("expected an error from UpdateREST when updating a non-existing prefix but got none") - } - testREST(t, container, http.StatusOK) -} - func TestParentResourceIsRequired(t *testing.T) { storage := &SimpleTypedStorage{ baseType: &genericapitesting.SimpleRoot{}, // a root scoped type diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go index f7d1d7b1767..851a570093a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go @@ -17,13 +17,11 @@ limitations under the License. package endpoints import ( - "fmt" "path" "time" "github.com/emicklei/go-restful" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -84,10 +82,6 @@ type APIGroupVersion struct { // match the keys in the Storage map above for subresources. SubresourceGroupVersionKind map[string]schema.GroupVersionKind - // ResourceLister is an interface that knows how to list resources - // for this API Group. - ResourceLister discovery.APIResourceLister - // EnableAPIResponseCompression indicates whether API Responses should support compression // if the client requests it via Accept-Encoding EnableAPIResponseCompression bool @@ -97,49 +91,6 @@ type APIGroupVersion struct { // It is expected that the provided path root prefix will serve all operations. Root MUST NOT end // in a slash. func (g *APIGroupVersion) InstallREST(container *restful.Container) error { - installer := g.newInstaller() - ws := installer.NewWebService() - apiResources, registrationErrors := installer.Install(ws) - lister := g.ResourceLister - if lister == nil { - lister = staticLister{apiResources} - } - versionDiscoveryHandler := discovery.NewAPIVersionHandler(g.Serializer, g.GroupVersion, lister, g.Context) - versionDiscoveryHandler.AddToWebService(ws) - container.Add(ws) - return utilerrors.NewAggregate(registrationErrors) -} - -// UpdateREST registers the REST handlers for this APIGroupVersion to an existing web service -// in the restful Container. It will use the prefix (root/version) to find the existing -// web service. If a web service does not exist within the container to support the prefix -// this method will return an error. -func (g *APIGroupVersion) UpdateREST(container *restful.Container) error { - installer := g.newInstaller() - var ws *restful.WebService = nil - - for i, s := range container.RegisteredWebServices() { - if s.RootPath() == installer.prefix { - ws = container.RegisteredWebServices()[i] - break - } - } - - if ws == nil { - return apierrors.NewInternalError(fmt.Errorf("unable to find an existing webservice for prefix %s", installer.prefix)) - } - apiResources, registrationErrors := installer.Install(ws) - lister := g.ResourceLister - if lister == nil { - lister = staticLister{apiResources} - } - versionDiscoveryHandler := discovery.NewAPIVersionHandler(g.Serializer, g.GroupVersion, lister, g.Context) - versionDiscoveryHandler.AddToWebService(ws) - return utilerrors.NewAggregate(registrationErrors) -} - -// newInstaller is a helper to create the installer. Used by InstallREST and UpdateREST. -func (g *APIGroupVersion) newInstaller() *APIInstaller { prefix := path.Join(g.Root, g.GroupVersion.Group, g.GroupVersion.Version) installer := &APIInstaller{ group: g, @@ -147,7 +98,12 @@ func (g *APIGroupVersion) newInstaller() *APIInstaller { minRequestTimeout: g.MinRequestTimeout, enableAPIResponseCompression: g.EnableAPIResponseCompression, } - return installer + + apiResources, ws, registrationErrors := installer.Install() + versionDiscoveryHandler := discovery.NewAPIVersionHandler(g.Serializer, g.GroupVersion, staticLister{apiResources}, g.Context) + versionDiscoveryHandler.AddToWebService(ws) + container.Add(ws) + return utilerrors.NewAggregate(registrationErrors) } // staticLister implements the APIResourceLister interface diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index b8f6a038689..19515f5bf2c 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -85,8 +85,10 @@ var toDiscoveryKubeVerb = map[string]string{ } // Install handlers for API resources. -func (a *APIInstaller) Install(ws *restful.WebService) (apiResources []metav1.APIResource, errors []error) { - errors = make([]error, 0) +func (a *APIInstaller) Install() ([]metav1.APIResource, *restful.WebService, []error) { + var apiResources []metav1.APIResource + var errors []error + ws := a.newWebService() proxyHandler := (&handlers.ProxyHandler{ Prefix: a.prefix + "/proxy/", @@ -112,11 +114,11 @@ func (a *APIInstaller) Install(ws *restful.WebService) (apiResources []metav1.AP apiResources = append(apiResources, *apiResource) } } - return apiResources, errors + return apiResources, ws, errors } -// NewWebService creates a new restful webservice with the api installer's prefix and version. -func (a *APIInstaller) NewWebService() *restful.WebService { +// newWebService creates a new restful webservice with the api installer's prefix and version. +func (a *APIInstaller) newWebService() *restful.WebService { ws := new(restful.WebService) ws.Path(a.prefix) // a.prefix contains "prefix/group/version" diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 266f53ec2b9..806f5dc9e60 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -108,9 +108,6 @@ type GenericAPIServer struct { // external (public internet) URLs for this GenericAPIServer. ExternalAddress string - // storage contains the RESTful endpoints exposed by this GenericAPIServer - storage map[string]rest.Storage - // Serializer controls how common API objects not in a group/version prefix are serialized for this server. // Individual APIGroups may define their own serializers. Serializer runtime.NegotiatedSerializer