feat(npm): compose the npm remote packument proxy per repository

🎯 What this MR does

Adds npm.RemoteProxy, the process-wide half of the npm remote read composition, plus the real-database harness every remote packument suite builds on. Step 10a of a 4-MR stack splitting an oversized branch.

Mounts nothing, and serves nothing. No handler exists yet, so every kind=remote npm read still answers the interim 501.

📚 The stack:

Step MR What it delivers
10a · this MR 👉 !1681 (closed) npm.RemoteProxy: one coalescing flight per remote repository, plus the shared test harness
10b !1682 (merged) npm.RemotePackumentHandler: variant selection, the two serve shapes, the response headers
10c !1683 (merged) The error mapping, and the upstream_rejected code
10d !1684 (closed) Composition root: mount, run recipe

🧩 The three pieces

1️⃣ One flight per remote repository

remote.SingleFlight's usage contract asks for exactly one long-lived instance per remote repository. Two instances for one repository coalesce nothing between them and grant it two independent max_concurrent_fills_per_repo budgets. A handler cannot honour that alone: it is one process-wide value serving every repository, and the repository arrives per request.

So RemoteProxy keys a map on (namespace_id, repositories.id) and creates an entry on first use, under the map's own lock rather than through a double-checked build that could publish two instances for one repository. Creation is pure composition with no I/O, which is what makes holding the lock across it acceptable. Composing per request instead would have shipped the coalescing inert and the fill cap enforcing nothing, invisible except as upstream traffic.

Nothing evicts yet. A naive eviction that dropped an entry with a flight in progress would hand the next caller a second instance, the exact condition the contract warns about; an idle sweep that cannot drop a live entry is #672. Until it lands the map holds one flight per distinct remote repository id this process has served, which is not the number the deployment currently holds: a deleted repository's entry stays behind, so the count only rises under provisioning churn. Each entry is small, so this is a slow leak rather than a figure to size memory against, and the type comment says exactly that.

2️⃣ What cannot be snapshotted

Two values would be wrong if the flight captured them from the request that first built it.

The cache store baked into the flight is built once, from that first row, and that is safe only because the flight writes through it and never reads a freshness verdict from it. Every verdict is taken by a per-request store, so a window an operator narrows through the management API takes effect on the next request with no composition to rebuild.

The upstream URL and bearer cannot be captured at all. remoteUpstreamConfigSource re-reads the row on every outbound request, so a rotated credential or a changed URL reaches the very next upstream request rather than the next restart. It is the live counterpart of the per-probe snapshot the health sweep composes, whose single-resolution scope is what makes a snapshot legitimate there and not here.

3️⃣ The harness, and the test that needs it

remote_proxy_integration_test.go wires the production npm remote datastore stores, the internal/storage/stub in-memory store (mirroring blob_storage_blobs rows into the real database so the cache tables' blob FKs resolve), and a scripted upstream client to a real PostgreSQL. 10b and 10c own only the behavior they pin; this file is reviewed once.

TestRemoteProxy_SharesOneFlightPerRepository asserts pointer identity across concurrent callers, and distinct instances across two repositories, against real seeded rows. Real rows matter because the key includes the repository id the database minted. It is the check no constructor signature can make and no unit test fails without.

npmremote.UpstreamConfigFrom hoists the row-to-UpstreamConfig rendering the health probe already had, so the probe and the proxy derive one value one way.

Verification

  • gofmt -l, go build ./..., go vet ./... with and without -tags=integration: clean.
  • go test -count=1 ./internal/format/npm/... ./internal/config/... ./cmd/artifact-registry/: green.
  • go test -tags=integration -run 'TestRemoteProxy|TestRemotePackument' ./internal/format/npm/: green, 38s.
  • golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0: 0 findings on any line this MR adds or changes, checked by mapping each finding against the MR's own hunks.

📏 Diff size

1257 LOC, past the 500 reviewable-LOC line in development-model.md. Split by file group:

Group Files LOC
Production remote_proxy.go 251, npmremote/upstream_request.go +25, npmremote/health_sweep.go -25 301
Tests remote_proxy_integration_test.go 668, remote_proxy_internal_test.go 165, npmremote/upstream_request_test.go +29, export_test.go +12 874
Docs docs/plans/2026-07-15-npm-remote.md +82 82

Tests are 70% of it, and 668 of those are the shared harness that 10b and 10c reuse rather than re-declare. The step's whole diff was 3787 LOC in one MR; this stack's largest piece is a third of that.

🧪 E2E scenario impact

None: nothing routes to this composition, and docs/testing/e2e/npm.md scopes remote npm repositories out until the capability ships.

Related to #347 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading