From c55c42b5d471dfc0d22d9b02ee020e87b0ee376b Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Tue, 19 Nov 2024 09:48:33 +0000
Subject: [PATCH] Add Rubocop rule for the Gitlab::HTTP_V2 usage
In the GitLab monolith codebase, we should use the `Gitlab::HTTP`
wrapper instead of `Gitlab::HTTP_V2` because we need to pass options
that are related to the GitLab instance setting.
---
keeps/delete_old_feature_flags.rb | 2 +-
keeps/helpers/groups.rb | 2 +-
keeps/helpers/reviewer_roulette.rb | 2 +-
keeps/quarantine_flaky_tests.rb | 2 +-
lib/gitlab/http.rb | 6 ----
rubocop/cop/gitlab/http_v2.rb | 42 +++++++++++++++++++++++++
spec/rubocop/cop/gitlab/http_v2_spec.rb | 40 +++++++++++++++++++++++
7 files changed, 86 insertions(+), 10 deletions(-)
create mode 100644 rubocop/cop/gitlab/http_v2.rb
create mode 100644 spec/rubocop/cop/gitlab/http_v2_spec.rb
diff --git a/keeps/delete_old_feature_flags.rb b/keeps/delete_old_feature_flags.rb
index dabb7ccba6d6..53ce1166a5f9 100644
--- a/keeps/delete_old_feature_flags.rb
+++ b/keeps/delete_old_feature_flags.rb
@@ -174,7 +174,7 @@ def get_rollout_issue(rollout_issue_url)
matches = ROLLOUT_ISSUE_URL_REGEX.match(rollout_issue_url)
return unless matches
- response = Gitlab::HTTP_V2.try_get(
+ response = Gitlab::HTTP_V2.try_get( # rubocop:disable Gitlab/HttpV2 -- Not running inside rails application
format(API_ISSUE_URL, project_path: CGI.escape(matches[:project_path]), issue_iid: matches[:issue_iid])
)
diff --git a/keeps/helpers/groups.rb b/keeps/helpers/groups.rb
index 8a4bc2ed23b6..ae94e5006314 100644
--- a/keeps/helpers/groups.rb
+++ b/keeps/helpers/groups.rb
@@ -55,7 +55,7 @@ def groups
def fetch_groups
@groups_json ||= begin
- response = Gitlab::HTTP_V2.get(GROUPS_JSON_URL)
+ response = Gitlab::HTTP_V2.get(GROUPS_JSON_URL) # rubocop:disable Gitlab/HttpV2 -- Not running inside rails application
unless (200..299).cover?(response.code)
raise Error,
diff --git a/keeps/helpers/reviewer_roulette.rb b/keeps/helpers/reviewer_roulette.rb
index de8c96397a4a..959c8d7ba2cf 100644
--- a/keeps/helpers/reviewer_roulette.rb
+++ b/keeps/helpers/reviewer_roulette.rb
@@ -49,7 +49,7 @@ def stats
end
def fetch_stats
- response = Gitlab::HTTP_V2.get(STATS_JSON_URL)
+ response = Gitlab::HTTP_V2.get(STATS_JSON_URL) # rubocop:disable Gitlab/HttpV2 -- Not running inside rails application
unless (200..299).cover?(response.code)
raise Error, "Failed to get roulette stats with response code: #{response.code} and body:\n#{response.body}"
diff --git a/keeps/quarantine_flaky_tests.rb b/keeps/quarantine_flaky_tests.rb
index 31daa999bfca..a12e59219dd5 100644
--- a/keeps/quarantine_flaky_tests.rb
+++ b/keeps/quarantine_flaky_tests.rb
@@ -131,7 +131,7 @@ def query_api(url)
end
def get(url)
- http_response = Gitlab::HTTP_V2.get(
+ http_response = Gitlab::HTTP_V2.get( # rubocop:disable Gitlab/HttpV2 -- Not running inside rails application
url,
headers: {
'User-Agent' => "GitLab-Housekeeper/#{self.class.name}",
diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb
index 8fa5dd1e0568..cd9248b5fc81 100644
--- a/lib/gitlab/http.rb
+++ b/lib/gitlab/http.rb
@@ -1,11 +1,5 @@
# frozen_string_literal: true
-#
-# IMPORTANT: With the new development of the 'gitlab-http' gem (https://gitlab.com/gitlab-org/gitlab/-/issues/415686),
-# no additional change should be implemented in this class. This class will be removed after migrating all
-# the usages to the new gem.
-#
-
module Gitlab
class HTTP
BlockedUrlError = Gitlab::HTTP_V2::BlockedUrlError
diff --git a/rubocop/cop/gitlab/http_v2.rb b/rubocop/cop/gitlab/http_v2.rb
new file mode 100644
index 000000000000..a65a2528109b
--- /dev/null
+++ b/rubocop/cop/gitlab/http_v2.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+begin
+ require 'gitlab-http'
+rescue LoadError
+ # Ignore the cop if the gem is not available
+ return
+end
+
+module RuboCop
+ module Cop
+ module Gitlab
+ class HttpV2 < RuboCop::Cop::Base
+ extend RuboCop::Cop::AutoCorrector
+
+ METHODS_LIST = ::Gitlab::HTTP_V2::SUPPORTED_HTTP_METHODS.join(', ').freeze
+ METHODS_PATTERN = ::Gitlab::HTTP_V2::SUPPORTED_HTTP_METHODS.map(&:inspect).join(' ').freeze
+
+ MSG_SEND = <<~MSG.freeze
+ Avoid calling `Gitlab::HTTP_V2` directly for the #{METHODS_LIST} methods.
+ Instead, use the `Gitlab::HTTP` wrapper.
+ MSG
+
+ def_node_matcher :http_v2_node?, <<~PATTERN
+ (send (const (const nil? :Gitlab) :HTTP_V2) {#{METHODS_PATTERN}} ...)
+ PATTERN
+
+ def on_send(node)
+ return unless http_v2_node?(node)
+
+ add_offense(node, message: MSG_SEND) do |corrector|
+ _, method_name, *arg_nodes = *node
+
+ replacement = "Gitlab::HTTP.#{method_name}(#{arg_nodes.map(&:source).join(', ')})"
+
+ corrector.replace(node.source_range, replacement)
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/rubocop/cop/gitlab/http_v2_spec.rb b/spec/rubocop/cop/gitlab/http_v2_spec.rb
new file mode 100644
index 000000000000..c0af7f46ffee
--- /dev/null
+++ b/spec/rubocop/cop/gitlab/http_v2_spec.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'rubocop_spec_helper'
+require_relative '../../../../rubocop/cop/gitlab/http_v2'
+
+RSpec.describe RuboCop::Cop::Gitlab::HttpV2, feature_category: :shared do
+ it 'flags the use of `Gitlab::HTTP_V2`' do
+ expect_offense(<<~RUBY)
+ Gitlab::HTTP_V2.get('https://gitlab.com')
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `Gitlab::HTTP_V2` directly [...]
+ RUBY
+ end
+
+ it 'flags the use of `Gitlab::HTTP_V2` with multiple arguments' do
+ expect_offense(<<~RUBY)
+ Gitlab::HTTP_V2.get('https://gitlab.com', headers: { 'Content-Type' => 'application/json' })
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `Gitlab::HTTP_V2` directly [...]
+ RUBY
+ end
+
+ it 'flags the use of `Gitlab::HTTP_V2` with multiple arguments and a block' do
+ expect_offense(<<~RUBY)
+ Gitlab::HTTP_V2.get('https://gitlab.com', headers: { 'Content-Type' => 'application/json' }) do |req|
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `Gitlab::HTTP_V2` directly [...]
+ end
+ RUBY
+ end
+
+ it 'does not flag the use of `Gitlab::HTTP_V2::UrlBlocker.validate!' do
+ expect_no_offenses(<<~RUBY)
+ Gitlab::HTTP_V2::UrlBlocker.validate!('https://gitlab.com')
+ RUBY
+ end
+
+ it 'does not flag the use of `Gitlab::HTTP.get`' do
+ expect_no_offenses(<<~RUBY)
+ Gitlab::HTTP.get('https://gitlab.com')
+ RUBY
+ end
+end
--
GitLab