feature: sync related resource status independently of origin

deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml

@@ -807,17 +807,19 @@ spec:
                         type: string
                       syncStatus:
                         description: |-
-                          SyncStatus enables synchronization of the status subresource in the same direction as
-                          the spec (from the origin side to the destination side). When enabled, the agent will
-                          use the status subresource endpoint to update the destination object's status.
-                          This requires the related resource to have a status subresource configured in its CRD.
+                          SyncStatus configures the side from which the status subresource is synchronized. The
+                          agent uses the status subresource endpoint to update the object on the opposite side.
+                          Both representations of the related resource must have a status subresource configured.
 
-                            - origin: kcp -> status is synced from kcp to the service cluster
-                            - origin: service -> status is synced from the service cluster to kcp
+                            - kcp: status is synced from kcp to the service cluster
+                            - service: status is synced from the service cluster to kcp
 
-                          For origin: service, Watch must also be configured so that changes to the related
-                          resource's status trigger reconciliation and ensure the informer cache is populated.
-                        type: boolean
+                          Except when both Origin and SyncStatus are kcp, Watch must also be configured so that
+                          status changes trigger reconciliation and the required informer caches are populated.
+                        enum:
+                          - service
+                          - kcp
+                        type: string
                       version:
                         description: |-
                           Version is the API version of the related resource. This can be left blank to automatically
@@ -826,9 +828,9 @@ spec:
                       watch:
                         description: |-
                           Watch configures how the agent identifies the owning primary object when a related
-                          resource with origin: kcp changes. When set, the agent sets up a watch on the related
-                          resource type and uses the configured rule to enqueue the correct primary object.
-                          Without this field, changes to origin:kcp related resources do not trigger reconciliation.
+                          resource changes. The agent always watches the origin side. When SyncStatus selects the
+                          opposite side, the agent watches that side as well. Without this field, changes to related
+                          resources do not trigger reconciliation.
                         properties:
                           byOwner:
                             description: |-
@@ -901,8 +903,8 @@ spec:
                         rule: '!has(self.group) || (has(self.resource) && has(self.version))'
                       - message: identity hashes can only be used with GVRs
                         rule: '!has(self.identityHash) || (has(self.group) && has(self.version) && has(self.resource))'
-                      - message: watch must be configured when origin is service and syncStatus is true
-                        rule: '!(self.origin == ''service'' && has(self.syncStatus) && self.syncStatus) || has(self.watch)'
+                      - message: watch must be configured unless origin and syncStatus are both kcp
+                        rule: '!has(self.syncStatus) || (self.origin == ''kcp'' && self.syncStatus == ''kcp'') || has(self.watch)'
                       - message: watch must be configured when cleanupPolicy is MatchOrigin and origin is service
                         rule: '!(has(self.cleanupPolicy) && self.cleanupPolicy == ''MatchOrigin'' && self.origin == ''service'') || has(self.watch)'
                       - message: 'cleanup:true conflicts with cleanupPolicy: Orphan'

internal/controller/sync/controller.go

@@ -36,7 +36,6 @@ 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"
@@ -171,7 +170,7 @@ func Create(
 		return nil, fmt.Errorf("failed to setup local-side watch: %w", err)
 	}
 
