docs(types): make ResourceIdentifier.Key() a stated, tested contract

internal/git/content_writer.go

@@ -86,7 +86,7 @@ func (w *contentWriter) isSensitiveIdentifier(id types.ResourceIdentifier) bool
 
 func (w *contentWriter) encryptSensitiveContent(ctx context.Context, event Event, plain []byte) ([]byte, error) {
 	meta := buildResourceMeta(event)
-	identityKey := sensitiveIdentityKey(meta.Identifier)
+	identityKey := meta.Identifier.Key()
 	digest := sha256.Sum256(plain)
 	currentMarker := sensitiveMarker{
 		UID:             meta.UID,
@@ -173,7 +173,3 @@ func buildResourceMeta(event Event) resourceMeta {
 	meta.Generation = event.Object.GetGeneration()
 	return meta
 }
-
-func sensitiveIdentityKey(id types.ResourceIdentifier) string {
-	return fmt.Sprintf("%s/%s/%s/%s/%s", id.Group, id.Version, id.Resource, id.Namespace, id.Name)
-}

internal/types/identifier.go

@@ -33,10 +33,43 @@ func NewResourceIdentifier(group, version, resource, namespace, name string) Res
 
 // Key returns a stable, fully-qualified identifier suitable for map keys and deduplication.
 //
-// Format (namespaced): "group/version/resource/namespace/name"
-// Format (cluster-scoped): "group/version/resource/name"
+// The exact string is a public contract. Tools built around GitOps Reverser key their own
+// rows on this identity and join them against ours, so a consumer that cannot import this
+// package (it lives under internal/) reimplements the format from this comment. Changing
+// any byte of it is a breaking change rather than a refactor, and
+// TestResourceIdentifier_Key_GoldenFormat is the gate that turns such a change into a
+// decision instead of a silent split.
 //
-// For core resources, Group is empty and the key begins with "/" (e.g., "/v1/secrets/ns/name").
+//	namespaced:      "{group}/{version}/{resource}/{namespace}/{name}"
+//	cluster-scoped:  "{group}/{version}/{resource}/{name}"
+//
+// Two rules a reimplementation has to get right, and they pull in opposite directions:
+//
+//   - A cluster-scoped resource DROPS the namespace segment; it does not emit an empty one.
+//     Always joining five parts yields "…/clusterroles//admin", which never joins.
+//   - A core-group resource has an EMPTY group segment, which it does emit, so the key
+//     leads with "/".
+//
+// The four shapes, which are the four cases of the golden test:
+//
+//	apps/v1/deployments/prod/api                        namespaced, grouped
+//	rbac.authorization.k8s.io/v1/clusterroles/admin     cluster-scoped, grouped
+//	/v1/secrets/prod/db                                 namespaced, core group
+//	/v1/nodes/node-1                                    cluster-scoped, core group
+//
+// # Key versus ToGitPath: which one is "the same resource"
+//
+// Key includes Version and [ResourceIdentifier.ToGitPath] deliberately excludes it, so the
+// two disagree about whether a preferred-version bump is the same object. The decision,
+// recorded at both methods: Key is the API-side identity — correct for in-process map keys,
+// deduplication and logs, where every participant observes one version at a time — and the
+// versionless, namespace-first path is the DURABLE identity of the object, which is why a
+// storage-version bump moves no file in Git.
+//
+// A join that must survive a storage-version bump is therefore keyed on the versionless
+// identity, not on Key. Consumers holding rows across releases should drop the version
+// segment (the second) rather than treat "apps/v1/deployments/prod/api" and
+// "apps/v2/deployments/prod/api" as two resources.
 func (r ResourceIdentifier) Key() string {
 	if r.Namespace != "" {
 		return fmt.Sprintf("%s/%s/%s/%s/%s", r.Group, r.Version, r.Resource, r.Namespace, r.Name)
@@ -55,6 +88,12 @@ func (r ResourceIdentifier) Key() string {
 // existing document is always edited in place at its current location (match-first),
 // so changing this shape never moves a file that is already in Git. See
 // docs/spec/gittarget-new-file-placement-rules.md.
+//
+// That omitted version is the other half of the decision recorded at
+// [ResourceIdentifier.Key]: this versionless identity is the durable one — the object stays
+// the same object across a preferred-version bump — while Key is the API-side identity and
+// splits on that bump. Neither is wrong; they answer different questions, and a caller
+// joining data that outlives a release wants this one.
 func (r ResourceIdentifier) ToGitPath() string {
 	scope := r.Namespace
 	if scope == "" {

internal/types/identifier_key_test.go

@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package types
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestResourceIdentifier_Key_GoldenFormat pins the exact strings ResourceIdentifier.Key()
+// produces.
+//
+// THIS FORMAT IS DEPENDED ON ACROSS PRODUCT BOUNDARIES. Tools built around GitOps Reverser
+// key their own rows on this identity and join them against ours; because this package
+// lives under internal/ they cannot import it, so they reimplement the format from the
+// method's doc comment. Changing any byte of it is a BREAKING CHANGE rather than a
+// refactor. If this test goes red, the question is not "how do I update the fixture" but
+// "who is holding rows keyed on the old string, and how do they learn".
+//
+// The four cases below are the four shapes of the format, not four samples of it: the two
+// optional segments (group, namespace) are absent and present independently, and they
+// behave in opposite ways — an empty group is emitted (the key leads with "/") while an
+// empty namespace is dropped (the key has four segments, not five with a hole).
+func TestResourceIdentifier_Key_GoldenFormat(t *testing.T) {
+	tests := []struct {
+		name       string
+		identifier ResourceIdentifier
+		want       string
+	}{
+		{
+			name: "namespaced, grouped - all five segments present",
+			identifier: ResourceIdentifier{
+				Group:     "apps",
+				Version:   "v1",
+				Resource:  "deployments",
+				Namespace: "prod",
+				Name:      "api",
+			},
+			want: "apps/v1/deployments/prod/api",
+		},
+		{
+			// The namespace segment is DROPPED, not emitted empty: a reimplementation
+			// that always joins five parts produces "…/clusterroles//admin" and never
+			// joins against this.
+			name: "cluster-scoped, grouped - four segments, no empty namespace",
+			identifier: ResourceIdentifier{
+				Group:     "rbac.authorization.k8s.io",
+				Version:   "v1",
+				Resource:  "clusterroles",
+				Namespace: "",
+				Name:      "admin",
+			},
+			want: "rbac.authorization.k8s.io/v1/clusterroles/admin",
+		},
+		{
+			// The core group is empty and IS emitted, so the key has a leading "/".
+			// Opposite rule to the namespace above.
+			name: "namespaced, core group - leading slash",
+			identifier: ResourceIdentifier{
+				Group:     "",
+				Version:   "v1",
+				Resource:  "secrets",
+				Namespace: "prod",
+				Name:      "db",
+			},
+			want: "/v1/secrets/prod/db",
+		},
+		{
+			// The sharpest case: both optional segments are empty and each takes the
+			// other's rule. One degenerates to a leading "/", the other vanishes.
+			name: "cluster-scoped, core group - both rules at once",
+			identifier: ResourceIdentifier{
+				Group:     "",
+				Version:   "v1",
+				Resource:  "nodes",
+				Namespace: "",
+				Name:      "node-1",
+			},
+			want: "/v1/nodes/node-1",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, tt.identifier.Key(),
+				"Key() format is a cross-product contract; see this test's doc comment before updating it")
+		})
+	}
+}
+
+// TestResourceIdentifier_Key_DistinguishesResources guards the property the format exists
+// for: two identifiers that differ anywhere must not collide. The cluster-scoped case is
+// the one worth stating, since dropping a segment is exactly how a format loses injectivity.
+func TestResourceIdentifier_Key_DistinguishesResources(t *testing.T) {
+	// A cluster-scoped resource named "admin" and a namespaced one in a namespace
+	// called "admin": the dropped segment must not let these read as the same key.
+	clusterScoped := ResourceIdentifier{Group: "g", Version: "v1", Resource: "widgets", Name: "admin"}
+	namespaced := ResourceIdentifier{Group: "g", Version: "v1", Resource: "widgets", Namespace: "admin", Name: "x"}
+	assert.NotEqual(t, clusterScoped.Key(), namespaced.Key())
+
+	// Every field participates.
+	base := ResourceIdentifier{Group: "apps", Version: "v1", Resource: "deployments", Namespace: "prod", Name: "api"}
+	variants := map[string]ResourceIdentifier{
+		"group":     {Group: "batch", Version: "v1", Resource: "deployments", Namespace: "prod", Name: "api"},
+		"version":   {Group: "apps", Version: "v2", Resource: "deployments", Namespace: "prod", Name: "api"},
+		"resource":  {Group: "apps", Version: "v1", Resource: "statefulsets", Namespace: "prod", Name: "api"},
+		"namespace": {Group: "apps", Version: "v1", Resource: "deployments", Namespace: "stage", Name: "api"},
+		"name":      {Group: "apps", Version: "v1", Resource: "deployments", Namespace: "prod", Name: "web"},
+	}
+	for field, v := range variants {
+		assert.NotEqual(t, base.Key(), v.Key(), "identifiers differing in %s must not share a key", field)
+	}
+}
+
+// TestResourceIdentifier_Key_VersionSplitsWhereGitPathDoesNot pins the documented
+// disagreement between the two identity functions, so the decision recorded at both
+// methods cannot quietly stop being true: a preferred-version bump is a new Key and the
+// same Git path.
+func TestResourceIdentifier_Key_VersionSplitsWhereGitPathDoesNot(t *testing.T) {
+	v1 := ResourceIdentifier{Group: "example.com", Version: "v1", Resource: "widgets", Namespace: "prod", Name: "a"}
+	v2 := v1
+	v2.Version = "v2"
+
+	assert.NotEqual(t, v1.Key(), v2.Key(), "Key is the API-side identity and includes the version")
+	assert.Equal(t, v1.ToGitPath(), v2.ToGitPath(), "the Git path is the durable identity and omits the version")
+}
+
+// ExampleResourceIdentifier_Key shows the four shapes of the key format, including the two
+// empty-segment rules that pull in opposite directions.
+func ExampleResourceIdentifier_Key() {
+	namespaced := ResourceIdentifier{
+		Group: "apps", Version: "v1", Resource: "deployments", Namespace: "prod", Name: "api",
+	}
+	clusterScoped := ResourceIdentifier{
+		Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterroles", Name: "admin",
+	}
+	coreNamespaced := ResourceIdentifier{
+		Version: "v1", Resource: "secrets", Namespace: "prod", Name: "db",
+	}
+	coreClusterScoped := ResourceIdentifier{
+		Version: "v1", Resource: "nodes", Name: "node-1",
+	}
+
+	fmt.Println(namespaced.Key())
+	fmt.Println(clusterScoped.Key())     // no empty namespace segment
+	fmt.Println(coreNamespaced.Key())    // empty group: a leading "/"
+	fmt.Println(coreClusterScoped.Key()) // both rules at once
+
+	// Output:
+	// apps/v1/deployments/prod/api
+	// rbac.authorization.k8s.io/v1/clusterroles/admin
+	// /v1/secrets/prod/db
+	// /v1/nodes/node-1
+}
+
+// ExampleResourceIdentifier_ToGitPath contrasts the two identity functions on one object:
+// the key carries the API version, the path deliberately does not.
+func ExampleResourceIdentifier_ToGitPath() {
+	deployment := ResourceIdentifier{
+		Group: "apps", Version: "v1", Resource: "deployments", Namespace: "prod", Name: "api",
+	}
+	node := ResourceIdentifier{Version: "v1", Resource: "nodes", Name: "node-1"}
+
+	fmt.Println(deployment.Key())
+	fmt.Println(deployment.ToGitPath())
+	fmt.Println(node.Key())
+	fmt.Println(node.ToGitPath()) // cluster-scoped resources live under "_cluster"
+
+	// Output:
+	// apps/v1/deployments/prod/api
+	// prod/apps/deployments/api.yaml
+	// /v1/nodes/node-1
+	// _cluster/nodes/node-1.yaml
+}

internal/types/reference.go

@@ -36,7 +36,11 @@ func (r ResourceReference) String() string {
 	return fmt.Sprintf("%s/%s", r.Namespace, r.Name)
 }
 
-// Key returns a string key suitable for map lookups.
+// Key returns a string key suitable for map lookups: "namespace/name".
+//
+// Not to be confused with [ResourceIdentifier.Key], which is the fully-qualified
+// "{group}/{version}/{resource}/{namespace}/{name}" identity of a watched object. This one
+// names a GitTarget-like object by reference and carries no group, version or resource.
 func (r ResourceReference) Key() string {
 	return r.String()
 }

pkg/manifestanalyzer/doc.go

@@ -28,7 +28,14 @@
 // are the values worth matching on.
 //
 // Everything under internal/ carries no guarantee either, and is not importable from
-// another module.
+// another module. One format from there is nonetheless a contract you may build on: a
+// resource's identity key is "{group}/{version}/{resource}/{namespace}/{name}", with the
+// namespace segment dropped (not emitted empty) for a cluster-scoped resource and an empty
+// group segment for core resources, so the four shapes are "apps/v1/deployments/prod/api",
+// "rbac.authorization.k8s.io/v1/clusterroles/admin", "/v1/secrets/prod/db" and
+// "/v1/nodes/node-1". It is specified and golden-tested at
+// ResourceIdentifier.Key in internal/types/identifier.go, which also records why a join
+// that must survive a storage-version bump keys on the versionless Git path instead.
 //
 // The command-line equivalents are `manifest-analyzer --mode scan-folder --format json` and
 // `--mode scan-repo --format json`, which emit exactly the documents [FolderReport]