Ensure empty serialized slices are zero-length, not null

cmd/libs/go2idl/conversion-gen/generators/conversion.go

@@ -30,6 +30,8 @@ import (
 	"k8s.io/gengo/namer"
 	"k8s.io/gengo/types"
 
+	"reflect"
+
 	"github.com/golang/glog"
 )
 
@@ -722,6 +724,15 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			outMemberType = &copied
 		}
 
+		// Determine if our destination field is a slice that should be output when empty.
+		// If it is, ensure a nil source slice converts to a zero-length destination slice.
+		// See http://issue.k8s.io/43203
+		persistEmptySlice := false
+		if outMemberType.Kind == types.Slice {
+			jsonTag := reflect.StructTag(outMember.Tags).Get("json")
+			persistEmptySlice = len(jsonTag) > 0 && !strings.Contains(jsonTag, ",omitempty")
+		}
+
 		args := argsFromType(inMemberType, outMemberType).With("name", inMember.Name)
 
 		// try a direct memory copy for any type that has exactly equivalent values
@@ -737,7 +748,15 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 				sw.Do("out.$.name$ = *(*$.outType|raw$)($.Pointer|raw$(&in.$.name$))\n", args)
 				continue
 			case types.Slice:
+				if persistEmptySlice {
+					sw.Do("if in.$.name$ == nil {\n", args)
+					sw.Do("out.$.name$ = make($.outType|raw$, 0)\n", args)
+					sw.Do("} else {\n", nil)
 					sw.Do("out.$.name$ = *(*$.outType|raw$)($.Pointer|raw$(&in.$.name$))\n", args)
+					sw.Do("}\n", nil)
+				} else {
+					sw.Do("out.$.name$ = *(*$.outType|raw$)($.Pointer|raw$(&in.$.name$))\n", args)
+				}
 				continue
 			}
 		}
@@ -787,7 +806,11 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			sw.Do("in, out := &in.$.name$, &out.$.name$\n", args)
 			g.generateFor(inMemberType, outMemberType, sw)
 			sw.Do("} else {\n", nil)
