Rename conversion.TerraformedConversion to conversion.ManagedConversion

- Add unit tests for conversion.RoundTrip

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

pkg/config/conversion/conversions.go

@@ -49,18 +49,18 @@ type PavedConversion interface {
 	ConvertPaved(src, target *fieldpath.Paved) (bool, error)
 }
 
-// TerraformedConversion defines a Conversion from a specific source
-// resource.Terraformed type to a target one. Generic Conversion
+// ManagedConversion defines a Conversion from a specific source
+// resource.Managed type to a target one. Generic Conversion
 // implementations may prefer to implement the PavedConversion interface.
-// Implementations of TerraformedConversion can do type assertions to
-// specific source and target types and so they are expected to be
+// Implementations of ManagedConversion can do type assertions to
+// specific source and target types, and so, they are expected to be
 // strongly typed.
-type TerraformedConversion interface {
+type ManagedConversion interface {
 	Conversion
-	// ConvertTerraformed converts from the `src` managed resource to the `dst`
+	// ConvertManaged converts from the `src` managed resource to the `dst`
 	// managed resource and returns `true` if the conversion has been done,
 	// `false` otherwise, together with any errors encountered.
-	ConvertTerraformed(src, target resource.Managed) (bool, error)
+	ConvertManaged(src, target resource.Managed) (bool, error)
 }
 
 type baseConversion struct {
@@ -124,16 +124,17 @@ type customConversion struct {
 	customConverter customConverter
 }
 
-func (cc *customConversion) ConvertTerraformed(src, target resource.Managed) (bool, error) {
-	if !cc.Applicable(src, target) {
+func (cc *customConversion) ConvertManaged(src, target resource.Managed) (bool, error) {
+	if !cc.Applicable(src, target) || cc.customConverter == nil {
 		return false, nil
 	}
-	if err := cc.customConverter(src, target); err != nil {
-		return false, err
-	}
-	return true, nil
+	return true, errors.Wrap(cc.customConverter(src, target), "failed to apply the converter function")
 }
 
+// NewCustomConverter returns a new Conversion from the specified
+// `sourceVersion` of an API to the specified `targetVersion` and invokes
+// the specified converter function to perform the conversion on the
+// managed resources.
 func NewCustomConverter(sourceVersion, targetVersion string, converter func(src, target resource.Managed) error) Conversion {
 	return &customConversion{
 		baseConversion:  newBaseConversion(sourceVersion, targetVersion),

pkg/config/conversion/conversions_test.go

@@ -8,11 +8,10 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/google/go-cmp/cmp"
 	"k8s.io/utils/ptr"
-
-	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
 )
 
 const (

pkg/controller/conversion/functions.go

@@ -15,8 +15,8 @@ import (
 
 // 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.
-func RoundTrip(dst, src resource.Terraformed) error { //nolint:gocyclo // considered breaking this according to the converters and I did not like it
+// 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
 	srcMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(src)
 	if err != nil {
 		return errors.Wrap(err, "cannot convert the conversion source object into the map[string]any representation")
@@ -35,7 +35,7 @@ func RoundTrip(dst, src resource.Terraformed) error { //nolint:gocyclo // consid
 	}
 	srcPaved := fieldpath.Pave(srcMap)
 	dstPaved := fieldpath.Pave(dstMap)
-	for _, c := range GetConversions(dst) {
+	for _, c := range r.GetConversions(dst) {
 		if pc, ok := c.(conversion.PavedConversion); ok {
 			if _, err := pc.ConvertPaved(srcPaved, dstPaved); err != nil {
 				return errors.Wrapf(err, "cannot apply the PavedConversion for the %q object", dst.GetTerraformResourceType())
@@ -48,13 +48,20 @@ func RoundTrip(dst, src resource.Terraformed) error { //nolint:gocyclo // consid
 		return errors.Wrap(err, "cannot convert the map[string]any representation of the conversion target object to the target object")
 	}
 
-	for _, c := range GetConversions(dst) {
-		if tc, ok := c.(conversion.TerraformedConversion); ok {
-			if _, err := tc.ConvertTerraformed(src, dst); err != nil {
+	for _, c := range r.GetConversions(dst) {
+		if tc, ok := c.(conversion.ManagedConversion); ok {
+			if _, err := tc.ConvertManaged(src, dst); err != nil {
 				return errors.Wrapf(err, "cannot apply the TerraformedConversion for the %q object", dst.GetTerraformResourceType())
 			}
 		}
 	}
 
 	return nil
 }
+
+// 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.
+func RoundTrip(dst, src resource.Terraformed) error {
+	return instance.RoundTrip(dst, src)
+}

pkg/controller/conversion/functions_test.go

@@ -0,0 +1,103 @@
+// SPDX-FileCopyrightText: 2023 The Crossplane Authors <https://crossplane.io>
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package conversion
+
+import (
+	"fmt"
+	"testing"
+
+	xpresource "github.com/crossplane/crossplane-runtime/pkg/resource"
+	"github.com/crossplane/crossplane-runtime/pkg/test"
+	"github.com/google/go-cmp/cmp"
+
+	"github.com/crossplane/upjet/pkg/config"
+	"github.com/crossplane/upjet/pkg/config/conversion"
+	"github.com/crossplane/upjet/pkg/resource"
+	"github.com/crossplane/upjet/pkg/resource/fake"
+)
+
+const (
+	key1      = "key1"
+	val1      = "val1"
+	key2      = "key2"
+	val2      = "val2"
+	commonKey = "commonKey"
+	commonVal = "commonVal"
+)
+
+func TestRoundTrip(t *testing.T) {
+	type args struct {
+		dst         resource.Terraformed
+		src         resource.Terraformed
+		conversions []conversion.Conversion
+	}
+	type want struct {
+		err error
+		dst resource.Terraformed
+	}
+	tests := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"SuccessfulRoundTrip": {
+			reason: "Source object is successfully copied into the target object.",
+			args: args{
+				dst: fake.NewTerraformed(),
+				src: fake.NewTerraformed(fake.WithParameters(fake.NewMap(key1, val1))),
+			},
+			want: want{
+				dst: fake.NewTerraformed(fake.WithParameters(fake.NewMap(key1, val1))),
+			},
+		},
+		"SuccessfulRoundTripWithConversions": {
+			reason: "Source object is successfully converted into the target object with a set of conversions.",
+			args: args{
+				dst: fake.NewTerraformed(),
+				src: fake.NewTerraformed(fake.WithParameters(fake.NewMap(commonKey, commonVal, key1, val1))),
+				conversions: []conversion.Conversion{
+					// 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(conversion.AllVersions, conversion.AllVersions, func(_, target xpresource.Managed) error {
+						tr := target.(*fake.Terraformed)
+						delete(tr.Parameters, key1)
+						return nil
+					}),
+					conversion.NewFieldRenameConversion(conversion.AllVersions, fmt.Sprintf("parameterizable.parameters.%s", key1), conversion.AllVersions, fmt.Sprintf("parameterizable.parameters.%s", key2)),
+				},
+			},
+			want: want{
+				dst: fake.NewTerraformed(fake.WithParameters(fake.NewMap(commonKey, commonVal, key2, val1))),
+			},
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			p := &config.Provider{
+				Resources: map[string]*config.Resource{
+					tc.args.dst.GetTerraformResourceType(): {
+						Conversions: tc.args.conversions,
+					},
+				},
+			}
+			r := &registry{}
+			if err := r.RegisterConversions(p); err != nil {
+				t.Fatalf("\n%s\nRegisterConversions(p): Failed to register the conversions with the registry.\n", tc.reason)
+			}
+			err := r.RoundTrip(tc.args.dst, tc.args.src)
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\nRoundTrip(dst, src): -wantErr, +gotErr:\n%s", tc.reason, diff)
+			}
+			if tc.want.err != nil {
+				return
+			}
+			if diff := cmp.Diff(tc.want.dst, tc.args.dst); diff != "" {
+				t.Errorf("\n%s\nRoundTrip(dst, src): -wantDst, +gotDst:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}

pkg/controller/conversion/registry.go

@@ -24,23 +24,37 @@ type registry struct {
 }
 
 // RegisterConversions registers the API version conversions from the specified
-// provider configuration.
-func RegisterConversions(provider *config.Provider) error {
-	if instance != nil {
+// provider configuration with this registry.
+func (r *registry) RegisterConversions(provider *config.Provider) error {
+	if r.provider != nil {
 		return errors.New(errAlreadyRegistered)
 	}
-	instance = &registry{
-		provider: provider,
-	}
+	r.provider = provider
 	return nil
 }
 
-// GetConversions returns the conversion.Conversions registered for the
-// Terraformed resource.
-func GetConversions(tr resource.Terraformed) []conversion.Conversion {
+// GetConversions returns the conversion.Conversions registered in this
+// registry for the specified Terraformed resource.
+func (r *registry) GetConversions(tr resource.Terraformed) []conversion.Conversion {
 	t := tr.GetTerraformResourceType()
-	if instance == nil || instance.provider == nil || instance.provider.Resources[t] == nil {
+	if r == nil || r.provider == nil || r.provider.Resources[t] == nil {
 		return nil
 	}
-	return instance.provider.Resources[t].Conversions
+	return r.provider.Resources[t].Conversions
+}
+
+// GetConversions returns the conversion.Conversions registered for the
+// specified Terraformed resource.
+func GetConversions(tr resource.Terraformed) []conversion.Conversion {
+	return instance.GetConversions(tr)
+}
+
+// RegisterConversions registers the API version conversions from the specified
+// provider configuration.
+func RegisterConversions(provider *config.Provider) error {
+	if instance != nil {
+		return errors.New(errAlreadyRegistered)
+	}
+	instance = &registry{}
+	return instance.RegisterConversions(provider)
 }

pkg/resource/fake/terraformed.go

@@ -122,3 +122,33 @@ func (t *Terraformed) DeepCopyObject() runtime.Object {
 	_ = json.Unmarshal(j, out)
 	return out
 }
+
+// Option is an option to modify the properties of a Terraformed object.
+type Option func(terraformed *Terraformed)
+
+// WithParameters sets the parameters of a Terraformed.
+func WithParameters(params map[string]any) Option {
+	return func(tr *Terraformed) {
+		tr.Parameters = params
+	}
+}
+
+// NewTerraformed initializes a new Terraformed with the given options.
+func NewTerraformed(opts ...Option) *Terraformed {
+	tr := &Terraformed{}
+	for _, o := range opts {
+		o(tr)
+	}
+	return tr
+}
+
+// NewMap prepares a map from the supplied key value parameters.
+// The parameters slice must be a sequence of key, value pairs and must have
+// an even length. The function will panic otherwise.
+func NewMap(keyValue ...string) map[string]any {
+	m := make(map[string]any, len(keyValue)/2)
+	for i := 0; i < len(keyValue)-1; i += 2 {
+		m[keyValue[i]] = keyValue[i+1]
+	}
+	return m
+}