diff --git a/pkg/api/meta/interfaces.go b/pkg/api/meta/interfaces.go index 0ba53cbb247..275f023594a 100644 --- a/pkg/api/meta/interfaces.go +++ b/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(gk schema.GroupKind) ([]*RESTMapping, error) + // RESTMappings returns all resource mappings for the provided group kind if no + // 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) diff --git a/pkg/api/meta/multirestmapper.go b/pkg/api/meta/multirestmapper.go index b47a58cc239..816eb321b65 100644 --- a/pkg/api/meta/multirestmapper.go +++ b/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 diff --git a/pkg/api/meta/multirestmapper_test.go b/pkg/api/meta/multirestmapper_test.go index a6cfb1507b0..6d306c3760c 100644 --- a/pkg/api/meta/multirestmapper_test.go +++ b/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 } diff --git a/pkg/api/meta/priority.go b/pkg/api/meta/priority.go index 9dc603a1a1c..f99273b6499 100644 --- a/pkg/api/meta/priority.go +++ b/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) { - return m.Delegate.RESTMappings(gk) +func (m PriorityRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*RESTMapping, error) { + return m.Delegate.RESTMappings(gk, versions...) } func (m PriorityRESTMapper) AliasesForResource(alias string) (aliases []string, ok bool) { diff --git a/pkg/api/meta/restmapper.go b/pkg/api/meta/restmapper.go index c31224afa67..eb2f6382b06 100644 --- a/pkg/api/meta/restmapper.go +++ b/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 - var gvk *schema.GroupVersionKind + mappings, err := m.RESTMappings(gk, versions...) + 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 } - - currGVK := gk.WithVersion(gv.Version) - if _, ok := m.kindToPluralResource[currGVK]; ok { - gvk = &currGVK - break - } + potentialGVK = append(potentialGVK, gk.WithVersion(gv.Version)) } } - if gvk == nil { + + if len(potentialGVK) == 0 { return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")} } - // Ensure we have a REST mapping - resource, ok := m.kindToPluralResource[*gvk] - if !ok { - 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] + for _, gvk := range potentialGVK { + //Ensure we have a REST mapping + res, 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, diff --git a/pkg/api/meta/restmapper_test.go b/pkg/api/meta/restmapper_test.go index c278c7eec72..6ab9fb642f2 100644 --- a/pkg/api/meta/restmapper_test.go +++ b/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"} diff --git a/pkg/client/typed/discovery/restmapper.go b/pkg/client/typed/discovery/restmapper.go index 53866584f1c..a1889f6aeab 100644 --- a/pkg/client/typed/discovery/restmapper.go +++ b/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 } diff --git a/pkg/kubectl/cmd/util/shortcut_restmapper.go b/pkg/kubectl/cmd/util/shortcut_restmapper.go index c85ac277340..357f69862fc 100644 --- a/pkg/kubectl/cmd/util/shortcut_restmapper.go +++ b/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) { - return e.RESTMapper.RESTMappings(gk) +func (e ShortcutExpander) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { + return e.RESTMapper.RESTMappings(gk, versions...) } // userResources are the resource names that apply to the primary, user facing resources used by diff --git a/pkg/registry/extensions/thirdpartyresourcedata/codec.go b/pkg/registry/extensions/thirdpartyresourcedata/codec.go index 936c45818ff..9dabd225b47 100644 --- a/pkg/registry/extensions/thirdpartyresourcedata/codec.go +++ b/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 }