feat(managementapi): serve the maven remote file reads (S17 Phase 6 plan: 16/42)

Step 15 has merged as feat(managementapi): serve the maven remote pac... (!1749 - merged) • Hayley Swimelar • 19.4. This now targets main and carries only Step 16's commits.

Why

Phase 6 Step 13 restructured the three artifact-read kind gates into explicit dispatch, with each format's remote arm answering a placeholder 404 until its read step fills it. Step 15 filled the Maven package and version arms. This fills the Maven file arms and introduces the cache freshness object.

Until it lands, a Maven remote repository's cached files are unreadable through the management API: both file routes answer 404 whatever the cache holds.

Plan: Step 16. Spec: S17 AC #95 through #100, Maven file slices.

What is worth a reviewer's attention

The list does not verify the chain, and that is deliberate. ListMavenRemoteFilesByVersion filters the file's own soft_deleted_at and binds the version id, nothing more, which keeps a page to a single index-ordered scan. AC #100 (closed) then rests on the route resolving the parent first. What enforces it is that listMavenRemoteFiles calls findMavenRemoteVersion before the page read, and that helper is the only production path into the store's list. The store records the obligation as a caller contract in its doc, and MavenRemoteVersionStore and the hosted MavenFileStore split the same way.

The detail read is one leaf-anchored statement rather than a probe per hop. It anchors on the (namespace_id, id) equality and joins up to the cached version, the cached package, and the repository binding, so every hop starts from one already identified row and cannot degrade into a parent-wide walk. It answers a single ErrNotFound for every break in the chain, so a guessed id from a sibling remote repository, an evicted parent, or another tenant is indistinguishable from a coordinate the upstream serves and this repository never pulled.

Neither read joins repositories, so a soft-deleted repository stays addressable through this store. That is the same split the hosted twin has, and the resolver every route passes through is what closes it. The new test pins both halves rather than leaving the second one in prose.

cache is optional on MavenFile, which is what keeps the File oneOf disambiguating. Neither branch lists it as required, and neither closes additionalProperties, so a remote Maven payload still fails NpmFile on created_at and an npm payload still fails MavenFile on the three checksums. Moving cache into either required list would break that, because every hosted payload omits the key. The contract sweep validates real responses through openapi3filter, so the invariant is checked mechanically rather than asserted. ArtifactCache is a shared component schema, so its descriptions deliberately say what the value is and avoid promising what the service does with it next, since npm and container will attach to the same schema with different fill paths.

The ArtifactCache schema states the rule for declaring cache, and MavenFile.cache states what it means there. The shared component named MavenFile as its only declarer, which Steps 18 and 20 falsify when npm files and container tags attach to the same schema. Nothing pins a description, so the sentence would have gone stale silently. ArtifactCache now says a schema declares the property when its rows track freshness, and the per-schema meaning sits at the property. OpenAPI 3.1 honors a description beside a $ref, which subject_digest already relies on.

The store-side page cap follows the remote family, not the hosted twin. MavenFileStore carries no cap and leaves the bound to its handler. maxMavenRemoteFilesPageSize mirrors maxMavenRemoteVersionsPageSize instead, so an unbounded LIMIT cannot hold a pool connection while materialising a whole version's cached file set.

Reviewable LOC

3864 insertions and 83 deletions across 21 files, 3947 reviewable LOC, past the 500 line. Split: 536 production across 8 files, 3400 test across 12, and 11 contract in api/openapi/v1.yaml. The production weight is internal/datastore/maven_remote_files.go (336, new), internal/managementapi/files.go (100), version_file_resources.go (43), and versions.go (21). The rest is 36 lines across four files, all of it seam and comment updates the new store's callers need.

Splitting does not help, for a reason specific to this step. listMavenRemoteFiles and writeMavenRemoteFileDetail are the only production callers of MavenRemoteFileStore, so a store-first MR ships 336 lines nothing reaches, and a handler-first MR does not compile. The contract change has the same shape: mavenRemoteFileResource sets cache, and TestMavenRemoteFileHandlers_ResponsesMatchOpenAPIContract validates real responses against api/openapi/v1.yaml, so the resource and the schema fail apart.

