Add sockets_directory setting

- Do not automatically use sockets directory, allow a user to set the
  directory

Signed-off-by: Robert Marshall <rmarshall@gitlab.com>

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

@@ -609,6 +609,7 @@ default['gitlab']['gitlab-workhorse']['enable'] = false
 default['gitlab']['gitlab-workhorse']['ha'] = false
 default['gitlab']['gitlab-workhorse']['listen_network'] = "unix"
 default['gitlab']['gitlab-workhorse']['listen_umask'] = 000
+default['gitlab']['gitlab-workhorse']['sockets_directory'] = "/var/opt/gitlab/gitlab-workhorse/sockets"
 default['gitlab']['gitlab-workhorse']['listen_addr'] = "/var/opt/gitlab/gitlab-workhorse/sockets/socket"
 default['gitlab']['gitlab-workhorse']['auth_backend'] = "http://localhost:8080"
 default['gitlab']['gitlab-workhorse']['auth_socket'] = nil

files/gitlab-cookbooks/gitlab/libraries/helpers/gitlab_workhorse_helper.rb

@@ -12,10 +12,7 @@ class GitlabWorkhorseHelper < BaseHelper
   def sockets_directory
     return unless unix_socket?
-    path = File.dirname(node['gitlab']['gitlab-workhorse']['listen_addr'])
-    return path if File.basename(path) == 'sockets'
-
-    File.join(path, 'sockets')
+    node['gitlab']['gitlab-workhorse']['sockets_directory']
   end
   def unix_socket?
@@ -26,13 +23,24 @@ class GitlabWorkhorseHelper < BaseHelper
     '/var/opt/gitlab/gitlab-workhorse/socket'
   end
-  def orphan_socket
+  def user_customized_socket?
     default_path = node.default['gitlab']['gitlab-workhorse']['listen_addr']
     configured_path = node['gitlab']['gitlab-workhorse']['listen_addr']
-    return deprecated_socket if default_path == configured_path
-    configured_path
+    default_path != configured_path
+  end
+
+  def user_customized_sockets_directory?
+    default_directory = node.default['gitlab']['gitlab-workhorse']['sockets_directory']
+    configured_directory = node['gitlab']['gitlab-workhorse']['sockets_directory']
+    default_directory != configured_directory
+  end
+
+  def orphan_socket
+    return deprecated_socket unless user_customized_socket?
+
+    node['gitlab']['gitlab-workhorse']['listen_addr']
   end
   def orphan_socket?
@@ -44,8 +52,14 @@ class GitlabWorkhorseHelper < BaseHelper
   end
   def listen_address
-    return File.join(sockets_directory, socket_file_name) if unix_socket?
-    node['gitlab']['gitlab-workhorse']['listen_addr']
+    return node['gitlab']['gitlab-workhorse']['listen_addr'] unless unix_socket?
+    selinux_socket = File.join(sockets_directory, socket_file_name)
+
+    return selinux_socket if user_customized_sockets_directory?
+
+    return node['gitlab']['gitlab-workhorse']['listen_addr'] if user_customized_socket?
+
+    selinux_socket
   end
 end

files/gitlab-cookbooks/gitlab/recipes/gitlab-workhorse.rb

@@ -37,6 +37,7 @@ if workhorse_helper.unix_socket?
     group account_helper.web_server_group
     mode '0750'
     recursive true
+    not_if { workhorse_helper.user_customized_socket? && !workhorse_helper.user_customized_sockets_directory? }
   end
 end

spec/chef/recipes/gitaly_spec.rb

@@ -488,13 +488,26 @@ RSpec.describe 'gitaly' do
   end
   context 'with a non-default workhorse unix socket' do
-    before do
-      stub_gitlab_rb(gitlab_workhorse: { listen_addr: '/fake/workhorse/socket' })
+    context 'without a socket directory set' do
+      before do
+        stub_gitlab_rb(gitlab_workhorse: { listen_addr: '/fake/workhorse/socket' })
+      end
+
+      it 'create config file with provided values' do
+        expect(chef_run).to render_file(config_path)
+          .with_content(%r{\[gitlab\]\s+url = 'http\+unix://%2Ffake%2Fworkhorse%2Fsocket'\s+relative_url_root = ''})
+      end
     end
-    it 'create config file with provided values' do
-      expect(chef_run).to render_file(config_path)
-        .with_content(%r{\[gitlab\]\s+url = 'http\+unix://%2Ffake%2Fworkhorse%2Fsockets%2Fsocket'\s+relative_url_root = ''})
+    context 'with a socket directory set' do
+      before do
+        stub_gitlab_rb(gitlab_workhorse: { listen_addr: '/fake/workhorse/socket', sockets_directory: '/fake/workhorse/sockets'})
+      end
+
+      it 'create config file with provided values' do
+        expect(chef_run).to render_file(config_path)
+          .with_content(%r{\[gitlab\]\s+url = 'http\+unix://%2Ffake%2Fworkhorse%2Fsockets%2Fsocket'\s+relative_url_root = ''})
+      end
     end
   end

