Commit 0e6757e8 authored by Nick Thomas's avatar Nick Thomas

Merge branch '6195-http-git-push-to-secondary-for-git-lfs-currently-does-not-work' into 'master'

Prep work to support git/git-lfs redirect to primary

See merge request gitlab-org/gitlab-ce!19728
parents a4618662 4203a8b3
......@@ -93,7 +93,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
end
def lfs_check_batch_operation!
if upload_request? && Gitlab::Database.read_only?
if batch_operation_disallowed?
render(
json: {
message: lfs_read_only_message
......@@ -104,6 +104,11 @@ class Projects::LfsApiController < Projects::GitHttpClientController
end
end
# Overridden in EE
def batch_operation_disallowed?
upload_request? && Gitlab::Database.read_only?
end
# Overridden in EE
def lfs_read_only_message
_('You cannot write to this read-only GitLab instance.')
......
......@@ -4,8 +4,18 @@ module Gitlab
class Controller
DISALLOWED_METHODS = %w(POST PATCH PUT DELETE).freeze
APPLICATION_JSON = 'application/json'.freeze
APPLICATION_JSON_TYPES = %W{#{APPLICATION_JSON} application/vnd.git-lfs+json}.freeze
ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'.freeze
WHITELISTED_GIT_ROUTES = {
'projects/git_http' => %w{git_upload_pack git_receive_pack}
}.freeze
WHITELISTED_GIT_LFS_ROUTES = {
'projects/lfs_api' => %w{batch},
'projects/lfs_locks_api' => %w{verify create unlock}
}.freeze
def initialize(app, env)
@app = app
@env = env
......@@ -36,7 +46,7 @@ module Gitlab
end
def json_request?
request.media_type == APPLICATION_JSON
APPLICATION_JSON_TYPES.include?(request.media_type)
end
def rack_flash
......@@ -63,22 +73,27 @@ module Gitlab
grack_route || ReadOnly.internal_routes.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route
end
def sidekiq_route
request.path.start_with?('/admin/sidekiq')
end
def grack_route
# Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.path.end_with?('.git/git-upload-pack')
return false unless
request.path.end_with?('.git/git-upload-pack', '.git/git-receive-pack')
route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack'
WHITELISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
def lfs_route
# Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.path.end_with?('/info/lfs/objects/batch')
unless request.path.end_with?('/info/lfs/objects/batch',
'/info/lfs/locks', '/info/lfs/locks/verify') ||
%r{/info/lfs/locks/\d+/unlock\z}.match?(request.path)
return false
end
WHITELISTED_GIT_LFS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch'
def sidekiq_route
request.path.start_with?('/admin/sidekiq')
end
end
end
......
......@@ -2,6 +2,7 @@ require 'spec_helper'
describe Gitlab::Middleware::ReadOnly do
include Rack::Test::Methods
using RSpec::Parameterized::TableSyntax
RSpec::Matchers.define :be_a_redirect do
match do |response|
......@@ -117,39 +118,41 @@ describe Gitlab::Middleware::ReadOnly do
context 'whitelisted requests' do
it 'expects a POST internal request to be allowed' do
expect(Rails.application.routes).not_to receive(:recognize_path)
response = request.post("/api/#{API::API.version}/internal")
expect(response).not_to be_a_redirect
expect(subject).not_to disallow_request
end
it 'expects a POST LFS request to batch URL to be allowed' do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/rouge.git/info/lfs/objects/batch')
it 'expects requests to sidekiq admin to be allowed' do
response = request.post('/admin/sidekiq')
expect(response).not_to be_a_redirect
expect(subject).not_to disallow_request
end
it 'expects a POST request to git-upload-pack URL to be allowed' do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/rouge.git/git-upload-pack')
response = request.get('/admin/sidekiq')
expect(response).not_to be_a_redirect
expect(subject).not_to disallow_request
end
it 'expects requests to sidekiq admin to be allowed' do
response = request.post('/admin/sidekiq')
expect(response).not_to be_a_redirect
expect(subject).not_to disallow_request
where(:description, :path) do
'LFS request to batch' | '/root/rouge.git/info/lfs/objects/batch'
'LFS request to locks verify' | '/root/rouge.git/info/lfs/locks/verify'
'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
'request to git-upload-pack' | '/root/rouge.git/git-upload-pack'
'request to git-receive-pack' | '/root/rouge.git/git-receive-pack'
end
response = request.get('/admin/sidekiq')
with_them do
it "expects a POST #{description} URL to be allowed" do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post(path)
expect(response).not_to be_a_redirect
expect(subject).not_to disallow_request
expect(response).not_to be_a_redirect
expect(subject).not_to disallow_request
end
end
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