AR delete surface/Step 2: Client container manifest and tag delete writes

What does this MR do and why?

This is Step 2 of 19 in the plan for the monolith artifact delete surface: delete support for versions, container manifests, container tags, files, and npm dist-tags, running from the Ruby client up through GraphQL mutations to the Vue affordances.

It can be reviewed and merged independently of Step 1, !253251 (merged). The two touch the same two files but at different anchors, and neither depends on the other's methods or helpers.

Files changed

• ee/lib/artifact_registry/client.rb: 2 new public methods, 1 new private path builder, and a widened private error-details allowlist, which is split into 3 small private predicates.
• ee/spec/lib/artifact_registry/client_spec.rb: 56 new examples.

Diff size: 335 insertions, 7 deletions across the 2 files.

The two new client methods

• #delete_manifest(slug:, repository_name:, format:, image_id:, digest:) calls DELETE /api/v1/{slug}/repositories/{repository_name}/{format}/images/{image_id}/manifests/{digest}.
• #delete_container_tag(slug:, repository_name:, format:, image_id:, tag_name:) calls DELETE /api/v1/{slug}/repositories/{repository_name}/{format}/images/{image_id}/tags/{tag_name}.

Both guard format against the existing IMAGE_FORMATS constant (docker, oci), not the ARTIFACT_FORMATS union (maven, npm, docker, oci) that the already merged #delete_artifact uses. Artifact Registry's committed OpenAPI contract types the format path parameter on these two routes as a container format only, so a Maven or npm repository has no reachable images under any format segment.

Both return nil. Artifact Registry answers 202 with no body on both routes. Acceptance is not completion: the 202 says the request was accepted, not that the delete has already been applied, so a caller that needs to know has to re-read the manifests list. Neither method returns a count or a job handle, and nothing is parsed from the response. Any other 2xx is refused as contract drift through the existing private raise_unexpected_success, rather than read as an acceptance.

Neither method retries. The client's connection-wide RETRY_OPTIONS[:methods] already excludes :delete, and both inherit that with no change here. A replayed DELETE is dangerous because Artifact Registry answers 404 for a manifest or tag the first attempt already removed, which would report a failure for a delete that worked. A spec asserts the DELETE is issued exactly once on a transport timeout.

Neither method takes or checks a repository kind. Artifact Registry reads kind off its own repository row and decides whether the request permanently deletes a hosted artifact or evicts a remote cached copy. Both arms answer 202, so the response does not distinguish them, and Artifact Registry is the authorization boundary here rather than the monolith.

Design decision 1: no local digest or tag-name validation

digest and tag_name are guarded only for presence and for bare . / .. dot-segments, through the existing private guard_segments!. There is no sha256:<hex> check on the digest and no tag-grammar check on the tag name.

That is deliberate and it comes from the contract. Artifact Registry's manifestDigest path parameter declares no digest pattern, and its tagName path parameter asserts no grammar and no upper length bound. Both answer 404 for a value they cannot resolve, whether or not the value is well formed, so that the URL exposes no syntax oracle. Validating the spelling in the client would convert that 404 into a local ArgumentError, a different error class for the caller to map, and one that leaks exactly the syntax information the route withholds. Contrast the upsert route, which does return 400 for a tag name the grammar rejects, because an upsert names the tag it wants created rather than probing which names exist. This MR does not touch upsert.

Specs cover a 404 on both a well-formed and a malformed value for each method, so the mapping is asserted rather than assumed.

Design decision 2: the nested-member path builder composes

Both routes address a member under a sub-collection, and no existing path builder returned that shape. The existing private builders are artifacts_path (a collection), artifact_path (a top-level member), sub_collection_path (a sub-collection), and bulk_delete_path.

The new private sub_collection_member_path appends /#{encode_segment(member_id)} to sub_collection_path rather than rebuilding the prefix, so it stands to sub_collection_path exactly as artifact_path stands to artifacts_path. Both new methods use it.

Design decision 3: the error-details allowlist widening, and why it needs its own predicate

The manifest delete is the one artifact delete another artifact can refuse. When a second manifest indexes the target, Artifact Registry answers 409, deletes nothing, and puts the digests of the blocking manifests in error.details.parents (the contract's ManifestDeleteConflict schema: at least one entry, no declared upper bound, uniqueItems: true). Those digests are the caller's next move, so a later step in the plan renders them in the UI error copy.

A 409 already reached the caller as ApiError through the client's default error arm, but the digests did not, because the private allowlisted_details carried exactly one details shape, the repository-verification endpoint's repository_ids, and returned nil for every other one. details bypasses the snippet redaction every other error field goes through, so the allowlist is what keeps a credential an intermediary might echo from riding along.

The widening needs its own element predicate rather than reusing the existing arm's. The repository_ids arm admits an element only when it is a String passing Gitlab::UUID.v7?, and parents carries digests, not ids. Pattern-matching the existing arm would inherit a UUIDv7 predicate that rejects every digest, which would leave parents dropped exactly as it is today, and the later UI step with nothing to render.

The new digest predicate is a new DIGEST_SHAPE_REGEX constant, /\A[a-z0-9]+(?:[+._-][a-z0-9]+)*:\h{32,128}\z/. It is looser than the contract's canonical Digest pattern (^sha256:[0-9a-f]{64}$) on purpose, and tighter than a bare String on purpose:

• Looser, because the contract types every parents entry as canonical Digest, but the plan's Step 2 acceptance criteria require that a 409 whose parents carry a non-canonical digest still reaches the caller, which is the case a borrowed UUIDv7 predicate silently drops (gitlab-org/ops/artifact-registry!2201 (merged)). Pinning the canonical pattern would turn any future spelling the contract does not yet describe into a silently dropped list, rather than one the caller can act on.
• Tighter than a bare String, because details skips the redaction, so the shape is what bounds what can pass through. A spec asserts that a parents list carrying a non-digest-shaped entry (a Bearer <jwt> string) is dropped rather than surfaced.

allowlisted_details now returns only the keys whose arm admitted a list, and nil when neither did, which matches the old behavior for a details shape it does not recognize. The repository_ids arm keeps the UUIDv7 rule it had, because those ids are also written to IAM afterwards, whose proto CEL regex accepts UUIDv7 alone.

A note on the digest's colon

The digest contains a colon, and the client's existing shared encode_segment percent-encodes it, so the wire path carries sha256%3A<hex> rather than sha256:<hex>. That was verified to be safe rather than assumed: Artifact Registry's management API routes on Go's net/http.ServeMux, which unescapes each path segment before matching, so the pattern matches and r.PathValue("digest") reads back the decoded sha256:<hex>. This was confirmed with a small local Go probe against a ServeMux carrying the same route pattern: both the percent-encoded and the raw-colon form matched and yielded the identical decoded digest.

Test coverage

56 new examples, driven by one shared example group named "a single container artifact delete" so both methods are held to the same posture, plus manifest-specific examples for the 409.

Shared, per method: a positive 202 per container format (docker and oci); an empty 202 body treated as success with nothing parsed; a 404 on both a resolvable-shaped and a malformed path value raising ApiError with the status, rather than resolving to nil or raising a validation error; 401 and 403 raising AuthorizationError rather than ApiError, with no error tracking; 500 and 503 raising UnavailableError with an echoed credential redacted in both the raised and the logged error; a 200 refused as contract drift; maven, npm, and an unserved format rejected before any request is made, asserted by request count; blank and bare dot-segment slug, repository name, image id, and member segment rejected before any request; percent-encoding of every path segment so a value cannot smuggle extra segments; the credential attached as a Bearer header and never in the request URI; and exactly one DELETE issued on a transport timeout.

Manifest-specific: a 409 carrying error.details.parents raising with those digests readable on the error; a 409 whose parents carry a non-canonical digest still reaching the caller; a 409 whose parents carry a non-digest-shaped entry dropped; and a 409 whose details carry no digests (empty details, an empty parents array, and a different key) raising with details nil rather than inventing any.

Full file result, confirmed locally: 732 examples, 0 failures (56 of them new). Rubocop clean on both files.

Feature flag and changelog

Ships dark behind the artifact_registry_ui feature flag, which is disabled. No changelog entry, because the change sits entirely behind a disabled flag. The commit carries an EE: true trailer.

References

• Implements this step: #627392 (closed)
• Parent scoping issue: #627172
• Plan merge request that gates this step, must merge first: gitlab-org/ops/artifact-registry!2201 (merged)
• Step 1, reviewable and mergeable independently of this one: !253251 (merged)
• The S05 client precedent both steps follow: !252683 (merged)

Screenshots or screen recordings

Not applicable. This is a client-layer change only, with no UI.

How to set up and validate locally

1. Run the specs.

   bundle exec rspec ee/spec/lib/artifact_registry/client_spec.rb

   Confirmed result: 732 examples, 0 failures (56 of them new).

2. Run a script against a local stub HTTP server. Save it as /tmp/ar_step2_check.rb and run it with bundle exec rails runner /tmp/ar_step2_check.rb.

   require 'socket'
   
   seen = []
   responses = [
     [202, ''],
     [202, ''],
     [409, { error: { code: 'conflict', message: 'the manifest is indexed by another manifest',
                      request_id: 'req-conflict-id',
                      details: { parents: ["sha256:#{'a' * 64}"] } } }.to_json],
     [409, { error: { code: 'conflict', message: 'the manifest is indexed by another manifest',
                      request_id: 'req-conflict-id',
                      details: { parents: ['not-a-digest'] } } }.to_json],
     [404, { error: { code: 'not_found', message: 'not found', request_id: 'req-404' } }.to_json]
   ]
   
   server = TCPServer.new('127.0.0.1', 0)
   port = server.addr[1]
   
   Thread.new do
     loop do
       socket = server.accept
       request_line = socket.gets
       while (line = socket.gets) && line != "\r\n"; end
       seen << request_line.strip
       status, body = responses.shift || [202, '']
       socket.print("HTTP/1.1 #{status} X\r\nContent-Type: application/json\r\nContent-Length: #{body.bytesize}\r\n\r\n#{body}")
       socket.close
     end
   rescue IOError, Errno::EBADF
     nil
   end
   
   exchange = Object.new
   exchange.define_singleton_method(:token_for) { |_user| 'stub-token' }
   
   client = ArtifactRegistry::Client.new(
     base_url: "http://127.0.0.1:#{port}", current_user: Object.new, token_exchange: exchange
   )
   
   digest = "sha256:#{'1' * 64}"
   
   puts "delete_manifest(docker)      => #{client.delete_manifest(slug: 'my-group', repository_name: 'my-repo', format: 'docker', image_id: 'img-1', digest: digest).inspect}"
   puts "delete_container_tag(oci)    => #{client.delete_container_tag(slug: 'my-group', repository_name: 'my-repo', format: 'oci', image_id: 'img-1', tag_name: 'v1.2.3').inspect}"
   
   begin
     client.delete_manifest(slug: 'my-group', repository_name: 'my-repo', format: 'docker', image_id: 'img-1', digest: digest)
   rescue ArtifactRegistry::Client::ApiError => e
     puts "409 with parents digests    => status=#{e.status} code=#{e.code} details=#{e.details.inspect}"
   end
   
   begin
     client.delete_manifest(slug: 'my-group', repository_name: 'my-repo', format: 'docker', image_id: 'img-1', digest: digest)
   rescue ArtifactRegistry::Client::ApiError => e
     puts "409 with a non-digest entry => status=#{e.status} details=#{e.details.inspect}"
   end
   
   begin
     client.delete_manifest(slug: 'my-group', repository_name: 'my-repo', format: 'docker', image_id: 'img-1', digest: 'malformed')
   rescue ArtifactRegistry::Client::ApiError => e
     puts "404 on a malformed digest   => status=#{e.status} code=#{e.code}"
   end
   
   [[:delete_manifest, :digest, 'maven'], [:delete_container_tag, :tag_name, 'npm']].each do |method, arg, format|
     client.public_send(method, slug: 'my-group', repository_name: 'my-repo', format: format, image_id: 'img-1', arg => 'x')
   rescue ArgumentError => e
     puts "#{method}(#{format}) => ArgumentError: #{e.message}"
   end
   
   [[:delete_manifest, :digest], [:delete_container_tag, :tag_name]].each do |method, arg|
     client.public_send(method, slug: 'my-group', repository_name: 'my-repo', format: 'docker', image_id: 'img-1', arg => '')
   rescue ArgumentError => e
     puts "#{method}(blank member) => ArgumentError: #{e.message}"
   end
   
   sleep 0.2
   puts "\nrequests the stub server saw:"
   seen.each { |line| puts "  #{line}" }
   server.close

   Confirmed output:

   delete_manifest(docker)      => nil
   delete_container_tag(oci)    => nil
   409 with parents digests    => status=409 code=conflict details={"parents"=>["sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]}
   409 with a non-digest entry => status=409 details=nil
   404 on a malformed digest   => status=404 code=not_found
   delete_manifest(maven) => ArgumentError: format must be one of: docker, oci
   delete_container_tag(npm) => ArgumentError: format must be one of: docker, oci
   delete_manifest(blank member) => ArgumentError: digest is required
   delete_container_tag(blank member) => ArgumentError: tag_name is required
   
   requests the stub server saw:
     DELETE /api/v1/my-group/repositories/my-repo/docker/images/img-1/manifests/sha256%3A1111111111111111111111111111111111111111111111111111111111111111 HTTP/1.1
     DELETE /api/v1/my-group/repositories/my-repo/oci/images/img-1/tags/v1.2.3 HTTP/1.1
     DELETE /api/v1/my-group/repositories/my-repo/docker/images/img-1/manifests/sha256%3A1111111111111111111111111111111111111111111111111111111111111111 HTTP/1.1
     DELETE /api/v1/my-group/repositories/my-repo/docker/images/img-1/manifests/sha256%3A1111111111111111111111111111111111111111111111111111111111111111 HTTP/1.1
     DELETE /api/v1/my-group/repositories/my-repo/docker/images/img-1/manifests/malformed HTTP/1.1

   Reading that output: the two paths match the contracted routes. The digest's colon is percent-encoded by the shared segment encoder and Go's ServeMux unescapes it back before matching, as noted above. The 409 reaches the caller with the blocking digest readable on the error, where before this change it would have been stripped. A parents entry that is not digest-shaped is dropped rather than surfaced. The malformed digest earns a 404 from the route rather than a local validation error. The container-format guard rejects maven and npm before any request is made. No request body is sent on either route. A stub server is the right level for a client-layer change here, because Artifact Registry's handler arms for both routes are already released, so this is not standing in for unimplemented behavior on the Artifact Registry side.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.