chore(managementapi): add the bulk delete selector decode

Why

Six bulk-delete routes each accept one of two selector bodies, and all six need the identical strict decode: exactly one of a 1-1,000-entry list or delete_all: true, entries spelled per selector, repeats applied as a set. Landing that decode once, ahead of the routes that consume it, is what keeps six handlers from growing six copies of it.

Nothing routes to it yet. The bulk-route steps wire the six selectors, so this MR ships no behavior a caller can reach.

Step 27 of the S17 Phase 4 plan, against the S17 spec.

The spec was silent on repeated property names, so this MR states the rejection in it: the body-shape paragraph of Bulk delete, its acceptance criterion, and the bad-request row of Error Cases. Numbering is unchanged, the criterion is rewritten in place.

What

Three calls a reviewer would otherwise have to reverse-engineer:

  • The body decodes into map[string]json.RawMessage, not a per-route struct. The accepted key varies by route, and "exactly one of two keys" is a property of the object's key set rather than of a Go type. One consequence is load-bearing: the strict decoder's unknown-field check does not apply to a map, so the key count and lookup are what reject a key the route does not accept.
  • The entry list is read under the cap, not built and then measured. Building it first cost heap proportional to the body rather than to the cap: 94 MB allocated for a 7 MB body against a 512Mi pod limit, so a few concurrent requests exceeded that limit on heap alone. Reading under the bound costs 24 MB on the same body. internal/jsonsafe's package doc names this payload shape and prescribes this element-count cap.
  • The all-zeros UUID is refused even though it spells canonically. An entry naming no artifact is otherwise a no-op, but the stores answer a zero id with a validation error rather than the not-found sentinel, so admitting one would accept a batch no worker can resolve after the caller already holds its 202.
  • A repeated property name is refused, and the selector reads its own object to do it. The shared object decode is the encoding/json v1 shim, which keeps the last of two members, so {"delete_all": false, "delete_all": true} emptied a collection while its body opened by saying it did not. Names compare with escapes resolved, so the plain and escaped spellings of one name repeat each other in either order. Failures still route through transport.ClassifyDecodeFailure, so the size-cap 413 is unchanged and no other caller of DecodeJSONBody moves in this MR.

Size

1,513 added lines against the 500 ceiling: 420 production, 1,089 test, and 4 spec. The remainder is the decode matrix: six selectors across both branches, the cap boundary from both sides, each selector's entry grammar, and the allocation budgets that pin the two bounded reads. Splitting it would separate the decode from the table that defines it.

Spec coverage

Scoped to the selector decode. decode reads the request body and nothing else, so every route, worker, schema, and contract row of S17 belongs to another step of the plan and travels with that step's MR.

Acceptance criteria

# Criterion Tests
AC-47 Every bulk selector takes the identifier its artifact's own resource is addressed by, and a non-canonical UUID, a rejected tag name, or a non-canonical digest returns 400 TestBulkSelectorSpecs_MapEachRoutePropertyToItsEntryRule, TestBulkSelectorSpec_Decode_AcceptsASubsetOfItsOwnEntryType, TestBulkSelectorSpec_Decode_RejectsAnotherRoutesListProperty, TestBulkSelectorSpec_Decode_RejectsEntriesItsGrammarRejects
AC-55 A batch of exactly the cap is accepted, one over is rejected including when the excess is a duplicate, and an empty, omitted, or null list is rejected TestBulkSelectorSpec_Decode_ListLengthBounds (each rejecting row asserts which rule answered it). The 202 half is the route steps'.
AC-56 A batch of repeated entries is applied as a set TestBulkSelectorSpec_Decode_CollapsesDuplicatesInFirstSeenOrder (order, not just count), TestBulkSelectorSpec_Decode_ListLengthBounds (the cap of one repeated entry). The 202 half is the route steps'.
AC-58 Entry values and list length are validated separately from the body shape TestBulkSelectorSpec_Decode_ListLengthBounds, TestBulkSelectorSpec_Decode_RejectsEntriesItsGrammarRejects. The oneOf and generated-client half is the contract steps'.
AC-59 A body carrying both properties, neither, or "delete_all": false returns 400, as does a JSON type mismatch, and so does a repeated property name TestBulkSelectorSpec_Decode_RejectsBodiesMatchingNeitherBranch, TestBulkSelectorSpec_Decode_AcceptsDeleteAllOnEveryRoute, TestBulkSelectorSpec_Decode_RejectsARepeatedPropertyName, TestBulkSelectorSpec_Decode_AcceptsAnEscapedPropertyName
AC-64 A non-canonical UUID and a rejected tag name return 400, and no failure response echoes any submitted entry TestBulkSelectorSpec_Decode_RejectsEntriesItsGrammarRejects (every row asserts the no-echo property and which rule rejected it), TestIsCanonicalUUIDEntry, TestIsCanonicalUUIDEntry_NarrowsBeyondUUIDParse, TestIsCanonicalUUIDEntry_RejectsTheAllZerosUUID, TestIsContainerTagNameEntry, TestIsCanonicalDigestEntry
AC-66 A tag named delete_all, all, or bulk_delete is an ordinary tag name TestBulkSelectorSpec_Decode_TakesSelectorSpellingsAsOrdinaryTagNames, TestIsContainerTagNameEntry. The tag-list and removal half is the tag steps'.

