From 9eb4ca43d8ef7931b5b6dac755e153fc9a10664e Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 18 Nov 2014 20:19:43 -0500 Subject: [PATCH 1/3] Move FakeRESTClient to client/fake.go --- pkg/client/fake.go | 39 ++++++++++++++++++++ pkg/kubectl/resthelper_test.go | 66 ++++++++++------------------------ 2 files changed, 57 insertions(+), 48 deletions(-) diff --git a/pkg/client/fake.go b/pkg/client/fake.go index 00c81c1bb05..3eb7d6b9b59 100644 --- a/pkg/client/fake.go +++ b/pkg/client/fake.go @@ -17,7 +17,11 @@ limitations under the License. package client import ( + "net/http" + "net/url" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/version" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" ) @@ -75,3 +79,38 @@ func (c *Fake) ServerAPIVersions() (*api.APIVersions, error) { c.Actions = append(c.Actions, FakeAction{Action: "get-apiversions", Value: nil}) return &api.APIVersions{Versions: []string{"v1beta1", "v1beta2"}}, nil } + +type HttpClientFunc func(*http.Request) (*http.Response, error) + +func (f HttpClientFunc) Do(req *http.Request) (*http.Response, error) { + return f(req) +} + +// FakeRESTClient provides a fake RESTClient interface. +type FakeRESTClient struct { + Client HTTPClient + Codec runtime.Codec + Req *http.Request + Resp *http.Response + Err error +} + +func (c *FakeRESTClient) Get() *Request { + return NewRequest(c, "GET", &url.URL{Host: "localhost"}, c.Codec) +} +func (c *FakeRESTClient) Put() *Request { + return NewRequest(c, "PUT", &url.URL{Host: "localhost"}, c.Codec) +} +func (c *FakeRESTClient) Post() *Request { + return NewRequest(c, "POST", &url.URL{Host: "localhost"}, c.Codec) +} +func (c *FakeRESTClient) Delete() *Request { + return NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, c.Codec) +} +func (c *FakeRESTClient) Do(req *http.Request) (*http.Response, error) { + c.Req = req + if c.Client != HTTPClient(nil) { + return c.Client.Do(req) + } + return c.Resp, c.Err +} diff --git a/pkg/kubectl/resthelper_test.go b/pkg/kubectl/resthelper_test.go index 42cb6b23a70..4509211d033 100644 --- a/pkg/kubectl/resthelper_test.go +++ b/pkg/kubectl/resthelper_test.go @@ -22,7 +22,6 @@ import ( "io" "io/ioutil" "net/http" - "net/url" "reflect" "strings" "testing" @@ -34,39 +33,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) -type httpClientFunc func(*http.Request) (*http.Response, error) - -func (f httpClientFunc) Do(req *http.Request) (*http.Response, error) { - return f(req) -} - -type FakeRESTClient struct { - Client client.HTTPClient - Req *http.Request - Resp *http.Response - Err error -} - -func (c *FakeRESTClient) Get() *client.Request { - return client.NewRequest(c, "GET", &url.URL{Host: "localhost"}, testapi.Codec()) -} -func (c *FakeRESTClient) Put() *client.Request { - return client.NewRequest(c, "PUT", &url.URL{Host: "localhost"}, testapi.Codec()) -} -func (c *FakeRESTClient) Post() *client.Request { - return client.NewRequest(c, "POST", &url.URL{Host: "localhost"}, testapi.Codec()) -} -func (c *FakeRESTClient) Delete() *client.Request { - return client.NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, testapi.Codec()) -} -func (c *FakeRESTClient) Do(req *http.Request) (*http.Response, error) { - c.Req = req - if c.Client != client.HTTPClient(nil) { - return c.Client.Do(req) - } - return c.Resp, c.Err -} - func objBody(obj runtime.Object) io.ReadCloser { return ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(testapi.Codec(), obj)))) } @@ -112,9 +78,10 @@ func TestRESTHelperDelete(t *testing.T) { }, } for _, test := range tests { - client := &FakeRESTClient{ - Resp: test.Resp, - Err: test.HttpErr, + client := &client.FakeRESTClient{ + Codec: testapi.Codec(), + Resp: test.Resp, + Err: test.HttpErr, } modifier := &RESTHelper{ RESTClient: client, @@ -147,7 +114,7 @@ func TestRESTHelperCreate(t *testing.T) { tests := []struct { Resp *http.Response - RespFunc httpClientFunc + RespFunc client.HttpClientFunc HttpErr error Modify bool Object runtime.Object @@ -192,9 +159,10 @@ func TestRESTHelperCreate(t *testing.T) { }, } for i, test := range tests { - client := &FakeRESTClient{ - Resp: test.Resp, - Err: test.HttpErr, + client := &client.FakeRESTClient{ + Codec: testapi.Codec(), + Resp: test.Resp, + Err: test.HttpErr, } if test.RespFunc != nil { client.Client = test.RespFunc @@ -275,9 +243,10 @@ func TestRESTHelperGet(t *testing.T) { }, } for _, test := range tests { - client := &FakeRESTClient{ - Resp: test.Resp, - Err: test.HttpErr, + client := &client.FakeRESTClient{ + Codec: testapi.Codec(), + Resp: test.Resp, + Err: test.HttpErr, } modifier := &RESTHelper{ RESTClient: client, @@ -317,7 +286,7 @@ func TestRESTHelperUpdate(t *testing.T) { tests := []struct { Resp *http.Response - RespFunc httpClientFunc + RespFunc client.HttpClientFunc HttpErr error Overwrite bool Object runtime.Object @@ -368,9 +337,10 @@ func TestRESTHelperUpdate(t *testing.T) { }, } for i, test := range tests { - client := &FakeRESTClient{ - Resp: test.Resp, - Err: test.HttpErr, + client := &client.FakeRESTClient{ + Codec: testapi.Codec(), + Resp: test.Resp, + Err: test.HttpErr, } if test.RespFunc != nil { client.Client = test.RespFunc From 811f77894cc28d777cb5b8918219d041fb4f9969 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 18 Nov 2014 20:19:58 -0500 Subject: [PATCH 2/3] Not having Kind set in Decode(1) is an error --- pkg/conversion/decode.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/conversion/decode.go b/pkg/conversion/decode.go index ff949b23333..7d17c05d220 100644 --- a/pkg/conversion/decode.go +++ b/pkg/conversion/decode.go @@ -36,6 +36,9 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) { if version == "" && s.InternalVersion != "" { return nil, fmt.Errorf("version not set in '%s'", string(data)) } + if kind == "" { + return nil, fmt.Errorf("kind not set in '%s'", string(data)) + } obj, err := s.NewObject(version, kind) if err != nil { return nil, err From 487e5cd061bc61637daa7da02ef17c10fb02a373 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 18 Nov 2014 13:04:10 -0500 Subject: [PATCH 3/3] Make GetPrinter take the necessary conversion arguments This fully encapsulates decisions about conversion within GetPrinter, which prevents users from accidentally not converting. --- pkg/kubectl/cmd/cmd_test.go | 117 +++++++++++++++++++++++++++ pkg/kubectl/cmd/describe_test.go | 50 ++++++++++++ pkg/kubectl/cmd/get.go | 20 ++--- pkg/kubectl/cmd/get_test.go | 51 ++++++++++++ pkg/kubectl/resource_printer.go | 110 +++++++++++-------------- pkg/kubectl/resource_printer_test.go | 95 +++++++++++++++------- 6 files changed, 341 insertions(+), 102 deletions(-) create mode 100644 pkg/kubectl/cmd/cmd_test.go create mode 100644 pkg/kubectl/cmd/describe_test.go create mode 100644 pkg/kubectl/cmd/get_test.go diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go new file mode 100644 index 00000000000..2bbb8f5199e --- /dev/null +++ b/pkg/kubectl/cmd/cmd_test.go @@ -0,0 +1,117 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd_test + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" + "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl" + . "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + + "github.com/spf13/cobra" +) + +type internalType struct { + Kind string + APIVersion string + + Name string +} + +type externalType struct { + Kind string `json:"kind"` + APIVersion string `json:"apiVersion"` + + Name string `json:"name"` +} + +func (*internalType) IsAnAPIObject() {} +func (*externalType) IsAnAPIObject() {} + +func newExternalScheme() (*runtime.Scheme, meta.RESTMapper, runtime.Codec) { + scheme := runtime.NewScheme() + scheme.AddKnownTypeWithName("", "Type", &internalType{}) + scheme.AddKnownTypeWithName("unlikelyversion", "Type", &externalType{}) + + codec := runtime.CodecFor(scheme, "unlikelyversion") + mapper := meta.NewDefaultRESTMapper([]string{"unlikelyversion"}, func(version string) (*meta.VersionInterfaces, bool) { + return &meta.VersionInterfaces{ + Codec: codec, + ObjectConvertor: scheme, + MetadataAccessor: meta.NewAccessor(), + }, (version == "unlikelyversion") + }) + mapper.Add(scheme, false, "unlikelyversion") + + return scheme, mapper, codec +} + +type testPrinter struct { + Obj runtime.Object + Err error +} + +func (t *testPrinter) PrintObj(obj runtime.Object, out io.Writer) error { + t.Obj = obj + fmt.Fprintf(out, "%#v", obj) + return t.Err +} + +type testDescriber struct { + Name, Namespace string + Output string + Err error +} + +func (t *testDescriber) Describe(namespace, name string) (output string, err error) { + t.Namespace, t.Name = namespace, name + return t.Output, t.Err +} + +type testFactory struct { + Client kubectl.RESTClient + Describer kubectl.Describer + Printer kubectl.ResourcePrinter + Err error +} + +func NewTestFactory() (*Factory, *testFactory, runtime.Codec) { + scheme, mapper, codec := newExternalScheme() + t := &testFactory{} + return &Factory{ + Mapper: mapper, + Typer: scheme, + Client: func(*cobra.Command, *meta.RESTMapping) (kubectl.RESTClient, error) { + return t.Client, t.Err + }, + Describer: func(*cobra.Command, *meta.RESTMapping) (kubectl.Describer, error) { + return t.Describer, t.Err + }, + Printer: func(cmd *cobra.Command, mapping *meta.RESTMapping, noHeaders bool) (kubectl.ResourcePrinter, error) { + return t.Printer, t.Err + }, + }, t, codec +} + +func objBody(codec runtime.Codec, obj runtime.Object) io.ReadCloser { + return ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(codec, obj)))) +} diff --git a/pkg/kubectl/cmd/describe_test.go b/pkg/kubectl/cmd/describe_test.go new file mode 100644 index 00000000000..81baed2ee05 --- /dev/null +++ b/pkg/kubectl/cmd/describe_test.go @@ -0,0 +1,50 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd_test + +import ( + "bytes" + "fmt" + "net/http" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" +) + +// Verifies that schemas that are not in the master tree of Kubernetes can be retrieved via Get. +func TestDescribeUnknownSchemaObject(t *testing.T) { + d := &testDescriber{Output: "test output"} + f, tf, codec := NewTestFactory() + tf.Describer = d + tf.Client = &client.FakeRESTClient{ + Codec: codec, + Resp: &http.Response{StatusCode: 200, Body: objBody(codec, &internalType{Name: "foo"})}, + } + buf := bytes.NewBuffer([]byte{}) + + cmd := f.NewCmdDescribe(buf) + cmd.Flags().String("namespace", "test", "") + cmd.Run(cmd, []string{"type", "foo"}) + + if d.Name != "foo" || d.Namespace != "test" { + t.Errorf("unexpected describer: %#v", d) + } + + if buf.String() != fmt.Sprintf("%s\n", d.Output) { + t.Errorf("unexpected output: %s", buf.String()) + } +} diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index 301ed8a8f3a..228ac8629b8 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -20,9 +20,9 @@ import ( "fmt" "io" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/spf13/cobra" ) @@ -66,27 +66,29 @@ Examples: if len(outputVersion) == 0 { outputVersion = mapping.APIVersion } - printer, err := kubectl.GetPrinter(outputVersion, outputFormat, templateFile, defaultPrinter) + + printer, err := kubectl.GetPrinter(outputFormat, templateFile, outputVersion, mapping.ObjectConvertor, defaultPrinter) checkErr(err) restHelper := kubectl.NewRESTHelper(client, mapping) obj, err := restHelper.Get(namespace, name, labelSelector) checkErr(err) - if !GetFlagBool(cmd, "watch-only") { + isWatch, isWatchOnly := GetFlagBool(cmd, "watch"), GetFlagBool(cmd, "watch-only") + + // print the current object + if !isWatchOnly { if err := printer.PrintObj(obj, out); err != nil { checkErr(fmt.Errorf("Unable to output the provided object: %v", err)) } } - if GetFlagBool(cmd, "watch") || GetFlagBool(cmd, "watch-only") { - vi, err := latest.InterfacesFor(outputVersion) + // print watched changes + if isWatch || isWatchOnly { + rv, err := mapping.MetadataAccessor.ResourceVersion(obj) checkErr(err) - rv, err := vi.MetadataAccessor.ResourceVersion(obj) - checkErr(err) - - w, err := restHelper.Watch(namespace, rv, labelSelector, labels.Set{}.AsSelector()) + w, err := restHelper.Watch(namespace, rv, labelSelector, labels.Everything()) checkErr(err) kubectl.WatchLoop(w, printer, out) diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go new file mode 100644 index 00000000000..13676ef723f --- /dev/null +++ b/pkg/kubectl/cmd/get_test.go @@ -0,0 +1,51 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd_test + +import ( + "bytes" + "fmt" + "net/http" + "reflect" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" +) + +// Verifies that schemas that are not in the master tree of Kubernetes can be retrieved via Get. +func TestGetUnknownSchemaObject(t *testing.T) { + f, tf, codec := NewTestFactory() + tf.Printer = &testPrinter{} + tf.Client = &client.FakeRESTClient{ + Codec: codec, + Resp: &http.Response{StatusCode: 200, Body: objBody(codec, &internalType{Name: "foo"})}, + } + buf := bytes.NewBuffer([]byte{}) + + cmd := f.NewCmdGet(buf) + cmd.Flags().String("namespace", "test", "") + cmd.Run(cmd, []string{"type", "foo"}) + + expected := &internalType{Name: "foo"} + actual := tf.Printer.(*testPrinter).Obj + if !reflect.DeepEqual(expected, actual) { + t.Errorf("unexpected object: %#v", actual) + } + if buf.String() != fmt.Sprintf("%#v", expected) { + t.Errorf("unexpected output: %s", buf.String()) + } +} diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index 3f517fa33d0..ca80711bd8d 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -28,40 +28,40 @@ import ( "text/template" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/golang/glog" "gopkg.in/v1/yaml" ) -// GetPrinter returns a resource printer and a bool indicating whether the object must be -// versioned for the given format. -func GetPrinter(version, format, templateFile string, defaultPrinter ResourcePrinter) (ResourcePrinter, error) { +// GetPrinter takes a format type, an optional format argument, a version and a convertor +// to be used if the underlying printer requires the object to be in a specific schema ( +// any of the generic formatters), and the default printer to use for this object. +func GetPrinter(format, formatArgument, version string, convertor runtime.ObjectConvertor, defaultPrinter ResourcePrinter) (ResourcePrinter, error) { var printer ResourcePrinter switch format { case "json": - printer = &JSONPrinter{version} + printer = &JSONPrinter{version, convertor} case "yaml": - printer = &YAMLPrinter{version} + printer = &YAMLPrinter{version, convertor} case "template": - if len(templateFile) == 0 { + if len(formatArgument) == 0 { return nil, fmt.Errorf("template format specified but no template given") } var err error - printer, err = NewTemplatePrinter(version, []byte(templateFile)) + printer, err = NewTemplatePrinter([]byte(formatArgument), version, convertor) if err != nil { - return nil, fmt.Errorf("error parsing template %s, %v\n", templateFile, err) + return nil, fmt.Errorf("error parsing template %s, %v\n", formatArgument, err) } case "templatefile": - if len(templateFile) == 0 { + if len(formatArgument) == 0 { return nil, fmt.Errorf("templatefile format specified but no template file given") } - data, err := ioutil.ReadFile(templateFile) + data, err := ioutil.ReadFile(formatArgument) if err != nil { - return nil, fmt.Errorf("error reading template %s, %v\n", templateFile, err) + return nil, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) } - printer, err = NewTemplatePrinter(version, data) + printer, err = NewTemplatePrinter(data, version, convertor) if err != nil { return nil, fmt.Errorf("error parsing template %s, %v\n", string(data), err) } @@ -73,28 +73,27 @@ func GetPrinter(version, format, templateFile string, defaultPrinter ResourcePri return printer, nil } -// ResourcePrinter is an interface that knows how to print API resources. +// ResourcePrinter is an interface that knows how to print runtime objects. type ResourcePrinter interface { // Print receives an arbitrary object, formats it and prints it to a writer. PrintObj(runtime.Object, io.Writer) error - - // Returns true if this printer emits properly versioned output. - IsVersioned() bool } -// JSONPrinter is an implementation of ResourcePrinter which prints as JSON. +// JSONPrinter is an implementation of ResourcePrinter which outputs an object as JSON. +// The input object is assumed to be in the internal version of an API and is converted +// to the given version first. type JSONPrinter struct { - version string + version string + convertor runtime.ObjectConvertor } // PrintObj is an implementation of ResourcePrinter.PrintObj which simply writes the object to the Writer. -func (j *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error { - vi, err := latest.InterfacesFor(j.version) +func (p *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error { + outObj, err := p.convertor.ConvertToVersion(obj, p.version) if err != nil { return err } - - data, err := vi.Codec.Encode(obj) + data, err := json.Marshal(outObj) if err != nil { return err } @@ -105,39 +104,20 @@ func (j *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error { return err } -// IsVersioned returns true. -func (*JSONPrinter) IsVersioned() bool { return true } - -func toVersionedMap(version string, obj runtime.Object) (map[string]interface{}, error) { - vi, err := latest.InterfacesFor(version) - if err != nil { - return nil, err - } - - data, err := vi.Codec.Encode(obj) - if err != nil { - return nil, err - } - outObj := map[string]interface{}{} - err = json.Unmarshal(data, &outObj) - if err != nil { - return nil, err - } - return outObj, nil -} - -// YAMLPrinter is an implementation of ResourcePrinter which prints as YAML. +// YAMLPrinter is an implementation of ResourcePrinter which outputs an object as YAML. +// The input object is assumed to be in the internal version of an API and is converted +// to the given version first. type YAMLPrinter struct { - version string + version string + convertor runtime.ObjectConvertor } // PrintObj prints the data as YAML. -func (y *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error { - outObj, err := toVersionedMap(y.version, obj) +func (p *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error { + outObj, err := p.convertor.ConvertToVersion(obj, p.version) if err != nil { return err } - output, err := yaml.Marshal(outObj) if err != nil { return err @@ -146,9 +126,6 @@ func (y *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error { return err } -// IsVersioned returns true. -func (*YAMLPrinter) IsVersioned() bool { return true } - type handlerEntry struct { columns []string printFunc reflect.Value @@ -164,9 +141,6 @@ type HumanReadablePrinter struct { lastType reflect.Type } -// IsVersioned returns false-- human readable printers do not make versioned output. -func (*HumanReadablePrinter) IsVersioned() bool { return false } - // NewHumanReadablePrinter creates a HumanReadablePrinter. func NewHumanReadablePrinter(noHeaders bool) *HumanReadablePrinter { printer := &HumanReadablePrinter{ @@ -387,28 +361,34 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er // TemplatePrinter is an implementation of ResourcePrinter which formats data with a Go Template. type TemplatePrinter struct { - version string - template *template.Template + template *template.Template + version string + convertor runtime.ObjectConvertor } -func NewTemplatePrinter(version string, tmpl []byte) (*TemplatePrinter, error) { +func NewTemplatePrinter(tmpl []byte, asVersion string, convertor runtime.ObjectConvertor) (*TemplatePrinter, error) { t, err := template.New("output").Parse(string(tmpl)) if err != nil { return nil, err } - return &TemplatePrinter{version, t}, nil + return &TemplatePrinter{t, asVersion, convertor}, nil } -// IsVersioned returns true. -func (*TemplatePrinter) IsVersioned() bool { return true } - // PrintObj formats the obj with the Go Template. -func (t *TemplatePrinter) PrintObj(obj runtime.Object, w io.Writer) error { - outObj, err := toVersionedMap(t.version, obj) +func (p *TemplatePrinter) PrintObj(obj runtime.Object, w io.Writer) error { + outObj, err := p.convertor.ConvertToVersion(obj, p.version) if err != nil { return err } - return t.template.Execute(w, outObj) + data, err := json.Marshal(outObj) + if err != nil { + return err + } + out := map[string]interface{}{} + if err := json.Unmarshal(data, &out); err != nil { + return err + } + return p.template.Execute(w, out) } func tabbedString(f func(io.Writer) error) (string, error) { diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go index ac34c32c42e..47dc714873c 100644 --- a/pkg/kubectl/resource_printer_test.go +++ b/pkg/kubectl/resource_printer_test.go @@ -25,7 +25,8 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "gopkg.in/v1/yaml" @@ -42,11 +43,9 @@ type testStruct struct { func (ts *testStruct) IsAnAPIObject() {} -const TestVersion = latest.Version - func init() { api.Scheme.AddKnownTypes("", &testStruct{}) - api.Scheme.AddKnownTypes(TestVersion, &testStruct{}) + api.Scheme.AddKnownTypes(testapi.Version(), &testStruct{}) } var testData = testStruct{ @@ -57,21 +56,67 @@ var testData = testStruct{ } func TestYAMLPrinter(t *testing.T) { - testPrinter(t, &YAMLPrinter{TestVersion}, yaml.Unmarshal) + testPrinter(t, &YAMLPrinter{testapi.Version(), api.Scheme}, yaml.Unmarshal) } func TestJSONPrinter(t *testing.T) { - testPrinter(t, &JSONPrinter{TestVersion}, json.Unmarshal) + testPrinter(t, &JSONPrinter{testapi.Version(), api.Scheme}, json.Unmarshal) +} + +type internalType struct { + Name string + Kind string +} + +type externalType struct { + Name string + Kind string `json:"kind"` +} + +func (*internalType) IsAnAPIObject() {} +func (*externalType) IsAnAPIObject() {} + +func newExternalScheme() *runtime.Scheme { + scheme := runtime.NewScheme() + scheme.AddKnownTypeWithName("", "Type", &internalType{}) + scheme.AddKnownTypeWithName("unlikelyversion", "Type", &externalType{}) + return scheme +} + +func TestPrintJSONForUnknownSchema(t *testing.T) { + buf := bytes.NewBuffer([]byte{}) + printer, err := GetPrinter("json", "", "unlikelyversion", newExternalScheme(), nil) + if err != nil { + t.Fatalf("unexpected error: %#v", err) + } + if err := printer.PrintObj(&internalType{Name: "foo"}, buf); err != nil { + t.Fatalf("unexpected error: %#v", err) + } + obj := map[string]interface{}{} + if err := json.Unmarshal(buf.Bytes(), &obj); err != nil { + t.Fatalf("unexpected error: %#v\n%s", err, buf.String()) + } + if obj["Name"] != "foo" { + t.Errorf("unexpected field: %#v", obj) + } +} + +func TestPrintJSONForUnknownSchemaAndWrongVersion(t *testing.T) { + buf := bytes.NewBuffer([]byte{}) + printer, err := GetPrinter("json", "", "badversion", newExternalScheme(), nil) + if err != nil { + t.Fatalf("unexpected error: %#v", err) + } + if err := printer.PrintObj(&internalType{Name: "foo"}, buf); err == nil { + t.Errorf("unexpected non-error") + } } func TestPrintJSON(t *testing.T) { buf := bytes.NewBuffer([]byte{}) - printer, err := GetPrinter(TestVersion, "json", "", nil) + printer, err := GetPrinter("json", "", testapi.Version(), api.Scheme, nil) if err != nil { - t.Errorf("unexpected error: %#v", err) - } - if !printer.IsVersioned() { - t.Errorf("printer should be versioned") + t.Fatalf("unexpected error: %#v", err) } printer.PrintObj(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}, buf) obj := map[string]interface{}{} @@ -82,12 +127,9 @@ func TestPrintJSON(t *testing.T) { func TestPrintYAML(t *testing.T) { buf := bytes.NewBuffer([]byte{}) - printer, err := GetPrinter(TestVersion, "yaml", "", nil) + printer, err := GetPrinter("yaml", "", testapi.Version(), api.Scheme, nil) if err != nil { - t.Errorf("unexpected error: %#v", err) - } - if !printer.IsVersioned() { - t.Errorf("printer should be versioned") + t.Fatalf("unexpected error: %#v", err) } printer.PrintObj(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}, buf) obj := map[string]interface{}{} @@ -98,13 +140,10 @@ func TestPrintYAML(t *testing.T) { func TestPrintTemplate(t *testing.T) { buf := bytes.NewBuffer([]byte{}) - printer, err := GetPrinter(TestVersion, "template", "{{.id}}", nil) + printer, err := GetPrinter("template", "{{.id}}", "v1beta1", api.Scheme, nil) if err != nil { t.Fatalf("unexpected error: %#v", err) } - if !printer.IsVersioned() { - t.Errorf("printer should be versioned") - } err = printer.PrintObj(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}, buf) if err != nil { t.Fatalf("unexpected error: %#v", err) @@ -115,19 +154,19 @@ func TestPrintTemplate(t *testing.T) { } func TestPrintEmptyTemplate(t *testing.T) { - if _, err := GetPrinter(TestVersion, "template", "", nil); err == nil { + if _, err := GetPrinter("template", "", testapi.Version(), api.Scheme, nil); err == nil { t.Errorf("unexpected non-error") } } func TestPrintBadTemplate(t *testing.T) { - if _, err := GetPrinter(TestVersion, "template", "{{ .Name", nil); err == nil { + if _, err := GetPrinter("template", "{{ .Name", testapi.Version(), api.Scheme, nil); err == nil { t.Errorf("unexpected non-error") } } func TestPrintBadTemplateFile(t *testing.T) { - if _, err := GetPrinter(TestVersion, "templatefile", "", nil); err == nil { + if _, err := GetPrinter("templatefile", "", testapi.Version(), api.Scheme, nil); err == nil { t.Errorf("unexpected non-error") } } @@ -147,7 +186,7 @@ func testPrinter(t *testing.T, printer ResourcePrinter, unmarshalFunc func(data } // Use real decode function to undo the versioning process. poutput = testStruct{} - err = latest.Codec.DecodeInto(buf.Bytes(), &poutput) + err = testapi.Codec().DecodeInto(buf.Bytes(), &poutput) if err != nil { t.Fatal(err) } @@ -164,11 +203,11 @@ func testPrinter(t *testing.T, printer ResourcePrinter, unmarshalFunc func(data // Verify that given function runs without error. err = unmarshalFunc(buf.Bytes(), &objOut) if err != nil { - t.Errorf("Unexpeted error: %#v", err) + t.Fatalf("unexpected error: %#v", err) } // Use real decode function to undo the versioning process. objOut = api.Pod{} - err = latest.Codec.DecodeInto(buf.Bytes(), &objOut) + err = testapi.Codec().DecodeInto(buf.Bytes(), &objOut) if err != nil { t.Fatal(err) } @@ -205,7 +244,7 @@ func TestCustomTypePrinting(t *testing.T) { buffer := &bytes.Buffer{} err := printer.PrintObj(&obj, buffer) if err != nil { - t.Errorf("An error occurred printing the custom type: %#v", err) + t.Fatalf("An error occurred printing the custom type: %#v", err) } expectedOutput := "Data\ntest object" if buffer.String() != expectedOutput { @@ -236,7 +275,7 @@ func TestUnknownTypePrinting(t *testing.T) { func TestTemplateEmitsVersionedObjects(t *testing.T) { // kind is always blank in memory and set on the wire - printer, err := NewTemplatePrinter(TestVersion, []byte(`{{.kind}}`)) + printer, err := NewTemplatePrinter([]byte(`{{.kind}}`), testapi.Version(), api.Scheme) if err != nil { t.Fatalf("tmpl fail: %v", err) }