Merge pull request #7769 from deads2k/fix-deep-copy

fix DeepCopy to properly support runtime.EmbeddedObject

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.

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))
+	}
+}