summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2018-07-02 13:20:57 +0000
committerNick Thomas <nick@gitlab.com>2018-07-02 13:20:57 +0000
commit0e6757e8423d45b53a4224041aa4ea95d2dadf2a (patch)
treed3e24e1fc36dbf283b41caa0e3f985fadca1d5ab
parenta4618662f4424eb90bfcb768fa145099d33de348 (diff)
parent4203a8b30b9f9b1a060f9f377592f7d29e542e7c (diff)
downloadgitlab-ce-0e6757e8423d45b53a4224041aa4ea95d2dadf2a.tar.gz
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
-rw-r--r--app/controllers/projects/lfs_api_controller.rb7
-rw-r--r--lib/gitlab/middleware/read_only/controller.rb33
-rw-r--r--spec/lib/gitlab/middleware/read_only_spec.rb35
3 files changed, 49 insertions, 26 deletions
diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb
index ee4ed674110..3f4962b543d 100644
--- a/app/controllers/projects/lfs_api_controller.rb
+++ b/app/controllers/projects/lfs_api_controller.rb
@@ -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
@@ -105,6 +105,11 @@ class Projects::LfsApiController < Projects::GitHttpClientController
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.')
end
diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb
index 45b644e6510..4a99b7cca5c 100644
--- a/lib/gitlab/middleware/read_only/controller.rb
+++ b/lib/gitlab/middleware/read_only/controller.rb
@@ -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
diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb
index 39ec2f37a83..5c398bc2063 100644
--- a/spec/lib/gitlab/middleware/read_only_spec.rb
+++ b/spec/lib/gitlab/middleware/read_only_spec.rb
@@ -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