Support subpath on GET for GetterWithOptions

Allows REST consumers to build paths like:

    /api/v1beta3/namespaces/foo/webhookresource/<name>/<encodedsecretinurl>

Also fixes parameter exposure for subresources (was only fixed for
v1beta3).

pkg/api/rest/rest.go

@@ -55,7 +55,8 @@ type Getter interface {
 }
 
 // GetterWithOptions is an object that retrieve a named RESTful resource and takes
-// additional options on the get request
+// additional options on the get request. It allows a caller to also receive the
+// subpath of the GET request.
 type GetterWithOptions interface {
 	// Get finds a resource in the storage by name and returns it.
 	// Although it can return an arbitrary error value, IsNotFound(err) is true for the
@@ -65,8 +66,12 @@ type GetterWithOptions interface {
 	Get(ctx api.Context, name string, options runtime.Object) (runtime.Object, error)
 
 	// NewGetOptions returns an empty options object that will be used to pass
-	// options to the Get method.
+	// options to the Get method. It may return a bool and a string, if true, the
-	NewGetOptions() runtime.Object
+	// value of the request path below the object will be included as the named
+	// string in the serialization of the runtime object. E.g., returning "path"
+	// will convert the trailing request scheme value to "path" in the map[string][]string
+	// passed to the convertor.
+	NewGetOptions() (runtime.Object, bool, string)
 }
 
 // Deleter is an object that can delete a named RESTful resource.

pkg/apiserver/api_installer.go

@@ -110,6 +110,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		// TODO: support deeper paths
 		return fmt.Errorf("api_installer allows only one or two segment paths (resource or resource/subresource)")
 	}
+	hasSubresource := len(subresource) > 0
 
 	object := storage.New()
 	_, kind, err := a.group.Typer.ObjectVersionAndKind(object)
@@ -177,10 +178,14 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		return err
 	}
 	versionedStatus := indirectArbitraryPointer(versionedStatusPtr)
-	var getOptions runtime.Object
+	var (
-	var getOptionsKind string
+		getOptions     runtime.Object
+		getOptionsKind string
+		getSubpath     bool
+		getSubpathKey  string
+	)
 	if isGetterWithOptions {
-		getOptions = getterWithOptions.NewGetOptions()
+		getOptions, getSubpath, getSubpathKey = getterWithOptions.NewGetOptions()
 		_, getOptionsKind, err = a.group.Typer.ObjectVersionAndKind(getOptions)
 		if err != nil {
 			return err
@@ -206,21 +211,26 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	// Get the list of actions for the given scope.
 	if scope.Name() != meta.RESTScopeNameNamespace {
 		resourcePath := resource
+		resourceParams := params
 		itemPath := resourcePath + "/{name}"
-		if len(subresource) > 0 {
-			itemPath = itemPath + "/" + subresource
-			resourcePath = itemPath
-		}
 		nameParams := append(params, nameParam)
 		proxyParams := append(nameParams, pathParam)
+		if hasSubresource {
+			itemPath = itemPath + "/" + subresource
+			resourcePath = itemPath
+			resourceParams = nameParams
+		}
 		namer := rootScopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath)}
 
 		// Handler for standard REST verbs (GET, PUT, POST and DELETE).
-		actions = appendIf(actions, action{"LIST", resourcePath, params, namer}, isLister)
+		actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer}, isLister)
-		actions = appendIf(actions, action{"POST", resourcePath, params, namer}, isCreater)
+		actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer}, isCreater)
-		actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, params, namer}, allowWatchList)
+		actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, resourceParams, namer}, allowWatchList)
 
 		actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter)
+		if getSubpath {
+			actions = appendIf(actions, action{"GET", itemPath + "/{path:*}", proxyParams, namer}, isGetter)
+		}
 		actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater)
 		actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher)
 		actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter)
@@ -238,25 +248,26 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			namespaceParams := []*restful.Parameter{namespaceParam}
 
 			resourcePath := namespacedPath
+			resourceParams := namespaceParams
 			itemPath := namespacedPath + "/{name}"
-			if len(subresource) > 0 {
-				itemPath = itemPath + "/" + subresource
-				resourcePath = itemPath
-			}
 			nameParams := append(namespaceParams, nameParam)
 			proxyParams := append(nameParams, pathParam)
