From 58df68e34e89d21884f0374ff969d742009998a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 30 Aug 2017 19:31:24 -0300 Subject: Support new /internal/post-receive API endpoint --- CHANGELOG | 3 + lib/gitlab_net.rb | 14 ++ lib/gitlab_post_receive.rb | 59 +++-- spec/gitlab_net_spec.rb | 40 ++++ spec/gitlab_post_receive_spec.rb | 312 ++++++++++++++------------ spec/vcr_cassettes/post-receive-not-found.yml | 42 ++++ spec/vcr_cassettes/post-receive.yml | 46 ++++ 7 files changed, 356 insertions(+), 160 deletions(-) create mode 100644 spec/vcr_cassettes/post-receive-not-found.yml create mode 100644 spec/vcr_cassettes/post-receive.yml diff --git a/CHANGELOG b/CHANGELOG index b45b86a..6d5a6f0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ +v5.9.0 + - Support new /internal/post-receive API endpoint for post-receive operations + v5.8.1 - Support old versions of ruby without monotonic clock diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 3acebea..b48f655 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -11,6 +11,7 @@ require_relative 'httpunix' class GitlabNet class ApiUnreachableError < StandardError; end + class NotFound < StandardError; end CHECK_TIMEOUT = 5 READ_TIMEOUT = 300 @@ -112,6 +113,19 @@ class GitlabNet false end + def post_receive(gl_repository, identifier, changes) + params = { + gl_repository: gl_repository, + identifier: identifier, + changes: changes + } + resp = post("#{host}/post_receive", params) + + raise NotFound if resp.code == '404' + + JSON.parse(resp.body) if resp.code == '200' + end + def redis_client redis_config = config.redis database = redis_config['database'] || 0 diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index 00a1b1b..aca1ee2 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -20,29 +20,20 @@ class GitlabPostReceive end def exec - result = update_redis + response = GitlabMetrics.measure("post-receive") do + api.post_receive(gl_repository, @actor, changes) + end - begin - broadcast_message = GitlabMetrics.measure("broadcast-message") do - api.broadcast_message - end + return false unless response - if broadcast_message.has_key?("message") - puts - print_broadcast_message(broadcast_message["message"]) - end + print_broadcast_message(response['broadcast_message']) if response['broadcast_message'] + print_merge_request_links(response['merge_request_urls']) if response['merge_request_urls'] - merge_request_urls = GitlabMetrics.measure("merge-request-urls") do - api.merge_request_urls(@gl_repository, @repo_path, @changes) - end - print_merge_request_links(merge_request_urls) - - api.notify_post_receive(gl_repository, repo_path) - rescue GitlabNet::ApiUnreachableError - nil - end - - result && GitlabReferenceCounter.new(repo_path).decrease + response['reference_counter_decreased'] + rescue GitlabNet::ApiUnreachableError + false + rescue GitlabNet::NotFound + fallback_post_receive end protected @@ -89,6 +80,7 @@ class GitlabPostReceive # message.scan returns a nested array of capture groups, so flatten. lines = message.scan(/(.{,#{text_width}})(?:\s|$)/)[0...-1].flatten + puts puts "=" * total_width puts @@ -127,4 +119,31 @@ class GitlabPostReceive false end end + + private + + def fallback_post_receive + result = update_redis + + begin + broadcast_message = GitlabMetrics.measure("broadcast-message") do + api.broadcast_message + end + + if broadcast_message.has_key?("message") + print_broadcast_message(broadcast_message["message"]) + end + + merge_request_urls = GitlabMetrics.measure("merge-request-urls") do + api.merge_request_urls(@gl_repository, @repo_path, @changes) + end + print_merge_request_links(merge_request_urls) + + api.notify_post_receive(gl_repository, repo_path) + rescue GitlabNet::ApiUnreachableError + nil + end + + result && GitlabReferenceCounter.new(repo_path).decrease + end end diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 3960c96..cbcef90 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -126,6 +126,46 @@ describe GitlabNet, vcr: true do end end + describe :post_receive do + let(:gl_repository) { "project-1" } + let(:changes) { "123456 789012 refs/heads/test\n654321 210987 refs/tags/tag" } + let(:params) do + { gl_repository: gl_repository, identifier: key, changes: changes } + end + let(:merge_request_urls) do + [{ + "branch_name" => "test", + "url" => "http://localhost:3000/gitlab-org/gitlab-test/merge_requests/7", + "new_merge_request" => false + }] + end + + subject { gitlab_net.post_receive(gl_repository, key, changes) } + + it 'sends the correct parameters' do + Net::HTTP::Post.any_instance.should_receive(:set_form_data).with(hash_including(params)) + + + VCR.use_cassette("post-receive") do + subject + end + end + + it 'calls /internal/post-receive' do + VCR.use_cassette("post-receive") do + expect(subject['merge_request_urls']).to eq(merge_request_urls) + expect(subject['broadcast_message']).to eq('Message') + expect(subject['reference_counter_decreased']).to eq(true) + end + end + + it 'throws a NotFound error when post-receive is not available' do + VCR.use_cassette("post-receive-not-found") do + expect { subject }.to raise_error(GitlabNet::NotFound) + end + end + end + describe :authorized_key do let (:ssh_key) { "rsa-key" } diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index 69e19e6..82a0f8c 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -4,173 +4,164 @@ require 'gitlab_post_receive' describe GitlabPostReceive do let(:repository_path) { "/home/git/repositories" } - let(:repo_name) { 'dzaporozhets/gitlab-ci' } - let(:actor) { 'key-123' } + let(:repo_name) { 'dzaporozhets/gitlab-ci' } + let(:actor) { 'key-123' } let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" } let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") } let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) } - let(:repo_path) { File.join(repository_path, repo_name) + ".git" } + let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:gl_repository) { "project-1" } let(:gitlab_post_receive) { GitlabPostReceive.new(gl_repository, repo_path, actor, wrongly_encoded_changes) } - let(:message) { "test " * 10 + "message " * 10 } + let(:broadcast_message) { "test " * 10 + "message " * 10 } let(:redis_client) { double('redis_client') } let(:enqueued_at) { Time.new(2016, 6, 23, 6, 59) } + let(:new_merge_request_urls) do + [{ + 'branch_name' => 'new_branch', + 'url' => 'http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch', + 'new_merge_request' => true + }] + end + let(:existing_merge_request_urls) do + [{ + 'branch_name' => 'feature_branch', + 'url' => 'http://localhost/dzaporozhets/gitlab-ci/merge_requests/1', + 'new_merge_request' => false + }] + end before do + $logger = double('logger').as_null_object # Global vars are bad GitlabConfig.any_instance.stub(repos_path: repository_path) - GitlabNet.any_instance.stub(broadcast_message: { }) - GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) { [] } - GitlabNet.any_instance.stub(notify_post_receive: true) - expect(Time).to receive(:now).and_return(enqueued_at) end describe "#exec" do - before do - allow_any_instance_of(GitlabNet).to receive(:redis_client).and_return(redis_client) - allow_any_instance_of(GitlabReferenceCounter).to receive(:redis_client).and_return(redis_client) - allow(redis_client).to receive(:get).and_return(1) - allow(redis_client).to receive(:incr).and_return(true) - allow(redis_client).to receive(:decr).and_return(0) - allow(redis_client).to receive(:rpush).and_return(true) - end + context 'when the new post_receive API endpoint is not available' do + before do + GitlabNet.any_instance.stub(broadcast_message: { }) + GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) { [] } + GitlabNet.any_instance.stub(notify_post_receive: true) + + allow_any_instance_of(GitlabNet).to receive(:post_receive).and_raise(GitlabNet::NotFound) + allow_any_instance_of(GitlabNet).to receive(:redis_client).and_return(redis_client) + allow_any_instance_of(GitlabReferenceCounter).to receive(:redis_client).and_return(redis_client) + allow(redis_client).to receive(:get).and_return(1) + allow(redis_client).to receive(:incr).and_return(true) + allow(redis_client).to receive(:decr).and_return(0) + allow(redis_client).to receive(:rpush).and_return(true) + expect(Time).to receive(:now).and_return(enqueued_at) + end - context 'Without broad cast message' do - context 'pushing new branch' do - before do - GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do - [{ - "branch_name" => "new_branch", - "url" => "http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", - "new_merge_request" => true - }] + context 'Without broad cast message' do + context 'pushing new branch' do + before do + GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do + new_merge_request_urls + end + end + + it "prints the new merge request url" do + assert_new_mr_printed(gitlab_post_receive) + + gitlab_post_receive.exec end end - it "prints the new merge request url" do - expect(redis_client).to receive(:rpush) + context 'pushing existing branch with merge request created' do + before do + GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do + existing_merge_request_urls + end + end - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "To create a merge request for new_branch, visit:" - ).ordered - expect(gitlab_post_receive).to receive(:puts).with( - " http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" - ).ordered - expect(gitlab_post_receive).to receive(:puts).ordered + it "prints the view merge request url" do + assert_existing_mr_printed(gitlab_post_receive) - gitlab_post_receive.exec + gitlab_post_receive.exec + end end end - context 'pushing existing branch with merge request created' do + context 'show broadcast message and merge request link' do before do GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do - [{ - "branch_name" => "feature_branch", - "url" => "http://localhost/dzaporozhets/gitlab-ci/merge_requests/1", - "new_merge_request" => false - }] + new_merge_request_urls end + GitlabNet.any_instance.stub(broadcast_message: { "message" => broadcast_message }) end - it "prints the view merge request url" do - expect(redis_client).to receive(:rpush) - - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "View merge request for feature_branch:" - ).ordered - expect(gitlab_post_receive).to receive(:puts).with( - " http://localhost/dzaporozhets/gitlab-ci/merge_requests/1" - ).ordered - expect(gitlab_post_receive).to receive(:puts).ordered + it 'prints the broadcast message and create new merge request link' do + assert_broadcast_message_printed(gitlab_post_receive) + assert_new_mr_printed(gitlab_post_receive) gitlab_post_receive.exec end end - end - context 'show broadcast message and merge request link' do - before do - GitlabNet.any_instance.stub(:merge_request_urls).with(gl_repository, repo_path, wrongly_encoded_changes) do - [{ - "branch_name" => "new_branch", - "url" => "http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch", - "new_merge_request" => true - }] - end - GitlabNet.any_instance.stub(broadcast_message: { "message" => message }) - end + context 'Sidekiq jobs' do + it "pushes a Sidekiq job onto the queue" do + expect(redis_client).to receive(:rpush).with( + 'resque:gitlab:queue:post_receive', + %Q/{"class":"PostReceive","args":["#{gl_repository}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}","enqueued_at":#{enqueued_at.to_f}}/ + ).and_return(true) - it 'prints the broadcast message and create new merge request link' do - expect(redis_client).to receive(:rpush) - - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "========================================================================" - ).ordered - expect(gitlab_post_receive).to receive(:puts).ordered - - expect(gitlab_post_receive).to receive(:puts).with( - " test test test test test test test test test test message message" - ).ordered - expect(gitlab_post_receive).to receive(:puts).with( - " message message message message message message message message" - ).ordered - - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "========================================================================" - ).ordered - - expect(gitlab_post_receive).to receive(:puts).ordered - expect(gitlab_post_receive).to receive(:puts).with( - "To create a merge request for new_branch, visit:" - ).ordered - expect(gitlab_post_receive).to receive(:puts).with( - " http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" - ).ordered - expect(gitlab_post_receive).to receive(:puts).ordered - - - gitlab_post_receive.exec - end - end + gitlab_post_receive.exec + end - context 'Sidekiq jobs' do - it "pushes a Sidekiq job onto the queue" do - expect(redis_client).to receive(:rpush).with( - 'resque:gitlab:queue:post_receive', - %Q/{"class":"PostReceive","args":["#{gl_repository}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}","enqueued_at":#{enqueued_at.to_f}}/ - ).and_return(true) + context 'when gl_repository is nil' do + let(:gl_repository) { nil } - gitlab_post_receive.exec - end + it "pushes a Sidekiq job with the repository path" do + expect(redis_client).to receive(:rpush).with( + 'resque:gitlab:queue:post_receive', + %Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}","enqueued_at":#{enqueued_at.to_f}}/ + ).and_return(true) - context 'when gl_repository is nil' do - let(:gl_repository) { nil } + gitlab_post_receive.exec + end + end + end - it "pushes a Sidekiq job with the repository path" do - expect(redis_client).to receive(:rpush).with( - 'resque:gitlab:queue:post_receive', - %Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}","enqueued_at":#{enqueued_at.to_f}}/ - ).and_return(true) + context 'reference counter' do + it 'decreases the reference counter for the project' do + expect_any_instance_of(GitlabReferenceCounter).to receive(:decrease).and_return(true) gitlab_post_receive.exec end + + context "when the redis command succeeds" do + before do + allow(redis_client).to receive(:decr).and_return(0) + end + + it "returns true" do + expect(gitlab_post_receive.exec).to eq(true) + end + end + + context "when the redis command fails" do + before do + allow(redis_client).to receive(:decr).and_raise('Fail') + end + + it "returns false" do + expect(gitlab_post_receive.exec).to eq(false) + end + end end - end - context 'reference counter' do - it 'decreases the reference counter for the project' do - expect_any_instance_of(GitlabReferenceCounter).to receive(:decrease).and_return(true) + context 'post_receive notification' do + it 'calls the api to notify the execution of the hook' do + expect_any_instance_of(GitlabNet).to receive(:notify_post_receive). + with(gl_repository, repo_path) - gitlab_post_receive.exec + gitlab_post_receive.exec + end end context "when the redis command succeeds" do before do - allow(redis_client).to receive(:decr).and_return(0) + allow(redis_client).to receive(:rpush).and_return(true) end it "returns true" do @@ -180,7 +171,7 @@ describe GitlabPostReceive do context "when the redis command fails" do before do - allow(redis_client).to receive(:decr).and_raise('Fail') + allow(redis_client).to receive(:rpush).and_raise('Fail') end it "returns false" do @@ -189,35 +180,76 @@ describe GitlabPostReceive do end end - context 'post_receive notification' do + context 'when the new post_receive API endpoint is available' do + let(:response) { { 'reference_counter_decreased' => true } } + it 'calls the api to notify the execution of the hook' do - expect_any_instance_of(GitlabNet).to receive(:notify_post_receive). - with(gl_repository, repo_path) + expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response) - gitlab_post_receive.exec + expect(gitlab_post_receive.exec).to eq(true) end - end - context "when the redis command succeeds" do + context 'merge request urls and broadcast messages' do + let(:response) do + { + 'reference_counter_decreased' => true, + 'merge_request_urls' => new_merge_request_urls, + 'broadcast_message' => broadcast_message + } + end - before do - allow(redis_client).to receive(:rpush).and_return(true) - end + it 'prints the merge request urls and broadcast message' do + expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response) + assert_broadcast_message_printed(gitlab_post_receive) + assert_new_mr_printed(gitlab_post_receive) - it "returns true" do - expect(gitlab_post_receive.exec).to eq(true) + expect(gitlab_post_receive.exec).to eq(true) + end end end + end - context "when the redis command fails" do + private + + def assert_new_mr_printed(gitlab_post_receive) + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "To create a merge request for new_branch, visit:" + ).ordered + expect(gitlab_post_receive).to receive(:puts).with( + " http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" + ).ordered + expect(gitlab_post_receive).to receive(:puts).ordered + end - before do - allow(redis_client).to receive(:rpush).and_raise('Fail') - end + def assert_existing_mr_printed(gitlab_post_receive) + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "View merge request for feature_branch:" + ).ordered + expect(gitlab_post_receive).to receive(:puts).with( + " http://localhost/dzaporozhets/gitlab-ci/merge_requests/1" + ).ordered + expect(gitlab_post_receive).to receive(:puts).ordered + end - it "returns false" do - expect(gitlab_post_receive.exec).to eq(false) - end - end + def assert_broadcast_message_printed(gitlab_post_receive) + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "========================================================================" + ).ordered + expect(gitlab_post_receive).to receive(:puts).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " test test test test test test test test test test message message" + ).ordered + expect(gitlab_post_receive).to receive(:puts).with( + " message message message message message message message message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "========================================================================" + ).ordered end end diff --git a/spec/vcr_cassettes/post-receive-not-found.yml b/spec/vcr_cassettes/post-receive-not-found.yml new file mode 100644 index 0000000..e89095d --- /dev/null +++ b/spec/vcr_cassettes/post-receive-not-found.yml @@ -0,0 +1,42 @@ +--- +http_interactions: +- request: + method: post + uri: http://localhost:3000/api/v4/internal/post_receive + body: + encoding: US-ASCII + string: gl_repository=project-1&identifier=key-1&changes=123456+789012+refs%2Fheads%2Ftest%0A654321+210987+refs%2Ftags%2Ftag&secret_token=0a3938d9d95d807e94d937af3a4fbbea%0A + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Content-Type: + - application/x-www-form-urlencoded + response: + status: + code: 404 + message: Not Found + headers: + Cache-Control: + - no-cache + Content-Length: + - '25' + Content-Type: + - application/json + Date: + - Wed, 30 Aug 2017 22:24:37 GMT + Vary: + - Origin + X-Request-Id: + - bbfdb1ed-99dc-4246-a606-3074ffd5d87b + X-Runtime: + - '0.459681' + body: + encoding: UTF-8 + string: '{"error":"404 Not Found"}' + http_version: + recorded_at: Wed, 30 Aug 2017 22:24:37 GMT +recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/post-receive.yml b/spec/vcr_cassettes/post-receive.yml new file mode 100644 index 0000000..c29f865 --- /dev/null +++ b/spec/vcr_cassettes/post-receive.yml @@ -0,0 +1,46 @@ +--- +http_interactions: +- request: + method: post + uri: http://localhost:3000/api/v4/internal/post_receive + body: + encoding: US-ASCII + string: gl_repository=project-1&identifier=key-1&changes=123456+789012+refs%2Fheads%2Ftest%0A654321+210987+refs%2Ftags%2Ftag&secret_token=0a3938d9d95d807e94d937af3a4fbbea%0A + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Content-Type: + - application/x-www-form-urlencoded + response: + status: + code: 200 + message: OK + headers: + Cache-Control: + - max-age=0, private, must-revalidate + Content-Length: + - '208' + Content-Type: + - application/json + Date: + - Wed, 30 Aug 2017 22:08:28 GMT + Etag: + - W/"1757d1411091b751684cde10119e0942" + Vary: + - Origin + X-Frame-Options: + - SAMEORIGIN + X-Request-Id: + - f7d422a7-c1a3-49d1-9b81-7fc48c954767 + X-Runtime: + - '0.687283' + body: + encoding: UTF-8 + string: '{"merge_request_urls":[{"branch_name":"test","url":"http://localhost:3000/gitlab-org/gitlab-test/merge_requests/7","new_merge_request":false}],"broadcast_message":"Message","reference_counter_decreased":true}' + http_version: + recorded_at: Wed, 30 Aug 2017 22:08:28 GMT +recorded_with: VCR 2.4.0 -- cgit v1.2.1