Artifact registry: Add the client artifact list reads

What does this MR do and why?

Adds ArtifactRegistry::Client#packages and #images, the two artifact list reads the hosted repository detail page renders, plus the value and page objects they return.

This change is behind the feature flag :artifact_registry_ui (dark, default_enabled: false). The methods are inert until a resolver calls them, which is a later step.

This is monolith/S05 Step 1, the client half of the artifact list. The GraphQL connections that call these methods are Steps 3 and 4.

One method per endpoint, and the format is the caller's

Artifact Registry answers artifacts per format family (ADR-009): Maven and npm return packages, the container formats return images. So there are two methods over GET /api/v1/:slug/repositories/:repository_name/:format/{packages,images}, and format: is the repository's own value — the client never infers it.

#packages picks its row class from that argument, and a format outside the endpoint's family raises ArgumentError before any credential is acquired or any request is made, so no row class is guessed for a format the endpoint cannot serve. This is the client's existing fail-closed posture, alongside guard_present! and guard_segments!.

A list 404 returns nil rather than raising

The repositories list raises on nothing but the slug: a 404 there can only mean an unresolved namespace. A caller reaches these methods holding a repository it has already read, so a 404 here also means the repository was deleted between the two reads, or the format segment does not match the repository's own. Existence-hiding leaves those indistinguishable to the caller, so the absence is reported as nil and logged for whoever has to tell them apart — the same nil-and-log posture #repositories already takes.

Rows read only the keys the page renders

The value objects are Hash-backed and expose only the Phase 1 fields, so a field the contract adds later is tolerated rather than breaking the read. id is on every row — the GraphQL element types need it for Apollo cache normalization.

class fields
MavenPackage id, group_id, artifact_id
NpmPackage id, name, scope, versions_count
Image id, name

last_downloaded_at and npm's tags_count are in the contract and deliberately unread: no Phase 1 surface renders them, and the specs assert no reader exists.

Changes

  • ee/lib/artifact_registry/client.rb#packages and #images over a shared private artifact_page helper carrying the segment guards, the keyset query, the Link-header cursor parse, and the 404 rescue. Both validate their format through one guard_format! against PACKAGE_FORMATS or IMAGE_FORMATS, and package_class keeps a raising else as a second line of defense, so a format added to the constant without a class here fails rather than resolving to whichever branch is last.
  • maven_package.rb, npm_package.rb, image.rb (new) — the row value objects. Two package classes rather than one so the GraphQL union in Step 3 can discriminate by class.
  • page.rb (new) — the rows plus both opaque cursors. See The page object is shared, not per format below.

No sort or order argument: the contract sorts artifact lists by name only, so Phase 1 has nothing to pass.

The page object is shared, not per format

This is why the MR touches files outside Step 1's scope.

The artifact reads first shipped PackagePage and ImagePage beside the existing RepositoryPage, which review found to be the same class three times with one reader renamed. All three are now a single ArtifactRegistry::Page exposing nodes, which is also the vocabulary the GraphQL connection built from these pages uses, and what the frontend fixtures already read.

Two consequences, both deletions:

  • paginates_lists.rb (monolith/S03) loses its nodes: parameter and the GitlabSecurity/PublicSend disable that parameter required. No caller ever passed it; it existed only in anticipation of pages naming their collection differently.
  • artifact_page takes no block. With one page class the #packages and #images blocks differed only in the row class, so the helper takes row_class: and builds the page itself.

repository_page.rb and its spec are deleted rather than left as an alias, so there is one page object rather than one plus a deprecated name.

How to set up and validate locally

The two spec groups must run in separate invocationsclient_spec.rb uses spec_helper and the value/page specs use fast_spec_helper, and one combined run aborts with "Detected that fast_spec_helper was already loaded before spec_helper".

bundle exec rspec ee/spec/lib/artifact_registry/client_spec.rb
bundle exec rspec ee/spec/lib/artifact_registry/{maven_package,npm_package,image,page}_spec.rb

205 and 22 examples respectively, all green. The AR HTTP interaction is WebMock-stubbed throughout, so no running Artifact Registry service is needed.

Because the shared Page reaches monolith/S03 and monolith/S04 code, these run too:

bundle exec rspec ee/spec/graphql/resolvers/concerns/artifact_registry/paginates_lists_spec.rb \
  ee/spec/graphql/resolvers/artifact_registry/repositories_resolver_spec.rb
bundle exec rspec ee/spec/requests/api/graphql/organizations/artifact_registry_repositories_spec.rb \
  ee/spec/graphql/resolvers/artifact_registry/base_resolver_spec.rb

21 examples each, all green.

Notes for review

  • The specs assert the credential never leaves the server side: acquired via token_for(current_user, slug), attached as a Bearer header and never in the request URI, and redacted from both the raised and the logged error when Artifact Registry echoes it back in an error envelope.
  • request_id is preserved on every surfaced error. The convention is monolith/S02's; it is asserted here because these are new call sites.
  • The 404-as-absence rescue is one helper, nil_on_missing. Whether the absence is logged stays with the caller: a missing repository is an ordinary browse outcome, while an unresolvable namespace or artifact path means a slug or name has drifted from Artifact Registry's. #delete_repository keeps its own rescue, since it matches on the error code too and answers true.
  • repository_name: rather than name: on these two methods, unlike the four repository methods. It matches the REST contract's parameter and stays unambiguous beside the name that npm packages and images carry in their own rows. The cost is that guard_present! interpolates the kwarg, so a blank value reports repository_name is required here and name is required elsewhere.
  • The three page specs collapsed into one. Each of their five predicates survives once, and the per-class row coverage the separate files gave is kept as four positive hits over Repository, MavenPackage, NpmPackage, and Image.

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.

References

Edited by Rahul Chanila

Merge request reports

Loading
Loading