extended RESTMappings method by a version parameter.

RESTMapping method can now rely on RESTMappings by passing versions parameter and taking the first match found by RESTMappings method. In addition
a UT that test the new method has been added.

The only change in logic to what was before is when calling RESTMapping
we search all defaultGroupVersion as opposed to just one when no mapping was found for provided versions.

pkg/api/meta/interfaces.go

@@ -177,8 +177,10 @@ type RESTMapper interface {
 
 	// RESTMapping identifies a preferred resource mapping for the provided group kind.
 	RESTMapping(gk schema.GroupKind, versions ...string) (*RESTMapping, error)
-	// RESTMappings returns all resource mappings for the provided group kind.
+	// RESTMappings returns all resource mappings for the provided group kind if no
-	RESTMappings(gk schema.GroupKind) ([]*RESTMapping, error)
+	// version search is provided. Otherwise identifies a preferred resource mapping for
+	// the provided version(s).
+	RESTMappings(gk schema.GroupKind, versions ...string) ([]*RESTMapping, error)
 
 	AliasesForResource(resource string) ([]string, bool)
 	ResourceSingularizer(resource string) (singular string, err error)

pkg/api/meta/multirestmapper.go

@@ -185,12 +185,12 @@ func (m MultiRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*
 
 // RESTMappings returns all possible RESTMappings for the provided group kind, or an error
 // if the type is not recognized.
-func (m MultiRESTMapper) RESTMappings(gk schema.GroupKind) ([]*RESTMapping, error) {
+func (m MultiRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*RESTMapping, error) {
 	var allMappings []*RESTMapping
 	var errors []error
 
 	for _, t := range m {
-		currMappings, err := t.RESTMappings(gk)
+		currMappings, err := t.RESTMappings(gk, versions...)
 		// ignore "no match" errors, but any other error percolates back up
 		if IsNoMatchError(err) {
 			continue

pkg/api/meta/multirestmapper_test.go

@@ -346,7 +346,7 @@ func (m fixedRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (m
 	return nil, m.err
 }
 
-func (m fixedRESTMapper) RESTMappings(gk schema.GroupKind) (mappings []*RESTMapping, err error) {
+func (m fixedRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) (mappings []*RESTMapping, err error) {
 	return m.mappings, m.err
 }
 

pkg/api/meta/priority.go

@@ -205,8 +205,8 @@ func (m PriorityRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string)
 	return nil, &AmbiguousKindError{PartialKind: gk.WithVersion(""), MatchingKinds: kinds}
 }
 
-func (m PriorityRESTMapper) RESTMappings(gk schema.GroupKind) ([]*RESTMapping, error) {
+func (m PriorityRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*RESTMapping, error) {
-	return m.Delegate.RESTMappings(gk)
+	return m.Delegate.RESTMappings(gk, versions...)
 }
 
 func (m PriorityRESTMapper) AliasesForResource(alias string) (aliases []string, ok bool) {

pkg/api/meta/restmapper.go

@@ -468,91 +468,56 @@ func (o resourceByPreferredGroupVersion) Less(i, j int) bool {
 // 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
 // version should be used to access the named group/kind.
-// TODO: consider refactoring to use RESTMappings in a way that preserves version ordering and preference
 func (m *DefaultRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*RESTMapping, error) {
-	// Pick an appropriate version
+	mappings, err := m.RESTMappings(gk, versions...)
-	var gvk *schema.GroupVersionKind
+	if err != nil {
+		return nil, err
+	}
+	if len(mappings) == 0 {
+		return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")}
+	}
+	// since we rely on RESTMappings method
+	// take the first match and return to the caller
+	// as this was the existing behavior.
+	return mappings[0], nil
+}
+
+// RESTMappings returns the RESTMappings for the provided group kind. If a version search order
+// is not provided, the search order provided to DefaultRESTMapper will be used.
+func (m *DefaultRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*RESTMapping, error) {
+	mappings := make([]*RESTMapping, 0)
+	potentialGVK := make([]schema.GroupVersionKind, 0)
 	hadVersion := false
+
+	// Pick an appropriate version
 	for _, version := range versions {
 		if len(version) == 0 || version == runtime.APIVersionInternal {
 			continue
 		}
-
 		currGVK := gk.WithVersion(version)
 		hadVersion = true
 		if _, ok := m.kindToPluralResource[currGVK]; ok {
-			gvk = &currGVK
+			potentialGVK = append(potentialGVK, currGVK)
 			break
 		}
 	}
 	// Use the default preferred versions
-	if !hadVersion && (gvk == nil) {
+	if !hadVersion && len(potentialGVK) == 0 {
 		for _, gv := range m.defaultGroupVersions {
 			if gv.Group != gk.Group {
 				continue
 			}
+			potentialGVK = append(potentialGVK, gk.WithVersion(gv.Version))
+		}
+	}
 
-			currGVK := gk.WithVersion(gv.Version)
+	if len(potentialGVK) == 0 {
-			if _, ok := m.kindToPluralResource[currGVK]; ok {
-				gvk = &currGVK
-				break
-			}
-		}
-	}
-	if gvk == nil {
 		return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")}
 	}
 
-	// Ensure we have a REST mapping
+	for _, gvk := range potentialGVK {
-	resource, ok := m.kindToPluralResource[*gvk]
+		//Ensure we have a REST mapping
-	if !ok {
+		res, ok := m.kindToPluralResource[gvk]
-		found := []schema.GroupVersion{}
-		for _, gv := range m.defaultGroupVersions {
-			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", gvk.Kind, found, gvk.GroupVersion().String())
-		}
-		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]
-	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)
-	}
-
-	interfaces, err := m.interfacesFunc(gvk.GroupVersion())
-	if err != nil {
-		return nil, fmt.Errorf("the provided version %q has no relevant versions: %v", gvk.GroupVersion().String(), err)
-	}
-
-	retVal := &RESTMapping{
-		Resource:         resource.Resource,
-		GroupVersionKind: *gvk,
-		Scope:            scope,
-
-		ObjectConvertor:  interfaces.ObjectConvertor,
-		MetadataAccessor: interfaces.MetadataAccessor,
-	}
-
-	return retVal, nil
-}
-
-// RESTMappings returns the RESTMappings for the provided group kind in a rough internal preferred order. If no
-// kind is found it will return a NoResourceMatchError.
-func (m *DefaultRESTMapper) RESTMappings(gk schema.GroupKind) ([]*RESTMapping, error) {
-	// Use the default preferred versions
-	var mappings []*RESTMapping
-	for _, gv := range m.defaultGroupVersions {
-		if gv.Group != gk.Group {
-			continue
-		}
-
-		gvk := gk.WithVersion(gv.Version)
-		gvr, ok := m.kindToPluralResource[gvk]
 		if !ok {
 			continue
 		}
@@ -569,7 +534,7 @@ func (m *DefaultRESTMapper) RESTMappings(gk schema.GroupKind) ([]*RESTMapping, e
 		}
 
 		mappings = append(mappings, &RESTMapping{
-			Resource:         gvr.Resource,
+			Resource:         res.Resource,
 			GroupVersionKind: gvk,
 			Scope:            scope,
 

pkg/api/meta/restmapper_test.go

@@ -651,6 +651,92 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
 	}
 }
 
+func TestRESTMapperRESTMappings(t *testing.T) {
+	testGroup := "tgroup"
+	testGroupVersion := schema.GroupVersion{Group: testGroup, Version: "v1"}
+
+	testCases := []struct {
+		Kind                string
+		APIGroupVersions    []schema.GroupVersion
+		DefaultVersions     []schema.GroupVersion
+		AddGroupVersionKind []schema.GroupVersionKind
+
+		ExpectedRESTMappings []*RESTMapping
+		Err                  bool
+	}{
+		{Kind: "Unknown", Err: true},
+		{Kind: "InternalObject", Err: true},
+
+		{DefaultVersions: []schema.GroupVersion{testGroupVersion}, Kind: "Unknown", Err: true},
+
+		// ask for specific version - not available - thus error
+		{DefaultVersions: []schema.GroupVersion{testGroupVersion}, Kind: "InternalObject", APIGroupVersions: []schema.GroupVersion{{Group: testGroup, Version: "v2"}}, Err: true},
+
+		// ask for specific version - available - check ExpectedRESTMappings
+		{
+			DefaultVersions:      []schema.GroupVersion{testGroupVersion},
+			Kind:                 "InternalObject",
+			APIGroupVersions:     []schema.GroupVersion{{Group: testGroup, Version: "v2"}},
+			AddGroupVersionKind:  []schema.GroupVersionKind{schema.GroupVersion{Group: testGroup, Version: "v2"}.WithKind("InternalObject")},
+			ExpectedRESTMappings: []*RESTMapping{{Resource: "internalobjects", GroupVersionKind: schema.GroupVersionKind{Group: testGroup, Version: "v2", Kind: "InternalObject"}}},
+		},
+
+		// ask for specific versions - only one available - check ExpectedRESTMappings
+		{
+			DefaultVersions:      []schema.GroupVersion{testGroupVersion},
+			Kind:                 "InternalObject",
+			APIGroupVersions:     []schema.GroupVersion{{Group: testGroup, Version: "v3"}, {Group: testGroup, Version: "v2"}},
+			AddGroupVersionKind:  []schema.GroupVersionKind{schema.GroupVersion{Group: testGroup, Version: "v2"}.WithKind("InternalObject")},
+			ExpectedRESTMappings: []*RESTMapping{{Resource: "internalobjects", GroupVersionKind: schema.GroupVersionKind{Group: testGroup, Version: "v2", Kind: "InternalObject"}}},
+		},
+
+		// do not ask for specific version - search through default versions - check ExpectedRESTMappings
+		{
+			DefaultVersions:      []schema.GroupVersion{testGroupVersion, {Group: testGroup, Version: "v2"}},
+			Kind:                 "InternalObject",
+			AddGroupVersionKind:  []schema.GroupVersionKind{schema.GroupVersion{Group: testGroup, Version: "v1"}.WithKind("InternalObject"), schema.GroupVersion{Group: testGroup, Version: "v2"}.WithKind("InternalObject")},
+			ExpectedRESTMappings: []*RESTMapping{{Resource: "internalobjects", GroupVersionKind: schema.GroupVersionKind{Group: testGroup, Version: "v1", Kind: "InternalObject"}}, {Resource: "internalobjects", GroupVersionKind: schema.GroupVersionKind{Group: testGroup, Version: "v2", Kind: "InternalObject"}}},
+		},
+	}
+
+	for i, testCase := range testCases {
+		mapper := NewDefaultRESTMapper(testCase.DefaultVersions, fakeInterfaces)
+		for _, gvk := range testCase.AddGroupVersionKind {
+			mapper.Add(gvk, RESTScopeNamespace)
+		}
+
+		preferredVersions := []string{}
+		for _, gv := range testCase.APIGroupVersions {
+			preferredVersions = append(preferredVersions, gv.Version)
+		}
+		gk := schema.GroupKind{Group: testGroup, Kind: testCase.Kind}
+
+		mappings, err := mapper.RESTMappings(gk, preferredVersions...)
+		hasErr := err != nil
+		if hasErr != testCase.Err {
+			t.Errorf("%d: unexpected error behavior %t: %v", i, testCase.Err, err)
+		}
+		if hasErr {
+			continue
+		}
+		if len(mappings) != len(testCase.ExpectedRESTMappings) {
+			t.Errorf("%d: unexpected number = %d of rest mappings was returned, expected = %d", i, len(mappings), len(testCase.ExpectedRESTMappings))
+		}
+		for j, mapping := range mappings {
+			exp := testCase.ExpectedRESTMappings[j]
+			if mapping.Resource != exp.Resource {
+				t.Errorf("%d - %d: unexpected resource: %#v", i, j, mapping)
+			}
+			if mapping.MetadataAccessor == nil || mapping.ObjectConvertor == nil {
+				t.Errorf("%d - %d: missing codec and accessor: %#v", i, j, mapping)
+			}
+			if mapping.GroupVersionKind != exp.GroupVersionKind {
+				t.Errorf("%d - %d: unexpected GroupVersionKind: %#v", i, j, mapping)
+			}
+		}
+	}
+}
+
 func TestRESTMapperReportsErrorOnBadVersion(t *testing.T) {
 	expectedGroupVersion1 := schema.GroupVersion{Group: "tgroup", Version: "test1"}
 	expectedGroupVersion2 := schema.GroupVersion{Group: "tgroup", Version: "test2"}

pkg/client/typed/discovery/restmapper.go

@@ -265,15 +265,15 @@ func (d *DeferredDiscoveryRESTMapper) RESTMapping(gk schema.GroupKind, versions
 // RESTMappings returns the RESTMappings for the provided group kind
 // in a rough internal preferred order. If no kind is found, it will
 // return a NoResourceMatchError.
-func (d *DeferredDiscoveryRESTMapper) RESTMappings(gk schema.GroupKind) (ms []*meta.RESTMapping, err error) {
+func (d *DeferredDiscoveryRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) (ms []*meta.RESTMapping, err error) {
 	del, err := d.getDelegate()
 	if err != nil {
 		return nil, err
 	}
-	ms, err = del.RESTMappings(gk)
+	ms, err = del.RESTMappings(gk, versions...)
 	if len(ms) == 0 && !d.cl.Fresh() {
 		d.Reset()
-		ms, err = d.RESTMappings(gk)
+		ms, err = d.RESTMappings(gk, versions...)
 	}
 	return
 }

pkg/kubectl/cmd/util/shortcut_restmapper.go

@@ -101,8 +101,8 @@ func (e ShortcutExpander) RESTMapping(gk schema.GroupKind, versions ...string) (
 	return e.RESTMapper.RESTMapping(gk, versions...)
 }
 
-func (e ShortcutExpander) RESTMappings(gk schema.GroupKind) ([]*meta.RESTMapping, error) {
+func (e ShortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
-	return e.RESTMapper.RESTMappings(gk)
+	return e.RESTMapper.RESTMappings(gk, versions...)
 }
 
 // userResources are the resource names that apply to the primary, user facing resources used by

pkg/registry/extensions/thirdpartyresourcedata/codec.go

@@ -152,7 +152,7 @@ func (t *thirdPartyResourceDataMapper) RESTMapping(gk schema.GroupKind, versions
 	return mapping, nil
 }
 
-func (t *thirdPartyResourceDataMapper) RESTMappings(gk schema.GroupKind) ([]*meta.RESTMapping, error) {
+func (t *thirdPartyResourceDataMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
 	if gk.Group != t.group {
 		return nil, fmt.Errorf("unknown group %q expected %s", gk.Group, t.group)
 	}
@@ -163,7 +163,7 @@ func (t *thirdPartyResourceDataMapper) RESTMappings(gk schema.GroupKind) ([]*met
 	// TODO figure out why we're doing this rewriting
 	extensionGK := schema.GroupKind{Group: extensions.GroupName, Kind: "ThirdPartyResourceData"}
 
-	mappings, err := t.mapper.RESTMappings(extensionGK)
+	mappings, err := t.mapper.RESTMappings(extensionGK, versions...)
 	if err != nil {
 		return nil, err
 	}