feat(managementapi): declare the container tag upsert operation

Stacked on feat(managementapi): declare the artifact delet... (!1445 - merged) • Hayley Swimelar • 19.3, merges after it.

Why

The tag-upsert PUT is the one Phase 4 artifact write that is not a delete, and the only operation that answers limit_exceeded. Declaring it ahead of its handler lets a consumer generate the client now, and the parent MR's pending-sweep set keeps the arms unserved without reddening the sweep. Step 2 of the merged S17 Phase 4 plan, whose Approach slices the phase into four contract MRs.

Nothing serves the operation yet, by design. PUT on the tag path answers the mux's text/plain 405 until the write-route scaffold registers it, which is the state the two pending entries record.

What

  • The tag cap binds per manifest, and the spec at this branch's base agrees. ContainerManifestPersister.moveTag counts container_tags by container_manifest_id and runs the check on the manifest the digest names for any real pointer change, skipping it only when the tag already points there. docs/specs/S17-rest-management-api.md carries that reading on main, and this branch is rebased past the MR that corrected it, docs(specs): correct four S17 claims the merged... (!1453 - merged) • Hayley Swimelar • 19.3. Creating a tag, or retargeting one, onto a manifest already at the cap answers 422 limit_exceeded. Repeating a tag's current digest answers 204, because it moves nothing.
  • The 422 is declared inline rather than referencing UnprocessableEntity. Its description is the only place the two codes a 422 carries are told apart, and the shared prose covers the repository writes instead. Contract tests pin that description clause by clause, not by keyword: an earlier keyword-anchored version passed a rewrite that scoped the count correctly and then called a retarget free, which mutation testing caught in review.
  • The cap is described without its number. manifest_max_tags is operator-configurable, and the repository's own testdata runs it at 500, so the contract says the target manifest "already holds its tag cap" rather than quoting 1,000. The document's numberless convention for configurable caps is what this follows.
  • No 409, and both successes are empty. A tag upsert refuses nothing, so the cap failure is a 422 rather than a conflict, asserted negatively. The spec states 201 on create and 204 on retarget and is silent on their bodies: a 204 cannot carry one, and a body on the 201 alone would make a client branch on status before knowing whether to decode, so the status is the whole signal and the caller re-reads tag detail. No Location on the 201 either, since a PUT creates at the request URI, and a test now pins that promise.
  • The shared tag_name parameter's 404 rule is scoped to the read and the delete. It read as a whole-path fact, which the upsert's grammar 400 contradicts quoted on its own. The parameter now also names who owns the grammar and warns that it is stricter than the maxLength: 255 in the same paragraph.
  • 413 is declared because the operation carries a body, matching createRepository and updateRepository. The cap itself stays middleware-owned.

Spec coverage

