Split pipeline chain builder validation class

8 files changed, 147 insertions, 104 deletions

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index b22904fa4f1..ecf9be6600c 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/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
 
diff --git a/lib/gitlab/ci/pipeline/chain/create.rb b/lib/gitlab/ci/pipeline/chain/create.rb
index 3b067ca6ace..8bd658b3069 100644
--- a/lib/gitlab/ci/pipeline/chain/create.rb
+++ b/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?
diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb
new file mode 100644
index 00000000000..02d81286f21
--- /dev/null
+++ b/lib/gitlab/ci/pipeline/chain/helpers.rb
@@ -0,0 +1,25 @@
+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
diff --git a/lib/gitlab/ci/pipeline/chain/validate.rb b/lib/gitlab/ci/pipeline/chain/validate.rb
deleted file mode 100644
index ec06b0f76c5..00000000000
--- a/lib/gitlab/ci/pipeline/chain/validate.rb
+++ /dev/null
@@ -1,101 +0,0 @@
-module Gitlab
-  module Ci
-    module Pipeline
-      module Chain
-        class Validate < Chain::Base
-          include Gitlab::Allowable
-
-          def perform!
-            validate_project! || validate_repository! || validate_pipeline!
-          end
-
-          def break?
-            @pipeline.errors.any? || @pipeline.persisted?
-          end
-
-          def allowed_to_trigger_pipeline?
-            if current_user
-              allowed_to_create?
-            else # legacy triggers don't have a corresponding user
-              !project.protected_for?(@pipeline.ref)
-            end
-          end
-
-          def allowed_to_create?
-            return unless can?(current_user, :create_pipeline, project)
-
-            access = Gitlab::UserAccess.new(current_user, project: project)
-
-            if branch?
-              access.can_update_branch?(@pipeline.ref)
-            elsif tag?
-              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
-  end
-end
diff --git a/lib/gitlab/ci/pipeline/chain/validate_abilities.rb b/lib/gitlab/ci/pipeline/chain/validate_abilities.rb
new file mode 100644
index 00000000000..28a9c0ba999
--- /dev/null
+++ b/lib/gitlab/ci/pipeline/chain/validate_abilities.rb
@@ -0,0 +1,52 @@
+module Gitlab
+  module Ci
+    module Pipeline
+      module Chain
+        class ValidateAbilities < Chain::Base
+          include Gitlab::Allowable
+          include Chain::Helpers
+
+          def perform!
+            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?
+          end
+
+          def allowed_to_trigger_pipeline?
+            if current_user
+              allowed_to_create?
+            else # legacy triggers don't have a corresponding user
+              !project.protected_for?(@pipeline.ref)
+            end
+          end
+
+          def allowed_to_create?
+            return unless can?(current_user, :create_pipeline, project)
+
+            access = Gitlab::UserAccess.new(current_user, project: project)
+
+            if branch_exists?
+              access.can_update_branch?(@pipeline.ref)
+            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
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/pipeline/chain/validate_config.rb b/lib/gitlab/ci/pipeline/chain/validate_config.rb
new file mode 100644
index 00000000000..0dba8731438
--- /dev/null
+++ b/lib/gitlab/ci/pipeline/chain/validate_config.rb
@@ -0,0 +1,33 @@
+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
diff --git a/lib/gitlab/ci/pipeline/chain/validate_repository.rb b/lib/gitlab/ci/pipeline/chain/validate_repository.rb
new file mode 100644
index 00000000000..4d1b88a7065
--- /dev/null
+++ b/lib/gitlab/ci/pipeline/chain/validate_repository.rb
@@ -0,0 +1,30 @@
+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
diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate_abilities_spec.rb
index 117f55fd8a7..1613df395d1 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/validate_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/validate_abilities_spec.rb
@@ -1,6 +1,6 @@
 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) }