Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

KEP-4222: Decode CBOR to UnstructuredList as UnstructuredJSONScheme does. #124775

Merged
merged 1 commit into from
May 23, 2024
Merged

KEP-4222: Decode CBOR to UnstructuredList as UnstructuredJSONScheme does. #124775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 75 additions & 6 deletions staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import (
"errors"
"fmt"
"io"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes"
Expand Down Expand Up @@ -137,16 +139,83 @@ func diagnose(data []byte) string {
return diag
}

// unmarshal unmarshals CBOR data from the provided byte slice into a Go object. If the decoder is
// configured to report strict errors, the first error return value may be a non-nil strict decoding
// error. If the last error return value is non-nil, then the unmarshal failed entirely and the
// state of the destination object should not be relied on.
func (s *serializer) unmarshal(data []byte, into interface{}) (strict, lax error) {
if u, ok := into.(runtime.Unstructured); ok {
var content map[string]interface{}
defer func() {
// TODO: The UnstructuredList implementation of SetUnstructuredContent is
// not identical to what unstructuredJSONScheme does: (1) it retains the
// "items" key in its Object field, and (2) it does not infer a singular
// Kind from the list's Kind and populate omitted apiVersion/kind for all
// entries in Items.
u.SetUnstructuredContent(content)
switch u := u.(type) {
case *unstructured.UnstructuredList:
// UnstructuredList's implementation of SetUnstructuredContent
// produces different objects than those produced by a decode using
// UnstructuredJSONScheme:
//
// 1. SetUnstructuredContent retains the "items" key in the list's
deads2k marked this conversation as resolved.
Show resolved Hide resolved
// Object field. It is omitted from Object when decoding with
// UnstructuredJSONScheme.
// 2. SetUnstructuredContent does not populate "apiVersion" and
// "kind" on each entry of its Items
// field. UnstructuredJSONScheme does, inferring the singular
deads2k marked this conversation as resolved.
Show resolved Hide resolved
// Kind from the list Kind.
// 3. SetUnstructuredContent ignores entries of "items" that are
// not JSON objects or are objects without
// "kind". UnstructuredJSONScheme returns an error in either
// case.
//
// UnstructuredJSONScheme's behavior is replicated here.
var items []interface{}
if uncast, present := content["items"]; present {
var cast bool
items, cast = uncast.([]interface{})
if !cast {
strict, lax = nil, fmt.Errorf("items field of UnstructuredList must be encoded as an array or null if present")
deads2k marked this conversation as resolved.
Show resolved Hide resolved
return
deads2k marked this conversation as resolved.
Show resolved Hide resolved
}
}
apiVersion, _ := content["apiVersion"].(string)
kind, _ := content["kind"].(string)
kind = strings.TrimSuffix(kind, "List")
var unstructureds []unstructured.Unstructured
if len(items) > 0 {
unstructureds = make([]unstructured.Unstructured, len(items))
}
for i := range items {
object, cast := items[i].(map[string]interface{})
if !cast {
strict, lax = nil, fmt.Errorf("elements of the items field of UnstructuredList must be encoded as a map")
return
deads2k marked this conversation as resolved.
Show resolved Hide resolved
}

// As in UnstructuredJSONScheme, only set the heuristic
// singular GVK when both "apiVersion" and "kind" are either
// missing, non-string, or empty.
object["apiVersion"], _ = object["apiVersion"].(string)
object["kind"], _ = object["kind"].(string)
if object["apiVersion"] == "" && object["kind"] == "" {
object["apiVersion"] = apiVersion
object["kind"] = kind
}

if object["kind"] == "" {
strict, lax = nil, runtime.NewMissingKindErr(diagnose(data))
return
}
if object["apiVersion"] == "" {
strict, lax = nil, runtime.NewMissingVersionErr(diagnose(data))
return
}

unstructureds[i].Object = object
}
delete(content, "items")
u.Object = content
u.Items = unstructureds
default:
u.SetUnstructuredContent(content)
}
}()
into = &content
}
Expand Down
147 changes: 147 additions & 0 deletions staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,33 @@ func TestEncode(t *testing.T) {
}
},
},
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was possible even before this PR, correct? So it's just additional data related to the new code IIUC

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, although the behavior of the "list element missing kind", "list element missing version", and item GVK heuristic cases would have had different expectations since the original implementation had been calling SetUnstructuredContent.

