chore(managementapi): the manifest read seams and their wiring (S17 Container Manifest Reads plan: 2/20)
Step 1 merged 2026-09-10, so this MR now targets main: test(managementapi): sweep the cached manifests... (!2506 - merged) • Hayley Swimelar • 19.4
Why
Step 2 of the merged S17 container manifest reads plan. All three manifest read routes need a manifest by digest and the raw route needs the payload bytes, so the seams and their composition-root wiring land here, ahead of every handler, and one interface change serves all three. No route is served, no response byte moves, and no log line moves.
What
Both by-digest store methods take an explicit db qrm.DB this package holds no
handle for, so neither store satisfies its widened seam directly any more. Two
adapters bind the pool per call (D10), following
containerRemoteEvictionResolverAdapter in the same file.
The adapters wrap the two new by-digest reads and forward the two list reads unwrapped. Both list arms are bound today and their handlers log the store's message, so a wrap there would move a shipped log line.
Deps.ContainerManifestPayloads has no reader until Step 20, so requireDeps
guards a seam no handler calls. The plan's accepted-smells section names it, and
the seam lands here so the wireManagementAPI signature change is reviewed
apart from the route it serves (D5).
Three things the plan's file list missed. The Deps seam count is 98 on main,
not the 97 the plan cites, so it moves to 99. read_handlers_integration_test.go
fills both manifest seams from bare stores and
container_remote_read_handlers_integration_test.go asserts one against a bare
store, so the integration rig restates both adapters, as it already does for the
eviction resolver.
Spec coverage
This step closes no acceptance criterion. It is the precondition for the three route steps, each of which reads through one of these seams.
| Seam | Criterion it serves | Step that closes it |
|---|---|---|
ContainerManifestReader.GetContainerManifestByDigest |
AC #128 (closed) detail, AC #134 (closed) referrers subject 404 |
6, 14 |
ContainerRemoteManifestReader.GetContainerRemoteManifestByDigest |
AC #137 (closed), the remote arm of all three | 6, 14, 20 |
manifestPayloadReader |
AC #136 (closed) raw payload | 20 |
Reviewable LOC
313 insertions and 41 deletions over 18 files: 129 source and 184 test. Under the 500-line guardrail.
Test plan
go build ./...andgofmt -lclean.go vetandgolangci-lint runclean on./cmd/...and./internal/managementapi/..., andgo vetalso underintegration,contract,integration,accountingfaults, andintegration,realbackend.- Integration lint over
./cmd/...reports 11 findings in four files this MR does not touch.--new-from-revagainst the parent branch reports 0. TestWireManagementAPI_ProductionEntryPointpasses against the new signature,TestWireManagementAPI_PanicsOnNilDependenciesgains the payload-store row, theDepsseam count reads 99, andTestContainerHandlers_ResponsesMatchOpenAPIContractpasses unchanged.- Unit and integration suites pass for both packages, the integration ones
against a Postgres 17 tuned to CI's
max_locks_per_transaction. scripts/ci/check-comment-caps.sh --base origin/mainreports OK.
Context for LLM agents
Why one seam per direction rather than one per read. D10. Steps 12, 14, 16,
and 18 each widen ContainerManifestReader with one more method, and a per-read
seam would give each its own Deps field, so each would write the same bump to
the seam-count literal with no dependency edge between them. Two identical edits
to one line merge clean and leave the count one short of the truth, and
requireDeps panics at construction rather than failing to compile.
containerRemoteManifestReaderAdapter is finished here: no later step widens the
remote seam.
Why the adapters name their store as a field rather than embedding it.
Embedding promotes the store's writers into a reader adapter, and it leaves the
promoted GetContainerManifestByDigest(ctx, db, ...) shadowed by the adapter's
own method of the same name and a different signature, which is legal Go that
reads as a mistake.
Why the wrap is asymmetric. The two list arms were bound directly to their
stores before this MR, and their handlers pass the error to
logAndWriteInternalError, so wrapping them would change a shipped log string
for no reader. The two by-digest arms are new, and the pool resolution behind
them is a failure worth naming, which is what
containerRemoteBulkManifestBinding does for the same read. %w keeps
datastore.ErrNotFound reachable either way.
Why managementPool(a.client) per call rather than a db func() qrm.DB
provider. client.DB() is nil until the database component starts, which is
after wireManagementAPI runs, so the handle cannot be resolved at wire time
either way. Both shapes resolve per call.
containerRemoteEvictionResolverAdapter in this same file takes the client,
manifestRowReaderAdapter in wire_oci.go takes a provider, and the same-file
precedent won. managementPool returns an explicitly nil interface rather than
a boxed typed nil, so the store's db == nil guard still fires.
Why the payload parameter is storage.BlobStore. The consumer interface is
unexported, so cmd/artifact-registry cannot name it. The parameter takes the
shared storage.BlobStore wire.go already builds and passes to wireOCI, and
it sits third to mirror wireOCI's (srv, client, store, containerCfg, ...)
order. Its nil guard runs last, so every earlier guard's case keeps the message
it pins.
Why the hosted fake carries no zero-namespace guard where the remote one
guards its ids. The doc on errContainerZeroID in
fake_container_readers_test.go records the decision: the stores' zero-namespace
guards are left out because the namespace id is read off a resolved namespace row
rather than off the URL, and only the two Find methods restate the sentinel. The
remote fake's guard mirrors the remote store, whose own doc says it departs from
the hosted twin deliberately.
Non-goals. No forwarding method for Steps 12, 14, 16, or 18: each adds
exactly one later. ListParentDigestsByChild is untouched (D8). No new suite:
the plan makes the fakes, the wiring fixtures, and the seam count this step's
tests, and the adapters are exercised by the existing wiring tests. The plan file
and its Status table are untouched, per the single-writer rule.
Related to #1150 (closed)