fix: address review comments on cross-namespace PR

- Use nil-safe targetName variable in field export log line (comment #2)
- Use lookup-or-create pattern in setCrossNsOptInRequiredCondition to
  avoid duplicate conditions (comment #3)
- Return ownerNamespace instead of empty string on error in
  ValidateCrossNamespaceReference for defensive fail-closed behavior (comment #4)
- Update tests to match new return value on error path

Author: kprahulraj

pkg/runtime/field_export_reconciler.go

@@ -159,13 +159,17 @@ func (r *fieldExportReconciler) Sync(
 		return desired, r.onError(ctx, &desired, ackerr.NewTerminalError(nsErr))
 	}
 	if isCrossNamespace {
+		targetName := ""
+		if desired.Spec.To != nil && desired.Spec.To.Name != nil {
+			targetName = *desired.Spec.To.Name
+		}
 		r.log.V(0).Info(
 			"cross-namespace field export detected; this behavior will be "+
 				"disabled by default in a future release. Set --enable-cross-namespace "+
 				"to preserve this behavior.",
 			"fieldExportNamespace", desired.Namespace,
-			"targetNamespace", r.getTargetNamespace(&desired),
-			"targetName", *desired.Spec.To.Name,
+			"targetNamespace", resolvedNamespace,
+			"targetName", targetName,
 		)
 		r.setCrossNsOptInRequiredCondition(&desired)
 	}
@@ -235,6 +239,14 @@ func (r *fieldExportReconciler) setCrossNsOptInRequiredCondition(
 		desired.Namespace + "\" targets namespace \"" + r.getTargetNamespace(desired) +
 		"\". Cross-namespace behavior will require explicit opt-in in a future release. " +
 		"Set --enable-cross-namespace=true to preserve this behavior."
+	// Use lookup-or-create pattern to avoid duplicate conditions
+	for i, c := range desired.Status.Conditions {
+		if c.Type == ackv1alpha1.ConditionTypeCrossNamespaceOptInRequired {
+			desired.Status.Conditions[i].Status = corev1.ConditionTrue
+			desired.Status.Conditions[i].Message = &message
+			return
+		}
+	}
 	condition := &ackv1alpha1.Condition{
 		Type:    ackv1alpha1.ConditionTypeCrossNamespaceOptInRequired,
 		Status:  corev1.ConditionTrue,

pkg/runtime/resource_reference.go

@@ -58,7 +58,7 @@ func ValidateCrossNamespaceReference(
 	if enableCrossNamespace {
 		return *refNamespace, true, nil
 	}
-	return "", false, ackerr.ResourceReferenceCrossNamespaceNotAllowedFor(
+	return ownerNamespace, false, ackerr.ResourceReferenceCrossNamespaceNotAllowedFor(
 		ownerNamespace, *refNamespace, refName,
 	)
 }

pkg/runtime/resource_reference_test.go

@@ -104,7 +104,7 @@ func TestValidateCrossNamespaceReference(t *testing.T) {
 			name:            "different namespace, flag disabled",
 			flag:            false,
 			refNs:           strPtr(otherNs),
-			expectedNs:      "",
+			expectedNs:      ownerNs,
 			expectedCrossNs: false,
 			expectErr:       true,
 		},
@@ -211,7 +211,7 @@ func TestValidateCrossNamespaceReferenceString(t *testing.T) {
 			name:            "different namespace string, flag disabled",
 			flag:            false,
 			refNs:           otherNs,
-			expectedNs:      "",
+			expectedNs:      ownerNs,
 			expectedCrossNs: false,
 			expectErr:       true,
 		},