name: "unstructuredlist",
in: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"apiVersion": "v",
"kind": "kList",
},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{"foo": int64(1)}},
{Object: map[string]interface{}{"foo": int64(2)}},
},
},
assertOnWriter: func() (io.Writer, func(t *testing.T)) {
var b bytes.Buffer
return &b, func(t *testing.T) {
// {'kind': 'kList', 'items': [{'foo': 1}, {'foo': 2}], 'apiVersion': 'v'}
if diff := cmp.Diff(b.Bytes(), []byte("\xd9\xd9\xf7\xa3\x44kind\x45kList\x45items\x82\xa1\x43foo\x01\xa1\x43foo\x02\x4aapiVersion\x41v")); diff != "" {
t.Errorf("unexpected diff:\n%s", diff)
}
}
},
assertOnError: func(t *testing.T, err error) {
if err != nil {
t.Errorf("expected nil error, got: %v", err)
}
},
},
} {
t.Run(tc.name, func(t *testing.T) {
s := NewSerializer(nil, nil)
Expand Down Expand Up @@ -417,6 +444,126 @@ func TestDecode(t *testing.T) {
}
},
},
{
benluddy marked this conversation as resolved.
Show resolved Hide resolved
name: "into unstructuredlist missing kind",
data: []byte("\xa1\x6aapiVersion\x61v"),
into: &unstructured.UnstructuredList{},
expectedObj: nil,
expectedGVK: &schema.GroupVersionKind{Version: "v"},
assertOnError: func(t *testing.T, err error) {
if !runtime.IsMissingKind(err) {
t.Errorf("expected MissingKind, got: %v", err)
}
},
},
{
name: "into unstructuredlist missing version",
data: []byte("\xa1\x64kind\x65kList"),
into: &unstructured.UnstructuredList{},
expectedObj: nil,
expectedGVK: &schema.GroupVersionKind{Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if !runtime.IsMissingVersion(err) {
t.Errorf("expected MissingVersion, got: %v", err)
}
},
},
{
name: "into unstructuredlist empty",
data: []byte("\xa2\x6aapiVersion\x61v\x64kind\x65kList"),
into: &unstructured.UnstructuredList{},
expectedObj: &unstructured.UnstructuredList{Object: map[string]interface{}{
"apiVersion": "v",
"kind": "kList",
}},
expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if err != nil {
t.Errorf("expected nil error, got: %v", err)
}
},
},
{
name: "into unstructuredlist nonempty",
data: []byte("\xa3\x6aapiVersion\x61v\x64kind\x65kList\x65items\x82\xa1\x63foo\x01\xa1\x63foo\x02"), // {"apiVersion": "v", "kind": "kList", "items": [{"foo": 1}, {"foo": 2}]}
into: &unstructured.UnstructuredList{},
expectedObj: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"apiVersion": "v",
"kind": "kList",
},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{"apiVersion": "v", "kind": "k", "foo": int64(1)}},
{Object: map[string]interface{}{"apiVersion": "v", "kind": "k", "foo": int64(2)}},
},
},
expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if err != nil {
t.Errorf("expected nil error, got: %v", err)
}
},
},
{
name: "into unstructuredlist item gvk present",
data: []byte("\xa3\x6aapiVersion\x61v\x64kind\x65kList\x65items\x81\xa2\x6aapiVersion\x62vv\x64kind\x62kk"), // {"apiVersion": "v", "kind": "kList", "items": [{"apiVersion": "vv", "kind": "kk"}]}
into: &unstructured.UnstructuredList{},
expectedObj: &unstructured.UnstructuredList{
Object: map[string]interface{}{
"apiVersion": "v",
"kind": "kList",
},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{"apiVersion": "vv", "kind": "kk"}},
},
},
expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if err != nil {
t.Errorf("expected nil error, got: %v", err)
}
},
},
{
name: "into unstructuredlist item missing kind",
data: []byte("\xa3\x6aapiVersion\x61v\x64kind\x65kList\x65items\x81\xa1\x6aapiVersion\x62vv"), // {"apiVersion": "v", "kind": "kList", "items": [{"apiVersion": "vv"}]}
metaFactory: &defaultMetaFactory{},
into: &unstructured.UnstructuredList{},
expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if !runtime.IsMissingKind(err) {
t.Errorf("expected MissingVersion, got: %v", err)
}
},
},
{
name: "into unstructuredlist item missing version",
data: []byte("\xa3\x6aapiVersion\x61v\x64kind\x65kList\x65items\x81\xa1\x64kind\x62kk"), // {"apiVersion": "v", "kind": "kList", "items": [{"kind": "kk"}]}
metaFactory: &defaultMetaFactory{},
into: &unstructured.UnstructuredList{},
expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if !runtime.IsMissingVersion(err) {
t.Errorf("expected MissingVersion, got: %v", err)
}
},
},
{
name: "using unstructuredlist creater",
data: []byte("\xa2\x6aapiVersion\x61v\x64kind\x65kList"),
metaFactory: &defaultMetaFactory{},
creater: stubCreater{obj: &unstructured.UnstructuredList{}},
expectedObj: &unstructured.UnstructuredList{Object: map[string]interface{}{
"apiVersion": "v",
"kind": "kList",
}},
expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "kList"},
assertOnError: func(t *testing.T, err error) {
if err != nil {
t.Errorf("expected nil error, got: %v", err)
}
},
},
} {
t.Run(tc.name, func(t *testing.T) {
s := newSerializer(tc.metaFactory, tc.creater, tc.typer, tc.options...)
Expand Down