Implement CodecFactoryOptions allowing clients to opt-in to Pretty encoders and Strict Decoders (#76805)

* Implement CodecFactoryOptions allowing clients to opt-in to Pretty encoders and Strict Decoders

This patch introduces a new CodecFactory constructor that's capable of building different collections of Serializers.
Note that this diverges from the current design where an initialized CodecFactory is a kitchen sink holding many forms of useful Serilaizers in a non-customizeable way.

This approach is a suggested solution from this thread:
https://groups.google.com/d/msg/kubernetes-sig-api-machinery/R4Iiz3rOYd0/O6aX15_AAQAJ

A universal, defaulting, strict decoder is needed for Component Config efforts.
Future work aims to make strict behavior the default -- right now it's not implemented in a way that is performant enough for the apiserver.

This patch adds a deprecation notice for `NewCodecFactory()`.
For now, uses of this public constructor remain unchanged, and the tests are still exercising it.
It's been modified, however, to simply wrap the `WithOptions` version.

* Extend NewCodecFactory() signature with options mutators

When no mutators are passed, the legacy behavior is implemented.
This allows code to continue using NewCodecFactory() without modifications.

This also reverts the deprecation notice from the previous commit.

* Make Pretty always default and remove public WithOptions constructor

* Reduce []serializerType{} patch

* Test Strict CodecFactory option when enabled/disabled

* Fix CodecFactoryOptions nits

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

@@ -48,28 +48,40 @@ type serializerType struct {
 	StreamSerializer runtime.Serializer
 }
 
-func newSerializersForScheme(scheme *runtime.Scheme, mf json.MetaFactory) []serializerType {
-	jsonSerializer := json.NewSerializer(mf, scheme, scheme, false)
-	jsonPrettySerializer := json.NewSerializer(mf, scheme, scheme, true)
-	yamlSerializer := json.NewYAMLSerializer(mf, scheme, scheme)
-	serializer := protobuf.NewSerializer(scheme, scheme)
-	raw := protobuf.NewRawSerializer(scheme, scheme)
+func newSerializersForScheme(scheme *runtime.Scheme, mf json.MetaFactory, options CodecFactoryOptions) []serializerType {
+	jsonSerializer := json.NewSerializerWithOptions(
+		mf, scheme, scheme,
+		json.SerializerOptions{Yaml: false, Pretty: false, Strict: options.Strict},
+	)
+	jsonSerializerType := serializerType{
+		AcceptContentTypes: []string{runtime.ContentTypeJSON},
+		ContentType:        runtime.ContentTypeJSON,
+		FileExtensions:     []string{"json"},
+		EncodesAsText:      true,
+		Serializer:         jsonSerializer,
+
+		Framer:           json.Framer,
+		StreamSerializer: jsonSerializer,
+	}
+	if options.Pretty {
+		jsonSerializerType.PrettySerializer = json.NewSerializerWithOptions(
+			mf, scheme, scheme,
+			json.SerializerOptions{Yaml: false, Pretty: true, Strict: options.Strict},
+		)
+	}
+
+	yamlSerializer := json.NewSerializerWithOptions(
+		mf, scheme, scheme,
+		json.SerializerOptions{Yaml: true, Pretty: false, Strict: options.Strict},
+	)
+	protoSerializer := protobuf.NewSerializer(scheme, scheme)
+	protoRawSerializer := protobuf.NewRawSerializer(scheme, scheme)
 
 	serializers := []serializerType{
+		jsonSerializerType,
 		{
-			AcceptContentTypes: []string{"application/json"},
-			ContentType:        "application/json",
-			FileExtensions:     []string{"json"},
-			EncodesAsText:      true,
-			Serializer:         jsonSerializer,
-			PrettySerializer:   jsonPrettySerializer,
-
-			Framer:           json.Framer,
-			StreamSerializer: jsonSerializer,
-		},
-		{
-			AcceptContentTypes: []string{"application/yaml"},
-			ContentType:        "application/yaml",
+			AcceptContentTypes: []string{runtime.ContentTypeYAML},
+			ContentType:        runtime.ContentTypeYAML,
 			FileExtensions:     []string{"yaml"},
 			EncodesAsText:      true,
 			Serializer:         yamlSerializer,
@@ -78,10 +90,10 @@ func newSerializersForScheme(scheme *runtime.Scheme, mf json.MetaFactory) []seri
 			AcceptContentTypes: []string{runtime.ContentTypeProtobuf},
 			ContentType:        runtime.ContentTypeProtobuf,
 			FileExtensions:     []string{"pb"},
-			Serializer:         serializer,
+			Serializer:         protoSerializer,
 
 			Framer:           protobuf.LengthDelimitedFramer,
-			StreamSerializer: raw,
+			StreamSerializer: protoRawSerializer,
 		},
 	}
 
@@ -104,14 +116,56 @@ type CodecFactory struct {
 	legacySerializer runtime.Serializer
 }
 
+// CodecFactoryOptions holds the options for configuring CodecFactory behavior
+type CodecFactoryOptions struct {
+	// Strict configures all serializers in strict mode
+	Strict bool
+	// Pretty includes a pretty serializer along with the non-pretty one
+	Pretty bool
+}
+
+// CodecFactoryOptionsMutator takes a pointer to an options struct and then modifies it.
+// Functions implementing this type can be passed to the NewCodecFactory() constructor.
+type CodecFactoryOptionsMutator func(*CodecFactoryOptions)
+
+// EnablePretty enables including a pretty serializer along with the non-pretty one
+func EnablePretty(options *CodecFactoryOptions) {
+	options.Pretty = true
+}
+
+// DisablePretty disables including a pretty serializer along with the non-pretty one
+func DisablePretty(options *CodecFactoryOptions) {
+	options.Pretty = false
+}
+
+// EnableStrict enables configuring all serializers in strict mode
+func EnableStrict(options *CodecFactoryOptions) {
+	options.Strict = true
+}
+
+// DisableStrict disables configuring all serializers in strict mode
+func DisableStrict(options *CodecFactoryOptions) {
+	options.Strict = false
+}
+
 // NewCodecFactory provides methods for retrieving serializers for the supported wire formats
 // and conversion wrappers to define preferred internal and external versions. In the future,
 // as the internal version is used less, callers may instead use a defaulting serializer and
 // only convert objects which are shared internally (Status, common API machinery).
+//
+// Mutators can be passed to change the CodecFactoryOptions before construction of the factory.
+// It is recommended to explicitly pass mutators instead of relying on defaults.
+// By default, Pretty is enabled -- this is conformant with previously supported behavior.
+//
 // TODO: allow other codecs to be compiled in?
 // TODO: accept a scheme interface
-func NewCodecFactory(scheme *runtime.Scheme) CodecFactory {
-	serializers := newSerializersForScheme(scheme, json.DefaultMetaFactory)
+func NewCodecFactory(scheme *runtime.Scheme, mutators ...CodecFactoryOptionsMutator) CodecFactory {
+	options := CodecFactoryOptions{Pretty: true}
+	for _, fn := range mutators {
+		fn(&options)
+	}
+
+	serializers := newSerializersForScheme(scheme, json.DefaultMetaFactory, options)
 	return newCodecFactory(scheme, serializers)
 }
 

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

@@ -87,7 +87,7 @@ func GetTestScheme() (*runtime.Scheme, runtime.Codec) {
 
 	s.AddUnversionedTypes(externalGV, &metav1.Status{})
 
-	cf := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}))
+	cf := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}, CodecFactoryOptions{Pretty: true, Strict: true}))
 	codec := cf.LegacyCodec(schema.GroupVersion{Version: "v1"})
 	return s, codec
 }
@@ -145,11 +145,11 @@ func TestTypes(t *testing.T) {
 
 func TestVersionedEncoding(t *testing.T) {
 	s, _ := GetTestScheme()
-	cf := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}))
+	cf := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}, CodecFactoryOptions{Pretty: true, Strict: true}))
 	info, _ := runtime.SerializerInfoForMediaType(cf.SupportedMediaTypes(), runtime.ContentTypeJSON)
 	encoder := info.Serializer
 
-	codec := cf.CodecForVersions(encoder, nil, schema.GroupVersion{Version: "v2"}, nil)
+	codec := cf.EncoderForVersion(encoder, schema.GroupVersion{Version: "v2"})
 	out, err := runtime.Encode(codec, &serializertesting.TestType1{})
 	if err != nil {
 		t.Fatal(err)
@@ -158,14 +158,14 @@ func TestVersionedEncoding(t *testing.T) {
 		t.Fatal(string(out))
 	}
 
-	codec = cf.CodecForVersions(encoder, nil, schema.GroupVersion{Version: "v3"}, nil)
+	codec = cf.EncoderForVersion(encoder, schema.GroupVersion{Version: "v3"})
 	_, err = runtime.Encode(codec, &serializertesting.TestType1{})
 	if err == nil {
 		t.Fatal(err)
 	}
 
 	// unversioned encode with no versions is written directly to wire
-	codec = cf.CodecForVersions(encoder, nil, runtime.InternalGroupVersioner, nil)
+	codec = cf.EncoderForVersion(encoder, runtime.InternalGroupVersioner)
 	out, err = runtime.Encode(codec, &serializertesting.TestType1{})
 	if err != nil {
 		t.Fatal(err)
@@ -196,6 +196,23 @@ func TestMultipleNames(t *testing.T) {
 	}
 }
 
+func TestStrictOption(t *testing.T) {
+	s, _ := GetTestScheme()
+	duplicateKeys := `{"myKindKey":"TestType3","myVersionKey":"v1","myVersionKey":"v1","A":"value"}`
+
+	strictCodec := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}, CodecFactoryOptions{Pretty: true, Strict: true})).LegacyCodec()
+	_, _, err := strictCodec.Decode([]byte(duplicateKeys), nil, nil)
+	if !runtime.IsStrictDecodingError(err) {
+		t.Fatalf("StrictDecodingError not returned on object with duplicate keys: %v, type: %v", err, reflect.TypeOf(err))
+	}
+
+	nonStrictCodec := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}, CodecFactoryOptions{Pretty: true, Strict: false})).LegacyCodec()
+	_, _, err = nonStrictCodec.Decode([]byte(duplicateKeys), nil, nil)
+	if runtime.IsStrictDecodingError(err) {
+		t.Fatalf("Non-Strict decoder returned a StrictDecodingError: %v", err)
+	}
+}
+
 func TestConvertTypesWhenDefaultNamesMatch(t *testing.T) {
 	internalGV := schema.GroupVersion{Version: runtime.APIVersionInternal}
 	externalGV := schema.GroupVersion{Version: "v1"}
@@ -218,7 +235,9 @@ func TestConvertTypesWhenDefaultNamesMatch(t *testing.T) {
 	}
 	expect := &serializertesting.TestType1{A: "test"}
 
-	codec := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{})).LegacyCodec(schema.GroupVersion{Version: "v1"})
+	codec := newCodecFactory(
+		s, newSerializersForScheme(s, testMetaFactory{}, CodecFactoryOptions{Pretty: true, Strict: true}),
+	).LegacyCodec(schema.GroupVersion{Version: "v1"})
 
 	obj, err := runtime.Decode(codec, data)
 	if err != nil {
@@ -309,7 +328,7 @@ func GetDirectCodecTestScheme() *runtime.Scheme {
 
 func TestDirectCodec(t *testing.T) {
 	s := GetDirectCodecTestScheme()
-	cf := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}))
+	cf := newCodecFactory(s, newSerializersForScheme(s, testMetaFactory{}, CodecFactoryOptions{Pretty: true, Strict: true}))
 	info, _ := runtime.SerializerInfoForMediaType(cf.SupportedMediaTypes(), runtime.ContentTypeJSON)
 	serializer := info.Serializer
 	df := cf.WithoutConversion()