Skip to content

Commit 45a74fa

Browse files
committed
Merge #857: feat!: Container infrastructure refactor (ADR-T-009)
0b28312 chore(packages): tighten helper-crate hygiene across three small spots (Peer Cat) 0036fc0 test(packages): broaden coverage of helper-crate public surfaces (ADR-T-009) (Peer Cat) 362996c fix(container): make sqlite e2e runner work under rootless podman (Peer Cat) b79e32d docs(adr-009): consolidate ADR & retire implementation plan (Peer Cat) 57b76e0 docs(container): close out ADR-T-009 with Phase 9 audit & docs (ADR-T-009 Phase 9) (Peer Cat) d8b6ad2 refactor(container): split compose for prod vs dev (ADR-T-009 Phase 8) (Peer Cat) a2a8527 refactor(container): wire entry-script contract (ADR-T-009 Phase 7) (Peer Cat) 1f473df feat(config-probe): ship JSON-shaped config-resolution helper (ADR-T-009 Phase 6) (Peer Cat) 8b4bfeb refactor(config): strip credentials from shipped TOMLs (ADR-T-009 Phase 5) (Peer Cat) 3818c20 refactor(container): split runtime base (ADR-T-009 Phase 4) (Peer Cat) d0d8e7e refactor(config): extract index-config crate (ADR-T-009 Phase 3) (Peer Cat) 457b6f0 refactor(container): extract helper binaries (ADR-T-009 Phase 2) (Peer Cat) 8308247 chore(container): Build Hygiene (ADR-T-009 D6, D9 build-context part) (Peer Cat) 97e58e9 docs: replace ADR-T-009 with structural container refactor (Peer Cat) 01e7c70 chore(container): infrastructure hardening (ADR-T-009 Phases 1 & 2) (Peer Cat) Pull request description: ## Summary Lands ADR-T-009 end to end — Phases 1–9 — turning the container surface from "a single multi-stage `Dockerfile` plus a free-form entry script" into a structurally split, audit-tracked, CI-guarded baseline. Fourteen commits on top of the develop HEAD, every commit references back to a numbered decision (D1–D9) in the consolidated ADR. > ⚠ **Breaking for operators.** Zero-config `docker run` is intentionally rejected: `tracker.token` and `database.connect_url` are now mandatory schema fields with no shipped defaults. The bare-`docker run` snippets in the README and containers.md have been updated to show the two required overrides. ## Why The pre-refactor container surface had grown nine independent pain points (R1–R10 in the ADR appendix), among them: - The Containerfile baked operator-runtime concerns (`API_PORT`, `IMPORTER_API_PORT`) as build-time `ARG`s. - The healthcheck and keypair helper binaries lived inside the application crate, dragging `reqwest`, `tokio`, `hyper`, `rustls`, and `native-tls` into anything that linked them. - The configuration schema sat in config, so any consumer that only wanted to read a TOML file pulled the entire HTTP/TLS dep closure. - compose.yaml shipped credentials as defaults, mixed dev-only conveniences (`mailcatcher`, `tty: true`) into a single file, and bound dev ports on every host interface. - The entry script silently `set -x`'d (leaking creds), had a buggy `USER_ID` short-circuit, and re-parsed TOML in shell to dispatch on the database driver. - The vendored `su-exec` had no audit record, no provenance trail, and no drift detection. This PR lands a structural fix for each. ## Highlights - **Two parallel runtime bases.** The Containerfile produces a lean `runtime_release` on `gcr.io/distroless/cc-debian13` with a single root-only `/bin/busybox` (`0700 root:root`) and a curated applet set, plus a `runtime_debug` on `:debug` for operator escape-hatches. Both share a `runtime_assets` collector and a `preflight_gate` that aggregates donor-validation stages so future base reshuffles fail at build time. (D4) - **Three new helper workspace crates**, each with their own dep closure free of HTTP/TLS: - `packages/index-cli-common` — shared P9 scaffolding (`refuse_if_stdout_is_tty`, `init_json_tracing`, `emit`, `BaseArgs`). - `packages/index-health-check` — `torrust-index-health-check` (renamed from `health_check`), rewritten on `std::net::TcpStream` with Happy Eyeballs. - `packages/index-auth-keypair` — `torrust-index-auth-keypair` (renamed from `torrust-generate-auth-keypair`), now emits `{"private_key_pem": "...", "public_key_pem": "..."}` instead of concatenated PEM blocks. (D5) - **Configuration schema extracted.** `packages/index-config` now owns the v2 schema, validator, loader, and permission value types. `cargo tree -p torrust-index-config -e normal` confirms `tokio`, `reqwest`, `sqlx`, `hyper`, `rustls`, `native-tls`, and `openssl` are all absent. The root crate keeps a thin `pub use torrust_index_config::*;` shim so every existing call site compiles unchanged. - **A typed config probe.** `packages/index-config-probe` — `torrust-index-config-probe` — emits a single line of JSON with `database.driver`, `database.path`, and `auth.{private,public}_key.{pem_set,path_set,source,path}`, giving the entry script a stable typed interface to dispatch on instead of re-parsing TOML in shell. (D3, P9) - **Entry script rewritten around a sourced library.** entry_script_sh runs under `set -eu`, sources pure helpers from entry_script_lib_sh, enforces the auth-key invariants (PEM/PATH mutual exclusion within a key, both-or-neither across the pair, single mechanism across the pair), and dispatches database seeding from the probe's typed `database.driver` field. A new test-only crate `packages/index-entry-script` drives the shell library from `cargo test` via `sh` subprocess. (D3) - **Refuse-if-root guard.** The previous `USER_ID >= 1000` rule rejected legitimate rootless-Podman / low-UID-CI / BSD configurations without stating its intent. The property the script actually enforces — *do not run as root* — is now the literal check: "non-negative integer, not 0". (D7, **breaking** for anyone relying on the old rule.) - **Compose split.** compose.yaml is now a production-shaped baseline (no `mailcatcher`, no `tty`, ports bound to `127.0.0.1` except `:3001`, every credential as a bare `${VAR}` with no default); compose.override.yaml re-attaches the dev sidecars; a top-level Makefile wraps both flows as `make up-dev` / `make up-prod`. The prod recipe runs a `_validate-prod-env` POSIX-`sh` validator first as defence in depth. (D1) - **Vendored su-exec audited.** New AUDIT.md records provenance (upstream `ncopa/su-exec`, MIT, vendored 2023-10-14), the choice rationale against `gosu`/`setpriv`/`su`/`runuser`, the file-change and CVE re-audit triggers (no calendar trigger, per D8), and an append-only `## Audit Log`. (D8) - **CI guards** (container.yaml) — three independent audits gate the `test` matrix: - `compose-baseline-no-mailcatcher` — re-introducing the dev mail sidecar into the production-shaped baseline fails the build. - `su-exec-audit` — drift between the file's SHA-256 and the latest `## Audit Log` entry fails the build. - `entry-env-docs` — every var listed in the entry script's `# ENTRY_ENV_VARS:` manifest must appear in containers.md. (D9) - **Build hygiene.** Build-time `ARG API_PORT` / `ARG IMPORTER_API_PORT` are gone (runtime `ENV` defaults remain). adr and docs are excluded from the build context. Helper binaries are tightened to `0500 root:root`; the application binary keeps `0755`. (D6, D9) ## Breaking changes | Surface | Before | After | |---|---|---| | Schema field name | `[net.tsl]` (typo) | `[net.tls]` — wire key, type, and JSON API response all renamed in lockstep. No alias. | | `tracker.token` | Defaulted to a placeholder | **Required** — no default, no shipped value. Set via `TORRUST_INDEX_CONFIG_OVERRIDE_TRACKER__TOKEN`. | | `database.connect_url` | Defaulted to an SQLite path | **Required** — no default. Set via `TORRUST_INDEX_CONFIG_OVERRIDE_DATABASE__CONNECT_URL`. | | `TORRUST_INDEX_DATABASE_DRIVER` | Runtime database dispatcher | First-boot TOML selector only — chooses which default `index.toml` is installed at `/etc/torrust/index/index.toml`. Runtime decisions come from `connect_url`. | | Auth keys | Mixed PEM/PATH per key was tolerated | PEM and PATH mutually exclusive per key; both keys configured or neither; single mechanism across the pair. (D3) | | `USER_ID` guard | `>= 1000` | "non-negative integer, not 0" | | Helper binary names | `health_check`, `torrust-generate-auth-keypair` | `torrust-index-health-check`, `torrust-index-auth-keypair` | | `torrust-index-auth-keypair` output | Concatenated PEM blocks (consumer scanned for markers) | Single JSON object, consumed via `jq -r .private_key_pem` | | Helper TTY-refusal exit code | `1` for the keypair helper, `2` for healthcheck | Unified on `2` via the shared `refuse_if_stdout_is_tty`. | ## Operator migration The `# Container` quickstart blocks in README.md and containers.md now show the two mandatory overrides. The minimum invocation is: ```bash docker run \ -e TORRUST_INDEX_CONFIG_OVERRIDE_TRACKER__TOKEN=… \ -e TORRUST_INDEX_CONFIG_OVERRIDE_DATABASE__CONNECT_URL=sqlite:///var/lib/torrust/index/database/index.sqlite3.db?mode=rwc \ -e USER_ID=$(id -u) \ … ``` For Podman: `--format docker` is required so the `HEALTHCHECK` directive survives the build (OCI manifests have no field for it). The e2e harness handles this automatically via `BUILDAH_FORMAT=docker`. ## Phase map | Phase | Commit | Theme | |---|---|---| | 1 & 2 (initial hardening) | `01e7c70` | Pre-ADR cleanups carried in for the dev-loop refactor. | | ADR rewrite | `97e58e9` | Replaces the tactical hardening ADR with the structural one. | | Build hygiene | `8308247` | D6, D9 build-context part. | | Phase 2 (helper extraction) | `457b6f0` | New crates, jq donor stage. | | Phase 3 (config crate) | `d0d8e7e` | `index-config` extracted; `Tsl` → `Tls`. | | Phase 4 (runtime base split) | `3818c20` | `runtime_release` / `runtime_debug`; D7 `USER_ID` rule. | | Phase 5 (strip credentials) | `8b4bfeb` | `tracker.token` / `database.connect_url` mandatory. | | Phase 6 (config probe) | `1f473df` | `index-config-probe`, `Info::from_env`. | | Phase 7 (entry script) | `a2a8527` | `set -eu`, sourced library, `index-entry-script`. | | Phase 8 (compose split) | `d8b6ad2` | compose.yaml / compose.override.yaml, Makefile, §8.6 latent fixes. | | Phase 9 (audit + docs + CI) | `57b76e0` | Three CI guards, su-exec `AUDIT.md`, env-var manifest. | | ADR consolidation | `b79e32d` | `009-implementation-plan.md` retired; refs re-anchored on §D-series. | | Rootless-podman fixes | `362996c` | E2e harness portability for `podman 5.8.2` / `podman-compose 1.5.0`. | | Helper-crate test sweep | `0036fc0` | Coverage backfill on `pub` / `pub(crate)` items in the new crates. | ## Tests A few hundred new tests across the four new crates plus the entry-script harness; the workspace suite (`cargo test --workspace --all-targets --all-features`) passes in both `dev` and `release` profiles, with and without default features. Doc-tests and `cargo doc` both build. ## References - 009-container-infrastructure-refactor.md — consolidated ADR, status: *Implemented (Phases 1–9 complete)*. - containers.md — operator guide. - CHANGELOG.md — release-shaped narrative under `[Unreleased]`. ACKs for top commit: da2ce7: ACK 0b28312 Tree-SHA512: 5c22d33c62ec8ef26e4c7ad4d4ff561edf7750b6549d359bf934b6bc0fe2902f010edeb2eed1a297a24ad3b09fa8e502b8ddc4ab438827385a7d67ac65973602
2 parents a401c0c + 0b28312 commit 45a74fa

