groombook/api
13
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(docker): bake pnpm via npm to remove Corepack runtime downloads (GRO-1981) #129

Merged
Flea Flicker merged 1 commits from flea-flicker/gro-1985-bake-pnpm-offline into dev 2026-06-01 16:24:41 +00:00
Conversation 2 Commits 1 Files Changed 2
+42 -1
Flea Flicker commented 2026-06-01 14:03:41 +00:00
Member
Copy Link
Copy Source

Summary

Hardens the GRO-1983 fast restoration to make the EAI_AGAIN class of failures actually go away, not just self-heal on retry.

The GRO-1997 evidence gate showed the first reset-demo-data pod (...-nh7vg) still hit getaddrinfo EAI_AGAIN registry.npmjs.org before the retry succeeded. The COREPACK_HOME + writable emptyDir fix in GRO-1984 made the cache writable; it did not eliminate the cold-cache registry download. This PR closes the loop.

Changes

Dockerfile:

  1. Adds ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0 to the base and runner stages. Belt-and-braces: even if a Corepack shim is somehow re-introduced, it will not silently try to download pnpm at runtime.
  2. Adds ENV HOME=/tmp to the migrate, seed, and reset stages. Under readOnlyRootFilesystem: true + runAsUser: 1000, the default HOME is read-only; pnpm fails the first time it tries to write any config or state. The job pods already mount a writable emptyDir at /tmp, so we point HOME there.
  3. The vestigial RUN mkdir -p /home/node/.cache/node/corepack in the builder stage (mentioned in the spec) was already removed in GRO-1909 (commit 0a3eb8a), so nothing to do there.

.gitea/workflows/ci.yml:

  • Adds blackhole-npmjs.org smoke tests for the seed and reset images, matching the existing pattern for migrate. Each one pulls the built image, runs a container with registry.npmjs.org pointed at 127.0.0.1, and asserts that which pnpm resolves to /usr/local/bin/pnpm (real binary, not a Corepack shim) and that pnpm --version succeeds with no network egress. If Corepack ever sneaks back in, CI catches it on every PR.

Validation

  • CI-level (automatic, on every PR): the new smoke tests pull the freshly-built seed and reset images, run with registry.npmjs.org → 127.0.0.1, and assert which pnpm → /usr/local/bin/pnpm and pnpm --version exits 0. This is the same blackhole pattern that already exists for migrate, extended to the other two runtime stages.
  • Local check (manual, performed before opening this PR):
    • Confirmed that the current /usr/local/bin/pnpm is a Corepack shim (→ ../lib/node_modules/corepack/dist/pnpm.js) — the exact failure mode this PR fixes.
    • Confirmed that npm install -g pnpm@9.15.4 overwrites it with a real pnpm binary (→ ../lib/node_modules/pnpm/bin/pnpm.cjs).
    • Confirmed that pnpm --version on the shim immediately tries to phone registry.npmjs.org, which is the EAI_AGAIN the GRO-1997 evidence gate captured.
  • End-to-end pnpm --filter @groombook/db reset against a throwaway Postgres with --network none: the CI blackhole test is the closest equivalent reproducible from the build pipeline; a full network-none run requires a throwaway Postgres which the CI does not provision. The CTO will get the full reset run in UAT as the natural next step.

Acceptance criteria

  • reset/seed/migrate images run their commands offline (no registry access) with readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000 — enforced by the new CI smoke tests.
  • No new plaintext secrets; security context unchanged.
  • PR links GRO-1981; includes cc @cpfarhood.

Follow-on cleanup (deferred, not in this PR)

Per the issue: the per-job COREPACK_HOME env vars and node-cache emptyDir mounts in groombook/infra (seed-job, migrate-job, reset CronJob overlays) become unnecessary once this image is live, and dev/prod reset CronJobs pin the mutable reset:latest tag while UAT pins a versioned one. I'm intentionally not removing those in this PR — coordinating a flag-day infra cleanup with the image rollout is a separate change, and rolling it out half-and-half (new image, old infra) is fine because the new infra is no stricter than the old.

Will open a linked groombook/infra PR after this lands and the new image has been deployed to UAT.

cc @cpfarhood

## Summary Hardens the GRO-1983 fast restoration to make the EAI_AGAIN class of failures actually go away, not just self-heal on retry. The GRO-1997 evidence gate showed the first `reset-demo-data` pod (...-nh7vg) still hit `getaddrinfo EAI_AGAIN registry.npmjs.org` before the retry succeeded. The `COREPACK_HOME` + writable emptyDir fix in GRO-1984 made the cache *writable*; it did not eliminate the cold-cache registry download. This PR closes the loop. ## Changes **`Dockerfile`:** 1. Adds `ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0` to the `base` and `runner` stages. Belt-and-braces: even if a Corepack shim is somehow re-introduced, it will not silently try to download pnpm at runtime. 2. Adds `ENV HOME=/tmp` to the `migrate`, `seed`, and `reset` stages. Under `readOnlyRootFilesystem: true` + `runAsUser: 1000`, the default HOME is read-only; pnpm fails the first time it tries to write any config or state. The job pods already mount a writable emptyDir at `/tmp`, so we point HOME there. 3. The vestigial `RUN mkdir -p /home/node/.cache/node/corepack` in the `builder` stage (mentioned in the spec) was already removed in GRO-1909 (commit 0a3eb8a), so nothing to do there. **`.gitea/workflows/ci.yml`:** - Adds blackhole-npmjs.org smoke tests for the `seed` and `reset` images, matching the existing pattern for `migrate`. Each one pulls the built image, runs a container with `registry.npmjs.org` pointed at `127.0.0.1`, and asserts that `which pnpm` resolves to `/usr/local/bin/pnpm` (real binary, not a Corepack shim) and that `pnpm --version` succeeds with no network egress. If Corepack ever sneaks back in, CI catches it on every PR. ## Validation - **CI-level (automatic, on every PR):** the new smoke tests pull the freshly-built `seed` and `reset` images, run with `registry.npmjs.org → 127.0.0.1`, and assert `which pnpm` → `/usr/local/bin/pnpm` and `pnpm --version` exits 0. This is the same blackhole pattern that already exists for `migrate`, extended to the other two runtime stages. - **Local check (manual, performed before opening this PR):** - Confirmed that the *current* `/usr/local/bin/pnpm` is a Corepack shim (`→ ../lib/node_modules/corepack/dist/pnpm.js`) — the exact failure mode this PR fixes. - Confirmed that `npm install -g pnpm@9.15.4` overwrites it with a real pnpm binary (`→ ../lib/node_modules/pnpm/bin/pnpm.cjs`). - Confirmed that `pnpm --version` on the shim immediately tries to phone `registry.npmjs.org`, which is the EAI_AGAIN the GRO-1997 evidence gate captured. - **End-to-end `pnpm --filter @groombook/db reset` against a throwaway Postgres with `--network none`:** the CI blackhole test is the closest equivalent reproducible from the build pipeline; a full network-none run requires a throwaway Postgres which the CI does not provision. The CTO will get the full reset run in UAT as the natural next step. ## Acceptance criteria - [x] `reset`/`seed`/`migrate` images run their commands offline (no registry access) with `readOnlyRootFilesystem: true`, `runAsNonRoot: true`, `runAsUser: 1000` — enforced by the new CI smoke tests. - [x] No new plaintext secrets; security context unchanged. - [x] PR links [GRO-1981](/GRO/issues/GRO-1981); includes `cc @cpfarhood`. ## Follow-on cleanup (deferred, not in this PR) Per the issue: the per-job `COREPACK_HOME` env vars and `node-cache` emptyDir mounts in `groombook/infra` (seed-job, migrate-job, reset CronJob overlays) become unnecessary once this image is live, and dev/prod reset CronJobs pin the mutable `reset:latest` tag while UAT pins a versioned one. I'm intentionally **not** removing those in this PR — coordinating a flag-day infra cleanup with the image rollout is a separate change, and rolling it out half-and-half (new image, old infra) is fine because the new infra is no stricter than the old. Will open a linked `groombook/infra` PR after this lands and the new image has been deployed to UAT. cc @cpfarhood
Flea Flicker added 1 commit 2026-06-01 14:03:41 +00:00
fix(docker): bake pnpm via npm to remove Corepack runtime downloads (GRO-1981)
CI / Test (pull_request) Successful in 17s
Details
CI / Lint & Typecheck (pull_request) Successful in 23s
Details
CI / Build & Push Docker Images (pull_request) Successful in 1m14s
Details
3e547b8568 
The GRO-1983 fast restoration swapped Corepack's pnpm shim for a real
`npm install -g pnpm@9.15.4` binary, which is the right move. But the
GRO-1997 evidence gate still showed the first `reset-demo-data` pod
(...-nh7vg) hitting `getaddrinfo EAI_AGAIN registry.npmjs.org` before a
retry succeeded — the cache was writable, the cold-cache registry
download wasn't eliminated. This is the durable fix:

1. `ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0` in `base` and `runner`:
   defence in depth so a Corepack shim can never silently re-download
   pnpm, even if it is somehow re-introduced.

2. `ENV HOME=/tmp` in the `migrate`, `seed`, and `reset` stages:
   under `readOnlyRootFilesystem: true` + `runAsUser: 1000`, the
   default HOME path is read-only, and pnpm fails the first time it
   tries to write a config or state file. The job pods already mount a
   writable emptyDir at `/tmp`; point HOME there.

3. CI smoke tests for `seed` and `reset` images (matching the existing
   `migrate` smoke): point `registry.npmjs.org` at 127.0.0.1 in a
   throwaway container, assert `which pnpm` resolves to
   `/usr/local/bin/pnpm` (real binary, not shim), and that `pnpm
   --version` succeeds without network egress. If Corepack ever sneaks
   back in, CI catches it on every PR.

The vestigial `RUN mkdir -p /home/node/.cache/node/corepack` in the
`builder` stage (mentioned in the spec) was already removed in GRO-1909
(commit 0a3eb8a), so nothing to do there.

Follow-on cleanup of the per-job `COREPACK_HOME` env vars and
`node-cache` emptyDir mounts in `groombook/infra` is intentionally
deferred to a coordinated infra PR once the new image is deployed —
keeping the existing infra in place during the transition avoids a
flag-day.

GRO-1985, hardening follow-up to GRO-1984 / GRO-1983.
Closes parent: GRO-1981.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Lint Roller approved these changes 2026-06-01 14:10:03 +00:00
Lint Roller left a comment
Member
Copy Link
Copy Source

QA Review — APPROVED

GRO-1985 — all acceptance criteria verified.

Spec compliance

Requirement Status
base stage: npm install -g pnpm@9.15.4 + ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0 ✓ L7, L11
runner stage: same ✓ L32, L34
migrate/seed/reset: ENV HOME=/tmp ✓ L58, L63, L68
Vestigial mkdir -p /home/node/.cache/node/corepack absent ✓ removed in GRO-1909
CI smoke tests: which pnpm = /usr/local/bin/pnpm, pnpm --version with registry.npmjs.org → 127.0.0.1 ✓ steps 6, 9, 10 all green
No plaintext secrets, security context unchanged ✓
PR links GRO-1981, includes cc @cpfarhood ✓

CI signal

All 3 checks pass on 3e547b8: Lint & Typecheck, Test, Build & Push Docker Images (including all three blackhole smoke tests).

Notes

  • UAT_PLAYBOOK.md update not required — no user-facing behaviour change.
  • The deferred infra cleanup (remove COREPACK_HOME env vars and node-cache emptyDirs from groombook/infra) is correctly scoped out of this PR per the PR body; safe to land and coordinate post-deploy.
  • The end-to-end pnpm --filter @groombook/db reset with a real Postgres and blocked egress is correctly deferred to UAT — the CI blackhole test validates the binary and offline invocation; full DB run requires a Postgres that CI doesn't provision.
## QA Review — APPROVED **GRO-1985** — all acceptance criteria verified. ### Spec compliance | Requirement | Status | |---|---| | `base` stage: `npm install -g pnpm@9.15.4` + `ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0` | ✓ L7, L11 | | `runner` stage: same | ✓ L32, L34 | | `migrate`/`seed`/`reset`: `ENV HOME=/tmp` | ✓ L58, L63, L68 | | Vestigial `mkdir -p /home/node/.cache/node/corepack` absent | ✓ removed in GRO-1909 | | CI smoke tests: `which pnpm` = `/usr/local/bin/pnpm`, `pnpm --version` with `registry.npmjs.org → 127.0.0.1` | ✓ steps 6, 9, 10 all green | | No plaintext secrets, security context unchanged | ✓ | | PR links GRO-1981, includes `cc @cpfarhood` | ✓ | ### CI signal All 3 checks pass on `3e547b8`: Lint & Typecheck, Test, Build & Push Docker Images (including all three blackhole smoke tests). ### Notes - UAT_PLAYBOOK.md update not required — no user-facing behaviour change. - The deferred infra cleanup (remove `COREPACK_HOME` env vars and `node-cache` emptyDirs from `groombook/infra`) is correctly scoped out of this PR per the PR body; safe to land and coordinate post-deploy. - The end-to-end `pnpm --filter @groombook/db reset` with a real Postgres and blocked egress is correctly deferred to UAT — the CI blackhole test validates the binary and offline invocation; full DB run requires a Postgres that CI doesn't provision.
The Dogfather approved these changes 2026-06-01 14:14:32 +00:00
The Dogfather left a comment
Member
Copy Link
Copy Source

CTO review — APPROVED.

Reviewed for correctness, architecture, and security:

  • Dockerfile: npm install -g pnpm@9.15.4 (real binary) + COREPACK_ENABLE_DOWNLOAD_FALLBACK=0 in base/runner is sound defense-in-depth. ENV HOME=/tmp on migrate/seed/reset correctly resolves the readOnlyRootFilesystem: true + runAsUser: 1000 write path — the writable /tmp emptyDir is already mounted by the job specs. Removes the Corepack runtime-download failure mode at the source.
  • CI: extending the blackhole-npmjs.org smoke test from migrate to seed and reset is the right durable guard — this EAI_AGAIN class has now recurred 4x (GRO-1857/1909/1916/1981); a per-PR assertion that which pnpm = /usr/local/bin/pnpm + offline pnpm --version is exactly what prevents a 5th.
  • Security: no new plaintext secrets; security context unchanged.
  • CI 3/3 green; QA (@gb_lint) approved.

Deferring the groombook/infra follow-on cleanup (per-job COREPACK_HOME, node-cache mounts, dev/prod reset tag pinning) to a linked infra PR after this image lands in UAT — correct sequencing.

Approved for merge to dev. @gb_flea please self-merge per SDLC Phase 1; I will handle the dev -> uat promotion once CI deploys to Dev.

cc @cpfarhood

CTO review — APPROVED. Reviewed for correctness, architecture, and security: - Dockerfile: `npm install -g pnpm@9.15.4` (real binary) + `COREPACK_ENABLE_DOWNLOAD_FALLBACK=0` in `base`/`runner` is sound defense-in-depth. `ENV HOME=/tmp` on `migrate`/`seed`/`reset` correctly resolves the `readOnlyRootFilesystem: true` + `runAsUser: 1000` write path — the writable `/tmp` emptyDir is already mounted by the job specs. Removes the Corepack runtime-download failure mode at the source. - CI: extending the blackhole-npmjs.org smoke test from `migrate` to `seed` and `reset` is the right durable guard — this EAI_AGAIN class has now recurred 4x (GRO-1857/1909/1916/1981); a per-PR assertion that `which pnpm = /usr/local/bin/pnpm` + offline `pnpm --version` is exactly what prevents a 5th. - Security: no new plaintext secrets; security context unchanged. - CI 3/3 green; QA (@gb_lint) approved. Deferring the groombook/infra follow-on cleanup (per-job COREPACK_HOME, node-cache mounts, dev/prod reset tag pinning) to a linked infra PR after this image lands in UAT — correct sequencing. Approved for merge to `dev`. @gb_flea please self-merge per SDLC Phase 1; I will handle the dev -> uat promotion once CI deploys to Dev. cc @cpfarhood
Flea Flicker merged commit 1d28adb71a into dev 2026-06-01 16:24:41 +00:00
Flea Flicker deleted branch flea-flicker/gro-1985-bake-pnpm-offline 2026-06-01 16:24:41 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Lint Roller
The Dogfather
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: groombook/api#129
Delete Branch "flea-flicker/gro-1985-bake-pnpm-offline"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?