spec/chef/recipes/gitlab-shell_spec.rb

@@ -174,17 +174,34 @@ RSpec.describe 'gitlab::gitlab-shell' do
     end
     context 'with a non-default workhorse unix socket' do
-      before do
-        stub_gitlab_rb(gitlab_workhorse: { listen_addr: '/fake/workhorse/socket' })
+      context 'without sockets_directory defined' do
+        before do
+          stub_gitlab_rb(gitlab_workhorse: { listen_addr: '/fake/workhorse/socket' })
+        end
+
+        it 'create config file with provided values' do
+          expect(chef_run).to create_templatesymlink('Create a config.yml and create a symlink to Rails root').with_variables(
+            hash_including(
+              gitlab_url: 'http+unix://%2Ffake%2Fworkhorse%2Fsocket',
+              gitlab_relative_path: ''
+            )
+          )
+        end
       end
-      it 'create config file with provided values' do
-        expect(chef_run).to create_templatesymlink('Create a config.yml and create a symlink to Rails root').with_variables(
-          hash_including(
-            gitlab_url: 'http+unix://%2Ffake%2Fworkhorse%2Fsockets%2Fsocket',
-            gitlab_relative_path: ''
+      context 'with sockets_directory defined' do
+        before do
+          stub_gitlab_rb(gitlab_workhorse: { listen_addr: '/fake/workhorse/socket', 'sockets_directory': '/fake/workhorse/sockets/' })
+        end
+
+        it 'create config file with provided values' do
+          expect(chef_run).to create_templatesymlink('Create a config.yml and create a symlink to Rails root').with_variables(
+            hash_including(
+              gitlab_url: 'http+unix://%2Ffake%2Fworkhorse%2Fsockets%2Fsocket',
+              gitlab_relative_path: ''
+            )
           )
-        )
+        end
       end
     end

spec/libraries/helpers/gitlab_workhorse_helper_spec.rb

@@ -102,33 +102,81 @@ RSpec.describe GitlabWorkhorseHelper do
       end
     end
-    context 'with custom workhorse configuration' do
-      cached(:chef_run) { converge_config }
-      before do
-        stub_gitlab_rb(
-          gitlab_workhorse: {
-            listen_network: 'unix',
-            listen_addr: deprecated_custom
-          }
-        )
-      end
-      describe '#orphan_socket' do
-        it 'returns the configured custom path' do
-          expect(subject.orphan_socket).to eq(deprecated_custom)
+    context 'with custom workhorse listen_addr' do
+      context 'without sockets_directory configured' do
+        cached(:chef_run) { converge_config }
+
+        before do
+          stub_gitlab_rb(
+            gitlab_workhorse: {
+              listen_network: 'unix',
+              listen_addr: deprecated_custom
+            }
+          )
+        end
+        describe '#user_customized_socket?' do
+          it 'should be true when listen_addr is set' do
+            expect(subject.user_customized_socket?).to be true
+          end
+        end
+        describe '#user_customized_sockets_directory?' do
+          it 'should be false when the directory is default' do
+            expect(subject.user_customized_sockets_directory?).to be false
+          end
+        end
+
+        describe '#orphan_socket' do
+          it 'returns the configured custom path' do
+            expect(subject.orphan_socket).to eq(deprecated_custom)
+          end
+        end
+
+        describe '#orphan_socket?' do
+          it 'is false when the listen address is customized' do
+            allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
+            expect(subject.orphan_socket?).to be false
+          end
         end
       end
-      describe '#orphan_socket?' do
-        it 'true when the orphan socket exists on disk' do
-          allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
-          expect(subject.orphan_socket?).to be true
+      context 'with sockets_directory configured' do
+        cached(:chef_run) { converge_config }
+
+        before do
+          stub_gitlab_rb(
+            gitlab_workhorse: {
+              listen_network: 'unix',
+              listen_addr: deprecated_custom,
+              sockets_directory: new_custom_directory
+            }
+          )
         end
-        it 'false when the orphan socket does not exist on disk' do
-          allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
-          expect(subject.orphan_socket?).to be true
+        describe "#user_customized_sockets_directory?" do
+          it 'should be true when the directory has been set' do
+            expect(subject.user_customized_sockets_directory?).to be true
+          end
+        end
+
+        describe '#orphan_socket' do
+          it 'returns the configured custom path' do
+            expect(subject.orphan_socket).to eq(deprecated_custom)
+          end
+        end
+
+        describe '#orphan_socket?' do
+          it 'is true when the listen address is customized' do
+            allow(File).to receive(:exist?).with(deprecated_custom).and_return(true)
+            expect(subject.orphan_socket?).to be true
+          end
+        end
+
+        describe '#listen_address' do
+          it 'returns the path the custom directory' do
+            expect(subject.listen_address).to eq(new_custom_path)
+          end
         end
       end
     end