diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index bb61e648d3a..cd9cdfa559a 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -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 diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index c4f5178429c..9436298f1a5 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -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) } diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index a853050583c..e7374f18df0 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -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 +} diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index af18d7d691a..8104361cec0 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -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) + } + } +} diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go index 89be5a9f500..1b2f26ce234 100644 --- a/pkg/apiserver/proxy.go +++ b/pkg/apiserver/proxy.go @@ -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) diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go index 73e4dba1f0d..109bde3f72f 100644 --- a/pkg/apiserver/proxy_test.go +++ b/pkg/apiserver/proxy_test.go @@ -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) + } } } } diff --git a/pkg/apiserver/redirect.go b/pkg/apiserver/redirect.go index a9832291f24..bdc9aa44fbf 100644 --- a/pkg/apiserver/redirect.go +++ b/pkg/apiserver/redirect.go @@ -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 } diff --git a/pkg/apiserver/redirect_test.go b/pkg/apiserver/redirect_test.go index 70bce205083..f85bfb8e70b 100644 --- a/pkg/apiserver/redirect_test.go +++ b/pkg/apiserver/redirect_test.go @@ -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) + } + } +} diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 9b31701844c..478328ec1f2 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -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= Timeout for synchronous requests, only applies if sync=true // labels= 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 diff --git a/pkg/apiserver/watch.go b/pkg/apiserver/watch.go index f8921196a8f..95dee6310ad 100644 --- a/pkg/apiserver/watch.go +++ b/pkg/apiserver/watch.go @@ -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 diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 92b986246ec..d7ada22541a 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -161,10 +161,18 @@ func (c *testClient) ValidateCommon(t *testing.T, err error) { } } +// convenience function to build paths +func buildResourcePath(namespace, resource string) string { + if len(namespace) > 0 { + return path.Join("ns", namespace, resource) + } + return resource +} + func TestListEmptyPods(t *testing.T) { ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/pods"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods")}, Response: Response{StatusCode: 200, Body: &api.PodList{}}, } podList, err := c.Setup().Pods(ns).List(labels.Everything()) @@ -174,7 +182,7 @@ func TestListEmptyPods(t *testing.T) { func TestListPods(t *testing.T) { ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/pods"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods")}, Response: Response{StatusCode: 200, Body: &api.PodList{ Items: []api.Pod{ @@ -206,7 +214,7 @@ func validateLabels(a, b string) bool { func TestListPodsLabels(t *testing.T) { ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/pods", Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods"), Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, Response: Response{ StatusCode: 200, Body: &api.PodList{ @@ -236,7 +244,7 @@ func TestListPodsLabels(t *testing.T) { func TestGetPod(t *testing.T) { ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/pods/foo"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/pods/foo")}, Response: Response{ StatusCode: 200, Body: &api.Pod{ @@ -268,15 +276,17 @@ func TestGetPodWithNoName(t *testing.T) { } func TestDeletePod(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "DELETE", Path: "/pods/foo"}, + Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/pods/foo")}, Response: Response{StatusCode: 200}, } - err := c.Setup().Pods(api.NamespaceDefault).Delete("foo") + err := c.Setup().Pods(ns).Delete("foo") c.Validate(t, nil, err) } func TestCreatePod(t *testing.T) { + ns := api.NamespaceDefault requestPod := &api.Pod{ Status: api.PodStatus{ Phase: api.PodRunning, @@ -289,17 +299,18 @@ func TestCreatePod(t *testing.T) { }, } c := &testClient{ - Request: testRequest{Method: "POST", Path: "/pods", Body: requestPod}, + Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/pods"), Body: requestPod}, Response: Response{ StatusCode: 200, Body: requestPod, }, } - receivedPod, err := c.Setup().Pods(api.NamespaceDefault).Create(requestPod) + receivedPod, err := c.Setup().Pods(ns).Create(requestPod) c.Validate(t, receivedPod, err) } func TestUpdatePod(t *testing.T) { + ns := api.NamespaceDefault requestPod := &api.Pod{ ObjectMeta: api.ObjectMeta{ Name: "foo", @@ -314,10 +325,10 @@ func TestUpdatePod(t *testing.T) { }, } c := &testClient{ - Request: testRequest{Method: "PUT", Path: "/pods/foo"}, + Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/pods/foo")}, Response: Response{StatusCode: 200, Body: requestPod}, } - receivedPod, err := c.Setup().Pods(api.NamespaceDefault).Update(requestPod) + receivedPod, err := c.Setup().Pods(ns).Update(requestPod) c.Validate(t, receivedPod, err) } @@ -350,8 +361,9 @@ func TestListControllers(t *testing.T) { } func TestGetController(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/replicationControllers/foo"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/replicationControllers/foo")}, Response: Response{ StatusCode: 200, Body: &api.ReplicationController{ @@ -369,7 +381,7 @@ func TestGetController(t *testing.T) { }, }, } - receivedController, err := c.Setup().ReplicationControllers(api.NamespaceDefault).Get("foo") + receivedController, err := c.Setup().ReplicationControllers(ns).Get("foo") c.Validate(t, receivedController, err) } @@ -385,11 +397,12 @@ func TestGetControllerWithNoName(t *testing.T) { } func TestUpdateController(t *testing.T) { + ns := api.NamespaceDefault requestController := &api.ReplicationController{ ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, } c := &testClient{ - Request: testRequest{Method: "PUT", Path: "/replicationControllers/foo"}, + Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/replicationControllers/foo")}, Response: Response{ StatusCode: 200, Body: &api.ReplicationController{ @@ -407,25 +420,27 @@ func TestUpdateController(t *testing.T) { }, }, } - receivedController, err := c.Setup().ReplicationControllers(api.NamespaceDefault).Update(requestController) + receivedController, err := c.Setup().ReplicationControllers(ns).Update(requestController) c.Validate(t, receivedController, err) } func TestDeleteController(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "DELETE", Path: "/replicationControllers/foo"}, + Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/replicationControllers/foo")}, Response: Response{StatusCode: 200}, } - err := c.Setup().ReplicationControllers(api.NamespaceDefault).Delete("foo") + err := c.Setup().ReplicationControllers(ns).Delete("foo") c.Validate(t, nil, err) } func TestCreateController(t *testing.T) { + ns := api.NamespaceDefault requestController := &api.ReplicationController{ ObjectMeta: api.ObjectMeta{Name: "foo"}, } c := &testClient{ - Request: testRequest{Method: "POST", Path: "/replicationControllers", Body: requestController}, + Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/replicationControllers"), Body: requestController}, Response: Response{ StatusCode: 200, Body: &api.ReplicationController{ @@ -443,7 +458,7 @@ func TestCreateController(t *testing.T) { }, }, } - receivedController, err := c.Setup().ReplicationControllers(api.NamespaceDefault).Create(requestController) + receivedController, err := c.Setup().ReplicationControllers(ns).Create(requestController) c.Validate(t, receivedController, err) } @@ -457,8 +472,9 @@ func body(obj runtime.Object, raw *string) *string { } func TestListServices(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/services"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services")}, Response: Response{StatusCode: 200, Body: &api.ServiceList{ Items: []api.Service{ @@ -480,14 +496,15 @@ func TestListServices(t *testing.T) { }, }, } - receivedServiceList, err := c.Setup().Services(api.NamespaceDefault).List(labels.Everything()) + receivedServiceList, err := c.Setup().Services(ns).List(labels.Everything()) t.Logf("received services: %v %#v", err, receivedServiceList) c.Validate(t, receivedServiceList, err) } func TestListServicesLabels(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/services", Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services"), Query: url.Values{"labels": []string{"foo=bar,name=baz"}}}, Response: Response{StatusCode: 200, Body: &api.ServiceList{ Items: []api.Service{ @@ -512,16 +529,17 @@ func TestListServicesLabels(t *testing.T) { c.Setup() c.QueryValidator["labels"] = validateLabels selector := labels.Set{"foo": "bar", "name": "baz"}.AsSelector() - receivedServiceList, err := c.Services(api.NamespaceDefault).List(selector) + receivedServiceList, err := c.Services(ns).List(selector) c.Validate(t, receivedServiceList, err) } func TestGetService(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/services/1"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/services/1")}, Response: Response{StatusCode: 200, Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}}, } - response, err := c.Setup().Services(api.NamespaceDefault).Get("1") + response, err := c.Setup().Services(ns).Get("1") c.Validate(t, response, err) } @@ -537,36 +555,40 @@ func TestGetServiceWithNoName(t *testing.T) { } func TestCreateService(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "POST", Path: "/services", Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}}, + Request: testRequest{Method: "POST", Path: buildResourcePath(ns, "/services"), Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}}, Response: Response{StatusCode: 200, Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}}, } - response, err := c.Setup().Services(api.NamespaceDefault).Create(&api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}) + response, err := c.Setup().Services(ns).Create(&api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}) c.Validate(t, response, err) } func TestUpdateService(t *testing.T) { + ns := api.NamespaceDefault svc := &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1", ResourceVersion: "1"}} c := &testClient{ - Request: testRequest{Method: "PUT", Path: "/services/service-1", Body: svc}, + Request: testRequest{Method: "PUT", Path: buildResourcePath(ns, "/services/service-1"), Body: svc}, Response: Response{StatusCode: 200, Body: svc}, } - response, err := c.Setup().Services(api.NamespaceDefault).Update(svc) + response, err := c.Setup().Services(ns).Update(svc) c.Validate(t, response, err) } func TestDeleteService(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "DELETE", Path: "/services/1"}, + Request: testRequest{Method: "DELETE", Path: buildResourcePath(ns, "/services/1")}, Response: Response{StatusCode: 200}, } - err := c.Setup().Services(api.NamespaceDefault).Delete("1") + err := c.Setup().Services(ns).Delete("1") c.Validate(t, nil, err) } func TestListEndpooints(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/endpoints"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/endpoints")}, Response: Response{StatusCode: 200, Body: &api.EndpointsList{ Items: []api.Endpoints{ @@ -578,16 +600,17 @@ func TestListEndpooints(t *testing.T) { }, }, } - receivedEndpointsList, err := c.Setup().Endpoints(api.NamespaceDefault).List(labels.Everything()) + receivedEndpointsList, err := c.Setup().Endpoints(ns).List(labels.Everything()) c.Validate(t, receivedEndpointsList, err) } func TestGetEndpoints(t *testing.T) { + ns := api.NamespaceDefault c := &testClient{ - Request: testRequest{Method: "GET", Path: "/endpoints/endpoint-1"}, + Request: testRequest{Method: "GET", Path: buildResourcePath(ns, "/endpoints/endpoint-1")}, Response: Response{StatusCode: 200, Body: &api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "endpoint-1"}}}, } - response, err := c.Setup().Endpoints(api.NamespaceDefault).Get("endpoint-1") + response, err := c.Setup().Endpoints(ns).Get("endpoint-1") c.Validate(t, response, err) } diff --git a/pkg/client/request.go b/pkg/client/request.go index 1e16b98eeaf..92509fa8ec0 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -136,7 +136,7 @@ func (r *Request) Namespace(namespace string) *Request { return r } if len(namespace) > 0 { - return r.setParam("namespace", namespace) + return r.Path("ns").Path(namespace) } return r } diff --git a/pkg/controller/replication_controller_test.go b/pkg/controller/replication_controller_test.go index be39c3df139..6f224b86463 100644 --- a/pkg/controller/replication_controller_test.go +++ b/pkg/controller/replication_controller_test.go @@ -221,7 +221,7 @@ func TestCreateReplica(t *testing.T) { }, Spec: controllerSpec.Spec.Template.Spec, } - fakeHandler.ValidateRequest(t, makeURL("/pods?namespace=default"), "POST", nil) + fakeHandler.ValidateRequest(t, makeURL("/ns/default/pods"), "POST", nil) actualPod, err := client.Codec.Decode([]byte(fakeHandler.RequestBody)) if err != nil { t.Errorf("Unexpected error: %#v", err) diff --git a/pkg/kubectl/resthelper.go b/pkg/kubectl/resthelper.go index eb36daa2ad7..b0fc6594e91 100644 --- a/pkg/kubectl/resthelper.go +++ b/pkg/kubectl/resthelper.go @@ -47,14 +47,14 @@ func NewRESTHelper(client RESTClient, mapping *meta.RESTMapping) *RESTHelper { } func (m *RESTHelper) Get(namespace, name string, selector labels.Selector) (runtime.Object, error) { - return m.RESTClient.Get().Path(m.Resource).Namespace(namespace).Path(name).SelectorParam("labels", selector).Do().Get() + return m.RESTClient.Get().Namespace(namespace).Path(m.Resource).Path(name).SelectorParam("labels", selector).Do().Get() } func (m *RESTHelper) Watch(namespace, resourceVersion string, labelSelector, fieldSelector labels.Selector) (watch.Interface, error) { return m.RESTClient.Get(). Path("watch"). - Path(m.Resource). Namespace(namespace). + Path(m.Resource). Param("resourceVersion", resourceVersion). SelectorParam("labels", labelSelector). SelectorParam("fields", fieldSelector). @@ -62,7 +62,7 @@ func (m *RESTHelper) Watch(namespace, resourceVersion string, labelSelector, fie } func (m *RESTHelper) Delete(namespace, name string) error { - return m.RESTClient.Delete().Path(m.Resource).Namespace(namespace).Path(name).Do().Error() + return m.RESTClient.Delete().Namespace(namespace).Path(m.Resource).Path(name).Do().Error() } func (m *RESTHelper) Create(namespace string, modify bool, data []byte) error { @@ -95,7 +95,7 @@ func (m *RESTHelper) Create(namespace string, modify bool, data []byte) error { } func createResource(c RESTClient, resourcePath, namespace string, data []byte) error { - return c.Post().Path(resourcePath).Namespace(namespace).Body(data).Do().Error() + return c.Post().Namespace(namespace).Path(resourcePath).Body(data).Do().Error() } func (m *RESTHelper) Update(namespace, name string, overwrite bool, data []byte) error { @@ -138,5 +138,5 @@ func (m *RESTHelper) Update(namespace, name string, overwrite bool, data []byte) } func updateResource(c RESTClient, resourcePath, namespace, name string, data []byte) error { - return c.Put().Path(resourcePath).Namespace(namespace).Path(name).Body(data).Do().Error() + return c.Put().Namespace(namespace).Path(resourcePath).Path(name).Body(data).Do().Error() } diff --git a/pkg/kubectl/resthelper_test.go b/pkg/kubectl/resthelper_test.go index 1c5025eb644..25e942c037c 100644 --- a/pkg/kubectl/resthelper_test.go +++ b/pkg/kubectl/resthelper_test.go @@ -37,6 +37,15 @@ func objBody(obj runtime.Object) io.ReadCloser { return ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(testapi.Codec(), obj)))) } +// splitPath returns the segments for a URL path. +func splitPath(path string) []string { + path = strings.Trim(path, "/") + if path == "" { + return []string{} + } + return strings.Split(path, "/") +} + func TestRESTHelperDelete(t *testing.T) { tests := []struct { Err bool @@ -65,12 +74,13 @@ func TestRESTHelperDelete(t *testing.T) { t.Errorf("unexpected method: %#v", req) return false } - if !strings.HasSuffix(req.URL.Path, "/foo") { - t.Errorf("url doesn't contain name: %#v", req) + parts := splitPath(req.URL.Path) + if parts[1] != "bar" { + t.Errorf("url doesn't contain namespace: %#v", req) return false } - if req.URL.Query().Get("namespace") != "bar" { - t.Errorf("url doesn't contain namespace: %#v", req) + if parts[2] != "foo" { + t.Errorf("url doesn't contain name: %#v", req) return false } return true @@ -105,7 +115,8 @@ func TestRESTHelperCreate(t *testing.T) { t.Errorf("unexpected method: %#v", req) return false } - if req.URL.Query().Get("namespace") != "bar" { + parts := splitPath(req.URL.Path) + if parts[1] != "bar" { t.Errorf("url doesn't contain namespace: %#v", req) return false } @@ -230,12 +241,13 @@ func TestRESTHelperGet(t *testing.T) { t.Errorf("unexpected method: %#v", req) return false } - if !strings.HasSuffix(req.URL.Path, "/foo") { - t.Errorf("url doesn't contain name: %#v", req) + parts := splitPath(req.URL.Path) + if parts[1] != "bar" { + t.Errorf("url doesn't contain namespace: %#v", req) return false } - if req.URL.Query().Get("namespace") != "bar" { - t.Errorf("url doesn't contain namespace: %#v", req) + if parts[2] != "foo" { + t.Errorf("url doesn't contain name: %#v", req) return false } return true @@ -273,12 +285,13 @@ func TestRESTHelperUpdate(t *testing.T) { t.Errorf("unexpected method: %#v", req) return false } - if !strings.HasSuffix(req.URL.Path, "/foo") { - t.Errorf("url doesn't contain name: %#v", req) + parts := splitPath(req.URL.Path) + if parts[1] != "bar" { + t.Errorf("url doesn't contain namespace: %#v", req) return false } - if req.URL.Query().Get("namespace") != "bar" { - t.Errorf("url doesn't contain namespace: %#v", req) + if parts[2] != "foo" { + t.Errorf("url doesn't contain name: %#v", req) return false } return true diff --git a/plugin/pkg/scheduler/factory/factory_test.go b/plugin/pkg/scheduler/factory/factory_test.go index 80e1864bc08..eda2932fffd 100644 --- a/plugin/pkg/scheduler/factory/factory_test.go +++ b/plugin/pkg/scheduler/factory/factory_test.go @@ -192,7 +192,7 @@ func TestDefaultErrorFunc(t *testing.T) { } mux := http.NewServeMux() // FakeHandler musn't be sent requests other than the one you want to test. - mux.Handle("/api/"+testapi.Version()+"/pods/foo", &handler) + mux.Handle("/api/"+testapi.Version()+"/ns/bar/pods/foo", &handler) server := httptest.NewServer(mux) defer server.Close() factory := NewConfigFactory(client.NewOrDie(&client.Config{Host: server.URL, Version: testapi.Version()})) @@ -213,7 +213,7 @@ func TestDefaultErrorFunc(t *testing.T) { if !exists { continue } - handler.ValidateRequest(t, "/api/"+testapi.Version()+"/pods/foo?namespace=bar", "GET", nil) + handler.ValidateRequest(t, "/api/"+testapi.Version()+"/ns/bar/pods/foo", "GET", nil) if e, a := testPod, got; !reflect.DeepEqual(e, a) { t.Errorf("Expected %v, got %v", e, a) }