Allow sidekiq-cluster to read YAML files with hashes

Previously, we only supported an array of strings. This allows an array
of hashes, where each hash has a `name` value.

ee/spec/lib/gitlab/sidekiq_config_spec.rb

@@ -58,23 +58,23 @@ describe Gitlab::SidekiqConfig do
 
       allow(described_class).to receive(:workers).and_return(workers)
 
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::FOSS_QUEUE_CONFIG_PATH)
-                       .and_return(YAML.dump([]))
+                       .and_return([])
     end
 
     it 'returns true if the YAML file does not match the application code' do
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::EE_QUEUE_CONFIG_PATH)
-                       .and_return(YAML.dump(['ldap_group_sync']))
+                       .and_return(['ldap_group_sync'])
 
       expect(described_class.all_queues_yml_outdated?).to be(true)
     end
 
     it 'returns false if the YAML file matches the application code' do
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::EE_QUEUE_CONFIG_PATH)
-                       .and_return(YAML.dump(%w[ldap_group_sync repository_update_mirror]))
+                       .and_return(%w[ldap_group_sync repository_update_mirror])
 
       expect(described_class.all_queues_yml_outdated?).to be(false)
     end

lib/gitlab/sidekiq_config.rb

@@ -66,12 +66,13 @@ module Gitlab
         workers.partition(&:ee?).reverse.map(&:sort)
       end
 
+      # YAML.load_file is OK here as we control the file contents
       def all_queues_yml_outdated?
         foss_workers, ee_workers = workers_for_all_queues_yml
 
-        return true if foss_workers != YAML.safe_load(File.read(FOSS_QUEUE_CONFIG_PATH))
+        return true if foss_workers != YAML.load_file(FOSS_QUEUE_CONFIG_PATH)
 
-        Gitlab.ee? && ee_workers != YAML.safe_load(File.read(EE_QUEUE_CONFIG_PATH))
+        Gitlab.ee? && ee_workers != YAML.load_file(EE_QUEUE_CONFIG_PATH)
       end
 
       def queues_for_sidekiq_queues_yml
@@ -89,9 +90,9 @@ module Gitlab
          remaining_queues.map(&:queue_and_weight)).sort
       end
 
+      # YAML.load_file is OK here as we control the file contents
       def sidekiq_queues_yml_outdated?
-        # YAML.load is OK here as we control the file contents
-        config_queues = YAML.load(File.read(SIDEKIQ_QUEUES_PATH))[:queues] # rubocop:disable Security/YAMLLoad
+        config_queues = YAML.load_file(SIDEKIQ_QUEUES_PATH)[:queues]
 
         queues_for_sidekiq_queues_yml != config_queues
       end

lib/gitlab/sidekiq_config/cli_methods.rb

@@ -23,8 +23,9 @@ module Gitlab
 
         @worker_queues[rails_path] ||= QUEUE_CONFIG_PATHS.flat_map do |path|
           full_path = File.join(rails_path, path)
+          queues = File.exist?(full_path) ? YAML.load_file(full_path) : []
 
-          File.exist?(full_path) ? YAML.load_file(full_path) : []
+          queues.map { |queue| queue.is_a?(Hash) ? queue[:name] : queue }
         end
       end
 

spec/lib/gitlab/sidekiq_config/cli_methods_spec.rb

@@ -33,20 +33,44 @@ describe Gitlab::SidekiqConfig::CliMethods do
     context 'when the file exists' do
       before do
         stub_exists(exists: true)
-        stub_contents(['queue_a'], ['queue_b'])
       end
 
-      it 'memoizes the result' do
-        result = described_class.worker_queues(dummy_root)
+      context 'when the file contains an array of strings' do
+        before do
+          stub_contents(['queue_a'], ['queue_b'])
+        end
 
-        stub_exists(exists: false)
+        it 'memoizes the result' do
+          result = described_class.worker_queues(dummy_root)
+
+          stub_exists(exists: false)
 
-        expect(described_class.worker_queues(dummy_root)).to eq(result)
+          expect(described_class.worker_queues(dummy_root)).to eq(result)
+        end
+
+        it 'flattens and joins the contents' do
+          expect(described_class.worker_queues(dummy_root))
+            .to contain_exactly('queue_a', 'queue_b')
+        end
       end
 
-      it 'flattens and joins the contents' do
-        expect(described_class.worker_queues(dummy_root))
-          .to contain_exactly('queue_a', 'queue_b')
+      context 'when the file contains an array of hashes' do
+        before do
+          stub_contents([{ name: 'queue_a' }], [{ name: 'queue_b' }])
+        end
+
+        it 'memoizes the result' do
+          result = described_class.worker_queues(dummy_root)
+
+          stub_exists(exists: false)
+
+          expect(described_class.worker_queues(dummy_root)).to eq(result)
+        end
+
+        it 'flattens and joins the values of the name field' do
+          expect(described_class.worker_queues(dummy_root))
+            .to contain_exactly('queue_a', 'queue_b')
+        end
       end
     end
 

spec/lib/gitlab/sidekiq_config_spec.rb

@@ -44,17 +44,17 @@ describe Gitlab::SidekiqConfig do
     end
 
     it 'returns true if the YAML file does not match the application code' do
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::FOSS_QUEUE_CONFIG_PATH)
-                       .and_return(YAML.dump(%w[post_receive merge]))
+                       .and_return(%w[post_receive merge])
 
       expect(described_class.all_queues_yml_outdated?).to be(true)
     end
 
     it 'returns false if the YAML file matches the application code' do
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::FOSS_QUEUE_CONFIG_PATH)
-                       .and_return(YAML.dump(%w[merge post_receive process_commit]))
+                       .and_return(%w[merge post_receive process_commit])
 
       expect(described_class.all_queues_yml_outdated?).to be(false)
     end
@@ -104,17 +104,17 @@ describe Gitlab::SidekiqConfig do
     end
 
     it 'returns true if the YAML file does not match the application code' do
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::SIDEKIQ_QUEUES_PATH)
-                       .and_return(YAML.dump(queues: expected_queues.reverse))
+                       .and_return(queues: expected_queues.reverse)
 
       expect(described_class.sidekiq_queues_yml_outdated?).to be(true)
     end
 
     it 'returns false if the YAML file matches the application code' do
-      allow(File).to receive(:read)
+      allow(YAML).to receive(:load_file)
                        .with(described_class::SIDEKIQ_QUEUES_PATH)
-                       .and_return(YAML.dump(queues: expected_queues))
+                       .and_return(queues: expected_queues)
 
       expect(described_class.sidekiq_queues_yml_outdated?).to be(false)
     end