From 7210299e069a7b75949fb464d4f0559aa53241ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Rose?= Date: Mon, 17 Aug 2026 08:01:48 +0000 Subject: [PATCH 1/3] Log when a related resource's origin object can't be found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveRelatedResourceObjectsInNamespaces silently swallowed a NotFound when fetching a related resource's origin object, with the comment assuming it could only happen in a narrow List/Get race. It also fires permanently whenever a related resource's object.template/reference/ selector computes a name that never matches a real object - a config bug, not a race - and in that case every reconcile of the primary object hits it again, silently, while still reporting success. Threads the existing *zap.SugaredLogger through resolveRelatedResourceObjects and resolveRelatedResourceObjectsInNamespaces (both call sites already had one available: processRelatedResource's own `log` param, and confirmOriginEmpty's `s.log`) and logs the related resource's identifier plus the computed namespace/name when this happens, so a misconfigured related resource is diagnosable from the agent's own logs instead of only from its source. Signed-off-by: René Rose --- internal/sync/syncer_related.go | 27 ++++++++++++++++++++------- internal/sync/syncer_related_test.go | 2 +- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 650da52..df48b9f 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -88,7 +88,7 @@ func (s *ResourceSyncer) processRelatedResource(ctx context.Context, log *zap.Su policy := relRes.EffectiveCleanupPolicy() // find the all objects on the origin side that match the given criteria - resolvedObjects, err := resolveRelatedResourceObjects(ctx, origin, dest, relRes) + resolvedObjects, err := resolveRelatedResourceObjects(ctx, log, origin, dest, relRes) if err != nil { return false, fmt.Errorf("failed to get resolve origin objects: %w", err) } @@ -479,7 +479,7 @@ func (s *ResourceSyncer) confirmOriginEmpty(ctx context.Context, origin, dest sy object: origin.object, } - resolved, err := resolveRelatedResourceObjects(ctx, liveOrigin, dest, relRes) + resolved, err := resolveRelatedResourceObjects(ctx, s.log, liveOrigin, dest, relRes) if err != nil { return false, true, err } @@ -495,7 +495,7 @@ type resolvedObject struct { destination types.NamespacedName } -func resolveRelatedResourceObjects(ctx context.Context, relatedOrigin, relatedDest syncSide, relRes syncagentv1alpha1.RelatedResourceSpec) ([]resolvedObject, error) { +func resolveRelatedResourceObjects(ctx context.Context, log *zap.SugaredLogger, relatedOrigin, relatedDest syncSide, relRes syncagentv1alpha1.RelatedResourceSpec) ([]resolvedObject, error) { // resolving the originNamespace first allows us to scope down any .List() calls later originNamespace := relatedOrigin.object.GetNamespace() destNamespace := relatedDest.object.GetNamespace() @@ -529,7 +529,7 @@ func resolveRelatedResourceObjects(ctx context.Context, relatedOrigin, relatedDe // this related resource configuration. Again, for label selectors this can be multiple, // otherwise at most 1. - objects, err := resolveRelatedResourceObjectsInNamespaces(ctx, relatedOrigin, relatedDest, relRes, relRes.Object.RelatedResourceObjectSpec, namespaceMap) + objects, err := resolveRelatedResourceObjectsInNamespaces(ctx, log, relatedOrigin, relatedDest, relRes, relRes.Object.RelatedResourceObjectSpec, namespaceMap) if err != nil { return nil, fmt.Errorf("failed to resolve objects: %w", err) } @@ -630,7 +630,7 @@ func mapSlices(a, b []string) map[string]string { return mapping } -func resolveRelatedResourceObjectsInNamespaces(ctx context.Context, relatedOrigin, relatedDest syncSide, relRes syncagentv1alpha1.RelatedResourceSpec, spec syncagentv1alpha1.RelatedResourceObjectSpec, namespaceMap map[string]string) ([]resolvedObject, error) { +func resolveRelatedResourceObjectsInNamespaces(ctx context.Context, log *zap.SugaredLogger, relatedOrigin, relatedDest syncSide, relRes syncagentv1alpha1.RelatedResourceSpec, spec syncagentv1alpha1.RelatedResourceObjectSpec, namespaceMap map[string]string) ([]resolvedObject, error) { result := []resolvedObject{} for originNamespace, destNamespace := range namespaceMap { @@ -653,9 +653,22 @@ func resolveRelatedResourceObjectsInNamespaces(ctx context.Context, relatedOrigi err = relatedOrigin.client.Get(ctx, types.NamespacedName{Name: originName, Namespace: originNamespace}, originObj) if err != nil { - // this should rarely happen, only if an object was deleted in between the .List() call - // above and the .Get() call here. + // This should rarely happen, only if an object was deleted in between the .List() + // call above and the .Get() call here. It can also happen permanently if a + // misconfigured object.template/reference/selector computes a name that never + // matches a real object - in that case this branch is hit on every single + // reconcile, forever, with the primary object's reconcile otherwise reporting + // success. Log it so that case is diagnosable without reading the source. if apierrors.IsNotFound(err) { + if log != nil { + log.Debugw( + "Origin object for related resource not found, skipping", + "identifier", relRes.Identifier, + "namespace", originNamespace, + "name", originName, + ) + } + continue } diff --git a/internal/sync/syncer_related_test.go b/internal/sync/syncer_related_test.go index ac04285..2935a60 100644 --- a/internal/sync/syncer_related_test.go +++ b/internal/sync/syncer_related_test.go @@ -636,7 +636,7 @@ func TestResolveRelatedResourceObjects(t *testing.T) { Object: testcase.objectSpec, } - foundObjects, err := resolveRelatedResourceObjects(t.Context(), originSide, destSide, pubRes) + foundObjects, err := resolveRelatedResourceObjects(t.Context(), zap.NewNop().Sugar(), originSide, destSide, pubRes) if err != nil { t.Fatalf("Failed to resolve related objects: %v", err) } From 9ccd5c5d1f17236fce432690a5465ea8abd21fa0 Mon Sep 17 00:00:00 2001 From: Rene Rose Date: Fri, 28 Aug 2026 07:46:20 +0200 Subject: [PATCH 2/3] Update syncer_related.go Remove log nil check Signed-off-by: Rene Rose --- internal/sync/syncer_related.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index df48b9f..c3d8a80 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -659,15 +659,13 @@ func resolveRelatedResourceObjectsInNamespaces(ctx context.Context, log *zap.Sug // matches a real object - in that case this branch is hit on every single // reconcile, forever, with the primary object's reconcile otherwise reporting // success. Log it so that case is diagnosable without reading the source. - if apierrors.IsNotFound(err) { - if log != nil { + if apierrors.IsNotFound(err) { log.Debugw( "Origin object for related resource not found, skipping", "identifier", relRes.Identifier, "namespace", originNamespace, "name", originName, - ) - } + ) continue } From 4f0d54a7c5815b67c8f55e3a57ef054017158fd4 Mon Sep 17 00:00:00 2001 From: Rene Rose Date: Mon, 31 Aug 2026 06:51:46 +0000 Subject: [PATCH 3/3] Fix indentation of related-resource not-found log statement The block added in the previous commit was not gofmt-clean: trailing whitespace after the if brace and on the closing paren, and the log.Debugw call indented one level too deep. This failed both 'make lint' (.golangci.yml enables the gofmt formatter) and hack/ci/verify.sh (make imports re-formats the file, producing a diff). Whitespace only, no behaviour change. Signed-off-by: Rene Rose --- internal/sync/syncer_related.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index c3d8a80..39327b2 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -659,13 +659,13 @@ func resolveRelatedResourceObjectsInNamespaces(ctx context.Context, log *zap.Sug // matches a real object - in that case this branch is hit on every single // reconcile, forever, with the primary object's reconcile otherwise reporting // success. Log it so that case is diagnosable without reading the source. - if apierrors.IsNotFound(err) { - log.Debugw( - "Origin object for related resource not found, skipping", - "identifier", relRes.Identifier, - "namespace", originNamespace, - "name", originName, - ) + if apierrors.IsNotFound(err) { + log.Debugw( + "Origin object for related resource not found, skipping", + "identifier", relRes.Identifier, + "namespace", originNamespace, + "name", originName, + ) continue }