Merge branch 'if-205273-project_auth_periodic_recompute' into 'master'

Periodically recompute project authorizations

See merge request gitlab-org/gitlab!34071

app/services/authorized_project_update/periodic_recalculate_service.rb

+# frozen_string_literal: true
+
+module AuthorizedProjectUpdate
+  class PeriodicRecalculateService
+    BATCH_SIZE = 480
+    DELAY_INTERVAL = 30.seconds.to_i
+
+    def execute
+      # Using this approach (instead of eg. User.each_batch) keeps the arguments
+      # the same for AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker
+      # even if the user list changes, so we can deduplicate these jobs.
+      (1..User.maximum(:id)).each_slice(BATCH_SIZE).with_index do |batch, index|
+        delay = DELAY_INTERVAL * index
+        AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker.perform_in(delay, *batch.minmax)
+      end
+    end
+  end
+end

app/services/authorized_project_update/recalculate_for_user_range_service.rb

+# frozen_string_literal: true
+
+module AuthorizedProjectUpdate
+  class RecalculateForUserRangeService
+    def initialize(start_user_id, end_user_id)
+      @start_user_id = start_user_id
+      @end_user_id = end_user_id
+    end
+
+    def execute
+      User.where(id: start_user_id..end_user_id).select(:id).find_each do |user| # rubocop: disable CodeReuse/ActiveRecord
+        Users::RefreshAuthorizedProjectsService.new(user).execute
+      end
+    end
+
+    private
+
+    attr_reader :start_user_id, :end_user_id
+  end
+end

app/services/users/refresh_authorized_projects_service.rb

@@ -85,8 +85,6 @@ module Users
     # remove - The IDs of the authorization rows to remove.
     # add - Rows to insert in the form `[user id, project id, access level]`
     def update_authorizations(remove = [], add = [])
-      return if remove.empty? && add.empty?
-
       User.transaction do
         user.remove_project_authorizations(remove) unless remove.empty?
         ProjectAuthorization.insert_authorizations(add) unless add.empty?

app/workers/all_queues.yml

@@ -19,6 +19,14 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: authorized_project_update:authorized_project_update_user_refresh_over_user_range
+  :feature_category: :authentication_and_authorization
+  :has_external_dependencies: 
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: authorized_project_update:authorized_project_update_user_refresh_with_low_urgency
   :feature_category: :authentication_and_authorization
   :has_external_dependencies: 
@@ -107,6 +115,14 @@
   :weight: 1
   :idempotent: 
   :tags: []
+- :name: cronjob:authorized_project_update_periodic_recalculate
+  :feature_category: :source_code_management
+  :has_external_dependencies: 
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: cronjob:ci_archive_traces_cron
   :feature_category: :continuous_integration
   :has_external_dependencies: 

app/workers/authorized_project_update/periodic_recalculate_worker.rb

+# frozen_string_literal: true
+
+module AuthorizedProjectUpdate
+  class PeriodicRecalculateWorker
+    include ApplicationWorker
+    # This worker does not perform work scoped to a context
+    include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
+
+    feature_category :source_code_management
+    urgency :low
+
+    idempotent!
+
+    def perform
+      if ::Feature.enabled?(:periodic_project_authorization_recalculation, default_enabled: true)
+        AuthorizedProjectUpdate::PeriodicRecalculateService.new.execute
+      end
+    end
+  end
+end

app/workers/authorized_project_update/user_refresh_over_user_range_worker.rb

+# frozen_string_literal: true
+
+module AuthorizedProjectUpdate
+  class UserRefreshOverUserRangeWorker
+    include ApplicationWorker
+
+    feature_category :authentication_and_authorization
+    urgency :low
+    queue_namespace :authorized_project_update
+    deduplicate :until_executing, including_scheduled: true
+
+    idempotent!
+
+    def perform(start_user_id, end_user_id)
+      if ::Feature.enabled?(:periodic_project_authorization_recalculation, default_enabled: true)
+        AuthorizedProjectUpdate::RecalculateForUserRangeService.new(start_user_id, end_user_id).execute
+      end
+    end
+  end
+end

changelogs/unreleased/205273-project_auth_periodic_recompute.yml

