Merge branch 'ci-disable-scheduled-pipelines-323341' into 'master'

Disable pipeline schedules when a user is blocked

See merge request gitlab-org/gitlab!56513

app/models/ci/pipeline_schedule.rb

@@ -7,6 +7,7 @@ module Ci
     include StripAttribute
     include Schedulable
     include Limitable
+    include EachBatch
 
     self.limit_name = 'ci_pipeline_schedules'
     self.limit_scope = :project
@@ -28,6 +29,7 @@ module Ci
     scope :active, -> { where(active: true) }
     scope :inactive, -> { where(active: false) }
     scope :preloaded, -> { preload(:owner, project: [:route]) }
+    scope :owned_by, ->(user) { where(owner: user) }
 
     accepts_nested_attributes_for :variables, allow_destroy: true
 

app/models/user.rb

@@ -351,6 +351,7 @@ class User < ApplicationRecord
     # For this reason the tradeoff is to disable this cop.
     after_transition any => :blocked do |user|
       Ci::CancelUserPipelinesService.new.execute(user)
+      Ci::DisableUserPipelineSchedulesService.new.execute(user)
     end
     # rubocop: enable CodeReuse/ServiceClass
   end

app/services/ci/disable_user_pipeline_schedules_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class DisableUserPipelineSchedulesService
+    def execute(user)
+      Ci::PipelineSchedule.active.owned_by(user).each_batch do |relation|
+        relation.update_all(active: false)
+      end
+    end
+  end
+end

changelogs/unreleased/ci-disable-scheduled-pipelines-323341.yml

+---
+title: Disable pipeline schedules when a user is blocked
+merge_request: 56513
+author:
+type: changed

db/migrate/20210312140029_add_owner_and_id_index_on_active_ci_pipeline_schedules.rb

+# frozen_string_literal: true
+
+class AddOwnerAndIdIndexOnActiveCiPipelineSchedules < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_ci_pipeline_schedules_on_owner_id_and_id_and_active'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_pipeline_schedules, [:owner_id, :id], where: "active = TRUE", name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :ci_pipeline_schedules, INDEX_NAME
+  end
+end

db/schema_migrations/20210312140029

+5d63a48f4a9327f683eff093d2862a0b88aa4249c94b2de9751ed6172c9b4799
\ No newline at end of file

db/structure.sql

@@ -22015,6 +22015,8 @@ CREATE INDEX index_ci_pipeline_schedules_on_next_run_at_and_active ON ci_pipelin
 
 CREATE INDEX index_ci_pipeline_schedules_on_owner_id ON ci_pipeline_schedules USING btree (owner_id);
 
+CREATE INDEX index_ci_pipeline_schedules_on_owner_id_and_id_and_active ON ci_pipeline_schedules USING btree (owner_id, id) WHERE (active = true);
+
 CREATE INDEX index_ci_pipeline_schedules_on_project_id ON ci_pipeline_schedules USING btree (project_id);
 
 CREATE UNIQUE INDEX index_ci_pipeline_variables_on_pipeline_id_and_key ON ci_pipeline_variables USING btree (pipeline_id, key);

spec/features/projects/pipelines/pipeline_spec.rb

@@ -709,9 +709,9 @@ RSpec.describe 'Pipeline', :js do
           end
         end
 
-        it 'displays the PipelineSchedule in an active state' do
+        it 'displays the PipelineSchedule in an inactive state' do
           visit project_pipeline_schedules_path(project)
-          page.click_link('Active')
+          page.click_link('Inactive')
 
           expect(page).to have_selector('table.ci-table > tbody > tr > td', text: 'blocked user schedule')
         end

spec/models/ci/pipeline_schedule_spec.rb

@@ -90,6 +90,18 @@ RSpec.describe Ci::PipelineSchedule do
     end
   end
 
+  describe '.owned_by' do
+    let(:user) { create(:user) }
+    let!(:owned_pipeline_schedule) { create(:ci_pipeline_schedule, owner: user) }
+    let!(:other_pipeline_schedule) { create(:ci_pipeline_schedule) }
+
+    subject { described_class.owned_by(user) }
+
+    it 'returns owned pipeline schedules' do
+      is_expected.to eq([owned_pipeline_schedule])
+    end
+  end
+
   describe '#set_next_run_at' do
     let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly) }
     let(:ideal_next_run_at) { pipeline_schedule.send(:ideal_next_run_from, Time.zone.now) }

spec/models/user_spec.rb

@@ -1766,7 +1766,7 @@ RSpec.describe User do
   end
 
   describe 'blocking user' do
-    let(:user) { create(:user, name: 'John Smith') }
+    let_it_be_with_refind(:user) { create(:user, name: 'John Smith') }
 
     it 'blocks user' do
       user.block
@@ -1789,6 +1789,14 @@ RSpec.describe User do
         user.block
       end
     end
+
+    context 'when user has active CI pipeline schedules' do
+      let_it_be(:schedule) { create(:ci_pipeline_schedule, active: true, owner: user) }
+
+      it 'disables any pipeline schedules' do
+        expect { user.block }.to change { schedule.reload.active? }.to(false)
+      end
+    end
   end
 
   describe 'deactivating a user' do

spec/services/ci/disable_user_pipeline_schedules_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::DisableUserPipelineSchedulesService do
+  describe '#execute' do
+    let(:user) { create(:user) }
+
+    subject(:service) { described_class.new.execute(user) }
+
+    context 'when user has active pipeline schedules' do
+      let(:owned_pipeline_schedule) { create(:ci_pipeline_schedule, active: true, owner: user) }
+
+      it 'disables all active pipeline schedules', :aggregate_failures do
+        expect { service }.to change { owned_pipeline_schedule.reload.active? }
+      end
+    end
+  end
+end