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 msTimings: planning 0.089ms, execution 0.019ms, total 0.108ms.
Migration mode did not run: this change adds no migration.
Related to #314