9 Commits

Author SHA1 Message Date
Nguyen Minh Phuc 08a529fa63 Get off Docker Hub, add a catalog values passthrough, fix the dind prune
ci / lint (push) Successful in 24s
ci / types (push) Successful in 34s
ci / unit (push) Successful in 26s
ci / security (push) Successful in 37s
ci / dockerfile (push) Successful in 6s
ci / chart (push) Successful in 7s
ci / integration (push) Successful in 41s
ci / image (api) (push) Successful in 2m9s
ci / image (reconciler) (push) Successful in 2m1s
ci / image (worker) (push) Successful in 2m7s
ci / bump (push) Successful in 13s
Docker Hub rate-limits anonymous pulls per source IP and every node here
shares one NAT address, so a busy afternoon fails an unrelated build with
`toomanyrequests`. Nothing in this repo needs to be there.

Every base image now comes from mirror.gcr.io (python, alpine/helm,
postgres) or ghcr.io (uv, trivy). Verified digest-for-digest against
Docker Hub before switching, including the superseded postgres digest
this repo still pins, so every existing pin stays valid — same bytes,
different transport.

catalog.yaml: the three bitnami entries named `bitnamilegacy/<chart>`, a
repo alias nothing in the worker image configures, so they could never
resolve at provision time. All five entries are now `oci://` refs, which
need no `helm repo add`, and all are on latest stable:

  elasticsearch 21.3.15 -> 22.1.6     redis    20.6.2 -> 27.0.15
  postgresql    16.4.5  -> 18.8.0     podinfo  6.7.1  -> 6.14.0

Moving the chart pull is only half of it, though: a bitnami chart
defaults its own images to registry-1.docker.io. CatalogEntry gains a
`values:` dict, merged under the size's replicas and resources, so an
entry can set `global.imageRegistry` and move the image pull too. Size
wins on conflict — otherwise an entry setting replicaCount would make
every size deploy the same shape. Deep merge, because a shallow one
drops sibling keys of a shared nested map.

Bitnami charts reject a substituted registry unless
`global.security.allowInsecureImages` is set. That check is about
provenance, and the mirror serves byte-identical manifests, so it is set
deliberately and only for entries whose digests were verified.

The dind prune had `--filter until=168h` on both prunes, and it got both
cases exactly backwards. `until` reads an image's CREATED time, so it
deleted trivy every leg (a released tool image is always older than any
window) while protecting the dangling build layers it existed to remove.
Measured on node0: 21 dangling images / 5.96GB, and exactly 1 of them
older than 168h. Trivy is protected by a tag now, so the image prune
drops the filter; buildx keeps it, where age genuinely matters.

Tests: +10 unit (deep merge, precedence, no-mutation, and a guard that
fails if any catalog entry points at Docker Hub). Both new guards were
control-tested by breaking the code and watching them fail. The API test
that hardcoded `21.3.15` now reads the catalog — its subject is where
the value comes from, not what it is.
2026-07-21 15:32:12 +00:00
Nguyen Minh Phuc bb8b14ef03 api: make the OpenAPI spec usable as documentation
ci / lint (push) Failing after 56s
ci / types (push) Has been skipped
ci / unit (push) Has been skipped
ci / integration (push) Has been skipped
ci / security (push) Has been skipped
ci / dockerfile (push) Has been skipped
ci / chart (push) Has been skipped
ci / image (api) (push) Has been skipped
ci / image (reconciler) (push) Has been skipped
ci / image (worker) (push) Has been skipped
ci / bump (push) Has been skipped
FastAPI already served /docs, /redoc and /openapi.json, and the ingress already
passed them through — the mechanism was there, the content was not. The schema
alone cannot tell a caller the three things they most need to know, and the
route docstrings explain implementation reasoning to a maintainer rather than
usage to a consumer.

Added to the spec itself, so it travels with the API rather than living in a
README the caller does not have:

  - An app description covering bearer auth, that every write is 202 + poll,
    the instance lifecycle, and the uniform {"code", "message"} error body.
  - Tag descriptions for `instances` and `ops`.
  - Field descriptions and worked examples on CreateInstanceRequest,
    InstanceResponse and ErrorBody, so /docs shows a valid payload instead of
    leaving callers to infer one.

