feat(managementapi): serve the remote repository create

Hayley Swimelarrequested to merge
hswimelar/s17-p6-step-24 into hswimelar/s17-p6-step-10

Stacked on feat(datastore): dispatch the repository create... (!1618 - merged) • Hayley Swimelar • 19.4.

This branch targets its parent, so the pipeline never merges against main and cannot see that main moved underneath it. Merging this head into main at d5d9bcec conflicts on four files, every one a comment or a wiring line rather than logic. None of it is resolvable here: the parent carries neither main's kind-dispatch scaffold (e4e28b15) nor its riverClients parameter, so writing the merged text now would describe code the target tree does not have. The retarget hop after the parent merges resolves it, against whatever main is then.

Retarget resolution plan, per conflicting file

Verified with git merge-tree --write-tree --messages d5d9bcec2 <head>. Four files conflict; internal/remote/audit.go, handler.go, and the test files auto-merge.

  • internal/managementapi/container_list.go and internal/managementapi/versions.go - main's e4e28b15 replaced the hosted narrowing with a four-arm kind switch. The switch bodies sit outside the conflict markers and auto-merge, so only one doc-comment paragraph per file conflicts, and both sides' text is wrong on the merged tree. Take main's code. Rewrite each paragraph to say the remote arm is live, because the create route admits kind=remote, while the virtual arm stays unreachable, because the create route still rejects an explicit virtual kind (createKindMessage). main's "Neither non-hosted arm is reachable today" and "nothing is reachable either way yet" are the two sentences that must not survive.
  • cmd/artifact-registry/wire_management.go - two regions. main added a riverClients *riverClientHolder parameter to wireManagementAPI and still builds the plain NewRepositoryStore; keep main's parameter and its guard, and keep this branch's NewRepositoryStoreWithRemoteCreates with its store construction. The second region is the //nolint token order alone (gocyclo,cyclop,funlen against funlen,gocyclo,cyclop), so either order resolves it, and the trailing comment should be reconciled by hand.
  • internal/managementapi/audit.go - main renamed the sink to UpstreamExclusionAuditSink, which this branch still calls UpstreamDenialAuditSink. The rename itself auto-merges in internal/remote/audit.go, so this is a comment-only pick: keep this branch's present-tense sentence, which is correct now that the tests exist, and adopt main's symbol name.

TestWireManagementAPI_WiresRemoteCreatorsIntoRepositoryStore is the check that the wire_management.go resolution kept the remote creators: it fails with a 500 if the merge drops them.

Summary

