From b6ddf8e1f75ba0c9b958722277bcf420d4364ee5 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Wed, 15 Jul 2026 18:37:56 +0000 Subject: [PATCH 1/2] feat(acceptance): accept benign repo-hygiene passengers instead of refusing the folder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A GitTarget subtree is operator-exclusive: any non-YAML file that is not an operator artifact (README.md/.sops.yaml) or a build directive was refused as foreign, stalling the whole folder. For the reverser's core case — adopting an existing repo, which has no .gittargetignore — a stray LICENSE, .gitkeep, or non-README markdown refused everything on first scan. Add a closed benign-passenger set accepted by default (isBenignPassenger, RoleBenignPassenger): *.md/*.markdown, LICENSE/LICENCE/COPYING/NOTICE, and .gitignore/.gitattributes/.gitkeep/.keep. Matched after the .gittargetignore filter so it stays user-suppressible and never muddies the operator-artifact role. Loose application data (notes.txt, values.json, deploy.sh), symlinks, and submodules stay refused — the acceptance ratchet holds because the set holds only files no future "own the subtree" behaviour would ever claim. Reverses the original minimal-set decision for the adoption case; recorded as D-foreign-7 in the stringency spec (role 3b). Regenerated corpus baseline shows repo-per-environment's three env folders flip refused→accepted (each blocked only by a .gitignore) and mixed-and-hostile's .gitkeep no longer counts, while deployment.json correctly stays foreign. Co-Authored-By: Claude Opus 4.8 (1M context) --- .coverage-baseline | 2 +- docs/design/support-boundary/README.md | 1 + .../gitpath-foreign-content-stringency.md | 45 ++++++++-- .../.gittargetignore | 10 ++- internal/git/gittargetignore_writer_test.go | 20 +++++ internal/git/plan_flush.go | 2 +- internal/manifestanalyzer/analyzer.go | 2 +- internal/manifestanalyzer/gittargetignore.go | 48 +++++++++- .../manifestanalyzer/gittargetignore_test.go | 87 +++++++++++++++++-- internal/manifestanalyzer/scan_repo.go | 6 +- test/fixtures/gitops-layouts/support-today.md | 12 +-- 11 files changed, 203 insertions(+), 32 deletions(-) diff --git a/.coverage-baseline b/.coverage-baseline index f7a6ad0b..a5216746 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -76.3 +76.4 diff --git a/docs/design/support-boundary/README.md b/docs/design/support-boundary/README.md index 00231a31..6e99f311 100644 --- a/docs/design/support-boundary/README.md +++ b/docs/design/support-boundary/README.md @@ -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. diff --git a/docs/spec/gitpath-foreign-content-stringency.md b/docs/spec/gitpath-foreign-content-stringency.md index 9376373e..bb7c5a91 100644 --- a/docs/spec/gitpath-foreign-content-stringency.md +++ b/docs/spec/gitpath-foreign-content-stringency.md @@ -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 @@ -148,6 +149,11 @@ ACCEPTED under spec.path 3. Operator artifact .sops.yaml, README.md (basename) + (WriterAllowlist + …) /.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) @@ -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 @@ -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 diff --git a/internal/git/bootstrapped-repo-template/.gittargetignore b/internal/git/bootstrapped-repo-template/.gittargetignore index b19509db..0df25e2c 100644 --- a/internal/git/bootstrapped-repo-template/.gittargetignore +++ b/internal/git/bootstrapped-repo-template/.gittargetignore @@ -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, @@ -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 diff --git a/internal/git/gittargetignore_writer_test.go b/internal/git/gittargetignore_writer_test.go index 0678179e..68b5baa7 100644 --- a/internal/git/gittargetignore_writer_test.go +++ b/internal/git/gittargetignore_writer_test.go @@ -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 TestWriter_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)) +} diff --git a/internal/git/plan_flush.go b/internal/git/plan_flush.go index 14fd544f..981a2326 100644 --- a/internal/git/plan_flush.go +++ b/internal/git/plan_flush.go @@ -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) diff --git a/internal/manifestanalyzer/analyzer.go b/internal/manifestanalyzer/analyzer.go index fa6820f1..956babb5 100644 --- a/internal/manifestanalyzer/analyzer.go +++ b/internal/manifestanalyzer/analyzer.go @@ -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) diff --git a/internal/manifestanalyzer/gittargetignore.go b/internal/manifestanalyzer/gittargetignore.go index 89095171..233dfe12 100644 --- a/internal/manifestanalyzer/gittargetignore.go +++ b/internal/manifestanalyzer/gittargetignore.go @@ -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. @@ -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. @@ -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 } @@ -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 diff --git a/internal/manifestanalyzer/gittargetignore_test.go b/internal/manifestanalyzer/gittargetignore_test.go index a7f436e6..6cefd987 100644 --- a/internal/manifestanalyzer/gittargetignore_test.go +++ b/internal/manifestanalyzer/gittargetignore_test.go @@ -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) } @@ -60,6 +60,81 @@ func TestForeignContent_NonYAMLFileRefused(t *testing.T) { } } +func TestBenignPassenger_AcceptedByDefault(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", "CONTRIBUTING.md", "sub/.gitkeep", ".gitignore"} { + 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 TestBenignPassenger_StillUserSuppressible(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(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. diff --git a/internal/manifestanalyzer/scan_repo.go b/internal/manifestanalyzer/scan_repo.go index f3573f0c..19b4bd4d 100644 --- a/internal/manifestanalyzer/scan_repo.go +++ b/internal/manifestanalyzer/scan_repo.go @@ -87,7 +87,8 @@ type ResourceCounts struct { Editable int `json:"editable"` // NonKRM is the number of non-KRM YAML documents and foreign (non-YAML/symlink) // entries in the candidate's own subtree. Retained build directives (kustomization - // files) are neither KRM nor NonKRM and are not counted. + // files), operator artifacts (README.md), and accepted benign passengers (a license, + // docs, .gitignore) are neither KRM nor NonKRM and are not counted. NonKRM int `json:"nonKrm"` } @@ -561,7 +562,8 @@ func reachedResourceFilesFrom(rootDir string, kusts map[string]*kustomizationDoc } // nonKRMUnder counts non-KRM YAML documents and foreign entries under dir. Retained -// build directives are excluded (they are neither KRM nor noise). +// build directives, operator artifacts, and accepted benign passengers are excluded +// (they are neither KRM nor noise). func nonKRMUnder(store *ManifestStore, dir string) int { n := 0 for _, d := range store.Diagnostics { diff --git a/test/fixtures/gitops-layouts/support-today.md b/test/fixtures/gitops-layouts/support-today.md index 539d87c8..f0e1240b 100644 --- a/test/fixtures/gitops-layouts/support-today.md +++ b/test/fixtures/gitops-layouts/support-today.md @@ -27,7 +27,7 @@ Reading rules: | 1-desired-state/argocd-app-of-apps | 0 | All reported candidates accepted | 4 | 0 | plain=4 | - | None | | 1-desired-state/argocd-plain | 0 | Partial | 1 | 1 | plain=2 | - | non-krm-yaml: ci-metadata.yaml: YAML is not a Kubernetes manifest | | 1-desired-state/flux-monorepo | 0 | Partial | 4 | 2 | kustomize-overlay=2, kustomize-single=4 | - | overlay-fan-out-unsupported: base "apps/base/frontend" is read from outside this folder's subtree and is shared by 2 render root(s); render-root scoping required
overlay-fan-out-unsupported: base "apps/base/frontend" is read from outside this folder's subtree and is shared by 2 render root(s); render-root scoping required | -| 1-desired-state/repo-per-environment | 0 | Partial | 6 | 3 | plain=9 | - | foreign-file: .gitignore: foreign file .gitignore is not a managed manifest; remove it or name it in .gittargetignore
foreign-file: .gitignore: foreign file .gitignore is not a managed manifest; remove it or name it in .gittargetignore
foreign-file: .gitignore: foreign file .gitignore is not a managed manifest; remove it or name it in .gittargetignore | +| 1-desired-state/repo-per-environment | 0 | All reported candidates accepted | 9 | 0 | plain=9 | - | None | | 2-rendered/argocd-external-helm | 0 | Partial | 2 | 1 | plain=3 | - | non-krm-yaml: values.yaml: YAML is not a Kubernetes manifest | | 2-rendered/helm-chart | 0 | All reported candidates accepted | 1 | 0 | plain=1 | - | None | | 2-rendered/helm-environment-values | 0 | All reported candidates accepted | 1 | 0 | plain=1 | - | None | @@ -82,18 +82,18 @@ Unsupported constructs: `none`. Fleet root: `false`. ## 1-desired-state/repo-per-environment -Reported rc `0`. Accepted `6`, refused `3`. +Reported rc `0`. Accepted `9`, refused `0`. Unsupported constructs: `none`. Fleet root: `false`. | Candidate | Layout | Accepted today | Namespace | rendered/editable/non-KRM | Refusal reasons | |---|---|---|---|---|---| -| `gitops-dev` | `plain` | false | `-` | 6/6/1 | foreign-file: .gitignore: foreign file .gitignore is not a managed manifest; remove it or name it in .gittargetignore | +| `gitops-dev` | `plain` | true | `-` | 6/6/0 | none | | `gitops-dev/apps/backend` | `plain` | true | `backend-dev` | 2/2/0 | none | | `gitops-dev/apps/frontend` | `plain` | true | `frontend-dev` | 2/2/0 | none | -| `gitops-production` | `plain` | false | `-` | 7/7/1 | foreign-file: .gitignore: foreign file .gitignore is not a managed manifest; remove it or name it in .gittargetignore | +| `gitops-production` | `plain` | true | `-` | 7/7/0 | none | | `gitops-production/apps/backend` | `plain` | true | `backend-production` | 2/2/0 | none | | `gitops-production/apps/frontend` | `plain` | true | `frontend-production` | 3/3/0 | none | -| `gitops-staging` | `plain` | false | `-` | 6/6/1 | foreign-file: .gitignore: foreign file .gitignore is not a managed manifest; remove it or name it in .gittargetignore | +| `gitops-staging` | `plain` | true | `-` | 6/6/0 | none | | `gitops-staging/apps/backend` | `plain` | true | `backend-staging` | 2/2/0 | none | | `gitops-staging/apps/frontend` | `plain` | true | `frontend-staging` | 2/2/0 | none | @@ -254,7 +254,7 @@ Unsupported constructs: `unparseable`. Fleet root: `false`. | Candidate | Layout | Accepted today | Namespace | rendered/editable/non-KRM | Refusal reasons | |---|---|---|---|---|---| -| `.` | `refused-structural` | false | `backend` | 0/0/6 | refused-structural: kustomization uses unsupported feature(s): unparseable (invalid Kustomization: json: unknown field "spec") | +| `.` | `refused-structural` | false | `backend` | 0/0/5 | refused-structural: kustomization uses unsupported feature(s): unparseable (invalid Kustomization: json: unknown field "spec") | | `crossplane` | `plain` | true | `-` | 1/1/0 | none | | `kro` | `plain` | true | `-` | 1/1/0 | none | | `mixed` | `plain` | false | `-` | 3/3/2 | impure-managed-file: bundle.yaml: a file with managed resources may contain only valid KRM documents; document #1 is a non-KRM document
impure-managed-file: bundle.yaml: a file with managed resources may contain only valid KRM documents; document #2 is an empty document
foreign-file: deployment.json: foreign file deployment.json is not a managed manifest; remove it or name it in .gittargetignore | From 5c7dbaf3c6858dbdc251833c04729b9c07da2621 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Wed, 15 Jul 2026 18:52:18 +0000 Subject: [PATCH 2/2] test: address benign passenger review feedback --- internal/git/gittargetignore_writer_test.go | 2 +- internal/manifestanalyzer/gittargetignore_test.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/git/gittargetignore_writer_test.go b/internal/git/gittargetignore_writer_test.go index 68b5baa7..f5404e9d 100644 --- a/internal/git/gittargetignore_writer_test.go +++ b/internal/git/gittargetignore_writer_test.go @@ -93,7 +93,7 @@ func TestWriter_IgnoredForeignFileAllowsWrite(t *testing.T) { // 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 TestWriter_BenignPassengerAllowsWrite(t *testing.T) { +func TestCommitPendingWrites_BenignPassengerAllowsWrite(t *testing.T) { tempDir := t.TempDir() remotePath := tempDir + "/remote.git" createBareRepo(t, remotePath) diff --git a/internal/manifestanalyzer/gittargetignore_test.go b/internal/manifestanalyzer/gittargetignore_test.go index 6cefd987..c9b12ace 100644 --- a/internal/manifestanalyzer/gittargetignore_test.go +++ b/internal/manifestanalyzer/gittargetignore_test.go @@ -60,7 +60,7 @@ func TestForeignContent_NonYAMLFileRefused(t *testing.T) { } } -func TestBenignPassenger_AcceptedByDefault(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. @@ -85,7 +85,9 @@ func TestBenignPassenger_AcceptedByDefault(t *testing.T) { // 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", "CONTRIBUTING.md", "sub/.gitkeep", ".gitignore"} { + 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) } @@ -95,7 +97,7 @@ func TestBenignPassenger_AcceptedByDefault(t *testing.T) { } } -func TestBenignPassenger_StillUserSuppressible(t *testing.T) { +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). @@ -113,7 +115,7 @@ func TestBenignPassenger_StillUserSuppressible(t *testing.T) { } } -func TestIsBenignPassenger(t *testing.T) { +func TestIsBenignPassenger_AcceptedAndRefused(t *testing.T) { accepted := []string{ "LICENSE", "LICENSE.txt", "LICENCE", "COPYING", "NOTICE", ".gitignore", ".gitattributes", ".gitkeep", ".keep",