110 files changed

Lines changed: 7818 additions & 1283 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.containerignore

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/.env
2+
/.env.local
3+
/.git
4+
/.git-blame-ignore
5+
/.github
6+
/.gitignore
7+
/.vscode
8+
/adr/
9+
/bin/
10+
/config-idx-back.local.toml
11+
/config-tracker.local.toml
12+
/config.local.toml
13+
/config.toml
14+
/contrib/dev-tools/container/
15+
/cspell.json
16+
/data_v2.db*
17+
/data.db
18+
/data.db*
19+
/docs/
20+
/project-words.txt
21+
/README.md
22+
/rustfmt.toml
23+
/storage/
24+
/target/

.dockerignore

Lines changed: 0 additions & 22 deletions
This file was deleted.

.dockerignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.containerignore

.github/workflows/container.yaml

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,90 @@ env:
1515
CARGO_TERM_COLOR: always
1616

1717
jobs:
18+
lints:
19+
name: Lints (Container infra)
20+
runs-on: ubuntu-latest
21+
22+
steps:
23+
- id: checkout
24+
name: Checkout Repository
25+
uses: actions/checkout@v6
26+
27+
# Phase 9 §9.1.3 — guard against re-introducing the
28+
# `mailcatcher` dev sidecar (or any SMTP/mail config)
29+
# into the production-shaped baseline. The override
30+
# file is *expected* to mention `mailcatcher` and is
31+
# deliberately excluded from the audit. Comments are
32+
# stripped before grepping so the explanatory header
33+
# in `compose.yaml` (which legitimately references
34+
# `mailcatcher` in prose) does not trip the audit;
35+
# we are looking for live YAML config, not docs.
36+
- id: compose-baseline-no-mailcatcher
37+
name: compose.yaml has no mailcatcher / SMTP wiring
38+
run: |
39+
set -eu
40+
# awk strips `# ...` comments while preserving line
41+
# numbering 1:1 with the source file, so any error
42+
# output points the reader at the real line.
43+
if awk '{ sub(/#.*/, ""); print }' compose.yaml \
44+
| grep -nE 'mailcatcher|MAILER|SMTP|smtp_'; then
45+
echo "::error file=compose.yaml::dev mail sidecar / SMTP config present in production-shaped baseline (ADR-T-009 §D1 / §8.1)"
46+
exit 1
47+
fi
48+
echo "compose.yaml clean."
49+
50+
# Phase 9 / ADR-T-009 §D8 — vendored `su-exec.c` must not change
51+
# without a fresh audit entry recording the new SHA-256
52+
# in contrib/dev-tools/su-exec/AUDIT.md.
53+
- id: su-exec-audit
54+
name: su-exec audit log matches vendored source
55+
run: |
56+
set -eu
57+
audit=contrib/dev-tools/su-exec/AUDIT.md
58+
test -s "$audit"
59+
recorded=$(sed -n '/^## Audit Log/,$ { s/^SHA-256: \([0-9a-f]\{64\}\)$/\1/p; }' "$audit" | tail -1)
60+
actual=$(sha256sum contrib/dev-tools/su-exec/su-exec.c | cut -d' ' -f1)
61+
if [ -z "$recorded" ]; then
62+
echo "::error file=$audit::no SHA-256 entry found in '## Audit Log' section (ADR-T-009 §D8)"
63+
exit 1
64+
fi
65+
if [ "$recorded" != "$actual" ]; then
66+
echo "::error file=$audit::recorded SHA-256 ($recorded) does not match contrib/dev-tools/su-exec/su-exec.c ($actual). Append a new dated audit entry per ADR-T-009 §D8."
67+
exit 1
68+
fi
69+
echo "su-exec audit current ($actual)."
70+
71+
# Phase 9 / ADR-T-009 Acceptance Criterion #7 — every env
72+
# var listed in the entry script's manifest block must be
73+
# documented in docs/containers.md.
74+
- id: entry-env-docs
75+
name: entry-script env vars documented
76+
run: |
77+
set -eu
78+
script=share/container/entry_script_sh
79+
vars=$(sed -n '/^# ENTRY_ENV_VARS:/,/^# END_ENTRY_ENV_VARS/p' "$script" \
80+
| grep -oE '[A-Z][A-Z0-9_]+' \
81+
| sort -u)
82+
if [ -z "$vars" ]; then
83+
echo "::error file=$script::ENTRY_ENV_VARS manifest block not found or empty (ADR-T-009 Acceptance Criterion #7)"
84+
exit 1
85+
fi
86+
missing=0
87+
for v in $vars; do
88+
grep -q "$v" docs/containers.md || {
89+
echo "::error file=docs/containers.md::env var '$v' is in the entry-script manifest but not documented"
90+
missing=1
91+
}
92+
done
93+
grep -q 'compose\.override\.yaml' docs/containers.md || {
94+
echo "::error file=docs/containers.md::two-file Compose split (compose.override.yaml) is not documented"
95+
missing=1
96+
}
97+
[ "$missing" -eq 0 ]
98+
1899
test:
19100
name: Test (Docker)
101+
needs: lints
20102
runs-on: ubuntu-latest
21103

