From 8e7f19c60bea4eec86844be1e0db12ebf30f105e Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 2 Dec 2017 22:55:49 -0800
Subject: Add button to run scheduled pipeline immediately

Closes #38741
---
 .../projects/pipeline_schedules_controller.rb      | 16 +++++++++-
 app/helpers/gitlab_routing_helper.rb               |  5 +++
 .../_pipeline_schedule.html.haml                   |  4 ++-
 app/workers/run_pipeline_schedule_worker.rb        | 20 ++++++++++++
 .../sh-add-schedule-pipeline-run-now.yml           |  5 +++
 config/routes/project.rb                           |  1 +
 .../projects/pipeline_schedules_controller_spec.rb | 26 +++++++++++++++-
 spec/workers/run_pipeline_schedule_worker_spec.rb  | 36 ++++++++++++++++++++++
 8 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 app/workers/run_pipeline_schedule_worker.rb
 create mode 100644 changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml
 create mode 100644 spec/workers/run_pipeline_schedule_worker_spec.rb

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index ec7c645df5a..9fc7935950d 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,9 +1,10 @@
 class Projects::PipelineSchedulesController < Projects::ApplicationController
   before_action :schedule, except: [:index, :new, :create]
 
+  before_action :authorize_create_pipeline!, only: [:run]
   before_action :authorize_read_pipeline_schedule!
   before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
-  before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create]
+  before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :run]
   before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
 
   def index
@@ -40,6 +41,19 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
     end
   end
 
+  def run
+    job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id)
+
+    flash[:notice] =
+      if job_id
+        'Successfully scheduled pipeline to run immediately'
+      else
+        'Unable to schedule a pipeline to run immediately'
+      end
+
+    redirect_to pipeline_schedules_path(@project)
+  end
+
   def take_ownership
     if schedule.update(owner: current_user)
       redirect_to pipeline_schedules_path(@project)
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index a77aa0ad2cc..5f72c6e64b6 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/app/helpers/gitlab_routing_helper.rb
@@ -182,6 +182,11 @@ module GitlabRoutingHelper
     edit_project_pipeline_schedule_path(project, schedule)
   end
 
+  def run_pipeline_schedule_path(schedule, *args)
+    project = schedule.project
+    run_project_pipeline_schedule_path(project, schedule, *args)
+  end
+
   def take_ownership_pipeline_schedule_path(schedule, *args)
     project = schedule.project
     take_ownership_project_pipeline_schedule_path(project, schedule, *args)
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index bd8c38292d6..57f0ffba9ff 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -26,10 +26,12 @@
           = pipeline_schedule.owner&.name
     %td
       .pull-right.btn-group
+        - if can?(current_user, :create_pipeline, @project)
+          = link_to run_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('Play'), class: 'btn' do
+            = icon('play')
         - if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
           = link_to take_ownership_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('PipelineSchedules|Take ownership'), class: 'btn' do
             = s_('PipelineSchedules|Take ownership')
-        - if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
           = link_to edit_pipeline_schedule_path(pipeline_schedule), title: _('Edit'), class: 'btn' do
             = icon('pencil')
         - if can?(current_user, :admin_pipeline_schedule, pipeline_schedule)
diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb
new file mode 100644
index 00000000000..ac6371f0871
--- /dev/null
+++ b/app/workers/run_pipeline_schedule_worker.rb
@@ -0,0 +1,20 @@
+class RunPipelineScheduleWorker
+  include Sidekiq::Worker
+  include PipelineQueue
+
+  enqueue_in group: :creation
+
+  def perform(schedule_id, user_id)
+    schedule = Ci::PipelineSchedule.find(schedule_id)
+    user = User.find(user_id)
+
+    run_pipeline_schedule(schedule, user)
+  end
+
+  def run_pipeline_schedule(schedule, user)
+    Ci::CreatePipelineService.new(schedule.project,
+                                  user,
+                                  ref: schedule.ref)
+      .execute(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: schedule)
+  end
+end
diff --git a/changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml b/changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml
new file mode 100644
index 00000000000..6d06f695f10
--- /dev/null
+++ b/changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml
@@ -0,0 +1,5 @@
+---
+title: Add button to run scheduled pipeline immediately
+merge_request:
+author:
+type: added
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 093da10f57f..3dcb21e45c5 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -179,6 +179,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       resources :pipeline_schedules, except: [:show] do
         member do
+          post :run
           post :take_ownership
         end
       end
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 4e52e261920..384a407a100 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -96,7 +96,7 @@ describe Projects::PipelineSchedulesController do
         end
       end
 
