summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2017-12-18 13:08:51 +0000
committerKamil Trzciński <ayufan@ayufan.eu>2017-12-18 13:08:51 +0000
commit924e00496c7d16887447b9384b66b555e5a9f911 (patch)
treeddf51b4cef3aa68eef7450a4c00eb28084f80eb9
parent38dd7263e15e84c08b62c97028e943c759384b94 (diff)
parent4b0465f20de1bf58326c7daf6876b63438f00d84 (diff)
downloadgitlab-ce-924e00496c7d16887447b9384b66b555e5a9f911.tar.gz
Merge branch 'sh-add-schedule-pipeline-run-now' into 'master'
Add button to run scheduled pipeline immediately Closes #38741 See merge request gitlab-org/gitlab-ce!15700
-rw-r--r--app/controllers/projects/pipeline_schedules_controller.rb31
-rw-r--r--app/helpers/gitlab_routing_helper.rb5
-rw-r--r--app/models/repository.rb6
-rw-r--r--app/policies/ci/pipeline_policy.rb16
-rw-r--r--app/policies/ci/pipeline_schedule_policy.rb10
-rw-r--r--app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml4
-rw-r--r--app/workers/run_pipeline_schedule_worker.rb22
-rw-r--r--changelogs/unreleased/sh-add-schedule-pipeline-run-now.yml5
-rw-r--r--config/routes/project.rb1
-rw-r--r--lib/gitlab/action_rate_limiter.rb47
-rw-r--r--spec/controllers/projects/pipeline_schedules_controller_spec.rb67
-rw-r--r--spec/lib/gitlab/action_rate_limiter_spec.rb29
-rw-r--r--spec/models/repository_spec.rb9
-rw-r--r--spec/policies/ci/pipeline_schedule_policy_spec.rb92
-rw-r--r--spec/workers/run_pipeline_schedule_worker_spec.rb39
15 files changed, 371 insertions, 12 deletions
diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index ec7c645df5a..b478e7b5e05 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,9 +1,11 @@
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]
- before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create]
+ before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create, :play]
before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
def index
@@ -40,6 +42,18 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
end
end
+ def play
+ job_id = RunPipelineScheduleWorker.perform_async(schedule.id, current_user.id)
+
+ 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
+
def take_ownership
if schedule.update(owner: current_user)
redirect_to pipeline_schedules_path(@project)
@@ -60,6 +74,17 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
private
+ 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
@schedule ||= project.pipeline_schedules.find(params[:id])
end
@@ -70,6 +95,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/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb
index a77aa0ad2cc..7f3c118c7ab 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 play_pipeline_schedule_path(schedule, *args)
+ project = schedule.project
+ play_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/models/repository.rb b/app/models/repository.rb
index 552a354d1ce..4ec8ec9c8b2 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/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 6b7598e1821..abcf536b2f7 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -2,13 +2,23 @@ module Ci
class PipelineSchedulePolicy < PipelinePolicy
alias_method :pipeline_schedule, :subject
+ condition(:protected_ref) do
+ ref_protected?(@user, @subject.project, @subject.project.repository.tag_exists?(@subject.ref), @subject.ref)
+ 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 bd8c38292d6..f8c4005a9e0 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, :play_pipeline_schedule, pipeline_schedule)
+ = 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
= 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..7725ad319a3
--- /dev/null
+++ b/app/workers/run_pipeline_schedule_worker.rb
@@ -0,0 +1,22 @@
+class RunPipelineScheduleWorker
+ include ApplicationWorker
+ include PipelineQueue
+
+ enqueue_in group: :creation
+
+ def perform(schedule_id, 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
+
+ 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..239b5480321 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 :play
post :take_ownership
end
end
diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb
new file mode 100644
index 00000000000..4cd3bdefda3
--- /dev/null
+++ b/lib/gitlab/action_rate_limiter.rb
@@ -0,0 +1,47 @@
+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
+
+ # 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_key(key)
+ value = redis.incr(cache_key)
+ redis.expire(cache_key, expiry_time) if value == 1
+ end
+
+ 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 4e52e261920..966ffdf6996 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -3,10 +3,12 @@ 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
+ render_views
+
let(:scope) { nil }
let!(:inactive_pipeline_schedule) do
create(:ci_pipeline_schedule, :inactive, project: project)
@@ -96,7 +98,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 +366,65 @@ describe Projects::PipelineSchedulesController do
end
end
+ describe 'POST #play', :clean_gitlab_redis_cache do
+ set(:user) { create(:user) }
+ let(:ref) { 'master' }
+
+ before do
+ project.add_developer(user)
+
+ sign_in(user)
+ end
+
+ context 'when an anonymous user makes the request' do
+ before do
+ sign_out(user)
+ end
+
+ 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')
+
+ post :play, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+
+ 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
+ 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
+
describe 'DELETE #destroy' do
set(:user) { create(:user) }
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..542fc03e555
--- /dev/null
+++ b/spec/lib/gitlab/action_rate_limiter_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe Gitlab::ActionRateLimiter do
+ let(:redis) { double('redis') }
+ 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) }
+
+ 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
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 799d99c0369..bdc430c9095 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1163,6 +1163,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'] }
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
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..481a84837f9
--- /dev/null
+++ b/spec/workers/run_pipeline_schedule_worker_spec.rb
@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+describe RunPipelineScheduleWorker do
+ describe '#perform' do
+ set(:project) { create(:project) }
+ set(:user) { create(:user) }
+ set(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project ) }
+ let(:worker) { described_class.new }
+
+ context 'when a project not found' do
+ it 'does not call the Service' do
+ expect(Ci::CreatePipelineService).not_to receive(:new)
+ 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).not_to receive(:run_pipeline_schedule)
+
+ worker.perform(pipeline_schedule.id, 10000)
+ end
+ end
+
+ context 'when everything is ok' do
+ 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