Merge pull request #28512 from smarterclayton/multirestmapper

Automatic merge from submit-queue

Implement a RESTMappings method

With the introduction of batch/v1 and batch/v2alpha1, any
MultiRESTMapper that has per groupversion mappers (as a naive discovery
client would create) would end up being unable to call RESTMapping() on
batch.Jobs. As we finish up discovery we will need to be able to choose
prioritized RESTMappings based on the service discovery doc.

This change implements RESTMappings(groupversion) which returns all
possible RESTMappings for that kind.  That allows a higher level call to
prioritize the returned mappings by server or client preferred version.

@deads2k
This commit is contained in:
k8s-merge-robot 2016-07-15 05:13:55 -07:00 committed by GitHub
commit 423106ccee
11 changed files with 390 additions and 36 deletions

View File

@ -38,19 +38,40 @@ func (e *AmbiguousResourceError) Error() string {
return fmt.Sprintf("%v matches multiple kinds %v", e.PartialResource, e.MatchingKinds)
case len(e.MatchingResources) > 0:
return fmt.Sprintf("%v matches multiple resources %v", e.PartialResource, e.MatchingResources)
}
return fmt.Sprintf("%v matches multiple resources or kinds", e.PartialResource)
}
func IsAmbiguousResourceError(err error) bool {
// AmbiguousKindError is returned if the RESTMapper finds multiple matches for a kind
type AmbiguousKindError struct {
PartialKind unversioned.GroupVersionKind
MatchingResources []unversioned.GroupVersionResource
MatchingKinds []unversioned.GroupVersionKind
}
func (e *AmbiguousKindError) Error() string {
switch {
case len(e.MatchingKinds) > 0 && len(e.MatchingResources) > 0:
return fmt.Sprintf("%v matches multiple resources %v and kinds %v", e.PartialKind, e.MatchingResources, e.MatchingKinds)
case len(e.MatchingKinds) > 0:
return fmt.Sprintf("%v matches multiple kinds %v", e.PartialKind, e.MatchingKinds)
case len(e.MatchingResources) > 0:
return fmt.Sprintf("%v matches multiple resources %v", e.PartialKind, e.MatchingResources)
}
return fmt.Sprintf("%v matches multiple resources or kinds", e.PartialKind)
}
func IsAmbiguousError(err error) bool {
if err == nil {
return false
}
_, ok := err.(*AmbiguousResourceError)
return ok
switch err.(type) {
case *AmbiguousResourceError, *AmbiguousKindError:
return true
default:
return false
}
}
// NoResourceMatchError is returned if the RESTMapper can't find any match for a resource
@ -62,11 +83,23 @@ func (e *NoResourceMatchError) Error() string {
return fmt.Sprintf("no matches for %v", e.PartialResource)
}
func IsNoResourceMatchError(err error) bool {
// NoKindMatchError is returned if the RESTMapper can't find any match for a kind
type NoKindMatchError struct {
PartialKind unversioned.GroupVersionKind
}
func (e *NoKindMatchError) Error() string {
return fmt.Sprintf("no matches for %v", e.PartialKind)
}
func IsNoMatchError(err error) bool {
if err == nil {
return false
}
_, ok := err.(*NoResourceMatchError)
return ok
switch err.(type) {
case *NoResourceMatchError, *NoKindMatchError:
return true
default:
return false
}
}

View File

@ -173,7 +173,10 @@ type RESTMapper interface {
// ResourcesFor takes a partial resource and returns back the list of potential resource in priority order
ResourcesFor(input unversioned.GroupVersionResource) ([]unversioned.GroupVersionResource, error)
// RESTMapping identifies a preferred resource mapping for the provided group kind.
RESTMapping(gk unversioned.GroupKind, versions ...string) (*RESTMapping, error)
// RESTMappings returns all resource mappings for the provided group kind.
RESTMappings(gk unversioned.GroupKind) ([]*RESTMapping, error)
AliasesForResource(resource string) ([]string, bool)
ResourceSingularizer(resource string) (singular string, err error)

View File

@ -56,7 +56,7 @@ func (m MultiRESTMapper) ResourcesFor(resource unversioned.GroupVersionResource)
for _, t := range m {
gvrs, err := t.ResourcesFor(resource)
// ignore "no match" errors, but any other error percolates back up
if IsNoResourceMatchError(err) {
if IsNoMatchError(err) {
continue
}
if err != nil {
@ -91,7 +91,7 @@ func (m MultiRESTMapper) KindsFor(resource unversioned.GroupVersionResource) (gv
for _, t := range m {
gvks, err := t.KindsFor(resource)
// ignore "no match" errors, but any other error percolates back up
if IsNoResourceMatchError(err) {
if IsNoMatchError(err) {
continue
}
if err != nil {
@ -155,7 +155,7 @@ func (m MultiRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...strin
for _, t := range m {
currMapping, err := t.RESTMapping(gk, versions...)
// ignore "no match" errors, but any other error percolates back up
if IsNoResourceMatchError(err) {
if IsNoMatchError(err) {
continue
}
if err != nil {
@ -171,12 +171,43 @@ func (m MultiRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...strin
return allMappings[0], nil
}
if len(allMappings) > 1 {
return nil, fmt.Errorf("multiple matches found for %v in %v", gk, versions)
var kinds []unversioned.GroupVersionKind
for _, m := range allMappings {
kinds = append(kinds, m.GroupVersionKind)
}
return nil, &AmbiguousKindError{PartialKind: gk.WithVersion(""), MatchingKinds: kinds}
}
if len(errors) > 0 {
return nil, utilerrors.NewAggregate(errors)
}
return nil, fmt.Errorf("no match found for %v in %v", gk, versions)
return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")}
}
// RESTMappings returns all possible RESTMappings for the provided group kind, or an error
// if the type is not recognized.
func (m MultiRESTMapper) RESTMappings(gk unversioned.GroupKind) ([]*RESTMapping, error) {
var allMappings []*RESTMapping
var errors []error
for _, t := range m {
currMappings, err := t.RESTMappings(gk)
// ignore "no match" errors, but any other error percolates back up
if IsNoMatchError(err) {
continue
}
if err != nil {
errors = append(errors, err)
continue
}
allMappings = append(allMappings, currMappings...)
}
if len(errors) > 0 {
return nil, utilerrors.NewAggregate(errors)
}
if len(allMappings) == 0 {
return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")}
}
return allMappings, nil
}
// AliasesForResource finds the first alias response for the provided mappers.

View File

@ -256,11 +256,68 @@ func TestMultiRESTMapperKindFor(t *testing.T) {
}
}
func TestMultiRESTMapperRESTMappings(t *testing.T) {
mapping1, mapping2 := &RESTMapping{}, &RESTMapping{}
tcs := []struct {
name string
mapper MultiRESTMapper
input unversioned.GroupKind
result []*RESTMapping
err error
}{
{
name: "empty",
mapper: MultiRESTMapper{},
input: unversioned.GroupKind{Kind: "Foo"},
result: nil,
err: &NoKindMatchError{PartialKind: unversioned.GroupVersionKind{Kind: "Foo"}},
},
{
name: "ignore not found",
mapper: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{PartialKind: unversioned.GroupVersionKind{Kind: "IGNORE_THIS"}}}},
input: unversioned.GroupKind{Kind: "Foo"},
result: nil,
err: &NoKindMatchError{PartialKind: unversioned.GroupVersionKind{Kind: "Foo"}},
},
{
name: "accept first failure",
mapper: MultiRESTMapper{fixedRESTMapper{err: errors.New("fail on this")}, fixedRESTMapper{mappings: []*RESTMapping{mapping1}}},
input: unversioned.GroupKind{Kind: "Foo"},
result: nil,
err: errors.New("fail on this"),
},
{
name: "return both",
mapper: MultiRESTMapper{fixedRESTMapper{mappings: []*RESTMapping{mapping1}}, fixedRESTMapper{mappings: []*RESTMapping{mapping2}}},
input: unversioned.GroupKind{Kind: "Foo"},
result: []*RESTMapping{mapping1, mapping2},
},
}
for _, tc := range tcs {
actualResult, actualErr := tc.mapper.RESTMappings(tc.input)
if e, a := tc.result, actualResult; !reflect.DeepEqual(e, a) {
t.Errorf("%s: expected %v, got %v", tc.name, e, a)
}
switch {
case tc.err == nil && actualErr == nil:
case tc.err == nil:
t.Errorf("%s: unexpected error: %v", tc.name, actualErr)
case actualErr == nil:
t.Errorf("%s: expected error: %v got nil", tc.name, tc.err)
case tc.err.Error() != actualErr.Error():
t.Errorf("%s: expected %v, got %v", tc.name, tc.err, actualErr)
}
}
}
type fixedRESTMapper struct {
resourcesFor []unversioned.GroupVersionResource
kindsFor []unversioned.GroupVersionKind
resourceFor unversioned.GroupVersionResource
kindFor unversioned.GroupVersionKind
mappings []*RESTMapping
err error
}
@ -289,6 +346,10 @@ func (m fixedRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...strin
return nil, m.err
}
func (m fixedRESTMapper) RESTMappings(gk unversioned.GroupKind) (mappings []*RESTMapping, err error) {
return m.mappings, m.err
}
func (m fixedRESTMapper) AliasesForResource(alias string) (aliases []string, ok bool) {
return nil, false
}

View File

@ -153,7 +153,60 @@ func kindMatches(pattern unversioned.GroupVersionKind, kind unversioned.GroupVer
}
func (m PriorityRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...string) (mapping *RESTMapping, err error) {
return m.Delegate.RESTMapping(gk, versions...)
mappings, err := m.Delegate.RESTMappings(gk)
if err != nil {
return nil, err
}
// any versions the user provides take priority
priorities := m.KindPriority
if len(versions) > 0 {
priorities = make([]unversioned.GroupVersionKind, 0, len(m.KindPriority)+len(versions))
for _, version := range versions {
gv, err := unversioned.ParseGroupVersion(version)
if err != nil {
return nil, err
}
priorities = append(priorities, gv.WithKind(AnyKind))
}
priorities = append(priorities, m.KindPriority...)
}
remaining := append([]*RESTMapping{}, mappings...)
for _, pattern := range priorities {
var matching []*RESTMapping
for _, m := range remaining {
if kindMatches(pattern, m.GroupVersionKind) {
matching = append(matching, m)
}
}
switch len(matching) {
case 0:
// if you have no matches, then nothing matched this pattern just move to the next
continue
case 1:
// one match, return
return matching[0], nil
default:
// more than one match, use the matched hits as the list moving to the next pattern.
// this way you can have a series of selection criteria
remaining = matching
}
}
if len(remaining) == 1 {
return remaining[0], nil
}
var kinds []unversioned.GroupVersionKind
for _, m := range mappings {
kinds = append(kinds, m.GroupVersionKind)
}
return nil, &AmbiguousKindError{PartialKind: gk.WithVersion(""), MatchingKinds: kinds}
}
func (m PriorityRESTMapper) RESTMappings(gk unversioned.GroupKind) ([]*RESTMapping, error) {
return m.Delegate.RESTMappings(gk)
}
func (m PriorityRESTMapper) AliasesForResource(alias string) (aliases []string, ok bool) {

View File

@ -17,6 +17,8 @@ limitations under the License.
package meta
import (
"errors"
"reflect"
"strings"
"testing"
@ -204,3 +206,104 @@ func TestPriorityRESTMapperKindForErrorHandling(t *testing.T) {
}
}
}
func TestPriorityRESTMapperRESTMapping(t *testing.T) {
mapping1 := &RESTMapping{
GroupVersionKind: unversioned.GroupVersionKind{Kind: "Foo", Version: "v1alpha1"},
}
mapping2 := &RESTMapping{
GroupVersionKind: unversioned.GroupVersionKind{Kind: "Foo", Version: "v1"},
}
mapping3 := &RESTMapping{
GroupVersionKind: unversioned.GroupVersionKind{Group: "other", Kind: "Foo", Version: "v1"},
}
allMappers := MultiRESTMapper{
fixedRESTMapper{mappings: []*RESTMapping{mapping1}},
fixedRESTMapper{mappings: []*RESTMapping{mapping2}},
fixedRESTMapper{mappings: []*RESTMapping{mapping3}},
}
tcs := []struct {
name string
mapper PriorityRESTMapper
input unversioned.GroupKind
result *RESTMapping
err error
}{
{
name: "empty",
mapper: PriorityRESTMapper{Delegate: MultiRESTMapper{}},
input: unversioned.GroupKind{Kind: "Foo"},
err: &NoKindMatchError{PartialKind: unversioned.GroupVersionKind{Kind: "Foo"}},
},
{
name: "ignore not found",
mapper: PriorityRESTMapper{Delegate: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{PartialKind: unversioned.GroupVersionKind{Kind: "IGNORE_THIS"}}}}},
input: unversioned.GroupKind{Kind: "Foo"},
err: &NoKindMatchError{PartialKind: unversioned.GroupVersionKind{Kind: "Foo"}},
},
{
name: "accept first failure",
mapper: PriorityRESTMapper{Delegate: MultiRESTMapper{fixedRESTMapper{err: errors.New("fail on this")}, fixedRESTMapper{mappings: []*RESTMapping{mapping1}}}},
input: unversioned.GroupKind{Kind: "Foo"},
err: errors.New("fail on this"),
},
{
name: "return error for ambiguous",
mapper: PriorityRESTMapper{
Delegate: allMappers,
},
input: unversioned.GroupKind{Kind: "Foo"},
err: &AmbiguousKindError{
PartialKind: unversioned.GroupVersionKind{Kind: "Foo"},
MatchingKinds: []unversioned.GroupVersionKind{
{Kind: "Foo", Version: "v1alpha1"},
{Kind: "Foo", Version: "v1"},
{Group: "other", Kind: "Foo", Version: "v1"},
},
},
},
{
name: "accept only item",
mapper: PriorityRESTMapper{
Delegate: fixedRESTMapper{mappings: []*RESTMapping{mapping1}},
},
input: unversioned.GroupKind{Kind: "Foo"},
result: mapping1,
},
{
name: "return single priority",
mapper: PriorityRESTMapper{
Delegate: allMappers,
KindPriority: []unversioned.GroupVersionKind{{Version: "v1", Kind: AnyKind}, {Version: "v1alpha1", Kind: AnyKind}},
},
input: unversioned.GroupKind{Kind: "Foo"},
result: mapping2,
},
{
name: "return out of group match",
mapper: PriorityRESTMapper{
Delegate: allMappers,
KindPriority: []unversioned.GroupVersionKind{{Group: AnyGroup, Version: "v1", Kind: AnyKind}, {Group: "other", Version: AnyVersion, Kind: AnyKind}},
},
input: unversioned.GroupKind{Kind: "Foo"},
result: mapping3,
},
}
for _, tc := range tcs {
actualResult, actualErr := tc.mapper.RESTMapping(tc.input)
if e, a := tc.result, actualResult; !reflect.DeepEqual(e, a) {
t.Errorf("%s: expected %v, got %v", tc.name, e, a)
}
switch {
case tc.err == nil && actualErr == nil:
case tc.err == nil:
t.Errorf("%s: unexpected error: %v", tc.name, actualErr)
case actualErr == nil:
t.Errorf("%s: expected error: %v got nil", tc.name, tc.err)
case tc.err.Error() != actualErr.Error():
t.Errorf("%s: expected %v, got %v", tc.name, tc.err, actualErr)
}
}
}

View File

@ -431,6 +431,7 @@ 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 unversioned.GroupKind, versions ...string) (*RESTMapping, error) {
// Pick an appropriate version
var gvk *unversioned.GroupVersionKind
@ -462,7 +463,7 @@ func (m *DefaultRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...st
}
}
if gvk == nil {
return nil, fmt.Errorf("no kind named %q is registered in versions %q", gk, versions)
return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")}
}
// Ensure we have a REST mapping
@ -503,6 +504,49 @@ func (m *DefaultRESTMapper) RESTMapping(gk unversioned.GroupKind, versions ...st
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 unversioned.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
}
// 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(), gvk.Kind)
}
interfaces, err := m.interfacesFunc(gvk.GroupVersion())
if err != nil {
return nil, fmt.Errorf("the provided version %q has no relevant versions", gvk.GroupVersion().String())
}
mappings = append(mappings, &RESTMapping{
Resource: gvr.Resource,
GroupVersionKind: gvk,
Scope: scope,
ObjectConvertor: interfaces.ObjectConvertor,
MetadataAccessor: interfaces.MetadataAccessor,
})
}
if len(mappings) == 0 {
return nil, &NoResourceMatchError{PartialResource: unversioned.GroupVersionResource{Group: gk.Group, Resource: gk.Kind}}
}
return mappings, nil
}
// AddResourceAlias maps aliases to resources
func (m *DefaultRESTMapper) AddResourceAlias(alias string, resources ...string) {
if len(resources) == 0 {

View File

@ -318,18 +318,21 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
outputRESTMapper := kubectl.OutputVersionMapper{RESTMapper: mapper, OutputVersions: []unversioned.GroupVersion{cmdApiVersion}}
priorityRESTMapper := meta.PriorityRESTMapper{
Delegate: outputRESTMapper,
ResourcePriority: []unversioned.GroupVersionResource{
{Group: api.GroupName, Version: meta.AnyVersion, Resource: meta.AnyResource},
{Group: autoscaling.GroupName, Version: meta.AnyVersion, Resource: meta.AnyResource},
{Group: extensions.GroupName, Version: meta.AnyVersion, Resource: meta.AnyResource},
{Group: federation.GroupName, Version: meta.AnyVersion, Resource: meta.AnyResource},
},
KindPriority: []unversioned.GroupVersionKind{
{Group: api.GroupName, Version: meta.AnyVersion, Kind: meta.AnyKind},
{Group: autoscaling.GroupName, Version: meta.AnyVersion, Kind: meta.AnyKind},
{Group: extensions.GroupName, Version: meta.AnyVersion, Kind: meta.AnyKind},
{Group: federation.GroupName, Version: meta.AnyVersion, Kind: meta.AnyKind},
},
}
// TODO: this should come from registered versions
groups := []string{api.GroupName, autoscaling.GroupName, extensions.GroupName, federation.GroupName, batch.GroupName}
// set a preferred version
for _, group := range groups {
gvs := registered.EnabledVersionsForGroup(group)
if len(gvs) == 0 {
continue
}
priorityRESTMapper.ResourcePriority = append(priorityRESTMapper.ResourcePriority, unversioned.GroupVersionResource{Group: group, Version: gvs[0].Version, Resource: meta.AnyResource})
priorityRESTMapper.KindPriority = append(priorityRESTMapper.KindPriority, unversioned.GroupVersionKind{Group: group, Version: gvs[0].Version, Kind: meta.AnyKind})
}
for _, group := range groups {
priorityRESTMapper.ResourcePriority = append(priorityRESTMapper.ResourcePriority, unversioned.GroupVersionResource{Group: group, Version: meta.AnyVersion, Resource: meta.AnyResource})
priorityRESTMapper.KindPriority = append(priorityRESTMapper.KindPriority, unversioned.GroupVersionKind{Group: group, Version: meta.AnyVersion, Kind: meta.AnyKind})
}
return priorityRESTMapper, api.Scheme
},

View File

@ -125,9 +125,7 @@ func checkErr(pref string, err error, handleErr func(string)) {
handleErr(MultilineError(prefix, errs))
}
if meta.IsNoResourceMatchError(err) {
noMatch := err.(*meta.NoResourceMatchError)
if noMatch, ok := err.(*meta.NoResourceMatchError); ok {
switch {
case len(noMatch.PartialResource.Group) > 0 && len(noMatch.PartialResource.Version) > 0:
handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in group %q and version %q", pref, noMatch.PartialResource.Resource, noMatch.PartialResource.Group, noMatch.PartialResource.Version))

View File

@ -127,14 +127,18 @@ func (e ShortcutExpander) ResourceFor(resource unversioned.GroupVersionResource)
return e.RESTMapper.ResourceFor(expandResourceShortcut(resource))
}
func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) {
return e.RESTMapper.ResourceSingularizer(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource)
}
func (e ShortcutExpander) RESTMapping(gk unversioned.GroupKind, versions ...string) (*meta.RESTMapping, error) {
return e.RESTMapper.RESTMapping(gk, versions...)
}
func (e ShortcutExpander) RESTMappings(gk unversioned.GroupKind) ([]*meta.RESTMapping, error) {
return e.RESTMapper.RESTMappings(gk)
}
func (e ShortcutExpander) ResourceSingularizer(resource string) (string, error) {
return e.RESTMapper.ResourceSingularizer(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource)
}
func (e ShortcutExpander) AliasesForResource(resource string) ([]string, bool) {
return e.RESTMapper.AliasesForResource(expandResourceShortcut(unversioned.GroupVersionResource{Resource: resource}).Resource)
}

View File

@ -152,6 +152,27 @@ func (t *thirdPartyResourceDataMapper) RESTMapping(gk unversioned.GroupKind, ver
return mapping, nil
}
func (t *thirdPartyResourceDataMapper) RESTMappings(gk unversioned.GroupKind) ([]*meta.RESTMapping, error) {
if gk.Group != t.group {
return nil, fmt.Errorf("unknown group %q expected %s", gk.Group, t.group)
}
if gk.Kind != "ThirdPartyResourceData" {
return nil, fmt.Errorf("unknown kind %s expected %s", gk.Kind, t.kind)
}
// TODO figure out why we're doing this rewriting
extensionGK := unversioned.GroupKind{Group: extensions.GroupName, Kind: "ThirdPartyResourceData"}
mappings, err := t.mapper.RESTMappings(extensionGK)
if err != nil {
return nil, err
}
for _, m := range mappings {
m.ObjectConvertor = &thirdPartyObjectConverter{m.ObjectConvertor}
}
return mappings, nil
}
func (t *thirdPartyResourceDataMapper) AliasesForResource(resource string) ([]string, bool) {
return t.mapper.AliasesForResource(resource)
}