-      context 'when variables_attributes has two variables and duplicted' do
+      context 'when variables_attributes has two variables and duplicated' do
         let(:schedule) do
           basic_param.merge({
             variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
@@ -364,6 +364,30 @@ describe Projects::PipelineSchedulesController do
     end
   end
 
+  describe 'POST #run' do
+    set(:user) { create(:user) }
+
+    context 'when a developer makes the request' do
+      before do
+        project.add_developer(user)
+        sign_in(user)
+      end
+
+      it 'executes a new pipeline' do
+        expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123')
+
+        go
+
+        expect(flash[:notice]).to eq 'Successfully scheduled pipeline to run immediately'
+        expect(response).to have_gitlab_http_status(302)
+      end
+    end
+
+    def go
+      post :run, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+    end
+  end
+
   describe 'DELETE #destroy' do
     set(:user) { create(:user) }
 
diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb
new file mode 100644
index 00000000000..4a88ac51f62
--- /dev/null
+++ b/spec/workers/run_pipeline_schedule_worker_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe RunPipelineScheduleWorker do
+  describe '#perform' do
+    let(:project) { create(:project) }
+    let(:worker) { described_class.new }
+    let(:user) { create(:user) }
+    let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) }
+
+    context 'when a project not found' do
+      it 'does not call the Service' do
+        expect(Ci::CreatePipelineService).not_to receive(:new)
+        expect { worker.perform(100000, user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context 'when a user not found' do
+      it 'does not call the Service' do
+        expect(Ci::CreatePipelineService).not_to receive(:new)
+        expect { worker.perform(pipeline_schedule.id, 10000) }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context 'when everything is ok' do
+      let(:project) { create(:project) }
+      let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
+
+      it 'calls the Service' do
+        expect(Ci::CreatePipelineService).to receive(:new).with(project, user, ref: pipeline_schedule.ref).and_return(create_pipeline_service)
+        expect(create_pipeline_service).to receive(:execute).with(:schedule, ignore_skip_ci: true, save_on_errors: false, schedule: pipeline_schedule)
+
+        worker.perform(pipeline_schedule.id, user.id)
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From f6966cfa63fab7e3c8847d69101c6c6a444fb85f Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 5 Dec 2017 22:24:20 -0800
Subject: Address some comments with running a pipeline schedule

---
 app/controllers/projects/pipeline_schedules_controller.rb |  6 +++---
 app/helpers/gitlab_routing_helper.rb                      |  4 ++--
 app/workers/run_pipeline_schedule_worker.rb               |  6 ++++--
 config/routes/project.rb                                  |  2 +-
 .../projects/pipeline_schedules_controller_spec.rb        |  4 ++--
 spec/workers/run_pipeline_schedule_worker_spec.rb         | 15 +++++++++------
 6 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index 9fc7935950d..38edb38f9fc 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,10 +1,10 @@
 class Projects::PipelineSchedulesController < Projects::ApplicationController
   before_action :schedule, except: [:index, :new, :create]
 
-  before_action :authorize_create_pipeline!, only: [:run]
+  before_action :authorize_create_pipeline!, only: [:play]
   before_action :authorize_read_pipeline_schedule!
   before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
-  before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :run]
+  before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play]
   before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
 
   def index
@@ -41,7 +41,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
     end
   end
 
-  def run
+  def play
     job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id)
 
     flash[:notice] =
diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index 5f72c6e64b6..7f3c118c7ab 100644
--- a/app/helpers/gitlab_routing_helper.rb
+++ b/app/helpers/gitlab_routing_helper.rb
@@ -182,9 +182,9 @@ module GitlabRoutingHelper
     edit_project_pipeline_schedule_path(project, schedule)
   end
 
-  def run_pipeline_schedule_path(schedule, *args)
+  def play_pipeline_schedule_path(schedule, *args)
     project = schedule.project
-    run_project_pipeline_schedule_path(project, schedule, *args)
+    play_project_pipeline_schedule_path(project, schedule, *args)
   end
 
   def take_ownership_pipeline_schedule_path(schedule, *args)
diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb
index ac6371f0871..3e0d3fed20a 100644
--- a/app/workers/run_pipeline_schedule_worker.rb
+++ b/app/workers/run_pipeline_schedule_worker.rb
@@ -5,8 +5,10 @@ class RunPipelineScheduleWorker
   enqueue_in group: :creation
 
   def perform(schedule_id, user_id)
-    schedule = Ci::PipelineSchedule.find(schedule_id)
-    user = User.find(user_id)
+    schedule = Ci::PipelineSchedule.find_by(id: schedule_id)
+    user = User.find_by(id: user_id)
+
+    return unless schedule && user
 
     run_pipeline_schedule(schedule, user)
   end
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 3dcb21e45c5..239b5480321 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -179,7 +179,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       resources :pipeline_schedules, except: [:show] do
         member do
-          post :run
+          post :play
           post :take_ownership
         end
       end
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 384a407a100..e875f5bce08 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -364,7 +364,7 @@ describe Projects::PipelineSchedulesController do
     end
   end
 
-  describe 'POST #run' do
+  describe 'POST #play' do
     set(:user) { create(:user) }
 
     context 'when a developer makes the request' do
