Merge branch 'ash.mckenzie/display-feedback' into 'master'

Display helpful feedback when proxying an SSH git push to secondary request

See merge request gitlab-org/gitlab-shell!244

lib/action/custom.rb

@@ -54,6 +54,8 @@ module Action
           raise UnsuccessfulError, 'Response was not valid JSON'
         end
 
+        inform_client(body['message']) if body['message']
+
         print_flush(body['result'])
 
         # In the context of the git push sequence of events, it's necessary to read
@@ -90,8 +92,12 @@ module Action
 
     def print_flush(str)
       return false unless str
-      print(Base64.decode64(str))
-      STDOUT.flush
+      $stdout.print(Base64.decode64(str))
+      $stdout.flush
+    end
+
+    def inform_client(str)
+      $stderr.puts(str)
     end
 
     def validate!

lib/http_helper.rb

@@ -83,7 +83,8 @@ module HTTPHelper
       $logger.info('finished HTTP request', method: method.to_s.upcase, url: url, duration: Time.new - start_time)
     end
 
-    if response.code == "200"
+    case response
+    when Net::HTTPSuccess, Net::HTTPMultipleChoices
       $logger.debug('Received response', code: response.code, body: response.body)
     else
       $logger.error('Call failed', method: method.to_s.upcase, url: url, code: response.code, body: response.body)

spec/action/custom_spec.rb

@@ -26,31 +26,62 @@ describe Action::Custom do
       end
 
       context 'that are valid' do
-        let(:payload) do
-          {
-            'action' => 'geo_proxy_to_primary',
-            'data' => {
-              'api_endpoints' => %w{/api/v4/fake/info_refs /api/v4/fake/push},
-              'gl_username' => 'user1',
-              'primary_repo' => 'http://localhost:3001/user1/repo1.git'
-            }
-          }
+        where(:primary_repo_data) do
+          [
+            [ 'http://localhost:3001/user1/repo1.git' ],
+            [{ 'http' => 'http://localhost:3001/user1/repo1.git' }],
+            [{ 'http' => 'http://localhost:3001/user1/repo1.git', 'ssh' => 'ssh://user@localhost:3002/user1/repo1.git' }]
+          ]
         end
 
-        context 'and responds correctly' do
-          it 'returns an instance of Net::HTTPCreated' do
-            VCR.use_cassette("custom-action-ok") do
-              expect(subject.execute).to be_instance_of(Net::HTTPCreated)
-            end
+        with_them do
+          let(:payload) do
+            {
+              'action' => 'geo_proxy_to_primary',
+              'data' => {
+                'api_endpoints' => %w{/api/v4/fake/info_refs /api/v4/fake/push},
+                'gl_username' => 'user1',
+                'primary_repo' => primary_repo_data
+              }
+            }
           end
-        end
 
-        context 'but responds incorrectly' do
-          it 'raises an UnsuccessfulError exception' do
-            VCR.use_cassette("custom-action-ok-not-json") do
-              expect {
+          context 'and responds correctly' do
+            it 'prints a Base64 encoded result to $stdout' do
+              VCR.use_cassette("custom-action-ok") do
+                expect($stdout).to receive(:print).with('info_refs-result').ordered
+                expect($stdout).to receive(:print).with('push-result').ordered
                 subject.execute
-              }.to raise_error(Action::Custom::UnsuccessfulError, 'Response was not valid JSON')
+              end
+            end
+
+            context 'with results printed to $stdout' do
+              before do
+                allow($stdout).to receive(:print).with('info_refs-result')
+                allow($stdout).to receive(:print).with('push-result')
+              end
+
+              it 'prints a message to $stderr' do
+                VCR.use_cassette("custom-action-ok-with-message") do
+                  expect { subject.execute }.to output(/NOTE: Message here/).to_stderr
+                end
+              end
+
+              it 'returns an instance of Net::HTTPCreated' do
+                VCR.use_cassette("custom-action-ok") do
+                  expect(subject.execute ).to be_instance_of(Net::HTTPCreated)
+                end
+              end
+            end
+          end
+
+          context 'but responds incorrectly' do
+            it 'raises an UnsuccessfulError exception' do
+              VCR.use_cassette("custom-action-ok-not-json") do
+                expect {
+                  subject.execute
+                }.to raise_error(Action::Custom::UnsuccessfulError, 'Response was not valid JSON')
+              end
             end
           end
         end

