Assign pipeline protected attribute in Populate

This removes the Protect pipeline chain step and assigns the protected
attribute in the Populate step instead.

5 files changed, 29 insertions, 63 deletions

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index ead6c99b4c7..f8d8ef04001 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -10,7 +10,6 @@ module Ci
                 Gitlab::Ci::Pipeline::Chain::Validate::Abilities,
                 Gitlab::Ci::Pipeline::Chain::Validate::Repository,
                 Gitlab::Ci::Pipeline::Chain::Validate::Config,
-                Gitlab::Ci::Pipeline::Chain::Protect,
                 Gitlab::Ci::Pipeline::Chain::Skip,
                 Gitlab::Ci::Pipeline::Chain::Populate,
                 Gitlab::Ci::Pipeline::Chain::Create].freeze
diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb
index 633d3cd4f6b..0405292a25b 100644
--- a/lib/gitlab/ci/pipeline/chain/populate.rb
+++ b/lib/gitlab/ci/pipeline/chain/populate.rb
@@ -13,6 +13,10 @@ module Gitlab
             # Allocate next IID. This operation must be outside of transactions of pipeline creations.
             pipeline.ensure_project_iid!
 
+            # Protect the pipeline. This is assigned in Populate instead of
+            # Build to prevent erroring out on ambiguous refs.
+            pipeline.protected = @command.protected_ref?
+
             ##
             # Populate pipeline with block argument of CreatePipelineService#execute.
             #
diff --git a/lib/gitlab/ci/pipeline/chain/protect.rb b/lib/gitlab/ci/pipeline/chain/protect.rb
deleted file mode 100644
index 4c743d6b988..00000000000
--- a/lib/gitlab/ci/pipeline/chain/protect.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
-  module Ci
-    module Pipeline
-      module Chain
-        class Protect < Chain::Base
-          def perform!
-            @pipeline.protected = @command.protected_ref?
-          end
-
-          def break?
-            @pipeline.protected? != @command.protected_ref?
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
index 1b014ecfaa4..3459939267a 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
@@ -79,6 +79,31 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
     end
   end
 
+  describe 'pipeline protect' do
+    subject { step.perform! }
+
+    context 'when ref is protected' do
+      before do
+        allow(project).to receive(:protected_for?).with('master').and_return(true)
+        allow(project).to receive(:protected_for?).with('refs/heads/master').and_return(true)
+      end
+
+      it 'does not protect the pipeline' do
+        subject
+
+        expect(pipeline.protected).to eq(true)
+      end
+    end
+
+    context 'when ref is not protected' do
+      it 'does not protect the pipeline' do
+        subject
+
+        expect(pipeline.protected).to eq(false)
+      end
+    end
+  end
+
   context 'when pipeline has validation errors' do
     let(:pipeline) do
       build(:ci_pipeline, project: project, ref: nil)
diff --git a/spec/lib/gitlab/ci/pipeline/chain/protect_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/protect_spec.rb
deleted file mode 100644
index 8523ebb83fc..00000000000
--- a/spec/lib/gitlab/ci/pipeline/chain/protect_spec.rb
+++ /dev/null
@@ -1,43 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Ci::Pipeline::Chain::Protect do
-  set(:project) { create(:project) }
-  set(:user) { create(:user) }
-
-  let(:pipeline) do
-    build(:ci_empty_pipeline, project: project, ref: 'master')
-  end
-
-  let(:command) do
-    Gitlab::Ci::Pipeline::Chain::Command.new(
-      project: project, current_user: user, origin_ref: 'master')
-  end
-
-  let(:step) { described_class.new(pipeline, command) }
-
-  context 'when the ref is protected' do
-    before do
-      allow(project).to receive(:protected_for?).with('master').and_return(true)
-
-      step.perform!
-    end
-
-    it 'protects the pipeline' do
-      expect(pipeline.protected).to eq(true)
-    end
-  end
-
-  context 'when the ref is not protected' do
-    before do
-      allow(project).to receive(:protected_for?).with('master').and_return(false)
-
-      step.perform!
-    end
-
-    it 'does not protect the pipeline' do
-      expect(pipeline.protected).to eq(false)
-    end
-  end
-end