From 37f5a9df07b25399ecd63589f1387bb5c584dec8 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 15 Jan 2015 12:00:57 -0800 Subject: [PATCH] make conversion do deep copies --- pkg/api/conversion.go | 12 ++++++++++ pkg/conversion/converter.go | 37 ++++++++++++++++++++++------- pkg/conversion/converter_test.go | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/pkg/api/conversion.go b/pkg/api/conversion.go index 6fc3c7d239f..2537718d1df 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/conversion.go @@ -17,8 +17,10 @@ limitations under the License. package api import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource" "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) // Codec is the identity codec for this package - it can only convert itself @@ -27,6 +29,16 @@ var Codec = runtime.CodecFor(Scheme, "") func init() { Scheme.AddConversionFuncs( + func(in *util.Time, out *util.Time, s conversion.Scope) error { + // Cannot deep copy these, because time.Time has unexported fields. + *out = *in + return nil + }, + func(in *resource.Quantity, out *resource.Quantity, s conversion.Scope) error { + // Cannot deep copy these, because inf.Dec has unexported fields. + *out = *in.Copy() + return nil + }, // Convert ContainerManifest to BoundPod func(in *ContainerManifest, out *BoundPod, s conversion.Scope) error { out.Spec.Containers = in.Containers diff --git a/pkg/conversion/converter.go b/pkg/conversion/converter.go index 753d95c74f2..1e2c2c42ab2 100644 --- a/pkg/conversion/converter.go +++ b/pkg/conversion/converter.go @@ -394,20 +394,29 @@ func (c *Converter) convert(sv, dv reflect.Value, scope *scope) error { func (c *Converter) defaultConvert(sv, dv reflect.Value, scope *scope) error { dt, st := dv.Type(), sv.Type() + if !dv.CanSet() { + return scope.error("Cannot set dest. (Tried to deep copy something with unexported fields?)") + } + if !scope.flags.IsSet(AllowDifferentFieldTypeNames) && c.NameFunc(dt) != c.NameFunc(st) { return scope.error( "type names don't match (%v, %v), and no conversion 'func (%v, %v) error' registered.", c.NameFunc(st), c.NameFunc(dt), 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 + switch st.Kind() { + case reflect.Map, reflect.Ptr, reflect.Slice, reflect.Interface, reflect.Struct: + // Don't copy these via assignment/conversion! + default: + // 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 { @@ -466,6 +475,18 @@ func (c *Converter) defaultConvert(sv, dv reflect.Value, scope *scope) error { } dv.SetMapIndex(dk, dkv) } + case reflect.Interface: + if sv.IsNil() { + // Don't copy a nil interface! + dv.Set(reflect.Zero(dt)) + return nil + } + tmpdv := reflect.New(sv.Elem().Type()).Elem() + if err := c.convert(sv.Elem(), tmpdv, scope); err != nil { + return err + } + dv.Set(reflect.ValueOf(tmpdv.Interface())) + return nil default: return scope.error("couldn't copy '%v' into '%v'; didn't understand types", st, dt) } diff --git a/pkg/conversion/converter_test.go b/pkg/conversion/converter_test.go index d1b3e2ba034..2ca555aac17 100644 --- a/pkg/conversion/converter_test.go +++ b/pkg/conversion/converter_test.go @@ -66,6 +66,46 @@ func TestConverter_DefaultConvert(t *testing.T) { } } +func TestConverter_DeepCopy(t *testing.T) { + type A struct { + Foo *string + Bar []string + Baz interface{} + Qux map[string]string + } + c := NewConverter() + c.Debug = t + + foo, baz := "foo", "baz" + x := A{ + Foo: &foo, + Bar: []string{"bar"}, + Baz: &baz, + Qux: map[string]string{"qux": "qux"}, + } + y := A{} + + if err := c.Convert(&x, &y, 0, nil); err != nil { + t.Fatalf("unexpected error %v", err) + } + *x.Foo = "foo2" + x.Bar[0] = "bar2" + *x.Baz.(*string) = "baz2" + x.Qux["qux"] = "qux2" + if e, a := *x.Foo, *y.Foo; e == a { + t.Errorf("expected difference between %v and %v", e, a) + } + if e, a := x.Bar, y.Bar; reflect.DeepEqual(e, a) { + t.Errorf("expected difference between %v and %v", e, a) + } + if e, a := *x.Baz.(*string), *y.Baz.(*string); e == a { + t.Errorf("expected difference between %v and %v", e, a) + } + if e, a := x.Qux, y.Qux; reflect.DeepEqual(e, a) { + t.Errorf("expected difference between %v and %v", e, a) + } +} + func TestConverter_CallsRegisteredFunctions(t *testing.T) { type A struct { Foo string