Commit 43eae78d authored by Ash McKenzie's avatar Ash McKenzie Committed by Douglas Barbosa Alexandre

Rename Geo SSH Proxy methods for consistency

Method names now more closely match the actual
git commands that are being executed under the
hood.
parent ebe3a79c
...@@ -71,11 +71,11 @@ module API ...@@ -71,11 +71,11 @@ module API
requires :primary_repo, type: String requires :primary_repo, type: String
end end
end end
post 'info_refs' do post 'info_refs_receive_pack' do
authenticate_by_gitlab_shell_token! authenticate_by_gitlab_shell_token!
params.delete(:secret_token) params.delete(:secret_token)
response = Gitlab::Geo::GitSSHProxy.new(params['data']).info_refs response = Gitlab::Geo::GitSSHProxy.new(params['data']).info_refs_receive_pack
status(response.code) status(response.code)
response.body response.body
end end
...@@ -91,11 +91,11 @@ module API ...@@ -91,11 +91,11 @@ module API
end end
requires :output, type: String, desc: 'Output from git-receive-pack' requires :output, type: String, desc: 'Output from git-receive-pack'
end end
post 'push' do post 'receive_pack' do
authenticate_by_gitlab_shell_token! authenticate_by_gitlab_shell_token!
params.delete(:secret_token) params.delete(:secret_token)
response = Gitlab::Geo::GitSSHProxy.new(params['data']).push(params['output']) response = Gitlab::Geo::GitSSHProxy.new(params['data']).receive_pack(params['output'])
status(response.code) status(response.code)
response.body response.body
end end
......
...@@ -87,8 +87,8 @@ module EE ...@@ -87,8 +87,8 @@ module EE
def custom_action_api_endpoints def custom_action_api_endpoints
[ [
api_v4_geo_proxy_git_push_ssh_info_refs_path, api_v4_geo_proxy_git_push_ssh_info_receive_pack_path,
api_v4_geo_proxy_git_push_ssh_push_path api_v4_geo_proxy_git_push_ssh_receive_pack_path
] ]
end end
end end
......
...@@ -39,8 +39,10 @@ module EE ...@@ -39,8 +39,10 @@ module EE
def geo_proxy_git_push_ssh_route? def geo_proxy_git_push_ssh_route?
routes = ::Gitlab::Middleware::ReadOnly::API_VERSIONS.map do |version| routes = ::Gitlab::Middleware::ReadOnly::API_VERSIONS.map do |version|
%W(/api/v#{version}/geo/proxy_git_push_ssh/info_refs %W(
/api/v#{version}/geo/proxy_git_push_ssh/push) /api/v#{version}/geo/proxy_git_push_ssh/info_refs_receive_pack
/api/v#{version}/geo/proxy_git_push_ssh/receive_pack
)
end end
routes.flatten.include?(request.path) routes.flatten.include?(request.path)
......
...@@ -5,9 +5,8 @@ module Gitlab ...@@ -5,9 +5,8 @@ module Gitlab
class GitSSHProxy class GitSSHProxy
HTTP_READ_TIMEOUT = 60 HTTP_READ_TIMEOUT = 60
INFO_REFS_CONTENT_TYPE = 'application/x-git-upload-pack-request'.freeze RECEIVE_PACK_REQUEST_CONTENT_TYPE = 'application/x-git-receive-pack-request'.freeze
PUSH_CONTENT_TYPE = 'application/x-git-receive-pack-request'.freeze RECEIVE_PACK_RESULT_CONTENT_TYPE = 'application/x-git-receive-pack-result'.freeze
PUSH_ACCEPT = 'application/x-git-receive-pack-result'.freeze
MustBeASecondaryNode = Class.new(StandardError) MustBeASecondaryNode = Class.new(StandardError)
...@@ -50,28 +49,30 @@ module Gitlab ...@@ -50,28 +49,30 @@ module Gitlab
@data = data @data = data
end end
def info_refs # For git push
def info_refs_receive_pack
ensure_secondary! ensure_secondary!
url = "#{primary_repo}/info/refs?service=git-receive-pack" url = "#{primary_repo}/info/refs?service=git-receive-pack"
headers = { 'Content-Type' => INFO_REFS_CONTENT_TYPE } headers = { 'Content-Type' => RECEIVE_PACK_REQUEST_CONTENT_TYPE }
resp = get(url, headers) resp = get(url, headers)
resp.body = remove_http_service_fragment_from(resp.body) if resp.is_a?(Net::HTTPSuccess) resp.body = remove_receive_pack_http_service_fragment_from(resp.body) if resp.is_a?(Net::HTTPSuccess)
APIResponse.from_http_response(resp, primary_repo) APIResponse.from_http_response(resp, primary_repo)
rescue => e rescue => e
handle_exception(e) handle_exception(e)
end end
def push(encoded_info_refs_response) # For git push
def receive_pack(encoded_info_refs_response)
ensure_secondary! ensure_secondary!
url = "#{primary_repo}/git-receive-pack" url = "#{primary_repo}/git-receive-pack"
headers = { 'Content-Type' => PUSH_CONTENT_TYPE, 'Accept' => PUSH_ACCEPT } headers = { 'Content-Type' => RECEIVE_PACK_REQUEST_CONTENT_TYPE, 'Accept' => RECEIVE_PACK_RESULT_CONTENT_TYPE }
info_refs_response = Base64.decode64(encoded_info_refs_response) info_refs_response = Base64.decode64(encoded_info_refs_response)
resp = post(url, info_refs_response, headers) resp = post(url, info_refs_response, headers)
APIResponse.from_http_response(resp, primary_repo) APIResponse.from_http_response(resp, primary_repo)
rescue => e rescue => e
handle_exception(e) handle_exception(e)
...@@ -130,7 +131,7 @@ module Gitlab ...@@ -130,7 +131,7 @@ module Gitlab
http.start { http.request(req) } http.start { http.request(req) }
end end
def remove_http_service_fragment_from(body) def remove_receive_pack_http_service_fragment_from(body)
# HTTP(S) and SSH responses are very similar, except for the fragment below. # HTTP(S) and SSH responses are very similar, except for the fragment below.
# As we're performing a git HTTP(S) request here, we'll get a HTTP(s) # As we're performing a git HTTP(S) request here, we'll get a HTTP(s)
# suitable git response. However, we're executing in the context of an # suitable git response. However, we're executing in the context of an
...@@ -139,6 +140,7 @@ module Gitlab ...@@ -139,6 +140,7 @@ module Gitlab
# #
# See Downloading Data > HTTP(S) section at: # See Downloading Data > HTTP(S) section at:
# https://git-scm.com/book/en/v2/Git-Internals-Transfer-Protocols # https://git-scm.com/book/en/v2/Git-Internals-Transfer-Protocols
#
body.gsub(/\A001f# service=git-receive-pack\n0000/, '') body.gsub(/\A001f# service=git-receive-pack\n0000/, '')
end end
......
...@@ -47,13 +47,13 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -47,13 +47,13 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
allow(base_request).to receive(:authorization).and_return('secret') allow(base_request).to receive(:authorization).and_return('secret')
end end
describe '#info_refs' do describe '#info_refs_receive_pack' do
context 'against primary node' do context 'against primary node' do
let(:current_node) { primary_node } let(:current_node) { primary_node }
it 'raises an exception' do it 'raises an exception' do
expect do expect do
subject.info_refs subject.info_refs_receive_pack
end.to raise_error(described_class::MustBeASecondaryNode, 'Node is not a secondary or there is no primary Geo node') end.to raise_error(described_class::MustBeASecondaryNode, 'Node is not a secondary or there is no primary Geo node')
end end
end end
...@@ -62,20 +62,20 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -62,20 +62,20 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
let(:current_node) { secondary_node } let(:current_node) { secondary_node }
let(:full_info_refs_url) { "#{primary_repo_http}/info/refs?service=git-receive-pack" } let(:full_info_refs_url) { "#{primary_repo_http}/info/refs?service=git-receive-pack" }
let(:info_refs_headers) { base_headers.merge('Content-Type' => 'application/x-git-upload-pack-request') } let(:info_refs_receive_pack_headers) { base_headers.merge('Content-Type' => 'application/x-git-upload-pack-request') }
let(:info_refs_http_body_full) { "001f# service=git-receive-pack\n0000#{info_refs_body_short}" } let(:info_refs_http_body_full) { "001f# service=git-receive-pack\n0000#{info_refs_body_short}" }
context 'authorization header is scoped' do context 'authorization header is scoped' do
it 'passes the scope when .info_refs is called' do it 'passes the scope when .info_refs_receive_pack is called' do
expect(Gitlab::Geo::BaseRequest).to receive(:new).with(scope: project.repository.full_path) expect(Gitlab::Geo::BaseRequest).to receive(:new).with(scope: project.repository.full_path)
subject.info_refs subject.info_refs_receive_pack
end end
it 'passes the scope when .push is called' do it 'passes the scope when .receive_pack is called' do
expect(Gitlab::Geo::BaseRequest).to receive(:new).with(scope: project.repository.full_path) expect(Gitlab::Geo::BaseRequest).to receive(:new).with(scope: project.repository.full_path)
subject.push(info_refs_body_short) subject.receive_pack(info_refs_body_short)
end end
end end
...@@ -87,23 +87,23 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -87,23 +87,23 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
end end
it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do
expect(subject.info_refs).to be_a(Gitlab::Geo::GitSSHProxy::FailedAPIResponse) expect(subject.info_refs_receive_pack).to be_a(Gitlab::Geo::GitSSHProxy::FailedAPIResponse)
end end
it 'has a code of 500' do it 'has a code of 500' do
expect(subject.info_refs.code).to be(500) expect(subject.info_refs_receive_pack.code).to be(500)
end end
it 'has a status of false' do it 'has a status of false' do
expect(subject.info_refs.body[:status]).to be_falsey expect(subject.info_refs_receive_pack.body[:status]).to be_falsey
end end
it 'has a messsage' do it 'has a messsage' do
expect(subject.info_refs.body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}") expect(subject.info_refs_receive_pack.body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}")
end end
it 'has no result' do it 'has no result' do
expect(subject.info_refs.body[:result]).to be_nil expect(subject.info_refs_receive_pack.body[:result]).to be_nil
end end
end end
...@@ -111,65 +111,65 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -111,65 +111,65 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
let(:error_msg) { 'dial unix /Users/ash/src/gdk/gdk-ee/gitlab.socket: connect: connection refused' } let(:error_msg) { 'dial unix /Users/ash/src/gdk/gdk-ee/gitlab.socket: connect: connection refused' }
before do before do
stub_request(:get, full_info_refs_url).to_return(status: 502, body: error_msg, headers: info_refs_headers) stub_request(:get, full_info_refs_url).to_return(status: 502, body: error_msg, headers: info_refs_receive_pack_headers)
end end
it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do
expect(subject.info_refs).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse) expect(subject.info_refs_receive_pack).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse)
end end
it 'has a code of 502' do it 'has a code of 502' do
expect(subject.info_refs.code).to be(502) expect(subject.info_refs_receive_pack.code).to be(502)
end end
it 'has a status of false' do it 'has a status of false' do
expect(subject.info_refs.body[:status]).to be_falsey expect(subject.info_refs_receive_pack.body[:status]).to be_falsey
end end
it 'has a messsage' do it 'has a messsage' do
expect(subject.info_refs.body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}") expect(subject.info_refs_receive_pack.body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}")
end end
it 'has no result' do it 'has no result' do
expect(subject.info_refs.body[:result]).to be_nil expect(subject.info_refs_receive_pack.body[:result]).to be_nil
end end
end end
context 'with a valid response' do context 'with a valid response' do
before do before do
stub_request(:get, full_info_refs_url).to_return(status: 200, body: info_refs_http_body_full, headers: info_refs_headers) stub_request(:get, full_info_refs_url).to_return(status: 200, body: info_refs_http_body_full, headers: info_refs_receive_pack_headers)
end end
it 'returns a Gitlab::Geo::GitSSHProxy::APIResponse' do it 'returns a Gitlab::Geo::GitSSHProxy::APIResponse' do
expect(subject.info_refs).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse) expect(subject.info_refs_receive_pack).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse)
end end
it 'has a code of 200' do it 'has a code of 200' do
expect(subject.info_refs.code).to be(200) expect(subject.info_refs_receive_pack.code).to be(200)
end end
it 'has a status of true' do it 'has a status of true' do
expect(subject.info_refs.body[:status]).to be_truthy expect(subject.info_refs_receive_pack.body[:status]).to be_truthy
end end
it 'has no messsage' do it 'has no messsage' do
expect(subject.info_refs.body[:message]).to be_nil expect(subject.info_refs_receive_pack.body[:message]).to be_nil
end end
it 'returns a modified body' do it 'returns a modified body' do
expect(subject.info_refs.body[:result]).to eql(Base64.encode64(info_refs_body_short)) expect(subject.info_refs_receive_pack.body[:result]).to eql(Base64.encode64(info_refs_body_short))
end end
end end
end end
end end
describe '#push' do describe '#receive_pack' do
context 'against primary node' do context 'against primary node' do
let(:current_node) { primary_node } let(:current_node) { primary_node }
it 'raises an exception' do it 'raises an exception' do
expect do expect do
subject.push(info_refs_body_short) subject.receive_pack(info_refs_body_short)
end.to raise_error(described_class::MustBeASecondaryNode) end.to raise_error(described_class::MustBeASecondaryNode)
end end
end end
...@@ -178,7 +178,7 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -178,7 +178,7 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
let(:current_node) { secondary_node } let(:current_node) { secondary_node }
let(:full_git_receive_pack_url) { "#{primary_repo_http}/git-receive-pack" } let(:full_git_receive_pack_url) { "#{primary_repo_http}/git-receive-pack" }
let(:push_headers) do let(:receive_pack_headers) do
base_headers.merge( base_headers.merge(
'Content-Type' => 'application/x-git-receive-pack-request', 'Content-Type' => 'application/x-git-receive-pack-request',
'Accept' => 'application/x-git-receive-pack-result' 'Accept' => 'application/x-git-receive-pack-result'
...@@ -193,15 +193,15 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -193,15 +193,15 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
end end
it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do
expect(subject.push(info_refs_body_short)).to be_a(Gitlab::Geo::GitSSHProxy::FailedAPIResponse) expect(subject.receive_pack(info_refs_body_short)).to be_a(Gitlab::Geo::GitSSHProxy::FailedAPIResponse)
end end
it 'has a messsage' do it 'has a messsage' do
expect(subject.push(info_refs_body_short).body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}") expect(subject.receive_pack(info_refs_body_short).body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}")
end end
it 'has no result' do it 'has no result' do
expect(subject.push(info_refs_body_short).body[:result]).to be_nil expect(subject.receive_pack(info_refs_body_short).body[:result]).to be_nil
end end
end end
...@@ -209,19 +209,19 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -209,19 +209,19 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
let(:error_msg) { 'dial unix /Users/ash/src/gdk/gdk-ee/gitlab.socket: connect: connection refused' } let(:error_msg) { 'dial unix /Users/ash/src/gdk/gdk-ee/gitlab.socket: connect: connection refused' }
before do before do
stub_request(:post, full_git_receive_pack_url).to_return(status: 502, body: error_msg, headers: push_headers) stub_request(:post, full_git_receive_pack_url).to_return(status: 502, body: error_msg, headers: receive_pack_headers)
end end
it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do it 'returns a Gitlab::Geo::GitSSHProxy::FailedAPIResponse' do
expect(subject.push(info_refs_body_short)).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse) expect(subject.receive_pack(info_refs_body_short)).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse)
end end
it 'has a messsage' do it 'has a messsage' do
expect(subject.push(info_refs_body_short).body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}") expect(subject.receive_pack(info_refs_body_short).body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}")
end end
it 'has no result' do it 'has no result' do
expect(subject.push(info_refs_body_short).body[:result]).to be_nil expect(subject.receive_pack(info_refs_body_short).body[:result]).to be_nil
end end
end end
...@@ -230,23 +230,23 @@ describe Gitlab::Geo::GitSSHProxy, :geo do ...@@ -230,23 +230,23 @@ describe Gitlab::Geo::GitSSHProxy, :geo do
let(:base64_encoded_body) { Base64.encode64(body) } let(:base64_encoded_body) { Base64.encode64(body) }
before do before do
stub_request(:post, full_git_receive_pack_url).to_return(status: 201, body: body, headers: push_headers) stub_request(:post, full_git_receive_pack_url).to_return(status: 201, body: body, headers: receive_pack_headers)
end end
it 'returns a Gitlab::Geo::GitSSHProxy::APIResponse' do it 'returns a Gitlab::Geo::GitSSHProxy::APIResponse' do
expect(subject.push(info_refs_body_short)).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse) expect(subject.receive_pack(info_refs_body_short)).to be_a(Gitlab::Geo::GitSSHProxy::APIResponse)
end end
it 'has a code of 201' do it 'has a code of 201' do
expect(subject.push(info_refs_body_short).code).to be(201) expect(subject.receive_pack(info_refs_body_short).code).to be(201)
end end
it 'has no messsage' do it 'has no messsage' do
expect(subject.push(info_refs_body_short).body[:message]).to be_nil expect(subject.receive_pack(info_refs_body_short).body[:message]).to be_nil
end end
it 'has a result' do it 'has a result' do
expect(subject.push(info_refs_body_short).body[:result]).to eql(base64_encoded_body) expect(subject.receive_pack(info_refs_body_short).body[:result]).to eql(base64_encoded_body)
end end
end end
end end
......
...@@ -426,7 +426,7 @@ describe API::Geo do ...@@ -426,7 +426,7 @@ describe API::Geo do
context 'where an exception occurs' do context 'where an exception occurs' do
it 'responds with 500' do it 'responds with 500' do
expect(git_push_ssh_proxy).to receive(:info_refs).and_raise('deliberate exception raised') expect(git_push_ssh_proxy).to receive(:info_refs_receive_pack).and_raise('deliberate exception raised')
post api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: secret_token, data: data } post api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: secret_token, data: data }
...@@ -447,7 +447,7 @@ describe API::Geo do ...@@ -447,7 +447,7 @@ describe API::Geo do
end end
it 'responds with 200' do it 'responds with 200' do
expect(git_push_ssh_proxy).to receive(:info_refs).and_return(api_response) expect(git_push_ssh_proxy).to receive(:info_refs_receive_pack).and_return(api_response)
post api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: secret_token, data: data } post api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: secret_token, data: data }
...@@ -487,8 +487,8 @@ describe API::Geo do ...@@ -487,8 +487,8 @@ describe API::Geo do
context 'where an exception occurs' do context 'where an exception occurs' do
it 'responds with 500' do it 'responds with 500' do
expect(git_push_ssh_proxy).to receive(:push).and_raise('deliberate exception raised') expect(git_push_ssh_proxy).to receive(:receive_pack).and_raise('deliberate exception raised')
post api('/geo/proxy_git_push_ssh/push'), params: { secret_token: secret_token, data: data, output: output } post api('/geo/proxy_git_ssh/receive_pack'), params: { secret_token: secret_token, data: data, output: output }
expect(response).to have_gitlab_http_status(:internal_server_error) expect(response).to have_gitlab_http_status(:internal_server_error)
expect(json_response['message']).to include('RuntimeError (deliberate exception raised)') expect(json_response['message']).to include('RuntimeError (deliberate exception raised)')
...@@ -507,7 +507,7 @@ describe API::Geo do ...@@ -507,7 +507,7 @@ describe API::Geo do
end end
it 'responds with 201' do it 'responds with 201' do
expect(git_push_ssh_proxy).to receive(:push).with(output).and_return(api_response) expect(git_push_ssh_proxy).to receive(:receive_pack).with(output).and_return(api_response)
post api('/geo/proxy_git_push_ssh/push'), params: { secret_token: secret_token, data: data, output: output } post api('/geo/proxy_git_push_ssh/push'), params: { secret_token: secret_token, data: data, output: output }
......
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