Merge pull request #325 from apelisse/marshal-v2

Use "jsonv2"/go-json-experimental to marshal OpenAPI v2

go.mod

@@ -25,6 +25,7 @@ require (
 	k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c
 	k8s.io/klog/v2 v2.2.0
 	k8s.io/utils v0.0.0-20210802155522-efc7438f0176
+	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.3
 	sigs.k8s.io/yaml v1.2.0
 )

go.sum

@@ -180,6 +180,8 @@ k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A=
 k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y=
 k8s.io/utils v0.0.0-20210802155522-efc7438f0176 h1:Mx0aa+SUAcNRQbs5jUzV8lkDlGFU8laZsY9jrcVX5SY=
 k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
+sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
+sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kFxnAMREiWFE=
 sigs.k8s.io/structured-merge-diff/v4 v4.2.3/go.mod h1:qjx8mGObPmV2aSZepjQjbmb2ihdVs8cGKBraizNC69E=
 sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=

pkg/aggregator/aggregator_test.go

@@ -35,7 +35,7 @@ type DebugSpec struct {
 }
 
 func (d DebugSpec) String() string {
-	bytes, err := json.MarshalIndent(d.Swagger, "", " ")
+	bytes, err := d.Swagger.MarshalJSON()
 	if err != nil {
 		return fmt.Sprintf("DebugSpec.String failed: %s", err)
 	}
@@ -1762,7 +1762,7 @@ func BenchmarkMergeSpecsIgnorePathConflictsWithKubeSpec(b *testing.B) {
 			}
 		}
 
-		specBytes, _ := json.Marshal(sp)
+		specBytes, _ := sp.MarshalJSON()
 		handler.ToProtoBinary(specBytes)
 
 		b.StopTimer()
@@ -2003,7 +2003,7 @@ func TestCloneSpec(t *testing.T) {
 }
 
 func cloneSpec(source *spec.Swagger) (*spec.Swagger, error) {
-	bytes, err := json.Marshal(source)
+	bytes, err := source.MarshalJSON()
 	if err != nil {
 		return nil, err
 	}

pkg/builder/openapi_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/emicklei/go-restful/v3"
 	"github.com/stretchr/testify/assert"
 	openapi "k8s.io/kube-openapi/pkg/common"
+	"k8s.io/kube-openapi/pkg/util/jsontesting"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 )
 
