Pass git object dir attributes as relative paths to Gitalyfix/pass-git-object-dir-attrs-as-relative

Fixes gitaly#629

7 files changed, 69 insertions, 30 deletions

diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index ff12df2fbb0..301092317fe 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.45.1
+0.46.0
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION
index 5508e17c23b..c5b7013b9c5 100644
--- a/GITLAB_SHELL_VERSION
+++ b/GITLAB_SHELL_VERSION
@@ -1,2 +1 @@
-5.9.3
-
+5.9.4
diff --git a/lib/gitlab/git/env.rb b/lib/gitlab/git/env.rb
index f80193ac553..80f0731cd99 100644
--- a/lib/gitlab/git/env.rb
+++ b/lib/gitlab/git/env.rb
@@ -11,9 +11,11 @@ module Gitlab
     #
     # This class is thread-safe via RequestStore.
     class Env
-      WHITELISTED_GIT_VARIABLES = %w[
+      WHITELISTED_VARIABLES = %w[
         GIT_OBJECT_DIRECTORY
+        GIT_OBJECT_DIRECTORY_RELATIVE
         GIT_ALTERNATE_OBJECT_DIRECTORIES
+        GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE
       ].freeze
 
       def self.set(env)
@@ -33,7 +35,7 @@ module Gitlab
       end
 
       def self.whitelist_git_env(env)
-        env.select { |key, _| WHITELISTED_GIT_VARIABLES.include?(key.to_s) }.with_indifferent_access
+        env.select { |key, _| WHITELISTED_VARIABLES.include?(key.to_s) }.with_indifferent_access
       end
     end
   end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index fa13920a3f3..e6f08e68f45 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -12,6 +12,10 @@ module Gitlab
         GIT_OBJECT_DIRECTORY
         GIT_ALTERNATE_OBJECT_DIRECTORIES
       ].freeze
+      ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES = %w[
+        GIT_OBJECT_DIRECTORY_RELATIVE
+        GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE
+      ].freeze
       SEARCH_CONTEXT_LINES = 3
 
       NoRepository = Class.new(StandardError)
@@ -1220,7 +1224,15 @@ module Gitlab
       end
 
       def alternate_object_directories
-        Gitlab::Git::Env.all.values_at(*ALLOWED_OBJECT_DIRECTORIES_VARIABLES).compact
+        relative_paths = Gitlab::Git::Env.all.values_at(*ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES).compact.flatten
+
+        if relative_paths.any?
+          relative_paths.map { |d| File.join(path, d) }
+        else
+          Gitlab::Git::Env.all.values_at(*ALLOWED_OBJECT_DIRECTORIES_VARIABLES)
+            .compact
+            .flat_map { |d| d.split(File::PATH_SEPARATOR) }
+        end
       end
 
       # Get the content of a blob for a given commit.  If the blob is a commit
diff --git a/lib/gitlab/gitaly_client/util.rb b/lib/gitlab/gitaly_client/util.rb
index da43c616b94..2d6d0f7b3e0 100644
--- a/lib/gitlab/gitaly_client/util.rb
+++ b/lib/gitlab/gitaly_client/util.rb
@@ -3,12 +3,17 @@ module Gitlab
     module Util
       class << self
         def repository(repository_storage, relative_path, gl_repository)
+          git_object_directory = Gitlab::Git::Env['GIT_OBJECT_DIRECTORY_RELATIVE'] ||
+            Gitlab::Git::Env['GIT_OBJECT_DIRECTORY']
+          git_alternate_object_directories = Array.wrap(Gitlab::Git::Env['GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE']) ||
+            Array.wrap(Gitlab::Git::Env['GIT_ALTERNATE_OBJECT_DIRECTORIES']).flat_map { |d| d.split(File::PATH_SEPARATOR) }
+
           Gitaly::Repository.new(
             storage_name: repository_storage,
             relative_path: relative_path,
             gl_repository: gl_repository,
-            git_object_directory: Gitlab::Git::Env['GIT_OBJECT_DIRECTORY'].to_s,
-            git_alternate_object_directories: Array.wrap(Gitlab::Git::Env['GIT_ALTERNATE_OBJECT_DIRECTORIES'])
+            git_object_directory: git_object_directory.to_s,
+            git_alternate_object_directories: git_alternate_object_directories
           )
         end
 
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 1ee4acfd193..d959562c951 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -68,31 +68,52 @@ describe Gitlab::Git::Repository, seed_helper: true do
       expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Repository::NoRepository)
     end
 
