Merge pull request #424 from ulucinar/fix-conversion-typemeta

Fix empty TypeMeta while running API conversions

pkg/controller/conversion/functions.go

@@ -8,6 +8,7 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 
 	"github.com/crossplane/upjet/pkg/config/conversion"
 	"github.com/crossplane/upjet/pkg/resource"
@@ -17,12 +18,32 @@ const (
 	errFmtPrioritizedManagedConversion = "cannot apply the PrioritizedManagedConversion for the %q object"
 	errFmtPavedConversion              = "cannot apply the PavedConversion for the %q object"
 	errFmtManagedConversion            = "cannot apply the ManagedConversion for the %q object"
+	errFmtGetGVK                       = "cannot get the GVK for the %s object of type %T"
 )
 
 // RoundTrip round-trips from `src` to `dst` via an unstructured map[string]any
 // representation of the `src` object and applies the registered webhook
 // conversion functions of this registry.
 func (r *registry) RoundTrip(dst, src resource.Terraformed) error { //nolint:gocyclo // considered breaking this according to the converters and I did not like it
+	if dst.GetObjectKind().GroupVersionKind().Version == "" {
+		gvk, err := apiutil.GVKForObject(dst, r.scheme)
+		if err != nil && !runtime.IsNotRegisteredError(err) {
+			return errors.Wrapf(err, errFmtGetGVK, "destination", dst)
+		}
+		if err == nil {
+			dst.GetObjectKind().SetGroupVersionKind(gvk)
+		}
+	}
+	if src.GetObjectKind().GroupVersionKind().Version == "" {
+		gvk, err := apiutil.GVKForObject(src, r.scheme)
+		if err != nil && !runtime.IsNotRegisteredError(err) {
+			return errors.Wrapf(err, errFmtGetGVK, "source", src)
+		}
+		if err == nil {
+			src.GetObjectKind().SetGroupVersionKind(gvk)
+		}
+	}
+
 	// first PrioritizedManagedConversions are run in their registration order
 	for _, c := range r.GetConversions(dst) {
 		if pc, ok := c.(conversion.PrioritizedManagedConversion); ok {

pkg/controller/conversion/functions_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/crossplane/upjet/pkg/config"
@@ -80,6 +81,32 @@ func TestRoundTrip(t *testing.T) {
 				dst: fake.NewTerraformed(fake.WithParameters(fake.NewMap(commonKey, commonVal, key2, val1))),
 			},
 		},
+		"SuccessfulRoundTripWithNonWildcardConversions": {
+			reason: "Source object is successfully converted into the target object with a set of non-wildcard conversions.",
+			args: args{
+				dst: fake.NewTerraformed(fake.WithTypeMeta(metav1.TypeMeta{})),
+				src: fake.NewTerraformed(fake.WithParameters(fake.NewMap(commonKey, commonVal, key1, val1)), fake.WithTypeMeta(metav1.TypeMeta{})),
+				conversions: []conversion.Conversion{
+					conversion.NewIdentityConversionExpandPaths(fake.Version, fake.Version, nil),
+					// Because the parameters of the fake.Terraformed is an unstructured
+					// map, all the fields of source (including key1) are successfully
+					// copied into dst by registry.RoundTrip.
+					// This conversion deletes the copied key "key1".
+					conversion.NewCustomConverter(fake.Version, fake.Version, func(_, target xpresource.Managed) error {
+						tr := target.(*fake.Terraformed)
+						delete(tr.Parameters, key1)
+						return nil
+					}),
+					conversion.NewFieldRenameConversion(fake.Version, fmt.Sprintf("parameterizable.parameters.%s", key1), fake.Version, fmt.Sprintf("parameterizable.parameters.%s", key2)),
+				},
+			},
+			want: want{
+				dst: fake.NewTerraformed(fake.WithParameters(fake.NewMap(commonKey, commonVal, key2, val1)), fake.WithTypeMeta(metav1.TypeMeta{
+					Kind:       fake.Kind,
+					APIVersion: fake.GroupVersion.String(),
+				})),
+			},
+		},
 		"RoundTripFailedPrioritizedConversion": {
 			reason: "Should return an error if a PrioritizedConversion fails.",
 			args: args{
@@ -125,6 +152,12 @@ func TestRoundTrip(t *testing.T) {
 			},
 		},
 	}
+
+	s := runtime.NewScheme()
+	if err := fake.AddToScheme(s); err != nil {
+		t.Fatalf("Failed to register the fake.Terraformed object with the runtime scheme")
+	}
+
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			p := &config.Provider{
@@ -134,7 +167,9 @@ func TestRoundTrip(t *testing.T) {
 					},
 				},
 			}
-			r := &registry{}
+			r := &registry{
+				scheme: s,
+			}
 			if err := r.RegisterConversions(p); err != nil {
 				t.Fatalf("\n%s\nRegisterConversions(p): Failed to register the conversions with the registry.\n", tc.reason)
 			}

pkg/controller/conversion/registry.go

@@ -6,6 +6,7 @@ package conversion
 
 import (
 	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"github.com/crossplane/upjet/pkg/config"
 	"github.com/crossplane/upjet/pkg/config/conversion"
@@ -21,6 +22,7 @@ var instance *registry
 // registry represents the conversion hook registry for a provider.
 type registry struct {
 	provider *config.Provider
+	scheme   *runtime.Scheme
 }
 
 // RegisterConversions registers the API version conversions from the specified
@@ -50,11 +52,16 @@ func GetConversions(tr resource.Terraformed) []conversion.Conversion {
 }
 
 // RegisterConversions registers the API version conversions from the specified
-// provider configuration.
-func RegisterConversions(provider *config.Provider) error {
+// provider configuration. The specified scheme should contain the registrations
+// for the types whose versions are to be converted. If a registration for a
+// Go schema is not found in the specified registry, RoundTrip does not error
+// but only wildcard conversions must be used with the registry.
+func RegisterConversions(provider *config.Provider, scheme *runtime.Scheme) error {
 	if instance != nil {
 		return errors.New(errAlreadyRegistered)
 	}
-	instance = &registry{}
+	instance = &registry{
+		scheme: scheme,
+	}
 	return instance.RegisterConversions(provider)
 }

pkg/resource/fake/terraformed.go

@@ -5,10 +5,14 @@
 package fake
 
 import (
+	"reflect"
+
 	"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/json"
+	"sigs.k8s.io/controller-runtime/pkg/scheme"
 )
 
 // Observable is mock Observable.
@@ -100,6 +104,7 @@ func (li *LateInitializer) LateInitialize(_ []byte) (bool, error) {
 
 // Terraformed is a mock that implements Terraformed interface.
 type Terraformed struct {
+	metav1.TypeMeta `json:",inline"`
 	fake.Managed
 	Observable
 	Parameterizable
@@ -109,7 +114,7 @@ type Terraformed struct {
 
 // GetObjectKind returns schema.ObjectKind.
 func (t *Terraformed) GetObjectKind() schema.ObjectKind {
-	return schema.EmptyObjectKind
+	return &t.TypeMeta
 }
 
 // DeepCopyObject returns a copy of the object as runtime.Object
@@ -133,9 +138,21 @@ func WithParameters(params map[string]any) Option {
 	}
 }
 
+// WithTypeMeta sets the TypeMeta of a Terraformed.
+func WithTypeMeta(t metav1.TypeMeta) Option {
+	return func(tr *Terraformed) {
+		tr.TypeMeta = t
+	}
+}
+
 // NewTerraformed initializes a new Terraformed with the given options.
 func NewTerraformed(opts ...Option) *Terraformed {
-	tr := &Terraformed{}
+	tr := &Terraformed{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       Kind,
+			APIVersion: GroupVersion.String(),
+		},
+	}
 	for _, o := range opts {
 		o(tr)
 	}
@@ -152,3 +169,28 @@ func NewMap(keyValue ...string) map[string]any {
 	}
 	return m
 }
+
+const (
+	// Group for the fake.Terraformed objects
+	Group = "fake.upjet.crossplane.io"
+	// Version for the fake.Terraformed objects
+	Version = "v1alpha1"
+)
+
+var (
+	// Kind is the Go type name of the Terraformed resource.
+	Kind = reflect.TypeOf(Terraformed{}).Name()
+
+	// GroupVersion is the API Group Version used to register the objects
+	GroupVersion = schema.GroupVersion{Group: Group, Version: Version}
+
+	// SchemeBuilder is used to add go types to the GroupVersionKind scheme
+	SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
+
+	// AddToScheme adds the types in this group-version to the given scheme.
+	AddToScheme = SchemeBuilder.AddToScheme
+)
+
+func init() {
+	SchemeBuilder.Register(&Terraformed{})
+}