Merge pull request #109019 from liggitt/null-fix

Fix inconsistent requirement for kind in strict json decoding
This commit is contained in:
Kubernetes Prow Robot 2022-03-25 15:35:25 -07:00 committed by GitHub
commit 58847ef702
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 354 additions and 10 deletions

View File

@ -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
}

View File

@ -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 {

View File

@ -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))
}
}