@@ -384,7 +384,7 @@ describe Projects::PipelineSchedulesController do
     end
 
     def go
-      post :run, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
     end
   end
 
diff --git a/spec/workers/run_pipeline_schedule_worker_spec.rb b/spec/workers/run_pipeline_schedule_worker_spec.rb
index 4a88ac51f62..481a84837f9 100644
--- a/spec/workers/run_pipeline_schedule_worker_spec.rb
+++ b/spec/workers/run_pipeline_schedule_worker_spec.rb
@@ -2,27 +2,30 @@ require 'spec_helper'
 
 describe RunPipelineScheduleWorker do
   describe '#perform' do
-    let(:project) { create(:project) }
+    set(:project) { create(:project) }
+    set(:user) { create(:user) }
+    set(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) }
     let(:worker) { described_class.new }
-    let(:user) { create(:user) }
-    let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) }
 
     context 'when a project not found' do
       it 'does not call the Service' do
         expect(Ci::CreatePipelineService).not_to receive(:new)
-        expect { worker.perform(100000, user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+        expect(worker).not_to receive(:run_pipeline_schedule)
+
+        worker.perform(100000, user.id)
       end
     end
 
     context 'when a user not found' do
       it 'does not call the Service' do
         expect(Ci::CreatePipelineService).not_to receive(:new)
-        expect { worker.perform(pipeline_schedule.id, 10000) }.to raise_error(ActiveRecord::RecordNotFound)
+        expect(worker).not_to receive(:run_pipeline_schedule)
+
+        worker.perform(pipeline_schedule.id, 10000)
       end
     end
 
     context 'when everything is ok' do
-      let(:project) { create(:project) }
       let(:create_pipeline_service) { instance_double(Ci::CreatePipelineService) }
 
       it 'calls the Service' do
-- 
cgit v1.2.1


From bc2d32aca0be46250bd02c9312d1064df024b621 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 5 Dec 2017 23:23:59 -0800
Subject: Create a play_pipeline_schedule policy and use it

---
 .../projects/pipeline_schedules_controller.rb       |  6 +++++-
 app/policies/ci/pipeline_schedule_policy.rb         | 18 ++++++++++++++++++
 .../pipeline_schedules/_pipeline_schedule.html.haml |  2 +-
 .../projects/pipeline_schedules_controller_spec.rb  | 21 ++++++++++++++++-----
 4 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index 38edb38f9fc..a4e865cb9da 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,7 +1,7 @@
 class Projects::PipelineSchedulesController < Projects::ApplicationController
   before_action :schedule, except: [:index, :new, :create]
 
-  before_action :authorize_create_pipeline!, only: [:play]
+  before_action :authorize_play_pipeline_schedule!, only: [:play]
   before_action :authorize_read_pipeline_schedule!
   before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
   before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play]
@@ -84,6 +84,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
         variables_attributes: [:id, :key, :value, :_destroy] )
   end
 
+  def authorize_play_pipeline_schedule!
+    return access_denied! unless can?(current_user, :play_pipeline_schedule, schedule)
+  end
+
   def authorize_update_pipeline_schedule!
     return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule)
   end
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index 6b7598e1821..8e7e129f135 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -2,13 +2,31 @@ module Ci
   class PipelineSchedulePolicy < PipelinePolicy
     alias_method :pipeline_schedule, :subject
 
+    condition(:protected_ref) do
+      access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
+
+      if @subject.project.repository.branch_exists?(@subject.ref)
+        access.can_update_branch?(@subject.ref)
+      elsif @subject.project.repository.tag_exists?(@subject.ref)
+        access.can_create_tag?(@subject.ref)
+      else
+        true
+      end
+    end
+
     condition(:owner_of_schedule) do
       can?(:developer_access) && pipeline_schedule.owned_by?(@user)
     end
 
+    rule { can?(:developer_access) }.policy do
+      enable :play_pipeline_schedule
+    end
+
     rule { can?(:master_access) | owner_of_schedule }.policy do
       enable :update_pipeline_schedule
       enable :admin_pipeline_schedule
     end
+
+    rule { protected_ref }.prevent :play_pipeline_schedule
   end
 end
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index 57f0ffba9ff..12d7e81c39e 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -26,7 +26,7 @@
           = pipeline_schedule.owner&.name
     %td
       .pull-right.btn-group
-        - if can?(current_user, :create_pipeline, @project)
+        - if can?(current_user, :play_pipeline_schedule, pipeline_schedule)
           = link_to run_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('Play'), class: 'btn' do
             = icon('play')
         - if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index e875f5bce08..3878b0a5e4f 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -3,8 +3,8 @@ require 'spec_helper'
 describe Projects::PipelineSchedulesController do
   include AccessMatchersForController
 
