Merge branch 'feature/gb/register-job-service-without-db-cross-joins' into 'master'

Disallow database cross-joins in RegisterJobService

See merge request gitlab-org/gitlab!71545

app/services/ci/register_job_service.rb

@@ -103,42 +103,40 @@ 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
-          if runner.instance_type?
-            queue.builds_for_shared_runner
-          elsif runner.group_type?
-            queue.builds_for_group_runner
-          else
-            queue.builds_for_project_runner
-          end
-        end
+      queue = ::Ci::Queue::BuildQueueService.new(runner)
 
-        if runner.ref_protected?
-          builds = queue.builds_for_protected_runner(builds)
+      builds = begin
+        if runner.instance_type?
+          queue.builds_for_shared_runner
+        elsif runner.group_type?
+          queue.builds_for_group_runner
+        else
+          queue.builds_for_project_runner
         end
+      end
 
-        # pick builds that does not have other tags than runner's one
-        builds = queue.builds_matching_tag_ids(builds, runner.tags.ids)
+      if runner.ref_protected?
+        builds = queue.builds_for_protected_runner(builds)
+      end
 
-        # pick builds that have at least one tag
-        unless runner.run_untagged?
-          builds = queue.builds_with_any_tags(builds)
-        end
+      # pick builds that does not have other tags than runner's one
+      builds = queue.builds_matching_tag_ids(builds, runner.tags.ids)
 
-        # pick builds that older than specified age
-        if params.key?(:job_age)
-          builds = queue.builds_queued_before(builds, params[:job_age].seconds.ago)
-        end
+      # pick builds that have at least one tag
+      unless runner.run_untagged?
+        builds = queue.builds_with_any_tags(builds)
+      end
 
-        build_ids = retrieve_queue(-> { queue.execute(builds) })
+      # pick builds that older than specified age
+      if params.key?(:job_age)
+        builds = queue.builds_queued_before(builds, params[:job_age].seconds.ago)
+      end
 
-        @metrics.observe_queue_size(-> { build_ids.size }, @runner.runner_type)
+      build_ids = retrieve_queue(-> { queue.execute(builds) })
 
-        build_ids.each { |build_id| yield Ci::Build.find(build_id) }
-      end
+      @metrics.observe_queue_size(-> { build_ids.size }, @runner.runner_type)
+
+      build_ids.each { |build_id| yield Ci::Build.find(build_id) }
     end
     # rubocop: enable CodeReuse/ActiveRecord
 

ee/spec/services/ci/register_job_service_spec.rb

@@ -55,6 +55,12 @@ RSpec.describe Ci::RegisterJobService, '#execute' do
             stub_feature_flags(ci_queueing_denormalize_ci_minutes_information: false)
           end
 
+          around do |example|
+            allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+              example.run
+            end
+          end
+
           it { is_expected.to be_kind_of(Ci::Build) }
         end
       end
@@ -103,6 +109,12 @@ RSpec.describe Ci::RegisterJobService, '#execute' do
             stub_feature_flags(ci_queueing_denormalize_ci_minutes_information: false)
           end
 
+          around do |example|
+            allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+              example.run
+            end
+          end
+
           it { is_expected.to be_nil }
         end
       end
@@ -307,6 +319,12 @@ RSpec.describe Ci::RegisterJobService, '#execute' do
       stub_feature_flags(ci_pending_builds_queue_source: false)
     end
 
+    around do |example|
+      allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+        example.run
+      end
+    end
+
     include_examples 'namespace minutes quota'
   end
 
@@ -328,6 +346,12 @@ RSpec.describe Ci::RegisterJobService, '#execute' do
         stub_feature_flags(ci_queueing_denormalize_shared_runners_information: false)
       end
 
+      around do |example|
+        allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+          example.run
+        end
+      end
+
       include_examples 'namespace minutes quota'
     end
   end

lib/gitlab/application_context.rb

@@ -124,7 +124,10 @@ module Gitlab
       strong_memoize(:runner_project) do
         next unless runner&.project_type?
 
