diff options
| author | Yuriy Andamasov <yuriy@vyos.io> | 2026-08-21 23:25:49 +0300 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2026-08-21 23:25:49 +0300 |
| commit | 3a1c6c3020c4ed819616098a91be670719e290cc (patch) | |
| tree | 758da979ad4f59d9f92988bede8bd241c3c6b3c8 /.github/workflows | |
| parent | 4a5950de1a4ad6e0a5390d1edb089f7139e7dfc5 (diff) | |
| download | vyos-documentation-3a1c6c3020c4ed819616098a91be670719e290cc.tar.gz vyos-documentation-3a1c6c3020c4ed819616098a91be670719e290cc.zip | |
fix: IS-572: correct verified defects in the Cloudflare Workers docs pipeline (#2209)
* fix: IS-572: correct verified defects in the Cloudflare Workers docs pipeline
Fixes raised against the LTS port PR but landing on rolling first, so the
backport runs in the right direction, plus one defect found while verifying
the nightly canary sweep.
- apex PDF: a 206 is now only answered to a request that actually carried a
Range header, and every R2Range shape is normalized to concrete bounds.
R2 reports a whole-object range on un-ranged gets, so keying the 206 off
obj.range alone made every plain GET of the 1.3 PDF a 206.
- apex PDF: a failed If-Match now maps to 412, not 304.
- apex sitemap index: entries derive their origin from the request, so the
canary index no longer points at production.
- UA policy: drop the dead Google-Extended entry (a robots.txt token, never
a UA) and log Applebot-Extended while still allowing Applebot, via a
most-specific-match precedence between the allow and log lists.
- docs-build: replace the check_head guard's exit 78 (neutral only in the
retired Actions v1 runtime) with an output gating every deploy/promote
step, so a superseded run ends green-and-skipped instead of red.
- smoke/parity: read CF Access service-token credentials from the
environment instead of argv, and pass them via env: in both workflows.
- smoke: stop asserting X-Docs-Build on the PDF probe — apex's R2 fallback
path legitimately never sets it, making the probe unpassable for 1.3.
- gates/smoke: strip critical-pages lines before testing for comments.
- parity: fetch each sitemap once, honouring the _SCHEME override.
Advances: IS-572
* fix: IS-572: harden check_head guard against a failed ls-remote
Phase-0 CodeRabbit findings on the new guard, both valid:
- set -euo pipefail + an explicit emptiness check, so a failed git ls-remote
cannot leave $remote empty and be misread as 'branch moved' (cut exits 0
on empty input) — that would silently skip a deploy that should have run.
- pass github.ref_name and github.sha through env vars rather than
interpolating them into the shell command.
Advances: IS-572
* fix: IS-572: address adversarial + CodeRabbit review on the CF docs gates
Answers the seven adversarial-review findings on vyos/vyos-documentation#2209,
five of which GitHub CodeRabbit independently reproduced on review 4946100889.
Range handling (apex Worker) — the reported defect was real but mis-diagnosed.
Probing a real R2 binding under vitest-pool-workers shows R2 never returns the
inverted/zero-length range the review predicted: for the Headers form this
Worker uses, R2 answers every unsatisfiable, malformed, multi-range and
unknown-unit Range by IGNORING it and returning the complete object with
range={offset:0,length:size} — indistinguishable from a satisfied whole-object
range, and it does not throw. (Only the object-literal form throws, code 10039,
which this Worker cannot reach.) Trusting obj.range therefore answered
`Range: bytes=99-` with 206 + `Content-Range: bytes 0-8/9` and the whole body,
so a client resuming mid-download would silently corrupt its file. Range intent
is now re-derived from the client's own header per RFC 9110 §14.1.2:
unsatisfiable → 416 + `Content-Range: bytes */size`, ignored → 200 with the
complete body, satisfiable → 206 as before. resolveRange() additionally clamps
its length at zero so its documented contract holds for every R2Range input.
Preconditions: reorder to RFC 9110 §13.2.2's MUST-ordered precedence
(If-Match, If-Unmodified-Since, If-None-Match, If-Modified-Since) so
`If-Match: "old"` + `If-None-Match: "new"` yields 412 rather than 304, and gate
304 on GET/HEAD per §13.1.2 so a conditional POST cannot be answered 304.
UA gate: a log match now wins a contest against an allow match unless the allow
entry strictly contains the log entry, so `GPTBot/1.0 DuckDuckBot` emits its
ua-log event instead of being swallowed by the longer allow needle. Applebot /
Applebot-Extended verdicts are unchanged and now pinned by tests.
parity.py: restore the exact-200 sitemap check (_OPENER returns for any 2xx, so
a 204 yielded an empty corpus and the gate passed having probed nothing) and
route the sitemap request through a single credential-attaching constructor, so
--sitemap-host against the Access-gated canary no longer 403s on every sitemap.
Secrets: --access-id/--access-secret removed from both gates in favour of the
paired env vars. An argv value is readable from the process table and captured
by `set -x`; no call site used the flags. smoke.py reads --critical-list via
Path.read_text() rather than leaking a file handle.
Tests: 115 -> 138 worker tests, 46 -> 50 gate tests. Every new behavioural test
was verified to fail against the pre-fix source.
Advances: IS-572
* test: IS-572: compare the forbidden UA token case-insensitively
bestMatch() lowercases every ua-policy.json entry before matching, so a
"google-extended" entry would be functionally identical to the "Google-Extended"
token this guard exists to keep out. toContain() compares primitives by strict
equality, so the lowercase variant passed the assertion and would have quietly
restored the entry.
Lowercase both sides so the assertion matches the matcher's own case semantics.
Verified by injecting a lowercase "google-extended" into the log list: the test
now fails, and passed before this change.
Advances: IS-572
* fix: IS-572: scope the CF Access token per host; correct range + precondition semantics
Round-3 adversarial review findings. Each fix has a test that fails against 5b4f40f9.
parity.py sent the CF Access service token to every host the run touched, which
pre-cutover includes --sitemap-host docs.vyos.io — still served by ReadTheDocs.
Credentials are now modelled as an Access record bound to --probe-host, and
build_request() attaches them per destination rather than per run, so the token
reaches only the host it belongs to while the post-cutover same-host
configuration stays credentialed.
The apex Worker's range handling assumed its own regex agreed with R2's parser.
It does not: R2 accepts ASCII space only, this one accepts \s, so a
tab-separated Range parsed here, was ignored there, and produced a 206 carrying
the whole object in answer to a request for three bytes. classifyRangeHeader now
returns the concrete bounds it derived and the Worker checks them against the
bytes R2 actually returned before promising a 206 — any parser divergence
degrades to a 200 instead of corrupting a resumed download. Range positions are
compared as digit strings, so specs above 2^53 no longer collapse into each
other, and Range is ignored outright on every method but GET (§14.2).
If-Range was silently dropped: R2Conditional carries no such validator, so a
stale range request was answered with bytes of the new representation under a
206. It is now evaluated against the object's own ETag, with a failed validator
serving the complete representation.
Preconditions were inferred from which headers were present, which cannot be
right in both directions — If-Match satisfied plus If-None-Match satisfied owed
a 304 and got a 412. The conditionals are filtered to those §13.2.2 applies to
this request before R2 sees them, and a body-less result is resolved by
evaluating the validators in §13.2.2 order.
Also drops the UA gate's narrow-allow carve-out. It compared the two matched
policy entries rather than the UA's token structure, so a request-controlled UA
quoting both tokens bought itself an allow. No pair in ua-policy.json used it.
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: cancel abandoned R2 body streams; drop stale --access-* comments
CodeRabbit round-3 follow-up.
R2 hands back a body on two paths whose response carries none, and dropping the
reference leaves the stream open holding its connection until GC. CodeRabbit
flagged the If-Range re-read; the larger instance is the 416, where R2 answers an
unsatisfiable Range with the COMPLETE object — 29.2 MiB for the 1.3 PDF — and the
response sends none of it. Both now go through discardBody(), which cancels so
the transfer is aborted rather than drained, and swallows failures because the
response on those paths is already decided.
Keeping the get-then-re-read rather than switching to head()-then-get(): a head()
would avoid opening the slice stream, but it would add a round-trip to the path
where If-Range MATCHES — the normal resumed download — to tidy the rare path
where it does not, and it would open a TOCTOU window that the current shape does
not have, since here the object whose validator was checked is the very response
served.
The R2 test mock returned string bodies, which is why this class of bug was
invisible to it. Bodies are now real ReadableStreams that record their own
cancellation, so the three new tests observe the actual stream lifecycle: the 416
cancels the whole object, the stale If-Range cancels the discarded slice, and a
served body is never cancelled.
Also corrects two comments that still described --access-id/--access-secret as a
manual fallback; the flags were removed in round 2 and argparse rejects them. A
repo-wide sweep found no other prose describing the removed interface. The
docs-build.yml change is one comment line inside an env: block; check_head is
untouched.
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: compare Access origins; ignore conditionals per method
Round-4 adversarial review findings.
parity.py: _authority() compared (hostname, port) verbatim, so `p.invalid`,
`p.invalid:443` and the trailing-dot FQDN form were three different origins.
Post-cutover both --sitemap-host and --probe-host name the same Access-gated
host; spelling the default port out on either cost the sitemap fetch its token
and 403'd it, reverting the same-host keeper case. Compare normalized origins
instead: lowercased host, single trailing dot stripped, the scheme's default
port folded to None (bare arguments read under _SCHEME). IDN/punycode
equivalence stays out of scope -- both hosts are ASCII literals passed by CI.
apex worker: applicablePreconditions() filtered conditionals by validator
applicability but not by method, so RFC 9110 13.2.1 -- "a server MUST ignore
the conditional request header fields ... such as CONNECT, OPTIONS, or TRACE"
-- was unmet: an OPTIONS carrying a stale If-Match was refused 412 where the
same request without it succeeded. Take the method and return an empty set for
those three; R2 is then given nothing to evaluate.
apex worker: the stale-If-Range re-read was a bare get(), dropping the other
preconditions, so a key rewritten between the two reads was served under a 200
though the request's If-Match excluded it. The legacy snapshot deploy's
force_pdf_refresh input does rewrite this key. Carry the same onlyIf into the
re-read and resolve a body-less result through preconditionStatus() on the new
object's validators.
Advances: IS-572
* fix: IS-572: scope the Access token by origin, not authority; hide the secret from repr
Round-5 CodeRabbit findings, both on the CF Access credential primitive in
scripts/docs_gates/parity.py.
_authority() returned (hostname, port) and dropped the scheme, so an https
credential also applied to the plaintext http URL of the same name --
Access("p.invalid", ...).applies_to("http://p.invalid/") was True, and
build_request() would then have attached CF-Access-Client-Id/-Secret to a
cleartext request. `http://p.invalid:80/` is the sharp case: 80 folds to None
under http, matching the https-scoped ("p.invalid", None) exactly.
Not reachable in current callers: every URL in the module is built from
_SCHEME and both workflow invocations pass bare hosts, so a single run is
single-scheme and the http value exists only for the test monkeypatch. Fixed
as a contract defect regardless -- applies_to() documents itself as comparing
ORIGINs, an origin is scheme+host+port, and over-broad credential scope is the
failure mode this primitive already produced once (round 2, token sent to
docs.vyos.io). Return (scheme, host, port), resolving a bare argument's scheme
under _SCHEME as before, and fold the default port against the origin's own
scheme so http :80 and https :443 stay distinct rather than collapsing
together. The four origin-normalization cases and the :8443 denial are
unchanged.
Access.client_secret is now dataclasses.field(repr=False). The default
dataclass repr rendered every field, so a failed assertion, a debug print or
any exception interpolating an Access would have put the service token into CI
output, which is durable and world-readable for this repo. The client id stays
in repr: it names which token without being the credential, and dropping it
would make a scoping failure much harder to diagnose. The secret remains
readable as a field, which is the only way it is ever consumed.
Swept the rest of scripts/docs_gates/ for the same exposure: smoke.Probe holds
no credential fields, smoke passes the id/secret as plain locals that reach no
output path, and the str(e) texts recorded in parity-report.json and the
SMOKE-FAIL lines carry urllib's code/reason/URL, never request headers.
Two tests added, both verified failing against the pre-fix source.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: roll back to the newest deployment's 100%-traffic version id
`wrangler deployments list --json` sorts ASCENDING by created_on
(wrangler 4.123.0 versionsDeploymentsListHandler), so `.[0]` selected the
OLDEST deployment, not the current one the stale comment claimed. It also
took the deployment `id`, but `wrangler rollback` resolves its positional
through fetchVersion() and needs a VERSION id.
Select newest-first by created_on and take the version_id serving 100% of
traffic, walking to the next-newest deployment when traffic is split across
versions -- mirroring wrangler's own fetchDefaultRollbackVersionId(), minus
its .shift() (it picks after deploying; we capture before). Non-array or
unparseable output now fails safe to an empty id instead of a wrong one.
Rewrite the stale comment to describe the actual JSON shape, note in the
consumer that rollback_id is a version id, and add --message for rollback
audit context (supported at 4.123.0; --yes supplies it non-interactively).
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: re-check the branch tip after smoke; harden rollback capture
Two production-safety fixes in docs-build.yml.
1. TOCTOU on the check_head guard. It resolved the branch tip ONCE, before the
candidate deploy and before the smoke gate (480s deadline), then gated
PROMOTE, the post-promote probe and the registry pointer publish on that
minutes-old answer. A push landing in that window left the run promoting a
superseded SHA to production and publishing a pointer naming it.
`cancel-in-progress: false` does not cover this: it makes the SECOND run
queue and skip, while the run already past the check sails on.
Add a second guard after smoke, immediately before the first
production-facing step, and move PROMOTE + probe + pointer publish onto its
output. The probe must move with PROMOTE: left on the first check it would
run after a skipped promote, see production serving the previous SHA, read
that as a promote failure and roll back a production nobody touched.
A branch that moved mid-run ends GREEN-and-skipped, matching the convention
the exit-78 redesign established: production and the registry are untouched,
and the queued run promotes the new tip, so there is nothing for an operator
to act on. The residual check-to-deploy window is one step boundary; no
compare-and-swap exists on the Cloudflare side to close it entirely.
2. The rollback-capture bootstrap branch swallowed every failure. `2>/dev/null`
plus a bare `if` could not tell "Worker does not exist yet" from auth,
network or malformed output, so a transient failure yielded an empty
rollback_id, logged "nothing to roll back to" and promoted with the
auto-rollback silently disarmed while the run looked healthy.
Only a genuine not-found may now produce an empty id. wrangler 4.123.0 exits
1 on every error path and writes nothing machine-readable to stdout when the
command fails, so the discriminator is the API error code it renders into
stderr: 10007/10090, the pair its own isWorkerNotFoundError() uses. Auth
carries 9106/10000 and connectivity failures carry no code, so neither can be
mistaken for bootstrap. Anything unmatched fails the step. Non-array output
now fails instead of being coerced to []; an empty list and a split-traffic
list are reported distinctly (notice / warning) rather than sharing one
ambiguous message with the failure paths.
The code pair was read out of the shipped 4.123.0 bundle, not observed
against a live missing Worker; the comment says so and asks for re-verification
on wrangler bumps.
Also corrects two comments the change falsifies: the concurrency note claiming
queued runs are covered by "the check_head guard", and the exit-78 note claiming
promote/registry gate on the first check.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: run the post-promote probe whenever the deploy succeeded
PROMOTE ran `wrangler deploy` and then the hostname purge, the tarball and
three `r2 object put` calls in one step, while the post-promote probe named no
status function in its `if:` and so carried an implicit success(). A transient
purge 5xx, a tar error or one failed upload therefore failed PROMOTE, skipped
the probe entirely, and left the new version LIVE on the production Worker,
never verified and never rolled back.
Split the step at the deploy. Everything fallible after it moves to a new
`promote_publish` step gated on `steps.promote.outcome == 'success'`, and the
probe becomes the verification finalizer: `!cancelled() && steps.promote.outcome
== 'success'`. `!cancelled()` counts as a status function, which is what
suppresses the implicit success() and lets the probe run after a failed
purge/upload; it is not `always()`, because firing an auto-rollback while an
operator tears the run down is the wrong reflex. Gating on PROMOTE's outcome
rather than re-reading check_head_2 is strictly stronger: 'skipped' on the
moved-branch path, 'success' only once the production deploy completed. The
registry-pointer publish keeps its bare condition, whose implicit success() is
now load-bearing — it is what stops a pointer from naming a rolled-back
generation.
Rollback-target capture no longer reaches backwards. The old jq flattened
`versions[]` across ALL deployments and took the first 100%-traffic id found
scanning newest→oldest, so a current deployment deliberately serving A/B at
90/10 resolved to an older deployment's version — and a probe failure would
"roll back" to it and destroy the split. It now inspects only the current
deployment and refuses otherwise. The verdict is discriminated inside jq so a
malformed record fails red instead of masquerading as split traffic:
`select(.percentage == 100) | .version_id` on `{"percentage":100}` yielded null,
`null // empty` yielded empty, and empty was read as "split" — an unparseable
API response promoting with the auto-rollback silently disarmed. Non-numeric
`percentage`, empty/absent `versions` and unsortable `created_on` are rejected
for the same reason.
The hostname purge is retried once. It is the one post-deploy failure with an
asymmetric knock-on: the probe measures the edge, so a purge that never lands
makes a healthy deploy look like a failed promote.
Finally, the apex PDF range-divergence fallback no longer answers with a 206
carrying bounds the client never requested (RFC 9110 §14.4). It discards the
slice and fails 503, keeping the r2-range-divergence log line that is the actual
alarm. Unreachable under today's workerd, which ignores multi-range.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* ci: IS-572: bind DOCS_CF_LIVE via env: in the purge/registry step
The purge branch expanded ${{ vars.DOCS_CF_LIVE }} directly into the shell
body while the other three consumers of the same value bind it through env:
(PDF carry-forward, candidate reset, post-promote probe). zizmor flags the
inline form as template expansion; the value is repo-controlled so it is not
exploitable, but the env form removes the class and matches the surrounding
steps.
* ci: IS-572: validate DOCS_CF_LIVE, pin wrangler, bound rollback percentages
Three Major findings from CodeRabbit review 4971096766, all outside-diff.
DOCS_CF_LIVE failed open. Four steps branch on it — the SKIP_PDF carry-forward
staleness check, the candidate-reset staleness check, the production hostname
purge and the post-promote probe — and every one compares against the literal
string "true", so an unset or misspelled repository variable silently took the
branch that skips the check, skips the purge and skips the probe, after which
the registry pointer published an unpurged, never-verified deployment on a green
run. Validate the value once at the top of the only job that reads it, so all
four comparisons are safe by construction instead of each re-deriving the guard.
`npx wrangler` in the previous-build metadata fetch ran before `npm ci`, so on
the normal (non-SKIP_PDF) path it downloaded whatever version the registry
served rather than the lockfile-pinned 4.123.0 every later step uses. Move the
install ahead of the first wrangler invocation and make it unconditional; the
inline `npm ci` in the SKIP_PDF branch and the conditional install step after
the check_head guard are both redundant once it is.
The rollback-target jq required `percentage` to be numeric but not in range, so
a current deployment reporting both 100 and 101 yielded exactly one 100%-traffic
match and classified as a determinate `ok` — arming a rollback with a version
selected from data already known to be nonsense. Out-of-range now routes to the
malformed path, never to `split:`.
Advances: IS-572
* ci: IS-572: validate rollback_id at capture, bind it via env at the finalizer
rollback_id originates from Cloudflare's API (`wrangler deployments list
--json`), so it is untrusted text. It was written to $GITHUB_OUTPUT
unvalidated and then reached shell source through a template expansion at
three sites in the post-promote finalizer.
The $GITHUB_OUTPUT write is the sharper of the two exposures: a value
containing a newline injects arbitrary ADDITIONAL step outputs regardless
of how carefully the consumer quotes, because no shell is involved. Validate
at capture — the only place that can stop it.
The character class is deliberately wider than the observed format. wrangler
4.123.0 performs no client-side validation of a version id (it interpolates
the string straight into the API URL path), so the canonical UUID shape is
what Cloudflare's bundled SDK documents rather than something the CLI
enforces. Hard-failing on anything but a UUID would turn a Cloudflare format
change into a blocked promote; hard-failing outside [A-Za-z0-9._-] cannot,
and still certainly excludes newlines, quotes, whitespace and every shell
metacharacter. A safe-but-non-UUID value warns instead of erroring.
Empty stays legitimate — it is the no-target state of all three benign paths
(first deploy, no deployments, split traffic) and the finalizer's -z branch
depends on it. A malformed non-empty value fails loudly, matching how the
surrounding capture already treats malformed API data.
At the finalizer, ROLLBACK_ID is bound through the step's env: mapping and
the three sites use "$ROLLBACK_ID", so the value is never substituted into
the shell source. The -z semantics are unchanged.
Advances: IS-572
* ci: IS-572: verify probe status, reconcile a failed pointer publish
Two merge blockers from the pre-merge adversarial gate on d079a2c9.
1. The post-promote probe accepted an error response as verification.
`withDocsHeaders` sets X-Docs-Build unconditionally, so error responses
carry it too; the probe read only that header and never the status line,
so a production answering 500 with the new SHA satisfied the match and
the registry pointer published for a generation serving errors.
The probe now parses the status line and requires an exact 200 alongside
the built SHA. Parsing resets on every status line, so 1xx interim
responses contribute nothing and only the final response block is read;
duplicate X-Docs-Build headers with identical values are accepted while
conflicting ones surface as "<conflicting>" and can never match. Requests
are bounded with --max-time 20 and the pipeline runs under pipefail so a
transport failure is distinguishable from a missing header. Early-attempt
non-200s and transport failures stay retryable — only the final attempt's
verdict decides. Every unverified outcome still rolls back, but the three
causes now carry distinguishable diagnoses.
--fail is deliberately not used: it collapses 4xx/5xx into curl exit 22
and discards the status line, which is the value needed to tell a broken
new version apart from a transport failure.
2. Production and the registry could diverge permanently. The round-8
promote/publish split correctly decoupled a publish failure from the
probe, but added no convergence action for the case where the probe then
succeeds: production served the new SHA while latest.json still named the
old one, with nothing to reconcile it. latest.json is the source of truth
for the candidate-reset staleness check and the SKIP_PDF carry-forward, so
both would later restore a generation production no longer serves.
A reconciliation step now runs when the promote and probe both succeeded
but the pointer did not publish. It converges FORWARD — re-uploading the
sha-scoped generation and only then publishing the pointer, so the pointer
can never name an incomplete generation — and on failure leaves the
pointer untouched and reports the divergence by name with both SHAs and a
copy-pasteable manual repair. Gating on the probe's success keeps it off
the rollback paths entirely, where production and the pointer already
agree. The pointer step's load-bearing implicit success() is unchanged.
Refs: IS-572
* ci: IS-572: drop registry-pointer reconciliation, keep the probe fix
Scope reduction. The `Reconcile registry pointer after a failed publish`
step introduced in the previous commit is removed; the post-promote probe
rewrite in that same commit stays.
Removing the step returns registry-divergence behaviour to what `rolling`
has today: when `promote_publish` fails after a successful deploy, the
pointer step's load-bearing implicit success() skips it, and production can
end up serving a SHA that `$slug/latest.json` does not name. That path is
now documented at the `Publish registry pointer` step and tracked in
https://vyos.dev/T9237 rather than repaired here.
Also reverts the DOCS_CF_LIVE header comment to "four later steps", which
restores its internal agreement with the "four comparisons downstream"
sentence in the same block.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
Diffstat (limited to '.github/workflows')
| -rw-r--r-- | .github/workflows/docs-build.yml | 509 | ||||
| -rw-r--r-- | .github/workflows/docs-canary-qa.yml | 21 |
2 files changed, 485 insertions, 45 deletions
diff --git a/.github/workflows/docs-build.yml b/.github/workflows/docs-build.yml index bdb9e41d..8855b8d9 100644 --- a/.github/workflows/docs-build.yml +++ b/.github/workflows/docs-build.yml @@ -13,8 +13,12 @@ on: concurrency: group: docs-build-${{ github.ref_name }} # false (not true): a cancel mid-promote would leave production unverified and the - # registry pointer stale — queued runs are safe because the check_head guard (§7.1) - # skips any run whose SHA is no longer the branch tip. + # registry pointer stale. Queueing instead is safe because BOTH check_head guards + # (§7.1) skip a run whose SHA is no longer the branch tip — the first before the + # candidate deploy, the second after the smoke gate and immediately before promote. + # A single up-front check was NOT enough: it says nothing about the run that is + # already past it, and the candidate deploy plus the smoke gate (480s deadline) give + # a push minutes of room to land in between. cancel-in-progress: false permissions: @@ -40,6 +44,37 @@ jobs: echo "pdf=$(jq -r --arg s "$(echo "$entry" | jq -r .slug)" \ '.versions[] | select(.slug==$s) | .pdf // empty' workers/versions.json)" >> "$GITHUB_OUTPUT" + # DOCS_CF_LIVE is the pre/post-cutover switch, and four later steps in this job + # branch on it: the SKIP_PDF carry-forward staleness check, the candidate-reset + # staleness check, the production hostname purge, and the post-promote probe. + # Every one of those comparisons is written against the literal string "true", so + # ANY other value — an unset variable, "True", "yes", a typo — silently takes the + # branch that skips the check, skips the purge and skips the probe, after which the + # registry pointer publishes an unpurged, never-verified deployment on a green run. + # Validate the value ONCE, here, at the top of the only job that reads it: the four + # comparisons downstream are then safe by construction rather than each re-deriving + # the same three-line guard. Placed before the docker build so a misconfigured + # variable costs seconds, not a full build. A future job that reads this variable + # needs its own gate — this one covers build-deploy only. + - name: Validate DOCS_CF_LIVE + env: + DOCS_CF_LIVE: ${{ vars.DOCS_CF_LIVE }} + run: | + set -eu + case "$DOCS_CF_LIVE" in + true|false) + echo "DOCS_CF_LIVE=$DOCS_CF_LIVE" + ;; + "") + echo "::error::repository variable DOCS_CF_LIVE is unset or empty — it must be set to exactly 'true' or 'false'" + exit 1 + ;; + *) + echo "::error::repository variable DOCS_CF_LIVE is '$DOCS_CF_LIVE' — it must be exactly 'true' or 'false' (lowercase, no quotes)" + exit 1 + ;; + esac + # Build image in-workflow from docker/Dockerfile (plan v4.1: digest pin dropped — # no published ghcr.io image exists; the checked-out commit IS the pin, and the # buildx GHA cache makes repeat builds cheap). @@ -117,6 +152,16 @@ jobs: cp -r docs/_build/html/. "dist/assets/en/$slug/" npx --yes pagefind@1.5.2 --site "dist/assets/en/$slug" + # Must precede the FIRST `npx wrangler` invocation in this job. Without + # workers/node_modules, `npx` silently downloads whatever wrangler the registry + # serves today, so the SKIP_PDF carry-forward and the previous-build metadata fetch + # below would run on an unpinned version while every later step — including the + # rollback path — runs the lockfile-pinned one. Unconditional on purpose: the + # metadata fetch below runs on every build, long before the check_head guard that + # used to gate this install. + - name: Install workers deps + run: cd workers && npm ci + - name: PDF into artifact (build or registry carry-forward) env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} @@ -130,7 +175,7 @@ jobs: # §5 + §7.1.4: SKIP_PDF refuses to run when registry is stale vs production. # Registry layout is pointer-indirected (§7.1.4 atomicity): resolve # $slug/latest.json → sha, then read the sha-scoped generation. - cd workers && npm ci && npx wrangler r2 object get "$REGISTRY_BUCKET/$slug/latest.json" --file /tmp/latest.json --remote && cd .. + cd workers && npx wrangler r2 object get "$REGISTRY_BUCKET/$slug/latest.json" --file /tmp/latest.json --remote && cd .. reg_sha=$(jq -r .sha /tmp/latest.json) if [ "$DOCS_CF_LIVE" = "true" ]; then prod_sha=$(curl -sI "https://docs.vyos.io/en/$slug/" | tr -d '\r' | awk -F': ' 'tolower($1)=="x-docs-build"{print $2}') @@ -191,16 +236,37 @@ jobs: --versions workers/versions.json \ $( [ -f previous-meta.json ] && echo --previous-meta previous-meta.json ) + # Exit 78 was the "neutral" status ONLY in the deprecated Actions v1 runtime. On the + # current runtime any non-zero exit fails the step and the job, so this guard's own + # "skipping deploy" message was a lie: every legitimate branch-moved race — the exact + # situation `cancel-in-progress: false` deliberately creates — surfaced as a red run. + # That erodes the guard's value as an alarm. Publish an output instead and gate the + # deploy steps on it, so a superseded run ends green-and-skipped. This is the FIRST + # of two checkpoints: it covers the candidate deploy and the smoke gate, which touch + # nothing production-facing. PROMOTE, the post-promote probe and the registry pointer + # publish gate on the post-smoke re-check further down, NOT on this output. - name: check_head guard (§7.1) + id: check_head + env: + REF_NAME: ${{ github.ref_name }} + BUILT_SHA: ${{ github.sha }} run: | - set -eu - remote=$(git ls-remote origin "refs/heads/${{ github.ref_name }}" | cut -f1) - [ "$remote" = "${{ github.sha }}" ] || { echo "branch moved — skipping deploy"; exit 78; } - - - name: Install workers deps - run: cd workers && npm ci + set -euo pipefail + remote=$(git ls-remote origin "refs/heads/$REF_NAME" | cut -f1) + # pipefail AND the emptiness check are both needed: `cut` exits 0 on empty input, so + # a failed ls-remote would otherwise leave $remote empty and be misread as "branch + # moved" — silently skipping a deploy that should have run. An unresolvable tip is + # an infrastructure failure, not a skip signal, so it fails the job loudly. + [ -n "$remote" ] || { echo "::error::could not resolve the remote tip of $REF_NAME"; exit 1; } + if [ "$remote" = "$BUILT_SHA" ]; then + echo "current=true" >> "$GITHUB_OUTPUT" + else + echo "current=false" >> "$GITHUB_OUTPUT" + echo "::notice::branch moved (tip is now $remote, this run built $BUILT_SHA) — skipping deploy, promote and registry steps" + fi - name: Deploy CANDIDATE + if: steps.check_head.outputs.current == 'true' env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -213,6 +279,16 @@ jobs: - name: Pre-traffic smoke via canary apex (§7.1.2) id: smoke + if: steps.check_head.outputs.current == 'true' + env: + # CF Access service token goes through the environment, NEVER through argv: a + # secret on the command line is readable from the process table for the life of + # the process and is captured verbatim by `set -x` traces and crash dumps. + # These environment variables are the ONLY supported way to pass the token: the + # --access-id / --access-secret flags were removed, and argparse now rejects them + # outright rather than ignoring them and leaving every probe to 403. + CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} + CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} run: | set -eu # NOTE: never use the GHA "cond AND format(...)" expression trick for optional @@ -227,12 +303,10 @@ jobs: python -m scripts.docs_gates.smoke \ --host docs-next.vyos.io --slug '${{ steps.matrix.outputs.slug }}' \ --expect-sha '${{ github.sha }}' \ - --access-id '${{ secrets.CF_ACCESS_CLIENT_ID }}' \ - --access-secret '${{ secrets.CF_ACCESS_CLIENT_SECRET }}' \ $pdf_arg - name: Candidate reset on smoke failure (§7.1.3) - if: failure() && steps.smoke.conclusion == 'failure' + if: failure() && steps.check_head.outputs.current == 'true' && steps.smoke.conclusion == 'failure' env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -257,8 +331,68 @@ jobs: --name '${{ steps.matrix.outputs.worker }}-candidate' \ --var DOCS_BUILD_SHA:"$reg_sha" --var DOCS_ENV:canary - - name: PROMOTE (capture rollback id → deploy → purge → registry, §7.1.4) + # TOCTOU re-check. The guard above evaluated the tip ONCE, before the candidate + # deploy and before the smoke gate (which alone budgets a 480s deadline), so a push + # landing in that window left this run promoting a superseded SHA to production and + # publishing a registry pointer naming it. `cancel-in-progress: false` does not help: + # it makes the SECOND run queue and skip, while the run already past the first check + # sails on. Re-resolve the tip now that smoke has passed, immediately before the + # first step that touches production, and gate PROMOTE on THIS output instead of the + # first one. The steps downstream of PROMOTE chain off PROMOTE's own `outcome` + # rather than reading this output again — a strictly stronger predicate, since + # `outcome` is 'skipped' exactly when this check says the branch moved AND is + # 'success' only once the production deploy has actually completed. + # The probe in particular must move with PROMOTE: gated on the first check it would + # still run after a skipped promote, see production serving the (correct) previous + # SHA, read that as a promote failure and roll back a production nobody touched. + # Residual window: this check → `wrangler deploy` is a single step boundary, seconds + # rather than minutes. It is not zero — Cloudflare offers no compare-and-swap on + # deploy — so the guard narrows the race, it does not eliminate it. + # The candidate deployed above needs no cleanup on the moved-branch path: candidate + # Workers carry no route, and the queued run for the new tip overwrites the candidate + # with its own deploy. + # Moved branch ends the run GREEN-and-skipped, matching the convention the first + # guard's redesign established (see the exit-78 note above): a legitimate branch race + # is not an operator-actionable failure, production and the registry pointer are + # untouched, and the queued run promotes the new tip. Failing red here would + # reintroduce exactly the false alarm that redesign removed. + - name: check_head re-guard after smoke (§7.1) + id: check_head_2 + if: steps.check_head.outputs.current == 'true' + env: + REF_NAME: ${{ github.ref_name }} + BUILT_SHA: ${{ github.sha }} + run: | + set -euo pipefail + remote=$(git ls-remote origin "refs/heads/$REF_NAME" | cut -f1) + # Same hardening as the first guard, and for the same reason: `cut` exits 0 on + # empty input, so pipefail AND the emptiness check are both needed — otherwise a + # failed ls-remote leaves $remote empty and is misread as "branch moved", turning + # an infrastructure failure into a silent skip of a promote that should have run. + [ -n "$remote" ] || { echo "::error::could not resolve the remote tip of $REF_NAME"; exit 1; } + if [ "$remote" = "$BUILT_SHA" ]; then + echo "current=true" >> "$GITHUB_OUTPUT" + else + echo "current=false" >> "$GITHUB_OUTPUT" + echo "::notice::branch moved during build/smoke (tip is now $remote, this run built $BUILT_SHA) — skipping promote, post-promote probe and registry pointer publish" + fi + + # This step ENDS at `wrangler deploy`. Everything that used to follow it in the same + # step — the hostname purge, the tarball, the three registry uploads — moved to the + # next step, because a failure in any of them failed THIS step, and the post-promote + # probe carried an implicit success() (its `if:` named no status function), so it was + # skipped. A transient purge 5xx or one failed upload therefore left the new version + # LIVE on the production Worker, never probed and never rolled back. + # The split is preferred over flagging `deployed=true` from inside the shell: the + # invariant then lives in the `if:` conditions where it is auditable statically, and + # nothing depends on outputs written by a step that went on to fail. + # Residual window it does NOT close: if `wrangler deploy` itself errors AFTER the + # deployment has taken effect, this step fails, the probe is skipped and production + # is unverified. Cloudflare offers no way to distinguish that from a deploy that + # never landed; the run is red either way, so it is an operator-visible state. + - name: PROMOTE (capture rollback id → deploy, §7.1.4) id: promote + if: steps.check_head_2.outputs.current == 'true' env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} @@ -268,28 +402,221 @@ jobs: slug='${{ steps.matrix.outputs.slug }}' worker='${{ steps.matrix.outputs.worker }}' cd workers - # Rollback target: verify this JSON shape on the FIRST real deploy — - # `npx wrangler deployments list --name "$worker" --json | jq .` — and adjust - # the jq path if wrangler's output differs (their JSON shape has churned across - # majors; the ID needed is the CURRENT deployment's, i.e. the newest entry). + # Rollback target, captured BEFORE the deploy below replaces production. + # `wrangler deployments list --json` (verified against wrangler 4.123.0's + # versionsDeploymentsListHandler) dumps the raw deployment objects sorted + # ASCENDING by created_on, so the CURRENT deployment is the LAST entry — NOT + # `.[0]`. Each entry looks like: + # {"id":"<deployment-id>","created_on":"<iso8601>","author_email":…, + # "versions":[{"version_id":"<version-id>","percentage":100}, …]} + # `wrangler rollback` takes a VERSION id (it resolves its positional through + # fetchVersion() and redeploys that version at 100%), never a deployment id, so + # we extract versions[].version_id — not the deployment's own `id`. + # SPLIT TRAFFIC: a deployment may spread traffic over several versions, and one + # with no single 100% version is not a safe rollback target. That is a state to + # REPORT, not to route around — see the extraction below. The reference point is + # wrangler's own fetchDefaultRollbackVersionId() minus its .shift(): wrangler + # picks AFTER deploying so it must skip the current deployment, whereas we + # capture BEFORE, so for us the newest entry is the good one we want back. # FIRST-DEPLOY BOOTSTRAP: the production Worker does not exist before the very - # first promote — `deployments list` fails/returns empty. That is a valid state: - # rollback_id stays empty, the deploy CREATES the Worker, and the post-promote - # step knows an empty id means "nothing to roll back to". - if deps=$(npx wrangler deployments list --name "$worker" --json 2>/dev/null); then - rollback_id=$(echo "$deps" | jq -r '.[0].id // empty') + # first promote, so `deployments list` fails. That is a valid state: rollback_id + # stays empty, the deploy CREATES the Worker, and the post-promote step knows an + # empty id means "nothing to roll back to". EVERY OTHER failure of that command — + # auth, network, an unparseable payload — must fail the step LOUDLY: an empty + # rollback_id promotes with the auto-rollback silently disarmed while the run + # still looks healthy, which is strictly worse than a red build. + # + # Telling the two apart cannot use the exit status: in wrangler 4.123.0 every + # error path ends in a single uncaught throw, so the process exits 1 regardless of + # cause, and a failed command writes nothing machine-readable to stdout. The only + # discriminator is the Cloudflare API error code wrangler renders into its stderr + # note as `... [code: NNNNN]`. Codes 10007 (script not found) and 10090 (legacy + # environment not found) are exactly the pair wrangler's own isWorkerNotFoundError() + # treats as "this Worker does not exist"; authentication failures carry 9106/10000 + # (its AUTHENTICATION_ERROR_CODES) and connectivity failures never reach the API + # so they carry no code at all — neither can be mistaken for the bootstrap case. + # The code was read out of the shipped 4.123.0 bundle, NOT observed against a live + # missing Worker, so re-verify this signature whenever wrangler is bumped; an + # unrecognised future spelling costs one loud red run, never a silent disarm. + # The note is esbuild-formatted (ANSI colour, wrapped to the terminal width), so + # strip escapes and collapse whitespace before matching. + rollback_id="" + list_err=$(mktemp) + if deps=$(npx wrangler deployments list --name "$worker" --json 2>"$list_err"); then + # Validate the shape up front and fail on anything else, rather than coercing a + # non-array to [] and reporting "no rollback target" for malformed output. + if ! printf '%s' "$deps" | jq -e 'type == "array"' >/dev/null 2>&1; then + echo "::error::wrangler deployments list --json for $worker did not return a JSON array — refusing to promote with the rollback target undetermined" + exit 1 + fi + # ONLY THE CURRENT (newest) DEPLOYMENT is a legitimate rollback target. An + # earlier spelling of this expression flattened `versions[]` across ALL + # deployments and took the first 100%-traffic id found scanning newest→oldest, + # so it reached BACKWARDS: a current deployment deliberately serving A/B at + # 90/10 resolved to some older deployment's 100% version, and a probe failure + # would then "roll back" to it and destroy the split that was set on purpose. + # Take the current deployment's single 100%-traffic version, or refuse. + # + # The verdict is discriminated inside jq and reported out here so that a + # MALFORMED record fails the step instead of masquerading as a determinate + # state. It used to masquerade: `select(.percentage == 100) | .version_id` on + # `{"percentage":100}` yields null, `null // empty` yields empty, and empty was + # read as "split traffic" — i.e. an unparseable API response promoted with the + # auto-rollback silently disarmed. The three good states stay distinguishable: + # no deployments at all → notice; a real traffic split on the CURRENT + # deployment → loud warning + promote unprotected; anything unparseable → red. + # + # `percentage` is required to be NUMERIC for the same reason: the API returns + # numbers, and a missing field or a string "100" slips past a `== 100` test, so + # a deployment that is actually at 100% would be reported as split and promote + # with the rollback disarmed. A percentage this step cannot read leaves it + # unable to tell "100%" from "a split", and guessing wrong destroys live + # traffic — so it fails red rather than guess. It must also be IN RANGE: a + # current deployment reporting both 100 and 101 yields exactly one + # 100%-traffic match, so the numeric check alone would classify plainly + # impossible data as a determinate "ok" and arm a rollback selected from it. + # Out-of-range therefore routes to the malformed path, never to "split:". + # + # Validation scope: `created_on` is checked on EVERY record because it decides + # which record is "current", so one unusable value corrupts the selection. + # `versions` / `version_id` are checked on the current record only — it is the + # only record this step reads, and failing on an older record's shape would + # block promotes over data with no bearing on the rollback target. + verdict=$(printf '%s' "$deps" | jq -r ' + def bad($why): "malformed:" + $why; + if length == 0 then "empty:" + elif any(.[]; type != "object") then + bad("a deployment entry is not an object") + elif any(.[]; (.created_on | type) != "string") then + bad("a deployment entry has no string created_on to order by") + else + (sort_by(.created_on) | last) as $cur + | if ($cur.versions | type) != "array" then + bad("the current deployment has no versions array") + elif ($cur.versions | length) == 0 then + bad("the current deployment has an empty versions array") + elif any($cur.versions[]; type != "object") then + bad("the current deployment has a non-object versions entry") + elif any($cur.versions[]; (.percentage | type) != "number") then + bad("the current deployment has a versions entry with a non-numeric percentage") + elif any($cur.versions[]; .percentage < 0 or .percentage > 100) then + bad("the current deployment has a versions entry with an out-of-range percentage") + else + [$cur.versions[] | select(.percentage == 100)] as $full + | if ($full | length) == 0 then "split:" + elif ($full | length) > 1 then + bad("the current deployment reports more than one 100%-traffic version") + elif (($full[0].version_id | type) != "string") or ($full[0].version_id == "") then + bad("the current deployment 100%-traffic version has no usable version_id") + else "ok:" + $full[0].version_id + end + end + end + ') || { echo "::error::could not evaluate the deployments payload for $worker — refusing to promote with the rollback target undetermined"; exit 1; } + case "${verdict%%:*}" in + ok) + rollback_id=${verdict#*:} + echo "rollback target for $worker: version $rollback_id" + ;; + empty) + echo "::notice::$worker has no deployments yet — nothing to roll back to; rollback disabled for this run" + ;; + split) + # Determinate state, not a swallowed failure: `wrangler rollback` takes one + # version id, so a split-traffic deployment has no single-version target to + # offer it. Warn loudly and promote unprotected rather than block a promote + # on a traffic split someone set deliberately. + echo "::warning::$worker's current deployment has no single 100%-traffic version (split traffic) — no safe rollback target; rollback disabled for this run" + ;; + *) + echo "::error::wrangler deployments list --json for $worker returned a record this step cannot trust (${verdict#*:}) — refusing to promote with the rollback target undetermined" + exit 1 + ;; + esac else - rollback_id="" - echo "first deploy for $worker — no prior deployment; rollback disabled for this run" + cat "$list_err" >&2 + esc=$(printf '\033') + if sed "s/${esc}\\[[0-9;]*m//g" "$list_err" | tr -s '[:space:]' ' ' \ + | grep -qE '\[code: (10007|10090)\]'; then + echo "::notice::production Worker $worker does not exist yet (API code 10007/10090) — first-deploy bootstrap; rollback disabled for this run" + else + echo "::error::wrangler deployments list --name $worker failed for a reason other than 'Worker does not exist' (see stderr above) — refusing to promote with the auto-rollback disarmed" + exit 1 + fi + fi + rm -f "$list_err" + # rollback_id came out of Cloudflare's API (`wrangler deployments list --json`), + # so it is untrusted text, and this is the last point before it becomes a step + # output. Two distinct exposures, only one of which careful quoting can close: + # * a NEWLINE in the value injects arbitrary ADDITIONAL step outputs into + # $GITHUB_OUTPUT. No shell is involved, so no amount of care at the consumer + # end helps; this check is the only place that can stop it. + # * shell metacharacters at the consumers, handled separately by passing the + # value through `env:` instead of a template expansion (see the finalizer). + # The accepted character class is deliberately WIDER than the observed format. + # wrangler 4.123.0 performs no client-side validation of a version id — it + # interpolates the string straight into the API URL path — so the canonical + # UUID shape below is what Cloudflare's own bundled SDK documents, not something + # the CLI enforces. Hard-failing on anything but a UUID would turn a Cloudflare + # format change into a blocked promote; hard-failing on anything outside + # [A-Za-z0-9._-] cannot, and still certainly excludes newlines, quotes, + # whitespace and every shell metacharacter. A non-UUID that is within the safe + # set is therefore a warning, not an error. + # EMPTY STAYS LEGITIMATE: it is the no-target state of all three benign paths + # above (first deploy, no deployments, split traffic) and the finalizer's -z + # branch depends on it. The malformed value is never echoed back. + if [ -n "$rollback_id" ]; then + case "$rollback_id" in + *[!A-Za-z0-9._-]*) + echo "::error::wrangler deployments list --json for $worker returned a version_id containing characters no Worker version id may contain — refusing to promote with an untrustworthy rollback target" + exit 1 + ;; + esac + if [ "${#rollback_id}" -gt 128 ]; then + echo "::error::wrangler deployments list --json for $worker returned a ${#rollback_id}-character version_id (max 128) — refusing to promote with an untrustworthy rollback target" + exit 1 + fi + if ! printf '%s' "$rollback_id" \ + | grep -qE '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$'; then + echo "::warning::$worker's rollback target '$rollback_id' is within the safe character set but is not the canonical UUID shape Cloudflare documents for version_id — accepted, but worth a look" + fi fi echo "rollback_id=$rollback_id" >> "$GITHUB_OUTPUT" npx wrangler deploy --config branch/wrangler.$(case "$slug" in rolling) echo rolling;; 1.5) echo v15;; 1.4) echo v14;; esac).jsonc \ --name "$worker" --var DOCS_BUILD_SHA:'${{ github.sha }}' --var DOCS_ENV:production - if [ "${{ vars.DOCS_CF_LIVE }}" = "true" ]; then - # hostname-scoped purge (§3.3; all-plans since 2025-04) + + # Gated on PROMOTE's OUTCOME, not on check_head_2 directly: `outcome` is 'skipped' + # exactly when the branch moved (PROMOTE carries the check_head_2 condition and its + # own implicit success()), and 'success' only once the production deploy completed. + # A failure in this step no longer suppresses the probe below — that is the whole + # point of the split. + - name: Purge + registry generation upload (§7.1.4) + id: promote_publish + if: steps.promote.outcome == 'success' + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + CF_ZONE_ID: ${{ vars.CF_ZONE_ID_VYOS_IO }} + DOCS_CF_LIVE: ${{ vars.DOCS_CF_LIVE }} + run: | + set -eu + slug='${{ steps.matrix.outputs.slug }}' + sha='${{ github.sha }}' + # hostname-scoped purge (§3.3; all-plans since 2025-04), retried once for the same + # reason the registry uploads below are. This purge is the one post-deploy failure + # whose knock-on effect is asymmetric: the probe measures the EDGE, so a purge that + # never lands leaves the edge serving the previous generation and the probe reads + # that as a failed promote — rolling back a deploy that was in fact fine. One retry + # removes the transient-5xx bulk of that class at no cost. What remains (a purge + # API that is genuinely down) still ends in a rollback, deliberately: see the probe + # step's note. + purge_hostname() { curl -sf -X POST "https://api.cloudflare.com/client/v4/zones/$CF_ZONE_ID/purge_cache" \ -H "Authorization: Bearer $CLOUDFLARE_API_TOKEN" -H "Content-Type: application/json" \ --data '{"hosts":["docs.vyos.io"]}' + } + if [ "$DOCS_CF_LIVE" = "true" ]; then + purge_hostname || { sleep 5; purge_hostname; } else echo "DOCS_CF_LIVE=false — pre-cutover: skipping production hostname purge" fi @@ -302,9 +629,8 @@ jobs: # a half-written generation (e.g. new tar.zst, still-old meta.json) mid upload. # Uploading under $slug/<sha>/* first — a key no prior generation ever reused — # keeps readers of the OLD pointer seeing a full, untouched old generation. - cd .. && tar --zstd -cf lastgood.tar.zst -C dist/assets . + tar --zstd -cf lastgood.tar.zst -C dist/assets . page_count=$(find "dist/assets/en/$slug" -name '*.html' | wc -l | tr -d ' ') - sha='${{ github.sha }}' printf '{"sha":"%s","page_count":%s}' "$sha" "$page_count" > lastgood.meta.json printf '{"sha":"%s"}' "$sha" > latest.json cd workers @@ -318,48 +644,153 @@ jobs: exit 1 fi + # THE VERIFICATION FINALIZER. Once `wrangler deploy` has succeeded, production has + # changed and this step must run whatever happens afterwards — so the condition is + # deliberately NOT the bare `steps.check_head_2...` form, which would pick up an + # implicit success() and be skipped by a failed purge or a failed registry upload, + # leaving production live and unverified. + # `!cancelled()` and not `always()`: on a cancelled run the deploy may not even have + # finished, and firing an auto-rollback while the operator is tearing the run down is + # the wrong reflex. `!cancelled()` counts as a status function, so it also suppresses + # the implicit success() — which is exactly what lets this run after a failed + # promote_publish. + # `steps.promote.outcome == 'success'` keeps both older guarantees intact: 'skipped' + # on the moved-branch path (so the probe cannot see production serving the correct + # previous SHA and read it as a promote failure), and never 'success' unless the + # rollback_id below was captured by a step that ran to completion. + # + # DELIBERATE: reaching here after a FAILED purge can roll back a deploy that was + # itself fine — the Worker took the new version, the edge kept serving the old one, + # and this probe measures the edge. That is the intended trade. The alternative, + # exiting without a rollback whenever the purge failed, leaves an UNVERIFIED version + # live on production, which is precisely the state this whole gate exists to prevent; + # a rollback instead returns production to the last version known to serve correctly, + # which is also what the stale edge is already serving, and the run is red either way + # so an operator looks at it. The transient half of that class is retried away in the + # purge step; what is left is a purge API that is down, and a known-good production + # is the right place to be sitting while it is. - name: Post-promote probe + auto-rollback (§7.1.5) + id: probe + if: ${{ !cancelled() && steps.promote.outcome == 'success' }} env: + # API-derived (Cloudflare `deployments list`), so it is bound here rather than + # expanded into the script text below: a template expansion is textual + # substitution into the shell source, an env var is not. Validated at capture. + ROLLBACK_ID: ${{ steps.promote.outputs.rollback_id }} CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} CF_ZONE_ID: ${{ vars.CF_ZONE_ID_VYOS_IO }} DOCS_CF_LIVE: ${{ vars.DOCS_CF_LIVE }} run: | set -eu + # pipefail: probe_once below is a pipeline whose FIRST command is the curl that can + # fail. Without it the pipeline's status is awk's, and a transport failure would be + # indistinguishable from a missing header. + set -o pipefail if [ "$DOCS_CF_LIVE" != "true" ]; then echo "DOCS_CF_LIVE=false — pre-cutover: docs.vyos.io still serves RTD; skipping production probe" exit 0 fi slug='${{ steps.matrix.outputs.slug }}' + # VERIFICATION REQUIRES BOTH A SUCCESSFUL STATUS AND THE BUILT SHA. `withDocsHeaders` + # (workers/branch/src/index.ts) sets X-Docs-Build UNCONDITIONALLY — error responses + # carry it too, which that function itself acknowledges by special-casing + # `status >= 400` for Cache-Control. Matching the header alone therefore accepted a + # production answering 500 with the new SHA as a verified promote, after which the + # registry pointer published for a generation that was serving errors. + # + # `--fail` is deliberately NOT used: it collapses every 4xx/5xx into curl exit 22 and + # discards the status line — the one value needed to tell a broken new version (500) + # apart from a transport failure. Parsing the status explicitly supersedes it. + # + # probe_once prints "<status> <sha>" for the FINAL response block. Resetting on every + # status line means 1xx interim responses (e.g. Cloudflare Early Hints) contribute + # nothing — only the last block's headers are read. Duplicate X-Docs-Build headers + # with IDENTICAL values state the same fact and are accepted; CONFLICTING values are + # untrustworthy and surface as "<conflicting>", which can never equal the built SHA. + probe_once() { + curl -sSI --max-time 20 "https://docs.vyos.io/en/$slug/" \ + | tr -d '\r' \ + | awk ' + /^HTTP\// { code = $2; n = 0; v = ""; next } + tolower($0) ~ /^x-docs-build:/ { + val = $0 + sub(/^[^:]*:[ \t]*/, "", val) + sub(/[ \t]+$/, "", val) + n++ + if (n == 1) { v = val } else if (val != v) { v = "<conflicting>" } + } + END { printf "%s %s\n", (code == "" ? "<no-status>" : code), (n == 0 ? "<absent>" : v) } + ' + } # Retry loop: the purge above is asynchronous, so an immediate probe can still # observe a stale edge response and trigger a false rollback. Poll up to 6 times - # (60s budget) before concluding the promote genuinely failed. - got="" + # (60s budget) before concluding the promote genuinely failed. A non-200 or a + # transport failure on an EARLY attempt is retried for the same reason a stale SHA + # is — only the FINAL attempt's verdict decides. + code="<no-status>" + got="<absent>" + verified=false for attempt in 1 2 3 4 5 6; do - got=$(curl -sI "https://docs.vyos.io/en/$slug/" | tr -d '\r' | awk -F': ' 'tolower($1)=="x-docs-build"{print $2}') - if [ "$got" = '${{ github.sha }}' ]; then + if out=$(probe_once); then + code=${out%% *} + got=${out#* } + else + # curl itself failed (DNS, TLS, connection refused, --max-time exceeded). + code="<transport-error>" + got="<none>" + fi + if [ "$code" = "200" ] && [ "$got" = '${{ github.sha }}' ]; then + verified=true break fi if [ "$attempt" -lt 6 ]; then - echo "::notice::post-promote probe attempt $attempt/6 saw stale edge (got $got) — retrying in 10s" + echo "::notice::post-promote probe attempt $attempt/6 unverified (status $code, X-Docs-Build $got) — retrying in 10s" sleep 10 fi done - if [ "$got" != '${{ github.sha }}' ]; then - if [ -z '${{ steps.promote.outputs.rollback_id }}' ]; then - echo "::error::post-promote probe failed (got $got) on FIRST deploy — no prior version to roll back to; investigate manually" + if [ "$verified" != "true" ]; then + # Same action for every unverified outcome — a rollback to the last version known + # to serve correctly — but distinguishable diagnoses, because "200 with the wrong + # SHA" (stale edge / promote never took) and "500 with the right SHA" (the new + # version is live and broken) point an operator at completely different causes. + if [ "$code" = "200" ]; then + reason="production answered 200 but served X-Docs-Build $got, not ${{ github.sha }}" + elif [ "$code" = "<transport-error>" ]; then + reason="production could not be reached at all after 6 attempts (see curl stderr above)" + else + reason="production answered status $code carrying X-Docs-Build $got — the deployed version is live but not serving successfully" + fi + if [ -z "$ROLLBACK_ID" ]; then + echo "::error::post-promote probe failed ($reason) but PROMOTE captured no rollback target (first deploy, or no 100%-traffic version) — investigate manually" exit 1 fi - echo "::error::post-promote probe failed (got $got) — rolling back" - cd workers && npx wrangler rollback '${{ steps.promote.outputs.rollback_id }}' \ - --name '${{ steps.matrix.outputs.worker }}' --yes + # rollback_id is a Worker VERSION id (see the PROMOTE capture above); + # `wrangler rollback <version-id>` redeploys that version at 100% traffic. + echo "::error::post-promote probe failed ($reason) — rolling back to version $ROLLBACK_ID" + cd workers && npx wrangler rollback "$ROLLBACK_ID" \ + --name '${{ steps.matrix.outputs.worker }}' \ + --message 'auto-rollback: docs-build post-promote probe failed' --yes curl -sf -X POST "https://api.cloudflare.com/client/v4/zones/$CF_ZONE_ID/purge_cache" \ -H "Authorization: Bearer $CLOUDFLARE_API_TOKEN" -H "Content-Type: application/json" \ --data '{"hosts":["docs.vyos.io"]}' exit 1 fi + # The IMPLICIT success() on this condition is load-bearing and must stay implicit: + # naming no status function is what makes this step skip whenever ANY earlier step + # failed — the probe (so a rolled-back generation never becomes the pointer target) + # and the purge/upload step (so a pointer never names a generation that was not + # fully uploaded). Adding always()/!cancelled() here would publish over a rollback. + # + # KNOWN GAP: because that success() is implicit, a failed `promote_publish` skips + # this step even when the probe went on to verify production serving this SHA — and + # a terminal failure here has the same effect. Either way production and + # $slug/latest.json disagree, and nothing in this job reconciles them. Tracked in + # https://vyos.dev/T9237. - name: Publish registry pointer (§7.1.4) + id: publish_pointer + if: steps.check_head_2.outputs.current == 'true' env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN_DOCS }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} diff --git a/.github/workflows/docs-canary-qa.yml b/.github/workflows/docs-canary-qa.yml index 7f3be7c0..59fc5849 100644 --- a/.github/workflows/docs-canary-qa.yml +++ b/.github/workflows/docs-canary-qa.yml @@ -11,6 +11,15 @@ jobs: steps: - uses: actions/checkout@v4 - name: Per-entry sweep (one page + PDF per versions.json entry, §3.4 gate c) + env: + # CF Access service token goes through the environment, NEVER through argv: a secret + # on the command line is readable from the process table for the life of the process + # and is captured verbatim by `set -x` traces and crash dumps. These environment + # variables are the ONLY supported way to pass the token: the --access-id / + # --access-secret flags were removed, and argparse now rejects them outright + # rather than ignoring them and leaving every probe to 403. + CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} + CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} run: | set -eu # Capture-first (not pipe/process-substitution-into-while): a malformed @@ -23,9 +32,7 @@ jobs: while read -r v; do slug=$(echo "$v" | jq -r .slug); pdf=$(echo "$v" | jq -r '.pdf // empty') if ! python -m scripts.docs_gates.smoke --host docs-next.vyos.io --slug "$slug" \ - --expect-sha SKIP --access-id '${{ secrets.CF_ACCESS_CLIENT_ID }}' \ - --access-secret '${{ secrets.CF_ACCESS_CLIENT_SECRET }}' \ - ${pdf:+--pdf "$pdf"}; then + --expect-sha SKIP ${pdf:+--pdf "$pdf"}; then echo "sweep-fail=$slug" | tee -a "$GITHUB_STEP_SUMMARY" fail=1 fi @@ -33,15 +40,17 @@ jobs: exit $fail # a broken advertised version MUST fail nightly QA (§3.4 gate c) # --expect-sha SKIP = presence-only assert; implemented in smoke.docs_build_ok (Task 3.2). - name: URL parity vs RTD (pre-cutover truth) + env: + # Same argv-secret rule as the sweep step above; parity.py reads these by default. + CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} + CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} run: | # --slugs pinned to CF-built versions: 1.3/1.2 have no RTD sitemaps (§15a.5); # legacy parity is the snapshot repo's crawl-inventory job (defense in depth # alongside the matching parity.py DEFAULT_SLUGS) python -m scripts.docs_gates.parity \ --sitemap-host docs.vyos.io --probe-host docs-next.vyos.io \ - --slugs rolling,1.5,1.4 \ - --access-id '${{ secrets.CF_ACCESS_CLIENT_ID }}' \ - --access-secret '${{ secrets.CF_ACCESS_CLIENT_SECRET }}' + --slugs rolling,1.5,1.4 - uses: actions/upload-artifact@v4 if: always() with: { name: parity-report, path: parity-report.json } |