Extract checks for job before assigning it to runner

so that we can easily extend this in EE.

app/models/commit_status_enums.rb

@@ -23,7 +23,8 @@ module CommitStatusEnums
       downstream_bridge_project_not_found: 1_002,
       invalid_bridge_trigger: 1_003,
       bridge_pipeline_is_child_pipeline: 1_006,
-      downstream_pipeline_creation_failed: 1_007
+      downstream_pipeline_creation_failed: 1_007,
+      secrets_provider_not_found: 1_008
     }
   end
 end

app/presenters/commit_status_presenter.rb

@@ -19,7 +19,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
     downstream_bridge_project_not_found: 'This job could not be executed because downstream bridge project could not be found',
     insufficient_bridge_permissions: 'This job could not be executed because of insufficient permissions to create a downstream pipeline',
     bridge_pipeline_is_child_pipeline: 'This job belongs to a child pipeline and cannot create further child pipelines',
-    downstream_pipeline_creation_failed: 'The downstream pipeline could not be created'
+    downstream_pipeline_creation_failed: 'The downstream pipeline could not be created',
+    secrets_provider_not_found: 'The secrets provider can not be found'
   }.freeze
 
   private_constant :CALLOUT_FAILURE_MESSAGES

app/services/ci/register_job_service.rb

@@ -107,23 +107,15 @@ module Ci
       build.runner_id = runner.id
       build.runner_session_attributes = params[:session] if params[:session].present?
 
-      unless build.has_valid_build_dependencies?
-        build.drop!(:missing_dependency_failure)
-        return false
-      end
-
-      unless build.supported_runner?(params.dig(:info, :features))
-        build.drop!(:runner_unsupported)
-        return false
-      end
+      failure_reason, _ = pre_assign_runner_checks.find { |_, check| check.call(build, params) }
 
-      if build.archived?
-        build.drop!(:archived_failure)
-        return false
+      if failure_reason
+        build.drop!(failure_reason)
+      else
+        build.run!
       end
 
-      build.run!
-      true
+      !failure_reason
     end
 
     def scheduler_failure!(build)
@@ -238,6 +230,14 @@ module Ci
     def job_queue_duration_seconds
       @job_queue_duration_seconds ||= Gitlab::Metrics.histogram(:job_queue_duration_seconds, 'Request handling execution time', {}, JOB_QUEUE_DURATION_SECONDS_BUCKETS)
     end
+
+    def pre_assign_runner_checks
+      {
+        missing_dependency_failure: -> (build, _) { !build.has_valid_build_dependencies? },
+        runner_unsupported: -> (build, params) { !build.supported_runner?(params.dig(:info, :features)) },
+        archived_failure: -> (build, _) { build.archived? }
+      }
+    end
   end
 end
 

ee/app/models/ee/ci/build.rb

@@ -148,8 +148,22 @@ module EE
         super + ee_runner_required_feature_names
       end
 
+      def secrets_provider?
+        variable_value('VAULT_SERVER_URL').present?
+      end
+
+      def variable_value(key)
+        variables_hash[key]
+      end
+
       private
 
+      def variables_hash
+        @variables_hash ||= variables.map do |variable|
+          [variable[:key], variable[:value]]
+        end.to_h
+      end
+
       def parse_security_artifact_blob(security_report, blob)
         report_clone = security_report.clone_as_blank
         ::Gitlab::Ci::Parsers.fabricate!(security_report.type).parse!(blob, report_clone)

ee/app/presenters/ee/ci/build_runner_presenter.rb

@@ -27,16 +27,6 @@ module EE
           }
         }
       end
-
-      def variable_value(key)
-        variables_hash[key]
-      end
-
-      def variables_hash
-        @variables_hash ||= variables.map do |variable|
-          [variable[:key], variable[:value]]
-        end.to_h
-      end
     end
   end
 end

ee/app/services/ee/ci/register_job_service.rb

@@ -8,6 +8,7 @@ module EE
     # and be included in the `RegisterJobService` service
     module RegisterJobService
       extend ActiveSupport::Concern
+      extend ::Gitlab::Utils::Override
 
       def execute(params = {})
         db_all_caught_up = ::Gitlab::Database::LoadBalancing::Sticking.all_caught_up?(:runner, runner.id)
@@ -71,6 +72,13 @@ module EE
       def shared_runner_build_limits_feature_enabled?
         ENV['DISABLE_SHARED_RUNNER_BUILD_MINUTES_LIMIT'].to_s != 'true'
       end
+
+      override :pre_assign_runner_checks
+      def pre_assign_runner_checks
+        super.merge({
+          secrets_provider_not_found: -> (build, _) { build.ci_secrets_management_available? && build.secrets? && !build.secrets_provider? }
+        })
+      end
     end
   end
 end

ee/spec/services/ci/register_job_service_spec.rb

@@ -188,5 +188,68 @@ RSpec.describe Ci::RegisterJobService do
         end
       end
     end
+
+    context 'secrets' do
+      let(:params) { { info: { features: { vault_secrets: true } } } }
+
+      subject(:service) { described_class.new(shared_runner) }
+
+      before do
+        stub_licensed_features(ci_secrets_management: true)
+      end
+
+      context 'when build has secrets defined' do
+        before do
+          pending_build.update!(
+            secrets: {
+              DATABASE_PASSWORD: {
+                vault: {
+                  engine: { name: 'kv-v2', path: 'kv-v2' },
+                  path: 'production/db',
+                  field: 'password'
+                }
+              }
+            }
+          )
+        end
+
+        context 'when there is no Vault server provided' do
+          it 'does not pick the build and drops the build' do
+            result = service.execute(params).build
+
+            aggregate_failures do
+              expect(result).to be_nil
+              expect(pending_build.reload).to be_failed
+              expect(pending_build.failure_reason).to eq('secrets_provider_not_found')
+              expect(pending_build).to be_secrets_provider_not_found
+            end
+          end
+        end
+
+        context 'when there is Vault server provided' do
+          it 'picks the build' do
+            create(:ci_variable, project: project, key: 'VAULT_SERVER_URL', value: 'https://vault.example.com')
+
+            build = service.execute(params).build
+
+            aggregate_failures do
+              expect(build).not_to be_nil
+              expect(build).to be_running
+            end
+          end
+        end
+      end
+
+      context 'when build has no secrets defined' do
+        it 'picks the build' do
+          build = service.execute(params).build
+
+          aggregate_failures do
+            expect(build).not_to be_nil
+            expect(build).to be_running
+          end
+        end
+      end
+    end
   end
 end

lib/gitlab/ci/status/build/failed.rb

@@ -24,7 +24,8 @@ module Gitlab
             downstream_bridge_project_not_found: 'downstream project could not be found',
             insufficient_bridge_permissions: 'no permissions to trigger downstream pipeline',
             bridge_pipeline_is_child_pipeline: 'creation of child pipeline not allowed from another child pipeline',
-            downstream_pipeline_creation_failed: 'downstream pipeline can not be created'
+            downstream_pipeline_creation_failed: 'downstream pipeline can not be created',
+            secrets_provider_not_found: 'secrets provider can not be found'
           }.freeze
 
           private_constant :REASONS