Split pipeline chain builder validation class

app/services/ci/create_pipeline_service.rb

@@ -2,7 +2,9 @@ module Ci
   class CreatePipelineService < BaseService
     attr_reader :pipeline
 
-    SEQUENCE = [Gitlab::Ci::Pipeline::Chain::Validate,
+    SEQUENCE = [Gitlab::Ci::Pipeline::Chain::ValidateAbilities,
+                Gitlab::Ci::Pipeline::Chain::ValidateRepository,
+                Gitlab::Ci::Pipeline::Chain::ValidateConfig,
                 Gitlab::Ci::Pipeline::Chain::Skip,
                 Gitlab::Ci::Pipeline::Chain::Create].freeze
 

lib/gitlab/ci/pipeline/chain/create.rb

@@ -3,6 +3,8 @@ module Gitlab
     module Pipeline
       module Chain
         class Create < Chain::Base
+          include Chain::Helpers
+
           def perform!
             ::Ci::Pipeline.transaction do
               pipeline.save!
@@ -16,7 +18,7 @@ module Gitlab
                 .execute(pipeline)
             end
           rescue ActiveRecord::RecordInvalid => e
-            pipeline.erros.add(:base, "Failed to persist the pipeline: #{e}")
+            error("Failed to persist the pipeline: #{e}")
           end
 
           def break?

lib/gitlab/ci/pipeline/chain/helpers.rb

+module Gitlab
+  module Ci
+    module Pipeline
+      module Chain
+        module Helpers
+          def branch_exists?
+            return @is_branch if defined?(@is_branch)
+
+            @is_branch = project.repository.branch_exists?(pipeline.ref)
+          end
+
+          def tag_exists?
+            return @is_tag if defined?(@is_tag)
+
+            @is_tag = project.repository.tag_exists?(pipeline.ref)
+          end
+
+          def error(message)
+            pipeline.errors.add(:base, message)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/pipeline/chain/validate.rb → lib/gitlab/ci/pipeline/chain/validate_abilities.rb

@@ -2,15 +2,26 @@ module Gitlab
   module Ci
     module Pipeline
       module Chain
-        class Validate < Chain::Base
+        class ValidateAbilities < Chain::Base
           include Gitlab::Allowable
+          include Chain::Helpers
 
           def perform!
-            validate_project! || validate_repository! || validate_pipeline!
+            unless project.builds_enabled?
+              return error('Pipelines are disabled!')
+            end
+
+            unless allowed_to_trigger_pipeline?
+              if can?(current_user, :create_pipeline, project)
+                return error("Insufficient permissions for protected ref '#{pipeline.ref}'")
+              else
+                return error('Insufficient permissions to create a new pipeline')
+              end
+            end
           end
 
           def break?
-            @pipeline.errors.any? || @pipeline.persisted?
+            @pipeline.errors.any?
           end
 
           def allowed_to_trigger_pipeline?
@@ -26,74 +37,14 @@ module Gitlab
 
             access = Gitlab::UserAccess.new(current_user, project: project)
 
-            if branch?
+            if branch_exists?
               access.can_update_branch?(@pipeline.ref)
-            elsif tag?
+            elsif tag_exists?
               access.can_create_tag?(@pipeline.ref)
             else
               true # Allow it for now and we'll reject when we check ref existence
             end
           end
-
-          private
-
-          def validate_project!
-            unless project.builds_enabled?
-              return error('Pipeline is disabled')
-            end
-
-            unless allowed_to_trigger_pipeline?
-              if can?(current_user, :create_pipeline, project)
-                return error("Insufficient permissions for protected ref '#{pipeline.ref}'")
-              else
-                return error('Insufficient permissions to create a new pipeline')
-              end
-            end
-          end
-
-          def validate_repository!
-            unless branch? || tag?
-              return error('Reference not found')
-            end
-
-            unless pipeline.sha
-              return error('Commit not found')
-            end
-          end
-
-          def validate_pipeline!
-            unless @pipeline.config_processor
-              unless @pipeline.ci_yaml_file
-                return error("Missing #{@pipeline.ci_yaml_file_path} file")
-              end
-
-              if @command.save_incompleted && @pipeline.has_yaml_errors?
-                @pipeline.drop
-              end
-
-              return error(@pipeline.yaml_errors)
-            end
-
-            unless @pipeline.has_stage_seeds?
-              return error('No stages / jobs for this pipeline.')
-            end
-          end
-
-          def branch?
-            return @is_branch if defined?(@is_branch)
-
-            @is_branch = project.repository.branch_exists?(pipeline.ref)
-          end
-
-          def tag?
-            return @is_tag if defined?(@is_tag)
-
-            @is_tag = project.repository.tag_exists?(pipeline.ref)
-          end
-
-          def error(message)
-            pipeline.errors.add(:base, message)
-          end
         end
       end
     end

lib/gitlab/ci/pipeline/chain/validate_config.rb

+module Gitlab
+  module Ci
+    module Pipeline
+      module Chain
+        class ValidateConfig < Chain::Base
+          include Chain::Helpers
+
+          def perform!
+            unless @pipeline.config_processor
+              unless @pipeline.ci_yaml_file
+                return error("Missing #{@pipeline.ci_yaml_file_path} file")
+              end
+
+              if @command.save_incompleted && @pipeline.has_yaml_errors?
+                @pipeline.drop
+              end
+
+              return error(@pipeline.yaml_errors)
+            end
+
+            unless @pipeline.has_stage_seeds?
+              return error('No stages / jobs for this pipeline.')
+            end
+          end
+
+          def break?
+            @pipeline.errors.any? || @pipeline.persisted?
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/pipeline/chain/validate_repository.rb

+module Gitlab
+  module Ci
+    module Pipeline
+      module Chain
+        class ValidateRepository < Chain::Base
+          include Chain::Helpers
+
+          def perform!
+            unless branch_exists? || tag_exists?
+              return error('Reference not found')
+            end
+
+            ## TODO, we check commit in the service, that is why
+            # there is no repository access here.
+            #
+            # Should we validate repository before building a pipeline?
+            #
+            unless pipeline.sha
+              return error('Commit not found')
+            end
+          end
+
+          def break?
+            @pipeline.errors.any?
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/pipeline/chain/validate_spec.rb → spec/lib/gitlab/ci/pipeline/chain/validate_abilities_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Ci::Pipeline::Chain::Validate do
+describe Gitlab::Ci::Pipeline::Chain::ValidateAbilities do
   describe '#allowed_to_create?' do
     let(:user) { create(:user) }
     let(:project) { create(:project, :repository) }