Merge branch 'mrchrisw-fix-ldap-sync' into 'master'

Fix ldap_sync_worker_cron and geo_bulk_notify_worker_cron add ldap_group_sync_worker

See merge request !1535

files/gitlab-config-template/gitlab.rb.template

@@ -1379,6 +1379,7 @@ external_url 'GENERATED_EXTERNAL_URL'
 # gitlab_rails['geo_backfill_worker_cron'] = "*/5 * * * *"
 # gitlab_rails['geo_download_dispatch_worker_cron'] = "*/10 * * * *"
 # gitlab_rails['ldap_sync_worker_cron'] = "30 1 * * *"
+# gitlab_rails['ldap_group_sync_worker_cron'] = "0 * * * *"
 # gitlab_rails['historical_data_worker_cron'] = "0 12 * * *"
 
 ################################################################################

files/gitlab-cookbooks/gitlab/attributes/default.rb

@@ -109,6 +109,7 @@ default['gitlab']['gitlab-rails']['admin_email_worker_cron'] = nil
 default['gitlab']['gitlab-rails']['repository_archive_cache_worker_cron'] = nil
 default['gitlab']['gitlab-rails']['historical_data_worker_cron'] = nil
 default['gitlab']['gitlab-rails']['ldap_sync_worker_cron'] = nil
+default['gitlab']['gitlab-rails']['ldap_group_sync_worker_cron'] = nil
 default['gitlab']['gitlab-rails']['geo_bulk_notify_worker_cron'] = nil
 default['gitlab']['gitlab-rails']['geo_backfill_worker_cron'] = nil
 default['gitlab']['gitlab-rails']['geo_download_dispatch_worker_cron'] = nil

files/gitlab-cookbooks/gitlab/templates/default/gitlab.yml.erb

@@ -196,14 +196,21 @@ production: &base
     # In addition to refreshing users when they log in,
     # periodically refresh LDAP users membership.
     # NOTE: This will only take effect if LDAP is enabled
-    <% unless @ldap_sync_worker.nil? %>
+    <% unless @ldap_sync_worker_cron.nil? %>
     ldap_sync_worker:
       cron: "<%= @ldap_sync_worker_cron %>"
     <% end %>
 
+    # GitLab LDAP group sync worker
+    # NOTE: This will only take effect if LDAP is enabled
+    <% unless @ldap_group_sync_worker_cron.nil? %>
+    ldap_group_sync_worker:
+      cron: "<%= @ldap_group_sync_worker_cron %>"
+    <% end %>
+
     # Gitlab Geo nodes notification worker
     # NOTE: This will only take effect if Geo is enabled
-    <% unless @geo_bulk_notify_worker.nil? %>
+    <% unless @geo_bulk_notify_worker_cron.nil? %>
     geo_bulk_notify_worker:
       cron: "<%= @geo_bulk_notify_worker_cron %>"
     <% end %>

spec/chef/recipes/gitlab-rails_spec.rb

@@ -291,6 +291,42 @@ describe 'gitlab::gitlab-rails' do
         expect(Gitlab['gitlab_rails']['stuck_ci_jobs_worker_cron']).to eq('0 1 2 * *')
       end
     end
+
+    context 'GitLab LDAP cron_jobs settings' do
+      context 'when ldap user sync worker is configured' do
+        it 'sets the cron value' do
+          stub_gitlab_rb(gitlab_rails: { ldap_sync_worker_cron: '40 2 * * *' })
+
+          expect(chef_run).to render_file(gitlab_yml_path)
+            .with_content(/ldap_sync_worker:\s+cron:\s+"40 2 \* \* \*"/)
+        end
+      end
+
+      context 'when ldap user sync worker is not configured' do
+        it 'does not set the cron value' do
+          expect(chef_run).to render_file(gitlab_yml_path).with_content { |content|
+            expect(content).not_to include('ldap_sync_worker')
+          }
+        end
+      end
+
+      context 'when ldap group sync worker is configured' do
+        it 'sets the cron value' do
+          stub_gitlab_rb(gitlab_rails: { ldap_group_sync_worker_cron: '1 0 * * *' })
+
+          expect(chef_run).to render_file(gitlab_yml_path)
+            .with_content(/ldap_group_sync_worker:\s+cron:\s+"1 0 \* \* \*"/)
+        end
+      end
+
+      context 'when ldap group sync worker is not configured' do
+        it 'does not set the cron value' do
+          expect(chef_run).to render_file(gitlab_yml_path).with_content { |content|
+            expect(content).not_to include('ldap_group_sync_worker')
+          }
+        end
+      end
+    end
   end
 
   context 'with environment variables' do