gitaly: revert a commit that broke config loading

2 files changed, 324 insertions, 1 deletions

diff --git a/pkgs/applications/version-management/gitlab/gitaly/0001-Revert-Allow-gitlabshell-config-to-be-passed-in-thro.patch b/pkgs/applications/version-management/gitlab/gitaly/0001-Revert-Allow-gitlabshell-config-to-be-passed-in-thro.patch
new file mode 100644
index 000000000000..ed19ab41f142
--- /dev/null
+++ b/pkgs/applications/version-management/gitlab/gitaly/0001-Revert-Allow-gitlabshell-config-to-be-passed-in-thro.patch
@@ -0,0 +1,320 @@
+From 3df6278a00d29a0941cf0142848fac8bfba09ebf Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Milan=20P=C3=A4ssler?= <me@pbb.lc>
+Date: Tue, 19 May 2020 01:12:04 +0200
+Subject: [PATCH] Revert "Allow gitlabshell config to be passed in through a
+ config variable"
+
+This reverts commit 14d8bae73b9e0d604c048d1cbcc1f4413c922bef.
+---
+ ruby/gitlab-shell/lib/gitlab_config.rb        |  46 +++----
+ ruby/gitlab-shell/lib/gitlab_init.rb          |  10 +-
+ ruby/gitlab-shell/spec/gitlab_config_spec.rb  | 116 +++++-------------
+ .../spec/gitlab_custom_hook_spec.rb           |   2 +-
+ ruby/gitlab-shell/spec/gitlab_logger_spec.rb  |   2 +-
+ ruby/gitlab-shell/spec/spec_helper.rb         |   4 +
+ 6 files changed, 54 insertions(+), 126 deletions(-)
+
+diff --git a/ruby/gitlab-shell/lib/gitlab_config.rb b/ruby/gitlab-shell/lib/gitlab_config.rb
+index 9020ff7f..19f03c34 100644
+--- a/ruby/gitlab-shell/lib/gitlab_config.rb
++++ b/ruby/gitlab-shell/lib/gitlab_config.rb
+@@ -2,46 +2,44 @@ require 'yaml'
+ 
+ class GitlabConfig
+   def secret_file
+-    fetch_from_config('secret_file', fetch_from_legacy_config('secret_file', File.join(ROOT_PATH, '.gitlab_shell_secret')))
++    fetch_from_legacy_config('secret_file',File.join(ROOT_PATH, '.gitlab_shell_secret'))
+   end
+ 
+   # Pass a default value because this is called from a repo's context; in which
+   # case, the repo's hooks directory should be the default.
+   #
+   def custom_hooks_dir(default: nil)
+-    fetch_from_config('custom_hooks_dir', fetch_from_legacy_config('custom_hooks_dir', File.join(ROOT_PATH, 'hooks')))
++    fetch_from_legacy_config('custom_hooks_dir', File.join(ROOT_PATH, 'hooks'))
+   end
+ 
+   def gitlab_url
+-    fetch_from_config('gitlab_url', fetch_from_legacy_config('gitlab_url',"http://localhost:8080").sub(%r{/*$}, ''))
++    fetch_from_legacy_config('gitlab_url',"http://localhost:8080").sub(%r{/*$}, '')
+   end
+ 
+   def http_settings
+-    fetch_from_config('http_settings', fetch_from_legacy_config('http_settings', {}))
++    fetch_from_legacy_config('http_settings', {})
+   end
+ 
+   def log_file
+-    log_path = Pathname.new(fetch_from_config('log_path', LOG_PATH))
++    return File.join(LOG_PATH, 'gitlab-shell.log') unless LOG_PATH.empty?
+ 
+-    log_path = ROOT_PATH if log_path === ''
+-
+-    return log_path.join('gitlab-shell.log')
++    fetch_from_legacy_config('log_file', File.join(ROOT_PATH, 'gitlab-shell.log'))
+   end
+ 
+   def log_level
+-    log_level = fetch_from_config('log_level', LOG_LEVEL)
+-
+-    return log_level unless log_level.empty?
++    return LOG_LEVEL unless LOG_LEVEL.empty?
+ 
+-    'INFO'
++    fetch_from_legacy_config('log_level', 'INFO')
+   end
+ 
+   def log_format
+-    log_format = fetch_from_config('log_format', LOG_FORMAT)
++    return LOG_FORMAT unless LOG_FORMAT.empty?
+ 
+-    return log_format unless log_format.empty?
++    fetch_from_legacy_config('log_format', 'text')
++  end
+ 
+-    'text'
++  def metrics_log_file
++    fetch_from_legacy_config('metrics_log_file', File.join(ROOT_PATH, 'gitlab-shell-metrics.log'))
+   end
+ 
+   def to_json
+@@ -53,6 +51,7 @@ class GitlabConfig
+       log_file: log_file,
+       log_level: log_level,
+       log_format: log_format,
++      metrics_log_file: metrics_log_file
+     }.to_json
+   end
+ 
+@@ -62,23 +61,8 @@ class GitlabConfig
+ 
+   private
+ 
+-  def fetch_from_config(key, default)
+-    value = config[key]
+-
+-    return default if value.nil? || value.empty?
+-
+-    value
+-  end
+-
+-  def config
+-    @config ||= JSON.parse(ENV.fetch('GITALY_GITLAB_SHELL_CONFIG', '{}'))
+-  end
+-
+   def legacy_config
+     # TODO: deprecate @legacy_config that is parsing the gitlab-shell config.yml
+-    legacy_file = ROOT_PATH.join('config.yml')
+-    return {} unless legacy_file.exist?
+-
+-    @legacy_config ||= YAML.load_file(legacy_file)
++    @legacy_config ||= YAML.load_file(File.join(ROOT_PATH, 'config.yml'))
+   end
+ end
+diff --git a/ruby/gitlab-shell/lib/gitlab_init.rb b/ruby/gitlab-shell/lib/gitlab_init.rb
+index 94913549..ce54e2d4 100644
+--- a/ruby/gitlab-shell/lib/gitlab_init.rb
++++ b/ruby/gitlab-shell/lib/gitlab_init.rb
+@@ -1,10 +1,8 @@
+ # GITLAB_SHELL_DIR has been deprecated
+-require 'pathname'
+-
+-ROOT_PATH = Pathname.new(ENV['GITALY_GITLAB_SHELL_DIR'] || ENV['GITLAB_SHELL_DIR'] || File.expand_path('..', __dir__)).freeze
+-LOG_PATH = Pathname.new(ENV.fetch('GITALY_LOG_DIR', ROOT_PATH))
+-LOG_LEVEL = ENV.fetch('GITALY_LOG_LEVEL', 'INFO')
+-LOG_FORMAT = ENV.fetch('GITALY_LOG_FORMAT', 'text')
++ROOT_PATH = ENV['GITALY_GITLAB_SHELL_DIR'] || ENV['GITLAB_SHELL_DIR'] || File.expand_path('..', __dir__)
++LOG_PATH = ENV.fetch('GITALY_LOG_DIR', "")
++LOG_LEVEL = ENV.fetch('GITALY_LOG_LEVEL', "")
++LOG_FORMAT = ENV.fetch('GITALY_LOG_FORMAT', "")
+ 
+ # We are transitioning parts of gitlab-shell into the gitaly project. In
+ # gitaly, GITALY_EMBEDDED will be true.
+diff --git a/ruby/gitlab-shell/spec/gitlab_config_spec.rb b/ruby/gitlab-shell/spec/gitlab_config_spec.rb
+index b0f12381..6de0cd08 100644
+--- a/ruby/gitlab-shell/spec/gitlab_config_spec.rb
++++ b/ruby/gitlab-shell/spec/gitlab_config_spec.rb
+@@ -1,112 +1,54 @@
+ require_relative 'spec_helper'
+ require_relative '../lib/gitlab_config'
+-require 'json'
+ 
+ describe GitlabConfig do
+   let(:config) { GitlabConfig.new }
++  let(:config_data) { {} }
+ 
+-  context "without ENV['GITALY_GITLAB_SHELL_CONFIG'] is passed in" do
+-    let(:config_data) { {} }
++  before { expect(YAML).to receive(:load_file).and_return(config_data) }
+ 
+-    before { expect(YAML).to receive(:load_file).and_return(config_data) }
++  describe '#gitlab_url' do
++    let(:url) { 'http://test.com' }
+ 
+-    describe '#gitlab_url' do
+-      let(:url) { 'http://test.com' }
++    subject { config.gitlab_url }
+ 
+-      subject { config.gitlab_url }
++    before { config_data['gitlab_url'] = url }
+ 
+-      before { config_data['gitlab_url'] = url }
++    it { is_expected.not_to be_empty }
++    it { is_expected.to eq(url) }
+ 
+-      it { is_expected.not_to be_empty }
+-      it { is_expected.to eq(url) }
+-
+-      context 'remove trailing slashes' do
+-        before { config_data['gitlab_url'] = url + '//' }
+-
+-        it { is_expected.to eq(url) }
+-      end
+-    end
+-
+-    describe '#secret_file' do
+-      subject { config.secret_file }
++    context 'remove trailing slashes' do
++      before { config_data['gitlab_url'] = url + '//' }
+ 
+-      it 'returns ".gitlab_shell_secret" by default' do
+-        is_expected.to eq(File.join(File.expand_path('..', __dir__),'.gitlab_shell_secret'))
+-      end
+-    end
+-
+-    describe '#fetch_from_legacy_config' do
+-      let(:key) { 'yaml_key' }
+-
+-      where(:yaml_value, :default, :expected_value) do
+-        [
+-          ['a', 'b', 'a'],
+-          [nil, 'b', 'b'],
+-          ['a', nil, 'a'],
+-          [nil, {}, {}]
+-        ]
+-      end
+-
+-      with_them do
+-        it 'returns the correct value' do
+-          config_data[key] = yaml_value
+-
+-          expect(config.fetch_from_legacy_config(key, default)).to eq(expected_value)
+-        end
+-      end
++      it { is_expected.to eq(url) }
+     end
+   end
+ 
+-  context "when ENV['GITALY_GITLAB_SHELL_CONFIG'] is passed in" do
+-    let(:config_data) { {'secret_file': 'path/to/secret/file',
+-      'custom_hooks_dir': '/path/to/custom_hooks',
+-      'gitlab_url': 'http://localhost:123454',
+-      'http_settings': {'user': 'user_123', 'password':'password123', 'ca_file': '/path/to/ca_file', 'ca_path': 'path/to/ca_path'},
+-      'log_path': '/path/to/log',
+-      'log_level': 'myloglevel',
+-      'log_format': 'log_format'} }
++  describe '#log_format' do
++    subject { config.log_format }
+ 
+-    before { allow(ENV).to receive(:fetch).with('GITALY_GITLAB_SHELL_CONFIG', '{}').and_return(config_data.to_json) }
+-
+-    describe '#secret_file' do
+-      it 'returns the correct secret_file' do
+-        expect(config.secret_file).to eq(config_data[:secret_file])
+-      end
+-    end
+-
+-    describe '#custom_hooks_dir' do
+-      it 'returns the correct custom_hooks_dir' do
+-        expect(config.custom_hooks_dir).to eq(config_data[:custom_hooks_dir])
+-      end
+-    end
+-
+-    describe '#http_settings' do
+-      it 'returns the correct http_settings' do
+-        expect(config.http_settings).to eq(config_data[:http_settings].transform_keys(&:to_s))
+-      end
++    it 'returns "text" by default' do
++      is_expected.to eq('text')
+     end
++  end
+ 
+-    describe '#gitlab_url' do
+-      it 'returns the correct gitlab_url' do
+-        expect(config.gitlab_url).to eq(config_data[:gitlab_url])
+-      end
+-    end
++  describe '#fetch_from_legacy_config' do
++    let(:key) { 'yaml_key' }
+ 
+-    describe '#log_path' do
+-      it 'returns the correct log_path' do
+-        expect(config.log_file).to eq(Pathname.new(File.join(config_data[:log_path], 'gitlab-shell.log')))
+-      end
++    where(:yaml_value, :default, :expected_value) do
++      [
++        ['a', 'b', 'a'],
++        [nil, 'b', 'b'],
++        ['a', nil, 'a'],
++        [nil, {}, {}]
++      ]
+     end
+ 
+-    describe '#log_level' do
+-      it 'returns the correct log_level' do
+-        expect(config.log_level).to eq(config_data[:log_level])
+-      end
+-    end
++    with_them do
++      it 'returns the correct value' do
++        config_data[key] = yaml_value
+ 
+-    describe '#log_format' do
+-      it 'returns the correct log_format' do
+-        expect(config.log_format).to eq(config_data[:log_format])
++        expect(config.fetch_from_legacy_config(key, default)).to eq(expected_value)
+       end
+     end
+   end
+diff --git a/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb b/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
+index c19b90f8..50865331 100644
+--- a/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
++++ b/ruby/gitlab-shell/spec/gitlab_custom_hook_spec.rb
+@@ -99,7 +99,7 @@ describe GitlabCustomHook do
+     FileUtils.symlink(File.join(tmp_root_path, 'hooks'), File.join(tmp_repo_path, 'hooks'))
+     FileUtils.symlink(File.join(ROOT_PATH, 'config.yml.example'), File.join(tmp_root_path, 'config.yml'))
+ 
+-    stub_const('ROOT_PATH', Pathname.new(tmp_root_path))
++    stub_const('ROOT_PATH', tmp_root_path)
+   end
+ 
+   after do
+diff --git a/ruby/gitlab-shell/spec/gitlab_logger_spec.rb b/ruby/gitlab-shell/spec/gitlab_logger_spec.rb
+index 0523a4cc..0da64735 100644
+--- a/ruby/gitlab-shell/spec/gitlab_logger_spec.rb
++++ b/ruby/gitlab-shell/spec/gitlab_logger_spec.rb
+@@ -142,7 +142,7 @@ describe GitlabLogger do
+       test_logger_status = system('bin/test-logger', msg)
+       expect(test_logger_status).to eq(true)
+ 
+-      grep_status = system('grep', '-q', '-e', msg, GitlabConfig.new.log_file.to_s)
++      grep_status = system('grep', '-q', '-e', msg, GitlabConfig.new.log_file)
+       expect(grep_status).to eq(true)
+     end
+   end
+diff --git a/ruby/gitlab-shell/spec/spec_helper.rb b/ruby/gitlab-shell/spec/spec_helper.rb
+index 684f87b1..be5ff9c6 100644
+--- a/ruby/gitlab-shell/spec/spec_helper.rb
++++ b/ruby/gitlab-shell/spec/spec_helper.rb
+@@ -10,4 +10,8 @@ Dir[File.expand_path('support/**/*.rb', __dir__)].each { |f| require f }
+ RSpec.configure do |config|
+   config.run_all_when_everything_filtered = true
+   config.filter_run :focus
++
++  config.before(:each) do
++    stub_const('ROOT_PATH', File.expand_path('..', __dir__))
++  end
+ end
+-- 
+2.26.2
+
diff --git a/pkgs/applications/version-management/gitlab/gitaly/default.nix b/pkgs/applications/version-management/gitlab/gitaly/default.nix
index 3fe4dbe8bd93..b79ed2dc84fc 100644
--- a/pkgs/applications/version-management/gitlab/gitaly/default.nix
+++ b/pkgs/applications/version-management/gitlab/gitaly/default.nix
@@ -40,7 +40,10 @@ in buildGoPackage rec {
 
   # Fix a check which assumes that hook files are writeable by their
   # owner.
-  patches = [ ./fix-executable-check.patch ];
+  patches = [
+    ./fix-executable-check.patch
+    ./0001-Revert-Allow-gitlabshell-config-to-be-passed-in-thro.patch
+  ];
 
   goPackagePath = "gitlab.com/gitlab-org/gitaly";