Handle aliases correctly in deepcopy/conversion

2025-07-23 11:50:44 +00:00 · 2016-06-12 22:58:06 -04:00 · 2016-06-12 22:58:06 -04:00 · 1c8b928908
commit 1c8b928908
parent 5f9e7a00b8
5 changed files with 75 additions and 53 deletions
--- a/cmd/libs/go2idl/conversion-gen/generators/conversion.go
+++ b/cmd/libs/go2idl/conversion-gen/generators/conversion.go
@ -367,6 +367,14 @@ func isDirectlyConvertible(in, out *types.Type, preexisting conversions) bool {
 	return false
 }

+// unwrapAlias recurses down aliased types to find the bedrock type.
+func unwrapAlias(in *types.Type) *types.Type {
+	if in.Kind == types.Alias {
+		return unwrapAlias(in.Underlying)
+	}
+	return in
+}
+
 func areTypesAliased(in, out *types.Type) bool {
 	// If one of the types is Alias, resolve it.
 	if in.Kind == types.Alias {
@ -685,11 +693,25 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			// this field has "genconversion=false" comment to ignore it.
 			continue
 		}
+		t, outT := m.Type, outMember.Type
+		// create a copy of both underlying types but give them the top level alias name (since aliases
+		// are assignable)
+		if underlying := unwrapAlias(t); underlying != t {
+			copied := *underlying
+			copied.Name = t.Name
+			t = &copied
+		}
+		if underlying := unwrapAlias(outT); underlying != outT {
+			copied := *underlying
+			copied.Name = outT.Name
+			outT = &copied
+		}
 		args := map[string]interface{}{
-			"inType":  m.Type,
-			"outType": outMember.Type,
+			"inType":  t,
+			"outType": outT,
 			"name":    m.Name,
 		}
+		// check based on the top level name, not the underlying names
 		if function, ok := g.preexists(m.Type, outMember.Type); ok {
 			args["function"] = function
 			sw.Do("if err := $.function|raw$(&in.$.name$, &out.$.name$, s); err != nil {\n", args)
@ -697,32 +719,32 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			sw.Do("}\n", nil)
 			continue
 		}
-		switch m.Type.Kind {
+		switch t.Kind {
 		case types.Builtin:
-			if m.Type == outMember.Type {
+			if t == outT {
 				sw.Do("out.$.name$ = in.$.name$\n", args)
 			} else {
 				sw.Do("out.$.name$ = $.outType|raw$(in.$.name$)\n", args)
 			}
 		case types.Map, types.Slice, types.Pointer:
-			if g.isDirectlyAssignable(m.Type, outMember.Type) {
+			if g.isDirectlyAssignable(t, outT) {
 				sw.Do("out.$.name$ = in.$.name$\n", args)
 				continue
 			}

 			sw.Do("if in.$.name$ != nil {\n", args)
 			sw.Do("in, out := &in.$.name$, &out.$.name$\n", args)
-			g.generateFor(m.Type, outMember.Type, sw)
+			g.generateFor(t, outT, sw)
 			sw.Do("} else {\n", nil)
 			sw.Do("out.$.name$ = nil\n", args)
 			sw.Do("}\n", nil)
 		case types.Struct:
-			if g.isDirectlyAssignable(m.Type, outMember.Type) {
+			if g.isDirectlyAssignable(t, outT) {
 				sw.Do("out.$.name$ = in.$.name$\n", args)
 				continue
 			}
-			if g.convertibleOnlyWithinPackage(m.Type, outMember.Type) {
-				funcName := g.funcNameTmpl(m.Type, outMember.Type)
+			if g.convertibleOnlyWithinPackage(t, outT) {
+				funcName := g.funcNameTmpl(t, outT)
 				sw.Do(fmt.Sprintf("if err := %s(&in.$.name$, &out.$.name$, s); err != nil {\n", funcName), args)
 			} else {
 				sw.Do("// TODO: Inefficient conversion - can we improve it?\n", nil)
@ -731,15 +753,15 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			sw.Do("return err\n", nil)
 			sw.Do("}\n", nil)
 		case types.Alias:
-			if outMember.Type.IsAssignable() {
-				if m.Type == outMember.Type {
+			if outT.IsAssignable() {
+				if t == outT {
 					sw.Do("out.$.name$ = in.$.name$\n", args)
 				} else {
 					sw.Do("out.$.name$ = $.outType|raw$(in.$.name$)\n", args)
 				}
 			} else {
-				if g.convertibleOnlyWithinPackage(m.Type, outMember.Type) {
-					funcName := g.funcNameTmpl(m.Type, outMember.Type)
+				if g.convertibleOnlyWithinPackage(t, outT) {
+					funcName := g.funcNameTmpl(t, outT)
 					sw.Do(fmt.Sprintf("if err := %s(&in.$.name$, &out.$.name$, s); err != nil {\n", funcName), args)
 				} else {
 					sw.Do("// TODO: Inefficient conversion - can we improve it?\n", nil)
@ -749,8 +771,8 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 				sw.Do("}\n", nil)
 			}
 		default:
-			if g.convertibleOnlyWithinPackage(m.Type, outMember.Type) {
-				funcName := g.funcNameTmpl(m.Type, outMember.Type)
+			if g.convertibleOnlyWithinPackage(t, outT) {
+				funcName := g.funcNameTmpl(t, outT)
 				sw.Do(fmt.Sprintf("if err := %s(&in.$.name$, &out.$.name$, s); err != nil {\n", funcName), args)
 			} else {
 				sw.Do("// TODO: Inefficient conversion - can we improve it?\n", nil)
--- a/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go
+++ b/cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go
@ -409,23 +409,29 @@ func (g *genDeepCopy) doSlice(t *types.Type, sw *generator.SnippetWriter) {

 func (g *genDeepCopy) doStruct(t *types.Type, sw *generator.SnippetWriter) {
 	for _, m := range t.Members {
+		t := m.Type
+		if t.Kind == types.Alias {
+			copied := *t.Underlying
+			copied.Name = t.Name
+			t = &copied
+		}
 		args := map[string]interface{}{
-			"type": m.Type,
+			"type": t,
 			"name": m.Name,
 		}
-		switch m.Type.Kind {
+		switch t.Kind {
 		case types.Builtin:
 			sw.Do("out.$.name$ = in.$.name$\n", args)
 		case types.Map, types.Slice, types.Pointer:
 			sw.Do("if in.$.name$ != nil {\n", args)
 			sw.Do("in, out := in.$.name$, &out.$.name$\n", args)
-			g.generateFor(m.Type, sw)
+			g.generateFor(t, sw)
 			sw.Do("} else {\n", nil)
 			sw.Do("out.$.name$ = nil\n", args)
 			sw.Do("}\n", nil)
 		case types.Struct:
-			if g.canInlineTypeFn(g.context, m.Type) {
-				funcName := g.funcNameTmpl(m.Type)
+			if g.canInlineTypeFn(g.context, t) {
+				funcName := g.funcNameTmpl(t)
 				sw.Do(fmt.Sprintf("if err := %s(in.$.name$, &out.$.name$, c); err != nil {\n", funcName), args)
 				sw.Do("return err\n", nil)
 				sw.Do("}\n", nil)
@ -437,17 +443,13 @@ func (g *genDeepCopy) doStruct(t *types.Type, sw *generator.SnippetWriter) {
 				sw.Do("}\n", nil)
 			}
 		default:
-			if m.Type.Kind == types.Alias && m.Type.Underlying.Kind == types.Builtin {
-				sw.Do("out.$.name$ = in.$.name$\n", args)
-			} else {
-				sw.Do("if in.$.name$ == nil {\n", args)
-				sw.Do("out.$.name$ = nil\n", args)
-				sw.Do("} else if newVal, err := c.DeepCopy(in.$.name$); err != nil {\n", args)
-				sw.Do("return err\n", nil)
-				sw.Do("} else {\n", nil)
-				sw.Do("out.$.name$ = newVal.($.type|raw$)\n", args)
-				sw.Do("}\n", nil)
-			}
+			sw.Do("if in.$.name$ == nil {\n", args)
+			sw.Do("out.$.name$ = nil\n", args)
+			sw.Do("} else if newVal, err := c.DeepCopy(in.$.name$); err != nil {\n", args)
+			sw.Do("return err\n", nil)
+			sw.Do("} else {\n", nil)
+			sw.Do("out.$.name$ = newVal.($.type|raw$)\n", args)
+			sw.Do("}\n", nil)
 		}
 	}
 }
--- a/cmd/libs/go2idl/deepcopy-gen/main.go
+++ b/cmd/libs/go2idl/deepcopy-gen/main.go
@ -38,6 +38,19 @@ func main() {

 	// Override defaults. These are Kubernetes specific input locations.
 	arguments.InputDirs = []string{
+		// generate all types, but do not register them
+		"+k8s.io/kubernetes/pkg/api/unversioned",
+
+		"-k8s.io/kubernetes/pkg/api/meta",
+		"-k8s.io/kubernetes/pkg/api/meta/metatypes",
+		"-k8s.io/kubernetes/pkg/api/resource",
+		"-k8s.io/kubernetes/pkg/conversion",
+		"-k8s.io/kubernetes/pkg/labels",
+		"-k8s.io/kubernetes/pkg/runtime",
+		"-k8s.io/kubernetes/pkg/runtime/serializer",
+		"-k8s.io/kubernetes/pkg/util/intstr",
+		"-k8s.io/kubernetes/pkg/util/sets",
+
 		"k8s.io/kubernetes/pkg/api",
 		"k8s.io/kubernetes/pkg/api/v1",
 		"k8s.io/kubernetes/pkg/apis/authentication.k8s.io",
@ -61,19 +74,6 @@ func main() {
 		"k8s.io/kubernetes/pkg/apis/rbac/v1alpha1",
 		"k8s.io/kubernetes/federation/apis/federation",
 		"k8s.io/kubernetes/federation/apis/federation/v1alpha1",
-
-		// generate all types, but do not register them
-		"+k8s.io/kubernetes/pkg/api/unversioned",
-
-		"-k8s.io/kubernetes/pkg/api/meta",
-		"-k8s.io/kubernetes/pkg/api/meta/metatypes",
-		"-k8s.io/kubernetes/pkg/api/resource",
-		"-k8s.io/kubernetes/pkg/conversion",
-		"-k8s.io/kubernetes/pkg/labels",
-		"-k8s.io/kubernetes/pkg/runtime",
-		"-k8s.io/kubernetes/pkg/runtime/serializer",
-		"-k8s.io/kubernetes/pkg/util/intstr",
-		"-k8s.io/kubernetes/pkg/util/sets",
 	}

 	if err := arguments.Execute(
--- a/cmd/libs/go2idl/parser/parse.go
+++ b/cmd/libs/go2idl/parser/parse.go
@ -591,9 +591,6 @@ func (b *Builder) walkType(u types.Universe, useName *types.Name, in tc.Type) *t
 			// "feature" for users. This flattens those types
 			// together.
 			name := tcNameToName(t.String())
-			if name.Name == "OptionalMap" {
-				fmt.Printf("DEBUG: flattening %T -> %T\n", t, t.Underlying())
-			}
 			if out := u.Type(name); out.Kind != types.Unknown {
 				return out // short circuit if we've already made this.
 			}
--- a/pkg/api/v1/conversion_generated.go
+++ b/pkg/api/v1/conversion_generated.go
@ -25,6 +25,7 @@ import (
 	resource "k8s.io/kubernetes/pkg/api/resource"
 	conversion "k8s.io/kubernetes/pkg/conversion"
 	runtime "k8s.io/kubernetes/pkg/runtime"
+	types "k8s.io/kubernetes/pkg/types"
 )

 func init() {
@ -3593,7 +3594,7 @@ func autoConvert_v1_ObjectMeta_To_api_ObjectMeta(in *ObjectMeta, out *api.Object
 	out.GenerateName = in.GenerateName
 	out.Namespace = in.Namespace
 	out.SelfLink = in.SelfLink
-	out.UID = in.UID
+	out.UID = types.UID(in.UID)
 	out.ResourceVersion = in.ResourceVersion
 	out.Generation = in.Generation
 	if err := api.Convert_unversioned_Time_To_unversioned_Time(&in.CreationTimestamp, &out.CreationTimestamp, s); err != nil {
@ -3627,7 +3628,7 @@ func autoConvert_api_ObjectMeta_To_v1_ObjectMeta(in *api.ObjectMeta, out *Object
 	out.GenerateName = in.GenerateName
 	out.Namespace = in.Namespace
 	out.SelfLink = in.SelfLink
-	out.UID = in.UID
+	out.UID = types.UID(in.UID)
 	out.ResourceVersion = in.ResourceVersion
 	out.Generation = in.Generation
 	if err := api.Convert_unversioned_Time_To_unversioned_Time(&in.CreationTimestamp, &out.CreationTimestamp, s); err != nil {
@ -3660,7 +3661,7 @@ func autoConvert_v1_ObjectReference_To_api_ObjectReference(in *ObjectReference,
 	out.Kind = in.Kind
 	out.Namespace = in.Namespace
 	out.Name = in.Name
-	out.UID = in.UID
+	out.UID = types.UID(in.UID)
 	out.APIVersion = in.APIVersion
 	out.ResourceVersion = in.ResourceVersion
 	out.FieldPath = in.FieldPath
@ -3675,7 +3676,7 @@ func autoConvert_api_ObjectReference_To_v1_ObjectReference(in *api.ObjectReferen
 	out.Kind = in.Kind
 	out.Namespace = in.Namespace
 	out.Name = in.Name
-	out.UID = in.UID
+	out.UID = types.UID(in.UID)
 	out.APIVersion = in.APIVersion
 	out.ResourceVersion = in.ResourceVersion
 	out.FieldPath = in.FieldPath
@ -3690,7 +3691,7 @@ func autoConvert_v1_OwnerReference_To_api_OwnerReference(in *OwnerReference, out
 	out.APIVersion = in.APIVersion
 	out.Kind = in.Kind
 	out.Name = in.Name
-	out.UID = in.UID
+	out.UID = types.UID(in.UID)
 	out.Controller = in.Controller
 	return nil
 }
@ -3703,7 +3704,7 @@ func autoConvert_api_OwnerReference_To_v1_OwnerReference(in *api.OwnerReference,
 	out.APIVersion = in.APIVersion
 	out.Kind = in.Kind
 	out.Name = in.Name
-	out.UID = in.UID
+	out.UID = types.UID(in.UID)
 	out.Controller = in.Controller
 	return nil
 }