Refactor reconcilers to use Knative ReconcileKind pattern

Move the key-splitting, resource-fetching, and not-found handling into a
generic internal.NewReconciler wrapper so that each reconciler only
implements ReconcileKind(ctx, *TypedResource) — matching the Knative
generated reconciler convention. The controller setup wraps the
reconciler with internal.NewReconciler, which bridges Reconcile(ctx, key)
to ReconcileKind(ctx, *T).

This removes duplicated boilerplate from all three reconcilers (push,
sync, resolver), eliminates their Reconcile/LeaderAware methods, and
makes splitKey an unexported implementation detail.

https://claude.ai/code/session_01Eu3LyxX3G1KCcw4aDzmHvp

pkg/reconciler/internal/key.go

@@ -2,9 +2,9 @@ package internal
 
 import "strings"
 
-// SplitKey splits a "namespace/name" key into its components.
+// splitKey splits a "namespace/name" key into its components.
 // If the key has no slash, the namespace is empty and the whole key is the name.
-func SplitKey(key string) (namespace, name string) {
+func splitKey(key string) (namespace, name string) {
 	namespace, name, _ = strings.Cut(key, "/")
 	if name == "" {
 		// No slash found: Cut returns (key, "", false). The key is the name.

pkg/reconciler/internal/key_test.go

@@ -39,7 +39,7 @@ func TestSplitKey(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.key, func(t *testing.T) {
-			ns, name := SplitKey(tt.key)
+			ns, name := splitKey(tt.key)
 			if ns != tt.wantNS {
 				t.Errorf("namespace = %q, want %q", ns, tt.wantNS)
 			}

pkg/reconciler/internal/reconcile.go

@@ -0,0 +1,67 @@
+package internal
+
+import (
+	"context"
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
+	"knative.dev/pkg/logging"
+	"knative.dev/pkg/reconciler"
+)
+
+// KindReconciler is the interface that a developer implements.
+// It mirrors the Knative generated reconciler pattern: the resource
+// has already been fetched and the developer only provides business logic.
+type KindReconciler[T any] interface {
+	ReconcileKind(ctx context.Context, o *T) reconciler.Event
+}
+
+// reconcilerImpl bridges between the controller's Reconcile(ctx, key) interface
+// and the developer's ReconcileKind(ctx, *T) method. It handles splitting the
+// workqueue key, fetching the typed resource, and ignoring not-found (deleted)
+// resources — exactly what Knative's generated reconcilers do.
+type reconcilerImpl[T any] struct {
+	get   func(ctx context.Context, namespace, name string) (*T, error)
+	inner KindReconciler[T]
+}
+
+// Verify interface conformance.
+var _ reconciler.LeaderAware = (*reconcilerImpl[any])(nil)
+
+// NewReconciler wraps a KindReconciler into a controller-compatible reconciler
+// that implements Reconcile(ctx, key) error. The get function fetches the typed
+// resource by namespace and name.
+func NewReconciler[T any](
+	get func(ctx context.Context, namespace, name string) (*T, error),
+	inner KindReconciler[T],
+) *reconcilerImpl[T] {
+	return &reconcilerImpl[T]{get: get, inner: inner}
+}
+
+// Reconcile implements the controller.Reconciler interface.
+func (r *reconcilerImpl[T]) Reconcile(ctx context.Context, key string) error {
+	logger := logging.FromContext(ctx)
+
+	namespace, name := splitKey(key)
+
+	o, err := r.get(ctx, namespace, name)
+	if apierrors.IsNotFound(err) {
+		logger.Debugf("%s/%s no longer exists", namespace, name)
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("getting %s/%s: %w", namespace, name, err)
+	}
+
+	return r.inner.ReconcileKind(ctx, o)
+}
+
+// Promote implements reconciler.LeaderAware.
+func (r *reconcilerImpl[T]) Promote(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error {
+	return nil
+}
+
+// Demote implements reconciler.LeaderAware.
+func (r *reconcilerImpl[T]) Demote(bkt reconciler.Bucket) {
+}

pkg/reconciler/push/controller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/dynamic/dynamicinformer"
@@ -15,6 +16,7 @@ import (
 
 	gitv1alpha1 "github.com/imjasonh/git-k8s/pkg/apis/git/v1alpha1"
 	gitclient "github.com/imjasonh/git-k8s/pkg/client"
+	"github.com/imjasonh/git-k8s/pkg/reconciler/internal"
 )
 
 var pushTransactionGVR = schema.GroupVersionResource{
@@ -45,7 +47,12 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
 		gitClient:     gitClient,
 	}
 
-	impl := controller.NewContext(ctx, r, controller.ControllerOptions{
+	impl := controller.NewContext(ctx, internal.NewReconciler(
+		func(ctx context.Context, namespace, name string) (*gitv1alpha1.GitPushTransaction, error) {
+			return gitClient.GitPushTransactions(namespace).Get(ctx, name, metav1.GetOptions{})
+		},
+		r,
+	), controller.ControllerOptions{
 		WorkQueueName: "GitPushTransactions",
 		Logger:        logger,
 	})
@@ -67,4 +74,3 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
 
 	return impl
 }
-

pkg/reconciler/push/push.go

@@ -11,17 +11,14 @@ import (
 	"github.com/go-git/go-git/v5/plumbing/transport/http"
 	"github.com/go-git/go-git/v5/storage/memory"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"knative.dev/pkg/logging"
 	"knative.dev/pkg/reconciler"
 
 	gitv1alpha1 "github.com/imjasonh/git-k8s/pkg/apis/git/v1alpha1"
 	gitclient "github.com/imjasonh/git-k8s/pkg/client"
-	"github.com/imjasonh/git-k8s/pkg/reconciler/internal"
 )
 
 // Reconciler implements the reconcile logic for GitPushTransaction.
@@ -31,20 +28,9 @@ type Reconciler struct {
 }
 
 // ReconcileKind processes a single GitPushTransaction resource.
-func (r *Reconciler) ReconcileKind(ctx context.Context, key string) reconciler.Event {
+func (r *Reconciler) ReconcileKind(ctx context.Context, txn *gitv1alpha1.GitPushTransaction) reconciler.Event {
 	logger := logging.FromContext(ctx)
-
+	namespace := txn.Namespace
-	namespace, name := internal.SplitKey(key)
-
-	// Fetch the GitPushTransaction.
-	txn, err := r.gitClient.GitPushTransactions(namespace).Get(ctx, name, metav1.GetOptions{})
-	if apierrors.IsNotFound(err) {
-		logger.Debugf("GitPushTransaction %s/%s no longer exists", namespace, name)
-		return nil
-	}
-	if err != nil {
-		return fmt.Errorf("getting GitPushTransaction %s/%s: %w", namespace, name, err)
-	}
 
 	// Skip if already terminal.
 	if txn.Status.Phase == gitv1alpha1.TransactionPhaseSucceeded ||
@@ -57,7 +43,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, key string) reconciler.E
 	now := metav1.Now()
 	txn.Status.Phase = gitv1alpha1.TransactionPhaseInProgress
 	txn.Status.StartTime = &now
-	txn, err = r.gitClient.GitPushTransactions(namespace).UpdateStatus(ctx, txn, metav1.UpdateOptions{})
+	txn, err := r.gitClient.GitPushTransactions(namespace).UpdateStatus(ctx, txn, metav1.UpdateOptions{})
 	if err != nil {
 		return fmt.Errorf("updating status to InProgress: %w", err)
 	}
@@ -95,7 +81,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, key string) reconciler.E
 		logger.Warnf("Failed to update branch CRs after push: %v", err)
 	}
 
-	logger.Infof("Successfully pushed transaction %s/%s", namespace, name)
+	logger.Infof("Successfully pushed transaction %s/%s", namespace, txn.Name)
 	return nil
 }
 
@@ -224,20 +210,3 @@ func (r *Reconciler) updateBranches(ctx context.Context, namespace string, txn *
 	}
 	return nil
 }
-
-// Ensure Reconciler implements the reconciler interface.
-var _ reconciler.LeaderAware = (*Reconciler)(nil)
-
-// Promote implements LeaderAware.
-func (r *Reconciler) Promote(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error {
-	return nil
-}
-
-// Demote implements LeaderAware.
-func (r *Reconciler) Demote(bkt reconciler.Bucket) {
-}
-
-// Reconcile implements the controller.Reconciler interface.
-func (r *Reconciler) Reconcile(ctx context.Context, key string) error {
-	return r.ReconcileKind(ctx, key)
-}

pkg/reconciler/resolver/controller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
@@ -16,6 +17,7 @@ import (
 
 	gitv1alpha1 "github.com/imjasonh/git-k8s/pkg/apis/git/v1alpha1"
 	gitclient "github.com/imjasonh/git-k8s/pkg/client"
+	"github.com/imjasonh/git-k8s/pkg/reconciler/internal"
 )
 
 var repoSyncGVR = schema.GroupVersionResource{
@@ -44,7 +46,12 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
 		gitClient:     gitClient,
 	}
 
-	impl := controller.NewContext(ctx, r, controller.ControllerOptions{
+	impl := controller.NewContext(ctx, internal.NewReconciler(
+		func(ctx context.Context, namespace, name string) (*gitv1alpha1.GitRepoSync, error) {
+			return gitClient.GitRepoSyncs(namespace).Get(ctx, name, metav1.GetOptions{})
+		},
+		r,
+	), controller.ControllerOptions{
 		WorkQueueName: "GitConflictResolver",
 		Logger:        logger,
 	})

pkg/reconciler/resolver/resolver.go

@@ -9,16 +9,13 @@ import (
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/storage/memory"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"knative.dev/pkg/logging"
 	"knative.dev/pkg/reconciler"
 
 	gitv1alpha1 "github.com/imjasonh/git-k8s/pkg/apis/git/v1alpha1"
 	gitclient "github.com/imjasonh/git-k8s/pkg/client"
-	"github.com/imjasonh/git-k8s/pkg/reconciler/internal"
 )
 
 // Reconciler implements the reconcile logic for resolving conflicted GitRepoSyncs.
@@ -27,28 +24,17 @@ type Reconciler struct {
 	gitClient     *gitclient.GitV1alpha1Client
 }
 
-// Reconcile implements the controller.Reconciler interface.
+// ReconcileKind processes a single GitRepoSync resource that is in the Conflicted phase.
-func (r *Reconciler) Reconcile(ctx context.Context, key string) error {
+func (r *Reconciler) ReconcileKind(ctx context.Context, syncObj *gitv1alpha1.GitRepoSync) reconciler.Event {
 	logger := logging.FromContext(ctx)
-
+	namespace := syncObj.Namespace
-	namespace, name := internal.SplitKey(key)
-
-	// Fetch the GitRepoSync.
-	syncObj, err := r.gitClient.GitRepoSyncs(namespace).Get(ctx, name, metav1.GetOptions{})
-	if apierrors.IsNotFound(err) {
-		logger.Debugf("GitRepoSync %s/%s no longer exists", namespace, name)
-		return nil
-	}
-	if err != nil {
-		return fmt.Errorf("getting GitRepoSync %s/%s: %w", namespace, name, err)
-	}
 
 	// Only process if still conflicted.
 	if syncObj.Status.Phase != gitv1alpha1.SyncPhaseConflicted {
 		return nil
 	}
 
-	logger.Infof("Attempting to resolve conflict for %s/%s", namespace, name)
+	logger.Infof("Attempting to resolve conflict for %s/%s", namespace, syncObj.Name)
 
 	// Get the repo URLs.
 	repoA, err := r.gitClient.GitRepositories(namespace).Get(ctx, syncObj.Spec.RepoA.Name, metav1.GetOptions{})
@@ -348,12 +334,3 @@ func (r *Reconciler) markManualIntervention(ctx context.Context, syncObj *gitv1a
 	_, err := r.gitClient.GitRepoSyncs(syncObj.Namespace).UpdateStatus(ctx, syncObj, metav1.UpdateOptions{})
 	return err
 }
-
-var _ reconciler.LeaderAware = (*Reconciler)(nil)
-
-func (r *Reconciler) Promote(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error {
-	return nil
-}
-
-func (r *Reconciler) Demote(bkt reconciler.Bucket) {
-}

pkg/reconciler/sync/controller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
@@ -16,6 +17,7 @@ import (
 
 	gitv1alpha1 "github.com/imjasonh/git-k8s/pkg/apis/git/v1alpha1"
 	gitclient "github.com/imjasonh/git-k8s/pkg/client"
+	"github.com/imjasonh/git-k8s/pkg/reconciler/internal"
 )
 
 var (
@@ -52,7 +54,12 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
 		gitClient:     gitClient,
 	}
 
-	impl := controller.NewContext(ctx, r, controller.ControllerOptions{
+	impl := controller.NewContext(ctx, internal.NewReconciler(
+		func(ctx context.Context, namespace, name string) (*gitv1alpha1.GitRepoSync, error) {
+			return gitClient.GitRepoSyncs(namespace).Get(ctx, name, metav1.GetOptions{})
+		},
+		r,
+	), controller.ControllerOptions{
 		WorkQueueName: "GitRepoSyncs",
 		Logger:        logger,
 	})

pkg/reconciler/sync/sync.go

@@ -7,16 +7,13 @@ import (
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/storage/memory"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"knative.dev/pkg/logging"
 	"knative.dev/pkg/reconciler"
 
 	gitv1alpha1 "github.com/imjasonh/git-k8s/pkg/apis/git/v1alpha1"
 	gitclient "github.com/imjasonh/git-k8s/pkg/client"
-	"github.com/imjasonh/git-k8s/pkg/reconciler/internal"
 )
 
 // Reconciler implements the reconcile logic for GitRepoSync.
@@ -25,21 +22,11 @@ type Reconciler struct {
 	gitClient     *gitclient.GitV1alpha1Client
 }
 
-// Reconcile implements the controller.Reconciler interface.
+// ReconcileKind processes a single GitRepoSync resource.
-func (r *Reconciler) Reconcile(ctx context.Context, key string) error {
+func (r *Reconciler) ReconcileKind(ctx context.Context, syncObj *gitv1alpha1.GitRepoSync) reconciler.Event {
 	logger := logging.FromContext(ctx)
-
+	namespace := syncObj.Namespace
-	namespace, name := internal.SplitKey(key)
+	name := syncObj.Name
-
-	// Fetch the GitRepoSync resource.
-	syncObj, err := r.gitClient.GitRepoSyncs(namespace).Get(ctx, name, metav1.GetOptions{})
-	if apierrors.IsNotFound(err) {
-		logger.Debugf("GitRepoSync %s/%s no longer exists", namespace, name)
-		return nil
-	}
-	if err != nil {
-		return fmt.Errorf("getting GitRepoSync %s/%s: %w", namespace, name, err)
-	}
 
 	// Get the branches for both repos.
 	branchA, err := r.findBranch(ctx, namespace, syncObj.Spec.RepoA.Name, syncObj.Spec.BranchName)
@@ -211,13 +198,3 @@ func (r *Reconciler) updateSyncStatus(ctx context.Context, syncObj *gitv1alpha1.
 	_, err := r.gitClient.GitRepoSyncs(syncObj.Namespace).UpdateStatus(ctx, syncObj, metav1.UpdateOptions{})
 	return err
 }
-
-// Ensure Reconciler implements the reconciler interface.
-var _ reconciler.LeaderAware = (*Reconciler)(nil)
-
-func (r *Reconciler) Promote(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error {
-	return nil
-}
-
-func (r *Reconciler) Demote(bkt reconciler.Bucket) {
-}