feat(managed): Add generic interfaces for external types

Add a wrapper for typed external clients that take over the conversion from resource.Managed to the actual Go struct that is always necessary in every controller. It does not reduce complexity as a whole but allows controllers to save some LoCs and focus on the important things. Keep the previous types as type aliases to avoid any breaking changes. Signed-off-by: Maximilian Blatt <[email protected]>
crossplane · Aug 23, 2024 · 7f2eeb1 · 7f2eeb1
1 parent 089f5f2
commit 7f2eeb1
Show file tree

Hide file tree

Showing 3 changed files with 204 additions and 30 deletions.
diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go
@@ -230,10 +230,14 @@ func (m ReferenceResolverFn) ResolveReferences(ctx context.Context, mg resource.
 
 // An ExternalConnecter produces a new ExternalClient given the supplied
 // Managed resource.
-type ExternalConnecter interface {
+type ExternalConnecter = TypedExternalConnecter[resource.Managed]
+
+// A TypedExternalConnecter produces a new ExternalClient given the supplied
+// Managed resource.
+type TypedExternalConnecter[managed resource.Managed] interface {
 	// Connect to the provider specified by the supplied managed resource and
 	// produce an ExternalClient.
-	Connect(ctx context.Context, mg resource.Managed) (ExternalClient, error)
+	Connect(ctx context.Context, mg managed) (TypedExternalClient[managed], error)
 }
 
 // An ExternalDisconnecter disconnects from a provider.
@@ -247,40 +251,58 @@ type ExternalDisconnecter interface {
 
 // A NopDisconnecter converts an ExternalConnecter into an
 // ExternalConnectDisconnecter with a no-op Disconnect method.
-type NopDisconnecter struct {
-	c ExternalConnecter
+type NopDisconnecter = TypedNopDisconnecter[resource.Managed]
+
+// A TypedNopDisconnecter converts an ExternalConnecter into an
+// ExternalConnectDisconnecter with a no-op Disconnect method.
+type TypedNopDisconnecter[managed resource.Managed] struct {
+	c TypedExternalConnecter[managed]
 }
 
 // Connect calls the underlying ExternalConnecter's Connect method.
-func (c *NopDisconnecter) Connect(ctx context.Context, mg resource.Managed) (ExternalClient, error) {
+func (c *TypedNopDisconnecter[managed]) Connect(ctx context.Context, mg managed) (TypedExternalClient[managed], error) {
 	return c.c.Connect(ctx, mg)
 }
 
 // Disconnect does nothing. It never returns an error.
-func (c *NopDisconnecter) Disconnect(_ context.Context) error {
+func (c *TypedNopDisconnecter[managed]) Disconnect(_ context.Context) error {
 	return nil
 }
 
 // NewNopDisconnecter converts an ExternalConnecter into an
 // ExternalConnectDisconnecter with a no-op Disconnect method.
 func NewNopDisconnecter(c ExternalConnecter) ExternalConnectDisconnecter {
-	return &NopDisconnecter{c}
+	return NewTypedNopDisconnecter(c)
+}
+
+// NewTypedNopDisconnecter converts an TypedExternalConnecter into an
+// ExternalConnectDisconnecter with a no-op Disconnect method.
+func NewTypedNopDisconnecter[managed resource.Managed](c TypedExternalConnecter[managed]) TypedExternalConnectDisconnecter[managed] {
+	return &TypedNopDisconnecter[managed]{c}
 }
 
 // An ExternalConnectDisconnecter produces a new ExternalClient given the supplied
 // Managed resource.
-type ExternalConnectDisconnecter interface {
-	ExternalConnecter
+type ExternalConnectDisconnecter = TypedExternalConnectDisconnecter[resource.Managed]
+
+// A TypedExternalConnectDisconnecter produces a new ExternalClient given the supplied
+// Managed resource.
+type TypedExternalConnectDisconnecter[managed resource.Managed] interface {
+	TypedExternalConnecter[managed]
 	ExternalDisconnecter
 }
 
 // An ExternalConnectorFn is a function that satisfies the ExternalConnecter
 // interface.
-type ExternalConnectorFn func(ctx context.Context, mg resource.Managed) (ExternalClient, error)
+type ExternalConnectorFn = TypedExternalConnectorFn[resource.Managed]
+
+// An TypedExternalConnectorFn is a function that satisfies the
+// TypedExternalConnecter interface.
+type TypedExternalConnectorFn[managed resource.Managed] func(ctx context.Context, mg managed) (TypedExternalClient[managed], error)
 
 // Connect to the provider specified by the supplied managed resource and
 // produce an ExternalClient.
-func (ec ExternalConnectorFn) Connect(ctx context.Context, mg resource.Managed) (ExternalClient, error) {
+func (ec TypedExternalConnectorFn[managed]) Connect(ctx context.Context, mg managed) (TypedExternalClient[managed], error) {
 	return ec(ctx, mg)
 }
 
@@ -295,19 +317,23 @@ func (ed ExternalDisconnectorFn) Disconnect(ctx context.Context) error {
 
 // ExternalConnectDisconnecterFns are functions that satisfy the
 // ExternalConnectDisconnecter interface.
-type ExternalConnectDisconnecterFns struct {
-	ConnectFn    func(ctx context.Context, mg resource.Managed) (ExternalClient, error)
+type ExternalConnectDisconnecterFns = TypedExternalConnectDisconnecterFns[resource.Managed]
+
+// TypedExternalConnectDisconnecterFns are functions that satisfy the
+// TypedExternalConnectDisconnecter interface.
+type TypedExternalConnectDisconnecterFns[managed resource.Managed] struct {
+	ConnectFn    func(ctx context.Context, mg managed) (TypedExternalClient[managed], error)
 	DisconnectFn func(ctx context.Context) error
 }
 
 // Connect to the provider specified by the supplied managed resource and
 // produce an ExternalClient.
-func (fns ExternalConnectDisconnecterFns) Connect(ctx context.Context, mg resource.Managed) (ExternalClient, error) {
+func (fns TypedExternalConnectDisconnecterFns[managed]) Connect(ctx context.Context, mg managed) (TypedExternalClient[managed], error) {
 	return fns.ConnectFn(ctx, mg)
 }
 
 // Disconnect from the provider and close the ExternalClient.
-func (fns ExternalConnectDisconnecterFns) Disconnect(ctx context.Context) error {
+func (fns TypedExternalConnectDisconnecterFns[managed]) Disconnect(ctx context.Context) error {
 	return fns.DisconnectFn(ctx)
 }
 
@@ -316,30 +342,37 @@ func (fns ExternalConnectDisconnecterFns) Disconnect(ctx context.Context) error
 // idempotent. For example, Create call should not return AlreadyExists error
 // if it's called again with the same parameters or Delete call should not
 // return error if there is an ongoing deletion or resource does not exist.
-type ExternalClient interface {
+type ExternalClient = TypedExternalClient[resource.Managed]
+
+// A TypedExternalClient manages the lifecycle of an external resource.
+// None of the calls here should be blocking. All of the calls should be
+// idempotent. For example, Create call should not return AlreadyExists error
+// if it's called again with the same parameters or Delete call should not
+// return error if there is an ongoing deletion or resource does not exist.
+type TypedExternalClient[managedType resource.Managed] interface {
 	// Observe the external resource the supplied Managed resource
 	// represents, if any. Observe implementations must not modify the
 	// external resource, but may update the supplied Managed resource to
 	// reflect the state of the external resource. Status modifications are
 	// automatically persisted unless ResourceLateInitialized is true - see
 	// ResourceLateInitialized for more detail.
-	Observe(ctx context.Context, mg resource.Managed) (ExternalObservation, error)
+	Observe(ctx context.Context, mg managedType) (ExternalObservation, error)
 
 	// Create an external resource per the specifications of the supplied
 	// Managed resource. Called when Observe reports that the associated
 	// external resource does not exist. Create implementations may update
 	// managed resource annotations, and those updates will be persisted.
 	// All other updates will be discarded.
-	Create(ctx context.Context, mg resource.Managed) (ExternalCreation, error)
+	Create(ctx context.Context, mg managedType) (ExternalCreation, error)
 
 	// Update the external resource represented by the supplied Managed
 	// resource, if necessary. Called unless Observe reports that the
 	// associated external resource is up to date.
-	Update(ctx context.Context, mg resource.Managed) (ExternalUpdate, error)
+	Update(ctx context.Context, mg managedType) (ExternalUpdate, error)
 
 	// Delete the external resource upon deletion of its associated Managed
 	// resource. Called when the managed resource has been deleted.
-	Delete(ctx context.Context, mg resource.Managed) (ExternalDelete, error)
+	Delete(ctx context.Context, mg managedType) (ExternalDelete, error)
 
 	// Disconnect from the provider and close the ExternalClient.
 	// Called at the end of reconcile loop. An ExternalClient not requiring
@@ -350,40 +383,44 @@ type ExternalClient interface {
 
 // ExternalClientFns are a series of functions that satisfy the ExternalClient
 // interface.
-type ExternalClientFns struct {
-	ObserveFn    func(ctx context.Context, mg resource.Managed) (ExternalObservation, error)
-	CreateFn     func(ctx context.Context, mg resource.Managed) (ExternalCreation, error)
-	UpdateFn     func(ctx context.Context, mg resource.Managed) (ExternalUpdate, error)
-	DeleteFn     func(ctx context.Context, mg resource.Managed) (ExternalDelete, error)
+type ExternalClientFns = TypedExternalClientFns[resource.Managed]
+
+// TypedExternalClientFns are a series of functions that satisfy the
+// ExternalClient interface.
+type TypedExternalClientFns[managed resource.Managed] struct {
+	ObserveFn    func(ctx context.Context, mg managed) (ExternalObservation, error)
+	CreateFn     func(ctx context.Context, mg managed) (ExternalCreation, error)
+	UpdateFn     func(ctx context.Context, mg managed) (ExternalUpdate, error)
+	DeleteFn     func(ctx context.Context, mg managed) (ExternalDelete, error)
 	DisconnectFn func(ctx context.Context) error
 }
 
 // Observe the external resource the supplied Managed resource represents, if
 // any.
-func (e ExternalClientFns) Observe(ctx context.Context, mg resource.Managed) (ExternalObservation, error) {
+func (e TypedExternalClientFns[managed]) Observe(ctx context.Context, mg managed) (ExternalObservation, error) {
 	return e.ObserveFn(ctx, mg)
 }
 
 // Create an external resource per the specifications of the supplied Managed
 // resource.
-func (e ExternalClientFns) Create(ctx context.Context, mg resource.Managed) (ExternalCreation, error) {
+func (e TypedExternalClientFns[managed]) Create(ctx context.Context, mg managed) (ExternalCreation, error) {
 	return e.CreateFn(ctx, mg)
 }
 
 // Update the external resource represented by the supplied Managed resource, if
 // necessary.
-func (e ExternalClientFns) Update(ctx context.Context, mg resource.Managed) (ExternalUpdate, error) {
+func (e TypedExternalClientFns[managed]) Update(ctx context.Context, mg managed) (ExternalUpdate, error) {
 	return e.UpdateFn(ctx, mg)
 }
 
 // Delete the external resource upon deletion of its associated Managed
 // resource.
-func (e ExternalClientFns) Delete(ctx context.Context, mg resource.Managed) (ExternalDelete, error) {
+func (e TypedExternalClientFns[managed]) Delete(ctx context.Context, mg managed) (ExternalDelete, error) {
 	return e.DeleteFn(ctx, mg)
 }
 
 // Disconnect the external client.
-func (e ExternalClientFns) Disconnect(ctx context.Context) error {
+func (e TypedExternalClientFns[managed]) Disconnect(ctx context.Context) error {
 	return e.DisconnectFn(ctx)
 }
 
@@ -644,6 +681,16 @@ func WithExternalConnecter(c ExternalConnecter) ReconcilerOption {
 	}
 }
 
+// WithTypedExternalConnector specifies how the Reconciler should connect to the API
+// used to sync and delete external resources.
+func WithTypedExternalConnector[managed resource.Managed](c TypedExternalConnecter[managed]) ReconcilerOption {
+	return func(r *Reconciler) {
+		r.external.ExternalConnectDisconnecter = &typedExternalConnectDisconnecterWrapper[managed]{
+			c: NewTypedNopDisconnecter(c),
+		}
+	}
+}
+
 // WithExternalConnectDisconnecter specifies how the Reconciler should connect and disconnect to the API
 // used to sync and delete external resources.
 //
@@ -654,6 +701,16 @@ func WithExternalConnectDisconnecter(c ExternalConnectDisconnecter) ReconcilerOp
 	}
 }
 
+// WithTypedExternalConnectDisconnecter specifies how the Reconciler should connect and disconnect to the API
+// used to sync and delete external resources.
+//
+// Deprecated: Please use Disconnect() on the ExternalClient for disconnecting from the provider.
+func WithTypedExternalConnectDisconnecter[managed resource.Managed](c TypedExternalConnectDisconnecter[managed]) ReconcilerOption {
+	return func(r *Reconciler) {
+		r.external.ExternalConnectDisconnecter = &typedExternalConnectDisconnecterWrapper[managed]{c}
+	}
+}
+
 // WithCriticalAnnotationUpdater specifies how the Reconciler should update a
 // managed resource's critical annotations. Implementations typically contain
 // some kind of retry logic to increase the likelihood that critical annotations

diff --git a/pkg/reconciler/managed/reconciler_test.go b/pkg/reconciler/managed/reconciler_test.go
@@ -1252,6 +1252,50 @@ func TestReconciler(t *testing.T) {
 			},
 			want: want{result: reconcile.Result{RequeueAfter: defaultPollInterval}},
 		},
+		"TypedReconcilerUpdateSuccessful": {
+			reason: "A successful managed resource update should trigger a requeue after a long wait.",
+			args: args{
+				m: &fake.Manager{
+					Client: &test.MockClient{
+						MockGet: test.NewMockGetFn(nil),
+						MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
+							want := &fake.Managed{}
+							want.SetConditions(xpv1.ReconcileSuccess())
+							if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
+								reason := "A successful managed resource update should be reported as a conditioned status."
+								t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
+							}
+							return nil
+						}),
+					},
+					Scheme: fake.SchemeWith(&fake.Managed{}),
+				},
+				mg: resource.ManagedKind(fake.GVK(&fake.Managed{})),
+				o: []ReconcilerOption{
+					WithInitializers(),
+					WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
+					WithTypedExternalConnector(TypedExternalConnectorFn[*fake.Managed](func(_ context.Context, _ *fake.Managed) (TypedExternalClient[*fake.Managed], error) {
+						c := &TypedExternalClientFns[*fake.Managed]{
+							ObserveFn: func(_ context.Context, _ *fake.Managed) (ExternalObservation, error) {
+								return ExternalObservation{ResourceExists: true, ResourceUpToDate: false}, nil
+							},
+							UpdateFn: func(_ context.Context, _ *fake.Managed) (ExternalUpdate, error) {
+								return ExternalUpdate{}, nil
+							},
+							DisconnectFn: func(_ context.Context) error {
+								return nil
+							},
+						}
+						return c, nil
+					})),
+					WithConnectionPublishers(),
+					WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
+				},
+			},
+			want: want{
+				result: reconcile.Result{RequeueAfter: defaultPollInterval},
+			},
+		},
 		"ReconciliationPausedSuccessful": {
 			reason: `If a managed resource has the pause annotation with value "true", there should be no further requeue requests.`,
 			args: args{

diff --git a/pkg/reconciler/managed/reconciler_typed.go b/pkg/reconciler/managed/reconciler_typed.go
@@ -0,0 +1,73 @@
+package managed
+
+import (
+	"context"
+
+	"github.com/crossplane/crossplane-runtime/pkg/errors"
+	"github.com/crossplane/crossplane-runtime/pkg/resource"
+)
+
+const errFmtUnexpectedObjectType = "unexpected object type %T"
+
+// typedExternalConnectDisconnecterWrapper wraps a TypedExternalConnecter to a
+// common ExternalConnector.
+type typedExternalConnectDisconnecterWrapper[managed resource.Managed] struct {
+	c TypedExternalConnectDisconnecter[managed]
+}
+
+func (c *typedExternalConnectDisconnecterWrapper[managed]) Connect(ctx context.Context, mg resource.Managed) (ExternalClient, error) {
+	cr, ok := mg.(managed)
+	if !ok {
+		return nil, errors.Errorf(errFmtUnexpectedObjectType, mg)
+	}
+	external, err := c.c.Connect(ctx, cr)
+	if err != nil {
+		return nil, err
+	}
+	return &typedExternalClientWrapper[managed]{c: external}, nil
+}
+
+func (c *typedExternalConnectDisconnecterWrapper[managed]) Disconnect(ctx context.Context) error {
+	return c.c.Disconnect(ctx)
+}
+
+// typedExternalClientWrapper wraps a TypedExternalClient to a common
+// ExternalClient.
+type typedExternalClientWrapper[managed resource.Managed] struct {
+	c TypedExternalClient[managed]
+}
+
+func (c *typedExternalClientWrapper[managed]) Observe(ctx context.Context, mg resource.Managed) (ExternalObservation, error) {
+	cr, ok := mg.(managed)
+	if !ok {
+		return ExternalObservation{}, errors.Errorf(errFmtUnexpectedObjectType, mg)
+	}
+	return c.c.Observe(ctx, cr)
+}
+
+func (c *typedExternalClientWrapper[managed]) Create(ctx context.Context, mg resource.Managed) (ExternalCreation, error) {
+	cr, ok := mg.(managed)
+	if !ok {
+		return ExternalCreation{}, errors.Errorf(errFmtUnexpectedObjectType, mg)
+	}
+	return c.c.Create(ctx, cr)
+}
+func (c *typedExternalClientWrapper[managed]) Update(ctx context.Context, mg resource.Managed) (ExternalUpdate, error) {
+	cr, ok := mg.(managed)
+	if !ok {
+		return ExternalUpdate{}, errors.Errorf(errFmtUnexpectedObjectType, mg)
+	}
+	return c.c.Update(ctx, cr)
+}
+
+func (c *typedExternalClientWrapper[managed]) Delete(ctx context.Context, mg resource.Managed) (ExternalDelete, error) {
+	cr, ok := mg.(managed)
+	if !ok {
+		return ExternalDelete{}, errors.Errorf(errFmtUnexpectedObjectType, mg)
+	}
+	return c.c.Delete(ctx, cr)
+}
+
+func (c *typedExternalClientWrapper[managed]) Disconnect(ctx context.Context) error {
+	return c.c.Disconnect(ctx)
+}