From 545a5a865f2b4c55a01a29f3c13a83006e72426d Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 16 Apr 2015 19:21:13 -0400 Subject: [PATCH] List output with differing types should be more resilient kubectl get can output a series of objects as a List in versioned form, but not all API objects are available in the same schema. Make the act of converting a []runtime.Object to api.List more robust and add a test to verify its behavior in Get. Makes it easier for client code to output unified objects. --- hack/test-cmd.sh | 1 + pkg/api/meta/restmapper.go | 39 +++++++++++++++++ pkg/conversion/encode.go | 2 +- pkg/kubectl/cmd/cmd_test.go | 35 ++++++++++++--- pkg/kubectl/cmd/get.go | 26 ++++------- pkg/kubectl/cmd/get_test.go | 79 ++++++++++++++++++++++++++++++++++ pkg/kubectl/resource/result.go | 65 ++++++++++++++++++++++++++++ pkg/runtime/helper.go | 22 ++++++++++ pkg/runtime/scheme.go | 15 +++++-- 9 files changed, 255 insertions(+), 29 deletions(-) diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 1583184b0b2..e8736b6ef02 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -162,6 +162,7 @@ for version in "${kube_api_versions[@]}"; do # Command kubectl create "${kube_flags[@]}" -f examples/limitrange/valid-pod.json # Post-condition: valid-pod POD is running + kubectl get "${kube_flags[@]}" pods -o json kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' kube::test::get_object_assert 'pod valid-pod' "{{$id_field}}" 'valid-pod' kube::test::get_object_assert 'pod/valid-pod' "{{$id_field}}" 'valid-pod' diff --git a/pkg/api/meta/restmapper.go b/pkg/api/meta/restmapper.go index 615749b44fa..117a942c328 100644 --- a/pkg/api/meta/restmapper.go +++ b/pkg/api/meta/restmapper.go @@ -251,3 +251,42 @@ func (m *DefaultRESTMapper) AliasesForResource(alias string) ([]string, bool) { } return nil, false } + +// MultiRESTMapper is a wrapper for multiple RESTMappers. +type MultiRESTMapper []RESTMapper + +// 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 +} + +// RESTMapping provides the REST mapping for the resource based on the resource +// 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) { + for _, t := range m { + mapping, err = t.RESTMapping(kind, versions...) + if err == nil { + return + } + } + return +} + +// AliasesForResource finds the first alias response for the provided mappers. +func (m MultiRESTMapper) AliasesForResource(alias string) (aliases []string, ok bool) { + for _, t := range m { + if aliases, ok = t.AliasesForResource(alias); ok { + return + } + } + return nil, false +} diff --git a/pkg/conversion/encode.go b/pkg/conversion/encode.go index fa753e0d3a2..e83e26cf878 100644 --- a/pkg/conversion/encode.go +++ b/pkg/conversion/encode.go @@ -53,7 +53,7 @@ func (s *Scheme) EncodeToVersion(obj interface{}, destVersion string) (data []by obj = maybeCopy(obj) v, _ := EnforcePtr(obj) // maybeCopy guarantees a pointer if _, registered := s.typeToVersion[v.Type()]; !registered { - return nil, fmt.Errorf("type %v is not registered and it will be impossible to Decode it, therefore Encode will refuse to encode it.", v.Type()) + return nil, fmt.Errorf("type %v is not registered for %q and it will be impossible to Decode it, therefore Encode will refuse to encode it.", v.Type(), destVersion) } objVersion, objKind, err := s.ObjectVersionAndKind(obj) diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 3ca8aeadc82..b4f4dadf3e1 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -49,23 +49,32 @@ type externalType struct { Name string `json:"name"` } -func (*internalType) IsAnAPIObject() {} -func (*externalType) IsAnAPIObject() {} +type ExternalType2 struct { + Kind string `json:"kind"` + APIVersion string `json:"apiVersion"` + + Name string `json:"name"` +} + +func (*internalType) IsAnAPIObject() {} +func (*externalType) IsAnAPIObject() {} +func (*ExternalType2) IsAnAPIObject() {} func newExternalScheme() (*runtime.Scheme, meta.RESTMapper, runtime.Codec) { scheme := runtime.NewScheme() scheme.AddKnownTypeWithName("", "Type", &internalType{}) scheme.AddKnownTypeWithName("unlikelyversion", "Type", &externalType{}) + scheme.AddKnownTypeWithName("v1beta1", "Type", &ExternalType2{}) codec := runtime.CodecFor(scheme, "unlikelyversion") - mapper := meta.NewDefaultRESTMapper([]string{"unlikelyversion"}, func(version string) (*meta.VersionInterfaces, bool) { + mapper := meta.NewDefaultRESTMapper([]string{"unlikelyversion", "v1beta1"}, func(version string) (*meta.VersionInterfaces, bool) { return &meta.VersionInterfaces{ - Codec: codec, + Codec: runtime.CodecFor(scheme, version), ObjectConvertor: scheme, MetadataAccessor: meta.NewAccessor(), - }, (version == "unlikelyversion") + }, (version == "v1beta1" || version == "unlikelyversion") }) - for _, version := range []string{"unlikelyversion"} { + for _, version := range []string{"unlikelyversion", "v1beta1"} { for kind := range scheme.KnownTypes(version) { mixedCase := false scope := meta.RESTScopeNamespace @@ -142,6 +151,20 @@ func NewTestFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { }, t, codec } +func NewMixedFactory(apiClient resource.RESTClient) (*cmdutil.Factory, *testFactory, runtime.Codec) { + f, t, c := NewTestFactory() + f.Object = func() (meta.RESTMapper, runtime.ObjectTyper) { + return meta.MultiRESTMapper{t.Mapper, latest.RESTMapper}, runtime.MultiObjectTyper{t.Typer, api.Scheme} + } + f.RESTClient = func(m *meta.RESTMapping) (resource.RESTClient, error) { + if m.ObjectConvertor == api.Scheme { + return apiClient, t.Err + } + return t.Client, t.Err + } + return f, t, c +} + func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) { t := &testFactory{ Validator: validation.NullSchema{}, diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index 24c9fc9f97b..4438c66dc1d 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -20,8 +20,6 @@ import ( "fmt" "io" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl" cmdutil "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd/util" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource" @@ -162,29 +160,21 @@ func RunGet(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string } defaultVersion := clientConfig.Version - // the outermost object will be converted to the output-version - version := cmdutil.OutputVersion(cmd, defaultVersion) - + singular := false r := b.Flatten().Do() - obj, err := r.Object() + infos, err := r.IntoSingular(&singular).Infos() if err != nil { return err } - // try conversion to all the possible versions - // TODO: simplify by adding a ResourceBuilder mode - versions := []string{version, latest.Version} - infos, _ := r.Infos() - for _, info := range infos { - versions = append(versions, info.Mapping.APIVersion) + // the outermost object will be converted to the output-version, but inner + // objects can use their mappings + version := cmdutil.OutputVersion(cmd, defaultVersion) + obj, err := resource.AsVersionedObject(infos, !singular, version) + if err != nil { + return err } - // TODO: add a new ResourceBuilder mode for Object() that attempts to ensure the objects - // are in the appropriate version if one exists (and if not, use the best effort). - // TODO: ensure api-version is set with the default preferred api version by the client - // builder on initialization - printer := kubectl.NewVersionedPrinter(printer, api.Scheme, versions...) - return printer.PrintObj(obj, out) } diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 7ba7177cc8a..95616a9807a 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -18,6 +18,7 @@ package cmd import ( "bytes" + encjson "encoding/json" "fmt" "io" "io/ioutil" @@ -138,6 +139,84 @@ func TestGetUnknownSchemaObject(t *testing.T) { } } +// Verifies that schemas that are not in the master tree of Kubernetes can be retrieved via Get. +// Because api.List is part of the Kube API, resource.Builder has to perform a conversion on +// api.Scheme, which may not have access to all objects, and not all objects are at the same +// internal versioning scheme. This test verifies that two isolated schemes (Test, and api.Scheme) +// can be conjoined into a single output object. +func TestGetUnknownSchemaObjectListGeneric(t *testing.T) { + testCases := map[string]struct { + output string + list string + obj1 string + obj2 string + }{ + "handles specific version": { + output: "v1beta3", + list: "v1beta3", + obj1: "unlikelyversion", + obj2: "v1beta3", + }, + "handles second specific version": { + output: "unlikelyversion", + list: "v1beta3", + obj1: "unlikelyversion", // doesn't have v1beta3 + obj2: "v1beta1", // version of the API response + }, + "handles common version": { + output: "v1beta1", + list: "v1beta1", + obj1: "unlikelyversion", // because test scheme defaults to unlikelyversion + obj2: "v1beta1", + }, + } + for k, test := range testCases { + apiCodec := runtime.CodecFor(api.Scheme, "v1beta1") + regularClient := &client.FakeRESTClient{ + Codec: apiCodec, + Client: client.HTTPClientFunc(func(req *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: 200, Body: objBody(apiCodec, &api.ReplicationController{ObjectMeta: api.ObjectMeta{Name: "foo"}})}, nil + }), + } + + f, tf, codec := NewMixedFactory(regularClient) + tf.Printer = &testPrinter{} + tf.Client = &client.FakeRESTClient{ + Codec: codec, + Client: client.HTTPClientFunc(func(req *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: 200, Body: objBody(codec, &internalType{Name: "foo"})}, nil + }), + } + tf.Namespace = "test" + tf.ClientConfig = &client.Config{Version: latest.Version} + buf := bytes.NewBuffer([]byte{}) + cmd := NewCmdGet(f, buf) + cmd.SetOutput(buf) + cmd.Flags().Set("output", "json") + cmd.Flags().Set("output-version", test.output) + err := RunGet(f, buf, cmd, []string{"type/foo", "replicationcontrollers/foo"}) + if err != nil { + t.Errorf("%s: unexpected error: %v", k, err) + continue + } + out := make(map[string]interface{}) + if err := encjson.Unmarshal(buf.Bytes(), &out); err != nil { + t.Errorf("%s: unexpected error: %v\n%s", k, err, buf.String()) + continue + } + if out["apiVersion"] != test.list { + t.Errorf("%s: unexpected list: %#v", k, out) + } + arr := out["items"].([]interface{}) + if arr[0].(map[string]interface{})["apiVersion"] != test.obj1 { + t.Errorf("%s: unexpected list: %#v", k, out) + } + if arr[1].(map[string]interface{})["apiVersion"] != test.obj2 { + t.Errorf("%s: unexpected list: %#v", k, out) + } + } +} + // Verifies that schemas that are not in the master tree of Kubernetes can be retrieved via Get. func TestGetSchemaObject(t *testing.T) { f, tf, _ := NewTestFactory() diff --git a/pkg/kubectl/resource/result.go b/pkg/kubectl/resource/result.go index 13e5b89638b..be75dd9099b 100644 --- a/pkg/kubectl/resource/result.go +++ b/pkg/kubectl/resource/result.go @@ -21,6 +21,7 @@ import ( "reflect" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -200,3 +201,67 @@ func (r *Result) Watch(resourceVersion string) (watch.Interface, error) { } return w.Watch(resourceVersion) } + +// AsVersionedObject converts a list of infos into a single object - either a List containing +// the objects as children, or if only a single Object is present, as that object. The provided +// version will be preferred as the conversion target, but the Object's mapping version will be +// used if that version is not present. +func AsVersionedObject(infos []*Info, forceList bool, version string) (runtime.Object, error) { + objects := []runtime.Object{} + for _, info := range infos { + if info.Object == nil { + continue + } + + // objects that are not part of api.Scheme must be converted to JSON + // TODO: convert to map[string]interface{}, attach to runtime.Unknown? + if len(version) > 0 { + if _, _, err := api.Scheme.ObjectVersionAndKind(info.Object); runtime.IsNotRegisteredError(err) { + // TODO: ideally this would encode to version, but we don't expose multiple codecs here. + data, err := info.Mapping.Codec.Encode(info.Object) + if err != nil { + return nil, err + } + objects = append(objects, &runtime.Unknown{RawJSON: data}) + continue + } + } + + converted, err := tryConvert(info.Mapping.ObjectConvertor, info.Object, version, info.Mapping.APIVersion) + if err != nil { + return nil, err + } + objects = append(objects, converted) + } + + var object runtime.Object + if len(objects) == 1 && !forceList { + object = objects[0] + } else { + object = &api.List{Items: objects} + converted, err := tryConvert(api.Scheme, object, version, latest.Version) + if err != nil { + return nil, err + } + object = converted + } + return object, nil +} + +// tryConvert attempts to convert the given object to the provided versions in order. This function assumes +// the object is in internal version. +func tryConvert(convertor runtime.ObjectConvertor, object runtime.Object, versions ...string) (runtime.Object, error) { + var last error + for _, version := range versions { + if len(version) == 0 { + return object, nil + } + obj, err := convertor.ConvertToVersion(object, version) + if err != nil { + last = err + continue + } + return obj, nil + } + return nil, last +} diff --git a/pkg/runtime/helper.go b/pkg/runtime/helper.go index 3d4df08091e..beff579144a 100644 --- a/pkg/runtime/helper.go +++ b/pkg/runtime/helper.go @@ -149,3 +149,25 @@ func FieldPtr(v reflect.Value, fieldName string, dest interface{}) error { } return fmt.Errorf("couldn't assign/convert %v to %v", field.Type(), v.Type()) } + +// MultiObjectTyper returns the types of objects across multiple schemes in order. +type MultiObjectTyper []ObjectTyper + +func (m MultiObjectTyper) DataVersionAndKind(data []byte) (version, kind string, err error) { + for _, t := range m { + version, kind, err = t.DataVersionAndKind(data) + if err == nil { + return + } + } + return +} +func (m MultiObjectTyper) ObjectVersionAndKind(obj Object) (version, kind string, err error) { + for _, t := range m { + version, kind, err = t.ObjectVersionAndKind(obj) + if err == nil { + return + } + } + return +} diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index 0e0ed67070b..b8047c7dc3c 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -18,9 +18,10 @@ package runtime import ( "fmt" - "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "net/url" "reflect" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" ) // Scheme defines methods for serializing and deserializing API objects. It @@ -147,8 +148,9 @@ func (self *Scheme) rawExtensionToEmbeddedObject(in *RawExtension, out *Embedded } // runtimeObjectToRawExtensionArray takes a list of objects and encodes them as RawExtension in the output version -// defined by the conversion.Scope. If objects must be encoded to different schema versions you should set them as -// runtime.Unknown in the internal version instead. +// defined by the conversion.Scope. If objects must be encoded to different schema versions than the default, you +// should encode them yourself with runtime.Unknown, or convert the object prior to invoking conversion. Objects +// outside of the current scheme must be added as runtime.Unknown. func (self *Scheme) runtimeObjectToRawExtensionArray(in *[]Object, out *[]RawExtension, s conversion.Scope) error { src := *in dest := make([]RawExtension, len(src)) @@ -160,7 +162,12 @@ func (self *Scheme) runtimeObjectToRawExtensionArray(in *[]Object, out *[]RawExt case *Unknown: dest[i].RawJSON = t.RawJSON default: - data, err := scheme.EncodeToVersion(src[i], outVersion) + version := outVersion + // if the object exists + if inVersion, _, err := scheme.ObjectVersionAndKind(src[i]); err == nil && len(inVersion) != 0 { + version = inVersion + } + data, err := scheme.EncodeToVersion(src[i], version) if err != nil { return err }