Merge pull request #123626 from benluddy/cbor-test-encode-no-duplicate-map-key

KEP-4222: Add tests for CBOR encoder handling of duplicate field names/tags.

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go

@@ -0,0 +1,77 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package modes_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/fxamacker/cbor/v2"
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestEncode(t *testing.T) {
+	for _, tc := range []struct {
+		name          string
+		modes         []cbor.EncMode
+		in            interface{}
+		want          []byte
+		assertOnError func(t *testing.T, e error)
+	}{
+		{
+			name: "all duplicate fields are ignored", // Matches behavior of JSON serializer.
+			in: struct {
+				A1 int `json:"a"`
+				A2 int `json:"a"` //nolint:govet // This is intentional to test that the encoder will not encode two map entries with the same key.
+			}{},
+			want:          []byte{0xa0}, // {}
+			assertOnError: assertNilError,
+		},
+		{
+			name: "only tagged field is considered if any are tagged", // Matches behavior of JSON serializer.
+			in: struct {
+				A       int
+				TaggedA int `json:"A"`
+			}{
+				A:       1,
+				TaggedA: 2,
+			},
+			want:          []byte{0xa1, 0x41, 0x41, 0x02}, // {"A": 2}
+			assertOnError: assertNilError,
+		},
+	} {
+		encModes := tc.modes
+		if len(encModes) == 0 {
+			encModes = allEncModes
+		}
+
+		for _, encMode := range encModes {
+			modeName, ok := encModeNames[encMode]
+			if !ok {
+				t.Fatal("test case configured to run against unrecognized mode")
+			}
+
+			t.Run(fmt.Sprintf("mode=%s/%s", modeName, tc.name), func(t *testing.T) {
+				out, err := encMode.Marshal(tc.in)
+				tc.assertOnError(t, err)
+				if diff := cmp.Diff(tc.want, out); diff != "" {
+					t.Errorf("unexpected output:\n%s", diff)
+				}
+			})
+		}
+	}
+}

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_test.go

@@ -17,10 +17,12 @@ limitations under the License.
 package json_test
 
 import (
+	"bytes"
 	"fmt"
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -29,6 +31,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	runtimetesting "k8s.io/apimachinery/pkg/runtime/testing"
 	"k8s.io/apimachinery/pkg/util/diff"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 type testDecodable struct {
@@ -933,3 +937,108 @@ func (t *mockTyper) ObjectKinds(obj runtime.Object) ([]schema.GroupVersionKind,
 func (t *mockTyper) Recognizes(_ schema.GroupVersionKind) bool {
 	return false
 }
+
+type testEncodableDuplicateTag struct {
+	metav1.TypeMeta `json:",inline"`
+
+	A1 int `json:"a"`
+	A2 int `json:"a"` //nolint:govet // This is intentional to test that the encoder will not encode two map entries with the same key.
+}
+
+func (testEncodableDuplicateTag) DeepCopyObject() runtime.Object {
+	panic("unimplemented")
+}
+
+type testEncodableTagMatchesUntaggedName struct {
+	metav1.TypeMeta `json:",inline"`
+
+	A       int
+	TaggedA int `json:"A"`
+}
+
+func (testEncodableTagMatchesUntaggedName) DeepCopyObject() runtime.Object {
+	panic("unimplemented")
+}
+
+type staticTextMarshaler int
+
+func (staticTextMarshaler) MarshalText() ([]byte, error) {
+	return []byte("static"), nil
+}
+
+type testEncodableMap[K comparable] map[K]interface{}
+
+func (testEncodableMap[K]) GetObjectKind() schema.ObjectKind {
+	panic("unimplemented")
+}
+
+func (testEncodableMap[K]) DeepCopyObject() runtime.Object {
+	panic("unimplemented")
+}
+
+func TestEncode(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		in   runtime.Object
+		want []byte
+	}{
+		// The Go visibility rules for struct fields are amended for JSON when deciding
+		// which field to marshal or unmarshal. If there are multiple fields at the same
+		// level, and that level is the least nested (and would therefore be the nesting
+		// level selected by the usual Go rules), the following extra rules apply:
+
+		// 1) Of those fields, if any are JSON-tagged, only tagged fields are considered,
+		//    even if there are multiple untagged fields that would otherwise conflict.
+		{
+			name: "only tagged field is considered if any are tagged",
+			in: &testEncodableTagMatchesUntaggedName{
+				A:       1,
+				TaggedA: 2,
+			},
+			want: []byte("{\"A\":2}\n"),
+		},
+		// 2) If there is exactly one field (tagged or not according to the first rule),
+		//    that is selected.
+		// 3) Otherwise there are multiple fields, and all are ignored; no error occurs.
+		{
+			name: "all duplicate fields are ignored",
+			in:   &testEncodableDuplicateTag{},
+			want: []byte("{}\n"),
+		},
+		{
+			name: "text marshaler keys can compare inequal but serialize to duplicates",
+			in: testEncodableMap[staticTextMarshaler]{
+				staticTextMarshaler(1): nil,
+				staticTextMarshaler(2): nil,
+			},
+			want: []byte("{\"static\":null,\"static\":null}\n"),
+		},
+		{
+			name: "time.Time keys can compare inequal but serialize to duplicates because time.Time implements TextMarshaler",
+			in: testEncodableMap[time.Time]{
+				time.Date(2222, 11, 30, 23, 59, 58, 57, time.UTC):              nil,
+				time.Date(2222, 11, 30, 23, 59, 58, 57, time.FixedZone("", 0)): nil,
+			},
+			want: []byte("{\"2222-11-30T23:59:58.000000057Z\":null,\"2222-11-30T23:59:58.000000057Z\":null}\n"),
+		},
+		{
+			name: "metav1.Time keys can compare inequal but serialize to duplicates because metav1.Time embeds time.Time which implements TextMarshaler",
+			in: testEncodableMap[metav1.Time]{
+				metav1.Date(2222, 11, 30, 23, 59, 58, 57, time.UTC):              nil,
+				metav1.Date(2222, 11, 30, 23, 59, 58, 57, time.FixedZone("", 0)): nil,
+			},
+			want: []byte("{\"2222-11-30T23:59:58.000000057Z\":null,\"2222-11-30T23:59:58.000000057Z\":null}\n"),
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var dst bytes.Buffer
+			s := json.NewSerializerWithOptions(json.DefaultMetaFactory, nil, nil, json.SerializerOptions{})
+			if err := s.Encode(tc.in, &dst); err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+			if diff := cmp.Diff(tc.want, dst.Bytes()); diff != "" {
+				t.Errorf("unexpected output:\n%s", diff)
+			}
+		})
+	}
+}