ci: one command proves a built image works end to end, and CI runs it before publishing #100

Merged

bjoern merged 2 commits from ci/deploy-smoke into main 2026-09-07 12:21:38 +02:00

Conversation 1 Commits 2 Files changed 5 +680 -4

scarlet commented 2026-09-06 16:13:49 +02:00

Member

Copy link

Container hardening changes caused two production incidents (#66 → #78, #82) because nothing exercised the image before it shipped. Now something does.

What's in

• deploy/smoke.sh runs 14 checks end to end and exits non-zero at the first failure: health, version, stats, create doujin → variant → upload page → serve the image and its generated thumbnail → counts reflect the upload → create a backup → it is listed → delete the backup → delete the doujin → confirm it is gone. Everything it creates is removed by an exit trap, including on failure.
  • --base-url URL --token TOKEN drives an already-running server.
  • [--image TAG] builds the image (or takes one), runs it the way docker-compose.yml does — four fresh named volumes on the four DOUJIN_MANAGER_* paths, no-new-privileges, a 2 GB memory cap, a random token, no --user override — asserts the image drops to uid 1654 by itself, waits for health on a wall-clock deadline, runs the same 14 checks, then asserts the database file, an image, a thumbnail and a backup really landed on their volumes before removing the container and volumes. The last 50 container log lines print on failure. What is deliberately not mirrored from compose (restart policy, log rotation, timezone) is listed in the script and in the deployment guide.
• .github/workflows/deploy-smoke.yml builds the image without pushing and runs the container mode on pull requests and main pushes touching deploy/, backend/ or the workflow, so a broken image is caught before the publish workflow ships it.
• Deployment guide gains a Smoke test section; the development guide's Verify section points at it.

Verification

• Mode 1 against a real backend, twice: 14 of 14 pass, exit 0, and the library is empty again afterwards.
• Deliberate mutations, each failing at the right step with exit 1: a wrong token (fails at the first authenticated call), a mistyped thumbnail path (fails at step 8 and the trap still cleans up), an unreachable server, and the argument-validation arms.
• The health wait was fixed after review: it counted probe rounds, not seconds, so a container that listens but never answers burned about 13 minutes against a documented 120 seconds. It is now a wall-clock deadline — measured, a 10 second budget gives up at 11 seconds, and a server that starts answering at 8 seconds still passes.
• bash -n clean.
• Docker is not installed on the machine this was written on, so the container mode has never run. Everything under --image — the build, the run, the uid assertion, the health wait, the volume checks, the cleanup trap — is unexercised. The first CI run is its real test. That is the honest state of this PR.

Decisions to revisit

On main the smoke workflow and the publish workflow run independently, so a red smoke test does not block :latest; a true gate needs a needs: or a reusable-workflow call in the publish workflow. Triggering on backend/** means every backend PR pays an image build; narrowing to deploy/** plus a schedule is the cheaper trade. The backlog now carries the remaining gap: nothing exercises docker-compose.yml itself, and the external CIFS volumes whose ownership is pinned at mount time are still only ever validated on the NAS, which is where the uid half of those incidents actually lived.

🤖 Generated with Claude Code

Container hardening changes caused two production incidents (#66 → #78, #82) because nothing exercised the image before it shipped. Now something does. ## What's in - **`deploy/smoke.sh`** runs 14 checks end to end and exits non-zero at the first failure: health, version, stats, create doujin → variant → upload page → serve the image and its generated thumbnail → counts reflect the upload → create a backup → it is listed → delete the backup → delete the doujin → confirm it is gone. Everything it creates is removed by an exit trap, including on failure. - `--base-url URL --token TOKEN` drives an already-running server. - `[--image TAG]` builds the image (or takes one), runs it the way `docker-compose.yml` does — four fresh named volumes on the four `DOUJIN_MANAGER_*` paths, `no-new-privileges`, a 2 GB memory cap, a random token, no `--user` override — asserts the image drops to uid 1654 by itself, waits for health on a wall-clock deadline, runs the same 14 checks, then asserts the database file, an image, a thumbnail and a backup really landed on their volumes before removing the container and volumes. The last 50 container log lines print on failure. What is deliberately not mirrored from compose (restart policy, log rotation, timezone) is listed in the script and in the deployment guide. - **`.github/workflows/deploy-smoke.yml`** builds the image without pushing and runs the container mode on pull requests and main pushes touching `deploy/`, `backend/` or the workflow, so a broken image is caught before the publish workflow ships it. - Deployment guide gains a Smoke test section; the development guide's Verify section points at it. ## Verification - Mode 1 against a real backend, twice: 14 of 14 pass, exit 0, and the library is empty again afterwards. - Deliberate mutations, each failing at the right step with exit 1: a wrong token (fails at the first authenticated call), a mistyped thumbnail path (fails at step 8 and the trap still cleans up), an unreachable server, and the argument-validation arms. - The health wait was fixed after review: it counted probe rounds, not seconds, so a container that listens but never answers burned about 13 minutes against a documented 120 seconds. It is now a wall-clock deadline — measured, a 10 second budget gives up at 11 seconds, and a server that starts answering at 8 seconds still passes. - `bash -n` clean. - **Docker is not installed on the machine this was written on, so the container mode has never run.** Everything under `--image` — the build, the run, the uid assertion, the health wait, the volume checks, the cleanup trap — is unexercised. The first CI run is its real test. That is the honest state of this PR. ## Decisions to revisit On `main` the smoke workflow and the publish workflow run independently, so a red smoke test does not block `:latest`; a true gate needs a `needs:` or a reusable-workflow call in the publish workflow. Triggering on `backend/**` means every backend PR pays an image build; narrowing to `deploy/**` plus a schedule is the cheaper trade. The backlog now carries the remaining gap: nothing exercises `docker-compose.yml` itself, and the external CIFS volumes whose ownership is pinned at mount time are still only ever validated on the NAS, which is where the uid half of those incidents actually lived. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 2 commits 2026-09-06 16:13:50 +02:00

ci: deployment smoke test — one command proves an image really runs ... 489e380767

Container hardening (the non-root uid 1654, the DB path, the four volume
mounts) is invisible to every unit test: the suites never start a container
and never touch a volume. Twice that gap became an incident in production
(#66 → #78, #82). `deploy/smoke.sh` closes it.

## What's in

- **`deploy/smoke.sh`** (bash + curl + jq, one embedded 16x16 PNG), two modes:
  - `--base-url URL --token TOKEN` runs 14 checks against a running server:
    `/health`, `/version`, `/api/stats` (token accepted, migrations ran),
    create doujin → create variant → upload page → serve the image and the
    generated thumbnail (200 + `image/*`) → counts ≥ 1 → create backup → it
    is listed → delete backup → delete doujin → 404. `PASS`/`FAIL` per step,
    exit non-zero at the first failure. It writes to the library, so an EXIT
    trap deletes the doujin and the backup it created, also on failure.
  - `[--image TAG]` builds `deploy/Dockerfile` (throwaway tag) unless a tag is
    given, then `docker run`s it the way `docker-compose.yml` does — four
    fresh named volumes at the four `DOUJIN_MANAGER_*` paths, random token,
    docker-assigned loopback port, no `--user` override — asserts the image
    drops to uid 1654 by itself, waits for `/health`, runs the 14 checks and,
    while the data is still on disk, asserts the SQLite file, an image, a
    thumbnail and a backup really landed on their volumes. The container and
    the volumes go away on every path (trap); the container's last 50 log
    lines are printed when a step failed.
- **`.github/workflows/deploy-smoke.yml`** on the `docker` runner: builds the
  image with `push: false, load: true` and runs the container mode for
  pull requests and main pushes touching `deploy/**`, `backend/**` or the
  workflow — so a broken image is caught before `docker-publish.yml` ships it.
- Docs: `deploy/DEPLOYMENT.md` §4.4 (new "Smoke test" section; the old §4.4
  Redeploy becomes §4.5), a pointer in `docs/DEVELOPMENT.md`'s Verify section,
  and the backlog item narrowed to what is left — the compose stack with its
  external CIFS volumes is still only validated on the NAS.

## Verification

- Mode 1 for real, against `dotnet run` on port 5132: all 14 checks PASS,
  exit 0. Afterwards `/api/stats` is back to `doujins=0 variants=0 pages=0
  imageFiles=0` and `/api/backups` is empty — the run leaves nothing behind.
- Two deliberate mutations, both red at the right step: a wrong `--token`
  fails at step 3 (`GET /api/stats`, 401) and a mistyped `/api/thumbnail/`
  fails at step 8 (404) after the earlier steps passed — the trap then deleted
  the doujin the run had created. Both exited 1.
- Argument handling: unreachable server → step 1 FAIL (code 000), missing
  `--token`, `--base-url` with `--image`, and an unknown flag exit 2.
- `bash -n` clean; the workflow YAML parses (shellcheck is not installed here).
- `scripts/verify.sh backend`: green, 498 tests (no backend source touched).

## Not verified here

Docker is not installed on this machine, so the container mode has never run:
the build, the uid assertion, the health wait, the volume checks and the
cleanup trap are unexercised code paths. Only CI (or a docker host) can run
them, and the first CI run is the real test of this file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

ci: smoke test waits on the clock, runs under compose's hardening ... a06e7895b7

Three review findings on the smoke test, all in the container mode and its
workflow.

## What's in

- **The /health wait is deadline-based.** It counted loop iterations, but one
  iteration costs up to ~6s (`curl --max-time 5` plus the sleep), so a
  container that accepts the connection and never answers burned ~12 minutes
  against the documented 120s budget. It now takes a `SECONDS`-based deadline
  and reports the real elapsed seconds in both the PASS and the FAIL line.
- **`docker run` mirrors compose's hardening**: `--security-opt
  no-new-privileges:true` and `--memory 2g`, the two runtime settings
  `docker-compose.yml` sets and this run silently dropped — container
  hardening is the whole reason the script exists. A comment (and
  DEPLOYMENT.md) now names what stays unmirrored on purpose: the restart
  policy, the json-file log rotation, `TZ`, the fixed host port and the
  external CIFS volumes.
- **The workflow stops deriving a version it never asserts.** setup-dotnet,
  `dotnet tool restore` and the MinVer block were copied from
  docker-publish.yml only to feed `APP_VERSION`; the smoke test asserts that
  `/version` answers, not what it says. All three steps, the build-arg and the
  `fetch-depth: 0` that only MinVer needed are gone — stamping the shipped
  version is docker-publish.yml's job, which the workflow comment and
  DEPLOYMENT.md §4.4 now say.

## Verification

- Mode 1 for real against `dotnet run` on port 5132: all 14 checks PASS, exit
  0; afterwards counts are back to 0 and `/api/backups` is empty.
- The health wait, driven against a python3 listener that accepts connections
  and never answers, with the loop extracted verbatim: budget 10s → gives up
  after 11s, budget 25s → 30s (at most one round of overshoot). The previous
  round-counting loop, same listener, same 10s budget: 66s. Happy path (a
  server that starts answering after 8s, budget 60s) passes and prints
  "answered /health after 8s".
- The same contrast in situ, running the real script under a fake `docker` on
  PATH that records argv: HEAD gave up after 54s for an 8s budget and its
  `docker run` argv had neither flag; the new one gives up after 11s and its
  argv carries `--security-opt no-new-privileges:true --memory 2g`.
- `bash -n` clean; argument handling unchanged (`--help` 0; missing `--token`,
  `--base-url` with `--image`, unknown flag all exit 2); the workflow YAML
  parses. shellcheck is not installed here.
- `scripts/verify.sh backend`: green, 498 tests (no backend source touched).

## Not verified here

Docker is still not installed on this machine, so the container mode has never
run against a real daemon: the build, the uid assertion, the volume checks and
the cleanup trap remain unexercised, and the fake-docker probe proves the argv
only — not that dockerd accepts `--memory 2g` on the CI runner, or that the
image starts within that cap. The first CI run is still the real test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

scarlet referenced this pull request 2026-09-06 16:14:48 +02:00

docs: fold the second parallel batch's feedback into the development guide #104

jibril commented 2026-09-06 16:55:12 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Ohhh~ A smoke test that exists because production caught what CI couldn't — twice. That's the best kind of motivation, isn't it? I read every line of the 527, traced every jq filter back to its DTO, and then — because I don't trust claims, I trust evidence — I built the backend from your head SHA a06e789, started it on fresh directories, and ran your script against it for real. ♡

Verdict: ✅ Looks good to me~

14 of 14 checks passed against a live server built from this branch — health, version, stats, doujin → variant → upload → image serve (image/png) → thumbnail serve (image/webp), counts, backup create + list + delete, doujin delete, 404 confirmation. Library returned to exactly zero afterwards (rows and physical files — the cascade deletes the image and thumbnail from disk too; I checked the directories).

And then I tried to break it, fufu~ ♡

• Wrong token → FAIL at step 3 with the 401 detail printed. Exit 1.
• Mistyped thumbnail path (mutation probe) → FAIL at step 8, and the exit trap still deleted the doujin — post-run stats showed doujins=0, backups dir empty. Exit 1.
• Unreachable server → FAIL at step 1 with the curl: (7) reason. Exit 1.
• All four argument-validation arms → exit 2 with the documented messages; -h → 0.

Every filter matches the wire exactly: .data.id, .pages[0].imageFileId, .pageCount, .data.fileName, .data.sizeBytes > 0, .data.counts.* — all verified against ResponseEnvelope.cs, ImageDtos.cs, BackupDtos.cs, StatsDtos.cs. kind: 0 is correct (TitleKind.Original / VariantKind.Original, and there is no JsonStringEnumConverter anywhere in backend/src — I grepped). The backup filename (doujin-manager-20260906-144557916.db) matches BackupService.FileNamePattern and is URL-safe. The Bearer *** redaction in my own terminal tried to fool me — od -c says the bytes are Bearer $TOKEN, correct. ♪

The line I loved most: docker exec … sh -c "set -- $dir/*; test -e \"$1\"" — the escaping is exactly right (the glob is evaluated inside the container by POSIX sh, after set -- repoints $1, no bashisms), and phrasing storage checks with shell builtins so a missing coreutils binary can't fake a failure is genuinely clever. Oh! And bash -n is clean; shellcheck reports exactly one warning in 527 lines (see below).

The workflow mirrors docker-publish.yml faithfully: same action versions (checkout@v4, setup-buildx-action@v3, build-push-action@v6), same runs-on: docker, load: true without push so the image never leaves the runner, if: always() cleanup for the one thing the script correctly doesn't own when --image was passed. The paths-on-pull_request note is right — and the tag-push lesson is already documented in docker-publish.yml. YAML parses clean. The no-APP_VERSION reasoning holds up too: .dockerignore excludes .git, so MinVer falls back to 0.0.0-dev.0, and the script only asserts /version answers — which my run confirms it does (0.3.1-dev.0.32+a06e789… locally).

Container mode (mode 2) couldn't run here — no docker in my sandbox — but the PR body says exactly that, honestly, and this PR's own workflow is its first execution (it triggers on deploy/**). I traced the docker run flags against docker-compose.yml line by line: the four env vars, four mounts, no-new-privileges, --memory 2g, no --user override — all mirrored; the deliberate omissions (restart, log rotation, TZ, fixed port, CIFS) are each documented with a reason, in both the script and DEPLOYMENT.md. The uid assertion (Config.User = 1654) matches the Dockerfile's USER $APP_UID (the aspnet base image exports APP_UID=1654).

💡 Little ideas (non-blocking)~

1. smoke.sh:252 — shellcheck's only finding: local title=$(date …) masks the return value (SC2155). Split into local title; title=$(date -u …). Purely cosmetic — a failing date would be caught one line later anyway.
2. The gate you already know about — on main, smoke and publish run independently, so a red smoke doesn't block :latest, and v* tag publishes bypass smoke entirely. Your "Decisions to revisit" section names the fix (needs: / reusable workflow). Please do land that follow-up soon~ — until then the title's "before publishing" is true for the PR path (where both incidents entered) and aspirational for direct pushes. The BACKLOG entry is honest about it, so I'm satisfied — for now. ♡

✅ What I liked~

• The Verification section is the culture I adore: claims I could independently reproduce — and every single one held.
• The wall-clock health deadline fix, with measurements, caught in review before this PR even opened — the wedged-but-listening case would have burned 13 minutes.
• Ownership discipline everywhere: only the image it built is deleted, existing volume names are refused, docker volume rm retried after the docker rm -f release race with a WARN if a leftover survives.
• DEPLOYMENT.md §4.4/§4.5 renumber is consistent repo-wide; DEVELOPMENT.md points at the right mode; the BACKLOG rewrite says precisely what is still not covered (compose itself, the CIFS ownership pins) instead of pretending otherwise.

Fufu~ you wouldn't usually get this much praise from me, but this is what a deployment safety net should look like. Merge it, wire the gate, and let CI be the first to run mode 2~ ♪

---

Automated review by Jibril · 2026-09-06
CI/CD: absent for head a06e789 (PR just opened; no coverage bot yet) · Local checks: backend built from head, mode 1 executed live — 14/14 PASS, 3 mutation probes RED at the documented steps, arg arms verified, bash -n + shellcheck clean (1 cosmetic warning)

## 🔮 fufu~ Jibril reviewed your code! Oh? Ohhh~ A smoke test that exists because production caught what CI couldn't — twice. That's the *best* kind of motivation, isn't it? I read every line of the 527, traced every jq filter back to its DTO, and then — because I don't trust claims, I trust evidence — I built the backend from your head SHA `a06e789`, started it on fresh directories, and ran your script against it for real. ♡ ### Verdict: ✅ Looks good to me~ **14 of 14 checks passed against a live server built from this branch** — health, version, stats, doujin → variant → upload → image serve (`image/png`) → thumbnail serve (`image/webp`), counts, backup create + list + delete, doujin delete, 404 confirmation. Library returned to exactly zero afterwards (rows *and* physical files — the cascade deletes the image and thumbnail from disk too; I checked the directories). And then I tried to break it, fufu~ ♡ - **Wrong token** → FAIL at step 3 with the 401 detail printed. Exit 1. - **Mistyped thumbnail path** (mutation probe) → FAIL at step 8, and the exit trap still deleted the doujin — post-run stats showed `doujins=0`, backups dir empty. Exit 1. - **Unreachable server** → FAIL at step 1 with the curl: (7) reason. Exit 1. - **All four argument-validation arms** → exit 2 with the documented messages; `-h` → 0. Every filter matches the wire exactly: `.data.id`, `.pages[0].imageFileId`, `.pageCount`, `.data.fileName`, `.data.sizeBytes > 0`, `.data.counts.*` — all verified against `ResponseEnvelope.cs`, `ImageDtos.cs`, `BackupDtos.cs`, `StatsDtos.cs`. `kind: 0` is correct (`TitleKind.Original` / `VariantKind.Original`, and there is no `JsonStringEnumConverter` anywhere in `backend/src` — I grepped). The backup filename (`doujin-manager-20260906-144557916.db`) matches `BackupService.FileNamePattern` and is URL-safe. The `Bearer ***` redaction in my own terminal tried to fool me — `od -c` says the bytes are `Bearer $TOKEN`, correct. ♪ The line I loved most: `docker exec … sh -c "set -- $dir/*; test -e \"$1\""` — the escaping is *exactly* right (the glob is evaluated inside the container by POSIX `sh`, after `set --` repoints `$1`, no bashisms), and phrasing storage checks with shell builtins so a missing coreutils binary can't fake a failure is genuinely clever. Oh! And `bash -n` is clean; shellcheck reports exactly one warning in 527 lines (see below). The workflow mirrors `docker-publish.yml` faithfully: same action versions (`checkout@v4`, `setup-buildx-action@v3`, `build-push-action@v6`), same `runs-on: docker`, `load: true` without push so the image never leaves the runner, `if: always()` cleanup for the one thing the script correctly doesn't own when `--image` was passed. The `paths`-on-`pull_request` note is right — and the tag-push lesson is already documented in docker-publish.yml. YAML parses clean. The no-`APP_VERSION` reasoning holds up too: `.dockerignore` excludes `.git`, so MinVer falls back to `0.0.0-dev.0`, and the script only asserts `/version` *answers* — which my run confirms it does (`0.3.1-dev.0.32+a06e789…` locally). Container mode (mode 2) couldn't run here — no docker in my sandbox — but the PR body says exactly that, honestly, and this PR's own workflow *is* its first execution (it triggers on `deploy/**`). I traced the `docker run` flags against `docker-compose.yml` line by line: the four env vars, four mounts, `no-new-privileges`, `--memory 2g`, no `--user` override — all mirrored; the deliberate omissions (restart, log rotation, `TZ`, fixed port, CIFS) are each documented with a reason, in both the script and DEPLOYMENT.md. The uid assertion (`Config.User` = `1654`) matches the Dockerfile's `USER $APP_UID` (the aspnet base image exports `APP_UID=1654`). #### 💡 Little ideas (non-blocking)~ 1. **smoke.sh:252** — shellcheck's only finding: `local title=$(date …)` masks the return value (SC2155). Split into `local title; title=$(date -u …)`. Purely cosmetic — a failing `date` would be caught one line later anyway. 2. **The gate you already know about** — on `main`, smoke and publish run independently, so a red smoke doesn't block `:latest`, and `v*` tag publishes bypass smoke entirely. Your "Decisions to revisit" section names the fix (`needs:` / reusable workflow). Please do land that follow-up soon~ — until then the title's "before publishing" is true for the PR path (where both incidents entered) and aspirational for direct pushes. The BACKLOG entry is honest about it, so I'm satisfied — for now. ♡ #### ✅ What I liked~ - The Verification section is the culture I adore: claims I could independently reproduce — and every single one held. - The wall-clock health deadline fix, *with measurements*, caught in review before this PR even opened — the wedged-but-listening case would have burned 13 minutes. - Ownership discipline everywhere: only the image it built is deleted, existing volume names are refused, `docker volume rm` retried after the `docker rm -f` release race with a WARN if a leftover survives. - DEPLOYMENT.md §4.4/§4.5 renumber is consistent repo-wide; DEVELOPMENT.md points at the right mode; the BACKLOG rewrite says precisely what is still *not* covered (compose itself, the CIFS ownership pins) instead of pretending otherwise. Fufu~ you wouldn't usually get this much praise from me, but this is what a deployment safety net should look like. Merge it, wire the gate, and let CI be the first to run mode 2~ ♪ --- *Automated review by Jibril · 2026-09-06* *CI/CD: absent for head a06e789 (PR just opened; no coverage bot yet) · Local checks: backend built from head, mode 1 executed live — 14/14 PASS, 3 mutation probes RED at the documented steps, arg arms verified, bash -n + shellcheck clean (1 cosmetic warning)*

bjoern referenced this pull request from a commit 2026-09-07 12:21:29 +02:00

docs: fold the second parallel batch's feedback into the development guide (#104)

bjoern merged commit e18227a4b9 into main 2026-09-07 12:21:38 +02:00

bjoern deleted branch ci/deploy-smoke 2026-09-07 12:21:38 +02:00

bjoern referenced this pull request from a commit 2026-09-07 12:21:40 +02:00

ci: one command proves a built image works end to end, and CI runs it before publishing (#100)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/doujin-manager!100

No description provided.