The bearer scheme was already exposed via HTTPBearer, which is what makes the
Authorize button in /docs work; there is now a test asserting it stays, along
with the description, the tags and the request example. Docs that are not
tested rot silently, and this is the artifact other teams integrate against.

README documents the three URLs and how to generate a client from the spec.
2026-07-21 05:33:44 +00:00
Nguyen Minh Phuc 66eb6cb0ee refactor: converge the patterns multiple authors left divergent
The codebase was written by several agents and had the same concept done more
than one way. This makes it read as one voice, with no behaviour change.

Dedup, each to a single canonical form:
  - INSTANCE_COLUMNS: the 13-column instances SELECT list existed as _COLUMNS in
    instances.py and reconcile.py (byte-identical) and inlined a third time in
    the worker. One exported constant now.
  - Settings.runtime_dsn: the three entrypoints each chose between str(pg_dsn)
    and pg_dsn.unicode_string(). One property.
  - yaml_tempfile: helm._ValuesFile and k8s._ManifestFile were the same
    write-yaml-to-a-temp-dir context manager. One helper in adapters/tempyaml.py.
  - services/_runtime.py: sleep_or_stop and install_stop_signals were copied
    between the worker and reconciler loops. One module, so shutdown behaviour
    cannot drift between them.
  - k8s.ensure_namespace used MANAGED_BY_LABEL/VALUE from helm.py instead of a
    hardcoded literal, so the managed-by label has one definition.
  - SvcforgeError is now the root of every svcforge exception (CatalogError,
    IllegalTransition, BadWindow, HandlerError), keeping each stdlib base in the
    MRO, so `except SvcforgeError` means what errors.py says it does.
  - ERROR_MAX_CHARS replaces the repeated `[-2000:]` truncation feeding the same
    error columns.
  - the reconciler reads settings.metrics_port like the worker, dropping its
    duplicate DEFAULT_METRICS_PORT and redundant --metrics-port option; the
    SVCFORGE_METRICS_PORT env override still applies through pydantic.

Two smaller correctness/consistency fixes:
  - RateLimitResult.retry_after_s computed its delta against datetime.now(UTC)
    while the limiter runs on an injectable clock, so it was meaningless under a
    FakeClock and drifted by request latency in production. It now carries a
    checked_at from the same clock as reset_at.
  - handle_provision's notifier.send is wrapped like the reconciler's: a flaky
    webhook after the READY CAS would fail the task, and the retry would hit the
    READY early-return and drop the notification, turning a good provision into a
    failed one.
2026-07-21 01:46:54 +00:00
Nguyen Minh Phuc d64c3c9f39 fix: three correctness bugs found in review + a dropped-log
fail() blanket-marked the instance `failed` for every dead-lettered task kind.
Only provision is correct. The others each left the instance in a state the
UPDATE then corrupted:
  - deprovision: `deleting` -> `failed` stranded the instance, because
    due_for_deprovision only re-selects ready/deleting, leaking the release
    reconcile.py promises to reclaim.
  - upgrade: helm --atomic rolled back, so the instance was still `ready` and
    serving the old version; `failed` mislabelled a healthy service and dropped
    it off the upgrade work-list.
  - verify: handle_verify already halted the rollout; the instance was `ready`.
Now gated on kind == provision, with a regression test per kind (control-tested
against the blanket UPDATE, which fails all three).

The API exception handler was registered on fastapi.HTTPException, a subclass
of starlette's. Starlette matches handlers by walking type(exc).__mro__, so
framework-raised 404/405 never hit it and returned {"detail": ...} instead of
ErrorBody. Registered on the starlette parent, and added a
RequestValidationError handler so body-validation 422s share the shape too.
Tests assert the ErrorBody shape for framework 404, 405, and a forbidden field.

helm._list_releases_via_api built the httpx client with verify=<ca path>, which
loads the CA eagerly and raises OSError on a half-mounted ServiceAccount — an
error the except clause did not catch, crashing the reconciler tick as a bare
bug. Now the CA is checked for readability alongside the token, and a missing
one means "not in-cluster" and falls back to helm. Also documents the no-limit
pagination invariant and pins it with a test.

