From a73e4f4623816e87efb1cc7368dbc16c1bc9ed0c Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 31 Jul 2014 12:02:49 -0700 Subject: [PATCH 1/3] Separate generic parts of api library into conversion package. --- pkg/conversion/converter.go | 235 +++++++++++++++++++++++++++++ pkg/conversion/converter_test.go | 81 ++++++++++ pkg/conversion/decode.go | 129 ++++++++++++++++ pkg/conversion/encode.go | 136 +++++++++++++++++ pkg/conversion/scheme.go | 223 +++++++++++++++++++++++++++ pkg/conversion/scheme_test.go | 248 +++++++++++++++++++++++++++++++ 6 files changed, 1052 insertions(+) create mode 100644 pkg/conversion/converter.go create mode 100644 pkg/conversion/converter_test.go create mode 100644 pkg/conversion/decode.go create mode 100644 pkg/conversion/encode.go create mode 100644 pkg/conversion/scheme.go create mode 100644 pkg/conversion/scheme_test.go diff --git a/pkg/conversion/converter.go b/pkg/conversion/converter.go new file mode 100644 index 00000000000..025d7c8cc88 --- /dev/null +++ b/pkg/conversion/converter.go @@ -0,0 +1,235 @@ +/* +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 conversion + +import ( + "fmt" + "reflect" +) + +type typePair struct { + source reflect.Type + dest reflect.Type +} + +// DebugLogger allows you to get debugging messages if necessary. +type DebugLogger interface { + Logf(format string, args ...interface{}) +} + +// Converter knows how to convert one type to another. +type Converter struct { + // Map from the conversion pair to a function which can + // do the conversion. + funcs map[typePair]reflect.Value + + // If true, print helpful debugging info. Quite verbose. + Debug DebugLogger +} + +// NewConverter makes a new Converter object. +func NewConverter() *Converter { + return &Converter{ + funcs: map[typePair]reflect.Value{}, + } +} + +// Register registers a conversion func with the Converter. conversionFunc must take +// two parameters, the input and output type. It must take a pointer to each. It must +// return an error. +// +// Example: +// c.Register(func(in *Pod, out *v1beta1.Pod) error { ... return nil }) +func (c *Converter) Register(conversionFunc interface{}) error { + fv := reflect.ValueOf(conversionFunc) + ft := fv.Type() + if ft.Kind() != reflect.Func { + return fmt.Errorf("expected func, got: %v", ft) + } + if ft.NumIn() != 2 { + return fmt.Errorf("expected two in params, got: %v", ft) + } + if ft.NumOut() != 1 { + return fmt.Errorf("expected one out param, got: %v", ft) + } + if ft.In(0).Kind() != reflect.Ptr { + return fmt.Errorf("expected pointer arg for in param 0, got: %v", ft) + } + if ft.In(1).Kind() != reflect.Ptr { + return fmt.Errorf("expected pointer arg for in param 1, got: %v", ft) + } + var forErrorType error + // This convolution is necessary, otherwise TypeOf picks up on the fact + // that forErrorType is nil. + errorType := reflect.TypeOf(&forErrorType).Elem() + if ft.Out(0) != errorType { + return fmt.Errorf("expected error return, got: %v", ft) + } + c.funcs[typePair{ft.In(0).Elem(), ft.In(1).Elem()}] = fv + return nil +} + +// FieldMatchingType contains a list of ways in which struct fields could be +// copied. These constants may be | combined. +type FieldMatchingFlags int + +const ( + // Loop through source fields, search for matching dest field + // to copy it into. Destination fields with no corresponding + // source field will be ignored. + SourceToDest FieldMatchingFlags = 1 << iota + // Loop through destiation fields, search for matching source + // field to copy it from. Source fields with no corresponding + // destination field will be ignored. If SourceToDest is + // specified, this flag is ignored. If niether is specified, + // this flag is the default. + DestFromSource + // Don't treat it as an error if the corresponding source or + // dest field can't be found. + IgnoreMissingFields + // Don't require type names to match. + AllowDifferentFieldNames +) + +// Returns true if the given flag or combination of flags is set. +func (f FieldMatchingFlags) IsSet(flag FieldMatchingFlags) bool { + return f&flag == flag +} + +// Convert will translate src to dest if it knows how. Both must be pointers. +// If no conversion func is registered and the default copying mechanism +// doesn't work on this type pair, an error will be returned. +// Not safe for objects with cyclic references! +func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags) error { + dv, sv := reflect.ValueOf(dest), reflect.ValueOf(src) + if dv.Kind() != reflect.Ptr { + return fmt.Errorf("Need pointer, but got %#v", dest) + } + if sv.Kind() != reflect.Ptr { + return fmt.Errorf("Need pointer, but got %#v", src) + } + dv = dv.Elem() + sv = sv.Elem() + if !dv.CanAddr() { + return fmt.Errorf("Can't write to dest") + } + return c.convert(sv, dv, flags) +} + +// convert recursively copies sv into dv, calling an appropriate conversion function if +// one is registered. +func (c *Converter) convert(sv, dv reflect.Value, flags FieldMatchingFlags) error { + dt, st := dv.Type(), sv.Type() + if fv, ok := c.funcs[typePair{st, dt}]; ok { + if c.Debug != nil { + c.Debug.Logf("Calling custom conversion of '%v' to '%v'", st, dt) + } + ret := fv.Call([]reflect.Value{sv.Addr(), dv.Addr()})[0].Interface() + // This convolution is necssary because nil interfaces won't convert + // to errors. + if ret == nil { + return nil + } + return ret.(error) + } + + if !flags.IsSet(AllowDifferentFieldNames) && dt.Name() != st.Name() { + return fmt.Errorf("Can't convert %v to %v because type names don't match.", st, dt) + } + + // This should handle all simple types. + if st.AssignableTo(dt) { + dv.Set(sv) + return nil + } + if st.ConvertibleTo(dt) { + dv.Set(sv.Convert(dt)) + return nil + } + + if c.Debug != nil { + c.Debug.Logf("Trying to convert '%v' to '%v'", st, dt) + } + + switch dv.Kind() { + case reflect.Struct: + listType := dt + if flags.IsSet(SourceToDest) { + listType = st + } + for i := 0; i < listType.NumField(); i++ { + f := listType.Field(i) + df := dv.FieldByName(f.Name) + sf := sv.FieldByName(f.Name) + if !df.IsValid() || !sf.IsValid() { + switch { + case flags.IsSet(IgnoreMissingFields): + // No error. + case flags.IsSet(SourceToDest): + return fmt.Errorf("%v not present in dest (%v to %v)", f.Name, st, dt) + default: + return fmt.Errorf("%v not present in src (%v to %v)", f.Name, st, dt) + } + continue + } + if err := c.convert(sf, df, flags); err != nil { + return err + } + } + case reflect.Slice: + if sv.IsNil() { + // Don't make a zero-length slice. + dv.Set(reflect.Zero(dt)) + return nil + } + dv.Set(reflect.MakeSlice(dt, sv.Len(), sv.Cap())) + for i := 0; i < sv.Len(); i++ { + if err := c.convert(sv.Index(i), dv.Index(i), flags); err != nil { + return err + } + } + case reflect.Ptr: + if sv.IsNil() { + // Don't copy a nil ptr! + dv.Set(reflect.Zero(dt)) + return nil + } + dv.Set(reflect.New(dt.Elem())) + return c.convert(sv.Elem(), dv.Elem(), flags) + case reflect.Map: + if sv.IsNil() { + // Don't copy a nil ptr! + dv.Set(reflect.Zero(dt)) + return nil + } + dv.Set(reflect.MakeMap(dt)) + for _, sk := range sv.MapKeys() { + dk := reflect.New(dt.Key()).Elem() + if err := c.convert(sk, dk, flags); err != nil { + return err + } + dkv := reflect.New(dt.Elem()).Elem() + if err := c.convert(sv.MapIndex(sk), dkv, flags); err != nil { + return err + } + dv.SetMapIndex(dk, dkv) + } + default: + return fmt.Errorf("Couldn't copy '%v' into '%v'", st, dt) + } + return nil +} diff --git a/pkg/conversion/converter_test.go b/pkg/conversion/converter_test.go new file mode 100644 index 00000000000..c79e2455d81 --- /dev/null +++ b/pkg/conversion/converter_test.go @@ -0,0 +1,81 @@ +/* +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 conversion + +import ( + "fmt" + "testing" +) + +func TestConverter(t *testing.T) { + type A struct { + Foo string + } + type B struct { + Bar string + } + type C struct{} + c := NewConverter() + err := c.Register(func(in *A, out *B) error { + out.Bar = in.Foo + return nil + }) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + err = c.Register(func(in *B, out *A) error { + out.Foo = in.Bar + return nil + }) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + + x := A{"hello, intrepid test reader!"} + y := B{} + + err = c.Convert(&x, &y, 0) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + if e, a := x.Foo, y.Bar; e != a { + t.Errorf("expected %v, got %v", e, a) + } + + z := B{"all your test are belong to us"} + w := A{} + + err = c.Convert(&z, &w, 0) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + if e, a := z.Bar, w.Foo; e != a { + t.Errorf("expected %v, got %v", e, a) + } + + err = c.Register(func(in *A, out *C) error { + return fmt.Errorf("C can't store an A, silly") + }) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + + err = c.Convert(&A{}, &C{}, 0) + if err == nil { + t.Errorf("unexpected non-error") + } +} diff --git a/pkg/conversion/decode.go b/pkg/conversion/decode.go new file mode 100644 index 00000000000..7c0da976f3b --- /dev/null +++ b/pkg/conversion/decode.go @@ -0,0 +1,129 @@ +/* +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 conversion + +import ( + "fmt" + + "gopkg.in/v1/yaml" +) + +// Decode converts a YAML or 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 refuse to decode +// objects without a version, because that's probably an error. +func (s *Scheme) Decode(data []byte) (interface{}, error) { + version, kind, err := s.DataAPIVersionAndKind(data) + if err != nil { + return nil, err + } + if version == "" { + return nil, fmt.Errorf("API Version not set in '%s'", string(data)) + } + obj, err := s.NewObject(version, kind) + if err != nil { + 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 { + return nil, err + } + + // Version and Kind should be blank in memory. + blankVersionAndKind := s.MetaInsertionFactory.Create("", "") + err = s.converter.Convert(blankVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + if err != nil { + return nil, err + } + + // Convert if needed. + if s.InternalVersion != version { + objOut, err := s.NewObject(s.InternalVersion, kind) + if err != nil { + return nil, err + } + err = s.converter.Convert(obj, objOut, 0) + if err != nil { + return nil, err + } + obj = objOut + } + return obj, nil +} + +// DecodeInto parses a YAML or 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 APIVersion 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 { + dataVersion, dataKind, err := s.DataAPIVersionAndKind(data) + if err != nil { + return err + } + objVersion, objKind, err := s.ObjectAPIVersionAndKind(obj) + if err != nil { + return err + } + if dataKind == "" { + // Assume objects with unset Kind fields are being unmarshalled into the + // correct type. + dataKind = objKind + } + if dataKind != objKind { + return fmt.Errorf("data of kind '%v', obj of type '%v'", dataKind, objKind) + } + if dataVersion == "" { + // Assume objects with unset Version fields are being unmarshalled into the + // correct type. + dataVersion = objVersion + } + + if objVersion == dataVersion { + // Easy case! + err = yaml.Unmarshal(data, obj) + if err != nil { + return err + } + } else { + external, err := s.NewObject(dataVersion, dataKind) + if err != nil { + return fmt.Errorf("Unable to create new object of type ('%s', '%s')", dataVersion, dataKind) + } + // 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 { + return err + } + err = s.converter.Convert(external, obj, 0) + if err != nil { + return err + } + } + + // Version and Kind should be blank in memory. + blankVersionAndKind := s.MetaInsertionFactory.Create("", "") + err = s.converter.Convert(blankVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + if err != nil { + return err + } + return nil +} diff --git a/pkg/conversion/encode.go b/pkg/conversion/encode.go new file mode 100644 index 00000000000..29b0ab5a0f3 --- /dev/null +++ b/pkg/conversion/encode.go @@ -0,0 +1,136 @@ +/* +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 conversion + +import ( + "bytes" + "encoding/json" + "fmt" +) + +// EncodeOrDie is a version of Encode which will panic instead of returning an error. For tests. +func (s *Scheme) EncodeOrDie(obj interface{}) string { + bytes, err := s.Encode(obj) + if err != nil { + panic(err) + } + return string(bytes) +} + +// Encode turns the given api object into an appropriate JSON string. +// Obj may be a pointer to a struct, or a struct. If a struct, a copy +// will be made, therefore it's recommended to pass a pointer to a +// struct. The type must have been registered. +// +// Memory/wire format differences: +// * Having to keep track of the Kind and APIVersion fields makes tests +// very annoying, so the rule is that they are set only in wire format +// (json), not when in native (memory) format. This is possible because +// both pieces of information are implicit in the go typed object. +// * An exception: note that, if there are embedded API objects of known +// type, for example, PodList{... Items []Pod ...}, these embedded +// objects must be of the same version of the object they are embedded +// within, and their APIVersion and Kind must both be empty. +// * Note that the exception does not apply to the APIObject type, which +// recursively does Encode()/Decode(), and is capable of expressing any +// API object. +// * Only versioned objects should be encoded. This means that, if you pass +// a native object, Encode will convert it to a versioned object. For +// example, an api.Pod will get converted to a v1beta1.Pod. However, if +// you pass in an object that's already versioned (v1beta1.Pod), Encode +// will not modify it. +// +// The purpose of the above complex conversion behavior is to allow us to +// change the memory format yet not break compatibility with any stored +// objects, whether they be in our storage layer (e.g., etcd), or in user's +// config files. +// +func (s *Scheme) Encode(obj interface{}) (data []byte, err error) { + return s.EncodeToVersion(obj, s.ExternalVersion) +} + +// EncodeToVersion is like Encode, but you may choose the version. +func (s *Scheme) EncodeToVersion(obj interface{}, destVersion string) (data []byte, err error) { + 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()) + } + + objVersion, objKind, err := s.ObjectAPIVersionAndKind(obj) + if err != nil { + return nil, err + } + + // Perform a conversion if necessary. + if objVersion != destVersion { + objOut, err := s.NewObject(destVersion, objKind) + if err != nil { + return nil, err + } + err = s.converter.Convert(obj, objOut, 0) + if err != nil { + return nil, err + } + obj = objOut + } + + // Version and Kind should be set on the wire. + setVersionAndKind := s.MetaInsertionFactory.Create(destVersion, objKind) + err = s.converter.Convert(setVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + if err != nil { + return nil, err + } + + // To add metadata, do some simple surgery on the JSON. + data, err = json.Marshal(obj) + if err != nil { + return nil, err + } + + // Version and Kind should be blank in memory. + blankVersionAndKind := s.MetaInsertionFactory.Create("", "") + err = s.converter.Convert(blankVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + if err != nil { + return nil, err + } + + return data, nil + + meta, err := json.Marshal(s.MetaInsertionFactory.Create(destVersion, objKind)) + if err != nil { + return nil, err + } + // Stick these together, omitting the last } from meta and the first { from + // data. Add a comma to meta if necessary. + metaN := len(meta) + if len(data) > 2 { + meta[metaN-1] = ',' // Add comma + } else { + meta = meta[:metaN-1] // Just remove } + } + together := append(meta, data[1:]...) + if s.Indent { + var out bytes.Buffer + err := json.Indent(&out, together, "", " ") + if err != nil { + return nil, err + } + return out.Bytes(), nil + } + return together, nil +} diff --git a/pkg/conversion/scheme.go b/pkg/conversion/scheme.go new file mode 100644 index 00000000000..7f49b0cee47 --- /dev/null +++ b/pkg/conversion/scheme.go @@ -0,0 +1,223 @@ +/* +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 conversion + +import ( + "fmt" + "reflect" + + "gopkg.in/v1/yaml" +) + +// MetaInsertionFactory is used to create an object to store and retrieve +// the version and kind information for all objects. The default uses the +// keys "apiVersion" and "kind" respectively. The object produced by this +// factory is used to clear the version and kind fields in memory, so it +// must match the layout of your actual api structs. (E.g., if you have your +// version and kind field inside an inlined struct, this must produce an +// inlined struct with the same field name.) +type MetaInsertionFactory interface { + // Create should make a new object with two fields. + // This object will be used to encode this metadata along with your + // API objects, so the tags on the fields you use shouldn't conflict. + Create(apiVersion, kind string) interface{} + // Interpret should take the same type of object that Create creates. + // It should return the version and kind information from this object. + Interpret(interface{}) (apiVersion, kind string) +} + +// Default is a global scheme. +var Default = NewScheme() + +// Scheme defines an entire encoding and decoding scheme. +type Scheme struct { + // versionMap allows one to figure out the go type of an object with + // the given version and name. + versionMap map[string]map[string]reflect.Type + + // typeToVersion allows one to figure out the version for a given go object. + // The reflect.Type we index by should *not* be a pointer. If the same type + // is registered for multiple versions, the last one wins. + typeToVersion map[reflect.Type]string + + // converter stores all registered conversion functions. It also has + // default coverting behavior. + converter *Converter + + // Indent will cause the JSON output from Encode to be indented, iff it is true. + Indent bool + + // InternalVersion is the default internal version. It is recommended that + // you use "" for the internal version. + InternalVersion string + + // ExternalVersion is the default external version. + ExternalVersion string + + // MetaInsertionFactory is used to create an object to store and retrieve + // the version and kind information for all objects. The default uses the + // keys "apiVersion" and "kind" respectively. + MetaInsertionFactory MetaInsertionFactory +} + +// NewScheme manufactures a new scheme. +func NewScheme() *Scheme { + return &Scheme{ + versionMap: map[string]map[string]reflect.Type{}, + typeToVersion: map[reflect.Type]string{}, + converter: NewConverter(), + InternalVersion: "", + ExternalVersion: "v1", + MetaInsertionFactory: metaInsertion{}, + } +} + +// AddKnownTypes registers the types of the arguments to the marshaller of the package api. +// Encode() refuses the object unless its type is registered with AddKnownTypes. +func (s *Scheme) AddKnownTypes(version string, types ...interface{}) { + knownTypes, found := s.versionMap[version] + if !found { + knownTypes = map[string]reflect.Type{} + s.versionMap[version] = knownTypes + } + for _, obj := range types { + t := reflect.TypeOf(obj) + if t.Kind() != reflect.Struct { + panic("All types must be structs.") + } + knownTypes[t.Name()] = t + s.typeToVersion[t] = version + } +} + +// NewObject returns a new object of the given version and name, +// or an error if it hasn't been registered. +func (s *Scheme) NewObject(versionName, typeName string) (interface{}, error) { + if types, ok := s.versionMap[versionName]; ok { + if t, ok := types[typeName]; ok { + return reflect.New(t).Interface(), nil + } + return nil, fmt.Errorf("No type '%v' for version '%v'", typeName, versionName) + } + return nil, fmt.Errorf("No version '%v'", versionName) +} + +// AddConversionFuncs adds functions to the list of conversion functions. The given +// functions should know how to convert between two API objects. We deduce how to call +// it from the types of its two parameters; see the comment for Converter.Register. +// +// Note that, if you need to copy sub-objects that didn't change, it's safe to call +// Convert() inside your conversionFuncs, as long as you don't start a conversion +// chain that's infinitely recursive. +// +// Also note that the default behavior, if you don't add a conversion function, is to +// sanely copy fields that have the same names. It's OK if the destination type has +// extra fields, but it must not remove any. So you only need to add a conversion +// function for things with changed/removed fields. +func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error { + for _, f := range conversionFuncs { + err := s.converter.Register(f) + if err != nil { + return err + } + } + return nil +} + +// Convert will attempt to convert in into out. Both must be pointers. +// For easy testing of conversion functions. Returns an error if the conversion isn't +// possible. +func (s *Scheme) Convert(in, out interface{}) error { + return s.converter.Convert(in, out, 0) +} + +// metaInsertion provides a default implementation of MetaInsertionFactory. +type metaInsertion struct { + JSONBase struct { + APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"` + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + } `json:",inline" yaml:",inline"` +} + +// Create should make a new object with two fields. +// This object will be used to encode this metadata along with your +// API objects, so the tags on the fields you use shouldn't conflict. +func (metaInsertion) Create(apiVersion, kind string) interface{} { + m := metaInsertion{} + m.JSONBase.APIVersion = apiVersion + m.JSONBase.Kind = kind + return &m +} + +// Interpret should take the same type of object that Create creates. +// It should return the version and kind information from this object. +func (metaInsertion) Interpret(in interface{}) (apiVersion, kind string) { + m := in.(*metaInsertion) + return m.JSONBase.APIVersion, m.JSONBase.Kind +} + +// DataAPIVersionAndKind will return the APIVersion and Kind of the given wire-format +// enconding of an API Object, or an error. +func (s *Scheme) DataAPIVersionAndKind(data []byte) (apiVersion, kind string, err error) { + findKind := s.MetaInsertionFactory.Create("", "") + // yaml is a superset of json, so we use it to decode here. That way, + // we understand both. + err = yaml.Unmarshal(data, findKind) + if err != nil { + return "", "", fmt.Errorf("couldn't get version/kind: %v", err) + } + apiVersion, kind = s.MetaInsertionFactory.Interpret(findKind) + return apiVersion, kind, nil +} + +// ObjectVersionAndKind returns the API version and kind of the go object, +// or an error if it's not a pointer or is unregistered. +func (s *Scheme) ObjectAPIVersionAndKind(obj interface{}) (apiVersion, kind string, err error) { + v, err := enforcePtr(obj) + if err != nil { + return "", "", err + } + t := v.Type() + if version, ok := s.typeToVersion[t]; !ok { + return "", "", fmt.Errorf("Unregistered type: %v", t) + } else { + return version, t.Name(), nil + } +} + +// maybeCopy copies obj if it is not a pointer, to get a settable/addressable +// object. Guaranteed to return a pointer. +func maybeCopy(obj interface{}) interface{} { + v := reflect.ValueOf(obj) + if v.Kind() == reflect.Ptr { + return obj + } + v2 := reflect.New(v.Type()) + v2.Elem().Set(v) + return v2.Interface() +} + +// Ensures that obj is a pointer of some sort. Returns a reflect.Value of the +// dereferenced pointer, ensuring that it is settable/addressable. +// Returns an error if this is not possible. +func enforcePtr(obj interface{}) (reflect.Value, error) { + v := reflect.ValueOf(obj) + if v.Kind() != reflect.Ptr { + return reflect.Value{}, fmt.Errorf("expected pointer, but got %v", v.Type().Name()) + } + return v.Elem(), nil +} diff --git a/pkg/conversion/scheme_test.go b/pkg/conversion/scheme_test.go new file mode 100644 index 00000000000..ad5aa610bd6 --- /dev/null +++ b/pkg/conversion/scheme_test.go @@ -0,0 +1,248 @@ +/* +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 conversion + +import ( + "encoding/json" + "flag" + "fmt" + "reflect" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" +) + +var fuzzIters = flag.Int("fuzz_iters", 10, "How many fuzzing iterations to do.") + +// Intended to be compatible with the default implementation of MetaInsertionFactory +type JSONBase struct { + ID string `yaml:"ID,omitempty" json:"ID,omitempty"` + APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"` + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` +} + +type TestType1 struct { + JSONBase `json:",inline" yaml:",inline"` + A string `yaml:"A,omitempty" json:"A,omitempty"` + B int `yaml:"B,omitempty" json:"B,omitempty"` + C int8 `yaml:"C,omitempty" json:"C,omitempty"` + D int16 `yaml:"D,omitempty" json:"D,omitempty"` + E int32 `yaml:"E,omitempty" json:"E,omitempty"` + F int64 `yaml:"F,omitempty" json:"F,omitempty"` + G uint `yaml:"G,omitempty" json:"G,omitempty"` + H uint8 `yaml:"H,omitempty" json:"H,omitempty"` + I uint16 `yaml:"I,omitempty" json:"I,omitempty"` + J uint32 `yaml:"J,omitempty" json:"J,omitempty"` + K uint64 `yaml:"K,omitempty" json:"K,omitempty"` + L bool `yaml:"L,omitempty" json:"L,omitempty"` + M map[string]int `yaml:"M,omitempty" json:"M,omitempty"` + N map[string]TestType2 `yaml:"N,omitempty" json:"N,omitempty"` + O *TestType2 `yaml:"O,omitempty" json:"O,omitempty"` + P []TestType2 `yaml:"Q,omitempty" json:"Q,omitempty"` +} + +type TestType2 struct { + A string `yaml:"A,omitempty" json:"A,omitempty"` + B int `yaml:"B,omitempty" json:"B,omitempty"` +} + +// We depend on the name of the external and internal types matching. Ordinarily, +// we'd accomplish this with an additional package, but since this is a test, we +// can just enclose stuff in a function to simulate that. +func externalTypeReturn() interface{} { + type TestType2 struct { + A string `yaml:"A,omitempty" json:"A,omitempty"` + B int `yaml:"B,omitempty" json:"B,omitempty"` + } + type TestType1 struct { + JSONBase `json:",inline" yaml:",inline"` + A string `yaml:"A,omitempty" json:"A,omitempty"` + B int `yaml:"B,omitempty" json:"B,omitempty"` + C int8 `yaml:"C,omitempty" json:"C,omitempty"` + D int16 `yaml:"D,omitempty" json:"D,omitempty"` + E int32 `yaml:"E,omitempty" json:"E,omitempty"` + F int64 `yaml:"F,omitempty" json:"F,omitempty"` + G uint `yaml:"G,omitempty" json:"G,omitempty"` + H uint8 `yaml:"H,omitempty" json:"H,omitempty"` + I uint16 `yaml:"I,omitempty" json:"I,omitempty"` + J uint32 `yaml:"J,omitempty" json:"J,omitempty"` + K uint64 `yaml:"K,omitempty" json:"K,omitempty"` + L bool `yaml:"L,omitempty" json:"L,omitempty"` + M map[string]int `yaml:"M,omitempty" json:"M,omitempty"` + N map[string]TestType2 `yaml:"N,omitempty" json:"N,omitempty"` + O *TestType2 `yaml:"O,omitempty" json:"O,omitempty"` + P []TestType2 `yaml:"Q,omitempty" json:"Q,omitempty"` + } + return TestType1{} +} + +type ExternalInternalSame struct { + JSONBase `json:",inline" yaml:",inline"` + A TestType2 `yaml:"A,omitempty" json:"A,omitempty"` +} + +// TestObjectFuzzer can randomly populate all the above objects. +var TestObjectFuzzer = util.NewFuzzer( + func(j *JSONBase) { + // We have to customize the randomization of JSONBases because their + // APIVersion and Kind must remain blank in memory. + j.APIVersion = "" + j.Kind = "" + j.ID = util.RandString() + }, + func(u *uint64) { + // TODO: Fix JSON/YAML packages and/or write custom encoding + // for uint64's. Somehow the LS *byte* of this is lost, but + // only when all 8 bytes are set. + *u = util.RandUint64() >> 8 + }, + func(u *uint) { + // TODO: Fix JSON/YAML packages and/or write custom encoding + // for uint64's. Somehow the LS *byte* of this is lost, but + // only when all 8 bytes are set. + *u = uint(util.RandUint64() >> 8) + }, +) + +// Returns a new Scheme set up with the test objects. +func GetTestScheme() *Scheme { + s := NewScheme() + s.AddKnownTypes("", TestType1{}, ExternalInternalSame{}) + s.AddKnownTypes("v1", externalTypeReturn(), ExternalInternalSame{}) + s.ExternalVersion = "v1" + s.InternalVersion = "" + return s +} + +func objDiff(a, b interface{}) string { + ab, err := json.Marshal(a) + if err != nil { + panic("a") + } + bb, err := json.Marshal(b) + if err != nil { + panic("b") + } + return util.StringDiff(string(ab), string(bb)) + + // An alternate diff attempt, in case json isn't showing you + // the difference. (reflect.DeepEqual makes a distinction between + // nil and empty slices, for example.) + return util.StringDiff( + fmt.Sprintf("%#v", a), + fmt.Sprintf("%#v", b), + ) +} + +func runTest(t *testing.T, source interface{}) { + name := reflect.TypeOf(source).Elem().Name() + TestObjectFuzzer.Fuzz(source) + + s := GetTestScheme() + data, err := s.Encode(source) + if err != nil { + t.Errorf("%v: %v (%#v)", name, err, source) + return + } + obj2, err := s.Decode(data) + if err != nil { + t.Errorf("%v: %v (%v)", name, err, string(data)) + return + } else { + if !reflect.DeepEqual(source, obj2) { + t.Errorf("1: %v: diff: %v", name, objDiff(source, obj2)) + return + } + } + obj3 := reflect.New(reflect.TypeOf(source).Elem()).Interface() + err = s.DecodeInto(data, obj3) + if err != nil { + t.Errorf("2: %v: %v", name, err) + return + } else { + if !reflect.DeepEqual(source, obj3) { + t.Errorf("3: %v: diff: %v", name, objDiff(source, obj3)) + return + } + } +} + +func TestTypes(t *testing.T) { + table := []interface{}{ + &TestType1{}, + &ExternalInternalSame{}, + } + for _, item := range table { + // Try a few times, since runTest uses random values. + for i := 0; i < *fuzzIters; i++ { + runTest(t, item) + } + } +} + +func TestEncode_NonPtr(t *testing.T) { + s := GetTestScheme() + tt := TestType1{A: "I'm not a pointer object"} + obj := interface{}(tt) + data, err := s.Encode(obj) + obj2, err2 := s.Decode(data) + if err != nil || err2 != nil { + t.Fatalf("Failure: '%v' '%v'", err, err2) + } + if _, ok := obj2.(*TestType1); !ok { + t.Fatalf("Got wrong type") + } + if !reflect.DeepEqual(obj2, &tt) { + t.Errorf("Expected:\n %#v,\n Got:\n %#v", &tt, obj2) + } +} + +func TestEncode_Ptr(t *testing.T) { + s := GetTestScheme() + tt := &TestType1{A: "I am a pointer object"} + obj := interface{}(tt) + data, err := s.Encode(obj) + obj2, err2 := s.Decode(data) + if err != nil || err2 != nil { + t.Fatalf("Failure: '%v' '%v'", err, err2) + } + if _, ok := obj2.(*TestType1); !ok { + t.Fatalf("Got wrong type") + } + if !reflect.DeepEqual(obj2, tt) { + t.Errorf("Expected:\n %#v,\n Got:\n %#v", &tt, obj2) + } +} + +func TestBadJSONRejection(t *testing.T) { + s := GetTestScheme() + badJSONs := [][]byte{ + []byte(`{"apiVersion":"v1"}`), // Missing kind + []byte(`{"kind":"TestType1"}`), // Missing version + []byte(`{"apiVersion":"v1","kind":"bar"}`), // Unknown kind + []byte(`{"apiVersion":"bar","kind":"TestType1"}`), // Unknown version + } + for _, b := range badJSONs { + if _, err := s.Decode(b); err == nil { + t.Errorf("Did not reject bad json: %s", string(b)) + } + } + badJSONKindMismatch := []byte(`{"apiVersion":"v1","kind": "ExternalInternalSame"}`) + if err := s.DecodeInto(badJSONKindMismatch, &TestType1{}); err == nil { + t.Errorf("Kind is set but doesn't match the object type: %s", badJSONKindMismatch) + } +} From 5c0f5e85e23c46a7d1c112b645c8d7e59a844a65 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 31 Jul 2014 12:10:15 -0700 Subject: [PATCH 2/3] Make api use converter package. --- pkg/api/converter.go | 195 --------------------------- pkg/api/converter_test.go | 110 --------------- pkg/api/helper.go | 273 ++------------------------------------ 3 files changed, 13 insertions(+), 565 deletions(-) delete mode 100644 pkg/api/converter.go delete mode 100644 pkg/api/converter_test.go diff --git a/pkg/api/converter.go b/pkg/api/converter.go deleted file mode 100644 index f82dd772d23..00000000000 --- a/pkg/api/converter.go +++ /dev/null @@ -1,195 +0,0 @@ -/* -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 api - -import ( - "fmt" - "reflect" -) - -type typePair struct { - source reflect.Type - dest reflect.Type -} - -type debugLogger interface { - Logf(format string, args ...interface{}) -} - -// Converter knows how to convert one type to another. -type Converter struct { - // Map from the conversion pair to a function which can - // do the conversion. - funcs map[typePair]reflect.Value - - // If true, print helpful debugging info. Quite verbose. - debug debugLogger -} - -// NewConverter makes a new Converter object. -func NewConverter() *Converter { - return &Converter{ - funcs: map[typePair]reflect.Value{}, - } -} - -// Register registers a conversion func with the Converter. conversionFunc must take -// two parameters, the input and output type. It must take a pointer to each. It must -// return an error. -// -// Example: -// c.Register(func(in *Pod, out *v1beta1.Pod) error { ... return nil }) -func (c *Converter) Register(conversionFunc interface{}) error { - fv := reflect.ValueOf(conversionFunc) - ft := fv.Type() - if ft.Kind() != reflect.Func { - return fmt.Errorf("expected func, got: %v", ft) - } - if ft.NumIn() != 2 { - return fmt.Errorf("expected two in params, got: %v", ft) - } - if ft.NumOut() != 1 { - return fmt.Errorf("expected one out param, got: %v", ft) - } - if ft.In(0).Kind() != reflect.Ptr { - return fmt.Errorf("expected pointer arg for in param 0, got: %v", ft) - } - if ft.In(1).Kind() != reflect.Ptr { - return fmt.Errorf("expected pointer arg for in param 1, got: %v", ft) - } - var forErrorType error - // This convolution is necessary, otherwise TypeOf picks up on the fact - // that forErrorType is nil. - errorType := reflect.TypeOf(&forErrorType).Elem() - if ft.Out(0) != errorType { - return fmt.Errorf("expected error return, got: %v", ft) - } - c.funcs[typePair{ft.In(0).Elem(), ft.In(1).Elem()}] = fv - return nil -} - -// Convert will translate src to dest if it knows how. Both must be pointers. -// If no conversion func is registered and the default copying mechanism -// doesn't work on this type pair, an error will be returned. -// Not safe for objects with cyclic references! -func (c *Converter) Convert(src, dest interface{}) error { - dv, sv := reflect.ValueOf(dest), reflect.ValueOf(src) - if dv.Kind() != reflect.Ptr { - return fmt.Errorf("Need pointer, but got %#v", dest) - } - if sv.Kind() != reflect.Ptr { - return fmt.Errorf("Need pointer, but got %#v", src) - } - dv = dv.Elem() - sv = sv.Elem() - if !dv.CanAddr() { - return fmt.Errorf("Can't write to dest") - } - return c.convert(sv, dv) -} - -// convert recursively copies sv into dv, calling an appropriate conversion function if -// one is registered. -func (c *Converter) convert(sv, dv reflect.Value) error { - dt, st := dv.Type(), sv.Type() - if fv, ok := c.funcs[typePair{st, dt}]; ok { - if c.debug != nil { - c.debug.Logf("Calling custom conversion of '%v' to '%v'", st, dt) - } - ret := fv.Call([]reflect.Value{sv.Addr(), dv.Addr()})[0].Interface() - // This convolution is necssary because nil interfaces won't convert - // to errors. - if ret == nil { - return nil - } - return ret.(error) - } - - if dt.Name() != st.Name() { - return fmt.Errorf("Type names don't match: %v, %v", dt.Name(), st.Name()) - } - - // This should handle all simple types. - if st.AssignableTo(dt) { - dv.Set(sv) - return nil - } - if st.ConvertibleTo(dt) { - dv.Set(sv.Convert(dt)) - return nil - } - - if c.debug != nil { - c.debug.Logf("Trying to convert '%v' to '%v'", st, dt) - } - - switch dv.Kind() { - case reflect.Struct: - for i := 0; i < dt.NumField(); i++ { - f := dv.Type().Field(i) - sf := sv.FieldByName(f.Name) - if !sf.IsValid() { - return fmt.Errorf("%v not present in source %v for dest %v", f.Name, sv.Type(), dv.Type()) - } - df := dv.Field(i) - if err := c.convert(sf, df); err != nil { - return err - } - } - case reflect.Slice: - if sv.IsNil() { - // Don't make a zero-length slice. - dv.Set(reflect.Zero(dt)) - return nil - } - dv.Set(reflect.MakeSlice(dt, sv.Len(), sv.Cap())) - for i := 0; i < sv.Len(); i++ { - if err := c.convert(sv.Index(i), dv.Index(i)); err != nil { - return err - } - } - case reflect.Ptr: - if sv.IsNil() { - // Don't copy a nil ptr! - dv.Set(reflect.Zero(dt)) - return nil - } - dv.Set(reflect.New(dt.Elem())) - return c.convert(sv.Elem(), dv.Elem()) - case reflect.Map: - if sv.IsNil() { - // Don't copy a nil ptr! - dv.Set(reflect.Zero(dt)) - return nil - } - dv.Set(reflect.MakeMap(dt)) - for _, sk := range sv.MapKeys() { - dk := reflect.New(dt.Key()).Elem() - if err := c.convert(sk, dk); err != nil { - return err - } - dkv := reflect.New(dt.Elem()).Elem() - if err := c.convert(sv.MapIndex(sk), dkv); err != nil { - return err - } - dv.SetMapIndex(dk, dkv) - } - default: - return fmt.Errorf("Couldn't copy '%v' into '%v'", st, dt) - } - return nil -} diff --git a/pkg/api/converter_test.go b/pkg/api/converter_test.go deleted file mode 100644 index 18a97e9298b..00000000000 --- a/pkg/api/converter_test.go +++ /dev/null @@ -1,110 +0,0 @@ -/* -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 api - -import ( - "fmt" - "testing" -) - -func TestConverter(t *testing.T) { - type A struct { - Foo string - } - type B struct { - Bar string - } - type C struct{} - c := NewConverter() - err := c.Register(func(in *A, out *B) error { - out.Bar = in.Foo - return nil - }) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - err = c.Register(func(in *B, out *A) error { - out.Foo = in.Bar - return nil - }) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - - x := A{"hello, intrepid test reader!"} - y := B{} - - err = c.Convert(&x, &y) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - if e, a := x.Foo, y.Bar; e != a { - t.Errorf("expected %v, got %v", e, a) - } - - z := B{"all your test are belong to us"} - w := A{} - - err = c.Convert(&z, &w) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - if e, a := z.Bar, w.Foo; e != a { - t.Errorf("expected %v, got %v", e, a) - } - - err = c.Register(func(in *A, out *C) error { - return fmt.Errorf("C can't store an A, silly") - }) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - - err = c.Convert(&A{}, &C{}) - if err == nil { - t.Errorf("unexpected non-error") - } -} - -func anonymousEmptyTypeNamedA() interface{} { - type A struct{} - return &A{} -} - -func TestCopyConvertor(t *testing.T) { - type A struct { - Bar string - } - type B struct { - Bar string - } - c := NewConverter() - err := c.Convert(anonymousEmptyTypeNamedA(), &A{}) - if err == nil { - t.Errorf("unexpected non-error") - } - - err = c.Convert(&A{}, anonymousEmptyTypeNamedA()) - if err != nil { - t.Fatalf("unexpected error %v", err) - } - - err = c.Convert(&B{}, &A{}) - if err == nil { - t.Errorf("unexpected non-error") - } -} diff --git a/pkg/api/helper.go b/pkg/api/helper.go index 8b4631b9c1b..91a631217bc 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -17,28 +17,20 @@ limitations under the License. package api import ( - "encoding/json" "fmt" "reflect" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "gopkg.in/v1/yaml" ) -// versionMap allows one to figure out the go type of an object with -// the given version and name. -var versionMap = map[string]map[string]reflect.Type{} - -// typeToVersion allows one to figure out the version for a given go object. -// The reflect.Type we index by should *not* be a pointer. If the same type -// is registered for multiple versions, the last one wins. -var typeToVersion = map[reflect.Type]string{} - -// theConverter stores all registered conversion functions. It also has -// default coverting behavior. -var theConverter = NewConverter() +var conversionScheme *conversion.Scheme func init() { + conversionScheme = conversion.NewScheme() + conversionScheme.InternalVersion = "" + conversionScheme.ExternalVersion = "v1beta1" AddKnownTypes("", PodList{}, Pod{}, @@ -98,31 +90,13 @@ func init() { // AddKnownTypes registers the types of the arguments to the marshaller of the package api. // Encode() refuses the object unless its type is registered with AddKnownTypes. func AddKnownTypes(version string, types ...interface{}) { - knownTypes, found := versionMap[version] - if !found { - knownTypes = map[string]reflect.Type{} - versionMap[version] = knownTypes - } - for _, obj := range types { - t := reflect.TypeOf(obj) - if t.Kind() != reflect.Struct { - panic("All types must be structs.") - } - knownTypes[t.Name()] = t - typeToVersion[t] = version - } + conversionScheme.AddKnownTypes(version, types...) } // New returns a new API object of the given version ("" for internal // representation) and name, or an error if it hasn't been registered. func New(versionName, typeName string) (interface{}, error) { - if types, ok := versionMap[versionName]; ok { - if t, ok := types[typeName]; ok { - return reflect.New(t).Interface(), nil - } - return nil, fmt.Errorf("No type '%v' for version '%v'", typeName, versionName) - } - return nil, fmt.Errorf("No version '%v'", versionName) + return conversionScheme.NewObject(versionName, typeName) } // AddConversionFuncs adds a function to the list of conversion functions. The given @@ -138,20 +112,14 @@ func New(versionName, typeName string) (interface{}, error) { // extra fields, but it must not remove any. So you only need to add a conversion // function for things with changed/removed fields. func AddConversionFuncs(conversionFuncs ...interface{}) error { - for _, f := range conversionFuncs { - err := theConverter.Register(f) - if err != nil { - return err - } - } - return nil + return conversionScheme.AddConversionFuncs(conversionFuncs...) } // Convert will attempt to convert in into out. Both must be pointers to API objects. // For easy testing of conversion functions. Returns an error if the conversion isn't // possible. func Convert(in, out interface{}) error { - return theConverter.Convert(in, out) + return conversionScheme.Convert(in, out) } // FindJSONBase takes an arbitary api type, returns pointer to its JSONBase field. @@ -196,11 +164,7 @@ func FindJSONBaseRO(obj interface{}) (JSONBase, error) { // EncodeOrDie is a version of Encode which will panic instead of returning an error. For tests. func EncodeOrDie(obj interface{}) string { - bytes, err := Encode(obj) - if err != nil { - panic(err) - } - return string(bytes) + return conversionScheme.EncodeOrDie(obj) } // Encode turns the given api object into an appropriate JSON string. @@ -238,93 +202,7 @@ func EncodeOrDie(obj interface{}) string { // upgraded. // func Encode(obj interface{}) (data []byte, err error) { - obj = maybeCopy(obj) - obj, err = maybeExternalize(obj) - if err != nil { - return nil, err - } - - jsonBase, err := prepareEncode(obj) - if err != nil { - return nil, err - } - - data, err = json.MarshalIndent(obj, "", " ") - if err != nil { - return nil, err - } - // Leave these blank in memory. - jsonBase.SetKind("") - jsonBase.SetAPIVersion("") - return data, err -} - -// Returns the API version of the go object, or an error if it's not a -// pointer or is unregistered. -func objAPIVersionAndName(obj interface{}) (apiVersion, name string, err error) { - v, err := enforcePtr(obj) - if err != nil { - return "", "", err - } - t := v.Type() - if version, ok := typeToVersion[t]; !ok { - return "", "", fmt.Errorf("Unregistered type: %v", t) - } else { - return version, t.Name(), nil - } -} - -// maybeExternalize converts obj to an external object if it isn't one already. -// obj must be a pointer. -func maybeExternalize(obj interface{}) (interface{}, error) { - version, _, err := objAPIVersionAndName(obj) - if err != nil { - return nil, err - } - if version != "" { - // Object is already of an external versioned type. - return obj, nil - } - return externalize(obj) -} - -// maybeCopy copies obj if it is not a pointer, to get a settable/addressable -// object. Guaranteed to return a pointer. -func maybeCopy(obj interface{}) interface{} { - v := reflect.ValueOf(obj) - if v.Kind() == reflect.Ptr { - return obj - } - v2 := reflect.New(v.Type()) - v2.Elem().Set(v) - return v2.Interface() -} - -// prepareEncode sets the APIVersion and Kind fields to match the go type in obj. -// Returns an error if the (version, name) pair isn't registered for the type or -// if the type is an internal, non-versioned object. -func prepareEncode(obj interface{}) (JSONBaseInterface, error) { - version, name, err := objAPIVersionAndName(obj) - if err != nil { - return nil, err - } - if version == "" { - return nil, fmt.Errorf("No version for '%v' (%#v); extremely inadvisable to write it in wire format.", name, obj) - } - jsonBase, err := FindJSONBase(obj) - if err != nil { - return nil, err - } - knownTypes, found := versionMap[version] - if !found { - return nil, fmt.Errorf("struct %s, %s won't be unmarshalable because it's not in known versions", version, name) - } - if _, contains := knownTypes[name]; !contains { - return nil, fmt.Errorf("struct %s, %s won't be unmarshalable because it's not in knownTypes", version, name) - } - jsonBase.SetAPIVersion(version) - jsonBase.SetKind(name) - return jsonBase, nil + return conversionScheme.Encode(obj) } // Ensures that obj is a pointer of some sort. Returns a reflect.Value of the @@ -359,35 +237,7 @@ func VersionAndKind(data []byte) (version, kind string, err error) { // by Encode. Only versioned objects (APIVersion != "") are accepted. The object // will be converted into the in-memory unversioned type before being returned. func Decode(data []byte) (interface{}, error) { - version, kind, err := VersionAndKind(data) - if err != nil { - return nil, err - } - if version == "" { - return nil, fmt.Errorf("API Version not set in '%s'", string(data)) - } - obj, err := New(version, kind) - if err != nil { - return nil, fmt.Errorf("Unable to create new object of type ('%s', '%s')", version, kind) - } - // 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 { - return nil, err - } - obj, err = internalize(obj) - if err != nil { - return nil, err - } - jsonBase, err := FindJSONBase(obj) - if err != nil { - return nil, err - } - // Don't leave these set. Type and version info is deducible from go's type. - jsonBase.SetKind("") - jsonBase.SetAPIVersion("") - return obj, nil + return conversionScheme.Decode(data) } // DecodeInto parses a YAML or JSON string and stores it in obj. Returns an error @@ -396,102 +246,5 @@ func Decode(data []byte) (interface{}, error) { // If obj's APIVersion doesn't match that in data, an attempt will be made to convert // data into obj's version. func DecodeInto(data []byte, obj interface{}) error { - dataVersion, dataKind, err := VersionAndKind(data) - if err != nil { - return err - } - objVersion, objKind, err := objAPIVersionAndName(obj) - if err != nil { - return err - } - if dataKind == "" { - // Assume objects with unset Kind fields are being unmarshalled into the - // correct type. - dataKind = objKind - } - if dataKind != objKind { - return fmt.Errorf("data of kind '%v', obj of type '%v'", dataKind, objKind) - } - if dataVersion == "" { - // Assume objects with unset Version fields are being unmarshalled into the - // correct type. - dataVersion = objVersion - } - - if objVersion == dataVersion { - // Easy case! - err = yaml.Unmarshal(data, obj) - if err != nil { - return err - } - } else { - // TODO: look up in our map to see if we can do this dataVersion -> objVersion - // conversion. - if objVersion != "" || dataVersion != "v1beta1" { - return fmt.Errorf("Can't convert from '%v' to '%v' for type '%v'", dataVersion, objVersion, dataKind) - } - - external, err := New(dataVersion, dataKind) - if err != nil { - return fmt.Errorf("Unable to create new object of type ('%s', '%s')", dataVersion, dataKind) - } - // 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 { - return err - } - internal, err := internalize(external) - if err != nil { - return err - } - // Copy to the provided object. - vObj := reflect.ValueOf(obj) - vInternal := reflect.ValueOf(internal) - if !vInternal.Type().AssignableTo(vObj.Type()) { - return fmt.Errorf("%s is not assignable to %s", vInternal.Type(), vObj.Type()) - } - vObj.Elem().Set(vInternal.Elem()) - } - - jsonBase, err := FindJSONBase(obj) - if err != nil { - return err - } - // Don't leave these set. Type and version info is deducible from go's type. - jsonBase.SetKind("") - jsonBase.SetAPIVersion("") - return nil -} - -func internalize(obj interface{}) (interface{}, error) { - _, objKind, err := objAPIVersionAndName(obj) - if err != nil { - return nil, err - } - objOut, err := New("", objKind) - if err != nil { - return nil, err - } - err = theConverter.Convert(obj, objOut) - if err != nil { - return nil, err - } - return objOut, nil -} - -func externalize(obj interface{}) (interface{}, error) { - _, objKind, err := objAPIVersionAndName(obj) - if err != nil { - return nil, err - } - objOut, err := New("v1beta1", objKind) - if err != nil { - return nil, err - } - err = theConverter.Convert(obj, objOut) - if err != nil { - return nil, err - } - return objOut, nil + return conversionScheme.DecodeInto(data, obj) } From 1cc7fce5234eae343297a4fa9ee2d7b1c0e625a7 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 1 Aug 2014 14:24:12 -0700 Subject: [PATCH 3/3] Add documentation and tests to conversion. --- pkg/api/helper.go | 26 ++++++ pkg/conversion/converter.go | 16 ++-- pkg/conversion/converter_test.go | 130 ++++++++++++++++++++++++++++- pkg/conversion/decode.go | 20 ++--- pkg/conversion/doc.go | 34 ++++++++ pkg/conversion/encode.go | 45 +++------- pkg/conversion/scheme.go | 66 ++++++++------- pkg/conversion/scheme_test.go | 139 ++++++++++++++++++------------- 8 files changed, 331 insertions(+), 145 deletions(-) create mode 100644 pkg/conversion/doc.go diff --git a/pkg/api/helper.go b/pkg/api/helper.go index 91a631217bc..0d7d5b6fdeb 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -31,6 +31,7 @@ func init() { conversionScheme = conversion.NewScheme() conversionScheme.InternalVersion = "" conversionScheme.ExternalVersion = "v1beta1" + conversionScheme.MetaInsertionFactory = metaInsertion{} AddKnownTypes("", PodList{}, Pod{}, @@ -248,3 +249,28 @@ func Decode(data []byte) (interface{}, error) { func DecodeInto(data []byte, obj interface{}) error { return conversionScheme.DecodeInto(data, obj) } + +// metaInsertion implements conversion.MetaInsertionFactory, which lets the conversion +// package figure out how to encode our object's types and versions. These fields are +// located in our JSONBase. +type metaInsertion struct { + JSONBase struct { + APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"` + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + } `json:",inline" yaml:",inline"` +} + +// Create returns a new metaInsertion with the version and kind fields set. +func (metaInsertion) Create(version, kind string) interface{} { + m := metaInsertion{} + m.JSONBase.APIVersion = version + m.JSONBase.Kind = kind + return &m +} + +// Interpret returns the version and kind information from in, which must be +// a metaInsertion pointer object. +func (metaInsertion) Interpret(in interface{}) (version, kind string) { + m := in.(*metaInsertion) + return m.JSONBase.APIVersion, m.JSONBase.Kind +} diff --git a/pkg/conversion/converter.go b/pkg/conversion/converter.go index 025d7c8cc88..c496bc5e841 100644 --- a/pkg/conversion/converter.go +++ b/pkg/conversion/converter.go @@ -88,21 +88,21 @@ func (c *Converter) Register(conversionFunc interface{}) error { type FieldMatchingFlags int const ( - // Loop through source fields, search for matching dest field - // to copy it into. Destination fields with no corresponding - // source field will be ignored. - SourceToDest FieldMatchingFlags = 1 << iota // Loop through destiation fields, search for matching source // field to copy it from. Source fields with no corresponding // destination field will be ignored. If SourceToDest is // specified, this flag is ignored. If niether is specified, - // this flag is the default. - DestFromSource + // or no flags are passed, this flag is the default. + DestFromSource FieldMatchingFlags = 0 + // Loop through source fields, search for matching dest field + // to copy it into. Destination fields with no corresponding + // source field will be ignored. + SourceToDest FieldMatchingFlags = 1 << iota // Don't treat it as an error if the corresponding source or // dest field can't be found. IgnoreMissingFields // Don't require type names to match. - AllowDifferentFieldNames + AllowDifferentFieldTypeNames ) // Returns true if the given flag or combination of flags is set. @@ -147,7 +147,7 @@ func (c *Converter) convert(sv, dv reflect.Value, flags FieldMatchingFlags) erro return ret.(error) } - if !flags.IsSet(AllowDifferentFieldNames) && dt.Name() != st.Name() { + if !flags.IsSet(AllowDifferentFieldTypeNames) && dt.Name() != st.Name() { return fmt.Errorf("Can't convert %v to %v because type names don't match.", st, dt) } diff --git a/pkg/conversion/converter_test.go b/pkg/conversion/converter_test.go index c79e2455d81..60fbd8bdbc6 100644 --- a/pkg/conversion/converter_test.go +++ b/pkg/conversion/converter_test.go @@ -18,10 +18,13 @@ package conversion import ( "fmt" + "reflect" "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) -func TestConverter(t *testing.T) { +func TestConverter_CallsRegisteredFunctions(t *testing.T) { type A struct { Foo string } @@ -79,3 +82,128 @@ func TestConverter(t *testing.T) { t.Errorf("unexpected non-error") } } + +func TestConverter_fuzz(t *testing.T) { + newAnonType := func() interface{} { + return reflect.New(reflect.TypeOf(externalTypeReturn())).Interface() + } + // Use the same types from the scheme test. + table := []struct { + from, to, check interface{} + }{ + {&TestType1{}, newAnonType(), &TestType1{}}, + {newAnonType(), &TestType1{}, newAnonType()}, + } + + f := util.NewFuzzer() + c := NewConverter() + + for i, item := range table { + for j := 0; j < *fuzzIters; j++ { + f.Fuzz(item.from) + err := c.Convert(item.from, item.to, 0) + if err != nil { + t.Errorf("(%v, %v): unexpected error: %v", i, j, err) + continue + } + err = c.Convert(item.to, item.check, 0) + if err != nil { + t.Errorf("(%v, %v): unexpected error: %v", i, j, err) + continue + } + if e, a := item.from, item.check; !reflect.DeepEqual(e, a) { + t.Errorf("(%v, %v): unexpected diff: %v", i, j, objDiff(e, a)) + } + } + } +} + +func TestConverter_flags(t *testing.T) { + type Foo struct{ A string } + type Bar struct{ A string } + table := []struct { + from, to interface{} + flags FieldMatchingFlags + shouldSucceed bool + }{ + // Check that DestFromSource allows extra fields only in source. + { + from: &struct{ A string }{}, + to: &struct{ A, B string }{}, + flags: DestFromSource, + shouldSucceed: false, + }, { + from: &struct{ A, B string }{}, + to: &struct{ A string }{}, + flags: DestFromSource, + shouldSucceed: true, + }, + + // Check that SourceToDest allows for extra fields only in dest. + { + from: &struct{ A string }{}, + to: &struct{ A, B string }{}, + flags: SourceToDest, + shouldSucceed: true, + }, { + from: &struct{ A, B string }{}, + to: &struct{ A string }{}, + flags: SourceToDest, + shouldSucceed: false, + }, + + // Check that IgnoreMissingFields makes the above failure cases pass. + { + from: &struct{ A string }{}, + to: &struct{ A, B string }{}, + flags: DestFromSource | IgnoreMissingFields, + shouldSucceed: true, + }, { + from: &struct{ A, B string }{}, + to: &struct{ A string }{}, + flags: SourceToDest | IgnoreMissingFields, + shouldSucceed: true, + }, + + // Check that the field type name must match unless + // AllowDifferentFieldTypeNames is specified. + { + from: &struct{ A, B Foo }{}, + to: &struct{ A Bar }{}, + flags: DestFromSource, + shouldSucceed: false, + }, { + from: &struct{ A Foo }{}, + to: &struct{ A, B Bar }{}, + flags: SourceToDest, + shouldSucceed: false, + }, { + from: &struct{ A, B Foo }{}, + to: &struct{ A Bar }{}, + flags: DestFromSource | AllowDifferentFieldTypeNames, + shouldSucceed: true, + }, { + from: &struct{ A Foo }{}, + to: &struct{ A, B Bar }{}, + flags: SourceToDest | AllowDifferentFieldTypeNames, + shouldSucceed: true, + }, + } + f := util.NewFuzzer() + c := NewConverter() + + for i, item := range table { + for j := 0; j < *fuzzIters; j++ { + f.Fuzz(item.from) + err := c.Convert(item.from, item.to, item.flags) + if item.shouldSucceed && err != nil { + t.Errorf("(%v, %v): unexpected error: %v", i, j, err) + continue + } + if !item.shouldSucceed && err == nil { + t.Errorf("(%v, %v): unexpected non-error", i, j) + continue + } + } + } +} diff --git a/pkg/conversion/decode.go b/pkg/conversion/decode.go index 7c0da976f3b..530f74ab571 100644 --- a/pkg/conversion/decode.go +++ b/pkg/conversion/decode.go @@ -28,12 +28,12 @@ import ( // s.InternalVersion type before being returned. Decode will refuse to decode // objects without a version, because that's probably an error. func (s *Scheme) Decode(data []byte) (interface{}, error) { - version, kind, err := s.DataAPIVersionAndKind(data) + version, kind, err := s.DataVersionAndKind(data) if err != nil { return nil, err } if version == "" { - return nil, fmt.Errorf("API Version not set in '%s'", string(data)) + return nil, fmt.Errorf("version not set in '%s'", string(data)) } obj, err := s.NewObject(version, kind) if err != nil { @@ -47,8 +47,7 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) { } // Version and Kind should be blank in memory. - blankVersionAndKind := s.MetaInsertionFactory.Create("", "") - err = s.converter.Convert(blankVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + err = s.SetVersionAndKind("", "", obj) if err != nil { return nil, err } @@ -71,14 +70,14 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) { // DecodeInto parses a YAML or 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 APIVersion doesn't match that in data, an attempt will be made to convert +// 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 { - dataVersion, dataKind, err := s.DataAPIVersionAndKind(data) + dataVersion, dataKind, err := s.DataVersionAndKind(data) if err != nil { return err } - objVersion, objKind, err := s.ObjectAPIVersionAndKind(obj) + objVersion, objKind, err := s.ObjectVersionAndKind(obj) if err != nil { return err } @@ -120,10 +119,5 @@ func (s *Scheme) DecodeInto(data []byte, obj interface{}) error { } // Version and Kind should be blank in memory. - blankVersionAndKind := s.MetaInsertionFactory.Create("", "") - err = s.converter.Convert(blankVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) - if err != nil { - return err - } - return nil + return s.SetVersionAndKind("", "", obj) } diff --git a/pkg/conversion/doc.go b/pkg/conversion/doc.go new file mode 100644 index 00000000000..aea8afa68cc --- /dev/null +++ b/pkg/conversion/doc.go @@ -0,0 +1,34 @@ +/* +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 conversion provides go object versioning and encoding/decoding +// mechanisms. +// +// Specifically, conversion provides a way for you to define multiple versions +// of the same object. You may write functions which implement conversion logic, +// but for the fields which did not change, copying is automated. This makes it +// easy to modify the structures you use in memory without affecting the format +// you store on disk or respond to in your external API calls. +// +// The second offering of this package is automated encoding/decoding. The version +// and type of the object is recorded in the output, so it can be recreated upon +// reading. Currently, conversion writes JSON output, and interprets both JSON +// and YAML input. +// +// In the future, we plan to more explicitly separate the above two mechanisms, and +// add more serialization options, such as gob. +// +package conversion diff --git a/pkg/conversion/encode.go b/pkg/conversion/encode.go index 29b0ab5a0f3..554a6dc4437 100644 --- a/pkg/conversion/encode.go +++ b/pkg/conversion/encode.go @@ -17,7 +17,6 @@ limitations under the License. package conversion import ( - "bytes" "encoding/json" "fmt" ) @@ -37,17 +36,17 @@ func (s *Scheme) EncodeOrDie(obj interface{}) string { // struct. The type must have been registered. // // Memory/wire format differences: -// * Having to keep track of the Kind and APIVersion fields makes tests +// * Having to keep track of the Kind and Version fields makes tests // very annoying, so the rule is that they are set only in wire format // (json), not when in native (memory) format. This is possible because // both pieces of information are implicit in the go typed object. // * An exception: note that, if there are embedded API objects of known // type, for example, PodList{... Items []Pod ...}, these embedded // objects must be of the same version of the object they are embedded -// within, and their APIVersion and Kind must both be empty. -// * Note that the exception does not apply to the APIObject type, which -// recursively does Encode()/Decode(), and is capable of expressing any -// API object. +// within, and their Version and Kind must both be empty. +// * Note that the exception does not apply to a generic APIObject type +// which recursively does Encode()/Decode(), and is capable of +// expressing any API object. // * Only versioned objects should be encoded. This means that, if you pass // a native object, Encode will convert it to a versioned object. For // example, an api.Pod will get converted to a v1beta1.Pod. However, if @@ -71,7 +70,7 @@ func (s *Scheme) EncodeToVersion(obj interface{}, destVersion string) (data []by 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()) } - objVersion, objKind, err := s.ObjectAPIVersionAndKind(obj) + objVersion, objKind, err := s.ObjectVersionAndKind(obj) if err != nil { return nil, err } @@ -90,8 +89,7 @@ func (s *Scheme) EncodeToVersion(obj interface{}, destVersion string) (data []by } // Version and Kind should be set on the wire. - setVersionAndKind := s.MetaInsertionFactory.Create(destVersion, objKind) - err = s.converter.Convert(setVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + err = s.SetVersionAndKind(destVersion, objKind, obj) if err != nil { return nil, err } @@ -102,35 +100,12 @@ func (s *Scheme) EncodeToVersion(obj interface{}, destVersion string) (data []by return nil, err } - // Version and Kind should be blank in memory. - blankVersionAndKind := s.MetaInsertionFactory.Create("", "") - err = s.converter.Convert(blankVersionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldNames) + // Version and Kind should be blank in memory. Reset them, since it's + // possible that we modified a user object and not a copy above. + err = s.SetVersionAndKind("", "", obj) if err != nil { return nil, err } return data, nil - - meta, err := json.Marshal(s.MetaInsertionFactory.Create(destVersion, objKind)) - if err != nil { - return nil, err - } - // Stick these together, omitting the last } from meta and the first { from - // data. Add a comma to meta if necessary. - metaN := len(meta) - if len(data) > 2 { - meta[metaN-1] = ',' // Add comma - } else { - meta = meta[:metaN-1] // Just remove } - } - together := append(meta, data[1:]...) - if s.Indent { - var out bytes.Buffer - err := json.Indent(&out, together, "", " ") - if err != nil { - return nil, err - } - return out.Bytes(), nil - } - return together, nil } diff --git a/pkg/conversion/scheme.go b/pkg/conversion/scheme.go index 7f49b0cee47..730328a9e97 100644 --- a/pkg/conversion/scheme.go +++ b/pkg/conversion/scheme.go @@ -25,7 +25,7 @@ import ( // MetaInsertionFactory is used to create an object to store and retrieve // the version and kind information for all objects. The default uses the -// keys "apiVersion" and "kind" respectively. The object produced by this +// keys "version" and "kind" respectively. The object produced by this // factory is used to clear the version and kind fields in memory, so it // must match the layout of your actual api structs. (E.g., if you have your // version and kind field inside an inlined struct, this must produce an @@ -34,15 +34,12 @@ type MetaInsertionFactory interface { // Create should make a new object with two fields. // This object will be used to encode this metadata along with your // API objects, so the tags on the fields you use shouldn't conflict. - Create(apiVersion, kind string) interface{} + Create(version, kind string) interface{} // Interpret should take the same type of object that Create creates. // It should return the version and kind information from this object. - Interpret(interface{}) (apiVersion, kind string) + Interpret(interface{}) (version, kind string) } -// Default is a global scheme. -var Default = NewScheme() - // Scheme defines an entire encoding and decoding scheme. type Scheme struct { // versionMap allows one to figure out the go type of an object with @@ -70,7 +67,7 @@ type Scheme struct { // MetaInsertionFactory is used to create an object to store and retrieve // the version and kind information for all objects. The default uses the - // keys "apiVersion" and "kind" respectively. + // keys "version" and "kind" respectively. MetaInsertionFactory MetaInsertionFactory } @@ -86,8 +83,10 @@ func NewScheme() *Scheme { } } -// AddKnownTypes registers the types of the arguments to the marshaller of the package api. -// Encode() refuses the object unless its type is registered with AddKnownTypes. +// AddKnownTypes registers all types passed in 'types' as being members of version 'version. +// Encode() will refuse objects unless their type has been registered with AddKnownTypes. +// All objects passed to types should be structs, not pointers to structs. The name that go +// reports for the struct becomes the "kind" field when encoding. func (s *Scheme) AddKnownTypes(version string, types ...interface{}) { knownTypes, found := s.versionMap[version] if !found { @@ -117,17 +116,18 @@ func (s *Scheme) NewObject(versionName, typeName string) (interface{}, error) { } // AddConversionFuncs adds functions to the list of conversion functions. The given -// functions should know how to convert between two API objects. We deduce how to call -// it from the types of its two parameters; see the comment for Converter.Register. +// functions should know how to convert between two of your API objects, or their +// sub-objects. We deduce how to call these functions from the types of their two +// parameters; see the comment for Converter.Register. // // Note that, if you need to copy sub-objects that didn't change, it's safe to call -// Convert() inside your conversionFuncs, as long as you don't start a conversion +// s.Convert() inside your conversionFuncs, as long as you don't start a conversion // chain that's infinitely recursive. // // Also note that the default behavior, if you don't add a conversion function, is to -// sanely copy fields that have the same names. It's OK if the destination type has -// extra fields, but it must not remove any. So you only need to add a conversion -// function for things with changed/removed fields. +// sanely copy fields that have the same names and same type names. It's OK if the +// destination type has extra fields, but it must not remove any. So you only need to +// add conversion functions for things with changed/removed fields. func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error { for _, f := range conversionFuncs { err := s.converter.Register(f) @@ -138,8 +138,8 @@ func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error { return nil } -// Convert will attempt to convert in into out. Both must be pointers. -// For easy testing of conversion functions. Returns an error if the conversion isn't +// Convert will attempt to convert in into out. Both must be pointers. For easy +// testing of conversion functions. Returns an error if the conversion isn't // possible. func (s *Scheme) Convert(in, out interface{}) error { return s.converter.Convert(in, out, 0) @@ -147,32 +147,30 @@ func (s *Scheme) Convert(in, out interface{}) error { // metaInsertion provides a default implementation of MetaInsertionFactory. type metaInsertion struct { - JSONBase struct { - APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"` - Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` - } `json:",inline" yaml:",inline"` + Version string `json:"version,omitempty" yaml:"version,omitempty"` + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` } // Create should make a new object with two fields. // This object will be used to encode this metadata along with your // API objects, so the tags on the fields you use shouldn't conflict. -func (metaInsertion) Create(apiVersion, kind string) interface{} { +func (metaInsertion) Create(version, kind string) interface{} { m := metaInsertion{} - m.JSONBase.APIVersion = apiVersion - m.JSONBase.Kind = kind + m.Version = version + m.Kind = kind return &m } // Interpret should take the same type of object that Create creates. // It should return the version and kind information from this object. -func (metaInsertion) Interpret(in interface{}) (apiVersion, kind string) { +func (metaInsertion) Interpret(in interface{}) (version, kind string) { m := in.(*metaInsertion) - return m.JSONBase.APIVersion, m.JSONBase.Kind + return m.Version, m.Kind } // DataAPIVersionAndKind will return the APIVersion and Kind of the given wire-format // enconding of an API Object, or an error. -func (s *Scheme) DataAPIVersionAndKind(data []byte) (apiVersion, kind string, err error) { +func (s *Scheme) DataVersionAndKind(data []byte) (version, kind string, err error) { findKind := s.MetaInsertionFactory.Create("", "") // yaml is a superset of json, so we use it to decode here. That way, // we understand both. @@ -180,13 +178,13 @@ func (s *Scheme) DataAPIVersionAndKind(data []byte) (apiVersion, kind string, er if err != nil { return "", "", fmt.Errorf("couldn't get version/kind: %v", err) } - apiVersion, kind = s.MetaInsertionFactory.Interpret(findKind) - return apiVersion, kind, nil + version, kind = s.MetaInsertionFactory.Interpret(findKind) + return version, kind, nil } // ObjectVersionAndKind returns the API version and kind of the go object, // or an error if it's not a pointer or is unregistered. -func (s *Scheme) ObjectAPIVersionAndKind(obj interface{}) (apiVersion, kind string, err error) { +func (s *Scheme) ObjectVersionAndKind(obj interface{}) (apiVersion, kind string, err error) { v, err := enforcePtr(obj) if err != nil { return "", "", err @@ -199,6 +197,14 @@ func (s *Scheme) ObjectAPIVersionAndKind(obj interface{}) (apiVersion, kind stri } } +// SetVersionAndKind sets the version and kind fields (with help from +// MetaInsertionFactory). Returns an error if this isn't possible. obj +// must be a pointer. +func (s *Scheme) SetVersionAndKind(version, kind string, obj interface{}) error { + versionAndKind := s.MetaInsertionFactory.Create(version, kind) + return s.converter.Convert(versionAndKind, obj, SourceToDest|IgnoreMissingFields|AllowDifferentFieldTypeNames) +} + // maybeCopy copies obj if it is not a pointer, to get a settable/addressable // object. Guaranteed to return a pointer. func maybeCopy(obj interface{}) interface{} { diff --git a/pkg/conversion/scheme_test.go b/pkg/conversion/scheme_test.go index ad5aa610bd6..642585cc21f 100644 --- a/pkg/conversion/scheme_test.go +++ b/pkg/conversion/scheme_test.go @@ -28,31 +28,33 @@ import ( var fuzzIters = flag.Int("fuzz_iters", 10, "How many fuzzing iterations to do.") -// Intended to be compatible with the default implementation of MetaInsertionFactory -type JSONBase struct { +// Test a weird version/kind embedding format. +type MyWeirdCustomEmbeddedVersionKindField struct { ID string `yaml:"ID,omitempty" json:"ID,omitempty"` - APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"` - Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + APIVersion string `json:"myVersionKey,omitempty" yaml:"myVersionKey,omitempty"` + ObjectKind string `json:"myKindKey,omitempty" yaml:"myKindKey,omitempty"` + Z string `yaml:"Z,omitempty" json:"Z,omitempty"` + Y uint64 `yaml:"Y,omitempty" json:"Y,omitempty"` } type TestType1 struct { - JSONBase `json:",inline" yaml:",inline"` - A string `yaml:"A,omitempty" json:"A,omitempty"` - B int `yaml:"B,omitempty" json:"B,omitempty"` - C int8 `yaml:"C,omitempty" json:"C,omitempty"` - D int16 `yaml:"D,omitempty" json:"D,omitempty"` - E int32 `yaml:"E,omitempty" json:"E,omitempty"` - F int64 `yaml:"F,omitempty" json:"F,omitempty"` - G uint `yaml:"G,omitempty" json:"G,omitempty"` - H uint8 `yaml:"H,omitempty" json:"H,omitempty"` - I uint16 `yaml:"I,omitempty" json:"I,omitempty"` - J uint32 `yaml:"J,omitempty" json:"J,omitempty"` - K uint64 `yaml:"K,omitempty" json:"K,omitempty"` - L bool `yaml:"L,omitempty" json:"L,omitempty"` - M map[string]int `yaml:"M,omitempty" json:"M,omitempty"` - N map[string]TestType2 `yaml:"N,omitempty" json:"N,omitempty"` - O *TestType2 `yaml:"O,omitempty" json:"O,omitempty"` - P []TestType2 `yaml:"Q,omitempty" json:"Q,omitempty"` + MyWeirdCustomEmbeddedVersionKindField `json:",inline" yaml:",inline"` + A string `yaml:"A,omitempty" json:"A,omitempty"` + B int `yaml:"B,omitempty" json:"B,omitempty"` + C int8 `yaml:"C,omitempty" json:"C,omitempty"` + D int16 `yaml:"D,omitempty" json:"D,omitempty"` + E int32 `yaml:"E,omitempty" json:"E,omitempty"` + F int64 `yaml:"F,omitempty" json:"F,omitempty"` + G uint `yaml:"G,omitempty" json:"G,omitempty"` + H uint8 `yaml:"H,omitempty" json:"H,omitempty"` + I uint16 `yaml:"I,omitempty" json:"I,omitempty"` + J uint32 `yaml:"J,omitempty" json:"J,omitempty"` + K uint64 `yaml:"K,omitempty" json:"K,omitempty"` + L bool `yaml:"L,omitempty" json:"L,omitempty"` + M map[string]int `yaml:"M,omitempty" json:"M,omitempty"` + N map[string]TestType2 `yaml:"N,omitempty" json:"N,omitempty"` + O *TestType2 `yaml:"O,omitempty" json:"O,omitempty"` + P []TestType2 `yaml:"Q,omitempty" json:"Q,omitempty"` } type TestType2 struct { @@ -69,39 +71,39 @@ func externalTypeReturn() interface{} { B int `yaml:"B,omitempty" json:"B,omitempty"` } type TestType1 struct { - JSONBase `json:",inline" yaml:",inline"` - A string `yaml:"A,omitempty" json:"A,omitempty"` - B int `yaml:"B,omitempty" json:"B,omitempty"` - C int8 `yaml:"C,omitempty" json:"C,omitempty"` - D int16 `yaml:"D,omitempty" json:"D,omitempty"` - E int32 `yaml:"E,omitempty" json:"E,omitempty"` - F int64 `yaml:"F,omitempty" json:"F,omitempty"` - G uint `yaml:"G,omitempty" json:"G,omitempty"` - H uint8 `yaml:"H,omitempty" json:"H,omitempty"` - I uint16 `yaml:"I,omitempty" json:"I,omitempty"` - J uint32 `yaml:"J,omitempty" json:"J,omitempty"` - K uint64 `yaml:"K,omitempty" json:"K,omitempty"` - L bool `yaml:"L,omitempty" json:"L,omitempty"` - M map[string]int `yaml:"M,omitempty" json:"M,omitempty"` - N map[string]TestType2 `yaml:"N,omitempty" json:"N,omitempty"` - O *TestType2 `yaml:"O,omitempty" json:"O,omitempty"` - P []TestType2 `yaml:"Q,omitempty" json:"Q,omitempty"` + MyWeirdCustomEmbeddedVersionKindField `json:",inline" yaml:",inline"` + A string `yaml:"A,omitempty" json:"A,omitempty"` + B int `yaml:"B,omitempty" json:"B,omitempty"` + C int8 `yaml:"C,omitempty" json:"C,omitempty"` + D int16 `yaml:"D,omitempty" json:"D,omitempty"` + E int32 `yaml:"E,omitempty" json:"E,omitempty"` + F int64 `yaml:"F,omitempty" json:"F,omitempty"` + G uint `yaml:"G,omitempty" json:"G,omitempty"` + H uint8 `yaml:"H,omitempty" json:"H,omitempty"` + I uint16 `yaml:"I,omitempty" json:"I,omitempty"` + J uint32 `yaml:"J,omitempty" json:"J,omitempty"` + K uint64 `yaml:"K,omitempty" json:"K,omitempty"` + L bool `yaml:"L,omitempty" json:"L,omitempty"` + M map[string]int `yaml:"M,omitempty" json:"M,omitempty"` + N map[string]TestType2 `yaml:"N,omitempty" json:"N,omitempty"` + O *TestType2 `yaml:"O,omitempty" json:"O,omitempty"` + P []TestType2 `yaml:"Q,omitempty" json:"Q,omitempty"` } return TestType1{} } type ExternalInternalSame struct { - JSONBase `json:",inline" yaml:",inline"` - A TestType2 `yaml:"A,omitempty" json:"A,omitempty"` + MyWeirdCustomEmbeddedVersionKindField `json:",inline" yaml:",inline"` + A TestType2 `yaml:"A,omitempty" json:"A,omitempty"` } // TestObjectFuzzer can randomly populate all the above objects. var TestObjectFuzzer = util.NewFuzzer( - func(j *JSONBase) { - // We have to customize the randomization of JSONBases because their + func(j *MyWeirdCustomEmbeddedVersionKindField) { + // We have to customize the randomization of MyWeirdCustomEmbeddedVersionKindFields because their // APIVersion and Kind must remain blank in memory. j.APIVersion = "" - j.Kind = "" + j.ObjectKind = "" j.ID = util.RandString() }, func(u *uint64) { @@ -125,9 +127,32 @@ func GetTestScheme() *Scheme { s.AddKnownTypes("v1", externalTypeReturn(), ExternalInternalSame{}) s.ExternalVersion = "v1" s.InternalVersion = "" + s.MetaInsertionFactory = testMetaInsertionFactory{} return s } +type testMetaInsertionFactory struct { + MyWeirdCustomEmbeddedVersionKindField struct { + APIVersion string `json:"myVersionKey,omitempty" yaml:"myVersionKey,omitempty"` + ObjectKind string `json:"myKindKey,omitempty" yaml:"myKindKey,omitempty"` + } `json:",inline" yaml:",inline"` +} + +// Create returns a new testMetaInsertionFactory with the version and kind fields set. +func (testMetaInsertionFactory) Create(version, kind string) interface{} { + m := testMetaInsertionFactory{} + m.MyWeirdCustomEmbeddedVersionKindField.APIVersion = version + m.MyWeirdCustomEmbeddedVersionKindField.ObjectKind = kind + return &m +} + +// Interpret returns the version and kind information from in, which must be +// a testMetaInsertionFactory pointer object. +func (testMetaInsertionFactory) Interpret(in interface{}) (version, kind string) { + m := in.(*testMetaInsertionFactory) + return m.MyWeirdCustomEmbeddedVersionKindField.APIVersion, m.MyWeirdCustomEmbeddedVersionKindField.ObjectKind +} + func objDiff(a, b interface{}) string { ab, err := json.Marshal(a) if err != nil { @@ -162,22 +187,20 @@ func runTest(t *testing.T, source interface{}) { if err != nil { t.Errorf("%v: %v (%v)", name, err, string(data)) return - } else { - if !reflect.DeepEqual(source, obj2) { - t.Errorf("1: %v: diff: %v", name, objDiff(source, obj2)) - return - } + } + if !reflect.DeepEqual(source, obj2) { + t.Errorf("1: %v: diff: %v", name, objDiff(source, obj2)) + return } obj3 := reflect.New(reflect.TypeOf(source).Elem()).Interface() err = s.DecodeInto(data, obj3) if err != nil { t.Errorf("2: %v: %v", name, err) return - } else { - if !reflect.DeepEqual(source, obj3) { - t.Errorf("3: %v: diff: %v", name, objDiff(source, obj3)) - return - } + } + if !reflect.DeepEqual(source, obj3) { + t.Errorf("3: %v: diff: %v", name, objDiff(source, obj3)) + return } } @@ -231,17 +254,17 @@ func TestEncode_Ptr(t *testing.T) { func TestBadJSONRejection(t *testing.T) { s := GetTestScheme() badJSONs := [][]byte{ - []byte(`{"apiVersion":"v1"}`), // Missing kind - []byte(`{"kind":"TestType1"}`), // Missing version - []byte(`{"apiVersion":"v1","kind":"bar"}`), // Unknown kind - []byte(`{"apiVersion":"bar","kind":"TestType1"}`), // Unknown version + []byte(`{"myVersionKey":"v1"}`), // Missing kind + []byte(`{"myKindKey":"TestType1"}`), // Missing version + []byte(`{"myVersionKey":"v1","myKindKey":"bar"}`), // Unknown kind + []byte(`{"myVersionKey":"bar","myKindKey":"TestType1"}`), // Unknown version } for _, b := range badJSONs { if _, err := s.Decode(b); err == nil { t.Errorf("Did not reject bad json: %s", string(b)) } } - badJSONKindMismatch := []byte(`{"apiVersion":"v1","kind": "ExternalInternalSame"}`) + badJSONKindMismatch := []byte(`{"myVersionKey":"v1","myKindKey":"ExternalInternalSame"}`) if err := s.DecodeInto(badJSONKindMismatch, &TestType1{}); err == nil { t.Errorf("Kind is set but doesn't match the object type: %s", badJSONKindMismatch) }