22104
strategy:

AGENTS.md

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ When working inside a package, prefer running only the `--package` tests,
1717
as the whole-project tests are slow to run. (Occasionally run the whole
1818
suite, for example when finishing up.)
1919

20+
## Commit Messages
21+
22+
When writing a commit message, be sure to review the last few commit messages to compare the style.
23+
2024
## Running Tests
2125

2226
When running tests, tee to a temp file (`/tmp/...`) and then grep that
@@ -44,6 +48,23 @@ API, perhaps using `#[doc(hidden)]` helpers when appropriate.
4448

4549
Every test file (module) should maintain an index of the tests contained in the module-doc. The primary purpose is to make it easy to scan the test files to detect duplicates or overlapping coverage. Please opportunistically create if missing.
4650

51+
## POSIX Paths
52+
53+
Treat paths as opaque byte sequences. POSIX permits any byte except
54+
`\0` (NUL) and `/` (the path separator) in a file or directory name,
55+
and there is no guarantee that the bytes are valid UTF-8. Concretely:
56+
57+
- Prefer `OsStr` / `OsString` / `Path` / `PathBuf` (or `Utf8Path`
58+
when UTF-8 really is a precondition you intend to enforce) over
59+
ad-hoc `String` handling.
60+
- Do not assume any particular character class — names may contain
61+
spaces, newlines, control bytes, leading dashes, or arbitrary
62+
non-UTF-8 bytes.
63+
- NUL termination is only required when crossing a libc/FFI
64+
boundary (e.g. `CString` for `open(2)`); interior NUL bytes are
65+
invalid for those APIs and must be rejected, not silently
66+
truncated.
67+
4768
## Cross-Reference Conventions
4869