spec/vcr_cassettes/custom-action-ok-with-message.yml

+---
+http_interactions:
+- request:
+    method: post
+    uri: http://localhost:3000/api/v4/fake/info_refs
+    body:
+      encoding: UTF-8
+      string: '{"data":{"gl_username":"user1","primary_repo":"http://localhost:3001/user1/repo1.git","gl_id":"key-1"},"output":"","secret_token":"0a3938d9d95d807e94d937af3a4fbbea"}'
+    headers:
+      Content-Type:
+      - application/json
+      Accept-Encoding:
+      - gzip;q=1.0,deflate;q=0.6,identity;q=0.3
+      Accept:
+      - "*/*"
+      User-Agent:
+      - Ruby
+      Host:
+      - localhost
+  response:
+    status:
+      code: 200
+      message: OK
+    headers:
+      Date:
+      - Fri, 20 Jul 2018 06:18:58 GMT
+      Connection:
+      - close
+      X-Frame-Options:
+      - SAMEORIGIN
+      X-Content-Type-Options:
+      - nosniff
+      Content-Type:
+      - application/json
+      Content-Length:
+      - '172'
+      Vary:
+      - Origin
+      Etag:
+      - W/"7d01e1e3dbcbe7cca9607461352f8244"
+      Cache-Control:
+      - max-age=0, private, must-revalidate
+      X-Request-Id:
+      - 03afa234-b6be-49ab-9392-4aa35c5dee25
+      X-Runtime:
+      - '1.436040'
+    body:
+      encoding: UTF-8
+      string: '{"result":"aW5mb19yZWZzLXJlc3VsdA==\n", "message":"NOTE: Message here"}'
+    http_version:
+  recorded_at: Fri, 20 Jul 2018 06:18:58 GMT
+- request:
+    method: post
+    uri: http://localhost:3000/api/v4/fake/push
+    body:
+      encoding: UTF-8
+      string: '{"data":{"gl_username":"user1","primary_repo":"http://localhost:3001/user1/repo1.git","gl_id":"key-1"},"output":"info_refs-result","secret_token":"0a3938d9d95d807e94d937af3a4fbbea"}'
+    headers:
+      Content-Type:
+      - application/json
+      Accept-Encoding:
+      - gzip;q=1.0,deflate;q=0.6,identity;q=0.3
+      Accept:
+      - "*/*"
+      User-Agent:
+      - Ruby
+      Host:
+      - localhost
+  response:
+    status:
+      code: 201
+      message: Created
+    headers:
+      Date:
+      - Fri, 20 Jul 2018 06:19:08 GMT
+      Connection:
+      - close
+      X-Frame-Options:
+      - SAMEORIGIN
+      X-Content-Type-Options:
+      - nosniff
+      Content-Type:
+      - application/json
+      Content-Length:
+      - '13'
+      Vary:
+      - Origin
+      Cache-Control:
+      - no-cache
+      X-Request-Id:
+      - 0c6894ac-7f8e-4cdb-871f-4cb64d3731ca
+      X-Runtime:
+      - '0.786754'
+    body:
+      encoding: UTF-8
+      string: '{"result":"cHVzaC1yZXN1bHQ=\n"}'
+    http_version:
+  recorded_at: Fri, 20 Jul 2018 06:19:08 GMT
+recorded_with: VCR 2.4.0

spec/vcr_cassettes/custom-action-ok.yml

@@ -46,7 +46,7 @@ http_interactions:
       - '1.436040'
     body:
       encoding: UTF-8
-      string: '{"result":"info_refs-result"}'
+      string: '{"result":"aW5mb19yZWZzLXJlc3VsdA==\n"}'
     http_version:
   recorded_at: Fri, 20 Jul 2018 06:18:58 GMT
 - request:
@@ -93,7 +93,7 @@ http_interactions:
       - '0.786754'
     body:
       encoding: UTF-8
-      string: '{"result":"push-result"}'
+      string: '{"result":"cHVzaC1yZXN1bHQ=\n"}'
     http_version:
   recorded_at: Fri, 20 Jul 2018 06:19:08 GMT
 recorded_with: VCR 2.4.0