Add a Gitlab::Git::Env to store Git-specific env thread-safely

Signed-off-by: Rémy Coutable <remy@rymai.me>

Former-commit-id: 05aa038c

lib/gitlab/git/env.rb

+module Gitlab
+  module Git
+    # Ephemeral (per request) storage for environment variables that some Git
+    # commands may need.
+    #
+    # For example, in pre-receive hooks, new objects are put in a temporary
+    # $GIT_OBJECT_DIRECTORY. Without it set, the new objects cannot be retrieved
+    # (this would break push rules for instance).
+    #
+    # This class is thread-safe via RequestStore.
+    class Env
+      WHITELISTED_GIT_VARIABLES = %w[
+        GIT_OBJECT_DIRECTORY
+        GIT_ALTERNATE_OBJECT_DIRECTORIES
+      ].freeze
+
+      def self.set(env)
+        return unless RequestStore.active?
+
+        RequestStore.store[:gitlab_git_env] = whitelist_git_env(env)
+      end
+
+      def self.all
+        return {} unless RequestStore.active?
+
+        RequestStore.fetch(:gitlab_git_env) { {} }
+      end
+
+      def self.[](key)
+        all[key]
+      end
+
+      def self.whitelist_git_env(env)
+        env.select { |key, _| WHITELISTED_GIT_VARIABLES.include?(key.to_s) }.with_indifferent_access
+      end
+    end
+  end
+end

spec/lib/gitlab/git/env_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Git::Env do
+  describe "#set" do
+    context 'with RequestStore.store disabled' do
+      before do
+        allow(RequestStore).to receive(:active?).and_return(false)
+      end
+
+      it 'does not store anything' do
+        described_class.set(GIT_OBJECT_DIRECTORY: 'foo')
+
+        expect(described_class.all).to be_empty
+      end
+    end
+
+    context 'with RequestStore.store enabled' do
+      before do
+        allow(RequestStore).to receive(:active?).and_return(true)
+      end
+
+      it 'whitelist some `GIT_*` variables and stores them using RequestStore' do
+        described_class.set(
+          GIT_OBJECT_DIRECTORY: 'foo',
+          GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar',
+          GIT_EXEC_PATH: 'baz',
+          PATH: '~/.bin:/bin')
+
+        expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo')
+        expect(described_class[:GIT_ALTERNATE_OBJECT_DIRECTORIES]).to eq('bar')
+        expect(described_class[:GIT_EXEC_PATH]).to be_nil
+        expect(described_class[:bar]).to be_nil
+      end
+    end
+  end
+
+  describe "#all" do
+    context 'with RequestStore.store enabled' do
+      before do
+        allow(RequestStore).to receive(:active?).and_return(true)
+        described_class.set(
+          GIT_OBJECT_DIRECTORY: 'foo',
+          GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar')
+      end
+
+      it 'returns an env hash' do
+        expect(described_class.all).to eq({
+          'GIT_OBJECT_DIRECTORY' => 'foo',
+          'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar'
+        })
+      end
+    end
+  end
+
+  describe "#[]" do
+    context 'with RequestStore.store enabled' do
+      before do
+        allow(RequestStore).to receive(:active?).and_return(true)
+      end
+
+      before do
+        described_class.set(
+          GIT_OBJECT_DIRECTORY: 'foo',
+          GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar')
+      end
+
+      it 'returns a stored value for an existing key' do
+        expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo')
+      end
+
+      it 'returns nil for an non-existing key' do
+        expect(described_class[:foo]).to be_nil
+      end
+    end
+  end
+
+  describe 'thread-safety' do
+    context 'with RequestStore.store enabled' do
+      before do
+        allow(RequestStore).to receive(:active?).and_return(true)
+        described_class.set(GIT_OBJECT_DIRECTORY: 'foo')
+      end
+
+      it 'is thread-safe' do
+        another_thread = Thread.new do
+          described_class.set(GIT_OBJECT_DIRECTORY: 'bar')
+
+          Thread.stop
+          described_class[:GIT_OBJECT_DIRECTORY]
+        end
+
+        # Ensure another_thread runs first
+        sleep 0.1 until another_thread.stop?
+
+        expect(described_class[:GIT_OBJECT_DIRECTORY]).to eq('foo')
+
+        another_thread.run
+        expect(another_thread.value).to eq('bar')
+      end
+    end
+  end
+end