From 2c06461d7cd774067518cd3b53002c819354bc5e Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Sat, 5 Sep 2026 02:28:21 -0700 Subject: [PATCH] Bound every directory-source Application by what the repo-server enforces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- .github/workflows/chart-provenance.yml | 48 +- .github/workflows/ci.yml | 14 + CLAUDE.md | 34 +- Taskfile.yaml | 9 +- contracts/repo-server.json | 7 + scripts/check-directory-manifest-size.py | 708 ++++++++++++++++++ scripts/directory-sources.json | 36 + scripts/tests/controls.py | 19 + scripts/tests/reverify-gates.sh | 53 +- scripts/tests/run.py | 8 +- scripts/tests/test_directory_manifest_size.py | 424 +++++++++++ 11 files changed, 1342 insertions(+), 18 deletions(-) create mode 100644 contracts/repo-server.json create mode 100755 scripts/check-directory-manifest-size.py create mode 100644 scripts/directory-sources.json create mode 100644 scripts/tests/test_directory_manifest_size.py diff --git a/.github/workflows/chart-provenance.yml b/.github/workflows/chart-provenance.yml index 4d7cdfc..e196fec 100644 --- a/.github/workflows/chart-provenance.yml +++ b/.github/workflows/chart-provenance.yml @@ -1,22 +1,24 @@ -name: chart provenance +name: upstream pins -# The live half of the chart-provenance check: is every chart this catalog pins -# still the chart it was pinned for, upstream, right now? +# The live half of every check that asks whether an upstream pin still resolves +# to what it was recorded against: charts, and the directories a directory-source +# Application asks the repo-server to combine. # # It is a separate workflow rather than a job in ci.yml on purpose, and there # are two reasons. # -# The verdict is not a function of this commit. A maintainer can deprecate a -# chart, or hand it to a different organisation, at any moment. Run in the merge -# path, that turns a pull request red for a reason the pull request did not -# cause — the same trap `mirror-check freshness` and `schemas:freshness` are -# kept off the blocking path to avoid. +# Neither verdict is a function of this commit. A maintainer can deprecate a +# chart, hand it to a different organisation, or move a tag, at any moment. Run +# in the merge path, that turns a pull request red for a reason the pull request +# did not cause — the same trap `mirror-check freshness` and `schemas:freshness` +# are kept off the blocking path to avoid. Resolving them also needs a clone or +# a registry round trip, which the merge path does not get to depend on. # # And the merge gate in ci.yml refuses any workflow containing a job it does not # watch, while counting a skipped dependency as a failure. A job that only runs # on a schedule cannot satisfy both, so it does not belong in that file. The -# offline half — every pin has a record, every record a pin — does run there, -# because that IS a function of the tree. +# offline halves — every pin has a record, every record a pin — do run there, +# because those ARE a function of the tree. on: schedule: # Mondays, ahead of the working week and after the weekend's upstream releases. @@ -54,3 +56,29 @@ jobs: # "current" for something that is no longer the same software. - name: Compare every pinned chart against its record run: ./scripts/check-chart-deprecation.py --live + + sizes: + name: pinned directory sources still measure what was recorded + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version-file: .python-version + - name: Install Python dependencies + run: pip install --require-hashes -r requirements.txt + + # A directory source over the repo-server's combined-manifest ceiling + # generates nothing, and says so as a ComparisonError with the sync column + # reading Unknown rather than as anything that looks like a failure. The + # size is a function of (repoURL, targetRevision, path), so the blocking + # half in ci.yml already fails when a pin moves away from its measurement. + # What only a clone can answer is whether a tag that nobody moved here was + # moved upstream underneath it. + - name: Re-measure every pinned directory source + run: ./scripts/check-directory-manifest-size.py --live diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1555099..1cf52b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -793,6 +793,20 @@ jobs: ./scripts/check-chart-deprecation.py ./scripts/check-chart-deprecation.py --self-test + # A source with a `path` and no chart is not automatically a kustomize + # directory: ArgoCD classifies by what the directory holds, and a Directory + # source is the one shape the repo-server measures against + # --max-combined-directory-manifests-size. Over that, it generates nothing + # and reports ComparisonError, which reads as `Unknown` in the sync column + # and surfaces waves later as whatever workload needed the kind that never + # installed. Offline like the chart half above: the sizes are keyed on + # (repoURL, targetRevision, path), so a pin cannot move without its + # measurement going stale here, and upstream/pins asks the live question. + - name: Directory sources fit the repo-server's combined-manifest ceiling + run: | + ./scripts/check-directory-manifest-size.py + ./scripts/check-directory-manifest-size.py --self-test + # ── Appset render gate ─────────────────────────────────────────────── # The Karpenter EC2NodeClass patch carries if/range control flow inside a # `patch: |-` string block. Every other gate treats that string as opaque — diff --git a/CLAUDE.md b/CLAUDE.md index c5ac656..6dffc19 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,6 +100,7 @@ task validate:label-values # Every k8s label value satisfies the API ser task validate:policy-admission # Prove no addon is denied by the Enforce-tier Kyverno policies (+ exclusion-list parity) task validate:externalsecret-keys # Each ExternalSecret names its remote secret once, and the delivering appset patches it task validate:secret-store-refs # Every secret-store reference names the one store this catalog declares, and the published contract states it +task validate:directory-manifest-size # Every directory-source Application fits the repo-server's combined-manifest ceiling task validate:dashboards # grafana.com dashboard ids exist and are AMG-saveable task validate:athena-panel-columns # Every column a CUR panel names is one the export delivers task validate:fork-safety # No hardcoded catalog repoURL in applied ApplicationSets (report-only locally) @@ -113,7 +114,7 @@ task validate:image-vulnerabilities # Every fixed CRITICAL in a rendered image i `task validate` runs the structural gates (lint, kustomize build, helm-render, ApplicationSet schema, sync-wave ordering, appset render, policy-admission, -secret-store references, dashboards, fork-safety). CI runs those plus several gates that have **no local +secret-store references, directory-source sizes, dashboards, fork-safety). CI runs those plus several gates that have **no local `task` target**, and one that has a target but is deliberately outside the aggregate, so a clean `task validate` is necessary but not sufficient: @@ -178,6 +179,24 @@ aggregate, so a clean `task validate` is necessary but not sufficient: delivered panel measures is a finding, because a figure compared to nothing is how the last wrong one survived. The figure has no independent existence: there is no constant to correct and none in the summary that anything trusts +- **Directory-source manifest sizes** — `scripts/check-directory-manifest-size.py`, + in the `appsets` job beside the offline chart-provenance half. The repo-server + refuses to generate a directory-type Application whose combined manifest files + exceed `--max-combined-directory-manifests-size`, and the Application then + reports `ComparisonError` with `Unknown` in the sync column — quieter than + `OutOfSync`, and the symptom arrives waves later as whatever workload needed + the kind that never installed. A source with a `path` is not automatically a + kustomize directory: ArgoCD takes an explicit `helm`/`kustomize`/`plugin`/ + `directory` block at its word and otherwise classifies by what the directory + holds, so the population is derived by running that decision over the tree + rather than from a list, and a kustomization deleted out of an overlay + reclassifies that source into the corpus. The ceiling is not derivable here — + it is a repo-server flag, and this catalog installs no ArgoCD — so it is gated + rather than derived: `contracts/repo-server.json` records what the host must be + configured for, and the repository that configures the repo-server is where the + two are held equal. Sizes in `scripts/directory-sources.json` are keyed on + (repoURL, targetRevision, path), so a pin cannot move without its measurement + going stale and failing the blocking gate on the pull request that moves it - **Image vulnerabilities** — `scripts/check-image-vulnerabilities.py` (CI job `image-vulnerabilities`). The one with a target of its own, `task validate:image-vulnerabilities`, kept out of the aggregate because it @@ -206,10 +225,15 @@ documents: `task validate` runs it report-only, CI runs it `--blocking`. - Standalone jobs on every PR: `helm-render` (templates every addon against its appset-pinned chart with base + each env's values — an unknown key fails here, not fleet-wide at sync), `policy-admission` (renders the whole fleet into its real destination namespaces and runs `kyverno apply` against the Enforce-tier best-practice/pod-security policies, so an addon landing in a namespace the policies don't exclude fails here instead of being denied at admission on a vended enforce cluster — also asserts all four exclusion lists stay identical, that every namespace the fleet lands a workload in is on that list, and that a deliberately non-compliant canary is denied by every rule, which is what proves the run evaluated anything), `appsets` (ApplicationSet schema + documented sync-wave ordering), `appset-render` (renders the Karpenter EC2NodeClass patch template the way the ArgoCD ApplicationSet controller does — Go text/template + sprig, `missingkey=error` — against fixture create/adopt/legacy cluster Secrets, so a control-flow edit that breaks the per-cluster render fails here instead of at sync), `secrets` (gitleaks over the working tree), plus the dashboard, fork-safety, and Kyverno policy gates - Chart pins in `applicationsets/`, the Go module, the CI tool downloads and the GitHub Actions are all watched by Renovate (`renovate.json`, extending the org preset at `nanohype/.github`) - A scheduled workflow, `.github/workflows/chart-provenance.yml`, runs weekly (Mondays) - and re-resolves every pinned chart against its recorded provenance via - `scripts/check-chart-deprecation.py --live` — so a chart that is deprecated, - moved, or no longer resolves to what it did at pin time surfaces on a schedule - rather than at the next sync + and re-resolves every upstream pin against what was recorded for it. One job + runs `scripts/check-chart-deprecation.py --live`, so a chart that is + deprecated, moved, or no longer resolves to what it did at pin time surfaces on + a schedule rather than at the next sync. The other runs + `scripts/check-directory-manifest-size.py --live`, which re-measures each + pinned directory source: the blocking half already fails when a pin moves away + from its measurement, and what only a clone can answer is whether a tag nobody + moved here was moved upstream underneath it. Both need the network, which is + why neither is on the merge path - Manual diff rendering available via `.github/workflows/diff.yml` ## Claude Code Tooling diff --git a/Taskfile.yaml b/Taskfile.yaml index 87f76ae..4476021 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -200,13 +200,19 @@ tasks: cmds: - ./scripts/check-secret-store-refs.py + validate:directory-manifest-size: + desc: "Directory-source gate — every directory source fits the combined-manifest ceiling the repo-server is configured for" + cmds: + - ./scripts/check-directory-manifest-size.py + - ./scripts/check-directory-manifest-size.py --self-test + validate:athena-panel-columns: desc: "Athena panel gate — every column a CUR panel names is one the export delivers" cmds: - ./scripts/check-athena-panel-columns.py validate: - desc: Run all validations (lint, build, helm-render, appset schema, sync waves, label values, appset render, policy admission, secret refs, athena panels, dashboard/fork-safety) + desc: Run all validations (lint, build, helm-render, appset schema, sync waves, label values, appset render, policy admission, secret refs, directory sizes, athena panels, dashboard/fork-safety) deps: - lint:yaml - lint:python @@ -219,6 +225,7 @@ tasks: - validate:policy-admission - validate:externalsecret-keys - validate:secret-store-refs + - validate:directory-manifest-size - validate:athena-panel-columns - validate:dashboards - validate:fork-safety diff --git a/contracts/repo-server.json b/contracts/repo-server.json new file mode 100644 index 0000000..730075b --- /dev/null +++ b/contracts/repo-server.json @@ -0,0 +1,7 @@ +{ + "_purpose": "The argocd-repo-server this catalog is installed against must be configured to combine at least this many bytes of manifests for a directory-type Application, or the directory sources in applicationsets/ generate nothing and their Applications report ComparisonError. The value is not derivable from this repository: it is a repo-server flag, and this catalog installs no ArgoCD and pins no ArgoCD version. It is recorded here so the repository that does install ArgoCD can assert that what it configures and what this catalog needs are the same number.", + "_setBy": "argocd-repo-server --max-combined-directory-manifests-size, equivalently ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE; `argocd-repo-server --help` prints the flag and the default it ships with.", + "_measuredBy": "scripts/check-directory-manifest-size.py, which measures every directory source the way the repo-server does and fails before a cluster does. Sizes live in scripts/directory-sources.json.", + "maxCombinedDirectoryManifestsSize": "20M", + "argoCdDefault": "10M" +} diff --git a/scripts/check-directory-manifest-size.py b/scripts/check-directory-manifest-size.py new file mode 100755 index 0000000..e9ca775 --- /dev/null +++ b/scripts/check-directory-manifest-size.py @@ -0,0 +1,708 @@ +#!/usr/bin/env python3 +"""Every directory-source Application fits the repo-server's combined-manifest ceiling. + + python3 scripts/check-directory-manifest-size.py # blocking gate, offline + python3 scripts/check-directory-manifest-size.py --live # scheduled, clones each pinned source + python3 scripts/check-directory-manifest-size.py --sync # re-measure and rewrite the records + python3 scripts/check-directory-manifest-size.py --self-test + +WHAT THE RUNTIME DOES + +argocd-repo-server walks a directory-type source, accumulates the size of every +file whose name matches `^.*\\.(yaml|yml|json|jsonnet)$` (jsonnet excluded from +the sum), and aborts generation the moment the running total exceeds +`--max-combined-directory-manifests-size`. The Application then renders nothing. +It does not go `OutOfSync`; it goes `ComparisonError`, and the sync column reads +`Unknown` — which is the state a human scanning a list of Applications is least +likely to stop at. The symptom surfaces waves later as whatever workload needed +the kind that never installed. + +WHICH SOURCES THIS IS ABOUT + +Not every source with a `path`. ArgoCD decides the source type first, and only +the Directory type is measured this way: an explicit `helm`, `kustomize` or +`plugin` block names the type outright, an explicit `directory` block names +Directory, and a source with none of them is classified by what the directory +holds — a `Chart.yaml` makes it Helm, a kustomization file makes it Kustomize, +anything else is Directory. So the population is derived by running that same +decision over the tree rather than by listing the sources somebody noticed. A +kustomization file deleted out of an overlay does not merely break a build; it +reclassifies that source into this gate's corpus. + +Scope is `applicationsets/*.yaml`, non-recursive, matching check-hardcoded-org.py +and check-catalog-revision.py: app-of-apps sources `applicationsets` without +`directory.recurse`, so the `opt-in/` subdirectory is not applied by an install +and is not what a cluster runs. + +WHERE THE CEILING COMES FROM, AND WHY IT IS NOT DERIVED HERE + +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 +and nothing to derive it against. 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, and the contract directory is what makes either assertable from +outside. + +The comparison itself is the runtime's, not a margin on it: the repo-server +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. Headroom +comes from somewhere better than a threshold fraction. The measurements are a +function of (repoURL, targetRevision, path), so a pin cannot move without its +record going stale, and a stale record fails the offline gate on the pull request +that moves the pin — every byte of growth arrives as a diff in +scripts/directory-sources.json, rather than one warning at whatever percentage +somebody picked. + +THE TWO QUESTIONS, AND WHY ONLY ONE BLOCKS + + default (offline, BLOCKING) — the corpus and the records agree, every + recorded size is under the contracted ceiling, and every in-repo + directory source measures under it right now. A function of the tree. + + --live (network, SCHEDULED) — resolve each pinned source at its revision and + confirm it still measures what the record says. Splitting it this way is + the same reason check-chart-deprecation.py splits: the merge path does + not get to depend on a clone of somebody else's repository. +""" + +from __future__ import annotations + +import argparse +import fnmatch +import importlib.util +import json +import os +import pathlib +import re +import subprocess +import sys +import tempfile +from dataclasses import dataclass + +# Shared precondition helper, loaded by path: these are hyphenated executables +# run from varying working directories. +_gl = pathlib.Path(__file__).resolve().parent / "gatelib.py" +_gs = importlib.util.spec_from_file_location("gatelib", _gl) +assert _gs and _gs.loader, f"{_gl} is not loadable as a module" +gatelib = importlib.util.module_from_spec(_gs) +sys.modules["gatelib"] = gatelib +_gs.loader.exec_module(gatelib) + +ROOT = pathlib.Path(__file__).resolve().parent.parent +APPSETS = ROOT / "applicationsets" +CONTRACT = ROOT / "contracts" / "repo-server.json" +# Beside the checker rather than beside the appsets, for the reason +# scripts/chart-provenance.json is: applicationsets/ is read by kubeconform as +# manifests, and a record living there would have to be exempted from a schema +# gate to exist. +RECORDS = ROOT / "scripts" / "directory-sources.json" + +# argocd-repo-server's own predicates, transcribed. A file counts as a manifest +# by NAME, before anything reads it. +MANIFEST_FILE = re.compile(r"^.*\.(yaml|yml|json|jsonnet)$") +# util/kustomize KustomizationNames. Any one of these makes a directory Kustomize. +KUSTOMIZATION_NAMES = ("kustomization.yaml", "kustomization.yml", "Kustomization") + +# The catalog's own repoURL is templated off the cluster Secret, never literal. +URL_ANNOTATION = "gitops/repo-url" + +# Seconds one `git clone` of a pinned source may take under --live/--sync. +NETWORK_TIMEOUT = 300 + +# resource.Quantity suffixes. Decimal and binary are different sizes and reading +# one as the other understates the ceiling, which is the direction that lets a +# source through. +SUFFIXES = { + "": 1, "k": 10**3, "M": 10**6, "G": 10**9, "T": 10**12, "P": 10**15, "E": 10**18, + "Ki": 2**10, "Mi": 2**20, "Gi": 2**30, "Ti": 2**40, "Pi": 2**50, "Ei": 2**60, +} +QUANTITY = re.compile(r"^(?P\d+)(?P[kMGTPE]|[KMGTPE]i|)$") + + +def die(msg: str) -> None: + print(f"directory-manifest-size: {msg}", file=sys.stderr) + sys.exit(1) + + +def quantity(text: str, where: str) -> int: + """A k8s resource.Quantity as a byte count, or exit 2 naming where it came from.""" + m = QUANTITY.fullmatch(str(text).strip()) + if not m: + print(f"Cannot run: {where} holds {text!r}, which is not a quantity " + f"argocd-repo-server would accept for its combined-manifest ceiling.") + sys.exit(gatelib.CANNOT_RUN) + return int(m.group("num")) * SUFFIXES[m.group("suffix")] + + +def human(n: int) -> str: + return f"{n / 10**6:.2f}M" + + +# ------------------------------------------------------------------ the corpus + + +@dataclass(frozen=True) +class Source: + """One ApplicationSet source ArgoCD would generate manifests for.""" + + appset: str + file: str + repo_url: str + target_revision: str + path: str + recurse: bool + include: str + exclude: str + # The in-tree directory this resolves to, when the source is this catalog and + # the path template expands. None means nothing here can measure it. + local: pathlib.Path | None + + @property + def templated(self) -> bool: + return any("{{" in v for v in (self.repo_url, self.target_revision, self.path)) + + +def sources_of(doc: dict) -> list[dict]: + """Every source block an ApplicationSet template declares.""" + spec = ((doc.get("spec") or {}).get("template") or {}).get("spec") or {} + out = [s for s in (spec.get("sources") or []) if isinstance(s, dict)] + if isinstance(spec.get("source"), dict): + out.append(spec["source"]) + return out + + +def explicit_type(src: dict) -> str | None: + """ArgoCD's ApplicationSource.ExplicitType, for the fields this catalog uses. + + A `chart` names a Helm source as surely as a `helm` block does; ArgoCD + reaches the same verdict by a different route, and either way such a source + is never measured as a directory. + """ + if src.get("helm") or src.get("chart"): + return "Helm" + if src.get("kustomize"): + return "Kustomize" + if src.get("plugin"): + return "Plugin" + if src.get("directory") is not None: + return "Directory" + return None + + +def discovered_type(local: pathlib.Path) -> str: + """ArgoCD's util/app/discovery, run against a directory that is in this tree.""" + if (local / "Chart.yaml").is_file(): + return "Helm" + if any((local / n).is_file() for n in KUSTOMIZATION_NAMES): + return "Kustomize" + return "Directory" + + +def expansions(template: str, doc: dict, environments: list[str]) -> list[str]: + """The paths a source template resolves to, expanded where that is possible. + + Only the two substitutions this catalog's paths use: the `path` an element of + a list generator carries, and the environment label off the cluster Secret. + Anything else comes back with its `{{` intact and the caller treats that + source as one this repository cannot resolve — a source to record as + unmeasurable, not one to drop. + """ + bases = [e["path"] for e in gatelib.list_elements(doc) if isinstance(e.get("path"), str)] + out = set() + for base in bases or [None]: + one = template.replace("{{ .path }}", base) if base else template + if '{{ index .metadata.labels "environment" }}' in one: + for env in environments: + out.add(one.replace('{{ index .metadata.labels "environment" }}', env)) + else: + out.add(one) + return sorted(out) + + +def environments() -> list[str]: + """The environment labels the overlays in this tree are written for.""" + found = {p.name for p in ROOT.glob("addons/*/*/overlays/*") if p.is_dir()} + found |= {p.name for p in ROOT.glob("policies/*/*/overlays/*") if p.is_dir()} + if not found: + print("Cannot run: no overlay directories under addons/ or policies/, so the " + "environments a templated source path expands over are unknown.") + sys.exit(gatelib.CANNOT_RUN) + return sorted(found) + + +def directory_sources() -> list[Source]: + """Every applied source ArgoCD would classify as a Directory, derived from the tree.""" + if not APPSETS.is_dir(): + print(f"Cannot run: {APPSETS} is not a directory. This gate examined nothing, " + f"which is not the same as finding nothing.") + sys.exit(gatelib.CANNOT_RUN) + + envs = environments() + appsets = 0 + considered = 0 + found: list[Source] = [] + + for path in sorted(p for p in APPSETS.glob("*.y*ml") if p.is_file()): + for doc in gatelib.read_yaml_all(path): + if not isinstance(doc, dict) or doc.get("kind") != "ApplicationSet": + continue + appsets += 1 + name = ((doc.get("metadata") or {}).get("name")) or path.stem + for src in sources_of(doc): + if "path" not in src: + continue + considered += 1 + kind = explicit_type(src) + directory = src.get("directory") or {} + template = str(src["path"]) + repo_url = str(src.get("repoURL", "")) + revision = str(src.get("targetRevision", "")) + + # A source pointing at this catalog resolves in the tree; anything + # else is somebody else's repository at a pinned revision. An + # expansion naming no directory contributes nothing — an addon + # with no hub overlay is one the cluster selector does not send + # there, and whether that selector and those overlays agree is + # what check-env-coverage.py asks. + locals_: list[pathlib.Path | None] = [None] + if URL_ANNOTATION in repo_url: + expanded = expansions(template, doc, envs) + existing = [ROOT / e for e in expanded if "{{" not in e and (ROOT / e).is_dir()] + if expanded and not existing: + die(f"{name} ({path.name}) sources {template!r} from this catalog " + f"and it resolves to no directory in the tree — this gate " + f"cannot classify a source it cannot find.") + locals_ = list(existing) or [None] + + for local in locals_: + verdict = kind or (discovered_type(local) if local else None) + # An unclassifiable REMOTE source is a directory source until + # something proves otherwise: ArgoCD falls through to Directory + # when it finds no Chart.yaml and no kustomization, and this + # repository can see neither. + undecidable = verdict is None and local is None + if verdict != "Directory" and not undecidable: + continue + found.append(Source( + appset=name, + file=path.name, + repo_url=repo_url, + target_revision=revision, + path=str(local.relative_to(ROOT)) if local else template, + recurse=bool(directory.get("recurse", False)), + include=str(directory.get("include", "")), + exclude=str(directory.get("exclude", "")), + local=local, + )) + + if not appsets: + die(f"read no ApplicationSet out of {APPSETS.relative_to(ROOT)} — the parser " + f"and the catalog disagree, and a run that examined nothing must not " + f"report that nothing is wrong.") + if not considered: + die(f"read {appsets} ApplicationSet(s) out of {APPSETS.relative_to(ROOT)} and " + f"not one source with a path. Every source in this catalog carries one, " + f"so the walk stopped seeing them rather than the catalog changing.") + return found + + +def keyed(sources: list[Source]) -> dict[str, Source]: + """Record keys: the ApplicationSet name, disambiguated by path only when it must be.""" + counts: dict[str, int] = {} + for s in sources: + counts[s.appset] = counts.get(s.appset, 0) + 1 + return {(s.appset if counts[s.appset] == 1 else f"{s.appset}#{s.path}"): s + for s in sources} + + +# ----------------------------------------------------------------- measurement + + +def measure(root: pathlib.Path, recurse: bool, include: str, exclude: str) -> tuple[int, int]: + """(bytes, files) exactly as argocd-repo-server accumulates them. + + Transcribed from getPotentiallyValidManifests: name-matched before it is read, + include/exclude applied to the path relative to the source root, jsonnet + counted as a manifest but not against the size, and the size taken from the + resolved file so a symlink contributes its target's bytes rather than the + length of its own name. + """ + total = 0 + files = 0 + for dirpath, dirnames, filenames in os.walk(root): + here = pathlib.Path(dirpath) + if not recurse and here != root: + dirnames[:] = [] + continue + for name in sorted(filenames): + if not MANIFEST_FILE.match(name): + continue + path = here / name + rel = str(path.relative_to(root)) + if exclude and fnmatch.fnmatch(rel, exclude): + continue + if include and not fnmatch.fnmatch(rel, include): + continue + if not path.is_file(): + continue # a symlink to something that is not a regular file + files += 1 + if name.endswith(".jsonnet"): + continue + total += path.stat().st_size + return total, files + + +def clone(src: Source, into: pathlib.Path) -> pathlib.Path: + """The pinned source directory, checked out. Exits 1 naming what would not resolve.""" + gatelib.require("git") + proc = subprocess.run( + ["git", "clone", "--quiet", "--depth", "1", "--branch", src.target_revision, + src.repo_url, str(into)], + capture_output=True, text=True, timeout=NETWORK_TIMEOUT) + if proc.returncode != 0: + last = ((proc.stderr or "") + (proc.stdout or "")).strip().splitlines() + die(f"{src.appset}: {src.repo_url} at {src.target_revision} would not clone — " + f"{last[-1][:200] if last else 'no output'}") + resolved = into / src.path + if not resolved.is_dir(): + die(f"{src.appset}: {src.repo_url} at {src.target_revision} has no {src.path}/. " + f"A source path that is not there generates nothing.") + return resolved + + +def remeasure(src: Source) -> tuple[int, int]: + with tempfile.TemporaryDirectory() as tmp: + return measure(clone(src, pathlib.Path(tmp) / "src"), + src.recurse, src.include, src.exclude) + + +# ------------------------------------------------------------------- the parts + + +def load_contract() -> dict: + doc = gatelib.read_json(CONTRACT) + if "maxCombinedDirectoryManifestsSize" not in doc: + print(f"Cannot run: {CONTRACT.relative_to(ROOT)} declares no " + f"maxCombinedDirectoryManifestsSize, so this gate has no ceiling to " + f"measure against and would report every source as fitting.") + sys.exit(gatelib.CANNOT_RUN) + return doc + + +def load_records() -> dict: + if not RECORDS.exists(): + die(f"{RECORDS.relative_to(ROOT)} does not exist. Run --sync to create it.") + return gatelib.read_json(RECORDS).get("sources", {}) + + +COORDINATES = ("repoURL", "targetRevision", "path", "recurse", "include", "exclude") + + +def coordinates(src: Source) -> dict: + return {"repoURL": src.repo_url, "targetRevision": src.target_revision, + "path": src.path, "recurse": src.recurse, + "include": src.include, "exclude": src.exclude} + + +# ---------------------------------------------------------------- offline gate + + +def check_offline(sources: list[Source], recorded: dict, contract: dict) -> int: + if "maxCombinedDirectoryManifestsSize" not in contract: + print(f"Cannot run: {CONTRACT.relative_to(ROOT)} declares no " + f"maxCombinedDirectoryManifestsSize, so this gate has no ceiling to " + f"measure against and would report every source as fitting.") + sys.exit(gatelib.CANNOT_RUN) + ceiling = quantity(contract["maxCombinedDirectoryManifestsSize"], + f"{CONTRACT.relative_to(ROOT)} maxCombinedDirectoryManifestsSize") + stock = quantity(contract.get("argoCdDefault", "10M"), + f"{CONTRACT.relative_to(ROOT)} argoCdDefault") + + problems: list[str] = [] + lines: list[str] = [] + corpus = keyed(sources) + + for key, src in sorted(corpus.items()): + if src.local is not None: + size, files = measure(src.local, src.recurse, src.include, src.exclude) + if files == 0: + problems.append( + f"{key} sources {src.path}/ from this catalog and it holds no " + f"manifest file. A directory source that generates nothing is not " + f"a directory source under the limit.") + continue + lines.append(f"{key:24} {human(size):>9} {100 * size / ceiling:5.1f}% of ceiling " + f"{files:3} file(s) in-tree") + if size > ceiling: + problems.append( + f"{key} is {human(size)}, over the " + f"{contract['maxCombinedDirectoryManifestsSize']} ceiling " + f"{CONTRACT.relative_to(ROOT)} records. The repo-server refuses to " + f"generate it and the Application reports ComparisonError, not " + f"OutOfSync — the sync column reads Unknown.") + continue + + rec = recorded.get(key) + if rec is None: + problems.append( + f"{key} ({src.file}) is a directory source with no entry in " + f"{RECORDS.relative_to(ROOT)}. Nothing bounds what it asks the " + f"repo-server to combine. Run --sync.") + continue + drift = [f for f in COORDINATES if rec.get(f) != coordinates(src)[f]] + if drift: + problems.append( + f"{key} is pinned at coordinates {RECORDS.relative_to(ROOT)} was not " + f"measured against ({', '.join(drift)}), so the recorded size is a " + f"measurement of something else. Re-measure with --sync.") + continue + if rec.get("bytes") is None: + if not rec.get("unbounded"): + problems.append( + f"{key} has an entry in {RECORDS.relative_to(ROOT)} with no size " + f"and no stated reason it cannot have one. A source nothing " + f"measured and nothing excused is the gap this gate exists to " + f"close.") + continue + lines.append(f"{key:24} {'unbounded':>9} {rec['unbounded']}") + continue + size, files = int(rec["bytes"]), int(rec.get("files", 0)) + if files == 0 or size == 0: + problems.append( + f"{key} is recorded in {RECORDS.relative_to(ROOT)} as {size} byte(s) " + f"across {files} file(s). A measurement of nothing recorded as a pass " + f"is the defect, not the evidence.") + continue + note = " EXCEEDS A STOCK REPO-SERVER" if size > stock else "" + lines.append(f"{key:24} {human(size):>9} {100 * size / ceiling:5.1f}% of ceiling " + f"{files:3} file(s) {src.target_revision}{note}") + if size > ceiling: + problems.append( + f"{key} measures {human(size)} at {src.target_revision}, over the " + f"{contract['maxCombinedDirectoryManifestsSize']} ceiling " + f"{CONTRACT.relative_to(ROOT)} records. The repo-server refuses to " + f"generate it and the Application reports ComparisonError, not " + f"OutOfSync — the sync column reads Unknown.") + + for key in sorted(set(recorded) - set(corpus)): + problems.append( + f"{key} has an entry in {RECORDS.relative_to(ROOT)} and is no longer a " + f"directory source in the catalog — drop it, or find out what " + f"reclassified the source.") + + for line in lines: + print(f" {line}") + if problems: + print(f"FAIL {len(problems)} problem(s) across {len(corpus)} directory source(s):") + for p in problems: + print(f" {p}") + return 1 + print(f"OK {len(corpus)} directory source(s), each under the " + f"{contract['maxCombinedDirectoryManifestsSize']} ceiling " + f"{CONTRACT.relative_to(ROOT)} records.") + return 0 + + +# ------------------------------------------------------------------ live check + + +def check_live(sources: list[Source], recorded: dict, contract: dict) -> int: + ceiling = quantity(contract["maxCombinedDirectoryManifestsSize"], + f"{CONTRACT.relative_to(ROOT)} maxCombinedDirectoryManifestsSize") + problems: list[str] = [] + checked = 0 + + for key, src in sorted(keyed(sources).items()): + if src.local is not None or src.templated: + continue + size, files = remeasure(src) + checked += 1 + if files == 0: + problems.append( + f"{key}: {src.repo_url} at {src.target_revision} has {src.path}/ but no " + f"manifest file in it. That source generates nothing.") + continue + print(f" {key:24} {human(size):>9} {files:3} file(s) {src.target_revision}") + rec = recorded.get(key) or {} + if rec.get("bytes") is not None and int(rec["bytes"]) != size: + problems.append( + f"{key} measures {size} byte(s) at {src.target_revision} and " + f"{RECORDS.relative_to(ROOT)} says {rec['bytes']}. The pin did not " + f"move, so the tag did — read what changed upstream before running " + f"--sync.") + if size > ceiling: + problems.append( + f"{key} measures {human(size)}, over the " + f"{contract['maxCombinedDirectoryManifestsSize']} ceiling " + f"{CONTRACT.relative_to(ROOT)} records.") + + if not checked: + die("resolved no pinned directory source, so this run measured nothing. " + "The offline gate reads the same corpus — if it finds sources and this " + "does not, the coordinates stopped resolving.") + if problems: + print(f"FAIL {len(problems)} problem(s) across {checked} pinned source(s):") + for p in problems: + print(f" {p}") + return 1 + print(f"OK all {checked} pinned directory source(s) measure what their record says.") + return 0 + + +# ------------------------------------------------------------------------ sync + + +def sync(sources: list[Source], recorded: dict) -> int: + out: dict[str, dict] = {} + for key, src in sorted(keyed(sources).items()): + if src.local is not None: + continue # measured on every offline run; a record would go stale + rec = dict(coordinates(src)) + if src.templated: + was = recorded.get(key) or {} + rec["bytes"] = None + rec["files"] = None + rec["unbounded"] = was.get("unbounded", "") + if not rec["unbounded"]: + die(f"{key} resolves its coordinates from a cluster annotation, so " + f"nothing here can measure it. Add an `unbounded` note to " + f"{RECORDS.relative_to(ROOT)} saying what bounds it instead, then " + f"re-run --sync.") + print(f" recorded {key:24} unbounded") + else: + size, files = remeasure(src) + if files == 0: + die(f"{key}: {src.path}/ at {src.target_revision} holds no manifest " + f"file. Recording a zero would make an unmeasured source look " + f"like a small one.") + rec["bytes"], rec["files"] = size, files + print(f" recorded {key:24} {human(size):>9} {files:3} file(s) " + f"{src.target_revision}") + out[key] = rec + + RECORDS.write_text(json.dumps({"_README": README, "sources": out}, indent=2) + "\n") + print(f"\nwrote {RECORDS.relative_to(ROOT)} ({len(out)} source(s))") + return 0 + + +README = ( + "What each directory-source Application asks argocd-repo-server to combine, " + "measured the way the repo-server measures it. A source over the ceiling in " + "contracts/repo-server.json generates nothing and reports ComparisonError, " + "which is quieter than OutOfSync. The sizes are a function of (repoURL, " + "targetRevision, path): moving a pin makes this record stale, and a stale " + "record fails the blocking gate, so a bump cannot land without the new size " + "landing in the same diff. Re-measure with " + "scripts/check-directory-manifest-size.py --sync." +) + + +# ------------------------------------------------------------------- self-test + + +def self_test() -> int: + """Break each input the offline verdict rests on and confirm it is rejected.""" + import contextlib + import copy + import io + + real_sources = directory_sources() + real_records = load_records() + real_contract = load_contract() + + def run(s, r, c): + with contextlib.redirect_stdout(io.StringIO()): + return check_offline(s, r, c) + + remote = [s for s in real_sources if s.local is None and not s.templated] + if not remote: + print("FAIL no pinned directory source to break — the corpus this self-test " + "reasons about is not there.") + return 1 + pinned = sorted(k for k, s in keyed(real_sources).items() + if s.local is None and not s.templated)[0] + + breaks = [] + + # The ceiling ArgoCD ships with, against the tree as it stands. This is the + # comparison that was never made: crds/full does not fit a stock repo-server. + stock = dict(real_contract) + stock["maxCombinedDirectoryManifestsSize"] = real_contract.get("argoCdDefault", "10M") + breaks.append(("a source measured against the stock ArgoCD ceiling", + real_sources, real_records, stock)) + + # A directory source nobody measured. + r = copy.deepcopy(real_records) + r.pop(pinned) + breaks.append(("a directory source with no size record", real_sources, r, real_contract)) + + # A record for a source the catalog no longer has. + r = copy.deepcopy(real_records) + r["retired-source"] = {"repoURL": "https://example.invalid", "targetRevision": "v0", + "path": "crds", "recurse": False, "include": "", "exclude": "", + "bytes": 1, "files": 1} + breaks.append(("a record no directory source claims", real_sources, r, real_contract)) + + # The pin moved and the measurement did not. + r = copy.deepcopy(real_records) + r[pinned]["targetRevision"] = "some-other-tag" + breaks.append(("a record measured at a revision nothing pins", + real_sources, r, real_contract)) + + # A measurement of nothing, recorded as a pass. + r = copy.deepcopy(real_records) + r[pinned]["bytes"], r[pinned]["files"] = 0, 0 + breaks.append(("a source recorded as zero bytes over zero files", + real_sources, r, real_contract)) + + # An unmeasurable source with no stated reason. + r = copy.deepcopy(real_records) + r[pinned]["bytes"] = None + breaks.append(("a source with no size and no reason it has none", + real_sources, r, real_contract)) + + failures = [] + for label, s, r, c in breaks: + if run(s, r, c) == 0: + failures.append(label) + print(f" ACCEPTED {label} <-- not caught") + else: + print(f" rejected {label}") + + if run(real_sources, real_records, real_contract) != 0: + failures.append("the shipped catalog does not pass") + print(" ACCEPTED (control) the shipped catalog is rejected") + else: + print(f" passed (control) the shipped catalog, {len(real_sources)} source(s)") + + if failures: + print(f"\nFAIL {len(failures)} break(s) not caught.") + return 1 + print(f"\nOK all {len(breaks)} breaks rejected, and the shipped catalog passes.") + return 0 + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--live", action="store_true", + help="clone each pinned source and re-measure it (network)") + ap.add_argument("--sync", action="store_true", + help="rewrite the size records from upstream (network)") + ap.add_argument("--self-test", action="store_true", + help="break the offline gate's inputs and confirm each is caught") + args = ap.parse_args(argv) + + if args.self_test: + return self_test() + if args.sync: + return sync(directory_sources(), load_records() if RECORDS.exists() else {}) + if args.live: + return check_live(directory_sources(), load_records(), load_contract()) + return check_offline(directory_sources(), load_records(), load_contract()) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/directory-sources.json b/scripts/directory-sources.json new file mode 100644 index 0000000..92e2e9b --- /dev/null +++ b/scripts/directory-sources.json @@ -0,0 +1,36 @@ +{ + "_README": "What each directory-source Application asks argocd-repo-server to combine, measured the way the repo-server measures it. A source over the ceiling in contracts/repo-server.json generates nothing and reports ComparisonError, which is quieter than OutOfSync. The sizes are a function of (repoURL, targetRevision, path): moving a pin makes this record stale, and a stale record fails the blocking gate, so a bump cannot land without the new size landing in the same diff. Re-measure with scripts/check-directory-manifest-size.py --sync.", + "sources": { + "argo-workflows-crds": { + "repoURL": "https://github.com/argoproj/argo-helm", + "targetRevision": "argo-workflows-1.0.23", + "path": "charts/argo-workflows/files/crds/full", + "recurse": false, + "include": "", + "exclude": "", + "bytes": 11102482, + "files": 8 + }, + "gateway-api-crds": { + "repoURL": "https://github.com/kubernetes-sigs/gateway-api", + "targetRevision": "v1.6.1", + "path": "config/crd/standard", + "recurse": false, + "include": "", + "exclude": "", + "bytes": 1169515, + "files": 11 + }, + "portal-tenants": { + "repoURL": "{{ index .metadata.annotations \"gitops/tenants-repo-url\" }}", + "targetRevision": "{{ index .metadata.annotations \"gitops/repo-branch\" }}", + "path": "{{ .path.path }}", + "recurse": true, + "include": "", + "exclude": "", + "bytes": null, + "files": null, + "unbounded": "Repository, revision and path all resolve from cluster annotations the portal sets, so no size is a function of this tree. What the tenant boundary manifests amount to is bounded where they are written, not here." + } + } +} diff --git a/scripts/tests/controls.py b/scripts/tests/controls.py index ff275da..77b6369 100755 --- a/scripts/tests/controls.py +++ b/scripts/tests/controls.py @@ -534,6 +534,22 @@ def m_secret_store_refs(root): marker="aws-secretsmanager") +def m_directory_manifest_size(root): + """Lower the contracted ceiling to the one argocd-repo-server ships with. + + Which is the configuration the incident happened under: the Argo Workflows + `crds/full` directory is over 10M, so at the stock ceiling the repo-server + refuses to generate that Application and it reports ComparisonError. Nothing + about the source changes — the same eight files, the same pin — so this is + the violation as it actually arrives, from a host configured lower than the + catalog needs rather than from a manifest anyone edited. + """ + return _sub(root, "contracts/repo-server.json", + '"maxCombinedDirectoryManifestsSize": "20M"', + '"maxCombinedDirectoryManifestsSize": "10M"', + marker='"maxCombinedDirectoryManifestsSize": "10M"') + + def m_chart_deprecation(root): """A recorded chart that nothing pins, which the offline gate must reject.""" import json @@ -613,6 +629,9 @@ def m_env_coverage(root): "does not spend", m_burn_rate_budgets), "check-secret-store-refs.py": ("a reference to a store the catalog does not " "declare", m_secret_store_refs), + "check-directory-manifest-size.py": ("a directory source over the ceiling the " + "repo-server is configured for", + m_directory_manifest_size), } diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index ea77d51..bbfb820 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -160,6 +160,7 @@ run 0 "check-alert-severity-routes.py" ./scripts/check-alert-severity-routes.py run 0 "check-env-coverage.py" ./scripts/check-env-coverage.py run 0 "check-burn-rate-budgets.py" ./scripts/check-burn-rate-budgets.py run 0 "check-secret-store-refs.py" ./scripts/check-secret-store-refs.py +run 0 "check-directory-manifest-size.py" ./scripts/check-directory-manifest-size.py run 0 "check-named-things.py" ./scripts/check-named-things.py run 0 "check-policy-validity.py" ./scripts/check-policy-validity.py run 0 "no-placeholders.sh" ./scripts/no-placeholders.sh @@ -486,6 +487,56 @@ run nonzero "secret-store-refs: the published contract drifts from the tree" \ ./scripts/check-secret-store-refs.py res $F +# The configuration the incident arrived under. Nothing about the source +# changes — same eight files, same pin — only the ceiling the host is +# configured for, and at the one ArgoCD ships with the repo-server refuses to +# generate the Application at all. +F=contracts/repo-server.json; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace('"maxCombinedDirectoryManifestsSize": "20M"', + '"maxCombinedDirectoryManifestsSize": "10M"', 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" lowered the ceiling to the one argocd-repo-server ships with") +PYQ +run nonzero "directory-manifest-size: the host is configured below what the catalog needs" \ + ./scripts/check-directory-manifest-size.py +res $F + +# The shape a Renovate pull request has. Moving the tag changes what the +# repo-server would combine, and the recorded measurement was taken against the +# tag that was there before — so a bump that grows the directory past the +# ceiling has to land its new size in the same diff. +F=applicationsets/argo-workflows-crds.yaml; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace("targetRevision: argo-workflows-1.0.23", + "targetRevision: argo-workflows-1.0.24", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" moved the pin and left the measurement where it was") +PYQ +run nonzero "directory-manifest-size: a pin moves away from what was measured" \ + ./scripts/check-directory-manifest-size.py +res $F + +# A directory source nothing bounds. The Application is valid in every way the +# rest of this repository can check and produces nothing on a cluster. +F=scripts/directory-sources.json; mut $F +python3 - "$F" <<'PYQ' +import pathlib,sys,json +p=pathlib.Path(sys.argv[1]); d=json.loads(p.read_text()) +d["sources"].pop("argo-workflows-crds") +p.write_text(json.dumps(d, indent=2)+"\n") +print(" dropped the size record for a directory source that still ships") +PYQ +run nonzero "directory-manifest-size: a directory source nothing measured" \ + ./scripts/check-directory-manifest-size.py +res $F + F=addons/bootstrap/cert-manager/values-hub.yaml; mut $F; rm -f $F run nonzero "env-coverage: deleted hub delta" ./scripts/check-env-coverage.py res $F @@ -598,7 +649,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=48 +MIN_CHECKS=52 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index a65162e..fa78c6d 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -68,6 +68,9 @@ # A name restated in eleven places, and the two readers it takes to find # them all — half the corpus is chart source that does not parse as YAML. "test_secret_store_refs", + # The source-type decision that says which sources a size limit even applies + # to, and the byte accounting the repo-server does once it has decided. + "test_directory_manifest_size", ) # A floor well under the real count. It catches "discovery found almost nothing", @@ -142,7 +145,7 @@ # largest in the tree, and among them are the gates on the paths testing-rubric # calls security-critical. So this figure being low is not offset by behavioural # coverage for precisely the files where that offset was being claimed. -COMBINED_FLOOR = 42 +COMBINED_FLOOR = 44 # A ceiling on gate scripts carrying NO unit coverage at all, complementing the # floors below. The floors stop a covered file regressing; nothing stopped a NEW @@ -184,6 +187,9 @@ "scripts/check-alert-severity-routes.py": 95, "scripts/check-image-vulnerabilities.py": 41, "scripts/check-image-pins.py": 88, + # The half of the directory-size gate that decides the verdict. The rest is + # `--sync`, `--live` and `--self-test`, which run against a clone. + "scripts/check-directory-manifest-size.py": 70, } diff --git a/scripts/tests/test_directory_manifest_size.py b/scripts/tests/test_directory_manifest_size.py new file mode 100644 index 0000000..3dead2f --- /dev/null +++ b/scripts/tests/test_directory_manifest_size.py @@ -0,0 +1,424 @@ +"""Unit tests for the directory-source size gate. + +The gate rests on two transcriptions of argocd-repo-server, and a mistake in +either is silent: the corpus would be the wrong set of sources, or the byte +total would be the wrong number, and in both cases the run still prints a size +and a verdict. So these concentrate there. + +WHICH SOURCES. Only the Directory type is measured against the combined-manifest +ceiling, and "has a path" is not that type. ArgoCD takes an explicit `helm`, +`kustomize`, `plugin` or `directory` block at its word and otherwise decides by +what the directory holds. A source misclassified as Kustomize leaves this gate +with nothing to say about it. + +HOW MANY BYTES. The repo-server matches manifests by NAME before it reads them, +applies include/exclude to the path relative to the source root, descends only +when `directory.recurse` is set, and counts a jsonnet file as a manifest while +leaving its size out of the total. Each of those is a way to measure a different +number than a cluster will. +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import tempfile +import unittest + +from gateloader import load + +gate = load("check-directory-manifest-size") + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + + +def source(**kw): + base = {"appset": "a", "file": "a.yaml", "repo_url": "https://example.invalid", + "target_revision": "v1", "path": "crds", "recurse": False, + "include": "", "exclude": "", "local": None} + base.update(kw) + return gate.Source(**base) + + +def record(**kw): + base = {"repoURL": "https://example.invalid", "targetRevision": "v1", + "path": "crds", "recurse": False, "include": "", "exclude": "", + "bytes": 1000, "files": 2} + base.update(kw) + return base + + +CONTRACT = {"maxCombinedDirectoryManifestsSize": "20M", "argoCdDefault": "10M"} + + +def verdict(sources, records, contract=None): + """(exit code, everything the gate printed).""" + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = gate.check_offline(sources, records, contract or CONTRACT) + return rc, buf.getvalue() + + +def write(root: pathlib.Path, rel: str, size: int) -> pathlib.Path: + path = root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(b"x" * size) + return path + + +class TheQuantityGrammar(unittest.TestCase): + """A ceiling read in the wrong base is a ceiling in the wrong place.""" + + def test_decimal_and_binary_suffixes_are_different_sizes(self): + self.assertEqual(gate.quantity("10M", "t"), 10_000_000) + self.assertEqual(gate.quantity("10Mi", "t"), 10_485_760) + + def test_every_suffix_the_api_accepts(self): + for text, want in (("512", 512), ("4k", 4_000), ("1G", 10**9), + ("1Ki", 1024), ("2Gi", 2 * 2**30)): + self.assertEqual(gate.quantity(text, "t"), want, text) + + def test_something_the_repo_server_would_not_accept_cannot_run(self): + # Not a finding: the gate has no ceiling, so every source would measure + # as fitting. That is exit 2, and it must not be exit 0. + for text in ("20MB", "20 M", "1.5M", "twenty", "", "-1M"): + with self.subTest(text=text), contextlib.redirect_stdout(io.StringIO()), \ + self.assertRaises(SystemExit) as raised: + gate.quantity(text, "t") + self.assertEqual(raised.exception.code, gate.gatelib.CANNOT_RUN, text) + + +class WhichSourcesAreMeasured(unittest.TestCase): + """ArgoCD decides the type first; only Directory is measured this way.""" + + def test_an_explicit_block_names_the_type(self): + self.assertEqual(gate.explicit_type({"helm": {"valueFiles": []}}), "Helm") + self.assertEqual(gate.explicit_type({"kustomize": {"patches": []}}), "Kustomize") + self.assertEqual(gate.explicit_type({"plugin": {"name": "x"}}), "Plugin") + self.assertEqual(gate.explicit_type({"directory": {"recurse": True}}), "Directory") + + def test_a_chart_is_a_helm_source_without_a_helm_block(self): + self.assertEqual(gate.explicit_type({"chart": "argo-cd"}), "Helm") + + def test_an_empty_directory_block_still_names_the_type(self): + # `directory: {}` is how a source asks for the defaults, and it is the + # difference between a source this gate measures and one it never sees. + self.assertEqual(gate.explicit_type({"directory": {}}), "Directory") + + def test_a_bare_path_names_nothing_and_is_decided_by_the_directory(self): + self.assertIsNone(gate.explicit_type({"path": "addons/x", "repoURL": "u"})) + + def test_the_directory_decides_when_nothing_else_has(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + self.assertEqual(gate.discovered_type(root), "Directory") + (root / "kustomization.yaml").write_text("resources: []\n") + self.assertEqual(gate.discovered_type(root), "Kustomize") + (root / "Chart.yaml").write_text("name: x\n") + self.assertEqual(gate.discovered_type(root), "Helm") + + def test_every_name_kustomize_answers_to(self): + for name in gate.KUSTOMIZATION_NAMES: + with tempfile.TemporaryDirectory() as tmp, self.subTest(name=name): + root = pathlib.Path(tmp) + (root / name).write_text("resources: []\n") + self.assertEqual(gate.discovered_type(root), "Kustomize") + + def test_a_kustomization_that_disappears_reclassifies_the_source(self): + # The reason the type is decided rather than assumed: deleting the + # kustomization does not merely break a build, it moves that source into + # the population a size limit applies to. + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + (root / "kustomization.yaml").write_text("resources: []\n") + self.assertEqual(gate.discovered_type(root), "Kustomize") + (root / "kustomization.yaml").unlink() + self.assertEqual(gate.discovered_type(root), "Directory") + + +class TheByteAccounting(unittest.TestCase): + """Transcribed from getPotentiallyValidManifests, one clause at a time.""" + + def test_only_names_the_repo_server_treats_as_manifests_are_counted(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + for name in ("a.yaml", "b.yml", "c.json"): + write(root, name, 100) + for name in ("README.md", "OWNERS", "d.txt", "e.yaml.bak"): + write(root, name, 1000) + self.assertEqual(gate.measure(root, False, "", ""), (300, 3)) + + def test_it_does_not_descend_unless_recurse_is_set(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "top.yaml", 10) + write(root, "nested/deep.yaml", 500) + self.assertEqual(gate.measure(root, False, "", ""), (10, 1)) + self.assertEqual(gate.measure(root, True, "", ""), (510, 2)) + + def test_jsonnet_counts_as_a_manifest_and_not_against_the_size(self): + # The repo-server's own comment: jsonnet manages its own memory. A gate + # that counted it would refuse a source the runtime accepts. + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "a.yaml", 40) + write(root, "b.jsonnet", 9_000) + self.assertEqual(gate.measure(root, False, "", ""), (40, 2)) + + def test_include_and_exclude_read_the_path_relative_to_the_source(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "keep.yaml", 10) + write(root, "drop.yaml", 700) + self.assertEqual(gate.measure(root, False, "keep.yaml", ""), (10, 1)) + self.assertEqual(gate.measure(root, False, "", "drop.yaml"), (10, 1)) + + def test_a_symlink_contributes_the_size_of_what_it_points_at(self): + # FileInfo.Size() on a symlink is the length of its target path, which is + # why the repo-server stats the resolved file. Measuring the link instead + # understates a large manifest as a handful of bytes. + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "real/big.yaml", 5_000) + (root / "link.yaml").symlink_to(root / "real" / "big.yaml") + self.assertEqual(gate.measure(root, False, "", ""), (5_000, 1)) + + def test_a_symlink_to_nothing_is_not_a_manifest(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "a.yaml", 10) + (root / "dangling.yaml").symlink_to(root / "absent.yaml") + self.assertEqual(gate.measure(root, False, "", ""), (10, 1)) + + def test_an_empty_directory_measures_no_files_rather_than_a_small_size(self): + with tempfile.TemporaryDirectory() as tmp: + self.assertEqual(gate.measure(pathlib.Path(tmp), True, "", ""), (0, 0)) + + +class ExpandingATemplatedPath(unittest.TestCase): + def doc(self, elements): + return {"spec": {"generators": [{"matrix": {"generators": [ + {"list": {"elements": elements}}]}}]}} + + def test_the_element_path_and_the_environment_label(self): + got = gate.expansions('{{ .path }}/overlays/{{ index .metadata.labels "environment" }}', + self.doc([{"path": "addons/x"}]), ["development", "hub"]) + self.assertEqual(got, ["addons/x/overlays/development", "addons/x/overlays/hub"]) + + def test_a_literal_path_expands_to_itself(self): + self.assertEqual(gate.expansions("config/crd/standard", self.doc([]), ["development"]), + ["config/crd/standard"]) + + def test_a_shape_it_cannot_expand_keeps_its_braces(self): + # Which is the point: an unexpanded path is reported as one this + # repository cannot resolve, not dropped from the corpus. + got = gate.expansions("{{ .path.path }}", self.doc([]), ["development"]) + self.assertEqual(got, ["{{ .path.path }}"]) + self.assertIn("{{", got[0]) + + +class TheVerdict(unittest.TestCase): + def test_a_source_under_the_ceiling_passes(self): + rc, said = verdict([source()], {"a": record()}) + self.assertEqual(rc, 0, said) + + def test_the_boundary_is_the_one_the_repo_server_draws(self): + # It accumulates and aborts when the total EXCEEDS the limit, so a source + # exactly filling the ceiling still generates. A gate stricter than the + # runtime by one byte rejects a tree that works. + rc, said = verdict([source()], {"a": record(bytes=20_000_000)}) + self.assertEqual(rc, 0, said) + rc, said = verdict([source()], {"a": record(bytes=20_000_001)}) + self.assertEqual(rc, 1) + self.assertIn("contracts/repo-server.json", said) + + def test_a_directory_source_with_no_record_is_rejected(self): + rc, said = verdict([source()], {}) + self.assertEqual(rc, 1) + self.assertIn("scripts/directory-sources.json", said) + + def test_a_record_no_source_claims_is_rejected(self): + rc, said = verdict([source()], {"a": record(), "gone": record()}) + self.assertEqual(rc, 1) + self.assertIn("gone", said) + + def test_a_measurement_taken_at_another_revision_is_rejected(self): + rc, said = verdict([source(target_revision="v2")], {"a": record()}) + self.assertEqual(rc, 1) + self.assertIn("targetRevision", said) + + def test_every_coordinate_the_measurement_depends_on_is_compared(self): + for field, changed in (("repo_url", {"repo_url": "https://other.invalid"}), + ("path", {"path": "crds/minimal"}), + ("recurse", {"recurse": True}), + ("include", {"include": "*.yaml"}), + ("exclude", {"exclude": "*.json"})): + with self.subTest(field=field): + rc, _ = verdict([source(**changed)], {"a": record()}) + self.assertEqual(rc, 1) + + def test_a_measurement_of_nothing_is_not_a_pass(self): + rc, said = verdict([source()], {"a": record(bytes=0, files=0)}) + self.assertEqual(rc, 1) + self.assertIn("not the evidence", said) + + def test_an_unmeasurable_source_needs_a_stated_reason(self): + templated = source(path="{{ .path.path }}") + rc, _ = verdict([templated], + {"a": record(path="{{ .path.path }}", bytes=None, files=None)}) + self.assertEqual(rc, 1) + rc, said = verdict([templated], + {"a": record(path="{{ .path.path }}", bytes=None, files=None, + unbounded="bounded where it is written")}) + self.assertEqual(rc, 0, said) + self.assertIn("bounded where it is written", said) + + def test_an_in_tree_source_is_measured_rather_than_recorded(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "a.yaml", 30) + rc, said = verdict([source(local=root, path="addons/x")], {}) + self.assertEqual(rc, 0, said) + self.assertIn("in-tree", said) + + def test_an_in_tree_source_holding_no_manifest_is_rejected(self): + with tempfile.TemporaryDirectory() as tmp: + rc, said = verdict([source(local=pathlib.Path(tmp), path="addons/x")], {}) + self.assertEqual(rc, 1) + self.assertIn("addons/x", said) + + def test_an_in_tree_source_meets_the_same_boundary(self): + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + write(root, "a.yaml", 20) + tight = {"maxCombinedDirectoryManifestsSize": "20", "argoCdDefault": "10"} + rc, said = verdict([source(local=root, path="addons/x")], {}, tight) + self.assertEqual(rc, 0, said) + write(root, "b.yaml", 1) + rc, _ = verdict([source(local=root, path="addons/x")], {}, tight) + self.assertEqual(rc, 1) + + def test_a_contract_declaring_no_ceiling_cannot_run(self): + # Exit 2, not 0: with no ceiling every source measures as fitting, and a + # gate that reports that has answered a question it never asked. + with contextlib.redirect_stdout(io.StringIO()), \ + self.assertRaises(SystemExit) as raised: + gate.check_offline([source()], {"a": record()}, {"argoCdDefault": "10M"}) + self.assertEqual(raised.exception.code, gate.gatelib.CANNOT_RUN) + + +class TheLiveVerdict(unittest.TestCase): + """The half that catches what the tree cannot: a tag moved upstream. + + Nothing in a commit records that. The measurement is a function of + (repoURL, targetRevision, path), so the blocking gate already fails when a + pin moves — what it cannot see is the pin staying still while the thing it + names changes underneath it. + """ + + def live(self, measured, sources, records, contract=None): + real = gate.remeasure + gate.remeasure = lambda src: measured + try: + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + rc = gate.check_live(sources, records, contract or CONTRACT) + return rc, buf.getvalue() + finally: + gate.remeasure = real + + def test_a_source_that_still_measures_what_was_recorded_passes(self): + rc, said = self.live((1000, 2), [source()], {"a": record()}) + self.assertEqual(rc, 0, said) + + def test_a_tag_that_moved_under_a_still_pin_is_rejected(self): + rc, said = self.live((2000, 3), [source()], {"a": record()}) + self.assertEqual(rc, 1) + self.assertIn("The pin did not move", said) + + def test_the_boundary_is_the_repo_servers_here_too(self): + rc, said = self.live((20_000_000, 2), [source()], + {"a": record(bytes=20_000_000)}) + self.assertEqual(rc, 0, said) + rc, _ = self.live((20_000_001, 2), [source()], + {"a": record(bytes=20_000_001)}) + self.assertEqual(rc, 1) + + def test_a_source_resolving_to_no_manifest_file_is_rejected(self): + # The empty comparison. A path that resolves and holds nothing generates + # nothing, and reporting it as under the limit is the defect. + rc, said = self.live((0, 0), [source()], {"a": record()}) + self.assertEqual(rc, 1) + self.assertIn("generates nothing", said) + + def test_resolving_nothing_at_all_is_not_a_pass(self): + with contextlib.redirect_stdout(io.StringIO()), \ + self.assertRaises(SystemExit) as raised: + self.live((1, 1), [source(path="{{ .path.path }}")], {}) + self.assertEqual(raised.exception.code, 1) + + +class TheRecordKeys(unittest.TestCase): + def test_one_source_per_appset_keys_on_the_appset(self): + self.assertEqual(sorted(gate.keyed([source(appset="crds")])), ["crds"]) + + def test_two_sources_in_one_appset_are_told_apart_by_path(self): + got = sorted(gate.keyed([source(appset="crds", path="a"), + source(appset="crds", path="b")])) + self.assertEqual(got, ["crds#a", "crds#b"]) + + +class TheShippedCatalog(unittest.TestCase): + """What the gate says about the tree it is shipped with.""" + + def setUp(self): + self.sources = gate.directory_sources() + self.keyed = gate.keyed(self.sources) + + def test_the_corpus_is_not_empty(self): + self.assertTrue(self.sources) + + def test_the_pinned_crd_directories_are_in_it(self): + # Both are plain manifest directories in someone else's repository, which + # is the shape the repo-server measures. + self.assertIn("argo-workflows-crds", self.keyed) + self.assertIn("gateway-api-crds", self.keyed) + + def test_no_kustomize_overlay_is_in_it(self): + # Every in-tree source this catalog ships carries a kustomization, so + # anything under addons/ or policies/ appearing here means one stopped + # being a kustomize root. + stray = [k for k, s in self.keyed.items() + if s.local is not None and s.path.startswith(("addons/", "policies/"))] + self.assertEqual(stray, []) + + def test_every_source_has_a_record_or_is_measured_in_tree(self): + recorded = gate.load_records() + for key, src in self.keyed.items(): + with self.subTest(key=key): + self.assertTrue(src.local is not None or key in recorded) + + def test_the_records_carry_a_size_or_a_reason_they_cannot(self): + for key, rec in gate.load_records().items(): + with self.subTest(key=key): + self.assertTrue(rec.get("bytes") is not None or rec.get("unbounded")) + + def test_the_contract_declares_a_ceiling_the_repo_server_would_accept(self): + contract = gate.load_contract() + self.assertGreater( + gate.quantity(contract["maxCombinedDirectoryManifestsSize"], "t"), 0) + + def test_the_catalog_passes_its_own_gate(self): + rc, said = verdict(self.sources, gate.load_records(), gate.load_contract()) + self.assertEqual(rc, 0, said) + + def test_the_record_file_is_what_sync_would_write(self): + # A hand-edited record is a measurement nobody took. + doc = json.loads((ROOT / "scripts" / "directory-sources.json").read_text()) + self.assertEqual(doc["_README"], gate.README) + + +if __name__ == "__main__": + unittest.main()