4970
Eagerly corrected when spotted in **any** file!
@@ -59,6 +80,13 @@ use their own `ADR-<PREFIX>-<NNN>` form without the `§` prefix.
5980
| `M-` | Mudlark | `packages/mudlark/docs/idea.md` |
6081
| `R-` | render-text-as-image | `packages/render-text-as-image/` |
6182

83+
Helper crates (`index-health-check`, `index-auth-keypair`,
84+
`index-config`, `index-config-probe`, `index-cli-common`,
85+
`index-entry-script`) are internal implementation details of
86+
the root crate and do not own separate ADRs or specification
87+
docs. They share the `T-` prefix for any cross-references
88+
that target them.
89+
6290
### General Rules
6391

6492
- Use `§§` for ranges: e.g. `§§IDEA M-12.2–12.5`.
@@ -77,5 +105,6 @@ use their own `ADR-<PREFIX>-<NNN>` form without the `§` prefix.
77105
To avoid partial or corrupted writes, always replace files atomically:
78106

79107
1. Read the file.
80-
2. Using the CLI, `rm` the file.
81-
3. Recreate the file.
108+
2. Write the new content to a temporary file
109+
3. Rename the temporary file to atomically overwrite the original file:
110+
`mv file.tmp file`

0 commit comments

Comments
 (0)