From 37071989a0b56674ca3e91c013fdbc6a01808648 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Fri, 14 Jun 2024 12:59:48 -0400 Subject: [PATCH] Support either JSON or CBOR in FieldsV1. The value of FieldsV1 is dependent upon its serialization. All existent FieldsV1 usage can safely assume either JSON null or a JSON object. This is notably different from RawExtension, which might hold arbitrary bytes. To retain backwards compatibility for existing programs, FieldsV1 values decoded from CBOR will be automatically transcoded to JSON. This will follow the same opt-out and migration plan as RawExtension. --- .../apimachinery/pkg/apis/meta/v1/helpers.go | 83 +++++- .../pkg/apis/meta/v1/helpers_test.go | 242 ++++++++++++++++++ .../pkg/runtime/serializer/cbor/cbor_test.go | 50 +++- .../pkg/runtime/serializer/cbor/raw.go | 18 ++ .../pkg/runtime/serializer/cbor/raw_test.go | 63 +++++ 5 files changed, 447 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go index 592dcb8a746..c748071ed7f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go @@ -24,8 +24,10 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" + cbor "k8s.io/apimachinery/pkg/runtime/serializer/cbor/direct" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/types" + utiljson "k8s.io/apimachinery/pkg/util/json" ) // LabelSelectorAsSelector converts the LabelSelector api type into a struct that implements @@ -280,13 +282,20 @@ func (f FieldsV1) MarshalJSON() ([]byte, error) { if f.Raw == nil { return []byte("null"), nil } + if f.getContentType() == fieldsV1InvalidOrValidCBORObject { + var u map[string]interface{} + if err := cbor.Unmarshal(f.Raw, &u); err != nil { + return nil, fmt.Errorf("metav1.FieldsV1 cbor invalid: %w", err) + } + return utiljson.Marshal(u) + } return f.Raw, nil } // UnmarshalJSON implements json.Unmarshaler func (f *FieldsV1) UnmarshalJSON(b []byte) error { if f == nil { - return errors.New("metav1.Fields: UnmarshalJSON on nil pointer") + return errors.New("metav1.FieldsV1: UnmarshalJSON on nil pointer") } if !bytes.Equal(b, []byte("null")) { f.Raw = append(f.Raw[0:0], b...) @@ -296,3 +305,75 @@ func (f *FieldsV1) UnmarshalJSON(b []byte) error { var _ json.Marshaler = FieldsV1{} var _ json.Unmarshaler = &FieldsV1{} + +func (f FieldsV1) MarshalCBOR() ([]byte, error) { + if f.Raw == nil { + return cbor.Marshal(nil) + } + if f.getContentType() == fieldsV1InvalidOrValidJSONObject { + var u map[string]interface{} + if err := utiljson.Unmarshal(f.Raw, &u); err != nil { + return nil, fmt.Errorf("metav1.FieldsV1 json invalid: %w", err) + } + return cbor.Marshal(u) + } + return f.Raw, nil +} + +var cborNull = []byte{0xf6} + +func (f *FieldsV1) UnmarshalCBOR(b []byte) error { + if f == nil { + return errors.New("metav1.FieldsV1: UnmarshalCBOR on nil pointer") + } + if !bytes.Equal(b, cborNull) { + f.Raw = append(f.Raw[0:0], b...) + } + return nil +} + +const ( + // fieldsV1InvalidOrEmpty indicates that a FieldsV1 either contains no raw bytes or its raw + // bytes don't represent an allowable value in any supported encoding. + fieldsV1InvalidOrEmpty = iota + + // fieldsV1InvalidOrValidJSONObject indicates that a FieldV1 either contains raw bytes that + // are a valid JSON encoding of an allowable value or don't represent an allowable value in + // any supported encoding. + fieldsV1InvalidOrValidJSONObject + + // fieldsV1InvalidOrValidCBORObject indicates that a FieldV1 either contains raw bytes that + // are a valid CBOR encoding of an allowable value or don't represent an allowable value in + // any supported encoding. + fieldsV1InvalidOrValidCBORObject +) + +// getContentType returns one of fieldsV1InvalidOrEmpty, fieldsV1InvalidOrValidJSONObject, +// fieldsV1InvalidOrValidCBORObject based on the value of Raw. +// +// Raw can be encoded in JSON or CBOR and is only valid if it is empty, null, or an object (map) +// value. It is invalid if it contains a JSON string, number, boolean, or array. If Raw is nonempty +// and represents an allowable value, then the initial byte unambiguously distinguishes a +// JSON-encoded value from a CBOR-encoded value. +// +// A valid JSON-encoded value can begin with any of the four JSON whitespace characters, the first +// character 'n' of null, or '{' (0x09, 0x0a, 0x0d, 0x20, 0x6e, or 0x7b, respectively). A valid +// CBOR-encoded value can begin with the null simple value, an initial byte with major type "map", +// or, if a tag-enclosed map, an initial byte with major type "tag" (0xf6, 0xa0...0xbf, or +// 0xc6...0xdb). The two sets of valid initial bytes don't intersect. +func (f FieldsV1) getContentType() int { + if len(f.Raw) > 0 { + p := f.Raw[0] + switch p { + case 'n', '{', '\t', '\r', '\n', ' ': + return fieldsV1InvalidOrValidJSONObject + case 0xf6: // null + return fieldsV1InvalidOrValidCBORObject + default: + if p >= 0xa0 && p <= 0xbf /* map */ || p >= 0xc6 && p <= 0xdb /* tag */ { + return fieldsV1InvalidOrValidCBORObject + } + } + } + return fieldsV1InvalidOrEmpty +} diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers_test.go index 8ae538e09b8..66a7a873a9c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers_test.go @@ -249,3 +249,245 @@ func TestSetMetaDataLabel(t *testing.T) { } } } + +func TestFieldsV1MarshalJSON(t *testing.T) { + for _, tc := range []struct { + Name string + FieldsV1 FieldsV1 + Want []byte + Error string + }{ + { + Name: "nil encodes as json null", + FieldsV1: FieldsV1{}, + Want: []byte(`null`), + }, + { + Name: "empty invalid json is returned as-is", + FieldsV1: FieldsV1{Raw: []byte{}}, + Want: []byte{}, + }, + { + Name: "cbor null is transcoded to json null", + FieldsV1: FieldsV1{Raw: []byte{0xf6}}, // null + Want: []byte(`null`), + }, + { + Name: "valid non-map cbor and valid non-object json is returned as-is", + FieldsV1: FieldsV1{Raw: []byte{0x30}}, + Want: []byte{0x30}, // Valid CBOR encoding of -17 and JSON encoding of 0! + }, + { + Name: "self-described cbor map is transcoded to json map", + FieldsV1: FieldsV1{Raw: []byte{0xd9, 0xd9, 0xf7, 0xa1, 0x43, 'f', 'o', 'o', 0x43, 'b', 'a', 'r'}}, // 55799({"foo":"bar"}) + Want: []byte(`{"foo":"bar"}`), + }, + { + Name: "json object is returned as-is", + FieldsV1: FieldsV1{Raw: []byte(" \t\r\n{\"foo\":\"bar\"}")}, + Want: []byte(" \t\r\n{\"foo\":\"bar\"}"), + }, + { + Name: "invalid json is returned as-is", + FieldsV1: FieldsV1{Raw: []byte(`{{`)}, + Want: []byte(`{{`), + }, + { + Name: "invalid cbor fails to transcode to json", + FieldsV1: FieldsV1{Raw: []byte{0xa1}}, + Error: "metav1.FieldsV1 cbor invalid: unexpected EOF", + }, + } { + t.Run(tc.Name, func(t *testing.T) { + got, err := tc.FieldsV1.MarshalJSON() + if err != nil { + if tc.Error == "" { + t.Fatalf("unexpected error: %v", err) + } + if msg := err.Error(); msg != tc.Error { + t.Fatalf("expected error %q, got %q", tc.Error, msg) + } + } else if tc.Error != "" { + t.Fatalf("expected error %q, got nil", tc.Error) + } + if diff := cmp.Diff(tc.Want, got); diff != "" { + t.Errorf("unexpected diff:\n%s", diff) + } + }) + } +} + +func TestFieldsV1MarshalCBOR(t *testing.T) { + for _, tc := range []struct { + Name string + FieldsV1 FieldsV1 + Want []byte + Error string + }{ + { + Name: "nil encodes as cbor null", + FieldsV1: FieldsV1{}, + Want: []byte{0xf6}, // null + }, + { + Name: "empty invalid cbor is returned as-is", + FieldsV1: FieldsV1{Raw: []byte{}}, + Want: []byte{}, + }, + { + Name: "json null is transcoded to cbor null", + FieldsV1: FieldsV1{Raw: []byte(`null`)}, + Want: []byte{0xf6}, // null + }, + { + Name: "valid non-map cbor and valid non-object json is returned as-is", + FieldsV1: FieldsV1{Raw: []byte{0x30}}, + Want: []byte{0x30}, // Valid CBOR encoding of -17 and JSON encoding of 0! + }, + { + Name: "json object is transcoded to cbor map", + FieldsV1: FieldsV1{Raw: []byte(" \t\r\n{\"foo\":\"bar\"}")}, + Want: []byte{0xa1, 0x43, 'f', 'o', 'o', 0x43, 'b', 'a', 'r'}, + }, + { + Name: "self-described cbor map is returned as-is", + FieldsV1: FieldsV1{Raw: []byte{0xd9, 0xd9, 0xf7, 0xa1, 0x43, 'f', 'o', 'o', 0x43, 'b', 'a', 'r'}}, // 55799({"foo":"bar"}) + Want: []byte{0xd9, 0xd9, 0xf7, 0xa1, 0x43, 'f', 'o', 'o', 0x43, 'b', 'a', 'r'}, // 55799({"foo":"bar"}) + }, + { + Name: "invalid json fails to transcode to cbor", + FieldsV1: FieldsV1{Raw: []byte(`{{`)}, + Error: "metav1.FieldsV1 json invalid: invalid character '{' looking for beginning of object key string", + }, + { + Name: "invalid cbor is returned as-is", + FieldsV1: FieldsV1{Raw: []byte{0xa1}}, + Want: []byte{0xa1}, + }, + } { + t.Run(tc.Name, func(t *testing.T) { + got, err := tc.FieldsV1.MarshalCBOR() + if err != nil { + if tc.Error == "" { + t.Fatalf("unexpected error: %v", err) + } + if msg := err.Error(); msg != tc.Error { + t.Fatalf("expected error %q, got %q", tc.Error, msg) + } + } else if tc.Error != "" { + t.Fatalf("expected error %q, got nil", tc.Error) + } + + if diff := cmp.Diff(tc.Want, got); diff != "" { + t.Errorf("unexpected diff:\n%s", diff) + } + }) + } +} + +func TestFieldsV1UnmarshalJSON(t *testing.T) { + for _, tc := range []struct { + Name string + JSON []byte + Into *FieldsV1 + Want *FieldsV1 + Error string + }{ + { + Name: "nil receiver returns error", + Into: nil, + Error: "metav1.FieldsV1: UnmarshalJSON on nil pointer", + }, + { + Name: "json null does not modify receiver", // conventional for json.Unmarshaler + JSON: []byte(`null`), + Into: &FieldsV1{Raw: []byte(`unmodified`)}, + Want: &FieldsV1{Raw: []byte(`unmodified`)}, + }, + { + Name: "valid input is copied verbatim", + JSON: []byte("{\"foo\":\"bar\"} \t\r\n"), + Into: &FieldsV1{}, + Want: &FieldsV1{Raw: []byte("{\"foo\":\"bar\"} \t\r\n")}, + }, + { + Name: "invalid input is copied verbatim", + JSON: []byte("{{"), + Into: &FieldsV1{}, + Want: &FieldsV1{Raw: []byte("{{")}, + }, + } { + t.Run(tc.Name, func(t *testing.T) { + got := tc.Into.DeepCopy() + err := got.UnmarshalJSON(tc.JSON) + if err != nil { + if tc.Error == "" { + t.Fatalf("unexpected error: %v", err) + } + if msg := err.Error(); msg != tc.Error { + t.Fatalf("expected error %q, got %q", tc.Error, msg) + } + } else if tc.Error != "" { + t.Fatalf("expected error %q, got nil", tc.Error) + } + + if diff := cmp.Diff(tc.Want, got); diff != "" { + t.Errorf("unexpected diff:\n%s", diff) + } + }) + } +} + +func TestFieldsV1UnmarshalCBOR(t *testing.T) { + for _, tc := range []struct { + Name string + CBOR []byte + Into *FieldsV1 + Want *FieldsV1 + Error string + }{ + { + Name: "nil receiver returns error", + Into: nil, + Want: nil, + Error: "metav1.FieldsV1: UnmarshalCBOR on nil pointer", + }, + { + Name: "cbor null does not modify receiver", + CBOR: []byte{0xf6}, + Into: &FieldsV1{Raw: []byte(`unmodified`)}, + Want: &FieldsV1{Raw: []byte(`unmodified`)}, + }, + { + Name: "valid input is copied verbatim", + CBOR: []byte{0xa1, 0x43, 'f', 'o', 'o', 0x43, 'b', 'a', 'r'}, + Into: &FieldsV1{}, + Want: &FieldsV1{Raw: []byte{0xa1, 0x43, 'f', 'o', 'o', 0x43, 'b', 'a', 'r'}}, + }, + { + Name: "invalid input is copied verbatim", + CBOR: []byte{0xff}, // UnmarshalCBOR should never be called with malformed input, testing anyway. + Into: &FieldsV1{}, + Want: &FieldsV1{Raw: []byte{0xff}}, + }, + } { + t.Run(tc.Name, func(t *testing.T) { + got := tc.Into.DeepCopy() + err := got.UnmarshalCBOR(tc.CBOR) + if err != nil { + if tc.Error == "" { + t.Fatalf("unexpected error: %v", err) + } + if msg := err.Error(); msg != tc.Error { + t.Fatalf("expected error %q, got %q", tc.Error, msg) + } + } else if tc.Error != "" { + t.Fatalf("expected error %q, got nil", tc.Error) + } + + if diff := cmp.Diff(tc.Want, got); diff != "" { + t.Errorf("unexpected diff:\n%s", diff) + } + }) + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go index b95e7a18aec..db0d7fc4adc 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go @@ -121,15 +121,27 @@ func (p *anyObject) UnmarshalCBOR(in []byte) error { return modes.Decode.Unmarshal(in, &p.Value) } -type structWithRawExtensionField struct { - Extension runtime.RawExtension `json:"extension"` +type structWithRawFields struct { + FieldsV1 metav1.FieldsV1 `json:"f"` + FieldsV1Pointer *metav1.FieldsV1 `json:"fp"` + RawExtension runtime.RawExtension `json:"r"` + RawExtensionPointer *runtime.RawExtension `json:"rp"` } -func (p structWithRawExtensionField) GetObjectKind() schema.ObjectKind { +func (structWithRawFields) GetObjectKind() schema.ObjectKind { return schema.EmptyObjectKind } -func (structWithRawExtensionField) DeepCopyObject() runtime.Object { +func (structWithRawFields) DeepCopyObject() runtime.Object { + panic("unimplemented") +} + +type structWithEmbeddedMetas struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` +} + +func (structWithEmbeddedMetas) DeepCopyObject() runtime.Object { panic("unimplemented") } @@ -277,13 +289,35 @@ func TestDecode(t *testing.T) { }, }, { - name: "rawextension transcoded", - data: []byte{0xa1, 0x49, 'e', 'x', 't', 'e', 'n', 's', 'i', 'o', 'n', 0xa1, 0x41, 'a', 0x01}, + name: "raw types transcoded", + data: []byte{0xa4, 0x41, 'f', 0xa1, 0x41, 'a', 0x01, 0x42, 'f', 'p', 0xa1, 0x41, 'z', 0x02, 0x41, 'r', 0xa1, 0x41, 'b', 0x03, 0x42, 'r', 'p', 0xa1, 0x41, 'y', 0x04}, gvk: &schema.GroupVersionKind{}, metaFactory: stubMetaFactory{gvk: &schema.GroupVersionKind{}}, typer: stubTyper{gvks: []schema.GroupVersionKind{{Group: "x", Version: "y", Kind: "z"}}}, - into: &structWithRawExtensionField{}, - expectedObj: &structWithRawExtensionField{Extension: runtime.RawExtension{Raw: []byte(`{"a":1}`)}}, + into: &structWithRawFields{}, + expectedObj: &structWithRawFields{ + FieldsV1: metav1.FieldsV1{Raw: []byte(`{"a":1}`)}, + FieldsV1Pointer: &metav1.FieldsV1{Raw: []byte(`{"z":2}`)}, + RawExtension: runtime.RawExtension{Raw: []byte(`{"b":3}`)}, + RawExtensionPointer: &runtime.RawExtension{Raw: []byte(`{"y":4}`)}, + }, + expectedGVK: &schema.GroupVersionKind{Group: "x", Version: "y", Kind: "z"}, + assertOnError: func(t *testing.T, err error) { + if err != nil { + t.Errorf("expected nil error, got: %v", err) + } + }, + }, + { + name: "object with embedded typemeta and objectmeta", + data: []byte("\xa2\x48metadata\xa1\x44name\x43foo\x44spec\xa0"), // {"metadata": {"name": "foo"}} + gvk: &schema.GroupVersionKind{}, + metaFactory: stubMetaFactory{gvk: &schema.GroupVersionKind{}}, + typer: stubTyper{gvks: []schema.GroupVersionKind{{Group: "x", Version: "y", Kind: "z"}}}, + into: &structWithEmbeddedMetas{}, + expectedObj: &structWithEmbeddedMetas{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + }, expectedGVK: &schema.GroupVersionKind{Group: "x", Version: "y", Kind: "z"}, assertOnError: func(t *testing.T, err error) { if err != nil { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw.go index bfcc9997210..09d1340f932 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw.go @@ -21,6 +21,7 @@ import ( "reflect" "sync" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -44,6 +45,23 @@ var rawTypeTranscodeFuncs = map[reflect.Type]func(reflect.Value) error{ re.Raw = j return nil }, + reflect.TypeFor[metav1.FieldsV1](): func(rv reflect.Value) error { + if !rv.CanAddr() { + return nil + } + fields := rv.Addr().Interface().(*metav1.FieldsV1) + if fields.Raw == nil { + // When Raw is nil it encodes to null. Don't change nil Raw values during + // transcoding, they would have unmarshalled from JSON as nil too. + return nil + } + j, err := fields.MarshalJSON() + if err != nil { + return fmt.Errorf("failed to transcode FieldsV1 to JSON: %w", err) + } + fields.Raw = j + return nil + }, } func transcodeRawTypes(v interface{}) error { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw_test.go index ea362acc048..e9af098bafd 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/raw_test.go @@ -20,6 +20,7 @@ import ( "fmt" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "github.com/google/go-cmp/cmp" @@ -112,6 +113,68 @@ func TestTranscodeRawTypes(t *testing.T) { In: &map[string][]runtime.RawExtension{"hello": {{Raw: []byte{0xd9, 0xd9, 0xf7, 0x07}}}}, Out: &map[string][]runtime.RawExtension{"hello": {{Raw: []byte(`7`)}}}, }, + { + In: &metav1.FieldsV1{Raw: []byte{0xa0}}, + Out: &metav1.FieldsV1{Raw: []byte(`{}`)}, + }, + { + In: &metav1.FieldsV1{}, + Out: &metav1.FieldsV1{}, + }, + { + In: metav1.FieldsV1{Raw: []byte{0xa0}}, + Out: metav1.FieldsV1{Raw: []byte{0xa0}}, // not addressable + }, + { + In: &[...]metav1.FieldsV1{{Raw: []byte{0xa0}}, {Raw: []byte{0xf6}}}, + Out: &[...]metav1.FieldsV1{{Raw: []byte(`{}`)}, {Raw: []byte(`null`)}}, + }, + { + In: &[0]metav1.FieldsV1{}, + Out: &[0]metav1.FieldsV1{}, + }, + { + In: &[]metav1.FieldsV1{{Raw: []byte{0xa0}}, {Raw: []byte{0xf6}}}, + Out: &[]metav1.FieldsV1{{Raw: []byte(`{}`)}, {Raw: []byte(`null`)}}, + }, + { + In: &[]metav1.FieldsV1{}, + Out: &[]metav1.FieldsV1{}, + }, + { + In: (*metav1.FieldsV1)(nil), + Out: (*metav1.FieldsV1)(nil), + }, + { + In: &struct{ I fmt.Stringer }{I: &metav1.FieldsV1{Raw: []byte{0xa0}}}, + Out: &struct{ I fmt.Stringer }{I: &metav1.FieldsV1{Raw: []byte(`{}`)}}, + }, + { + In: &struct { + E metav1.FieldsV1 + I int64 + }{E: metav1.FieldsV1{Raw: []byte{0xa0}}, I: 7}, + Out: &struct { + E metav1.FieldsV1 + I int64 + }{E: metav1.FieldsV1{Raw: []byte(`{}`)}, I: 7}, + }, + { + In: &struct { + metav1.FieldsV1 + }{FieldsV1: metav1.FieldsV1{Raw: []byte{0xa0}}}, + Out: &struct { + metav1.FieldsV1 + }{FieldsV1: metav1.FieldsV1{Raw: []byte(`{}`)}}, + }, + { + In: &map[string]metav1.FieldsV1{"hello": {Raw: []byte{0xa0}}}, + Out: &map[string]metav1.FieldsV1{"hello": {Raw: []byte{0xa0}}}, // not addressable + }, + { + In: &map[string][]metav1.FieldsV1{"hello": {{Raw: []byte{0xa0}}}}, + Out: &map[string][]metav1.FieldsV1{"hello": {{Raw: []byte(`{}`)}}}, + }, } { t.Run(fmt.Sprintf("%#v", tc.In), func(t *testing.T) { if err := transcodeRawTypes(tc.In); err != nil {