feat(managementapi): serve container remote eviction and the upsert 405 (S17 Phase 6 plan: 30/42)

Why

Container management-API deletes cannot reach a kind=remote repository. The shared container kind dispatcher answers every non-hosted kind with the existence-hiding 404 before a handler runs, so a remote repository's cached images and manifests have no eviction path, and PUT on a remote tag reports 404 instead of telling the caller the method is not available there.

This is Step 30 of the Phase 6 plan. Its gate is clear: the eviction composers landed in chore(datastore): add the container remote evic... (!1589 - merged) • Hayley Swimelar • 19.4, and the container image, manifest, and tag DELETE handlers plus the tag PUT upsert are all on main.

What

Five things a reviewer would otherwise have to reconstruct.

The armed handlers bypass the shared kind dispatcher instead of widening it. resolveContainerRepository's dispatch is unchanged, and only its comments move. The three armed handlers call the kind-agnostic resolveContainerArtifactRepository and switch on the resolved row's kind locally, the shape the Maven sibling takes in feat(managementapi): serve the maven remote pac... (!1726 - merged) • Hayley Swimelar • 19.4. Widening the shared dispatcher would collide head-on with feat(managementapi): serve container remote ima... (!1747 - merged) • Hayley Swimelar • 19.4 and feat(managementapi): serve the container remote... (!1748 - merged) • Hayley Swimelar • 19.4, both open and both rewriting that same switch for the remote reads.

The change reaches past the plan's declared file list, which named internal/managementapi/ and api/openapi/v1.yaml. It also adds one datastore method (FindContainerRemoteRepositoryID, the only resolve the chain was missing) with its query_names.go entry, the cmd/artifact-registry adapter and wiring, the Bruno request's status list, and the id-keyed finder count in docs/dev/database-query-patterns.md. The wiring is not optional: without it the route nil-panics on the first remote request. The adapter deliberately uses main's per-call managementPool pattern rather than the poolProvider closure on the unmerged Maven sibling, so nothing duplicate-defines when that lands.

The 405 is declared on upsertContainerTag, which the document's own Error schema previously ruled out ("Operations declare neither 405 nor 501 individually"). That sentence is amended rather than left false: this 405 is the handler's, driven by repository kind, and permanent, unlike the router-level 405 the rule was written about. kin-openapi response validation requires the status declared in the MR that answers it.

Nothing here is reachable from a client yet. internal/managementapi/create.go rejects every non-hosted kind with a 422, so a remote container repository cannot be created through the API. Every test seeds container_remote_* rows directly, and the arms go live when repository creation opens up.

AC #101's re-cache tail is not satisfiable at this base. The criterion ends "a subsequent protocol pull of the same coordinate re-fetches and re-caches it as a fresh row", but no production wiring fills the OCI dispatcher's remote read slots (oci.WithRemoteManifest and its three siblings appear only in tests), so a remote pull answers 501. The eviction's observable effect is that the cached row stops resolving, and the description and comments claim only that.

Spec coverage

