From 8df914ae87e4cda1a68740f7692db73a2cc621ac Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Fri, 1 Mar 2024 17:26:10 -0500 Subject: [PATCH] Add tests for CBOR encoder handling of duplicate field names/tags. --- .../cbor/internal/modes/encode_test.go | 77 +++++++++++++ .../pkg/runtime/serializer/json/json_test.go | 109 ++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go new file mode 100644 index 00000000000..1d9b8e47880 --- /dev/null +++ b/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) + } + }) + } + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_test.go index 07a74e2157c..501f0d26188 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_test.go +++ b/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) + } + }) + } +}