resolve Christoph's comments

Author: iakmc

internal/controller/sync/controller.go

@@ -36,6 +36,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
@@ -164,11 +165,13 @@ func Create(
 		return nil, fmt.Errorf("failed to setup local-side watch: %w", err)
 	}
 
-	// Watch origin:kcp related resources so that changes to them trigger reconciliation
-	// of the owning primary object. Only related resources with a Watch config are covered.
-	watchedGVKs := sets.New[schema.GroupVersionKind]()
+	// Watch related resources on their origin side so that changes to them trigger
+	// reconciliation of the owning primary object. Only related resources with a Watch
+	// config are covered. Deduplication is per-origin to allow the same GVK on both sides.
+	watchedKcpGVKs := sets.New[schema.GroupVersionKind]()
+	watchedServiceGVKs := sets.New[schema.GroupVersionKind]()
 	for _, relRes := range pubRes.Spec.Related {
-		if relRes.Origin != syncagentv1alpha1.RelatedResourceOriginKcp || relRes.Watch == nil {
+		if relRes.Watch == nil {
 			continue
 		}
 
@@ -178,57 +181,134 @@ func Create(
 			Resource: relRes.Resource,
 		}
 
-		// Use the local REST mapper to determine the Kind.
-		gvk, err := localManager.GetRESTMapper().KindFor(gvr)
-		if err != nil {
-			log.Warnw("Failed to determine Kind for origin:kcp related resource, skipping watch", "gvr", gvr, "error", err)
-			continue
+		// Use the REST mapper of the origin side: related resources may have projected GVKs
+		// that differ between kcp and the service cluster, so we must resolve using the
+		// mapper that actually knows about the GVR on that side.
+		var originRESTMapper meta.RESTMapper
+		if relRes.Origin == syncagentv1alpha1.RelatedResourceOriginKcp {
+			originRESTMapper = remoteManager.GetLocalManager().GetRESTMapper()
+		} else {
+			originRESTMapper = localManager.GetRESTMapper()
 		}
 
-		// Deduplicate: only set up one watch per GVK.
-		if watchedGVKs.Has(gvk) {
-			continue
+		gvk, err := originRESTMapper.KindFor(gvr)
+		if err != nil {
+			return nil, fmt.Errorf("failed to determine Kind for related resource %v (origin: %s): %w", gvr, relRes.Origin, err)
 		}
-		watchedGVKs.Insert(gvk)
 
 		relatedDummy := &unstructured.Unstructured{}
 		relatedDummy.SetGroupVersionKind(gvk)
 
-		var enqueueForRelated mchandler.TypedEventHandlerFunc[*unstructured.Unstructured, mcreconcile.Request]
+		if relRes.Origin == syncagentv1alpha1.RelatedResourceOriginKcp {
+			// Deduplicate: only set up one watch per GVK per origin side.
+			if watchedKcpGVKs.Has(gvk) {
+				continue
+			}
+			watchedKcpGVKs.Insert(gvk)
+
+			// The related resource lives in the kcp workspace; watch it via MultiClusterWatch.
+			var enqueueForRelated mchandler.TypedEventHandlerFunc[*unstructured.Unstructured, mcreconcile.Request]
+
+			switch {
+			case relRes.Watch.ByOwner != nil:
+				ownerGVK := remoteDummy.GroupVersionKind()
+				enqueueForRelated = func(clusterName string, _ cluster.Cluster) handler.TypedEventHandler[*unstructured.Unstructured, mcreconcile.Request] {
+					return &byOwnerEventHandler{
+						clusterName: clusterName,
+						ownerGVK:    ownerGVK,
+					}
+				}
 
-		switch {
-		case relRes.Watch.ByOwner != nil:
-			ownerKind := relRes.Watch.ByOwner.Kind
-			enqueueForRelated = func(clusterName string, _ cluster.Cluster) handler.TypedEventHandler[*unstructured.Unstructured, mcreconcile.Request] {
-				return &byOwnerEventHandler{
-					clusterName: clusterName,
-					ownerKind:   ownerKind,
+			case relRes.Watch.BySelector != nil:
+				labelSelector := relRes.Watch.BySelector
+				primaryDummy := remoteDummy.DeepCopy()
+				enqueueForRelated = func(clusterName string, cl cluster.Cluster) handler.TypedEventHandler[*unstructured.Unstructured, mcreconcile.Request] {
+					return &bySelectorEventHandler{
+						clusterName:   clusterName,
+						client:        cl.GetClient(),
+						primaryDummy:  primaryDummy,
+						labelSelector: labelSelector,
+						log:           log,
+					}
 				}
+
+			default:
+				return nil, fmt.Errorf("related resource %v (origin: %s) has Watch set but neither byOwner nor bySelector configured", gvk, relRes.Origin)
 			}
 
-		case relRes.Watch.ByLabel != nil:
-			labelTemplates := relRes.Watch.ByLabel
-			primaryDummy := remoteDummy.DeepCopy()
-			enqueueForRelated = func(clusterName string, cl cluster.Cluster) handler.TypedEventHandler[*unstructured.Unstructured, mcreconcile.Request] {
-				return &byLabelEventHandler{
-					clusterName:    clusterName,
-					client:         cl.GetClient(),
-					primaryDummy:   primaryDummy,
-					labelTemplates: labelTemplates,
-					log:            log,
+			if err := c.MultiClusterWatch(mcsource.TypedKind(relatedDummy, enqueueForRelated)); err != nil {
+				return nil, fmt.Errorf("failed to setup watch for kcp-origin related resource %v: %w", gvk, err)
+			}
+		} else {
+			// Deduplicate: only set up one watch per GVK per origin side.
+			if watchedServiceGVKs.Has(gvk) {
+				continue
+			}
+			watchedServiceGVKs.Insert(gvk)
+
+			// The related resource lives on the service cluster; watch it via the local manager.
+			// Map the changed related resource back to the remote (kcp) primary object by going
+			// through the local primary, which carries the remote cluster/name in its metadata.
+			localClient := localManager.GetClient()
+
+			var enqueueForRelated handler.TypedEventHandler[*unstructured.Unstructured, mcreconcile.Request]
+
+			switch {
+			case relRes.Watch.ByOwner != nil:
+				ownerGVK := localDummy.GroupVersionKind()
+				primaryDummy := localDummy.DeepCopy()
+				enqueueForRelated = handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, obj *unstructured.Unstructured) []mcreconcile.Request {
+					for _, ref := range obj.GetOwnerReferences() {
+						refGV, err := schema.ParseGroupVersion(ref.APIVersion)
+						if err != nil || refGV.Group != ownerGVK.Group || refGV.Version != ownerGVK.Version || ref.Kind != ownerGVK.Kind {
+							continue
+						}
+						localPrimary := primaryDummy.DeepCopy()
+						if err := localClient.Get(ctx, types.NamespacedName{Namespace: obj.GetNamespace(), Name: ref.Name}, localPrimary); err != nil {
+							log.Warnw("Failed to fetch local primary for byOwner watch", "owner", ref.Name, "error", err)
+							return nil
+						}
+						if req := sync.RemoteNameForLocalObject(localPrimary); req != nil {
+							return []mcreconcile.Request{*req}
+						}
+						return nil
+					}
+					return nil
+				})
+
+			case relRes.Watch.BySelector != nil:
+				selector, err := metav1.LabelSelectorAsSelector(relRes.Watch.BySelector)
+				if err != nil {
+					return nil, fmt.Errorf("failed to convert bySelector for service-origin related resource %v: %w", gvk, err)
 				}
+				primaryDummy := localDummy.DeepCopy()
+				enqueueForRelated = handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, _ *unstructured.Unstructured) []mcreconcile.Request {
+					primaryList := &unstructured.UnstructuredList{}
+					primaryList.SetAPIVersion(primaryDummy.GetAPIVersion())
+					primaryList.SetKind(primaryDummy.GetKind() + "List")
+					if err := localClient.List(ctx, primaryList, &ctrlruntimeclient.ListOptions{LabelSelector: selector}); err != nil {
+						log.Warnw("Failed to list local primary objects for bySelector watch", "selector", selector.String(), "error", err)
+						return nil
+					}
+					var reqs []mcreconcile.Request
+					for i := range primaryList.Items {
+						if req := sync.RemoteNameForLocalObject(&primaryList.Items[i]); req != nil {
+							reqs = append(reqs, *req)
+						}
+					}
+					return reqs
+				})
+
+			default:
+				return nil, fmt.Errorf("related resource %v (origin: %s) has Watch set but neither byOwner nor bySelector configured", gvk, relRes.Origin)
 			}
 
-		default:
-			log.Warnw("origin:kcp related resource has Watch set but neither byOwner nor byLabel configured, skipping", "gvk", gvk)
-			continue
-		}
-
-		if err := c.MultiClusterWatch(mcsource.TypedKind(relatedDummy, enqueueForRelated)); err != nil {
-			return nil, fmt.Errorf("failed to setup watch for origin:kcp related resource %v: %w", gvk, err)
+			if err := c.Watch(source.TypedKind(localManager.GetCache(), relatedDummy, enqueueForRelated)); err != nil {
+				return nil, fmt.Errorf("failed to setup watch for service-origin related resource %v: %w", gvk, err)
+			}
 		}
 
-		log.Infow("Set up watch for origin:kcp related resource", "gvk", gvk)
+		log.Infow("Set up watch for related resource", "gvk", gvk, "origin", relRes.Origin)
 	}
 
 	log.Info("Done setting up unmanaged controller.")

internal/controller/sync/related_handlers.go

@@ -18,12 +18,13 @@ package sync
 
 import (
 	"context"
+	"fmt"
 
 	"go.uber.org/zap"
 
-	"github.com/kcp-dev/api-syncagent/internal/sync/templating"
-
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/workqueue"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -33,10 +34,10 @@ import (
 )
 
 // byOwnerEventHandler enqueues the primary object by inspecting the OwnerReferences
-// of the changed related object and finding one with the configured Kind.
+// of the changed related object and finding one matching the configured GVK.
 type byOwnerEventHandler struct {
 	clusterName string
-	ownerKind   string
+	ownerGVK    schema.GroupVersionKind
 }
 
 func (h *byOwnerEventHandler) Create(_ context.Context, evt event.TypedCreateEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
@@ -57,7 +58,11 @@ func (h *byOwnerEventHandler) Generic(_ context.Context, evt event.TypedGenericE
 
 func (h *byOwnerEventHandler) enqueue(obj *unstructured.Unstructured, q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
 	for _, ref := range obj.GetOwnerReferences() {
-		if ref.Kind == h.ownerKind {
+		refGV, err := schema.ParseGroupVersion(ref.APIVersion)
+		if err != nil {
+			continue
+		}
+		if refGV.Group == h.ownerGVK.Group && refGV.Version == h.ownerGVK.Version && ref.Kind == h.ownerGVK.Kind {
 			q.Add(mcreconcile.Request{
 				ClusterName: h.clusterName,
 				Request: reconcile.Request{
@@ -72,60 +77,46 @@ func (h *byOwnerEventHandler) enqueue(obj *unstructured.Unstructured, q workqueu
 	}
 }
 
-// byLabelEventHandler enqueues primary objects by evaluating label templates against
-// the changed related object and listing primaries matching the resulting label selector.
-type byLabelEventHandler struct {
-	clusterName    string
-	client         ctrlruntimeclient.Client
-	primaryDummy   *unstructured.Unstructured
-	labelTemplates map[string]string
-	log            *zap.SugaredLogger
+// bySelectorEventHandler enqueues primary objects by listing primaries matching the configured
+// label selector whenever a related object changes.
+type bySelectorEventHandler struct {
+	clusterName   string
+	client        ctrlruntimeclient.Client
+	primaryDummy  *unstructured.Unstructured
+	labelSelector *metav1.LabelSelector
+	log           *zap.SugaredLogger
 }
 
-func (h *byLabelEventHandler) Create(ctx context.Context, evt event.TypedCreateEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
+func (h *bySelectorEventHandler) Create(ctx context.Context, evt event.TypedCreateEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
 	h.enqueue(ctx, evt.Object, q)
 }
 
-func (h *byLabelEventHandler) Update(ctx context.Context, evt event.TypedUpdateEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
+func (h *bySelectorEventHandler) Update(ctx context.Context, evt event.TypedUpdateEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
 	h.enqueue(ctx, evt.ObjectNew, q)
 }
 
-func (h *byLabelEventHandler) Delete(ctx context.Context, evt event.TypedDeleteEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
+func (h *bySelectorEventHandler) Delete(ctx context.Context, evt event.TypedDeleteEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
 	h.enqueue(ctx, evt.Object, q)
 }
 
-func (h *byLabelEventHandler) Generic(ctx context.Context, evt event.TypedGenericEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
+func (h *bySelectorEventHandler) Generic(ctx context.Context, evt event.TypedGenericEvent[*unstructured.Unstructured], q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
 	h.enqueue(ctx, evt.Object, q)
 }
 
-func (h *byLabelEventHandler) enqueue(ctx context.Context, obj *unstructured.Unstructured, q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
-	// Build the template context using the changed related object.
-	data := map[string]any{
-		"watchObject": map[string]any{
-			"name":      obj.GetName(),
-			"namespace": obj.GetNamespace(),
-			"labels":    obj.GetLabels(),
-		},
-	}
-
-	// Evaluate each label template to build the selector.
-	matchingLabels := ctrlruntimeclient.MatchingLabels{}
-	for key, tpl := range h.labelTemplates {
-		value, err := templating.Render(tpl, data)
-		if err != nil {
-			h.log.Warnw("Failed to evaluate byLabel template", "key", key, "template", tpl, "error", err)
-			return
-		}
-		matchingLabels[key] = value
+func (h *bySelectorEventHandler) enqueue(ctx context.Context, _ *unstructured.Unstructured, q workqueue.TypedRateLimitingInterface[mcreconcile.Request]) {
+	selector, err := metav1.LabelSelectorAsSelector(h.labelSelector)
+	if err != nil {
+		h.log.Warnw("Failed to convert bySelector selector", "error", err)
+		return
 	}
 
-	// List primary objects matching the derived label selector.
+	// List primary objects matching the label selector.
 	primaryList := &unstructured.UnstructuredList{}
 	primaryList.SetAPIVersion(h.primaryDummy.GetAPIVersion())
 	primaryList.SetKind(h.primaryDummy.GetKind() + "List")
 
-	if err := h.client.List(ctx, primaryList, matchingLabels); err != nil {
-		h.log.Warnw("Failed to list primary objects for byLabel watch", "selector", matchingLabels, "error", err)
+	if err := h.client.List(ctx, primaryList, &ctrlruntimeclient.ListOptions{LabelSelector: selector}); err != nil {
+		h.log.Warnw("Failed to list primary objects for bySelector watch", "selector", fmt.Sprintf("%v", selector), "error", err)
 		return
 	}
 

sdk/apis/syncagent/v1alpha1/published_resource.go

@@ -266,27 +266,27 @@ type RelatedResourceSpec struct {
 
 // RelatedResourceWatch configures how the watch handler maps a changed related resource
 // back to its owning primary object.
-// Exactly one of ByOwner or ByLabel must be set.
+// Exactly one of ByOwner or BySelector must be set.
+// +kubebuilder:validation:XValidation:rule="has(self.byOwner) != has(self.bySelector)",message="exactly one of byOwner or bySelector must be set"
 type RelatedResourceWatch struct {
 	// ByOwner configures the watch handler to inspect the OwnerReferences of the changed
 	// object. When an OwnerReference with the given Kind is found, the referenced owner
 	// is enqueued as the primary object.
 	// +optional
 	ByOwner *RelatedResourceWatchByOwner `json:"byOwner,omitempty"`
 
-	// ByLabel configures the watch handler to list primary objects matching a label selector
-	// derived from the changed object. Each map key is a label key on the primary object;
-	// each value is a Go template expression evaluated with the changed object available as
-	// .watchObject (with fields .name, .namespace, .labels).
+	// BySelector configures the watch handler to list primary objects matching the given label
+	// selector. When a related object changes, all primary objects matching this selector
+	// are enqueued for reconciliation.
 	// +optional
-	ByLabel map[string]string `json:"byLabel,omitempty"`
+	BySelector *metav1.LabelSelector `json:"bySelector,omitempty"`
 }
 
 // RelatedResourceWatchByOwner configures reverse lookup via OwnerReferences.
-type RelatedResourceWatchByOwner struct {
-	// Kind is the Kind to look for in the OwnerReferences of the changed related object.
-	Kind string `json:"kind"`
-}
+// The agent already knows the GVK of the primary object, so no further configuration
+// is needed: when a related object changes, its OwnerReferences are inspected for a
+// reference whose Kind matches the primary object's Kind.
+type RelatedResourceWatchByOwner struct{}
 
 // RelatedResourceProjection describes how the source GVK of a related resource (i.e.
 // the GVK on the related resource's origin side) should be modified when an object