Deploy: the nginx conf names differ per server, so each slot names its candidates - #376
Merged
Merged
Conversation
…s candidates The 3.1 run of 03-09 stopped in phase 2b. Production has /etc/nginx/sites-enabled/paramant-public.conf but no paramant-live.conf, so the script refused to back up a conf it could not find. That refusal was right, but the name it was looking for was wrong: deploy/signup-fix-deploy.sh edits paramant.conf on that same host, so the backend conf is very likely called paramant.conf there. The two conf names are now two slots, and a slot is a list of candidates separated by "|". The default is paramant-public.conf paramant-live.conf|paramant.conf On the server the first candidate of each slot that is really in sites-enabled is chosen, and the choice is logged as "nginxconf <slot> resolved to <name>". Every later step uses the resolved name and never the candidate string: the 2b backup, the 5c edits, the phase 6 checks, the rollback preconditions in 8a and the restore in 8c. The backup is filed under the resolved name and phase 8 resolves the same way, so a rollback looks for the file that is really there. A slot with no candidate at all still stops the run, with the same hint to set PARAMANT_NGINX_CONFS, which keeps working in the new syntax. A resolved conf without the ParaID deny still FATALs in 5c, and that message now names the confs it read. paramant.conf is an assumption, not a confirmed reading of the live server. The script does not lean on it: it looks on the server every run and says which name it landed on. The resolution lives in one shell function that travels with every remote block that touches a conf, so the server and tests/deploy-3.1-dryrun.test.sh run the same text. The suite grew from 200 to 239 checks: the resolver on its own, 5c against a replica that has paramant.conf instead of paramant-live.conf, against one that has both, and against one that has neither, plus 8a and 8c against the production naming.
Apolloccrypt
added a commit
that referenced
this pull request
Sep 3, 2026
…aims Four things the review was right about. The scrubber's lookbehind excluded ":", so "ip:203.0.113.42" and "ipv6:2001:db8:85a3::1" went through untouched. A colon in front of an address is the ordinary shape of a log line, not a reason to skip it. The lookbehind is [0-9A-Za-z.] now, which still leaves std::vector, ns::method and a bare "::" in prose alone, and the "::" alternative requires at least one hextet after it so prose cannot match. Ten separator shapes are in the test, and the four-part version number the scrubber over-masks is pinned as an accepted trade rather than fixed. The comment no longer calls itself a last line of defence: it is a backstop, the call sites carry the claim, and the gaps are written down. The :8090 access_log off was not pinned. The block filter in tests/site-claims.test.mjs matches a docroot or a 127.0.0.1 upstream and :8090 has neither, and the doesNotMatch above it only fires on a log pointed at a file, so deleting the line left every suite green. The filter takes the block now, there is an assert that names it, and the comment claiming the block only fronts the Outlook add-in is corrected: it answers for location /dicom/. Deleting the line fails; pointing it at a file fails. The justification in the conf was wrong. set_real_ip_from and real_ip_header are in the :8080 block, not this one, and location /dicom/ forwards no X-Real-IP, so $remote_addr there is 127.0.0.1 and the line never carried a client IP. It carried the request line, URI, timing, status, referrer and user-agent. Same correction for what crosses to the Fly host: the request line, the client's own headers and the body, not the IP. And the access_log off does not reach the server. Phase 5c of deploy/deploy-3.1.sh (around line 1226) edits the live confs by anchor and never copies this file. Claim limited to the repo conf in the note and in site-claims row 102; the 5c edit waits for #376, which reshapes 5c. The unused eslint-disable in the test is gone.
Apolloccrypt
added a commit
that referenced
this pull request
Sep 3, 2026
…p, and the dry-run suite runs in CI Two things the review of #376 found. Phase 2b and phase 5c resolve the conf slots independently. That is deliberate: each asks the server what is in sites-enabled at the moment it runs. It also means they can disagree, because phases 3 and 4 pull, build and recreate in between. If sites-enabled is rearranged in that window, 5c resolves to a conf 2b never backed up. Every FATAL in 5c then calls restore(), restore() could only put back what was filed, and the run ended on "Restoring the backed up confs" with that conf left edited and nothing to roll it back to. Reproduced: paramant-public.conf restored, the backend conf LEFT MODIFIED. 5c now requires a backup under this run's TS for every resolved conf, checked before the first sed, so a run that stops there has written nothing. It reports "before confs without a backup" and the deploy asserts that it is zero. restore() walks the resolved names instead of globbing the backup dir, so what it puts back is exactly what was edited, and it says which conf it restored. tests/deploy-3.1-dryrun.test.sh ran in no workflow at all, which docs/ONBOARDING.md said in as many words. So a green pull request said nothing about the only script in this repo that recreates production containers, and two rounds of nginx changes landed on that silence. It is now the test.yml job deploy-dryrun: no server, no secrets, pinned checkout SHA like the other jobs. Do NOT add it to the required contexts yet; give it three green runs on main first. ONBOARDING is corrected on both rows. Documentation fix: #376 said phase 6 uses the resolved conf name. It does not. Phase 6 reads nginx -T, the config that is actually loaded, and probes the site over HTTP, so it cannot be affected by what the file on disk is called. DEPLOY-3.1.md now says that, and names the correction. The suite goes from 239 to 252 checks. New: the rename-between-2b-and-5c scenario stops with nothing written and both confs byte for byte unchanged, the same fixture with the backup in place runs through, and a FATAL after the edits puts both resolved confs back. Every 5c fixture now seeds the 2b backups first, which is what a real run has done by the time 5c starts.
Apolloccrypt
added a commit
that referenced
this pull request
Sep 3, 2026
…after it judged the previous block Deploy run 4 (TS 20260903-0216) stopped in phase 2b on STOP .env backup is a real file with content -- the server never printed 'after .env backup bytes' while the server had printed exactly that line, 1420 bytes, and had resolved both confs, paramant.conf included at 14682 bytes. The measurement was fine; the script could not see it. The wrapper added in #376 was a pipeline: remote_nginx() { { printf '%s\n' "$NGINX_RESOLVE_SNIPPET"; cat; } | remote "$@"; } Every stage of a pipeline is a subshell, so remote() and _remote_run() set REMOTE_OUT and REMOTE_RC in a child that then exited. In the parent both still held whatever the previous remote block had left, so every expect after a remote_nginx call judged the wrong output, and a non-zero REMOTE_RC from such a block would not have stopped the deploy at all. remote_nginx now reads the body itself and hands remote() its stdin through a redirect, so remote() runs in the calling shell and both variables land where the asserts look. The block-level tests extract heredoc bodies and never touch the wrapper, which is why 252 green checks missed this. The new check drives the real remote_nginx() with _remote_run() stubbed, seeds REMOTE_OUT and REMOTE_RC with stale values, and asserts the caller sees the new ones, that the resolver still arrives before the body, and that the body arrives intact. The pipeline spelling is kept as a negative control, so the check cannot pass on a harness that would wave the bug through, and a grep refuses any remote call on the right-hand side of a pipe. 259 checks. On the server run 4 created rollback tags and backups and nothing else. No container was recreated, no conf edited, no docroot written.
Apolloccrypt
added a commit
that referenced
this pull request
Sep 3, 2026
…the dry-run suite runs in CI, and remote_nginx no longer loses the server output to a subshell (#378) Run 4 of the 3.1 deploy stopped in phase 2b on 'the server never printed' while the server had printed it: remote_nginx from #376 piped into remote, so REMOTE_OUT and REMOTE_RC were set in a subshell and every expect after it judged the previous block. The wrapper now reads the body first and feeds remote through process substitution; block 6i-8 calls the real wrapper with a stubbed transport and keeps the pipeline spelling as a negative control. Phase 5c checks for the phase 2b backup of every resolved conf before the first sed and stops with the conf and the backup path when one is missing, and restore() iterates the resolved names instead of a glob. tests/deploy-3.1-dryrun.test.sh runs as the deploy-dryrun job in test.yml, no server, no secrets, not yet a required context. Reviewer ran both wrapper spellings side by side, the rename-between-phases replica (no sed wrote, hashes identical) and the suite in a network-less cage: 259 checks, four sabotages red.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feit uit productie
Deploy-log
deploy/logs/deploy-3.1-20260903-0141.log, fase 2b: op de serverstaat
/etc/nginx/sites-enabled/paramant-public.conf(14355 bytes), maarparamant-live.confis er niet. Het script stopte terecht met "setPARAMANT_NGINX_CONFS if they are named differently". De cockpit kan geen
omgevingsvariabele meegeven aan een run, dus het script moet dit zelf oplossen.
deploy/signup-fix-deploy.sh(regels 56-73) bewerkt op diezelfde server/etc/nginx/sites-enabled/paramant.conf, met daarinreal_ip_header. Dat is debackend-conf. Waarschijnlijk, niet bevestigd.
Fix
Elke conf is nu een slot met een kandidatenlijst, gescheiden door
|. Default:Op de server wordt per slot de eerste kandidaat gekozen die echt in
sites-enabledstaat, en dat wordt gelogd alsnginxconf <slot> resolved to <naam>. Alles daarna gebruikt de opgeloste naam, nooit de kandidaatstring: de2b-backup, de 5c-edits (alle vier, de
/pararules301 van #369 inbegrepen), defase 6-checks, de rollback-precondities in 8a en het herstel in 8c. De backup
wordt onder de opgeloste naam weggeschreven en fase 8 lost hetzelfde op, dus een
rollback zoekt het bestand dat er is.
Geen enkele kandidaat aanwezig blijft STOP, met dezelfde hint.
PARAMANT_NGINX_CONFSwerkt door in de nieuwe syntaxis. Een opgeloste confzonder de ParaID-deny FATALt nog steeds in 5c, en die melding noemt nu de
opgeloste namen.
De resolutie is een shell-functie die meereist met elk remote-blok dat een conf
aanraakt, dus de server en de dry-run-test draaien dezelfde tekst.
Bewijs
bash tests/deploy-3.1-dryrun.test.sh: 239 checks groen (was 200). Nieuw in 6i:paramant-live.conf, alleenparamant.confaanwezig kiest die, geen van beide geeft een ABSENT-regel diematcht op het patroon waar 2b op stopt, en een kapotte symlink telt niet mee
paramant.confin plaats vanparamant-live.conf:lost op, en alle 5c-edits landen aantoonbaar in het bestand dat
paramant.confheetparamant-live.confen laatparamant.confonaangeroerd (md5 ongewijzigd)opgeloste naam en zoekt nooit naar
paramant-live.conf.pre-3.1-*, 8c hersteltin het bestand dat het slot oploste. Backup weg is 1 missing, dus de rollback
stopt
Verder groen:
bash tests/static-sanity.sh(PASS, alle harde checks),scripts/check-test-declarations.sh(114 suites),scripts/check-commit-style.sh.Shellcheck is op deze machine niet geinstalleerd; de suite slaat hem over en
bash -nis schoon.Sabotage
De resolutie de kandidaatstring letterlijk laten teruggeven
(
rc_chosen="$rc_slot") maakt 26 checks rood, verdeeld over 6i-1 tot en met6i-6: 5c en 8c exiten 1, 8a mist een backup, en de "resolved to"-regels
verdwijnen uit het log. Daarna teruggezet, 239 weer groen.
Noot
paramant.confis een aanname op grond vandeploy/signup-fix-deploy.sh, geenbevestigde lezing van de live server. Het script leunt er niet op: het kijkt bij
elke run op de server zelf en zegt op welke naam het uitkwam. Blijkt de
backend-conf anders te heten, dan stopt 2b nog steeds met dezelfde hint en is de
kandidaat een regel bij te werken.