Merge pull request kubernetes#124775 from benluddy/cbor-unstructuredlist

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

Author: k8s-ci-robot

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

@@ -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"
@@ -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
+				//      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
+				//      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")
+						return
+					}
+				}
+				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
+					}
+
+					// 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
 	}

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go

@@ -156,6 +156,33 @@ func TestEncode(t *testing.T) {
 				}
 			},
 		},
+		{
+			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)
@@ -417,6 +444,126 @@ func TestDecode(t *testing.T) {
 				}
 			},
 		},
+		{
+			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...)