Commit 6c339816 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'sh-optimize-read-only-check' into 'master'

Optimize read-only middleware so that it does not consume as much CPU

Closes #40185 and gitlab-com/infrastructure#3240

See merge request gitlab-org/gitlab-ce!15504
parents 9961ac03 aeb2f49f
...@@ -58,7 +58,7 @@ module Gitlab ...@@ -58,7 +58,7 @@ module Gitlab
end end
def last_visited_url def last_visited_url
@env['HTTP_REFERER'] || rack_session['user_return_to'] || Rails.application.routes.url_helpers.root_url @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url
end end
def route_hash def route_hash
...@@ -74,10 +74,16 @@ module Gitlab ...@@ -74,10 +74,16 @@ module Gitlab
end end
def grack_route 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')
route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack'
end end
def lfs_route 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')
route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch'
end end
end end
......
...@@ -84,14 +84,23 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -84,14 +84,23 @@ describe Gitlab::Middleware::ReadOnly do
end end
it 'expects POST of new file that looks like an LFS batch url to be disallowed' do it 'expects POST of new file that looks like an LFS batch url to be disallowed' do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch') response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch')
expect(response).to be_a_redirect expect(response).to be_a_redirect
expect(subject).to disallow_request expect(subject).to disallow_request
end end
it 'returns last_vistited_url for disallowed request' do
response = request.post('/test_request')
expect(response.location).to eq 'http://localhost/'
end
context 'whitelisted requests' do context 'whitelisted requests' do
it 'expects a POST internal request to be allowed' 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") response = request.post("/api/#{API::API.version}/internal")
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
...@@ -99,6 +108,7 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -99,6 +108,7 @@ describe Gitlab::Middleware::ReadOnly do
end end
it 'expects a POST LFS request to batch URL to be allowed' do 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') response = request.post('/root/rouge.git/info/lfs/objects/batch')
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
...@@ -106,6 +116,7 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -106,6 +116,7 @@ describe Gitlab::Middleware::ReadOnly do
end end
it 'expects a POST request to git-upload-pack URL to be allowed' do 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.post('/root/rouge.git/git-upload-pack')
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
......
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