@@ -467,15 +468,17 @@ func TestBuildOpenAPISpec(t *testing.T) {
 	if !assert.NoError(err) {
 		return
 	}
-	expected_json, err := json.Marshal(expected)
+	expected_json, err := expected.MarshalJSON()
 	if !assert.NoError(err) {
 		return
 	}
-	actual_json, err := json.Marshal(swagger)
+	actual_json, err := swagger.MarshalJSON()
 	if !assert.NoError(err) {
 		return
 	}
-	assert.Equal(string(expected_json), string(actual_json))
+	if err := jsontesting.JsonCompare(expected_json, actual_json); err != nil {
+		t.Error(err)
+	}
 }
 
 func TestBuildOpenAPIDefinitionsForResource(t *testing.T) {
@@ -495,7 +498,9 @@ func TestBuildOpenAPIDefinitionsForResource(t *testing.T) {
 	if !assert.NoError(err) {
 		return
 	}
-	assert.Equal(string(expected_json), string(actual_json))
+	if err := jsontesting.JsonCompare(expected_json, actual_json); err != nil {
+		t.Error(err)
+	}
 }
 
 func TestBuildOpenAPIDefinitionsForResourceWithExtensionV2Schema(t *testing.T) {

pkg/builder3/openapi_test.go

@@ -28,6 +28,7 @@ import (
 
 	openapi "k8s.io/kube-openapi/pkg/common"
 	"k8s.io/kube-openapi/pkg/spec3"
+	"k8s.io/kube-openapi/pkg/util/jsontesting"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 )
 
@@ -462,5 +463,7 @@ func TestBuildOpenAPISpec(t *testing.T) {
 	if !assert.NoError(err) {
 		return
 	}
-	assert.Equal(string(expected_json), string(actual_json))
+	if err := jsontesting.JsonCompare(expected_json, actual_json); err != nil {
+		t.Error(err)
+	}
 }

pkg/handler/handler.go

@@ -19,7 +19,6 @@ package handler
 import (
 	"bytes"
 	"crypto/sha512"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -106,7 +105,7 @@ func (o *OpenAPIService) UpdateSpec(openapiSpec *spec.Swagger) (err error) {
 	o.rwMutex.Lock()
 	defer o.rwMutex.Unlock()
 	o.jsonCache = o.jsonCache.New(func() ([]byte, error) {
-		return json.Marshal(openapiSpec)
+		return openapiSpec.MarshalJSON()
 	})
 	o.protoCache = o.protoCache.New(func() ([]byte, error) {
 		json, err := o.jsonCache.Get()

pkg/handler/handler_test.go

@@ -27,7 +27,7 @@ func TestRegisterOpenAPIVersionedService(t *testing.T) {
 		t.Errorf("Unexpected error in unmarshalling SwaggerJSON: %v", err)
 	}
 
-	returnedJSON, err := json.Marshal(s)
+	returnedJSON, err := s.MarshalJSON()
 	if err != nil {
 		t.Errorf("Unexpected error in preparing returnedJSON: %v", err)
 	}

pkg/internal/flags.go

@@ -19,3 +19,6 @@ package internal
 // Used by tests to selectively disable experimental JSON unmarshaler
 var UseOptimizedJSONUnmarshaling bool = true
 var UseOptimizedJSONUnmarshalingV3 bool = false
+
+// Used by tests to selectively disable experimental JSON marshaler
+var UseOptimizedJSONMarshaling bool = true

pkg/internal/serialization.go

@@ -18,8 +18,15 @@ package internal
 
 import (
 	"github.com/go-openapi/jsonreference"
+	jsonv2 "k8s.io/kube-openapi/pkg/internal/third_party/go-json-experiment/json"
 )
 
+// DeterministicMarshal calls the jsonv2 library with the deterministic
+// flag in order to have stable marshaling.
+func DeterministicMarshal(in any) ([]byte, error) {
+	return jsonv2.MarshalOptions{Deterministic: true}.Marshal(jsonv2.EncodeOptions{}, in)
+}
+
 // JSONRefFromMap populates a json reference object if the map v contains a $ref key.
 func JSONRefFromMap(jsonRef *jsonreference.Ref, v map[string]interface{}) error {
 	if v == nil {

pkg/openapiconv/convert_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	"k8s.io/kube-openapi/pkg/spec3"
+	"k8s.io/kube-openapi/pkg/util/jsontesting"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 )
 
@@ -51,19 +52,19 @@ func TestConvert(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		openAPIV2JSONBeforeConversion, err := json.Marshal(swaggerSpec)
+		openAPIV2JSONBeforeConversion, err := swaggerSpec.MarshalJSON()
 		if err != nil {
 			t.Fatal(err)
 		}
 
 		convertedV3Spec := ConvertV2ToV3(&swaggerSpec)
 
-		openAPIV2JSONAfterConversion, err := json.Marshal(swaggerSpec)
+		openAPIV2JSONAfterConversion, err := swaggerSpec.MarshalJSON()
 		if err != nil {
 			t.Fatal(err)
 		}
-		if !reflect.DeepEqual(openAPIV2JSONBeforeConversion, openAPIV2JSONAfterConversion) {
-			t.Errorf("Expected OpenAPI V2 to be untouched before and after conversion")
+		if err := jsontesting.JsonCompare(openAPIV2JSONBeforeConversion, openAPIV2JSONAfterConversion); err != nil {
+			t.Errorf("Expected OpenAPI V2 to be untouched before and after conversion: %v", err)
 		}
 
 		spec3JSON, err := os.ReadFile(filepath.Join("testdata_generated_from_k8s/v3_" + tc.groupVersion + ".json"))

pkg/schemamutation/walker_test.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	fuzz "github.com/google/gofuzz"
+	"k8s.io/kube-openapi/pkg/util/jsontesting"
 	"k8s.io/kube-openapi/pkg/util/sets"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 )
@@ -220,7 +221,7 @@ func TestReplaceReferences(t *testing.T) {
 			}
 
 			// find refs
-			bs, err := json.Marshal(s)
+			bs, err := s.MarshalJSON()
 			if err != nil {
 				t.Fatalf("failed to marshal swagger: %v", err)
 			}
@@ -244,7 +245,7 @@ func TestReplaceReferences(t *testing.T) {
 				}
 			}
 
-			origString, err := json.Marshal(s)
+			origString, err := s.MarshalJSON()
 			if err != nil {
 				t.Fatalf("failed to marshal swagger: %v", err)
 			}
@@ -364,8 +365,8 @@ func TestReplaceSchema(t *testing.T) {
 			if err != nil {
 				t.Fatalf("cannot marshal mutated schema: %v", err)
 			}
-			if mutatedWithString != string(newBytes) {
-				t.Fatalf("mutation result mismatches, got %q but expected %q, diff: %s\n", mutatedWithString, newBytes, stringDiff(mutatedWithString, string(newBytes)))
+			if err := jsontesting.JsonCompare(newBytes, []byte(mutatedWithString)); err != nil {
+				t.Error(err)
 			}
 			if !strings.Contains(origJSON, `"enum":[`) {
 				t.Logf("did not contain enum, skipping enum checks")
@@ -390,13 +391,13 @@ func TestReplaceSchema(t *testing.T) {
 }
 
 func cloneSwagger(orig *spec.Swagger) (*spec.Swagger, error) {
-	bs, err := json.Marshal(orig)
+	bs, err := orig.MarshalJSON()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error marshaling: %v", err)
 	}
 	s := &spec.Swagger{}
 	if err := json.Unmarshal(bs, s); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error unmarshaling: %v", err)
 	}
 	return s, nil
 }

pkg/spec3/component_test.go

@@ -20,11 +20,12 @@ import (
 	"encoding/json"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
+	"k8s.io/kube-openapi/pkg/util/jsontesting"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 
-	"k8s.io/kube-openapi/pkg/spec3"
 	"reflect"
+
+	"k8s.io/kube-openapi/pkg/spec3"
 )
 
 func TestSchemasJSONSerialization(t *testing.T) {
@@ -159,9 +160,8 @@ func TestSchemasJSONSerialization(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			serializedTarget := string(rawTarget)
-			if !cmp.Equal(serializedTarget, tc.expectedOutput) {
-				t.Fatalf("diff %s", cmp.Diff(serializedTarget, tc.expectedOutput))
+			if err := jsontesting.JsonCompare([]byte(tc.expectedOutput), rawTarget); err != nil {
+				t.Error(err)
 			}
 
 			var expected spec3.Components

pkg/spec3/operation_test.go

@@ -20,8 +20,8 @@ import (
 	"encoding/json"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	"k8s.io/kube-openapi/pkg/spec3"
+	"k8s.io/kube-openapi/pkg/util/jsontesting"
 	"k8s.io/kube-openapi/pkg/validation/spec"
 )
 
@@ -109,8 +109,8 @@ func TestOperationJSONSerialization(t *testing.T) {
 				t.Fatal(err)
 			}
 			serializedTarget := string(rawTarget)
-			if !cmp.Equal(serializedTarget, tc.expectedOutput) {
-				t.Fatalf("diff %s", cmp.Diff(serializedTarget, tc.expectedOutput))
+			if err := jsontesting.JsonCompare([]byte(tc.expectedOutput), []byte(serializedTarget)); err != nil {
+				t.Fatalf("diff %s", err)
 			}
 		})
 	}

pkg/util/jsontesting/json_roundtrip.go

@@ -22,9 +22,25 @@ import (
 	"reflect"
 	"strings"
 
+	kjson "sigs.k8s.io/json"
+
+	"github.com/go-openapi/jsonreference"
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 )
 
+func JsonCompare(got, want []byte) error {
+	if d := cmp.Diff(got, want, cmp.Transformer("JSONBytes", func(in []byte) (out interface{}) {
+		if strictErrors, err := kjson.UnmarshalStrict(in, &out); strictErrors != nil || err != nil {
+			return in
+		}
+		return out
+	})); d != "" {
+		return fmt.Errorf("JSON mismatch (-got +want):\n%s", d)
+	}
+	return nil
+}
+
 type RoundTripTestCase struct {
 	Name   string
 	JSON   string
@@ -39,7 +55,12 @@ type RoundTripTestCase struct {
 	ExpectedUnmarshalError string
 }
 
-func (t RoundTripTestCase) RoundTripTest(example json.Unmarshaler) error {
+type MarshalerUnmarshaler interface {
+	json.Unmarshaler
+	json.Marshaler
+}
+
+func (t RoundTripTestCase) RoundTripTest(example MarshalerUnmarshaler) error {
 	var jsonBytes []byte
 	var err error
 
@@ -79,14 +100,19 @@ func (t RoundTripTestCase) RoundTripTest(example json.Unmarshaler) error {
 	}
 
 	if t.Object != nil && !reflect.DeepEqual(t.Object, example) {
-		return fmt.Errorf("test case expected to unmarshal to specific value: %v", cmp.Diff(t.Object, example))
+		return fmt.Errorf("test case expected to unmarshal to specific value: %v", cmp.Diff(t.Object, example, cmpopts.IgnoreUnexported(jsonreference.Ref{})))
 	}
 
-	reEncoded, err := json.MarshalIndent(example, "", "    ")
+	reEncoded, err := json.Marshal(example)
 	if err != nil {
 		return fmt.Errorf("failed to marshal decoded value: %w", err)
 	}
 
+	reEncodedV2, err := example.MarshalJSON()
+	if err != nil {
+		return fmt.Errorf("failed to marshal decoded value with v2: %w", err)
+	}
+
 	// Check expected marshal error if it has not yet been checked
 	// (for case where JSON is provided, and object is not)
 	if testFinished, err := expectError(err, "marshal", t.ExpectedMarshalError); testFinished {
@@ -109,5 +135,9 @@ func (t RoundTripTestCase) RoundTripTest(example json.Unmarshaler) error {
 		return fmt.Errorf("expected equal values: %v", cmp.Diff(expected, actual))
 	}
 
+	if err := JsonCompare(reEncoded, reEncodedV2); err != nil {
+		return err
+	}
+
 	return nil
 }