build: use slim node image in production - #29
Merged
Merged
Conversation
Any failure - database down, timeout, login - now serves the last good result from disk with content-created-at set, and logs reason=error. 500 only when there is no cache.
Write to a temp file in the same directory and rename into place, so a concurrent reader cannot get a half-written file.
Gives test.js a route whose query never succeeds, so the error path can be exercised without taking the database down.
Covers the 200 from cache with content-created-at reporting the cache age, and the 500 when no cache exists.
The test writes the cache file from the test container while the app reads it from another. Under the delegated bind mount that write is not immediately visible, so the assertion raced it.
mssql@9 pulls @azure/identity@2 -> @azure/msal-node@1, whose engines field stops at Node 18, and yarn treats that as a hard error. mssql 11.0.2 installs cleanly and keeps NTLM auth: config.domain still selects the ntlm authentication type, so production auth is unaffected. Drags tedious 15 -> 18.6.2. Its changed connection defaults around encryption need no explicit setting: encrypt defaults to true and trustServerCertificate is honoured, so the config shape in config.dev.js.dist connects unchanged. Claude-Session: https://claude.ai/code/session_012qXhBodStu75USEqkDEHKH
node:18 is end of life, no security updates since April 2025. Claude-Session: https://claude.ai/code/session_012qXhBodStu75USEqkDEHKH
sql.connect() keeps a single process-wide pool: the first call creates it and every later call gets that pool back and discards the config it was handed. With more than one connection configured, whichever route was hit first decided the database for all of them. Verified against the installed mssql 11.0.2 - the behaviour is unchanged from 9. Keep one ConnectionPool per named connection, created on first use, with the connect promise cached per name and dropped again if it rejects so a later request retries.
The seeded stack has one database, so the second connection points at a host that does not exist: the route can only answer if it was handed another connection's pool. Fails with 200 !== 500 on the previous app.js.
nginx was a pure proxy - no static files, no cache, no auth - and it resolved node's IP once at startup, so a node container returning on a new IP meant a permanent 502 while nginx itself stayed healthy (F1). The traefik labels move to node, including the basic auth middleware on staging and the www redirect, so the routers keep their middlewares. The node image has no EXPOSE, so the port is stated explicitly. Claude-Session: https://claude.ai/code/session_012qXhBodStu75USEqkDEHKH
The hand-written vhost overwrote X-Forwarded-For with traefik's own IP and never set X-Forwarded-Proto, which is why the index page emitted http:// links over TLS (F12). With nginx gone traefik sets both correctly; express only needs to be told to believe the one hop. Claude-Session: https://claude.ai/code/session_012qXhBodStu75USEqkDEHKH
ITK's php images create deploy at 1042 and the templates set user:
${COMPOSE_USER:-deploy}. The deployed tree on the server is owned by that
user, so running as node (1000) would fail every cache write with EACCES.
1042 has no passwd entry in the node image, which leaves HOME=/ and makes
standard fail with EACCES on its cache, so HOME is set to /tmp.
The runner owns the checkout as uid 1001, so CI chowns it to 1042 before
the stack runs.
ITK's templates use COMPOSE_USER as a user name (deploy), which the node image does not have, so a shared env setting it would stop the container.
Handing the whole checkout to 1042 leaves .git unwritable for the runner's checkout post-step.
After a deploy the config is the production one, so the checks that need the seeded database and the routes from config.dev.js.dist cannot run. What is left still queries the real database, which is the point: the deploy check so far fetched / only, and / runs no query.
Wraps the container commands the project already uses, and brings the deploy on the server into the repository: scripts/deploy, scripts/test and the docker-compose wrapper lived in ~/www/musikcsv/scripts/, outside version control, and the wrapper pinned the v1 docker-compose binary.
Production only runs yarn install and yarn start, so it needs none of the toolchain the full tag carries: 332 MB against 1546 MB, same Node 24.21.0, and slim still ships yarn 1.22.22, npm and corepack, so the existing commands are untouched. A cold yarn install --frozen-lockfile was verified in the slim image, and test.js passes 11/11 against the production compose file with the app served by node:24-slim. Local development stays on the full image for the toolchain.
The production image was never tested: CI and task test ran the dev compose on the full image. A cold yarn install --frozen-lockfile, standard and test.js (11/11, and 6/6 with SMOKE=1) pass on node:24-slim.
turegjorup
force-pushed
the
feature/8293-taskfile
branch
from
September 23, 2026 07:18
16b00dd to
8ed9a3a
Compare
turegjorup
force-pushed
the
feature/8293-slim-image
branch
from
September 23, 2026 07:19
219d3a4 to
888d62f
Compare
rimi-itk
approved these changes
Sep 24, 2026
turegjorup
force-pushed
the
feature/8293-taskfile
branch
10 times, most recently
from
September 24, 2026 11:31
fa94c6e to
b46b40e
Compare
# Conflicts: # README.md # Taskfile.yml # config.js.dist # docker-compose.server.yml # docker-compose.yml
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
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.
Production runs
yarn installandyarn startand nothing else, so it needs no toolchain.https://leantime.itkdev.dk/#/tickets/showTicket/8293
Changes
docker-compose.server.yml:node:24→node:24-slim.docker-compose.yml: the same, so local development,task testand CI run the image production runs.332 MB against 1546 MB, same Node version. The slim tag still ships yarn 1.22.22, npm and corepack, so the deploy path's
yarn installand theyarn startcommand keep working untouched. A coldyarn install --frozen-lockfile,standardand the test suite all pass on slim.Not switching to npm: the repo has
yarn.lockand nopackage-lock.json, and generating one would re-resolve every transitive version.node:24-slimis a floating tag andtask deploypulls, so a deploy can pick up a newer Node 24 minor.Verify
Expected:
node:24-slimin both configs,v24.xand1.22.22from the image,11/11 passed.Stacked on #28.