-	if err := setupRelatedResourceWatches(c, localManager, remoteManager, pubRes, localDummy, remoteDummy, log); err != nil {
+	if err := setupRelatedResourceWatches(c, localManager, pubRes, localDummy, remoteDummy, log); err != nil {
 		return nil, err
 	}
 
@@ -180,86 +179,89 @@ func Create(
 	return c, nil
 }
 
-// setupRelatedResourceWatches sets up watches for all related resources that have a Watch
-// config, on their respective origin side, so that changes trigger primary reconciliation.
+// setupRelatedResourceWatches sets up watches for all related resources that have a Watch config.
+// The origin side is always watched. When status is synchronized from the opposite side, that side
+// is watched as well so changes in either direction trigger primary reconciliation.
 func setupRelatedResourceWatches(
 	c mccontroller.Controller,
 	localManager manager.Manager,
-	remoteManager mcmanager.Manager,
 	pubRes *syncagentv1alpha1.PublishedResource,
 	localDummy, remoteDummy *unstructured.Unstructured,
 	log *zap.SugaredLogger,
 ) error {
-	// Deduplication is per-origin to allow the same GVK on both sides.
-	watchedKcpGVKs := sets.New[schema.GroupVersionKind]()
+	// Deduplication is per-side to allow the same resource on both sides.
+	watchedKcpGVRs := sets.New[schema.GroupVersionResource]()
 	watchedServiceGVKs := sets.New[schema.GroupVersionKind]()
 
 	for _, relRes := range pubRes.Spec.Related {
 		if relRes.Watch == nil {
 			continue
 		}
 
-		// this handles the legacy .Kind-based related resources
-		gvr := projection.RelatedResourceGVR(&relRes)
-
-		// 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()
-		}
-
-		gvk, err := originRESTMapper.KindFor(gvr)
-		if err != nil {
-			return fmt.Errorf("failed to determine Kind for related resource %v (origin: %s): %w", gvr, relRes.Origin, err)
-		}
+		watchKcp := relRes.Origin == syncagentv1alpha1.RelatedResourceOriginKcp ||
+			(relRes.SyncStatus != nil && *relRes.SyncStatus == syncagentv1alpha1.RelatedResourceOriginKcp)
+		if watchKcp {
+			kcpGVR := projection.RelatedResourceKcpGVR(&relRes)
+			if !watchedKcpGVRs.Has(kcpGVR) {
+				enqueueForRelated, err := buildKcpRelatedHandler(relRes.Watch, kcpGVR, remoteDummy, log)
+				if err != nil {
+					return err
+				}
 
-		relatedDummy := &unstructured.Unstructured{}
-		relatedDummy.SetGroupVersionKind(gvk)
+				relatedSource := mcsource.TypedKind(&unstructured.Unstructured{}, enqueueForRelated).WithProjection(
+					func(cl cluster.Cluster, obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
+						gvk, err := cl.GetRESTMapper().KindFor(kcpGVR)
+						if err != nil {
+							return nil, fmt.Errorf("failed to determine Kind for kcp-side related resource %v: %w", kcpGVR, err)
+						}
+
+						projected := obj.DeepCopy()
+						projected.SetGroupVersionKind(gvk)
+						return projected, nil
+					},
+				)
+				if err := c.MultiClusterWatch(relatedSource); err != nil {
+					return fmt.Errorf("failed to setup watch for related resource %v on kcp side: %w", kcpGVR, err)
+				}
 
-		if relRes.Origin == syncagentv1alpha1.RelatedResourceOriginKcp {
-			if watchedKcpGVKs.Has(gvk) {
-				continue
+				watchedKcpGVRs.Insert(kcpGVR)
+				log.Infow("Set up watch for related resource", "gvr", kcpGVR, "side", syncagentv1alpha1.RelatedResourceOriginKcp)
 			}
-			watchedKcpGVKs.Insert(gvk)
+		}
 
-			enqueueForRelated, err := buildKcpRelatedHandler(relRes.Watch, gvk, remoteDummy, log)
+		watchService := relRes.Origin == syncagentv1alpha1.RelatedResourceOriginService ||
+			(relRes.SyncStatus != nil && *relRes.SyncStatus == syncagentv1alpha1.RelatedResourceOriginService)
+		if watchService {
+			serviceGVR := projection.RelatedResourceServiceGVK(&relRes)
+			gvk, err := localManager.GetRESTMapper().KindFor(serviceGVR)
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to determine Kind for service-side related resource %v: %w", serviceGVR, err)
 			}
+			if !watchedServiceGVKs.Has(gvk) {
+				relatedDummy := &unstructured.Unstructured{}
+				relatedDummy.SetGroupVersionKind(gvk)
 
-			if err := c.MultiClusterWatch(mcsource.TypedKind(relatedDummy, enqueueForRelated)); err != nil {
-				return fmt.Errorf("failed to setup watch for kcp-origin related resource %v: %w", gvk, err)
-			}
-		} else {
-			if watchedServiceGVKs.Has(gvk) {
-				continue
-			}
-			watchedServiceGVKs.Insert(gvk)
-
-			enqueueForRelated, err := buildServiceRelatedHandler(relRes.Watch, gvk, localDummy, localManager, log)
-			if err != nil {
-				return err
-			}
+				enqueueForRelated, err := buildServiceRelatedHandler(relRes.Watch, gvk, localDummy, localManager, log)
+				if err != nil {
+					return err
+				}
+				if err := c.Watch(source.TypedKind(localManager.GetCache(), relatedDummy, enqueueForRelated)); err != nil {
+					return fmt.Errorf("failed to setup watch for related resource %v on service side: %w", gvk, err)
+				}
 
-			if err := c.Watch(source.TypedKind(localManager.GetCache(), relatedDummy, enqueueForRelated)); err != nil {
-				return fmt.Errorf("failed to setup watch for service-origin related resource %v: %w", gvk, err)
+				watchedServiceGVKs.Insert(gvk)
+				log.Infow("Set up watch for related resource", "gvk", gvk, "side", syncagentv1alpha1.RelatedResourceOriginService)
 			}
 		}
-
-		log.Infow("Set up watch for related resource", "gvk", gvk, "origin", relRes.Origin)
 	}
 
 	return nil
 }
 
-// buildKcpRelatedHandler constructs the per-cluster event handler for a kcp-origin related resource.
+// buildKcpRelatedHandler constructs the per-cluster event handler for a kcp-side related resource.
 func buildKcpRelatedHandler(
 	watch *syncagentv1alpha1.RelatedResourceWatch,
-	gvk schema.GroupVersionKind,
+	gvr schema.GroupVersionResource,
 	remoteDummy *unstructured.Unstructured,
 	log *zap.SugaredLogger,
 ) (mchandler.TypedEventHandlerFunc[*unstructured.Unstructured, mcreconcile.Request], error) {
@@ -287,11 +289,11 @@ func buildKcpRelatedHandler(
 		}, nil
 
 	default:
-		return nil, fmt.Errorf("related resource %v (origin: kcp) has Watch set but neither byOwner nor bySelector configured", gvk)
+		return nil, fmt.Errorf("related resource %v on kcp side has Watch set but neither byOwner nor bySelector configured", gvr)
 	}
 }
 
-// buildServiceRelatedHandler constructs the event handler for a service-cluster-origin related resource.
+// buildServiceRelatedHandler constructs the event handler for a service-side related resource.
 // It maps the changed related resource back to the remote (kcp) primary via sync metadata on the local primary.
 func buildServiceRelatedHandler(
 	watch *syncagentv1alpha1.RelatedResourceWatch,
@@ -328,7 +330,7 @@ func buildServiceRelatedHandler(
 	case watch.BySelector != nil:
 		selector, err := metav1.LabelSelectorAsSelector(watch.BySelector)
 		if err != nil {
-			return nil, fmt.Errorf("failed to convert bySelector for service-origin related resource %v: %w", gvk, err)
+			return nil, fmt.Errorf("failed to convert bySelector for service-side related resource %v: %w", gvk, err)
 		}
 		primaryDummy := localDummy.DeepCopy()
 		return handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, _ *unstructured.Unstructured) []mcreconcile.Request {
@@ -349,7 +351,7 @@ func buildServiceRelatedHandler(
 		}), nil
 
 	default:
-		return nil, fmt.Errorf("related resource %v (origin: service) has Watch set but neither byOwner nor bySelector configured", gvk)
+		return nil, fmt.Errorf("related resource %v on service side has Watch set but neither byOwner nor bySelector configured", gvk)
 	}
 }
 

internal/sync/object_syncer.go

@@ -159,6 +159,12 @@ func (s *objectSyncer) Sync(ctx context.Context, log *zap.SugaredLogger, source,
 			return false, nil
 		}
 
+		// Status is removed from the SSA payload and must be synchronized through
+		// the status subresource separately.
+		if err := s.syncObjectStatusForward(ctx, log, source, newDest); err != nil {
+			return false, err
+		}
+
 		return s.syncObjectStatus(ctx, log, source, newDest)
 	}
 
@@ -399,7 +405,7 @@ func (s *objectSyncer) syncObjectStatusForward(ctx context.Context, log *zap.Sug
 			if apierrors.IsNotFound(err) {
 				// The /status subresource does not exist on the destination CRD.
 				// Retrying will not help; emit a warning so the user knows what to fix.
-				s.recordEvent(ctx, source, dest, corev1.EventTypeWarning, "StatusSubresourceMissing", "Cannot sync status: the destination resource does not have a status subresource. Set syncStatus: false or add subresources.status to the CRD.")
+				s.recordEvent(ctx, source, dest, corev1.EventTypeWarning, "StatusSubresourceMissing", "Cannot sync status: the destination resource does not have a status subresource. Unset syncStatus or add subresources.status to the CRD.")
 				return nil
 			}
 			return fmt.Errorf("failed to update destination object status: %w", err)

internal/sync/syncer_related.go

@@ -154,11 +154,13 @@ func (s *ResourceSyncer) processRelatedResource(ctx context.Context, log *zap.Su
 		deletionPolicies[relatedCopyKey(resolved.destination.Namespace, resolved.destination.Name)] = deletionPolicy
 
 		// When status sync is enabled, include "status" in subresources so it is stripped from
-		// the spec patch (avoiding a no-op write on resources that have a status subresource).
-		// The status is then separately written via the status subresource endpoint by syncStatusForward.
+		// the spec patch and written separately via the status subresource endpoint.
 		relatedSubresources := []string(nil)
-		if relRes.SyncStatus {
+		syncStatusForward, syncStatusBack := false, false
+		if relRes.SyncStatus != nil {
 			relatedSubresources = []string{"status"}
+			syncStatusForward = *relRes.SyncStatus == relRes.Origin
+			syncStatusBack = *relRes.SyncStatus != relRes.Origin
 		}
 
 		syncer := objectSyncer{
@@ -178,8 +180,8 @@ func (s *ResourceSyncer) processRelatedResource(ctx context.Context, log *zap.Su
 				return dest, nil
 			},
 			subresources:      relatedSubresources,
-			syncStatusBack:    false,
-			syncStatusForward: relRes.SyncStatus,
+			syncStatusBack:    syncStatusBack,
+			syncStatusForward: syncStatusForward,
 			// if the origin is on the remote side, we want to add a finalizer to make
 			// sure we can clean up properly; when forceDelete is enabled, we are in deletion mode and
 			// want to force the deletion, so we ignore the related object's origin (it was taken into

internal/sync/syncer_ssa_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/kcp-dev/logicalcluster/v3"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -180,6 +181,46 @@ func TestObjectSyncerFieldManager(t *testing.T) {
 	}
 }
 
+func TestObjectSyncerServerSideApplySyncsForwardStatus(t *testing.T) {
+	const readyPhase = "ready"
+
+	source := makeUnstructuredWithStatus("my-widget", "", map[string]interface{}{"phase": readyPhase})
+	dest := makeUnstructuredWithStatus("my-widget", "", map[string]interface{}{"phase": "pending"})
+	destClient := buildFakeClientWithStatus(dest)
+
+	syncer := &objectSyncer{
+		useServerSideApply: true,
+		syncStatusForward:  true,
+		subresources:       []string{"status"},
+		destCreator: func(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
+			return obj.DeepCopy(), nil
+		},
+		eventObjSide: syncSideSource,
+	}
+
+	ctx := WithEventRecorder(t.Context(), record.NewFakeRecorder(10))
+	if _, err := syncer.Sync(
+		ctx,
+		zap.NewNop().Sugar(),
+		syncSide{object: source},
+		syncSide{object: dest, client: destClient},
+	); err != nil {
+		t.Fatalf("Sync returned an error: %v", err)
+	}
+
+	got := makeUnstructuredWithStatus("my-widget", "", nil)
+	if err := destClient.Get(ctx, ctrlruntimeclient.ObjectKeyFromObject(dest), got); err != nil {
+		t.Fatalf("Failed to get destination object: %v", err)
+	}
+	phase, _, err := unstructured.NestedString(got.Object, "status", "phase")
+	if err != nil {
+		t.Fatalf("Failed to read destination status: %v", err)
+	}
+	if phase != readyPhase {
+		t.Errorf("Expected destination status.phase=ready, got %q", phase)
+	}
+}
+
 // TestSyncerServerSideApplyFieldOwner verifies that the syncer issues a
 // Server-Side Apply call with the expected FieldOwner and ForceOwnership
 // options and that the resulting object carries managedFields owned by the