From 46208ec3264bb7dc04017a5090e87d2f429d79c1 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Wed, 7 Nov 2018 17:06:01 +1100 Subject: [PATCH] Geo: push to secondary now in gitlab-workhorse --- ee/lib/api/geo.rb | 23 +- ee/lib/ee/gitlab/geo_git_access.rb | 2 +- ee/lib/gitlab/geo/git_push_ssh_proxy.rb | 150 ----------- .../lib/gitlab/geo/git_push_ssh_proxy_spec.rb | 245 ------------------ ee/spec/requests/api/geo_spec.rb | 78 ++---- 5 files changed, 37 insertions(+), 461 deletions(-) delete mode 100644 ee/lib/gitlab/geo/git_push_ssh_proxy.rb delete mode 100644 ee/spec/lib/gitlab/geo/git_push_ssh_proxy_spec.rb diff --git a/ee/lib/api/geo.rb b/ee/lib/api/geo.rb index 2403c4fd426bb1..f62ce357dc2f0c 100644 --- a/ee/lib/api/geo.rb +++ b/ee/lib/api/geo.rb @@ -58,6 +58,21 @@ def sanitized_node_status_params resource 'proxy_git_push_ssh' do format :json + helpers do + def custom_action_data_response + status 200 + content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE + + { + 'proxy_git_push_ssh' => { + 'gitlab_shell_custom_action_data' => params['data'], + 'gitlab_shell_output' => params['output'], + 'authorization' => Gitlab::Geo::BaseRequest.new.authorization + }.to_json + } + end + end + # Responsible for making HTTP GET /repo.git/info/refs?service=git-receive-pack # request *from* secondary gitlab-shell to primary # @@ -72,9 +87,7 @@ def sanitized_node_status_params authenticate_by_gitlab_shell_token! params.delete(:secret_token) - response = Gitlab::Geo::GitPushSSHProxy.new(params['data']).info_refs - status(response.code) - response.body + custom_action_data_response end # Responsible for making HTTP POST /repo.git/git-receive-pack @@ -92,9 +105,7 @@ def sanitized_node_status_params authenticate_by_gitlab_shell_token! params.delete(:secret_token) - response = Gitlab::Geo::GitPushSSHProxy.new(params['data']).push(params['output']) - status(response.code) - response.body + custom_action_data_response end end end diff --git a/ee/lib/ee/gitlab/geo_git_access.rb b/ee/lib/ee/gitlab/geo_git_access.rb index 859ee3f574bd66..18dfc410def53a 100644 --- a/ee/lib/ee/gitlab/geo_git_access.rb +++ b/ee/lib/ee/gitlab/geo_git_access.rb @@ -76,7 +76,7 @@ def primary_ssh_url_to_repo end def proxying_to_primary_message - ::Gitlab::Geo::GitPushSSHProxy.inform_client_message(primary_ssh_url_to_repo) + "You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_ssh_url_to_repo}" end def custom_action_api_endpoints diff --git a/ee/lib/gitlab/geo/git_push_ssh_proxy.rb b/ee/lib/gitlab/geo/git_push_ssh_proxy.rb deleted file mode 100644 index dc63ddb0707ea0..00000000000000 --- a/ee/lib/gitlab/geo/git_push_ssh_proxy.rb +++ /dev/null @@ -1,150 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Geo - class GitPushSSHProxy - HTTP_READ_TIMEOUT = 10 - - INFO_REFS_CONTENT_TYPE = 'application/x-git-upload-pack-request'.freeze - PUSH_CONTENT_TYPE = 'application/x-git-receive-pack-request'.freeze - PUSH_ACCEPT = 'application/x-git-receive-pack-result'.freeze - - MustBeASecondaryNode = Class.new(StandardError) - - class APIResponse - attr_reader :code, :body - - def initialize(code, body) - @code = code - @body = body - end - - def self.from_http_response(response, primary_repo) - success = response.is_a?(Net::HTTPSuccess) - body = response.body.to_s - - if success - result = Base64.encode64(body) - else - message = failed_message(body, primary_repo) - end - - new(response.code.to_i, status: success, message: message, result: result) - end - - def self.failed_message(str, primary_repo) - "Failed to contact primary #{primary_repo}\nError: #{str}" - end - end - - class FailedAPIResponse < APIResponse - def self.from_exception(ex_message, primary_repo, code: 500) - new(code.to_i, - status: false, - message: failed_message(ex_message, primary_repo), - result: nil) - end - end - - def initialize(data) - @data = data - end - - def self.inform_client_message(primary_repo_ssh) - "You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_repo_ssh}" - end - - def info_refs - ensure_secondary! - - url = "#{primary_repo}/info/refs?service=git-receive-pack" - headers = { 'Content-Type' => INFO_REFS_CONTENT_TYPE } - - resp = get(url, headers) - resp.body = remove_http_service_fragment_from(resp.body) if resp.is_a?(Net::HTTPSuccess) - - APIResponse.from_http_response(resp, primary_repo) - rescue => e - handle_exception(e) - end - - def push(encoded_info_refs_response) - ensure_secondary! - - url = "#{primary_repo}/git-receive-pack" - headers = { 'Content-Type' => PUSH_CONTENT_TYPE, 'Accept' => PUSH_ACCEPT } - info_refs_response = Base64.decode64(encoded_info_refs_response) - - resp = post(url, info_refs_response, headers) - APIResponse.from_http_response(resp, primary_repo) - rescue => e - handle_exception(e) - end - - private - - attr_reader :data - - def handle_exception(ex) - case ex - when MustBeASecondaryNode - raise(ex) - else - FailedAPIResponse.from_exception(ex.message, primary_repo) - end - end - - def primary_repo - @primary_repo ||= data['primary_repo'] - end - - def gl_id - @gl_id ||= data['gl_id'] - end - - def base_headers - @base_headers ||= { - 'Geo-GL-Id' => gl_id, - 'Authorization' => Gitlab::Geo::BaseRequest.new.authorization - } - end - - def get(url, headers) - request(url, Net::HTTP::Get, headers) - end - - def post(url, body, headers) - request(url, Net::HTTP::Post, headers, body: body) - end - - def request(url, klass, headers, body: nil) - headers = base_headers.merge(headers) - uri = URI.parse(url) - req = klass.new(uri, headers) - req.body = body if body - - http = Net::HTTP.new(uri.hostname, uri.port) - http.read_timeout = HTTP_READ_TIMEOUT - http.use_ssl = true if uri.is_a?(URI::HTTPS) - - http.start { http.request(req) } - end - - def remove_http_service_fragment_from(body) - # 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) - # suitable git response. However, we're executing in the context of an - # SSH session so we need to make the response suitable for what git over - # SSH expects. - # - # See Downloading Data > HTTP(S) section at: - # https://git-scm.com/book/en/v2/Git-Internals-Transfer-Protocols - body.gsub(/\A001f# service=git-receive-pack\n0000/, '') - end - - def ensure_secondary! - raise MustBeASecondaryNode, 'Node is not a secondary or there is no primary Geo node' unless Gitlab::Geo.secondary_with_primary? - end - end - end -end diff --git a/ee/spec/lib/gitlab/geo/git_push_ssh_proxy_spec.rb b/ee/spec/lib/gitlab/geo/git_push_ssh_proxy_spec.rb deleted file mode 100644 index 60c4c26fe16931..00000000000000 --- a/ee/spec/lib/gitlab/geo/git_push_ssh_proxy_spec.rb +++ /dev/null @@ -1,245 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Geo::GitPushSSHProxy, :geo do - include ::EE::GeoHelpers - - set(:primary_node) { create(:geo_node, :primary) } - set(:secondary_node) { create(:geo_node) } - - let(:current_node) { nil } - let(:project) { create(:project, :repository) } - let(:user) { project.creator } - let(:key) { create(:key, user: user) } - let(:base_request) { double(Gitlab::Geo::BaseRequest.new.authorization) } - - let(:info_refs_body_short) do - "008f43ba78b7912f7bf7ef1d7c3b8a0e5ae14a759dfa refs/heads/masterreport-status delete-refs side-band-64k quiet atomic ofs-delta agent=git/2.18.0\n0000" - end - - let(:base_headers) do - { - 'Geo-GL-Id' => "key-#{key.id}", - 'Authorization' => 'secret' - } - end - - let(:primary_repo_http) { geo_primary_http_url_to_repo(project) } - let(:primary_repo_ssh) { geo_primary_ssh_url_to_repo(project) } - - let(:data) do - { - 'gl_id' => "key-#{key.id}", - 'primary_repo' => primary_repo_http - } - end - - describe '.inform_client_message' do - it 'returns a message, with the ssh address' do - expect(described_class.inform_client_message(primary_repo_ssh)).to eql("You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_repo_ssh}") - end - end - - context 'instance methods' do - subject { described_class.new(data) } - - before do - stub_current_geo_node(current_node) - - allow(Gitlab::Geo::BaseRequest).to receive(:new).and_return(base_request) - allow(base_request).to receive(:authorization).and_return('secret') - end - - describe '#info_refs' do - context 'against primary node' do - let(:current_node) { primary_node } - - it 'raises an exception' do - expect do - subject.info_refs - end.to raise_error(described_class::MustBeASecondaryNode, 'Node is not a secondary or there is no primary Geo node') - end - end - - context 'against secondary node' do - let(:current_node) { secondary_node } - - 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_http_body_full) { "001f# service=git-receive-pack\n0000#{info_refs_body_short}" } - - context 'with a failed response' do - let(:error_msg) { 'execution expired' } - - before do - stub_request(:get, full_info_refs_url).to_timeout - end - - it 'returns a Gitlab::Geo::GitPushSSHProxy::FailedAPIResponse' do - expect(subject.info_refs).to be_a(Gitlab::Geo::GitPushSSHProxy::FailedAPIResponse) - end - - it 'has a code of 500' do - expect(subject.info_refs.code).to be(500) - end - - it 'has a status of false' do - expect(subject.info_refs.body[:status]).to be_falsey - end - - it 'has a messsage' do - expect(subject.info_refs.body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}") - end - - it 'has no result' do - expect(subject.info_refs.body[:result]).to be_nil - end - end - - context 'with an invalid response' do - let(:error_msg) { 'dial unix /Users/ash/src/gdk/gdk-ee/gitlab.socket: connect: connection refused' } - - before do - stub_request(:get, full_info_refs_url).to_return(status: 502, body: error_msg, headers: info_refs_headers) - end - - it 'returns a Gitlab::Geo::GitPushSSHProxy::FailedAPIResponse' do - expect(subject.info_refs).to be_a(Gitlab::Geo::GitPushSSHProxy::APIResponse) - end - - it 'has a code of 502' do - expect(subject.info_refs.code).to be(502) - end - - it 'has a status of false' do - expect(subject.info_refs.body[:status]).to be_falsey - end - - it 'has a messsage' do - expect(subject.info_refs.body[:message]).to eql("Failed to contact primary #{primary_repo_http}\nError: #{error_msg}") - end - - it 'has no result' do - expect(subject.info_refs.body[:result]).to be_nil - end - end - - context 'with a valid response' do - before do - stub_request(:get, full_info_refs_url).to_return(status: 200, body: info_refs_http_body_full, headers: info_refs_headers) - end - - it 'returns a Gitlab::Geo::GitPushSSHProxy::APIResponse' do - expect(subject.info_refs).to be_a(Gitlab::Geo::GitPushSSHProxy::APIResponse) - end - - it 'has a code of 200' do - expect(subject.info_refs.code).to be(200) - end - - it 'has a status of true' do - expect(subject.info_refs.body[:status]).to be_truthy - end - - it 'has no messsage' do - expect(subject.info_refs.body[:message]).to be_nil - end - - it 'returns a modified body' do - expect(subject.info_refs.body[:result]).to eql(Base64.encode64(info_refs_body_short)) - end - end - end - end - - describe '#push' do - context 'against primary node' do - let(:current_node) { primary_node } - - it 'raises an exception' do - expect do - subject.push(info_refs_body_short) - end.to raise_error(described_class::MustBeASecondaryNode) - end - end - - context 'against secondary node' do - let(:current_node) { secondary_node } - - let(:full_git_receive_pack_url) { "#{primary_repo_http}/git-receive-pack" } - let(:push_headers) do - base_headers.merge( - 'Content-Type' => 'application/x-git-receive-pack-request', - 'Accept' => 'application/x-git-receive-pack-result' - ) - end - - context 'with a failed response' do - let(:error_msg) { 'execution expired' } - - before do - stub_request(:post, full_git_receive_pack_url).to_timeout - end - - it 'returns a Gitlab::Geo::GitPushSSHProxy::FailedAPIResponse' do - expect(subject.push(info_refs_body_short)).to be_a(Gitlab::Geo::GitPushSSHProxy::FailedAPIResponse) - end - - 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}") - end - - it 'has no result' do - expect(subject.push(info_refs_body_short).body[:result]).to be_nil - end - end - - context 'with an invalid response' do - let(:error_msg) { 'dial unix /Users/ash/src/gdk/gdk-ee/gitlab.socket: connect: connection refused' } - - before do - stub_request(:post, full_git_receive_pack_url).to_return(status: 502, body: error_msg, headers: push_headers) - end - - it 'returns a Gitlab::Geo::GitPushSSHProxy::FailedAPIResponse' do - expect(subject.push(info_refs_body_short)).to be_a(Gitlab::Geo::GitPushSSHProxy::APIResponse) - end - - 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}") - end - - it 'has no result' do - expect(subject.push(info_refs_body_short).body[:result]).to be_nil - end - end - - context 'with a valid response' do - let(:body) { '' } - let(:base64_encoded_body) { Base64.encode64(body) } - - before do - stub_request(:post, full_git_receive_pack_url).to_return(status: 201, body: body, headers: push_headers) - end - - it 'returns a Gitlab::Geo::GitPushSSHProxy::APIResponse' do - expect(subject.push(info_refs_body_short)).to be_a(Gitlab::Geo::GitPushSSHProxy::APIResponse) - end - - it 'has a code of 201' do - expect(subject.push(info_refs_body_short).code).to be(201) - end - - it 'has no messsage' do - expect(subject.push(info_refs_body_short).body[:message]).to be_nil - end - - it 'has a result' do - expect(subject.push(info_refs_body_short).body[:result]).to eql(base64_encoded_body) - end - end - end - end - end -end diff --git a/ee/spec/requests/api/geo_spec.rb b/ee/spec/requests/api/geo_spec.rb index f892e4529e06ce..193cdd9d642c5f 100644 --- a/ee/spec/requests/api/geo_spec.rb +++ b/ee/spec/requests/api/geo_spec.rb @@ -306,7 +306,7 @@ describe '/geo/proxy_git_push_ssh' do let(:secret_token) { Gitlab::Shell.secret_token } let(:primary_repo) { 'http://localhost:3001/testuser/repo.git' } - let(:data) { { primary_repo: primary_repo, gl_id: 'key-1', gl_username: 'testuser' } } + let(:data) { { 'primary_repo' => primary_repo, 'gl_id' => 'key-1', 'gl_username' => 'testuser' } } before do stub_current_geo_node(secondary_node) @@ -323,12 +323,6 @@ end context 'with all required params' do - let(:git_push_ssh_proxy) { double(Gitlab::Geo::GitPushSSHProxy) } - - before do - allow(Gitlab::Geo::GitPushSSHProxy).to receive(:new).with(data).and_return(git_push_ssh_proxy) - end - context 'with an invalid secret_token' do it 'responds with 401' do post(api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: 'invalid', data: data }) @@ -338,35 +332,20 @@ end end - context 'where an exception occurs' do - it 'responds with 500' do - expect(git_push_ssh_proxy).to receive(:info_refs).and_raise('deliberate exception raised') - - post api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: secret_token, data: data } - - expect(response).to have_gitlab_http_status(500) - expect(json_response['message']).to include('RuntimeError (deliberate exception raised)') - expect(json_response['result']).to be_nil - end - end - context 'with a valid secret token' do - let(:http_response) { double(Net::HTTPOK, code: 200, body: 'something here') } - let(:api_response) { Gitlab::Geo::GitPushSSHProxy::APIResponse.from_http_response(http_response, primary_repo) } + it 'responds with 200' do + post api('/geo/proxy_git_push_ssh/info_refs'), { secret_token: secret_token, data: data } - before do - # Mocking a real Net::HTTPSuccess is very difficult as it's not - # easy to instantiate the class due to the way it sets the body - expect(http_response).to receive(:is_a?).with(Net::HTTPSuccess).and_return(true) - end + expect(response).to have_gitlab_http_status(200) + expect(response.headers['Content-Type']).to eq('application/vnd.gitlab-workhorse+json') - it 'responds with 200' do - expect(git_push_ssh_proxy).to receive(:info_refs).and_return(api_response) + expect(json_response).to have_key('proxy_git_push_ssh') - post api('/geo/proxy_git_push_ssh/info_refs'), params: { secret_token: secret_token, data: data } + proxy_git_push_ssh = JSON.parse(json_response['proxy_git_push_ssh']) - expect(response).to have_gitlab_http_status(200) - expect(Base64.decode64(json_response['result'])).to eql('something here') + expect(proxy_git_push_ssh['gitlab_shell_custom_action_data']).to eq(data) + expect(proxy_git_push_ssh['gitlab_shell_output']).to be_nil + expect(proxy_git_push_ssh['authorization']).to match(/^GL-Geo .+$/) end end end @@ -384,11 +363,6 @@ context 'with all required params' do let(:output) { Base64.encode64('info_refs content') } - let(:git_push_ssh_proxy) { double(Gitlab::Geo::GitPushSSHProxy) } - - before do - allow(Gitlab::Geo::GitPushSSHProxy).to receive(:new).with(data).and_return(git_push_ssh_proxy) - end context 'with an invalid secret_token' do it 'responds with 401' do @@ -399,34 +373,20 @@ end end - context 'where an exception occurs' do - it 'responds with 500' do - expect(git_push_ssh_proxy).to receive(:push).and_raise('deliberate exception raised') - post api('/geo/proxy_git_push_ssh/push'), params: { secret_token: secret_token, data: data, output: output } - - expect(response).to have_gitlab_http_status(500) - expect(json_response['message']).to include('RuntimeError (deliberate exception raised)') - expect(json_response['result']).to be_nil - end - end - context 'with a valid secret token' do - let(:http_response) { double(Net::HTTPCreated, code: 201, body: 'something here', class: Net::HTTPCreated) } - let(:api_response) { Gitlab::Geo::GitPushSSHProxy::APIResponse.from_http_response(http_response, primary_repo) } + it 'responds with 201' do + post api('/geo/proxy_git_push_ssh/push'), { secret_token: secret_token, data: data, output: output } - before do - # Mocking a real Net::HTTPSuccess is very difficult as it's not - # easy to instantiate the class due to the way it sets the body - expect(http_response).to receive(:is_a?).with(Net::HTTPSuccess).and_return(true) - end + expect(response).to have_gitlab_http_status(200) + expect(response.headers['Content-Type']).to eq('application/vnd.gitlab-workhorse+json') - it 'responds with 201' do - expect(git_push_ssh_proxy).to receive(:push).with(output).and_return(api_response) + expect(json_response).to have_key('proxy_git_push_ssh') - post api('/geo/proxy_git_push_ssh/push'), params: { secret_token: secret_token, data: data, output: output } + proxy_git_push_ssh = JSON.parse(json_response['proxy_git_push_ssh']) - expect(response).to have_gitlab_http_status(201) - expect(Base64.decode64(json_response['result'])).to eql('something here') + expect(proxy_git_push_ssh['gitlab_shell_custom_action_data']).to eq(data) + expect(proxy_git_push_ssh['gitlab_shell_output']).to eq(output) + expect(proxy_git_push_ssh['authorization']).to match(/^GL-Geo .+$/) end end end -- GitLab