Multiversion CRDs & Conversion Webhooks

[ulucinar]

Description of your changes

Depends on: #326, #331

This PR adds support for generating CRDs with multiple versions and generating the conversion.Hub and conversion.Convertible implementations for the resource.Terraformed resources.

The generated webhooks will invoke a chain of Conversions while converting from/to Hub versions to/from the spoke versions. There are currently two conversion types, conversion.PavedConversion and conversion.ManagedConversion.

PavedConversion is an optimized Conversion between two fieldpath.Paved objects. PavedConversion implementations for a specific source and target version pair are chained together and the source and the destination objects are paved once at the beginning of the chained PavedConversion.ConvertPaved calls. The target fieldpath.Paved object is then converted into the original resource.Terraformed object at the end of the chained calls. This prevents the intermediate conversions between the fieldpath.Paved and the resource.Terraformed representations of the same object. The fieldpath.Paved representation is convenient for writing generic Conversion implementations not bound to a specific type.

ManagedConversion defines a Conversion from a specific source resource.Managed to a target one. Generic Conversion implementations may prefer to implement the PavedConversion interface. Implementations of the ManagedConversion can do type assertions to the specific source and target types and so, they are expected to be strongly typed.

We are also providing some high-level Conversions to implement some common API conversion tasks. conversion.NewFieldRenameConversion returns a new PavedConversion that implements a field renaming conversion from a specified source version to a specified target version of an API. conversion.NewCustomConverter can be used to initialize a new ManagedConversion that invokes a specified conversion function on a source and destination resource.Managed pair.

The upjet resource configuration framework is extended with a configuration parameter to register a chain of Conversions to be invoked on a Hub object while converting between this Hub and the spokes. A sample configuration is as follows:

// an example renaming from `triggers` in the `v1beta1` API to `eventTriggers` in the `v1beta2` API
func Configure(p *ujconfig.Provider) {
	p.AddResourceConfigurator("group_resource", func(r *ujconfig.Resource) {
		r.Kind = "Resource"
		r.Version = "v1beta2"
		r.Conversions = append(r.Conversions, conversion.NewFieldRenameConversion("v1beta1", "spec.forProvider.triggers", "v1beta2", "spec.forProvider.eventTriggers"))
...

Or, you can register a strongly typed conversion using the conversion.NewCustomConverter function as follows:

// an example strongly typed custom conversion between the `v1beta1` and `v1beta2` groups
func Configure(p *config.Provider) {
	p.AddResourceConfigurator("aws_autoscaling_group", func(r *config.Resource) {
...
		r.Version = "v1beta2"
		r.Conversions = append(r.Conversions,
			conversion.NewCustomConverter("v1beta1", "v1beta2", func(src, target xpresource.Managed) error {
				srcTyped := src.(*v1beta1.AutoscalingGroup)
				targetTyped := target.(*v1beta2.AutoscalingGroup)
...

This PR also introduces a Conversion registry through which one can register the Conversions to be invoked by the generated conversion webhooks. Because the configured Conversions are owned by the config.Provider for the provider, the following example call can be used to register those configured Conversions:

kingpin.FatalIfError(conversion.RegisterConversions(o.Provider), "Cannot initialize the webhook conversion registry")

, where o.Provider is the *config.Provider instance that represents the upjet provider's configuration.

I have:

• Read and followed Upjet's contribution process.
• Run make reviewable to ensure this PR is ready for review.
• Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Tested against https://github.com/ulucinar/upjet-provider-template/tree/multiversion-crds and crossplane-contrib/provider-upjet-aws#1075.

TODO:

• Increase unit test coverage

[sergenyalcin]

Thanks @ulucinar LGTM. I left two comments for documenting. I validated the PR in the provider-aws tests and custom converters effort.

[sergenyalcin]

nit: Only for documenting, we may consider adding a filtering option for deciding whether to copy all possible fields to the destination. This can also be handled in converters.

[ulucinar]

I agree. Let's see the frequency of the need for that functionality and if we frequently need to do this, then we can add a filtering API. Currently, as discussed, we can just let the RoundTrip copy everything and in the conversion function, we can unset the unwanted fields. This is comparable effort to preparing a filter, i.e., instead of passing a filter to RoundTrip, unsetting a field in a custom converter. Here, the assumption is we are dealing with a custom converter, which can be strongly typed.

[sergenyalcin]

Is this indirection because we don't have a registry object on the provider side? Now, we consume the universal variable instance in the upjet side, right? Is there any specific reason for this? Is creating and consuming a registry object on the provider side an option?

[ulucinar]

Thanks for the question. We need to invoke the registered conversion functions from the conversion webhooks. So, the conversion webhook needs to know the chain of the conversion functions available for a (spoke or hub) version. Unfortunately, unlike the reconcilers where the reconciliations for a given GVK happen under the context of a reconciler object that we register with the controller-runtime manager, a similar context variable that's directly owned (initialized) by the provider does not exist. Hence, we use a global registry that can convert a given managed resource kind to the conversion functions associated with it. We can initialize this global registry in the provider codebase but I've preferred to do this in upjet so that we will not need to repeat this for different providers.

The conversion.Convertible implementation that upjet generates currently relies on this global registry that upjet initializes using the conversion function configuration provided by the provider. We could extend upjet provider configuration and let the provider specify the global conversion registry to upjet and then, upjet could use that registry instead of the one that it directly owns (in the generated conversion.Convertible implementations). For now, I've avoided this indirection by letting upjet own this global registry.

[ulucinar]

Related to this discussion, we had better get rid of this global registry (whether it's owned by upjet or the provider) if we can find a way to set the context for the conversion webhooks. To the best of my knowledge, it's currently not possible with how the conversion.Convertible implementations are hooked into the controller runtime's webhook server. Of course, if we just replace the webhook server implementation itself and use a custom machinery instead of the conversion.Convertible, we can achieve this but I don't believe it would be worth the effort.