# Criterion (Step 30 slice) Where
AC #101 Image slice: remote DELETE returns 202, target stops resolving, no outbound upstream request TestContainerImageDelete_RemoteEvictsAndAnswers202, ..._RemoteServesBothContainerFormats, ..._RemoteSecondDeleteIs404. No upstream seam is reachable from the handler, which makes "no outbound request" structural.
AC #101 Manifest slice, addressed by digest TestContainerRemoteManifestDelete_EvictsAndAnswers202, ..._ServesBothContainerFormats, ..._SecondDeleteIs404
AC #101 Tail: a subsequent pull re-fetches and re-caches Not satisfiable at this base, see What above. The store-level rule is already pinned by TestContainerRemoteEvictor_EvictContainerRemoteImage_MarkedNameRecaches.
AC #102 (closed) Evictions emit no artifact_registry_artifact_deleted TestContainerImageDelete_RemoteEvictsAndAnswers202, TestContainerRemoteManifestDelete_EvictsAndAnswers202, plus requireContainerEvictNoSideEffects on every non-202 row of both not-found matrices
AC #102 (closed) Hosted emission unchanged TestContainerImageDelete_RemoteKindDispatch and TestContainerRemoteManifestDelete_KindDispatch assert one event on the hosted row beside none on the remote row. The pre-existing hosted tests are untouched
AC #106 Upsert half: 405, Allow: GET, DELETE, nothing changes TestContainerTagUpsert_RemoteRepositoryIs405, ..._RemoteRepositoryChangesNothing, ..._RemoteRefusalBeatsTheLeafRejections, ..._ResponsesMatchOpenAPIContract, TestContract_TagUpsert_DeclaresItsResponses
AC #106 DELETE half: untag evicts with 202 A later step's. TestContainerTagDelete_RemoteRepositoryIsStillTheExistenceHiding404 pins today's 404 as the tripwire that change flips.
AC #107 A target a live cached manifest indexes returns 409 with the parent digests TestContainerRemoteManifestDelete_IndexedManifestIs409WithParents, ..._ConflictBodyMatchesTheHostedRefusal, ..._ResponsesMatchOpenAPIContract, and a real-relationship-row walk in internal/managementapi/container_remote_eviction_integration_test.go
AC #107 One indexed only by an already-marked manifest returns 202 TestContainerRemoteManifestDelete_EmptyParentsIs202 at the handler, and the marked-parent case against real SQL in the integration walk
AC #107 A bulk batch naming an index and its child evicts both A later step's (container remote bulk pass). Out of scope.
Error cases and security considerations
Endpoint / condition Where
Tag upsert, kind is remote: 405 method_not_allowed with Allow: GET, DELETE TestContainerTagUpsert_RemoteRepositoryIs405, ..._ResponsesMatchOpenAPIContract, TestContract_TagUpsert_DeclaresItsResponses
Remote manifest eviction blocked by a live parent: 409 with digests in error.details.parents TestContainerRemoteManifestDelete_IndexedManifestIs409WithParents, ..._ConflictBodyMatchesTheHostedRefusal
Kind is virtual: 404 TestContainerImageDelete_RemoteKindDispatch, TestContainerRemoteManifestDelete_KindDispatch, TestContainerTagUpsert_NonRemoteKindsKeepTheirAnswers
Format differs from the :format segment: 404 The maven-remote row of TestContainerImageDelete_RemoteNotFoundMatrix and TestContainerRemoteManifestDelete_NotFoundMatrix
Parent or artifact missing, non-canonical, soft-deleted, or outside the chain: 404 TestContainerImageDelete_RemoteNotFoundMatrix, ..._RemoteRejectsNonCanonicalIDs, TestContainerRemoteManifestDelete_NotFoundMatrix
Unexpected failure: 500 TestContainerImageDelete_RemoteFailureMatrix, TestContainerRemoteManifestDelete_FailureMatrix, and the two evictor-failure log tests
Malformed JSON body on the remote upsert Refused ahead of the decode: TestContainerTagUpsert_RemoteRefusalBeatsTheLeafRejections. Validating a body the route can never accept would hand back a syntax oracle.
Method neither the path nor a sibling wildcard registers: 405 Router-owned and unchanged: TestContract_MethodNotAllowed_MatchesErrorSchema
Concern Where
Existence hiding on writes Every break in the two not-found matrices answers its level's envelope and reaches no write seam
Tenant isolation across the full parent chain The foreign-repository and foreign-namespace survival rows, and the cross-image survival row
No echoed input in a failure response NotContains assertions on every not-found row, plus the 405's message assertions
The 409 names server-derived digests The details key set is exactly {parents}, and the target digest is absent from the body
Cancellation is never a not-found Handler and store both: a cancelled context surfaces context.Canceled and explicitly not ErrNotFound, which the 404 would otherwise hide
Authorization S08 and S09 stubs, the accepted closed-beta posture the spec records. Not enforced here.
Injection Jet builder with bound parameters.

Test plan

go build ./... && go vet -tags=integration ./internal/... ./cmd/artifact-registry/
go test ./internal/managementapi/... ./internal/datastore/...
go test -tags=integration ./internal/managementapi/... ./internal/datastore/...

Integration needs PostgreSQL with max_locks_per_transaction=4096. Stock configuration fails roughly 95 tests. Locally the tagged managementapi run is 839 tests and 3443 subtests green against PostgreSQL 17.10, the container remote eviction walk included.

golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/... ./cmd/... exits 1 on a pre-existing repo-wide baseline of 4408 issues (3868 contextcheck) that lives entirely in integration-tagged files CI never compiles. Zero findings land on any line this branch touches, and internal/managementapi reports 0.

Reviewable size

4624 added lines, past the 500-line guardrail, split 1090 production Go / 3442 test Go / 92 non-Go. Tests are 74% of the diff: three handler suites (one per armed route), an 858-line shared fake bundle, and a real-database walk of both arms. The production half does not split usefully, because the three arms share one seam file, one resolver, and one wiring change, so cutting it into three MRs would either duplicate that shared half or stack three MRs that cannot be reviewed independently. The plan budgeted roughly 250 production and 600 test lines. Both halves overran: production is 1090 added and 927 net once the hosted handlers moved out of their closures, and tests are 3442.

e2e scenarios

No scenario is added. docs/testing/e2e/docker.md already lists "Virtual and remote (proxy/cache) repositories" under "Out of scope until the capability ships", and repository creation still refuses every non-hosted kind, so there is no e2e-reachable path to cover.

Plan status row

Step 30's row in the plan's Status table stays empty here. Phase 6 fills those rows in one batch from a single branch, because every step branch editing the same table conflicts with every other one. Steps 29 and 31 are in the same state.

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.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.ContainerRemoteRepositoryStore.FindContainerRemoteRepositoryID Limit -> Index Scan unique_container_remote_repositories_namespace_id_repository_id 1 / 1 8.30 0.019ms 3 / 0 1/64
datastore.ContainerRemoteRepositoryStore.FindContainerRemoteRepositoryID

Summary: Plan matches the method's intent. The probe is satisfied by an Index Scan over the unique (namespace_id, repository_id) index, and the bound namespace_id prunes to one of the 64 hash partitions at plan time, so the scan never reaches the other 63. Estimated and actual rows agree (1 / 1) and execution stays at 0.019 ms against 5000 seeded rows in the target partition. No anomalies.

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

Rendered SQL:

SELECT container_remote_repositories.id AS "container_remote_repositories.id"
FROM public.container_remote_repositories
WHERE (container_remote_repositories.namespace_id = $1::uuid) AND (container_remote_repositories.repository_id = $2::uuid)
LIMIT $3;

Bound args: [bbd24480-3c52-43a7-bdc4-e100ffdc896a, d4887611-ce1b-4dbd-aa84-ba73df7dfc40, 1]

Plan:

 Limit  (cost=0.28..8.30 rows=1 width=16) (actual time=0.007..0.008 rows=1 loops=1)
   Buffers: shared hit=3
   ->  Index Scan using container_remote_repositories__namespace_id_repository_id_idx28 on container_remote_repositories_p31 container_remote_repositories  (cost=0.28..8.30 rows=1 width=16) (actual time=0.007..0.007 rows=1 loops=1)
         Index Cond: ((namespace_id = 'bbd24480-3c52-43a7-bdc4-e100ffdc896a'::uuid) AND (repository_id = 'd4887611-ce1b-4dbd-aa84-ba73df7dfc40'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=3
 Planning Time: 0.089 ms
 Execution Time: 0.019 ms

Timings: planning 0.089ms, execution 0.019ms, total 0.108ms.

Migration mode did not run: this change adds no migration.

Related to #314

Edited by Hayley Swimelar

Merge request reports

Loading
Loading