fix(attribution): resolve the audit route from one ClusterProvider read

internal/controller/gittarget_controller.go

@@ -201,6 +201,13 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{RequeueAfter: RequeueSteadyInterval}, nil
 	}
 
+	// One read of the source ClusterProvider serves everything below it: the audit route captured on
+	// Declare and the ClusterProviderReady projection. Reading it twice invited the two to disagree.
+	sourceProvider, sourceProviderErr := r.resolveSourceClusterProvider(ctx, &target)
+	if sourceProviderErr != nil {
+		return ctrl.Result{}, sourceProviderErr
+	}
+
 	// Ensure watch-first data-plane streams exist, then project source and target readiness
 	// into the kstatus trio.
 	streamsSettling := false
@@ -219,7 +226,7 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			ctx,
 			gitDest,
 			target.SourceCluster(),
-			r.auditRouteFor(ctx, &target),
+			sourceProvider.AuditRoute(),
 			target.EffectivePruneMode(),
 			gitPathWasRefused,
 		); declareErr != nil {
@@ -260,7 +267,7 @@ func (r *GitTargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		sourceReach = r.EventRouter.WatchManager.SourceClusterReachable(target.SourceCluster())
 	}
 	providerStatus, providerReason, providerMessage := r.gitProviderReadiness(ctx, &target, providerNS)
-	cpStatus, cpReason, cpMessage := r.clusterProviderReadiness(ctx, &target)
+	cpStatus, cpReason, cpMessage := clusterProviderReadiness(sourceProvider)
 	r.projectSourceAndProvider(&target, sourceReach, providerStatus, providerReason, providerMessage,
 		cpStatus, cpReason, cpMessage)
 	streamsSettling = streamsSettling || sourceReach.State != "True" ||

internal/controller/gittarget_controller_test.go

@@ -1114,7 +1114,11 @@ var _ = Describe("GitTarget Controller Security", func() {
 			// Delete the secret to simulate accidental removal.
 			Expect(k8sClient.Delete(ctx, &firstSecret)).Should(Succeed())
 
-			// The secret watch should trigger re-reconciliation and recreate it.
+			// Recreation rides the PERIODIC requeue, not a watch: this controller deliberately runs
+			// no control-plane Secret watch (SetupWithManager explains why), so nothing enqueues the
+			// GitTarget when its age-key Secret disappears. The wait must therefore exceed a full
+			// RequeueStreamSettleInterval — with the shared 10s `timeout` it equalled one, so a
+			// deletion landing just after a reconcile lost the race by milliseconds.
 			Eventually(func(g Gomega) {
 				var recreated corev1.Secret
 				err := k8sClient.Get(ctx, secretKey, &recreated)
@@ -1123,7 +1127,7 @@ var _ = Describe("GitTarget Controller Security", func() {
 				g.Expect(ageKeyName).NotTo(BeEmpty())
 				g.Expect(string(ageKeyValue)).To(ContainSubstring("AGE-SECRET-KEY-"))
 				g.Expect(recreated.Annotations).To(HaveKey(encryptionSecretRecipientAnnoKey))
-			}, timeout, interval).Should(Succeed())
+			}, RequeueStreamSettleInterval+timeout, interval).Should(Succeed())
 
 			Expect(k8sClient.Delete(ctx, target)).Should(Succeed())
 			Expect(k8sClient.Delete(ctx, gitProvider)).Should(Succeed())

internal/controller/gittarget_source_cluster.go

@@ -71,50 +71,52 @@ const (
 	// the GitTarget, so one `kubectl get gittarget` shows whether the SOURCE cluster's provider is
 	// healthy — distinct from SourceClusterReachable (the data plane's runtime reach) and
 	// GitProviderReady (the destination). It follows the GitProviderReady contract: only an
-	// EXPLICIT Ready=False downgrades the GitTarget; a not-found or not-yet-reported provider is
-	// Unknown and does not (so a single-cluster install that has not yet installed the "default"
-	// provider is not held down).
+	// EXPLICIT Ready=False downgrades the GitTarget; a provider that has not reported readiness yet
+	// is Unknown and does not. A MISSING provider never reaches this projection at all — that is the
+	// Validated gate's hard denial (GitTargetReasonClusterProviderNotFound), not a readiness axis.
 	GitTargetConditionClusterProviderReady = "ClusterProviderReady"
 	// GitTargetReasonClusterProviderNotReady is the ClusterProviderReady=False/Unknown reason.
 	GitTargetReasonClusterProviderNotReady = "ClusterProviderNotReady"
 	// GitTargetReasonClusterProviderReady is the ClusterProviderReady=True reason.
 	GitTargetReasonClusterProviderReady = "ClusterProviderReady"
 )
 
-// auditRouteFor resolves the audit route this GitTarget's attribution facts are keyed under, from
-// the referenced ClusterProvider's spec.attribution.auditRoute. It is read here, on the control
-// plane, so the watch data plane can capture it on Declare and never needs a Kubernetes client of
-// its own to attribute an event.
+// resolveSourceClusterProvider reads the ClusterProvider this GitTarget mirrors through, ONCE per
+// reconcile, for the two consumers that need it: the audit route captured on Declare, and the
+// ClusterProviderReady projection. It runs AFTER the Validated gate, which already proved through
+// authz.GitTargetAdmitted that this provider exists and admits this namespace — so a failure here
+// means the provider vanished (or the apiserver blinked) mid-reconcile, and it is returned as an
+// error so the reconcile requeues.
 //
-// An unreadable provider falls back to the provider NAME, which is exactly what AuditRoute()
-// defaults to. So a transient read failure resolves the same route a provider that sets nothing
-// would, and never a route that silently matches no facts.
-func (r *GitTargetReconciler) auditRouteFor(
+// Requeuing rather than guessing a route is the point. spec.attribution.auditRoute may be ANY
+// string, so a route inferred from the provider NAME is only correct for a provider that declares
+// none: guess it for a provider that declares `shared-route` and this target's streams are declared
+// under a partition the audit webhook never writes to. Attribution then resolves nobody — or, if
+// some other provider is NAMED `shared-route`, resolves that cluster's facts and writes a wrong
+// author into Git.
+func (r *GitTargetReconciler) resolveSourceClusterProvider(
 	ctx context.Context,
 	target *configbutleraiv1alpha3.GitTarget,
-) string {
+) (*configbutleraiv1alpha3.ClusterProvider, error) {
 	name := target.SourceCluster()
 	var cp configbutleraiv1alpha3.ClusterProvider
 	if err := r.Get(ctx, k8stypes.NamespacedName{Name: name}, &cp); err != nil {
-		return name
+		return nil, fmt.Errorf("read source ClusterProvider %q: %w", name, err)
 	}
-	return cp.AuditRoute()
+	return &cp, nil
 }
 
-// clusterProviderReadiness reads the referenced ClusterProvider's Ready condition and projects it,
-// mirroring gitProviderReadiness. It returns Unknown — which does NOT downgrade Ready — when the
-// provider cannot be observed (not found, or no Ready condition yet), so a not-yet-installed
-// "default" provider never blocks a local GitTarget; only an explicit Ready=False downgrades.
-func (r *GitTargetReconciler) clusterProviderReadiness(
-	ctx context.Context,
-	target *configbutleraiv1alpha3.GitTarget,
+// clusterProviderReadiness projects the referenced ClusterProvider's Ready condition, mirroring
+// gitProviderReadiness. It takes the provider resolveSourceClusterProvider already read rather than
+// reading its own, so the readiness reported and the audit route declared always describe the same
+// observation of the same object.
+//
+// Only an EXPLICIT Ready=False downgrades the GitTarget; a provider that has not reported readiness
+// yet is Unknown, which does not.
+func clusterProviderReadiness(
+	cp *configbutleraiv1alpha3.ClusterProvider,
 ) (metav1.ConditionStatus, string, string) {
-	name := target.SourceCluster()
-	var cp configbutleraiv1alpha3.ClusterProvider
-	if err := r.Get(ctx, k8stypes.NamespacedName{Name: name}, &cp); err != nil {
-		return metav1.ConditionUnknown, GitTargetReasonClusterProviderNotReady,
-			fmt.Sprintf("referenced ClusterProvider %q readiness not observed: %v", name, err)
-	}
+	name := cp.Name
 	c := findCondition(cp.Status.Conditions, ConditionTypeReady)
 	switch {
 	case c == nil:

internal/controller/gittarget_source_cluster_test.go

@@ -412,6 +412,9 @@ func TestClusterProviderReadiness_AllScenarios(t *testing.T) {
 		Message: "validating",
 	}
 
+	// The absent-provider case is deliberately NOT here: this projection runs only after the
+	// Validated gate admitted the target through an existing ClusterProvider, and it is handed the
+	// provider that gate's reconcile already read. A provider that cannot be read never reaches it.
 	tests := []struct {
 		name string
 		cp   *configbutleraiv1alpha3.ClusterProvider
@@ -420,7 +423,6 @@ func TestClusterProviderReadiness_AllScenarios(t *testing.T) {
 		{"ready", provider([]metav1.Condition{ready}), metav1.ConditionTrue},
 		{"not ready -> False (downgrades)", provider([]metav1.Condition{notReady}), metav1.ConditionFalse},
 		{"no condition -> Unknown (does not downgrade)", provider(nil), metav1.ConditionUnknown},
-		{"absent provider -> Unknown", nil, metav1.ConditionUnknown},
 		// Only an EXPLICIT Ready=False downgrades. A provider reporting Ready=Unknown is mid-flight,
 		// not broken, so it must not hold its GitTargets down.
 		{
@@ -431,18 +433,7 @@ func TestClusterProviderReadiness_AllScenarios(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			builder := fake.NewClientBuilder().WithScheme(scScheme(t))
-			if tc.cp != nil {
-				builder = builder.WithObjects(tc.cp)
-			}
-			r := &GitTargetReconciler{Client: builder.Build()}
-			target := &configbutleraiv1alpha3.GitTarget{
-				ObjectMeta: metav1.ObjectMeta{Name: "gt", Namespace: "team-a"},
-				Spec: configbutleraiv1alpha3.GitTargetSpec{
-					ClusterProviderRef: &configbutleraiv1alpha3.ClusterProviderReference{Name: "prod-eu-1"},
-				},
-			}
-			status, _, msg := r.clusterProviderReadiness(context.Background(), target)
+			status, _, msg := clusterProviderReadiness(tc.cp)
 			assert.Equal(t, tc.want, status)
 			assert.NotEmpty(t, msg)
 		})
@@ -494,14 +485,15 @@ func TestGitProviderReadiness_AllScenarios(t *testing.T) {
 	}
 }
 
-// TestAuditRouteFor covers how a GitTarget learns the audit route its author facts are keyed under.
-// The controller reads it here, on the control plane, so the watch data plane needs no Kubernetes
-// client of its own to attribute an event.
+// TestResolveSourceClusterProvider covers the single read that feeds both the audit route a
+// GitTarget's streams are declared under and its ClusterProviderReady projection.
 //
-// The fallback is the load-bearing case: an unreadable provider must resolve the provider NAME,
-// which is exactly what AuditRoute() defaults to. Resolving anything else would key the read to a
-// partition the audit handler never writes, which is the failure this whole change removes.
-func TestAuditRouteFor(t *testing.T) {
+// The load-bearing case is the unreadable provider: it must ERROR, so the reconcile requeues
+// without declaring. Resolving a route by falling back to the provider NAME is right only for a
+// provider that declares none — do it for one that declares `shared-route` and this target's
+// streams are keyed to a partition the audit handler never writes, which is exactly the silent
+// attribution loss this resolution exists to prevent.
+func TestResolveSourceClusterProvider(t *testing.T) {
 	const providerName = "srcns-delegating"
 
 	target := func() *configbutleraiv1alpha3.GitTarget {
@@ -520,38 +512,44 @@ func TestAuditRouteFor(t *testing.T) {
 	}
 
 	tests := []struct {
-		name    string
-		objects []client.Object
-		want    string
+		name      string
+		objects   []client.Object
+		wantRoute string
 	}{
 		{
 			name: "a declared route is used",
 			objects: []client.Object{
 				provider(&configbutleraiv1alpha3.ClusterProviderAttribution{AuditRoute: "default"}),
 			},
-			want: "default",
+			wantRoute: "default",
 		},
 		{
-			name:    "an empty attribution block falls back to the provider name",
-			objects: []client.Object{provider(&configbutleraiv1alpha3.ClusterProviderAttribution{})},
-			want:    providerName,
+			name:      "an empty attribution block resolves the provider name",
+			objects:   []client.Object{provider(&configbutleraiv1alpha3.ClusterProviderAttribution{})},
+			wantRoute: providerName,
 		},
 		{
-			name:    "no attribution block falls back to the provider name",
-			objects: []client.Object{provider(nil)},
-			want:    providerName,
-		},
-		{
-			name:    "an unreadable provider falls back to the provider name",
-			objects: nil,
-			want:    providerName,
+			name:      "no attribution block resolves the provider name",
+			objects:   []client.Object{provider(nil)},
+			wantRoute: providerName,
 		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			cl := fake.NewClientBuilder().WithScheme(scScheme(t)).WithObjects(tc.objects...).Build()
 			r := &GitTargetReconciler{Client: cl}
-			assert.Equal(t, tc.want, r.auditRouteFor(context.Background(), target()))
+			cp, err := r.resolveSourceClusterProvider(context.Background(), target())
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantRoute, cp.AuditRoute())
 		})
 	}
+
+	t.Run("an unreadable provider errors instead of guessing a route", func(t *testing.T) {
+		cl := fake.NewClientBuilder().WithScheme(scScheme(t)).Build()
+		r := &GitTargetReconciler{Client: cl}
+		cp, err := r.resolveSourceClusterProvider(context.Background(), target())
+		require.Error(t, err, "a guessed route silently mis-keys this target's attribution facts")
+		assert.Nil(t, cp)
+		assert.Contains(t, err.Error(), providerName)
+	})
 }