Merge branch 'async-ldap-2' into 'master'

Update LDAP group links asynchronously



See merge request !190

app/workers/ldap_group_links_worker.rb

+class LdapGroupLinksWorker
+  include Sidekiq::Worker
+
+  def perform(user_id)
+    user = User.find(user_id)
+    logger.info "Updating LDAP group memberships for user #{user.id} (#{user.email})"
+    access = Gitlab::LDAP::Access.new(user)
+    access.update_ldap_group_links
+  end
+end

app/workers/ldap_sync_worker.rb

@@ -8,7 +8,9 @@ class LdapSyncWorker
     Rails.logger.info "Performing daily LDAP sync task."
     User.ldap.find_each(batch_size: 100).each do |ldap_user|
       Rails.logger.debug "Syncing user #{ldap_user.username}, #{ldap_user.email}"
-      Gitlab::LDAP::Access.allowed?(ldap_user)
+      # Use the 'update_ldap_group_links_synchronously' option to avoid creating a ton
+      # of new Sidekiq jobs all at once.
+      Gitlab::LDAP::Access.allowed?(ldap_user, update_ldap_group_links_synchronously: true)
     end
   end
 end

lib/gitlab/ldap/access.rb

@@ -14,11 +14,11 @@ module Gitlab
         end
       end
 
-      def self.allowed?(user)
+      def self.allowed?(user, options={})
         self.open(user) do |access|
           # Whether user is allowed, or not, we should update
           # permissions to keep things clean
-          access.update_permissions
+          access.update_permissions(options)
           if access.allowed?
             access.update_user
             user.last_credential_check_at = Time.now
@@ -75,8 +75,14 @@ module Gitlab
         update_kerberos_identity if import_kerberos_identities?
       end
 
-      def update_permissions
-        update_ldap_group_links if group_base.present?
+      def update_permissions(options)
+        if group_base.present?
+          if options[:update_ldap_group_links_synchronously]
+            update_ldap_group_links
+          else
+            LdapGroupLinksWorker.perform_async(user.id)
+          end
+        end
         update_admin_status if admin_group.present?
       end
 

spec/lib/gitlab/ldap/access_spec.rb

@@ -117,11 +117,12 @@ describe Gitlab::LDAP::Access, lib: true do
   end
 
   describe '#update_permissions' do
-    subject { access.update_permissions }
+    subject { access.update_permissions({}) }
 
     it 'does update group permissions with a group base configured' do
       allow(access).to receive_messages(group_base: 'my-group-base')
-      expect(access).to receive(:update_ldap_group_links)
+      expect(access).not_to receive(:update_ldap_group_links)
+      expect(LdapGroupLinksWorker).to receive(:perform_async).with(user.id)
 
       subject
     end
@@ -129,6 +130,7 @@ describe Gitlab::LDAP::Access, lib: true do
     it 'does not update group permissions without a group base configured' do
       allow(access).to receive_messages(group_base: '')
       expect(access).not_to receive(:update_ldap_group_links)
+      expect(LdapGroupLinksWorker).not_to receive(:perform_async)
 
       subject
     end
@@ -146,6 +148,16 @@ describe Gitlab::LDAP::Access, lib: true do
 
       subject
     end
+
+    context 'when synchronously updating group permissions' do
+      it 'updates group permissions directly' do
+        allow(access).to receive_messages(group_base: 'my-group-base')
+        expect(LdapGroupLinksWorker).not_to receive(:perform_async)
+        expect(access).to receive(:update_ldap_group_links)
+  
+        access.update_permissions(update_ldap_group_links_synchronously: true)
+      end      
+    end
   end
 
   describe :update_kerberos_identity do