Fix null JSON round tripping

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/conversion.go

@@ -17,6 +17,8 @@ limitations under the License.
 package v1
 
 import (
+	"bytes"
+
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/conversion"
@@ -36,21 +38,30 @@ func Convert_apiextensions_JSONSchemaProps_To_v1_JSONSchemaProps(in *apiextensio
 	return nil
 }
 
+var nullLiteral = []byte(`null`)
+
 func Convert_apiextensions_JSON_To_v1_JSON(in *apiextensions.JSON, out *JSON, s conversion.Scope) error {
 	raw, err := json.Marshal(*in)
 	if err != nil {
 		return err
 	}
+	if len(raw) == 0 || bytes.Equal(raw, nullLiteral) {
+		// match JSON#UnmarshalJSON treatment of literal nulls
+		out.Raw = nil
+	} else {
 		out.Raw = raw
+	}
 	return nil
 }
 
 func Convert_v1_JSON_To_apiextensions_JSON(in *JSON, out *apiextensions.JSON, s conversion.Scope) error {
 	if in != nil {
 		var i interface{}
+		if len(in.Raw) > 0 && !bytes.Equal(in.Raw, nullLiteral) {
 			if err := json.Unmarshal(in.Raw, &i); err != nil {
 				return err
 			}
+		}
 		*out = i
 	} else {
 		out = nil

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/conversion_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1
 
 import (
+	"encoding/json"
 	"reflect"
 	"strings"
 	"testing"
@@ -605,3 +606,57 @@ func TestJSONConversion(t *testing.T) {
 		}
 	}
 }
+
+func TestJSONRoundTrip(t *testing.T) {
+	testcases := []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{
+			name: "nulls",
+			in:   `{"default":null,"enum":null,"example":null}`,
+			out:  `{}`,
+		},
+		{
+			name: "null values",
+			in:   `{"default":{"test":null},"enum":[null],"example":{"test":null}}`,
+			out:  `{"default":{"test":null},"enum":[null],"example":{"test":null}}`,
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	// add internal and external types
+	if err := apiextensions.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	if err := AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			external := &JSONSchemaProps{}
+			if err := json.Unmarshal([]byte(tc.in), external); err != nil {
+				t.Fatal(err)
+			}
+
+			internal := &apiextensions.JSONSchemaProps{}
+			if err := scheme.Convert(external, internal, nil); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			roundtripped := &JSONSchemaProps{}
+			if err := scheme.Convert(internal, roundtripped, nil); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			out, err := json.Marshal(roundtripped)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if string(out) != string(tc.out) {
+				t.Fatalf("expected\n%s\ngot\n%s", string(tc.out), string(out))
+			}
+		})
+	}
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/marshal.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1
 
 import (
+	"bytes"
 	"errors"
 
 	"k8s.io/apimachinery/pkg/util/json"
@@ -128,7 +129,7 @@ func (s JSON) MarshalJSON() ([]byte, error) {
 }
 
 func (s *JSON) UnmarshalJSON(data []byte) error {
-	if len(data) > 0 && string(data) != "null" {
+	if len(data) > 0 && !bytes.Equal(data, nullLiteral) {
 		s.Raw = data
 	}
 	return nil

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/conversion.go

@@ -17,6 +17,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"bytes"
+
 	"k8s.io/apimachinery/pkg/conversion"
 	"k8s.io/apimachinery/pkg/util/json"
 
@@ -36,21 +38,30 @@ func Convert_apiextensions_JSONSchemaProps_To_v1beta1_JSONSchemaProps(in *apiext
 	return nil
 }
 
+var nullLiteral = []byte(`null`)
+
 func Convert_apiextensions_JSON_To_v1beta1_JSON(in *apiextensions.JSON, out *JSON, s conversion.Scope) error {
 	raw, err := json.Marshal(*in)
 	if err != nil {
 		return err
 	}
+	if len(raw) == 0 || bytes.Equal(raw, nullLiteral) {
+		// match JSON#UnmarshalJSON treatment of literal nulls
+		out.Raw = nil
+	} else {
 		out.Raw = raw
+	}
 	return nil
 }
 
 func Convert_v1beta1_JSON_To_apiextensions_JSON(in *JSON, out *apiextensions.JSON, s conversion.Scope) error {
 	if in != nil {
 		var i interface{}
+		if len(in.Raw) > 0 && !bytes.Equal(in.Raw, nullLiteral) {
 			if err := json.Unmarshal(in.Raw, &i); err != nil {
 				return err
 			}
+		}
 		*out = i
 	} else {
 		out = nil

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/conversion_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
+	"encoding/json"
 	"reflect"
 	"testing"
 
@@ -111,3 +112,57 @@ func TestJSONConversion(t *testing.T) {
 		}
 	}
 }
+
+func TestJSONRoundTrip(t *testing.T) {
+	testcases := []struct {
+		name string
+		in   string
+		out  string
+	}{
+		{
+			name: "nulls",
+			in:   `{"default":null,"enum":null,"example":null}`,
+			out:  `{}`,
+		},
+		{
+			name: "null values",
+			in:   `{"default":{"test":null},"enum":[null],"example":{"test":null}}`,
+			out:  `{"default":{"test":null},"enum":[null],"example":{"test":null}}`,
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	// add internal and external types
+	if err := apiextensions.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	if err := AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			external := &JSONSchemaProps{}
+			if err := json.Unmarshal([]byte(tc.in), external); err != nil {
+				t.Fatal(err)
+			}
+
+			internal := &apiextensions.JSONSchemaProps{}
+			if err := scheme.Convert(external, internal, nil); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			roundtripped := &JSONSchemaProps{}
+			if err := scheme.Convert(internal, roundtripped, nil); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			out, err := json.Marshal(roundtripped)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if string(out) != string(tc.out) {
+				t.Fatalf("expected\n%s\ngot\n%s", string(tc.out), string(out))
+			}
+		})
+	}
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta1
 
 import (
+	"bytes"
 	"errors"
 
 	"k8s.io/apimachinery/pkg/util/json"
@@ -128,7 +129,7 @@ func (s JSON) MarshalJSON() ([]byte, error) {
 }
 
 func (s *JSON) UnmarshalJSON(data []byte) error {
-	if len(data) > 0 && string(data) != "null" {
+	if len(data) > 0 && !bytes.Equal(data, nullLiteral) {
 		s.Raw = data
 	}
 	return nil