-  set(:project) { create(:project, :public) }
-  let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
+  set(:project) { create(:project, :public, :repository) }
+  set(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
 
   describe 'GET #index' do
     let(:scope) { nil }
@@ -366,25 +366,36 @@ describe Projects::PipelineSchedulesController do
 
   describe 'POST #play' do
     set(:user) { create(:user) }
+    let(:ref) { 'master' }
 
     context 'when a developer makes the request' do
       before do
         project.add_developer(user)
+
         sign_in(user)
       end
 
       it 'executes a new pipeline' do
         expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123')
 
-        go
+        post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
 
         expect(flash[:notice]).to eq 'Successfully scheduled pipeline to run immediately'
         expect(response).to have_gitlab_http_status(302)
       end
     end
 
-    def go
-      post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+    context 'when a developer attempts to schedule a protected ref' do
+      it 'does not allow pipeline to be executed' do
+        create(:protected_branch, project: project, name: ref)
+        protected_schedule = create(:ci_pipeline_schedule, project: project, ref: ref)
+
+        expect(RunPipelineScheduleWorker).not_to receive(:perform_async)
+
+        post :play, namespace_id: project.namespace.to_param, project_id: project, id: protected_schedule.id
+
+        expect(response).to have_gitlab_http_status(404)
+      end
     end
   end
 
-- 
cgit v1.2.1


From 87118872c94ab465d400090e247b1e4ae90c2d82 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 5 Dec 2017 23:36:49 -0800
Subject: Fix conditions for checking pipeline schedule rules

---
 app/policies/ci/pipeline_schedule_policy.rb                        | 6 +++---
 app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index 8e7e129f135..c0a2bb963e9 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -6,11 +6,11 @@ module Ci
       access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
 
       if @subject.project.repository.branch_exists?(@subject.ref)
-        access.can_update_branch?(@subject.ref)
+        !access.can_update_branch?(@subject.ref)
       elsif @subject.project.repository.tag_exists?(@subject.ref)
-        access.can_create_tag?(@subject.ref)
+        !access.can_create_tag?(@subject.ref)
       else
-        true
+        false
       end
     end
 
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index 12d7e81c39e..f8c4005a9e0 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -27,7 +27,7 @@
     %td
       .pull-right.btn-group
         - if can?(current_user, :play_pipeline_schedule, pipeline_schedule)
-          = link_to run_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('Play'), class: 'btn' do
+          = link_to play_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('Play'), class: 'btn' do
             = icon('play')
         - if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
           = link_to take_ownership_pipeline_schedule_path(pipeline_schedule), method: :post, title: s_('PipelineSchedules|Take ownership'), class: 'btn' do
-- 
cgit v1.2.1


From ad3732955389024b8a209455f9a889d5ebf411b9 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 7 Dec 2017 23:49:55 -0800
Subject: Refactor common protected ref check

---
 app/policies/ci/pipeline_policy.rb                | 16 ++--
 app/policies/ci/pipeline_schedule_policy.rb       | 10 +--
 spec/policies/ci/pipeline_schedule_policy_spec.rb | 92 +++++++++++++++++++++++
 3 files changed, 102 insertions(+), 16 deletions(-)
 create mode 100644 spec/policies/ci/pipeline_schedule_policy_spec.rb

diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb
index 4e689a9efd5..6363c382ff8 100644
--- a/app/policies/ci/pipeline_policy.rb
+++ b/app/policies/ci/pipeline_policy.rb
@@ -2,16 +2,18 @@ module Ci
   class PipelinePolicy < BasePolicy
     delegate { @subject.project }
 
-    condition(:protected_ref) do
-      access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
+    condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.tag?, @subject.ref) }
 
-      if @subject.tag?
-        !access.can_create_tag?(@subject.ref)
+    rule { protected_ref }.prevent :update_pipeline
+
+    def ref_protected?(user, project, tag, ref)
+      access = ::Gitlab::UserAccess.new(user, project: project)
+
+      if tag
+        !access.can_create_tag?(ref)
       else
-        !access.can_update_branch?(@subject.ref)
+        !access.can_update_branch?(ref)
       end
     end
-
-    rule { protected_ref }.prevent :update_pipeline
   end
 end
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index c0a2bb963e9..abcf536b2f7 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -3,15 +3,7 @@ module Ci
     alias_method :pipeline_schedule, :subject
 
     condition(:protected_ref) do
-      access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
-
-      if @subject.project.repository.branch_exists?(@subject.ref)
-        !access.can_update_branch?(@subject.ref)
-      elsif @subject.project.repository.tag_exists?(@subject.ref)
-        !access.can_create_tag?(@subject.ref)
-      else
-        false
-      end
+      ref_protected?(@user, @subject.project, @subject.project.repository.tag_exists?(@subject.ref), @subject.ref)
     end
 
     condition(:owner_of_schedule) do
diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb
new file mode 100644
index 00000000000..1b0e9fac355
--- /dev/null
+++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb
@@ -0,0 +1,92 @@
+require 'spec_helper'
+
+describe Ci::PipelineSchedulePolicy, :models do
+  set(:user) { create(:user) }
+  set(:project) { create(:project, :repository) }
+  set(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) }
+
+  let(:policy) do
+    described_class.new(user, pipeline_schedule)
+  end
+
+  describe 'rules' do
+    describe 'rules for protected ref' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'when no one can push or merge to the branch' do
+        before do
+          create(:protected_branch, :no_one_can_push,
+                 name: pipeline_schedule.ref, project: project)
+        end
+
+        it 'does not include ability to play pipeline schedule' do
+          expect(policy).to be_disallowed :play_pipeline_schedule
+        end
+      end
+
+      context 'when developers can push to the branch' do
+        before do
+          create(:protected_branch, :developers_can_merge,
+                 name: pipeline_schedule.ref, project: project)
+        end
+
+        it 'includes ability to update pipeline' do
+          expect(policy).to be_allowed :play_pipeline_schedule
+        end
+      end
+
+      context 'when no one can create the tag' do
+        let(:tag) { 'v1.0.0' }
+
+        before do
+          pipeline_schedule.update(ref: tag)
+
+          create(:protected_tag, :no_one_can_create,
+                 name: pipeline_schedule.ref, project: project)
+        end
+
+        it 'does not include ability to play pipeline schedule' do
+          expect(policy).to be_disallowed :play_pipeline_schedule
+        end
+      end
+
+      context 'when no one can create the tag but it is not a tag' do
+        before do
+          create(:protected_tag, :no_one_can_create,
+                 name: pipeline_schedule.ref, project: project)
+        end
+
+        it 'includes ability to play pipeline schedule' do
+          expect(policy).to be_allowed :play_pipeline_schedule
+        end
+      end
+    end
+
+    describe 'rules for owner of schedule' do
+      before do
+        project.add_developer(user)
+        pipeline_schedule.update(owner: user)
+      end
+
+      it 'includes abilities to do do all operations on pipeline schedule' do
+        expect(policy).to be_allowed :play_pipeline_schedule
+        expect(policy).to be_allowed :update_pipeline_schedule
+        expect(policy).to be_allowed :admin_pipeline_schedule
+      end
+    end
+
+    describe 'rules for a master' do
+      before do
+        project.add_master(user)
+      end
+
+      it 'includes abilities to do do all operations on pipeline schedule' do
+        expect(policy).to be_allowed :play_pipeline_schedule
+        expect(policy).to be_allowed :update_pipeline_schedule
+        expect(policy).to be_allowed :admin_pipeline_schedule
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From 0ea70802e19cbe11c6af0f6750200bb137225940 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Thu, 7 Dec 2017 23:58:05 -0800
Subject: Fix Sidekiq worker and make flash message return a link to the
 pipelines page

---
 app/controllers/projects/pipeline_schedules_controller.rb       | 2 +-
 app/workers/run_pipeline_schedule_worker.rb                     | 2 +-
 spec/controllers/projects/pipeline_schedules_controller_spec.rb | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index a4e865cb9da..fe77d8eabeb 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -46,7 +46,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
 
     flash[:notice] =
       if job_id
-        'Successfully scheduled pipeline to run immediately'
+        "Successfully scheduled a pipeline to run. Go to the <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details".html_safe
       else
         'Unable to schedule a pipeline to run immediately'
       end
diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb
index 3e0d3fed20a..7725ad319a3 100644
--- a/app/workers/run_pipeline_schedule_worker.rb
+++ b/app/workers/run_pipeline_schedule_worker.rb
@@ -1,5 +1,5 @@
 class RunPipelineScheduleWorker
-  include Sidekiq::Worker
+  include ApplicationWorker
   include PipelineQueue
 
   enqueue_in group: :creation
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 3878b0a5e4f..debc20ee467 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -380,7 +380,7 @@ describe Projects::PipelineSchedulesController do
 
         post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
 
-        expect(flash[:notice]).to eq 'Successfully scheduled pipeline to run immediately'
+        expect(flash[:notice]).to start_with 'Successfully scheduled a pipeline to run'
         expect(response).to have_gitlab_http_status(302)
       end
     end
-- 
cgit v1.2.1


From f8c3a58a54d622193a0cf15777a0d0631289278c Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 8 Dec 2017 22:20:28 -0800
Subject: Avoid Gitaly N+1 calls by caching tag_names

---
 app/models/repository.rb                                        | 6 ++++++
 spec/controllers/projects/pipeline_schedules_controller_spec.rb | 2 ++
 spec/models/repository_spec.rb                                  | 9 +++++++++
 3 files changed, 17 insertions(+)

diff --git a/app/models/repository.rb b/app/models/repository.rb
index c0e31eca8da..2413e60bc76 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -221,6 +221,12 @@ class Repository
     branch_names.include?(branch_name)
   end
 
