From a08ecd7fe3d76051337f757e8980e0a4226c6977 Mon Sep 17 00:00:00 2001 From: Alper Rifat Ulucinar Date: Thu, 25 Jul 2024 13:58:16 +0300 Subject: [PATCH] Fix empty TypeMeta while running API conversions Signed-off-by: Alper Rifat Ulucinar --- pkg/controller/conversion/functions.go | 21 ++++++++++ pkg/controller/conversion/functions_test.go | 37 ++++++++++++++++- pkg/controller/conversion/registry.go | 13 ++++-- pkg/resource/fake/terraformed.go | 46 ++++++++++++++++++++- 4 files changed, 111 insertions(+), 6 deletions(-) diff --git a/pkg/controller/conversion/functions.go b/pkg/controller/conversion/functions.go index da4f46f5..35403b56 100644 --- a/pkg/controller/conversion/functions.go +++ b/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 { diff --git a/pkg/controller/conversion/functions_test.go b/pkg/controller/conversion/functions_test.go index 2df01aee..77ca6662 100644 --- a/pkg/controller/conversion/functions_test.go +++ b/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 := ®istry{} + r := ®istry{ + 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) } diff --git a/pkg/controller/conversion/registry.go b/pkg/controller/conversion/registry.go index ab1fac5b..66baf61f 100644 --- a/pkg/controller/conversion/registry.go +++ b/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 = ®istry{} + instance = ®istry{ + scheme: scheme, + } return instance.RegisterConversions(provider) } diff --git a/pkg/resource/fake/terraformed.go b/pkg/resource/fake/terraformed.go index ed891bb4..9197260b 100644 --- a/pkg/resource/fake/terraformed.go +++ b/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{}) +}