use upstream patch for git 2.11 support backport

This commit is contained in:
Praveen Arimbrathodiyil 2017-02-07 11:23:02 +05:30
parent 4d3caf404e
commit f6c070a8a4
3 changed files with 207 additions and 260 deletions

View file

@ -1,227 +0,0 @@
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -3,6 +3,8 @@
class Internal < Grape::API
before { authenticate_by_gitlab_shell_token! }
+ helpers ::API::Helpers::InternalHelpers
+
namespace 'internal' do
# Check if git command is allowed to project
#
@@ -14,36 +16,6 @@
# ref - branch name
# forced_push - forced_push
# protocol - Git access protocol being used, e.g. HTTP or SSH
- #
-
- helpers do
- def wiki?
- @wiki ||= params[:project].end_with?('.wiki') &&
- !Project.find_with_namespace(params[:project])
- end
-
- def project
- @project ||= begin
- project_path = params[:project]
-
- # Check for *.wiki repositories.
- # Strip out the .wiki from the pathname before finding the
- # project. This applies the correct project permissions to
- # the wiki repository as well.
- project_path.chomp!('.wiki') if wiki?
-
- Project.find_with_namespace(project_path)
- end
- end
-
- def ssh_authentication_abilities
- [
- :read_project,
- :download_code,
- :push_code
- ]
- end
- end
post "/allowed" do
status 200
--- /dev/null
+++ b/lib/api/helpers/internal_helpers.rb
@@ -0,0 +1,57 @@
+module API
+ module Helpers
+ module InternalHelpers
+ # Project paths may be any of the following:
+ # * /repository/storage/path/namespace/project
+ # * /namespace/project
+ # * namespace/project
+ #
+ # In addition, they may have a '.git' extension and multiple namespaces
+ #
+ # Transform all these cases to 'namespace/project'
+ def clean_project_path(project_path, storage_paths = Repository.storages.values)
+ project_path = project_path.sub(/\.git\z/, '')
+
+ storage_paths.each do |storage_path|
+ storage_path = File.expand_path(storage_path)
+
+ if project_path.start_with?(storage_path)
+ project_path = project_path.sub(storage_path, '')
+ break
+ end
+ end
+
+ project_path.sub(/\A\//, '')
+ end
+
+ def project_path
+ @project_path ||= clean_project_path(params[:project])
+ end
+
+ def wiki?
+ @wiki ||= project_path.end_with?('.wiki') &&
+ !Project.find_with_namespace(project_path)
+ end
+
+ def project
+ @project ||= begin
+ # Check for *.wiki repositories.
+ # Strip out the .wiki from the pathname before finding the
+ # project. This applies the correct project permissions to
+ # the wiki repository as well.
+ project_path.chomp!('.wiki') if wiki?
+
+ Project.find_with_namespace(project_path)
+ end
+ end
+
+ def ssh_authentication_abilities
+ [
+ :read_project,
+ :download_code,
+ :push_code
+ ]
+ end
+ end
+ end
+end
--- /dev/null
+++ b/spec/requests/api/api_internal_helpers_spec.rb
@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe ::API::Helpers::InternalHelpers do
+ include ::API::Helpers::InternalHelpers
+
+ describe '.clean_project_path' do
+ project = 'namespace/project'
+ namespaced = File.join('namespace2', project)
+
+ {
+ File.join(Dir.pwd, project) => project,
+ File.join(Dir.pwd, namespaced) => namespaced,
+ project => project,
+ namespaced => namespaced,
+ project + '.git' => project,
+ namespaced + '.git' => namespaced,
+ "/" + project => project,
+ "/" + namespaced => namespaced,
+ }.each do |project_path, expected|
+ context project_path do
+ # Relative and absolute storage paths, with and without trailing /
+ ['.', './', Dir.pwd, Dir.pwd + '/'].each do |storage_path|
+ context "storage path is #{storage_path}" do
+ subject { clean_project_path(project_path, [storage_path]) }
+
+ it { is_expected.to eq(expected) }
+ end
+ end
+ end
+ end
+ end
+end
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -191,6 +191,26 @@
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
end
+
+ context 'project as /namespace/project' do
+ it do
+ pull(key, project_with_repo_path('/' + project.path_with_namespace))
+
+ expect(response).to have_http_status(200)
+ expect(json_response["status"]).to be_truthy
+ expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
+ end
+ end
+
+ context 'project as namespace/project' do
+ it do
+ pull(key, project_with_repo_path(project.path_with_namespace))
+
+ expect(response).to have_http_status(200)
+ expect(json_response["status"]).to be_truthy
+ expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
+ end
+ end
end
end
@@ -299,7 +319,7 @@
context 'project does not exist' do
it do
- pull(key, OpenStruct.new(path_with_namespace: 'gitlab/notexists'))
+ pull(key, project_with_repo_path('gitlab/notexist'))
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_falsey
@@ -392,11 +412,17 @@
end
end
+ def project_with_repo_path(path)
+ double().tap do |fake_project|
+ allow(fake_project).to receive_message_chain('repository.path_to_repo' => path)
+ end
+ end
+
def pull(key, project, protocol = 'ssh')
post(
api("/internal/allowed"),
key_id: key.id,
- project: project.path_with_namespace,
+ project: project.repository.path_to_repo,
action: 'git-upload-pack',
secret_token: secret_token,
protocol: protocol
@@ -408,7 +434,7 @@
api("/internal/allowed"),
changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master',
key_id: key.id,
- project: project.path_with_namespace,
+ project: project.repository.path_to_repo,
action: 'git-receive-pack',
secret_token: secret_token,
protocol: protocol
@@ -420,7 +446,7 @@
api("/internal/allowed"),
ref: 'master',
key_id: key.id,
- project: project.path_with_namespace,
+ project: project.repository.path_to_repo,
action: 'git-upload-archive',
secret_token: secret_token,
protocol: 'ssh'
@@ -432,7 +458,7 @@
api("/internal/lfs_authenticate"),
key_id: key_id,
secret_token: secret_token,
- project: project.path_with_namespace
+ project: project.repository.path_to_repo
)
end
end

View file

@ -1,27 +1,81 @@
From 8f3edeb7d50e8e8176627f668cda489935eccdf3 Mon Sep 17 00:00:00 2001
From daf83fa62c940b0da7dc4e0893586b6a9a2dbbf9 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 19 Dec 2016 09:37:16 +0000
Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Accept environment variables from the `pre-receive` script ## Summary 1. Starting version 2.11, git changed the way the pre-receive flow works. - Previously, the new potential objects would be added to the main repo. If the pre-receive passes, the new objects stay in the repo but are linked up. If the pre-receive fails, the new objects stay orphaned in the repo, and are cleaned up during the next `git gc`. - In 2.11, the new potential objects are added to a temporary "alternate object directory", that git creates for this purpose. If the pre-receive passes, the objects from the alternate object directory are migrated to the main repo. If the pre-receive fails the alternate object directory is simply deleted. 2. In our workflow, the pre-recieve script (in `gitlab-shell`) calls the `/allowed` endpoint, which calls out directly to git to perform various checks. These direct calls to git do _not_ have the necessary environment variables set which allow access to the "alternate object directory" (explained above). Therefore these calls to git are not able to access any of the new potential objects to be added during this push. 3. We fix this by accepting the relevant environment variables (`GIT_ALTERNATE_OBJECT_DIRECTORIES`, `GIT_OBJECT_DIRECTORY`, and `GIT_QUARANTINE_PATH`) on the `/allowed` endpoint, and then include these environment variables while calling out to git. 4. This commit includes these environment variables while making the "force push" check. ## Issue Numbers - Closes #25301 (assuming the corresponding `gitlab-shell` MR has been merged in first) - Corresponding `gitlab-shell` MR: gitlab-org/gitlab-shell!112 - Corresponding EE MR: gitlab-org/gitlab-ee!964 ## Tasks - [#25301/!7967/!112] Git version 2.11.0 - Can't push to protected branch as master or developer - [x] Investigate - [x] Implementation - [x] `force_push.rb` should use the relevant environment variables - [x] Any other instances of `/allowed` calling out to git directly? - [x] Verify that the fix works over SSH as well - [x] Can we trim the number of env variables? Do we need all 3? - [x] Whitelist variables. Server shouldn't pass through _any_ env variable passed in - [x] Any security implications? - [x] Check for force push return code - [x] Shouldn't be able to opt-out from the force push check by passing an env variable - [x] Tests - [x] CE - [x] Added - [x] Passing - [x] Shell - [x] Added - [x] Passing - [x] Meta - [x] CHANGELOG entry created - [x] Branch has no merge conflicts with `master` - [x] Squashed related commits together - [x] EE merge request - [x] Review - [x] Endboss - [ ] Follow-up - [x] Make sure EE is working as expected - [x] [CE] Gitlab changes without gitlab-shell changes shouldn't raise any exceptions - [x] [CE] Gitlab-shell changes without gitlab changes shouldn't raise any exceptions - [x] [EE] Gitlab changes without gitlab-shell changes shouldn't raise any exceptions - [x] [EE] Gitlab-shell changes without gitlab changes shouldn't raise any exceptions - [ ] Wait for merge - [ ] CE - [ ] EE - [x] Shell See merge request !7967
Subject: [PATCH 1/3] [8.13 Backport] Merge branch
'25301-git-2.11-force-push-bug' into 'master'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Accept environment variables from the `pre-receive` script
1. Starting version 2.11, git changed the way the pre-receive flow works.
- Previously, the new potential objects would be added to the main repo. If the pre-receive passes, the new objects stay in the repo but are linked up. If the pre-receive fails, the new objects stay orphaned in the repo, and are cleaned up during the next `git gc`.
- In 2.11, the new potential objects are added to a temporary "alternate object directory", that git creates for this purpose. If the pre-receive passes, the objects from the alternate object directory are migrated to the main repo. If the pre-receive fails the alternate object directory is simply deleted.
2. In our workflow, the pre-recieve script (in `gitlab-shell`) calls the
`/allowed` endpoint, which calls out directly to git to perform
various checks. These direct calls to git do _not_ have the necessary
environment variables set which allow access to the "alternate object
directory" (explained above). Therefore these calls to git are not able to
access any of the new potential objects to be added during this push.
3. We fix this by accepting the relevant environment variables
(`GIT_ALTERNATE_OBJECT_DIRECTORIES`, `GIT_OBJECT_DIRECTORY`, and
`GIT_QUARANTINE_PATH`) on the `/allowed` endpoint, and then include
these environment variables while calling out to git.
4. This commit includes these environment variables while making the "force
push" check.
See https://gitlab.com/gitlab-org/gitlab-shell/merge_requests/120
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
lib/api/helpers/internal_helpers.rb | 8 ++++++++
lib/api/internal.rb | 6 +++++-
lib/gitlab/checks/change_access.rb | 5 +++--
lib/gitlab/checks/force_push.rb | 11 ++++++++---
lib/gitlab/git/rev_list.rb | 42 ++++++++++++++++++++++++++++++++++++++++++
lib/gitlab/git_access.rb | 5 +++--
lib/gitlab/popen.rb | 4 ++--
spec/lib/gitlab/checks/force_push_spec.rb | 19 +++++++++++++++++++
spec/lib/gitlab/git/rev_list_spec.rb | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 147 insertions(+), 10 deletions(-)
.../unreleased/25301-git-2-11-force-push-bug.yml | 4 ++
lib/api/internal.rb | 14 +++++-
lib/gitlab/checks/change_access.rb | 5 +-
lib/gitlab/checks/force_push.rb | 11 +++--
lib/gitlab/git/rev_list.rb | 42 +++++++++++++++++
lib/gitlab/git_access.rb | 5 +-
lib/gitlab/popen.rb | 4 +-
spec/lib/gitlab/checks/force_push_spec.rb | 19 ++++++++
spec/lib/gitlab/git/rev_list_spec.rb | 53 ++++++++++++++++++++++
9 files changed, 147 insertions(+), 10 deletions(-)
create mode 100644 changelogs/unreleased/25301-git-2-11-force-push-bug.yml
create mode 100644 lib/gitlab/git/rev_list.rb
create mode 100644 spec/lib/gitlab/checks/force_push_spec.rb
create mode 100644 spec/lib/gitlab/git/rev_list_spec.rb
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 0000000..afe5772
--- /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/lib/api/internal.rb b/lib/api/internal.rb
index 9a5d1ec..89e47a7 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -33,7 +33,11 @@
@@ -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
@ -34,9 +88,11 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
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 cb10652..3d20301 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -3,11 +3,12 @@
@@ -3,11 +3,12 @@ module Gitlab
class ChangeAccess
attr_reader :user_access, :project
@ -50,7 +106,7 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
end
def exec
@@ -68,7 +69,7 @@
@@ -68,7 +69,7 @@ module Gitlab
end
def forced_push?
@ -59,6 +115,8 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
end
def matching_merge_request?
diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb
index 5fe8655..de0c904 100644
--- a/lib/gitlab/checks/force_push.rb
+++ b/lib/gitlab/checks/force_push.rb
@@ -1,15 +1,20 @@
@ -85,6 +143,9 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
end
end
end
diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb
new file mode 100644
index 0000000..25e9d61
--- /dev/null
+++ b/lib/gitlab/git/rev_list.rb
@@ -0,0 +1,42 @@
@ -130,9 +191,11 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
+ end
+ end
+end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index bcbf645..74e8713 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -17,12 +17,13 @@
@@ -17,12 +17,13 @@ module Gitlab
attr_reader :actor, :project, :protocol, :user_access, :authentication_abilities
@ -147,7 +210,7 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
end
def check(cmd, changes)
@@ -99,7 +100,7 @@
@@ -99,7 +100,7 @@ module Gitlab
end
def change_access_check(change)
@ -156,9 +219,11 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
end
def protocol_allowed?
diff --git a/lib/gitlab/popen.rb b/lib/gitlab/popen.rb
index cc74bb2..4bc5cda 100644
--- a/lib/gitlab/popen.rb
+++ b/lib/gitlab/popen.rb
@@ -5,13 +5,13 @@
@@ -5,13 +5,13 @@ module Gitlab
module Popen
extend self
@ -174,6 +239,9 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
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 0000000..f628801
--- /dev/null
+++ b/spec/lib/gitlab/checks/force_push_spec.rb
@@ -0,0 +1,19 @@
@ -196,6 +264,9 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
+ 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 0000000..444639a
--- /dev/null
+++ b/spec/lib/gitlab/git/rev_list_spec.rb
@@ -0,0 +1,53 @@
@ -252,20 +323,124 @@ Subject: [PATCH] Merge branch '25301-git-2.11-force-push-bug' into 'master' Acce
+ end
+ end
+end
--- a/lib/api/helpers/internal_helpers.rb
+++ b/lib/api/helpers/internal_helpers.rb
@@ -52,6 +52,14 @@
:push_code
]
--
2.10.2
From 0ce20138298eaebfb9e8225d21e7b0088716e5ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Tue, 20 Dec 2016 09:45:37 +0100
Subject: [PATCH 2/3] Reject blank environment vcariables in
Gitlab::Git::RevList
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
lib/gitlab/git/rev_list.rb | 4 ++--
spec/lib/gitlab/git/rev_list_spec.rb | 7 +++++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb
index 25e9d61..79dd0cf 100644
--- a/lib/gitlab/git/rev_list.rb
+++ b/lib/gitlab/git/rev_list.rb
@@ -22,7 +22,7 @@ module Gitlab
def valid?
environment_variables.all? do |(name, value)|
- value.start_with?(project.repository.path_to_repo)
+ value.to_s.start_with?(project.repository.path_to_repo)
end
end
@@ -35,7 +35,7 @@ module Gitlab
end
def environment_variables
- @environment_variables ||= env.slice(*ALLOWED_VARIABLES)
+ @environment_variables ||= env.slice(*ALLOWED_VARIABLES).compact
end
+
+ def parse_allowed_environment_variables
+ return if params[:env].blank?
+
+ JSON.parse(params[:env])
+
+ rescue JSON::ParserError
+ end
end
end
end
diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb
index 444639a..1f9c987 100644
--- a/spec/lib/gitlab/git/rev_list_spec.rb
+++ b/spec/lib/gitlab/git/rev_list_spec.rb
@@ -26,6 +26,13 @@ describe Gitlab::Git::RevList, lib: true do
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
--
2.10.2
From b54b031638e7a98c1e51b369cff53602db40e4b0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Mon, 6 Feb 2017 10:04:21 +0100
Subject: [PATCH 3/3] Update gitlab-shell to 3.6.7
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
GITLAB_SHELL_VERSION | 2 +-
changelogs/unreleased/use-gitlab-shell-3-6-7.yml | 4 ++++
doc/update/8.12-to-8.13.md | 4 ++--
3 files changed, 7 insertions(+), 3 deletions(-)
create mode 100644 changelogs/unreleased/use-gitlab-shell-3-6-7.yml
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION
index 4f2c1d1..5b34131 100644
--- a/GITLAB_SHELL_VERSION
+++ b/GITLAB_SHELL_VERSION
@@ -1 +1 @@
-3.6.6
+3.6.7
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 0000000..c6600ce
--- /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 c0084d9..6457ec9 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
--
2.10.2

View file

@ -8,5 +8,4 @@ pid-log-paths.patch
052-relax-grape.patch
0200-remove-order-dependency-in-label-finder-spec.patch
0210-use-jquery-ui-rails6.patch
0250-gitlab-shell-4.0-compat.patch
0300-git-2-11-support.patch