+  def tag_exists?(tag_name)
+    return false unless raw_repository
+
+    tag_names.include?(tag_name)
+  end
+
   def ref_exists?(ref)
     !!raw_repository&.ref_exists?(ref)
   rescue ArgumentError
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index debc20ee467..8888573e882 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -7,6 +7,8 @@ describe Projects::PipelineSchedulesController do
   set(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
 
   describe 'GET #index' do
+    render_views
+
     let(:scope) { nil }
     let!(:inactive_pipeline_schedule) do
       create(:ci_pipeline_schedule, :inactive, project: project)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 129fce74f45..08eb563082f 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1157,6 +1157,15 @@ describe Repository do
     end
   end
 
+  describe '#tag_exists?' do
+    it 'uses tag_names' do
+      allow(repository).to receive(:tag_names).and_return(['foobar'])
+
+      expect(repository.tag_exists?('foobar')).to eq(true)
+      expect(repository.tag_exists?('master')).to eq(false)
+    end
+  end
+
   describe '#branch_names', :use_clean_rails_memory_store_caching do
     let(:fake_branch_names) { ['foobar'] }
 
-- 
cgit v1.2.1


From 54f13b1ec8542dc5085e0367734e8344c2c3d01e Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 9 Dec 2017 01:01:42 -0800
Subject: Add rate limiting to guard against excessive scheduling of pipelines

---
 .../projects/pipeline_schedules_controller.rb      | 11 ++++++++
 lib/gitlab/action_rate_limiter.rb                  | 31 ++++++++++++++++++++++
 .../projects/pipeline_schedules_controller_spec.rb |  2 +-
 3 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 lib/gitlab/action_rate_limiter.rb

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index fe77d8eabeb..b7a0a3591cd 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -42,6 +42,13 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
   end
 
   def play
+    limiter = ::Gitlab::ActionRateLimiter.new(action: 'play_pipeline_schedule')
+
+    if limiter.throttled?(throttle_key, 1)
+      flash[:notice] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
+      return redirect_to pipeline_schedules_path(@project)
+    end
+
     job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id)
 
     flash[:notice] =
@@ -74,6 +81,10 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
 
   private
 
+  def throttle_key
+    "user:#{current_user.id}:schedule:#{schedule.id}"
+  end
+
   def schedule
     @schedule ||= project.pipeline_schedules.find(params[:id])
   end
diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb
new file mode 100644
index 00000000000..c3af583a3ed
--- /dev/null
+++ b/lib/gitlab/action_rate_limiter.rb
@@ -0,0 +1,31 @@
+module Gitlab
+  # This class implements a simple rate limiter that can be used to throttle
+  # certain actions. Unlike Rack Attack and Rack::Throttle, which operate at
+  # the middleware level, this can be used at the controller level.
+  class ActionRateLimiter
+    TIME_TO_EXPIRE = 60 # 1 min
+
+    attr_accessor :action, :expiry_time
+
+    def initialize(action:, expiry_time: TIME_TO_EXPIRE)
+      @action = action
+      @expiry_time = expiry_time
+    end
+
+    def increment(key)
+      value = 0
+
+      Gitlab::Redis::Cache.with do |redis|
+        cache_key = "action_rate_limiter:#{action}:#{key}"
+        value = redis.incr(cache_key)
+        redis.expire(cache_key, expiry_time) if value == 1
+      end
+
+      value.to_i
+    end
+
+    def throttled?(key, threshold_value)
+      self.increment(key) > threshold_value
+    end
+  end
+end
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 8888573e882..844c62ef005 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -366,7 +366,7 @@ describe Projects::PipelineSchedulesController do
     end
   end
 
-  describe 'POST #play' do
+  describe 'POST #play', :clean_gitlab_redis_cache do
     set(:user) { create(:user) }
     let(:ref) { 'master' }
 
-- 
cgit v1.2.1


From ef78f67f4a2e19d204f5f4d4770649be1fe7bee9 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 9 Dec 2017 15:08:27 -0800
Subject: Add a spec for rate limiting pipeline schedules

---
 app/controllers/projects/pipeline_schedules_controller.rb      |  4 ++--
 .../controllers/projects/pipeline_schedules_controller_spec.rb | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index b7a0a3591cd..87878667e9b 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -45,7 +45,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
     limiter = ::Gitlab::ActionRateLimiter.new(action: 'play_pipeline_schedule')
 
     if limiter.throttled?(throttle_key, 1)
-      flash[:notice] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
+      flash[:alert] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
       return redirect_to pipeline_schedules_path(@project)
     end
 
@@ -53,7 +53,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
 
     flash[:notice] =
       if job_id
-        "Successfully scheduled a pipeline to run. Go to the <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details".html_safe
+        "Successfully scheduled a pipeline to run. Go to the <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details.".html_safe
       else
         'Unable to schedule a pipeline to run immediately'
       end
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 844c62ef005..ffc1259eb8f 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -385,6 +385,16 @@ describe Projects::PipelineSchedulesController do
         expect(flash[:notice]).to start_with 'Successfully scheduled a pipeline to run'
         expect(response).to have_gitlab_http_status(302)
       end
+
+      it 'prevents users from scheduling the same pipeline repeatedly' do
+        2.times do
+          post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+        end
+
+        expect(flash.to_a.size).to eq(2)
+        expect(flash[:alert]).to eq 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
+        expect(response).to have_gitlab_http_status(302)
+      end
     end
 
     context 'when a developer attempts to schedule a protected ref' do
-- 
cgit v1.2.1


From 8b939bfab769810ba56f5f4ca18632fd7ae435db Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 9 Dec 2017 15:45:01 -0800
Subject: Add spec for ActionRateLimiter

---
 lib/gitlab/action_rate_limiter.rb           |  4 ++--
 spec/lib/gitlab/action_rate_limiter_spec.rb | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 spec/lib/gitlab/action_rate_limiter_spec.rb

diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb
index c3af583a3ed..7b231ac14ca 100644
--- a/lib/gitlab/action_rate_limiter.rb
+++ b/lib/gitlab/action_rate_limiter.rb
@@ -16,12 +16,12 @@ module Gitlab
       value = 0
 
       Gitlab::Redis::Cache.with do |redis|
-        cache_key = "action_rate_limiter:#{action}:#{key}"
+        cache_key = "action_rate_limiter:#{action.to_s}:#{key}"
         value = redis.incr(cache_key)
         redis.expire(cache_key, expiry_time) if value == 1
       end
 
-      value.to_i
+      value
     end
 
     def throttled?(key, threshold_value)
diff --git a/spec/lib/gitlab/action_rate_limiter_spec.rb b/spec/lib/gitlab/action_rate_limiter_spec.rb
new file mode 100644
index 00000000000..84661605623
--- /dev/null
+++ b/spec/lib/gitlab/action_rate_limiter_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe Gitlab::ActionRateLimiter do
+  let(:redis) { double('redis') }
+  let(:key) { 'user:1' }
+  let(:cache_key) { "action_rate_limiter:test_action:#{key}" }
+
+  subject { described_class.new(action: :test_action, expiry_time: 100) }
+
+  before do
+    allow(Gitlab::Redis::Cache).to receive(:with).and_yield(redis)
+  end
+
+  it 'increases the throttle count and sets the expire time' do
+    expect(redis).to receive(:incr).with(cache_key).and_return(1)
+    expect(redis).to receive(:expire).with(cache_key, 100)
+
+    expect(subject.throttled?(key, 1)).to be false
+  end
+
+  it 'returns true if the key is throttled' do
+    expect(redis).to receive(:incr).with(cache_key).and_return(2)
+    expect(redis).not_to receive(:expire)
+
+    expect(subject.throttled?(key, 1)).to be true
+  end
+end
-- 
cgit v1.2.1


From 02e7e499d4011733c1cf0f6fb675dd2d309f0d52 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 9 Dec 2017 21:00:34 -0800
Subject: Fix Rubocop offense and use a symbol instead of a string

---
 app/controllers/projects/pipeline_schedules_controller.rb | 2 +-
 lib/gitlab/action_rate_limiter.rb                         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index 87878667e9b..dd6593650e7 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -42,7 +42,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
   end
 
   def play
-    limiter = ::Gitlab::ActionRateLimiter.new(action: 'play_pipeline_schedule')
+    limiter = ::Gitlab::ActionRateLimiter.new(action: :play_pipeline_schedule)
 
     if limiter.throttled?(throttle_key, 1)
       flash[:alert] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb
index 7b231ac14ca..53add503c60 100644
--- a/lib/gitlab/action_rate_limiter.rb
+++ b/lib/gitlab/action_rate_limiter.rb
@@ -16,7 +16,7 @@ module Gitlab
       value = 0
 
       Gitlab::Redis::Cache.with do |redis|
-        cache_key = "action_rate_limiter:#{action.to_s}:#{key}"
+        cache_key = "action_rate_limiter:#{action}:#{key}"
         value = redis.incr(cache_key)
         redis.expire(cache_key, expiry_time) if value == 1
       end
-- 
cgit v1.2.1


From 4b0465f20de1bf58326c7daf6876b63438f00d84 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 12 Dec 2017 15:46:05 -0800
Subject: Address review comments with playing pipeline scheduler

---
 .../projects/pipeline_schedules_controller.rb      | 30 +++++++++++-----------
 lib/gitlab/action_rate_limiter.rb                  | 18 ++++++++++++-
 .../projects/pipeline_schedules_controller_spec.rb | 20 ++++++++++++---
 spec/lib/gitlab/action_rate_limiter_spec.rb        |  6 +++--
 4 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index dd6593650e7..b478e7b5e05 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,6 +1,7 @@
 class Projects::PipelineSchedulesController < Projects::ApplicationController
   before_action :schedule, except: [:index, :new, :create]
 
