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 ./... and gofmt -l clean. go vet and golangci-lint run clean on ./cmd/... and ./internal/managementapi/..., and go vet also under integration, contract, integration,accountingfaults, and integration,realbackend.
  • Integration lint over ./cmd/... reports 11 findings in four files this MR does not touch. --new-from-rev against the parent branch reports 0.
  • TestWireManagementAPI_ProductionEntryPoint passes against the new signature, TestWireManagementAPI_PanicsOnNilDependencies gains the payload-store row, the Deps seam count reads 99, and TestContainerHandlers_ResponsesMatchOpenAPIContract passes 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/main reports 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.

🤖 Generated with Claude Code

Related to #1150 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading