feat(managementapi): serve the container artifact reads

Why

The five container artifact routes answered 501. The monolith repository-detail and artifact-detail slices need the container_images, container_tags, and container_manifests rows the S12 push path writes, and Step 3 seeded these placeholders for this step to fill.

This is Step 10 of the merged plan in docs/plans/2026-07-22-s17-phase3-format-artifact-reads.md, covering the container handlers: images list and detail, tags list and detail, and the per-image manifests list with its include_referrers toggle.

Retargeted to main. This step depended on Steps 1, 3, 4, and 5, and all four have merged, so it no longer needs the integration branch. Rebasing onto main absorbed three changes those steps made after this branch forked: the per-family resolver split, the kind return FindByNameInNamespace gained, and the staged markers Step 1's contract carries.

What (the non-obvious parts)

Query parameters are validated before the repository resolve. Resolving first would answer 400 for a repository the caller can see and 404 for one it cannot, which turns ?limit=0 into the existence oracle the rest of the surface denies. The 400 tables assert zero store calls, the repository reader included.

A container-format repository with no container_repositories row is a logged 500, not a 404. RepositoryStore.Create writes parent and child in one transaction, so a miss is a broken invariant, and folding it into the existence-hiding 404 would report every image of that repository absent while the defect stayed invisible.

The two child lists read their parent image first, and tag detail does not. The lists need it for the 404-versus-200 [] distinction the spec draws, and the manifests list needs it for isolation as well, because container_manifests carries no repository column and its store scopes by namespace and image alone. Tag detail's own query joins up to the image and pins the repository, so one round trip verifies the whole chain.

Three shared test files gained a per-step block. The new Deps fields are required, so every construction site (newTestDeps, newIntegrationHandler, managementWireDeps) needs them. The two 501 route sweeps now discriminate per row, the wire sweep by an expected status and the handler sweep by a skip flag, so Steps 11 and 12 flip only their own rows. Integration coverage went into a new file rather than the shared one for the same reason.

The diff is 4066 lines across 18 files, over the 500 ceiling. 1113 of that is production code, much of it doc comments, and the rest is the test suite this step's acceptance criteria call for.

Test plan

go test ./internal/managementapi/ ./cmd/artifact-registry/
go test -race ./internal/managementapi/
ARTIFACT_REGISTRY_DATABASE_TEST_DSN=... go test -tags=integration ./internal/managementapi/
go-lint-ci ./internal/managementapi/... ./cmd/artifact-registry/...
go-lint-ci --build-tags=integration ./internal/managementapi/...

Ran locally: unit green, -race green, managementapi integration green (14 s) against PostgreSQL via testcontainers, both lint runs report 0 issues. The contract sweep validates all thirteen container response classes against api/openapi/v1.yaml.

Conformance: not applicable. This is management-API surface, not Maven, npm, or OCI protocol behavior.

E2e scenarios: no catalog change. The plan's Testing Strategy settles it. The catalogs cover protocol-client journeys, and artifact browsing is a UI journey that lands with the monolith S05 and S06 slices, which own the catalog additions.

Spec coverage

Spec Implementation Tests
AC #15 images list by name with keyset pagination, detail fields, missing id 404 handleContainerImageList, handleContainerImageDetail TestContainerImageList_Defaults, ..._KeysetWalk, TestContainerImageDetail_Returns200, ..._NotFound_IsOneResponse, TestContainerReadsIntegration_ImageKeysetWalk
AC #16 (closed) tags list by name, detail carries manifest_digest, missing name 404 handleContainerTagList, handleContainerTagDetail TestContainerTagList_Defaults, ..._KeysetWalk, TestContainerTagDetail_Returns200, ..._NotFound_IsOneResponse, TestContainerReadsIntegration_TagDetailJoinsManifestDigest
AC #17 (closed) manifests list newest-first with the eight documented fields handleContainerManifestList, containerManifestFromRow TestContainerManifestList_Defaults, ..._ResourceFields, ..._Order
AC #18 (closed) referrers excluded by default, returned on include_referrers=true, any other value 400 parseContainerManifestListQuery TestContainerManifestList_IncludeReferrers, ..._ReferrerFields, ..._WalkKeepsIncludeReferrers, TestContainerReadsIntegration_ManifestReferrerPartition
AC #24 format-segment mismatch, an id outside the chain, and a non-UUID id all 404 resolveContainerRepository, findContainerImageForRequest, parsePathID TestContainerImageList_RepositoryResolution, TestContainerImageDetail_NotFound_IsOneResponse, TestContainerTagList_ImageOfAnotherRepository_Returns404, TestContainerReadsIntegration_FormatSegmentMismatch, ..._ImageDetailIsRepositoryScoped
AC #25 an existing parent with no children returns 200 [] parent read before each list TestContainerImageList_EmptyPage_ReturnsEmptyJSONArray, TestContainerTagList_TaglessImage_Returns200Empty, TestContainerManifestList_ParentResolution
AC #26 (closed) invalid sort, order, limit, or cursor returns 400 with the S01 envelope the two query parsers TestContainerImageList_InvalidParams_Return400, TestContainerTagList_InvalidParams_Return400, TestContainerManifestList_InvalidParams_Return400
AC #27 (closed) hosted responses carry no cache key the three DTOs the exact-key assertions in TestContainerImageDetail_Returns200, TestContainerTagDetail_Returns200, TestContainerManifestList_ResourceFields

