Merge branch 'sh-smplify-external-validation-error-codes' into 'master'

Simplify error code handling for external pipeline validation

See merge request gitlab-org/gitlab!61190

changelogs/unreleased/sh-smplify-external-validation-error-codes.yml

+---
+title: Simplify error code handling for external pipeline validation
+merge_request: 61190
+author:
+type: changed

doc/administration/external_pipeline_validation.md

@@ -17,7 +17,7 @@ data as payload. The response code from the external service determines if GitLa
 should accept or reject the pipeline. If the response is:
 
 - `200`, the pipeline is accepted.
-- `4XX`, the pipeline is rejected.
+- `406`, the pipeline is rejected.
 - Other codes, the pipeline is accepted and logged.
 
 If there's an error or the request times out, the pipeline is accepted.

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

@@ -12,8 +12,7 @@ module Gitlab
 
             DEFAULT_VALIDATION_REQUEST_TIMEOUT = 5
             ACCEPTED_STATUS = 200
-            DOT_COM_REJECTED_STATUS = 406
-            GENERAL_REJECTED_STATUS = (400..499).freeze
+            REJECTED_STATUS = 406
 
             def perform!
               pipeline_authorized = validate_external
@@ -34,14 +33,13 @@ module Gitlab
               return true unless validation_service_url
 
               # 200 - accepted
-              # 406 - not accepted on GitLab.com
-              # 4XX - not accepted for other installations
+              # 406 - rejected
               # everything else - accepted and logged
               response_code = validate_service_request.code
               case response_code
               when ACCEPTED_STATUS
                 true
-              when rejected_status
+              when REJECTED_STATUS
                 false
               else
                 raise InvalidResponseCode, "Unsupported response code received from Validation Service: #{response_code}"
@@ -52,14 +50,6 @@ module Gitlab
               true
             end
 
-            def rejected_status
-              if Gitlab.com?
-                DOT_COM_REJECTED_STATUS
-              else
-                GENERAL_REJECTED_STATUS
-              end
-            end
-
             def validate_service_request
               headers = {
                 'X-Gitlab-Correlation-id' => Labkit::Correlation::CorrelationId.current_id,

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

@@ -43,7 +43,6 @@ 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
@@ -57,7 +56,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
 
     before do
       stub_env('EXTERNAL_VALIDATION_SERVICE_URL', validation_service_url)
-      allow(Gitlab).to receive(:com?).and_return(dot_com)
       allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('correlation-id')
     end
 
@@ -199,34 +197,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::External do
       end
     end
 
-    context 'when not on .com' do
-      let(:dot_com) { false }
-
-      before do
-        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 not authorized', project_id: project.id, user_id: user.id)
-
-        perform!
-      end
-    end
-
     context 'when validation returns 406 Not Acceptable' do
       before do
         stub_request(:post, validation_service_url).to_return(status: 406, body: "{}")