Repository navigation
docs: close Phase 1 with the review points and the live test results - #20
Conversation
AbdikhaniAbdirashid
left a comment
AbdikhaniAbdirashid
left a comment
There was a problem hiding this comment.
Verified both checks rather than taking them on trust.
The GHCR tag list works as written and returns the four images, newest 9e9a7a76… — which matches what /version serves live, so the list isn't stale. Switching from a guessed tag to tags/list also removes the failure mode where a correct public package looked private just because the tag you tried didn't exist.
nc -G on Ubuntu:
$ nc -z -G 3 127.0.0.1 1
nc: invalid option -- 'G'
OpenBSD netcat (Debian patchlevel 1.226-1ubuntu2)
So it exits on the usage error before it ever opens a socket. The "both directions" rule is the real fix here, not the flag table: a check that only expects a failure can't tell a blocked port from a command that never ran. Same shape as the CEL bug in #21 — something that passed for the wrong reason, and only a positive control would have caught it. Worth us both being able to say that out loud.
Confirmed Phase 1 is closed the way this claims: https://api.myrelay.tech/version serves 9e9a7a7, exactly origin/main's HEAD, and /health/ready answers 200.
The two-replicas wording is the right correction. Zero-downtime rollouts and protection against one crashing process, not high availability — one node is one failure domain, and claiming otherwise would not survive a follow-up question.
One note on the reboot check, non-blocking. Comparing /version to git rev-parse origin/main is exact today, but only because pipeline.yml has no path filter on push. I went looking for a hole here and found the opposite: GHCR has four images for nine merges on main, which looked wrong until I traced it — the three without images (#15, #14, #19) merged while the old api.yml still had paths: ["api/**", ".github/workflows/api.yml"], and #16 removed that. So the gap is history, not a live bug.
Worth a half-sentence in the runbook anyway, something like "valid because every merge to main is built and deployed", so that if anyone ever re-adds a path filter the check doesn't silently start reporting a failed deploy after every docs merge.
Approving.
Rebased onto `main` after #20 and #21. ## What Steps 2.1–2.3 of Phase 2: Postgres 18.6 on a disk that survives deletion. Nothing here is applied by the pipeline: the `deployer` account can't create StorageClasses, Secrets or StatefulSets, on purpose. | Step | File | What | |---|---|---| | 2.1 | `k8s/platform/storageclass-retain.yaml` | A second StorageClass, `local-path-retain`: like k3s's `local-path`, but `reclaimPolicy: Retain`. Deleting the claim (or the namespace) keeps the data folder instead of `rm -rf`-ing it | | 2.2 | `infra/server/bootstrap-secrets.sh` | Creates the Secrets `postgres-auth` (superuser password) and `relay-db` (the app's password + its `DATABASE_URL`) on the server, **only if missing**, from `openssl rand -hex 32`. Prints only `created`/`kept`; values never on a command line, in output or in a file | | 2.3 | `k8s/data/postgres.yaml` | Headless Service `postgres` (ClusterIP only) + StatefulSet: `postgres:18.6` pinned by digest, uid 999, Pod Security `restricted`, read-only root, `max_connections=100`, readiness `pg_isready` over TCP, no liveness probe, 60 s grace, a 10Gi claim on `local-path-retain` | | 2.3 | `k8s/data/initdb/10-create-relay.sh` | Runs once, on an empty volume: creates the login `relay` (not a superuser) and the database `relay` owned by it. Password via psql `\getenv` + `:'pw'`, and `log_min_error_statement = panic` so a failing statement can't write the password to Postgres's log | **Docs:** - `k8s/README.md`: a row for `k8s/data/` ("never the pipeline"), and the StorageClass in the platform row. Also your two limits from #21: the policy only sees Services (a Traefik `IngressRouteTCP` wouldn't be checked), and both guards cover only `relay`. - `infra/server/README.md`: section 6 becomes the real data section: - the apply order (StorageClass → secrets → ConfigMap → Postgres), and why a login check must come from another pod; - **6a** re-attaching a kept disk: scale to 0 first, then re-bind with `volumeName`; never remove finalizers; - **6b** a lost Secret with a kept disk: `ALTER ROLE` from the pod's own env with psql `\getenv`. Tested: a password with `'`, `;`, `$`, `"` works, the old one is refused, and there are 0 lines of it in the log; - **6c** a first start that failed halfway. - Your half-sentence from #20 in "Reboots": the `/version == origin/main` check is valid because every merge is built and deployed. ## Why these choices - **Retain, in our own class:** `local-path` can't be changed (`reclaimPolicy` is immutable, and k3s re-applies its own copy). Retain protects against `kubectl` mistakes, not against losing the server's disk; only backups do that (Phase 6). - **A StatefulSet:** a Deployment's rolling update would start a second Postgres on the same data folder. - **Applied by an admin:** a merged PR can't change the database's major version or its disk. - **The app is not a superuser:** `POSTGRES_USER=relay` would make it one, able to run shell commands on the database server (`COPY … TO PROGRAM`). - **No liveness probe:** it could kill Postgres in the middle of a crash recovery, in a loop. - **Readiness over TCP:** on the first start a temporary server answers only on the local socket. ## Tested before the PR Locally in Docker (the same digest, uid 999, read-only root, no capabilities): - a password containing `'`, `;`, `"`, `\`, `$` and an attempted `CREATE TABLE pwned()`: the login works with exactly that password, one character less is refused, and no `pwned` table exists; - a forced failing `CREATE ROLE`: the password appears **0** times in Postgres's log. Control without the `SET`: the full statement and password are logged. `bootstrap-secrets.sh` in a throwaway namespace: first run `created` ×2, second run `kept` ×2 with unchanged `resourceVersion`s, keys `password` / `password,url`, no 64-hex string in any output. A fake-kubectl test: when the check can't reach the server, it stops before any `create`. Postgres in a throwaway namespace `pg-test` with the **same Pod Security labels** (`restricted`, v1.36), filled by `bootstrap-secrets.sh`, then deleted: | Check | Result | |---|---| | Pod / PVC / Service | Ready, 0 restarts; Bound; ClusterIP `None`, no NodePort | | Pod Security | No warning on apply, 0 PodSecurity events | | Version, checksums, max_connections | 18.6, on, 100 | | Login from **another pod** with the Secret's `url` | `logged in as relay in relay` | | Wrong password from another pod | `password authentication failed` | | `relay`'s flags (superuser, createrole, createdb, replication) | all false | | `COPY … TO PROGRAM` as `relay` | `permission denied` | | Delete the pod | "received fast shutdown request" (the image's SIGINT reaches Postgres), shut down in 26 ms, back with the same data | Found while testing: the image's `pg_hba.conf` trusts the local socket and `127.0.0.1` **without a password**; only connections from other pods need one. So login checks run from a separate pod. Only admins can `kubectl exec` into `postgres-0` (the deployer has no exec). ## After merging (an admin, in this order, output posted here) 1. `k8s/platform/storageclass-retain.yaml` → `kubectl get sc` shows `local-path-retain … Retain`, `local-path` still the default. 2. `ssh relay 'bash -s' < infra/server/bootstrap-secrets.sh` → `created` ×2. 3. The ConfigMap `postgres-init` from `10-create-relay.sh` (the command is in the header of `postgres.yaml`), then `postgres.yaml`. 4. The checks above, now in `relay`, plus from outside: `nc -z -G 3 81.88.23.71 443` succeeds and `nc -z -G 3 81.88.23.71 5432` fails. 5. **The deletion drill, before any real data:** a marker row survives deleting the pod, the StatefulSet, and the PVC (Postgres scaled to 0 first, then the kept PV re-attached with `volumeName`). ## For the reviewer - Anything that should be applied by the pipeline instead of by hand, or the other way round? - Is "trusted inside the pod, password from outside" acceptable to you? - The 10Gi is only a label: local-path doesn't enforce sizes. Disk usage is watched in Phase 7.
Closes Phase 1 in the docs: Abdikhani's review points from #17 and #18, plus what the live tests showed.
From the reviews
nc -Gis macOS-only: on Ubuntu's netcat-openbsd it's a usage error, soncexits 1 before connecting, and a "must fail" check passes even when the port is open. The runbook and CONTEXT now say: 443 must answer AND 30795 must not. On macOS use-G, on Linux-w./versionshowsorigin/main's commit, so a reboot can't quietly bring back an old image (infra: capture the server's base setup, a rebuild runbook and the firewall fix #17).api-testanddeploy-script-testhave been on since 29 Oct, and the runbook explains whatrequire_last_push_approvalreally does.DENIED. Tested as written: it lists the 4 images built so far.Phase 1 is done
The live tests (29 Oct)
absent traffic (20 clients, keep-alive, 5 min). 76,466 requests, 0 failed. The rollout took 4 s, and the image pull about 1 s.relay-deployredeployed the last good commit. During the test, 119,824 requests, 0 failed, and all 1,787/versionchecks answered the live version. The old pods were never stopped.