chore: move the layout corpus into test/, execute shape 7's refusal, and give every design page one status label

api/v1alpha3/clusterprovider_types.go

@@ -68,20 +68,10 @@ type ClusterProviderSpec struct {
 	// +optional
 	KubeConfig *meta.KubeConfigReference `json:"kubeConfig,omitempty"`
 
-	// Design rationale, kept out of the generated CRD description by the blank line below.
-	//
-	// This is the one namespace policy that survived the source-scope deletion, and it survived
-	// because the boundary it draws is available nowhere else. Source-cluster RBAC bounds what a
-	// CREDENTIAL may read; it cannot express which control-plane tenant may WIELD that credential,
-	// because the tenant is not a subject in the source cluster at all. Deleting it would make a
-	// shared source credential usable from any namespace that can create a GitTarget.
-	//
-	// Its selector is affordable in a way the deleted source-side one was not: it reads
-	// CONTROL-cluster Namespace labels, locally, with no cross-cluster call and no degradation
-	// path. Both halves stay.
-	//
-	// The rename is what makes it readable now that the two allowed*Namespaces fields no longer sit
-	// side by side to disambiguate each other. See docs/design/source-scope-simplification.md.
+	// The one namespace policy that survived the source-scope deletion: source-cluster RBAC bounds
+	// what a credential may READ, and cannot express which control-plane tenant may WIELD it.
+	// Without this, a shared source credential is usable from any namespace that can create a
+	// GitTarget. The selector reads control-cluster labels locally, so it has no degradation path.
 
 	// AccessFrom is the deny-by-default policy for which CONTROL-CLUSTER namespaces may reference
 	// this provider from a GitTarget. Empty (or omitted) means no namespace may reference it. Its
@@ -90,29 +80,11 @@ type ClusterProviderSpec struct {
 	// +optional
 	AccessFrom *NamespaceMatcher `json:"accessFrom,omitempty"`
 
-	// Design rationale, kept out of the generated CRD description by the blank line below.
-	//
-	// Remote and in-cluster providers use the same mechanism but deserve very different sign-off.
-	// For a REMOTE provider the config-plane namespace and the source namespace are on different
-	// clusters, so their sharing a name never was a boundary and naming one widens nothing. For an
-	// IN-CLUSTER provider (kubeConfig omitted) the same-name coupling WAS the boundary: setting this
-	// deliberately bypasses live namespace RBAC, letting the owner of an admitted GitTarget in one
-	// namespace mirror another namespace's objects — read through the operator's own cluster-wide
-	// credential — into a Git destination they control. That is legitimate for a cluster-admin to
-	// grant on purpose, and must never happen by default or as a side effect of another field,
-	// which is why this exists and defaults to false. LOCALITY is not the switch: in-cluster-ness
-	// follows from spec.kubeConfig, and neither that nor the provider's name decides this.
-	//
-	// The name keeps "Source" deliberately. This object carries two namespace planes, and an
-	// allowAnyNamespace sitting directly beneath accessFrom would read as a modifier on it.
-	// allowCrossNamespace was the other candidate, borrowing Flux's --no-cross-namespace-refs
-	// vocabulary, and it was not taken: in Flux the phrase means references across namespaces in
-	// ONE cluster, while here the far side is a namespace in a DIFFERENT cluster. "Crossing" is
-	// literally true only for the in-cluster provider; "any" is literally true for both.
-	//
-	// It stays a boolean because there are two states and no third one is in view: impersonation
-	// and source-side selectors are both out, so an enum would only leave room for something
-	// nobody can name.
+	// The dangerous case is an IN-CLUSTER provider (kubeConfig omitted), where same-name coupling
+	// WAS the boundary: setting this bypasses live namespace RBAC, letting the owner of an admitted
+	// GitTarget mirror another namespace's objects through the operator's own cluster-wide
+	// credential into a destination they control. Legitimate to grant deliberately, never by
+	// default or as a side effect of another field, which is why it defaults to false.
 
 	// AllowAnySourceNamespace delegates SOURCE-namespace selection to the GitTargets this provider
 	// admits. While false (the default) a WatchRule mirroring through this provider may watch only
@@ -254,7 +226,7 @@ func (p *ClusterProvider) IsInCluster() bool {
 // 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).
+// enforced with CEL and then reversed before shipping.
 func (p *ClusterProvider) AuditRoute() string {
 	if p.Spec.Attribution == nil || p.Spec.Attribution.AuditRoute == "" {
 		return p.Name

api/v1alpha3/clusterwatchrule_types.go

@@ -111,14 +111,9 @@ type ClusterResourceRule struct {
 	// +kubebuilder:validation:items:Pattern=`^[^/]*$`
 	Resources []string `json:"resources"`
 
-	// Design rationale, kept out of the generated CRD description by the blank line below.
-	//
-	// The field is retained in the schema purely so that re-applying a manifest that still says
-	// "Namespaced" FAILS. Deleting it outright would be worse and silent twice over: CRD pruning
-	// happens on write, so the value would be dropped without an error and the rule would quietly
-	// stop mirroring namespaced objects; and a stored pre-release object would keep its value in
-	// etcd with no Go field left to read, leaving the controller nothing to refuse. The narrowed
-	// enum rejects it at admission, and the compile path refuses a stored value.
+	// Retained in the schema purely so re-applying a manifest that still says "Namespaced" FAILS.
+	// Deleting it would be silent twice over: pruning drops the value without an error, and a
+	// stored pre-release object would keep its value with no Go field left to refuse it.
 
 	// Scope is REMOVED as a choice: a ClusterWatchRule is cluster-scoped only, so "Cluster" is the
 	// only accepted value and also the default, making the field omittable. To watch NAMESPACED
@@ -169,12 +164,9 @@ type ClusterWatchRuleStatus struct {
 	Streams *WatchRuleStreamsStatus `json:"streams,omitempty"`
 }
 
-// Design rationale, kept out of the generated CRD description by the blank line below.
-//
-// Cluster-scoped objects have no namespace, so no namespace policy is a bound for them: a
-// ClusterWatchRule is intentionally cluster-global and is limited only by its source credential's
-// Kubernetes RBAC. Isolating cluster-scoped objects between tenants therefore takes separate
-// credentials/ClusterProviders.
+// Cluster-scoped objects have no namespace, so no namespace policy bounds them: this is
+// cluster-global, limited only by its source credential's RBAC. Isolating tenants takes separate
+// ClusterProviders.
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status

api/v1alpha3/examples_test.go

@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha3
+
+import (
+	"bufio"
+	"bytes"
+	"errors"
+	"io"
+	"os"
+	"path/filepath"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
+	"sigs.k8s.io/yaml"
+)
+
+// exampleRoots are the folders whose manifests a reader is invited to copy: the samples, the
+// worked examples in the layout corpus, the playground, and the e2e setup fixtures that are checked
+// in as YAML rather than rendered from a template.
+var exampleRoots = []string{
+	"../../config/samples",
+	"../../test/fixtures/layout-corpus",
+	"../../test/playground",
+	"../../test/e2e/setup",
+}
+
+// TestExamplesDecodeStrictly folds every checked-in example of our own kinds through the REAL types
+// with strict decoding, which is the only thing that catches a field the API no longer has.
+//
+// It exists because the fields the breaking wave removed are PRUNED rather than refused: an example
+// still naming one applies cleanly, does nothing, and reports nothing, so a stale example is
+// invisible both to a reader and to a cluster. The layout corpus already decodes the GitTargets it
+// executes, but an example folder with no input/ is executed by nothing, and that is exactly where
+// a `spec.commit.author` survived a rename.
+//
+// Every document is parsed before anything decides whether to skip it. A document this test cannot
+// read, or one in our own group naming a version or kind we do not serve, is a failure rather than
+// something quietly passed over as somebody else's schema: those are the shapes a stale example
+// takes, so skipping them would leave the hole this test was written to close.
+func TestExamplesDecodeStrictly(t *testing.T) {
+	decoded := 0
+	for _, root := range exampleRoots {
+		require.NoError(t, filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+			if err != nil || info.IsDir() {
+				return err
+			}
+			if ext := filepath.Ext(path); ext != ".yaml" && ext != ".yml" {
+				return nil
+			}
+			decoded += decodeExampleFile(t, path)
+			return nil
+		}))
+	}
+	require.NotZero(t, decoded, "the example roots moved: this test decoded nothing")
+}
+
+// decodeExampleFile strict-decodes every document of our own group in one file and returns how many
+// it decoded. It reads documents through a YAML reader rather than splitting on a "---" line, which
+// is not the separator YAML actually defines: a document can open with one, and the sequence can
+// appear inside a block scalar.
+func decodeExampleFile(t *testing.T, path string) int {
+	t.Helper()
+
+	raw, err := os.ReadFile(path)
+	require.NoError(t, err)
+
+	reader := k8syaml.NewYAMLReader(bufio.NewReader(bytes.NewReader(raw)))
+	decoded := 0
+	for i := 0; ; i++ {
+		doc, err := reader.Read()
+		if errors.Is(err, io.EOF) {
+			return decoded
+		}
+		require.NoError(t, err, "%s: document %d could not be read", path, i)
+		if len(bytes.TrimSpace(doc)) == 0 {
+			continue
+		}
+
+		var parsed any
+		require.NoError(t, yaml.Unmarshal(doc, &parsed),
+			"%s: document %d is not parseable YAML", path, i)
+
+		// A document that is valid YAML but not a mapping (a bare list, a scalar) carries no
+		// apiVersion and cannot be one of ours.
+		fields, isMapping := parsed.(map[string]any)
+		if !isMapping {
+			continue
+		}
+		apiVersion, _ := fields["apiVersion"].(string)
+		gv, err := schema.ParseGroupVersion(apiVersion)
+		if err != nil || gv.Group != GroupVersion.Group {
+			// Somebody else's schema, including the neighbouring examples.configbutler.ai and
+			// manifestanalyzer.configbutler.ai groups, which are not this API.
+			continue
+		}
+
+		require.Equal(t, GroupVersion.Version, gv.Version,
+			"%s: document %d is %q; this group serves only %s, and an example on a version the "+
+				"operator no longer installs cannot be applied", path, i, apiVersion, GroupVersion)
+
+		kind, _ := fields["kind"].(string)
+		obj := newExampleObject(kind)
+		require.NotNil(t, obj, "%s: document %d declares kind %q, which %s does not serve",
+			path, i, kind, GroupVersion)
+
+		require.NoError(t, yaml.UnmarshalStrict(doc, obj),
+			"%s: document %d names a field this API does not have; applied to a cluster it would "+
+				"be pruned in silence", path, i)
+		decoded++
+	}
+}
+
+// newExampleObject returns an empty object of the named kind, or nil when this group does not serve
+// it. Every root kind this API registers belongs here: a kind missing from the switch would make a
+// valid example fail rather than be checked, which is why the list is asserted against the scheme
+// by TestExampleKindsCoverTheScheme.
+func newExampleObject(kind string) any {
+	switch kind {
+	case "GitProvider":
+		return &GitProvider{}
+	case "GitTarget":
+		return &GitTarget{}
+	case "ClusterProvider":
+		return &ClusterProvider{}
+	case "WatchRule":
+		return &WatchRule{}
+	case "ClusterWatchRule":
+		return &ClusterWatchRule{}
+	case "CommitRequest":
+		return &CommitRequest{}
+	}
+	return nil
+}
+
+// TestExampleKindsCoverTheScheme pins newExampleObject to the scheme, so a kind added to this API
+// group cannot quietly fall outside the example guard. Without it, adding a CRD and an example for
+// it in the same change would leave that example unchecked: newExampleObject returns nil, and the
+// document reads as somebody else's schema.
+//
+// List kinds are excluded. A manifest is a single object, never a List, so an example carrying one
+// is not a case the guard has to decode.
+func TestExampleKindsCoverTheScheme(t *testing.T) {
+	s := runtime.NewScheme()
+	require.NoError(t, AddToScheme(s))
+
+	ourPackage := reflect.TypeOf(GitTarget{}).PkgPath()
+	for gvk, goType := range s.AllKnownTypes() {
+		if gvk.GroupVersion() != GroupVersion || strings.HasSuffix(gvk.Kind, "List") {
+			continue
+		}
+		// Every scheme carries meta kinds (GetOptions, WatchEvent, ...) under each registered
+		// group version. They are apimachinery's types, not ours, and no example declares one, so
+		// identify them by the package they come from rather than by keeping a list of names.
+		if goType.PkgPath() != ourPackage {
+			continue
+		}
+		require.NotNil(t, newExampleObject(gvk.Kind),
+			"%s is registered in the scheme but newExampleObject does not build one, so every "+
+				"example of it is skipped by TestExamplesDecodeStrictly", gvk.Kind)
+	}
+}