🐛 PartialMetadataObject should preserve GVK information

Currently we have a bug that GVK information is not passed into mapping
functions when using the OnlyMetadata option in the builder. This patch
adds a wrapper that preserves the GVK information when events come in.

Signed-off-by: Vince Prignano <vincepri@vmware.com>

Author: vincepri

pkg/builder/controller.go

@@ -199,47 +199,45 @@ func (blder *Builder) Build(r reconcile.Reconciler) (controller.Controller, erro
 	return blder.ctrl, nil
 }
 
-func (blder *Builder) project(obj client.Object, proj objectProjection) (client.Object, error) {
+func (blder *Builder) project(obj client.Object, hdl handler.EventHandler, proj objectProjection) (client.Object, handler.EventHandler, error) {
 	switch proj {
 	case projectAsNormal:
-		return obj, nil
+		return obj, hdl, nil
 	case projectAsMetadata:
 		metaObj := &metav1.PartialObjectMetadata{}
 		gvk, err := getGvk(obj, blder.mgr.GetScheme())
 		if err != nil {
-			return nil, fmt.Errorf("unable to determine GVK of %T for a metadata-only watch: %w", obj, err)
+			return nil, nil, fmt.Errorf("unable to determine GVK of %T for a metadata-only watch: %w", obj, err)
 		}
 		metaObj.SetGroupVersionKind(gvk)
-		return metaObj, nil
+		return metaObj, handler.WrapPartialObjectMetadataHandler(metaObj, hdl), err
 	default:
 		panic(fmt.Sprintf("unexpected projection type %v on type %T, should not be possible since this is an internal field", proj, obj))
 	}
 }
 
 func (blder *Builder) doWatch() error {
 	// Reconcile type
-	typeForSrc, err := blder.project(blder.forInput.object, blder.forInput.objectProjection)
+	typeForSrc, hdler, err := blder.project(blder.forInput.object, &handler.EnqueueRequestForObject{}, blder.forInput.objectProjection)
 	if err != nil {
 		return err
 	}
 	src := &source.Kind{Type: typeForSrc}
-	hdler := &handler.EnqueueRequestForObject{}
 	allPredicates := append(blder.globalPredicates, blder.forInput.predicates...)
 	if err := blder.ctrl.Watch(src, hdler, allPredicates...); err != nil {
 		return err
 	}
 
 	// Watches the managed types
 	for _, own := range blder.ownsInput {
-		typeForSrc, err := blder.project(own.object, own.objectProjection)
+		typeForSrc, hdler, err := blder.project(own.object, &handler.EnqueueRequestForOwner{
+			OwnerType:    blder.forInput.object,
+			IsController: true,
+		}, own.objectProjection)
 		if err != nil {
 			return err
 		}
 		src := &source.Kind{Type: typeForSrc}
-		hdler := &handler.EnqueueRequestForOwner{
-			OwnerType:    blder.forInput.object,
-			IsController: true,
-		}
 		allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
 		allPredicates = append(allPredicates, own.predicates...)
 		if err := blder.ctrl.Watch(src, hdler, allPredicates...); err != nil {
@@ -252,16 +250,19 @@ func (blder *Builder) doWatch() error {
 		allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
 		allPredicates = append(allPredicates, w.predicates...)
 
+		watchHandler := w.eventhandler
+
 		// If the source of this watch is of type *source.Kind, project it.
 		if srckind, ok := w.src.(*source.Kind); ok {
-			typeForSrc, err := blder.project(srckind.Type, w.objectProjection)
+			typeForSrc, hdl, err := blder.project(srckind.Type, w.eventhandler, w.objectProjection)
 			if err != nil {
 				return err
 			}
 			srckind.Type = typeForSrc
+			watchHandler = hdl
 		}
 
-		if err := blder.ctrl.Watch(w.src, w.eventhandler, allPredicates...); err != nil {
+		if err := blder.ctrl.Watch(w.src, watchHandler, allPredicates...); err != nil {
 			return err
 		}
 	}

pkg/builder/controller_test.go

@@ -36,6 +36,7 @@ import (
 
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/config/v1alpha1"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -409,6 +410,9 @@ var _ = Describe("application", func() {
 				Watches(&source.Kind{Type: &appsv1.StatefulSet{}},
 					handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {
 						ometa := o.(*metav1.PartialObjectMetadata)
+						if _, err := apiutil.GVKForObject(o, mgr.GetScheme()); err != nil {
+							Expect(err).NotTo(HaveOccurred())
+						}
 						statefulSetMaps <- ometa
 						return nil
 					}),
@@ -583,9 +587,9 @@ func doReconcileTest(ctx context.Context, nameSuffix string, blder *Builder, mgr
 	Expect(err).NotTo(HaveOccurred())
 
 	By("Waiting for the ReplicaSet Reconcile")
-	Eventually(ch).Should(Receive(Equal(reconcile.Request{
-		NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})))
-
+	rec := reconcile.Request{
+		NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}}
+	Eventually(ch).Should(Receive(Equal(rec)), rec.String())
 }
 
 var _ runtime.Object = &fakeType{}

pkg/handler/metadata_wrapper.go

@@ -0,0 +1,75 @@
+package handler
+
+import (
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/util/workqueue"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
+)
+
+type metadataOnlyWrapper struct {
+	gvk     schema.GroupVersionKind
+	handler EventHandler
+}
+
+func (e *metadataOnlyWrapper) restoreGVK(o client.Object) {
+	pom := o.(*metav1.PartialObjectMetadata)
+	pom.SetGroupVersionKind(e.gvk)
+}
+
+// Create implements EventHandler
+func (e *metadataOnlyWrapper) Create(evt event.CreateEvent, q workqueue.RateLimitingInterface) {
+	e.restoreGVK(evt.Object)
+	e.handler.Create(evt, q)
+}
+
+// Update implements EventHandler
+func (e *metadataOnlyWrapper) Update(evt event.UpdateEvent, q workqueue.RateLimitingInterface) {
+	e.restoreGVK(evt.ObjectOld)
+	e.restoreGVK(evt.ObjectNew)
+	e.handler.Update(evt, q)
+}
+
+// Delete implements EventHandler
+func (e *metadataOnlyWrapper) Delete(evt event.DeleteEvent, q workqueue.RateLimitingInterface) {
+	e.restoreGVK(evt.Object)
+	e.handler.Delete(evt, q)
+}
+
+// Generic implements EventHandler
+func (e *metadataOnlyWrapper) Generic(evt event.GenericEvent, q workqueue.RateLimitingInterface) {
+	e.restoreGVK(evt.Object)
+	e.handler.Generic(evt, q)
+}
+
+var _ inject.Scheme = &metadataOnlyWrapper{}
+
+func (e *metadataOnlyWrapper) InjectScheme(s *runtime.Scheme) error {
+	if i, ok := e.handler.(inject.Scheme); ok {
+		return i.InjectScheme(s)
+	}
+	return nil
+}
+
+var _ inject.Mapper = &metadataOnlyWrapper{}
+
+func (e *metadataOnlyWrapper) InjectMapper(m meta.RESTMapper) error {
+	if i, ok := e.handler.(inject.Mapper); ok {
+		return i.InjectMapper(m)
+	}
+	return nil
+}
+
+// WrapPartialObjectMetadataHandler wraps an event handler for partial metadata objects
+// which restores the GVK for PartialObjectMetadata objects which loses this information when
+// events come from the informer code.
+func WrapPartialObjectMetadataHandler(o *metav1.PartialObjectMetadata, hdl EventHandler) EventHandler {
+	return &metadataOnlyWrapper{
+		gvk:     o.GroupVersionKind(),
+		handler: hdl,
+	}
+}