From 72817a080174b5c4b68503598ecbdf515e3fada1 Mon Sep 17 00:00:00 2001 From: deads2k Date: Fri, 10 Apr 2015 12:42:52 -0400 Subject: [PATCH 1/2] add support for authorizing subresources --- pkg/apiserver/handlers.go | 46 +++++++++---------- pkg/apiserver/handlers_test.go | 83 ++++++++++++++++++---------------- 2 files changed, 68 insertions(+), 61 deletions(-) diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 87ead96c77f..3e897ed89e0 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -19,7 +19,6 @@ package apiserver import ( "fmt" "net/http" - "path" "regexp" "runtime/debug" "strings" @@ -43,6 +42,11 @@ var specialVerbs = map[string]bool{ "watch": true, } +// namespaceSubresouces is a set of all the subresources available on a namespace resource. This is a special case because +// URLs look like api/v1beta3/namspaces//[subresource | resource]. We need to be able to distinguish the two +// different cases. +var namespaceSubresources = util.NewStringSet("status", "finalize") + // Constant for the retry-after interval on rate limiting. // TODO: maybe make this dynamic? or user-adjustable? const RetryAfter = "1" @@ -240,6 +244,8 @@ type APIRequestInfo struct { Namespace string // Resource is the name of the resource being requested. This is not the kind. For example: pods Resource string + // Subresource is the name of the subresource being requested. This is not the kind or the resource. For example: status for a pods/pod-name/status + Subresource string // Kind is the type of object being manipulated. For example: Pod Kind string // Name is empty for some verbs, but if the request directly indicates a name (not in body content) then this field is filled in. @@ -251,17 +257,6 @@ type APIRequestInfo struct { Raw []string } -// URLPath returns the URL path for this request, including /{resource}/{name} if present but nothing -// following that. -func (info APIRequestInfo) URLPath() string { - p := info.Parts - if n := len(p); n > 2 { - // Only take resource and name - p = p[:2] - } - return path.Join("/", path.Join(info.Raw...), path.Join(p...)) -} - type APIRequestInfoResolver struct { APIPrefixes util.StringSet RestMapper meta.RESTMapper @@ -340,22 +335,20 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques // URL forms: /namespaces/{namespace}/{kind}/*, where parts are adjusted to be relative to kind if currentParts[0] == "namespaces" { - if len(currentParts) < 3 { - requestInfo.Resource = "namespaces" - if len(currentParts) > 1 { - requestInfo.Namespace = currentParts[1] - } - } else { - requestInfo.Resource = currentParts[2] + if len(currentParts) > 1 { requestInfo.Namespace = currentParts[1] - currentParts = currentParts[2:] + + // if there is another step after the namespace name and it is not a known namespace subresource + // move currentParts to include it as a resource in its own right + if len(currentParts) > 2 && !namespaceSubresources.Has(currentParts[2]) { + currentParts = currentParts[2:] + } } } else { // URL forms: /{resource}/* // URL forms: POST /{resource} is a legacy API convention to create in "default" namespace // URL forms: /{resource}/{resourceName} use the "default" namespace if omitted from query param // URL forms: /{resource} assume cross-namespace operation if omitted from query param - requestInfo.Resource = currentParts[0] requestInfo.Namespace = req.URL.Query().Get("namespace") if len(requestInfo.Namespace) == 0 { if len(currentParts) > 1 || req.Method == "POST" { @@ -371,9 +364,16 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques // Raw should have everything not in Parts requestInfo.Raw = requestInfo.Raw[:len(requestInfo.Raw)-len(currentParts)] - // if there's another part remaining after the kind, then that's the resource name - if len(requestInfo.Parts) >= 2 { + // parts look like: resource/resourceName/subresource/other/stuff/we/don't/interpret + switch { + case len(requestInfo.Parts) >= 3: + requestInfo.Subresource = requestInfo.Parts[2] + fallthrough + case len(requestInfo.Parts) == 2: requestInfo.Name = requestInfo.Parts[1] + fallthrough + case len(requestInfo.Parts) == 1: + requestInfo.Resource = requestInfo.Parts[0] } // if there's no name on the request and we thought it was a get before, then the actual verb is a list diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index 40f0a9a72cb..9015f60be59 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -135,49 +135,56 @@ func TestReadOnly(t *testing.T) { func TestGetAPIRequestInfo(t *testing.T) { successCases := []struct { - method string - url string - expectedVerb string - expectedAPIVersion string - expectedNamespace string - expectedResource string - expectedKind string - expectedName string - expectedParts []string + method string + url string + expectedVerb string + expectedAPIVersion string + expectedNamespace string + expectedResource string + expectedSubresource string + expectedKind string + expectedName string + expectedParts []string }{ // resource paths - {"GET", "/namespaces", "list", "", "", "namespaces", "Namespace", "", []string{"namespaces"}}, - {"GET", "/namespaces/other", "get", "", "other", "namespaces", "Namespace", "other", []string{"namespaces", "other"}}, + {"GET", "/namespaces", "list", "", "", "namespaces", "", "Namespace", "", []string{"namespaces"}}, + {"GET", "/namespaces/other", "get", "", "other", "namespaces", "", "Namespace", "other", []string{"namespaces", "other"}}, - {"GET", "/namespaces/other/pods", "list", "", "other", "pods", "Pod", "", []string{"pods"}}, - {"GET", "/namespaces/other/pods/foo", "get", "", "other", "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/pods", "list", "", api.NamespaceAll, "pods", "Pod", "", []string{"pods"}}, - {"POST", "/pods", "create", "", api.NamespaceDefault, "pods", "Pod", "", []string{"pods"}}, - {"GET", "/pods/foo", "get", "", api.NamespaceDefault, "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/pods/foo?namespace=other", "get", "", "other", "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/pods?namespace=other", "list", "", "other", "pods", "Pod", "", []string{"pods"}}, + {"GET", "/namespaces/other/pods", "list", "", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/namespaces/other/pods/foo", "get", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/pods", "list", "", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, + {"POST", "/pods", "create", "", api.NamespaceDefault, "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/pods/foo", "get", "", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/pods/foo?namespace=other", "get", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/pods?namespace=other", "list", "", "other", "pods", "", "Pod", "", []string{"pods"}}, // special verbs - {"GET", "/proxy/namespaces/other/pods/foo", "proxy", "", "other", "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/proxy/pods/foo", "proxy", "", api.NamespaceDefault, "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/redirect/namespaces/other/pods/foo", "redirect", "", "other", "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/redirect/pods/foo", "redirect", "", api.NamespaceDefault, "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/watch/pods", "watch", "", api.NamespaceAll, "pods", "Pod", "", []string{"pods"}}, - {"GET", "/watch/namespaces/other/pods", "watch", "", "other", "pods", "Pod", "", []string{"pods"}}, + {"GET", "/proxy/namespaces/other/pods/foo", "proxy", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/proxy/pods/foo", "proxy", "", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/redirect/namespaces/other/pods/foo", "redirect", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/redirect/pods/foo", "redirect", "", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/watch/pods", "watch", "", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/watch/namespaces/other/pods", "watch", "", "other", "pods", "", "Pod", "", []string{"pods"}}, // fully-qualified paths - {"GET", "/api/v1beta1/namespaces/other/pods", "list", "v1beta1", "other", "pods", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1beta1/namespaces/other/pods/foo", "get", "v1beta1", "other", "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1beta1/pods", "list", "v1beta1", api.NamespaceAll, "pods", "Pod", "", []string{"pods"}}, - {"POST", "/api/v1beta1/pods", "create", "v1beta1", api.NamespaceDefault, "pods", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1beta1/pods/foo", "get", "v1beta1", api.NamespaceDefault, "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1beta1/pods/foo?namespace=other", "get", "v1beta1", "other", "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1beta1/pods?namespace=other", "list", "v1beta1", "other", "pods", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1beta1/proxy/pods/foo", "proxy", "v1beta1", api.NamespaceDefault, "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1beta1/redirect/pods/foo", "redirect", "v1beta1", api.NamespaceDefault, "pods", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", "/api/v1beta1/watch/pods", "watch", "v1beta1", api.NamespaceAll, "pods", "Pod", "", []string{"pods"}}, - {"GET", "/api/v1beta1/watch/namespaces/other/pods", "watch", "v1beta1", "other", "pods", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1beta1/namespaces/other/pods", "list", "v1beta1", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1beta1/namespaces/other/pods/foo", "get", "v1beta1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1beta1/pods", "list", "v1beta1", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, + {"POST", "/api/v1beta1/pods", "create", "v1beta1", api.NamespaceDefault, "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1beta1/pods/foo", "get", "v1beta1", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1beta1/pods/foo?namespace=other", "get", "v1beta1", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1beta1/pods?namespace=other", "list", "v1beta1", "other", "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1beta1/proxy/pods/foo", "proxy", "v1beta1", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1beta1/redirect/pods/foo", "redirect", "v1beta1", api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/api/v1beta1/watch/pods", "watch", "v1beta1", api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, + {"GET", "/api/v1beta1/watch/namespaces/other/pods", "watch", "v1beta1", "other", "pods", "", "Pod", "", []string{"pods"}}, + + // subresource identification + {"GET", "/namespaces/other/pods/foo/status", "get", "", "other", "pods", "status", "Pod", "foo", []string{"pods", "foo", "status"}}, + {"GET", "/namespaces/other/finalize", "get", "", "other", "namespaces", "finalize", "Namespace", "other", []string{"namespaces", "other", "finalize"}}, + {"PUT", "/namespaces/other/status", "update", "", "other", "namespaces", "status", "Namespace", "other", []string{"namespaces", "other", "status"}}, + {"PUT", "/namespaces/other/anything", "update", "", "other", "anything", "", "", "", []string{"anything"}}, } apiRequestInfoResolver := &APIRequestInfoResolver{util.NewStringSet("api"), latest.RESTMapper} @@ -204,15 +211,15 @@ func TestGetAPIRequestInfo(t *testing.T) { if successCase.expectedResource != apiRequestInfo.Resource { t.Errorf("Unexpected resource for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedResource, apiRequestInfo.Resource) } + if successCase.expectedSubresource != apiRequestInfo.Subresource { + t.Errorf("Unexpected resource for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedSubresource, apiRequestInfo.Subresource) + } if successCase.expectedName != apiRequestInfo.Name { t.Errorf("Unexpected name for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedName, apiRequestInfo.Name) } if !reflect.DeepEqual(successCase.expectedParts, apiRequestInfo.Parts) { t.Errorf("Unexpected parts for url: %s, expected: %v, actual: %v", successCase.url, successCase.expectedParts, apiRequestInfo.Parts) } - if e, a := strings.Split(successCase.url, "?")[0], apiRequestInfo.URLPath(); e != a { - t.Errorf("Expected %v, got %v", e, a) - } } errorCases := map[string]string{ From c17ffb7c4c87b1f292d3ba1a8e34dcac8a01c75c Mon Sep 17 00:00:00 2001 From: deads2k Date: Fri, 10 Apr 2015 13:37:13 -0400 Subject: [PATCH 2/2] comments 1: comments --- pkg/apiserver/handlers.go | 12 +++++------- pkg/apiserver/handlers_test.go | 4 +--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 3e897ed89e0..ab0d83e6d8d 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -42,11 +42,6 @@ var specialVerbs = map[string]bool{ "watch": true, } -// namespaceSubresouces is a set of all the subresources available on a namespace resource. This is a special case because -// URLs look like api/v1beta3/namspaces//[subresource | resource]. We need to be able to distinguish the two -// different cases. -var namespaceSubresources = util.NewStringSet("status", "finalize") - // Constant for the retry-after interval on rate limiting. // TODO: maybe make this dynamic? or user-adjustable? const RetryAfter = "1" @@ -244,7 +239,9 @@ type APIRequestInfo struct { Namespace string // Resource is the name of the resource being requested. This is not the kind. For example: pods Resource string - // Subresource is the name of the subresource being requested. This is not the kind or the resource. For example: status for a pods/pod-name/status + // Subresource is the name of the subresource being requested. This is a different resource, scoped to the parent resource, but it may have a different kind. + // For instance, /pods has the resource "pods" and the kind "Pod", while /pods/foo/status has the resource "pods", the sub resource "status", and the kind "Pod" + // (because status operates on pods). The binding resource for a pod though may be /pods/foo/binding, which has resource "pods", subresource "binding", and kind "Binding". Subresource string // Kind is the type of object being manipulated. For example: Pod Kind string @@ -262,6 +259,7 @@ type APIRequestInfoResolver struct { RestMapper meta.RESTMapper } +// TODO write an integration test against the swagger doc to test the APIRequestInfo and match up behavior to responses // GetAPIRequestInfo returns the information from the http request. If error is not nil, APIRequestInfo holds the information as best it is known before the failure // Valid Inputs: // Storage paths @@ -340,7 +338,7 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques // if there is another step after the namespace name and it is not a known namespace subresource // move currentParts to include it as a resource in its own right - if len(currentParts) > 2 && !namespaceSubresources.Has(currentParts[2]) { + if len(currentParts) > 2 { currentParts = currentParts[2:] } } diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index 9015f60be59..af36e9965a9 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -182,9 +182,7 @@ func TestGetAPIRequestInfo(t *testing.T) { // subresource identification {"GET", "/namespaces/other/pods/foo/status", "get", "", "other", "pods", "status", "Pod", "foo", []string{"pods", "foo", "status"}}, - {"GET", "/namespaces/other/finalize", "get", "", "other", "namespaces", "finalize", "Namespace", "other", []string{"namespaces", "other", "finalize"}}, - {"PUT", "/namespaces/other/status", "update", "", "other", "namespaces", "status", "Namespace", "other", []string{"namespaces", "other", "status"}}, - {"PUT", "/namespaces/other/anything", "update", "", "other", "anything", "", "", "", []string{"anything"}}, + {"PUT", "/namespaces/other/finalize", "update", "", "other", "finalize", "", "", "", []string{"finalize"}}, } apiRequestInfoResolver := &APIRequestInfoResolver{util.NewStringSet("api"), latest.RESTMapper}