Fix HTTP_V2 config leak breaking unrelated specs

What does this MR do and why?

Fixes the root cause of a flaky failure in spec/models/remote_mirror_spec.rb (RemoteMirror validations self-referencing url grandfathers an existing self-mirror on update), reported as a master-broken incident.

Root cause

Two specs mutate the process-global Gitlab::HTTP_V2.configuration and never restore it:

  • Keeps::Helpers::GitlabApiHelper#initialize (production code, exercised by spec/keeps/helpers/gitlab_api_helper_spec.rb via let(:helper) { described_class.new }) sets config.allowed_internal_uris = [].
  • The protocol: 'https' example in spec/initializers/gitlab_http_spec.rb reloads the initializer, rebuilding allowed_internal_uris with the https scheme.

Gitlab::HTTP_V2.configuration is a memoized singleton, so either change leaks for the rest of the RSpec process. Once allowed_internal_uris no longer contains the instance's own http://localhost:80, UrlBlocker#internal? stops treating the instance host as internal, and the always-on public_url validation on RemoteMirror#url blocks a self-referencing http://localhost URL with Url is blocked: Requests to localhost are not allowed — so mirror.save returns false and the grandfather example fails.

This only reproduced when a leaking spec ran before remote_mirror_spec.rb in the same process, which is why it appeared intermittently and specifically in the no_gitaly_transactions scheduled pipeline (different Knapsack grouping than MR pipelines).

Reproduction

bundle exec rspec spec/keeps/helpers/gitlab_api_helper_spec.rb spec/models/remote_mirror_spec.rb
# => grandfather example fails: "Url is blocked: Requests to localhost are not allowed"

Confirmed via a temporary diagnostic MR (!246962 (closed)), whose no_gitaly_transactions job printed allowed_internal_uris=[], internal?(url)=false, saved=false errors=["Url is blocked: Requests to localhost are not allowed"].

Fix

Restore the global Gitlab::HTTP_V2 configuration after each affected example (the keeps spec reloads the initializer; the initializer spec saves/restores allowed_internal_uris). This stops the leak from poisoning any other spec, so the original remote_mirror_spec.rb test — reverted to its previous form in this MR — passes reliably. Test-only change; no changelog entry.

References

Edited by Stan Hu

Merge request reports

Loading