+			if persistEmptySlice {
+				sw.Do("out.$.name$ = make($.outType|raw$, 0)\n", args)
+			} else {
 				sw.Do("out.$.name$ = nil\n", args)
+			}
 			sw.Do("}\n", nil)
 		case types.Struct:
 			if g.isDirectlyAssignable(inMemberType, outMemberType) {

pkg/apis/apps/v1beta1/defaults_test.go

@@ -20,6 +20,7 @@ import (
 	"reflect"
 	"testing"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/kubernetes/pkg/api"
@@ -173,7 +174,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 			t.Errorf("unexpected object: %v", got)
 			t.FailNow()
 		}
-		if !reflect.DeepEqual(got.Spec, expected.Spec) {
+		if !apiequality.Semantic.DeepEqual(got.Spec, expected.Spec) {
 			t.Errorf("object mismatch!\nexpected:\n\t%+v\ngot:\n\t%+v", got.Spec, expected.Spec)
 		}
 	}

pkg/apis/extensions/v1beta1/defaults_test.go

@@ -20,6 +20,7 @@ import (
 	"reflect"
 	"testing"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -156,7 +157,7 @@ func TestSetDefaultDaemonSet(t *testing.T) {
 			t.Errorf("(%d) unexpected object: %v", i, got)
 			t.FailNow()
 		}
-		if !reflect.DeepEqual(got.Spec, expected.Spec) {
+		if !apiequality.Semantic.DeepEqual(got.Spec, expected.Spec) {
 			t.Errorf("(%d) got different than expected\ngot:\n\t%+v\nexpected:\n\t%+v", i, got.Spec, expected.Spec)
 		}
 	}
@@ -295,7 +296,7 @@ func TestSetDefaultDeployment(t *testing.T) {
 			t.Errorf("unexpected object: %v", got)
 			t.FailNow()
 		}
-		if !reflect.DeepEqual(got.Spec, expected.Spec) {
+		if !apiequality.Semantic.DeepEqual(got.Spec, expected.Spec) {
 			t.Errorf("object mismatch!\nexpected:\n\t%+v\ngot:\n\t%+v", got.Spec, expected.Spec)
 		}
 	}

pkg/kubectl/cmd/util/factory_test.go

@@ -25,12 +25,12 @@ import (
 	"os"
 	"os/user"
 	"path"
-	"reflect"
 	"sort"
 	"strings"
 	"testing"
 	"time"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -623,7 +623,7 @@ func TestGetFirstPod(t *testing.T) {
 			t.Errorf("%s: expected %d pods, got %d", test.name, test.expectedNum, numPods)
 			continue
 		}
-		if !reflect.DeepEqual(test.expected, pod) {
+		if !apiequality.Semantic.DeepEqual(test.expected, pod) {
 			t.Errorf("%s:\nexpected pod:\n%#v\ngot:\n%#v\n\n", test.name, test.expected, pod)
 		}
 	}

pkg/kubectl/cmd/util/helpers_test.go

@@ -21,11 +21,11 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
-	"reflect"
 	"strings"
 	"syscall"
 	"testing"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -194,7 +194,7 @@ func TestMerge(t *testing.T) {
 		if !test.expectErr {
 			if err != nil {
 				t.Errorf("testcase[%d], unexpected error: %v", i, err)
-			} else if !reflect.DeepEqual(out, test.expected) {
+			} else if !apiequality.Semantic.DeepEqual(out, test.expected) {
 				t.Errorf("\n\ntestcase[%d]\nexpected:\n%+v\nsaw:\n%+v", i, test.expected, out)
 			}
 		}
@@ -374,7 +374,7 @@ func TestMaybeConvert(t *testing.T) {
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}
-		if !reflect.DeepEqual(test.expected, obj) {
+		if !apiequality.Semantic.DeepEqual(test.expected, obj) {
 			t.Errorf("expected:\n%#v\nsaw:\n%#v\n", test.expected, obj)
 		}
 	}

pkg/kubectl/resource/builder_test.go

@@ -556,7 +556,7 @@ func TestResourceByName(t *testing.T) {
 	if err != nil || !singleItemImplied || len(test.Infos) != 1 {
 		t.Fatalf("unexpected response: %v %t %#v", err, singleItemImplied, test.Infos)
 	}
-	if !reflect.DeepEqual(&pods.Items[0], test.Objects()[0]) {
+	if !apiequality.Semantic.DeepEqual(&pods.Items[0], test.Objects()[0]) {
 		t.Errorf("unexpected object: %#v", test.Objects()[0])
 	}
 
@@ -621,10 +621,10 @@ func TestResourceNames(t *testing.T) {
 	if err != nil || len(test.Infos) != 2 {
 		t.Fatalf("unexpected response: %v %#v", err, test.Infos)
 	}
-	if !reflect.DeepEqual(&pods.Items[0], test.Objects()[0]) {
+	if !apiequality.Semantic.DeepEqual(&pods.Items[0], test.Objects()[0]) {
 		t.Errorf("unexpected object: \n%#v, expected: \n%#v", test.Objects()[0], &pods.Items[0])
 	}
-	if !reflect.DeepEqual(&svc.Items[0], test.Objects()[1]) {
+	if !apiequality.Semantic.DeepEqual(&svc.Items[0], test.Objects()[1]) {
 		t.Errorf("unexpected object: \n%#v, expected: \n%#v", test.Objects()[1], &svc.Items[0])
 	}
 }
@@ -698,7 +698,7 @@ func TestResourceByNameAndEmptySelector(t *testing.T) {
 	if err != nil || !singleItemImplied || len(infos) != 1 {
 		t.Fatalf("unexpected response: %v %t %#v", err, singleItemImplied, infos)
 	}
-	if !reflect.DeepEqual(&pods.Items[0], infos[0].Object) {
+	if !apiequality.Semantic.DeepEqual(&pods.Items[0], infos[0].Object) {
 		t.Errorf("unexpected object: %#v", infos[0])
 	}
 

pkg/kubectl/rolling_updater_test.go

@@ -26,6 +26,7 @@ import (
 	"testing"
 	"time"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -1475,7 +1476,7 @@ func TestUpdateRcWithRetries(t *testing.T) {
 				updates = updates[1:]
 				// We should always get an update with a valid rc even when the get fails. The rc should always
 				// contain the update.
-				if c, ok := readOrDie(t, req, codec).(*api.ReplicationController); !ok || !reflect.DeepEqual(rc, c) {
+				if c, ok := readOrDie(t, req, codec).(*api.ReplicationController); !ok || !apiequality.Semantic.DeepEqual(rc, c) {
 					t.Errorf("Unexpected update body, got %+v expected %+v", c, rc)
 				} else if sel, ok := c.Spec.Selector["baz"]; !ok || sel != "foobar" {
 					t.Errorf("Expected selector label update, got %+v", c.Spec.Selector)