RESTMapper should take into account multiple versions

When a CLI command `kubectl get rc --api-version=v1beta3` is called,
the API resource name should match v1beta3, not whatever the default
RESTMapper version is.  This allows the correct resource name to be
returned ("replicationcontrollers", instead of "replicationControllers").
This commit is contained in:
Clayton Coleman 2014-12-12 14:05:27 -05:00
parent 9b6aec5e22
commit 8bef68d475
9 changed files with 76 additions and 38 deletions

View File

@ -228,7 +228,7 @@ func TestRESTMapper(t *testing.T) {
}
for _, version := range Versions {
mapping, err := RESTMapper.RESTMapping(version, "ReplicationController")
mapping, err := RESTMapper.RESTMapping("ReplicationController", version)
if err != nil {
t.Errorf("unexpected error: %v", err)
}

View File

@ -106,5 +106,5 @@ type RESTMapping struct {
// consumers of Kubernetes compatible REST APIs as defined in docs/api-conventions.md.
type RESTMapper interface {
VersionAndKindForResource(resource string) (defaultVersion, kind string, err error)
RESTMapping(version, kind string) (*RESTMapping, error)
RESTMapping(kind string, versions ...string) (*RESTMapping, error)
}

View File

@ -122,21 +122,34 @@ func (m *DefaultRESTMapper) VersionAndKindForResource(resource string) (defaultV
}
// RESTMapping returns a struct representing the resource path and conversion interfaces a
// RESTClient should use to operate on the provided version and kind. If a version is not
// provided, the search order provided to DefaultRESTMapper will be used to resolve which
// RESTClient should use to operate on the provided 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.
func (m *DefaultRESTMapper) RESTMapping(version, kind string) (*RESTMapping, error) {
// Default to a version with this kind
if len(version) == 0 {
func (m *DefaultRESTMapper) RESTMapping(kind string, versions ...string) (*RESTMapping, error) {
// Pick an appropriate version
var version string
hadVersion := false
for _, v := range versions {
if len(v) == 0 {
continue
}
hadVersion = true
if _, ok := m.reverse[typeMeta{APIVersion: v, Kind: kind}]; ok {
version = v
break
}
}
// Use the default preferred versions
if !hadVersion && len(version) == 0 {
for _, v := range m.versions {
if _, ok := m.reverse[typeMeta{APIVersion: v, Kind: kind}]; ok {
version = v
break
}
}
if len(version) == 0 {
return nil, fmt.Errorf("no object named %q is registered", kind)
}
}
if len(version) == 0 {
return nil, fmt.Errorf("no object named %q is registered", kind)
}
// Ensure we have a REST mapping

View File

@ -122,31 +122,39 @@ func TestKindToResource(t *testing.T) {
func TestRESTMapperRESTMapping(t *testing.T) {
testCases := []struct {
Kind, APIVersion string
MixedCase bool
Kind string
APIVersions []string
MixedCase bool
DefaultVersions []string
Resource string
Version string
Err bool
}{
{Kind: "Unknown", APIVersion: "", Err: true},
{Kind: "Unknown", Err: true},
{Kind: "InternalObject", Err: true},
{Kind: "InternalObject", APIVersion: "test", Resource: "internalobjects"},
{Kind: "InternalObject", APIVersion: "test", Resource: "internalobjects"},
{Kind: "InternalObject", APIVersion: "", Resource: "internalobjects", Version: "test"},
{DefaultVersions: []string{"test"}, Kind: "Unknown", Err: true},
{Kind: "InternalObject", APIVersion: "test", Resource: "internalobjects"},
{Kind: "InternalObject", APIVersion: "test", MixedCase: true, Resource: "internalObjects"},
{DefaultVersions: []string{"test"}, Kind: "InternalObject", APIVersions: []string{"test"}, Resource: "internalobjects"},
{DefaultVersions: []string{"test"}, Kind: "InternalObject", APIVersions: []string{"test"}, Resource: "internalobjects"},
{DefaultVersions: []string{"test"}, Kind: "InternalObject", APIVersions: []string{"test"}, Resource: "internalobjects"},
{DefaultVersions: []string{"test"}, Kind: "InternalObject", APIVersions: []string{}, Resource: "internalobjects", Version: "test"},
{DefaultVersions: []string{"test"}, Kind: "InternalObject", APIVersions: []string{"test"}, Resource: "internalobjects"},
{DefaultVersions: []string{"test"}, Kind: "InternalObject", APIVersions: []string{"test"}, MixedCase: true, Resource: "internalObjects"},
// TODO: add test for a resource that exists in one version but not another
}
for i, testCase := range testCases {
mapper := NewDefaultRESTMapper([]string{"test"}, fakeInterfaces)
mapper := NewDefaultRESTMapper(testCase.DefaultVersions, fakeInterfaces)
scheme := runtime.NewScheme()
scheme.AddKnownTypes("test", &InternalObject{})
mapper.Add(scheme, testCase.MixedCase, "test")
mapping, err := mapper.RESTMapping(testCase.APIVersion, testCase.Kind)
mapping, err := mapper.RESTMapping(testCase.Kind, testCase.APIVersions...)
hasErr := err != nil
if hasErr != testCase.Err {
t.Errorf("%d: unexpected error behavior %t: %v", i, testCase.Err, err)
@ -159,7 +167,7 @@ func TestRESTMapperRESTMapping(t *testing.T) {
}
version := testCase.Version
if version == "" {
version = testCase.APIVersion
version = testCase.APIVersions[0]
}
if mapping.APIVersion != version {
t.Errorf("%d: unexpected version: %#v", i, mapping)
@ -179,37 +187,51 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
mapper.Add(scheme, false, "test1", "test2")
// pick default matching object kind based on search order
mapping, err := mapper.RESTMapping("", "OtherObject")
mapping, err := mapper.RESTMapping("OtherObject")
if err != nil {
t.Errorf("unexpected error: %v", err)
t.Fatalf("unexpected error: %v", err)
}
if mapping.Resource != "otherobjects" || mapping.APIVersion != "test2" {
t.Errorf("unexpected mapping: %#v", mapping)
}
mapping, err = mapper.RESTMapping("", "InternalObject")
mapping, err = mapper.RESTMapping("InternalObject")
if err != nil {
t.Errorf("unexpected error: %v", err)
t.Fatalf("unexpected error: %v", err)
}
if mapping.Resource != "internalobjects" || mapping.APIVersion != "test1" {
t.Errorf("unexpected mapping: %#v", mapping)
}
// mismatch of version
mapping, err = mapper.RESTMapping("test2", "InternalObject")
mapping, err = mapper.RESTMapping("InternalObject", "test2")
if err == nil {
t.Errorf("unexpected non-error")
}
mapping, err = mapper.RESTMapping("test1", "OtherObject")
mapping, err = mapper.RESTMapping("OtherObject", "test1")
if err == nil {
t.Errorf("unexpected non-error")
}
// not in the search versions
mapping, err = mapper.RESTMapping("test3", "OtherObject")
mapping, err = mapper.RESTMapping("OtherObject", "test3")
if err == nil {
t.Errorf("unexpected non-error")
}
// explicit search order
mapping, err = mapper.RESTMapping("OtherObject", "test3", "test1")
if err == nil {
t.Errorf("unexpected non-error")
}
mapping, err = mapper.RESTMapping("OtherObject", "test3", "test2")
if err != nil {
t.Fatalf("unexpected non-error")
}
if mapping.Resource != "otherobjects" || mapping.APIVersion != "test2" {
t.Errorf("unexpected mapping: %#v", mapping)
}
}
func TestRESTMapperReportsErrorOnBadVersion(t *testing.T) {
@ -218,7 +240,7 @@ func TestRESTMapperReportsErrorOnBadVersion(t *testing.T) {
scheme.AddKnownTypes("test1", &InternalObject{})
mapper.Add(scheme, false, "test1")
_, err := mapper.RESTMapping("test1", "InternalObject")
_, err := mapper.RESTMapping("InternalObject", "test1")
if err == nil {
t.Errorf("unexpected non-error")
}

View File

@ -45,7 +45,7 @@ func CreateObjects(typer runtime.ObjectTyper, mapper meta.RESTMapper, clientFor
continue
}
mapping, err := mapper.RESTMapping(version, kind)
mapping, err := mapper.RESTMapping(kind, version)
if err != nil {
allErrors = append(allErrors, fmt.Errorf("Config.item[%d] mapping: %v", i, err))
continue

View File

@ -44,7 +44,7 @@ func DataToObjects(m meta.RESTMapper, t runtime.ObjectTyper, data []byte) (resul
continue
}
mapping, err := m.RESTMapping(version, kind)
mapping, err := m.RESTMapping(kind, version)
if err != nil {
errors = append(errors, fmt.Errorf("item[%d] mapping: %v", i, err))
continue

View File

@ -37,6 +37,7 @@ func TestDescribeUnknownSchemaObject(t *testing.T) {
buf := bytes.NewBuffer([]byte{})
cmd := f.NewCmdDescribe(buf)
cmd.Flags().String("api-version", "default", "")
cmd.Flags().String("namespace", "test", "")
cmd.Run(cmd, []string{"type", "foo"})

View File

@ -37,6 +37,7 @@ func TestGetUnknownSchemaObject(t *testing.T) {
buf := bytes.NewBuffer([]byte{})
cmd := f.NewCmdGet(buf)
cmd.Flags().String("api-version", "default", "")
cmd.Flags().String("namespace", "test", "")
cmd.Run(cmd, []string{"type", "foo"})

View File

@ -201,13 +201,13 @@ func ResourceFromArgsOrFile(cmd *cobra.Command, args []string, filename string,
usageError(cmd, "Must specify filename or command line params")
}
version, kind, err := mapper.VersionAndKindForResource(resource)
defaultVersion, kind, err := mapper.VersionAndKindForResource(resource)
if err != nil {
// The error returned by mapper is "no resource defined", which is a usage error
usageError(cmd, err.Error())
}
mapping, err = mapper.RESTMapping(version, kind)
version := GetFlagString(cmd, "api-version")
mapping, err = mapper.RESTMapping(kind, version, defaultVersion)
checkErr(err)
return
}
@ -242,7 +242,7 @@ func ResourceFromArgs(cmd *cobra.Command, args []string, mapper meta.RESTMapper)
version, kind, err := mapper.VersionAndKindForResource(resource)
checkErr(err)
mapping, err = mapper.RESTMapping(version, kind)
mapping, err = mapper.RESTMapping(kind, version)
checkErr(err)
return
}
@ -268,10 +268,11 @@ func ResourceOrTypeFromArgs(cmd *cobra.Command, args []string, mapper meta.RESTM
}
}
version, kind, err := mapper.VersionAndKindForResource(resource)
defaultVersion, kind, err := mapper.VersionAndKindForResource(resource)
checkErr(err)
mapping, err = mapper.RESTMapping(version, kind)
version := GetFlagString(cmd, "api-version")
mapping, err = mapper.RESTMapping(kind, version, defaultVersion)
checkErr(err)
return
@ -296,7 +297,7 @@ func ResourceFromFile(filename string, typer runtime.ObjectTyper, mapper meta.RE
err = schema.ValidateBytes(data)
checkErr(err)
mapping, err = mapper.RESTMapping(version, kind)
mapping, err = mapper.RESTMapping(kind, version)
checkErr(err)
obj, err := mapping.Codec.Decode(data)