Implement a strict deserialization mode for api-machinery

staging/src/k8s.io/apimachinery/pkg/runtime/error.go

@@ -120,3 +120,32 @@ func IsMissingVersion(err error) bool {
 	_, ok := err.(*missingVersionErr)
 	return ok
 }
+
+// strictDecodingError is a base error type that is returned by a strict Decoder such
+// as UniversalStrictDecoder.
+type strictDecodingError struct {
+	message string
+	data    string
+}
+
+// NewStrictDecodingError creates a new strictDecodingError object.
+func NewStrictDecodingError(message string, data string) error {
+	return &strictDecodingError{
+		message: message,
+		data:    data,
+	}
+}
+
+func (e *strictDecodingError) Error() string {
+	return fmt.Sprintf("strict decoder error for %s: %s", e.data, e.message)
+}
+
+// IsStrictDecodingError returns true if the error indicates that the provided object
+// strictness violations.
+func IsStrictDecodingError(err error) bool {
+	if err == nil {
+		return false
+	}
+	_, ok := err.(*strictDecodingError)
+	return ok
+}

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

@@ -35,34 +35,50 @@ import (
 
 // NewSerializer creates a JSON serializer that handles encoding versioned objects into the proper JSON form. If typer
 // is not nil, the object has the group, version, and kind fields set.
+// Deprecated: use NewSerializerWithOptions instead.
 func NewSerializer(meta MetaFactory, creater runtime.ObjectCreater, typer runtime.ObjectTyper, pretty bool) *Serializer {
-	return &Serializer{
-		meta:    meta,
-		creater: creater,
-		typer:   typer,
-		yaml:    false,
-		pretty:  pretty,
-	}
+	return NewSerializerWithOptions(meta, creater, typer, &SerializerOptions{false, pretty, false})
 }
 
 // NewYAMLSerializer creates a YAML serializer that handles encoding versioned objects into the proper YAML form. If typer
 // is not nil, the object has the group, version, and kind fields set. This serializer supports only the subset of YAML that
 // matches JSON, and will error if constructs are used that do not serialize to JSON.
+// Deprecated: use NewSerializerWithOptions instead.
 func NewYAMLSerializer(meta MetaFactory, creater runtime.ObjectCreater, typer runtime.ObjectTyper) *Serializer {
+	return NewSerializerWithOptions(meta, creater, typer, &SerializerOptions{true, false, false})
+}
+
+// NewSerializerWithOptions creates a JSON/YAML serializer that handles encoding versioned objects into the proper JSON/YAML
+// form. If typer is not nil, the object has the group, version, and kind fields set.
+func NewSerializerWithOptions(meta MetaFactory, creater runtime.ObjectCreater, typer runtime.ObjectTyper, serializerOptions *SerializerOptions) *Serializer {
 	return &Serializer{
-		meta:    meta,
-		creater: creater,
-		typer:   typer,
-		yaml:    true,
+		meta:              meta,
+		creater:           creater,
+		typer:             typer,
+		SerializerOptions: serializerOptions,
 	}
 }
 
+// SerializerOptions holds the options which are used to creating a JSON/YAML serializer.
+// For example:
+// (1) we can creates a JSON serializer once we set `Yaml` to `false`.
+// (2) we can creates a YAML serializer once we set `Yaml` to `true`. This serializer supports only the subset of YAML that
+//     matches JSON, and will error if constructs are used that do not serialize to JSON.
+//     Please note that `Pretty` is silently ignored when `Yaml` is `true`.
+// (3) we can creates a strict JSON/YAML serializer that can also return errors of type strictDecodingError, once we set
+//    `Strict` to `true`. And note that this serializer is not as performant as the non-strict variant, and should not be
+//    used in fast paths.
+type SerializerOptions struct {
+	Yaml   bool
+	Pretty bool
+	Strict bool
+}
+
 type Serializer struct {
 	meta    MetaFactory
 	creater runtime.ObjectCreater
 	typer   runtime.ObjectTyper
-	yaml    bool
-	pretty  bool
+	*SerializerOptions
 }
 
 // Serializer implements Serializer
@@ -119,11 +135,28 @@ func CaseSensitiveJsonIterator() jsoniter.API {
 	return config
 }
 
-// Private copy of jsoniter to try to shield against possible mutations
+// StrictCaseSensitiveJsonIterator returns a jsoniterator API that's configured to be
+// case-sensitive, but also disallows unknown fields when unmarshalling. It is compatible with
+// the encoding/json standard library.
+func StrictCaseSensitiveJsonIterator() jsoniter.API {
+	config := jsoniter.Config{
+		EscapeHTML:             true,
+		SortMapKeys:            true,
+		ValidateJsonRawMessage: true,
+		CaseSensitive:          true,
+		DisallowUnknownFields:  true,
+	}.Froze()
+	// Force jsoniter to decode number to interface{} via int64/float64, if possible.
+	config.RegisterExtension(&customNumberExtension{})
+	return config
+}
+
+// Private copies of jsoniter to try to shield against possible mutations
 // from outside. Still does not protect from package level jsoniter.Register*() functions - someone calling them
 // in some other library will mess with every usage of the jsoniter library in the whole program.
 // See https://github.com/json-iterator/go/issues/265
 var caseSensitiveJsonIterator = CaseSensitiveJsonIterator()
+var strictCaseSensitiveJsonIterator = StrictCaseSensitiveJsonIterator()
 
 // gvkWithDefaults returns group kind and version defaulting from provided default
 func gvkWithDefaults(actual, defaultGVK schema.GroupVersionKind) schema.GroupVersionKind {
@@ -160,7 +193,7 @@ func (s *Serializer) Decode(originalData []byte, gvk *schema.GroupVersionKind, i
 	}
 
 	data := originalData
-	if s.yaml {
+	if s.Yaml {
 		altered, err := yaml.YAMLToJSON(data)
 		if err != nil {
 			return nil, nil, err
@@ -216,12 +249,38 @@ func (s *Serializer) Decode(originalData []byte, gvk *schema.GroupVersionKind, i
 	if err := caseSensitiveJsonIterator.Unmarshal(data, obj); err != nil {
 		return nil, actual, err
 	}
+
+	// If the deserializer is non-strict, return successfully here.
+	if !s.Strict {
+		return obj, actual, nil
+	}
+
+	// In strict mode pass the data trough the YAMLToJSONStrict converter.
+	// This is done to catch duplicate fields regardless of encoding (JSON or YAML). For JSON data,
+	// the output would equal the input, unless there is a parsing error such as duplicate fields.
+	// As we know this was successful in the non-strict case, the only error that may be returned here
+	// is because of the newly-added strictness. hence we know we can return the typed strictDecoderError
+	// the actual error is that the object contains duplicate fields.
+	altered, err := yaml.YAMLToJSONStrict(originalData)
+	if err != nil {
+		return nil, actual, runtime.NewStrictDecodingError(err.Error(), string(originalData))
+	}
+	// As performance is not an issue for now for the strict deserializer (one has regardless to do
+	// the unmarshal twice), we take the sanitized, altered data that is guaranteed to have no duplicated
+	// fields, and unmarshal this into a copy of the already-populated obj. Any error that occurs here is
+	// due to that a matching field doesn't exist in the object. hence we can return a typed strictDecoderError,
+	// the actual error is that the object contains unknown field.
+	strictObj := obj.DeepCopyObject()
+	if err := strictCaseSensitiveJsonIterator.Unmarshal(altered, strictObj); err != nil {
+		return nil, actual, runtime.NewStrictDecodingError(err.Error(), string(originalData))
+	}
+	// Always return the same object as the non-strict serializer to avoid any deviations.
 	return obj, actual, nil
 }
 
 // Encode serializes the provided object to the given writer.
 func (s *Serializer) Encode(obj runtime.Object, w io.Writer) error {
-	if s.yaml {
+	if s.Yaml {
 		json, err := caseSensitiveJsonIterator.Marshal(obj)
 		if err != nil {
 			return err
@@ -234,7 +293,7 @@ func (s *Serializer) Encode(obj runtime.Object, w io.Writer) error {
 		return err
 	}
 
-	if s.pretty {
+	if s.Pretty {
 		data, err := caseSensitiveJsonIterator.MarshalIndent(obj, "", "  ")
 		if err != nil {
 			return err
@@ -248,7 +307,7 @@ func (s *Serializer) Encode(obj runtime.Object, w io.Writer) error {
 
 // RecognizesData implements the RecognizingDecoder interface.
 func (s *Serializer) RecognizesData(peek io.Reader) (ok, unknown bool, err error) {
-	if s.yaml {
+	if s.Yaml {
 		// we could potentially look for '---'
 		return false, true, nil
 	}

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_test.go

@@ -59,8 +59,22 @@ type DecodableSpec struct {
 func (d *testDecodable) GetObjectKind() schema.ObjectKind                { return d }
 func (d *testDecodable) SetGroupVersionKind(gvk schema.GroupVersionKind) { d.gvk = gvk }
 func (d *testDecodable) GroupVersionKind() schema.GroupVersionKind       { return d.gvk }
-func (d *testDecodable) DeepCopyObject() runtime.Object {
-	panic("testDecodable does not support DeepCopy")
+func (in *testDecodable) DeepCopyObject() runtime.Object {
+	if in == nil {
+		return nil
+	}
+	out := new(testDecodable)
+	in.DeepCopyInto(out)
+	return out
+}
+func (in *testDecodable) DeepCopyInto(out *testDecodable) {
+	*out = *in
+	out.Other = in.Other
+	out.Value = in.Value
+	out.Spec = in.Spec
+	out.Interface = in.Interface
+	out.gvk = in.gvk
+	return
 }
 
 func TestDecode(t *testing.T) {
@@ -69,6 +83,7 @@ func TestDecode(t *testing.T) {
 		typer   runtime.ObjectTyper
 		yaml    bool
 		pretty  bool
+		strict  bool
 
 		data       []byte
 		defaultGVK *schema.GroupVersionKind
@@ -274,14 +289,138 @@ func TestDecode(t *testing.T) {
 				Spec: DecodableSpec{A: 1, H: 3},
 			},
 		},
+		// Unknown fields should return an error from the strict JSON deserializer.
+		{
+			data:        []byte(`{"unknown": 1}`),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			errFn: func(err error) bool {
+				return strings.Contains(err.Error(), "found unknown field")
+			},
+			strict: true,
+		},
+		// Unknown fields should return an error from the strict YAML deserializer.
+		{
+			data:        []byte("unknown: 1\n"),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			errFn: func(err error) bool {
+				return strings.Contains(err.Error(), "found unknown field")
+			},
+			yaml:   true,
+			strict: true,
+		},
+		// Duplicate fields should return an error from the strict JSON deserializer.
+		{
+			data:        []byte(`{"value":1,"value":1}`),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			errFn: func(err error) bool {
+				return strings.Contains(err.Error(), "already set in map")
+			},
+			strict: true,
+		},
+		// Duplicate fields should return an error from the strict YAML deserializer.
+		{
+			data: []byte("value: 1\n" +
+				"value: 1\n"),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			errFn: func(err error) bool {
+				return strings.Contains(err.Error(), "already set in map")
+			},
+			yaml:   true,
+			strict: true,
+		},
+		// Strict JSON decode should fail for untagged fields.
+		{
+			data:        []byte(`{"kind":"Test","apiVersion":"other/blah","value":1,"Other":"test"}`),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			errFn: func(err error) bool {
+				return strings.Contains(err.Error(), "found unknown field")
+			},
+			strict: true,
+		},
+		// Strict YAML decode should fail for untagged fields.
+		{
+			data: []byte("kind: Test\n" +
+				"apiVersion: other/blah\n" +
+				"value: 1\n" +
+				"Other: test\n"),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			errFn: func(err error) bool {
+				return strings.Contains(err.Error(), "found unknown field")
+			},
+			yaml:   true,
+			strict: true,
+		},
+		// Strict JSON decode into unregistered objects directly.
+		{
+			data:        []byte(`{"kind":"Test","apiVersion":"other/blah","value":1,"Other":"test"}`),
+			into:        &testDecodable{},
+			typer:       &mockTyper{err: runtime.NewNotRegisteredErrForKind("mock", schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"})},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			expectedObject: &testDecodable{
+				Other: "test",
+				Value: 1,
+			},
+			strict: true,
+		},
+		// Strict YAML decode into unregistered objects directly.
+		{
+			data: []byte("kind: Test\n" +
+				"apiVersion: other/blah\n" +
+				"value: 1\n" +
+				"Other: test\n"),
+			into:        &testDecodable{},
+			typer:       &mockTyper{err: runtime.NewNotRegisteredErrForKind("mock", schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"})},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			expectedObject: &testDecodable{
+				Other: "test",
+				Value: 1,
+			},
+			yaml:   true,
+			strict: true,
+		},
+		// Valid strict JSON decode without GVK.
+		{
+			data:        []byte(`{"value":1234}`),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			expectedObject: &testDecodable{
+				Value: 1234,
+			},
+			strict: true,
+		},
+		// Valid strict YAML decode without GVK.
+		{
+			data:        []byte("value: 1234\n"),
+			into:        &testDecodable{},
+			typer:       &mockTyper{gvk: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"}},
+			expectedGVK: &schema.GroupVersionKind{Kind: "Test", Group: "other", Version: "blah"},
+			expectedObject: &testDecodable{
+				Value: 1234,
+			},
+			yaml:   true,
+			strict: true,
+		},
 	}
 
 	for i, test := range testCases {
 		var s runtime.Serializer
 		if test.yaml {
-			s = json.NewYAMLSerializer(json.DefaultMetaFactory, test.creater, test.typer)
+			s = json.NewSerializerWithOptions(json.DefaultMetaFactory, test.creater, test.typer, &json.SerializerOptions{Yaml: test.yaml, Pretty: false, Strict: test.strict})
 		} else {
-			s = json.NewSerializer(json.DefaultMetaFactory, test.creater, test.typer, test.pretty)
+			s = json.NewSerializerWithOptions(json.DefaultMetaFactory, test.creater, test.typer, &json.SerializerOptions{Yaml: test.yaml, Pretty: test.pretty, Strict: test.strict})
 		}
 		obj, gvk, err := s.Decode([]byte(test.data), test.defaultGVK, test.into)