That leaves tests-from-code as the only available cut, which the project forbids. The 6.5x ratio is where the size sits: four new test files carry 3279 of the 3362 test lines, and each integration case seeds a namespace, a remote repository, a cached package, and a cached version before it can assert anything.

Test plan

Unit and integration suites in internal/datastore and internal/managementapi. Integration needs Postgres with max_locks_per_transaction=4096, per Local testcontainers Postgres needs max_locks_p... (#657) • Unassigned.

export ARTIFACT_REGISTRY_DATABASE_TEST_DSN=...
go test -tags=integration ./internal/datastore/ -run MavenRemote -count=1
go test -tags=integration ./internal/managementapi/ -count=1

Verified green, plus go build ./..., go vet in both tag modes, golangci-lint untagged at zero issues and integration-tagged with --max-same-issues=0 --max-issues-per-linter=0 reporting zero findings on this branch's files, and redocly lint on the contract.

EXPLAIN evidence

Measured on PostgreSQL 17.10, the project's own test container, at 5000 rows under the target version plus a 2000-row sibling.

The list rides unique_maven_remote_files_ns_id_ver_id_file_name in both directions with no Sort Key:, no Sort Method:, and no Rows Removed by Filter, pruned to one maven_remote_files partition. The detail read prunes maven_remote_files, maven_remote_versions, and maven_remote_packages to one partition each. Both build the expected 64-partition Append over blob_storage_blobs with Index Cond: ((namespace_id = ...) AND (sha256 = maven_remote_files.blob_sha256)), 63 partitions never executed.

Two things this branch checked rather than assumed.

soft_deleted_at IS NULL earns the partial index, and the explicit maven_remote_version_id IS NOT NULL does not. Dropping the IS NOT NULL conjunct leaves the plan and the Index Cond byte-identical, because a strict equality already implies it. Dropping soft_deleted_at IS NULL instead loses the index and adds a materialized Sort. The conjunct stays for the reason the store doc gives, which is that relaxing the version binding must not silently expose the package-level rows, and not because the planner needs it.

The per-partition child index name misidentifies the index. maven_remote_files_pNN_namespace_id_maven_remote_version_id_idx is exactly 63 characters, hits the identifier cap, loses its file_name suffix, and lands on the name the plain two-column index would otherwise have taken. That index's child is maven_remote_files_pNN_namespace_id_maven_remote_version_i_idx1 instead. An operator reading auto_explain output concludes the wrong index is in play. The Index Cond separates them only on a cursored page, where file_name appears in it, so resolve the child through pg_inherits the way the EXPLAIN test already does. A comment on the statement now says this.

Both EXPLAIN gates are non-vacuous, proven by mutation rather than argued. Collapsing the fixture back to one shared blob digest fails all three list subtests on the Memoize gate, and fails the detail read on the blob-probe Index Cond gate, where the plan degrades to Join Filter: (blob_storage_blobs.sha256 = maven_remote_files.blob_sha256) with Rows Removed by Join Filter: 1. The planning-to-execution ratio is deliberately not asserted in any comment, because two measurements of it on this shape disagree by roughly 6x depending on backend cache state.

Rolling-deploy skew

During one deploy window a client following a Link cursor into a pre-MR pod gets the old 404 on the two flipped routes. No cursor is misread, and nothing diverges once the window closes.

Plan contradictions

Step 16's Files bullet names three paths. This change also touches cmd/artifact-registry/wire_management.go, internal/datastore/query_names.go, internal/datastore/maven_files.go, and three files under internal/managementapi/ (handler.go, version_file_resources.go, versions.go). Those carry the composition-root wiring and its nil guard, the two new statement names, the MavenRemoteFileReader seam, the ArtifactCache resource and its mapper, and the comment updates the new dispatch invalidated. None is a scope change, and the bullet describes the work rather than the file set.

artifact.go and packages.go were on that list until this rebase. ArtifactCache and the Maven remote log-key group both reached main ahead of this branch, so the branch no longer diffs either file.

The plan is also wrong in the other direction, in two places.

It marks the datastore work (Modify). internal/datastore/maven_remote_files.go is a new file, not a modification, and it accounts for 336 of the 536 production lines.

It promises api/openapi/v1.yaml gets "the cache object on MavenFile and this family's nullable widenings". There is no widening. None of AC #97 (closed)'s null-mapping columns exists on a maven_remote_files row, and md5 was already [string, 'null'], so the Maven file family needed none. Beyond the cache object and the File oneOf note documenting it, the contract gets no other edit.

E2e scenario impact: none

docs/testing/e2e/maven.md scopes virtual and remote (proxy/cache) Maven repositories out to S30 and S14, and the catalogs are protocol-client driven while this is a management-surface read.

Spec coverage

Spec coverage table

Spec: docs/specs/S17-rest-management-api.md

Acceptance criteria

# Criterion (this step's slice) Tests
AC #95 Maven file lists and details serve a remote repository from its cache tables TestMavenRemoteFileList_ServesTheCache, TestMavenRemoteFileDetail_ServesTheCache, TestMavenRemoteFileStore_ReadsWhatTheCacheFillWrote, TestMavenRemoteChain_ReadsServeWhatTheWritePathsBuilt, TestMavenRemoteFileStore_RowFidelity (including /an_empty_artifact_reports_size_0_through_both_reads, so a dropped blob join cannot hide behind a legal zero)
AC #95 ... with Phase 3's sort (file_name, both directions) TestMavenRemoteFileList_HonoursQueryParameters/order=desc_reaches_the_store, TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_KeysetWalkBothDirections, TestMavenRemoteFileOrderBy, TestListMavenRemoteFilesByVersionStmt
AC #95 ... with Phase 3's pagination (keyset, Link, hasMore) TestMavenRemoteFileList_HonoursQueryParameters/a_Link_walk_covers_every_cached_file_exactly_once, .../a_backward_walk_retraces_the_page_it_came_from, .../the_page-size_default_and_cap_are_the_handler's, TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_HasMoreSignal, ..._FullFinalPage (a row count divisible by the page size, the one shape where a hasMore read off a full page still answers correctly on every other case), ..._UnmatchedCursor, ..._BoundaryLimits, ..._LimitMaxIntSaturates, TestNextMavenRemoteFileCursor, TestMavenRemoteFileKeysetBound
AC #95 ... with Phase 3's error rules TestMavenRemoteFileReads_InvalidQueryIs400, TestMavenRemoteFileReads_VirtualRepositoryIs404, TestMavenRemoteFileReads_StoreFailureIs500
AC #95 ... and a coordinate the upstream serves but nobody has pulled is absent TestMavenRemoteFileList_EmptyPageIsAnEmptyArray/a_cached_version_with_nothing_cached_under_it_is_an_empty_200, .../a_version_the_cache_does_not_hold_is_a_404, TestMavenRemoteFileList_ServesTheCache/an_existing_version_with_no_cached_files_is_an_empty_200, .../a_version_the_cache_does_not_hold_is_a_404_before_any_file_read, TestMavenRemoteFileDetail_ServesTheCache/an_id_the_cache_does_not_hold_is_the_artifact_404
AC #95 ... scoped to the tenant and the remote repository in the URL TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_ChainScoping, TestMavenRemoteFileStore_FindMavenRemoteFileByID, TestFindMavenRemoteFileByIDStmt, TestMavenRemoteFileDetail_ServesTheCache/the_detail_read_is_scoped_by_the_remote_child_row, TestMavenRemoteFileList_ServesTheCache/the_parent_version_resolves_through_the_remote_child_row
AC #95 Container, npm, and Maven package/version slices Out of scope: Steps 17-20 own the npm and container families, and the Maven package and version slices merged with Step 15. TestMavenRemoteReads_NeighbouringRoutesKeepTheir404 pins that the Maven version delete still answers its 404 on the repository this step serves.
AC #96 (closed) Remote Maven file responses carry cache with upstream_checked_at and a nullable upstream_etag TestMavenRemoteFile_CarriesTheCacheEnvelope (list rows and detail, both branches of the validator), TestMavenRemoteFileColumns_CarryTheCacheAndNoCreatedAt, TestListMavenRemoteFilesByVersionStmt, TestMavenRemoteFileStore_RowFidelity
AC #96 (closed) Hosted responses omit the key TestMavenFileReads_HostedResponsesOmitTheCacheKey (hosted list and hosted detail)
AC #96 (closed) npm file and container tag cache Out of scope: Steps 18 and 20.
AC #97 (closed) Remote Maven file bodies serialize no invented column TestMavenRemoteFile_BodyCarriesTheFileKeysAndNoCreatedAt, TestMavenRemoteFileColumns_CarryTheCacheAndNoCreatedAt. None of the columns AC #97 (closed) names (version publish attribution, npm package counters, container tag timestamps, npm file created_at, manifest artifact_type and subject_digest) exists on a Maven remote file row, so the slice is a positive assertion: the body carries exactly the MavenFile keys plus cache, and no created_at.
AC #97 (closed) The columns AC #97 (closed) names, on their own resources Out of scope: version attribution merged with Step 15, and the rest are Steps 17-20.
AC #98 A package-level Maven metadata row is absent from remote file lists and returns 404 on detail by id TestMavenRemoteFileStore_PackageLevelMetadataRowIsUnaddressable (seeded with the same file name as a version-level row, so the exclusion cannot be name-based), TestMavenRemoteFileReads_MarkedRowIsAbsent/a_package-level_metadata_row_is_absent_from_the_list, .../a_package-level_metadata_row_is_unaddressable_on_detail, TestListMavenRemoteFilesByVersionStmt, TestFindMavenRemoteFileByIDStmt, TestMavenRemoteFileStore_ReadsWhatTheCacheFillWrote (the writer's own package-level row)
AC #98 The npm dist-tag clause Out of scope: Step 17. Pre-existing TestPackageSubtreeRoutes_NonHostedRepositoryIs404.
AC #99 (closed) Every artifact route on a virtual repository returns 404 TestMavenRemoteFileReads_VirtualRepositoryIs404 (both file routes, driven on a rig whose cache rows are seeded, so a kind dispatch that admitted virtual would answer 200)
AC #100 (closed) An evicted-but-unreaped row (soft_deleted_at) is absent from every list and detail read TestMavenRemoteFileStore_SoftDeleteIsInvisibleAtAllThreeLevels (file, version, and package levels), TestMavenRemoteFileReads_MarkedRowIsAbsent, TestListMavenRemoteFilesByVersionStmt, TestFindMavenRemoteFileByIDStmt, TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_HasMoreSignal
AC #100 (closed) ... and the fourth level neither read joins TestMavenRemoteFileStore_SoftDeletedRepositoryIsTheCallersToEnforce. The reads never join repositories, so a soft-deleted repository stays addressable through the store. The test pins both halves: the reads return the row anyway, and RepositoryStore.FindByName refuses it, which is the compensating control every route passes through.

Error cases

Condition Tests
Repository kind is virtual404 TestMavenRemoteFileReads_VirtualRepositoryIs404
Parent version missing, soft-deleted, or outside the chain → 404 before any file read TestMavenRemoteFileList_ServesTheCache/a_version_the_cache_does_not_hold_is_a_404_before_any_file_read, TestMavenRemoteFileList_EmptyPageIsAnEmptyArray/a_version_the_cache_does_not_hold_is_a_404, TestMavenRemoteFileStore_SoftDeleteIsInvisibleAtAllThreeLevels
File id missing, not a valid UUID, soft-deleted, package-level, or outside the chain → 404 TestMavenRemoteFileDetail_ServesTheCache, TestMavenRemoteFileReads_MarkedRowIsAbsent, TestMavenRemoteFileStore_FindMavenRemoteFileByID, TestMavenRemoteFileStore_PackageLevelMetadataRowIsUnaddressable, pre-existing TestVersionFileRoutes_NotFoundCases for the malformed-UUID parse
Parent repository soft-deleted → 404 at the resolver, not at the store TestMavenRemoteFileStore_SoftDeletedRepositoryIsTheCallersToEnforce, pre-existing TestVersionFileRoutes_SoftDeletedRepositoryIs404
Invalid sort, order, limit, or an undecodable or boundary-free cursor → 400 TestMavenRemoteFileReads_InvalidQueryIs400, TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_ArgumentGuards, ..._GuardsHoldAgainstTheDatabase, ..._AcceptsPopulatedCursor, ..._AcceptsBoundaryLimit, TestMavenRemoteFileStore_FindMavenRemoteFileByID_ArgumentGuards. A limit past the server maximum clamps rather than failing, and TestMavenRemoteFileList_HonoursQueryParameters pins that half.
Unexpected server failure → 500, with row ids as log fields and the cause never in the body TestMavenRemoteFileReads_StoreFailureIs500, TestMavenRemoteFileReads_StoreFailureLogsTheRowIdentifiers, TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_QueryFailure, TestWrapMavenRemoteFileErr, TestMavenRemoteFileStore_ZeroValueReadsPanic, TestNewMavenRemoteFileStore_NilClient
A nil row with no error from the detail store → 500, never a live file reported absent TestMavenRemoteFileReads_StoreFailureIs500/the_file_detail_returns_a_nil_row_without_an_error
A management dependency is nil at construction → panic at startup TestNewHandler_PanicsOnNilDependency/nil_maven_remote_file_reader, TestWireManagementAPIWithDeps_PanicsOnNilDependencies (MavenRemoteFiles row)
Repository format differs from the :format segment → 404 Pre-existing TestVersionFileRoutes_NonHostedRepositoryIs404 and the package-family format sweeps. Unchanged by this step.
Slug resolves to no namespace → 404 Pre-existing TestHandler_UnknownSlug_Returns404WithEnvelope. Unchanged by this step.
Authentication or authorization failure → 401/403 Owned by S08 and S09, both stubbed, so no route here can produce either.

Security considerations

Concern Tests
Tenant isolation: every query scoped by namespace_id, full parent chain verified from the URL down TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_ChainScoping, TestMavenRemoteFileStore_FindMavenRemoteFileByID, TestFindMavenRemoteFileByIDStmt, TestListMavenRemoteFilesByVersionStmt, TestMavenRemoteChain_ReadsServeWhatTheWritePathsBuilt/the_detail_read_refuses_the_binding's_own_repositories_id
Existence hiding: a guessed id from another remote repository or namespace answers the same 404 TestMavenRemoteFileStore_FindMavenRemoteFileByID, TestMavenRemoteFileDetail_ServesTheCache/an_id_from_another_remote_repository_or_tenant_answers_the_same_404, TestWrapMavenRemoteFileErr
Soft-deleted repository does not become readable through the cache tables TestMavenRemoteFileStore_SoftDeletedRepositoryIsTheCallersToEnforce (the resolver filter asserted as a live probe, so a resolver that stopped filtering fails here rather than surviving in prose)
Injection: Jet builder with bound parameters throughout TestListMavenRemoteFilesByVersionStmt and TestMavenRemoteFileKeysetBound (the cursor asserted as a bind, never inlined), plus the argument-guard tables rejecting NUL, control bytes, invalid UTF-8, and over-long boundaries
Error strings carry no row identifiers, and the caller logs them as structured fields TestWrapMavenRemoteFileErr, TestMavenRemoteFileReads_StoreFailureLogsTheRowIdentifiers
Read cost: reads stay partition-pruned and index-bound TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_DeepPageIsIndexBacked (5000 rows, both directions, no post-scan Sort, no Rows Removed by Filter), TestFindMavenRemoteFileByIDStmt_PrunesEachChainHop, TestMavenRemoteFileStore_ListMavenRemoteFilesByVersion_BoundaryLimits
Upstream credentials, SSRF, URL retarget Not reachable from a read route here. The resolver discards the resolved repository's URL, and neither statement selects a credential column.

Related to #314

Edited by Hayley Swimelar

Merge request reports

Loading
Loading