Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .coverage-baseline
Original file line number Diff line number Diff line change
@@ -1 +1 @@
76.3
76.4
1 change: 1 addition & 0 deletions docs/design/support-boundary/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ flowchart LR
| Path-based strategic-merge patch | The render is accepted; the patch is read-only context. We tolerate it but do not author or edit patches yet. |
| Render verification | A proposed batch is built with kustomize before any bytes are written. Mismatch or blast-radius change refuses the flush. |
| Write boundary | Writes never leave `spec.path`, and a file read by more than one render root is never edited in place. |
| Foreign-content boundary | A GitTarget subtree is operator-exclusive: loose scripts, binaries, and symlinks refuse the folder. Inert repo-hygiene passengers — documentation (`*.md`), a license, and `.gitignore`/`.gitattributes`/`.gitkeep` — are accepted so adopting an existing repo does not stall on them. Anything else is named in a root `.gittargetignore`. |

The remaining overlay gap is deliberately narrow: creating a **new object** and adding its
`resources:` entry needs a placement/write-path correction. It is planned, not shipped.
Expand Down
45 changes: 37 additions & 8 deletions docs/spec/gitpath-foreign-content-stringency.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
> landed. Where it lives:
> - **Matcher, foreign classification, denylist, role policy** —
> [internal/manifestanalyzer/gittargetignore.go](../../internal/manifestanalyzer/gittargetignore.go)
> (`ClassifyEntry`, `LoadGitTargetIgnore`, `IgnoreMatcher`, `foreignContentRefusals`).
> (`ClassifyEntry`, `LoadGitTargetIgnore`, `IgnoreMatcher`, `foreignContentRefusals`,
> `isBenignPassenger` — the closed benign-passenger set added 2026-07-15 under D-foreign-7).
> - **Scan wiring** — `collectFiles`
> ([analyzer.go](../../internal/manifestanalyzer/analyzer.go)) and the writer's
> `scanWorktreeSubtree` ([plan_flush.go](../../internal/git/plan_flush.go)) both produce a
Expand Down Expand Up @@ -148,6 +149,11 @@ ACCEPTED under spec.path
3. Operator artifact .sops.yaml, README.md (basename) + (WriterAllowlist + …)
<spec.path>/.gittargetignore (ROOT only) —
operator-authored, retained, passive
3b. Benign passenger a closed set of inert repo-hygiene basenames: (isBenignPassenger, §6)
*.md / *.markdown, LICENSE / COPYING / NOTICE,
.gitignore / .gitattributes / .gitkeep / .keep —
USER-authored, accepted, never managed; matched
AFTER the ignore filter, so still suppressible
4. User-ignored anything matching the root .gittargetignore — (§4, NEW)
NEVER READ
5. (empty directories) harmless; git does not track them (ignore)
Expand Down Expand Up @@ -403,13 +409,21 @@ semantics rather than reinventing glob handling.
and `.gittargetignore` must be added. Audit the full bootstrap template
([bootstrapped_repo_template.go](../../internal/git/bootstrapped_repo_template.go)) for any other
non-KRM file it stages and allowlist each one explicitly.
- **Keep the hardcoded accepted set minimal; push "common benign" files into the bootstrapped
`.gittargetignore`.** Rather than hardcoding `LICENSE` / `.gitignore` / `.gitattributes` into the operator's
allowlist (they are *user* content, not operator artifacts, so they would muddy role 3), the bootstrapped
`.gittargetignore` (§4.2) ships them as **commented example patterns**. A user who adds a `LICENSE`
uncomments one line — an in-repo, discoverable, versioned fix — instead of relying on operator-baked API
behaviour. The hardcoded set stays exactly the operator's own artifacts + build directives. (Supersedes
the old "starter allowlist" idea; see D-foreign-3.)
- **Accept a closed set of inert repo-hygiene passengers by default; keep everything else in the
`.gittargetignore` escape hatch (D-foreign-7, supersedes the original minimal-set stance).** The
original decision hardcoded *nothing* beyond the operator's own artifacts and pushed `LICENSE` /
`.gitignore` / `.gitattributes` into the bootstrapped `.gittargetignore` as commented examples. That is
correct for a folder the operator *bootstraps*, but wrong for the reverser's core case — **adopting an
existing repo**, which has no `.gittargetignore`, so a stray `LICENSE`, a `.gitkeep`, or a non-README
`.md` refuses the whole folder on first scan. So a small, closed **benign-passenger** set is now accepted
by default (`isBenignPassenger`, role 3b): `*.md` / `*.markdown`, `LICENSE` / `LICENCE` / `COPYING` /
`NOTICE`, and `.gitignore` / `.gitattributes` / `.gitkeep` / `.keep`. This does **not** break the ratchet
(§1): the set holds only files no future "own the subtree" behaviour would ever want to claim — loose
application data (`notes.txt`, `values.json`, `deploy.sh`) stays foreign and refused. These are *user*
content, not operator artifacts, so they are matched **after** the ignore filter (role 3b, not role 3):
a user can still `.gittargetignore` one, and it can never shadow or be confused with an operator artifact.
The bootstrapped `.gittargetignore` keeps its examples for the *non*-hygiene passengers a user might still
want (a `docs/` subtree, a kept-but-unmanaged YAML).
- **Symlinks: refuse, do not skip.** A writer that materializes into a folder containing a symlink can
follow it out of the subtree; silently skipping it hides a real hazard. A user who genuinely wants a
symlink left alone can `.gittargetignore` it — explicit and versioned — rather than us tolerating it
Expand Down Expand Up @@ -507,6 +521,21 @@ semantics rather than reinventing glob handling.
out prominently in that feature's design. Optional later hardening (raw-scan read of ignored files) closes
the manually-moved-file residual edge.

### Amended later (2026-07-15)

- **D-foreign-7 — accept a closed benign-passenger set by default — DECIDED (§3 role 3b, §6).** Supersedes
the original "hardcode nothing beyond operator artifacts" stance for a small, closed set of inert
repo-hygiene files: `*.md` / `*.markdown`, `LICENSE` / `LICENCE` / `COPYING` / `NOTICE`, and
`.gitignore` / `.gitattributes` / `.gitkeep` / `.keep` (`isBenignPassenger`). Motivation: **adopting an
existing repo** — the reverser's core use case — has no `.gittargetignore`, so refusing the whole folder
over a `LICENSE` or a `.gitkeep` is a bad first-scan experience. Ratchet-safe because the set holds only
files no future "own the subtree" behaviour (wrap-to-ConfigMap, faithful sweep) would ever claim; loose
application data stays foreign. Modeled as a **distinct role from operator artifacts** (matched *after*
the ignore filter, so still user-suppressible and never able to shadow an operator artifact), which keeps
role 3's "operator-authored, un-suppressible" semantics clean — the exact modeling concern the original
minimal-set decision raised. The bootstrapped `.gittargetignore` keeps its examples for the remaining
non-hygiene passengers (a `docs/` subtree, a kept-but-unmanaged YAML).

---

## 9. One-paragraph summary
Expand Down
10 changes: 6 additions & 4 deletions internal/git/bootstrapped-repo-template/.gittargetignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
# Shipped fully commented: out of the box it ignores NOTHING, so the strict default stays in
# force until you deliberately opt a file out. Uncomment or add patterns as needed.
#
# You do NOT need to list common repo-hygiene files here — documentation (*.md, *.markdown),
# a license (LICENSE / COPYING / NOTICE), and Git metadata (.gitignore / .gitattributes /
# .gitkeep) are accepted automatically. This file is for the OTHER passengers: a whole
# subtree to leave alone, or a non-managed YAML you want kept in the repo.
#
# WARNING — never ignore a path the operator writes (a managed manifest). Doing so would
# blind the operator to its own file: it could not update or delete it, and would re-create
# it forever. The operator refuses any flush whose write path a pattern here would shadow,
Expand All @@ -16,9 +21,6 @@
#
# Examples (uncomment the ones you need):
#
# LICENSE # a license file kept in the mirror
# .gitignore # a hand-maintained git ignore
# .gitattributes
# docs/ # a hand-maintained docs subtree the operator should leave alone
# *.md # loose markdown notes
# scratch/ # a working directory kept out of the mirror
# legacy/old-config.yaml # a YAML file kept in the repo but not operator-managed
20 changes: 20 additions & 0 deletions internal/git/gittargetignore_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,23 @@ func TestWriter_IgnoredForeignFileAllowsWrite(t *testing.T) {

require.NoError(t, worker.commitPendingWrites([]PendingWrite{*pendingWrite}, false))
}

// TestWriter_BenignPassengerAllowsWrite proves the live writer accepts a folder whose only
// non-managed content is an inert hygiene passenger (a LICENSE) — without any
// .gittargetignore — so adopting an existing repo does not stall on repo-hygiene files.
func TestCommitPendingWrites_BenignPassengerAllowsWrite(t *testing.T) {
tempDir := t.TempDir()
remotePath := tempDir + "/remote.git"
createBareRepo(t, remotePath)
remoteURL := "file://" + remotePath

simulateClientCommitOnDisk(t, remoteURL, "main", "LICENSE", "Apache-2.0")

worker, err := newTestBranchWorker(remoteURL, "test-repo", "main")
require.NoError(t, err)
event := createTestEvent(t, "pod-c")
pendingWrite, err := worker.buildGroupedPendingWrite(worker.ctx, []Event{event})
require.NoError(t, err)

require.NoError(t, worker.commitPendingWrites([]PendingWrite{*pendingWrite}, false))
}
2 changes: 1 addition & 1 deletion internal/git/plan_flush.go
Original file line number Diff line number Diff line change
Expand Up @@ -1459,7 +1459,7 @@ func scanWorktreeSubtree(absBase string) (manifestanalyzer.FolderScan, error) {
return readErr
}
scan.YAMLFiles = append(scan.YAMLFiles, manifestedit.FileContent{Path: rel, Content: content})
case manifestanalyzer.RoleOperatorArtifact:
case manifestanalyzer.RoleOperatorArtifact, manifestanalyzer.RoleBenignPassenger:
scan.NonYAML = append(scan.NonYAML, rel)
case manifestanalyzer.RoleForeignFile:
scan.NonYAML = append(scan.NonYAML, rel)
Expand Down
2 changes: 1 addition & 1 deletion internal/manifestanalyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func collectFiles(fsys fs.FS) FolderScan {
return nil //nolint:nilerr // an unreadable file must not abort the whole scan
}
scan.YAMLFiles = append(scan.YAMLFiles, manifestedit.FileContent{Path: path, Content: content})
case RoleOperatorArtifact:
case RoleOperatorArtifact, RoleBenignPassenger:
scan.NonYAML = append(scan.NonYAML, path)
case RoleForeignFile:
scan.NonYAML = append(scan.NonYAML, path)
Expand Down
48 changes: 45 additions & 3 deletions internal/manifestanalyzer/gittargetignore.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ const (
// RoleOperatorArtifact is an accepted non-YAML operator artifact (README.md). It is
// listed in the report's non-YAML inventory but is never foreign.
RoleOperatorArtifact
// RoleBenignPassenger is an accepted non-YAML file the operator never manages but that a
// careful human keeps in a GitOps folder: documentation (*.md / *.markdown), a license
// (LICENSE / COPYING / NOTICE), and Git-hygiene metadata (.gitignore / .gitattributes /
// .gitkeep / .keep). Unlike an operator artifact it is USER content, so it is recognized
// AFTER the root .gittargetignore filter (a user may still ignore it) rather than being
// protected from it. It is listed in the non-YAML inventory and is never foreign, so its
// mere presence no longer refuses the whole folder — the reverser can adopt an existing
// repo without a human first hand-writing a .gittargetignore for its README and license.
RoleBenignPassenger
// RoleForeignFile is a foreign non-YAML regular file: refused.
RoleForeignFile
// RoleForeignSymlink is a foreign symlink: refused.
Expand All @@ -86,11 +95,14 @@ const (
// matcher (nil when the path carries no .gittargetignore). It is a pure function — the
// single source of truth for the precedence in §4.1 of the design:
//
// operator artifacts + build directives → root .gittargetignore filter → managed KRM / foreign
// operator artifacts + build directives → root .gittargetignore filter →
// managed KRM → benign passenger → foreign
//
// so a user cannot use .gittargetignore to hide the operator's own files (README.md,
// .sops.yaml) or to silence a hard-kustomize refusal (kustomization.yaml), while every
// other unknown non-YAML entry is refused unless an ignore pattern names it.
// .sops.yaml) or to silence a hard-kustomize refusal (kustomization.yaml). Benign-passenger
// hygiene files (a license, docs, .gitignore) are accepted after the ignore filter — so they
// no longer refuse a folder yet remain user-suppressible — while every other unknown non-YAML
// entry is refused unless an ignore pattern names it.
func ClassifyEntry(rel string, d fs.DirEntry, ignore *IgnoreMatcher) EntryRole {
// Symlinks are foreign wherever they appear and whatever they are named — a writer
// could follow one out of the subtree. The only way to keep one is to ignore it.
Expand Down Expand Up @@ -128,6 +140,9 @@ func ClassifyEntry(rel string, d fs.DirEntry, ignore *IgnoreMatcher) EntryRole {
if isYAMLFile(rel) {
return RoleManagedYAML
}
if isBenignPassenger(rel) {
return RoleBenignPassenger
}
return RoleForeignFile
}

Expand All @@ -150,6 +165,33 @@ func isRecognizedArtifact(path string) bool {
// an operator artifact (role 3). It mirrors the constant the bootstrap template uses.
const sopsConfigBasename = ".sops.yaml"

// isBenignPassenger reports whether a non-YAML file is inert repo-hygiene the operator
// accepts by default without managing it: documentation, a license, or Git metadata. The
// set is deliberately small and closed. The acceptance ratchet
// (docs/spec/gitpath-foreign-content-stringency.md §1) makes accepting a shape irreversible,
// so this holds only files no future "own the subtree" behaviour (a wrap-to-ConfigMap, a
// faithful sweep) would ever want to claim — never loose application data such as notes.txt,
// values.json, or deploy.sh, which stay foreign and refused. Matching is by basename at any
// depth, mirroring how the equivalent .gittargetignore glob (e.g. `*.md`, `LICENSE`) matches.
func isBenignPassenger(path string) bool {
base := filepathBase(path)
switch base {
case ".gitignore", ".gitattributes", ".gitkeep", ".keep",
"LICENSE", "LICENSE.txt", "LICENCE", "LICENCE.txt",
"COPYING", "COPYING.txt", "NOTICE", "NOTICE.txt":
return true
}
return hasMarkdownExt(base)
}

// hasMarkdownExt reports whether a basename is a Markdown document. It stays case-sensitive
// on the lower-case extensions so it matches the way a `*.md` .gittargetignore glob behaves
// on a Linux checkout — the operator's own README.md is already handled earlier as an
// operator artifact, so this only classifies the remaining loose documentation.
func hasMarkdownExt(base string) bool {
return strings.HasSuffix(base, ".md") || strings.HasSuffix(base, ".markdown")
}

// IgnoreMatcher is the parsed, active root .gittargetignore: a go-git gitignore matcher
// plus the raw patterns it was built from. It is reused git's own matching semantics
// rather than reinventing glob handling. A nil *IgnoreMatcher matches nothing, so callers
Expand Down
89 changes: 83 additions & 6 deletions internal/manifestanalyzer/gittargetignore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,16 @@ func accHasIssue(acc Acceptance, kind IssueKind, path string) bool {

func TestForeignContent_NonYAMLFileRefused(t *testing.T) {
fsys := fstest.MapFS{
"deploy.yaml": {Data: []byte(deployYAML)},
"secrets.txt": {Data: []byte("db-password=hunter2")},
"deploy.sh": {Data: []byte("#!/bin/sh\n")},
"blob.bin": {Data: []byte{0x00, 0x01}},
"sub/notes.md": {Data: []byte("# notes")},
"deploy.yaml": {Data: []byte(deployYAML)},
"secrets.txt": {Data: []byte("db-password=hunter2")},
"deploy.sh": {Data: []byte("#!/bin/sh\n")},
"blob.bin": {Data: []byte{0x00, 0x01}},
"sub/values.json": {Data: []byte(`{"k":"v"}`)},
}
store := BuildStore(context.Background(), fsys, nil)

got := foreignPaths(store)
for _, want := range []string{"secrets.txt", "deploy.sh", "blob.bin", "sub/notes.md"} {
for _, want := range []string{"secrets.txt", "deploy.sh", "blob.bin", "sub/values.json"} {
if got[want] != ForeignFile {
t.Errorf("path %q: foreign kind = %q, want %q", want, got[want], ForeignFile)
}
Expand All @@ -60,6 +60,83 @@ func TestForeignContent_NonYAMLFileRefused(t *testing.T) {
}
}

func TestCollectFiles_BenignPassengerAcceptedByDefault(t *testing.T) {
// Inert repo-hygiene files — docs, a license, and Git metadata — are accepted without a
// .gittargetignore, so adopting an existing repo does not refuse the whole folder over a
// LICENSE, a stray .gitkeep, or documentation that is not the operator's own README.
fsys := fstest.MapFS{
"deploy.yaml": {Data: []byte(deployYAML)},
"LICENSE": {Data: []byte("Apache-2.0")},
"COPYING": {Data: []byte("legal")},
"CONTRIBUTING.md": {Data: []byte("# contributing")},
"docs/guide.markdown": {Data: []byte("# guide")},
".gitignore": {Data: []byte("*.log\n")},
".gitattributes": {Data: []byte("*.yaml text\n")},
"sub/.gitkeep": {Data: []byte("")},
}
store := BuildStore(context.Background(), fsys, nil)

if len(store.Foreign) != 0 {
t.Fatalf("benign-passenger hygiene files must not be foreign; got %+v", store.Foreign)
}
if acc := AcceptStructureOnly(store); !acc.Accepted {
t.Errorf("expected acceptance for a folder of hygiene passengers; got %+v", acc.Issues)
}
// They are still recorded in the non-YAML inventory (accepted, never managed), and the
// managed manifest is modeled as usual.
scan := collectFiles(fsys)
for _, want := range []string{
"LICENSE", "COPYING", "CONTRIBUTING.md", "docs/guide.markdown", ".gitignore", ".gitattributes", "sub/.gitkeep",
} {
if !containsString(scan.NonYAML, want) {
t.Errorf("%q should appear in the non-YAML inventory; got %+v", want, scan.NonYAML)
}
}
if _, ok := store.FilesByPath["deploy.yaml"]; !ok {
t.Error("the managed manifest must still be modeled alongside benign passengers")
}
}

func TestCollectFiles_BenignPassengerStillUserSuppressible(t *testing.T) {
// A benign passenger is USER content, matched after the ignore filter, so a user can still
// .gittargetignore it to drop it from the inventory entirely (unlike an operator artifact,
// which survives an ignore rule).
fsys := fstest.MapFS{
"deploy.yaml": {Data: []byte(deployYAML)},
"NOTES.md": {Data: []byte("# notes")},
".gittargetignore": {Data: []byte("NOTES.md\n")},
}
scan := collectFiles(fsys)
if containsString(scan.NonYAML, "NOTES.md") {
t.Error("a benign passenger named in .gittargetignore must be dropped (never read)")
}
if len(scan.Foreign) != 0 {
t.Errorf("an ignored passenger is not foreign; got %+v", scan.Foreign)
}
}

func TestIsBenignPassenger_AcceptedAndRefused(t *testing.T) {
accepted := []string{
"LICENSE", "LICENSE.txt", "LICENCE", "COPYING", "NOTICE",
".gitignore", ".gitattributes", ".gitkeep", ".keep",
"README.md", "notes.md", "a/b/guide.markdown",
}
for _, p := range accepted {
if !isBenignPassenger(p) {
t.Errorf("isBenignPassenger(%q) = false, want true", p)
}
}
refused := []string{
"notes.txt", "values.json", "deploy.sh", "Chart.yaml", "blob.bin",
"license", "readme", "MD", "sub.markdown.tar",
}
for _, p := range refused {
if isBenignPassenger(p) {
t.Errorf("isBenignPassenger(%q) = true, want false", p)
}
}
}

func TestForeignContent_OperatorArtifactsAccepted(t *testing.T) {
// README.md is an operator artifact (role 3); the root .gittargetignore is recognized
// positionally; a deeply nested README.md is still basename-matched as an artifact.
Expand Down
Loading
Loading