Merge pull request #10426 from csrwng/api_versioned_options

API Server: Use versioned objects for GET and CONNECT operations

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",

pkg/apiserver/api_installer.go

@@ -205,6 +205,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	versionedStatus := indirectArbitraryPointer(versionedStatusPtr)
 	var (
 		getOptions          runtime.Object
+		versionedGetOptions runtime.Object
 		getOptionsKind      string
 		getSubpath          bool
 		getSubpathKey       string
@@ -215,11 +216,16 @@ 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
+		versionedConnectOptions runtime.Object
 		connectOptionsKind      string
 		connectSubpath          bool
 		connectSubpathKey       string
@@ -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 versionedConnectOptions != nil {
-					if err := addObjectParams(ws, route, connectOptions); err != nil {
+					if err := addObjectParams(ws, route, versionedConnectOptions); err != nil {
 						return err
 					}
 				}

pkg/apiserver/apiserver_test.go

@@ -255,8 +255,8 @@ func (*SimpleRoot) IsAnAPIObject() {}
 
 type SimpleGetOptions struct {
 	api.TypeMeta `json:",inline"`
-	Param1       string `json:"param1"`
+	Param1       string `json:"param1" description:"description for param1"`
-	Param2       string `json:"param2"`
+	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"