redis.py logged through stdlib logging with extra={"team": team}, which the
structlog bridge drops on the floor — the trap notify.py documents. Switched to
a bound logger with team as a kwarg.
2026-07-21 01:46:54 +00:00
Nguyen Minh Phuc c537073c21 deps: everything to latest stable
ci / dockerfile (push) Has been cancelled
ci / types (push) Has been cancelled
ci / lint (push) Has been cancelled
ci / security (push) Has been cancelled
ci / chart (push) Has been cancelled
ci / image (api) (push) Has been cancelled
ci / image (reconciler) (push) Has been cancelled
ci / image (worker) (push) Has been cancelled
ci / integration (push) Has been cancelled
ci / unit (push) Has been cancelled
ci / bump (push) Has been cancelled
Python 3.12 -> 3.14, postgres 16 -> 18, uv 0.5.11 -> 0.11.29,
trivy 0.58.1 -> 0.72.0, gitleaks 8.21.2 -> 8.30.1, yq 4.44.6 -> 4.53.3,
and every action re-pinned to the SHA of its latest tag (checkout v7,
setup-uv v8, buildx v4, login v4, hadolint v3.3.0). helm stays 3.21.3:
already current for 3.x, and helm 4 is a breaking change, not a CVE fix.

trivy mattered most. A vulnerability scanner fourteen minor versions behind is
the one stale pin that hides all the others.

ruff target-version is deliberately py313 while the runtime is 3.14. It
controls the syntax the formatter may emit, and at py314 it rewrites
'except (A, B):' into PEP 758's unparenthesized form — which reads exactly
like Python 2's 'except E, name:' and is a hard SyntaxError below 3.14. No
semantic gain, real readability cost, in a repo meant to be read.

Verified on 3.14: ruff, ruff format, mypy --strict, 166 tests, helm lint,
bandit, pip-audit. The digest guard still rejects placeholder digests.

Risk carried knowingly: the bumped actions run on node24. If act_runner only
provides node20, every job fails at action startup and this commit is the
revert.
2026-07-19 09:39:38 +00:00
Nguyen Minh Phuc 4544765ec5 test: scope the process-group assertion to this run's own children
test_timeout_kills_the_whole_process_group asserted that `pgrep -f 'sleep 300'`
returns nothing. That is machine-global: it matches a leftover from an earlier run of
the same test, any unrelated 'sleep 300' on the box, and the shell running pgrep,
whose own command line contains the pattern being searched for.

Observed three spurious matches on a dev box, which produced a red run that looked
like a Python 3.14 regression in the helm timeout kill. It was not — the same test
failed identically on 3.12. A test that fails for reasons unrelated to the code is as
useless as one that cannot fail.

The sleep duration is now derived from the pid, so it cannot collide with another run,
and the test asserts up front that its own pattern matches nothing before it starts.

Verified: 3 consecutive passes on each of Python 3.12 and 3.14.
2026-07-19 09:39:38 +00:00
Nguyen Minh Phuc 4a426dbe50 worker: create namespaces with helm, drop the kubectl binary
ci / lint (push) Successful in 25s
ci / unit (push) Successful in 59s
ci / types (push) Successful in 1m8s
ci / dockerfile (push) Successful in 13s
ci / chart (push) Successful in 8s
ci / security (push) Successful in 1m2s
ci / integration (push) Successful in 54s
ci / image (api) (push) Successful in 1m6s
ci / image (reconciler) (push) Successful in 3m6s
ci / image (worker) (push) Successful in 2m28s
ci / bump (push) Has been cancelled
trivy took the worker image 39 -> 18 -> 5 findings across two version bumps, and
the last 5 (4x golang.org/x/net, 1x Go stdlib) live in kubectl v1.36.2 — the
newest kubectl that exists. No release clears them; upstream has not rebuilt
against the patched Go yet. Chasing the version further has no end.

kubectl was in that image for exactly one call: ensure_namespace before helm.
`helm upgrade --install --create-namespace` does the same thing, idempotently,
as part of the install it already runs. So the binary goes, and its vendored CVEs
go with it. read_secret had no callers.

