Simplify api_installer and setup methods

pkg/apiserver/apiserver.go

@@ -29,7 +29,6 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
@@ -91,72 +90,38 @@ type Mux interface {
 	HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request))
 }
 
-// defaultAPIServer exposes nested objects for testability.
-type defaultAPIServer struct {
-	http.Handler
-	group *APIGroupVersion
-}
-
-// Handle returns a Handler function that exposes the provided storage interfaces
-// as RESTful resources at prefix, serialized by codec, and also includes the support
-// http resources.
-// Note: This method is used only in tests.
-func Handle(storage map[string]RESTStorage, codec runtime.Codec, root string, version string, linker runtime.SelfLinker, admissionControl admission.Interface, contextMapper api.RequestContextMapper, mapper meta.RESTMapper) http.Handler {
-	prefix := path.Join(root, version)
-	group := NewAPIGroupVersion(storage, codec, root, prefix, linker, admissionControl, contextMapper, mapper)
-	container := restful.NewContainer()
-	container.Router(restful.CurlyRouter{})
-	mux := container.ServeMux
-	group.InstallREST(container, root, version)
-	ws := new(restful.WebService)
-	InstallSupport(mux, ws)
-	container.Add(ws)
-	return &defaultAPIServer{mux, group}
-}
-
 // APIGroupVersion is a helper for exposing RESTStorage objects as http.Handlers via go-restful
 // It handles URLs of the form:
 // /${storage_key}[/${object_name}]
 // Where 'storage_key' points to a RESTStorage object stored in storage.
 type APIGroupVersion struct {
-	storage map[string]RESTStorage
-	codec   runtime.Codec
-	prefix  string
-	linker  runtime.SelfLinker
-	admit   admission.Interface
-	context api.RequestContextMapper
-	mapper  meta.RESTMapper
-	// TODO: put me into a cleaner interface
-	info *APIRequestInfoResolver
-}
+	Storage map[string]RESTStorage
 
-// NewAPIGroupVersion returns an object that will serve a set of REST resources and their
-// associated operations.  The provided codec controls serialization and deserialization.
-// This is a helper method for registering multiple sets of REST handlers under different
-// prefixes onto a server.
-// TODO: add multitype codec serialization
-func NewAPIGroupVersion(storage map[string]RESTStorage, codec runtime.Codec, root, prefix string, linker runtime.SelfLinker, admissionControl admission.Interface, contextMapper api.RequestContextMapper, mapper meta.RESTMapper) *APIGroupVersion {
-	return &APIGroupVersion{
-		storage: storage,
-		codec:   codec,
-		prefix:  prefix,
-		linker:  linker,
-		admit:   admissionControl,
-		context: contextMapper,
-		mapper:  mapper,
-		info:    &APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(root, "/")), latest.RESTMapper},
-	}
+	Root    string
+	Version string
+
+	Mapper meta.RESTMapper
+
+	Codec   runtime.Codec
+	Typer   runtime.ObjectTyper
+	Creater runtime.ObjectCreater
+	Linker  runtime.SelfLinker
+
+	Admit   admission.Interface
+	Context api.RequestContextMapper
 }
 
 // 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.
-func (g *APIGroupVersion) InstallREST(container *restful.Container, root string, version string) error {
-	prefix := path.Join(root, version)
+func (g *APIGroupVersion) InstallREST(container *restful.Container) error {
+	info := &APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(g.Root, "/")), g.Mapper}
+
+	prefix := path.Join(g.Root, g.Version)
 	installer := &APIInstaller{
-		group:   g,
-		prefix:  prefix,
-		version: version,
+		group:  g,
+		info:   info,
+		prefix: prefix,
 	}
 	ws, registrationErrors := installer.Install()
 	container.Add(ws)

pkg/apiserver/proxy_test.go

@@ -280,14 +280,10 @@ func TestProxy(t *testing.T) {
 			expectedResourceNamespace: item.reqNamespace,
 		}
 
-		namespaceHandler := Handle(map[string]RESTStorage{
-			"foo": simpleStorage,
-		}, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, namespaceMapper)
+		namespaceHandler := handleNamespaced(map[string]RESTStorage{"foo": simpleStorage})
 		namespaceServer := httptest.NewServer(namespaceHandler)
 		defer namespaceServer.Close()
