feat(oci): serve cached container remote blob reads (S16 plan: 11c/26)
Why
Step 11c of the S16 container remote plan,
the last of the step's three MRs. 11a landed the download-signal seam and 11b the shared
response-header writer, the digest gate, and the manifest arm. This one fills the remaining
read slot: a kind=2 blob route that resolves a cached row now serves it instead of
answering the dispatcher's 501.
What
RemoteBlobHandler.ServeRemoteRead serves an already-cached blob. It runs S12's digest
grammar itself before the cache lookup, because the hosted per-action handlers that run it
sit below where the dispatcher branches on repository kind. On a hit it honours the
ADR-005 delivery mode — 307 under redirect, streamed under proxy — and forces no stream,
so the operator's setting decides. It composes 11b's writeRemoteReadHeaders rather than
assembling headers of its own, handles Range (206, 416 with Content-Range: bytes */<total>),
answers HEAD from BlobInfo without opening anything and never redirects it, and records
one download signal per delivered read. A miss still answers the interim 501; Step 15
replaces it.
cmd/artifact-registry/wire_oci.go wires the arm into Step 10.5's WithRemoteBlob slot,
and the boot guard asserts the slot is filled by distinguishing the arm's own fill 501
from the dispatcher's slot-unwired one.
The plan changes are in a separate MR
Per CLAUDE.md guardrail 5, step MRs do not edit the plan file. This branch carries none;
its Step 11 corrections are in
!1762 (merged), alongside 11b's.
Read !1762 (merged)'s Step 11 section, not main's, to review this MR against the plan. The entry on
main is the uncorrected one: it says 11c modifies remote_serve.go where this branch creates
remote_blob_serve.go, and gives 11c criteria 38 and 39, which 11b owns outright. !1762 (merged) is the
corrected version and carries this MR's Files cell, its criteria allocation, and the record of what
11b's two review rounds cost this arm.
The two have no merge-order dependency in either direction and touch disjoint files — !1762 (merged) changes
only docs/plans/, this changes nothing there — so they cannot conflict and can land in either
order.
Merge order
Clear. 11b (!1707 (merged))
has merged, so this no longer stacks on anything: it is rebased onto main and targets main, and
the diff below is this arm's own work rather than 11b's carried along.
The Draft flag was set because the base was about to move. That reason is spent — the base has moved and the branch is on it.
Reviewable size
15 files, +3,711 −68, measured three-dot against the merge base the way GitLab counts it. Past the 500-line ceiling development-model.md sets, so the justification, by file group:
| Group | Lines |
|---|---|
remote_blob_serve_test.go — digest gate, cache-hit serve, delivery mode, failures, signals |
1,312 |
remote_blob_serve_fixtures_test.go — fixtures, fakes, harness |
735 |
remote_blob_serve_range_test.go — Range semantics |
319 |
internal/format/oci/remote_blob_serve.go — the arm |
679 |
Run recipe (driver.sh, SKILL.md) |
387 |
Wiring (wire_oci_remote.go, boot test, wire_oci.go, wire_oci_remote_test.go) |
140 |
Doc comments, the shared resolver's parameter, the goconst hoist, the shared copy-failure emitters both arms call |
139 |
Two thirds is test code. Splitting it across MRs would separate the arm from the suite that exercises it, and the plan's own Ships as three stacked MRs argument already cut Step 11 at the two seams that produce reviewable units — a third cut would put the arm in one MR and its tests in another.
The suite is split across three files rather than one, which is the cut !1707 (merged) was asked to make on the manifest arm and made — expressly so this arm would not grow a two-thousand-line suite. It had grown one (2,183 lines) before this branch was rebased onto that round. Both cuts follow banners the file already carried, and the resulting shape matches the manifest arm's almost line for line (1,219 / 704 / 206 there, 1,312 / 735 / 319 here).
No test body changed and none was added or dropped. The top-level declaration lists before and after the split are identical, as are the 44 subtest row names; both were diffed rather than read.
Test plan
internal/format/oci/remote_blob_serve_test.go and its two siblings cover the arm end to end: the digest gate
ahead of the cache lookup (five refusal rows, including the sha512: prefix taking
400 DIGEST_INVALID on this route where the manifest route takes 404), the cache-hit
serve under all three delivery-mode values, Range in four RFC 9110 forms, the
416-on-GET against 200-on-HEAD pair through one fixture, Content-Length equality
across the verbs asserted against each other rather than a literal, and one download signal
per delivered read with none on a 416, a mid-stream failure, a short copy, or any 500.
cmd/artifact-registry/wire_oci_boot_integration_test.go adds the blob slot's boot guard.
No e2e scenario is added or affected. Both
docs/testing/e2e/oci.md
and docker.md list "Virtual and remote (proxy/cache) repositories" under Out of scope
until the capability ships, and it has not: no fill path exists, so a client pulling from
a remote repository still gets the interim 501 unless a row was seeded out of band.
Container/OCI conformance is unreached for the same reason and has not been run.
driver.sh smoke: 24 passed, 0 failed, re-run from a cold start on the current tree, per
guardrail 20. The recipe's previously recorded run predated three changes to the arm, one of them a
new refusal path, so it was re-run rather than relied on and the recipe now records what this one
covered.
The same boot drove the arm itself through seed-remote, which is what that subcommand exists for.
Every answer it produces was checked against a database that had just been created:
| Request | Answer |
|---|---|
GET a cached blob |
200, Content-Length: 7296, Docker-Content-Digest, Content-Disposition: attachment, nosniff |
HEAD the same |
200, same Content-Length and digest, no body |
Range: bytes=0-9 |
206, Content-Range: bytes 0-9/7296, Content-Length: 10 |
Range: bytes=999999- |
416, Content-Range: bytes */7296 |
| uncached digest | 501 with Cache-Control: no-store |
| malformed digest | 400 DIGEST_INVALID, detail invalid digest format |
For LLM Agents
Detail a reviewer does not need but an agent picking this up does
Branch state
Rebased onto main past 11b's merge with git rebase --onto origin/main e20060fee, which
replays only this branch's own commits — 11b merged as squash 8e76e78a8, so replaying from
the old base would have re-applied its whole unsquashed series. Backup ref
backup/11c-pre-rebase-20260820 holds the pre-rebase tip 550606799, which is what this MR
showed before.
Measure this diff three-dot (git diff origin/main...HEAD). Two-dot picks up whatever
landed on main after the merge base as spurious deletions — at the time of writing that is
1,657 lines of lifecycle_reap_maven_remote*, which this branch does not touch.
The recurring conflict during that rebase was cmd/artifact-registry/wire_oci.go with an
empty "ours" side: 11b's second round moved buildOCIRemoteManifestHandler and the
staleness selector into a new wire_oci_remote.go, so every commit here touching the blob
builder replayed as "add that whole block back to wire_oci.go". Resolution each time is to
take only buildOCIRemoteBlobHandler from the incoming side and put it in
wire_oci_remote.go. ~/resolve-wire-oci.py automates it.
Five shapes shared with the manifest arm, across two review rounds
11b's review rounds produced fixes true of both arms, and the branches conflict only on lines both edit — so a shape written separately produces no conflict and nothing reports it. The first round found three, all discharged here:
- Short copy.
streamCachedBlobjudgesio.Copy's count as well as its error; a clean copy that moved fewer bytes than the row promised is reported undelivered, so the download signal does not fire and hold a truncated payload out of eviction. Logged at Error withfields.WrittenBytes, matchingmaven.logCopyOutcome,npm.recordCopyOutcome, and the read-back behinderrRemoteCacheReadBackLengthMismatch. The manifest arm still logs this at Warn withcopied_bytesand owes the raise — that one is 11b's, not fixed here. - Exported zero value.
NewRemoteBlobHandler's doc says the nil-seam check closes the constructor path and no other, with a matching row inTestNewRemoteBlobHandler_NilSeamPanics. - Parent repository id. 11b added
repositoryIDtoremoteReadSubject. The blob arm builds that struct with a keyed literal, which compiled unchanged and left the field at the zero UUID — every blob-read failure loggingrepository_id=00000000-0000-0000-0000-000000000000, worse than an absent field because it looks populated and matches every tenant. The rebase conflicts on the doc comment above the struct and not on the literal, so it surfaces the harmless half and hides the harmful one. Fixed, andrequireRemoteBlobServeSubjectAttrsnow takes the parent id as a parameter and requires it — a parameter rather than a package constant because the fixture mints one per test, so a zero value matches none of them.
A second round on 11b found two more, and moved three files. The two shapes:
- Disagreeing sums on a cache row.
serveCachedManifestbuiltDocker-Content-Digestfrom the row'sdigestcolumn and opened the payload atblob_sha256with nothing checking the two hold the same sum, so a well-formed but disagreeing pair advertised one digest and streamed the bytes the other addressed.serveCachedBlobreads the same two columns the same way and had the same hole. It refuses such a row now as the500it already sends for a corrupt one, with a disagreeing-pair row inTestRemoteBlobServe_StorageFault_500that seeds a well-formed digest addressing no seeded object whileblob_sha256still addresses the real one, so nothing else on the route faults and only the equality refuses it. Mutation-checked: without the guard that row serves a200carrying the wrongDocker-Content-Digest. The justification is this route's own rather than the manifest arm's borrowed — a layer has one representation, so its OCI digest is the sha256 of the bytesblob_sha256addresses, where the manifest arm has to argue it from every upstream fetch asking for the identity representation. - The lifted repository resolver. 11b moved
resolveRepositoryandstreamCachedManifestoff*RemoteManifestHandlerto package level explicitly so this arm would not copy them. This branch's own(h *RemoteBlobHandler) resolveRepositoryis deleted and it callsresolveRemoteRepositoryinstead. That left one thing open: the log message inside it readoci remote manifest read: …and would have reported a blob read's failure under the manifest arm's wording. It takes the message from the caller now and each arm passes its own, so an operator filtering by route gets the route they filtered for. A parameter rather than a label onremoteReadSubject, because the failure happens before a subject exists and each arm has exactly one such line.
The three moved files are not shapes but they are what a stale Files list gets wrong:
buildOCIRemoteManifestHandler and the staleness selector left wire_oci.go for
wire_oci_remote.go, that selector's test left wire_oci_test.go for
wire_oci_remote_test.go, and remote_serve_test.go split its fixtures and header-writer
cases out. None changes what this arm does; all three change where it does it. That is the
half a rebase reports as a conflict and a plan written before the round does not — which is
why !1762 (merged) needed a second pass.
Two files outside the arm
internal/format/oci/blob.go and upload.go take two new constants. The gate's five
refusal rows push "invalid digest format" to five occurrences and
"unsupported digest algorithm" to three, crossing goconst's threshold — and goconst
anchors the finding at the production site, so the _test.go$ exclusion does not help.
Both reasons are hoisted beside digestInvalidReason, the same move the "reference"
detail key took. The package reported 3 issues before this and reports 0 now.
cmd/artifact-registry/wire_oci_remote.go and wire_oci_remote_test.go: the blob arm's
builder goes beside the manifest arm's, which 11b's second round moved there, and the blob arm
takes its staleness window through the selector the manifest arm already uses rather than
reading the config field inline — so remoteManifestStalenessWindow becomes a shared
remoteStalenessWindow and its test is renamed. One selector rather than one per arm: the
guard is against a call site naming the wrong field, and both arms are covered the moment
neither names a field at all. wire_oci.go keeps only the slot append, two lines.
Verification run
Re-run after the rebase onto main and the two round-two commits, not carried over:
go build ./... clean · gofmt -l empty · go vet clean · go test ./internal/format/oci/... ./cmd/artifact-registry/... 2,687 passed across 3 packages · golangci-lint 0 issues on
both packages, re-run after the suite split.
Integration-tagged lint (--build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false, per guardrail 7) reports 12 findings across
./internal/format/oci/ ./cmd/artifact-registry/, every one of them on a line this branch does
not add — checked against the added line ranges rather than by filename, since insertions shift
pre-existing findings down into a diff's own files.
Three assertions mutation-checked, each restored by sha256: blanking Docker-Content-Digest in
the shared writer reds the 206 test; deleting repositoryID reproduces the zero UUID in the
captured log and reds two tests; and removing the disagreeing-sums guard makes that row serve a
200 with the wrong digest instead of a 500.
Related to #288