+---
+title: Periodically recompute project authorizations
+merge_request: 34071
+author:
+type: added

config/initializers/1_settings.rb

@@ -499,6 +499,9 @@ Settings.cron_jobs['x509_issuer_crl_check_worker']['job_class'] = 'X509IssuerCrl
 Settings.cron_jobs['users_create_statistics_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['users_create_statistics_worker']['cron'] ||= '2 15 * * *'
 Settings.cron_jobs['users_create_statistics_worker']['job_class'] = 'Users::CreateStatisticsWorker'
+Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker']['cron'] ||= '45 1 * * 6'
+Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker']['job_class'] = 'AuthorizedProjectUpdate::PeriodicRecalculateWorker'
 
 Gitlab.ee do
   Settings.cron_jobs['adjourned_group_deletion_worker'] ||= Settingslogic.new({})

spec/services/authorized_project_update/periodic_recalculate_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuthorizedProjectUpdate::PeriodicRecalculateService do
+  subject(:service) { described_class.new }
+
+  describe '#execute' do
+    let(:batch_size) { 2 }
+
+    let_it_be(:users) { create_list(:user, 4) }
+
+    before do
+      stub_const('AuthorizedProjectUpdate::PeriodicRecalculateService::BATCH_SIZE', batch_size)
+
+      User.delete([users[1], users[2]])
+    end
+
+    it 'calls AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker' do
+      (1..User.maximum(:id)).each_slice(batch_size).with_index do |batch, index|
+        delay = AuthorizedProjectUpdate::PeriodicRecalculateService::DELAY_INTERVAL * index
+
+        expect(AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker).to(
+          receive(:perform_in).with(delay, *batch.minmax))
+      end
+
+      service.execute
+    end
+  end
+end

spec/services/authorized_project_update/recalculate_for_user_range_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuthorizedProjectUpdate::RecalculateForUserRangeService do
+  describe '#execute' do
+    let_it_be(:users) { create_list(:user, 2) }
+
+    it 'calls Users::RefreshAuthorizedProjectsService' do
+      users.each do |user|
+        expect(Users::RefreshAuthorizedProjectsService).to(
+          receive(:new).with(user).and_call_original)
+      end
+
+      range = users.map(&:id).minmax
+      described_class.new(*range).execute
+    end
+  end
+end

spec/workers/authorized_project_update/periodic_recalculate_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuthorizedProjectUpdate::PeriodicRecalculateWorker do
+  describe '#perform' do
+    it 'calls AuthorizedProjectUpdate::PeriodicRecalculateService' do
+      expect_next_instance_of(AuthorizedProjectUpdate::PeriodicRecalculateService) do |service|
+        expect(service).to receive(:execute)
+      end
+
+      subject.perform
+    end
+
+    context 'feature flag :periodic_project_authorization_recalculation is disabled' do
+      before do
+        stub_feature_flags(periodic_project_authorization_recalculation: false)
+      end
+
+      it 'does not call AuthorizedProjectUpdate::PeriodicRecalculateService' do
+        expect(AuthorizedProjectUpdate::PeriodicRecalculateService).not_to receive(:new)
+
+        subject.perform
+      end
+    end
+  end
+end

spec/workers/authorized_project_update/user_refresh_over_user_range_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuthorizedProjectUpdate::UserRefreshOverUserRangeWorker do
+  let(:start_user_id) { 42 }
+  let(:end_user_id) { 4242 }
+
+  describe '#perform' do
+    it 'calls AuthorizedProjectUpdate::RecalculateForUserRangeService' do
+      expect_next_instance_of(AuthorizedProjectUpdate::RecalculateForUserRangeService) do |service|
+        expect(service).to receive(:execute)
+      end
+
+      subject.perform(start_user_id, end_user_id)
+    end
+
+    context 'feature flag :periodic_project_authorization_recalculation is disabled' do
+      before do
+        stub_feature_flags(periodic_project_authorization_recalculation: false)
+      end
+
+      it 'does not call AuthorizedProjectUpdate::RecalculateForUserRangeService' do
+        expect(AuthorizedProjectUpdate::RecalculateForUserRangeService).not_to receive(:new)
+
+        subject.perform(start_user_id, end_user_id)
+      end
+    end
+  end
+end