fix(deploy): commit production's compose and bring it under the drift guard (#118)
Production refused to boot with "UPLOADS_DIR is required and is not set" while UPLOADS_DIR was set in Portainer's stack variables. Both statements were true at once. Portainer substitutes stack variables into the compose file rather than handing them to the container, so a variable with no line in the file never reaches the app — behaviour the QA compose already warns about at its ADMIN_GATE_SECRET entry, hit in production where nothing was watching for it.
Nothing could have caught it. The drift guard reads docker-compose.qa.yml, and production ran from a Portainer stack outside the repository that no test could see. That is worse than an even gap: UPLOADS_DIR is in ALWAYS_REQUIRED and the test was green, so the natural reading was that the deploying environments set it. QA did. Production did not.
So production's compose is now a file in the repository, deployed as a git repository stack rather than pasted into the web editor — otherwise the committed copy and the running copy drift apart again, which is the whole problem.
Values are hardcoded rather than interpolated wherever they are not secrets. Only a secret has a reason to stay out of the repository, and every interpolation is another chance for the failure above. UPLOADS_DIR in particular has to agree with the volume mapping, and splitting it across two files is how they drift.
The guard now runs over every deployment rather than QA alone, and checks each by handing its parsed entries to validateEnv itself rather than restating the rules. A restatement is one more copy to drift; running the real validator means the file is checked against exactly what the container checks at boot. Interpolated ${SECRET} values count as present, which is right — what is being guarded is that the line exists, since that is what decides whether the value reaches the container.
Environments differ on purpose, so the expectations are registered per file rather than shared: QA is demo mode with a mail allowlist and no PayPal credentials, production is the reverse of all three. A root-level compose file that is not registered fails the last test, so adding an environment forces the decision instead of silently inheriting whatever the loop asserted.
Verified by removing the UPLOADS_DIR line from the production file and confirming three tests fail, one of them reproducing the exact boot error. A guard of this kind that has never been seen to fire is indistinguishable from one that cannot.
Two things found while writing this and deliberately not changed here. RESERVATION_MINUTES is set in QA's compose and read nowhere in the code — drift in the opposite direction, which #118's scan half should catch. And production publishes 32750 on every interface exactly as QA does; that is #117, and the reasoning is recorded in a comment at the ports block rather than acted on, since changing it needs the proxy host entry repointed in the same pass.
Refs #118
This commit is contained in:
@@ -1,28 +1,67 @@
|
||||
import { readFileSync } from 'fs';
|
||||
import { readFileSync, readdirSync } from 'fs';
|
||||
import path from 'path';
|
||||
import { ALWAYS_REQUIRED } from '../../src/envValidation';
|
||||
import { ALWAYS_REQUIRED, validateEnv } from '../../src/envValidation';
|
||||
|
||||
/**
|
||||
* The guard for #107.
|
||||
* The guard for #107 and #118.
|
||||
*
|
||||
* That failure was not the missing variable — it was that nothing connected two
|
||||
* files. envValidation.ts gained a required variable and docker-compose.qa.yml
|
||||
* did not set it, and nothing noticed until the container refused to boot on a
|
||||
* #107 was not the missing variable — it was that nothing connected two files.
|
||||
* envValidation.ts gained a required variable and docker-compose.qa.yml did
|
||||
* not set it, and nothing noticed until the container refused to boot on a
|
||||
* deploy. CI passed throughout, because CI sets its own environment and never
|
||||
* reads the compose file.
|
||||
*
|
||||
* So this reads the real list from the validator rather than a copy. A copy
|
||||
* would pass forever while the next added variable went unguarded in precisely
|
||||
* the same way.
|
||||
* the same way. For the same reason the per-file check below runs `validateEnv`
|
||||
* itself rather than restating its rules: the point is that each deploying file
|
||||
* satisfies the validator, and any restatement here is one more thing to drift.
|
||||
*
|
||||
* What this cannot cover: production runs from a Portainer stack outside this
|
||||
* repository, so nothing here can check it. Adding a required variable still
|
||||
* means updating that stack by hand, and this test is not evidence that it was
|
||||
* done.
|
||||
* #118 was the other half. This used to read QA's file alone, while production
|
||||
* ran from a Portainer stack outside the repository that nothing could check.
|
||||
* That was worse than an even gap, because a green test implied a coverage it
|
||||
* did not have: UPLOADS_DIR is in ALWAYS_REQUIRED, QA set it, production did
|
||||
* not, and on 2026-08-23 production refused to boot for exactly that reason —
|
||||
* while the variable was set in Portainer, where stack variables are
|
||||
* substituted into the compose file rather than handed to the container, so a
|
||||
* variable with no line in the file never reaches the app at all.
|
||||
*/
|
||||
|
||||
const COMPOSE_PATH = path.resolve(__dirname, '..', '..', '..', 'docker-compose.qa.yml');
|
||||
const compose = readFileSync(COMPOSE_PATH, 'utf8');
|
||||
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..');
|
||||
|
||||
/**
|
||||
* Every deployment this repository describes, and what is true of each.
|
||||
*
|
||||
* Registered rather than discovered blindly, because the environments differ on
|
||||
* purpose — QA runs in demo mode with no PayPal credentials and a mail
|
||||
* allowlist, production is the reverse of all three — so "the files agree with
|
||||
* each other" would be the wrong assertion. What each file has to satisfy is
|
||||
* the validator, plus the handful of things below that are properties of that
|
||||
* environment rather than of the code.
|
||||
*
|
||||
* A compose file at the repository root that is not registered here fails the
|
||||
* last test in this file. That is deliberate: adding an environment should
|
||||
* force the decision about what is true of it, rather than silently inheriting
|
||||
* whatever the loop happened to assert.
|
||||
*/
|
||||
const DEPLOYMENTS = [
|
||||
{
|
||||
file: 'docker-compose.qa.yml',
|
||||
demoMode: 'true',
|
||||
// The entire safety property of QA: delivery is restricted to named
|
||||
// recipients, so a run against a database full of test fixtures cannot
|
||||
// email a real customer. Asserted because the compose file claims it must
|
||||
// not depend on somebody remembering to set something.
|
||||
requiresMailAllowlist: true
|
||||
},
|
||||
{
|
||||
file: 'docker-compose.prod.yml',
|
||||
demoMode: 'false',
|
||||
// Deliberately unrestricted. Production has to be able to reach real
|
||||
// customers, and it is the one environment where that is correct.
|
||||
requiresMailAllowlist: false
|
||||
}
|
||||
] as const;
|
||||
|
||||
// Only real environment entries — `- NAME=value` at an indented list position.
|
||||
// A mention inside a comment cannot match, because a comment line starts with #.
|
||||
@@ -43,9 +82,10 @@ function environmentEntries(source: string): Map<string, string> {
|
||||
return entries;
|
||||
}
|
||||
|
||||
const entries = environmentEntries(compose);
|
||||
describe.each(DEPLOYMENTS)('$file provides everything the app requires to boot', (deployment) => {
|
||||
const compose = readFileSync(path.join(REPO_ROOT, deployment.file), 'utf8');
|
||||
const entries = environmentEntries(compose);
|
||||
|
||||
describe('the QA compose file provides everything the app requires to boot', () => {
|
||||
it('parsed some environment entries at all', () => {
|
||||
// Guards the guard: a regex that matched nothing would make every
|
||||
// assertion below vacuously true.
|
||||
@@ -56,17 +96,32 @@ describe('the QA compose file provides everything the app requires to boot', ()
|
||||
expect(entries.has(name)).toBe(true);
|
||||
});
|
||||
|
||||
// DEMO_MODE is required too, but validated separately from ALWAYS_REQUIRED
|
||||
// because its rule is stricter than presence — it must be exactly 'true' or
|
||||
// 'false'. Named explicitly here so it is not missed by reading only the list.
|
||||
it('sets DEMO_MODE, to one of the two values that are allowed', () => {
|
||||
expect(entries.has('DEMO_MODE')).toBe(true);
|
||||
expect(['true', 'false']).toContain(entries.get('DEMO_MODE'));
|
||||
/**
|
||||
* The strongest form of this check, and the one that cannot drift: hand the
|
||||
* file's own entries to the real validator and require it to be satisfied.
|
||||
*
|
||||
* An interpolated `${SECRET}` counts as present, which is correct — what is
|
||||
* being checked is that the *line exists*, since that is what decides whether
|
||||
* the value reaches the container. Whether the stack variable behind it is
|
||||
* set is a different failure, and one the app already reports clearly at boot.
|
||||
*
|
||||
* Errors only. Production warns about MAIL_ALLOWLIST by design, and silencing
|
||||
* that warning would remove the notice that this environment can email real
|
||||
* customers.
|
||||
*/
|
||||
it('satisfies validateEnv, the same check the container runs at boot', () => {
|
||||
const { errors } = validateEnv(Object.fromEntries(entries));
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
|
||||
it(`sets DEMO_MODE to ${deployment.demoMode}`, () => {
|
||||
expect(entries.get('DEMO_MODE')).toBe(deployment.demoMode);
|
||||
});
|
||||
|
||||
// The reason UPLOADS_DIR is hardcoded rather than taken from a stack
|
||||
// variable is that it has to agree with the volume mapping. That claim is
|
||||
// only worth making if something checks it.
|
||||
// only worth making if something checks it — and its absence from production
|
||||
// is what stopped that stack booting.
|
||||
it('points UPLOADS_DIR at the directory the uploads volume is mounted on', () => {
|
||||
const uploadsDir = entries.get('UPLOADS_DIR');
|
||||
expect(uploadsDir).toBeTruthy();
|
||||
@@ -79,4 +134,28 @@ describe('the QA compose file provides everything the app requires to boot', ()
|
||||
it('references ADMIN_GATE_SECRET from the stack rather than holding a value', () => {
|
||||
expect(entries.get('ADMIN_GATE_SECRET')).toBe('${ADMIN_GATE_SECRET}');
|
||||
});
|
||||
|
||||
if (deployment.requiresMailAllowlist) {
|
||||
it('restricts delivery with MAIL_ALLOWLIST', () => {
|
||||
expect(entries.get('MAIL_ALLOWLIST')).toBeTruthy();
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('every deployment in the repository is under this guard', () => {
|
||||
// The failure #118 was about was a file nothing read. A new environment added
|
||||
// as a compose file and not registered above would reproduce it exactly, so
|
||||
// the omission has to fail rather than pass quietly.
|
||||
//
|
||||
// Scoped to the repository root: backend/docker-compose.test.yml is a bare
|
||||
// Postgres for the integration suite, with no app service and nothing here to
|
||||
// say about it.
|
||||
it('registers every root-level compose file in DEPLOYMENTS', () => {
|
||||
const found = readdirSync(REPO_ROOT).filter(
|
||||
(name) => /^docker-compose\..+\.ya?ml$/.test(name)
|
||||
);
|
||||
const registered = DEPLOYMENTS.map((d) => d.file);
|
||||
|
||||
expect(found.slice().sort()).toEqual(registered.slice().sort());
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user