feat(managementapi): serve the url change on the update (S17 Phase 6 plan: 34/42)

Why

Re-pointing a remote repository at a new upstream must not keep the old host's credentials or keep serving its cache. This makes url writable on the settings PATCH and adds the URL-change transaction: the audited credential clear (with the same-request exception), the health reset, the post-commit failure-counter reset, and the eviction pass enqueued on the update's own transaction. A refused enqueue answers 503 with nothing committed, and credentials_cleared in the response tells the caller what happened.

Depends on

feat(managementapi): container remote bulk pass... (!1805 - merged) • Hayley Swimelar • 19.4 (step 33, the container remote bulk pass) merges first. The plan orders this step after all three remote bulk passes, and the maven and npm passes are merged. A docker or oci URL change deployed before !1805 (merged)'s worker fails closed (errContainerBulkKindUnarmed) and parks on River retries: it executes when the remote arm deploys, and River discards it only past the retry horizon. Keep this MR off the merge train until !1805 (merged) lands.

Research findings

Three places the plan or spec needed correcting, recorded rather than silently fixed:

  • The step's Files list names internal/managementapi/update.go, api/openapi/v1.yaml, and api/bruno/ only. The enqueue-rides-the-update-transaction contract forces more: the per-format store results gain URLChanged and PreviousURL, the composers take an eviction-enqueue hook invoked inside the update transaction, and the composition root wires it plus the failure-counter resetter.
  • AC #83's update slice is disputed by AC #83's create-time coverage is fetch-only: up... (#715) • Hayley Swimelar • 19.4: the low-port and hostname covers run at fetch time only, and the spec overstates create-time checking. This MR implements update-side url validation as exact parity with create through the shared decode and parse path, and adds no fetch-time covers.
  • AC #76 reads unconditionally (credentials_cleared: true on any URL change without new credentials) while the settings-table row reads the other way. Behavior follows AC #76: the marker mirrors the audited clear action, so a re-point with nothing stored still reports true. Flagging for the spec author.

What (the non-obvious parts)

  • The eviction enqueue and every settings write share one transaction, and a refused enqueue answers 503 with nothing committed. Two log emissions survive that rollback by design: the store's credential-audit record and the enqueuer's accepted-enqueue line.
  • The acceptance instant is statement_timestamp(), not now(), because now() pins at the transaction's BEGIN. The remote delete_all pass has no acceptance boundary. The stamp exists because the selector validation rejects a zero instant.
  • The failure-counter reset runs after commit and degrades to a warning. With no cache Redis the composition root wires a documented nop resetter.
  • The eviction reuses the existing mgmtapi:bulk-delete-* kinds: the spec defines the URL-change eviction as the same delete_all pass, so no new job kind.
  • The rebase onto current main tipped newTestDeps and managementWireDeps past funlen's cap (sibling growth plus this MR's one field each). The fix removes one blank separator per function rather than widening a //nolint.

Reviewable size

About 3,484 LOC, over the 500 guardrail: tests 2,545, production Go 794, contracts 144, docs 1. A split would separate the schema widening, the handler transaction, and the 503 contract, which the same-MR rule keeps together (no step publishes a request shape its handler does not accept, or answers a status its operation does not declare). Tests carry the bulk: three per-format store walks, the wiring composition test, and the refusal paths.

Test plan

  • Unit and full -tags=integration suites green on internal/managementapi, internal/datastore, and cmd/artifact-registry against tuned PG 17 (max_locks_per_transaction=4096).
  • TestEveryStatementIsInstrumented, golangci-lint (untagged and integration-tagged) 0 findings on touched files, comment-caps gate 0 findings, redocly, markdownlint.
  • End to end: TestUpdateHandlerIntegration_URLChangeWalk (the full transaction), TestUpdateHandlerIntegration_URLChangeEnqueueRefused (503, nothing committed), and the wiring composition walk in cmd/artifact-registry/wire_management_url_change_integration_test.go (per-family kind pairing through the production route table).
  • e2e catalog row e2e.npm.remote.url-change-evicts added.
Notes for LLM agents
  • Rejected: a dedicated URL-change job kind (the spec's eviction is the delete_all pass, and a new kind would duplicate the per-format worker registry for the same statements). Rejected: unifying the enqueue arm's disconnect handling (499, shared with the four bulk routes) with the store arm's (WARN, nothing written): each follows its family's documented, tested design, and unifying here would re-split against the bulk family.
  • Non-goals: fetch-time URL covers at write time (AC #83's create-time coverage is fetch-only: up... (#715) • Hayley Swimelar • 19.4 owns the spec correction), the container remote bulk worker arm (feat(managementapi): container remote bulk pass... (!1805 - merged) • Hayley Swimelar • 19.4), and step 35's tag-untag eviction arm.
  • The zero-cache-window carve-out in the bruno example and the e2e row is deliberate: zero means cache-forever and is the stored default for the known-immutable public registries, so an old-host fill committing behind the eviction pass can outlive it indefinitely.

Related to #314

Merge request reports

Loading
Loading