Merge branch '2294-cron-quotes' into 'master'

Fix gitlab.yml template to quote sidekiq-cron properly

Closes #2294

See merge request !1526

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

@@ -165,24 +165,24 @@ production: &base
   cron_jobs:
     # Flag stuck CI builds as failed
     stuck_ci_jobs_worker:
-      cron: <%= @stuck_ci_jobs_worker_cron %>
+      cron: "<%= @stuck_ci_jobs_worker_cron %>"
     # Remove expired build artifacts
     expire_build_artifacts_worker:
-      cron: <%= @expire_build_artifacts_worker_cron %>
+      cron: "<%= @expire_build_artifacts_worker_cron %>"
     # Schedule pipelines in the near future
     trigger_schedule_worker:
-      cron: <%= @trigger_schedule_worker_cron %>
+      cron: "<%= @trigger_schedule_worker_cron %>"
     # Periodically run 'git fsck' on all repositories. If started more than
     # once per hour you will have concurrent 'git fsck' jobs.
     repository_check_worker:
-      cron: <%= @repository_check_worker_cron %>
+      cron: "<%= @repository_check_worker_cron %>"
     # Send admin emails once a week
     admin_email_worker:
-      cron: <%= @admin_email_worker_cron %>
+      cron: "<%= @admin_email_worker_cron %>"
     # Remove outdated repository archives
     repository_archive_cache_worker:
-      cron: <%= @repository_archive_cache_worker_cron %>
+      cron: "<%= @repository_archive_cache_worker_cron %>"
     ##
     # GitLab EE only jobs:
@@ -190,7 +190,7 @@ production: &base
     # Snapshot active users statistics
     <% unless @historical_data_worker_cron.nil? %>
     historical_data_worker:
-      cron: <%= @historical_data_worker_cron %>
+      cron: "<%= @historical_data_worker_cron %>"
     <% end %>
     # In addition to refreshing users when they log in,
@@ -198,28 +198,28 @@ production: &base
     # NOTE: This will only take effect if LDAP is enabled
     <% unless @ldap_sync_worker.nil? %>
     ldap_sync_worker:
-      cron: <%= @ldap_sync_worker_cron %>
+      cron: "<%= @ldap_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? %>
     geo_bulk_notify_worker:
-      cron: <%= @geo_bulk_notify_worker_cron %>
+      cron: "<%= @geo_bulk_notify_worker_cron %>"
     <% end %>
     # GitLab Geo backfill worker
     # NOTE: This will only take effect if Geo is enabled
     <% unless @geo_backfill_worker_cron.nil? %>
     geo_backfill_worker:
-      cron: <%= @geo_backfill_worker_cron %>
+      cron: "<%= @geo_backfill_worker_cron %>"
     <% end %>
     # GitLab Geo file download worker
     # NOTE: This will only take effect if Geo is enabled
     <% unless @geo_download_dispatch_worker_cron.nil? %>
     geo_download_dispatch_worker:
-      cron: <%= @geo_download_dispatch_worker_cron %>
+      cron: "<%= @geo_download_dispatch_worker_cron %>"
     <% end %>
   #

spec/chef/recipes/gitlab-rails_spec.rb

@@ -9,8 +9,15 @@ describe 'gitlab::gitlab-rails' do
   end
   context 'when manage-storage-directories is disabled' do
-    before do
-      stub_gitlab_rb(gitlab_rails: { shared_path: '/tmp/shared' }, manage_storage_directories: { enable: false })
+    cached(:chef_run) do
+      RSpec::Mocks.with_temporary_scope do
+        stub_gitlab_rb(gitlab_rails: { shared_path: '/tmp/shared',
+                                       uploads_directory: '/tmp/uploads',
+                                       builds_directory: '/tmp/builds' },
+                       manage_storage_directories: { enable: false })
+      end
+
+      ChefSpec::SoloRunner.new(step_into: %w(templatesymlink)).converge('gitlab::default')
     end
     it 'does not create the shared directory' do
@@ -26,12 +33,10 @@ describe 'gitlab::gitlab-rails' do
     end
     it 'does not create the uploads storage directory' do
-      stub_gitlab_rb(gitlab_rails: { uploads_directory: '/tmp/uploads' })
       expect(chef_run).not_to run_ruby_block('directory resource: /tmp/uploads')
     end
     it 'does not create the ci builds directory' do
-      stub_gitlab_rb(gitlab_ci: { builds_directory: '/tmp/builds' })
       expect(chef_run).not_to run_ruby_block('directory resource: /tmp/builds')
     end
@@ -41,8 +46,14 @@ describe 'gitlab::gitlab-rails' do
   end
   context 'when manage-storage-directories is enabled' do
-    before do
-      stub_gitlab_rb(gitlab_rails: { shared_path: '/tmp/shared' })
+    cached(:chef_run) do
+      RSpec::Mocks.with_temporary_scope do
+        stub_gitlab_rb(gitlab_rails: { shared_path: '/tmp/shared',
+                                       uploads_directory: '/tmp/uploads' },
+                       gitlab_ci: { builds_directory: '/tmp/builds' })
+      end
+
+      ChefSpec::SoloRunner.new(step_into: %w(templatesymlink)).converge('gitlab::default')
     end
     it 'creates the shared directory' do
@@ -58,12 +69,10 @@ describe 'gitlab::gitlab-rails' do
     end
     it 'creates the uploads directory' do
-      stub_gitlab_rb(gitlab_rails: { uploads_directory: '/tmp/uploads' })
       expect(chef_run).to run_ruby_block('directory resource: /tmp/uploads')
     end
     it 'creates the ci builds directory' do
-      stub_gitlab_rb(gitlab_ci: { builds_directory: '/tmp/builds' })
       expect(chef_run).to run_ruby_block('directory resource: /tmp/builds')
     end
@@ -169,7 +178,7 @@ describe 'gitlab::gitlab-rails' do
           stub_gitlab_rb(gitlab_rails: { geo_backfill_worker_cron: '1 2 3 4 5' })
           expect(chef_run).to render_file(gitlab_yml_path)
-            .with_content(/geo_backfill_worker:\s+cron:\s+1 2 3 4 5/)
+            .with_content(/geo_backfill_worker:\s+cron:\s+"1 2 3 4 5"/)
         end
       end
@@ -186,7 +195,7 @@ describe 'gitlab::gitlab-rails' do
           stub_gitlab_rb(gitlab_rails: { geo_download_dispatch_worker_cron: '1 2 3 4 5' })
           expect(chef_run).to render_file(gitlab_yml_path)
-            .with_content(/geo_download_dispatch_worker:\s+cron:\s+1 2 3 4 5/)
+            .with_content(/geo_download_dispatch_worker:\s+cron:\s+"1 2 3 4 5"/)
         end
       end
@@ -205,7 +214,7 @@ describe 'gitlab::gitlab-rails' do
           stub_gitlab_rb(gitlab_rails: { trigger_schedule_worker_cron: '1 2 3 4 5' })
           expect(chef_run).to render_file(gitlab_yml_path)
-            .with_content(/trigger_schedule_worker:\s+cron:\s+1 2 3 4 5/)
+            .with_content(/trigger_schedule_worker:\s+cron:\s+"1 2 3 4 5"/)
         end
       end
@@ -342,6 +351,10 @@ describe 'gitlab::gitlab-rails' do
       let(:templatesymlink_link) { chef_run.link("Link /opt/gitlab/embedded/service/gitlab-rails/config/database.yml to /var/opt/gitlab/gitlab-rails/etc/database.yml") }
       context 'by default' do
+        cached(:chef_run) do
+          ChefSpec::SoloRunner.new(step_into: %w(templatesymlink)).converge('gitlab::default')
+        end
+
         it 'creates the template' do
           expect(chef_run).to create_template('/var/opt/gitlab/gitlab-rails/etc/database.yml')
             .with(
@@ -395,8 +408,12 @@ describe 'gitlab::gitlab-rails' do
         end
         context 'when one postgresql listen_address is used' do
-          before do
-            stub_gitlab_rb(postgresql: { listen_address: "127.0.0.1" })
+          cached(:chef_run) do
+            RSpec::Mocks.with_temporary_scope do
+              stub_gitlab_rb(postgresql: { listen_address: "127.0.0.1" })
+            end
+
+            ChefSpec::SoloRunner.new(step_into: %w(templatesymlink)).converge('gitlab::default')
           end
           it 'creates the postgres configuration file with one listen_address and database.yml file with one host' do
@@ -462,6 +479,10 @@ describe 'gitlab::gitlab-rails' do
       let(:templatesymlink_link) { chef_run.link("Link /opt/gitlab/embedded/service/gitlab-rails/.gitlab_workhorse_secret to /var/opt/gitlab/gitlab-rails/etc/gitlab_workhorse_secret") }
       context 'by default' do
+        cached(:chef_run) do
+          ChefSpec::SoloRunner.new(step_into: %w(templatesymlink)).converge('gitlab::default')
+        end
+
         it 'creates the template' do
           expect(chef_run).to create_template('/var/opt/gitlab/gitlab-rails/etc/gitlab_workhorse_secret')
             .with(
@@ -489,8 +510,12 @@ describe 'gitlab::gitlab-rails' do
       end
       context 'with specific gitlab_workhorse_secret' do
-        before do
-          stub_gitlab_rb(gitlab_workhorse: { secret_token: 'abc123-gitlab-workhorse' })
+        cached(:chef_run) do
+          RSpec::Mocks.with_temporary_scope do
+            stub_gitlab_rb(gitlab_workhorse: { secret_token: 'abc123-gitlab-workhorse' })
+          end
+
+          ChefSpec::SoloRunner.new(step_into: %w(templatesymlink)).converge('gitlab::default')
         end
         it 'renders the correct node attribute' do