Add the repository packages connection on a Artifact registry detail type

What does this MR do and why?

Adds the packages keyset connection for Maven and npm repositories, over the merged client packages read, plus the organization seam the connection needs to reach that client.

This change is behind the feature flag :artifact_registry_ui (dark, default_enabled: false). It is monolith/S05 Step 3, and the first AR slice to mount a child connection that issues its own Artifact Registry request.

Important

This MR deviates from the merged monolith/S05 spec and needs a spec author's decision before it merges. The spec places both artifact connections on the shared ArtifactRegistryRepository type. This MR mounts packages on a new ArtifactRegistryRepositoryDetails type instead. The reasoning is in Why the connection is on a detail type below. If that reasoning is rejected, the last commit (61617eb4bab3) can be dropped on its own and the rest of the MR stands.

The elements are a union, not one type with nullable fields

Artifact Registry answers Maven and npm packages with structurally different rows (ADR-009). A single element type would make every Maven field nullable-but-set and every npm field nullable-but-set in alternation, erasing that split. So this adds ArtifactRegistryPackage, a BaseUnion over ArtifactRegistryMavenPackage (id, groupId, artifactId) and ArtifactRegistryNpmPackage (id, name, nullable scope, versionsCount), mirroring the FOSS PackageMetadata union.

resolve_type fails closed: a package class with no member type raises rather than resolving to whichever branch happens to be last. That is asserted.

The organization seam

ArtifactRegistry::RepositoryPresenter carries the organization a repository was read through. The value object holds no back-reference, so a connection mounted on the repository type has no other way to reach the slug and the memoized client its own read needs.

Rejected: context.scoped_set!, which works but hides the data flow in mutable context where the wrapper keeps it on the object graph the type system already documents.

Why the connection is on a detail type

packages issues one Artifact Registry request per repository, and nothing batches it. With the connection on the shared type, this was a valid query:

organization {
  artifactRegistryRepositories(first: 100) { nodes { packages { nodes { id } } } }
}

That is 1 + 100 sequential AR round trips from a single GraphQL request. Nothing in the codebase prevents it: GitlabSchema sets default_max_page_size 100, neither the field nor the resolvers declare max_page_size or complexity, and static complexity cannot see this cost because it scores the query shape rather than the row count. A 100-row fan-out scores like a cheap query.

graphql-ruby emits a subclass as an unrelated object type with no shared interface, so a client holding an ArtifactRegistryRepository from the list cannot select packages on it even through an inline fragment. The query above now fails validation. That is the property a numeric guard cannot provide, and it is asserted directly in ee/spec/graphql/types/artifact_registry/repository_details_type_spec.rb.

The spec already accepts this reasoning one level down: it caps artifact pages at 20 rows because "an uncapped page would multiply AR requests per query", then mounts the connection on a list that defaults to 100 wide. Once a child connection lands on the package elements, the two shapes compose and that cap does not survive the level above it.

The concrete cost: fragment-based selections break

Worth naming explicitly in the spec decision, because it is a stronger break than merely losing access to packages. organization.artifactRegistryRepository moves from ArtifactRegistryRepository to ArtifactRegistryRepositoryDetails, and since graphql-ruby emits the subclass as an unrelated type with no shared interface, a query that spreads a fragment declared on ArtifactRegistryRepository under that field now fails validation.

That is acceptable here only because the field is experiment behind a disabled flag, so it has no clients to break. It is the cost the spec author is being asked to accept, not a detail of the implementation.

Precedent for the pattern in this codebase: VirtualRegistries UpstreamType / UpstreamDetailsType (same group, current milestones, expensive child connection on the detail type only), plus ContainerRepositoryDetailsType, Packages::PackageDetailsType, and GoogleCloud::ArtifactRegistry::DockerImageDetailsType.

The split closes the fan-out across repositories. Two bounds close what it does not, both added in review:

  • max_page_size: 20 on the field. ArtifactRegistry::PaginatesLists reads the cap off the field and falls back to the schema default of 100 without it, so first: 100 was forwarding limit: 100 to Artifact Registry.
  • FieldCallCount, limit: 1 on the resolver. One operation could still select the connection under several aliases on the same repository, each resolution its own AR request. The extension raises before the resolve body runs, so the second selection costs no client call.

