Merge branch 'ci-external-valiation-service-updates' into 'master'

Restrict external pipeline validation to a single not acceptable code [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56856

config/feature_flags/development/ci_external_validation_service.yml

+---
+name: ci_external_validation_service
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56856
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323935
+milestone: '13.11'
+type: development
+group: group::continuous integration
+default_enabled: false

doc/administration/external_pipeline_validation.md

@@ -22,12 +22,12 @@ invalidated.
 Response Code Legend:
 
 - `200` - Accepted
-- `4xx` - Not Accepted
+- `406` - Not Accepted
 - Other Codes - Accepted and Logged
 
 ## Configuration
 
-Set the `EXTERNAL_VALIDATION_SERVICE_URL` to the external service URL.
+Set the `EXTERNAL_VALIDATION_SERVICE_URL` to the external service URL and enable `ci_external_validation_service` feature flag.
 
 ## Payload Schema
 

lib/gitlab/ci/pipeline/chain/validate/external.rb

@@ -11,8 +11,13 @@ module Gitlab
             InvalidResponseCode = Class.new(StandardError)
 
             VALIDATION_REQUEST_TIMEOUT = 5
+            ACCEPTED_STATUS = 200
+            DOT_COM_REJECTED_STATUS = 406
+            GENERAL_REJECTED_STATUS = (400..499).freeze
 
             def perform!
+              return unless enabled?
+
               pipeline_authorized = validate_external
 
               log_message = pipeline_authorized ? 'authorized' : 'not authorized'
@@ -27,27 +32,42 @@ module Gitlab
 
             private
 
+            def enabled?
+              return true unless Gitlab.com?
+
+              ::Feature.enabled?(:ci_external_validation_service, project, default_enabled: :yaml)
+            end
+
             def validate_external
               return true unless validation_service_url
 
               # 200 - accepted
-              # 4xx - not accepted
+              # 406 - not accepted on GitLab.com
+              # 4XX - not accepted for other installations
               # everything else - accepted and logged
               response_code = validate_service_request.code
               case response_code
-              when 200
+              when ACCEPTED_STATUS
                 true
-              when 400..499
+              when rejected_status
                 false
               else
                 raise InvalidResponseCode, "Unsupported response code received from Validation Service: #{response_code}"
               end
             rescue => ex
-              Gitlab::ErrorTracking.track_exception(ex)
+              Gitlab::ErrorTracking.track_exception(ex, project_id: project.id)
 
               true
             end
 
+            def rejected_status
+              if Gitlab.com?
+                DOT_COM_REJECTED_STATUS
+              else
+                GENERAL_REJECTED_STATUS
+              end
+            end
+
             def validate_service_request
               Gitlab::HTTP.post(
                 validation_service_url, timeout: VALIDATION_REQUEST_TIMEOUT,

spec/lib/gitlab/ci/pipeline/chain/validate/external_spec.rb

@@ -42,6 +42,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
   end
 
   let(:save_incompleted) { true }
+  let(:dot_com) { true }
   let(:command) do
     Gitlab::Ci::Pipeline::Chain::Command.new(
       project: project, current_user: user, yaml_processor_result: yaml_processor_result, save_incompleted: save_incompleted
@@ -51,9 +52,77 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
   describe '#perform!' do
     subject(:perform!) { step.perform! }
 
-    context 'when validation returns true' do
+    let(:validation_service_url) { 'https://validation-service.external/' }
+
+    before do
+      stub_env('EXTERNAL_VALIDATION_SERVICE_URL', validation_service_url)
+      allow(Gitlab).to receive(:com?).and_return(dot_com)
+    end
+
+    shared_examples 'successful external authorization' do
+      it 'does not drop the pipeline' do
+        perform!
+
+        expect(pipeline.status).not_to eq('failed')
+        expect(pipeline.errors).to be_empty
+      end
+
+      it 'does not break the chain' do
+        perform!
+
+        expect(step.break?).to be false
+      end
+
+      it 'logs the authorization' do
+        expect(Gitlab::AppLogger).to receive(:info).with(message: 'Pipeline authorized', project_id: project.id, user_id: user.id)
+
+        perform!
+      end
+    end
+
+    context 'when validation returns 200 OK' do
+      before do
+        stub_request(:post, validation_service_url).to_return(status: 200, body: "{}")
+      end
+
+      it_behaves_like 'successful external authorization'
+    end
+
+    context 'when validation returns 404 Not Found' do
+      before do
+        stub_request(:post, validation_service_url).to_return(status: 404, body: "{}")
+      end
+
+      it_behaves_like 'successful external authorization'
+    end
+
+    context 'when validation returns 500 Internal Server Error' do
+      before do
+        stub_request(:post, validation_service_url).to_return(status: 500, body: "{}")
+      end
+
+      it_behaves_like 'successful external authorization'
+    end
+
+    context 'when validation raises exceptions' do
+      before do
+        stub_request(:post, validation_service_url).to_raise(Net::OpenTimeout)
+      end
+
+      it_behaves_like 'successful external authorization'
+
+      it 'logs exceptions' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception)
+          .with(instance_of(Net::OpenTimeout), { project_id: project.id })
+
+        perform!
+      end
+    end
+
+    context 'when the feature flag is disabled' do
       before do
-        allow(step).to receive(:validate_external).and_return(true)
+        stub_feature_flags(ci_external_validation_service: false)
+        stub_request(:post, validation_service_url)
       end
 
       it 'does not drop the pipeline' do
@@ -69,16 +138,45 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
         expect(step.break?).to be false
       end
 
+      it 'does not make requests' do
+        perform!
+
+        expect(WebMock).not_to have_requested(:post, validation_service_url)
+      end
+    end
+
+    context 'when not on .com' do
+      let(:dot_com) { false }
+
+      before do
+        stub_feature_flags(ci_external_validation_service: false)
+        stub_request(:post, validation_service_url).to_return(status: 404, body: "{}")
+      end
+
+      it 'drops the pipeline' do
+        perform!
+
+        expect(pipeline.status).to eq('failed')
+        expect(pipeline).to be_persisted
+        expect(pipeline.errors.to_a).to include('External validation failed')
+      end
+
+      it 'breaks the chain' do
+        perform!
+
+        expect(step.break?).to be true
+      end
+
       it 'logs the authorization' do
-        expect(Gitlab::AppLogger).to receive(:info).with(message: 'Pipeline authorized', project_id: project.id, user_id: user.id)
+        expect(Gitlab::AppLogger).to receive(:info).with(message: 'Pipeline not authorized', project_id: project.id, user_id: user.id)
 
         perform!
       end
     end
 
-    context 'when validation return false' do
+    context 'when validation returns 406 Not Acceptable' do
       before do
-        allow(step).to receive(:validate_external).and_return(false)
+        stub_request(:post, validation_service_url).to_return(status: 406, body: "{}")
       end
 
       it 'drops the pipeline' do