+			if hasSubresource {
+				itemPath = itemPath + "/" + subresource
+				resourcePath = itemPath
+				resourceParams = nameParams
+			}
 			namer := scopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath), false}
 
-			actions = appendIf(actions, action{"LIST", resourcePath, namespaceParams, namer}, isLister)
+			actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer}, isLister)
-			// Some paths ("/pods/{name}/binding", I'm looking at you) contain an embedded '{name}')
+			actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer}, isCreater)
-			if strings.Contains(resourcePath, "{name}") {
+			// DEPRECATED
-				actions = appendIf(actions, action{"POST", resourcePath, nameParams, namer}, isCreater)
+			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, resourceParams, namer}, allowWatchList)
-			} else {
-				actions = appendIf(actions, action{"POST", resourcePath, namespaceParams, namer}, isCreater)
-			}
-			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, namespaceParams, namer}, allowWatchList)
 
 			actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter)
+			if getSubpath {
+				actions = appendIf(actions, action{"GET", itemPath + "/{path:*}", proxyParams, namer}, isGetter)
+			}
 			actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater)
 			actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher)
 			actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter)
@@ -278,20 +289,25 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 
 			basePath := resource
 			resourcePath := basePath
+			resourceParams := namespaceParams
 			itemPath := resourcePath + "/{name}"
-			if len(subresource) > 0 {
-				itemPath = itemPath + "/" + subresource
-				resourcePath = itemPath
-			}
 			nameParams := append(namespaceParams, nameParam)
 			proxyParams := append(nameParams, pathParam)
+			if hasSubresource {
+				itemPath = itemPath + "/" + subresource
+				resourcePath = itemPath
+				resourceParams = nameParams
+			}
 			namer := legacyScopeNaming{scope, a.group.Linker, gpath.Join(a.prefix, itemPath)}
 
-			actions = appendIf(actions, action{"LIST", resourcePath, namespaceParams, namer}, isLister)
+			actions = appendIf(actions, action{"LIST", resourcePath, resourceParams, namer}, isLister)
-			actions = appendIf(actions, action{"POST", resourcePath, namespaceParams, namer}, isCreater)
+			actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer}, isCreater)
-			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, namespaceParams, namer}, allowWatchList)
+			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resourcePath, resourceParams, namer}, allowWatchList)
 
 			actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter)
+			if getSubpath {
+				actions = appendIf(actions, action{"GET", itemPath + "/{path:*}", proxyParams, namer}, isGetter)
+			}
 			actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater)
 			actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher)
 			actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter)
@@ -336,7 +352,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		case "GET": // Get a resource.
 			var handler restful.RouteFunction
 			if isGetterWithOptions {
-				handler = GetResourceWithOptions(getterWithOptions, reqScope, getOptionsKind)
+				handler = GetResourceWithOptions(getterWithOptions, reqScope, getOptionsKind, getSubpath, getSubpathKey)
 			} else {
 				handler = GetResource(getter, reqScope)
 			}

pkg/apiserver/apiserver_test.go

@@ -239,6 +239,7 @@ type SimpleGetOptions struct {
 	api.TypeMeta `json:",inline"`
 	Param1       string `json:"param1"`
 	Param2       string `json:"param2"`
+	Path         string `json:"atAPath"`
 }
 
 func (*SimpleGetOptions) IsAnAPIObject() {}
@@ -459,9 +460,12 @@ func (m *MetadataRESTStorage) ProducesMIMETypes(method string) []string {
 	return m.types
 }
 
+var _ rest.StorageMetadata = &MetadataRESTStorage{}
+
 type GetWithOptionsRESTStorage struct {
 	*SimpleRESTStorage
 	optionsReceived runtime.Object
+	takesPath       string
 }
 
 func (r *GetWithOptionsRESTStorage) Get(ctx api.Context, name string, options runtime.Object) (runtime.Object, error) {
@@ -472,10 +476,15 @@ func (r *GetWithOptionsRESTStorage) Get(ctx api.Context, name string, options ru
 	return r.SimpleRESTStorage.Get(ctx, name)
 }
 
-func (r *GetWithOptionsRESTStorage) NewGetOptions() runtime.Object {
+func (r *GetWithOptionsRESTStorage) NewGetOptions() (runtime.Object, bool, string) {
-	return &SimpleGetOptions{}
+	if len(r.takesPath) > 0 {
+		return &SimpleGetOptions{}, true, r.takesPath
+	}
+	return &SimpleGetOptions{}, false, ""
 }
 
+var _ rest.GetterWithOptions = &GetWithOptionsRESTStorage{}
+
 func extractBody(response *http.Response, object runtime.Object) (string, error) {
 	defer response.Body.Close()
 	body, err := ioutil.ReadAll(response.Body)
@@ -963,6 +972,47 @@ func TestGetWithOptions(t *testing.T) {
 	}
 }
 
+func TestGetWithOptionsAndPath(t *testing.T) {
+	storage := map[string]rest.Storage{}
+	simpleStorage := GetWithOptionsRESTStorage{
+		SimpleRESTStorage: &SimpleRESTStorage{
+			item: Simple{
+				Other: "foo",
+			},
+		},
+		takesPath: "atAPath",
+	}
+	storage["simple"] = &simpleStorage
+	handler := handle(storage)
+	server := httptest.NewServer(handler)
+	defer server.Close()
+
+	resp, err := http.Get(server.URL + "/api/version/simple/id/a/different/path?param1=test1&param2=test2&atAPath=not")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("unexpected response: %#v", resp)
+	}
+	var itemOut Simple
+	body, err := extractBody(resp, &itemOut)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	if itemOut.Name != simpleStorage.item.Name {
+		t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simpleStorage.item, string(body))
+	}
+
+	opts, ok := simpleStorage.optionsReceived.(*SimpleGetOptions)
+	if !ok {
+		t.Errorf("Unexpected options object received: %#v", simpleStorage.optionsReceived)
+		return
+	}
+	if opts.Param1 != "test1" || opts.Param2 != "test2" || opts.Path != "a/different/path" {
+		t.Errorf("Did not receive expected options: %#v", opts)
+	}
+}
 func TestGetAlternateSelfLink(t *testing.T) {
 	storage := map[string]rest.Storage{}
 	simpleStorage := SimpleRESTStorage{

pkg/apiserver/resthandler.go

@@ -113,10 +113,19 @@ func GetResource(r rest.Getter, scope RequestScope) restful.RouteFunction {
 }
 
 // GetResourceWithOptions returns a function that handles retrieving a single resource from a rest.Storage object.
-func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, getOptionsKind string) restful.RouteFunction {
+func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, getOptionsKind string, subpath bool, subpathKey string) restful.RouteFunction {
 	return getResourceHandler(scope,
 		func(ctx api.Context, name string, req *restful.Request) (runtime.Object, error) {
-			opts, err := queryToObject(req.Request.URL.Query(), scope, getOptionsKind)
+			query := req.Request.URL.Query()
+			if subpath {
+				newQuery := make(url.Values)
+				for k, v := range query {
+					newQuery[k] = v
+				}
+				newQuery[subpathKey] = []string{req.PathParameter("path")}
+				query = newQuery
+			}
+			opts, err := queryToObject(query, scope, getOptionsKind)
 			if err != nil {
 				return nil, err
 			}

pkg/registry/pod/etcd/etcd.go

@@ -110,6 +110,8 @@ func (r *BindingREST) New() runtime.Object {
 	return &api.Binding{}
 }
 
+var _ = rest.Creater(&BindingREST{})
+
 // Create ensures a pod is bound to a specific host.
 func (r *BindingREST) Create(ctx api.Context, obj runtime.Object) (out runtime.Object, err error) {
 	binding := obj.(*api.Binding)
@@ -197,6 +199,9 @@ type LogREST struct {
 	kubeletConn client.ConnectionInfoGetter
 }
 
+// LogREST implements GetterWithOptions
+var _ = rest.GetterWithOptions(&LogREST{})
+
 // New creates a new Pod log options object
 func (r *LogREST) New() runtime.Object {
 	return &api.PodLogOptions{}
@@ -221,6 +226,6 @@ func (r *LogREST) Get(ctx api.Context, name string, opts runtime.Object) (runtim
 }
 
 // NewGetOptions creates a new options object
-func (r *LogREST) NewGetOptions() runtime.Object {
+func (r *LogREST) NewGetOptions() (runtime.Object, bool, string) {
-	return &api.PodLogOptions{}
+	return &api.PodLogOptions{}, false, ""
 }