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

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

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)

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

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)
-
-      if @subject.tag?
-        !access.can_create_tag?(@subject.ref)
-      else
-        !access.can_update_branch?(@subject.ref)
-      end
-    end
+    condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.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?(ref)
+      end
+    end
   end
 end

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

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)

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

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

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

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

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) }
 

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

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'] }
 

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

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