S05/Step 1: Add Artifact Registry client artifact-delete writes

This MR targets master and is the base of a stack. Steps 2 and 3 sit on top of it and will be linked from the tracking issue.

TLDR; Updates the AR Client with the methods to call the AR endpoints to bulk delete artifacts and delete one artifact.

There is no separate endpoint for cache eviction or deleting a hosted package. The same endpoint handles both. The AR backend has logic to determine if it is hosted or remote. If it's remote, it deletes the cache, if it's hosted it deletes the package.

So with these steps, we'll implement both at the same time.

What this does

This adds two artifact-delete write methods to the Artifact Registry Ruby client at ee/lib/artifact_registry/client.rb: bulk_delete_artifacts(slug:, repository_name:, format:), which posts {"delete_all": true} to AR's bulk-delete route and takes every artifact in a repository, and delete_artifact(slug:, repository_name:, format:, id:), which deletes one artifact. Both return nothing, because AR answers both routes with 202 and no body.

A private helper, artifact_collection(format), derives the route collection from the repository format (packages for maven and npm, images for docker and oci), raising ArgumentError naming both lists for any other format, so it fails before any request is made.

Dispatch is AR's, not the client's

AR reads kind from its own repositories row: hosted permanently deletes the published artifacts, remote evicts the cached rows so a later pull re-caches, and virtual answers 404, owning no artifact rows to act on. Both live arms answer 202, so the response does not distinguish them. The client gets no kind argument, and a repository name doesn't encode one, so it neither does nor can guard this.

The mutations in !252723 (merged) add no kind check either: AR dispatches on kind and authorizes the write (the delete_artifact ability, ADR-021).

This MR's specs assert remote maven works on both methods, correct against AR's handler code but not reflected in AR's own contract: api/openapi/v1.yaml's bulkDeletePackages and deletePackage name only npm as evicting on a remote repository, and deleteContainerImage omits the remote arm. A doc fix is being raised separately against AR.

Retry change (read this part)

This narrows the client's connection-wide Faraday retry allowlist to exclude DELETE: RETRY_OPTIONS[:methods] goes from Faraday::Retry::Middleware::IDEMPOTENT_METHODS to (Faraday::Retry::Middleware::IDEMPOTENT_METHODS - [:delete]).freeze. Reason: if the response to a successful delete is lost in transport, a retry replays the request against an artifact already gone, AR answers 404, and the caller reports failure for a delete that worked.

A retry_if predicate scoped to one call cannot fix this alone: faraday-retry's retry_request? is @options.methods.include?(env[:method]) || @options.retry_if.call(env, exception), an OR, so a predicate can add retries but never subtract one for a method still in methods. Narrowing is not, though, the only workable fix: methods: [] plus a retry_if predicate and req.options.context gives a working per-call opt-out. Narrowing was chosen because it fails safe by default (a DELETE added later opts out rather than in), which the per-call alternative lacks.

Cost: delete_repository is the client's only other DELETE. It rescues its own 404 as a successful delete, so a lost response on a delete that had actually succeeded used to replay into 404 and resolve as success. That path is gone: the same scenario now raises UnavailableError. No data is lost, since repeating the delete succeeds through the first-attempt rescue. This is bounded: the idempotence rescue keeps direct coverage via a single-404 example needing no retry, and delete_repository's only caller sits behind the disabled artifact_registry_ui flag.

Testing

Specs are WebMock-stubbed: a positive case per format (maven, npm, docker, oci) on both methods, a bulk 503 on both collections, a single-artifact 404 raising ApiError with the status, a request-count assertion proving no retry on the single delete plus a companion proving an idempotent GET still retries, and an unlisted format raising before any request. Credentials are never in a URI or body, and an echoed credential is redacted in both the raised and logged error.

AR's handler arms for these routes are implemented and released (from v1.387.0, latest v1.437.0), so WebMock is the right level, not a placeholder against unimplemented behaviour. What is unverified is which AR version the integration environment runs.

Feature flag and changelog

This ships dark behind artifact_registry_ui, which is disabled. No changelog entry, since the change sits entirely behind a disabled flag.

References

Tracked by #626611 (closed), which covers all three steps.

Edited by Fiona McCawley

Merge request reports

Loading
Loading