feat(oci): manifest GET and HEAD content negotiation (S12 Step 13)

Why

S12 Step 13 implements Manifest Pull: GET and HEAD on manifests/<reference> with Accept content negotiation, by-tag and by-digest resolution, and the HEAD headers-only contract. This is the read counterpart to Step 12's manifest push (merged). Without it a pushed manifest cannot be retrieved, so docker pull and oras pull cannot resolve a tag or digest to its manifest body.

Built on main (Steps 1-12 merged). The read path is independent of the other Phase D steps (14 DELETE, 16 mount): it reads manifests persisted by Step 12 through the Step-3 datastore method, wired at the Step-6 dispatcher.

What (non-obvious)

  • Content-Length sources from the payload blob, not container_manifests.size. The row's size is the tree size (payload plus the sum of child sizes). Content-Length must be the manifest payload's own byte length, read from storage (OpenBlob().Size() on GET, BlobInfo on HEAD). An integration test caught the initial wrong read against the row size.
  • Absent Accept header is treated as permissive (*/*, returns 200). The spec text ("the stored type is in the client's Accept list, or Accept is */*") is silent on a missing Accept header. The code takes the RFC 9110 reading: no Accept means accept the stored type. Reviewer: please confirm against the spec author's intent before merge.
  • The read streams stored bytes verbatim, with no re-parse. S-6 (manifest payload size-limit and JSON validation) is owned by Step 12's push path. The read path does not re-parse, so S-6 is not re-verified here. The byte-identity checks assert no mutation instead.
  • The dispatcher gate widened one line. isManifestRequest (handler.go) now delegates GET and HEAD. This sits just outside the plan's Step 13 Files: list (manifest.go, store.go), and is required for the endpoints to be reachable.

Four commits: the two-agent flow lands a failing test skeleton, then the implementation. Two test(oci): commits apply pre-push review findings: redirect (307) and mid-stream copy-failure coverage the streaming fakes did not exercise, plus a table-naming alignment.

Test plan

go test ./internal/format/oci/... (unit and DB-backed integration), go vet (default and integration tags), and golangci-lint 2.12 all pass. The manifest-pull specs run under conformance:oci.

Spec coverage (S12 Step 13 slice: AC-22, P-1, E-8, S-6):

# Criterion Tests
AC-22 Content negotiation: stored type in Accept (or */*) returns the manifest, mismatch is 404 TestManifestGet_ContentNegotiation (8 Accept scenarios), TestManifestGet_ByTagAndByDigest, TestManifestHead_MismatchReturns404
P-1 Roundtrip integrity: GET returns byte-identical payload, Docker-Content-Digest and Content-Type match the stored media_type TestManifestGet_ContentNegotiation, TestManifestGet_ByTagAndByDigest, TestStoreGetManifest_TagPull_NullAnnotations, TestStoreGetManifest_DigestPull_NullAnnotations (integration)
E-8 MANIFEST_UNKNOWN 404: not found by tag or digest, or type not in Accept TestManifestGet_ContentNegotiation (mismatch rows), TestManifestGet_TagNotFoundReturns404, TestManifestGet_DigestNotFoundReturns404, TestManifestHead_MismatchReturns404
E-4 DIGEST_INVALID 400: malformed sha256: reference (bonus) TestManifestGet_MalformedDigestReturns400
E-10 NAME_UNKNOWN 404 at the image tier (bonus) TestManifestGet_ImageNotFoundReturns404NameUnknown
S-6 Manifest payload validation Owned by Step 12 (push parses and size-checks). The read path streams verbatim. Byte-identity in TestManifestGet_ContentNegotiation and TestStoreGetManifest_TagPull_NullAnnotations asserts no mutation.
S-9 Invariant 9: no internal detail in error envelopes TestManifestGet_ReadErrorReturns500 (no SQL, host, PK, or object-path leak in the 500 body)
S-10 Invariant 10: X-Content-Type-Options: nosniff on every response TestManifestGet_NoSniffHeader

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand the row for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.ResolveManifestDigest Limit → Nested Loop container_tags_pNN_…_name_idx, container_manifests_pNN_pkey 1 / 1 16.62 0.044 ms 6 / 0 1 + 1
datastore.ResolveManifestDigest

Summary: Plan matches the method's intent. The namespace_id literal prunes container_tags to one of 64 hash partitions, where an Index Scan on the (namespace_id, container_image_id, name) index finds the tag, and the join's namespace_id equality propagates the same literal to container_manifests, pruning it to one partition too and resolving the digest by primary key. The container_image_id equality lands as a Filter on the single PK-matched manifest row (the method's self-defending cross-image guard), which costs nothing. Plan and actual rows match (1 / 1) and execution stays at 0.044 ms over 5000 seeded tags and manifests, all buffer hits. No anomalies.

Seed shape: namespaces=1, repositories=1, container_repositories=1, container_images=1, blob_storage_blobs=1, blob_storage_attachments=1, container_manifests=5000, container_tags=5000

Rendered SQL:

SELECT container_manifests.digest AS "container_manifests.digest"
FROM public.container_tags
     INNER JOIN public.container_manifests ON (((container_manifests.id = container_tags.container_manifest_id) AND (container_manifests.namespace_id = container_tags.namespace_id)) AND (container_manifests.container_image_id = container_tags.container_image_id))
WHERE ((container_tags.namespace_id = $1::uuid) AND (container_tags.container_image_id = $2)) AND (container_tags.name = $3::text)
LIMIT $4;

Bound args: [<namespace_id uuid>, <container_image_id>, 'review-prep-tag-002500', 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.56..16.62 rows=1 width=33) (actual time=0.023..0.024 rows=1 loops=1)
   Buffers: shared hit=6
   ->  Nested Loop  (cost=0.56..16.62 rows=1 width=33) (actual time=0.022..0.023 rows=1 loops=1)
         Buffers: shared hit=6
         ->  Index Scan using container_tags_p50_namespace_id_container_image_id_name_idx on container_tags_p50 container_tags  (cost=0.28..8.30 rows=1 width=32) (actual time=0.013..0.013 rows=1 loops=1)
               Index Cond: ((namespace_id = '…'::uuid) AND (container_image_id = '…'::bigint) AND (name = 'review-prep-tag-002500'::text))
               Buffers: shared hit=3
         ->  Index Scan using container_manifests_p50_pkey on container_manifests_p50 container_manifests  (cost=0.28..8.30 rows=1 width=65) (actual time=0.008..0.008 rows=1 loops=1)
               Index Cond: ((id = container_tags.container_manifest_id) AND (namespace_id = '…'::uuid))
               Filter: (container_image_id = '…'::bigint)
               Buffers: shared hit=3
 Planning:
   Buffers: shared hit=658
 Planning Time: 2.241 ms
 Execution Time: 0.044 ms

Timings: planning 2.241 ms, execution 0.044 ms, total 2.285 ms.

Related to #19 (closed)

Context for LLM reviewers

Design rationale.

  • Content-Length from the payload blob, not the row. container_manifests.size is the tree size (payload plus children) used for quota and GC accounting. The HTTP Content-Length for a manifest GET or HEAD is the payload's own length. Sourcing it from the blob (OpenBlob().Size() or BlobInfo) is correct. Sourcing from the row over-reports for any manifest with children (an image index). Rejected: read the row size and adjust. There is no reliable adjustment. The payload length is authoritative.
  • Absent Accept means */*. Rejected alternative: treat a missing Accept as a 404 mismatch. That breaks real clients (curl, older Docker) that omit Accept and expect the stored manifest. RFC 9110 §12.5.1 defines a missing Accept as "all media types acceptable". Flagged in the description for spec-author confirmation because S12's Manifest Pull text does not address the missing-header case.
  • isManifestRequest widening. Step 12 left the dispatcher gate returning 501 for manifest GET and HEAD. Routing the new handlers requires widening that one predicate. It is outside the plan's literal Files list for Step 13 but unavoidable for the endpoints to be reachable.
  • headResponseWriter body suppression. HEAD reuses the GET path and wraps the ResponseWriter to drop the body while emitting identical headers, rather than duplicating the handler. Header parity is asserted by TestManifestHead_GetParity.

Non-goals (deferred, not omissions).

  • Manifest DELETE: Step 14.
  • Garbage collection, soft-delete, reachability: S20 (this endpoint reads only).
  • The GET redirect (307) delivery mode is implemented per S06 but exercised only by the added store-seam test. Neither in-memory fake returns a redirect, mirroring the blob handler's coverage shape.
  • operation-field alignment for the OCI wide-event emitter family: a cross-step observability follow-up, same as Step 17.
Edited by Hayley Swimelar

Merge request reports

Loading
Loading