From e32bf64da083edd714605d0ed917c2a447b703bf Mon Sep 17 00:00:00 2001 From: Milan Pässler Date: Mon, 18 May 2020 23:50:02 +0200 Subject: gitaly: revert a commit that broke config loading --- ...w-gitlabshell-config-to-be-passed-in-thro.patch | 320 +++++++++++++++++++++ .../version-management/gitlab/gitaly/default.nix | 5 +- 2 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 pkgs/applications/version-management/gitlab/gitaly/0001-Revert-Allow-gitlabshell-config-to-be-passed-in-thro.patch 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?= +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"; -- cgit 1.4.1