diff --git a/pkg/api/latest/latest_test.go b/pkg/api/latest/latest_test.go index d954ded8ef2..bedb8a1fcae 100644 --- a/pkg/api/latest/latest_test.go +++ b/pkg/api/latest/latest_test.go @@ -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) } diff --git a/pkg/api/meta/interfaces.go b/pkg/api/meta/interfaces.go index dd1dba33d0f..9ae4a001bb2 100644 --- a/pkg/api/meta/interfaces.go +++ b/pkg/api/meta/interfaces.go @@ -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) } diff --git a/pkg/api/meta/restmapper.go b/pkg/api/meta/restmapper.go index df83f6cc638..9ce592e4396 100644 --- a/pkg/api/meta/restmapper.go +++ b/pkg/api/meta/restmapper.go @@ -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 diff --git a/pkg/api/meta/restmapper_test.go b/pkg/api/meta/restmapper_test.go index 7941332ab3f..02eb507777f 100644 --- a/pkg/api/meta/restmapper_test.go +++ b/pkg/api/meta/restmapper_test.go @@ -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") } diff --git a/pkg/client/clientcmd/client_builder.go b/pkg/client/clientcmd/client_builder.go index 80ef5b704fe..201c8a83be8 100644 --- a/pkg/client/clientcmd/client_builder.go +++ b/pkg/client/clientcmd/client_builder.go @@ -33,10 +33,15 @@ import ( type Builder interface { // BindFlags must bind and keep track of all the flags required to build a client config object BindFlags(flags *pflag.FlagSet) - // Config uses the values of the bound flags and builds a complete client config - Config() (*client.Config, error) // Client calls BuildConfig under the covers and uses that config to return a client Client() (*client.Client, error) + + // Config uses the values of the bound flags and builds a complete client config + Config() (*client.Config, error) + // Override invokes Config(), then passes that to the provided function, and returns a new + // builder that will use that config as its default. If Config() returns an error for the default + // values the function will not be invoked, and the error will be available when Client() is called. + Override(func(*client.Config)) Builder } // cmdAuthInfo is used to track whether flags have been set @@ -58,6 +63,8 @@ type builder struct { apiserver string apiVersion string matchApiVersion bool + + config *client.Config } // NewBuilder returns a valid Builder that uses the passed authLoader. If authLoader is nil, the NewDefaultAuthLoader is used. @@ -124,6 +131,26 @@ func (builder *builder) Client() (*client.Client, error) { // Config implements Builder func (builder *builder) Config() (*client.Config, error) { + if builder.config != nil { + return builder.config, nil + } + return builder.newConfig() +} + +// Override implements Builder +func (builder *builder) Override(fn func(*client.Config)) Builder { + config, err := builder.newConfig() + if err != nil { + return builder + } + fn(config) + b := *builder + b.config = config + return &b +} + +// newConfig creates a new config object for this builder +func (builder *builder) newConfig() (*client.Config, error) { clientConfig := client.Config{} if len(builder.apiserver) > 0 { clientConfig.Host = builder.apiserver @@ -140,12 +167,11 @@ func (builder *builder) Config() (*client.Config, error) { authInfoFileFound := true authInfo, err := builder.authLoader.LoadAuth(builder.authPath) if authInfo == nil && err != nil { // only consider failing if we don't have any auth info - if os.IsNotExist(err) { // if it's just a case of a missing file, simply flag the auth as not found and use the command line arguments - authInfoFileFound = false - authInfo = &clientauth.Info{} - } else { + if !os.IsNotExist(err) { // if it's just a case of a missing file, simply flag the auth as not found and use the command line arguments return nil, err } + authInfoFileFound = false + authInfo = &clientauth.Info{} } // If provided, the command line options override options from the auth file diff --git a/pkg/client/clientcmd/client_builder_test.go b/pkg/client/clientcmd/client_builder_test.go index cb2a7b45e68..6262fe83cac 100644 --- a/pkg/client/clientcmd/client_builder_test.go +++ b/pkg/client/clientcmd/client_builder_test.go @@ -29,6 +29,7 @@ import ( "github.com/spf13/pflag" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth" ) @@ -251,6 +252,40 @@ func TestLoadClientAuthInfoOrPrompt(t *testing.T) { } } +func TestOverride(t *testing.T) { + b := NewBuilder(nil) + cfg, err := b.Config() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Version != "" { + t.Errorf("unexpected default config version") + } + + newCfg, err := b.Override(func(cfg *client.Config) { + if cfg.Version != "" { + t.Errorf("unexpected default config version") + } + cfg.Version = "test" + }).Config() + + if newCfg.Version != "test" { + t.Errorf("unexpected override config version") + } + + if cfg.Version != "" { + t.Errorf("original object should not change") + } + + cfg, err = b.Config() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Version != "" { + t.Errorf("override should not be persistent") + } +} + func matchStringArg(expected, got string, t *testing.T) { if expected != got { t.Errorf("Expected %v, got %v", expected, got) diff --git a/pkg/config/config.go b/pkg/config/config.go index 36f77e1b75f..3a66498f02b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -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 diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go index cffdb909296..50ce9b710f6 100644 --- a/pkg/kubectl/cmd/cmd.go +++ b/pkg/kubectl/cmd/cmd.go @@ -64,7 +64,9 @@ func NewFactory(clientBuilder clientcmd.Builder) *Factory { } }, Client: func(cmd *cobra.Command, mapping *meta.RESTMapping) (kubectl.RESTClient, error) { - return clientBuilder.Client() + return clientBuilder.Override(func(c *client.Config) { + c.Version = mapping.APIVersion + }).Client() }, Describer: func(cmd *cobra.Command, mapping *meta.RESTMapping) (kubectl.Describer, error) { client, err := clientBuilder.Client() diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 417ee212c00..f81458a9412 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -45,7 +45,7 @@ Examples: } schema, err := f.Validator(cmd) checkErr(err) - mapping, namespace, name, data := ResourceFromFile(filename, f.Typer, f.Mapper, schema) + mapping, namespace, name, data := ResourceFromFile(cmd, filename, f.Typer, f.Mapper, schema) client, err := f.Client(cmd, mapping) checkErr(err) diff --git a/pkg/kubectl/cmd/createall.go b/pkg/kubectl/cmd/createall.go index af9995fab3c..b1ea7bb2d9a 100644 --- a/pkg/kubectl/cmd/createall.go +++ b/pkg/kubectl/cmd/createall.go @@ -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 diff --git a/pkg/kubectl/cmd/describe_test.go b/pkg/kubectl/cmd/describe_test.go index 81baed2ee05..914aa2097dd 100644 --- a/pkg/kubectl/cmd/describe_test.go +++ b/pkg/kubectl/cmd/describe_test.go @@ -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"}) diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 13676ef723f..0f5b58200f7 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -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"}) diff --git a/pkg/kubectl/cmd/resource.go b/pkg/kubectl/cmd/resource.go index aa72fa22f1d..9055a308d58 100644 --- a/pkg/kubectl/cmd/resource.go +++ b/pkg/kubectl/cmd/resource.go @@ -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 } @@ -216,7 +216,7 @@ func ResourceFromArgsOrFile(cmd *cobra.Command, args []string, filename string, usageError(cmd, "Must specify filename or command line params") } - mapping, namespace, name, _ = ResourceFromFile(filename, typer, mapper, schema) + mapping, namespace, name, _ = ResourceFromFile(cmd, filename, typer, mapper, schema) if len(name) == 0 { checkErr(fmt.Errorf("the resource in the provided file has no name (or ID) defined")) } @@ -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 @@ -280,23 +281,24 @@ func ResourceOrTypeFromArgs(cmd *cobra.Command, args []string, mapper meta.RESTM // ResourceFromFile retrieves the name and namespace from a valid file. If the file does not // resolve to a known type an error is returned. The returned mapping can be used to determine // the correct REST endpoint to modify this resource with. -func ResourceFromFile(filename string, typer runtime.ObjectTyper, mapper meta.RESTMapper, schema validation.Schema) (mapping *meta.RESTMapping, namespace, name string, data []byte) { +func ResourceFromFile(cmd *cobra.Command, filename string, typer runtime.ObjectTyper, mapper meta.RESTMapper, schema validation.Schema) (mapping *meta.RESTMapping, namespace, name string, data []byte) { configData, err := ReadConfigData(filename) checkErr(err) data = configData - version, kind, err := typer.DataVersionAndKind(data) + objVersion, kind, err := typer.DataVersionAndKind(data) checkErr(err) // TODO: allow unversioned objects? - if len(version) == 0 { + if len(objVersion) == 0 { checkErr(fmt.Errorf("the resource in the provided file has no apiVersion defined")) } err = schema.ValidateBytes(data) checkErr(err) - mapping, err = mapper.RESTMapping(version, kind) + // decode using the version stored with the object (allows codec to vary across versions) + mapping, err = mapper.RESTMapping(kind, objVersion) checkErr(err) obj, err := mapping.Codec.Decode(data) @@ -308,6 +310,13 @@ func ResourceFromFile(filename string, typer runtime.ObjectTyper, mapper meta.RE name, err = meta.Name(obj) checkErr(err) + // if the preferred API version differs, get a different mapper + version := GetFlagString(cmd, "api-version") + if version != objVersion { + mapping, err = mapper.RESTMapping(kind, version) + checkErr(err) + } + return } diff --git a/pkg/kubectl/cmd/update.go b/pkg/kubectl/cmd/update.go index 2a35cddbd78..8f812bc070c 100644 --- a/pkg/kubectl/cmd/update.go +++ b/pkg/kubectl/cmd/update.go @@ -45,7 +45,7 @@ Examples: } schema, err := f.Validator(cmd) checkErr(err) - mapping, namespace, name, data := ResourceFromFile(filename, f.Typer, f.Mapper, schema) + mapping, namespace, name, data := ResourceFromFile(cmd, filename, f.Typer, f.Mapper, schema) client, err := f.Client(cmd, mapping) checkErr(err)