Spec rule Test
The document defines every Phase 4 endpoint and declares each operation's failure responses (the tag-upsert slice, AC #30) TestContract_TagUpsert_DeclaresItsResponses
PUT returns 201 on create and 204 on retarget (AC #52, contract half) same, closed status set plus both successes declaring no content and the 201 declaring no Location
A well-formed digest naming no manifest returns 422, a malformed digest or rejected :tag_name returns 400 (AC #53) same, plus TestContract_UpsertContainerTagRequest_ValidatesBodies (eight bodies: wrong algorithm, short hex, uppercase hex, non-string, null, unknown field, missing digest)
The cap failure is 422 limit_exceeded on the target manifest, a retarget at the cap answers the same 422, and an unchanged digest answers 204 (AC #54) same, clause-level description assertions, mutation-proven against the image-scoped rewrite
A new envelope code lands as a transport constant and an enum value together TestContract_ErrorCodeEnum_MatchesTransportCodes in both this package and internal/gitlabapi, whose known list claims the full transport set
A declared arm with no handler is exempt only while it answers as unserved, and an entry naming no declared arm fails as orphaned TestContractOperations_AreRegisteredAndServed, the two upsertContainerTag entries and its closing loop
Input validation runs before any artifact access the request schema's required digest, closed property set, and canonical digest pattern
No failure response echoes a submitted digest the 422 pins to the plain Error envelope, so it declares no details shape a digest could travel back in

Behavioral halves belong to the handler step. This MR ships no handler, so nothing here creates a tag, moves one, or counts tags on a manifest.

No docs/testing/ e2e scenario is added or affected. This MR declares a contract and registers no route, and the plan's Testing Strategy assigns the management-write journeys in docs/testing/e2e/docker.md and docs/testing/e2e/oci.md to the container handler steps.

Two files outside the step's declared list

  • docs/dev/api-style.md: its status table read "Successful GET, PATCH, PUT" for 200 and 204 for DELETE alone. This MR declares the document's first PUT, so it is the change that creates the contradiction. Three rows widened, nothing else.
  • internal/gitlabapi/contract_test.go: one line. That test's known slice is documented as the full transport constant set, which CodeLimitExceeded would otherwise falsify.

Noted for follow-up, outside this MR

  • The plan's Step 19 carries the stale cap reading in three places: 422 limit_exceeded "on create only", "retarget-at-cap succeeds", and a test bullet reading "cap at the boundary (create rejected, retarget allowed)". A handler built to that text would violate this contract, and nothing here can catch it. Step 19 needs the corrected reading the way this step got it.
  • Step 19 should derive 201 against 204 from the write, not from a probe. The contract says the two report whether the tag existed. A read-then-write discriminator answers 201 twice for concurrent same-name creates, where RETURNING (xmax = 0) on the upsert does not.
  • The v1.yaml comment reading "The Phase 3 artifact resources below leave additionalProperties open" now sits above one closed request schema. It is narrowly true, since it speaks of response resources, and the schema explains its own closure. Naming that subject explicitly is a one-word reword on a line the sibling contract MRs also touch, so it waits for them.
  • docs/dev/bruno.md is not updated here. Its coverage sentence and request counts are shared with the sibling contract MRs adding Bruno files right now, so reconciling per branch would collide three ways and still be wrong once the others land. The plan's Status table row rides separately: the four contract rows fill in one pass through docs(plans): fill the wave-one status rows and ... (!1467 - merged) • Hayley Swimelar • 19.3.
Context for LLM agents

Rationale

  • Referencing the shared UnprocessableEntity response for the 422. Rejected because that description names the repository-write conditions and no limit_exceeded, and this operation's 422 is the one place the two codes are distinguished for a client that has to tell a condition clearing after cleanup from one that never does.
  • Anchoring the 422 prose assertions on keywords ("target manifest", "the same 422"). Rejected after a mutation showed a rewrite satisfying both anchors while restoring the per-image reading. The assertions now anchor whole clauses against a whitespace-normalized description, and the mutation fails them in both directions.
  • Quoting the cap as 1,000 in the contract. Rejected because manifest_max_tags is configurable and a client hard-coding the document's number breaks on any deployment that lowers it. ADR-004 stays cited, with attribution, in the Bruno docs and the test rationale.
  • A resource body on the 201. Rejected because its paired 204 cannot carry one, so the asymmetry would push a status branch into every client, and the spec's own acceptance criterion has the caller read the new manifest_digest from tag detail.
  • Putting the Bruno request in the artifact-deletes folder. Rejected because that folder's docs block asserts every request in it is destructive and answers 202, and an upsert is neither. It gets its own container-tags folder at seq: 3, leaving 4 and up for the sibling bulk folders.
  • An ADR-004 link in the 422 description. Rejected because api/openapi/v1.yaml references no ADR anywhere and renders on Pages. The Bruno docs names ADR-004, where two .bru files already cite ADR-009.
  • Squashing the simplifier commit into the test-author commit. Rejected because re-forming that commit re-spends the project's single --no-verify carve-out, which covers only the commit the test-first flow produces. The intermediate state never left this branch, and MR-level squash stays available.

Non-goals

  • Serving the operation, registering the route, or writing the handler. The write-route scaffold and the container tag upsert handler step own those, and the two pending entries exist precisely so this MR leaves the arms unserved.
  • Correcting docs/specs/S17-rest-management-api.md. That correction merged separately and this branch is rebased past it, so the spec here already reads as the contract does.
  • Declaring the manifests-per-image cap. defaultContainerImageMaxManifests is 25,000, mapped from ADR-004's versions-per-package, and no route in this step exposes it.
  • Enforcing the cap, the digest resolve, or the tag grammar. All three are handler-side, and the contract only has to make their outcomes expressible.
  • Mapping CodeLimitExceeded to a status. No code-to-status map exists in internal/transport, so the constant cannot fail open ahead of its first emitter. Its only live sibling today is the OCI push path's own 400, in a different envelope.

Related to #313 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading