feat(oci): manifest push endpoint (S12 Step 12)
Why
S12 Step 12 implements the OCI manifest push endpoint (PUT /v2/<slug>/container/<repo>/<image>/manifests/<reference>). Without it the dispatcher leaves manifest PUT on the interim 501 and the manifest-push conformance specs fail.
Per the OCI-hosted plan Step 12 and S12 Manifest Push: a 7-step flow validates an untrusted manifest payload (size, type detection, digest, references, subject and annotations) then persists it transactionally (6a payload write to CAS, 6b the database transaction), returning 201. This is the highest-risk step in the OCI surface: a security boundary (untrusted JSON parsing), data integrity (the persist transaction), and a complex state machine in one endpoint.
Review requirement (plan Step 12): two L3 reviewers, at least one with a parser-attack or security lens, distinct from the test- and implementation-authors.
Size. ~850 LOC of production code, over the 500 reviewable-LOC target; the flow, error accumulation, and persistence land together so the contract reviews end-to-end. The rest is the unit, property, fuzz, and integration suite.
What (non-obvious)
- The flow (
push_flow.go) is a pure state machine: it accumulates inputs, outputs, and errors but performs no I/O. Side effects (SHA-256, existence queries, storage writes) live in the handler, which calls the flow for transitions, so the flow is unit-testable without fixtures. - The 6b transaction lives in
datastore.ContainerManifestPersister(mirrors Step 9'sContainerBlobLinker), keeping raw SQL in the datastore layer per ADR-023; the format package never importsdatabase/sql. - Image Index size (
own_payload + Σ child tree sizes) is derived from the same batchedChildManifestsExistquery that proves existence: onecontainer_manifestsquery per push regardless of child count (TestImageIndexSizeComputationConstantQueriesguards it). Per-descriptor existence is oneWHERE digest = ANY(...)per target table, never per-row. - The manifest cap (25,000) is race-bounded at N-1 overshoot under N concurrent pushes (closing it would need an image-row lock on the push hot path); the tag cap is race-free via
SELECT ... FOR UPDATEon the manifest row. - Partial failure: if 6a (CAS write) succeeds and 6b fails, the staged payload is an orphan reclaimed by ADR-011, and the handler returns 500 with
oci.manifest.partial_persist=trueon the wide event. Cleanup is not retried inline. - Conformance stub FK fix (cross-workstream). S06 Step 6 added a validating FK from
blob_storage_attachmentstoblob_storage_blobs. The conformance harness runs the in-memory storage stub over a real database, so the parentblob_storage_blobsrow lived only in memory while the format-side attachment inserts (blob upload and manifest push) hit real Postgres, failing the FK and returning 500 on every finalize. That cappedmainat 19/80 and is not introduced by this MR. Thedevelopment_stubswiring now mirrors each committed row into the database through a newdatastore.BlobStorageBlobStore(plus a stubWithBlobRowPersisterhook), transitional until the real S06 BlobStore writes the row itself at Step 18, whereBlobStorageBlobStore.Createis the writer it reuses.
Test plan
go test -race ./internal/format/oci/... ./internal/datastore/... ./cmd/artifact-registry/...and the-tags=integrationsuite pass locally.- A rapid state machine drives the 7-step flow; a rapid property pins the tree-size invariant;
FuzzManifestParsefuzzes the parser (now wired into thefuzz:ocijob). - Conformance:
conformance:ocirises from 19/80 to 47/80 (job 14882476049), past the 41/80 pre-regression baseline. The manifest-push specs turn green, and the stub FK fix (under "What") restores the blob-upload specs a main-wide regression had cascaded to 500. The 28 remaining failures are all501for not-yet-built endpoints (manifest GET/HEAD/DELETE, content discovery), and the job staysallow_failureuntil Step 18.
Spec coverage
Spec: docs/specs/S12-container-oci-hosted.md
| # | Item | Tests |
|---|---|---|
| AC-6 | Manifest push (OCI Image Manifest by tag) | TestManifestPush_ByTag |
| AC-7 | Manifest push (OCI Image Index) | TestManifestPush_ImageIndex, TestImageIndexSizeComputationConstantQueries |
| AC-8 | Manifest push (with subject, subject exists) | TestManifestPush_SubjectExists |
| AC-9 | Manifest push (with subject, subject missing) | TestManifestPush_SubjectMissing |
| AC-10 | Manifest push (Docker Schema 2) | TestManifestPush_DockerSchema2 |
| AC-11 | Manifest push (Schema 1 rejection) | TestManifestPush_Errors, TestDetectManifestType |
| AC-12 | Manifest push (missing blob) | TestManifestPush_Errors, TestManifestPush_MissingConfigAndLayer |
| AC-13 | Manifest push (size limit, 413) | TestManifestPush_Errors |
| AC-14 | Manifest push (error accumulation) | TestPushFlow_ErrorAccumulation, TestManifestPush_MissingConfigAndLayer, TestManifestPush_MalformedBodyNoPhantomReferenceErrors, TestStateMachinePushFlow |
| AC-30 | Manifest push (by digest only) | TestManifestPush_ByDigestOnly |
| P-4 | Push idempotency | TestManifestPush_Idempotent, TestPropertyPushIdempotency |
| E-5 | MANIFEST_BLOB_UNKNOWN (400) |
TestManifestPush_MissingConfigAndLayer, TestManifestPush_Errors |
| E-6 | MANIFEST_INVALID (400) |
TestManifestPush_Errors, TestDetectManifestType_UnsupportedSchema |
| E-7 | MANIFEST_INVALID (413) |
TestManifestPush_Errors |
| E-13 | MANIFEST_LIMIT_EXCEEDED (400) |
TestManifestPush_ManifestLimitExceeded, TestManifestPushManifestLimitExceededIntegration, TestManifestPushConcurrentOvershootBounded |
| E-14 | TAG_LIMIT_EXCEEDED (400) |
TestManifestPushTagLimitExceeded, TestManifestPushTagLimitExceededIntegration |
| S-6 | Manifest payload 4 MB, JSON-only | TestManifestPush_Errors, FuzzManifestParse |
| S-9 | Manifest size limit, reject before parse | TestManifestPush_Errors |
| S-10 | Max references per manifest (200) | TestManifestPush_ReferenceCountOverCapSkipsStep4, TestPushFlow_ReferenceCountExceededGatesStep4 |
| S-13 | Digest verification on writes | TestPushFlow_CanEnter_DigestVerifyAlwaysRuns, TestManifestPush_ByDigestOnly |
Context for reviewers and agents
The fix(oci): address Step 12 manifest-push validation findings commit applies pre-push /validate-step findings: it enforces schemaVersion == 2 on the header- and mediaType-driven detection paths (the gate ran only on the structural fallback, so a valid Content-Type accepted any schemaVersion), rejects a subject descriptor that omits size (was indistinguishable from size: 0 because the parsed size was a plain int64), and threads the observed count into the MANIFEST_LIMIT_EXCEEDED / TAG_LIMIT_EXCEEDED detail (store.go had dropped it for a bare sentinel). It also adds the plan's named TestImageIndexSizeComputationConstantQueries guard and wires FuzzManifestParse into CI.
Non-goals (deferred, not omissions):
- Per-repository counter accounting (
repositories.size_bytes,last_updated_at) — deferred to S22. - GC coordination and manifest-push serialization locks — deferred to S20; the 25,000 manifest cap is intentionally race-bounded at N-1 overshoot.
- Soft-delete — S12 is hard-delete-only per the plan's Decisions; the
container_*tables have nosoft_deleted_at. - Inline orphan cleanup on a 6a/6b partial failure — the staged payload is reclaimed by ADR-011 reconciliation, surfaced via
oci.manifest.partial_persist.
ADR-004 publishes a 250 KB manifest cap; the merged S12 spec sets 4 MB (OCI Distribution SHOULD-level). The code matches the spec; reconciling ADR-004 is a separate handbook-repo change.
Related to #19 (closed)