From 2c1a689952ec34e3f9ecb7bcd1772c3fa35c9597 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 26 Apr 2018 16:21:38 -0400 Subject: [PATCH] Collapse onto request scope convertor --- .../pkg/apiserver/customresource_handler.go | 6 ++--- .../apiserver/pkg/endpoints/apiserver_test.go | 26 +++++++++++-------- .../apiserver/pkg/endpoints/handlers/patch.go | 11 ++------ .../apiserver/pkg/endpoints/installer.go | 6 ++--- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 100304bd824..4c0c01ba20d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -236,7 +236,7 @@ func (r *crdHandler) serveResource(w http.ResponseWriter, req *http.Request, req case "update": return handlers.UpdateResource(storage, requestScope, r.admission) case "patch": - return handlers.PatchResource(storage, requestScope, r.admission, unstructured.UnstructuredObjectConverter{}, supportedTypes) + return handlers.PatchResource(storage, requestScope, r.admission, supportedTypes) case "delete": allowsOptions := true return handlers.DeleteResource(storage, allowsOptions, requestScope, r.admission) @@ -259,7 +259,7 @@ func (r *crdHandler) serveStatus(w http.ResponseWriter, req *http.Request, reque case "update": return handlers.UpdateResource(storage, requestScope, r.admission) case "patch": - return handlers.PatchResource(storage, requestScope, r.admission, unstructured.UnstructuredObjectConverter{}, supportedTypes) + return handlers.PatchResource(storage, requestScope, r.admission, supportedTypes) default: http.Error(w, fmt.Sprintf("unhandled verb %q", requestInfo.Verb), http.StatusMethodNotAllowed) return nil @@ -276,7 +276,7 @@ func (r *crdHandler) serveScale(w http.ResponseWriter, req *http.Request, reques case "update": return handlers.UpdateResource(storage, requestScope, r.admission) case "patch": - return handlers.PatchResource(storage, requestScope, r.admission, unstructured.UnstructuredObjectConverter{}, supportedTypes) + return handlers.PatchResource(storage, requestScope, r.admission, supportedTypes) default: http.Error(w, fmt.Sprintf("unhandled verb %q", requestInfo.Verb), http.StatusMethodNotAllowed) return nil diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 78ef764479f..28aaa603d84 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -245,6 +245,7 @@ func handleInternal(storage map[string]rest.Storage, admissionControl admission. Creater: scheme, Convertor: scheme, + UnsafeConvertor: runtime.UnsafeObjectConvertor(scheme), Defaulter: scheme, Typer: scheme, Linker: selfLinker, @@ -3170,6 +3171,7 @@ func TestParentResourceIsRequired(t *testing.T) { Root: "/" + prefix, Creater: scheme, Convertor: scheme, + UnsafeConvertor: runtime.UnsafeObjectConvertor(scheme), Defaulter: scheme, Typer: scheme, Linker: selfLinker, @@ -3197,12 +3199,13 @@ func TestParentResourceIsRequired(t *testing.T) { "simple": &SimpleRESTStorage{}, "simple/sub": storage, }, - Root: "/" + prefix, - Creater: scheme, - Convertor: scheme, - Defaulter: scheme, - Typer: scheme, - Linker: selfLinker, + Root: "/" + prefix, + Creater: scheme, + Convertor: scheme, + UnsafeConvertor: runtime.UnsafeObjectConvertor(scheme), + Defaulter: scheme, + Typer: scheme, + Linker: selfLinker, Admit: admissionControl, @@ -3822,11 +3825,12 @@ func TestXGSubresource(t *testing.T) { group := APIGroupVersion{ Storage: storage, - Creater: scheme, - Convertor: scheme, - Defaulter: scheme, - Typer: scheme, - Linker: selfLinker, + Creater: scheme, + Convertor: scheme, + UnsafeConvertor: runtime.UnsafeObjectConvertor(scheme), + Defaulter: scheme, + Typer: scheme, + Linker: selfLinker, ParameterCodec: parameterCodec, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 0b6496c4480..0ed6dee7cb4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -42,7 +42,7 @@ import ( ) // PatchResource returns a function that will handle a resource patch. -func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface, converter runtime.ObjectConvertor, patchTypes []string) http.HandlerFunc { +func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface, patchTypes []string) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { // For performance tracking purposes. trace := utiltrace.New("Patch " + req.URL.Path) @@ -77,14 +77,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface ctx := req.Context() ctx = request.WithNamespace(ctx, namespace) - // TODO: this is NOT using the scope's convertor [sic]. Figure - // out if this is intentional or not. Perhaps it matters on - // subresources? Rename this parameter if this is purposful and - // delete it (using scope.Convertor instead) otherwise. - // - // Already some tests set this converter but apparently not the - // scope's unsafeConvertor. - schemaReferenceObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion()) + schemaReferenceObj, err := scope.UnsafeConvertor.ConvertToVersion(r.New(), scope.Kind.GroupVersion()) if err != nil { scope.err(err, w, req) return diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index e9a292d8ef7..006b218ea33 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -629,7 +629,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag string(types.MergePatchType), string(types.StrategicMergePatchType), } - handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, requestScope, restfulPatchResource(patcher, reqScope, admit, a.group.Convertor, supportedTypes)) + handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, requestScope, restfulPatchResource(patcher, reqScope, admit, supportedTypes)) route := ws.PATCH(action.Path).To(handler). Doc(doc). Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")). @@ -1005,9 +1005,9 @@ func restfulUpdateResource(r rest.Updater, scope handlers.RequestScope, admit ad } } -func restfulPatchResource(r rest.Patcher, scope handlers.RequestScope, admit admission.Interface, converter runtime.ObjectConvertor, supportedTypes []string) restful.RouteFunction { +func restfulPatchResource(r rest.Patcher, scope handlers.RequestScope, admit admission.Interface, supportedTypes []string) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { - handlers.PatchResource(r, scope, admit, converter, supportedTypes)(res.ResponseWriter, req.Request) + handlers.PatchResource(r, scope, admit, supportedTypes)(res.ResponseWriter, req.Request) } }