diff --git a/README.md b/README.md index 0f8eccd..aab7e55 100755 --- a/README.md +++ b/README.md @@ -97,6 +97,10 @@ mkdir -p /tmp/redefined-uploads `PUBLIC_URL` is required only alongside SMTP because its only job is building links in email; an environment that cannot send mail does not need it. `UPLOADS_DIR` has no such reprieve — its fallback of `/app/uploads` is correct inside the container and wrong everywhere else. +**Adding to the required list has reach beyond this repository.** A variable added to `ALWAYS_REQUIRED` must also be set in every environment that deploys, and there are two of those. `docker-compose.qa.yml` is checked automatically — `backend/tests/unit/composeEnvironment.test.ts` reads the validator's own list and fails if the compose file does not set something on it, which is what #107 existed to prevent from recurring. Production runs from a Portainer stack **outside this repository**, so nothing can check it: that one has to be updated by hand, before the deploy rather than during it. + +Note also that setting a variable in Portainer's stack environment is not the same as giving it to the container. Stack variables are interpolated into the compose file as `${VAR}`; a service receives exactly what its own `environment:` block lists. A variable with no line there never arrives, however carefully it was set in the UI. + **Note (PowerShell):** environment variables set with `$env:` only last for the current terminal session/tab. If you close and reopen VS Code's terminal, you'll need to re-run step 3 before starting the backend again. ### 4. Run the backend diff --git a/backend/src/envValidation.ts b/backend/src/envValidation.ts index 55ce96c..2d6155d 100644 --- a/backend/src/envValidation.ts +++ b/backend/src/envValidation.ts @@ -27,7 +27,13 @@ export interface EnvValidation { } // Without these the process cannot do its job at all. -const ALWAYS_REQUIRED = [ +// +// Exported so tests/unit/composeEnvironment.test.ts can assert the deploying +// environment actually sets them. #107 happened because this list grew and +// docker-compose.qa.yml did not: the check has to read this list rather than a +// copy of it, or the next variable added here goes unguarded in exactly the +// same way. +export const ALWAYS_REQUIRED = [ 'PGHOST', 'PGPORT', 'PGUSER', diff --git a/backend/tests/unit/composeEnvironment.test.ts b/backend/tests/unit/composeEnvironment.test.ts new file mode 100644 index 0000000..add172a --- /dev/null +++ b/backend/tests/unit/composeEnvironment.test.ts @@ -0,0 +1,76 @@ +import { readFileSync } from 'fs'; +import path from 'path'; +import { ALWAYS_REQUIRED } from '../../src/envValidation'; + +/** + * The guard for #107. + * + * 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 + * 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. + * + * 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. + */ + +const COMPOSE_PATH = path.resolve(__dirname, '..', '..', '..', 'docker-compose.qa.yml'); +const compose = readFileSync(COMPOSE_PATH, 'utf8'); + +// Only real environment entries — `- NAME=value` at an indented list position. +// A mention inside a comment cannot match, because a comment line starts with #. +function environmentEntries(source: string): Map { + const entries = new Map(); + for (const line of source.split('\n')) { + const match = /^\s+- ([A-Z_0-9]+)=(.*)$/.exec(line); + if (match) { + entries.set(match[1], match[2].trim()); + } + } + return entries; +} + +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. + expect(entries.size).toBeGreaterThan(5); + }); + + it.each([...ALWAYS_REQUIRED])('sets %s', (name) => { + 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 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. + it('points UPLOADS_DIR at the directory the uploads volume is mounted on', () => { + const uploadsDir = entries.get('UPLOADS_DIR'); + expect(uploadsDir).toBeTruthy(); + expect(compose).toContain(`:${uploadsDir}`); + }); + + // Interpolated rather than hardcoded, so the secret itself never enters the + // repository. Present as a reference is what matters; an unset stack variable + // resolves to empty, which the validator and the gate both read as "off". + it('references ADMIN_GATE_SECRET from the stack rather than holding a value', () => { + expect(entries.get('ADMIN_GATE_SECRET')).toBe('${ADMIN_GATE_SECRET}'); + }); +});