diff --git a/pkg/conversion/converter.go b/pkg/conversion/converter.go index 43d3a4feccd..f61433910a3 100644 --- a/pkg/conversion/converter.go +++ b/pkg/conversion/converter.go @@ -40,17 +40,17 @@ type Converter struct { // If non-nil, will be called to print helpful debugging info. Quite verbose. Debug DebugLogger - // Name is called to retrieve the name of a type; this name is used for the + // NameFunc is called to retrieve the name of a type; this name is used for the // purpose of deciding whether two types match or not (i.e., will we attempt to // do a conversion). The default returns the go type name. - Name func(t reflect.Type) string + NameFunc func(t reflect.Type) string } // NewConverter creates a new Converter object. func NewConverter() *Converter { return &Converter{ - funcs: map[typePair]reflect.Value{}, - Name: func(t reflect.Type) string { return t.Name() }, + funcs: map[typePair]reflect.Value{}, + NameFunc: func(t reflect.Type) string { return t.Name() }, } } @@ -71,62 +71,70 @@ type Scope interface { Flags() FieldMatchingFlags // Meta returns any information originally passed to Convert. - Meta() map[string]interface{} + Meta() *Meta +} + +// Meta is supplied by Scheme, when it calls Convert. +type Meta struct { + SrcVersion string + DestVersion string + + // TODO: If needed, add a user data field here. } // scope contains information about an ongoing conversion. type scope struct { converter *Converter - meta map[string]interface{} + meta *Meta flags FieldMatchingFlags srcTagStack []reflect.StructTag destTagStack []reflect.StructTag } // push adds a level to the src/dest tag stacks. -func (sa *scope) push() { - sa.srcTagStack = append(sa.srcTagStack, "") - sa.destTagStack = append(sa.destTagStack, "") +func (s *scope) push() { + s.srcTagStack = append(s.srcTagStack, "") + s.destTagStack = append(s.destTagStack, "") } // pop removes a level to the src/dest tag stacks. -func (sa *scope) pop() { - n := len(sa.srcTagStack) - sa.srcTagStack = sa.srcTagStack[:n-1] - sa.destTagStack = sa.destTagStack[:n-1] +func (s *scope) pop() { + n := len(s.srcTagStack) + s.srcTagStack = s.srcTagStack[:n-1] + s.destTagStack = s.destTagStack[:n-1] } -func (sa *scope) setSrcTag(tag reflect.StructTag) { - sa.srcTagStack[len(sa.srcTagStack)-1] = tag +func (s *scope) setSrcTag(tag reflect.StructTag) { + s.srcTagStack[len(s.srcTagStack)-1] = tag } -func (sa *scope) setDestTag(tag reflect.StructTag) { - sa.destTagStack[len(sa.destTagStack)-1] = tag +func (s *scope) setDestTag(tag reflect.StructTag) { + s.destTagStack[len(s.destTagStack)-1] = tag } // Convert continues a conversion. -func (sa *scope) Convert(src, dest interface{}, flags FieldMatchingFlags) error { - return sa.converter.Convert(src, dest, flags, sa.meta) +func (s *scope) Convert(src, dest interface{}, flags FieldMatchingFlags) error { + return s.converter.Convert(src, dest, flags, s.meta) } // SrcTag returns the tag of the struct containing the current source item, if any. -func (sa *scope) SrcTag() reflect.StructTag { - return sa.srcTagStack[len(sa.srcTagStack)-1] +func (s *scope) SrcTag() reflect.StructTag { + return s.srcTagStack[len(s.srcTagStack)-1] } // DestTag returns the tag of the struct containing the current dest item, if any. -func (sa *scope) DestTag() reflect.StructTag { - return sa.destTagStack[len(sa.destTagStack)-1] +func (s *scope) DestTag() reflect.StructTag { + return s.destTagStack[len(s.destTagStack)-1] } // Flags returns the flags with which the current conversion was started. -func (sa *scope) Flags() FieldMatchingFlags { - return sa.flags +func (s *scope) Flags() FieldMatchingFlags { + return s.flags } // Meta returns the meta object that was originally passed to Convert. -func (sa *scope) Meta() map[string]interface{} { - return sa.meta +func (s *scope) Meta() *Meta { + return s.meta } // Register registers a conversion func with the Converter. conversionFunc must take @@ -202,9 +210,9 @@ func (f FieldMatchingFlags) IsSet(flag FieldMatchingFlags) bool { // Read the comments on the various FieldMatchingFlags constants to understand // what the 'flags' parameter does. // 'meta' is given to allow you to pass information to conversion functions, -// it is not used by Convert other than storing it in the scope. +// it is not used by Convert() other than storing it in the scope. // Not safe for objects with cyclic references! -func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, meta map[string]interface{}) error { +func (c *Converter) Convert(src, dest interface{}, flags FieldMatchingFlags, meta *Meta) error { dv, sv := reflect.ValueOf(dest), reflect.ValueOf(src) if dv.Kind() != reflect.Ptr { return fmt.Errorf("Need pointer, but got %#v", dest) @@ -244,7 +252,7 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error { return ret.(error) } - if !scope.flags.IsSet(AllowDifferentFieldTypeNames) && c.Name(dt) != c.Name(st) { + if !scope.flags.IsSet(AllowDifferentFieldTypeNames) && c.NameFunc(dt) != c.NameFunc(st) { 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 c12c7633a06..573c9791ded 100644 --- a/pkg/conversion/converter_test.go +++ b/pkg/conversion/converter_test.go @@ -103,7 +103,7 @@ func TestConverter_fuzz(t *testing.T) { f := fuzz.New().NilChance(.5).NumElements(0, 100) c := NewConverter() - c.Name = func(t reflect.Type) string { + c.NameFunc = func(t reflect.Type) string { // Hide the fact that we don't have separate packages for these things. return map[reflect.Type]string{ reflect.TypeOf(TestType1{}): "TestType1", @@ -168,7 +168,7 @@ func TestConverter_meta(t *testing.T) { checks := 0 err := c.Register( func(in *Foo, out *Bar, s Scope) error { - if s.Meta()["test"] != "passes" { + if s.Meta() == nil || s.Meta().SrcVersion != "test" || s.Meta().DestVersion != "passes" { t.Errorf("Meta did not get passed!") } checks++ @@ -181,7 +181,7 @@ func TestConverter_meta(t *testing.T) { } err = c.Register( func(in *string, out *string, s Scope) error { - if s.Meta()["test"] != "passes" { + if s.Meta() == nil || s.Meta().SrcVersion != "test" || s.Meta().DestVersion != "passes" { t.Errorf("Meta did not get passed a second time!") } checks++ @@ -191,7 +191,7 @@ func TestConverter_meta(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - err = c.Convert(&Foo{}, &Bar{}, 0, map[string]interface{}{"test": "passes"}) + err = c.Convert(&Foo{}, &Bar{}, 0, &Meta{SrcVersion: "test", DestVersion: "passes"}) if err != nil { t.Fatalf("Unexpected error: %v", err) } diff --git a/pkg/conversion/scheme.go b/pkg/conversion/scheme.go index ce1cd66f559..c77b80e4703 100644 --- a/pkg/conversion/scheme.go +++ b/pkg/conversion/scheme.go @@ -86,15 +86,19 @@ func NewScheme() *Scheme { ExternalVersion: "v1", MetaInsertionFactory: metaInsertion{}, } - s.converter.Name = func(t reflect.Type) string { - if kind, ok := s.typeToKind[t]; ok { - return kind - } - return t.Name() - } + s.converter.NameFunc = s.nameFunc return s } +// nameFunc returns the name of the type that we wish to use for encoding. Defaults to +// the go name of the type if the type is not registered. +func (s *Scheme) nameFunc(t reflect.Type) string { + if kind, ok := s.typeToKind[t]; ok { + return kind + } + return t.Name() +} + // 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 pointers to structs. The name that go reports for @@ -161,22 +165,25 @@ func (s *Scheme) NewObject(versionName, typeName string) (interface{}, error) { // // Note that, if you need to copy sub-objects that didn't change, you can use the // conversion.Scope object that will be passed to your conversion function. -// Additionally, all conversions started by Scheme will set the "srcVersion" and -// "destVersion" keys on the meta object. Example: +// Additionally, all conversions started by Scheme will set the SrcVersion and +// DestVersion fields on the Meta object. Example: // // s.AddConversionFuncs( // func(in *InternalObject, out *ExternalObject, scope conversion.Scope) error { -// // You can depend on this being set to the source version, e.g., "". -// s.Meta()["srcVersion"].(string) +// // You can depend on Meta() being non-nil, and this being set to +// // the source version, e.g., "" +// s.Meta().SrcVersion // // You can depend on this being set to the destination version, // // e.g., "v1beta1". -// s.Meta()["destVersion"].(string) +// s.Meta().DestVersion // // Call scope.Convert to copy sub-fields. // s.Convert(&in.SubFieldThatMoved, &out.NewLocation.NewName, 0) // return nil // }, // ) // +// (For more detail about conversion functions, see Converter.Register's comment.) +// // Also note that the default behavior, if you don't add a conversion function, is to // 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 @@ -196,7 +203,7 @@ func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error { // possible. You can call this with types that haven't been registered (for example, // a to test conversion of types that are nested within registered types), but in // that case, the conversion.Scope object passed to your conversion functions won't -// have "srcVersion" or "destVersion" keys set correctly in Meta(). +// have SrcVersion or DestVersion fields set correctly in Meta(). func (s *Scheme) Convert(in, out interface{}) error { inVersion := "unknown" outVersion := "unknown" @@ -209,11 +216,11 @@ func (s *Scheme) Convert(in, out interface{}) error { return s.converter.Convert(in, out, 0, s.generateConvertMeta(inVersion, outVersion)) } -// generateConvertMeta assembles a map for the meta value we pass to Convert. -func (s *Scheme) generateConvertMeta(srcVersion, destVersion string) map[string]interface{} { - return map[string]interface{}{ - "srcVersion": srcVersion, - "destVersion": destVersion, +// generateConvertMeta constructs the meta value we pass to Convert. +func (s *Scheme) generateConvertMeta(srcVersion, destVersion string) *Meta { + return &Meta{ + SrcVersion: srcVersion, + DestVersion: destVersion, } } diff --git a/pkg/conversion/scheme_test.go b/pkg/conversion/scheme_test.go index bcb304c357f..d59f6dde450 100644 --- a/pkg/conversion/scheme_test.go +++ b/pkg/conversion/scheme_test.go @@ -293,10 +293,10 @@ func TestMetaValues(t *testing.T) { // Register functions to verify that scope.Meta() gets set correctly. err := s.AddConversionFuncs( func(in *InternalSimple, out *ExternalSimple, scope Scope) error { - if e, a := "", scope.Meta()["srcVersion"].(string); e != a { + if e, a := "", scope.Meta().SrcVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } - if e, a := "externalVersion", scope.Meta()["destVersion"].(string); e != a { + if e, a := "externalVersion", scope.Meta().DestVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } scope.Convert(&in.TestString, &out.TestString, 0) @@ -304,10 +304,10 @@ func TestMetaValues(t *testing.T) { return nil }, func(in *ExternalSimple, out *InternalSimple, scope Scope) error { - if e, a := "externalVersion", scope.Meta()["srcVersion"].(string); e != a { + if e, a := "externalVersion", scope.Meta().SrcVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } - if e, a := "", scope.Meta()["destVersion"].(string); e != a { + if e, a := "", scope.Meta().DestVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } scope.Convert(&in.TestString, &out.TestString, 0) @@ -381,10 +381,10 @@ func TestMetaValuesUnregisteredConvert(t *testing.T) { // Register functions to verify that scope.Meta() gets set correctly. err := s.AddConversionFuncs( func(in *InternalSimple, out *ExternalSimple, scope Scope) error { - if e, a := "unknown", scope.Meta()["srcVersion"].(string); e != a { + if e, a := "unknown", scope.Meta().SrcVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } - if e, a := "unknown", scope.Meta()["destVersion"].(string); e != a { + if e, a := "unknown", scope.Meta().DestVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } scope.Convert(&in.TestString, &out.TestString, 0) diff --git a/pkg/kubecfg/parse_test.go b/pkg/kubecfg/parse_test.go index f09b47fe9b5..5681bfd7610 100644 --- a/pkg/kubecfg/parse_test.go +++ b/pkg/kubecfg/parse_test.go @@ -17,6 +17,7 @@ limitations under the License. package kubecfg import ( + "encoding/json" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -34,7 +35,9 @@ func TestParseBadStorage(t *testing.T) { func DoParseTest(t *testing.T, storage string, obj runtime.Object, p *Parser) { jsonData, _ := runtime.DefaultCodec.Encode(obj) - yamlData, _ := yaml.Marshal(obj) + var tmp map[string]interface{} + json.Unmarshal(jsonData, &tmp) + yamlData, _ := yaml.Marshal(tmp) t.Logf("Intermediate yaml:\n%v\n", string(yamlData)) t.Logf("Intermediate json:\n%v\n", string(jsonData)) jsonGot, jsonErr := p.ToWireFormat(jsonData, storage, runtime.DefaultCodec) diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index a90c67da31e..64cacb6d8b1 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -54,8 +54,8 @@ func GetScheme(schemeName string) *Scheme { // from a conversion.Scope. func fromScope(s conversion.Scope) (inVersion, outVersion string, scheme *Scheme) { scheme = DefaultScheme - inVersion = s.Meta()["srcVersion"].(string) - outVersion = s.Meta()["destVersion"].(string) + inVersion = s.Meta().SrcVersion + outVersion = s.Meta().DestVersion // If a scheme tag was provided, use it. Look at the struct tag corresponding // to version "". if name := s.SrcTag().Get("scheme"); inVersion == "" && name != "" { diff --git a/pkg/runtime/scheme_test.go b/pkg/runtime/scheme_test.go index b44454ba42d..34a3fcf83a5 100644 --- a/pkg/runtime/scheme_test.go +++ b/pkg/runtime/scheme_test.go @@ -52,10 +52,10 @@ func TestScheme(t *testing.T) { // Register functions to verify that scope.Meta() gets set correctly. err := runtime.DefaultScheme.AddConversionFuncs( func(in *InternalSimple, out *ExternalSimple, scope conversion.Scope) error { - if e, a := "", scope.Meta()["srcVersion"].(string); e != a { + if e, a := "", scope.Meta().SrcVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } - if e, a := "externalVersion", scope.Meta()["destVersion"].(string); e != a { + if e, a := "externalVersion", scope.Meta().DestVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } scope.Convert(&in.JSONBase, &out.JSONBase, 0) @@ -64,10 +64,10 @@ func TestScheme(t *testing.T) { return nil }, func(in *ExternalSimple, out *InternalSimple, scope conversion.Scope) error { - if e, a := "externalVersion", scope.Meta()["srcVersion"].(string); e != a { + if e, a := "externalVersion", scope.Meta().SrcVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } - if e, a := "", scope.Meta()["destVersion"].(string); e != a { + if e, a := "", scope.Meta().DestVersion; e != a { t.Errorf("Expected '%v', got '%v'", e, a) } scope.Convert(&in.JSONBase, &out.JSONBase, 0)