Merge pull request #13525 from pweil-/register-to-existing

Auto commit by PR queue bot

pkg/apiserver/api_installer.go

@@ -61,11 +61,8 @@ type documentable interface {
 var errEmptyName = errors.NewBadRequest("name must be provided")
 
 // Installs handlers for API resources.
-func (a *APIInstaller) Install() (ws *restful.WebService, errors []error) {
-	errors = make([]error, 0)
-
-	// Create the WebService.
-	ws = a.newWebService()
+func (a *APIInstaller) Install(ws *restful.WebService) []error {
+	errors := make([]error, 0)
 
 	proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.group.Storage, a.group.Codec, a.group.Context, a.info, a.proxyDialerFn})
 
@@ -82,10 +79,11 @@ func (a *APIInstaller) Install() (ws *restful.WebService, errors []error) {
 			errors = append(errors, err)
 		}
 	}
-	return ws, errors
+	return errors
 }
 
-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)
 	ws.Doc("API at " + a.prefix + " version " + a.group.Version)

pkg/apiserver/apiserver.go

@@ -114,8 +114,39 @@ const (
 
 // InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container.
 // It is expected that the provided path root prefix will serve all operations. Root MUST NOT end
-// in a slash. A restful WebService is created for the group and version.
+// in a slash.
 func (g *APIGroupVersion) InstallREST(container *restful.Container) error {
+	installer := g.newInstaller()
+	ws := installer.NewWebService()
+	registrationErrors := installer.Install(ws)
+	container.Add(ws)
+	return errors.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))
+	}
+
+	return errors.NewAggregate(installer.Install(ws))
+}
+
+// newInstaller is a helper to create the installer.  Used by InstallREST and UpdateREST.
+func (g *APIGroupVersion) newInstaller() *APIInstaller {
 	info := &APIRequestInfoResolver{sets.NewString(strings.TrimPrefix(g.Root, "/")), g.Mapper}
 
 	prefix := path.Join(g.Root, g.Version)
@@ -126,9 +157,7 @@ func (g *APIGroupVersion) InstallREST(container *restful.Container) error {
 		minRequestTimeout: g.MinRequestTimeout,
 		proxyDialerFn:     g.ProxyDialerFn,
 	}
-	ws, registrationErrors := installer.Install()
-	container.Add(ws)
-	return errors.NewAggregate(registrationErrors)
+	return installer
 }
 
 // TODO: document all handlers

pkg/apiserver/apiserver_test.go

@@ -2008,6 +2008,84 @@ 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:      "/api",
+			Creater:   api.Scheme,
+			Convertor: api.Scheme,
+			Typer:     api.Scheme,
+			Linker:    selfLinker,
+
+			Admit:   admissionControl,
+			Context: requestContextMapper,
+			Mapper:  namespaceMapper,
+
+			Version:       newVersion,
+			ServerVersion: newVersion,
+			Codec:         newCodec,
+		}
+	}
+
+	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) {
+		w := httptest.NewRecorder()
+		container.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/api/version2/namespaces/test/foo/test"}})
+		if w.Code != http.StatusOK {
+			t.Fatalf("expected OK: %#v", w)
+		}
+
+		w = httptest.NewRecorder()
+		container.ServeHTTP(w, &http.Request{Method: "GET", URL: &url.URL{Path: "/api/version2/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: &SimpleRoot{}, // a root scoped type