feat(managementapi): serve npm's virtual kind (S17 Phase 6 plan: 37/42)

Why

Phase 1 shipped create with a hosted-only kind gate and no removal contract for the virtual kind, so a virtual create answers 422 for every format and a virtual DELETE answers 500 whatever destructive carries: Repository DELETE answers 500 for the virtual k... (#819) • Unassigned. Phase 6 ends both one format at a time, gated on each format's virtual schema, and npm's is the schema that exists on main. The npm upstream-association surface that follows has no repository to hang off until this lands.

Acceptance is the npm slices of AC #71 (closed) (a virtual create's 201 carries no settings key and an empty upstream list) and AC #109 (closed) (204 under either destructive value, associations removed in the same transaction). Maven keeps the 422 on create, so #819 stays open for it and this MR cites it rather than closing it.

What

The virtual child row is added beside the kind-agnostic format child, never swapped for it. npm_repositories carries every npm repository whatever its kind, and the npm resolver narrows on no kind predicate, so a create that swapped the children would leave every npm protocol route unable to resolve the repository. internal/datastore/npm_virtual_repositories.md carries that and the file's other reasoning.

The delete takes the virtual child row's FOR UPDATE lock before the cascade. One DELETE on the parent still does all the removal, because every foreign key into the virtual child and its junction tables declares ON DELETE CASCADE, but it runs inside a transaction that locks the child row first, the order an upstream association mutation will take. The lock deliberately gates no active parent: Delete filters no soft_deleted_at, so requiring one would strand every tombstoned virtual repository, which nothing on either path could then remove.

repositoryDeleteKindGate is untouched, and Delete branches to the virtual arm ahead of it. The virtual arm resolves its own refusal from the dispatch that resolves the lock, so the kind gate answers for the bare cascade only. Its other caller, RepositoryReaper.finalize, runs a bare DELETE on the walk chunk's handle and holds no child-row lock, so it keeps refusing every virtual kind.

The per-format seams are the container step's, which merged first, which is why they look the way they do. virtualChildInsert and virtualChildDeleteLock read one {insert, lock} map keyed by format, each returning the refusal sentinel for an absent key, and VirtualRepositoryCreateSupported publishes the same verdict to the handler, so the 422 and the store cannot drift. This step adds the npm entry beside the container family's.

One ADR-009 deviation, disclosed rather than closed. ADR-009 requires a 409 under either destructive value for a repository any virtual repository lists as an upstream, stating that destructive intent does not reach it. Instead, the junction key's 23503 becomes ErrRepositoryNotEmpty, and the handler's existing destructive=true arm tombstones a row the reap finalizer then refuses forever on its kind-only gate. The container step opened that path first, for docker and oci, and this one adds npm. After it, the only way in is a direct database write of a row the spec already forbids, because no route writes either junction table. AC #110 (closed) is the acceptance criterion, the plan assigns the guard to Steps 39 to 42, and internal/datastore/repositories.md records the deviation beside the code that has it.

Rollback is a one-way door. Once an npm virtual repository exists, reverting the delete arm leaves it undeletable, which is the defect this fixes. Revert both arms or neither. Mid-rollout, an un-upgraded pod answers 500 for a delete of a repository an upgraded pod created, resolving as the fleet converges.

No contract schema change. Kind's enum already carries virtual and a virtual create declares no settings, so api/openapi/v1.yaml moves prose only. The Bruno collection carries a request per served kind and format, so npm's virtual create joins it.

Size. 1202 reviewable lines, of which 915 are tests and 145 the contract, Bruno, and doc files, leaving 142 of production Go, under the guardrail's ceiling on its own. Splitting create from delete would leave npm virtual repositories answering 500 on DELETE for the window between the two MRs, which is the defect #819 records.

Test plan

# Criterion Tests
AC #71 (closed) POST with kind=virtual creates a virtual repository whose response carries no settings key and whose upstream list is empty TestCreateHandler_VirtualKind_Npm_Returns201, TestVirtualNpmIntegration_CreateReadDeleteWalk, TestRepositoryStore_Create_VirtualNpm, TestRepositoryStore_Create_VirtualNpmRollsBackWhole
AC #109 (closed) DELETE on a virtual repository returns 204 under either destructive value with its associations removed in the same transaction TestVirtualNpmIntegration_CreateReadDeleteWalk (both values), TestRepositoryStore_Delete_VirtualNpmRemovesAssociations, TestRepositoryStore_Create_VirtualNpmThenDelete
Error path Tests
kind=virtual on a format serving none: 422, no rows written TestCreateHandler_NonHostedKind_Returns422, TestValidateCreateKind_PerFormat, TestRepositoryStore_Create_KindGateFailsClosed, TestRepositoryStore_Delete_VirtualKindFailsClosed
A kind outside the enumeration: 422 naming the accepted kinds TestCreateHandler_NonHostedKind_Returns422, TestCreateKindMessageFor_TextsDiffer
settings on a virtual create: 400 at the handler, typed sentinel at the store TestCreateHandler_VirtualKind_Npm_WithSettings_Returns400, TestRepositoryCreateKindGate, TestRepositoryStore_Create_KindDispatchGuards
Absent id, or one in another namespace, on the virtual delete path TestRepositoryStore_Delete_VirtualNpmNotFound
A tombstoned virtual parent still deletes (work item 829) TestRepositoryStore_Delete_VirtualNpmTombstonedParent
A virtual parent carrying no virtual child row still deletes TestRepositoryStore_Delete_VirtualNpmWithoutChildRow
A repository that is a live association's upstream cannot delete TestRepositoryStore_Delete_VirtualNpmUpstreamTargetBlocked
Nil context, nil handle, or a zero id on either child-row write TestNpmVirtualRepositoryWrites_ArgumentGuards, TestNpmVirtualRepositoryWrites_GuardsRunBeforeAnyStatement
Insert and lock dispatch drift across every format TestVirtualChildDispatches_AgreeOnEveryFormat, TestVirtualChildDispatches_ServeNpm
The lock statement takes FOR UPDATE and filters no soft_deleted_at TestLockNpmVirtualRepositoryStmt_TakesForUpdateOnBothKeyColumns
The tolerated missing child row leaves a record TestRepositoryStore_Delete_VirtualNpmWithoutChildRow
The audit guards stay closed on the virtual kind TestCreateHandler_VirtualCreate_EmitsNoAuditEvent, TestDeleteHandler_VirtualRepository_EmitsNoAuditEvent
The per-format cap counts virtual rows (AC #72) TestRepositoryStore_Create_VirtualNpmOccupiesCapRoom

Gates: golangci-lint in both modes, the integration-tagged run included because CI cannot compile those files, plus the comment-caps gate, both unit suites, and the integration suites against a Postgres with max_locks_per_transaction=4096. Red-then-green was measured on the branch, the test commit failing with both child-row writes panicking and both new query names at zero call sites.

e2e scenarios: docs/testing/e2e/npm.md listed virtual npm repositories as wholly out of scope, which these two routes falsify, so its scope note now carves them in and holds the client journeys and the upstream associations out. No scenario is added: the plan's Testing Strategy records Phase 6's e2e impact as none, because the catalogs are protocol-client driven and the monolith slices that consume these endpoints own those additions.

Conformance: not applicable, no protocol-path behavior changes.

Context for LLM agents

Rationale

  • Widen repositoryDeleteKindGate to dispatch on format, so one gate answers for both removers. Rejected: its other caller, RepositoryReaper.finalize, runs a bare DELETE on the walk chunk's handle and holds no virtual child-row lock, so admitting a virtual kind there would remove one without the ordering the spec requires of the removal.
  • Inject the npm virtual child writes through a constructor seam, as the three remote creators are. Rejected: that wiring is Step 24's create-enablement gate, and a virtual arm that ships serving needs no gate a wiring slip could trip. Two unexported package-level functions instead, matching insertFormatChild.
  • A switch per resolver, refusing in the default arm. Rejected on review: two switches one function apart had to agree on the served set, and a test held them in agreement rather than the code. One {insert, lock} map entry per served format makes that agreement structural. The map is deliberately partial where formatChildInserts and remoteChildCreates are total, so each resolver still answers an absent key with its own sentinel rather than a totality test the virtual arms cannot pass.
  • Refuse the delete when the lock finds no virtual child row. Rejected: the concurrent-delete case already answers not-found through the parent DELETE's zero-row count, and refusing would wedge a parent that neither Delete nor the reap finalizer can remove.
  • Chosen: two resolvers that return the refusal sentinel as their error, one exported predicate publishing the create verdict, and a virtual delete branch ahead of the untouched kind gate.

Consequences: a format's virtual arm is one map entry plus one file, and the reaper keeps failing closed on every virtual kind until a remover that takes the lock exists.

Non-goals

  • The upstream association store, its five routes, and the delete-side 409 for a repository some virtual repository lists as an upstream. Steps 39 to 42 own them, and the junction FK's NO ACTION answers today by aborting that delete as ErrRepositoryNotEmpty.
  • The lock-order test against a concurrent associate. It needs the association write path, so it arrives with Step 40.
  • A writeDeleteStoreError arm for ErrRepositoryDeleteVirtualKind. Within one binary, create refuses the kinds that can still return it, so only a direct database write reaches it, and each format's own step closes its share. The mixed-fleet window is the rollback note above.
  • npm virtual protocol serving. A created virtual repository resolves its binding and answers 501 on the npm routes until S31's serve surface lands, which is that slice's window.
  • Maven virtual creates. maven_virtual_repositories does not exist in the tree. The container arm landed with Step 38, against these same seams.

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 each row's details 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.RepositoryStore.Delete.KindProbe Limit repositories_p59_pkey 1 / 1 8.30 0.012ms 3 / 0 1/64
datastore.RepositoryStore.deleteVirtualRepository Delete repositories_p14_pkey 0 / 0 8.30 0.233ms 68 / 0 1/64
datastore.insertNpmVirtualRepository Insert n/a 0 / 0 0.01 0.169ms 2 / 2 1/64
datastore.lockNpmVirtualRepository LockRows npm_virtual_repositories_p06_namespace_id_repository_id_idx 1 / 1 8.31 0.022ms 4 / 0 1/64
datastore.RepositoryStore.Delete.KindProbe

Summary: The probe reads the two deciding columns by primary key, and the plan matches: an Index Scan on a partition of pk_repositories with both key columns bound, so the namespace literal prunes to one of 64 partitions. Estimate and actual agree at 1 row against 5000 seeded rows in the partition. No anomalies.

Seed shape: namespaces=1, repositories=5000

Rendered SQL:

SELECT repositories.kind AS "repositories.kind",
     repositories.format AS "repositories.format"
FROM public.repositories
WHERE (repositories.namespace_id = $1::uuid) AND (repositories.id = $2::uuid)
LIMIT $3;

Bound args: [d4ff4103-7e50-700f-9417-32cd325567e3, 35c3b9bb-aeac-794c-85d2-16e8272fc0bc, 1]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.28..8.30 rows=1 width=4) (actual time=0.011..0.012 rows=1 loops=1)
   Buffers: shared hit=3
   ->  Index Scan using repositories_p59_pkey on repositories_p59 repositories  (cost=0.28..8.30 rows=1 width=4) (actual time=0.010..0.011 rows=1 loops=1)
         Index Cond: ((id = '35c3b9bb-aeac-794c-85d2-16e8272fc0bc'::uuid) AND (namespace_id = 'd4ff4103-7e50-700f-9417-32cd325567e3'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=354
 Planning Time: 1.202 ms
 Execution Time: 0.022 ms

Timings: planning 1.202ms, execution 0.022ms, total 1.224ms.

datastore.RepositoryStore.deleteVirtualRepository

Summary: The cascading delete reaches its row by primary key on one partition, and execution is dominated by the twelve foreign-key triggers the cascade fires rather than by the scan, which costs 0.010ms of the 13.284ms. Those triggers are the removal contract: npm_virtual_repositories_..._fkey14 clears the virtual child row, and npm_virtual_repository_upstr_upstream_repository_id_name_fkey14 is the NO ACTION check that aborts the delete when the repository is still a listed upstream. No anomalies. Seeded at 50 rows instead of 5000 the same statement falls back to a Seq Scan over the partition, which is the low-cardinality artifact the methodology warns about rather than a plan defect, so the figures above are the 5000-row run.

Seed shape: namespaces=1, repositories=5000, npm_virtual_repositories=1

Rendered SQL:

DELETE FROM public.repositories
WHERE (repositories.namespace_id = $1::uuid) AND (repositories.id = $2::uuid);

Bound args: [c0517997-c154-7874-ad5a-cfdf42b2df26, 58ca6bc0-363c-72e3-bd91-6815583f8113]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Delete on repositories  (cost=0.28..8.30 rows=0 width=0) (actual time=0.233..0.233 rows=0 loops=1)
   Delete on repositories_p14 repositories_1
   Buffers: shared hit=68
   ->  Index Scan using repositories_p14_pkey on repositories_p14 repositories_1  (cost=0.28..8.30 rows=1 width=10) (actual time=0.009..0.010 rows=1 loops=1)
         Index Cond: ((id = '58ca6bc0-363c-72e3-bd91-6815583f8113'::uuid) AND (namespace_id = 'c0517997-c154-7874-ad5a-cfdf42b2df26'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=138
 Planning Time: 0.636 ms
 Trigger for constraint container_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.063 calls=1
 Trigger for constraint npm_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.020 calls=1
 Trigger for constraint maven_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.003 calls=1
 Trigger for constraint repository_collection_reposit_repository_id_namespace_id_fkey14 on repositories_p14: time=0.962 calls=1
 Trigger for constraint npm_remote_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.361 calls=1
 Trigger for constraint maven_remote_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.309 calls=1
 Trigger for constraint container_remote_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.338 calls=1
 Trigger for constraint npm_virtual_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=0.893 calls=1
 Trigger for constraint npm_virtual_repository_upstr_upstream_repository_id_name_fkey14 on repositories_p14: time=1.498 calls=1
 Trigger for constraint container_virtual_repositorie_repository_id_namespace_id_fkey14 on repositories_p14: time=0.976 calls=1
 Trigger for constraint container_virtual_repository_upstream_repository_id_name_fkey14 on repositories_p14: time=1.298 calls=1
 Trigger for constraint npm_virtual_repository_upstr_npm_virtual_repository_id_n_fkey14 on npm_virtual_repositories_p14: time=0.135 calls=1
 Execution Time: 13.284 ms

Timings: planning 0.636ms, execution 13.284ms, total 13.920ms.

datastore.insertNpmVirtualRepository

Summary: A single-row insert routed by namespace_id to one of 64 partitions, with the two foreign-key triggers that hold the child to its namespace and its parent repository. No scan, so no index applies. No anomalies.

Seed shape: namespaces=1, repositories=1

Rendered SQL:

INSERT INTO public.npm_virtual_repositories (id, namespace_id, repository_id)
VALUES ($1::uuid, $2::uuid, $3::uuid);

Bound args: [b7169cdb-d065-74ff-a931-ea6be4597acd, 883bc474-5508-7168-b25d-f357bde0b5ed, 69c6aa81-dd04-723f-a6b4-d84a72ac6131]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Insert on npm_virtual_repositories  (cost=0.00..0.01 rows=0 width=0) (actual time=0.169..0.169 rows=0 loops=1)
   Buffers: shared hit=2 read=2 dirtied=5 written=3
   ->  Result  (cost=0.00..0.01 rows=1 width=48) (actual time=0.001..0.001 rows=1 loops=1)
 Planning Time: 0.034 ms
 Trigger for constraint fk_nvr_namespace_id_namespaces on npm_virtual_repositories_p35: time=0.153 calls=1
 Trigger for constraint fk_nvr_repository_id_repositories on npm_virtual_repositories_p35: time=1.452 calls=1
 Execution Time: 1.872 ms

Timings: planning 0.034ms, execution 1.872ms, total 1.906ms.

datastore.lockNpmVirtualRepository

Summary: The delete-time lock takes the row through a partition of unique_nvr_ns_id_repository_id, the unique index the partition key leads, so the read prunes to one of 64 partitions and returns at most one row by construction. LockRows above the Index Scan is the FOR UPDATE. Estimate and actual agree at 1 row against 5000 seeded child rows in the partition. No anomalies.

Seed shape: namespaces=1, repositories=5000, npm_virtual_repositories=5000

Rendered SQL:

SELECT npm_virtual_repositories.id AS "npm_virtual_repositories.id"
FROM public.npm_virtual_repositories
WHERE (npm_virtual_repositories.namespace_id = $1::uuid) AND (npm_virtual_repositories.repository_id = $2::uuid)
FOR UPDATE;

Bound args: [652f0e59-c1c0-71e2-9cd3-b2cd73bc94be, fd1aa7b7-0472-77da-a809-8014fe07b7b6]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 LockRows  (cost=0.28..8.31 rows=1 width=26) (actual time=0.021..0.022 rows=1 loops=1)
   Buffers: shared hit=4
   ->  Index Scan using npm_virtual_repositories_p06_namespace_id_repository_id_idx on npm_virtual_repositories_p06 npm_virtual_repositories  (cost=0.28..8.30 rows=1 width=26) (actual time=0.016..0.017 rows=1 loops=1)
         Index Cond: ((namespace_id = '652f0e59-c1c0-71e2-9cd3-b2cd73bc94be'::uuid) AND (repository_id = 'fd1aa7b7-0472-77da-a809-8014fe07b7b6'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=60
 Planning Time: 0.697 ms
 Execution Time: 0.050 ms

Timings: planning 0.697ms, execution 0.050ms, total 0.747ms.

Related to #314

Edited by Hayley Swimelar

Merge request reports

Loading
Loading