Make strict json unstructured decoding consistent with non-strict dec…

…oding
kubernetes · Mar 25, 2022 · 37952b7 · 37952b7
1 parent 48d3fde
commit 37952b7
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 7 deletions.
diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go
@@ -166,7 +166,20 @@ func (s *Serializer) Decode(originalData []byte, gvk *schema.GroupVersionKind, i
 			strictErrs, err := s.unmarshal(into, data, originalData)
 			if err != nil {
 				return nil, actual, err
-			} else if len(strictErrs) > 0 {
+			}
+
+			// when decoding directly into a provided unstructured object,
+			// extract the actual gvk decoded from the provided data,
+			// and ensure it is non-empty.
+			if isUnstructured {
+				*actual = into.GetObjectKind().GroupVersionKind()
+				if len(actual.Kind) == 0 {
+					return nil, actual, runtime.NewMissingKindErr(string(originalData))
+				}
+				// TODO(109023): require apiVersion here as well once unstructuredJSONScheme#Decode does
+			}
+
+			if len(strictErrs) > 0 {
 				return into, actual, runtime.NewStrictDecodingError(strictErrs)
 			}
 			return into, actual, nil
@@ -261,9 +274,9 @@ func (s *Serializer) unmarshal(into runtime.Object, data, originalData []byte) (
 	var strictJSONErrs []error
 	if u, isUnstructured := into.(runtime.Unstructured); isUnstructured {
 		// Unstructured is a custom unmarshaler that gets delegated
-		// to, so inorder to detect strict JSON errors we need
+		// to, so in order to detect strict JSON errors we need
 		// to unmarshal directly into the object.
-		m := u.UnstructuredContent()
+		m := map[string]interface{}{}
 		strictJSONErrs, err = kjson.UnmarshalStrict(data, &m)
 		u.SetUnstructuredContent(m)
 	} else {

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
@@ -638,22 +638,23 @@ func TestDecode(t *testing.T) {
 		},
 		// Duplicate fields should return an error from the strict JSON deserializer for unstructured.
 		{
-			data:        []byte(`{"value":1,"value":1}`),
+			data:        []byte(`{"kind":"Custom","value":1,"value":1}`),
 			into:        &unstructured.Unstructured{},
 			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
-			expectedGVK: &schema.GroupVersionKind{},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Custom"},
 			errFn: func(err error) bool {
 				return strings.Contains(err.Error(), `duplicate field "value"`)
 			},
 			strict: true,
 		},
 		// Duplicate fields should return an error from the strict YAML deserializer for unstructured.
 		{
-			data: []byte("value: 1\n" +
+			data: []byte("kind: Custom\n" +
+				"value: 1\n" +
 				"value: 1\n"),
 			into:        &unstructured.Unstructured{},
 			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
-			expectedGVK: &schema.GroupVersionKind{},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Custom"},
 			errFn: func(err error) bool {
 				return strings.Contains(err.Error(), `"value" already set in map`)
 			},