Merge pull request #23 from luxas/decode_unknowns

Allow decoding Unknown objects, and also convert CRDs at encode time

pkg/serializer/convertor.go

@@ -16,6 +16,7 @@ var (
 	errOutMustBeHub       = errors.New("if in object is Convertible, out must be Hub")
 	errInMustBeHub        = errors.New("if out object is Convertible, in must be Hub")
 	errMustNotHaveTwoHubs = errors.New("in and out must not both be Hubs")
+	errObjMustNotBeBoth   = errors.New("given object must not implement both the Convertible and Hub interfaces")
 )
 
 func newConverter(scheme *runtime.Scheme) *converter {
@@ -64,15 +65,15 @@ func (c *converter) ConvertToHub(in runtime.Object) (runtime.Object, error) {
 	return c.convertor.ConvertToVersion(in, nil)
 }
 
-func newObjectConvertor(scheme *runtime.Scheme, convertToHub bool) *objectConvertor {
-	return &objectConvertor{scheme, convertToHub}
+func newObjectConvertor(scheme *runtime.Scheme, doConversion bool) *objectConvertor {
+	return &objectConvertor{scheme, doConversion}
 }
 
 // objectConvertor implements runtime.ObjectConvertor. See k8s.io/apimachinery/pkg/runtime/serializer/versioning.go for
 // how this objectConvertor is used (e.g. in codec.Decode())
 type objectConvertor struct {
-	scheme         *runtime.Scheme
-	doConvertToHub bool
+	scheme       *runtime.Scheme
+	doConversion bool
 }
 
 // Convert attempts to convert one object into another, or returns an error. This
@@ -163,23 +164,72 @@ func (c *objectConvertor) convertFromHub(in conversion.Hub, out conversion.Conve
 // This method is similar to Convert() but handles specific details of choosing the correct
 // output version.
 // This function might return errors of type *CRDConversionError.
-func (c *objectConvertor) ConvertToVersion(in runtime.Object, _ runtime.GroupVersioner) (runtime.Object, error) {
+func (c *objectConvertor) ConvertToVersion(in runtime.Object, groupVersioner runtime.GroupVersioner) (runtime.Object, error) {
 	// This function is called at Decode(All)-time. If we requested a conversion to internal, just proceed
 	// as before, using the scheme's ConvertToVersion function. But if we don't want to convert the newly-decoded
 	// external object, we can just do nothing and the object will stay unconverted.
-	if !c.doConvertToHub {
+	// doConversion is always true in the Encode codepath.
+	if !c.doConversion {
 		// DeepCopy the object to make sure that although in would be somehow modified, it doesn't affect out
 		return in.DeepCopyObject(), nil
 	}
 
-	// If this is a controller-runtime CRD convertible, convert it to the Hub type and return it
-	convertible, ok := in.(conversion.Convertible)
-	if ok {
+	// At this point we know we are either in the ConvertToHub Decode(All) codepath, or Encode
+	// Check whether "in" is a CRD-type object
+	convertible, isConvertible := in.(conversion.Convertible)
+	_, isHub := in.(conversion.Hub)
+
+	// Return quickly if neither of the objects are CRD-types, using the "classic" API machinery
+	if !isHub && !isConvertible {
+		// Convert normally using the specified groupversion
+		return c.scheme.ConvertToVersion(in, groupVersioner)
+
+	} else if isHub && isConvertible { // Validate that the object isn't crazy and implements both interfaces
+		return nil, NewCRDConversionError(nil, CRDConversionErrorCauseInvalidArgs, errObjMustNotBeBoth)
+	}
+
+	// We now know that either isHub or isConvertible is true, but not both
+	// If we are in the Decode codepath, the groupVersioner will be internal
+	// We'll need to take special care to convert the object into a Hub
+	if groupVersioner == runtime.InternalGroupVersioner {
+		// As a "ConvertToHub" was asked for, and the in object already is a Hub, just return a deepcopy
+		if isHub {
+			return in.DeepCopyObject(), nil
+		}
+
+		// Otherwise, convert it to a Hub
 		return c.convertToHub(convertible)
 	}
 
-	// Convert normally into the internal version using the internal groupversioner.
-	return c.scheme.ConvertToVersion(in, runtime.InternalGroupVersioner)
+	// At this point we are in the encode codepath. The groupversioner given specifies what
+	// groupVersion to convert into
+
+	// Get what group version was asked for
+	gv, ok := groupVersioner.(schema.GroupVersion)
+	if !ok {
+		return nil, fmt.Errorf("couldn't get groupversion from groupversioner: %v", groupVersioner)
+	}
+
+	// Get the groupversionkind for the in object
+	inGVK, err := gvkForObject(c.scheme, in)
+	if err != nil {
+		return nil, fmt.Errorf("couldn't get GVK for hub: %w", err)
+	}
+	// Assume the in and out (Hub and Convertible) kinds match (in encoded form)
+	outGVK := gv.WithKind(inGVK.Kind)
+
+	// Create the out object
+	out, err := c.scheme.New(outGVK)
+	if err != nil {
+		return nil, fmt.Errorf("can't create new obj of gvk %s: %w", outGVK, err)
+	}
+
+	// Run the generic convert in-into-out function, which will properly handle this CRD case
+	if err := c.Convert(in, out, nil); err != nil {
+		return nil, err
+	}
+
+	return out, nil
 }
 
 func (c *objectConvertor) convertToHub(in conversion.Convertible) (runtime.Object, error) {

pkg/serializer/decode.go

@@ -22,24 +22,31 @@ type DecodingOptions struct {
 	// will be converted into its hub (or internal, where applicable) representation. Otherwise, the decoded
 	// object will be left in its external representation. (Default: false)
 	ConvertToHub *bool
+
 	// Parse the YAML/JSON in strict mode, returning a specific error if the input
 	// contains duplicate or unknown fields or formatting errors. (Default: true)
 	Strict *bool
+
 	// Automatically default the decoded object. (Default: false)
 	Default *bool
+
 	// Only applicable for Decoder.DecodeAll(). If the underlying data contains a v1.List,
 	// the items of the list will be traversed, decoded into their respective types, and
 	// appended to the returned slice. The v1.List will in this case not be returned.
 	// This conversion does NOT support preserving comments. If the given scheme doesn't
 	// recognize the v1.List, before using it will be registered automatically. (Default: true)
 	DecodeListElements *bool
+
 	// Whether to preserve YAML comments internally. This only works for objects embedding metav1.ObjectMeta.
 	// Only applicable to ContentTypeYAML framers.
 	// Using any other framer will be silently ignored. Usage of this option also requires setting
 	// the PreserveComments in EncodingOptions, too. (Default: false)
 	PreserveComments *bool
 
-	// TODO: Add a DecodeUnknown option
+	// DecodeUnknown specifies whether decode objects with an unknown GroupVersionKind into a
+	// *runtime.Unknown object when running Decode(All) (true value) or to return an error when
+	// any unrecognized type is found (false value). (Default: false)
+	DecodeUnknown *bool
 }
 
 type DecodingOptionsFunc func(*DecodingOptions)
@@ -74,6 +81,12 @@ func WithCommentsDecode(comments bool) DecodingOptionsFunc {
 	}
 }
 
+func WithUnknownDecode(unknown bool) DecodingOptionsFunc {
+	return func(opts *DecodingOptions) {
+		opts.DecodeUnknown = &unknown
+	}
+}
+
 func WithDecodingOptions(newOpts DecodingOptions) DecodingOptionsFunc {
 	return func(opts *DecodingOptions) {
 		// TODO: Null-check all of these before using them
@@ -88,6 +101,7 @@ func defaultDecodeOpts() *DecodingOptions {
 		Default:            util.BoolPtr(false),
 		DecodeListElements: util.BoolPtr(true),
 		PreserveComments:   util.BoolPtr(false),
+		DecodeUnknown:      util.BoolPtr(false),
 	}
 }
 
@@ -119,6 +133,8 @@ type decoder struct {
 // If opts.ConvertToHub is true, the decoded external object will be converted into its hub
 // 	(or internal, if applicable) representation.
 // 	Otherwise, the decoded object will be left in the external representation.
+// If opts.DecodeUnknown is true, any type with an unrecognized apiVersion/kind will be returned as a
+// 	*runtime.Unknown object instead of returning a UnrecognizedTypeError.
 // opts.DecodeListElements is not applicable in this call.
 func (d *decoder) Decode(fr FrameReader) (runtime.Object, error) {
 	// Read a frame from the FrameReader
@@ -144,7 +160,15 @@ func (d *decoder) decode(doc []byte, into runtime.Object, ct ContentType) (runti
 	// TODO: Make sure any possible strict errors are returned/handled properly
 	obj, gvk, err := d.decoder.Decode(doc, nil, into)
 	if err != nil {
+		// If we asked to decode unknown objects, we are in the Decode(All) (not Into)
+		// codepath, and the error returned was due to that the kind was not registered
+		// in the scheme, decode the document as a *runtime.Unknown
+		if *d.opts.DecodeUnknown && !intoGiven && runtime.IsNotRegisteredError(err) {
+			return d.decodeUnknown(doc, ct)
+		}
 		// Give the user good errors wrt missing group & version
+		// TODO: It might be unnecessary to unmarshal twice (as we do in handleDecodeError),
+		// as gvk was returned from Decode above.
 		return nil, d.handleDecodeError(doc, err)
 	}
 
@@ -180,6 +204,9 @@ func (d *decoder) decode(doc []byte, into runtime.Object, ct ContentType) (runti
 // 	a returned failed because of the strictness using k8s.io/apimachinery/pkg/runtime.IsStrictDecodingError.
 // opts.DecodeListElements is not applicable in this call.
 // opts.ConvertToHub is not applicable in this call.
+// opts.DecodeUnknown is not applicable in this call. In case you want to decode an object into a
+// 	*runtime.Unknown, just create a runtime.Unknown object and pass the pointer as obj into DecodeInto
+// 	and it'll work.
 func (d *decoder) DecodeInto(fr FrameReader, into runtime.Object) error {
 	// Read a frame from the FrameReader.
 	// TODO: Make sure to test the case when doc might contain something, and err is io.EOF
@@ -206,6 +233,8 @@ func (d *decoder) DecodeInto(fr FrameReader, into runtime.Object) error {
 // If opts.DecodeListElements is true and the underlying data contains a v1.List,
 // 	the items of the list will be traversed and decoded into their respective types, which are
 // 	added into the returning slice. The v1.List will in this case not be returned.
+// If opts.DecodeUnknown is true, any type with an unrecognized apiVersion/kind will be returned as a
+// 	*runtime.Unknown object instead of returning a UnrecognizedTypeError.
 func (d *decoder) DecodeAll(fr FrameReader) ([]runtime.Object, error) {
 	objs := []runtime.Object{}
 	for {
@@ -228,6 +257,15 @@ func (d *decoder) DecodeAll(fr FrameReader) ([]runtime.Object, error) {
 	return objs, nil
 }
 
+// decodeUnknown decodes bytes of a certain content type into a returned *runtime.Unknown object
+func (d *decoder) decodeUnknown(doc []byte, ct ContentType) (runtime.Object, error) {
+	// Do a DecodeInto the new pointer to the object we've got. The resulting into object is
+	// also returned.
+	// The content type isn't really used here, as runtime.Unknown will never implement
+	// ObjectMeta, but the signature needs it so we'll just forward it
+	return d.decode(doc, &runtime.Unknown{}, ct)
+}
+
 func (d *decoder) handleDecodeError(doc []byte, origErr error) error {
 	// Parse the document's TypeMeta information
 	gvk, err := extractYAMLTypeMeta(doc)
@@ -289,9 +327,22 @@ func newDecoder(schemeAndCodec *schemeAndCodec, opts DecodingOptions) Decoder {
 		Strict: *opts.Strict,
 	})
 
-	codec := newConversionCodecForScheme(schemeAndCodec.scheme, nil, s, nil, runtime.InternalGroupVersioner, *opts.Default, *opts.ConvertToHub)
+	decodeCodec := decoderForVersion(schemeAndCodec.scheme, s, *opts.Default, *opts.ConvertToHub)
+
+	return &decoder{schemeAndCodec, decodeCodec, opts}
+}
 
-	return &decoder{schemeAndCodec, codec, opts}
+// decoderForVersion is used instead of CodecFactory.DecoderForVersion, as we want to use our own converter
+func decoderForVersion(scheme *runtime.Scheme, decoder *json.Serializer, doDefaulting, doConversion bool) runtime.Decoder {
+	return newConversionCodecForScheme(
+		scheme,
+		nil,                            // no encoder
+		decoder,                        // our custom JSON serializer
+		nil,                            // no target encode groupversion
+		runtime.InternalGroupVersioner, // if conversion should happen for classic types, convert into internal
+		doDefaulting,                   // default if specified
+		doConversion,                   // convert to the hub type conditionally when decoding
+	)
 }
 
 // newConversionCodecForScheme is a convenience method for callers that are using a scheme.

pkg/serializer/encode.go

@@ -124,7 +124,7 @@ func (e *encoder) EncodeForGroupVersion(fw FrameWriter, obj runtime.Object, gv s
 	}
 
 	// Get a version-specific encoder for the specified groupversion
-	versionEncoder := e.codecs.EncoderForVersion(encoder, gv)
+	versionEncoder := encoderForVersion(e.scheme, encoder, gv)
 
 	// Cast the object to a metav1.Object to get access to annotations
 	metaobj, ok := toMetaObject(obj)
@@ -136,3 +136,16 @@ func (e *encoder) EncodeForGroupVersion(fw FrameWriter, obj runtime.Object, gv s
 	// Specialize the encoder for a specific gv and encode the object
 	return e.encodeWithCommentSupport(versionEncoder, fw, obj, metaobj)
 }
+
+// encoderForVersion is used instead of CodecFactory.EncoderForVersion, as we want to use our own converter
+func encoderForVersion(scheme *runtime.Scheme, encoder runtime.Encoder, gv schema.GroupVersion) runtime.Encoder {
+	return newConversionCodecForScheme(
+		scheme,
+		encoder, // what content-type encoder to use
+		nil,     // no decoder
+		gv,      // specify what the target encode groupversion is
+		nil,     // no target decode groupversion
+		false,   // no defaulting
+		true,    // convert if needed before encode
+	)
+}

pkg/serializer/serializer.go

@@ -93,8 +93,11 @@ type Decoder interface {
 	// 	a returned failed because of the strictness using k8s.io/apimachinery/pkg/runtime.IsStrictDecodingError.
 	// If opts.ConvertToHub is true, the decoded external object will be converted into its internal representation.
 	// 	Otherwise, the decoded object will be left in the external representation.
+	// If opts.DecodeUnknown is true, any type with an unrecognized apiVersion/kind will be returned as a
+	// 	*runtime.Unknown object instead of returning a UnrecognizedTypeError.
 	// opts.DecodeListElements is not applicable in this call.
 	Decode(fr FrameReader) (runtime.Object, error)
+
 	// DecodeInto decodes the next document in the FrameReader stream into obj if the types are matching.
 	// If there are multiple documents in the underlying stream, this call will read one
 	// 	document and return it. Decode might be invoked for getting new documents until it
@@ -109,6 +112,9 @@ type Decoder interface {
 	// 	a returned failed because of the strictness using k8s.io/apimachinery/pkg/runtime.IsStrictDecodingError.
 	// opts.DecodeListElements is not applicable in this call.
 	// opts.ConvertToHub is not applicable in this call.
+	// opts.DecodeUnknown is not applicable in this call. In case you want to decode an object into a
+	// 	*runtime.Unknown, just create a runtime.Unknown object and pass the pointer as obj into DecodeInto
+	// 	and it'll work.
 	DecodeInto(fr FrameReader, obj runtime.Object) error
 
 	// DecodeAll returns the decoded objects from all documents in the FrameReader stream. The underlying
@@ -124,6 +130,8 @@ type Decoder interface {
 	// If opts.DecodeListElements is true and the underlying data contains a v1.List,
 	// 	the items of the list will be traversed and decoded into their respective types, which are
 	// 	added into the returning slice. The v1.List will in this case not be returned.
+	// If opts.DecodeUnknown is true, any type with an unrecognized apiVersion/kind will be returned as a
+	// 	*runtime.Unknown object instead of returning a UnrecognizedTypeError.
 	DecodeAll(fr FrameReader) ([]runtime.Object, error)
 }