Error cases

Condition Status Code Tests
Malformed JSON body 400 bad_request TestBulkSelectorSpec_Decode_RejectsBodiesMatchingNeitherBranch (non-object, empty, trailing data), TestBulkSelectorSpec_Decode_NonObjectBody_NamesNoGoType
Body matching neither selector branch: both properties, neither, or "delete_all": false 400 bad_request TestBulkSelectorSpec_Decode_RejectsBodiesMatchingNeitherBranch
Empty, omitted, or oversized list, a non-canonical UUID, a rejected tag_names entry, a JSON type mismatch, an unknown field, a repeated property name, or trailing data 400 bad_request TestBulkSelectorSpec_Decode_ListLengthBounds, TestBulkSelectorSpec_Decode_RejectsEntriesItsGrammarRejects, TestBulkSelectorSpec_Decode_RejectsBodiesMatchingNeitherBranch, TestBulkSelectorSpec_Decode_RejectsAnotherRoutesListProperty, TestBulkSelectorSpec_Decode_RejectsARepeatedPropertyName, TestBulkSelectorSpec_Decode_RejectsElementsThatAreNotStrings
Body over the request size cap 413 request_entity_too_large TestBulkSelectorSpec_Decode_OversizedBodyWithoutContentLength_Returns413 (the chunked path, where the cap trips mid-read)
Job backend unavailable at enqueue 503 service_unavailable Owned by the enqueue seam step. Decode never reaches the job backend.
Slug resolves to no namespace, auth 401 and 403, method 405 - - Router and middleware. Not reachable from a body decode.

Security considerations

Concern Tests
Echoed input: no failure repeats a submitted entry TestBulkSelectorSpec_Decode_RejectsEntriesItsGrammarRejects (per row), TestBulkSelectorSpec_Decode_NonObjectBody_NamesNoGoType extends it to the Go decode-target type
Input validation before any database access Every test drives decode with no store, namespace, or mux in reach, so a decode that touched one would not compile against them
Bounded blast radius: both reads are bounded, not just checked afterwards TestBulkSelectorSpec_Decode_OverCapListBoundsItsAllocation (the entry list), TestBulkSelectorSpec_Decode_ManyMemberObjectBoundsItsAllocation (the enclosing object)
Authentication, authorization, existence hiding, tenant isolation Not reached: the decode holds no namespace, no path value, and no query. Covered by the route and handler steps.

Test plan

go test ./internal/managementapi/ and -race, plus golangci-lint in both the default and --build-tags=integration modes. No database, no rig: the decode reaches its whole contract without a mux, a namespace, or a store.

Context for LLM agents

Rationale

  • Per-route decode struct with DisallowUnknownFields. Rejected: six near-identical structs, and a struct admits a sibling route's property through the decode before any check can reject it, so the exactly-one rule would still be hand-written on top.
  • []string unmarshal then bound the length. Rejected on measurement: 94 MB allocated for a 7 MB body versus 24 MB read under the bound, against a 512Mi pod limit with no GOMEMLIMIT and no in-flight limiter.
  • Reject an over-long raw list by byte length before decoding. Rejected as unsound: whitespace and \uXXXX escapes make a legal in-cap list arbitrarily long in bytes, so the bound would refuse valid batches on a destructive route.
  • internal/jsonsafe for the bound. It does not fit: its own package doc says it bounds nesting depth and object-key count only, never flat-array width, and names this shape as the case needing a caller-supplied element-count cap.
  • Adopting oci.ParseDigest. Rejected: it accepts uppercase hex and canonicalizes it, and an alias spelling of an identifier is refused here rather than silently rewritten.

Non-goals

  • A machine-readable discriminator for the four rejection classes. All four are 400 bad_request differing only in message, so a route cannot log which rule an integrator broke without string-comparing. Adding one means widening requestError, which this package's other handlers share.
  • Extracting the container tag grammar into a leaf package both managementapi and internal/format/oci import. internal/format/oci/ociroute is the precedent, and the extraction needs a third file.
  • The spec's claim that a bulk body is validated before any database round trip. slugMiddleware resolves the slug before every handler, so the round trip is already spent. The code comment says what is true, and the spec sentence needs its own patch.

Related to #313 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading