Commit 467b9e5d authored by Mike Kozono's avatar Mike Kozono

Geo: Fix LFS for single Git URL

When a Geo primary site receives a batch upload request, it will respond
with a URL to itself, rather than a custom clone URL configured in
https://docs.gitlab.com/ee/user/admin_area/settings/visibility_and_access_controls.html#custom-git-clone-url-for-https.
This is safe because it is already required that a Geo primary site be
accessible via its `external_url`, and in fact other parts of the
push-to-secondary request flow already depend on this to work.
parent 555c70b9
...@@ -91,12 +91,17 @@ module Repositories ...@@ -91,12 +91,17 @@ module Repositories
def upload_actions(object) def upload_actions(object)
{ {
upload: { upload: {
href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}", href: "#{upload_http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}",
header: upload_headers header: upload_headers
} }
} }
end end
# Overridden in EE
def upload_http_url_to_repo
project.http_url_to_repo
end
def upload_headers def upload_headers
headers = { headers = {
Authorization: authorization_header, Authorization: authorization_header,
......
...@@ -19,6 +19,13 @@ module EE ...@@ -19,6 +19,13 @@ module EE
false false
end end
override :upload_http_url_to_repo
def upload_http_url_to_repo
return geo_primary_http_url_to_repo(project) if ::Gitlab::Geo.primary?
super
end
override :lfs_read_only_message override :lfs_read_only_message
def lfs_read_only_message def lfs_read_only_message
return super unless ::Gitlab::Geo.secondary_with_primary? return super unless ::Gitlab::Geo.secondary_with_primary?
......
---
title: 'Geo: Fix LFS for location-aware Git URL'
merge_request: 50415
author:
type: fixed
...@@ -4,6 +4,7 @@ require 'spec_helper' ...@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe 'Git LFS API and storage' do RSpec.describe 'Git LFS API and storage' do
include WorkhorseHelpers include WorkhorseHelpers
include EE::GeoHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let!(:lfs_object) { create(:lfs_object, :with_file) } let!(:lfs_object) { create(:lfs_object, :with_file) }
...@@ -43,14 +44,10 @@ RSpec.describe 'Git LFS API and storage' do ...@@ -43,14 +44,10 @@ RSpec.describe 'Git LFS API and storage' do
end end
describe 'when handling lfs batch request' do describe 'when handling lfs batch request' do
let(:update_lfs_permissions) { } subject(:batch_request) { post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers }
let(:update_user_permissions) { }
before do before do
enable_lfs enable_lfs
update_lfs_permissions
update_user_permissions
post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
end end
describe 'upload' do describe 'upload' do
...@@ -70,7 +67,7 @@ RSpec.describe 'Git LFS API and storage' do ...@@ -70,7 +67,7 @@ RSpec.describe 'Git LFS API and storage' do
let(:sample_oid) { '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' } let(:sample_oid) { '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' }
context 'and project is above the limit' do context 'and project is above the limit' do
let(:update_lfs_permissions) do before do
allow_next_instance_of(Gitlab::RepositorySizeChecker) do |checker| allow_next_instance_of(Gitlab::RepositorySizeChecker) do |checker|
allow(checker).to receive_messages( allow(checker).to receive_messages(
enabled?: true, enabled?: true,
...@@ -81,13 +78,15 @@ RSpec.describe 'Git LFS API and storage' do ...@@ -81,13 +78,15 @@ RSpec.describe 'Git LFS API and storage' do
end end
it 'responds with status 406' do it 'responds with status 406' do
batch_request
expect(response).to have_gitlab_http_status(:not_acceptable) expect(response).to have_gitlab_http_status(:not_acceptable)
expect(json_response['message']).to eql('Your push has been rejected, because this repository has exceeded its size limit of 100 MB by 160 MB. Please contact your GitLab administrator for more information.') expect(json_response['message']).to eql('Your push has been rejected, because this repository has exceeded its size limit of 100 MB by 160 MB. Please contact your GitLab administrator for more information.')
end end
end end
context 'and project will go over the limit' do context 'and project will go over the limit' do
let(:update_lfs_permissions) do before do
allow_next_instance_of(Gitlab::RepositorySizeChecker) do |checker| allow_next_instance_of(Gitlab::RepositorySizeChecker) do |checker|
allow(checker).to receive_messages( allow(checker).to receive_messages(
enabled?: true, enabled?: true,
...@@ -98,6 +97,8 @@ RSpec.describe 'Git LFS API and storage' do ...@@ -98,6 +97,8 @@ RSpec.describe 'Git LFS API and storage' do
end end
it 'responds with status 406' do it 'responds with status 406' do
batch_request
expect(response).to have_gitlab_http_status(:not_acceptable) expect(response).to have_gitlab_http_status(:not_acceptable)
expect(json_response['documentation_url']).to include('/help') expect(json_response['documentation_url']).to include('/help')
expect(json_response['message']).to eql('Your push has been rejected, because this repository has exceeded its size limit of 300 MB by 50 MB. Please contact your GitLab administrator for more information.') expect(json_response['message']).to eql('Your push has been rejected, because this repository has exceeded its size limit of 300 MB by 50 MB. Please contact your GitLab administrator for more information.')
...@@ -108,18 +109,75 @@ RSpec.describe 'Git LFS API and storage' do ...@@ -108,18 +109,75 @@ RSpec.describe 'Git LFS API and storage' do
describe 'when request is authenticated' do describe 'when request is authenticated' do
context 'when user has project push access' do context 'when user has project push access' do
let(:authorization) { authorize_user } let(:authorization) { authorize_user }
let(:update_user_permissions) { project.add_developer(user) }
before do
project.add_developer(user)
end
context 'when pushing a lfs object that does not exist' do context 'when pushing a lfs object that does not exist' do
it_behaves_like 'pushes new LFS objects' it_behaves_like 'pushes new LFS objects'
end end
context 'when Geo is not enabled' do
context 'when custom_http_clone_url_root is not configured' do
it 'returns hrefs based on external_url' do
batch_request
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['objects'].first['actions']['upload']['href']).to start_with(Gitlab::Routing.url_helpers.root_url)
end
end
context 'when custom_http_clone_url_root is configured' do
before do
stub_application_setting(custom_http_clone_url_root: 'http://customized')
end
it 'returns hrefs based on custom_http_clone_url_root' do
batch_request
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['objects'].first['actions']['upload']['href']).to start_with('http://customized')
end
end
end
context 'when this site is a Geo primary site' do
let(:primary) { create(:geo_node, :primary) }
before do
stub_current_geo_node(primary)
end
context 'when custom_http_clone_url_root is not configured' do
it 'returns hrefs based on the Geo primary site URL' do
batch_request
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['objects'].first['actions']['upload']['href']).to start_with(primary.url)
end
end
context 'when custom_http_clone_url_root is configured' do
before do
stub_application_setting(custom_http_clone_url_root: 'http://customized')
end
it 'returns hrefs based on the Geo primary site URL' do
batch_request
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['objects'].first['actions']['upload']['href']).to start_with(primary.url)
end
end
end
end end
context 'when deploy key has project push access' do context 'when deploy key has project push access' do
let(:key) { create(:deploy_key) } let(:key) { create(:deploy_key) }
let(:authorization) { authorize_deploy_key } let(:authorization) { authorize_deploy_key }
let(:update_user_permissions) do before do
project.deploy_keys_projects.create(deploy_key: key, can_push: true) project.deploy_keys_projects.create(deploy_key: key, can_push: true)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment