feat(oci): serve a virtual repository's blob route (S32 plan: 16/20)

Summary

This MR fills the blob route of a kind=1 container repository. GET and HEAD on /v2/<name>/blobs/<digest> now resolve through the virtual repository's upstreams and stop answering the interim 501.

  • The handler runs the digest gate first, then maps the request's repository to its container_virtual_repositories row through the finder, then calls Resolve.
  • A warm blob follows the configured delivery mode. A cold full GET streams under both modes. A cold HEAD or ranged GET drains the fill, then answers from the committed row.
  • A committed read is a follower whose leader's fill already committed. It closes its copy unread and answers every request shape from the row, as the standalone kind=2 route does. Under redirect delivery a full GET from such a follower answers 307.
  • The route arms its own write deadline. Without it, net/http's server.timeouts.write (10s in config.example.yaml) cuts every cold layer that takes longer, and no such layer caches. The pre-fetch arm covers resolve_timeout plus the fill budget. The re-arm after Resolve takes the larger of the fill tail and server.timeouts.write, so an operator who raised that key for large layers keeps the raised value.
  • The stream-failure log now receives the re-armed instant. A copy that the armed deadline cut logs at Error as a deadline kill, not as client churn.

Plan amendment

!2941 (merged) amends Step 16 and is still open. It adds the server.timeouts.write floor and the committed-read branch to the plan. Until !2941 (merged) merges, this code follows !2941 (merged)'s plan text, not the plan on main.

Governing ADRs

I read each ADR against the diff. The local mirror was fresh (scripts/adr-freshness.sh exited 0).

  • ADR-005 (delivery mode): a warm read follows the namespace's delivery mode. A HEAD never redirects. A cold full GET streams under both modes, as S16's standalone route does.
  • ADR-008 (content-addressable storage): the fill carries the expected digest. A mismatch caches nothing and asks no lower-priority upstream.
  • ADR-009 (API design): GET and HEAD on the blob route are the verbs ADR-009 lists for every kind.
  • ADR-013 (storage backend): warm reads go through the storage layer's OpenBlob.
  • ADR-021 (authorization): the addressed virtual repository's read_artifact role governs the read. The handler adds no check per upstream.
  • ADR-023 (code structure): the new files stay flat in internal/format/oci and import no sibling format package.
  • ADR-001, ADR-004, ADR-007, ADR-012: I found no conflict.

Testing

Spec coverage

Criterion (plan Step 16) Test
The list read uses the finder's id, on a fixture whose two ids differ TestVirtualBlobHandler_KeysTheListReadByTheFindersID, TestResolveVirtualRepository_ReturnsTheFindersIDAndWritesNothing, TestWireOCIWithStore_VirtualBlobRouteResolvesTheVirtualRepositorysUpstreams
A missing container_virtual_repositories row answers 500 TestVirtualBlobHandler_AMissingVirtualRowAnswers500BeforeAnyResolution, TestResolveVirtualRepository_AMissingVirtualRowAnswers500, TestWireOCIWithStore_AVirtualBlobRouteWithNoVirtualRowAnswers500
A departed client answers 499 with no Error record TestResolveVirtualRepository_AFinderErrorForADepartedClientAnswers499, TestVirtualBlobHandler_AnAbandonedResolutionAnswers499
A warm digest pull makes no upstream call TestWireOCIWithStore_AWarmVirtualBlobPullMakesNoUpstreamRequest
A malformed digest answers 400 DIGEST_INVALID before resolution TestVirtualBlobHandler_DigestGateAnswersBeforeAnyResolution
A cold full GET fills and streams under both delivery modes TestVirtualBlobHandler_Delivery (the two "cold full GET streams" rows)
A cold HEAD and a cold ranged GET drain, then answer from the row; ranged GET under redirect answers 307 TestVirtualBlobHandler_Delivery (the "cold HEAD" and "cold ranged GET" rows)
A Range on a HEAD answers 200 with the full headers, never 206 TestVirtualBlobHandler_Delivery (the warm and cold HEAD with Range rows)
A ranged request on a stored blob follows the delivery mode TestVirtualBlobHandler_Delivery (the two "warm remote range" rows)
A committed read closes its copy unread and answers from the row TestVirtualBlobHandler_Delivery (the four "committed-read" rows), TestWireOCIWithStore_CommittedReadVirtualFollowersAreAnsweredFromTheRow
A cold fill longer than server.timeouts.write completes and writes its row TestWireOCIWithStore_AColdVirtualFillOutlivingTheWriteTimeoutWritesItsCacheRow, TestVirtualBlobHandler_AResolutionOutlivingTheWriteTimeoutStillDeliversTheWholeBody
A winner's digest mismatch answers 503 with a handler Retry-After, caches nothing, and asks no lower upstream; a streamed GET is cut short TestWireOCIWithStore_AVirtualWinnersDigestMismatchServesNothingInItsPlace, TestVirtualBlobHandler_ADrainedDigestMismatchAnswers503UnderAHandlerRetryAfter, TestVirtualBlobHandler_AStreamedDigestMismatchCutsTheBodyShort
A relayed 429 keeps the upstream's own Retry-After TestVirtualBlobHandler_MapsAFailedResolutionThroughTheBlobPolicy
Distinct cold blobs fill in parallel up to max_concurrent_fills_per_repo TestWireOCIWithStore_DistinctColdVirtualBlobsFillInParallelUpToTheCap
Two virtuals and a direct read of their remote issue one upstream GET and one cache write TestWireOCIWithStore_ColdBlobReadsAcrossVirtualsAndTheirRemoteIssueOneUpstreamGET
Concurrent reads of one cold blob issue one upstream GET TestWireOCIWithStore_ConcurrentColdVirtualReadsOfOneBlobIssueOneUpstreamGET
The route arms the resolution budget, then re-arms from the instant Resolve returns TestVirtualBlobHandler_ArmsTheResolutionBudgetThenReArmsToTheTail, TestVirtualBlobHandler_ReArmsFromTheInstantResolveReturns
A ServerWriteTimeout above the tail sets the re-arm; one below leaves the tail TestVirtualBlobHandler_FloorsTheTailAtTheServerWriteTimeout
A re-arm that fails keeps the first arm's instant, which the connection still holds TestVirtualBlobHandler_RearmKeepsTheFirstInstant
A failed first arm skips the re-arm and logs the virtual route's own message, with no remote_repository_id TestVirtualBlobHandler_SkipsTheReArmWhereTheFirstArmFailed
A stream cut after the re-armed deadline logs msgVirtualStreamDeadline at Error, with a write error that is not a net.Error timeout; copyStored classifies against the same instant TestVirtualServe_ClassifiesAFailedStreamAgainstTheArmedDeadline
The builder reads each of the five budget terms from its own config field TestOCIVirtualBlobBudget_ReadsEachTermFromItsOwnField, TestOCIRemoteWiringFrom_CarriesTheClientAndBothBudgetsToTheirOwnFields
The download is booked on the supplying position TestVirtualBlobHandler_RecordsTheDownloadOnTheSupplyingPosition

TestVirtualBlobHandler_AStreamFailureBetweenTheTwoArmsIsNotADeadlineKill covers the hand-off between the two arms in one direction only. I accepted that gap.

Dropped subtests

Step 15's manifest suite has not landed, so this suite has no Step 15 suite to diff against. When Step 15 lands, it runs that diff against this suite.

Against the standalone remote_blob_test.go suite, this suite leaves out the criteria that Step 12's virtual_serve_test.go already covers on main. Those are range handling (TestVirtualServe_BlobRanges), stored-copy and mid-stream failures (TestVirtualServe_StoredCopyFailures, TestVirtualServe_StreamedBlobFailureInterruptsResponse, TestVirtualServe_RefusedStreamIsNotFramedAsCompleteToTheClient), and client hangup (TestVirtualServe_ClientHangupIsNotAborted, TestVirtualServe_ClientLeavingDuringAFailedCallIsNotInternal). Upstream status relays are covered by TestVirtualBlobHandler_MapsAFailedResolutionThroughTheBlobPolicy.

The MR removes TestWireOCIWithStore_VirtualBlobRouteAnswersItsInterim501, because the route no longer answers 501. TestWireOCIWithStore_AVirtualBlobRouteWithNoUpstreamsAnswersBlobUnknown covers the route in its place.

Local runs

  • Build, go vet (with and without -tags=integration), and the unit tests in internal/format/oci, cmd/artifact-registry, internal/virtual, and internal/config pass.
  • The OCI wiring integration tests and the datastore ContainerVirtual integration tests pass.
  • golangci-lint reports 0 issues on the changed packages in plain mode. With --build-tags=integration it reports 0 issues in this MR's files. The 11 it reports are in npm, maven, and dispatcher integration tests that this MR does not touch.
  • The run recipe's driver.sh smoke did not run: this machine gets a quay.io 401 for minio/minio.

Conformance

The conformance:oci:* jobs run on this pipeline, because the diff touches internal/format/oci. The suite pushes and pulls against a hosted repository. So it guards the shared serve code against a regression, but it does not drive the virtual route.

Size

The diff changes 2,884 lines: source 345, tests 2,433, docs and config 30, run recipe 76. That is past the 500-line guideline.

A split would not help. The write-deadline arming must land with the route, because the route cannot cache a cold layer without it. The tests ship with the code they cover.

The tests are about twice the plan's ~1,120-line estimate. The estimate dates from the plan's first draft. Three criteria arrived later: the finder criteria (Step 4a), the server.timeouts.write floor (!2941 (merged)), and the committed-read follower. The delivery table also crosses cache state, method, Range, delivery mode, and committed read.

File Lines
internal/format/oci/virtual_blob_test.go 1,448
cmd/artifact-registry/wire_oci_virtual_blob_integration_test.go 693
internal/format/oci/virtual_repository_test.go 129
cmd/artifact-registry/wire_oci_virtual_blob_test.go 125
cmd/artifact-registry/wire_oci_virtual_integration_test.go (the interim-501 test removed) 34
cmd/artifact-registry/wire_oci_remote_test.go 4

Notes for reviewers and later steps

  • Plan Files entry: it says "Their test files (Create)". wire_oci_remote_test.go and wire_oci_virtual_integration_test.go are modified instead.
  • Run recipe: the recipe moves with the route, and the smoke count stays at 80. Three blob checks replace the three interim-501 blob checks. Some edits fall outside Step 16's anchor, and the Step 15 author will meet them:
    • The smoke summary in SKILL.md now names the manifest route's interim answer on its own.
    • The seed paragraph, the manifest-route section, and the 501 troubleshooting row now name only the manifest route.
    • In driver.sh, the seed-container-virtual hint and the comments on check_container_virtual_manifest and the smoke block now name only the manifest route.
  • The token cache has a window where a cancelled leader affects its followers. #1453 (closed) tracks it.
  • e2e catalog: no scenario changes here. Step 18 owns the container virtual e2e catalog, and docs/testing/e2e/oci.md keeps virtual client journeys out of scope until then.
  • User docs: no change. docs/user/ says a Docker or OCI virtual repository accepts no upstreams yet, and that stays true. No route adds an upstream to one, so no customer can reach this route with an upstream. The end-of-plan docs-author pass covers S32.

Related to #291 (closed)

🤖 Generated with Claude Code

Merge request reports

Loading
Loading