Record why GitOps is ArgoCD, and empty the permissive default AppProject - #12
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Documents two GitOps/security-related decisions and hardens ArgoCD multi-tenancy by neutralizing the built-in permissive default AppProject so it cannot be used as an unrestricted deployment boundary.
Changes:
- Add an explicit
AppProjectmanifest overriding ArgoCD’s built-indefaultproject with empty allowlists. - Record decision log entry 13 (ArgoCD over Flux for this model) and entry 14 (empty the
defaultAppProject) indocs/decisions.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| k8s/argocd/projects/default.yaml | Defines an intentionally empty default AppProject to prevent unrestricted deployments via the built-in project. |
| docs/decisions.md | Adds decision entries documenting the ArgoCD-vs-Flux choice and the rationale for emptying the built-in default project. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
hakan-persson
added a commit
that referenced
this pull request
Aug 22, 2026
The header said docs/decisions.md entry 13, which is the ArgoCD-vs-Flux decision. The rationale for emptying the built-in project is entry 14. Written while the two entries were still being numbered. From Copilot's review of #12. The same slip was caught by hand on #9, where a comment said entry 11 for what became entry 10 — so this is a repeating failure mode rather than a one-off, and the fix is cheap to verify: every decisions.md entry pointer in the repo was checked against the actual headings, and all 19 now resolve to the right entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rgoCD Two decisions.md entries and one manifest, from questions asked at a presentation to potential project teams. Entry 12 records the ArgoCD-vs-Flux decision as settled. It was made 2026-08-12 and re-checked against the running cluster before being written down; both load-bearing claims still hold. The reasoning had never been recorded and the question had been asked twice with nothing to point at, which is the failure mode this file exists to prevent. The argument is multi-tenancy: an AppProject expresses "only these kinds, only these namespaces, only this repo" as one reviewable file, while Flux reaches the same place through per-project ServiceAccount impersonation, which scatters kind allowlisting across RBAC and drops the source-repo constraint entirely. The entry concedes where Flux would have been better and states the migration cost plainly rather than claiming lock-in. One argument in the original assessment has since reversed and is recorded as such: it noted that running ArgoCD without a GUI forfeits its main advantage. Project teams have now asked to see their own sync status, which ArgoCD serves through the Dex already deployed and Flux has no first-party answer for. Entry 13 and default.yaml close something the re-check surfaced. ArgoCD creates the `default` AppProject at startup permitting any repo, any namespace and every cluster-scoped kind, and upstream documents that it cannot be deleted, only modified, recommending it be emptied in multi-tenant setups. Left alone it is a fully permissive project sitting beside the whitelist that entry 12 identifies as the entire security boundary. Verified before writing: nothing references it (23 Applications on `infra`, 2 on `project-infra`) and project developers cannot create Applications at all, by impersonation. So this was not a live escalation, and the entry says so explicitly to stop a future reader over-reading it. The point is that "nobody uses it" is a weaker guarantee than "it cannot be used". A server-side dry-run applies cleanly. Numbered 12 and 13 after a rebase: PR #11 took entry 11 while this was open, so the original 13/14 would have left a gap at 12. Every decisions.md entry pointer in the repo was re-checked against the actual headings afterwards — 22 of them across README, docs and Bicep — and all resolve. Copilot caught one of these pointers being wrong on the first push, and the same slip had appeared once before on #9, so the audit is now the routine rather than the exception. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hakan-persson
force-pushed
the
fix/lock-default-appproject
branch
from
August 22, 2026 20:36
20c9a12 to
1b039dc
Compare
Entry 13 said "the AppProject whitelist that entry 13 identifies as the entire security boundary", pointing readers at itself instead of entry 12, which is where that argument is actually made. Introduced by the rebase: PR #11 took entry 11 while this branch was open, so 13/14 became 12/13. The renumbering caught the headings, the index table and the manifest pointer, and missed this one in prose. The audit after the previous fix missed it too, and the reason is worth recording: that check matched `decisions.md entry N`, which is how OTHER files reference the document. Inside decisions.md the references are a bare "entry N", so nothing in the pattern could match them. The check now also walks decisions.md itself, tracking which entry each line falls under, which additionally catches self-references — the exact shape of this bug. Both intra-file references now resolve and neither is self-referential; the 22 cross-file pointers are unchanged and still resolve. Second Copilot finding of the same class on this PR. The first was a wrong pointer in the manifest, this one a wrong pointer in prose — both from renumbering. Three occurrences across two PRs now, which makes the case for a CI check rather than another manual sweep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hakan-persson
added a commit
that referenced
this pull request
Aug 22, 2026
Three stale entry pointers shipped across two PRs, all from renumbering an entry while a branch was open: a manifest comment naming entry 11 for what became entry 10 (#9, caught by hand), the same shape again in #12 (caught by Copilot), and an entry in decisions.md referring to itself rather than to the entry that makes the argument (#12, caught by Copilot). Manual sweeps are not working, and the third occurrence shows why: the audit run after the second reported clean while the bug was already present, because it matched only the "decisions.md entry N" form that other files use and was structurally blind to the bare "entry N" form used inside decisions.md itself. A check that returns a clean result on an incomplete search is worse than no check. The job asserts four things, each corresponding to a failure that has occurred or that the same edit produces: entry numbers form a gap-free run from 1, which is what a rebase breaks when another PR claims the number a branch was using; every entry has an index row and every row links to a real anchor; bare references inside decisions.md resolve and none points at the entry containing it; and references from every other tracked file resolve. Verified against all four failure shapes rather than the happy path alone — a self-reference, a pointer at a non-existent entry, a numbering gap, and an entry missing its index row — each failing with a message naming the file, the line and what to fix. Clean tree reports 13 entries and 37 pointers checked. A narrower relative of the doc-refs job written on the #3 branch and dropped before merge: that one checked file paths and section anchors, this one checks entry numbers. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two questions came out of a presentation to potential project teams. This answers
one of them in the repo and closes something the answering surfaced.
Entry 13 — GitOps is ArgoCD, not Flux (settled)
The decision was made 2026-08-12 but never written down, and has now been asked
twice with nothing to point at. Re-checked against the running cluster before
recording it; both load-bearing claims still hold:
_project-gitops.yaml.examplestill declares itself the security boundary, andall four controls are intact —
sourceRepospinned to one repo,destinationsto two namespaces,
clusterResourceWhitelist: [], 15 whitelisted kinds.project-infra.yamlstill carries the "escalation path in this cluster"warning for the privileged lane.
The case has strengthened since: decision 6 (#2) and decision 8 (#3) both lean
further on the AppProject boundary, so the switching cost has gone up.
The entry concedes where Flux would have been better — ApplicationSet is
genuinely awkward here, and
HelmReleasedoes a realhelm upgradewhere ArgoCDrenders and applies — and states the migration cost plainly (Applications and
ApplicationSets are mechanical; AppProjects need a from-scratch RBAC redesign)
rather than claiming lock-in.
One argument has reversed and is recorded as such. The original assessment
noted that running ArgoCD with no GUI forfeits its main advantage. Project teams
have now asked to see their own sync status — ArgoCD serves that natively through
the Dex we already run; Flux has no first-party UI, and Weave GitOps lost its
corporate backing. What was a point against is now a point for.
Entry 14 +
default.yaml— empty the built-in projectFound while re-checking the above. ArgoCD creates
defaultat startup permittingany repo, any namespace, and every cluster-scoped kind:
Upstream documents that it cannot be deleted, only modified, and recommends
emptying it in multi-tenant setups. Left alone it is a fully permissive project
sitting next to the whitelist that entry 13 identifies as the entire security
boundary.
This was not a live escalation, and the entry says so explicitly so nobody
over-reads it later. Verified: nothing references it (23 Applications on
infra,2 on
project-infra), and project developers cannot create Applications at all —checked by impersonation,
noinargocdandnoacross all namespaces. Thepoint is that "nobody uses it" is a weaker guarantee than "it cannot be used", and
the stronger one costs nine lines.
It lands in
k8s/argocd/projects/, so the wave -1argocd-projectsapp adopts andenforces it — the same adoption path that file already documents.
Consequence, by design: an Application that omits
spec.projector namesdefaultwill now fail to sync rather than deploy unrestricted. If somethingbreaks after this, it is telling you it never named a project.
Verified
Server-side dry-run applies cleanly against the live cluster. Four CI jobs pass
(74 manifests, up from 73). All
decisions.mdanchors resolve.Entry numbering skips 11 and 12, which are reserved by a branch in flight; entry
13 notes this so the gap does not read as a mistake.