From 0d796c184d1c151e1c5b2caa69f819f6c0e0eafc Mon Sep 17 00:00:00 2001 From: deads2k Date: Tue, 5 May 2015 10:36:39 -0400 Subject: [PATCH] fix DeepCopy to properly support runtime.EmbeddedObject --- pkg/conversion/scheme.go | 55 ++++++++++++++++++++++++++++++++++++ pkg/runtime/embedded_test.go | 38 +++++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/pkg/conversion/scheme.go b/pkg/conversion/scheme.go index 88580f730ec..f842f103104 100644 --- a/pkg/conversion/scheme.go +++ b/pkg/conversion/scheme.go @@ -17,6 +17,7 @@ limitations under the License. package conversion import ( + "encoding/gob" "fmt" "reflect" ) @@ -112,6 +113,10 @@ func (s *Scheme) AddKnownTypes(version string, types ...interface{}) { knownTypes[t.Name()] = t s.typeToVersion[t] = version s.typeToKind[t] = append(s.typeToKind[t], t.Name()) + + // Register with gob so that DeepCopy can recognize all of our objects. This is creating a static list, but it appears that gob itself wants a static list + gobName := getGobTypeName(obj) + gob.RegisterName(gobName, obj) } } @@ -135,6 +140,56 @@ func (s *Scheme) AddKnownTypeWithName(version, kind string, obj interface{}) { knownTypes[kind] = t s.typeToVersion[t] = version s.typeToKind[t] = append(s.typeToKind[t], kind) + + // Register with gob so that DeepCopy can recognize all of our objects. This is creating a static list, but it appears that gob itself wants a static list + gobName := getGobTypeName(obj) + gob.RegisterName(gobName, obj) +} + +// getGobTypeName creates a fully unique type name for the object being passed through. There is a bug in the gob encoder's name mechanism that they are unwilling to fix +// due to backwards compatibility concerns. See https://github.com/golang/go/blob/master/src/encoding/gob/type.go#L857 . This gives us a fully qualified name to avoid +// conflicts amongst our objects and since we all agree on the names, this should be safe +func getGobTypeName(value interface{}) string { + // mostly copied from gob/type.go + + // Default to printed representation for unnamed types + rt := reflect.TypeOf(value) + name := rt.String() + + // But for named types (or pointers to them), qualify with import path (but see inner comment). + // Dereference one pointer looking for a named type. + star := "" + if rt.Name() == "" { + if pt := rt; pt.Kind() == reflect.Ptr { + star = "*" + // NOTE: The following line should be rt = pt.Elem() to implement + // what the comment above claims, but fixing it would break compatibility + // with existing gobs. + // + // Given package p imported as "full/p" with these definitions: + // package p + // type T1 struct { ... } + // this table shows the intended and actual strings used by gob to + // name the types: + // + // Type Correct string Actual string + // + // T1 full/p.T1 full/p.T1 + // *T1 *full/p.T1 *p.T1 + // + // The missing full path cannot be fixed without breaking existing gob decoders. + rt = pt.Elem() // TWEAKED HERE + } + } + if rt.Name() != "" { + if rt.PkgPath() == "" { + name = star + rt.Name() + } else { + name = star + rt.PkgPath() + "." + rt.Name() + } + } + + return name } // KnownTypes returns an array of the types that are known for a particular version. diff --git a/pkg/runtime/embedded_test.go b/pkg/runtime/embedded_test.go index 2683668f1f7..6f7ea92dd36 100644 --- a/pkg/runtime/embedded_test.go +++ b/pkg/runtime/embedded_test.go @@ -21,6 +21,7 @@ import ( "reflect" "testing" + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -201,3 +202,40 @@ func TestEmbeddedObject(t *testing.T) { t.Errorf("Expected embedded objects to be nil: %#v", a) } } + +// TestDeepCopyOfEmbeddedObject checks to make sure that EmbeddedObject's can be passed through DeepCopy with fidelity +func TestDeepCopyOfEmbeddedObject(t *testing.T) { + s := runtime.NewScheme() + s.AddKnownTypes("", &EmbeddedTest{}) + s.AddKnownTypeWithName("v1test", "EmbeddedTest", &EmbeddedTestExternal{}) + + original := &EmbeddedTest{ + ID: "outer", + Object: runtime.EmbeddedObject{ + &EmbeddedTest{ + ID: "inner", + }, + }, + } + + originalData, err := s.EncodeToVersion(original, "v1test") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + t.Logf("originalRole = %v\n", string(originalData)) + + copyOfOriginal, err := conversion.DeepCopy(original) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + copiedData, err := s.EncodeToVersion(copyOfOriginal.(runtime.Object), "v1test") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + t.Logf("copyOfRole = %v\n", string(copiedData)) + + if !reflect.DeepEqual(original, copyOfOriginal) { + t.Errorf("expected \n%v\n, got \n%v", string(originalData), string(copiedData)) + } +}