Phase 1's POST /repositories answers every non-hosted kind with a static 422. The lineage below this branch already landed the per-format remote stores, the create and delete kind dispatch, and the settings read serialization, so the datastore can assemble, serve, and delete a remote row. Only the handler gate remained. Step 24 of the merged Phase 6 plan ends the remote half of that 422: POST accepts kind=remote with a required settings.url for every format, echoes the stored settings on the 201, and emits the create and delete audit events, the first consumers of the management audit seam. Virtual creates keep the 422 until the virtual steps land, and a created remote's protocol pulls answer 501 until the owning proxy slices land (the plan's accepted windows).

Non-obvious parts:

  • The audit emissions record committed writes only. Create emits after the transaction commits and before response assembly, so a committed create whose settings echo then fails still audits (TestCreateHandler_RemoteCreate_FailedSettingsEcho_KeepsAuditDropsUsage pins the ordering). Delete emits only for remote rows, keyed off the pre-delete row read, which is safe because kind is immutable and a zero-row delete answers 404 before any emission.
  • cmd/artifact-registry/wire_management.go sits outside the plan step's file list, but the rewire belongs here: the composition root still built the store with plain NewRepositoryStore, whose remote arm fails closed, so a production remote create would 500. Switching to NewRepositoryStoreWithRemoteCreates closes the merge-order window the parent MR's description names, and no other plan step owns the rewire.
  • PATCH on a remote row becomes newly reachable with two gaps a later step owns: the PATCH response carries no settings key, and no update audit event exists until Step 34 lands. The plan sanctions that window.
  • The request-side settings schema is a permissive union (anyOf branches requiring only url), while the response Settings stays a closed oneOf. The spec sets that direction split deliberately.

3060 reviewable lines: 2208 test, 471 production Go, 205 OpenAPI, 143 Bruno, and 33 docs. Splitting the enablement from its tests would break the test-first authorship the step MRs follow, and the production Go diff is 471 lines.

No docs/testing/ scenario is added or affected: the e2e catalogs are per-format protocol journeys, their remote scope-outs (proxy and cache journeys, owned by later slices) still hold, and a remote repository's protocol journey answers 501 until the proxy slices land.

Governing ADRs

ADR-004: the per-format cap counts rows of every kind, and the handler surfaces the store's cap sentinel as 422. ADR-021: the audit actor renders the resolved identity's origin claim tokens through the seam the plan's audit step landed.

Testing

gofmt, go build ./..., and go vet ./... all pass. golangci-lint 2.12 runs clean in both modes with --max-same-issues=0 --max-issues-per-linter=0: plain across the four touched package trees, and --build-tags=integration on internal/managementapi. Unit suites pass for managementapi, datastore, remote, and cmd. The cmd integration suite covers the production mount: one kind=remote create per remote creator through wireManagementAPI, which answers 500 on a revert to the plain NewRepositoryStore. The managementapi integration suite passes against testcontainers PostgreSQL, covering the new create-read-delete walks per format, the mixed-kind cap battery, and the no-upstream-request assertion. redocly validates both contracts, and the four Bruno remote-create examples ride the same commit as the contract change.

Spec coverage

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

Handler slice of the cited criteria. The datastore halves landed with the parent MRs, and criteria not listed belong to other steps of the merged plan. Error-case and security-consideration mappings are in the body of the test(managementapi): cover remote create enablement and audit events commit.

# Criterion (this step's slice) Tests
AC-69 POST with kind=remote and settings.url creates each format; the 201 echoes the stored settings (has_credentials: false when credential-less, unknown health, null checked-at) TestCreateHandler_RemoteCreate_CreatesEachFormat, TestWriteRemoteIntegration_CreateReadDeleteWalk, TestWireManagementAPI_WiresRemoteCreatorsIntoRepositoryStore (the production mount, one create per creator); the one-transaction rollback clause is the store's, pinned by the datastore create suites under the parent MRs
AC-70 No settings, or settings without url, 400; settings on a hosted create 400; an undefined per-format field 400; window bounds 400 TestCreateHandler_RemoteCreate_SettingsPresenceRules, TestCreateHandler_RemoteCreate_SettingsRejections, TestCreateHandler_RemoteCreate_SubmittedWindowsReachStore (accepting edges), TestCreateHandler_RemoteCreate_ValidationOrder, TestContract_SettingsRequest_AnyOfClosedBranches (the same two promises read off the contract, which validates responses only); the update and virtual slices belong to other steps
AC-72 The cap counts every kind: a namespace at the cap across mixed kinds rejects the next create with 422 TestWriteRemoteIntegration_FormatCapCountsEveryKind (999 seeded hosted plus 1 remote; hosted and remote both overflow), TestCreateHandler_RemoteCreate_CapSentinel_Returns422AndEmitsNothing
AC-73 (create) Credentials stored, never echoed, has_credentials: true; one-sided pair, over-length, and :-carrying username 400s TestCreateHandler_RemoteCreate_CredentialAccepts, TestCreateHandler_RemoteCreate_CredentialRejections, the walk (Maven pair and npm token end to end)
AC-79 (create event) One audit event naming the actor TestCreateHandler_RemoteCreate_CreatesEachFormat (exactly one event, field assertions), TestCreateHandler_RemoteCreate_ActorFromIdentityChain, TestCreateHandler_EmitsRepositoryCreatedEvent_RemoteKind; negatives TestCreateHandler_HostedCreate_EmitsNoAuditEvent, the cap sentinel, TestCreateHandler_RemoteCreate_FailedSettingsEcho_KeepsAuditDropsUsage
AC-79 (delete event) One audit event naming the actor TestDeleteHandler_RemoteRepository_EmitsOneDeleteEvent, TestDeleteHandler_RemoteDelete_ActorFromIdentityChain; negatives TestDeleteHandler_HostedRepository_EmitsNoAuditEvent, TestDeleteHandler_RemoteRepository_FailedDelete_EmitsNothing, TestDeleteHandler_MissingRepository_EmitsNothing
AC-83 (create) Read-only settings field 422; invalid url 400; no DNS resolution and no upstream request TestCreateHandler_RemoteCreate_SettingsRejections (all four read-only fields, ParseUpstreamBase refusals including both port rules), TestCreateHandler_RemoteCreate_IssuesNoUpstreamRequest, TestWriteRemoteIntegration_CreateIssuesNoUpstreamRequest (httptest zero-hit, .invalid host 201), TestContract_SettingsRequest_AnyOfClosedBranches (the four fields' readOnly markings)
AC-84 The npmjs/Central pin stores cache_validity_hours 0; an explicit value in the same request wins TestWriteRemoteIntegration_KnownRegistryPin
AC-94 (remote) The list kind=remote filter returns the new rows The walk's list slice per format; the kind=virtual slice belongs to the virtual steps

Related to #314

Edited by Hayley Swimelar

Merge request reports