From 6918a4d32ed200e6e5e4715fcf049c32d86c5488 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 16 Mar 2015 23:43:59 -0400 Subject: [PATCH] Replace automatic YAML decoding with opt-in YAML decoding Base codecs no longer automically handle YAML. Instead, clients must convert to JSON first via yaml.ToJSON and runtime.YAMLDecoder. --- examples/examples_test.go | 18 ++++++++++++-- pkg/api/validation/schema.go | 16 ++++++++----- pkg/client/clientcmd/api/latest/latest.go | 3 ++- pkg/conversion/decode.go | 29 +++++++---------------- pkg/conversion/meta.go | 13 ++++------ pkg/kubectl/resource/mapper.go | 6 +++++ pkg/kubectl/resource_printer_test.go | 4 ++-- 7 files changed, 49 insertions(+), 40 deletions(-) diff --git a/examples/examples_test.go b/examples/examples_test.go index a672401da46..8c5ce092e53 100644 --- a/examples/examples_test.go +++ b/examples/examples_test.go @@ -29,6 +29,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/yaml" "github.com/golang/glog" ) @@ -88,6 +89,15 @@ func walkJSONFiles(inDir string, fn func(name, path string, data []byte)) error return err } name := strings.TrimSuffix(file, ext) + + if ext == ".yaml" { + out, err := yaml.ToJSON(data) + if err != nil { + return err + } + data = out + } + fn(name, path, data) } return nil @@ -215,14 +225,18 @@ func TestReadme(t *testing.T) { //t.Logf("testing (%s): \n%s", subtype, content) expectedType := &api.Pod{} - if err := latest.Codec.DecodeInto([]byte(content), expectedType); err != nil { + json, err := yaml.ToJSON([]byte(content)) + if err != nil { + t.Errorf("%s could not be converted to JSON: %v\n%s", path, err, string(content)) + } + if err := latest.Codec.DecodeInto(json, expectedType); err != nil { t.Errorf("%s did not decode correctly: %v\n%s", path, err, string(content)) continue } if errors := validateObject(expectedType); len(errors) > 0 { t.Errorf("%s did not validate correctly: %v", path, errors) } - _, err := latest.Codec.Encode(expectedType) + _, err = latest.Codec.Encode(expectedType) if err != nil { t.Errorf("Could not encode object: %v", err) continue diff --git a/pkg/api/validation/schema.go b/pkg/api/validation/schema.go index 4bfaa7536e7..37757614757 100644 --- a/pkg/api/validation/schema.go +++ b/pkg/api/validation/schema.go @@ -22,9 +22,9 @@ import ( "reflect" "strings" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/yaml" "github.com/emicklei/go-restful/swagger" "github.com/golang/glog" - "gopkg.in/yaml.v2" ) type InvalidTypeError struct { @@ -65,11 +65,15 @@ func NewSwaggerSchemaFromBytes(data []byte) (Schema, error) { func (s *SwaggerSchema) ValidateBytes(data []byte) error { var obj interface{} - err := yaml.Unmarshal(data, &obj) + out, err := yaml.ToJSON(data) if err != nil { return err } - fields := obj.(map[interface{}]interface{}) + data = out + if err := json.Unmarshal(data, &obj); err != nil { + return err + } + fields := obj.(map[string]interface{}) apiVersion := fields["apiVersion"].(string) kind := fields["kind"].(string) return s.ValidateObject(obj, apiVersion, "", apiVersion+"."+kind) @@ -84,12 +88,12 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, apiVersion, fieldName, t return nil } properties := model.Properties - fields := obj.(map[interface{}]interface{}) + fields := obj.(map[string]interface{}) if len(fieldName) > 0 { fieldName = fieldName + "." } for key, value := range fields { - details, ok := properties[key.(string)] + details, ok := properties[key] if !ok { glog.V(2).Infof("couldn't find properties for %s, skipping", key) continue @@ -99,7 +103,7 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, apiVersion, fieldName, t glog.V(2).Infof("Skipping nil field: %s", key) continue } - err := s.validateField(value, apiVersion, fieldName+key.(string), fieldType, &details) + err := s.validateField(value, apiVersion, fieldName+key, fieldType, &details) if err != nil { glog.Errorf("Validation failed for: %s, %v", key, value) return err diff --git a/pkg/client/clientcmd/api/latest/latest.go b/pkg/client/clientcmd/api/latest/latest.go index be7657ae2b5..260650700e8 100644 --- a/pkg/client/clientcmd/api/latest/latest.go +++ b/pkg/client/clientcmd/api/latest/latest.go @@ -18,6 +18,7 @@ package latest import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api/v1" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) // Version is the string that represents the current external default version. @@ -37,4 +38,4 @@ var Versions = []string{"v1"} // the latest supported version. Use this Codec when writing to // disk, a data store that is not dynamically versioned, or in tests. // This codec can decode any object that Kubernetes is aware of. -var Codec = v1.Codec +var Codec = runtime.YAMLDecoder(v1.Codec) diff --git a/pkg/conversion/decode.go b/pkg/conversion/decode.go index f8ef14d1798..1c98fae9bbf 100644 --- a/pkg/conversion/decode.go +++ b/pkg/conversion/decode.go @@ -17,13 +17,12 @@ limitations under the License. package conversion import ( + "encoding/json" "errors" "fmt" - - "github.com/ghodss/yaml" ) -// Decode converts a YAML or JSON string back into a pointer to an api object. +// Decode converts a JSON string back into a pointer to an api object. // Deduces the type based upon the fields added by the MetaInsertionFactory // technique. The object will be converted, if necessary, into the // s.InternalVersion type before being returned. Decode will not decode @@ -44,16 +43,12 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) { return nil, err } - // yaml is a superset of json, so we use it to decode here. That way, - // we understand both. - err = yaml.Unmarshal(data, obj) - if err != nil { + if err := json.Unmarshal(data, obj); err != nil { return nil, err } // Version and Kind should be blank in memory. - err = s.SetVersionAndKind("", "", obj) - if err != nil { + if err := s.SetVersionAndKind("", "", obj); err != nil { return nil, err } @@ -63,8 +58,7 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) { if err != nil { return nil, err } - err = s.converter.Convert(obj, objOut, 0, s.generateConvertMeta(version, s.InternalVersion)) - if err != nil { + if err := s.converter.Convert(obj, objOut, 0, s.generateConvertMeta(version, s.InternalVersion)); err != nil { return nil, err } obj = objOut @@ -72,16 +66,13 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) { return obj, nil } -// DecodeInto parses a YAML or JSON string and stores it in obj. Returns an error +// DecodeInto parses a JSON string and stores it in obj. Returns an error // if data.Kind is set and doesn't match the type of obj. Obj should be a // pointer to an api type. // If obj's version doesn't match that in data, an attempt will be made to convert // data into obj's version. func (s *Scheme) DecodeInto(data []byte, obj interface{}) error { if len(data) == 0 { - // This is valid YAML, but it's a bad idea not to return an error - // for an empty string-- that's almost certainly not what the caller - // was expecting. return errors.New("empty input") } dataVersion, dataKind, err := s.DataVersionAndKind(data) @@ -107,14 +98,10 @@ func (s *Scheme) DecodeInto(data []byte, obj interface{}) error { if err != nil { return err } - // yaml is a superset of json, so we use it to decode here. That way, - // we understand both. - err = yaml.Unmarshal(data, external) - if err != nil { + if err := json.Unmarshal(data, external); err != nil { return err } - err = s.converter.Convert(external, obj, 0, s.generateConvertMeta(dataVersion, objVersion)) - if err != nil { + if err := s.converter.Convert(external, obj, 0, s.generateConvertMeta(dataVersion, objVersion)); err != nil { return err } diff --git a/pkg/conversion/meta.go b/pkg/conversion/meta.go index 18c0d966e93..5dccd560303 100644 --- a/pkg/conversion/meta.go +++ b/pkg/conversion/meta.go @@ -17,10 +17,9 @@ limitations under the License. package conversion import ( + "encoding/json" "fmt" "reflect" - - "github.com/ghodss/yaml" ) // MetaFactory is used to store and retrieve the version and kind @@ -33,13 +32,13 @@ type MetaFactory interface { Interpret(data []byte) (version, kind string, err error) } -// DefaultMetaFactory is a default factory for versioning objects in JSON/YAML. The object +// DefaultMetaFactory is a default factory for versioning objects in JSON. The object // in memory and in the default JSON serialization will use the "kind" and "apiVersion" // fields. var DefaultMetaFactory = SimpleMetaFactory{KindField: "Kind", VersionField: "APIVersion"} // SimpleMetaFactory provides default methods for retrieving the type and version of objects -// that are identified with an "apiVersion" and "kind" fields in their JSON/YAML +// that are identified with an "apiVersion" and "kind" fields in their JSON // serialization. It may be parameterized with the names of the fields in memory, or an // optional list of base structs to search for those fields in memory. type SimpleMetaFactory struct { @@ -51,16 +50,14 @@ type SimpleMetaFactory struct { BaseFields []string } -// Interpret will return the APIVersion and Kind of the JSON/YAML wire-format +// Interpret will return the APIVersion and Kind of the JSON wire-format // encoding of an object, or an error. func (SimpleMetaFactory) Interpret(data []byte) (version, kind string, err error) { findKind := struct { APIVersion string `json:"apiVersion,omitempty"` Kind string `json:"kind,omitempty"` }{} - // yaml is a superset of json, so we use it to decode here. That way, - // we understand both. - err = yaml.Unmarshal(data, &findKind) + err = json.Unmarshal(data, &findKind) if err != nil { return "", "", fmt.Errorf("couldn't get version/kind: %v", err) } diff --git a/pkg/kubectl/resource/mapper.go b/pkg/kubectl/resource/mapper.go index 874eddbe87b..bb97e08c144 100644 --- a/pkg/kubectl/resource/mapper.go +++ b/pkg/kubectl/resource/mapper.go @@ -22,6 +22,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/yaml" ) // Mapper is a convenience struct for holding references to the three interfaces @@ -36,6 +37,11 @@ type Mapper struct { // if any of the decoding or client lookup steps fail. Name and namespace will be // set into Info if the mapping's MetadataAccessor can retrieve them. func (m *Mapper) InfoForData(data []byte, source string) (*Info, error) { + json, err := yaml.ToJSON(data) + if err != nil { + return nil, fmt.Errorf("unable to parse %q: %v", err) + } + data = json version, kind, err := m.DataVersionAndKind(data) if err != nil { return nil, fmt.Errorf("unable to get type info from %q: %v", source, err) diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go index cee1ef07a18..ba5f5c4232c 100644 --- a/pkg/kubectl/resource_printer_test.go +++ b/pkg/kubectl/resource_printer_test.go @@ -196,7 +196,7 @@ func testPrinter(t *testing.T, printer ResourcePrinter, unmarshalFunc func(data } // Use real decode function to undo the versioning process. poutput = testStruct{} - err = testapi.Codec().DecodeInto(buf.Bytes(), &poutput) + err = runtime.YAMLDecoder(testapi.Codec()).DecodeInto(buf.Bytes(), &poutput) if err != nil { t.Fatal(err) } @@ -217,7 +217,7 @@ func testPrinter(t *testing.T, printer ResourcePrinter, unmarshalFunc func(data } // Use real decode function to undo the versioning process. objOut = api.Pod{} - err = testapi.Codec().DecodeInto(buf.Bytes(), &objOut) + err = runtime.YAMLDecoder(testapi.Codec()).DecodeInto(buf.Bytes(), &objOut) if err != nil { t.Fatal(err) }