-        projects = runner.projects.take(2) # rubocop: disable CodeReuse/ActiveRecord
+        projects = ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/342147') do
+          runner.projects.take(2) # rubocop: disable CodeReuse/ActiveRecord
+        end
+
         projects.first if projects.one?
       end
     end

spec/services/ci/register_job_service_spec.rb

@@ -87,19 +87,25 @@ module Ci
           end
 
           context 'for specific runner' do
-            context 'with FF disabled' do
+            context 'with tables decoupling disabled' do
               before do
                 stub_feature_flags(
                   ci_pending_builds_project_runners_decoupling: false,
                   ci_queueing_builds_enabled_checks: false)
               end
 
+              around do |example|
+                allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+                  example.run
+                end
+              end
+
               it 'does not pick a build' do
                 expect(execute(specific_runner)).to be_nil
               end
             end
 
-            context 'with FF enabled' do
+            context 'with tables decoupling enabled' do
               before do
                 stub_feature_flags(
                   ci_pending_builds_project_runners_decoupling: true,
@@ -266,17 +272,23 @@ module Ci
           context 'and uses project runner' do
             let(:build) { execute(specific_runner) }
 
-            context 'with FF disabled' do
+            context 'with tables decoupling disabled' do
               before do
                 stub_feature_flags(
                   ci_pending_builds_project_runners_decoupling: false,
                   ci_queueing_builds_enabled_checks: false)
               end
 
+              around do |example|
+                allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+                  example.run
+                end
+              end
+
               it { expect(build).to be_nil }
             end
 
-            context 'with FF enabled' do
+            context 'with tables decoupling enabled' do
               before do
                 stub_feature_flags(
                   ci_pending_builds_project_runners_decoupling: true,
@@ -791,6 +803,12 @@ module Ci
             stub_feature_flags(ci_queueing_denormalize_shared_runners_information: false)
           end
 
+          around do |example|
+            allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+              example.run
+            end
+          end
+
           include_examples 'handles runner assignment'
         end
 
@@ -807,6 +825,12 @@ module Ci
             stub_feature_flags(ci_queueing_denormalize_tags_information: false)
           end
 
+          around do |example|
+            allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+              example.run
+            end
+          end
+
           include_examples 'handles runner assignment'
         end
 
@@ -815,6 +839,12 @@ module Ci
             stub_feature_flags(ci_queueing_denormalize_namespace_traversal_ids: false)
           end
 
+          around do |example|
+            allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+              example.run
+            end
+          end
+
           include_examples 'handles runner assignment'
         end
       end
@@ -824,6 +854,12 @@ module Ci
           stub_feature_flags(ci_pending_builds_queue_source: false)
         end
 
+        around do |example|
+          allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332952') do
+            example.run
+          end
+        end
+
         include_examples 'handles runner assignment'
       end
     end

spec/support/database/cross-join-allowlist.yml

@@ -81,7 +81,6 @@
 - "./spec/presenters/ci/build_runner_presenter_spec.rb"
 - "./spec/presenters/ci/pipeline_presenter_spec.rb"
 - "./spec/presenters/packages/detail/package_presenter_spec.rb"
-- "./spec/requests/api/ci/runner/jobs_request_post_spec.rb"
 - "./spec/requests/api/ci/runner/runners_post_spec.rb"
 - "./spec/requests/api/ci/runners_spec.rb"
 - "./spec/requests/api/commit_statuses_spec.rb"
@@ -105,7 +104,6 @@
 - "./spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb"
 - "./spec/services/ci/job_artifacts/destroy_associations_service_spec.rb"
 - "./spec/services/ci/job_artifacts/destroy_batch_service_spec.rb"
-- "./spec/services/ci/register_job_service_spec.rb"
 - "./spec/services/deployments/older_deployments_drop_service_spec.rb"
 - "./spec/services/environments/stop_service_spec.rb"
 - "./spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb"

spec/support/database/prevent_cross_joins.rb

@@ -64,6 +64,10 @@ module Database
       ensure
         ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
       end
+
+      def allow_cross_joins_across_databases(url:, &block)
+        ::Gitlab::Database.allow_cross_joins_across_databases(url: url, &block)
+      end
     end
 
     module GitlabDatabaseMixin