rest mappings cannot logically be object converters

Kubernetes-commit: 6900f8856f8cd9a6c94a156b9e4a9fee0c16f807

pkg/endpoints/apiserver_test.go

@@ -117,7 +117,6 @@ var parameterCodec = runtime.NewParameterCodec(scheme)
 
 var accessor = meta.NewAccessor()
 var selfLinker runtime.SelfLinker = accessor
-var mapper, namespaceMapper meta.RESTMapper // The mappers with namespace and with legacy namespace scopes.
 var admissionControl admission.Interface
 
 func init() {
@@ -203,25 +202,6 @@ func init() {
 	addTestTypes()
 	addNewTestTypes()
 
-	nsMapper := newMapper()
-
-	// enumerate all supported versions, get the kinds, and register with
-	// the mapper how to address our resources
-	for _, gv := range groupVersions {
-		for kind := range scheme.KnownTypes(gv) {
-			gvk := gv.WithKind(kind)
-			root := bool(kind == "SimpleRoot")
-			if root {
-				nsMapper.Add(gvk, meta.RESTScopeRoot)
-			} else {
-				nsMapper.Add(gvk, meta.RESTScopeNamespace)
-			}
-		}
-	}
-
-	mapper = nsMapper
-	namespaceMapper = nsMapper
-
 	scheme.AddFieldLabelConversionFunc(grouplessGroupVersion.String(), "Simple",
 		func(label, value string) (string, string, error) {
 			return label, value, nil
@@ -268,7 +248,7 @@ func handleInternal(storage map[string]rest.Storage, admissionControl admission.
 		Defaulter:       scheme,
 		Typer:           scheme,
 		Linker:          selfLinker,
-		Mapper:    namespaceMapper,
+		RootScopedKinds: sets.NewString("SimpleRoot"),
 
 		ParameterCodec: parameterCodec,
 
@@ -841,7 +821,6 @@ func TestNotFound(t *testing.T) {
 
 		if response.StatusCode != v.Status {
 			t.Errorf("Expected %d for %s (%s), Got %#v", v.Status, v.Method, k, response)
-			t.Errorf("MAPPER: %v", mapper)
 		}
 	}
 }
@@ -3194,9 +3173,9 @@ func TestParentResourceIsRequired(t *testing.T) {
 		Defaulter:       scheme,
 		Typer:           scheme,
 		Linker:          selfLinker,
+		RootScopedKinds: sets.NewString("SimpleRoot"),
 
 		Admit: admissionControl,
-		Mapper: namespaceMapper,
 
 		GroupVersion:           newGroupVersion,
 		OptionsExternalVersion: &newGroupVersion,
@@ -3226,7 +3205,6 @@ func TestParentResourceIsRequired(t *testing.T) {
 		Linker:    selfLinker,
 
 		Admit: admissionControl,
-		Mapper: namespaceMapper,
 
 		GroupVersion:           newGroupVersion,
 		OptionsExternalVersion: &newGroupVersion,
@@ -3806,6 +3784,8 @@ type SimpleXGSubresourceRESTStorage struct {
 	itemGVK schema.GroupVersionKind
 }
 
+var _ = rest.GroupVersionKindProvider(&SimpleXGSubresourceRESTStorage{})
+
 func (storage *SimpleXGSubresourceRESTStorage) New() runtime.Object {
 	return &genericapitesting.SimpleXGSubresource{}
 }
@@ -3814,12 +3794,14 @@ func (storage *SimpleXGSubresourceRESTStorage) Get(ctx context.Context, id strin
 	return storage.item.DeepCopyObject(), nil
 }
 
-var _ = rest.GroupVersionKindProvider(&SimpleXGSubresourceRESTStorage{})
-
 func (storage *SimpleXGSubresourceRESTStorage) GroupVersionKind(containingGV schema.GroupVersion) schema.GroupVersionKind {
 	return storage.itemGVK
 }
 
+func (*SimpleXGSubresourceRESTStorage) ClusterScoped() bool {
+	return false
+}
+
 func TestXGSubresource(t *testing.T) {
 	container := restful.NewContainer()
 	container.Router(restful.CurlyRouter{})
@@ -3845,7 +3827,6 @@ func TestXGSubresource(t *testing.T) {
 		Defaulter: scheme,
 		Typer:     scheme,
 		Linker:    selfLinker,
-		Mapper:    namespaceMapper,
 
 		ParameterCodec: parameterCodec,
 

pkg/endpoints/groupversion.go

@@ -22,11 +22,11 @@ import (
 
 	"github.com/emicklei/go-restful"
 
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/endpoints/discovery"
 	"k8s.io/apiserver/pkg/registry/rest"
@@ -55,7 +55,8 @@ type APIGroupVersion struct {
 	// version (for when the inevitable meta/v2 group emerges).
 	MetaGroupVersion *schema.GroupVersion
 
-	Mapper meta.RESTMapper
+	// RootScopedKinds are the root scoped kinds for the primary GroupVersion
+	RootScopedKinds sets.String
 
 	// Serializer is used to determine how to convert responses from API methods into bytes to send over
 	// the wire.

pkg/endpoints/installer.go

@@ -28,7 +28,6 @@ import (
 
 	restful "github.com/emicklei/go-restful"
 
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/conversion"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -133,18 +132,18 @@ func (a *APIInstaller) newWebService() *restful.WebService {
 }
 
 // getResourceKind returns the external group version kind registered for the given storage
-// object. If the storage object is a subresource and has an override supplied for it, it returns
+// object and whether or not the kind is cluster scoped. If the storage object is a subresource and has an override supplied for it, it returns
 // the group version kind supplied in the override.
-func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schema.GroupVersionKind, error) {
+func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schema.GroupVersionKind, bool, error) {
 	// Let the storage tell us exactly what GVK it has
 	if gvkProvider, ok := storage.(rest.GroupVersionKindProvider); ok {
-		return gvkProvider.GroupVersionKind(a.group.GroupVersion), nil
+		return gvkProvider.GroupVersionKind(a.group.GroupVersion), gvkProvider.ClusterScoped(), nil
 	}
 
 	object := storage.New()
 	fqKinds, _, err := a.group.Typer.ObjectKinds(object)
 	if err != nil {
-		return schema.GroupVersionKind{}, err
+		return schema.GroupVersionKind{}, false, err
 	}
 
 	// a given go type can have multiple potential fully qualified kinds.  Find the one that corresponds with the group
@@ -157,32 +156,11 @@ func (a *APIInstaller) getResourceKind(path string, storage rest.Storage) (schem
 		}
 	}
 	if fqKindToRegister.Empty() {
-		return schema.GroupVersionKind{}, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion)
+		return schema.GroupVersionKind{}, false, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion)
 	}
-	return fqKindToRegister, nil
-}
 
-// restMapping returns rest mapper for the resource.
+	// group is guaranteed to match based on the check above
-// Example REST paths that this mapper maps.
+	return fqKindToRegister, a.group.RootScopedKinds.Has(fqKindToRegister.Kind), nil
-// 1. Resource only, no subresource:
-//      Resource Type:    batch/v1.Job (input args: resource = "jobs")
-//      REST path:        /apis/batch/v1/namespaces/{namespace}/job/{name}
-// 2. Subresource and its parent belong to different API groups and/or versions:
-//      Resource Type:    extensions/v1beta1.ReplicaSet (input args: resource = "replicasets")
-//      Subresource Type: autoscaling/v1.Scale
-//      REST path:        /apis/extensions/v1beta1/namespaces/{namespace}/replicaset/{name}/scale
-func (a *APIInstaller) restMapping(resource string) (*meta.RESTMapping, error) {
-	// subresources must have parent resources, and follow the namespacing rules of their parent.
-	// So get the storage of the resource (which is the parent resource in case of subresources)
-	storage, ok := a.group.Storage[resource]
-	if !ok {
-		return nil, fmt.Errorf("unable to locate the storage object for resource: %s", resource)
-	}
-	fqKindToRegister, err := a.getResourceKind(resource, storage)
-	if err != nil {
-		return nil, fmt.Errorf("unable to locate fully qualified kind for mapper resource %s: %v", resource, err)
-	}
-	return a.group.Mapper.RESTMapping(fqKindToRegister.GroupKind(), fqKindToRegister.Version)
 }
 
 func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storage, ws *restful.WebService) (*metav1.APIResource, error) {
@@ -198,12 +176,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		return nil, err
 	}
 
-	mapping, err := a.restMapping(resource)
+	fqKindToRegister, clusterScoped, err := a.getResourceKind(path, storage)
-	if err != nil {
-		return nil, err
-	}
-
-	fqKindToRegister, err := a.getResourceKind(path, storage)
 	if err != nil {
 		return nil, err
 	}
@@ -338,7 +311,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	}
 
 	allowWatchList := isWatcher && isLister // watching on lists is allowed only for kinds that support both watch and list.
-	scope := mapping.Scope
 	nameParam := ws.PathParameter("name", "name of the "+kind).DataType("string")
 	pathParam := ws.PathParameter("path", "path to the resource").DataType("string")
 
@@ -357,8 +329,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 
 	var apiResource metav1.APIResource
 	// Get the list of actions for the given scope.
-	switch scope.Name() {
+	switch {
-	case meta.RESTScopeNameRoot:
+	case clusterScoped:
 		// Handle non-namespace scoped resources like nodes.
 		resourcePath := resource
 		resourceParams := params
@@ -402,10 +374,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		actions = appendIf(actions, action{"CONNECT", itemPath, nameParams, namer, false}, isConnecter)
 		actions = appendIf(actions, action{"CONNECT", itemPath + "/{path:*}", proxyParams, namer, false}, isConnecter && connectSubpath)
 		break
-	case meta.RESTScopeNameNamespace:
+	default:
+		namespaceParamName := "namespaces"
 		// Handler for standard REST verbs (GET, PUT, POST and DELETE).
-		namespaceParam := ws.PathParameter(scope.ArgumentName(), scope.ParamDescription()).DataType("string")
+		namespaceParam := ws.PathParameter("namespace", "object name and auth scope, such as for teams and projects").DataType("string")
-		namespacedPath := scope.ParamName() + "/{" + scope.ArgumentName() + "}/" + resource
+		namespacedPath := namespaceParamName + "/{" + "namespace" + "}/" + resource
 		namespaceParams := []*restful.Parameter{namespaceParam}
 
 		resourcePath := namespacedPath
@@ -426,7 +399,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		namer := handlers.ContextBasedNaming{
 			SelfLinker:         a.group.Linker,
 			ClusterScoped:      false,
-			SelfLinkPathPrefix: gpath.Join(a.prefix, scope.ParamName()) + "/",
+			SelfLinkPathPrefix: gpath.Join(a.prefix, namespaceParamName) + "/",
 			SelfLinkPathSuffix: itemPathSuffix,
 		}
 
@@ -455,8 +428,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			actions = appendIf(actions, action{"WATCHLIST", "watch/" + resource, params, namer, true}, allowWatchList)
 		}
 		break
-	default:
-		return nil, fmt.Errorf("unsupported restscope: %s", scope.Name())
 	}
 
 	// Create Routes for the actions.
@@ -539,7 +510,11 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 
 		// If there is a subresource, kind should be the parent's kind.
 		if hasSubresource {
-			fqParentKind, err := a.getResourceKind(resource, a.group.Storage[resource])
+			parentStorage, ok := a.group.Storage[resource]
+			if !ok {
+				return nil, fmt.Errorf("missing parent storage: %q", resource)
+			}
+			fqParentKind, _, err := a.getResourceKind(resource, parentStorage)
 			if err != nil {
 				return nil, err
 			}
@@ -654,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, mapping.ObjectConvertor, supportedTypes))
+			handler := metrics.InstrumentRouteFunc(action.Verb, resource, subresource, requestScope, restfulPatchResource(patcher, reqScope, admit, a.group.Convertor, supportedTypes))
 			route := ws.PATCH(action.Path).To(handler).
 				Doc(doc).
 				Param(ws.QueryParameter("pretty", "If 'true', then the output is pretty printed.")).

pkg/registry/rest/rest.go

@@ -83,6 +83,7 @@ type CategoriesProvider interface {
 // TODO KindProvider (only used by federation) should be removed and replaced with this, but that presents greater risk late in 1.8.
 type GroupVersionKindProvider interface {
 	GroupVersionKind(containingGV schema.GroupVersion) schema.GroupVersionKind
+	ClusterScoped() bool
 }
 
 // Lister is an object that can retrieve resources that match the provided field and label criteria.

pkg/server/genericapiserver.go

@@ -423,7 +423,7 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV
 		Defaulter:       apiGroupInfo.Scheme,
 		Typer:           apiGroupInfo.Scheme,
 		Linker:          apiGroupInfo.GroupMeta.SelfLinker,
-		Mapper:          apiGroupInfo.GroupMeta.RESTMapper,
+		RootScopedKinds: apiGroupInfo.GroupMeta.RootScopedKinds,
 
 		Admit:                        s.admissionControl,
 		MinRequestTimeout:            s.minRequestTimeout,