feat(oci): kind dispatch, write 405s, 501 read stubs (S16 Step 10)

What

Step 10 of the S16 container remote slice: the OCI dispatcher's repository-kind branch. One seam decides, before any per-action delegation, what a resolved request gets based on repositories.kind — hosted keeps reaching the per-action handlers, remote answers its writes 405 UNSUPPORTED and routes its four reads to named 501 stubs, and virtual answers an interim 501, carrying no-store, on every route.

Those are the dispatcher's answers, and the dispatcher answers last: authentication and authorization decide ahead of it on every kind, and the namespace write gate decides inside it but ahead of the branch — so a mutating request on a suspended namespace is 403 DENIED whatever the kind. The spec's new "Gates ahead of the kind branch" states that precedence once, and the per-kind claims elsewhere in it are written against that section.

One consequence to read before the 405 rows below: the authorization route table maps only the method each upload action serves, so five of the ten blob-upload method-and-route pairs — PATCH/PUT/GET/DELETE on the no-id route, POST on the with-id one — are answered 403 before resolution on a deployed instance and never reach the dispatcher's 405. The spec lists them. Widening that table is not done here: the same five already 403 on a hosted repository, so it would move a surface older than this slice. A test composing the authorization middleware with the dispatcher, one case per pair, is the coverage that would pin this; it needs a cross-package harness this MR is too large to carry, so it is tracked in #566 rather than added here.

  1. serveByRepositoryKind branches on the Kind the resolution already carries, above every per-action delegation. Every arm is a tail call that writes the whole response, so no arm reports back to the branch and the two non-hosted kinds never reach a handler that reads the hosted container_* tables.
  2. Manifest PUT/DELETE and blob DELETE on a kind=2 repository answer 405 UNSUPPORTED with Allow: GET, HEAD. Every blob-upload route answers 405 UNSUPPORTED with an Allow that is present and empty, since no read verb is available on those routes either.
  3. The four remote reads — manifest, blob, tag list, referrers — each route to a distinct 501 stub in remote_stub.go, carrying no-store. Later steps replace them in groups rather than one at a time — Step 11 ships the manifest and blob cache-hit serve together, Step 16 both live lists — and the four bodies differ so that whichever grouping lands, a replacement cannot leave a survivor answering another arm's route with the suite still green.
  4. namespace.Resolver.ResolveRepository refuses a repositories.kind outside {0,1,2} with the new ErrRepositoryKindUnknown, and datastore.ValidRepositoryKind is exported as the one predicate over the taxonomy.

No upstream is contacted and no cache table is read: this step is the seam and its rejections only. The remote read arms are stubs until Step 11 lands the cache-hit serve path.

Plan: docs/plans/2026-07-30-container-remote.md, "Step 10: kind dispatch + write unavailability + 501 read stubs".

Notes for the reviewer

A kind=1 repository gets its own 501 arm rather than the hosted one, and this follows npm rather than Maven. The code first grouped virtual with hosted, which sent a virtual manifest PUT into the hosted push path and accepted a client's content into a repository whose content is meant to come from its upstreams. Keying on "not hosted" instead would have sent it to the remote arms, whose cache tables belong to remote repositories. Neither this spec nor S14 or S15 named an interim answer, so it was settled on shipped code, where the two siblings agree on a read and diverge on a write: npm answers 501 from its own arm, Maven answers 405 from an arm it shares with remote. This follows npm, because Maven's shared arm advertises Allow: GET, HEAD, naming read verbs a container virtual repository does not implement either. The reasoning is in the spec's ## Resolutions.

The blob-upload 405 sends an Allow that is present and empty, not an omitted one. RFC 9110 requires the header on every 405 and these routes retain no verb to name. S15 had already settled the identical case for write-only npm routes, so the spec sentence was corrected here rather than left ambiguous for the next format. Both halves are asserted — presence through the header map rather than Get, which cannot tell absent from empty.

The out-of-taxonomy kind guard sits at the resolver, and moving it there moved the client-visible answer. The check belongs at the boundary that admits the value rather than at the switch that reads it, because a switch returning early on its success case never reaches its own fail-closed arm. The consequence is worth stating plainly: the authorization middleware calls the same resolver ahead of the dispatcher wherever it is wired, so on a deployed instance it refuses first and answers its fail-closed 503 with no body, and the dispatcher's 500 INTERNAL is what a client sees only with no authorization middleware. The namespace and repository ids are logged by neither — the only arm that logs them is the dispatcher's, which a resolver-built resolution no longer reaches. Accepted rather than fixed, because check_repositories_kind makes the condition unreachable until someone widens it, and answering it through the format's own renderer would mean a kind-specific arm in shared authorization code. Both the Error Cases row and the ## Resolutions entry now say this, and the Error Cases row adds that a 503 is retryable to an OCI client, so whoever widens the constraint reads it before writing a runbook against a 500.

The plan's "construction-time guard against an unpopulated kind" was not buildable as written. datastore.RepositoryKindHosted is the int16 zero value, so an unassigned field is a valid hosted kind and no check tells the two apart. Both siblings hit the same wall and document the hole rather than closing it. The plan's Step 10 entry is amended to say what ships instead, and a test pins the zero value as taking the hosted arm so a renumbering that made the two distinguishable fails loudly.

One place a non-hosted repository still reaches hosted-table code, and S12 says to leave it there. resolveMountSource resolves a second repository from ?from= with no kind branch, then probes the hosted blob join with it. A kind=2 source owns no rows there, so the request takes the 202 source_missing fallback — which is exactly what S12 requires: mount returns 201 or 202 and no other status, and never an error attributable to the from value, to avoid leaking information about the source. Gating the source on kind with any error status would contradict that on three counts. Left as is, and named here because it becomes a real question once the cache tables land: whether a cached remote blob is mountable.

That S12 line is narrower after this MR than the sentence above implies, and the narrowing is this MR's. resolveMountSource falls back to 202 source_missing only on the two not-found sentinels; every other resolver error becomes a 500. The new ErrRepositoryKindUnknown is one of those others, so an out-of-taxonomy from value now is an error attributable to the from value — the one thing S12 says mount never answers. It is unreachable while the repositories CHECK constraint holds, which is why it is not gated here, but whoever widens that constraint meets it, and the sentence is written down rather than left to be rediscovered.

The log field is repository_kind_code, not repository_kind. The usage-data AR context already owns the latter for the string form of the same dimension, and the four sibling emitters use the suffix for that reason. The line first shipped as repository_kind, which an operator query grouping OCI logs by numeric kind would have missed.

No mixed-version deploy window to worry about. Old and new instances would answer differently for a kind=1 or kind=2 container repository, but the management API rejects any kind but hosted at creation and no production path wires the container remote store, so no such repository can exist yet. Nothing here persists a new field or changes a cache key.

Testing

internal/format/oci/remote_dispatch_test.go drives every (action, method) pair through the branch. Beyond the per-row assertions it pins three things no single row establishes: that the 405 and 501 come from the dispatcher itself, asserted as "no per-action spy was called" rather than as a status; that the four read stubs write four distinct bodies, and that none of them reuses the virtual arm's, since the two kinds share a status and code; and that the branch is keyed on the remote kind exactly rather than on "not hosted", with a positive-hit row for every value of the taxonomy including the unset one. A fourth pair of rows pins the write gate ahead of the branch: the same manifest PUT runs serviceable and suspended on each non-hosted kind, so the row names the 405 or 501 the gate takes precedence over. The regression guards — hosted routing, the resolver-miss tiers, path validation ordering — pass against the modified dispatcher with hosted behavior held fixed, which is what makes them guards: ServeHTTP was restructured here (base-endpoint extraction, a tuple-returning resolveAndGate, the kind branch), and the hosted answers did not move.

The fall-through table is exhaustive over the six methods the OCI distribution spec uses on these routes rather than a sample of them: it drives every (action, method) pair that reaches the default arm — the complement, per route, of the verbs that route's own case claims — so a remote-specific answer appearing on any of them fails the suite instead of shipping unobserved. OPTIONS, CONNECT, TRACE, and extension methods reach the same arm and are driven nowhere; the method set is open, so the claim stops at the six the table enumerates.

Assertions were mutation-checked rather than trusted green. Making the virtual arm fall through to hosted reds three per-kind rows; giving the upload 405 a non-empty Allow reds all ten upload rows; disabling the resolver guard reds the taxonomy-refusal subtest. The assertions added in review were checked the same way: dropping Cache-Control from a read stub or from the virtual arm, deleting the three attrs from the fail-closed log line, collapsing the remote 405 message into the base endpoint's, reverting the write gate to a constant false, and re-emitting the namespace id through slog.Any each red their own rows — the last of those while leaving the pre-existing value comparison green, which is why the attribute kinds are now asserted too.

Conformance: not run locally; the harness seeds only kind=0 repositories, so it cannot reach any arm this step adds. CI covers the hosted regression, which is the part that matters here — every hosted route now passes through the new branch before delegating.

e2e scenarios: no catalog change. docs/testing/e2e/oci.md already lists remote and virtual container repositories as out of scope until the capability ships, and the plan assigns the catalog update to Step 18.

Size

Over the 500-line ceiling in development-model.md and over the plan's own Est. for this step, which predates the three blockers that widened it. Most of the diff is the test matrix; production Go is the smallest of the three parts, with the spec and plan text between them.

One split does exist, and the plan names it: the dispatch is confined to the OCI package, the taxonomy guard is not. The guard is self-contained — handler.go never calls ValidRepositoryKind, and the only production caller is the resolver in internal/namespace/namespace.go — so it could have shipped on its own. It is not being split out now: the review rounds and the AppSec passes on this branch are all against the combined diff, and re-cutting it would discard that and re-open both halves.

What is not splittable is the rest. The seam is one switch plus the arms it dispatches to, and the arms are the point — a part that landed serveByRepositoryKind without its 405 and 501 bodies would merge a branch with nothing behind it. Splitting the tests off contradicts the guardrail that every MR ships tests for the code it introduces.

The test file's size is the enumerated matrix the mirrored-suite guardrail asks for: every write verb on every route that offers one, all ten blob-upload route-and-method pairs, all four read arms plus their distinctness, every value of the kind taxonomy as a positive hit on three routes each, both resolver miss tiers, and the fall-through rows proving a verb no kind offers gets the same answer hosted gives.

Review passes

Three passes ran on this branch:

  • A behavior-preserving simplification pass over the dispatcher and its suite.
  • /validate-step, which returned blockers on all three of the readings above — the virtual arm, the empty Allow, and where the out-of-taxonomy guard belongs. Each was settled against the shipped siblings rather than on local reasoning, and all three are written up in the spec's ## Resolutions.
  • /review-branch, which returned no blockers and four warnings, all fixed here: the Maven parity claim was wrong in three places, the suite's file header still described the pre-fix virtual behavior, the spec and ErrRepositoryKindUnknown's doc both published a 500 the deployed chain answers as a 503, and the log field took the reserved name. Its remaining observations are either fixed alongside or recorded above.

Follows up in #566 (the composed authorization-plus-dispatcher coverage this MR defers).

Related to #288

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading