diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index be75150c58d2e..56140da460f76 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -20,7 +20,6 @@ import ( "fmt" "net" "net/http" - "net/url" gpath "path" "reflect" "sort" @@ -282,7 +281,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", nameParams, namer}, isConnecter && connectSubpath) } else { - // v1beta3 format with namespace in path + // v1beta3+ format with namespace in path if scope.ParamPath() { // Handler for standard REST verbs (GET, PUT, POST and DELETE). namespaceParam := ws.PathParameter(scope.ParamName(), scope.ParamDescription()).DataType("string") @@ -325,42 +324,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag actions = appendIf(actions, action{"LIST", resource, params, namer}, isLister) actions = appendIf(actions, action{"POST", resource, params, namer}, isCreater) actions = appendIf(actions, action{"WATCHLIST", "watch/" + resource, params, namer}, allowWatchList) - } else { - // Handler for standard REST verbs (GET, PUT, POST and DELETE). - // v1beta1/v1beta2 format where namespace was a query parameter - namespaceParam := ws.QueryParameter(scope.ParamName(), scope.ParamDescription()).DataType("string") - namespaceParams := []*restful.Parameter{namespaceParam} - - basePath := resource - resourcePath := basePath - resourceParams := namespaceParams - itemPath := resourcePath + "/{name}" - 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, resourceParams, namer}, isLister) - actions = appendIf(actions, action{"POST", resourcePath, resourceParams, namer}, isCreater) - 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) - actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) - actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath + "/{path:*}", proxyParams, namer}, isRedirector) - actions = appendIf(actions, action{"PROXY", "proxy/" + itemPath, nameParams, namer}, isRedirector) - actions = appendIf(actions, action{"CONNECT", itemPath, nameParams, namer}, isConnecter) - actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", nameParams, namer}, isConnecter && connectSubpath) + // Namespace as param is no longer supported + return fmt.Errorf("namespace as a parameter is no longer supported") } } @@ -712,6 +678,9 @@ func (n scopeNaming) GenerateLink(req *restful.Request, obj runtime.Object) (pat return "", "", err } } + if len(name) == 0 { + return "", "", errEmptyName + } path = strings.Replace(n.itemPath, "{name}", name, 1) path = strings.Replace(path, "{"+n.scope.ParamName()+"}", namespace, 1) return path, "", nil @@ -738,88 +707,6 @@ func (n scopeNaming) ObjectName(obj runtime.Object) (namespace, name string, err return namespace, name, err } -// legacyScopeNaming modifies a scopeNaming to read namespace from the query. It implements -// ScopeNamer for older query based namespace parameters. -type legacyScopeNaming struct { - scope meta.RESTScope - runtime.SelfLinker - itemPath string -} - -// legacyScopeNaming implements ScopeNamer -var _ ScopeNamer = legacyScopeNaming{} - -// Namespace returns the namespace from the query or the default. -func (n legacyScopeNaming) Namespace(req *restful.Request) (namespace string, err error) { - if n.scope.Name() == meta.RESTScopeNameRoot { - return api.NamespaceNone, nil - } - values, ok := req.Request.URL.Query()[n.scope.ParamName()] - if !ok || len(values) == 0 { - // legacy behavior - if req.Request.Method == "POST" || len(req.PathParameter("name")) > 0 { - return api.NamespaceDefault, nil - } - return api.NamespaceAll, nil - } - return values[0], nil -} - -// Name returns the name from the path, the namespace (or default), or an error if the -// name is empty. -func (n legacyScopeNaming) Name(req *restful.Request) (namespace, name string, err error) { - namespace, _ = n.Namespace(req) - name = req.PathParameter("name") - if len(name) == 0 { - return "", "", errEmptyName - } - return namespace, name, nil -} - -// GenerateLink returns the appropriate path and query to locate an object by its canonical path. -func (n legacyScopeNaming) GenerateLink(req *restful.Request, obj runtime.Object) (path, query string, err error) { - namespace, name, err := n.ObjectName(obj) - if err != nil { - return "", "", err - } - if len(name) == 0 { - return "", "", errEmptyName - } - path = strings.Replace(n.itemPath, "{name}", name, -1) - values := make(url.Values) - values.Set(n.scope.ParamName(), namespace) - query = values.Encode() - return path, query, nil -} - -// GenerateListLink returns the appropriate path and query to locate a list by its canonical path. -func (n legacyScopeNaming) GenerateListLink(req *restful.Request) (path, query string, err error) { - namespace, err := n.Namespace(req) - if err != nil { - return "", "", err - } - path = req.Request.URL.Path - values := make(url.Values) - values.Set(n.scope.ParamName(), namespace) - query = values.Encode() - return path, query, nil -} - -// ObjectName returns the name and namespace set on the object, or an error if the -// name cannot be returned. -// TODO: distinguish between objects with name/namespace and without via a specific error. -func (n legacyScopeNaming) ObjectName(obj runtime.Object) (namespace, name string, err error) { - name, err = n.SelfLinker.Name(obj) - if err != nil { - return "", "", err - } - namespace, err = n.SelfLinker.Namespace(obj) - if err != nil { - return "", "", err - } - return namespace, name, err -} - // This magic incantation returns *ptrToObject for an arbitrary pointer func indirectArbitraryPointer(ptrToObject interface{}) interface{} { return reflect.Indirect(reflect.ValueOf(ptrToObject)).Interface() diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 9e707c921496c..4de719e791a3d 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -64,7 +64,7 @@ var newCodec = runtime.CodecFor(api.Scheme, newVersion) var accessor = meta.NewAccessor() var versioner runtime.ResourceVersioner = accessor var selfLinker runtime.SelfLinker = accessor -var mapper, namespaceMapper, legacyNamespaceMapper meta.RESTMapper // The mappers with namespace and with legacy namespace scopes. +var mapper, namespaceMapper meta.RESTMapper // The mappers with namespace and with legacy namespace scopes. var admissionControl admission.Interface var requestContextMapper api.RequestContextMapper @@ -134,26 +134,21 @@ func init() { addNewTestTypes() nsMapper := newMapper() - legacyNsMapper := newMapper() // enumerate all supported versions, get the kinds, and register with // the mapper how to address our resources for _, version := range versions { for kind := range api.Scheme.KnownTypes(version) { - mixedCase := true root := kind == "SimpleRoot" if root { - legacyNsMapper.Add(meta.RESTScopeRoot, kind, version, mixedCase) - nsMapper.Add(meta.RESTScopeRoot, kind, version, mixedCase) + nsMapper.Add(meta.RESTScopeRoot, kind, version, false) } else { - legacyNsMapper.Add(meta.RESTScopeNamespaceLegacy, kind, version, mixedCase) - nsMapper.Add(meta.RESTScopeNamespace, kind, version, mixedCase) + nsMapper.Add(meta.RESTScopeNamespace, kind, version, false) } } } - mapper = legacyNsMapper - legacyNamespaceMapper = legacyNsMapper + mapper = nsMapper namespaceMapper = nsMapper admissionControl = admit.NewAlwaysAdmit() requestContextMapper = api.NewRequestContextMapper() @@ -220,7 +215,7 @@ func handleInternal(legacy bool, storage map[string]rest.Storage, admissionContr group.Version = testVersion group.ServerVersion = testVersion group.Codec = codec - group.Mapper = legacyNamespaceMapper + group.Mapper = namespaceMapper } else { group.Version = newVersion group.ServerVersion = newVersion @@ -612,20 +607,40 @@ func TestNotFound(t *testing.T) { Status int } cases := map[string]T{ - "PATCH method": {"PATCH", "/api/version/foo", http.StatusMethodNotAllowed}, - "GET long prefix": {"GET", "/api/", http.StatusNotFound}, - "GET missing storage": {"GET", "/api/version/blah", http.StatusNotFound}, - "GET with extra segment": {"GET", "/api/version/foo/bar/baz", http.StatusNotFound}, - "POST with extra segment": {"POST", "/api/version/foo/bar", http.StatusMethodNotAllowed}, - "DELETE without extra segment": {"DELETE", "/api/version/foo", http.StatusMethodNotAllowed}, - "DELETE with extra segment": {"DELETE", "/api/version/foo/bar/baz", http.StatusNotFound}, - "PUT without extra segment": {"PUT", "/api/version/foo", http.StatusMethodNotAllowed}, - "PUT with extra segment": {"PUT", "/api/version/foo/bar/baz", http.StatusNotFound}, - "watch missing storage": {"GET", "/api/version/watch/", http.StatusNotFound}, - "watch with bad method": {"POST", "/api/version/watch/foo/bar", http.StatusMethodNotAllowed}, + // Positive checks to make sure everything is wired correctly + "GET root": {"GET", "/api/version/simpleroots", http.StatusOK}, + // TODO: JTL: "GET root item": {"GET", "/api/version/simpleroots/bar", http.StatusOK}, + "GET namespaced": {"GET", "/api/version/namespaces/ns/simples", http.StatusOK}, + // TODO: JTL: "GET namespaced item": {"GET", "/api/version/namespaces/ns/simples/bar", http.StatusOK}, + + "GET long prefix": {"GET", "/api/", http.StatusNotFound}, + + "root PATCH method": {"PATCH", "/api/version/simpleroots", http.StatusMethodNotAllowed}, + "root GET missing storage": {"GET", "/api/version/blah", http.StatusNotFound}, + "root GET with extra segment": {"GET", "/api/version/simpleroots/bar/baz", http.StatusNotFound}, + // TODO: JTL: "root POST with extra segment": {"POST", "/api/version/simpleroots/bar", http.StatusMethodNotAllowed}, + "root DELETE without extra segment": {"DELETE", "/api/version/simpleroots", http.StatusMethodNotAllowed}, + "root DELETE with extra segment": {"DELETE", "/api/version/simpleroots/bar/baz", http.StatusNotFound}, + "root PUT without extra segment": {"PUT", "/api/version/simpleroots", http.StatusMethodNotAllowed}, + "root PUT with extra segment": {"PUT", "/api/version/simpleroots/bar/baz", http.StatusNotFound}, + "root watch missing storage": {"GET", "/api/version/watch/", http.StatusNotFound}, + // TODO: JTL: "root watch with bad method": {"POST", "/api/version/watch/simpleroot/bar", http.StatusMethodNotAllowed}, + + "namespaced PATCH method": {"PATCH", "/api/version/namespaces/ns/simples", http.StatusMethodNotAllowed}, + "namespaced GET long prefix": {"GET", "/api/", http.StatusNotFound}, + "namespaced GET missing storage": {"GET", "/api/version/blah", http.StatusNotFound}, + "namespaced GET with extra segment": {"GET", "/api/version/namespaces/ns/simples/bar/baz", http.StatusNotFound}, + "namespaced POST with extra segment": {"POST", "/api/version/namespaces/ns/simples/bar", http.StatusMethodNotAllowed}, + "namespaced DELETE without extra segment": {"DELETE", "/api/version/namespaces/ns/simples", http.StatusMethodNotAllowed}, + "namespaced DELETE with extra segment": {"DELETE", "/api/version/namespaces/ns/simples/bar/baz", http.StatusNotFound}, + "namespaced PUT without extra segment": {"PUT", "/api/version/namespaces/ns/simples", http.StatusMethodNotAllowed}, + "namespaced PUT with extra segment": {"PUT", "/api/version/namespaces/ns/simples/bar/baz", http.StatusNotFound}, + "namespaced watch missing storage": {"GET", "/api/version/watch/", http.StatusNotFound}, + "namespaced watch with bad method": {"POST", "/api/version/watch/namespaces/ns/simples/bar", http.StatusMethodNotAllowed}, } handler := handle(map[string]rest.Storage{ - "foo": &SimpleRESTStorage{}, + "simples": &SimpleRESTStorage{}, + "simpleroots": &SimpleRESTStorage{}, }) server := httptest.NewServer(handler) defer server.Close() @@ -738,31 +753,53 @@ func TestList(t *testing.T) { label string field string }{ + // legacy namespace param is ignored { - url: "/api/version/simple", + url: "/api/version/simple?namespace=", namespace: "", - selfLink: "/api/version/simple?namespace=", + selfLink: "/api/version/simple", legacy: true, }, { url: "/api/version/simple?namespace=other", - namespace: "other", - selfLink: "/api/version/simple?namespace=other", + namespace: "", + selfLink: "/api/version/simple", legacy: true, }, { url: "/api/version/simple?namespace=other&labels=a%3Db&fields=c%3Dd", + namespace: "", + selfLink: "/api/version/simple", + legacy: true, + label: "a=b", + field: "c=d", + }, + // legacy api version is honored + { + url: "/api/version/simple", + namespace: "", + selfLink: "/api/version/simple", + legacy: true, + }, + { + url: "/api/version/namespaces/other/simple", + namespace: "other", + selfLink: "/api/version/namespaces/other/simple", + legacy: true, + }, + { + url: "/api/version/namespaces/other/simple?labels=a%3Db&fields=c%3Dd", namespace: "other", - selfLink: "/api/version/simple?namespace=other", + selfLink: "/api/version/namespaces/other/simple", legacy: true, label: "a=b", field: "c=d", }, // list items across all namespaces { - url: "/api/version/simple?namespace=", + url: "/api/version/simple", namespace: "", - selfLink: "/api/version/simple?namespace=", + selfLink: "/api/version/simple", legacy: true, }, // list items in a namespace in the path @@ -896,10 +933,10 @@ func TestNonEmptyList(t *testing.T) { if listOut.Items[0].Other != simpleStorage.list[0].Other { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) } - if listOut.SelfLink != "/api/version/simple?namespace=" { + if listOut.SelfLink != "/api/version/simple" { t.Errorf("unexpected list self link: %#v", listOut) } - expectedSelfLink := "/api/version/simple/something?namespace=other" + expectedSelfLink := "/api/version/namespaces/other/simple/something" if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) } @@ -943,7 +980,7 @@ func TestSelfLinkSkipsEmptyName(t *testing.T) { if listOut.Items[0].Other != simpleStorage.list[0].Other { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) } - if listOut.SelfLink != "/api/version/simple?namespace=" { + if listOut.SelfLink != "/api/version/simple" { t.Errorf("unexpected list self link: %#v", listOut) } expectedSelfLink := "" @@ -981,7 +1018,7 @@ func TestGet(t *testing.T) { } selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/api/version/simple/id?namespace=default", + expectedSet: "/api/version/namespaces/default/simple/id", name: "id", namespace: "default", } @@ -990,7 +1027,7 @@ func TestGet(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/id") + resp, err := http.Get(server.URL + "/api/version/namespaces/default/simple/id") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1022,7 +1059,7 @@ func TestGetBinary(t *testing.T) { server := httptest.NewServer(handle(map[string]rest.Storage{"simple": &simpleStorage})) defer server.Close() - req, _ := http.NewRequest("GET", server.URL+"/api/version/simple/binary", nil) + req, _ := http.NewRequest("GET", server.URL+"/api/version/namespaces/default/simple/binary", nil) req.Header.Add("Accept", "text/other, */*") resp, err := http.DefaultClient.Do(req) if err != nil { @@ -1055,7 +1092,7 @@ func TestGetWithOptions(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/id?param1=test1¶m2=test2") + resp, err := http.Get(server.URL + "/api/version/namespaces/default/simple/id?param1=test1¶m2=test2") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1097,7 +1134,7 @@ func TestGetWithOptionsAndPath(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/id/a/different/path?param1=test1¶m2=test2&atAPath=not") + resp, err := http.Get(server.URL + "/api/version/namespaces/default/simple/id/a/different/path?param1=test1¶m2=test2&atAPath=not") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1132,7 +1169,7 @@ func TestGetAlternateSelfLink(t *testing.T) { } selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/api/version/simple/id?namespace=test", + expectedSet: "/api/version/namespaces/test/simple/id", name: "id", namespace: "test", } @@ -1141,7 +1178,7 @@ func TestGetAlternateSelfLink(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/id?namespace=test") + resp, err := http.Get(server.URL + "/api/version/namespaces/test/simple/id") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1234,7 +1271,7 @@ func TestConnect(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/" + itemID + "/connect") + resp, err := http.Get(server.URL + "/api/version/namespaces/default/simple/" + itemID + "/connect") if err != nil { t.Errorf("unexpected error: %v", err) @@ -1272,7 +1309,7 @@ func TestConnectWithOptions(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/" + itemID + "/connect?param1=value1¶m2=value2") + resp, err := http.Get(server.URL + "/api/version/namespaces/default/simple/" + itemID + "/connect?param1=value1¶m2=value2") if err != nil { t.Errorf("unexpected error: %v", err) @@ -1319,7 +1356,7 @@ func TestConnectWithOptionsAndPath(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - resp, err := http.Get(server.URL + "/api/version/simple/" + itemID + "/connect/" + testPath + "?param1=value1¶m2=value2") + resp, err := http.Get(server.URL + "/api/version/namespaces/default/simple/" + itemID + "/connect/" + testPath + "?param1=value1¶m2=value2") if err != nil { t.Errorf("unexpected error: %v", err) @@ -1360,7 +1397,7 @@ func TestDelete(t *testing.T) { defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/namespaces/default/simple/"+ID, nil) res, err := client.Do(request) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1392,7 +1429,7 @@ func TestDeleteWithOptions(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) res, err := client.Do(request) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1421,7 +1458,7 @@ func TestLegacyDelete(t *testing.T) { defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/namespaces/default/simple/"+ID, nil) res, err := client.Do(request) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1453,7 +1490,7 @@ func TestLegacyDeleteIgnoresOptions(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) res, err := client.Do(request) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1479,7 +1516,7 @@ func TestDeleteInvokesAdmissionControl(t *testing.T) { defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/namespaces/default/simple/"+ID, nil) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1501,7 +1538,7 @@ func TestDeleteMissing(t *testing.T) { defer server.Close() client := http.Client{} - request, err := http.NewRequest("DELETE", server.URL+"/api/version/simple/"+ID, nil) + request, err := http.NewRequest("DELETE", server.URL+"/api/version/namespaces/default/simple/"+ID, nil) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1526,7 +1563,7 @@ func TestPatch(t *testing.T) { storage["simple"] = &simpleStorage selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/api/version/simple/" + ID + "?namespace=default", + expectedSet: "/api/version/namespaces/default/simple/" + ID, name: ID, namespace: api.NamespaceDefault, } @@ -1535,7 +1572,7 @@ func TestPatch(t *testing.T) { defer server.Close() client := http.Client{} - request, err := http.NewRequest("PATCH", server.URL+"/api/version/simple/"+ID, bytes.NewReader([]byte(`{"labels":{"foo":"bar"}}`))) + request, err := http.NewRequest("PATCH", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader([]byte(`{"labels":{"foo":"bar"}}`))) request.Header.Set("Content-Type", "application/merge-patch+json") _, err = client.Do(request) if err != nil { @@ -1567,7 +1604,7 @@ func TestPatchRequiresMatchingName(t *testing.T) { defer server.Close() client := http.Client{} - request, err := http.NewRequest("PATCH", server.URL+"/api/version/simple/"+ID, bytes.NewReader([]byte(`{"metadata":{"name":"idbar"}}`))) + request, err := http.NewRequest("PATCH", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader([]byte(`{"metadata":{"name":"idbar"}}`))) request.Header.Set("Content-Type", "application/merge-patch+json") response, err := client.Do(request) if err != nil { @@ -1585,7 +1622,7 @@ func TestUpdate(t *testing.T) { storage["simple"] = &simpleStorage selfLinker := &setTestSelfLinker{ t: t, - expectedSet: "/api/version/simple/" + ID + "?namespace=default", + expectedSet: "/api/version/namespaces/default/simple/" + ID, name: ID, namespace: api.NamespaceDefault, } @@ -1607,7 +1644,7 @@ func TestUpdate(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) _, err = client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1644,7 +1681,7 @@ func TestUpdateInvokesAdmissionControl(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1673,7 +1710,7 @@ func TestUpdateRequiresMatchingName(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1705,7 +1742,7 @@ func TestUpdateAllowsMissingNamespace(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1743,7 +1780,7 @@ func TestUpdateAllowsMismatchedNamespaceOnError(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) _, err = client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1780,7 +1817,7 @@ func TestUpdatePreventsMismatchedNamespace(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1814,7 +1851,7 @@ func TestUpdateMissing(t *testing.T) { } client := http.Client{} - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/"+ID, bytes.NewReader(body)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/"+ID, bytes.NewReader(body)) response, err := client.Do(request) if err != nil { t.Errorf("unexpected error: %v", err) @@ -1838,7 +1875,7 @@ func TestCreateNotFound(t *testing.T) { simple := &Simple{Other: "foo"} data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/api/version/simple", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/simple", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1861,7 +1898,7 @@ func TestCreateChecksDecode(t *testing.T) { simple := &api.Pod{} data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/api/version/simple", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/simple", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1964,7 +2001,7 @@ func TestCreateWithName(t *testing.T) { simple := &Simple{Other: "foo"} data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/api/version/simple/"+pathName+"/sub", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/simple/"+pathName+"/sub", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1988,7 +2025,7 @@ func TestUpdateChecksDecode(t *testing.T) { simple := &api.Pod{} data, _ := codec.Encode(simple) - request, err := http.NewRequest("PUT", server.URL+"/api/version/simple/bar", bytes.NewBuffer(data)) + request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/bar", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -2047,7 +2084,7 @@ func TestCreate(t *testing.T) { t: t, name: "bar", namespace: "default", - expectedSet: "/api/version/foo/bar?namespace=default", + expectedSet: "/api/version/namespaces/default/foo/bar", } handler := handleLinker(map[string]rest.Storage{"foo": &storage}, selfLinker) server := httptest.NewServer(handler) @@ -2058,7 +2095,7 @@ func TestCreate(t *testing.T) { Other: "bar", } data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/api/version/foo", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/foo", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -2103,7 +2140,7 @@ func TestCreateInNamespace(t *testing.T) { t: t, name: "bar", namespace: "other", - expectedSet: "/api/version/foo/bar?namespace=other", + expectedSet: "/api/version/namespaces/other/foo/bar", } handler := handleLinker(map[string]rest.Storage{"foo": &storage}, selfLinker) server := httptest.NewServer(handler) @@ -2114,7 +2151,7 @@ func TestCreateInNamespace(t *testing.T) { Other: "bar", } data, _ := codec.Encode(simple) - request, err := http.NewRequest("POST", server.URL+"/api/version/foo?namespace=other", bytes.NewBuffer(data)) + request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/other/foo", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -2225,7 +2262,7 @@ func TestDelayReturnsError(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/api/version/foo/bar", server.URL), nil, http.StatusConflict) + status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/api/version/namespaces/default/foo/bar", server.URL), nil, http.StatusConflict) if status.Status != api.StatusFailure || status.Message == "" || status.Details == nil || status.Reason != api.StatusReasonAlreadyExists { t.Errorf("Unexpected status %#v", status) } diff --git a/pkg/apiserver/handlers.go b/pkg/apiserver/handlers.go index 3b1bc2b045aed..6670f155aa352 100644 --- a/pkg/apiserver/handlers.go +++ b/pkg/apiserver/handlers.go @@ -267,8 +267,6 @@ type APIRequestInfoResolver struct { // /namespaces/{namespace}/{resource}/{resourceName} // /{resource} // /{resource}/{resourceName} -// /{resource}/{resourceName}?namespace={namespace} -// /{resource}?namespace={namespace} // // Special verbs: // /proxy/{resource}/{resourceName} @@ -341,18 +339,7 @@ func (r *APIRequestInfoResolver) GetAPIRequestInfo(req *http.Request) (APIReques } } } 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.Namespace = req.URL.Query().Get("namespace") - if len(requestInfo.Namespace) == 0 { - if len(currentParts) > 1 || req.Method == "POST" { - requestInfo.Namespace = api.NamespaceDefault - } else { - requestInfo.Namespace = api.NamespaceAll - } - } + requestInfo.Namespace = api.NamespaceNone } // parsing successful, so we now know the proper value for .Parts diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index 994320d76a762..5128dd5e02dcb 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -163,16 +163,12 @@ func TestGetAPIRequestInfo(t *testing.T) { {"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/foo", "get", "", "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", "/namespaces/other/pods", "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"}}, @@ -180,9 +176,9 @@ func TestGetAPIRequestInfo(t *testing.T) { {"GET", getPath("pods", "other", ""), "list", testapi.Version(), "other", "pods", "", "Pod", "", []string{"pods"}}, {"GET", getPath("pods", "other", "foo"), "get", testapi.Version(), "other", "pods", "", "Pod", "foo", []string{"pods", "foo"}}, {"GET", getPath("pods", "", ""), "list", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, - {"POST", getPath("pods", "", ""), "create", testapi.Version(), api.NamespaceDefault, "pods", "", "Pod", "", []string{"pods"}}, - {"GET", getPath("pods", "", "foo"), "get", testapi.Version(), api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, - {"GET", pathWithPrefix("proxy", "pods", "", "foo"), "proxy", testapi.Version(), api.NamespaceDefault, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"POST", getPath("pods", "", ""), "create", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, + {"GET", getPath("pods", "", "foo"), "get", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, + {"GET", pathWithPrefix("proxy", "pods", "", "foo"), "proxy", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "foo", []string{"pods", "foo"}}, {"GET", pathWithPrefix("watch", "pods", "", ""), "watch", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, {"GET", pathWithPrefix("redirect", "pods", "", ""), "redirect", testapi.Version(), api.NamespaceAll, "pods", "", "Pod", "", []string{"pods"}}, {"GET", pathWithPrefix("watch", "pods", "other", ""), "watch", testapi.Version(), "other", "pods", "", "Pod", "", []string{"pods"}}, diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go index 45218d6cf1e76..bc66097089edd 100644 --- a/pkg/apiserver/proxy_test.go +++ b/pkg/apiserver/proxy_test.go @@ -86,9 +86,6 @@ func TestProxy(t *testing.T) { namespaceHandler := handleNamespaced(map[string]rest.Storage{"foo": simpleStorage}) namespaceServer := httptest.NewServer(namespaceHandler) defer namespaceServer.Close() - legacyNamespaceHandler := handle(map[string]rest.Storage{"foo": simpleStorage}) - legacyNamespaceServer := httptest.NewServer(legacyNamespaceHandler) - defer legacyNamespaceServer.Close() // test each supported URL pattern for finding the redirection resource in the proxy in a particular namespace serverPatterns := []struct { @@ -96,7 +93,6 @@ func TestProxy(t *testing.T) { proxyTestPattern string }{ {namespaceServer, "/api/version2/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path}, - {legacyNamespaceServer, "/api/version/proxy/foo/id" + item.path + "?namespace=" + item.reqNamespace}, } for _, serverPattern := range serverPatterns { diff --git a/pkg/apiserver/watch_test.go b/pkg/apiserver/watch_test.go index 6143ed92c1c7c..8a4d7f1ce3c7e 100644 --- a/pkg/apiserver/watch_test.go +++ b/pkg/apiserver/watch_test.go @@ -166,14 +166,20 @@ func TestWatchHTTP(t *testing.T) { func TestWatchParamParsing(t *testing.T) { simpleStorage := &SimpleRESTStorage{} - handler := handle(map[string]rest.Storage{"simples": simpleStorage}) + handler := handle(map[string]rest.Storage{ + "simples": simpleStorage, + "simpleroots": simpleStorage, + }) server := httptest.NewServer(handler) defer server.Close() dest, _ := url.Parse(server.URL) - dest.Path = "/api/" + testVersion + "/watch/simples" + + rootPath := "/api/" + testVersion + "/watch/simples" + namespacedPath := "/api/" + testVersion + "/watch/namespaces/other/simpleroots" table := []struct { + path string rawQuery string resourceVersion string labelSelector string @@ -181,30 +187,63 @@ func TestWatchParamParsing(t *testing.T) { namespace string }{ { + path: rootPath, rawQuery: "resourceVersion=1234", resourceVersion: "1234", labelSelector: "", fieldSelector: "", namespace: api.NamespaceAll, }, { - rawQuery: "namespace=default&resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo", + path: rootPath, + rawQuery: "resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo", resourceVersion: "314159", labelSelector: "name=foo", fieldSelector: "Host=", - namespace: api.NamespaceDefault, + namespace: api.NamespaceAll, }, { - rawQuery: "namespace=watchother&fields=id%3dfoo&resourceVersion=1492", + path: rootPath, + rawQuery: "fields=id%3dfoo&resourceVersion=1492", resourceVersion: "1492", labelSelector: "", fieldSelector: "id=foo", - namespace: "watchother", + namespace: api.NamespaceAll, }, { + path: rootPath, rawQuery: "", resourceVersion: "", labelSelector: "", fieldSelector: "", namespace: api.NamespaceAll, }, + { + path: namespacedPath, + rawQuery: "resourceVersion=1234", + resourceVersion: "1234", + labelSelector: "", + fieldSelector: "", + namespace: "other", + }, { + path: namespacedPath, + rawQuery: "resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo", + resourceVersion: "314159", + labelSelector: "name=foo", + fieldSelector: "Host=", + namespace: "other", + }, { + path: namespacedPath, + rawQuery: "fields=id%3dfoo&resourceVersion=1492", + resourceVersion: "1492", + labelSelector: "", + fieldSelector: "id=foo", + namespace: "other", + }, { + path: namespacedPath, + rawQuery: "", + resourceVersion: "", + labelSelector: "", + fieldSelector: "", + namespace: "other", + }, } for _, item := range table { @@ -212,6 +251,7 @@ func TestWatchParamParsing(t *testing.T) { simpleStorage.requestedFieldSelector = fields.Everything() simpleStorage.requestedResourceVersion = "5" // Prove this is set in all cases simpleStorage.requestedResourceNamespace = "" + dest.Path = item.path dest.RawQuery = item.rawQuery resp, err := http.Get(dest.String()) if err != nil {