Gate every Artifact Registry surface on a configured base URL

Rahul Chanilarequested to merge
617489-unconfigured-ar-base-url-step-1 into master

What does this MR do and why?

An instance whose artifact_registry.api_url is unset renders the Artifact Registry pages, hides the URL-copy affordances without saying why, and answers every GraphQL query with the literal message base_url is required. The setting has no production default (config/initializers/1_settings.rb defaults it only in development and test), so this is the state a self-managed instance is in until an operator acts.

This introduces ArtifactRegistry::Configuration with one predicate, configured?, and applies it to every surface that has to agree on it:

  • ArtifactRegistryGating#ensure_artifact_registry_available!, covering all three Artifact Registry controllers
  • Sidebars::Organizations::Menus::ArtifactRegistryMenu#render?
  • EE::Sidebars::Organizations::Menus::SettingsMenu#show_artifact_registry_menu_item?
  • both mount-data helpers, through Configuration.client_base_url
  • the organization overview page, through a new artifact_registry_configured mount-data key

One predicate decides the route gate and the browser-facing value alike, so they cannot disagree and a rendered page always carries a non-nil client_base_url. HTTP_ORIGIN_PATTERN is deleted, since the degenerate origins it rejected can no longer be produced.

One rule set for the gate and the client

configured? admits an http(s) scheme with a host and no user information, query, fragment, or path deeper than /. Those four rules are not restated here: Configuration.base_url_violation(uri) returns the reason a parsed URL cannot serve as a base URL, or nil when it can. configured? checks for nil; ArtifactRegistry::Client#validate_base_url! raises the returned string as its ConfigurationError message, keeping its own parse, rescue and error reporting. So the gate and construction cannot drift.

That covers construction only. guard_service_transport! refuses plaintext HTTP in production per request, and the gate does not mirror it, so a production instance set to http://ar.internal:8080 passes configured?, renders, and then refuses every service-authenticated call. Nothing reaches that path until a service credential has a caller, so it is latent rather than live.

A distinct exception class, not a message swap

Base-URL faults now raise ArtifactRegistry::Client::ConfigurationError rather than ArgumentError. ArtifactRegistry::RendersErrors rescues ::ArgumentError broadly and forwards e.message into the response, where the setting's name does not belong, so the operator fault needs a class outside that hierarchy. The client's caller-input ArgumentError sites are unchanged and still surface their own messages, which the spec asserts so a later widening of the new class fails a test.

ConfigurationError maps to service-unavailable and logs nothing. An unconfigured instance is silent on every path, and the absent setting is the only signal: only an operator can change the condition, so a line per refused navigation or per query adds nothing, and a polling GraphQL client would otherwise write one unaggregated line per request. Gitlab::ErrorTracking.log_exception writes through trackers: [Logger], with no Sentry-side grouping to collapse the repeats.

The no-credential AuthorizationError path still logs; that condition is not operator-fixable by configuration. With the log gone, ConfigurationError and UnavailableError render identically and share one rescue clause.

Deliberate non-changes

  • No gate on the resolver or the mutation. A resolver gate would return a null field, which is already documented as the signal for artifact_registry_ui being off. A second cause would make that description wrong. The client constructor stays the single enforcement point for callers that bypass the routes.
  • artifact_registry_configured is its own mount-data key, not folded into can_read_artifact_registry. The two answers have different remedies, so keeping them separate lets an operator tell a permission problem from a configuration one from the page's data attribute alone.
  • Organizations::Settings::ArtifactRegistryHelper is unchanged. It composes Organizations::ArtifactRegistryHelper and inherits the delegation. Its spec is updated, since the behaviour it asserts changes.

Feature flag

artifact_registry_ui, unchanged. The new condition applies whether the flag is on or off.

References

Screenshots or screen recordings

No new UI. The change only removes affordances, and only on an instance whose base URL is unset or malformed:

State Organization overview Navigation Artifact Registry routes
Configured (well-formed origin) unchanged from today unchanged unchanged
Unset or malformed no "Go to Artifact Registry" link, no Artifact Registry wording in the empty state both entries absent 404

A correctly configured instance is pixel-identical before and after, which is why no before/after pair is attached.

How to set up and validate locally

  1. Enable the flag and confirm the feature works as it does today:

    Feature.enable(:artifact_registry_ui)

    With artifact_registry.api_url set to a well-formed origin, the sidebar entries, the organization overview link, and the Artifact Registry pages all behave as before.

  2. Comment the key out of config/gitlab.yml, restart, and reload the organization overview. The "Go to Artifact Registry" link and the Artifact Registry wording are gone, both navigation entries are absent, and /-/organizations/<path>/-/artifact_registry returns 404.

  3. Set it to https://ar.example.com/api/v1, http://, ftp://host, or https://user:pass@ar.example.com. Each behaves as the unset case.

  4. With the setting unset, query the API directly and confirm the response says the service is unavailable and never names the setting:

    query { organization(fullPath: "<path>") { artifactRegistryRepositories { nodes { name } } } }

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.

Tests

430 examples, 0 failures across the eleven RSpec files, and 10/10 in app_spec.js. Both renders_errors operations are asserted, because the concern's other arms diverge by operation and this one does not. All three Artifact Registry routes assert the 404, rather than one standing in for the others, through a shared example the five gated surfaces consume.

Configuration's accept and reject tables assert configured?, client_base_url and construction of a real client per row, so the gate and the client are checked against one another over 24 values rather than in separate places.

Related to #617489 (closed)

Edited by Rahul Chanila

Merge request reports