fix(attribution)!: partition author facts by declared audit route

api/v1alpha3/audit_route_test.go

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha3
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// TestClusterProvider_AuditRoute covers the defaulting rule the whole feature rests on: an unset
+// auditRoute resolves to the provider's own name, which is what every install already partitions its
+// facts by, so adding the field changes nothing until someone sets it.
+func TestClusterProvider_AuditRoute(t *testing.T) {
+	tests := []struct {
+		name     string
+		provider ClusterProvider
+		want     string
+	}{
+		{
+			name:     "no attribution block falls back to the object name",
+			provider: ClusterProvider{ObjectMeta: metav1.ObjectMeta{Name: "prod-eu-1"}},
+			want:     "prod-eu-1",
+		},
+		{
+			name: "an empty auditRoute falls back to the object name",
+			provider: ClusterProvider{
+				ObjectMeta: metav1.ObjectMeta{Name: "prod-eu-1"},
+				Spec:       ClusterProviderSpec{Attribution: &ClusterProviderAttribution{}},
+			},
+			want: "prod-eu-1",
+		},
+		{
+			name: "a declared route wins over the object name",
+			provider: ClusterProvider{
+				ObjectMeta: metav1.ObjectMeta{Name: "tenant-acme-delegating"},
+				Spec: ClusterProviderSpec{
+					Attribution: &ClusterProviderAttribution{AuditRoute: "prod-eu-1"},
+				},
+			},
+			want: "prod-eu-1",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, tt.provider.AuditRoute())
+		})
+	}
+}
+
+// TestClusterProvider_AuditRoute_SharedBySeveralProviders is the reported bug, stated as an API
+// invariant. An API server has one audit webhook backend and posts under one route, so two
+// ClusterProviders naming one physical cluster can only both resolve authors if they agree on that
+// route. Before this field they could not: each read under its own name, and every commit through
+// the provider the API server did NOT post under was authored attribution-unresolved.
+//
+// Locality is deliberately not involved. Both providers here omit kubeConfig, which is what makes
+// them in-cluster, and it is the declared route rather than that fact which joins them.
+func TestClusterProvider_AuditRoute_SharedBySeveralProviders(t *testing.T) {
+	fedByTheAPIServer := ClusterProvider{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
+	delegating := ClusterProvider{
+		ObjectMeta: metav1.ObjectMeta{Name: "srcns-delegating"},
+		Spec: ClusterProviderSpec{
+			AllowSourceNamespaceOverride: true,
+			Attribution:                  &ClusterProviderAttribution{AuditRoute: "default"},
+		},
+	}
+
+	assert.True(t, fedByTheAPIServer.IsInCluster())
+	assert.True(t, delegating.IsInCluster())
+	assert.Equal(t, fedByTheAPIServer.AuditRoute(), delegating.AuditRoute(),
+		"two providers on one cluster must read one partition, or only the routed one is attributed")
+
+	// A provider on a genuinely different cluster keeps its own partition, so the join can never
+	// cross-credit. This is what a declared route buys that keying by object UID alone would not:
+	// an etcd-snapshot clone reproduces object UIDs exactly, and only a human-chosen name separates
+	// the copy from its origin.
+	clone := ClusterProvider{ObjectMeta: metav1.ObjectMeta{Name: "prod-restored"}}
+	assert.NotEqual(t, fedByTheAPIServer.AuditRoute(), clone.AuditRoute())
+}

api/v1alpha3/clusterprovider_types.go

@@ -10,8 +10,8 @@ import (
 // DefaultClusterProviderName is the conventionally opinionated ClusterProvider name that an
 // omitted GitTarget.spec.clusterProviderRef points at. That defaulting is its ONLY special
 // behavior: it is an ordinary user-created object that may omit kubeConfig (the operator's own
-// in-cluster config) or set it to mirror a remote cluster, it is existence-gated on its
-// /audit-webhook/default route like every other name, and it is never created by the operator.
+// in-cluster config) or set it to mirror a remote cluster, its audit route defaults to its name like
+// every other provider's, and it is never created by the operator.
 const DefaultClusterProviderName = "default"
 
 // ClusterProviderReference references the cluster-scoped ClusterProvider a GitTarget sources
@@ -118,6 +118,35 @@ type ClusterProviderSpec struct {
 	// +optional
 	// +kubebuilder:validation:Minimum=1
 	Burst *int32 `json:"burst,omitempty"`
+
+	// Attribution groups this cluster's author-attribution settings. The block is spelled
+	// "attribution" rather than "authorAttribution" even though the operator flags are
+	// --author-attribution-*: the prefix groups a flat flag namespace, and a block on a
+	// source-cluster object already supplies that scope.
+	// +optional
+	Attribution *ClusterProviderAttribution `json:"attribution,omitempty"`
+}
+
+// ClusterProviderAttribution holds the per-cluster author-attribution settings. It exists as a
+// block so later per-cluster knobs (grace, mode) have a home beside auditRoute.
+type ClusterProviderAttribution struct {
+	// AuditRoute is the route this cluster's audit events arrive on. The sender is the API server's
+	// webhook backend (https://kubernetes.io/docs/tasks/debug/debug-cluster/audit/#webhook-backend),
+	// and this is the <name> segment its configured URL ends in: /audit-webhook/<name>. When several
+	// logical clusters share one backend it is instead the value of the audit-event annotation named
+	// by --author-attribution-audit-route-annotation-key.
+	//
+	// It partitions the attribution facts, so two ClusterProviders carrying the same route read one
+	// cluster's facts, and two carrying different routes can never cross-credit an author.
+	//
+	// Empty means metadata.name. Set it when several ClusterProviders name one cluster, since an API
+	// server has one webhook backend and so posts under one route: every other provider for that
+	// cluster must be pointed at the same route or it resolves no authors at all. Set it also when
+	// the events are labelled for something other than this object, such as a kcp logical cluster.
+	// +optional
+	// +kubebuilder:validation:MaxLength=253
+	// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
+	AuditRoute string `json:"auditRoute,omitempty"`
 }
 
 // ClusterProviderStatus defines the observed state of ClusterProvider.
@@ -150,8 +179,10 @@ type ClusterProviderStatus struct {
 // ClusterProvider is the cluster-scoped, read-side peer of GitProvider: it names a SOURCE cluster a
 // GitTarget mirrors FROM, and owns that cluster's connectivity credential (spec.kubeConfig),
 // namespace-access authorization (spec.allowedNamespaces), and per-cluster status. Its NAME is the
-// cluster's identity everywhere — the /audit-webhook/<name> ingress route and the attribution
-// fact-index key. No name is special: "default" is merely what an omitted
+// cluster's identity for the watch data plane, and the DEFAULT for its audit route: attribution
+// facts are partitioned by spec.attribution.auditRoute, which falls back to this name. Several
+// providers may name one cluster by declaring one route, which is what an API server with a single
+// audit webhook backend requires. No name is special: "default" is merely what an omitted
 // GitTarget.spec.clusterProviderRef points at, and it may just as well name a remote cluster, since
 // in-cluster-ness follows from spec.kubeConfig (omitted = in-cluster) rather than from the name.
 //
@@ -191,6 +222,23 @@ func (p *ClusterProvider) IsInCluster() bool {
 	return p.Spec.KubeConfig == nil
 }
 
+// AuditRoute is the identity this provider's attribution facts are keyed under: the route its
+// cluster's audit events arrive on, defaulting to the provider's own name. It is resolved through
+// this method rather than read off the field so no caller ever handles the empty case, the same
+// shape as (api/v1alpha3).GitTarget.SourceCluster().
+//
+// The default is what makes this change invisible to an existing install: one ClusterProvider per
+// cluster already partitions its facts by name, so an unset field resolves exactly what it always
+// resolved. Deliberately NOT conditional on locality: defaulting an in-cluster provider to the
+// literal "default" would make that name reserved for the local cluster again, a rule this project
+// enforced with CEL and then reversed before shipping (docs/finished/multi-cluster-author-attribution.md).
+func (p *ClusterProvider) AuditRoute() string {
+	if p.Spec.Attribution == nil || p.Spec.Attribution.AuditRoute == "" {
+		return p.Name
+	}
+	return p.Spec.Attribution.AuditRoute
+}
+
 // AllowsNamespace reports whether a namespace (by name and labels) may reference this provider
 // from a GitTarget, per spec.allowedNamespaces. It is DENY-BY-DEFAULT: a provider with no
 // allowedNamespaces policy (neither names nor selector) admits no namespace. Names and selector

api/v1alpha3/gittarget_types.go

@@ -318,9 +318,10 @@ type GitTarget struct {
 // SourceCluster is the identity the watch data plane keys a GitTarget's source cluster on: the
 // referenced ClusterProvider's NAME. It defaults to "default" when clusterProviderRef is unset —
 // so a source-cluster-unaware caller still gets a concrete, non-empty name, and there is no ""
-// sentinel. That name is a convention, not a claim about which physical cluster it is. The name is
-// the cluster's identity everywhere: the fact-index key, the GVK→GVR registry key, and the
-// /audit-webhook/<name> route.
+// sentinel. That name is a convention, not a claim about which physical cluster it is. The name
+// keys the GVK→GVR registry and the watch context. It is NOT the attribution partition: facts are
+// keyed by the referenced ClusterProvider's AuditRoute(), which defaults to this name but may
+// differ when several providers share one cluster's single audit stream.
 func (g *GitTarget) SourceCluster() string {
 	if g.Spec.ClusterProviderRef == nil || g.Spec.ClusterProviderRef.Name == "" {
 		return DefaultClusterProviderName

charts/gitops-reverser/README.md

@@ -194,7 +194,7 @@ nodeSelector:
 | `attribution.enabled` | Run audit ingress and name mirrored-resource commit authors from matching kube-apiserver audit facts | `false` |
 | `attribution.ttl` | How long an attribution fact is retained waiting for the matching watch event to join it | `10m` |
 | `attribution.grace` | Bounded per-event wait for a matching audit fact before a watch event ships as the committer | `3s` |
-| `attribution.clusterAnnotationKey` | Audit-event annotation naming the `ClusterProvider` each event belongs to. Empty keeps audit routes named (`/audit-webhook/<name>`). Set it only for a control plane emitting **one shared audit stream** for several logical clusters: it enables the bare `/audit-webhook`, which resolves the source cluster per event. An event with no annotation, or naming a provider that does not exist, is rejected (counted and logged) and never credited to a fallback | `""` |
+| `attribution.auditRouteAnnotationKey` | Audit-event annotation naming the **audit route** each event belongs to. Empty keeps audit routes named (`/audit-webhook/<audit-route>`). Set it only for a control plane emitting **one shared audit stream** for several logical clusters: it enables the bare `/audit-webhook`, which reads the route per event. A `ClusterProvider` joins a route via `spec.attribution.auditRoute` (default: its own name). An event with no annotation is rejected (counted and logged) and never credited to a fallback | `""` |
 | `clusterProvider.createDefault` | Render and own a `ClusterProvider` named `default` — the source cluster a `GitTarget` mirrors from when it omits `spec.clusterProviderRef`. The **operator never creates one**, so without this you commit the object yourself. Chart-owned: turning it off makes Helm delete the provider it created, and a `GitTarget` referencing a missing provider is held unready (`ClusterProviderNotFound`). The `quickstart` values never create one | `true` |
 | `clusterProvider.default.kubeConfig.secretRef.name` | Secret (release namespace) holding a kubeconfig for the rendered `default` provider. Empty means the operator's **own in-cluster** cluster; a name points `default` at a **remote** cluster instead — the name is a convention, not a claim about which cluster it is | `""` |
 | `clusterProvider.default.kubeConfig.secretRef.key` | Key within that Secret. Empty reads `value` then `value.yaml` | `""` |
@@ -238,7 +238,7 @@ See [`values.yaml`](values.yaml) for complete configuration options.
 When `attribution.enabled=true`, `https://<service>:9444/audit-webhook/<cluster-provider-name>`
 receives audit events from kube-apiserver — audit routes are **named**, including
 `/audit-webhook/default`. The bare `/audit-webhook` is rejected with **400** unless
-`attribution.clusterAnnotationKey` is set, which turns it into the shared-stream endpoint that
+`attribution.auditRouteAnnotationKey` is set, which turns it into the shared-stream endpoint that
 resolves each event's source cluster from that annotation. The operator extracts a minimal attribution fact from each (auditID, user, verb,
 resourceVersion, GVR, namespace, name, UID, status, timestamps) into the Redis attribution index
 (populated only when audit attribution is enabled). When a Redis endpoint is configured it also stores

charts/gitops-reverser/templates/deployment.yaml

@@ -83,8 +83,8 @@ spec:
             - --author-attribution={{ .Values.attribution.enabled }}
             - --author-attribution-ttl={{ .Values.attribution.ttl }}
             - --author-attribution-grace={{ .Values.attribution.grace }}
-            {{- if .Values.attribution.clusterAnnotationKey }}
-            - --author-attribution-cluster-annotation-key={{ .Values.attribution.clusterAnnotationKey }}
+            {{- if .Values.attribution.auditRouteAnnotationKey }}
+            - --author-attribution-audit-route-annotation-key={{ .Values.attribution.auditRouteAnnotationKey }}
             {{- end }}
             {{- if .Values.servers.admission.enabled }}
             - --admission-webhook

charts/gitops-reverser/values.schema.json

@@ -275,7 +275,7 @@
         "enabled": { "type": "boolean" },
         "ttl": { "$ref": "#/$defs/duration" },
         "grace": { "$ref": "#/$defs/duration" },
-        "clusterAnnotationKey": {
+        "auditRouteAnnotationKey": {
           "type": "string",
           "description": "Audit-event annotation naming the ClusterProvider each event belongs to. Empty (default) keeps audit routes named (/audit-webhook/<name>); set it only for one shared audit stream carrying several logical clusters, which enables the bare /audit-webhook endpoint."
         }

charts/gitops-reverser/values.yaml

@@ -213,14 +213,15 @@ attribution:
   # Bounded per-event wait for a matching audit fact before a watch event ships as the committer.
   # Larger values raise attribution hit-rate at the cost of commit latency.
   grace: "3s"
-  # Audit-event annotation naming the ClusterProvider each event belongs to. Audit routes are
-  # normally NAMED (/audit-webhook/<name>, including /audit-webhook/default), and this is empty.
+  # Audit-event annotation naming the AUDIT ROUTE each event belongs to. Audit routes are normally
+  # NAMED (/audit-webhook/<audit-route>, including /audit-webhook/default), and this is empty.
   # Set it only for a control plane that emits ONE shared audit stream for several logical
   # clusters: it enables the bare /audit-webhook endpoint, which reads this annotation from every
-  # event, so one batch may fan out to several source clusters. An event with no annotation, or
-  # naming a ClusterProvider that does not exist, is rejected (counted and logged) and never
-  # credited to a fallback. See docs/configuration.md.
-  clusterAnnotationKey: ""
+  # event, so one batch may fan out to several routes. A ClusterProvider joins a route by setting
+  # spec.attribution.auditRoute to the same value; it defaults to the provider's own name. An event
+  # with no annotation is rejected (counted and logged) and never credited to a fallback.
+  # See docs/configuration.md.
+  auditRouteAnnotationKey: ""
 
 # The source cluster a GitTarget mirrors FROM is a ClusterProvider, and a GitTarget that omits
 # spec.clusterProviderRef references one named "default". The operator NEVER creates a