From 07b5930a7038fda621d201190a0e5257b0b0677c Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 15 Jun 2015 15:39:06 -0700 Subject: [PATCH] Remove the redirect verb. --- api/swagger-spec/v1.json | 100 ------------------------- api/swagger-spec/v1beta3.json | 100 ------------------------- pkg/apiserver/api_installer.go | 22 +----- pkg/apiserver/redirect.go | 104 -------------------------- pkg/apiserver/redirect_test.go | 129 --------------------------------- 5 files changed, 2 insertions(+), 453 deletions(-) delete mode 100644 pkg/apiserver/redirect.go delete mode 100644 pkg/apiserver/redirect_test.go diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index d56c46dfd8c..a5be4f0e6d2 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -3212,34 +3212,6 @@ } ] }, - { - "path": "/api/v1/redirect/nodes/{name}", - "description": "API at /api/v1 version v1", - "operations": [ - { - "type": "string", - "method": "GET", - "summary": "redirect GET request to Node", - "nickname": "redirectNode", - "parameters": [ - { - "type": "string", - "paramType": "path", - "name": "name", - "description": "name of the Node", - "required": true, - "allowMultiple": false - } - ], - "produces": [ - "*/*" - ], - "consumes": [ - "*/*" - ] - } - ] - }, { "path": "/api/v1/proxy/nodes/{name}/{path:*}", "description": "API at /api/v1 version v1", @@ -5302,42 +5274,6 @@ } ] }, - { - "path": "/api/v1/redirect/namespaces/{namespaces}/pods/{name}", - "description": "API at /api/v1 version v1", - "operations": [ - { - "type": "string", - "method": "GET", - "summary": "redirect GET request to Pod", - "nickname": "redirectPod", - "parameters": [ - { - "type": "string", - "paramType": "path", - "name": "namespaces", - "description": "object name and auth scope, such as for teams and projects", - "required": true, - "allowMultiple": false - }, - { - "type": "string", - "paramType": "path", - "name": "name", - "description": "name of the Pod", - "required": true, - "allowMultiple": false - } - ], - "produces": [ - "*/*" - ], - "consumes": [ - "*/*" - ] - } - ] - }, { "path": "/api/v1/proxy/namespaces/{namespaces}/pods/{name}/{path:*}", "description": "API at /api/v1 version v1", @@ -10187,42 +10123,6 @@ } ] }, - { - "path": "/api/v1/redirect/namespaces/{namespaces}/services/{name}", - "description": "API at /api/v1 version v1", - "operations": [ - { - "type": "string", - "method": "GET", - "summary": "redirect GET request to Service", - "nickname": "redirectService", - "parameters": [ - { - "type": "string", - "paramType": "path", - "name": "namespaces", - "description": "object name and auth scope, such as for teams and projects", - "required": true, - "allowMultiple": false - }, - { - "type": "string", - "paramType": "path", - "name": "name", - "description": "name of the Service", - "required": true, - "allowMultiple": false - } - ], - "produces": [ - "*/*" - ], - "consumes": [ - "*/*" - ] - } - ] - }, { "path": "/api/v1/proxy/namespaces/{namespaces}/services/{name}/{path:*}", "description": "API at /api/v1 version v1", diff --git a/api/swagger-spec/v1beta3.json b/api/swagger-spec/v1beta3.json index bf0824e5437..5b85d1f2b77 100644 --- a/api/swagger-spec/v1beta3.json +++ b/api/swagger-spec/v1beta3.json @@ -3212,34 +3212,6 @@ } ] }, - { - "path": "/api/v1beta3/redirect/nodes/{name}", - "description": "API at /api/v1beta3 version v1beta3", - "operations": [ - { - "type": "string", - "method": "GET", - "summary": "redirect GET request to Node", - "nickname": "redirectNode", - "parameters": [ - { - "type": "string", - "paramType": "path", - "name": "name", - "description": "name of the Node", - "required": true, - "allowMultiple": false - } - ], - "produces": [ - "*/*" - ], - "consumes": [ - "*/*" - ] - } - ] - }, { "path": "/api/v1beta3/proxy/nodes/{name}/{path:*}", "description": "API at /api/v1beta3 version v1beta3", @@ -5302,42 +5274,6 @@ } ] }, - { - "path": "/api/v1beta3/redirect/namespaces/{namespaces}/pods/{name}", - "description": "API at /api/v1beta3 version v1beta3", - "operations": [ - { - "type": "string", - "method": "GET", - "summary": "redirect GET request to Pod", - "nickname": "redirectPod", - "parameters": [ - { - "type": "string", - "paramType": "path", - "name": "namespaces", - "description": "object name and auth scope, such as for teams and projects", - "required": true, - "allowMultiple": false - }, - { - "type": "string", - "paramType": "path", - "name": "name", - "description": "name of the Pod", - "required": true, - "allowMultiple": false - } - ], - "produces": [ - "*/*" - ], - "consumes": [ - "*/*" - ] - } - ] - }, { "path": "/api/v1beta3/proxy/namespaces/{namespaces}/pods/{name}/{path:*}", "description": "API at /api/v1beta3 version v1beta3", @@ -10187,42 +10123,6 @@ } ] }, - { - "path": "/api/v1beta3/redirect/namespaces/{namespaces}/services/{name}", - "description": "API at /api/v1beta3 version v1beta3", - "operations": [ - { - "type": "string", - "method": "GET", - "summary": "redirect GET request to Service", - "nickname": "redirectService", - "parameters": [ - { - "type": "string", - "paramType": "path", - "name": "namespaces", - "description": "object name and auth scope, such as for teams and projects", - "required": true, - "allowMultiple": false - }, - { - "type": "string", - "paramType": "path", - "name": "name", - "description": "name of the Service", - "required": true, - "allowMultiple": false - } - ], - "produces": [ - "*/*" - ], - "consumes": [ - "*/*" - ] - } - ] - }, { "path": "/api/v1beta3/proxy/namespaces/{namespaces}/services/{name}/{path:*}", "description": "API at /api/v1beta3 version v1beta3", diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 0d94e0900c4..be75150c58d 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -62,7 +62,6 @@ func (a *APIInstaller) Install(proxyDialer func(network, addr string) (net.Conn, // Create the WebService. ws = a.newWebService() - redirectHandler := (&RedirectHandler{a.group.Storage, a.group.Codec, a.group.Context, a.info}) proxyHandler := (&ProxyHandler{a.prefix + "/proxy/", a.group.Storage, a.group.Codec, a.group.Context, a.info, proxyDialer}) // Register the paths in a deterministic (sorted) order to get a deterministic swagger spec. @@ -74,7 +73,7 @@ func (a *APIInstaller) Install(proxyDialer func(network, addr string) (net.Conn, } sort.Strings(paths) for _, path := range paths { - if err := a.registerResourceHandlers(path, a.group.Storage[path], ws, redirectHandler, proxyHandler); err != nil { + if err := a.registerResourceHandlers(path, a.group.Storage[path], ws, proxyHandler); err != nil { errors = append(errors, err) } } @@ -93,7 +92,7 @@ func (a *APIInstaller) newWebService() *restful.WebService { return ws } -func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storage, ws *restful.WebService, redirectHandler, proxyHandler http.Handler) error { +func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storage, ws *restful.WebService, proxyHandler http.Handler) error { admit := a.group.Admit context := a.group.Context @@ -277,7 +276,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag 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{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) 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) @@ -316,7 +314,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag 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{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) 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) @@ -360,7 +357,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag 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{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) 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) @@ -568,20 +564,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } addParams(route, action.Params) ws.Route(route) - case "REDIRECT": // Get the redirect URL for a resource. - doc := "redirect GET request to " + kind - if hasSubresource { - doc = "redirect GET request to " + subresource + " of " + kind - } - route := ws.GET(action.Path).To(routeFunction(redirectHandler)). - Filter(m). - Doc(doc). - Operation("redirect" + kind + strings.Title(subresource)). - Produces("*/*"). - Consumes("*/*"). - Writes("string") - addParams(route, action.Params) - ws.Route(route) case "PROXY": // Proxy requests to a resource. // Accept all methods as per https://github.com/GoogleCloudPlatform/kubernetes/issues/3996 addProxyRoute(ws, "GET", a.prefix, action.Path, proxyHandler, kind, resource, subresource, hasSubresource, action.Params) diff --git a/pkg/apiserver/redirect.go b/pkg/apiserver/redirect.go deleted file mode 100644 index 99ba1dcb819..00000000000 --- a/pkg/apiserver/redirect.go +++ /dev/null @@ -1,104 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package apiserver - -import ( - "net/http" - "time" - - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" - "github.com/GoogleCloudPlatform/kubernetes/pkg/httplog" - "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" -) - -type RedirectHandler struct { - storage map[string]rest.Storage - codec runtime.Codec - context api.RequestContextMapper - apiRequestInfoResolver *APIRequestInfoResolver -} - -func (r *RedirectHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - var verb string - var apiResource string - var httpCode int - reqStart := time.Now() - defer monitor(&verb, &apiResource, util.GetClient(req), &httpCode, reqStart) - - requestInfo, err := r.apiRequestInfoResolver.GetAPIRequestInfo(req) - if err != nil { - notFound(w, req) - httpCode = http.StatusNotFound - return - } - verb = requestInfo.Verb - resource, parts := requestInfo.Resource, requestInfo.Parts - ctx, ok := r.context.Get(req) - if !ok { - ctx = api.NewContext() - } - ctx = api.WithNamespace(ctx, requestInfo.Namespace) - - // redirection requires /resource/resourceName path parts - if len(parts) != 2 || req.Method != "GET" { - notFound(w, req) - httpCode = http.StatusNotFound - return - } - id := parts[1] - storage, ok := r.storage[resource] - if !ok { - httplog.LogOf(req, w).Addf("'%v' has no storage object", resource) - notFound(w, req) - httpCode = http.StatusNotFound - return - } - apiResource = resource - - redirector, ok := storage.(rest.Redirector) - if !ok { - httplog.LogOf(req, w).Addf("'%v' is not a redirector", resource) - httpCode = errorJSON(errors.NewMethodNotSupported(resource, "redirect"), r.codec, w) - return - } - - location, _, err := redirector.ResourceLocation(ctx, id) - if err != nil { - status := errToAPIStatus(err) - writeJSON(status.Code, r.codec, status, w, true) - httpCode = status.Code - return - } - if location == nil { - httplog.LogOf(req, w).Addf("ResourceLocation for %v returned nil", id) - notFound(w, req) - httpCode = http.StatusNotFound - return - } - - // Default to http - if location.Scheme == "" { - location.Scheme = "http" - } - - w.Header().Set("Location", location.String()) - w.WriteHeader(http.StatusTemporaryRedirect) - httpCode = http.StatusTemporaryRedirect -} diff --git a/pkg/apiserver/redirect_test.go b/pkg/apiserver/redirect_test.go deleted file mode 100644 index d8e407b4bdc..00000000000 --- a/pkg/apiserver/redirect_test.go +++ /dev/null @@ -1,129 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package apiserver - -import ( - "errors" - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" -) - -func TestRedirect(t *testing.T) { - simpleStorage := &SimpleRESTStorage{ - errors: map[string]error{}, - expectedResourceNamespace: "default", - } - handler := handle(map[string]rest.Storage{"foo": simpleStorage}) - 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 = &url.URL{Host: item.id} - resp, err := client.Get(server.URL + "/api/version/redirect/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 := "http://"+item.id, resp.Header.Get("Location"); e != a { - t.Errorf("Expected %v, got %v", e, a) - } - } -} - -func TestRedirectWithNamespaces(t *testing.T) { - simpleStorage := &SimpleRESTStorage{ - errors: map[string]error{}, - expectedResourceNamespace: "other", - } - handler := handleNamespaced(map[string]rest.Storage{"foo": simpleStorage}) - 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 = &url.URL{Host: item.id} - resp, err := client.Get(server.URL + "/api/version2/redirect/namespaces/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 := "http://"+item.id, resp.Header.Get("Location"); e != a { - t.Errorf("Expected %v, got %v", e, a) - } - } -}