+  before_action :play_rate_limit, only: [:play]
   before_action :authorize_play_pipeline_schedule!, only: [:play]
   before_action :authorize_read_pipeline_schedule!
   before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
@@ -42,21 +43,13 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
   end
 
   def play
-    limiter = ::Gitlab::ActionRateLimiter.new(action: :play_pipeline_schedule)
-
-    if limiter.throttled?(throttle_key, 1)
-      flash[:alert] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
-      return redirect_to pipeline_schedules_path(@project)
-    end
-
     job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id)
 
-    flash[:notice] =
-      if job_id
-        "Successfully scheduled a pipeline to run. Go to the <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details.".html_safe
-      else
-        'Unable to schedule a pipeline to run immediately'
-      end
+    if job_id
+      flash[:notice] = "Successfully scheduled a pipeline to run. Go to the <a href=\"#{project_pipelines_path(@project)}\">Pipelines page</a> for details.".html_safe
+    else
+      flash[:alert] = 'Unable to schedule a pipeline to run immediately'
+    end
 
     redirect_to pipeline_schedules_path(@project)
   end
@@ -81,8 +74,15 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
 
   private
 
-  def throttle_key
-    "user:#{current_user.id}:schedule:#{schedule.id}"
+  def play_rate_limit
+    return unless current_user
+
+    limiter = ::Gitlab::ActionRateLimiter.new(action: :play_pipeline_schedule)
+
+    return unless limiter.throttled?([current_user, schedule], 1)
+
+    flash[:alert] = 'You cannot play this scheduled pipeline at the moment. Please wait a minute.'
+    redirect_to pipeline_schedules_path(@project)
   end
 
   def schedule
diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb
index 53add503c60..4cd3bdefda3 100644
--- a/lib/gitlab/action_rate_limiter.rb
+++ b/lib/gitlab/action_rate_limiter.rb
@@ -12,11 +12,15 @@ module Gitlab
       @expiry_time = expiry_time
     end
 
+    # Increments the given cache key and increments the value by 1 with the
+    # given expiration time. Returns the incremented value.
+    #
+    # key - An array of ActiveRecord instances
     def increment(key)
       value = 0
 
       Gitlab::Redis::Cache.with do |redis|
-        cache_key = "action_rate_limiter:#{action}:#{key}"
+        cache_key = action_key(key)
         value = redis.incr(cache_key)
         redis.expire(cache_key, expiry_time) if value == 1
       end
@@ -24,8 +28,20 @@ module Gitlab
       value
     end
 
+    # Increments the given key and returns true if the action should
+    # be throttled.
+    #
+    # key - An array of ActiveRecord instances
+    # threshold_value - The maximum number of times this action should occur in the given time interval
     def throttled?(key, threshold_value)
       self.increment(key) > threshold_value
     end
+
+    private
+
+    def action_key(key)
+      serialized = key.map { |obj| "#{obj.class.model_name.to_s.underscore}:#{obj.id}" }.join(":")
+      "action_rate_limiter:#{action}:#{serialized}"
+    end
   end
 end
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index ffc1259eb8f..966ffdf6996 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -370,13 +370,27 @@ describe Projects::PipelineSchedulesController do
     set(:user) { create(:user) }
     let(:ref) { 'master' }
 
-    context 'when a developer makes the request' do
+    before do
+      project.add_developer(user)
+
+      sign_in(user)
+    end
+
+    context 'when an anonymous user makes the request' do
       before do
-        project.add_developer(user)
+        sign_out(user)
+      end
 
-        sign_in(user)
+      it 'does not allow pipeline to be executed' do
+        expect(RunPipelineScheduleWorker).not_to receive(:perform_async)
+
+        post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+
+        expect(response).to have_gitlab_http_status(404)
       end
+    end
 
+    context 'when a developer makes the request' do
       it 'executes a new pipeline' do
         expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123')
 
diff --git a/spec/lib/gitlab/action_rate_limiter_spec.rb b/spec/lib/gitlab/action_rate_limiter_spec.rb
index 84661605623..542fc03e555 100644
--- a/spec/lib/gitlab/action_rate_limiter_spec.rb
+++ b/spec/lib/gitlab/action_rate_limiter_spec.rb
@@ -2,8 +2,10 @@ require 'spec_helper'
 
 describe Gitlab::ActionRateLimiter do
   let(:redis) { double('redis') }
-  let(:key) { 'user:1' }
-  let(:cache_key) { "action_rate_limiter:test_action:#{key}" }
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:key) { [user, project] }
+  let(:cache_key) { "action_rate_limiter:test_action:user:#{user.id}:project:#{project.id}" }
 
   subject { described_class.new(action: :test_action, expiry_time: 100) }
 
-- 
cgit v1.2.1