Move namespace from query param to path part

This commit is contained in:
derekwaynecarr
2014-12-09 14:23:21 -05:00
parent 58ba3c7faa
commit 7cf664439f
16 changed files with 388 additions and 156 deletions

View File

@@ -222,6 +222,7 @@ func (g *APIGroupVersion) InstallREST(container *restful.Container, root string,
for path, storage := range g.handler.storage {
registerResourceHandlers(ws, version, path, storage, kinds, h)
registerResourceHandlers(ws, version, "ns/{namespace}/"+path, storage, kinds, h)
}
// TODO: port the rest of these. Sadly, if we don't, we'll have inconsistent

View File

@@ -683,7 +683,7 @@ func TestSyncCreate(t *testing.T) {
t: t,
name: "bar",
namespace: "other",
expectedSet: "/prefix/version/foo/bar?namespace=other",
expectedSet: "/prefix/version/ns/other/foo/bar",
}
handler := Handle(map[string]RESTStorage{
"foo": &storage,
@@ -696,7 +696,7 @@ func TestSyncCreate(t *testing.T) {
Other: "bar",
}
data, _ := codec.Encode(simple)
request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo?sync=true", bytes.NewBuffer(data))
request, err := http.NewRequest("POST", server.URL+"/prefix/version/ns/other/foo?sync=true", bytes.NewBuffer(data))
if err != nil {
t.Errorf("unexpected error: %v", err)
}

View File

@@ -23,6 +23,7 @@ import (
"runtime/debug"
"strings"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/authorizer"
authhandlers "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/handlers"
"github.com/GoogleCloudPlatform/kubernetes/pkg/httplog"
@@ -40,24 +41,6 @@ var specialVerbs = map[string]bool{
"watch": true,
}
// KindFromRequest returns Kind if Kind can be extracted from the request. Otherwise, the empty string.
func KindFromRequest(req http.Request) string {
// TODO: find a way to keep this code's assumptions about paths up to date with changes in the code. Maybe instead
// of directly adding handler's code to the master's Mux, have a function which forces the structure when adding
// them.
parts := splitPath(req.URL.Path)
if len(parts) > 2 && parts[0] == "api" {
if _, ok := specialVerbs[parts[2]]; ok {
if len(parts) > 3 {
return parts[3]
}
} else {
return parts[2]
}
}
return ""
}
// IsReadOnlyReq() is true for any (or at least many) request which has no observable
// side effects on state of apiserver (though there may be internal side effects like
// caching and logging).
@@ -185,14 +168,16 @@ func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attrib
attribs.ReadOnly = IsReadOnlyReq(*req)
namespace, kind, _, _ := KindAndNamespace(req)
// If a path follows the conventions of the REST object store, then
// we can extract the object Kind. Otherwise, not.
attribs.Kind = KindFromRequest(*req)
attribs.Kind = kind
// If the request specifies a namespace, then the namespace is filled in.
// Assumes there is no empty string namespace. Unspecified results
// in empty (does not understand defaulting rules.)
attribs.Namespace = req.URL.Query().Get("namespace")
attribs.Namespace = namespace
return &attribs
}
@@ -208,3 +193,79 @@ func WithAuthorizationCheck(handler http.Handler, getAttribs RequestAttributeGet
forbidden(w, req)
})
}
// KindAndNamespace returns the kind, namespace, and path parts for the request relative to /{kind}/{name}
// Valid Inputs:
// Storage paths
// /ns/{namespace}/{kind}
// /ns/{namespace}/{kind}/{resourceName}
// /{kind}
// /{kind}/{resourceName}
// /{kind}/{resourceName}?namespace={namespace}
// /{kind}?namespace={namespace}
//
// Special verbs:
// /proxy/{kind}/{resourceName}
// /proxy/ns/{namespace}/{kind}/{resourceName}
// /redirect/ns/{namespace}/{kind}/{resourceName}
// /redirect/{kind}/{resourceName}
// /watch/{kind}
// /watch/ns/{namespace}/{kind}
//
// Fully qualified paths for above:
// /api/{version}/*
// /api/{version}/*
func KindAndNamespace(req *http.Request) (namespace, kind string, parts []string, err error) {
parts = splitPath(req.URL.Path)
if len(parts) < 1 {
err = fmt.Errorf("Unable to determine kind and namespace from an empty URL path")
return
}
// handle input of form /api/{version}/* by adjusting special paths
if parts[0] == "api" {
if len(parts) > 2 {
parts = parts[2:]
} else {
err = fmt.Errorf("Unable to determine kind and namespace from url, %v", req.URL)
return
}
}
// handle input of form /{specialVerb}/*
if _, ok := specialVerbs[parts[0]]; ok {
if len(parts) > 1 {
parts = parts[1:]
} else {
err = fmt.Errorf("Unable to determine kind and namespace from url, %v", req.URL)
return
}
}
// URL forms: /ns/{namespace}/{kind}/*, where parts are adjusted to be relative to kind
if parts[0] == "ns" {
if len(parts) < 3 {
err = fmt.Errorf("ResourceTypeAndNamespace expects a path of form /ns/{namespace}/*")
return
}
namespace = parts[1]
kind = parts[2]
parts = parts[2:]
return
}
// URL forms: /{kind}/*
// URL forms: POST /{kind} is a legacy API convention to create in "default" namespace
// URL forms: /{kind}/{resourceName} use the "default" namespace if omitted from query param
// URL forms: /{kind} assume cross-namespace operation if omitted from query param
kind = parts[0]
namespace = req.URL.Query().Get("namespace")
if len(namespace) == 0 {
if len(parts) > 1 || req.Method == "POST" {
namespace = api.NamespaceDefault
} else {
namespace = api.NamespaceAll
}
}
return
}

View File

@@ -19,7 +19,10 @@ package apiserver
import (
"net/http"
"net/http/httptest"
"reflect"
"testing"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
)
type fakeRL bool
@@ -59,3 +62,75 @@ func TestReadOnly(t *testing.T) {
http.DefaultClient.Do(req)
}
}
func TestKindAndNamespace(t *testing.T) {
successCases := []struct {
url string
expectedNamespace string
expectedKind string
expectedParts []string
}{
// resource paths
{"/ns/other/pods", "other", "pods", []string{"pods"}},
{"/ns/other/pods/foo", "other", "pods", []string{"pods", "foo"}},
{"/pods", api.NamespaceAll, "pods", []string{"pods"}},
{"/pods/foo", api.NamespaceDefault, "pods", []string{"pods", "foo"}},
{"/pods/foo?namespace=other", "other", "pods", []string{"pods", "foo"}},
{"/pods?namespace=other", "other", "pods", []string{"pods"}},
// special verbs
{"/proxy/ns/other/pods/foo", "other", "pods", []string{"pods", "foo"}},
{"/proxy/pods/foo", api.NamespaceDefault, "pods", []string{"pods", "foo"}},
{"/redirect/ns/other/pods/foo", "other", "pods", []string{"pods", "foo"}},
{"/redirect/pods/foo", api.NamespaceDefault, "pods", []string{"pods", "foo"}},
{"/watch/pods", api.NamespaceAll, "pods", []string{"pods"}},
{"/watch/ns/other/pods", "other", "pods", []string{"pods"}},
// full-qualified paths
{"/api/v1beta1/ns/other/pods", "other", "pods", []string{"pods"}},
{"/api/v1beta1/ns/other/pods/foo", "other", "pods", []string{"pods", "foo"}},
{"/api/v1beta1/pods", api.NamespaceAll, "pods", []string{"pods"}},
{"/api/v1beta1/pods/foo", api.NamespaceDefault, "pods", []string{"pods", "foo"}},
{"/api/v1beta1/pods/foo?namespace=other", "other", "pods", []string{"pods", "foo"}},
{"/api/v1beta1/pods?namespace=other", "other", "pods", []string{"pods"}},
{"/api/v1beta1/proxy/pods/foo", api.NamespaceDefault, "pods", []string{"pods", "foo"}},
{"/api/v1beta1/redirect/pods/foo", api.NamespaceDefault, "pods", []string{"pods", "foo"}},
{"/api/v1beta1/watch/pods", api.NamespaceAll, "pods", []string{"pods"}},
{"/api/v1beta1/watch/ns/other/pods", "other", "pods", []string{"pods"}},
}
for _, successCase := range successCases {
req, _ := http.NewRequest("GET", successCase.url, nil)
namespace, kind, parts, err := KindAndNamespace(req)
if err != nil {
t.Errorf("Unexpected error for url: %s", successCase.url)
}
if successCase.expectedNamespace != namespace {
t.Errorf("Unexpected namespace for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedNamespace, namespace)
}
if successCase.expectedKind != kind {
t.Errorf("Unexpected resourceType for url: %s, expected: %s, actual: %s", successCase.url, successCase.expectedKind, kind)
}
if !reflect.DeepEqual(successCase.expectedParts, parts) {
t.Errorf("Unexpected parts for url: %s, expected: %v, actual: %v", successCase.url, successCase.expectedParts, parts)
}
}
errorCases := map[string]string{
"no resource path": "/",
"missing resource type": "/ns/other",
"just apiversion": "/api/v1beta1/",
"apiversion with no resource": "/api/v1beta1/",
"apiversion with just namespace": "/api/v1beta1/ns/other",
}
for k, v := range errorCases {
req, err := http.NewRequest("GET", v, nil)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
_, _, _, err = KindAndNamespace(req)
if err == nil {
t.Errorf("Expected error for key: %s", k)
}
}
}

View File

@@ -78,35 +78,31 @@ type ProxyHandler struct {
}
func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
// use the default namespace to address the service
ctx := api.NewDefaultContext()
// if not in default namespace, provide the query parameter
// TODO this will need to go in the path in the future and not as a query parameter
namespace := req.URL.Query().Get("namespace")
if len(namespace) > 0 {
ctx = api.WithNamespace(ctx, namespace)
namespace, kind, parts, err := KindAndNamespace(req)
if err != nil {
notFound(w, req)
return
}
parts := strings.SplitN(req.URL.Path, "/", 3)
ctx := api.WithNamespace(api.NewContext(), namespace)
if len(parts) < 2 {
notFound(w, req)
return
}
resourceName := parts[0]
id := parts[1]
rest := ""
if len(parts) == 3 {
rest = parts[2]
}
storage, ok := r.storage[resourceName]
storage, ok := r.storage[kind]
if !ok {
httplog.LogOf(req, w).Addf("'%v' has no storage object", resourceName)
httplog.LogOf(req, w).Addf("'%v' has no storage object", kind)
notFound(w, req)
return
}
redirector, ok := storage.(Redirector)
if !ok {
httplog.LogOf(req, w).Addf("'%v' is not a redirector", resourceName)
httplog.LogOf(req, w).Addf("'%v' is not a redirector", kind)
notFound(w, req)
return
}
@@ -150,7 +146,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
proxy.Transport = &proxyTransport{
proxyScheme: req.URL.Scheme,
proxyHost: req.URL.Host,
proxyPathPrepend: path.Join(r.prefix, resourceName, id),
proxyPathPrepend: path.Join(r.prefix, "ns", namespace, kind, id),
}
proxy.FlushInterval = 200 * time.Millisecond
proxy.ServeHTTP(w, newReq)

View File

@@ -142,7 +142,7 @@ func TestProxy(t *testing.T) {
{"POST", "/some/other/dir", "question", "answer", "default"},
{"PUT", "/some/dir/id", "different question", "answer", "default"},
{"DELETE", "/some/dir/id", "", "ok", "default"},
{"GET", "/some/dir/id?namespace=other", "", "answer", "other"},
{"GET", "/some/dir/id", "", "answer", "other"},
}
for _, item := range table {
@@ -169,27 +169,34 @@ func TestProxy(t *testing.T) {
server := httptest.NewServer(handler)
defer server.Close()
req, err := http.NewRequest(
item.method,
server.URL+"/prefix/version/proxy/foo/id"+item.path,
strings.NewReader(item.reqBody),
)
if err != nil {
t.Errorf("%v - unexpected error %v", item.method, err)
continue
// test each supported URL pattern for finding the redirection resource in the proxy in a particular namespace
proxyTestPatterns := []string{
"/prefix/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace,
"/prefix/version/proxy/ns/" + item.reqNamespace + "/foo/id" + item.path,
}
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Errorf("%v - unexpected error %v", item.method, err)
continue
}
gotResp, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Errorf("%v - unexpected error %v", item.method, err)
}
resp.Body.Close()
if e, a := item.respBody, string(gotResp); e != a {
t.Errorf("%v - expected %v, got %v", item.method, e, a)
for _, proxyTestPattern := range proxyTestPatterns {
req, err := http.NewRequest(
item.method,
server.URL+proxyTestPattern,
strings.NewReader(item.reqBody),
)
if err != nil {
t.Errorf("%v - unexpected error %v", item.method, err)
continue
}
resp, err := http.DefaultClient.Do(req)
if err != nil {
t.Errorf("%v - unexpected error %v", item.method, err)
continue
}
gotResp, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Errorf("%v - unexpected error %v", item.method, err)
}
resp.Body.Close()
if e, a := item.respBody, string(gotResp); e != a {
t.Errorf("%v - expected %v, got %v", item.method, e, a)
}
}
}
}

View File

@@ -30,28 +30,29 @@ type RedirectHandler struct {
}
func (r *RedirectHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
ctx := api.NewDefaultContext()
namespace := req.URL.Query().Get("namespace")
if len(namespace) > 0 {
ctx = api.WithNamespace(ctx, namespace)
namespace, kind, parts, err := KindAndNamespace(req)
if err != nil {
notFound(w, req)
return
}
parts := splitPath(req.URL.Path)
ctx := api.WithNamespace(api.NewContext(), namespace)
// redirection requires /kind/resourceName path parts
if len(parts) != 2 || req.Method != "GET" {
notFound(w, req)
return
}
resourceName := parts[0]
id := parts[1]
storage, ok := r.storage[resourceName]
storage, ok := r.storage[kind]
if !ok {
httplog.LogOf(req, w).Addf("'%v' has no storage object", resourceName)
httplog.LogOf(req, w).Addf("'%v' has no storage object", kind)
notFound(w, req)
return
}
redirector, ok := storage.(Redirector)
if !ok {
httplog.LogOf(req, w).Addf("'%v' is not a redirector", resourceName)
httplog.LogOf(req, w).Addf("'%v' is not a redirector", kind)
notFound(w, req)
return
}

View File

@@ -76,3 +76,56 @@ func TestRedirect(t *testing.T) {
}
}
}
func TestRedirectWithNamespaces(t *testing.T) {
simpleStorage := &SimpleRESTStorage{
errors: map[string]error{},
expectedResourceNamespace: "other",
}
handler := Handle(map[string]RESTStorage{
"foo": simpleStorage,
}, codec, "/prefix", "version", selfLinker)
server := httptest.NewServer(handler)
defer server.Close()
dontFollow := errors.New("don't follow")
client := http.Client{
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return dontFollow
},
}
table := []struct {
id string
err error
code int
}{
{"cozy", nil, http.StatusTemporaryRedirect},
{"horse", errors.New("no such id"), http.StatusInternalServerError},
}
for _, item := range table {
simpleStorage.errors["resourceLocation"] = item.err
simpleStorage.resourceLocation = item.id
resp, err := client.Get(server.URL + "/prefix/version/redirect/ns/other/foo/" + item.id)
if resp == nil {
t.Fatalf("Unexpected nil resp")
}
resp.Body.Close()
if e, a := item.code, resp.StatusCode; e != a {
t.Errorf("Expected %v, got %v", e, a)
}
if e, a := item.id, simpleStorage.requestedResourceLocationID; e != a {
t.Errorf("Expected %v, got %v", e, a)
}
if item.err != nil {
continue
}
if err == nil || err.(*url.Error).Err != dontFollow {
t.Errorf("Unexpected err %#v", err)
}
if e, a := item.id, resp.Header.Get("Location"); e != a {
t.Errorf("Expected %v, got %v", e, a)
}
}
}

View File

@@ -41,19 +41,19 @@ type RESTHandler struct {
// ServeHTTP handles requests to all RESTStorage objects.
func (h *RESTHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
parts := splitPath(req.URL.Path)
if len(parts) < 1 {
namespace, kind, parts, err := KindAndNamespace(req)
if err != nil {
notFound(w, req)
return
}
storage := h.storage[parts[0]]
storage := h.storage[kind]
if storage == nil {
httplog.LogOf(req, w).Addf("'%v' has no storage object", parts[0])
httplog.LogOf(req, w).Addf("'%v' has no storage object", kind)
notFound(w, req)
return
}
h.handleRESTStorage(parts, req, w, storage)
h.handleRESTStorage(parts, req, w, storage, namespace)
}
// Sets the SelfLink field of the object.
@@ -66,12 +66,17 @@ func (h *RESTHandler) setSelfLink(obj runtime.Object, req *http.Request) error {
if err != nil {
return err
}
// TODO Remove this when namespace is in path
// we need to add namespace as a query param, if its not in the resource path
if len(namespace) > 0 {
query := newURL.Query()
query.Set("namespace", namespace)
newURL.RawQuery = query.Encode()
parts := splitPath(req.URL.Path)
if parts[0] != "ns" {
query := newURL.Query()
query.Set("namespace", namespace)
newURL.RawQuery = query.Encode()
}
}
err = h.selfLinker.SetSelfLink(obj, newURL.String())
if err != nil {
return err
@@ -107,11 +112,14 @@ func (h *RESTHandler) setSelfLinkAddName(obj runtime.Object, req *http.Request)
newURL.Path = path.Join(h.canonicalPrefix, req.URL.Path, name)
newURL.RawQuery = ""
newURL.Fragment = ""
// TODO Remove this when namespace is in path
// we need to add namespace as a query param, if its not in the resource path
if len(namespace) > 0 {
query := newURL.Query()
query.Set("namespace", namespace)
newURL.RawQuery = query.Encode()
parts := splitPath(req.URL.Path)
if parts[0] != "ns" {
query := newURL.Query()
query.Set("namespace", namespace)
newURL.RawQuery = query.Encode()
}
}
return h.selfLinker.SetSelfLink(obj, newURL.String())
}
@@ -138,18 +146,10 @@ func curry(f func(runtime.Object, *http.Request) error, req *http.Request) func(
// sync=[false|true] Synchronous request (only applies to create, update, delete operations)
// timeout=<duration> Timeout for synchronous requests, only applies if sync=true
// labels=<label-selector> Used for filtering list operations
func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w http.ResponseWriter, storage RESTStorage) {
ctx := api.NewContext()
func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w http.ResponseWriter, storage RESTStorage, namespace string) {
ctx := api.WithNamespace(api.NewContext(), namespace)
sync := req.URL.Query().Get("sync") == "true"
timeout := parseTimeout(req.URL.Query().Get("timeout"))
// TODO for now, we pull namespace from query parameter, but according to spec, it must go in resource path in future PR
// if a namespace if specified, it's always used.
// for list/watch operations, a namespace is not required if omitted.
// for all other operations, if namespace is omitted, we will default to default namespace.
namespace := req.URL.Query().Get("namespace")
if len(namespace) > 0 {
ctx = api.WithNamespace(ctx, namespace)
}
switch req.Method {
case "GET":
switch len(parts) {
@@ -175,7 +175,7 @@ func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w htt
}
writeJSON(http.StatusOK, h.codec, list, w)
case 2:
item, err := storage.Get(api.WithNamespaceDefaultIfNone(ctx), parts[1])
item, err := storage.Get(ctx, parts[1])
if err != nil {
errorJSON(err, h.codec, w)
return
@@ -205,7 +205,7 @@ func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w htt
errorJSON(err, h.codec, w)
return
}
out, err := storage.Create(api.WithNamespaceDefaultIfNone(ctx), obj)
out, err := storage.Create(ctx, obj)
if err != nil {
errorJSON(err, h.codec, w)
return
@@ -218,7 +218,7 @@ func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w htt
notFound(w, req)
return
}
out, err := storage.Delete(api.WithNamespaceDefaultIfNone(ctx), parts[1])
out, err := storage.Delete(ctx, parts[1])
if err != nil {
errorJSON(err, h.codec, w)
return
@@ -242,7 +242,7 @@ func (h *RESTHandler) handleRESTStorage(parts []string, req *http.Request, w htt
errorJSON(err, h.codec, w)
return
}
out, err := storage.Update(api.WithNamespaceDefaultIfNone(ctx), obj)
out, err := storage.Update(ctx, obj)
if err != nil {
errorJSON(err, h.codec, w)
return

View File

@@ -77,17 +77,19 @@ func isWebsocketRequest(req *http.Request) bool {
// ServeHTTP processes watch requests.
func (h *WatchHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
ctx := api.NewContext()
namespace := req.URL.Query().Get("namespace")
if len(namespace) > 0 {
ctx = api.WithNamespace(ctx, namespace)
}
parts := splitPath(req.URL.Path)
if len(parts) < 1 || req.Method != "GET" {
if req.Method != "GET" {
notFound(w, req)
return
}
storage := h.storage[parts[0]]
namespace, kind, _, err := KindAndNamespace(req)
if err != nil {
notFound(w, req)
return
}
ctx := api.WithNamespace(api.NewContext(), namespace)
storage := h.storage[kind]
if storage == nil {
notFound(w, req)
return