chore(datastore): container image and tag reads (S17 Phase 3 Step 4)
Why
The management API addresses container images by id under a repository URL and tags by name under an image, so its reads have to verify that chain. Neither read on main can. ContainerImageStore.FindByID matches (namespace_id, id) alone, so it would serve an image belonging to another repository of the same namespace, and ContainerTagStore.ListByImage returns names only, in a case-insensitive order the spec's sort vocabulary does not offer.
This is Step 4 of the merged plan in docs/plans/2026-07-22-s17-phase3-format-artifact-reads.md, covering the container image and tag datastore reads. The handlers that call them are Step 10, so nothing is wired yet. Related to S17 Phase 3: format artifact reads (#312 - closed) • Hayley Swimelar
What (the non-obvious parts)
Every read verifies its whole parent chain in the query rather than relying on the handler to have resolved the parent first. The handler still does that resolution, because the 404-vs-200 [] distinction needs it, but nothing enforces the call order and Steps 10 to 12 wire the handlers separately from this store. The tags list originally left the repository scope to that contract. The AppSec review flagged it, and the fix is in, verified to leave the keyset on its unique index.
fk_container_tags_container_manifest_id_container_manifests keys only (container_manifest_id, namespace_id), so nothing in the schema stops a tag from pointing at another image's manifest. The container_image_id equality in the manifest join is what keeps such a row out of both reads, and TestContainerTagStore_ManagementReads_ExcludeCrossImageManifest fails if the predicate is dropped.
Both cursors carry name alone, because it is unique within the parent scope. An id tiebreaker would only add a post-scan filter.
The diff is 2302 reviewable LOC, over the 500 ceiling. 692 of that is production code, most of it doc comments, and the rest is the test suite this step's acceptance criteria call for. The plan already splits container datastore reads across two MRs, with manifests in Step 5.
Test plan
go test ./internal/datastore/
ARTIFACT_REGISTRY_DATABASE_TEST_DSN=... go test -tags=integration ./internal/datastore/...EXPLAIN assertions pin each list to its unique index with no post-scan Sort and every joined table pruned to one hash partition. Ran locally against PostgreSQL 16.14: unit green, internal/datastore integration green (194 s), migrations green (316 s).
Spec coverage
| Spec | Implementation | Tests |
|---|---|---|
| AC #15 images list by name, keyset-paginated | ListContainerImages |
TestContainerImageStore_ListContainerImages, ..._KeysetWalk |
AC #15 image detail with last_downloaded_at, missing id not found |
FindContainerImageInRepository |
TestContainerImageStore_FindContainerImageInRepository |
| AC #16 (closed) tags list by name | ListContainerTagsPage |
TestContainerTagStore_ListContainerTagsPage, ..._KeysetWalk |
| AC #16 (closed) tag detail carries the tagged manifest's digest | FindContainerTagByName |
TestContainerTagStore_FindContainerTagByName (digest subtest on the list) |
| AC #24 an id outside the chain is not found | repository and image scoping on every read | ..._ChainScoping, the detail cross-repository and cross-namespace subtests, ..._ExcludeCrossImageManifest |
| AC #25 an existing parent with no children lists empty | both lists return an empty page | ..._ChainScoping empty-repository and tagless-image cases |
AC #26 (closed) invalid sort, order, limit, or cursor |
handler-side validation (the Phase 1 parseListQuery pattern) maps to 400. The store's typed guards are the backstop, a loud 500 if ever reached |
..._ArgumentGuards, TestContainerImageSortColumn_Valid and siblings |
| Artifact lists keyset backing | the four statement builders | ..._DeepPageIsIndexBacked, Find*Stmt_SinglePartitionPrune |
Context for LLM agents
Design rationale
Return jet models, not the existing ContainerImage struct. The management image resource needs last_downloaded_at, which ContainerImage omits. Adding the field would change a type the protocol write path returns, and the plan keeps protocol methods untouched. RepositoryStore.List already returns []model.Repositories, so []model.ContainerImages is the established shape.
Every read chain-verifies, including the lists. The first cut left the tags list scoped to namespace and image only, on the reasoning that the handler resolves the image through FindContainerImageInRepository anyway. The AppSec review called that an unenforced caller contract, which it was, so the list now takes containerRepositoryID too. The original objection was that a second join could cost the paginated read its index-only keyset. That was measured, not assumed: EXPLAIN (ANALYZE, BUFFERS) on the joined statement keeps the Index Scan on unique_container_tags_ns_id_ci_id_name with the cursor bound inside Index Cond, adds no Sort node, and costs one extra buffer. Both tag reads now share containerTagChainJoin, so the chain shape lives in one place.
Cursors carry name alone. Rejected: mirroring RepositoryCursor's {ID, Name, ...} shape. The extra field would be dead, and a row-value bound over (name, id) on a unique key reintroduces id and forces a post-scan filter. RepositoryCursor's own name arm bounds on name alone for the same reason.
Empty-cursor rejection. A non-nil cursor with an empty name is rejected rather than treated as page one. Without it, name > '' admits every row and a client following Link headers loops on page one forever. The spec maps a cursor carrying no boundary to 400. The cost is nil because no writer produces an empty image or tag name.
Non-goals
- No handler wiring.
handler.goandwire_management.goare untouched. Steps 10 to 12 own them, and Step 3 seeds their anchors. - No new index. Both orders ride existing unique indexes (
unique_container_images_ns_id_cr_id_name,unique_container_tags_ns_id_ci_id_name), so this step does not depend on the Step 2 migration. - No soft-delete filter.
container_images,container_tags, andcontainer_manifestscarry nosoft_deleted_at. If S20 adds container soft-delete, these reads gain the exclusion then. - No named-query metrics. The store-wide
TODO(#54)andTODO(#92-followup-S03)notes now name the new methods. The metric helper lands with S03. ListByImageunchanged. The OCI tag lister keeps its names-only projection and case-insensitive order. Neither read is expressible in the other's shape.
Known local-gate deviations
TestWireStorage_CloudCDNPresentincmd/artifact-registryfails locally with "could not find default credentials". Pre-existing, needs GCP ADC, unrelated to this diff.golangci-lint --build-tags=integrationreportsgoconstandcontextcheckon the new integration test helpers. CI never lints integration files, the same two categories already account for 100 warnings across six sibling suites, and the fix would add package-level symbols that collide with the six concurrent sibling step branches. Left as is deliberately.
Database Review Evidence
Queries
Collected with EXPLAIN (ANALYZE, BUFFERS) on the exact statements the four builders emit (stmt.Sql()), against an ephemeral PostgreSQL 17.10 container at the canonical GL_PG_CURR_VERSION. Seed: 5,000 images under one container repository, 5,000 tags and 500 manifests under one image, ANALYZEd before each plan. The tag plans were re-collected after the AppSec fix added the repository scope. Migration mode did not run: this MR adds no migration.
| Query | Plan | Index | Partitions | Buffers | Exec |
|---|---|---|---|---|---|
ListContainerImages name asc, cursor at row 2500 |
Limit over Index Scan | container_images_pNN_namespace_id_container_repository_id_n_idx |
1 | 3 hit | 0.041 ms |
ListContainerImages name desc, cursor at row 2500 |
Limit over Index Scan Backward | same | 1 | 3 hit | 0.039 ms |
FindContainerImageInRepository |
Limit over Index Scan | container_images_pNN_pkey |
1 | 3 hit | 0.018 ms |
ListContainerTagsPage name asc, cursor at row 2500 |
Limit over nested Nested Loops, Memoize on the manifest side | container_tags_pNN_namespace_id_container_image_id_name_idx plus container_manifests_pNN_pkey |
1 each | 7 hit | 0.087 ms |
ListContainerTagsPage name desc, cursor at row 2500 |
same, backward scan | same | 1 each | 7 hit | 0.081 ms |
FindContainerTagByName |
Limit over Merge Join over Nested Loop | container_tags_pNN_..._name_idx, container_manifests_pNN_pkey |
1 each | 9 hit | 0.037 ms |
Notes.
Every read prunes to one of the 64 hash partitions per table. On the joined reads the literal namespace_id sits only on container_tags. The join predicates place the other tables' namespace_id in the same equivalence class, so the planner derives the same constant, which the plans confirm.
Both list keysets fold into the index range: the cursor bound appears inside Index Cond alongside the parent keys, never as a post-index Filter, and no plan contains a Sort node for the page order. Descending runs as a backward scan on the same index. Buffer counts do not grow with the cursor depth.
Both detail reads carry their extra chain key as a single-row Filter on top of a primary-key Index Cond. That is the intended shape: the primary key locates the row and the chain key decides whether it is servable.
The container_images probe both tag reads carry for the repository scope is a single-row lookup. On this fixture it plans as a one-row Seq Scan because the image partition holds exactly one row. With 5,000 images in the partition the same probe used container_images_pNN_namespace_id_container_repository_id_l_idx. Single-partition and single-row either way.
FindContainerTagByName plans as a merge join on this fixture because all 500 manifests sit under the one image, so container_image_id is not selective on the manifests side and the planner drives from there. With a production distribution the driving side is the single tag row, since (namespace_id, container_image_id, name) is unique. The ListContainerTagsPage plans show that shape (Memoize over a manifests primary-key lookup). No flag: the read stays index-driven and single-partition either way, its one-row sort is an in-memory 25 kB quicksort, and it completes in 0.079 ms.
No unbounded scan, no missing index, no multi-partition read, and no external sort in any plan.
A valid image id reached through the wrong repository (the scoping fixture) does not short-circuit: the join emits no rows, the limit never fills, and the scan walks the image's whole tag list from the keyset index. The failed container_images probe executes once into a Materialize node, so each tag row pays a join filter against a cached empty set rather than a re-probe. Measured on the EXPLAIN-test harness (PostgreSQL 16) at 5,000 tags: 122 buffers and 0.8 ms, against 7 buffers and 0.07 ms for the matching read, single-partition and index-driven throughout. Unreachable in the intended flow, which resolves the image through FindContainerImageInRepository first.