diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go index d26c6cff4e6f..2e33283ef221 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go @@ -340,6 +340,7 @@ func (s unstructuredJSONScheme) Decode(data []byte, _ *schema.GroupVersionKind, if len(gvk.Kind) == 0 { return nil, &gvk, runtime.NewMissingKindErr(string(data)) } + // TODO(109023): require apiVersion here as well return obj, &gvk, nil } 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 index 6c082f660eee..1ae4a32eb720 100644 --- 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 index 5b653970adf0..07a74e2157c6 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 @@ -110,7 +110,7 @@ func (d *testDecodeCoercion) DeepCopyInto(out *testDecodeCoercion) { } func TestDecode(t *testing.T) { - testCases := []struct { + type testCase struct { creater runtime.ObjectCreater typer runtime.ObjectTyper yaml bool @@ -124,13 +124,294 @@ func TestDecode(t *testing.T) { errFn func(error) bool expectedObject runtime.Object expectedGVK *schema.GroupVersionKind - }{ + } + + testCases := []testCase{ + // missing metadata without into, typed creater + { + data: []byte("{}"), + + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, { data: []byte("{}"), + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"kind":"Foo"}`), + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'apiVersion' is missing in") }, + }, + { + data: []byte(`{"kind":"Foo"}`), + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'apiVersion' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"apiVersion":"foo/v1"}`), + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, + { + data: []byte(`{"apiVersion":"foo/v1"}`), + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &testDecodable{}}, + + expectedObject: &testDecodable{TypeMeta: metav1.TypeMeta{APIVersion: "/v1", Kind: "Foo"}}, + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + }, + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &testDecodable{}}, + + expectedObject: &testDecodable{TypeMeta: metav1.TypeMeta{APIVersion: "/v1", Kind: "Foo"}}, + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + strict: true, + }, + + // missing metadata with unstructured into + { + data: []byte("{}"), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + expectedGVK: &schema.GroupVersionKind{}, errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, }, + { + data: []byte("{}"), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"kind": "Foo"}}, + // TODO(109023): expect this to error; unstructured decoding currently only requires kind to be set, not apiVersion + }, + { + data: []byte(`{"kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"kind": "Foo"}}, + strict: true, + // TODO(109023): expect this to error; unstructured decoding currently only requires kind to be set, not apiVersion + }, + + { + data: []byte(`{"apiVersion":"foo/v1"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, + { + data: []byte(`{"apiVersion":"foo/v1"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "/v1", "kind": "Foo"}}, + }, + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{}, + + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "/v1", "kind": "Foo"}}, + strict: true, + }, + + // missing metadata with unstructured into providing metadata + { + data: []byte("{}"), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, + { + data: []byte("{}"), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"kind": "Foo"}}, + // TODO(109023): expect this to error; unstructured decoding currently only requires kind to be set, not apiVersion + }, + { + data: []byte(`{"kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"kind": "Foo"}}, + strict: true, + // TODO(109023): expect this to error; unstructured decoding currently only requires kind to be set, not apiVersion + }, + + { + data: []byte(`{"apiVersion":"foo/v1"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, + { + data: []byte(`{"apiVersion":"foo/v1"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "/v1", "kind": "Foo"}}, + }, + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + into: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "into/v1", "kind": "Into"}}, + + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "/v1", "kind": "Foo"}}, + strict: true, + }, + + // missing metadata without into, unstructured creater + { + data: []byte("{}"), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, + { + data: []byte("{}"), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'apiVersion' is missing in") }, + }, + { + data: []byte(`{"kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Foo"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'apiVersion' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"apiVersion":"foo/v1"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + }, + { + data: []byte(`{"apiVersion":"foo/v1"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{Group: "foo", Version: "v1"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "Object 'Kind' is missing in") }, + strict: true, + }, + + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "/v1", "kind": "Foo"}}, + }, + { + data: []byte(`{"apiVersion":"/v1","kind":"Foo"}`), + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + creater: &mockCreater{obj: &unstructured.Unstructured{}}, + + expectedGVK: &schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Foo"}, + expectedObject: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "/v1", "kind": "Foo"}}, + strict: true, + }, + + // creator errors { data: []byte("{}"), defaultGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, @@ -139,6 +420,15 @@ func TestDecode(t *testing.T) { expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, errFn: func(err error) bool { return err.Error() == "fake error" }, }, + { + data: []byte("{}"), + defaultGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, + creater: &mockCreater{err: fmt.Errorf("fake error")}, + + expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, + errFn: func(err error) bool { return err.Error() == "fake error" }, + }, + // creator typed { data: []byte("{}"), defaultGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, @@ -146,6 +436,14 @@ func TestDecode(t *testing.T) { expectedObject: &testDecodable{}, expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, }, + { + data: []byte("{}"), + defaultGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, + creater: &mockCreater{obj: &testDecodable{}}, + expectedObject: &testDecodable{}, + expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}, + strict: true, + }, // version without group is not defaulted { @@ -340,10 +638,10 @@ 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"`) }, @@ -351,11 +649,12 @@ func TestDecode(t *testing.T) { }, // 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`) }, @@ -415,7 +714,27 @@ func TestDecode(t *testing.T) { yaml: true, strict: true, }, - + // Invalid strict JSON, results in json parse error: + { + data: []byte("foo"), + into: &unstructured.Unstructured{}, + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + errFn: func(err error) bool { + return strings.Contains(err.Error(), `json parse error: invalid character 'o'`) + }, + strict: true, + }, + // empty JSON strict, results in missing kind error + { + data: []byte("{}"), + into: &unstructured.Unstructured{}, + typer: &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}}, + expectedGVK: &schema.GroupVersionKind{}, + errFn: func(err error) bool { + return strings.Contains(err.Error(), `Object 'Kind' is missing`) + }, + strict: true, + }, // coerce from null { data: []byte(`{"bool":null,"int":null,"int32":null,"int64":null,"float32":null,"float64":null,"string":null,"array":null,"map":null,"struct":null}`), @@ -526,6 +845,10 @@ func TestDecode(t *testing.T) { }, } + logTestCase := func(t *testing.T, tc testCase) { + t.Logf("data=%s\n\tinto=%T, yaml=%v, strict=%v", string(tc.data), tc.into, tc.yaml, tc.strict) + } + for i, test := range testCases { var s runtime.Serializer if test.yaml { @@ -536,32 +859,39 @@ func TestDecode(t *testing.T) { obj, gvk, err := s.Decode([]byte(test.data), test.defaultGVK, test.into) if !reflect.DeepEqual(test.expectedGVK, gvk) { + logTestCase(t, test) t.Errorf("%d: unexpected GVK: %v", i, gvk) } switch { case err == nil && test.errFn != nil: + logTestCase(t, test) t.Errorf("%d: failed: not getting the expected error", i) continue case err != nil && test.errFn == nil: + logTestCase(t, test) t.Errorf("%d: failed: %v", i, err) continue case err != nil: if !test.errFn(err) { + logTestCase(t, test) t.Errorf("%d: failed: %v", i, err) } if !runtime.IsStrictDecodingError(err) && obj != nil { + logTestCase(t, test) t.Errorf("%d: should have returned nil object", i) } continue } if test.into != nil && test.into != obj { + logTestCase(t, test) t.Errorf("%d: expected into to be returned: %v", i, obj) continue } if !reflect.DeepEqual(test.expectedObject, obj) { + logTestCase(t, test) t.Errorf("%d: unexpected object:\n%s", i, diff.ObjectGoPrintSideBySide(test.expectedObject, obj)) } }