-		legacyNamespaceHandler := Handle(map[string]RESTStorage{
-			"foo": simpleStorage,
-		}, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, legacyNamespaceMapper)
+		legacyNamespaceHandler := handle(map[string]RESTStorage{"foo": simpleStorage})
 		legacyNamespaceServer := httptest.NewServer(legacyNamespaceHandler)
 		defer legacyNamespaceServer.Close()
 
@@ -296,8 +292,8 @@ func TestProxy(t *testing.T) {
 			server           *httptest.Server
 			proxyTestPattern string
 		}{
-			{namespaceServer, "/prefix/version/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
-			{legacyNamespaceServer, "/prefix/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace},
+			{namespaceServer, "/api/version/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
+			{legacyNamespaceServer, "/api/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace},
 		}
 
 		for _, serverPattern := range serverPatterns {
@@ -344,14 +340,12 @@ func TestProxyUpgrade(t *testing.T) {
 		expectedResourceNamespace: "myns",
 	}
 
-	namespaceHandler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, namespaceMapper)
+	namespaceHandler := handleNamespaced(map[string]RESTStorage{"foo": simpleStorage})
 
 	server := httptest.NewServer(namespaceHandler)
 	defer server.Close()
 
-	ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/prefix/version/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/")
+	ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/api/version/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/")
 	if err != nil {
 		t.Fatalf("websocket dial err: %s", err)
 	}

pkg/apiserver/redirect_test.go

@@ -29,9 +29,7 @@ func TestRedirect(t *testing.T) {
 		errors: map[string]error{},
 		expectedResourceNamespace: "default",
 	}
-	handler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, mapper)
+	handler := handle(map[string]RESTStorage{"foo": simpleStorage})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
@@ -54,7 +52,7 @@ func TestRedirect(t *testing.T) {
 	for _, item := range table {
 		simpleStorage.errors["resourceLocation"] = item.err
 		simpleStorage.resourceLocation = item.id
-		resp, err := client.Get(server.URL + "/prefix/version/redirect/foo/" + item.id)
+		resp, err := client.Get(server.URL + "/api/version/redirect/foo/" + item.id)
 		if resp == nil {
 			t.Fatalf("Unexpected nil resp")
 		}
@@ -82,9 +80,7 @@ func TestRedirectWithNamespaces(t *testing.T) {
 		errors: map[string]error{},
 		expectedResourceNamespace: "other",
 	}
-	handler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/prefix", "version", selfLinker, admissionControl, requestContextMapper, namespaceMapper)
+	handler := handleNamespaced(map[string]RESTStorage{"foo": simpleStorage})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
@@ -107,7 +103,7 @@ func TestRedirectWithNamespaces(t *testing.T) {
 	for _, item := range table {
 		simpleStorage.errors["resourceLocation"] = item.err
 		simpleStorage.resourceLocation = item.id
-		resp, err := client.Get(server.URL + "/prefix/version/redirect/namespaces/other/foo/" + item.id)
+		resp, err := client.Get(server.URL + "/api/version/redirect/namespaces/other/foo/" + item.id)
 		if resp == nil {
 			t.Fatalf("Unexpected nil resp")
 		}

pkg/apiserver/resthandler.go

@@ -17,6 +17,7 @@ limitations under the License.
 package apiserver
 
 import (
+	"fmt"
 	"net/http"
 	"net/url"
 	gpath "path"
@@ -97,7 +98,7 @@ func parseSelectorQueryParams(query url.Values, version, apiResource string) (la
 }
 
 // ListResource returns a function that handles retrieving a list of resources from a RESTStorage object.
-func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, requestInfoResolver *APIRequestInfoResolver) restful.RouteFunction {
+func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, version, apiResource string) restful.RouteFunction {
 	return func(req *restful.Request, res *restful.Response) {
 		w := res.ResponseWriter
 
@@ -109,13 +110,7 @@ func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runti
 		ctx := ctxFn(req)
 		ctx = api.WithNamespace(ctx, namespace)
 
-		requestInfo, err := requestInfoResolver.GetAPIRequestInfo(req.Request)
-		if err != nil {
-			errorJSON(err, codec, w)
-			return
-		}
-
-		label, field, err := parseSelectorQueryParams(req.Request.URL.Query(), requestInfo.APIVersion, requestInfo.Resource)
+		label, field, err := parseSelectorQueryParams(req.Request.URL.Query(), version, apiResource)
 		if err != nil {
 			errorJSON(err, codec, w)
 			return
@@ -135,7 +130,7 @@ func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runti
 }
 
 // CreateResource returns a function that will handle a resource creation.
-func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction {
+func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, typer runtime.ObjectTyper, resource string, admit admission.Interface) restful.RouteFunction {
 	return func(req *restful.Request, res *restful.Response) {
 		w := res.ResponseWriter
 
@@ -158,6 +153,7 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec ru
 
 		obj := r.New()
 		if err := codec.DecodeInto(body, obj); err != nil {
+			err = transformDecodeError(typer, err, obj, body)
 			errorJSON(err, codec, w)
 			return
 		}
@@ -190,7 +186,7 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec ru
 }
 
 // UpdateResource returns a function that will handle a resource update
-func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, resource string, admit admission.Interface) restful.RouteFunction {
+func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, typer runtime.ObjectTyper, resource string, admit admission.Interface) restful.RouteFunction {
 	return func(req *restful.Request, res *restful.Response) {
 		w := res.ResponseWriter
 
@@ -213,6 +209,7 @@ func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec ru
 
 		obj := r.New()
 		if err := codec.DecodeInto(body, obj); err != nil {
+			err = transformDecodeError(typer, err, obj, body)
 			errorJSON(err, codec, w)
 			return
 		}
@@ -346,6 +343,18 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object,
 	}
 }
 
+// transformDecodeError adds additional information when a decode fails.
+func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime.Object, body []byte) error {
+	_, kind, err := typer.ObjectVersionAndKind(into)
+	if err != nil {
+		return err
+	}
+	if version, dataKind, err := typer.DataVersionAndKind(body); err == nil && len(dataKind) > 0 {
+		return errors.NewBadRequest(fmt.Sprintf("%s in version %s cannot be handled as a %s: %v", dataKind, version, kind, baseErr))
+	}
+	return errors.NewBadRequest(fmt.Sprintf("the object provided is unrecognized (must be of type %s): %v", kind, baseErr))
+}
+
 // setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request
 // plus the path and query generated by the provided linkFunc
 func setSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) error {

pkg/apiserver/watch_test.go

@@ -49,9 +49,7 @@ var watchTestTable = []struct {
 func TestWatchWebsocket(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	_ = ResourceWatcher(simpleStorage) // Give compile error if this doesn't work.
-	handler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper)
+	handler := handle(map[string]RESTStorage{"foo": simpleStorage})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
@@ -103,9 +101,7 @@ func TestWatchWebsocket(t *testing.T) {
 
 func TestWatchHTTP(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
-	handler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper)
+	handler := handle(map[string]RESTStorage{"foo": simpleStorage})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 	client := http.Client{}
@@ -170,9 +166,7 @@ func TestWatchParamParsing(t *testing.T) {
 			return label, value, nil
 		})
 	simpleStorage := &SimpleRESTStorage{}
-	handler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/api", testVersion, selfLinker, admissionControl, requestContextMapper, mapper)
+	handler := handle(map[string]RESTStorage{"foo": simpleStorage})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
@@ -242,9 +236,7 @@ func TestWatchParamParsing(t *testing.T) {
 
 func TestWatchProtocolSelection(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
-	handler := Handle(map[string]RESTStorage{
-		"foo": simpleStorage,
-	}, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper)
+	handler := handle(map[string]RESTStorage{"foo": simpleStorage})
 	server := httptest.NewServer(handler)
 	defer server.Close()
 	defer server.CloseClientConnections()

pkg/master/master.go

@@ -30,7 +30,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/admission"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3"
@@ -55,7 +54,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/resourcequotausage"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/secret"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/service"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/ui"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -425,14 +423,14 @@ func (m *Master) init(c *Config) {
 	}
 
 	apiVersions := []string{"v1beta1", "v1beta2"}
-	if err := apiserver.NewAPIGroupVersion(m.api_v1beta1()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta1"); err != nil {
+	if err := m.api_v1beta1().InstallREST(m.handlerContainer); err != nil {
 		glog.Fatalf("Unable to setup API v1beta1: %v", err)
 	}
-	if err := apiserver.NewAPIGroupVersion(m.api_v1beta2()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta2"); err != nil {
+	if err := m.api_v1beta2().InstallREST(m.handlerContainer); err != nil {
 		glog.Fatalf("Unable to setup API v1beta2: %v", err)
 	}
 	if c.EnableV1Beta3 {
-		if err := apiserver.NewAPIGroupVersion(m.api_v1beta3()).InstallREST(m.handlerContainer, c.APIPrefix, "v1beta3"); err != nil {
+		if err := m.api_v1beta3().InstallREST(m.handlerContainer); err != nil {
 			glog.Fatalf("Unable to setup API v1beta3: %v", err)
 		}
 		apiVersions = []string{"v1beta1", "v1beta2", "v1beta3"}
@@ -569,26 +567,49 @@ func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server {
 	return serversToValidate
 }
 
+func (m *Master) defaultAPIGroupVersion() *apiserver.APIGroupVersion {
+	return &apiserver.APIGroupVersion{
+		Root: m.apiPrefix,
+
+		Mapper: latest.RESTMapper,
+
+		Creater: api.Scheme,
+		Typer:   api.Scheme,
+		Linker:  latest.SelfLinker,
+
+		Admit:   m.admissionControl,
+		Context: m.requestContextMapper,
+	}
+}
+
 // api_v1beta1 returns the resources and codec for API version v1beta1.
-func (m *Master) api_v1beta1() (map[string]apiserver.RESTStorage, runtime.Codec, string, string, runtime.SelfLinker, admission.Interface, api.RequestContextMapper, meta.RESTMapper) {
+func (m *Master) api_v1beta1() *apiserver.APIGroupVersion {
 	storage := make(map[string]apiserver.RESTStorage)
 	for k, v := range m.storage {
 		storage[k] = v
 	}
-	return storage, v1beta1.Codec, "api", "/api/v1beta1", latest.SelfLinker, m.admissionControl, m.requestContextMapper, latest.RESTMapper
+	version := m.defaultAPIGroupVersion()
+	version.Storage = storage
+	version.Version = "v1beta1"
+	version.Codec = v1beta1.Codec
+	return version
 }
 
 // api_v1beta2 returns the resources and codec for API version v1beta2.
-func (m *Master) api_v1beta2() (map[string]apiserver.RESTStorage, runtime.Codec, string, string, runtime.SelfLinker, admission.Interface, api.RequestContextMapper, meta.RESTMapper) {
+func (m *Master) api_v1beta2() *apiserver.APIGroupVersion {
 	storage := make(map[string]apiserver.RESTStorage)
 	for k, v := range m.storage {
 		storage[k] = v
 	}
-	return storage, v1beta2.Codec, "api", "/api/v1beta2", latest.SelfLinker, m.admissionControl, m.requestContextMapper, latest.RESTMapper
+	version := m.defaultAPIGroupVersion()
+	version.Storage = storage
+	version.Version = "v1beta2"
+	version.Codec = v1beta2.Codec
+	return version
 }
 
 // api_v1beta3 returns the resources and codec for API version v1beta3.
-func (m *Master) api_v1beta3() (map[string]apiserver.RESTStorage, runtime.Codec, string, string, runtime.SelfLinker, admission.Interface, api.RequestContextMapper, meta.RESTMapper) {
+func (m *Master) api_v1beta3() *apiserver.APIGroupVersion {
 	storage := make(map[string]apiserver.RESTStorage)
 	for k, v := range m.storage {
 		if k == "minions" {
@@ -596,5 +617,9 @@ func (m *Master) api_v1beta3() (map[string]apiserver.RESTStorage, runtime.Codec,
 		}
 		storage[strings.ToLower(k)] = v
 	}
-	return storage, v1beta3.Codec, "api", "/api/v1beta3", latest.SelfLinker, m.admissionControl, m.requestContextMapper, latest.RESTMapper
+	version := m.defaultAPIGroupVersion()
+	version.Storage = storage
+	version.Version = "v1beta3"
+	version.Codec = v1beta3.Codec
+	return version
 }

pkg/runtime/interfaces.go

@@ -45,6 +45,11 @@ type ObjectTyper interface {
 	ObjectVersionAndKind(Object) (version, kind string, err error)
 }
 
+// ObjectCreater contains methods for instantiating an object by kind and version.
+type ObjectCreater interface {
+	New(version, kind string) (out Object, err error)
+}
+
 // ResourceVersioner provides methods for setting and retrieving
 // the resource version from an API object.
 type ResourceVersioner interface {