diff options
| -rw-r--r-- | lib/gitlab/checks/force_push.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/git/rev_list.rb | 14 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/rev_list_spec.rb | 65 |
3 files changed, 30 insertions, 51 deletions
diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb index e1c967a1f89..de0c9049ebf 100644 --- a/lib/gitlab/checks/force_push.rb +++ b/lib/gitlab/checks/force_push.rb @@ -13,7 +13,7 @@ module Gitlab if exit_status == 0 missed_ref.present? else - raise RuntimeError, "Got a non-zero exit code while calling out to `git rev-list` in the force-push check." + raise "Got a non-zero exit code while calling out to `git rev-list` in the force-push check." end end end diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb index ecd038e04df..25e9d619697 100644 --- a/lib/gitlab/git/rev_list.rb +++ b/lib/gitlab/git/rev_list.rb @@ -1,11 +1,9 @@ -# Call out to the `git rev-list` command - module Gitlab module Git class RevList attr_reader :project, :env - ALLOWED_VARIABLES = %w(GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES).freeze + ALLOWED_VARIABLES = %w[GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES].freeze def initialize(oldrev, newrev, project:, env: nil) @project = project @@ -23,8 +21,8 @@ module Gitlab end def valid? - env.slice(*ALLOWED_VARIABLES).all? do |(name, value)| - value =~ /^#{project.repository.path_to_repo}/ + environment_variables.all? do |(name, value)| + value.start_with?(project.repository.path_to_repo) end end @@ -33,7 +31,11 @@ module Gitlab def parse_environment_variables return {} unless valid? - env.slice(*ALLOWED_VARIABLES) + environment_variables + end + + def environment_variables + @environment_variables ||= env.slice(*ALLOWED_VARIABLES) end end end diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb index f76aca29107..444639acbaa 100644 --- a/spec/lib/gitlab/git/rev_list_spec.rb +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -4,49 +4,28 @@ describe Gitlab::Git::RevList, lib: true do let(:project) { create(:project) } context "validations" do - context "GIT_OBJECT_DIRECTORY" do - it "accepts values starting with the project repo path" do - env = { "GIT_OBJECT_DIRECTORY" => "#{project.repository.path_to_repo}/objects" } - rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) + described_class::ALLOWED_VARIABLES.each do |var| + context var do + it "accepts values starting with the project repo path" do + env = { var => "#{project.repository.path_to_repo}/objects" } + rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) - expect(rev_list).to be_valid - end - - it "rejects values starting not with the project repo path" do - env = { "GIT_OBJECT_DIRECTORY" => "/some/other/path" } - rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) - - expect(rev_list).not_to be_valid - end - - it "rejects values containing the project repo path but not starting with it" do - env = { "GIT_OBJECT_DIRECTORY" => "/some/other/path/#{project.repository.path_to_repo}" } - rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) - - expect(rev_list).not_to be_valid - end - end + expect(rev_list).to be_valid + end - context "GIT_ALTERNATE_OBJECT_DIRECTORIES" do - it "accepts values starting with the project repo path" do - env = { "GIT_ALTERNATE_OBJECT_DIRECTORIES" => project.repository.path_to_repo } - rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) + it "rejects values starting not with the project repo path" do + env = { var => "/some/other/path" } + rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) - expect(rev_list).to be_valid - end - - it "rejects values starting not with the project repo path" do - env = { "GIT_ALTERNATE_OBJECT_DIRECTORIES" => "/some/other/path" } - rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) - - expect(rev_list).not_to be_valid - end + expect(rev_list).not_to be_valid + end - it "rejects values containing the project repo path but not starting with it" do - env = { "GIT_ALTERNATE_OBJECT_DIRECTORIES" => "/some/other/path/#{project.repository.path_to_repo}" } - rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) + it "rejects values containing the project repo path but not starting with it" do + env = { var => "/some/other/path/#{project.repository.path_to_repo}" } + rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) - expect(rev_list).not_to be_valid + expect(rev_list).not_to be_valid + end end end end @@ -57,20 +36,18 @@ describe Gitlab::Git::RevList, lib: true do it "calls out to `popen` without environment variables if the record is invalid" do allow(rev_list).to receive(:valid?).and_return(false) - allow(Open3).to receive(:popen3) - rev_list.execute + expect(Open3).to receive(:popen3).with(hash_excluding(env), any_args) - expect(Open3).to have_received(:popen3).with(hash_excluding(env), any_args) + rev_list.execute end it "calls out to `popen` with environment variables if the record is valid" do allow(rev_list).to receive(:valid?).and_return(true) - allow(Open3).to receive(:popen3) - rev_list.execute + expect(Open3).to receive(:popen3).with(hash_including(env), any_args) - expect(Open3).to have_received(:popen3).with(hash_including(env), any_args) + rev_list.execute end end end |