diff --git a/docs/INDEX.md b/docs/INDEX.md index 13b64344..e5a05645 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -65,12 +65,13 @@ says what we support and refuse** — and then its kustomize field taxonomy, the write boundary, the orchestrator/expansion line, and how secrets are handled. -Twelve other open items: +Thirteen other open items: | Doc | Open question | |---|---| | [`docs-linting.md`](design/docs-linting.md) | how to mechanize [`style-guide.md`](style-guide.md) with markdownlint-cli2 and Vale. Both are wired into `task lint`, gated on the files [`.docs-lint-scope`](../.docs-lint-scope) lists rather than the whole tree: 102 of 174 files fail markdownlint and 148 of 174 fail Vale, so the two backlogs need different gates. Open: how the scope list grows to cover the tree, the `MD013` limit, and whether `AGENTS.md` and the chart READMEs are in scope | | [`attribution-fact-identity.md`](design/attribution-fact-identity.md) | several `ClusterProvider`s may name one physical cluster, but a kube-apiserver posts audit to one route, so only one of those names is ever fed and every other one authors `unknown (attribution unresolved)`. Proposes a declared `spec.attribution.auditRoute` that partitions the facts instead of `metadata.name`, so several providers can share one cluster's facts while cloned clusters stay separate, ingestion loses its last Kubernetes read, and a misrouted provider becomes loud. Renames the key infix and the annotation-key flag to the same word | +| [`analyzer-consumer-contract-asks.md`](design/analyzer-consumer-contract-asks.md) | three asks from the first downstream consumer of `pkg/manifestanalyzer`, each the same failure at a different layer — a distinction our code knows, published as prose no test defends. Permanence on a `RefusalReason` (they shipped "not supported yet" for `invalid-yaml`, because one deprecated code was the only signal the type offered); the analyzer's version in its own JSON report, plus a signed release binary; and `ResourceIdentifier.Key()`'s string format as a documented contract with a golden test — it has **no test at all** today. Carries our classification of all seventeen internal `IssueKind` values (the public package exports only fourteen, and three codes can reach a consumer with no constant to match on); open: the per-construct split for `unsupported-kustomize`, and whether `Key()` or `ToGitPath()` is the identity when a preferred version bumps | | [`watch-and-catalog-architecture.md`](design/watch-and-catalog-architecture.md) | the target three-layer watch model — **needs a human call before building** | | [`metrics-observability-plan.md`](design/metrics-observability-plan.md) | the watch-stage metrics do not exist yet | | [`reconcile-triggering.md`](design/reconcile-triggering.md) | which controllers still fail to wake up | @@ -90,7 +91,12 @@ grouping strategies nothing else covers. [`ha-gittarget-distribution-plan.md`](future/ha-gittarget-distribution-plan.md) is the HA plan `architecture.md` cites three times (and the reason Redis is required). [`least-privilege-remaining-work.md`](future/least-privilege-remaining-work.md) has -three open RBAC items. Five more ideas sit beside them. +three open RBAC items. +[`config-surface-for-a-structured-repository.md`](future/config-surface-for-a-structured-repository.md) +reviews the configuration docs and argues the API never caught up with what the +folder analysis learned — a look-before-you-write mode, a `status.layout` +projection, an inference switch, and moving `commitWindow` onto the GitTarget. +Five more ideas sit beside them. ## History — [`finished/`](finished/) diff --git a/docs/architecture.md b/docs/architecture.md index 19cb3733..70937804 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -530,7 +530,8 @@ team-a-config/ # GitTarget spec.path The **built-in default** path is `{spec.path}/{namespace}/{group}/{resource}/{name}.yaml`, namespace first, the API group omitted for core resources, no version segment, and a `.sops.yaml` suffix for -sensitive resources; a cluster-scoped resource uses the literal `cluster/` in place of the namespace. +sensitive resources; a cluster-scoped resource uses the literal `_cluster/` in place of the namespace +(an illegal Kubernetes namespace name, so it can never clash with a real one). But that default is only the cold-start seed: a new resource first follows its **siblings'** existing layout, and a `GitTarget` can declare its own placement policy. Details and the placement policy are in [File Placement](#file-placement). @@ -859,7 +860,7 @@ refreshes that source. ### TypeRegistry and followability - **Source**: [internal/typeset/](../internal/typeset/) -- **Design**: [design/manifest/version2/type-followability.md](spec/type-followability.md) +- **Design**: [type-followability.md](spec/type-followability.md) `internal/typeset` is the single decision surface for "can this type be followed?" Each `TypeRecord` carries GVK/GVR identity, scope and preferred version facts, origin classification, subresource facts @@ -1278,7 +1279,7 @@ Deeper dives live under [docs/design/](design/): - [Typeset owns discovery grace](spec/typeset-owns-discovery-grace.md) - [Kubernetes API resource catalog](facts/kubernetes-api-resource-catalog.md) - [GitTarget status design](spec/status-conditions-guide.md) -- [GitTarget lifecycle and repo architecture](architecture.md) +- [The support contract: what we edit, what we refuse, and why](design/support-boundary/support-contract.md) - [Git credentials interop](finished/git-credentials-interop.md) - [SOPS/age key management](finished/sops-repo-bootstrap-and-key-management-architecture.md) - [Commit signing](commit-signing.md) diff --git a/docs/configuration.md b/docs/configuration.md index bf36fbd0..340d4e38 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -661,8 +661,8 @@ The boundaries that keep it predictable: want all ConfigMaps bundled even though none exist yet" is a job for `byType` below. The full ladder, tie-break rules, and edge cases are in -[design/manifest/version2/gittarget-new-file-placement-rules.md](spec/gittarget-new-file-placement-rules.md); -the vision behind it is [design/manifest/file-agnostic-placement.md](spec/gittarget-new-file-placement-rules.md). +[gittarget-new-file-placement-rules.md](spec/gittarget-new-file-placement-rules.md), which also carries +the vision the sibling-inference step serves. #### Declaring a layout (`byType` / `default`) diff --git a/docs/design/analyzer-consumer-contract-asks.md b/docs/design/analyzer-consumer-contract-asks.md new file mode 100644 index 00000000..d2835e1e --- /dev/null +++ b/docs/design/analyzer-consumer-contract-asks.md @@ -0,0 +1,561 @@ +# Three asks from a downstream consumer of the analyzer + +> Status: accepted as work, scheduled as three independent PRs. Written for an implementing +> party — every claim below was checked against the tree at `v0.39.1`. +> Date: 2026-07-26, decisions recorded 2026-07-27. +> Companion to [config-surface-for-a-structured-repository.md](../future/config-surface-for-a-structured-repository.md), +> which makes the same argument one layer up: the engine learned something and the +> contract did not carry it. + +**Two decisions are settled and are not open in implementation:** + +1. **The KRM envelope is adopted** (Ask 2). `apiVersion`/`kind` replaces `schemaVersion`, + `spec` carries the scan request and `status` the findings. This is a deliberate breaking + change to the JSON contract, taken now because there are two consumers and both are + asking for version clarity in this same document. +2. **The three unclassified refusal codes are decided during implementation** (Ask 1). The + implementing session proposes a permanence per *raise site* from the code, and raises + each as a review comment on its PR for the maintainer to accept or overturn. They do not + ship as `PermanenceUnknown`, and they do not block the rest of Ask 1. + +Each ask is one PR, in the order below — Ask 3 is unblocked, Ask 1 is scaffolding plus +fourteen settled classifications, Ask 2 carries the breaking change. + +The asks come from **two independent consumer teams**, arriving separately and landing on +the same seams. One links `pkg/manifestanalyzer` as a module and execs `manifest-analyzer +--mode scan-repo --format json` from a second tool that does not link us; it runs the +reverser as an image pinned to the module by two constants with a build-breaking test on +its side. The other verified everything against `v0.39.1` by measurement — running the +binary over a corpus and reading the JSON — and filed four issues. + +That convergence is the strongest evidence in this document. Two teams that never spoke to +each other hit the same three seams, and every one is the same complaint: **a fact our doc +comments assert, carried by prose nothing tests, so it drifts and a consumer encodes the +stale version.** One of them shipped a wrong sentence to a real user because of it. + +The second team's four issues map onto this document as: their #1 → Ask 1, their #2 → the +`LayoutKustomizeOverlay` fix already made, their #3 → Ask 2, their #4 → Ask 3. Where they +asked for something narrower than what is written here, this document says so. + +One thing they were explicit about, so it is recorded before someone "fixes" it: **the +minimal-version-selection diamond is not our bug.** It existed because their tool linked +our module; it stopped linking, and that is the end of it. Do not restructure the module +to solve it. + +--- + +## Ask 1 — put the permanence of a refusal in the data + +### What they asked for + +A third field on `RefusalReason`, filled in by the check that raised it, saying whether +the refusal can ever stop being one. Plus the same field on `Issue`, which is the same +taxonomy one level down. + +### Why — verified + +The consumer's onboarding wizard shows a human every folder in their repository and why +each one cannot be picked. *"Not supported yet"* is a wait; *"cannot be synced"* is a +redesign; and the two sentences were split on the only signal the type offers, a match on +one code: + +```go +if r.Code == manifestanalyzer.ReasonRefusedStructural { + fc.Permanent = true // everything else fell through to "not supported yet" +} +``` + +They took that from our doc comments, and both halves of what they read are now wrong. + +**The axis went degenerate.** `ReasonOverlayFanOutUnsupported` is marked `// Deprecated: +no longer emitted` at [`pkg/manifestanalyzer/repo.go:42`](../../pkg/manifestanalyzer/repo.go) +— render-root scoping shipped and external-base overlays are adopted. That was the right +change, and it deleted one side of a two-valued distinction a consumer had already +encoded, with no signal that it had happened. + +**The stale comment that caused it is still there.** `LayoutKustomizeOverlay` at +[`pkg/manifestanalyzer/repo.go:23`](../../pkg/manifestanalyzer/repo.go) says such folders +are "Refused today, with a forward-looking reason", four lines above the constant saying +the scanner now adopts them. That is the sentence their code was written from. **Fixed in +the same commit as this document** — but the fix is worth nothing on its own, because +nothing stops the next one. + +**The code space was never two-valued.** `RefusalReason.Code` is populated from exactly +two places: the literal `ReasonRefusedStructural`, and `issuesToReasons` +([`internal/manifestanalyzer/scan_repo.go:409`](../../internal/manifestanalyzer/scan_repo.go)), +which sets `Code: string(iss.Kind)` for **every** acceptance issue. So the live value set +is the **seventeen** internal `IssueKind` constants plus `refused-structural` — and most of +them are neither permanent nor pending. They are **fixable today by the person looking at +the screen**. A folder refused for `invalid-yaml` is one broken document from working, and +its owner was told "not supported yet". + +**A second gap surfaced while counting them, and it needs fixing in the same pass.** +`internal/manifestanalyzer` defines seventeen kinds; `pkg/manifestanalyzer` re-declares +**fourteen**. Because `issuesToReasons` copies the internal string through verbatim, three +codes can reach a consumer that has no exported constant to match on: + +- `kustomize-render-refused` +- `render-does-not-match-live` +- `unplaceable-edit` + +A consumer doing the correct thing — matching on our published constants — silently fails +to recognise all three. Either export them or state that they never reach `ScanRepo`; the +implementer should confirm which by checking whether these write-time kinds can appear in a +structure-only scan. Whatever the answer, the counts must stop disagreeing. + +**And the block a consumer reads first is the one that misleads.** The second team measured +a real corpus and got back `unsupported-kustomize` and `non-krm-yaml` as refusal codes, then +went looking for them. `repo.go` has this: + +```go +// Refusal reason codes a candidate may carry. +const ( + ReasonOverlayFanOutUnsupported = "overlay-fan-out-unsupported" // Deprecated: no longer emitted + ReasonRefusedStructural = "refused-structural" +) +``` + +A block headed "Refusal reason codes a candidate may carry" that lists **two of eighteen**, +one of them retired. The codes they measured do exist as `IssueKind` constants — but nothing +says `RefusalReason.Code` draws from `IssueKind`, so a reader who finds this block reasonably +concludes it is the enumeration and treats everything else as unknown. That is a more direct +cause of their wrong sentence than the `Layout` comment was. + +Two fixes, both cheap: + +- **Type it.** `RefusalReason.Code` becomes `IssueKind` rather than `string`, which makes the + relationship compile-checked instead of stated. If that is too strong for a JSON-facing + field, document it: *"Code is an [IssueKind] value, or [ReasonRefusedStructural]."* +- **Fix the block's header** so it says what it holds: reason codes that are *not* issue + kinds. As written it claims to be the enumeration, and it is not. + +They are correct that no consumer can maintain this table outside our repo. We added +fifteen codes without any of them carrying permanence. + +### The shape + +Additive, and inert for anyone who ignores it: + +```go +// Permanence says whether a refusal can ever stop being one. It is set by the check +// that raised the refusal, because only that check knows. Consumers MUST treat an +// unrecognised or absent value as PermanenceUnknown and say nothing about the future. +type Permanence string + +const ( + PermanenceUnknown Permanence = "" // not classified; say nothing + PermanenceFixable Permanence = "fixable" // change the repo or the GitTarget + PermanencePending Permanence = "pending-upstream" // a future release may accept it + PermanencePermanent Permanence = "permanent" // the support boundary; never a "not yet" +) + +// Actor names who can act on a fixable refusal. Empty when unclassified or when +// nobody can act. +type Actor string + +const ( + ActorUnknown Actor = "" + ActorAuthor Actor = "repository-author" // the person who owns the files + ActorPlatform Actor = "platform-operator" // the person who owns the GitTarget +) + +type RefusalReason struct { + Code string `json:"code"` + Detail string `json:"detail"` + // Permanence is empty when the check did not classify itself. + Permanence Permanence `json:"permanence,omitempty"` + // Actor is empty unless Permanence is PermanenceFixable. + Actor Actor `json:"actor,omitempty"` +} +``` + +`Issue` takes both fields for the same reason: a policy that treats an issue as blocking +is making this decision already, without the data to make it. + +**Four values, where the second team proposed two** (`permanent | conditional`). Take the +four deliberately: a two-valued enum drops the *not-yet* axis, which is the exact +distinction that went degenerate when `ReasonOverlayFanOutUnsupported` was retired and the +exact one that made their table wrong. Collapsing `pending-upstream` into `conditional` +tells a user to go fix something no user can fix. And `PermanenceUnknown` has to exist as +the zero value regardless, or an unclassified path emits a confident wrong answer instead +of silence. Their `retryable` naming is worth considering for the `fixable` value; the axis +count is the part not to compromise on. + +They also note that adding a field is additive and need not move `SchemaVersion`. Correct, +and it stays correct under the KRM envelope recommended in Ask 2 — that envelope is a +separate, deliberate breaking change, and this field should not wait for it. + +Ship the actor field. The consumer offered it as optional; it is the cheaper half of the +same constant, and two of our codes are fixable **only** by the platform operator, whom +their wizard does not have on the screen. Without it, `out-of-scope` renders as "fix your +repository" to someone who cannot. + +### The classification — our answers + +This is the part no consumer can write, so it is written here in full. Derived from each +kind's own doc comment and its raise site. + +The fourteen rows below plus the three deferred to the next section account for all +seventeen internal kinds; `refused-structural` is the eighteenth value and is not an +`IssueKind`. Check that arithmetic when adding a kind — it is the only thing keeping this +table honest until the test below exists. + +| Code | Permanence | Actor | Basis | +|---|---|---|---| +| `invalid-yaml` | fixable | author | the document does not parse | +| `duplicate-identity` | fixable | author | two documents claim one identity | +| `impure-managed-file` | fixable | author | split the file | +| `mixed-managed-allowlisted` | fixable | author | move the kustomization to its own file | +| `ignore-shadows-managed` | fixable | author | narrow the `.gittargetignore` pattern | +| `non-krm-yaml` | fixable | author | remove it, or ignore it | +| `foreign-file` | fixable | author | remove it, or ignore it | +| `foreign-symlink` | fixable | author | the *rule* is permanent, the *folder* is not — remove the link | +| `foreign-submodule` | fixable | author | as above; relocate the submodule | +| `out-of-scope` | fixable | **platform** | widen the GitTarget's scope | +| `write-escapes-scope` | fixable | **platform** | widen `spec.path`, or re-place the write | +| `render-does-not-match-live` | fixable | **platform** | a diverged live value is out-of-band substitution, not a render artifact | +| `write-fan-in` | **pending** | — | the doc comment says per-render-root scoping generalizes this | +| `unplaceable-edit` | **permanent** | — | its comment argues the alternative is measurably wrong, not merely risky | +| `refused-structural` | **permanent** | — | the support boundary by definition | + +Note what the `foreign-symlink` row settles, because it is the rule for every future +check: **permanence classifies the folder's prospects, not the rule's.** A rule we will +never relax can still produce a refusal the author clears in one commit. Classify what the +reader can do, since that is the sentence the field exists to write. + +### Three that need a decision, not a lookup + +**How these three get settled — decided.** The implementing session reads each raise site, +proposes a permanence for it, and raises the proposal as a review comment on its own PR for +the maintainer to accept or overturn. They ship classified, never as `PermanenceUnknown`, +and they do not hold up the fourteen above. The starting positions below are inputs to that +proposal, not conclusions. + +**`unsupported-kustomize` — must be classified per construct at the raise site.** This is +the case that proves the whole ask: one code, both answers. Its doc comment lists +generators, components, Helm inflation, replacements, transformers, name prefixes and +remote bases; `v0.37.0` shipped exactly the kind of loosening that moves one of those, and +`patches:` already moved. Suggested starting split, for the implementer to confirm +construct by construct: + +- remote bases, Helm inflation → `pending-upstream` +- generators, replacements, transformers, name prefix/suffix → **decide**; they are the + constructs whose output the writer cannot map back to source at all, which reads + permanent, but so did `patches:` before it moved + +A static per-code map cannot express this. The field can, and it is why the field goes on +the emitted reason rather than into a table beside the constants. + +**`unresolved-krm` — splits at the raise site.** A kind absent because its CRD is not +installed is `fixable`/`platform`. A kind that is ambiguous, unserved, or missing a verb is +`pending` at best. One code, two answers, same argument as above. + +**`kustomize-render-refused` — decide what it classifies.** It refuses a *write*, not a +folder, so "can this folder ever be picked" is the wrong question for it. Either classify +it `permanent` (the oracle will always refuse a write it cannot vouch for) or leave it +`Unknown` deliberately and say so in its comment. + +### Implementation notes + +- The constant goes at the **raise site**, not into a map beside the type. A map is the + thing that drifted; a field on the emitted value cannot be raised without a decision. +- `issuesToReasons` becomes the projection for both new fields — it is already the single + choke point through which every issue becomes a reason. +- Add a test that **every** `IssueKind` constant is classified, failing on a new + unclassified kind. That test is the ask. Without it this document is prose again in two + releases, which is how we got here. +- One test per constant is **not sufficient**, and the three decision cases above are why: + `unsupported-kustomize`, `unresolved-krm` and `kustomize-render-refused` each classify + differently depending on which branch raised them, so a per-constant test passes while a + new branch emits `PermanenceUnknown`. Cover each emission path, not each constant. This + is the one place the ask costs more than a constant per site, and skipping it reproduces + the original bug one level down. +- Reconcile the internal and public kind sets in the same pass, so the classification test + covers the codes a consumer can actually receive rather than the subset we export. +- `PermanenceUnknown` must stay the zero value so an unclassified path degrades to silence + rather than to a confident wrong sentence. + +### Meanwhile, downstream + +They have flipped their default from "not supported yet" to unknown — an unrecognised code +now says only "this folder cannot be picked" — and deleted their `Permanent bool`. That is +worse for their user than the truth and is the honest maximum from `{Code, Detail}`. + +--- + +## Ask 2 — a JSON document from a binary is a contract with no compile-time signal + +### What they asked for + +Four things, in their order of value: + +1. The analyzer's own version **in the report**. +2. A `manifest-analyzer --version` flag — printing the release version **and the + `SchemaVersion` it emits**, so one exec answers both questions. +3. One sentence on what a `SchemaVersion` bump asserts, and whether a reader should refuse + a version it does not know. +4. The analyzer binary attached to the release we already sign. + +The second team measured the current flag set to confirm the gap: `manifest-analyzer +--version` returns `flag provided but not defined: -version`, and the full set is +`-context`, `-format`, `-kubeconfig`, `-mode`, `-policy`. + +### Why — verified + +Our package doc invites this consumer by name: *"Exec the binary if Go is not your +language; import this package if it is."* When they link the package, a rename is a build +failure. When they exec the binary, the document is all they hold, and it does not say +what produced it. `RepoReport` and `FolderReport` carry `schemaVersion` and nothing else +about provenance; the flag set in `cmd/manifest-analyzer/main.go` is `--mode`, `--format`, +`--policy`, `--kubeconfig`, `--context`, with no `--version`. + +Their exec'd binary is a **third** pin on the same release, alongside the module and the +image that their build already guards. It is installed by `go install` and recorded +nowhere. A report from one release consumed against a writer from another is a wrong +answer that looks entirely normal. + +`--version` alone does not close that — it is a second exec and an assumption that it was +the same binary. **The field in the report is the fix.** The flag is the convenience. + +### The shape, and why it is nearly free + +The machinery exists. [`cmd/buildinfo.go`](../../cmd/buildinfo.go) holds +ldflags-injected `version`/`gitCommit`/`buildDate` and serves them on `/build-info`; the +`Dockerfile` passes `-X main.version=${VERSION}`. + +For the `go install github.com/ConfigButler/gitops-reverser/cmd/manifest-analyzer@vX.Y.Z` +path our own package doc recommends, **ldflags do not apply at all** — +`runtime/debug.ReadBuildInfo()` returns the module version for free, with no build change +and no release-workflow change. Use ldflags when set, fall back to `ReadBuildInfo`, and +emit `"dev"` for a plain `go build`. + +**The settled shape is `status.generator`**, an object, never `omitempty`. The second team +proposed `{name, version}` on the grounds that it is "the version of this that survives +being piped into another tool", and their shape wins: a bare version string says which +release without saying which *tool*, and a piped report is exactly where that bites. `kind` +names the document; `generator.name` names what produced it — two different facts a flat +string conflates. + +```go +// Generator names the build that produced a report. Never empty: a report that cannot +// say what produced it is the failure this field exists to prevent. +type Generator struct { + // Name is the producing tool, e.g. "manifest-analyzer". + Name string `json:"name"` + // Version is the release, e.g. "v0.39.1", or "dev" for a build carrying no version. + Version string `json:"version"` +} +``` + +Both reports carry it at `status.generator`. The fallback chain is ldflags → +`debug.ReadBuildInfo()` → the literal `"dev"`, so it is non-empty on every path including +`go run`. A consumer may then read an absent `generator` as "produced before this shipped" +without having to distinguish that from "produced by a build that did not know itself". + +### On `SchemaVersion` — the question the envelope answers + +Our doc states the consumer's duties well: pin a version, ignore unknown fields, do not +switch on prose. It never states what a **bump asserts**, which left three questions open: + +- Does a bump mean fields were removed, that a field's meaning changed, or either? +- Should a reader that knows `v1` hard-fail on `v2`, or attempt a best-effort parse? +- Since adding a field never bumps it, what is left that *does*? + +**All three are answered by adopting the KRM envelope below**, which is why that decision +was taken rather than writing a bespoke policy to answer them one at a time. + +### Decided: the report becomes a KRM document, and the question dissolves + +The three questions above are ones the Kubernetes API conventions already answer, in a +document every consumer of a GitOps tool has read. So stop writing our own versioning +policy and adopt theirs — give the report an `apiVersion` and a `kind`: + +```yaml +apiVersion: manifestanalyzer.configbutler.ai/v1alpha1 +kind: RepoReport +spec: # what was asked for + root: /repo + mode: scan-repo +status: # what was found + generator: {name: manifest-analyzer, version: v0.39.1} + candidates: [...] + summary: {...} +``` + +`apiVersion` replaces `schemaVersion` outright. "What does a bump assert" becomes the +published alpha/beta/GA contract; "should a reader hard-fail on a version it does not know" +becomes yes, by the same rule every Kubernetes client already follows. We answer three open +questions by citing a document instead of writing one. + +Four further things fall out, none of which the bespoke shape gives us: + +- **The `spec`/`status` split is honest here**, which was the surprise. `spec` is the scan + request (root, mode, policy) and `status` is the observation. Our own CRDs are built that + way, so the report reads like the rest of the product. Today's `root` field floats at the + top level marked "Informational" precisely because there is nowhere for a request to go. +- **YAML becomes the obvious serialization**, which makes `--format yaml` a natural third + option: diffable, reviewable, and committable with the tooling the user already runs. +- **Existing tooling works** — `yq`, `jq`, `kubectl --dry-run` shape assumptions, and + apimachinery's `TypeMeta` for anyone who does link us. +- **It is on-message.** A product whose thesis is that cluster state belongs in Git as KRM + should not emit a bespoke JSON envelope to describe it. + +### Two things to carve out, and one cost + +**No `metadata`.** A KRM document invites `metadata.name`, and a report has no identity — it +observes a path at an instant. A synthesized name is noise, and worse, it suggests the +document can be applied. Kpt's own `kind: ResourceList` carries `apiVersion`, `kind`, `items` +and `results` with no `metadata`, so the envelope-without-metadata shape has precedent. + +**Never served, never registered.** No CRD, no group registration, not applyable. State that +in the type's doc comment, because the shape will make someone try. The related failure is +mild but worth knowing: a report saved into a watched folder is refused either way — today +as `foreign-file`, and as a KRM document as `unresolved-krm`, which reads as "we tried to +manage this and could not" rather than "this is not ours". A marginally worse message, not a +blocker. + +**The cost is one breaking change to the JSON contract**, and our only consumer holds golden +fixtures generated from it. That argues for doing it **now** and inside Ask 2 rather than +after: they are asking us for version clarity in this same document, there is exactly one +consumer to coordinate with, and every later release makes it dearer. It does not disturb +their mirrored-struct design — if anything `TypeMeta` is one more thing they can mirror. + +**It does not replace Ask 2.** `apiVersion` versions the *contract*; `status.generator` +records the *build*. Items 1, 2 and 4 stand unchanged; only the `SchemaVersion` question +above dissolves. + +### On the release asset + +Their devcontainer pins every tool from a release asset — `task`, `kubectl`, `kustomize`, +`helm`, `k3d`, `flux`, `flux-operator` with `sha256sum -c`. The analyzer is the only one +that cannot be, because we publish no binary; the tool that runs it does `go install` from +source. That is the single unverifiable link in an otherwise pinned toolchain, and it is +the tool deciding which folders they offer a tenant. + +**The two teams differ here, and the resolution is to do both.** The first explicitly does +*not* want a `sha256sums` file, because attestation is stronger. The second asks for +"publishing the binary with checksums, if that is cheap alongside the existing release job" +— its devcontainer verifies every other tool that way, so checksums are the mechanism its +existing tooling already speaks. + +`v0.39.1` already ships `crds.yaml`, `install.yaml` and `sbom.spdx.json`, each with a +`.intoto.jsonl` provenance attestation and a `.sigstore.json` bundle +([`.github/workflows/release.yml`](../../.github/workflows/release.yml)). Attach the binary +to that job — verifiable with `gh attestation verify` — **and** emit a `sha256sums` file +beside it. The attestation is the stronger claim; the checksum is the one a `curl | sha256sum +-c` line in a Dockerfile can consume without a GitHub token. Neither team is served by +choosing. + +Decision for the implementer: **which platforms.** `linux/amd64` and `linux/arm64` covers +their devcontainer; `darwin/arm64` is the obvious third. Each is another matrix leg on a +job that already signs. + +--- + +## Ask 3 — `ResourceIdentifier.Key()` is a cross-product identity contract + +### What they asked for + +Document the **string format** of `ResourceIdentifier.Key()` where a consumer can find it, +and put a golden test on the exact strings **in our repo**. + +**Explicitly not: export the type.** Worth recording why, because the generous answer is +the wrong one. Importing anything from our module puts a `require` line in *their* go.mod, +and minimal version selection then raises *their* build to whatever that tool pins — +defeating the exact-release pin their build test guards. A mirrored struct plus a format +test is their design, not a stopgap. **A stable documented format is worth more to them +than an importable type, and costs us less.** + +### Why — verified + +Two products must agree on what "the same resource" is or every join between them is +silently wrong: one keys a row on our identity, the other reports a verdict about it. +Today that agreement is enforced by a byte-for-byte test **on their side**, against a type +in **our** `internal/`. It breaks for them, late, after a version bump they chose, and +never for us. + +And they are right that we have nothing. +[`internal/types/identifier_test.go`](../../internal/types/identifier_test.go) covers +`ToGitPath`, `IsClusterScoped` and `String`. **`Key()` has no test at all.** We can change +its format today and every gate stays green. + +### Be specific about which `Key()` + +There are two, and "the key format" is ambiguous: + +```go +// internal/types/identifier.go:40 — the one this ask is about +func (r ResourceIdentifier) Key() string // "{group}/{version}/{resource}/{namespace}/{name}" + // cluster-scoped: the namespace segment is dropped + // core group: empty, so the key leads with "/v1/secrets/…" + +// internal/types/reference.go:40 — not this one +func (r ResourceReference) Key() string // "namespace/name" +``` + +The golden test pins **four** shapes — the second team named the fourth, and it is the one +this document first missed. All four were confirmed by running them: + +| case | expected | +|---|---| +| namespaced, grouped | `apps/v1/deployments/prod/api` | +| cluster-scoped, grouped | `rbac.authorization.k8s.io/v1/clusterroles/admin` | +| namespaced, core group | `/v1/secrets/prod/db` | +| **cluster-scoped, core group** | `/v1/nodes/node-1` | + +The four-segment cluster-scoped form is the sharp edge: `Key()` **drops** the namespace +segment rather than emitting an empty one, so a naive reimplementation that always joins +five parts produces `…/clusterroles//admin` and never joins. The core cluster-scoped case +is the sharpest of the four, since both the group and the namespace are empty — one +degenerates to a leading `/` and the other vanishes, and a reimplementation has to get two +opposite rules right in one string. + +Their ask includes wording, and it is worth taking verbatim: the test carries a comment +saying the format is **depended on across product boundaries, so changing it is a breaking +change rather than a refactor**. That sentence is what turns a red test from an obstacle +into a decision point. + +### The trap worth naming + +`Key()` includes `Version`; `ToGitPath()` deliberately excludes it, for a documented and +correct reason — the operator writes one version per object, so a version segment would +churn the path on a preferred-version bump. + +Both are right for their own job. Together they mean **our two identity functions disagree +about whether a preferred-version bump is the same resource.** A cross-product join keyed +on `Key()` splits in two when a CRD's storage version moves, while the Git path correctly +does not move at all. + +Decide which of the two **is** the identity and say so where both are defined. The +consumer would rather have a versionless key beside the current one than build their own — +which is the cheap answer if the decision goes that way, and it also removes the last +reason for anyone to reimplement either function. + +### Scope + +Small, and none of it constrains a pre-1.0 module: + +- Godoc on `Key()` naming the format as public, with the four shapes. +- A table-driven golden test on the exact strings, in `internal/types`. +- One line in the package doc of `pkg/manifestanalyzer` pointing at the format, since that + is where a consumer looks. +- The version-identity decision, recorded beside both methods. + +A golden test in our repo turns their late breakage into a red CI run at the moment +someone makes the change, which is where the decision is being taken. It forbids nothing. +It makes the change deliberate. + +--- + +## Why these three belong together + +Each is the same failure at a different layer: a distinction our code knows, published as +prose no test defends. + +- Ask 1 — the check knows whether a refusal is forever; the type carries a code. +- Ask 2 — the binary knows its version; the document carries a schema marker. +- Ask 3 — `Key()` is an identity contract; the format lives in a comment. + +Each fix is a value plus a test that fails when the value stops being true. The cost is a +constant per site. What it buys is the end of a class of bug where we are correct, the +consumer is careful, and the user still gets a wrong sentence. diff --git a/docs/future/config-surface-for-a-structured-repository.md b/docs/future/config-surface-for-a-structured-repository.md new file mode 100644 index 00000000..3a8f8dec --- /dev/null +++ b/docs/future/config-surface-for-a-structured-repository.md @@ -0,0 +1,415 @@ +# The configuration surface, now that a folder has structure + +> Status: proposal — review findings and an ordered plan. Nothing here binds until scheduled. +> Date: 2026-07-24 +> Companion to [flux-maintainer-review-status-and-config-model.md](flux-maintainer-review-status-and-config-model.md), +> whose §4 "Then (API surface)" block (F6, F9, F10) this plan absorbs and extends. + +## The one-sentence finding + +The product's premise — *the kube-apiserver writes back into Git* — is sound and worth +building on. What has drifted is that **`spec.path` is still one string**, while the folder +it names has quietly acquired structure: a layout, render roots, read-only context, build +directives, an inferred placement convention, and a support verdict. The operator now knows +a great deal about that folder, and the API offers the user **no way to say what they expect, +no way to see what was understood, and no way to look before writing.** + +Everything below follows from that one gap. + +--- + +## Part A — Review of `docs/configuration.md` and `docs/architecture.md` + +### A1. Factual errors (fixed on this branch) + +| Where | Was | Now | +|---|---|---| +| `architecture.md:533` | cluster-scoped resources use the literal `` `cluster/` `` | `` `_cluster/` `` — verified against `internal/manifestanalyzer/placement.go:418`, `internal/types/identifier.go:66`, and `configuration.md:700`, which all say `_cluster` | +| `configuration.md:664-665` | two links, both labelled `design/manifest/…` (a path layout that no longer exists), both resolving to the *same* file, one of them claiming to be a different document ("the vision behind it") | one correctly-labelled link | +| `architecture.md:862` | link label `design/manifest/version2/type-followability.md` | correct label | +| `architecture.md:1281` | "GitTarget lifecycle and repo architecture" linked to `architecture.md` — i.e. to itself | links to the support contract, which is what that list was missing | + +### A2. `configuration.md` is stale against the CRD it documents + +The GitTarget status list (`configuration.md:515-524`) names `Ready`, `Reconciling`, `Stalled`, +`Validated`, `EncryptionConfigured`, `StreamsRunning`, `GitPathAccepted`. The CRD's own printer +columns (`gittarget_types.go:283-297`) also ship **`RenderMatchesLive`**, **`GitProviderReady`**, +and **`ClusterProviderReady`**. `RenderMatchesLive` is the condition that carries the entire +kustomize-verification story — the newest and least obvious part of the product — and the +configuration guide does not mention it once. A user who hits it has nowhere to read what it means. + +### A3. The structural problem: three documents are wearing one trench coat + +`configuration.md` is ~1150 lines and is simultaneously + +- a **tutorial** (the setup flow, the quickstart pointers), +- a **field reference** (every spec field of six kinds), and +- a **design-rationale record** (why the provider scopes differ, why omitted ≠ empty, why the + sweep fails closed, why `default` is never auto-created). + +The rationale is genuinely good — better than what Flux has written down, as the maintainer +review says — but it is interleaved with the reference at a 1:1 ratio, so the reference is +unusable at speed and the rationale is unfindable on purpose. Concretely, the reader who wants +to answer *"which folder do I point this at?"* has to get through ~460 lines of Git credential +dialects, known-hosts precedence, commit templates and signing before reaching `GitTarget` at all. + +Recommendation (not urgent, but do it before there are users): + +- `configuration.md` → **reference only**: one section per kind, fields, defaults, a table, an + example. Rationale moves out to `docs/spec/`, linked inline as "why:". +- A new `docs/onboarding.md` → **the tutorial**: pick a repo, scan it, pick a folder, apply, verify. + This is where Part B's missing product step lives. +- Credentials interop (`configuration.md:95-136`) is a *guide*, not configuration — it belongs + beside `github-setup-guide.md`. + +### A4. Three surfaces are described as if they were one + +`configuration.md`'s "Audit ingestion settings" section (line 1027 onward) freely mixes: + +- CRD fields (`ClusterProvider.spec.attribution.auditRoute`), +- Helm values (`attribution.enabled`, `queue.redis.addr`), +- controller flags (`--author-attribution`, `--author-attribution-grace`, `--redis-key-prefix`), + +with no visual marker for which is which. `attribution.enabled` and `--author-attribution` are +the *same knob* under two names in the same paragraph. There is a `docs/config-flag-conventions.md` +already; this section should adopt its vocabulary and label every knob with its surface. + +### A5. `architecture.md` is in good shape + +It is long but it is *one* document with one job, and the ground-rules → mental-model → detail +progression works. Two notes: + +- The "Design documents" list at the end (`:1263`) omits the entire + `docs/design/support-boundary/` tree, which is now where the product's most consequential + reasoning lives. `support-contract.md` should be linked from the top of the file, not the bottom. +- "Operational boundaries" (`:1210`) is the most honest section in the repo and should be + linked from the README. It is currently only reachable by reading 1200 lines. + +--- + +## Part B — What the config surface is missing, and why + +### B0. The direction is right; the API did not keep up + +The doubt in the original question — *"analysing git folders, deciding where to place files, +understanding Kustomize … it's all very cool, but it requires more understanding to pick which +folder to reverse"* — is correct, and it is not an argument against the work. It is an argument +that the work **finished in the engine and never reached the API**. + +The engine now answers, per folder: what layout is this, which directories are render roots, +which files are read-only build context, which are build directives, which documents are +ambiguous, is this folder supportable at all, and where would a new resource go. Every one of +those answers exists in `internal/manifestanalyzer` and in the `manifest-analyzer` CLI. The +GitTarget API exposes exactly **one bit** of it: `GitPathAccepted`. + +That asymmetry produces four concrete problems. + +### B1. There is no "look before you write" + +For a product whose job is *writing to your GitOps repository*, first contact is: apply a +GitTarget, and it starts committing. `spec.prune.mode` guards deletions and is well designed — +but nothing guards the *initial* write, which for a populated folder is the scary one. + +**Proposal: `GitTarget.spec.mode`.** + +```yaml +spec: + mode: Observe # Observe | Write (default Write, or Observe — see below) +``` + +`Observe` runs the entire pipeline — watches, acceptance gate, layout analysis, placement +resolution, plan construction — and reports what it *would* do, without touching the remote. +It is the field the current direction most obviously demands, because the folder analysis is +now the interesting part and it is invisible until after the first commit. + +This is **not** `spec.suspend` (F6 in the maintainer review). Suspend means *stop reconciling +this object*; Observe means *keep reconciling and keep telling me, just don't write*. Ship both; +they answer different questions and Flux users expect `suspend` by name. + +Whether `Observe` should be the **default** is the interesting call. Arguments for: a +reverse-GitOps tool defaulting to "writes to your repo on apply" is a startling default, and +there are no users yet to break. Arguments against: it adds a step to the quickstart, which is +currently the product's best asset. Recommendation: **default `Write`, but make the chart's +`quickstart` render `Observe`** — the fast path stays fast, and the deliberate path is safe. + +### B2. A refusal is a boolean, not a diagnosis + +`GitPathAccepted=False` + reason `UnsupportedContent` + a message string is the whole +report. What the operator actually computed — layout kind, render roots, external bases, +tolerated context files, ambiguous documents — reaches the user only through the message, +or through `-v1` logs, or by running a separate CLI against a separate checkout. + +**Proposal: `GitTarget.status.layout`**, bounded and count-based, exactly like the existing +`status.streams` and `status.retention` precedents: + +```yaml +status: + layout: + kind: KustomizeOverlay # PlainManifests | KustomizeRoot | KustomizeOverlay | Mixed | Empty + renderRoots: 1 + externalBases: 1 # rendered as read-only context, outside spec.path + contextFiles: 3 # rendered, never written + buildDirectives: 1 # kustomization.yaml / .sops.yaml — retained, never swept + ambiguousDocuments: 0 # reachable from >1 render root; degrade to plain in-place edit + observedTime: "2026-07-24T…" +``` + +This is the highest-value, lowest-risk item in this document. The analyzer computes all of it +already; this is a projection, not new logic. It turns "the operator did something clever with +my repo" into "the operator says my repo is an overlay with one external base and three context +files", which is the difference between trusting the product and not. + +`ambiguousDocuments` deserves special mention. Per `configuration.md:779-784`, a document +reachable from more than one render root with differing override chains **silently degrades** +to plain in-place editing, recorded only as a store diagnostic at debug verbosity. That is a +correctness-relevant fallback that is currently invisible in the API. It is exactly the class +of surprise the original question is worried about. + +### B3. Inference cannot be turned off + +Sibling inference (`configuration.md:626-661`) is good, careful work — the namespace-agnosticism +proof, the sensitive/plaintext firewall, the deterministic tie-break. But it is **unconditional**. +A user who wants determinism — "if I didn't declare it, use the canonical path, do not read my +repo's mind" — has no field to say so. + +**Proposal: `GitTarget.spec.placement.mode`.** + +| Value | Ladder that runs | +|---|---| +| `Infer` (default, today's behaviour) | `byType` → `default` → sibling inference → canonical | +| `Declared` | `byType` → `default` → canonical. Inference off. | +| `Strict` | `byType` → `default` only; a type with no declared route is **refused**, not guessed. | + +One enum, no new machinery — the ladder already exists, this just truncates it. `Strict` is what +a platform team standardising a repo layout across many clusters will want, and it converts a +silent surprise into a loud condition. + +### B4. `commitWindow` is on the wrong object + +Flagged in the maintainer review (§3) and worth acting on now. The split today: + +| `GitProvider` (namespaced) holds | Which is really about | +|---|---| +| `url`, `secretRef`, `knownHostsRef`, `allowedBranches` | **the connection** ✅ | +| `commit.committer`, `commit.signing` | **the repo's identity on the platform** ✅ | +| `push.commitWindow` | **a workload's write cadence** ❌ | +| `commit.message.*Template` | **a workload's commit style** ❌ | + +Consequence: two GitTargets sharing one repository — a fast-moving app folder and a +cluster-RBAC mirror — cannot have different batching or different subjects. That is not +hypothetical; it is the normal shape as soon as a second GitTarget exists. + +**Proposal:** move `push.commitWindow` and `commit.message` to `GitTarget`. Keep `committer` +and `signing` on `GitProvider` (they are properties of the key registered with the Git host). + +Implementation note, so this is not underestimated: the commit window is owned by the +`BranchWorker`, which is keyed per `(provider namespace, provider, branch)` and **shared across +GitTargets**. But the open window is already keyed by `(author, GitTarget)` +(`architecture.md:974-980`) — so the duration becomes a property of the open window rather than +of the worker. The worker keeps serializing the branch; only the timer moves. That is a real +change but a contained one, and it is materially easier now than after a release. + +Do it in the same breaking wave as F6/F12, not separately. + +### B5. Onboarding is a product step the docs pretend does not exist + +`docs/design/support-boundary/repo-discovery-and-onboarding-scan.md` is explicit and correct: +**the operator never discovers.** A GitTarget is told exactly one subtree. Argo and Flux scan; +we deliberately do not. + +That architectural call is right — and it makes onboarding a **step the product owes the user**, +performed by `manifest-analyzer --mode scan-repo`, which answers precisely the question the +original doubt raises: *which folders can become GitTargets, what layout is each, which are +supported and why not, and what GitTarget/WatchRule would express each one.* + +That CLI is mentioned in `docs/style-guide.md` and `docs/UPGRADING.md`. It appears in +`configuration.md` **zero times** and in `architecture.md` **zero times**. The tool that closes +the gap exists and is undocumented where the gap is felt. + +**Proposal:** `docs/onboarding.md`, step 0 of every non-quickstart install, with the scan output +of a real repo and the GitTargets it suggests. Link it from the README, from `configuration.md`'s +`spec.path` section, and from `architecture.md`'s configuration model. + +### B6. Absorb the outstanding maintainer-review items into this wave + +Still open from the companion review, all API-surface, all cheaper now than after release: + +- **F6** — `spec.suspend` on GitTarget / WatchRule / ClusterWatchRule / GitProvider; + `spec.interval` on GitProvider at minimum; jitter the requeue; + `reconcile.configbutler.ai/requestedAt` + `status.lastHandledReconcileAt`. +- **F10** — CommitRequest lifecycle: `ttlSecondsAfterFinished` or an ownerRef, plus the + `delete` verb. Today they accumulate in etcd forever and the controller cannot reap them. +- **F9** — verify the stored `scope: Namespaced` status-write path on the minimum supported + Kubernetes version (one envtest). +- **F12 remainder** — unify the six near-identical reference shapes onto `fluxcd/pkg/apis/meta` + where they match. + +Plus one this review adds: **the `ProviderNotFound` message for the literal name `default`** +should name the fix (`clusterProvider.createDefault` in the chart, or commit the object). It is +the most likely first-run support ticket and it is a one-line message change. + +--- + +## Part C — Worked examples + +Three repositories, run through today's configuration and through the proposal. These are the +shapes the layout corpus (`test/fixtures/gitops-layouts/`) already covers. + +### C1. Per-environment folders — the `commitWindow` problem + +```text +repo/ + clusters/prod/… # ~40 objects, changes hourly + clusters/staging/… # ~40 objects, changes constantly during the day + platform/rbac/… # ClusterRoles, changes monthly +``` + +Three GitTargets, one GitProvider, one branch. Today they **must** share +`push.commitWindow: 5s` and one set of message templates. + +What you actually want: `staging` batching at `60s` (a `kubectl apply -k` storm should be one +commit, not twelve), `platform/rbac` at `0s` (every RBAC change is individually meaningful and +should be individually reviewable in `git log`), `prod` at the `5s` default. + +Today: impossible without three GitProviders pointing at the same URL — which means three +credential Secrets, three signing configurations, and three `ls-remote` probes per interval +against the same host, for a batching preference. **This is the clearest argument for B4.** + +With B4: + +```yaml +kind: GitTarget +metadata: {name: platform-rbac} +spec: + providerRef: {name: platform-repo} + branch: main + path: platform/rbac + push: + commitWindow: "0s" # every RBAC change is its own commit + commit: + message: + groupTemplate: "rbac: {{.Author}} changed {{.Count}} object(s)" +``` + +### C2. `base` + `overlays` — the folder-choice problem + +```text +apps/podinfo/ + base/ kustomization.yaml + deployment.yaml + overlays/prod/ kustomization.yaml (resources: [../../base], images:, replicas:) + overlays/test/ kustomization.yaml (resources: [../../base], images:) +``` + +**Which folder is the GitTarget?** The API gives no help, and the two plausible answers behave +very differently: + +- `path: apps/podinfo/overlays/prod` — the supported, designed case. The base is read as + read-only context; writes stay in the overlay; a `kubectl set image` lands on the overlay's + own `images:` entry. This is what render-root scoping shipped for. +- `path: apps/podinfo` — **also accepted.** Two render roots inside one target. `base/deployment.yaml` + is now reachable from both, with differing override chains, so per `configuration.md:779-784` + it is **ambiguous** and the writer silently degrades to plain in-place editing of the base — + which means a `kubectl scale` in *prod* rewrites the **base**, changing test too. + +Both report `GitPathAccepted=True` and `Ready=True`. Nothing in `kubectl get gittarget` +distinguishes them. The degradation is a debug log line. + +That is the single most important example in this document, and it is the concrete form of the +original doubt. The proposal addresses it on three sides: + +1. **B5 / scan-repo** tells you `apps/podinfo/overlays/prod` and `…/test` are the GitTargets, + before you apply anything. +2. **B2 / `status.layout`** makes the wrong choice visible after you apply: + `kind: Mixed, renderRoots: 2, ambiguousDocuments: 1`. +3. **B1 / `mode: Observe`** means the wrong choice costs you a status read, not a commit that + changed the wrong environment. + +Optionally a fourth: a `spec.expect.layout: KustomizeOverlay` that refuses when the observed +layout differs. I would **not** ship that yet — `status.layout` first, and see whether anyone +wants to gate on it. Publish the observation before inventing the assertion. + +### C3. A bundle file and a new namespace — the inference problem + +```text +clusters/prod/ + all.yaml # 9 ConfigMaps in one multi-document file, + # spanning namespaces team-a and team-b + team-a/secrets/db.sops.yaml +``` + +The two namespaces are load-bearing, not decoration. Per `configuration.md:652-655`, a resource +in a namespace the target has never written joins an existing cohort **only** when that cohort +has proven itself namespace-agnostic by already holding more than one — one directory holding one +namespace is indistinguishable from a per-namespace layout whose second namespace has not arrived +yet. So with a single-namespace `all.yaml` the answer below inverts, and the new ConfigMap takes +the canonical path. + +Today (`configuration.md:638-646`): a new ConfigMap is **appended to `all.yaml`**, including one +in a brand-new `billing` namespace, because the bundle has cleared that bar. A new Secret goes to +`team-a/secrets/`. Both are the right calls, and both are *inferred from mutable repo state*. + +The failure mode is not the logic; it is that a **human editing the repo changes the operator's +behaviour without touching any Kubernetes object.** Delete every `team-b` ConfigMap from +`all.yaml` and the bundle stops being namespace-agnostic, so the next ConfigMap in a new namespace +takes the canonical path instead. Nothing in the GitTarget changed. Nothing in its status says the +placement basis moved — and this is a sharper version of the same point, because the file the +inference reads is still there and still full of ConfigMaps. + +With B3, a team that finds this unacceptable writes one field: + +```yaml +spec: + placement: + mode: Declared # never read the repo's mind + byType: + v1/configmaps: "{namespace}/configmaps.yaml" + v1/secrets: "{namespace}/secrets/{name}.yaml" +``` + +…and gets the same practical layout, permanently, from a declaration rather than an inference. +Inference stays the default and stays excellent for "point me at an existing repo and it just +works" — which remains the product's best demo. + +--- + +## Part D — Ordered plan + +### Wave 0 — docs, no code (this branch has started it) + +1. ✅ Fix the four factual/link errors (A1). +2. Add `RenderMatchesLive`, `GitProviderReady`, `ClusterProviderReady` to `configuration.md`'s + status list, with one line each (A2). +3. Write `docs/onboarding.md` around `manifest-analyzer --mode scan-repo`, with the C2 repo as + the worked example. Link it from README, `configuration.md`, `architecture.md` (B5). +4. Label every knob in the attribution section with its surface: CRD field / Helm value / + controller flag (A4). +5. Link `support-contract.md` from the top of `architecture.md` and add the support-boundary + tree to its design-document list (A5). + +### Wave 1 — the breaking API wave, before any release + +All of these change or add a spec field. Do them together, in one `feat(api)!` sequence, while +still `v1alpha3` and while there are no users. + +1. **B4** — `push.commitWindow` and `commit.message` move `GitProvider` → `GitTarget`. +2. **B1** — `GitTarget.spec.mode: Observe|Write`; chart quickstart renders `Observe`. +3. **F6** — `spec.suspend` on the four reconciled kinds; `spec.interval` on GitProvider; + requeue jitter; `reconcile.configbutler.ai/requestedAt` + `status.lastHandledReconcileAt`. +4. **B3** — `spec.placement.mode: Infer|Declared|Strict`. +5. **F10** — CommitRequest TTL/ownerRef + the `delete` verb. + +### Wave 2 — legibility, non-breaking, ships any time after + +1. **B2** — `GitTarget.status.layout`, including `ambiguousDocuments`. Highest value per line + of code in this document. +2. The `default` ClusterProvider not-found message names the fix (B6). +3. **F9** — the one envtest. +4. `configuration.md` splits into reference + rationale + guides (A3). + +### Explicitly not now + +- `spec.expect.layout` — publish the observation (B2) before inventing the assertion. +- Making `Observe` the global default — revisit once there is one real user's onboarding to + watch. +- Ordered placement rules (Option A of the placement design) — `byType` + `mode` covers the + demand we can actually see. diff --git a/pkg/manifestanalyzer/repo.go b/pkg/manifestanalyzer/repo.go index 8144f9fc..b40a7a05 100644 --- a/pkg/manifestanalyzer/repo.go +++ b/pkg/manifestanalyzer/repo.go @@ -21,8 +21,10 @@ const ( // resources graph stays within its own subtree. Accepted. LayoutKustomizeSingle Layout = "kustomize-single" // LayoutKustomizeOverlay is a render root reaching a base outside its own subtree - // (the classic base/ + overlays/{env} shape). Refused today, with a forward-looking - // reason: it becomes accepted when render-root scoping ships. + // (the classic base/ + overlays/{env} shape). Accepted: render-root scoping shipped, + // so the base is read as read-only context and writes stay in the overlay. The + // candidate's editable count shows how much of what it renders it can own. An overlay + // refused for a real fault carries that fault's own code, never a forward-looking one. LayoutKustomizeOverlay Layout = "kustomize-overlay" // LayoutRefusedStructural is a render root whose kustomization uses a construct the // writer cannot map back to editable source. This is the permanent support boundary,