build-docker-image-and-host-on-this-forgejo-instance-370 #383

Merged

johnsturgeon merged 2 commits from build-docker-image-and-host-on-this-forgejo-instance-370 into main

2026-08-14 20:44:46 +02:00

johnsturgeon commented

2026-08-14 20:33:32 +02:00

Owner

No description provided.

johnsturgeon added 2 commits

2026-08-14 20:33:32 +02:00

Build the image in CI and deploy it from the registry (#370 ) 1657d59f00

Production built its own image on the prod host from a git checkout, then
bind-mounted that checkout over the image's /srv/app and /srv/alembic --
so the image was nearly decorative and `git pull` could swap migration
scripts out from under a live container.

Now a merge to main builds once on the runner and pushes to the Forgejo
registry, and production pulls a tag. Deployment is compose plus one
environment variable.

- Add .forgejo/workflows/build-image.yml, pushing :latest, the
  pyproject.toml version, and the short SHA. Single-arch by necessity:
  the runner is an unprivileged LXC and cannot register binfmt handlers.
- Rewrite compose.prod.yml as a deployment descriptor: no build context,
  no bind mounts, no env_file. Pulls over forgejo.sturgeon.me, which
  serves /v2/ correctly and needs no credentials.
- Bake the 1Password CLI and config/op.env into the image. The entrypoint
  re-execs under `op run` before the DB wait, so no resolved .env exists
  on the host, in the image, or in `docker inspect`. `op run` rather than
  `op inject` because sourcing injected output runs secret values through
  shell parsing, mangling anything containing $, a backtick or a quote.
- Fix .dockerignore patterns that silently missed their targets: bare
  `.env*` and `node_modules/` only match the context root, so
  config/.env.production was being shipped to the build daemon.
- Delete the pre-Docker deploy cluster (deploy.sh, update.sh,
  create_prod_env.sh, tgfp-web.service) and bump_version.sh, which had
  eroded into a wrapper around `uv version` that committed the whole
  index.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Bump version to 2.7.0

Claude Code Review / claude-review (pull_request) Successful in 1m38s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 19s

Details

5a56adb538

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

forgejo-actions commented

2026-08-14 20:35:10 +02:00

I've reviewed the full change. This is an infrastructure PR (image build + Forgejo registry hosting); no application logic changed. Everything checks out — the Dockerfile layering is sound, the entrypoint's op run re-exec and recursion guard are correct, the excluded node_modules is safe because Tailwind is precompiled to app/static/css/app.css, /ping exists for the healthcheck, and no dangling references to the deleted scripts remain.

This PR cleanly moves the image build off the production host into a Forgejo Actions workflow and makes the running image the single source of code, secrets template, and migrations. The design is well-reasoned and unusually well-documented; I found no correctness bugs and only a few minor points worth considering.

Worth fixing

Dockerfile:32-35 — the op binary is downloaded with no integrity check. The version is pinned (good, reproducible), but the zip is fetched over the network and installed without verifying a checksum or signature. Since this binary later handles every production secret, a compromised/MITM'd cache.agilebits.com response would be baked straight into the image. 1Password publishes SHA256 sums alongside the release; piping through a sha256sum -c step would close the gap for the cost of one line.

Nits

.forgejo/workflows/build-image.yml:88-92 — cancelled push can transiently leave :latest stale, but self-heals. With cancel-in-progress: true, a build cancelled mid-push (order is sha, version, latest) could stop after pushing :sha but before :latest. The next merge's build corrects it, so the concurrency reasoning in the comment holds; just noting the window exists.
README.md:100 — the clone URL now hardcodes the internal IP ssh://git@192.168.192.129/.... Fine for a homelab, but if the LXC moves this drifts, same as the address note already called out for compose.prod.yml. A hostname would age better.
docker/entrypoint.sh:55-66 — the DB wait reads os.environ["DATABASE_URL"] unguarded. If op resolution silently produces an empty value (or the ambient-env path is taken without it set), this raises a bare KeyError rather than the clearer "DB not reachable" / missing-secret message. Not wrong — it fails loudly — but a targeted error would save a debugging step during a rotation mishap.

No test coverage concerns: the diff is config/CI/docs only, with no Python behavior to cover.

I've reviewed the full change. This is an infrastructure PR (image build + Forgejo registry hosting); no application logic changed. Everything checks out — the Dockerfile layering is sound, the entrypoint's `op run` re-exec and recursion guard are correct, the excluded `node_modules` is safe because Tailwind is precompiled to `app/static/css/app.css`, `/ping` exists for the healthcheck, and no dangling references to the deleted scripts remain. --- This PR cleanly moves the image build off the production host into a Forgejo Actions workflow and makes the running image the single source of code, secrets template, and migrations. The design is well-reasoned and unusually well-documented; I found no correctness bugs and only a few minor points worth considering. ### Worth fixing - **`Dockerfile:32-35` — the `op` binary is downloaded with no integrity check.** The version is pinned (good, reproducible), but the zip is fetched over the network and installed without verifying a checksum or signature. Since this binary later handles every production secret, a compromised/MITM'd `cache.agilebits.com` response would be baked straight into the image. 1Password publishes SHA256 sums alongside the release; piping through a `sha256sum -c` step would close the gap for the cost of one line. ### Nits - **`.forgejo/workflows/build-image.yml:88-92` — cancelled push can transiently leave `:latest` stale, but self-heals.** With `cancel-in-progress: true`, a build cancelled mid-push (order is `sha`, `version`, `latest`) could stop after pushing `:sha` but before `:latest`. The next merge's build corrects it, so the concurrency reasoning in the comment holds; just noting the window exists. - **`README.md:100` — the clone URL now hardcodes the internal IP `ssh://git@192.168.192.129/...`.** Fine for a homelab, but if the LXC moves this drifts, same as the address note already called out for `compose.prod.yml`. A hostname would age better. - **`docker/entrypoint.sh:55-66` — the DB wait reads `os.environ["DATABASE_URL"]` unguarded.** If `op` resolution silently produces an empty value (or the ambient-env path is taken without it set), this raises a bare `KeyError` rather than the clearer "DB not reachable" / missing-secret message. Not wrong — it fails loudly — but a targeted error would save a debugging step during a rotation mishap. No test coverage concerns: the diff is config/CI/docs only, with no Python behavior to cover.

johnsturgeon merged commit bb706426cb into main

2026-08-14 20:44:46 +02:00

johnsturgeon referenced this pull request from a commit

2026-08-14 20:44:47 +02:00

build-docker-image-and-host-on-this-forgejo-instance-370 (#383)

johnsturgeon deleted branch build-docker-image-and-host-on-this-forgejo-instance-370

2026-08-14 20:44:52 +02:00