Merge pull request #17335 from deads2k/gv-restmapper-api

pkg/api/install/install_test.go

@@ -74,20 +74,22 @@ func TestInterfacesFor(t *testing.T) {
 }
 
 func TestRESTMapper(t *testing.T) {
-	if v, k, err := latest.GroupOrDie("").RESTMapper.VersionAndKindForResource("replicationcontrollers"); err != nil || v != "v1" || k != "ReplicationController" {
-		t.Errorf("unexpected version mapping: %s %s %v", v, k, err)
-	}
+	gv := unversioned.GroupVersion{Group: "", Version: "v1"}
+	rcGVK := gv.WithKind("ReplicationController")
+	podTemplateGVK := gv.WithKind("PodTemplate")
 
-	expectedGroupVersion := unversioned.GroupVersion{Version: "v1"}
+	if gvk, err := latest.GroupOrDie("").RESTMapper.KindFor("replicationcontrollers"); err != nil || gvk != rcGVK {
+		t.Errorf("unexpected version mapping: %v %v", gvk, err)
+	}
 
-	if m, err := latest.GroupOrDie("").RESTMapper.RESTMapping("PodTemplate", ""); err != nil || m.GroupVersionKind.GroupVersion() != expectedGroupVersion || m.Resource != "podtemplates" {
+	if m, err := latest.GroupOrDie("").RESTMapper.RESTMapping(podTemplateGVK.GroupKind(), ""); err != nil || m.GroupVersionKind != podTemplateGVK || m.Resource != "podtemplates" {
 		t.Errorf("unexpected version mapping: %#v %v", m, err)
 	}
 
 	for _, version := range latest.GroupOrDie("").Versions {
 		currGroupVersion := unversioned.GroupVersion{Version: version}
 
-		mapping, err := latest.GroupOrDie("").RESTMapper.RESTMapping("ReplicationController", currGroupVersion.String())
+		mapping, err := latest.GroupOrDie("").RESTMapper.RESTMapping(rcGVK.GroupKind(), currGroupVersion.String())
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}

pkg/api/meta/interfaces.go

@@ -147,11 +147,11 @@ type RESTMapping struct {
 // TODO(caesarxuchao): Add proper multi-group support so that kinds & resources are
 // scoped to groups. See http://issues.k8s.io/12413 and http://issues.k8s.io/10009.
 type RESTMapper interface {
-	VersionAndKindForResource(resource string) (defaultVersion, kind string, err error)
-	// TODO(caesarxuchao): Remove GroupForResource when multi-group support is in (since
-	// group will be part of the version).
-	GroupForResource(resource string) (string, error)
-	RESTMapping(kind string, versions ...string) (*RESTMapping, error)
+	// KindFor takes a resource and returns back the unambiguous Kind (GroupVersionKind)
+	KindFor(resource string) (unversioned.GroupVersionKind, error)
+
+	RESTMapping(gk unversioned.GroupKind, versions ...string) (*RESTMapping, error)
+
 	AliasesForResource(resource string) ([]string, bool)
 	ResourceSingularizer(resource string) (singular string, err error)
 	ResourceIsValid(resource string) bool

pkg/api/meta/restmapper.go

@@ -80,6 +80,8 @@ type DefaultRESTMapper struct {
 	interfacesFunc VersionInterfacesFunc
 }
 
+var _ RESTMapper = &DefaultRESTMapper{}
+
 // VersionInterfacesFunc returns the appropriate codec, typer, and metadata accessor for a
 // given api version, or an error if no such api version exists.
 type VersionInterfacesFunc func(apiVersion string) (*VersionInterfaces, error)
@@ -163,95 +165,69 @@ func (m *DefaultRESTMapper) ResourceSingularizer(resource string) (singular stri
 }
 
 // VersionAndKindForResource implements RESTMapper
-func (m *DefaultRESTMapper) VersionAndKindForResource(resource string) (gvString, kind string, err error) {
+func (m *DefaultRESTMapper) KindFor(resource string) (unversioned.GroupVersionKind, error) {
 	gvk, ok := m.resourceToKind[strings.ToLower(resource)]
 	if !ok {
-		return "", "", fmt.Errorf("in version and kind for resource, no resource %q has been defined", resource)
-	}
-	return gvk.GroupVersion().String(), gvk.Kind, nil
-}
-
-func (m *DefaultRESTMapper) GroupForResource(resource string) (string, error) {
-	gvk, exists := m.resourceToKind[strings.ToLower(resource)]
-	if !exists {
-		return "", fmt.Errorf("in group for resource, no resource %q has been defined", resource)
+		return gvk, fmt.Errorf("in version and kind for resource, no resource %q has been defined", resource)
 	}
-
-	return gvk.Group, nil
+	return gvk, nil
 }
 
 // RESTMapping returns a struct representing the resource path and conversion interfaces a
-// RESTClient should use to operate on the provided kind in order of versions. If a version search
+// RESTClient should use to operate on the provided group/kind in order of versions. If a version search
 // order is not provided, the search order provided to DefaultRESTMapper will be used to resolve which
-// APIVersion should be used to access the named kind.
-// TODO version here in this RESTMapper means just APIVersion, but the RESTMapper API is intended to handle multiple groups
-// So this API is broken.  The RESTMapper test made it clear that versions here were API versions, but the code tries to use
-// them with group/version tuples.
-// TODO this should probably become RESTMapping(GroupKind, versions ...string)
-func (m *DefaultRESTMapper) RESTMapping(kind string, versions ...string) (*RESTMapping, error) {
-	// TODO, this looks really strange, but once this API is update, the version detection becomes clean again
-	// because you won't be able to request cross-group kinds
-	hadVersion := false
-	for _, gvString := range versions {
-		currGroupVersion, err := unversioned.ParseGroupVersion(gvString)
-		if err != nil {
-			return nil, err
-		}
-		if len(currGroupVersion.Version) != 0 {
-			hadVersion = true
-		}
-	}
-
+// version should be used to access the named group/kind.
+func (m *DefaultRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...string) (*RESTMapping, error) {
 	// Pick an appropriate version
-	var groupVersion *unversioned.GroupVersion
-	for _, v := range versions {
-		if len(v) == 0 {
+	var gvk *unversioned.GroupVersionKind
+	hadVersion := false
+	for _, version := range versions {
+		if len(version) == 0 {
 			continue
 		}
-		currGroupVersion, err := unversioned.ParseGroupVersion(v)
-		if err != nil {
-			return nil, err
-		}
 
-		currGVK := currGroupVersion.WithKind(kind)
+		currGVK := gk.WithVersion(version)
+		hadVersion = true
 		if _, ok := m.kindToPluralResource[currGVK]; ok {
-			groupVersion = &currGroupVersion
+			gvk = &currGVK
 			break
 		}
 	}
 	// Use the default preferred versions
-	if !hadVersion && (groupVersion == nil) {
-		for _, currGroupVersion := range m.defaultGroupVersions {
-			currGVK := currGroupVersion.WithKind(kind)
+	if !hadVersion && (gvk == nil) {
+		for _, gv := range m.defaultGroupVersions {
+			if gv.Group != gk.Group {
+				continue
+			}
+
+			currGVK := gk.WithVersion(gv.Version)
 			if _, ok := m.kindToPluralResource[currGVK]; ok {
-				groupVersion = &currGroupVersion
+				gvk = &currGVK
 				break
 			}
 		}
 	}
-	if groupVersion == nil {
-		return nil, fmt.Errorf("no kind named %q is registered in versions %q", kind, versions)
+	if gvk == nil {
+		return nil, fmt.Errorf("no kind named %q is registered in versions %q", gk, versions)
 	}
 
-	gvk := groupVersion.WithKind(kind)
-
 	// Ensure we have a REST mapping
-	resource, ok := m.kindToPluralResource[gvk]
+	resource, ok := m.kindToPluralResource[*gvk]
 	if !ok {
 		found := []unversioned.GroupVersion{}
 		for _, gv := range m.defaultGroupVersions {
-			if _, ok := m.kindToPluralResource[gvk]; ok {
+			if _, ok := m.kindToPluralResource[*gvk]; ok {
 				found = append(found, gv)
 			}
 		}
 		if len(found) > 0 {
-			return nil, fmt.Errorf("object with kind %q exists in versions %v, not %v", kind, found, *groupVersion)
+			return nil, fmt.Errorf("object with kind %q exists in versions %v, not %v", gvk.Kind, found, gvk.GroupVersion().String())
 		}
-		return nil, fmt.Errorf("the provided version %q and kind %q cannot be mapped to a supported object", groupVersion, kind)
+		return nil, fmt.Errorf("the provided version %q and kind %q cannot be mapped to a supported object", gvk.GroupVersion().String(), gvk.Kind)
 	}
 
 	// Ensure we have a REST scope
-	scope, ok := m.kindToScope[gvk]
+	scope, ok := m.kindToScope[*gvk]
 	if !ok {
 		return nil, fmt.Errorf("the provided version %q and kind %q cannot be mapped to a supported scope", gvk.GroupVersion().String(), gvk.Kind)
 	}
@@ -263,7 +239,7 @@ func (m *DefaultRESTMapper) RESTMapping(kind string, versions ...string) (*RESTM
 
 	retVal := &RESTMapping{
 		Resource:         resource,
-		GroupVersionKind: gvk,
+		GroupVersionKind: *gvk,
 		Scope:            scope,
 
 		Codec:            interfaces.Codec,
@@ -295,7 +271,7 @@ func (m *DefaultRESTMapper) AliasesForResource(alias string) ([]string, bool) {
 
 // ResourceIsValid takes a string (kind) and checks if it's a valid resource
 func (m *DefaultRESTMapper) ResourceIsValid(resource string) bool {
-	_, _, err := m.VersionAndKindForResource(resource)
+	_, err := m.KindFor(resource)
 	return err == nil
 }
 
@@ -317,21 +293,9 @@ func (m MultiRESTMapper) ResourceSingularizer(resource string) (singular string,
 // VersionAndKindForResource provides the Version and Kind  mappings for the
 // REST resources. This implementation supports multiple REST schemas and return
 // the first match.
-func (m MultiRESTMapper) VersionAndKindForResource(resource string) (defaultVersion, kind string, err error) {
-	for _, t := range m {
-		defaultVersion, kind, err = t.VersionAndKindForResource(resource)
-		if err == nil {
-			return
-		}
-	}
-	return
-}
-
-// GroupForResource provides the Group mappings for the REST resources. This
-// implementation supports multiple REST schemas and returns the first match.
-func (m MultiRESTMapper) GroupForResource(resource string) (group string, err error) {
+func (m MultiRESTMapper) KindFor(resource string) (gvk unversioned.GroupVersionKind, err error) {
 	for _, t := range m {
-		group, err = t.GroupForResource(resource)
+		gvk, err = t.KindFor(resource)
 		if err == nil {
 			return
 		}
@@ -339,12 +303,12 @@ func (m MultiRESTMapper) GroupForResource(resource string) (group string, err er
 	return
 }
 
-// RESTMapping provides the REST mapping for the resource based on the resource
+// RESTMapping provides the REST mapping for the resource based on the
 // kind and version. This implementation supports multiple REST schemas and
 // return the first match.
-func (m MultiRESTMapper) RESTMapping(kind string, versions ...string) (mapping *RESTMapping, err error) {
+func (m MultiRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...string) (mapping *RESTMapping, err error) {
 	for _, t := range m {
-		mapping, err = t.RESTMapping(kind, versions...)
+		mapping, err = t.RESTMapping(gk, versions...)
 		if err == nil {
 			return
 		}

pkg/api/meta/restmapper_test.go

@@ -115,7 +115,7 @@ func TestRESTMapperVersionAndKindForResource(t *testing.T) {
 		if len(testCase.ExpectedGVK.Kind) != 0 {
 			mapper.Add(testCase.ExpectedGVK, RESTScopeNamespace, testCase.MixedCase)
 		}
-		v, k, err := mapper.VersionAndKindForResource(testCase.Resource)
+		actualGVK, err := mapper.KindFor(testCase.Resource)
 
 		hasErr := err != nil
 		if hasErr != testCase.Err {
@@ -126,13 +126,6 @@ func TestRESTMapperVersionAndKindForResource(t *testing.T) {
 			continue
 		}
 
-		actualGV, err := unversioned.ParseGroupVersion(v)
-		if err != nil {
-			t.Errorf("%d: unexpected error: %v", i, err)
-			continue
-		}
-		actualGVK := unversioned.NewGroupVersionKind(actualGV, k)
-
 		if actualGVK != testCase.ExpectedGVK {
 			t.Errorf("%d: unexpected version and kind: e=%s a=%s", i, testCase.ExpectedGVK, actualGVK)
 		}
@@ -153,15 +146,15 @@ func TestRESTMapperGroupForResource(t *testing.T) {
 	for i, testCase := range testCases {
 		mapper := NewDefaultRESTMapper([]unversioned.GroupVersion{testCase.GroupVersionKind.GroupVersion()}, fakeInterfaces)
 		mapper.Add(testCase.GroupVersionKind, RESTScopeNamespace, false)
-		g, err := mapper.GroupForResource(testCase.Resource)
+		actualGVK, err := mapper.KindFor(testCase.Resource)
 		if testCase.Err {
 			if err == nil {
 				t.Errorf("%d: expected error", i)
 			}
 		} else if err != nil {
 			t.Errorf("%d: unexpected error: %v", i, err)
-		} else if g != testCase.GroupVersionKind.Group {
-			t.Errorf("%d: expected group %q, got %q", i, testCase.GroupVersionKind.Group, g)
+		} else if actualGVK != testCase.GroupVersionKind {
+			t.Errorf("%d: expected group %q, got %q", i, testCase.GroupVersionKind, actualGVK)
 		}
 	}
 }
@@ -268,12 +261,13 @@ func TestRESTMapperRESTMapping(t *testing.T) {
 		mapper := NewDefaultRESTMapper(testCase.DefaultVersions, fakeInterfaces)
 		mapper.Add(internalGroupVersion.WithKind("InternalObject"), RESTScopeNamespace, testCase.MixedCase)
 
-		deprecatedGroupVersionStrings := []string{}
+		preferredVersions := []string{}
 		for _, gv := range testCase.APIGroupVersions {
-			deprecatedGroupVersionStrings = append(deprecatedGroupVersionStrings, gv.String())
+			preferredVersions = append(preferredVersions, gv.Version)
 		}
+		gk := unversioned.GroupKind{Group: testGroup, Kind: testCase.Kind}
 
-		mapping, err := mapper.RESTMapping(testCase.Kind, deprecatedGroupVersionStrings...)
+		mapping, err := mapper.RESTMapping(gk, preferredVersions...)
 		hasErr := err != nil
 		if hasErr != testCase.Err {
 			t.Errorf("%d: unexpected error behavior %t: %v", i, testCase.Err, err)
@@ -304,13 +298,15 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
 	expectedGroupVersion1 := unversioned.GroupVersion{Group: "tgroup", Version: "test1"}
 	expectedGroupVersion2 := unversioned.GroupVersion{Group: "tgroup", Version: "test2"}
 	expectedGroupVersion3 := unversioned.GroupVersion{Group: "tgroup", Version: "test3"}
+	internalObjectGK := unversioned.GroupKind{Group: "tgroup", Kind: "InternalObject"}
+	otherObjectGK := unversioned.GroupKind{Group: "tgroup", Kind: "OtherObject"}
 
 	mapper := NewDefaultRESTMapper([]unversioned.GroupVersion{expectedGroupVersion1, expectedGroupVersion2}, fakeInterfaces)
 	mapper.Add(expectedGroupVersion1.WithKind("InternalObject"), RESTScopeNamespace, false)
 	mapper.Add(expectedGroupVersion2.WithKind("OtherObject"), RESTScopeNamespace, false)
 
 	// pick default matching object kind based on search order
-	mapping, err := mapper.RESTMapping("OtherObject")
+	mapping, err := mapper.RESTMapping(otherObjectGK)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -318,7 +314,7 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
 		t.Errorf("unexpected mapping: %#v", mapping)
 	}
 
-	mapping, err = mapper.RESTMapping("InternalObject")
+	mapping, err = mapper.RESTMapping(internalObjectGK)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -327,28 +323,28 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
 	}
 
 	// mismatch of version
-	mapping, err = mapper.RESTMapping("InternalObject", expectedGroupVersion2.String())
+	mapping, err = mapper.RESTMapping(internalObjectGK, expectedGroupVersion2.Version)
 	if err == nil {
 		t.Errorf("unexpected non-error")
 	}
-	mapping, err = mapper.RESTMapping("OtherObject", expectedGroupVersion1.String())
+	mapping, err = mapper.RESTMapping(otherObjectGK, expectedGroupVersion1.Version)
 	if err == nil {
 		t.Errorf("unexpected non-error")
 	}
 
 	// not in the search versions
-	mapping, err = mapper.RESTMapping("OtherObject", expectedGroupVersion3.String())
+	mapping, err = mapper.RESTMapping(otherObjectGK, expectedGroupVersion3.Version)
 	if err == nil {
 		t.Errorf("unexpected non-error")
 	}
 
 	// explicit search order
-	mapping, err = mapper.RESTMapping("OtherObject", expectedGroupVersion3.String(), expectedGroupVersion1.String())
+	mapping, err = mapper.RESTMapping(otherObjectGK, expectedGroupVersion3.Version, expectedGroupVersion1.Version)
 	if err == nil {
 		t.Errorf("unexpected non-error")
 	}
 
-	mapping, err = mapper.RESTMapping("OtherObject", expectedGroupVersion3.String(), expectedGroupVersion2.String())
+	mapping, err = mapper.RESTMapping(otherObjectGK, expectedGroupVersion3.Version, expectedGroupVersion2.Version)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -360,10 +356,11 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
 func TestRESTMapperReportsErrorOnBadVersion(t *testing.T) {
 	expectedGroupVersion1 := unversioned.GroupVersion{Group: "tgroup", Version: "test1"}
 	expectedGroupVersion2 := unversioned.GroupVersion{Group: "tgroup", Version: "test2"}
+	internalObjectGK := unversioned.GroupKind{Group: "tgroup", Kind: "InternalObject"}
 
 	mapper := NewDefaultRESTMapper([]unversioned.GroupVersion{expectedGroupVersion1, expectedGroupVersion2}, unmatchedVersionInterfaces)
 	mapper.Add(expectedGroupVersion1.WithKind("InternalObject"), RESTScopeNamespace, false)
-	_, err := mapper.RESTMapping("InternalObject", expectedGroupVersion1.String())
+	_, err := mapper.RESTMapping(internalObjectGK, expectedGroupVersion1.Version)
 	if err == nil {
 		t.Errorf("unexpected non-error")
 	}

pkg/api/unversioned/group_version.go

@@ -22,6 +22,21 @@ import (
 	"strings"
 )
 
+// 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 {
+	Group string
+	Kind  string
+}
+
+func (gk GroupKind) WithVersion(version string) GroupVersionKind {
+	return GroupVersionKind{Group: gk.Group, Version: version, Kind: gk.Kind}
+}
+
+func (gk *GroupKind) String() string {
+	return gk.Group + ", Kind=" + gk.Kind
+}
+
 // GroupVersionKind unambiguously identifies a kind.  It doesn't anonymously include GroupVersion
 // to avoid automatic coersion.  It doesn't use a GroupVersion to avoid custom marshalling
 type GroupVersionKind struct {
@@ -35,6 +50,10 @@ func NewGroupVersionKind(gv GroupVersion, kind string) GroupVersionKind {
 	return GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: kind}
 }
 
+func (gvk GroupVersionKind) GroupKind() GroupKind {
+	return GroupKind{Group: gvk.Group, Kind: gvk.Kind}
+}
+
 func (gvk GroupVersionKind) GroupVersion() GroupVersion {
 	return GroupVersion{Group: gvk.Group, Version: gvk.Version}
 }

pkg/apis/componentconfig/install/install_test.go

@@ -54,23 +54,19 @@ func TestInterfacesFor(t *testing.T) {
 }
 
 func TestRESTMapper(t *testing.T) {
-	expectedGroupVersion := unversioned.GroupVersion{Group: "componentconfig", Version: "v1alpha1"}
+	gv := unversioned.GroupVersion{Group: "componentconfig", Version: "v1alpha1"}
+	proxyGVK := gv.WithKind("KubeProxyConfiguration")
 
-	if v, k, err := latest.GroupOrDie("componentconfig").RESTMapper.VersionAndKindForResource("kubeproxyconfiguration"); err != nil || v != expectedGroupVersion.String() || k != "KubeProxyConfiguration" {
-		t.Errorf("unexpected version mapping: %q %q %v", v, k, err)
+	if gvk, err := latest.GroupOrDie("componentconfig").RESTMapper.KindFor("kubeproxyconfiguration"); err != nil || gvk != proxyGVK {
+		t.Errorf("unexpected version mapping: %v %v", gvk, err)
 	}
 
-	if m, err := latest.GroupOrDie("componentconfig").RESTMapper.RESTMapping("KubeProxyConfiguration", ""); err != nil || m.GroupVersionKind.GroupVersion() != expectedGroupVersion || m.Resource != "kubeproxyconfigurations" {
+	if m, err := latest.GroupOrDie("componentconfig").RESTMapper.RESTMapping(proxyGVK.GroupKind(), ""); err != nil || m.GroupVersionKind != proxyGVK || m.Resource != "kubeproxyconfigurations" {
 		t.Errorf("unexpected version mapping: %#v %v", m, err)
 	}
 
-	for _, groupVersionString := range latest.GroupOrDie("componentconfig").GroupVersions {
-		gv, err := unversioned.ParseGroupVersion(groupVersionString)
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
-			continue
-		}
-		mapping, err := latest.GroupOrDie("componentconfig").RESTMapper.RESTMapping("KubeProxyConfiguration", gv.String())
+	for _, version := range latest.GroupOrDie("componentconfig").Versions {
+		mapping, err := latest.GroupOrDie("componentconfig").RESTMapper.RESTMapping(proxyGVK.GroupKind(), version)
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 			continue

pkg/apis/extensions/install/install_test.go

@@ -75,20 +75,20 @@ func TestInterfacesFor(t *testing.T) {
 }
 
 func TestRESTMapper(t *testing.T) {
-	expectedGroupVersion := unversioned.GroupVersion{Group: "extensions", Version: "v1beta1"}
+	gv := unversioned.GroupVersion{Group: "extensions", Version: "v1beta1"}
+	hpaGVK := gv.WithKind("HorizontalPodAutoscaler")
+	daemonSetGVK := gv.WithKind("DaemonSet")
 
-	if v, k, err := latest.GroupOrDie("extensions").RESTMapper.VersionAndKindForResource("horizontalpodautoscalers"); err != nil || v != expectedGroupVersion.String() || k != "HorizontalPodAutoscaler" {
-		t.Errorf("unexpected version mapping: %s %s %v", v, k, err)
+	if gvk, err := latest.GroupOrDie("extensions").RESTMapper.KindFor("horizontalpodautoscalers"); err != nil || gvk != hpaGVK {
+		t.Errorf("unexpected version mapping: %v %v", gvk, err)
 	}
 
-	if m, err := latest.GroupOrDie("extensions").RESTMapper.RESTMapping("DaemonSet", ""); err != nil || m.GroupVersionKind.GroupVersion() != expectedGroupVersion || m.Resource != "daemonsets" {
+	if m, err := latest.GroupOrDie("extensions").RESTMapper.RESTMapping(daemonSetGVK.GroupKind(), ""); err != nil || m.GroupVersionKind != daemonSetGVK || m.Resource != "daemonsets" {
 		t.Errorf("unexpected version mapping: %#v %v", m, err)
 	}
 
-	for _, groupVersionString := range latest.GroupOrDie("extensions").GroupVersions {
-		gv, err := unversioned.ParseGroupVersion(groupVersionString)
-
-		mapping, err := latest.GroupOrDie("extensions").RESTMapper.RESTMapping("HorizontalPodAutoscaler", gv.String())
+	for _, version := range latest.GroupOrDie("extensions").Versions {
+		mapping, err := latest.GroupOrDie("extensions").RESTMapper.RESTMapping(hpaGVK.GroupKind(), version)
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}

pkg/apiserver/api_installer.go

@@ -126,13 +126,15 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	if err != nil {
 		return nil, err
 	}
+	gvk := a.group.GroupVersion.WithKind(kind)
+
 	versionedPtr, err := a.group.Creater.New(a.group.GroupVersion.String(), kind)
 	if err != nil {
 		return nil, err
 	}
 	versionedObject := indirectArbitraryPointer(versionedPtr)
 
-	mapping, err := a.group.Mapper.RESTMapping(kind, a.group.GroupVersion.String())
+	mapping, err := a.group.Mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 	if err != nil {
 		return nil, err
 	}
@@ -148,7 +150,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		if err != nil {
 			return nil, err
 		}
-		parentMapping, err := a.group.Mapper.RESTMapping(parentKind, a.group.GroupVersion.String())
+		parentGVK := a.group.GroupVersion.WithKind(parentKind)
+
+		parentMapping, err := a.group.Mapper.RESTMapping(parentGVK.GroupKind(), parentGVK.Version)
 		if err != nil {
 			return nil, err
 		}

pkg/client/unversioned/testclient/fixture.go

@@ -59,15 +59,10 @@ type ObjectScheme interface {
 func ObjectReaction(o ObjectRetriever, mapper meta.RESTMapper) ReactionFunc {
 
 	return func(action Action) (bool, runtime.Object, error) {
-		gvString, kind, err := mapper.VersionAndKindForResource(action.GetResource())
+		gvk, err := mapper.KindFor(action.GetResource())
 		if err != nil {
 			return false, nil, fmt.Errorf("unrecognized action %s: %v", action.GetResource(), err)
 		}
-		gv, err := unversioned.ParseGroupVersion(gvString)
-		if err != nil {
-			return false, nil, err
-		}
-		gvk := gv.WithKind(kind)
 
 		// TODO: have mapper return a Kind for a subresource?
 		switch castAction := action.(type) {

pkg/kubectl/cmd/explain.go

@@ -82,13 +82,13 @@ func RunExplain(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st
 	}
 
 	// TODO: We should deduce the group for a resource by discovering the supported resources at server.
-	group, err := mapper.GroupForResource(inModel)
+	gvk, err := mapper.KindFor(inModel)
 	if err != nil {
 		return err
 	}
 
 	if len(apiV) == 0 {
-		groupMeta, err := latest.Group(group)
+		groupMeta, err := latest.Group(gvk.Group)
 		if err != nil {
 			return err
 		}

pkg/kubectl/cmd/run.go

@@ -25,6 +25,7 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/kubectl"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -370,10 +371,15 @@ func createGeneratedObject(f *cmdutil.Factory, cmd *cobra.Command, generator kub
 	}
 
 	mapper, typer := f.Object()
-	version, kind, err := typer.ObjectVersionAndKind(obj)
+	gvString, kind, err := typer.ObjectVersionAndKind(obj)
 	if err != nil {
 		return nil, "", nil, nil, err
 	}
+	gv, err := unversioned.ParseGroupVersion(gvString)
+	if err != nil {
+		return nil, "", nil, nil, err
+	}
+	gvk := gv.WithKind(kind)
 
 	if len(overrides) > 0 {
 		obj, err = cmdutil.Merge(obj, overrides, kind)
@@ -382,7 +388,7 @@ func createGeneratedObject(f *cmdutil.Factory, cmd *cobra.Command, generator kub
 		}
 	}
 
-	mapping, err := mapper.RESTMapping(kind, version)
+	mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 	if err != nil {
 		return nil, "", nil, nil, err
 	}

pkg/kubectl/cmd/util/factory.go

@@ -151,7 +151,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			return clients.ClientConfigForVersion("")
 		},
 		RESTClient: func(mapping *meta.RESTMapping) (resource.RESTClient, error) {
-			group, err := api.RESTMapper.GroupForResource(mapping.Resource)
+			gvk, err := api.RESTMapper.KindFor(mapping.Resource)
 			if err != nil {
 				return nil, err
 			}
@@ -159,7 +159,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			if err != nil {
 				return nil, err
 			}
-			switch group {
+			switch gvk.Group {
 			case "":
 				return client.RESTClient, nil
 			case "extensions":
@@ -168,7 +168,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			return nil, fmt.Errorf("unable to get RESTClient for resource '%s'", mapping.Resource)
 		},
 		Describer: func(mapping *meta.RESTMapping) (kubectl.Describer, error) {
-			group, err := api.RESTMapper.GroupForResource(mapping.Resource)
+			gvk, err := api.RESTMapper.KindFor(mapping.Resource)
 			if err != nil {
 				return nil, err
 			}
@@ -176,7 +176,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			if err != nil {
 				return nil, err
 			}
-			if describer, ok := kubectl.DescriberFor(group, mapping.GroupVersionKind.Kind, client); ok {
+			if describer, ok := kubectl.DescriberFor(gvk.Group, mapping.GroupVersionKind.Kind, client); ok {
 				return describer, nil
 			}
 			return nil, fmt.Errorf("no description has been implemented for %q", mapping.Kind)
@@ -497,11 +497,11 @@ func (c *clientSwaggerSchema) ValidateBytes(data []byte) error {
 		return fmt.Errorf("API version %q isn't supported, only supports API versions %q", version, registered.RegisteredGroupVersions)
 	}
 	resource, _ := meta.KindToResource(kind, false)
-	group, err := c.mapper.GroupForResource(resource)
+	gvk, err := c.mapper.KindFor(resource)
 	if err != nil {
 		return fmt.Errorf("could not find api group for %s: %v", kind, err)
 	}
-	if group == "extensions" {
+	if gvk.Group == "extensions" {
 		if c.c.ExtensionsClient == nil {
 			return errors.New("unable to validate: no experimental client")
 		}
@@ -567,12 +567,16 @@ func DefaultClientConfig(flags *pflag.FlagSet) clientcmd.ClientConfig {
 // PrintObject prints an api object given command line flags to modify the output format
 func (f *Factory) PrintObject(cmd *cobra.Command, obj runtime.Object, out io.Writer) error {
 	mapper, _ := f.Object()
-	_, kind, err := api.Scheme.ObjectVersionAndKind(obj)
+	gvString, kind, err := api.Scheme.ObjectVersionAndKind(obj)
+	if err != nil {
+		return err
+	}
+	gv, err := unversioned.ParseGroupVersion(gvString)
 	if err != nil {
 		return err
 	}
 
-	mapping, err := mapper.RESTMapping(kind)
+	mapping, err := mapper.RESTMapping(unversioned.GroupKind{Group: gv.Group, Kind: kind})
 	if err != nil {
 		return err
 	}

pkg/kubectl/kubectl.go

@@ -22,6 +22,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 )
 
 const kubectlAnnotationPrefix = "kubectl.kubernetes.io/"
@@ -50,13 +51,13 @@ type OutputVersionMapper struct {
 }
 
 // RESTMapping implements meta.RESTMapper by prepending the output version to the preferred version list.
-func (m OutputVersionMapper) RESTMapping(kind string, versions ...string) (*meta.RESTMapping, error) {
-	mapping, err := m.RESTMapper.RESTMapping(kind, m.OutputVersion)
+func (m OutputVersionMapper) RESTMapping(gk unversioned.GroupKind, versions ...string) (*meta.RESTMapping, error) {
+	mapping, err := m.RESTMapper.RESTMapping(gk, m.OutputVersion)
 	if err == nil {
 		return mapping, nil
 	}
 
-	return m.RESTMapper.RESTMapping(kind, versions...)
+	return m.RESTMapper.RESTMapping(gk, versions...)
 }
 
 // ShortcutExpander is a RESTMapper that can be used for Kubernetes
@@ -65,12 +66,13 @@ type ShortcutExpander struct {
 	meta.RESTMapper
 }
 
-// VersionAndKindForResource implements meta.RESTMapper. It expands the resource first, then invokes the wrapped
+var _ meta.RESTMapper = &ShortcutExpander{}
+
+// KindFor implements meta.RESTMapper. It expands the resource first, then invokes the wrapped
 // mapper.
-func (e ShortcutExpander) VersionAndKindForResource(resource string) (defaultVersion, kind string, err error) {
+func (e ShortcutExpander) KindFor(resource string) (unversioned.GroupVersionKind, error) {
 	resource = expandResourceShortcut(resource)
-	defaultVersion, kind, err = e.RESTMapper.VersionAndKindForResource(resource)
-	return defaultVersion, kind, err
+	return e.RESTMapper.KindFor(resource)
 }
 
 // ResourceIsValid takes a string (kind) and checks if it's a valid resource.

pkg/kubectl/resource/builder.go

@@ -426,11 +426,11 @@ func (b *Builder) resourceMappings() ([]*meta.RESTMapping, error) {
 	}
 	mappings := []*meta.RESTMapping{}
 	for _, r := range b.resources {
-		version, kind, err := b.mapper.VersionAndKindForResource(r)
+		gvk, err := b.mapper.KindFor(r)
 		if err != nil {
 			return nil, err
 		}
-		mapping, err := b.mapper.RESTMapping(kind, version)
+		mapping, err := b.mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 		if err != nil {
 			return nil, err
 		}
@@ -446,11 +446,11 @@ func (b *Builder) resourceTupleMappings() (map[string]*meta.RESTMapping, error)
 		if _, ok := mappings[r.Resource]; ok {
 			continue
 		}
-		version, kind, err := b.mapper.VersionAndKindForResource(r.Resource)
+		gvk, err := b.mapper.KindFor(r.Resource)
 		if err != nil {
 			return nil, err
 		}
-		mapping, err := b.mapper.RESTMapping(kind, version)
+		mapping, err := b.mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 		if err != nil {
 			return nil, err
 		}

pkg/kubectl/resource/mapper.go

@@ -59,7 +59,7 @@ func (m *Mapper) InfoForData(data []byte, source string) (*Info, error) {
 	if kind == "" {
 		return nil, fmt.Errorf("kind not set in %q", source)
 	}
-	mapping, err := m.RESTMapping(kind, version)
+	mapping, err := m.RESTMapping(unversioned.GroupKind{Group: gv.Group, Kind: kind}, gv.Version)
 	if err != nil {
 		return nil, fmt.Errorf("unable to recognize %q: %v", source, err)
 	}
@@ -97,11 +97,15 @@ func (m *Mapper) InfoForData(data []byte, source string) (*Info, error) {
 // if the object cannot be introspected. Name and namespace will be set into Info
 // if the mapping's MetadataAccessor can retrieve them.
 func (m *Mapper) InfoForObject(obj runtime.Object) (*Info, error) {
-	version, kind, err := m.ObjectVersionAndKind(obj)
+	gvString, kind, err := m.ObjectVersionAndKind(obj)
 	if err != nil {
 		return nil, fmt.Errorf("unable to get type info from the object %q: %v", reflect.TypeOf(obj), err)
 	}
-	mapping, err := m.RESTMapping(kind, version)
+	gv, err := unversioned.ParseGroupVersion(gvString)
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse group/version from %q: %v", gvString, err)
+	}
+	mapping, err := m.RESTMapping(unversioned.GroupKind{Group: gv.Group, Kind: kind}, gv.Version)
 	if err != nil {
 		return nil, fmt.Errorf("unable to recognize %q: %v", kind, err)
 	}

pkg/registry/thirdpartyresourcedata/codec.go

@@ -39,28 +39,37 @@ type thirdPartyResourceDataMapper struct {
 	group   string
 }
 
+var _ meta.RESTMapper = &thirdPartyResourceDataMapper{}
+
 func (t *thirdPartyResourceDataMapper) isThirdPartyResource(resource string) bool {
 	return resource == strings.ToLower(t.kind)+"s"
 }
 
-func (t *thirdPartyResourceDataMapper) GroupForResource(resource string) (string, error) {
+func (t *thirdPartyResourceDataMapper) KindFor(resource string) (unversioned.GroupVersionKind, error) {
 	if t.isThirdPartyResource(resource) {
-		return t.group, nil
+		return unversioned.GroupVersionKind{Group: t.group, Version: t.version, Kind: t.kind}, nil
 	}
-	return t.mapper.GroupForResource(resource)
+	return t.mapper.KindFor(resource)
 }
 
-func (t *thirdPartyResourceDataMapper) RESTMapping(kind string, groupVersions ...string) (*meta.RESTMapping, error) {
-	if len(groupVersions) != 1 {
-		return nil, fmt.Errorf("unexpected set of groupVersions: %v", groupVersions)
+func (t *thirdPartyResourceDataMapper) RESTMapping(gk unversioned.GroupKind, versions ...string) (*meta.RESTMapping, error) {
+	if len(versions) != 1 {
+		return nil, fmt.Errorf("unexpected set of versions: %v", versions)
+	}
+	if gk.Group != t.group {
+		return nil, fmt.Errorf("unknown group %q expected %s", gk.Group, t.group)
 	}
-	if groupVersions[0] != apiutil.GetGroupVersion(t.group, t.version) {
-		return nil, fmt.Errorf("unknown version %s expected %s", groupVersions[0], apiutil.GetGroupVersion(t.group, t.version))
+	if gk.Kind != "ThirdPartyResourceData" {
+		return nil, fmt.Errorf("unknown kind %s expected %s", gk.Kind, t.kind)
 	}
-	if kind != "ThirdPartyResourceData" {
-		return nil, fmt.Errorf("unknown kind %s expected %s", kind, t.kind)
+	if versions[0] != t.version {
+		return nil, fmt.Errorf("unknown version %q expected %q", versions[0], t.version)
 	}
-	mapping, err := t.mapper.RESTMapping("ThirdPartyResourceData", latest.GroupOrDie("extensions").GroupVersion)
+
+	// TODO figure out why we're doing this rewriting
+	extensionGK := unversioned.GroupKind{Group: "extensions", Kind: "ThirdPartyResourceData"}
+
+	mapping, err := t.mapper.RESTMapping(extensionGK, latest.GroupOrDie("extensions").Version)
 	if err != nil {
 		return nil, err
 	}
@@ -76,13 +85,6 @@ func (t *thirdPartyResourceDataMapper) ResourceSingularizer(resource string) (si
 	return t.mapper.ResourceSingularizer(resource)
 }
 
-func (t *thirdPartyResourceDataMapper) VersionAndKindForResource(resource string) (defaultVersion, kind string, err error) {
-	if t.isThirdPartyResource(resource) {
-		return t.version, t.kind, nil
-	}
-	return t.mapper.VersionAndKindForResource(resource)
-}
-
 // ResourceIsValid takes a string (kind) and checks if it's a valid resource
 func (t *thirdPartyResourceDataMapper) ResourceIsValid(resource string) bool {
 	return t.isThirdPartyResource(resource) || t.mapper.ResourceIsValid(resource)

plugin/pkg/admission/namespace/autoprovision/admission.go

@@ -48,11 +48,11 @@ type provision struct {
 }
 
 func (p *provision) Admit(a admission.Attributes) (err error) {
-	defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource())
+	gvk, err := api.RESTMapper.KindFor(a.GetResource())
 	if err != nil {
 		return admission.NewForbidden(a, err)
 	}
-	mapping, err := api.RESTMapper.RESTMapping(kind, defaultVersion)
+	mapping, err := api.RESTMapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 	if err != nil {
 		return admission.NewForbidden(a, err)
 	}

plugin/pkg/admission/namespace/exists/admission.go

@@ -49,11 +49,11 @@ type exists struct {
 }
 
 func (e *exists) Admit(a admission.Attributes) (err error) {
-	defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource())
+	gvk, err := api.RESTMapper.KindFor(a.GetResource())
 	if err != nil {
 		return errors.NewInternalError(err)
 	}
-	mapping, err := api.RESTMapper.RESTMapping(kind, defaultVersion)
+	mapping, err := api.RESTMapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 	if err != nil {
 		return errors.NewInternalError(err)
 	}

plugin/pkg/admission/namespace/lifecycle/admission.go

@@ -57,11 +57,11 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
 		return errors.NewForbidden(a.GetKind(), a.GetName(), fmt.Errorf("this namespace may not be deleted"))
 	}
 
-	defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource())
+	gvk, err := api.RESTMapper.KindFor(a.GetResource())
 	if err != nil {
 		return errors.NewInternalError(err)
 	}
-	mapping, err := api.RESTMapper.RESTMapping(kind, defaultVersion)
+	mapping, err := api.RESTMapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 	if err != nil {
 		return errors.NewInternalError(err)
 	}