Tradeoff recorded: the namespace no longer gets an svcforge.io/team label, since
--create-namespace makes a bare one. Nothing reads that label today.
2026-07-18 13:05:22 +00:00
Nguyen Minh Phuc c76154aeaa review: fix 26 findings from a 4-agent audit
ci / lint (push) Successful in 34s
ci / unit (push) Successful in 1m41s
ci / types (push) Successful in 1m41s
ci / dockerfile (push) Successful in 18s
ci / security (push) Successful in 1m27s
ci / chart (push) Failing after 1m11s
ci / integration (push) Successful in 1m10s
ci / image (api) (push) Has been skipped
ci / image (reconciler) (push) Has been skipped
ci / image (worker) (push) Has been skipped
ci / bump (push) Has been skipped
CORRECTNESS
- lost-lease race: complete()/fail() did not check ownership, so a worker whose
  lease expired could mark a task done while another worker was running it, or
  requeue a task someone else owned. Reproduced, fixed with a CAS on
  (state, locked_by), pinned by two regression tests.
- worker died on report failure: _run_one's docstring claimed no exception
  escapes the TaskGroup; fail()/complete() were outside the guarded block, so a
  DB blip cancelled every sibling provision on the pod.
- claim query used an INNER join, which could strand a just-claimed task and
  report 'queue empty'. LEFT join.
- InstanceRepo.set_error bypassed the state machine and had no callers. Deleted.
- handle_deprovision ignored its CAS result, so a wrong-state instance kept a
  dangling endpoint and got re-provisioned by the drift check 60s later.
- handle_verify re-notified on every retry: five pages for one halt.

DEPLOY-BREAKING
- the migration Job could never succeed: no Dockerfile copied migrations/, and
  migrate.py resolved the path relative to the source tree, which only works for
  an editable install. Added COPY + SVCFORGE_MIGRATIONS_DIR.
- ServiceMonitor selector did not match the Service: API metrics never scraped.
- SvcforgeReconcilerStale fired permanently from every pod, because the gauge is
  module-level and every service exports it as 0. Scoped to the reconciler job.
- SvcforgeTaskFailed latched forever on a monotonic counter. Now increase()[15m].
- the digest guard accepted the all-zeros placeholder.
- worker terminationGracePeriodSeconds was 60s against a 600s helm timeout.

DEAD CODE THAT SHOULD NOT HAVE BEEN
- adapters/k8s.py was never called, so tenant namespaces were never created and
  the first provision for a new team would fail. Wired into handle_provision.
- adapters/redis.py was never imported by any service. Rate limiting is now wired
  into the API, failing open.
- Settings.check_production() had no callers. Given an explicit environment and
  called from every entrypoint.

OBSERVABILITY
- the API never called obs.setup(): no JSON logs, no trace correlation, log_json
  silently inert.
- LogNotifier's structured fields were discarded by the stdlib->structlog bridge.
- bind_task_context cleared the 'service' binding for the life of every task.
- split tasks_failed into task_attempts_failed and tasks_dead_lettered.

SECURITY
- trivy correctly blocked the worker/reconciler images: helm 3.16.2 and kubectl
  1.31.2 carry CRITICAL Go stdlib CVEs. Bumped to helm 3.21.3 and kubectl 1.35.3,
  which also closes a four-minor skew against the v1.35.3 cluster.

TESTS THAT COULD NOT FAIL
- the concurrency cap test passed on a fully serial worker.
- the alert/metric cross-check asserted a hardcoded list instead of reading the
  chart, so it could not catch a rename on the chart side.
- fixed OTel tracer-provider pollution between test files.

DOCS
- ARCHITECTURE.md: mermaid diagrams, user stories, and the helm-vs-ArgoCD
  guarantee (verified with --dry-run=server).
- AGENTS.md + CLAUDE.md.
- prose sweep for back-and-forth phrasing across 19 files.
2026-07-18 12:13:49 +00:00
Nguyen Minh Phuc 50c2fe2a1e svcforge: reference implementation
ci / lint (push) Successful in 1m19s
ci / unit (push) Failing after 1m2s
ci / integration (push) Has been skipped
ci / types (push) Successful in 1m37s
ci / security (push) Failing after 38s
ci / dockerfile (push) Successful in 14s
ci / image (api) (push) Has been skipped
ci / image (reconciler) (push) Has been skipped
ci / image (worker) (push) Has been skipped
ci / bump (push) Has been skipped
Complete working build of the system learn-python/ teaches.
164 tests, mypy --strict clean, domain coverage 99%.
2026-07-17 10:44:54 +00:00