Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix empty TypeMeta while running API conversions #424

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions pkg/controller/conversion/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
37 changes: 36 additions & 1 deletion pkg/controller/conversion/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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)
}
Expand Down
13 changes: 10 additions & 3 deletions pkg/controller/conversion/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
46 changes: 44 additions & 2 deletions pkg/resource/fake/terraformed.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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{})
}
Loading