-    context 'with no Git env stored' do
-      before do
-        expect(Gitlab::Git::Env).to receive(:all).and_return({})
-      end
+    describe 'alternates keyword argument' do
+      context 'with no Git env stored' do
+        before do
+          allow(Gitlab::Git::Env).to receive(:all).and_return({})
+        end
 
-      it "whitelist some variables and pass them via the alternates keyword argument" do
-        expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: [])
+        it "is passed an empty array" do
+          expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: [])
 
-        repository.rugged
+          repository.rugged
+        end
       end
-    end
 
-    context 'with some Git env stored' do
-      before do
-        expect(Gitlab::Git::Env).to receive(:all).and_return({
-          'GIT_OBJECT_DIRECTORY' => 'foo',
-          'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar',
-          'GIT_OTHER' => 'another_env'
-        })
+      context 'with absolute and relative Git object dir envvars stored' do
+        before do
+          allow(Gitlab::Git::Env).to receive(:all).and_return({
+            'GIT_OBJECT_DIRECTORY_RELATIVE' => './objects/foo',
+            'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['./objects/bar', './objects/baz'],
+            'GIT_OBJECT_DIRECTORY' => 'ignored',
+            'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'ignored:ignored',
+            'GIT_OTHER' => 'another_env'
+          })
+        end
+
+        it "is passed the relative object dir envvars after being converted to absolute ones" do
+          alternates = %w[foo bar baz].map { |d| File.join(repository.path, './objects', d) }
+          expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: alternates)
+
+          repository.rugged
+        end
       end
 
-      it "whitelist some variables and pass them via the alternates keyword argument" do
-        expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: %w[foo bar])
+      context 'with only absolute Git object dir envvars stored' do
+        before do
+          allow(Gitlab::Git::Env).to receive(:all).and_return({
+            'GIT_OBJECT_DIRECTORY' => 'foo',
+            'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar:baz',
+            'GIT_OTHER' => 'another_env'
+          })
+        end
 
-        repository.rugged
+        it "is passed the absolute object dir envvars as is" do
+          expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: %w[foo bar baz])
+
+          repository.rugged
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb
index 498f6886bee..c0c29552494 100644
--- a/spec/lib/gitlab/gitaly_client/util_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/util_spec.rb
@@ -6,16 +6,16 @@ describe Gitlab::GitalyClient::Util do
     let(:relative_path) { 'my/repo.git' }
     let(:gl_repository) { 'project-1' }
     let(:git_object_directory) { '.git/objects' }
-    let(:git_alternate_object_directory) { '/dir/one:/dir/two' }
+    let(:git_alternate_object_directory) { ['/dir/one', '/dir/two'] }
 
     subject do
       described_class.repository(repository_storage, relative_path, gl_repository)
     end
 
     it 'creates a Gitaly::Repository with the given data' do
-      expect(Gitlab::Git::Env).to receive(:[]).with('GIT_OBJECT_DIRECTORY')
+      allow(Gitlab::Git::Env).to receive(:[]).with('GIT_OBJECT_DIRECTORY_RELATIVE')
         .and_return(git_object_directory)
-      expect(Gitlab::Git::Env).to receive(:[]).with('GIT_ALTERNATE_OBJECT_DIRECTORIES')
+      allow(Gitlab::Git::Env).to receive(:[]).with('GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE')
         .and_return(git_alternate_object_directory)
 
       expect(subject).to be_a(Gitaly::Repository)
@@ -23,7 +23,7 @@ describe Gitlab::GitalyClient::Util do
       expect(subject.relative_path).to eq(relative_path)
       expect(subject.gl_repository).to eq(gl_repository)
       expect(subject.git_object_directory).to eq(git_object_directory)
-      expect(subject.git_alternate_object_directories).to eq([git_alternate_object_directory])
+      expect(subject.git_alternate_object_directories).to eq(git_alternate_object_directory)
     end
   end