Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

update RESTMapper API #17335

Merged
merged 2 commits into from
Nov 25, 2015
Merged

update RESTMapper API #17335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c4fb5b
make RESTMapper.KindFor
deads2k Nov 16, 2015
8679925
update RESTMapping API to be properly typed
deads2k Nov 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions pkg/api/install/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/api/meta/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
110 changes: 37 additions & 73 deletions pkg/api/meta/restmapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down Expand Up @@ -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
}

Expand All @@ -317,34 +293,22 @@ 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
}
}
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
}
Expand Down
41 changes: 19 additions & 22 deletions pkg/api/meta/restmapper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand All @@ -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)
}
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -304,21 +298,23 @@ 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)
}
if mapping.Resource != "otherobjects" || mapping.GroupVersionKind.GroupVersion() != expectedGroupVersion2 {
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)
}
Expand All @@ -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)
}
Expand All @@ -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")
}
Expand Down