Complexity covers neither: it scores the query shape, not the row count or the per-resolution HTTP cost.

Two things fell out of the split rather than being engineered:

  • RepositoriesResolver no longer wraps its nodes in a presenter. That wrap existed only so a child connection on a listed repository could reach the organization, and the list mounts no child connection now.
  • Repositories::Create and Update return the plain type, which no longer carries packages, so selecting it through a mutation payload can no longer raise NoMethodError.

Alternative considered and rejected: Gitlab::Graphql::Limit::FieldCallCount with limit: 1, which is one line and needs no spec change. It yields a runtime error rather than a validation error, leaves packages documented on every list node before rejecting it at runtime, and fixes neither the presenter wrap nor the mutation payload.

The local typedefs stopped redeclaring the schema

The second commit fixes generate-apollo-graphql-schema, which the first commit broke. The frontend's local @client typedefs declared the package types, the union, their connection, and ArtifactRegistryRepository.packages while no server backing existed. Adding those to the real schema made each one a duplicate definition, and Apollo refuses to build such a schema.

The typedefs now reference what the schema owns instead of defining it: the two package types became extend type blocks carrying only versions, which is still local. What stays local is otherwise unchanged.

The shared possible types were regenerated

The fourth commit fixes graphql-verify, which broke at the graphql_possible_types_extraction.js --check step. That step asserts app/assets/javascripts/graphql_shared/possible_types.json matches the union and interface members the schema declares, and adding ArtifactRegistryPackage left the committed file stale. Apollo needs those members to match an inline fragment, so a stale file renders the Maven and npm cells blank rather than erroring.

Notes for review

Three behaviors are worth a reviewer's eye. Each is a monolith convention the plan's design collided with, and each was established empirically:

  • The resolver short-circuits a non-package format instead of calling the client. ArtifactRegistry::Client#packages guards on PACKAGE_FORMATS and raises ArgumentError before issuing a request, so a container repository could never earn the 404 the spec says resolves this connection null. The short-circuit produces that same null, with no client call and no error.
  • The presenter authorizes through declarative_policy_subject, not declarative_policy_delegate. The delegate form picks the policy class from the delegate but instantiates it with the original subject, so OrganizationPolicy runs against the presenter and fails on @subject.user?.
  • Resolvers::BaseResolver#object unwraps a presenter to its subject, which would drop the organization before PackagesResolver could read it. The resolver reads the pre-unwrap object via a private presented_repository and leaves object meaning what it means everywhere else.

Two further notes:

  • The package element types carry no authorize and no skip_type_authorization. They are policy-less value objects, and declaring the ability makes the union redactor raise. RepositoryDetailsType inherits authorize :read_artifact_registry from its parent and carries a Graphql/AuthorizeTypes disable, because that cop reads only the class body.
  • field :id needs resolver_method:, not method:. resolver_method defaults to the field name and is checked on the type instance first, so method: would still route the read through BaseObject#id and encode a global ID these value objects cannot produce. That reasoning now lives once, in ArtifactRegistry::ExposesElementId, which both package types include.

Known follow-ups, deliberately not in this MR

  • images will hit the same Apollo collision. ArtifactRegistryImage and ArtifactRegistryImageConnection are still local typedef definitions, so Step 4 needs the same extend type edit. It should also mount images on the detail type.
  • The packages document still carries @client. The schema answers the connection now, but get_repository_packages.query.graphql keeps the directive and mock_resolvers.js keeps answering it. Removing both makes the page render server data, which needs handler data in repository_detail_spec.js and the feature spec that goes with it, so it is its own change. mock_resolvers_spec.js carries a named exception for it (LOCALLY_ANSWERED_SCHEMA_FIELDS) so the local-versus-schema invariant still fails loudly everywhere else.
  • cache_config.js still exports a local possibleTypes for the union. It is now redundant: both lib/graphql.js and mock_apollo_helper.js read the generated possible_types.json, which carries the union as of this MR. It is left in place to keep this diff to the CI fix; the note in repositories/index.js about dropping it with the mock can go one step earlier now.

Multiversion compatibility

Danger flags this MR because it touches GraphQL backend and frontend code together. The rolling-deployment hazard it describes does not arise here, though the reasoning changed when monolith/S14 landed on master during review and this branch was rebased onto it. What follows is re-derived from the current diff.

The hazard is a frontend that requests a new field from a backend which has not finished rolling out. This MR adds no selection to any document:

  • No query or mutation document changed. git diff --name-only origin/master...HEAD -- '*.graphql' returns graphql/typedefs.graphql alone, and that change only removes local declarations the schema now owns and retargets the rest onto ArtifactRegistryRepositoryDetails.
  • get_repository_detail.query.graphql does send server fields, as of S14 — but every one of them already exists on master. Renaming the type the field returns does not add a selection, and an older backend answering that field answers the same fields it answers today.
  • The remaining frontend changes are cache and local-resolver wiring (cache_config.js, cache_update.js, mock_resolvers.js, constants.js, repositories_edit_form.vue) plus a generated possible_types.json. None of them reaches the server.

What is a genuine ordering constraint is the type rename itself, and it is satisfied by construction: ArtifactRegistryRepositoryDetails ships in this same MR as the field's return type, so no frontend revision ever asks an older backend for it.

On top of that, the whole surface sits behind the disabled artifact_registry_ui flag, which is the "gate the frontend behind a feature flag" option the warning offers.

@gl_introduced marks fields the frontend newly consumes from the server. This MR introduces no such field, so it does not apply.

Diff size

1064 reviewable LOC, past the 500 the development model asks me to justify:

group LOC
production Ruby (9 files) +216 / -3
frontend typedefs (1 file) +11 / -20
specs (10 files) +837 / -5
generated artifacts, schema + possible types (excluded from review) +539 / -2

Danger's line count (1633) includes the three generated files.

On Danger's commit-body warnings: rewrapping those bodies to 72 columns means rewriting four already-pushed commits and force-pushing, which would re-anchor the review threads already sitting on this branch. Squash is enabled on this MR instead, so the individual bodies never reach master.

Production is 216 LOC across nine files. The bulk is the spec suite, which does not split usefully: separating the connection from its request spec would land untested code, and the union, the resolver, and the presenter are one seam a reviewer reads together. The three commits are separable, and the detail-type commit can be dropped alone if its reasoning is rejected.

How to set up and validate locally

  1. Enable the flag: Feature.enable(:artifact_registry_ui) in rails c.

  2. Query an organization's repository and select the connection:

    query {
      organization(id: "gid://gitlab/Organizations::Organization/1") {
        artifactRegistryRepository(name: "my-maven-repo") {
          name
          packages(first: 5) {
            pageInfo { hasNextPage endCursor }
            nodes {
              __typename
              ... on ArtifactRegistryMavenPackage { id groupId artifactId }
              ... on ArtifactRegistryNpmPackage { id name scope versionsCount }
            }
          }
        }
      }
    }
  3. Confirm a docker or oci repository resolves packages null without an error, and that disabling the flag resolves it null with no AR call.

  4. Confirm the guard: selecting packages inside artifactRegistryRepositories is rejected as an invalid query.

Verification performed

  • rspec over the Artifact Registry GraphQL surface (requests, resolvers, resolver concerns, types, presenters, mutations): 182 examples, 0 failures.
  • jest ee/spec/frontend/packages_and_registries/artifact_registry: 40 suites, 980 tests, all passing.
  • rake gitlab:graphql:validate: passes.
  • rubocop clean on every touched file.
  • Schema and reference docs regenerated with bundle exec rake gitlab:graphql:update_all; regenerating again after the ExposesElementId extraction produced a byte-identical result, confirming that commit is a refactor only.
  • The Apollo CLI lives only in a dedicated CI image, so the schema-collision fix was verified mechanically instead: every typedef definition and every extend field was cross-checked against introspection_result.json, with zero collisions.
  • The graphql-verify steps were run locally and pass: gitlab:graphql:validate, gitlab:graphql:check_docs, gitlab:graphql:check_introspection_sync, and graphql_possible_types_extraction.js --check.

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