Merge branch 'allowlist_some_ci_cross_joins' into 'master'

Add allow list on callsite offenders

See merge request gitlab-org/gitlab!69055

.cross-join-allowlist.yml

@@ -97,7 +97,6 @@
 - "./ee/spec/services/ci/minutes/refresh_cached_data_service_spec.rb"
 - "./ee/spec/services/ci/pipeline_creation/drop_not_runnable_builds_service_spec.rb"
 - "./ee/spec/services/ci/process_pipeline_service_spec.rb"
-- "./ee/spec/services/ci/register_job_service_spec.rb"
 - "./ee/spec/services/ci/retry_build_service_spec.rb"
 - "./ee/spec/services/ci/retry_pipeline_service_spec.rb"
 - "./ee/spec/services/ci/trigger_downstream_subscription_service_spec.rb"

app/finders/ci/pipelines_for_merge_request_finder.rb

@@ -29,6 +29,7 @@ module Ci
 
     # Fetch all pipelines without permission check.
     def all
+      ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336891') do
         strong_memoize(:all_pipelines) do
           next Ci::Pipeline.none unless source_project
 
@@ -42,6 +43,7 @@ module Ci
           sort(pipelines)
         end
       end
+    end
 
     private
 

app/models/ci/runner.rb

@@ -420,16 +420,20 @@ module Ci
     end
 
     def no_projects
+      ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
         if projects.any?
           errors.add(:runner, 'cannot have projects assigned')
         end
       end
+    end
 
     def no_groups
+      ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338659') do
         if groups.any?
           errors.add(:runner, 'cannot have groups assigned')
         end
       end
+    end
 
     def any_project
       unless projects.any?

app/services/ci/register_job_service.rb

@@ -103,6 +103,7 @@ module Ci
 
     # rubocop: disable CodeReuse/ActiveRecord
     def each_build(params, &blk)
+      ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/339429') do
         queue = ::Ci::Queue::BuildQueueService.new(runner)
 
         builds = begin
@@ -138,6 +139,7 @@ module Ci
 
         build_ids.each { |build_id| yield Ci::Build.find(build_id) }
       end
+    end
     # rubocop: enable CodeReuse/ActiveRecord
 
     def retrieve_queue(queue_query_proc)

lib/gitlab/database.rb

@@ -145,6 +145,7 @@ module Gitlab
     def self.allow_cross_joins_across_databases(url:)
       # this method is implemented in:
       # spec/support/database/prevent_cross_joins.rb
+      yield
     end
 
     # This method will allow cross database modifications within the block

spec/support/database/prevent_cross_joins.rb

@@ -11,7 +11,7 @@
 #
 # class User
 #   def ci_owned_runners
-#     ::Gitlab::Database.allow_cross_joins_across_databases!(url: link-to-issue-url)
+#     ::Gitlab::Database.allow_cross_joins_across_databases(url: link-to-issue-url)
 #
 #     ...
 #   end
@@ -21,8 +21,10 @@ module Database
   module PreventCrossJoins
     CrossJoinAcrossUnsupportedTablesError = Class.new(StandardError)
 
+    ALLOW_THREAD_KEY = :allow_cross_joins_across_databases
+
     def self.validate_cross_joins!(sql)
-      return if Thread.current[:allow_cross_joins_across_databases]
+      return if Thread.current[ALLOW_THREAD_KEY]
 
       # Allow spec/support/database_cleaner.rb queries to disable/enable triggers for many tables
       # See https://gitlab.com/gitlab-org/gitlab/-/issues/339396
@@ -55,7 +57,7 @@ module Database
           ::Database::PreventCrossJoins.validate_cross_joins!(event.payload[:sql])
         end
 
-        Thread.current[:allow_cross_joins_across_databases] = false
+        Thread.current[ALLOW_THREAD_KEY] = false
 
         yield
       ensure
@@ -65,8 +67,12 @@ module Database
 
     module GitlabDatabaseMixin
       def allow_cross_joins_across_databases(url:)
-        Thread.current[:allow_cross_joins_across_databases] = true
-        super
+        old_value = Thread.current[ALLOW_THREAD_KEY]
+        Thread.current[ALLOW_THREAD_KEY] = true
+
+        yield
+      ensure
+        Thread.current[ALLOW_THREAD_KEY] = old_value
       end
     end
   end

spec/support_specs/database/prevent_cross_joins_spec.rb

@@ -24,9 +24,13 @@ RSpec.describe Database::PreventCrossJoins do
 
       context 'when allow_cross_joins_across_databases is used' do
         it 'does not raise exception' do
-          Gitlab::Database.allow_cross_joins_across_databases(url: 'http://issue-url')
+          expect { main_and_ci_query_allowlisted }.not_to raise_error
+        end
+      end
 
-          expect { main_and_ci_query }.not_to raise_error
+      context 'when allow_cross_joins_across_databases is used' do
+        it 'does not raise exception' do
+          expect { main_and_ci_query_allowlist_nested }.not_to raise_error
         end
       end
     end
@@ -34,6 +38,20 @@ RSpec.describe Database::PreventCrossJoins do
 
   private
 
+  def main_and_ci_query_allowlisted
+    Gitlab::Database.allow_cross_joins_across_databases(url: 'http://issue-url') do
+      main_and_ci_query
+    end
+  end
+
+  def main_and_ci_query_allowlist_nested
+    Gitlab::Database.allow_cross_joins_across_databases(url: 'http://issue-url') do
+      main_and_ci_query_allowlisted
+
+      main_and_ci_query
+    end
+  end
+
   def main_only_query
     Issue.joins(:project).last
   end