From a7dd09ec47d6031a39d2090fc4cb10e13e44b995 Mon Sep 17 00:00:00 2001 From: deads2k Date: Mon, 30 Nov 2015 11:29:19 -0500 Subject: [PATCH] update requestScope to fully qualify kind and resource --- pkg/api/unversioned/group_version.go | 27 ++++++++-- pkg/apiserver/api_installer.go | 58 +++++++++++++-------- pkg/apiserver/apiserver.go | 4 +- pkg/apiserver/resthandler.go | 77 +++++++++++----------------- 4 files changed, 91 insertions(+), 75 deletions(-) diff --git a/pkg/api/unversioned/group_version.go b/pkg/api/unversioned/group_version.go index b4884dbd116..ccb61d0be8b 100644 --- a/pkg/api/unversioned/group_version.go +++ b/pkg/api/unversioned/group_version.go @@ -22,6 +22,22 @@ import ( "strings" ) +// GroupVersionResource unambiguously identifies a resource. It doesn't anonymously include GroupVersion +// to avoid automatic coersion. It doesn't use a GroupVersion to avoid custom marshalling +type GroupVersionResource struct { + Group string + Version string + Resource string +} + +func (gvr GroupVersionResource) GroupVersion() GroupVersion { + return GroupVersion{Group: gvr.Group, Version: gvr.Version} +} + +func (gvr *GroupVersionResource) String() string { + return gvr.Group + "/" + gvr.Version + ", Resource=" + gvr.Resource +} + // GroupKind specifies a Group and a Kind, but does not force a version. This is useful for identifying // concepts during lookup stages without having partially valid types type GroupKind struct { @@ -45,9 +61,9 @@ type GroupVersionKind struct { Kind string } -// TODO remove this -func NewGroupVersionKind(gv GroupVersion, kind string) GroupVersionKind { - return GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: kind} +// IsEmpty returns true if group, version, and kind are empty +func (gvk GroupVersionKind) IsEmpty() bool { + return len(gvk.Group) == 0 && len(gvk.Version) == 0 && len(gvk.Kind) == 0 } func (gvk GroupVersionKind) GroupKind() GroupKind { @@ -125,6 +141,11 @@ func (gv GroupVersion) WithKind(kind string) GroupVersionKind { return GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: kind} } +// WithResource creates a GroupVersionResource based on the method receiver's GroupVersion and the passed Resource. +func (gv GroupVersion) WithResource(resource string) GroupVersionResource { + return GroupVersionResource{Group: gv.Group, Version: gv.Version, Resource: resource} +} + // MarshalJSON implements the json.Marshaller interface. func (gv GroupVersion) MarshalJSON() ([]byte, error) { s := gv.String() diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index d9543328594..ff8d88d141d 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -216,18 +216,27 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } versionedStatus := indirectArbitraryPointer(versionedStatusPtr) var ( - getOptions runtime.Object - versionedGetOptions runtime.Object - getOptionsKind string - getSubpath bool - getSubpathKey string + getOptions runtime.Object + versionedGetOptions runtime.Object + getOptionsInternalKind unversioned.GroupVersionKind + getOptionsExternalKind unversioned.GroupVersionKind + getSubpath bool + getSubpathKey string ) if isGetterWithOptions { getOptions, getSubpath, getSubpathKey = getterWithOptions.NewGetOptions() - _, getOptionsKind, err = a.group.Typer.ObjectVersionAndKind(getOptions) + getOptionsGVString, getOptionsKind, err := a.group.Typer.ObjectVersionAndKind(getOptions) if err != nil { return nil, err } + gv, err := unversioned.ParseGroupVersion(getOptionsGVString) + if err != nil { + return nil, err + } + getOptionsInternalKind = gv.WithKind(getOptionsKind) + // TODO this should be a list of all the different external versions we can coerce into the internalKind + getOptionsExternalKind = serverGroupVersion.WithKind(getOptionsKind) + versionedGetOptions, err = a.group.Creater.New(serverGroupVersion.String(), getOptionsKind) if err != nil { return nil, err @@ -236,19 +245,28 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag } var ( - connectOptions runtime.Object - versionedConnectOptions runtime.Object - connectOptionsKind string - connectSubpath bool - connectSubpathKey string + connectOptions runtime.Object + versionedConnectOptions runtime.Object + connectOptionsInternalKind unversioned.GroupVersionKind + connectOptionsExternalKind unversioned.GroupVersionKind + connectSubpath bool + connectSubpathKey string ) if isConnecter { connectOptions, connectSubpath, connectSubpathKey = connecter.NewConnectOptions() if connectOptions != nil { - _, connectOptionsKind, err = a.group.Typer.ObjectVersionAndKind(connectOptions) + connectOptionsGVString, connectOptionsKind, err := a.group.Typer.ObjectVersionAndKind(connectOptions) if err != nil { return nil, err } + gv, err := unversioned.ParseGroupVersion(connectOptionsGVString) + if err != nil { + return nil, err + } + connectOptionsInternalKind = gv.WithKind(connectOptionsKind) + // TODO this should be a list of all the different external versions we can coerce into the internalKind + connectOptionsExternalKind = serverGroupVersion.WithKind(connectOptionsKind) + versionedConnectOptions, err = a.group.Creater.New(serverGroupVersion.String(), connectOptionsKind) } } @@ -383,14 +401,10 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag Creater: a.group.Creater, Convertor: a.group.Convertor, Codec: mapping.Codec, - APIVersion: a.group.GroupVersion.String(), - // TODO, this internal version needs to plumbed through from the caller - // this works in all the cases we have now - InternalVersion: unversioned.GroupVersion{Group: a.group.GroupVersion.Group}, - ServerAPIVersion: serverGroupVersion.String(), - Resource: resource, - Subresource: subresource, - Kind: kind, + + Resource: a.group.GroupVersion.WithResource(resource), + Subresource: subresource, + Kind: a.group.GroupVersion.WithKind(kind), } for _, action := range actions { reqScope.Namer = action.Namer @@ -403,7 +417,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag case "GET": // Get a resource. var handler restful.RouteFunction if isGetterWithOptions { - handler = GetResourceWithOptions(getterWithOptions, reqScope, getOptionsKind, getSubpath, getSubpathKey) + handler = GetResourceWithOptions(getterWithOptions, reqScope, getOptionsInternalKind, getOptionsExternalKind, getSubpath, getSubpathKey) } else { handler = GetResource(getter, reqScope) } @@ -584,7 +598,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag doc = "connect " + method + " requests to " + subresource + " of " + kind } route := ws.Method(method).Path(action.Path). - To(ConnectResource(connecter, reqScope, admit, connectOptionsKind, path, connectSubpath, connectSubpathKey)). + To(ConnectResource(connecter, reqScope, admit, connectOptionsInternalKind, connectOptionsExternalKind, path, connectSubpath, connectSubpathKey)). Filter(m). Doc(doc). Operation("connect" + strings.Title(strings.ToLower(method)) + namespaced + kind + strings.Title(subresource)). diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 6e645503d91..9cf6de86f48 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -336,9 +336,9 @@ func SupportedResourcesHandler(groupVersion unversioned.GroupVersion, apiResourc // response. The Accept header and current API version will be passed in, and the output will be copied // directly to the response body. If content type is returned it is used, otherwise the content type will // be "application/octet-stream". All other objects are sent to standard JSON serialization. -func write(statusCode int, apiVersion string, codec runtime.Codec, object runtime.Object, w http.ResponseWriter, req *http.Request) { +func write(statusCode int, groupVersion unversioned.GroupVersion, codec runtime.Codec, object runtime.Object, w http.ResponseWriter, req *http.Request) { if stream, ok := object.(rest.ResourceStreamer); ok { - out, flush, contentType, err := stream.InputStream(apiVersion, req.Header.Get("Accept")) + out, flush, contentType, err := stream.InputStream(groupVersion.String(), req.Header.Get("Accept")) if err != nil { errorJSONFatal(err, codec, w) return diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 0fadf604428..16d0dc77498 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -73,14 +73,9 @@ type RequestScope struct { Creater runtime.ObjectCreater Convertor runtime.ObjectConvertor - Resource string - Subresource string - Kind string - APIVersion string - InternalVersion unversioned.GroupVersion - - // The version of apiserver resources to use - ServerAPIVersion string + Resource unversioned.GroupVersionResource + Kind unversioned.GroupVersionKind + Subresource string } // getterFunc performs a get request with the given context and object name. The request @@ -112,7 +107,7 @@ func getResourceHandler(scope RequestScope, getter getterFunc) restful.RouteFunc errorJSON(err, scope.Codec, w) return } - write(http.StatusOK, scope.APIVersion, scope.Codec, result, w, req.Request) + write(http.StatusOK, scope.Kind.GroupVersion(), scope.Codec, result, w, req.Request) } } @@ -125,10 +120,10 @@ func GetResource(r rest.Getter, scope RequestScope) restful.RouteFunction { } // GetResourceWithOptions returns a function that handles retrieving a single resource from a rest.Storage object. -func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, getOptionsKind string, subpath bool, subpathKey string) restful.RouteFunction { +func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, internalKind, externalKind unversioned.GroupVersionKind, subpath bool, subpathKey string) restful.RouteFunction { return getResourceHandler(scope, func(ctx api.Context, name string, req *restful.Request) (runtime.Object, error) { - opts, err := getRequestOptions(req, scope, getOptionsKind, subpath, subpathKey) + opts, err := getRequestOptions(req, scope, internalKind, externalKind, subpath, subpathKey) if err != nil { return nil, err } @@ -136,10 +131,11 @@ func GetResourceWithOptions(r rest.GetterWithOptions, scope RequestScope, getOpt }) } -func getRequestOptions(req *restful.Request, scope RequestScope, kind string, subpath bool, subpathKey string) (runtime.Object, error) { - if len(kind) == 0 { +func getRequestOptions(req *restful.Request, scope RequestScope, internalKind, externalKind unversioned.GroupVersionKind, subpath bool, subpathKey string) (runtime.Object, error) { + if internalKind.IsEmpty() { return nil, nil } + query := req.Request.URL.Query() if subpath { newQuery := make(url.Values) @@ -150,30 +146,15 @@ func getRequestOptions(req *restful.Request, scope RequestScope, kind string, su query = newQuery } - // TODO Options a mess. Basically the intent is: - // 1. try to decode using the expected external GroupVersion - // 2. if that fails, fall back to the old external serialization being used before, which was - // "v1" and decode into the unversioned/legacykube group - gvString := scope.APIVersion - internalGVString := scope.InternalVersion.String() - - versioned, err := scope.Creater.New(gvString, kind) + versioned, err := scope.Creater.New(externalKind.GroupVersion().String(), externalKind.Kind) if err != nil { - gvString = "v1" - internalGVString = "" - - var secondErr error - versioned, secondErr = scope.Creater.New(gvString, kind) - // if we have an error, return the original failure - if secondErr != nil { - return nil, err - } + return nil, err } if err := scope.Codec.DecodeParametersInto(query, versioned); err != nil { return nil, errors.NewBadRequest(err.Error()) } - out, err := scope.Convertor.ConvertToVersion(versioned, internalGVString) + out, err := scope.Convertor.ConvertToVersion(versioned, internalKind.GroupVersion().String()) if err != nil { // programmer error return nil, err @@ -182,7 +163,7 @@ func getRequestOptions(req *restful.Request, scope RequestScope, kind string, su } // ConnectResource returns a function that handles a connect request on a rest.Storage object. -func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admission.Interface, connectOptionsKind, restPath string, subpath bool, subpathKey string) restful.RouteFunction { +func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admission.Interface, internalKind, externalKind unversioned.GroupVersionKind, restPath string, subpath bool, subpathKey string) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter namespace, name, err := scope.Namer.Name(req) @@ -192,7 +173,7 @@ func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admissi } ctx := scope.ContextFunc(req) ctx = api.WithNamespace(ctx, namespace) - opts, err := getRequestOptions(req, scope, connectOptionsKind, subpath, subpathKey) + opts, err := getRequestOptions(req, scope, internalKind, externalKind, subpath, subpathKey) if err != nil { errorJSON(err, scope.Codec, w) return @@ -205,7 +186,7 @@ func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admissi } userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(connectRequest, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, userInfo)) + err = admit.Admit(admission.NewAttributesRecord(connectRequest, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Connect, userInfo)) if err != nil { errorJSON(err, scope.Codec, w) return @@ -228,7 +209,7 @@ type responder struct { } func (r *responder) Object(statusCode int, obj runtime.Object) { - write(statusCode, r.scope.APIVersion, r.scope.Codec, obj, r.w, r.req) + write(statusCode, r.scope.Kind.GroupVersion(), r.scope.Codec, obj, r.w, r.req) } func (r *responder) Error(err error) { @@ -267,7 +248,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch // TODO: DecodeParametersInto should do this. if opts.FieldSelector.Selector != nil { fn := func(label, value string) (newLabel, newValue string, err error) { - return scope.Convertor.ConvertFieldLabel(scope.APIVersion, scope.Kind, label, value) + return scope.Convertor.ConvertFieldLabel(scope.Kind.GroupVersion().String(), scope.Kind.Kind, label, value) } if opts.FieldSelector.Selector, err = opts.FieldSelector.Selector.Transform(fn); err != nil { // TODO: allow bad request to set field causes based on query parameters @@ -325,7 +306,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch errorJSON(err, scope.Codec, w) return } - write(http.StatusOK, scope.APIVersion, scope.Codec, result, w, req.Request) + write(http.StatusOK, scope.Kind.GroupVersion(), scope.Codec, result, w, req.Request) } } @@ -361,7 +342,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object obj := r.New() // TODO this cleans up with proper typing - if err := scope.Codec.DecodeIntoWithSpecifiedVersionKind(body, obj, unversioned.ParseGroupVersionOrDie(scope.APIVersion).WithKind(scope.Kind)); err != nil { + if err := scope.Codec.DecodeIntoWithSpecifiedVersionKind(body, obj, scope.Kind); err != nil { err = transformDecodeError(typer, err, obj, body) errorJSON(err, scope.Codec, w) return @@ -370,7 +351,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object if admit != nil && admit.Handles(admission.Create) { userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, userInfo)) + err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Create, userInfo)) if err != nil { errorJSON(err, scope.Codec, w) return @@ -394,7 +375,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object return } - write(http.StatusCreated, scope.APIVersion, scope.Codec, result, w, req.Request) + write(http.StatusCreated, scope.Kind.GroupVersion(), scope.Codec, result, w, req.Request) } } @@ -441,14 +422,14 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper if admit.Handles(admission.Update) { userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo)) + err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Update, userInfo)) if err != nil { errorJSON(err, scope.Codec, w) return } } - versionedObj, err := converter.ConvertToVersion(r.New(), scope.APIVersion) + versionedObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion().String()) if err != nil { errorJSON(err, scope.Codec, w) return @@ -478,7 +459,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper return } - write(http.StatusOK, scope.APIVersion, scope.Codec, result, w, req.Request) + write(http.StatusOK, scope.Kind.GroupVersion(), scope.Codec, result, w, req.Request) } } @@ -594,7 +575,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType } obj := r.New() - if err := scope.Codec.DecodeIntoWithSpecifiedVersionKind(body, obj, unversioned.ParseGroupVersionOrDie(scope.APIVersion).WithKind(scope.Kind)); err != nil { + if err := scope.Codec.DecodeIntoWithSpecifiedVersionKind(body, obj, scope.Kind); err != nil { err = transformDecodeError(typer, err, obj, body) errorJSON(err, scope.Codec, w) return @@ -608,7 +589,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType if admit != nil && admit.Handles(admission.Update) { userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo)) + err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Update, userInfo)) if err != nil { errorJSON(err, scope.Codec, w) return @@ -673,7 +654,7 @@ func DeleteResource(r rest.GracefulDeleter, checkBody bool, scope RequestScope, if admit != nil && admit.Handles(admission.Delete) { userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, userInfo)) + err = admit.Admit(admission.NewAttributesRecord(nil, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Delete, userInfo)) if err != nil { errorJSON(err, scope.Codec, w) return @@ -696,7 +677,7 @@ func DeleteResource(r rest.GracefulDeleter, checkBody bool, scope RequestScope, Code: http.StatusOK, Details: &unversioned.StatusDetails{ Name: name, - Kind: scope.Kind, + Kind: scope.Kind.Kind, }, } } else { @@ -708,7 +689,7 @@ func DeleteResource(r rest.GracefulDeleter, checkBody bool, scope RequestScope, } } } - write(http.StatusOK, scope.APIVersion, scope.Codec, result, w, req.Request) + write(http.StatusOK, scope.Kind.GroupVersion(), scope.Codec, result, w, req.Request) } }