Add more extensive tests to apiserver for variations in version

Formalize v1beta1 and v1beta3 style APIs in our test cases.

pkg/apiserver/api_installer.go

@@ -670,6 +670,12 @@ func addParams(route *restful.RouteBuilder, params []*restful.Parameter) {
 	}
 }
 
+// addObjectParams converts a runtime.Object into a set of go-restful Param() definitions on the route.
+// The object must be a pointer to a struct; only fields at the top level of the struct that are not
+// themselves interfaces or structs are used; only fields with a json tag that is non empty (the standard
+// Go JSON behavior for omitting a field) become query parameters. The name of the query parameter is
+// the JSON field name. If a description struct tag is set on the field, that description is used on the
+// query parameter. In essence, it converts a standard JSON top level object into a query param schema.
 func addObjectParams(ws *restful.WebService, route *restful.RouteBuilder, obj runtime.Object) error {
 	sv, err := conversion.EnforcePtr(obj)
 	if err != nil {

pkg/apiserver/apiserver.go

@@ -204,7 +204,11 @@ func APIVersionHandler(versions ...string) restful.RouteFunction {
 	}
 }
 
-// write renders a returned runtime.Object to the response as a stream or an encoded object.
+// write renders a returned runtime.Object to the response as a stream or an encoded object. If the object
+// returned by the response implements rest.ResourceStreamer that interface will be used to render the
+// response. The Accept header and current API version will be passed in, and the output will be copied
+// directly to the response body. If content type is returned it is used, otherwise the content type will
+// be "application/octet-stream". All other objects are sent to standard JSON serialization.
 func write(statusCode int, apiVersion string, codec runtime.Codec, object runtime.Object, w http.ResponseWriter, req *http.Request) {
 	if stream, ok := object.(rest.ResourceStreamer); ok {
 		out, contentType, err := stream.InputStream(apiVersion, req.Header.Get("Accept"))

pkg/apiserver/apiserver_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
@@ -54,11 +55,21 @@ func convert(obj runtime.Object) (runtime.Object, error) {
 	return obj, nil
 }
 
-// This creates a fake API version, similar to api/latest.go
+// This creates a fake API version, similar to api/latest.go for a v1beta1 equivalent api. It is distinct
+// from the Kubernetes API versions to allow clients to properly distinguish the two.
 const testVersion = "version"
 
-var versions = []string{testVersion}
-var codec = runtime.CodecFor(api.Scheme, testVersion)
+// The equivalent of the Kubernetes v1beta3 API.
+const testVersion2 = "version2"
+
+var versions = []string{testVersion, testVersion2}
+var legacyCodec = runtime.CodecFor(api.Scheme, testVersion)
+var codec = runtime.CodecFor(api.Scheme, testVersion2)
+
+// these codecs reflect ListOptions/DeleteOptions coming from the serverAPIversion
+var versionServerCodec = runtime.CodecFor(api.Scheme, "v1beta1")
+var version2ServerCodec = runtime.CodecFor(api.Scheme, "v1beta3")
+
 var accessor = meta.NewAccessor()
 var versioner runtime.ResourceVersioner = accessor
 var selfLinker runtime.SelfLinker = accessor
@@ -69,6 +80,12 @@ var requestContextMapper api.RequestContextMapper
 func interfacesFor(version string) (*meta.VersionInterfaces, error) {
 	switch version {
 	case testVersion:
+		return &meta.VersionInterfaces{
+			Codec:            legacyCodec,
+			ObjectConvertor:  api.Scheme,
+			MetadataAccessor: accessor,
+		}, nil
+	case testVersion2:
 		return &meta.VersionInterfaces{
 			Codec:            codec,
 			ObjectConvertor:  api.Scheme,
@@ -100,7 +117,10 @@ func init() {
 	api.Scheme.AddKnownTypes("", &Simple{}, &SimpleList{}, &api.Status{}, &api.ListOptions{})
 	// "version" version
 	// TODO: Use versioned api objects?
-	api.Scheme.AddKnownTypes(testVersion, &Simple{}, &SimpleList{}, &v1beta1.DeleteOptions{}, &v1beta1.Status{}, &v1beta1.ListOptions{})
+	api.Scheme.AddKnownTypes(testVersion, &Simple{}, &SimpleList{}, &v1beta1.Status{})
+	// "version2" version
+	// TODO: Use versioned api objects?
+	api.Scheme.AddKnownTypes(testVersion2, &Simple{}, &SimpleList{}, &v1beta3.Status{})
 
 	nsMapper := newMapper()
 	legacyNsMapper := newMapper()
@@ -118,6 +138,18 @@ func init() {
 	namespaceMapper = nsMapper
 	admissionControl = admit.NewAlwaysAdmit()
 	requestContextMapper = api.NewRequestContextMapper()
+
+	//mapper.(*meta.DefaultRESTMapper).Add(meta.RESTScopeNamespaceLegacy, "Simple", testVersion, false)
+	api.Scheme.AddFieldLabelConversionFunc(testVersion, "Simple",
+		func(label, value string) (string, string, error) {
+			return label, value, nil
+		},
+	)
+	api.Scheme.AddFieldLabelConversionFunc(testVersion2, "Simple",
+		func(label, value string) (string, string, error) {
+			return label, value, nil
+		},
+	)
 }
 
 // defaultAPIServer exposes nested objects for testability.
@@ -129,46 +161,61 @@ type defaultAPIServer struct {
 
 // uses the default settings
 func handle(storage map[string]rest.Storage) http.Handler {
-	return handleInternal(storage, admissionControl, mapper, selfLinker)
+	return handleInternal(true, storage, admissionControl, selfLinker)
+}
+
+// uses the default settings for a v1beta3 compatible api
+func handleNew(storage map[string]rest.Storage) http.Handler {
+	return handleInternal(false, storage, admissionControl, selfLinker)
 }
 
 // tests with a deny admission controller
 func handleDeny(storage map[string]rest.Storage) http.Handler {
-	return handleInternal(storage, deny.NewAlwaysDeny(), mapper, selfLinker)
+	return handleInternal(true, storage, deny.NewAlwaysDeny(), selfLinker)
 }
 
 // tests using the new namespace scope mechanism
 func handleNamespaced(storage map[string]rest.Storage) http.Handler {
-	return handleInternal(storage, admissionControl, namespaceMapper, selfLinker)
+	return handleInternal(false, storage, admissionControl, selfLinker)
 }
 
 // tests using a custom self linker
 func handleLinker(storage map[string]rest.Storage, selfLinker runtime.SelfLinker) http.Handler {
-	return handleInternal(storage, admissionControl, mapper, selfLinker)
+	return handleInternal(true, storage, admissionControl, selfLinker)
 }
 
-func handleInternal(storage map[string]rest.Storage, admissionControl admission.Interface, mapper meta.RESTMapper, selfLinker runtime.SelfLinker) http.Handler {
+func handleInternal(legacy bool, storage map[string]rest.Storage, admissionControl admission.Interface, selfLinker runtime.SelfLinker) http.Handler {
 	group := &APIGroupVersion{
 		Storage: storage,
 
-		Mapper: mapper,
-
-		Root:    "/api",
-		Version: testVersion,
+		Root: "/api",
 
 		Creater:   api.Scheme,
 		Convertor: api.Scheme,
 		Typer:     api.Scheme,
-		Codec:     codec,
 		Linker:    selfLinker,
 
 		Admit:   admissionControl,
 		Context: requestContextMapper,
 	}
+	if legacy {
+		group.Version = testVersion
+		group.ServerVersion = "v1beta1"
+		group.Codec = legacyCodec
+		group.Mapper = legacyNamespaceMapper
+	} else {
+		group.Version = testVersion2
+		group.ServerVersion = "v1beta3"
+		group.Codec = codec
+		group.Mapper = namespaceMapper
+	}
+
 	container := restful.NewContainer()
 	container.Router(restful.CurlyRouter{})
 	mux := container.ServeMux
-	group.InstallREST(container)
+	if err := group.InstallREST(container); err != nil {
+		panic(fmt.Sprintf("unable to install container %s: %v", group.Version, err))
+	}
 	ws := new(restful.WebService)
 	InstallSupport(mux, ws)
 	container.Add(ws)
@@ -557,27 +604,27 @@ func TestList(t *testing.T) {
 		},
 		// list items in a namespace, v1beta3+
 		{
-			url:       "/api/version/namespaces/default/simple",
+			url:       "/api/version2/namespaces/default/simple",
 			namespace: "default",
-			selfLink:  "/api/version/namespaces/default/simple",
+			selfLink:  "/api/version2/namespaces/default/simple",
 		},
 		{
-			url:       "/api/version/namespaces/other/simple",
+			url:       "/api/version2/namespaces/other/simple",
 			namespace: "other",
-			selfLink:  "/api/version/namespaces/other/simple",
+			selfLink:  "/api/version2/namespaces/other/simple",
 		},
 		{
-			url:       "/api/version/namespaces/other/simple?labels=a%3Db&fields=c%3Dd",
+			url:       "/api/version2/namespaces/other/simple?labelSelector=a%3Db&fieldSelector=c%3Dd",
 			namespace: "other",
-			selfLink:  "/api/version/namespaces/other/simple",
+			selfLink:  "/api/version2/namespaces/other/simple",
 			label:     "a=b",
 			field:     "c=d",
 		},
 		// list items across all namespaces
 		{
-			url:       "/api/version/simple",
+			url:       "/api/version2/simple",
 			namespace: "",
-			selfLink:  "/api/version/simple",
+			selfLink:  "/api/version2/simple",
 		},
 	}
 	for i, testCase := range testCases {
@@ -593,7 +640,7 @@ func TestList(t *testing.T) {
 		if testCase.legacy {
 			handler = handleLinker(storage, selfLinker)
 		} else {
-			handler = handleInternal(storage, admissionControl, namespaceMapper, selfLinker)
+			handler = handleInternal(false, storage, admissionControl, selfLinker)
 		}
 		server := httptest.NewServer(handler)
 		defer server.Close()
@@ -605,6 +652,9 @@ func TestList(t *testing.T) {
 		}
 		if resp.StatusCode != http.StatusOK {
 			t.Errorf("%d: unexpected status: %d, Expected: %d, %#v", i, resp.StatusCode, http.StatusOK, resp)
+			body, _ := ioutil.ReadAll(resp.Body)
+			t.Logf("%d: body: %s", string(body))
+			continue
 		}
 		// TODO: future, restore get links
 		if !selfLinker.called {
@@ -875,16 +925,16 @@ func TestGetNamespaceSelfLink(t *testing.T) {
 	}
 	selfLinker := &setTestSelfLinker{
 		t:           t,
-		expectedSet: "/api/version/namespaces/foo/simple/id",
+		expectedSet: "/api/version2/namespaces/foo/simple/id",
 		name:        "id",
 		namespace:   "foo",
 	}
 	storage["simple"] = &simpleStorage
-	handler := handleInternal(storage, admissionControl, namespaceMapper, selfLinker)
+	handler := handleInternal(false, storage, admissionControl, selfLinker)
 	server := httptest.NewServer(handler)
 	defer server.Close()
 
-	resp, err := http.Get(server.URL + "/api/version/namespaces/foo/simple/id")
+	resp, err := http.Get(server.URL + "/api/version2/namespaces/foo/simple/id")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -959,7 +1009,7 @@ func TestDeleteWithOptions(t *testing.T) {
 	item := &api.DeleteOptions{
 		GracePeriodSeconds: &grace,
 	}
-	body, err := codec.Encode(item)
+	body, err := versionServerCodec.Encode(item)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -1020,7 +1070,7 @@ func TestLegacyDeleteIgnoresOptions(t *testing.T) {
 	defer server.Close()
 
 	item := api.NewDeleteOptions(300)
-	body, err := codec.Encode(item)
+	body, err := versionServerCodec.Encode(item)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -1629,7 +1679,7 @@ func TestCreateInvokesAdmissionControl(t *testing.T) {
 		namespace:   "other",
 		expectedSet: "/api/version/foo/bar?namespace=other",
 	}
-	handler := handleInternal(map[string]rest.Storage{"foo": &storage}, deny.NewAlwaysDeny(), mapper, selfLinker)
+	handler := handleInternal(true, map[string]rest.Storage{"foo": &storage}, deny.NewAlwaysDeny(), selfLinker)
 	server := httptest.NewServer(handler)
 	defer server.Close()
 	client := http.Client{}

pkg/apiserver/proxy_test.go

@@ -294,7 +294,7 @@ func TestProxy(t *testing.T) {
 			server           *httptest.Server
 			proxyTestPattern string
 		}{
-			{namespaceServer, "/api/version/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
+			{namespaceServer, "/api/version2/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
 			{legacyNamespaceServer, "/api/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace},
 		}
 
@@ -348,7 +348,7 @@ func TestProxyUpgrade(t *testing.T) {
 	server := httptest.NewServer(namespaceHandler)
 	defer server.Close()
 
-	ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/api/version/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/")
+	ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/api/version2/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

@@ -105,7 +105,7 @@ func TestRedirectWithNamespaces(t *testing.T) {
 	for _, item := range table {
 		simpleStorage.errors["resourceLocation"] = item.err
 		simpleStorage.resourceLocation = &url.URL{Host: item.id}
-		resp, err := client.Get(server.URL + "/api/version/redirect/namespaces/other/foo/" + item.id)
+		resp, err := client.Get(server.URL + "/api/version2/redirect/namespaces/other/foo/" + item.id)
 		if resp == nil {
 			t.Fatalf("Unexpected nil resp")
 		}

pkg/apiserver/watch_test.go

@@ -25,7 +25,6 @@ import (
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@@ -49,14 +48,6 @@ var watchTestTable = []struct {
 	{watch.Deleted, &Simple{ObjectMeta: api.ObjectMeta{Name: "bar"}}},
 }
 
-func init() {
-	mapper.(*meta.DefaultRESTMapper).Add(meta.RESTScopeNamespaceLegacy, "Simple", testVersion, false)
-	api.Scheme.AddFieldLabelConversionFunc(testVersion, "Simple",
-		func(label, value string) (string, string, error) {
-			return label, value, nil
-		})
-}
-
 func TestWatchWebsocket(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	_ = rest.Watcher(simpleStorage) // Give compile error if this doesn't work.