From 4d69cf1ae94f67ae6176bf61669abe47e2eb2f39 Mon Sep 17 00:00:00 2001 From: Pat Welch Date: Sun, 14 Jun 2026 10:25:49 -0700 Subject: [PATCH 1/2] feat(deploy): serving-path cutover gate + quiesce-timeout backout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Increment 4 of Batch 4C (docs/PLAN_4c_renderers.md). Two kayak-deploy.sh changes. Serving-path gate (when SERVING_CUTOVER=yes, before any mutation) — the check deferred from PR #190/#192 that the host is FULLY cut over, not half: - OUTPUT_DIR: every re-pointed unit that pins OUTPUT_DIR (the pipeline drop-in) must equal $KAYAK_DOCROOT, else the hourly pipeline builds a tree nobody serves — a silent half-cutover the symlink switch can't catch. Read via `systemctl show -p Environment` (Environment= drop-ins, not the EnvironmentFile, so pre-cutover units have none and skip). - nginx / FPM (knob-gated on KAYAK_NGINX_DOCROOT_CONF / KAYAK_FPM_POOL, which the 4C runbook sets): nginx roots at $KAYAK_DOCROOT, the ACME `root /var/www/certbot;` survives (PR #194 review #2 — a blanket root-sed would break renewal), and the FPM open_basedir leads with $KAYAK_DOCROOT. Unset ⇒ warn+skip (can't guess the paths) rather than silently pass. Quiesce-timeout backout (the deploy_quiesce_timeout_followup fix): the drain timeout's explicit `exit 1` doesn't fire the ERR trap even under -E, so a consumer that wouldn't drain left the host in maintenance with consumers stopped and no rollback (nothing mutated, but the site stayed down). It now restarts consumers + clears maintenance before exiting. Drain bound parameterized (KAYAK_DRAIN_TIMEOUT/_INTERVAL, prod defaults 120/2) so the backout is slow-testable in ~2 s. Tests: a half-cutover gate test (OUTPUT_DIR mismatch, nginx legacy root, clobbered certbot root, open_basedir missing the docroot — each refused pre-mutation) and a quiesce-timeout backout test. Existing 5 slow activation tests unchanged and green (new checks skip when OUTPUT_DIR/knobs absent). Full slow suite 7 passed; non-slow 1761 passed; ruff/format/shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- deploy/deploy.env.example | 9 ++ deploy/kayak-deploy.sh | 81 +++++++++- docs/PLAN_4c_renderers.md | 24 +-- tests/test_scripts/test_kayak_deploy.py | 190 ++++++++++++++++++++++++ 4 files changed, 289 insertions(+), 15 deletions(-) diff --git a/deploy/deploy.env.example b/deploy/deploy.env.example index eb0136fe..87233482 100644 --- a/deploy/deploy.env.example +++ b/deploy/deploy.env.example @@ -48,6 +48,15 @@ KAYAK_APP_USER=pat # by the web user (ACL). #KAYAK_DOCROOT=/var/cache/kayak/docroot +# Serving-path verification (4C). When set, the activation gate (only reached +# when SERVING_CUTOVER=yes) verifies that nginx roots at $KAYAK_DOCROOT and the +# certbot ACME root survives, and that the FPM pool's open_basedir leads with +# $KAYAK_DOCROOT — so a half-cutover (consumers re-pointed but the web layer still +# on the old docroot, or vice versa) is refused. Unset = the gate WARNS and skips +# that check (it can't guess the paths). The 4C cutover runbook sets both. +#KAYAK_NGINX_DOCROOT_CONF=/etc/nginx/snippets/levels-common.conf +#KAYAK_FPM_POOL=/etc/php/8.4/fpm/pool.d/kayak.conf + # NOTE: any value with spaces MUST be quoted — this file is sourced by the # shell, so e.g. KAYAK_HOST_UNITS="a.service b.service" (unquoted, the shell # tries to RUN the 2nd word as a command and the deploy aborts at config load). diff --git a/deploy/kayak-deploy.sh b/deploy/kayak-deploy.sh index 7324ac70..8cccce6f 100755 --- a/deploy/kayak-deploy.sh +++ b/deploy/kayak-deploy.sh @@ -29,6 +29,12 @@ # steps run as this user via runuser) # SERVING_CUTOVER must be "yes" to activate (set by the 4C # runbook once nginx/FPM point at current) +# KAYAK_NGINX_DOCROOT_CONF nginx snippet whose `root` is the docroot; +# when set, the gate verifies it roots at +# $KAYAK_DOCROOT and the certbot ACME root +# survives (4C runbook sets it; unset = warn+skip) +# KAYAK_FPM_POOL PHP-FPM pool file; when set, the gate verifies +# its open_basedir leads with $KAYAK_DOCROOT # Path overrides (mainly for tests / the clean-VM rehearsal): # KAYAK_DEPLOY_ROOT release root (default /opt/kayak) # KAYAK_DEPLOY_TMPDIR scratch base — MUST be real disk, not a @@ -570,8 +576,64 @@ for u in $KAYAK_UNITS; do exit 1 ;; esac + # If this unit pins OUTPUT_DIR (the render-units cutover drop-in sets it on the + # pipeline, the only consumer that builds), it must be the docroot the deployer + # builds and nginx serves — else the hourly pipeline writes a tree nobody + # serves, a silent half-cutover the symlink switch can't catch. `show -p + # Environment` reflects Environment= directives (the drop-in), not the + # EnvironmentFile, so pre-cutover units (no drop-in) simply have none and skip. + env_out="$("$SYSTEMCTL" show -p Environment --value "$svc" 2>/dev/null || true)" + case " $env_out " in + *" OUTPUT_DIR=$KAYAK_DOCROOT "*) : ;; + *" OUTPUT_DIR="*) + echo "Error: $svc OUTPUT_DIR != KAYAK_DOCROOT ($KAYAK_DOCROOT)." >&2 + echo " (Environment: $env_out) — re-render the cutover drop-in (4C)." >&2 + exit 1 + ;; + esac done +# The serving layer must point at the docroot too (the gate deferred from PR #190/ +# #192 to here). Knob-gated: the 4C runbook sets these to the live nginx snippet + +# FPM pool; absent them the deployer can't know the paths, so it WARNS rather than +# silently passing. nginx roots the docroot in ONE shared snippet (the vhosts +# carry none), and the FPM open_basedir is one pool line — so a single grep each. +if [ -n "${KAYAK_NGINX_DOCROOT_CONF:-}" ]; then + if [ ! -r "$KAYAK_NGINX_DOCROOT_CONF" ]; then + echo "Error: KAYAK_NGINX_DOCROOT_CONF=$KAYAK_NGINX_DOCROOT_CONF not readable" >&2 + exit 1 + fi + if ! grep -qE "^[[:space:]]*root[[:space:]]+$KAYAK_DOCROOT;" "$KAYAK_NGINX_DOCROOT_CONF"; then + echo "Error: nginx does not root at $KAYAK_DOCROOT ($KAYAK_NGINX_DOCROOT_CONF)." >&2 + echo " Run \`levels render-serving\` and apply it (Batch 4C)." >&2 + exit 1 + fi + # The ACME challenge root must survive — a blanket root-substitution at cutover + # would clobber it and break cert renewal (PR #194 review #2). + if ! grep -qE "^[[:space:]]*root[[:space:]]+/var/www/certbot;" "$KAYAK_NGINX_DOCROOT_CONF"; then + echo "Error: the certbot ACME root is missing from $KAYAK_NGINX_DOCROOT_CONF" >&2 + echo " (a global root-substitution clobbered it — renewal would break)." >&2 + exit 1 + fi +else + echo "kayak-deploy: WARNING: KAYAK_NGINX_DOCROOT_CONF unset — skipping the nginx" >&2 + echo " serving-root check (set it in $CONF so the cutover is verified)." >&2 +fi +if [ -n "${KAYAK_FPM_POOL:-}" ]; then + if [ ! -r "$KAYAK_FPM_POOL" ]; then + echo "Error: KAYAK_FPM_POOL=$KAYAK_FPM_POOL not readable" >&2 + exit 1 + fi + if ! grep -qE "open_basedir\][[:space:]]*=[[:space:]]*$KAYAK_DOCROOT:" "$KAYAK_FPM_POOL"; then + echo "Error: PHP-FPM open_basedir does not lead with $KAYAK_DOCROOT ($KAYAK_FPM_POOL)." >&2 + echo " Run \`levels render-serving\` and apply it (Batch 4C)." >&2 + exit 1 + fi +else + echo "kayak-deploy: WARNING: KAYAK_FPM_POOL unset — skipping the FPM open_basedir" >&2 + echo " check (set it in $CONF so the cutover is verified)." >&2 +fi + # --------------------------------------------------------------------------- # Phase 3 — activate (system mutation; everything before the symlink switch # is undone by the rollback path) @@ -709,15 +771,28 @@ for u in $KAYAK_UNITS; do *.service) SERVICES="$SERVICES $u" ;; esac done +# Drain bound: how long to wait for a still-running consumer, and the poll +# interval. Parameterized so the timeout-backout path is testable without a real +# 120 s wait (defaults are the production values). +: "${KAYAK_DRAIN_TIMEOUT:=120}" +: "${KAYAK_DRAIN_INTERVAL:=2}" waited=0 for svc in $SERVICES; do while "$SYSTEMCTL" is-active --quiet "$svc" 2>/dev/null; do - if [ "$waited" -ge 120 ]; then + if [ "$waited" -ge "$KAYAK_DRAIN_TIMEOUT" ]; then echo "Error: $svc still active after ${waited}s — refusing to mutate the DB under it" >&2 + # Nothing has mutated yet (this is before the DB backup; MUTATED=0), + # but maintenance is ON and the consumers are STOPPED. An explicit + # `exit` does NOT fire the ERR trap (even under -E), so back the + # no-mutation state out by hand — otherwise a drain timeout leaves the + # site down with consumers stopped until an operator clears it + # (PR #192 review — the quiesce-timeout sibling of the errtrace gap). + for s in $KAYAK_UNITS; do "$SYSTEMCTL" start "$s" 2>/dev/null || true; done + rm -f "$ROOT/maintenance" exit 1 fi - sleep 2 - waited=$((waited + 2)) + sleep "$KAYAK_DRAIN_INTERVAL" + waited=$((waited + KAYAK_DRAIN_INTERVAL)) done done log "consumers quiesced" diff --git a/docs/PLAN_4c_renderers.md b/docs/PLAN_4c_renderers.md index d8588128..3a25ae1f 100644 --- a/docs/PLAN_4c_renderers.md +++ b/docs/PLAN_4c_renderers.md @@ -140,18 +140,18 @@ must carry the per-unit write-path set, not a blanket one. `docroot`). *(merged, PR #193)* 3. **`levels render-serving` + tests.** The nginx `root` + FPM `open_basedir` directives from `host.docroot`/`service_home`. *(this PR)* -4. **Deployer serving-path gate + quiesce-timeout fix** (`deploy/kayak-deploy.sh`): - when `SERVING_CUTOVER=yes`, verify nginx root / FPM `open_basedir` / unit - `OUTPUT_DIR`+`ReadWritePaths` resolve to `$KAYAK_DOCROOT`; back out maintenance - on a drain timeout (the [[deploy_quiesce_timeout_followup]] fix). Branch off - #192. Slow-test the gate. - - **Apply-step caveat (PR #194 review #2):** `conf/snippets/levels-common.conf` - has TWO `root` directives — the docroot (~line 30) and the ACME - `root /var/www/certbot;` (~line 305). The cutover apply must target the - docroot line specifically (a blanket `sed 's/^\s*root .*/…/'` would clobber - the certbot root and break renewal), and this gate should verify the certbot - root survived. (`test_host_render_serving.py` already asserts there's exactly - one non-certbot `root`, so a structural change trips CI.) +4. **Deployer serving-path gate + quiesce-timeout fix** (`deploy/kayak-deploy.sh`). + *(this PR)* When `SERVING_CUTOVER=yes` the gate now verifies, before any + mutation: every re-pointed unit that pins `OUTPUT_DIR` matches `$KAYAK_DOCROOT` + (via `systemctl show -p Environment`); and — when `KAYAK_NGINX_DOCROOT_CONF` / + `KAYAK_FPM_POOL` are set (the 4C runbook sets them; unset = warn+skip) — that + nginx roots at `$KAYAK_DOCROOT`, the ACME `root /var/www/certbot;` survives (PR + #194 review #2 — a blanket root-sed would break renewal), and the FPM + `open_basedir` leads with `$KAYAK_DOCROOT`. The quiesce drain-timeout now backs + out maintenance + restarts consumers before its explicit `exit` (which doesn't + fire the ERR trap even under `-E`) — the [[deploy_quiesce_timeout_followup]] + fix. Drain bound parameterized (`KAYAK_DRAIN_TIMEOUT`/`_INTERVAL`) so the + backout is slow-testable without a 120 s wait. 5. **Derive `KAYAK_UNITS`/`KAYAK_HOST_UNITS` from installed timers** (closes the complete-consumer-enumeration item; resolves D-CONSUMER). 6. **Runbook §5 rewrite** (`deploy/INSTALL-paired-release.md` on `b4c-paired-install`): diff --git a/tests/test_scripts/test_kayak_deploy.py b/tests/test_scripts/test_kayak_deploy.py index d30f7e07..fd4c0604 100644 --- a/tests/test_scripts/test_kayak_deploy.py +++ b/tests/test_scripts/test_kayak_deploy.py @@ -752,3 +752,193 @@ def activate(site_url: str, *, health_url: str | None) -> subprocess.CompletedPr dt = "".join(p.read_text(errors="ignore") for p in docroot.rglob("*") if p.is_file()) assert "first.example.org" in dt assert "second.example.org" not in dt + + +@pytest.mark.slow +def test_serving_path_gate_refuses_half_cutover( + tmp_path: Path, deploy_root: Path, engine_repo, dataset_repo +) -> None: + """The SERVING_CUTOVER gate refuses a half-cutover before any mutation: a + consumer OUTPUT_DIR that isn't KAYAK_DOCROOT, nginx not rooting at it, a + clobbered certbot ACME root, or an FPM open_basedir missing it (the gate + deferred from PR #190/#192, + PR #194 review #2 certbot check).""" + ds_repo, ds_sha = dataset_repo + eng_repo, engine_sha = engine_repo + fixture_ds = _REPO / "tests" / "fixtures" / "dataset" + db = tmp_path / "kayak.db" + _init_db(db, fixture_ds) + root = deploy_root + docroot = tmp_path / "docroot" + runtime_config = tmp_path / "runtime-config.json" + runuser_log = tmp_path / "runuser.log" + me = subprocess.run(["id", "-un"], capture_output=True, text=True, check=True).stdout.strip() + + installer = tmp_path / "install-config.sh" + installer.write_text(f'#!/bin/sh\ncat > "{runtime_config}"\n') + installer.chmod(0o755) + runuser = tmp_path / "runuser.sh" + runuser.write_text(f'#!/bin/sh\nshift 3\necho "$@" >> "{runuser_log}"\nexec "$@"\n') + runuser.chmod(0o755) + + # systemctl stub: ExecStart → the release binary (passes the run-from-current + # check); Environment → the contents of envfile (the OUTPUT_DIR under test); + # is-active → inactive so the drain loop exits at once. + envfile = tmp_path / "stub-env.txt" + envfile.write_text(f"OUTPUT_DIR={docroot} DATASET_DIR={root}/current/dataset") + systemctl = tmp_path / "systemctl.sh" + systemctl.write_text( + "#!/bin/sh\n" + 'if [ "$1" = show ]; then\n' + f' if [ "$3" = ExecStart ]; then echo "{root}/current/venv/bin/levels pipeline";\n' + f' elif [ "$3" = Environment ]; then cat "{envfile}"; fi\n' + " exit 0\n" + "fi\n" + 'case "$1" in is-active) exit 1 ;; *) exit 0 ;; esac\n' + ) + systemctl.chmod(0o755) + + nginx_conf = tmp_path / "levels-common.conf" + fpm_pool = tmp_path / "kayak.conf" + + def good_nginx() -> None: + nginx_conf.write_text(f" root {docroot};\n root /var/www/certbot;\n") + + def good_fpm() -> None: + fpm_pool.write_text(f"php_admin_value[open_basedir] = {docroot}:/home/x/var:/home/x/DB\n") + + good_nginx() + good_fpm() + conf = _write_conf( + tmp_path, + str(eng_repo), + str(ds_repo), + ENGINE_BRANCH="test-main", + DATASET_BRANCH="main", + SERVING_CUTOVER="yes", + ) + host_env = tmp_path / "host.env" + host_env.write_text(f"SITE_URL=https://x.example.org\nSQLITE_PATH={db}\n") + + def activate() -> subprocess.CompletedProcess[str]: + env = { + "KAYAK_DEPLOY_CONF": str(conf), + "KAYAK_DEPLOY_ROOT": str(root), + "KAYAK_DOCROOT": str(docroot), + "KAYAK_HOST_ENV": str(host_env), + "KAYAK_RUNTIME_CONFIG": str(runtime_config), + "KAYAK_CONFIG_INSTALLER": str(installer), + "KAYAK_SYSTEMCTL": str(systemctl), + "KAYAK_PRIVILEGED": "yes", + "KAYAK_APP_USER": me, + "KAYAK_RUNUSER": str(runuser), + "HOME": str(tmp_path), + "SUDO_USER": "", + "KAYAK_UNITS": "kayak-pipeline.timer", + "KAYAK_NGINX_DOCROOT_CONF": str(nginx_conf), + "KAYAK_FPM_POOL": str(fpm_pool), + } + return _run(["--engine-ref", engine_sha, "--dataset-ref", ds_sha], env, timeout=900) + + # 1) Everything points at the docroot → activation succeeds (and stages the + # release, which the gate-refusal cases below reuse — fast). + ok = activate() + assert ok.returncode == 0, ok.stderr + + # 2) A consumer building into the wrong tree → refuse. + envfile.write_text(f"OUTPUT_DIR=/wrong/tree DATASET_DIR={root}/current/dataset") + bad = activate() + assert bad.returncode != 0 + assert "OUTPUT_DIR != KAYAK_DOCROOT" in bad.stderr + envfile.write_text(f"OUTPUT_DIR={docroot} DATASET_DIR={root}/current/dataset") + + # 3) nginx still rooting the legacy docroot → refuse. + nginx_conf.write_text(" root /home/pat/public_html;\n root /var/www/certbot;\n") + bad = activate() + assert bad.returncode != 0 and "nginx does not root at" in bad.stderr + good_nginx() + + # 4) certbot ACME root clobbered by a blanket root-substitution → refuse. + nginx_conf.write_text(f" root {docroot};\n") + bad = activate() + assert bad.returncode != 0 and "certbot ACME root is missing" in bad.stderr + good_nginx() + + # 5) FPM open_basedir not leading with the docroot → refuse. + fpm_pool.write_text("php_admin_value[open_basedir] = /home/pat/public_html:/home/x/DB\n") + bad = activate() + assert bad.returncode != 0 and "open_basedir does not lead with" in bad.stderr + + # None of the refusals mutated the DB (the gate is pre-backup). + assert ".backup" not in runuser_log.read_text() or runuser_log.read_text().count(".backup") == 1 + + +@pytest.mark.slow +def test_quiesce_timeout_backs_out_maintenance( + tmp_path: Path, deploy_root: Path, engine_repo, dataset_repo +) -> None: + """A consumer that won't drain leaves NOTHING mutated, so the timeout must + back out maintenance + restart consumers rather than `exit 1` into a stuck + down state (PR #192 review — the quiesce-timeout sibling of the errtrace gap).""" + ds_repo, ds_sha = dataset_repo + eng_repo, engine_sha = engine_repo + fixture_ds = _REPO / "tests" / "fixtures" / "dataset" + db = tmp_path / "kayak.db" + _init_db(db, fixture_ds) + root = deploy_root + runtime_config = tmp_path / "runtime-config.json" + runuser_log = tmp_path / "runuser.log" + systemctl_log = tmp_path / "systemctl.log" + me = subprocess.run(["id", "-un"], capture_output=True, text=True, check=True).stdout.strip() + + installer = tmp_path / "install-config.sh" + installer.write_text(f'#!/bin/sh\ncat > "{runtime_config}"\n') + installer.chmod(0o755) + runuser = tmp_path / "runuser.sh" + runuser.write_text(f'#!/bin/sh\nshift 3\necho "$@" >> "{runuser_log}"\nexec "$@"\n') + runuser.chmod(0o755) + # is-active reports ACTIVE (exit 0) forever → the drain loop never converges. + systemctl = tmp_path / "systemctl.sh" + systemctl.write_text( + f'#!/bin/sh\necho "$@" >> "{systemctl_log}"\n' + 'if [ "$1" = show ]; then\n' + f' if [ "$3" = ExecStart ]; then echo "{root}/current/venv/bin/levels pipeline"; fi\n' + " exit 0\n" + "fi\n" + 'case "$1" in is-active) exit 0 ;; *) exit 0 ;; esac\n' + ) + systemctl.chmod(0o755) + conf = _write_conf( + tmp_path, + str(eng_repo), + str(ds_repo), + ENGINE_BRANCH="test-main", + DATASET_BRANCH="main", + SERVING_CUTOVER="yes", + ) + host_env = tmp_path / "host.env" + host_env.write_text(f"SITE_URL=https://x.example.org\nSQLITE_PATH={db}\n") + env = { + "KAYAK_DEPLOY_CONF": str(conf), + "KAYAK_DEPLOY_ROOT": str(root), + "KAYAK_DOCROOT": str(tmp_path / "docroot"), + "KAYAK_HOST_ENV": str(host_env), + "KAYAK_RUNTIME_CONFIG": str(runtime_config), + "KAYAK_CONFIG_INSTALLER": str(installer), + "KAYAK_SYSTEMCTL": str(systemctl), + "KAYAK_PRIVILEGED": "yes", + "KAYAK_APP_USER": me, + "KAYAK_RUNUSER": str(runuser), + "HOME": str(tmp_path), + "SUDO_USER": "", + "KAYAK_UNITS": "kayak-pipeline.timer", + # Tiny drain bound so the loop times out in ~2 s, not 120 s. + "KAYAK_DRAIN_TIMEOUT": "2", + "KAYAK_DRAIN_INTERVAL": "1", + } + p = _run(["--engine-ref", engine_sha, "--dataset-ref", ds_sha], env, timeout=900) + assert p.returncode != 0 + assert "still active after" in p.stderr + # Backed out: maintenance cleared, consumers restarted, DB never touched. + assert not (root / "maintenance").exists() + assert "start kayak-pipeline.timer" in systemctl_log.read_text() + assert not runuser_log.exists() or ".backup" not in runuser_log.read_text() From 492ee8985741c7b7ba7d7bb578b6f748665ae234 Mon Sep 17 00:00:00 2001 From: Pat Welch Date: Sun, 14 Jun 2026 11:34:59 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(deploy):=20address=20PR=20#195=20review?= =?UTF-8?q?=20=E2=80=94=20fail-closed=20serving=20gate=20+=20exclusivity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review findings on the serving-path gate, all closed: 1. (Medium) FAIL-CLOSED on the serving knobs. SERVING_CUTOVER=yes is set only by the same 4C runbook that sets KAYAK_NGINX_DOCROOT_CONF/KAYAK_FPM_POOL, so an unset knob under cutover is now an error, not a warn+skip — warn-skip let exactly the nginx-side half-cutover (consumers re-pointed, web layer still on the old docroot) sail through, which is the gate's reason to exist. --stage-only exits before this gate, so it is unaffected. 2. (Low–Medium) EXCLUSIVE nginx root, not presence-only. Every `root` must be the docroot or the ACME /var/www/certbot root — a leftover legacy `root /home/pat/public_html;` (an apply that appended instead of replacing) passed the presence checks yet nginx serves the LAST root. 3. (Low) Anchored the FPM open_basedir grep at line start so a `;`-commented line can't satisfy it (mirrors the nginx check). 4. (Nits) $KAYAK_DOCROOT is now regex-escaped before interpolation into the grep patterns — negated-class `sed 's#[^[:alnum:]_/-]#\&#g'`, portable across GNU + BSD sed (the metachar-list form trips BSD, which the local slow tests use). The hardcoded /var/www/certbot carries a note for genericization (step 7). Plus a bug the new fail-closed test caught in the fix itself: `: "${VAR:?}"` returned exit 0 because the cleanup EXIT trap masks a :? expansion-error exit (bash exits with the trap's `return 0`). Switched both checks to explicit `exit 1`, which is preserved across the trap (verified; bare `set -e` failures are too — the two pre-existing :? fire before the EXIT trap is armed, so unaffected). Tests: the gate test gains exclusive-root, fail-closed (unset knobs), and commented-open_basedir cases; the five activation tests now supply the required serving knobs via a shared `_serving_knobs` helper. Full slow suite 7 passed; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- deploy/deploy.env.example | 13 +-- deploy/kayak-deploy.sh | 109 +++++++++++++++--------- docs/PLAN_4c_renderers.md | 13 +-- tests/test_scripts/test_kayak_deploy.py | 47 +++++++++- 4 files changed, 126 insertions(+), 56 deletions(-) diff --git a/deploy/deploy.env.example b/deploy/deploy.env.example index 87233482..8bc663c8 100644 --- a/deploy/deploy.env.example +++ b/deploy/deploy.env.example @@ -48,12 +48,13 @@ KAYAK_APP_USER=pat # by the web user (ACL). #KAYAK_DOCROOT=/var/cache/kayak/docroot -# Serving-path verification (4C). When set, the activation gate (only reached -# when SERVING_CUTOVER=yes) verifies that nginx roots at $KAYAK_DOCROOT and the -# certbot ACME root survives, and that the FPM pool's open_basedir leads with -# $KAYAK_DOCROOT — so a half-cutover (consumers re-pointed but the web layer still -# on the old docroot, or vice versa) is refused. Unset = the gate WARNS and skips -# that check (it can't guess the paths). The 4C cutover runbook sets both. +# Serving-path verification (4C). REQUIRED when SERVING_CUTOVER=yes: the +# activation gate verifies nginx roots ONLY at $KAYAK_DOCROOT (with the certbot +# ACME root surviving) and the FPM pool's open_basedir leads with $KAYAK_DOCROOT — +# so a half-cutover (consumers re-pointed but the web layer still on the old +# docroot, or vice versa) is refused. The gate FAILS CLOSED: unset while +# SERVING_CUTOVER=yes is an error, not a skip (the same 4C runbook sets both +# SERVING_CUTOVER and these, so requiring them costs nothing in the real flow). #KAYAK_NGINX_DOCROOT_CONF=/etc/nginx/snippets/levels-common.conf #KAYAK_FPM_POOL=/etc/php/8.4/fpm/pool.d/kayak.conf diff --git a/deploy/kayak-deploy.sh b/deploy/kayak-deploy.sh index 8cccce6f..65b7284a 100755 --- a/deploy/kayak-deploy.sh +++ b/deploy/kayak-deploy.sh @@ -30,11 +30,12 @@ # SERVING_CUTOVER must be "yes" to activate (set by the 4C # runbook once nginx/FPM point at current) # KAYAK_NGINX_DOCROOT_CONF nginx snippet whose `root` is the docroot; -# when set, the gate verifies it roots at -# $KAYAK_DOCROOT and the certbot ACME root -# survives (4C runbook sets it; unset = warn+skip) -# KAYAK_FPM_POOL PHP-FPM pool file; when set, the gate verifies -# its open_basedir leads with $KAYAK_DOCROOT +# REQUIRED when SERVING_CUTOVER=yes — the gate +# verifies it roots ONLY at $KAYAK_DOCROOT (+ the +# surviving certbot ACME root). 4C runbook sets it. +# KAYAK_FPM_POOL PHP-FPM pool file; REQUIRED when +# SERVING_CUTOVER=yes — the gate verifies its +# open_basedir leads with $KAYAK_DOCROOT # Path overrides (mainly for tests / the clean-VM rehearsal): # KAYAK_DEPLOY_ROOT release root (default /opt/kayak) # KAYAK_DEPLOY_TMPDIR scratch base — MUST be real disk, not a @@ -594,44 +595,68 @@ for u in $KAYAK_UNITS; do done # The serving layer must point at the docroot too (the gate deferred from PR #190/ -# #192 to here). Knob-gated: the 4C runbook sets these to the live nginx snippet + -# FPM pool; absent them the deployer can't know the paths, so it WARNS rather than -# silently passing. nginx roots the docroot in ONE shared snippet (the vhosts -# carry none), and the FPM open_basedir is one pool line — so a single grep each. -if [ -n "${KAYAK_NGINX_DOCROOT_CONF:-}" ]; then - if [ ! -r "$KAYAK_NGINX_DOCROOT_CONF" ]; then - echo "Error: KAYAK_NGINX_DOCROOT_CONF=$KAYAK_NGINX_DOCROOT_CONF not readable" >&2 - exit 1 - fi - if ! grep -qE "^[[:space:]]*root[[:space:]]+$KAYAK_DOCROOT;" "$KAYAK_NGINX_DOCROOT_CONF"; then - echo "Error: nginx does not root at $KAYAK_DOCROOT ($KAYAK_NGINX_DOCROOT_CONF)." >&2 - echo " Run \`levels render-serving\` and apply it (Batch 4C)." >&2 - exit 1 - fi - # The ACME challenge root must survive — a blanket root-substitution at cutover - # would clobber it and break cert renewal (PR #194 review #2). - if ! grep -qE "^[[:space:]]*root[[:space:]]+/var/www/certbot;" "$KAYAK_NGINX_DOCROOT_CONF"; then - echo "Error: the certbot ACME root is missing from $KAYAK_NGINX_DOCROOT_CONF" >&2 - echo " (a global root-substitution clobbered it — renewal would break)." >&2 - exit 1 - fi -else - echo "kayak-deploy: WARNING: KAYAK_NGINX_DOCROOT_CONF unset — skipping the nginx" >&2 - echo " serving-root check (set it in $CONF so the cutover is verified)." >&2 +# #192 to here). FAIL-CLOSED: SERVING_CUTOVER=yes is itself set only by the same 4C +# runbook that sets these knobs, so when cutover is on, an UNSET knob is a misconfig +# — warn-skip would let exactly the nginx-side half-cutover (consumers re-pointed, +# web layer still on the old docroot) sail through, which is the gate's whole +# reason to exist (PR #195 review #1). --stage-only exits before this gate, so it +# is unaffected. $KAYAK_DOCROOT is an operator env value, so escape its ERE +# metachars before interpolating it into the grep patterns (PR #195 review #4). +# Escape every char that isn't a safe path char (alnum / _ / - /), so any ERE +# metachar in the docroot becomes literal. Negated-class form (not a metachar +# list) — portable across GNU + BSD sed (the bracket-list form trips BSD). +_docroot_re="$(printf '%s' "$KAYAK_DOCROOT" | sed 's#[^[:alnum:]_/-]#\\&#g')" +# Explicit `exit 1`, NOT `: "${VAR:?}"`: the EXIT-trap (cleanup) runs on a `:?` +# expansion-error exit and bash then exits with the TRAP's status (0), masking the +# failure — explicit `exit 1` is preserved across the trap (verified). +if [ -z "${KAYAK_NGINX_DOCROOT_CONF:-}" ]; then + echo "Error: KAYAK_NGINX_DOCROOT_CONF must be set when SERVING_CUTOVER=yes" >&2 + echo " (the nginx snippet whose root is the docroot; the 4C runbook sets it)." >&2 + exit 1 fi -if [ -n "${KAYAK_FPM_POOL:-}" ]; then - if [ ! -r "$KAYAK_FPM_POOL" ]; then - echo "Error: KAYAK_FPM_POOL=$KAYAK_FPM_POOL not readable" >&2 - exit 1 - fi - if ! grep -qE "open_basedir\][[:space:]]*=[[:space:]]*$KAYAK_DOCROOT:" "$KAYAK_FPM_POOL"; then - echo "Error: PHP-FPM open_basedir does not lead with $KAYAK_DOCROOT ($KAYAK_FPM_POOL)." >&2 - echo " Run \`levels render-serving\` and apply it (Batch 4C)." >&2 - exit 1 - fi -else - echo "kayak-deploy: WARNING: KAYAK_FPM_POOL unset — skipping the FPM open_basedir" >&2 - echo " check (set it in $CONF so the cutover is verified)." >&2 +if [ ! -r "$KAYAK_NGINX_DOCROOT_CONF" ]; then + echo "Error: KAYAK_NGINX_DOCROOT_CONF=$KAYAK_NGINX_DOCROOT_CONF not readable" >&2 + exit 1 +fi +# nginx roots the docroot in ONE shared snippet (the vhosts carry none). +if ! grep -qE "^[[:space:]]*root[[:space:]]+${_docroot_re};" "$KAYAK_NGINX_DOCROOT_CONF"; then + echo "Error: nginx does not root at $KAYAK_DOCROOT ($KAYAK_NGINX_DOCROOT_CONF)." >&2 + echo " Run \`levels render-serving\` and apply it (Batch 4C)." >&2 + exit 1 +fi +# The ACME challenge root must survive a cutover root-substitution (PR #194 review +# #2). /var/www/certbot is hardcoded (WKCC); genericization (4C step 7) knobs it. +if ! grep -qE "^[[:space:]]*root[[:space:]]+/var/www/certbot;" "$KAYAK_NGINX_DOCROOT_CONF"; then + echo "Error: the certbot ACME root is missing from $KAYAK_NGINX_DOCROOT_CONF" >&2 + echo " (a global root-substitution clobbered it — renewal would break)." >&2 + exit 1 +fi +# EXCLUSIVE, not just present: every `root` must be the docroot or the ACME root. +# A leftover legacy `root /home/pat/public_html;` (an apply that APPENDED instead +# of replacing) passes the presence checks, but nginx serves the LAST root — +# silently the legacy tree (PR #195 review #2). +_stray_root="$(grep -nE "^[[:space:]]*root[[:space:]]" "$KAYAK_NGINX_DOCROOT_CONF" \ + | grep -vE "[[:space:]](${_docroot_re}|/var/www/certbot);[[:space:]]*\$" || true)" +if [ -n "$_stray_root" ]; then + echo "Error: unexpected nginx root in $KAYAK_NGINX_DOCROOT_CONF — every root must" >&2 + echo " be $KAYAK_DOCROOT or /var/www/certbot: $_stray_root" >&2 + exit 1 +fi +if [ -z "${KAYAK_FPM_POOL:-}" ]; then + echo "Error: KAYAK_FPM_POOL must be set when SERVING_CUTOVER=yes" >&2 + echo " (the PHP-FPM pool file; the 4C runbook sets it)." >&2 + exit 1 +fi +if [ ! -r "$KAYAK_FPM_POOL" ]; then + echo "Error: KAYAK_FPM_POOL=$KAYAK_FPM_POOL not readable" >&2 + exit 1 +fi +# Anchored at line start (PR #195 review #3) so a `;`-commented open_basedir can't +# satisfy it; render-serving leads the list with the docroot. +if ! grep -qE "^[[:space:]]*php_admin_value\[open_basedir\][[:space:]]*=[[:space:]]*${_docroot_re}:" "$KAYAK_FPM_POOL"; then + echo "Error: PHP-FPM open_basedir does not lead with $KAYAK_DOCROOT ($KAYAK_FPM_POOL)." >&2 + echo " Run \`levels render-serving\` and apply it (Batch 4C)." >&2 + exit 1 fi # --------------------------------------------------------------------------- diff --git a/docs/PLAN_4c_renderers.md b/docs/PLAN_4c_renderers.md index 3a25ae1f..db000cf4 100644 --- a/docs/PLAN_4c_renderers.md +++ b/docs/PLAN_4c_renderers.md @@ -143,11 +143,14 @@ must carry the per-unit write-path set, not a blanket one. 4. **Deployer serving-path gate + quiesce-timeout fix** (`deploy/kayak-deploy.sh`). *(this PR)* When `SERVING_CUTOVER=yes` the gate now verifies, before any mutation: every re-pointed unit that pins `OUTPUT_DIR` matches `$KAYAK_DOCROOT` - (via `systemctl show -p Environment`); and — when `KAYAK_NGINX_DOCROOT_CONF` / - `KAYAK_FPM_POOL` are set (the 4C runbook sets them; unset = warn+skip) — that - nginx roots at `$KAYAK_DOCROOT`, the ACME `root /var/www/certbot;` survives (PR - #194 review #2 — a blanket root-sed would break renewal), and the FPM - `open_basedir` leads with `$KAYAK_DOCROOT`. The quiesce drain-timeout now backs + (via `systemctl show -p Environment`); and — `KAYAK_NGINX_DOCROOT_CONF` / + `KAYAK_FPM_POOL` are REQUIRED under `SERVING_CUTOVER=yes` (fail-closed, PR #195 + review #1 — the same runbook sets both, and warn-skip would let an nginx + half-cutover through) — that nginx roots ONLY at `$KAYAK_DOCROOT` (exclusive: a + leftover legacy `root` is refused, since nginx serves the last; PR #195 review + #2), the ACME `root /var/www/certbot;` survives (PR #194 review #2 — a blanket + root-sed would break renewal), and the FPM `open_basedir` (anchored grep) leads + with `$KAYAK_DOCROOT`. The quiesce drain-timeout now backs out maintenance + restarts consumers before its explicit `exit` (which doesn't fire the ERR trap even under `-E`) — the [[deploy_quiesce_timeout_followup]] fix. Drain bound parameterized (`KAYAK_DRAIN_TIMEOUT`/`_INTERVAL`) so the diff --git a/tests/test_scripts/test_kayak_deploy.py b/tests/test_scripts/test_kayak_deploy.py index fd4c0604..9ec5310c 100644 --- a/tests/test_scripts/test_kayak_deploy.py +++ b/tests/test_scripts/test_kayak_deploy.py @@ -438,6 +438,7 @@ def activate(site_url: str, *, health_url: str | None) -> subprocess.CompletedPr "KAYAK_RUNTIME_CONFIG": str(runtime_config), "KAYAK_CONFIG_INSTALLER": str(installer), "KAYAK_SYSTEMCTL": str(systemctl), + **_serving_knobs(tmp_path, docroot), # Force the privileged branch with a same-user runuser shim so the # app-user DB boundary (backup/restore/build via run_app) is # actually exercised — it is a pass-through otherwise. @@ -530,6 +531,17 @@ def _activation_stubs(tmp_path: Path, root: Path, runtime_config: Path) -> dict[ } +def _serving_knobs(tmp_path: Path, docroot: Path) -> dict[str, str]: + """Good nginx/FPM serving fixtures + the knobs the gate now REQUIRES under + SERVING_CUTOVER=yes (fail-closed, PR #195 review #1). nginx roots only the + docroot + the ACME root; the FPM open_basedir leads with the docroot.""" + nginx = tmp_path / "levels-common.conf" + nginx.write_text(f" root {docroot};\n root /var/www/certbot;\n") + fpm = tmp_path / "kayak-pool.conf" + fpm.write_text(f"php_admin_value[open_basedir] = {docroot}:{tmp_path}/var:{tmp_path}/DB\n") + return {"KAYAK_NGINX_DOCROOT_CONF": str(nginx), "KAYAK_FPM_POOL": str(fpm)} + + @pytest.mark.slow def test_activation_prunes_old_releases( tmp_path: Path, deploy_root: Path, engine_repo, dataset_repo @@ -570,6 +582,7 @@ def activate(site_url: str) -> Path: "HOME": str(tmp_path), "SUDO_USER": "", "KAYAK_UNITS": "kayak-pipeline.timer", + **_serving_knobs(tmp_path, tmp_path / "docroot"), # Keep ONLY current + previous, so the 3rd activation prunes the 1st. "KAYAK_KEEP_RELEASES": "0", } @@ -658,6 +671,7 @@ def activate(site_url: str) -> subprocess.CompletedProcess[str]: "HOME": str(tmp_path), "SUDO_USER": "", "KAYAK_UNITS": "kayak-pipeline.timer", + **_serving_knobs(tmp_path, docroot), } return _run(["--engine-ref", engine_sha, "--dataset-ref", ds_sha], env, timeout=900) @@ -727,6 +741,7 @@ def activate(site_url: str, *, health_url: str | None) -> subprocess.CompletedPr "HOME": str(tmp_path), "SUDO_USER": "", "KAYAK_UNITS": "kayak-pipeline.timer", + **_serving_knobs(tmp_path, docroot), } if health_url is not None: env["HEALTH_URL"] = health_url @@ -819,7 +834,7 @@ def good_fpm() -> None: host_env = tmp_path / "host.env" host_env.write_text(f"SITE_URL=https://x.example.org\nSQLITE_PATH={db}\n") - def activate() -> subprocess.CompletedProcess[str]: + def activate(*, with_knobs: bool = True) -> subprocess.CompletedProcess[str]: env = { "KAYAK_DEPLOY_CONF": str(conf), "KAYAK_DEPLOY_ROOT": str(root), @@ -834,9 +849,10 @@ def activate() -> subprocess.CompletedProcess[str]: "HOME": str(tmp_path), "SUDO_USER": "", "KAYAK_UNITS": "kayak-pipeline.timer", - "KAYAK_NGINX_DOCROOT_CONF": str(nginx_conf), - "KAYAK_FPM_POOL": str(fpm_pool), } + if with_knobs: + env["KAYAK_NGINX_DOCROOT_CONF"] = str(nginx_conf) + env["KAYAK_FPM_POOL"] = str(fpm_pool) return _run(["--engine-ref", engine_sha, "--dataset-ref", ds_sha], env, timeout=900) # 1) Everything points at the docroot → activation succeeds (and stages the @@ -867,6 +883,30 @@ def activate() -> subprocess.CompletedProcess[str]: fpm_pool.write_text("php_admin_value[open_basedir] = /home/pat/public_html:/home/x/DB\n") bad = activate() assert bad.returncode != 0 and "open_basedir does not lead with" in bad.stderr + good_fpm() + + # 6) A leftover legacy root alongside the docroot + certbot → refuse: presence + # isn't enough, every root must be the docroot or the ACME root, else nginx + # serves the LAST one (PR #195 review #2). + nginx_conf.write_text( + f" root {docroot};\n root /home/pat/public_html;\n root /var/www/certbot;\n" + ) + bad = activate() + assert bad.returncode != 0 and "unexpected nginx root" in bad.stderr + good_nginx() + + # 7) SERVING_CUTOVER=yes but the serving knobs unset → fail-closed, not skipped + # (PR #195 review #1 — warn-skip would let an nginx half-cutover through). + bad = activate(with_knobs=False) + assert bad.returncode != 0 + assert "KAYAK_NGINX_DOCROOT_CONF" in bad.stderr + + # 8) A `;`-commented open_basedir must not satisfy the anchored check → refuse + # (PR #195 review #3). + fpm_pool.write_text(f"; php_admin_value[open_basedir] = {docroot}:/home/x/DB\n") + bad = activate() + assert bad.returncode != 0 and "open_basedir does not lead with" in bad.stderr + good_fpm() # None of the refusals mutated the DB (the gate is pre-backup). assert ".backup" not in runuser_log.read_text() or runuser_log.read_text().count(".backup") == 1 @@ -931,6 +971,7 @@ def test_quiesce_timeout_backs_out_maintenance( "HOME": str(tmp_path), "SUDO_USER": "", "KAYAK_UNITS": "kayak-pipeline.timer", + **_serving_knobs(tmp_path, tmp_path / "docroot"), # Tiny drain bound so the loop times out in ~2 s, not 120 s. "KAYAK_DRAIN_TIMEOUT": "2", "KAYAK_DRAIN_INTERVAL": "1",