AC #23 (soft-delete invisibility) does not apply: no container table carries soft_deleted_at.

Context for LLM agents

Design rationale

Resolve the repository through Step 3's shared helper, then the child row separately. Every container query is scoped by container_repository_id, which lives on container_repositories and not on the shared repositories row, so each request costs two by-name lookups. Rejected: resolving through ContainerRepositoryStore.FindByNameInNamespace alone, which returns the format and would collapse the two into one query. It would bypass resolveContainerArtifactRepository, so the format-mismatch semantics and the unmapped-enum 500 would diverge from Steps 11 and 12, and it would duplicate logic Step 3 landed and tested. Collapsing the two properly needs a datastore method keyed on repository_id, which is Step 4's scope.

One shared parse function for the images and tags lists. Both take the identical vocabulary (sort=name, order, limit, cursor), so the parsed form carries a plain descending bool that each handler maps to its own store's order enum. The two stores declare separate order types, which is what would otherwise force two parsers.

Cursor codecs and Link builders are container-scoped, not shared with Steps 11 and 12. They are generic over the row and cursor type, so the sibling steps could reuse them, but naming them for the whole artifact surface would have made three concurrent branches declare the same package-level symbols. Unifying them once all three land is a follow-up worth taking.

Every new package-level symbol is container-prefixed, including the sort-value constants. containerSortValueName rather than sortValueName, because the packages list also sorts by name and Step 11 would plausibly declare the unprefixed form. In the previous round two steps independently declared negativeLimitCase in one package, both MRs were individually green, git reported no conflict, and the package still failed to compile once both landed.

A 500 for a missing child row, weighed against a 404. The 404 would be consistent with the surrounding existence-hiding, and it is what a caller would see for any other chain break. It loses because the chain break here is impossible through the write path: answering 404 would silently report an entire repository empty while a data-integrity defect went unlogged. The 500 is logged with the store's wrapped cause.

Non-goals

  • No datastore change. Steps 4 and 5 own the reads this calls, including the AppSec fix that added container_repository_id to ListContainerTagsParams.
  • No OpenAPI schema change. Step 1 landed the contract for all thirteen routes. This step only clears the staged markers on the five it serves: x-not-implemented and the "Not yet implemented." description lead. TestContractOperations_RegisteredOrMarkedStaged enforces both directions, so a marker left on a served route fails as loudly as one cleared with no handler behind it.
  • No narrowed manifest projection. ListContainerManifests projects annotations, up to roughly 520 KB per row, which the resource withholds. Step 5 documented and accepted that, and the handler clamps limit to 100. Changing it needs a row type of its own, in the store.
  • No plan Status table edit. Three sibling steps would edit the same rows. It gets filled once, afterwards.

Pipeline state

Both failures this section previously described are gone on main. The golangci_lint job failed on the integration branch because Steps 6 through 9 each crossed goconst's three-occurrence threshold only once the others were present; those steps have landed and the findings were fixed with them, so go-lint-ci ./... is clean here. The project-wide Danger gem break (Base is not a class) is also fixed, so danger-review runs normally.

Known local-gate deviations

  • TestWireStorage_CloudCDNPresent in cmd/artifact-registry fails locally with "could not find default credentials". Pre-existing, needs GCP ADC, and wire_storage_test.go is byte-identical to the target branch.
  • golangci-lint --build-tags=integration ./cmd/artifact-registry/... reports 8 issues, all in wire_npm_* and wire_oci_boot_* files this diff does not touch. CI never lints integration files. The same run over ./internal/managementapi/... is clean.

Merge-order notes for a sibling step

Six files carry edits Steps 11 and 12 also need. The four anchored regions of wire_management.go and the per-step blocks in handler.go (the Deps struct, requireDeps, and the route table) should merge cleanly.

The rest will not. An actual merge of Steps 10 and 11 conflicts at all three test construction sites, and handler_test.go conflicts for certain: Step 11 adds the same implemented field and sweep guard with different comments, and Step 12 uses a name switch rather than the flag, so the two mechanisms do not converge without a hand merge. Whoever merges second should expect to resolve those by hand rather than trusting a clean apply.

wireManagementAPI's doc comment also lost its "no artifact store is constructed here yet" clause, which this step made false.

Heads-up on a landed-after dependency: refactor(datastore): jet-convert container_mani... (!1230 - merged) • Hayley Swimelar • 19.3 narrows the manifests-list row type, which retypes this MR's manifest seam and stales the annotations sentence in container_resources.go. That MR promises to absorb the retype, so nothing is owed here, but the next reader of this seam should know it moves.

Related to #312 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading