Bound every directory-source Application by what the repo-server enforces - #246
Merged
Conversation
…rces argocd-repo-server refuses to generate manifests for a directory-type source whose combined manifest files exceed --max-combined-directory-manifests-size. Nothing in this catalog compared anything against that limit, so the first place it could be met was a sync on a cluster — and not visibly, because the Application reports ComparisonError with `Unknown` in the sync column rather than OutOfSync, and the symptom arrives waves later as whatever workload needed the kind that never installed. ─────────────────── What is measured, and which sources ─────────────────── scripts/check-directory-manifest-size.py transcribes the runtime rather than approximating it. getPotentiallyValidManifests matches a manifest by NAME before it reads it, applies directory.include/exclude to the path relative to the source root, descends only when directory.recurse is set, counts a jsonnet file as a manifest while leaving its size out of the total, and takes each size from the resolved file so a symlink contributes its target's bytes. The comparison is the runtime's too: it accumulates and aborts when the total EXCEEDS the limit, so a source exactly filling the ceiling still generates and this rejects at the same byte. A gate stricter by one byte would reject a tree that works. The population is derived, not listed. A source with a `path` is not automatically a kustomize directory: ArgoCD takes an explicit helm, kustomize, plugin or directory block at its word, and otherwise classifies by what the directory holds — Chart.yaml is Helm, a kustomization file is Kustomize, anything else is Directory. Running that decision over the tree is what makes the corpus a property rather than a note: a kustomization deleted out of an overlay does not merely break a build, it moves that source into the set a size limit applies to. Scope is applicationsets/*.yaml non-recursive, matching check-hardcoded-org.py and check-catalog-revision.py, because app-of-apps sources that directory without directory.recurse. Three sources qualify today. Two resolve: the Argo Workflows crds/full directory and the Gateway API standard CRDs, both plain manifest directories in someone else's repository at a pinned tag. The third, portal-tenants, takes its repository, revision and path from cluster annotations the portal sets, so no size is a function of this tree; it is recorded as unmeasurable with the reason, and a source that becomes unmeasurable without one is a finding. ─────────────────── Where the ceiling comes from ─────────────────── It is not derivable here and the gate says so rather than implying otherwise. The limit is a repo-server flag, set by whatever installs ArgoCD; this catalog installs none and pins no ArgoCD version, so there is nothing in this tree to read it from. It is therefore GATED, not derived: contracts/repo-server.json records the size the host must be configured for, and the repository that configures the repo-server is where the two are held equal. contracts/secret-store.json publishes a value this catalog declares for others to assert against; this one runs the other way, publishing what this catalog needs of its host. What is NOT written down is the measurement. scripts/directory-sources.json keys each size on (repoURL, targetRevision, path), so a pin cannot move without its record going stale, and a stale record fails the blocking gate on the pull request that moves the pin. That is where headroom comes from: every byte of growth arrives as a diff a reviewer sees, rather than one warning at whatever percentage somebody picked. The first Renovate bump of the Argo Workflows tag lands red until it carries its new size. An empty comparison fails everywhere it can occur — no ApplicationSet parsed, no source with a path, a directory source with no record, a record with neither a size nor a stated reason it has none, and a resolved path holding no manifest file. A size check that measured nothing and reported "under the limit" is the defect it exists to catch. ─────────────────── Split, and proof ─────────────────── The offline half blocks in the `appsets` job beside the offline chart-provenance half, because it is a function of the tree. The live half re-clones each pinned source and runs on the weekly schedule, which is the only thing that can answer whether a tag nobody moved here was moved upstream underneath it. That workflow now covers both kinds of upstream pin. Proven red against the configuration the incident arrived under: with the ceiling lowered to the one argocd-repo-server ships with, crds/full at 11.10M is rejected by name, with nothing about the source changed. That case is the positive control in scripts/tests/controls.py, the first break in the gate's own --self-test, and one of three plants in reverify-gates.sh alongside a pin that moves away from its measurement and a directory source nothing measured. 47 unit tests cover the source-type decision and the byte accounting clause by clause; each was checked by reverting the behaviour and confirming the test that names it fails. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com>
CI Results
All checks passed. |
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
1 participant
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.
Closes #198.
The gap
argocd-repo-serverrefuses to generate manifests for a directory-type source whose combined manifest files exceed--max-combined-directory-manifests-size. Nothing here compared anything against that limit, so the first place it could be met was a sync on a cluster — and not visibly: the Application reportsComparisonErrorwithUnknownin the sync column rather thanOutOfSync, and the symptom arrives waves later as whatever workload needed the kind that never installed.Which sources, derived rather than listed
A source with a
pathis not automatically a kustomize directory. ArgoCD takes an explicithelm/kustomize/plugin/directoryblock at its word and otherwise classifies by what the directory holds —Chart.yamlis Helm, a kustomization file is Kustomize, anything else is Directory. Running that decision over the tree is what makes the corpus a property: a kustomization deleted out of an overlay does not merely break a build, it moves that source into the set a size limit applies to.Three sources qualify:
argo-workflows-crds—charts/argo-workflows/files/crds/full@argo-workflows-1.0.23gateway-api-crds—config/crd/standard@v1.6.1portal-tenantsportal-tenantsis recorded as unmeasurable with its reason; a source that becomes unmeasurable without one is a finding.The measurement is the runtime's
Transcribed from
getPotentiallyValidManifests: manifests matched by name before they are read,include/excludeapplied relative to the source root, descent only underdirectory.recurse, jsonnet counted as a manifest but not against the size, and each size taken from the resolved file so a symlink contributes its target's bytes. The comparison too — it accumulates and aborts when the total exceeds the limit, so a source exactly filling the ceiling still generates and this rejects at the same byte. A gate stricter by one byte would reject a tree that works.The ceiling is gated, not derived — and why
It is a repo-server flag, set by whatever installs ArgoCD. This catalog installs none and pins no ArgoCD version, so there is nothing in this tree to read it from.
contracts/repo-server.jsonrecords what the host must be configured for, and the repository that configures the repo-server is where the two are held equal.contracts/secret-store.jsonpublishes a value this catalog declares for others to assert against; this one runs the other way, publishing what this catalog needs of its host.What is not written down is the measurement.
scripts/directory-sources.jsonkeys each size on(repoURL, targetRevision, path), so a pin cannot move without its record going stale, and a stale record fails the blocking gate on the pull request that moves the pin. That is where headroom comes from: every byte of growth arrives as a diff a reviewer sees, rather than one warning at whatever percentage somebody picked. The first Renovate bump of the Argo Workflows tag lands red until it carries its new size.Empty comparisons fail
No ApplicationSet parsed; no source with a path; a directory source with no record; a record with neither a size nor a stated reason it has none; a resolved path holding no manifest file. A size check that measured nothing and reported "under the limit" is the defect it exists to catch.
Split
appsetsjob beside the offline chart-provenance half, because it is a function of the tree.chart-provenance.ymlnow covers both kinds of upstream pin.Proof
Red against the configuration the incident arrived under — ceiling lowered to the one ArgoCD ships with, nothing about the source changed:
That case is the positive control in
controls.py, the first break in the gate's own--self-test, and one of three plants inreverify-gates.sh(alongside a pin that moves away from its measurement, and a directory source nothing measured). 47 unit tests cover the source-type decision and the byte accounting clause by clause; each was checked by reverting the behaviour and confirming the test that names it fails.task validategreen,controls.py21/21,reverify-gates.sh55/0,empty-corpus.py34/34, ruff + mypy clean.