From a84fa79a01e0ad279f451687ad6de2d260649b03 Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Fri, 26 Jun 2015 17:10:28 -0400 Subject: [PATCH] Use versioned objects for GET and CONNECT operations --- api/swagger-spec/v1.json | 216 ++++++++++++++++++++++++++++++++ pkg/apiserver/api_installer.go | 29 +++-- pkg/apiserver/apiserver_test.go | 72 ++++++++++- 3 files changed, 304 insertions(+), 13 deletions(-) diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index 8552cf8386a..fd65af7ce09 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -5706,6 +5706,54 @@ "summary": "connect GET requests to exec of Pod", "nickname": "connectGetNamespacedPodExec", "parameters": [ + { + "type": "boolean", + "paramType": "query", + "name": "stdin", + "description": "redirect the standard input stream of the pod for this call; defaults to false", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "stdout", + "description": "redirect the standard output stream of the pod for this call; defaults to true", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "stderr", + "description": "redirect the standard error stream of the pod for this call; defaults to true", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "tty", + "description": "allocate a terminal for this exec call; defaults to false", + "required": false, + "allowMultiple": false + }, + { + "type": "string", + "paramType": "query", + "name": "container", + "description": "the container in which to execute the command. Defaults to only container if there is only one container in the pod.", + "required": false, + "allowMultiple": false + }, + { + "type": "", + "paramType": "query", + "name": "command", + "description": "the command to execute; argv array; not executed within a shell", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -5736,6 +5784,54 @@ "summary": "connect POST requests to exec of Pod", "nickname": "connectPostNamespacedPodExec", "parameters": [ + { + "type": "boolean", + "paramType": "query", + "name": "stdin", + "description": "redirect the standard input stream of the pod for this call; defaults to false", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "stdout", + "description": "redirect the standard output stream of the pod for this call; defaults to true", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "stderr", + "description": "redirect the standard error stream of the pod for this call; defaults to true", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "tty", + "description": "allocate a terminal for this exec call; defaults to false", + "required": false, + "allowMultiple": false + }, + { + "type": "string", + "paramType": "query", + "name": "container", + "description": "the container in which to execute the command. Defaults to only container if there is only one container in the pod.", + "required": false, + "allowMultiple": false + }, + { + "type": "", + "paramType": "query", + "name": "command", + "description": "the command to execute; argv array; not executed within a shell", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -5780,6 +5876,30 @@ "required": false, "allowMultiple": false }, + { + "type": "string", + "paramType": "query", + "name": "container", + "description": "the container for which to stream logs; defaults to only container if there is one container in the pod", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "follow", + "description": "follow the log stream of the pod; defaults to false", + "required": false, + "allowMultiple": false + }, + { + "type": "boolean", + "paramType": "query", + "name": "previous", + "description": "return previous terminated container logs; defaults to false", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -5889,6 +6009,14 @@ "summary": "connect GET requests to proxy of Pod", "nickname": "connectGetNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -5919,6 +6047,14 @@ "summary": "connect POST requests to proxy of Pod", "nickname": "connectPostNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -5949,6 +6085,14 @@ "summary": "connect PUT requests to proxy of Pod", "nickname": "connectPutNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -5979,6 +6123,14 @@ "summary": "connect DELETE requests to proxy of Pod", "nickname": "connectDeleteNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6009,6 +6161,14 @@ "summary": "connect HEAD requests to proxy of Pod", "nickname": "connectHeadNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6039,6 +6199,14 @@ "summary": "connect OPTIONS requests to proxy of Pod", "nickname": "connectOptionsNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6075,6 +6243,14 @@ "summary": "connect GET requests to proxy of Pod", "nickname": "connectGetNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6113,6 +6289,14 @@ "summary": "connect POST requests to proxy of Pod", "nickname": "connectPostNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6151,6 +6335,14 @@ "summary": "connect PUT requests to proxy of Pod", "nickname": "connectPutNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6189,6 +6381,14 @@ "summary": "connect DELETE requests to proxy of Pod", "nickname": "connectDeleteNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6227,6 +6427,14 @@ "summary": "connect HEAD requests to proxy of Pod", "nickname": "connectHeadNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", @@ -6265,6 +6473,14 @@ "summary": "connect OPTIONS requests to proxy of Pod", "nickname": "connectOptionsNamespacedPodProxy", "parameters": [ + { + "type": "string", + "paramType": "query", + "name": "path", + "description": "URL path to use in proxy request to pod", + "required": false, + "allowMultiple": false + }, { "type": "string", "paramType": "path", diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 3a53e3a671a..debd438936c 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -204,10 +204,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } versionedStatus := indirectArbitraryPointer(versionedStatusPtr) var ( - getOptions runtime.Object - getOptionsKind string - getSubpath bool - getSubpathKey string + getOptions runtime.Object + versionedGetOptions runtime.Object + getOptionsKind string + getSubpath bool + getSubpathKey string ) if isGetterWithOptions { getOptions, getSubpath, getSubpathKey = getterWithOptions.NewGetOptions() @@ -215,14 +216,19 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if err != nil { return err } + versionedGetOptions, err = a.group.Creater.New(serverVersion, getOptionsKind) + if err != nil { + return err + } isGetter = true } var ( - connectOptions runtime.Object - connectOptionsKind string - connectSubpath bool - connectSubpathKey string + connectOptions runtime.Object + versionedConnectOptions runtime.Object + connectOptionsKind string + connectSubpath bool + connectSubpathKey string ) if isConnecter { connectOptions, connectSubpath, connectSubpathKey = connecter.NewConnectOptions() @@ -231,6 +237,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag if err != nil { return err } + versionedConnectOptions, err = a.group.Creater.New(serverVersion, connectOptionsKind) } } @@ -390,7 +397,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag Returns(http.StatusOK, "OK", versionedObject). Writes(versionedObject) if isGetterWithOptions { - if err := addObjectParams(ws, route, getOptions); err != nil { + if err := addObjectParams(ws, route, versionedGetOptions); err != nil { return err } } @@ -561,8 +568,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag Produces("*/*"). Consumes("*/*"). Writes("string") - if connectOptions != nil { - if err := addObjectParams(ws, route, connectOptions); err != nil { + if versionedConnectOptions != nil { + if err := addObjectParams(ws, route, versionedConnectOptions); err != nil { return err } } diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 2b23cb036a5..9b01efe127e 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -255,8 +255,8 @@ func (*SimpleRoot) IsAnAPIObject() {} type SimpleGetOptions struct { api.TypeMeta `json:",inline"` - Param1 string `json:"param1"` - Param2 string `json:"param2"` + Param1 string `json:"param1" description:"description for param1"` + Param2 string `json:"param2" description:"description for param2"` Path string `json:"atAPath"` } @@ -1078,6 +1078,47 @@ func TestGetBinary(t *testing.T) { } } +func validateSimpleGetOptionsParams(t *testing.T, route *restful.Route) { + // Validate name and description + expectedParams := map[string]string{ + "param1": "description for param1", + "param2": "description for param2", + "atAPath": "", + } + for _, p := range route.ParameterDocs { + data := p.Data() + if desc, exists := expectedParams[data.Name]; exists { + if desc != data.Description { + t.Errorf("unexpected description for parameter %s: %s\n", data.Name, data.Description) + } + delete(expectedParams, data.Name) + } + } + if len(expectedParams) > 0 { + t.Errorf("did not find all expected parameters: %#v", expectedParams) + } +} + +func TestGetWithOptionsRouteParams(t *testing.T) { + storage := map[string]rest.Storage{} + simpleStorage := GetWithOptionsRESTStorage{ + SimpleRESTStorage: &SimpleRESTStorage{}, + } + storage["simple"] = &simpleStorage + handler := handle(storage) + ws := handler.(*defaultAPIServer).container.RegisteredWebServices() + if len(ws) == 0 { + t.Fatal("no web services registered") + } + routes := ws[0].Routes() + for i := range routes { + if routes[i].Method == "GET" && routes[i].Operation == "readNamespacedSimple" { + validateSimpleGetOptionsParams(t, &routes[i]) + break + } + } +} + func TestGetWithOptions(t *testing.T) { storage := map[string]rest.Storage{} simpleStorage := GetWithOptionsRESTStorage{ @@ -1292,6 +1333,33 @@ func TestConnect(t *testing.T) { } } +func TestConnectWithOptionsRouteParams(t *testing.T) { + connectStorage := &ConnecterRESTStorage{ + connectHandler: &SimpleConnectHandler{}, + emptyConnectOptions: &SimpleGetOptions{}, + } + storage := map[string]rest.Storage{ + "simple": &SimpleRESTStorage{}, + "simple/connect": connectStorage, + } + handler := handle(storage) + ws := handler.(*defaultAPIServer).container.RegisteredWebServices() + if len(ws) == 0 { + t.Fatal("no web services registered") + } + routes := ws[0].Routes() + for i := range routes { + switch routes[i].Operation { + case "connectGetNamespacedSimpleConnect": + case "connectPostNamespacedSimpleConnect": + case "connectPutNamespacedSimpleConnect": + case "connectDeleteNamespacedSimpleConnect": + validateSimpleGetOptionsParams(t, &routes[i]) + + } + } +} + func TestConnectWithOptions(t *testing.T) { responseText := "Hello World" itemID := "theID"