diff options
author | Rémy Coutable <remy@rymai.me> | 2017-02-08 14:49:28 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-02-08 14:49:28 +0000 |
commit | 31d6e247041d4bb13aa712dcbbfad10ee5e403b8 (patch) | |
tree | 2d6500c76b7a3d9e67e32c1cff0a849e8965ab32 | |
parent | f0453905418dab64a1a53ff67044a5eb8f7aa333 (diff) | |
parent | b54b031638e7a98c1e51b369cff53602db40e4b0 (diff) | |
download | gitlab-ce-8-13-stable.tar.gz |
Merge branch 'backport-7967-and-8189-to-8-13-stable' into '8-13-stable'
8-13-stable
Backport !7967 and !8189 to `8-13-stable`
See merge request !8991
-rw-r--r-- | GITLAB_SHELL_VERSION | 2 | ||||
-rw-r--r-- | changelogs/unreleased/25301-git-2-11-force-push-bug.yml | 4 | ||||
-rw-r--r-- | changelogs/unreleased/use-gitlab-shell-3-6-7.yml | 4 | ||||
-rw-r--r-- | doc/update/8.12-to-8.13.md | 4 | ||||
-rw-r--r-- | lib/api/internal.rb | 14 | ||||
-rw-r--r-- | lib/gitlab/checks/change_access.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/checks/force_push.rb | 11 | ||||
-rw-r--r-- | lib/gitlab/git/rev_list.rb | 42 | ||||
-rw-r--r-- | lib/gitlab/git_access.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/popen.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/checks/force_push_spec.rb | 19 | ||||
-rw-r--r-- | spec/lib/gitlab/git/rev_list_spec.rb | 60 |
12 files changed, 161 insertions, 13 deletions
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 4f2c1d15f6d..5b3413147c9 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -3.6.6 +3.6.7 diff --git a/changelogs/unreleased/25301-git-2-11-force-push-bug.yml b/changelogs/unreleased/25301-git-2-11-force-push-bug.yml new file mode 100644 index 00000000000..afe57729c48 --- /dev/null +++ b/changelogs/unreleased/25301-git-2-11-force-push-bug.yml @@ -0,0 +1,4 @@ +--- +title: Accept environment variables from the `pre-receive` script +merge_request: 7967 +author: diff --git a/changelogs/unreleased/use-gitlab-shell-3-6-7.yml b/changelogs/unreleased/use-gitlab-shell-3-6-7.yml new file mode 100644 index 00000000000..c6600cead87 --- /dev/null +++ b/changelogs/unreleased/use-gitlab-shell-3-6-7.yml @@ -0,0 +1,4 @@ +--- +title: Use gitlab-shell v3.6.7 +merge_request: +author: diff --git a/doc/update/8.12-to-8.13.md b/doc/update/8.12-to-8.13.md index c0084d9d59c..6457ec9b663 100644 --- a/doc/update/8.12-to-8.13.md +++ b/doc/update/8.12-to-8.13.md @@ -72,7 +72,7 @@ sudo -u git -H git checkout 8-13-stable-ee ```bash cd /home/git/gitlab-shell sudo -u git -H git fetch --all --tags -sudo -u git -H git checkout v3.6.6 +sudo -u git -H git checkout v3.6.7 ``` ### 6. Update gitlab-workhorse @@ -166,7 +166,7 @@ See [smtp_settings.rb.sample] as an example. Ensure you're still up-to-date with the latest init script changes: sudo cp lib/support/init.d/gitlab /etc/init.d/gitlab - + For Ubuntu 16.04.1 LTS: sudo systemctl daemon-reload diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 9a5d1ece070..89e47a7b0f5 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -43,6 +43,14 @@ module API :push_code ] end + + def parse_allowed_environment_variables + return if params[:env].blank? + + JSON.parse(params[:env]) + + rescue JSON::ParserError + end end post "/allowed" do @@ -61,7 +69,11 @@ module API if wiki? Gitlab::GitAccessWiki.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities) else - Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities) + Gitlab::GitAccess.new(actor, + project, + protocol, + authentication_abilities: ssh_authentication_abilities, + env: parse_allowed_environment_variables) end access_status = access.check(params[:action], params[:changes]) diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index cb1065223d4..3d203017d9f 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -3,11 +3,12 @@ module Gitlab class ChangeAccess attr_reader :user_access, :project - def initialize(change, user_access:, project:) + def initialize(change, user_access:, project:, env: {}) @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) @branch_name = Gitlab::Git.branch_name(@ref) @user_access = user_access @project = project + @env = env end def exec @@ -68,7 +69,7 @@ module Gitlab end def forced_push? - Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev) + Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev, env: @env) end def matching_merge_request? diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb index 5fe86553bd0..de0c9049ebf 100644 --- a/lib/gitlab/checks/force_push.rb +++ b/lib/gitlab/checks/force_push.rb @@ -1,15 +1,20 @@ module Gitlab module Checks class ForcePush - def self.force_push?(project, oldrev, newrev) + def self.force_push?(project, oldrev, newrev, env: {}) return false if project.empty_repo? # Created or deleted branch if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev) false else - missed_ref, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list --max-count=1 #{oldrev} ^#{newrev})) - missed_ref.present? + missed_ref, exit_status = Gitlab::Git::RevList.new(oldrev, newrev, project: project, env: env).execute + + if exit_status == 0 + missed_ref.present? + else + raise "Got a non-zero exit code while calling out to `git rev-list` in the force-push check." + end end end end diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb new file mode 100644 index 00000000000..79dd0cf7df2 --- /dev/null +++ b/lib/gitlab/git/rev_list.rb @@ -0,0 +1,42 @@ +module Gitlab + module Git + class RevList + attr_reader :project, :env + + ALLOWED_VARIABLES = %w[GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES].freeze + + def initialize(oldrev, newrev, project:, env: nil) + @project = project + @env = env.presence || {} + @args = [Gitlab.config.git.bin_path, + "--git-dir=#{project.repository.path_to_repo}", + "rev-list", + "--max-count=1", + oldrev, + "^#{newrev}"] + end + + def execute + Gitlab::Popen.popen(@args, nil, parse_environment_variables) + end + + def valid? + environment_variables.all? do |(name, value)| + value.to_s.start_with?(project.repository.path_to_repo) + end + end + + private + + def parse_environment_variables + return {} unless valid? + + environment_variables + end + + def environment_variables + @environment_variables ||= env.slice(*ALLOWED_VARIABLES).compact + end + end + end +end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index bcbf6455998..74e8713de10 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -17,12 +17,13 @@ module Gitlab attr_reader :actor, :project, :protocol, :user_access, :authentication_abilities - def initialize(actor, project, protocol, authentication_abilities:) + def initialize(actor, project, protocol, authentication_abilities:, env: {}) @actor = actor @project = project @protocol = protocol @authentication_abilities = authentication_abilities @user_access = UserAccess.new(user, project: project) + @env = env end def check(cmd, changes) @@ -99,7 +100,7 @@ module Gitlab end def change_access_check(change) - Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec + Checks::ChangeAccess.new(change, user_access: user_access, project: project, env: @env).exec end def protocol_allowed? diff --git a/lib/gitlab/popen.rb b/lib/gitlab/popen.rb index cc74bb29087..4bc5cda8cb5 100644 --- a/lib/gitlab/popen.rb +++ b/lib/gitlab/popen.rb @@ -5,13 +5,13 @@ module Gitlab module Popen extend self - def popen(cmd, path = nil) + def popen(cmd, path = nil, vars = {}) unless cmd.is_a?(Array) raise "System commands must be given as an array of strings" end path ||= Dir.pwd - vars = { "PWD" => path } + vars['PWD'] = path options = { chdir: path } unless File.directory?(path) diff --git a/spec/lib/gitlab/checks/force_push_spec.rb b/spec/lib/gitlab/checks/force_push_spec.rb new file mode 100644 index 00000000000..f6288011494 --- /dev/null +++ b/spec/lib/gitlab/checks/force_push_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::Checks::ChangeAccess, lib: true do + let(:project) { create(:project) } + + context "exit code checking" do + it "does not raise a runtime error if the `popen` call to git returns a zero exit code" do + allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0]) + + expect { Gitlab::Checks::ForcePush.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error + end + + it "raises a runtime error if the `popen` call to git returns a non-zero exit code" do + allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1]) + + expect { Gitlab::Checks::ForcePush.force_push?(project, 'oldrev', 'newrev') }.to raise_error(RuntimeError) + end + end +end diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb new file mode 100644 index 00000000000..1f9c987be0b --- /dev/null +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -0,0 +1,60 @@ +require 'spec_helper' + +describe Gitlab::Git::RevList, lib: true do + let(:project) { create(:project) } + + context "validations" do + 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 = { var => "/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 = { 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 + end + + it "ignores nil values" do + env = { var => nil } + rev_list = described_class.new('oldrev', 'newrev', project: project, env: env) + + expect(rev_list).to be_valid + end + end + end + end + + context "#execute" do + let(:env) { { "GIT_OBJECT_DIRECTORY" => project.repository.path_to_repo } } + let(:rev_list) { Gitlab::Git::RevList.new('oldrev', 'newrev', project: project, env: env) } + + it "calls out to `popen` without environment variables if the record is invalid" do + allow(rev_list).to receive(:valid?).and_return(false) + + expect(Open3).to receive(: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) + + expect(Open3).to receive(:popen3).with(hash_including(env), any_args) + + rev_list.execute + end + end +end |