Merge branch 'ash.mckenzie/minor-tidy-up' into 'master'

Refactor for re-usability for future MR's

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

.rubocop.yml

@@ -67,3 +67,6 @@ Style/RegexpLiteral:
 
 Style/SpecialGlobalVars:
   Enabled: false
+
+Style/FrozenStringLiteralComment:
+  Enabled: false

.ruby-version

+2.3.7

Gemfile

 source "http://rubygems.org"
 
 group :development, :test do
-  gem 'guard'
-  gem 'guard-rspec'
-  gem 'rspec', '~> 2.14.0'
+  gem 'guard', '~> 1.5.0'
+  gem 'guard-rspec', '~> 2.1.0'
+  gem 'listen', '~> 0.5.0'
+  gem 'rspec', '~> 2.0'
+  gem 'rspec-its', '~> 1.0.0'
   gem 'rubocop', '0.49.1', require: false
-  gem 'simplecov', require: false
-  gem 'vcr'
-  gem 'webmock'
+  gem 'simplecov', '~> 0.9.0', require: false
+  gem 'vcr', '~> 2.4.0'
+  gem 'webmock', '~> 1.9.0'
 end

Gemfile.lock

 GEM
   remote: http://rubygems.org/
   specs:
-    addressable (2.3.2)
+    addressable (2.5.2)
+      public_suffix (>= 2.0.2, < 4.0)
     ast (2.4.0)
-    coderay (1.0.8)
-    crack (0.3.2)
-    diff-lcs (1.2.5)
+    coderay (1.1.2)
+    crack (0.4.3)
+      safe_yaml (~> 1.0.0)
+    diff-lcs (1.3)
     docile (1.1.5)
     guard (1.5.4)
       listen (>= 0.4.2)
@@ -16,28 +18,31 @@ GEM
       guard (>= 1.1)
       rspec (~> 2.11)
     listen (0.5.3)
-    lumberjack (1.0.2)
-    method_source (0.8.1)
-    multi_json (1.10.1)
+    lumberjack (1.0.13)
+    method_source (0.9.0)
+    multi_json (1.13.1)
     parallel (1.12.1)
-    parser (2.5.0.2)
+    parser (2.5.1.2)
       ast (~> 2.4.0)
-    powerpack (0.1.1)
-    pry (0.9.10)
-      coderay (~> 1.0.5)
-      method_source (~> 0.8)
-      slop (~> 3.3.1)
+    powerpack (0.1.2)
+    pry (0.11.3)
+      coderay (~> 1.1.0)
+      method_source (~> 0.9.0)
+    public_suffix (3.0.2)
     rainbow (2.2.2)
       rake
-    rake (12.3.0)
-    rspec (2.14.1)
-      rspec-core (~> 2.14.0)
-      rspec-expectations (~> 2.14.0)
-      rspec-mocks (~> 2.14.0)
-    rspec-core (2.14.8)
-    rspec-expectations (2.14.5)
+    rake (12.3.1)
+    rspec (2.99.0)
+      rspec-core (~> 2.99.0)
+      rspec-expectations (~> 2.99.0)
+      rspec-mocks (~> 2.99.0)
+    rspec-core (2.99.2)
+    rspec-expectations (2.99.2)
       diff-lcs (>= 1.1.3, < 2.0)
-    rspec-mocks (2.14.6)
+    rspec-its (1.0.1)
+      rspec-core (>= 2.99.0.beta1)
+      rspec-expectations (>= 2.99.0.beta1)
+    rspec-mocks (2.99.4)
     rubocop (0.49.1)
       parallel (~> 1.10)
       parser (>= 2.3.3.1, < 3.0)
@@ -46,30 +51,32 @@ GEM
       ruby-progressbar (~> 1.7)
       unicode-display_width (~> 1.0, >= 1.0.1)
     ruby-progressbar (1.9.0)
-    simplecov (0.9.1)
+    safe_yaml (1.0.4)
+    simplecov (0.9.2)
       docile (~> 1.1.0)
       multi_json (~> 1.0)
-      simplecov-html (~> 0.8.0)
-    simplecov-html (0.8.0)
-    slop (3.3.3)
-    thor (0.19.1)
-    unicode-display_width (1.3.0)
+      simplecov-html (~> 0.9.0)
+    simplecov-html (0.9.0)
+    thor (0.20.0)
+    unicode-display_width (1.4.0)
     vcr (2.4.0)
-    webmock (1.9.0)
+    webmock (1.9.3)
       addressable (>= 2.2.7)
-      crack (>= 0.1.7)
+      crack (>= 0.3.2)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
-  guard
-  guard-rspec
-  rspec (~> 2.14.0)
+  guard (~> 1.5.0)
+  guard-rspec (~> 2.1.0)
+  listen (~> 0.5.0)
+  rspec (~> 2.0)
+  rspec-its (~> 1.0.0)
   rubocop (= 0.49.1)
-  simplecov
-  vcr
-  webmock
+  simplecov (~> 0.9.0)
+  vcr (~> 2.4.0)
+  webmock (~> 1.9.0)
 
 BUNDLED WITH
-   1.16.1
+   1.16.3

lib/gitlab_net.rb

@@ -7,13 +7,15 @@ require_relative 'gitlab_logger'
 require_relative 'gitlab_access'
 require_relative 'gitlab_lfs_authentication'
 require_relative 'httpunix'
+require_relative 'http_helper'
 
 class GitlabNet # rubocop:disable Metrics/ClassLength
+  include HTTPHelper
+
   class ApiUnreachableError < StandardError; end
   class NotFound < StandardError; end
 
   CHECK_TIMEOUT = 5
-  READ_TIMEOUT = 300
 
   def check_access(cmd, gl_repository, repo, actor, changes, protocol, env: {})
     changes = changes.join("\n") unless changes.is_a?(String)
@@ -33,7 +35,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
       params[:user_id] = actor.gsub("user-", "")
     end
 
-    url = "#{host}/allowed"
+    url = "#{internal_api_endpoint}/allowed"
     resp = post(url, params)
 
     if resp.code == '200'
@@ -50,7 +52,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
 
   def discover(key)
     key_id = key.gsub("key-", "")
-    resp = get("#{host}/discover?key_id=#{key_id}")
+    resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}")
     JSON.parse(resp.body) rescue nil
   end
 
@@ -60,7 +62,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
       key_id: key.gsub('key-', '')
     }
 
-    resp = post("#{host}/lfs_authenticate", params)
+    resp = post("#{internal_api_endpoint}/lfs_authenticate", params)
 
     if resp.code == '200'
       GitlabLfsAuthentication.build_from_json(resp.body)
@@ -68,14 +70,14 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   end
 
   def broadcast_message
-    resp = get("#{host}/broadcast_message")
+    resp = get("#{internal_api_endpoint}/broadcast_message")
     JSON.parse(resp.body) rescue {}
   end
 
   def merge_request_urls(gl_repository, repo_path, changes)
     changes = changes.join("\n") unless changes.is_a?(String)
     changes = changes.encode('UTF-8', 'ASCII', invalid: :replace, replace: '')
-    url = "#{host}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
+    url = "#{internal_api_endpoint}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
     url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository
     resp = get(url)
 
@@ -89,11 +91,11 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   end
 
   def check
-    get("#{host}/check", read_timeout: CHECK_TIMEOUT)
+    get("#{internal_api_endpoint}/check", options: { read_timeout: CHECK_TIMEOUT })
   end
 
   def authorized_key(key)
-    resp = get("#{host}/authorized_keys?key=#{URI.escape(key, '+/=')}")
+    resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}")
     JSON.parse(resp.body) if resp.code == "200"
   rescue
     nil
@@ -101,7 +103,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
 
   def two_factor_recovery_codes(key)
     key_id = key.gsub('key-', '')
-    resp = post("#{host}/two_factor_recovery_codes", key_id: key_id)
+    resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id)
 
     JSON.parse(resp.body) if resp.code == '200'
   rescue
@@ -110,7 +112,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
 
   def notify_post_receive(gl_repository, repo_path)
     params = { gl_repository: gl_repository, project: repo_path }
-    resp = post("#{host}/notify_post_receive", params)
+    resp = post("#{internal_api_endpoint}/notify_post_receive", params)
 
     resp.code == '200'
   rescue
@@ -123,7 +125,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
       identifier: identifier,
       changes: changes
     }
-    resp = post("#{host}/post_receive", params)
+    resp = post("#{internal_api_endpoint}/post_receive", params)
 
     raise NotFound if resp.code == '404'
 
@@ -131,7 +133,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   end
 
   def pre_receive(gl_repository)
-    resp = post("#{host}/pre_receive", gl_repository: gl_repository)
+    resp = post("#{internal_api_endpoint}/pre_receive", gl_repository: gl_repository)
 
     raise NotFound if resp.code == '404'
 
@@ -143,107 +145,4 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   def sanitize_path(repo)
     repo.delete("'")
   end
-
-  def config
-    @config ||= GitlabConfig.new
-  end
-
-  def host
-    "#{config.gitlab_url}/api/v4/internal"
-  end
-
-  def http_client_for(uri, options = {})
-    http = if uri.is_a?(URI::HTTPUNIX)
-             Net::HTTPUNIX.new(uri.hostname)
-           else
-             Net::HTTP.new(uri.host, uri.port)
-           end
-
-    http.read_timeout = options[:read_timeout] || read_timeout
-
-    if uri.is_a?(URI::HTTPS)
-      http.use_ssl = true
-      http.cert_store = cert_store
-      http.verify_mode = OpenSSL::SSL::VERIFY_NONE if config.http_settings['self_signed_cert']
-    end
-
-    http
-  end
-
-  def http_request_for(method, uri, params = {})
-    request_klass = method == :get ? Net::HTTP::Get : Net::HTTP::Post
-    request = request_klass.new(uri.request_uri)
-
-    user = config.http_settings['user']
-    password = config.http_settings['password']
-    request.basic_auth(user, password) if user && password
-
-    request.set_form_data(params.merge(secret_token: secret_token))
-
-    if uri.is_a?(URI::HTTPUNIX)
-      # The HTTPUNIX HTTP client does not set a correct Host header. This can
-      # lead to 400 Bad Request responses.
-      request['Host'] = 'localhost'
-    end
-
-    request
-  end
-
-  def request(method, url, params = {}, options = {})
-    $logger.debug('Performing request', method: method.to_s.upcase, url: url)
-
-    uri = URI.parse(url)
-
-    http = http_client_for(uri, options)
-    request = http_request_for(method, uri, params)
-
-    begin
-      start_time = Time.new
-      response = http.start { http.request(request) }
-    rescue => e
-      $logger.warn('Failed to connect to internal API', method: method.to_s.upcase, url: url, error: e)
-      raise ApiUnreachableError
-    ensure
-      $logger.info('finished HTTP request', method: method.to_s.upcase, url: url, duration: Time.new - start_time)
-    end
-
-    if response.code == "200"
-      $logger.debug('Received response', code: response.code, body: response.body)
-    else
-      $logger.error('API call failed', method: method.to_s.upcase, url: url, code: response.code, body: response.body)
-    end
-
-    response
-  end
-
-  def get(url, options = {})
-    request(:get, url, {}, options)
-  end
-
-  def post(url, params)
-    request(:post, url, params)
-  end
-
-  def cert_store
-    @cert_store ||= begin
-      store = OpenSSL::X509::Store.new
-      store.set_default_paths
-
-      ca_file = config.http_settings['ca_file']
-      store.add_file(ca_file) if ca_file
-
-      ca_path = config.http_settings['ca_path']
-      store.add_path(ca_path) if ca_path
-
-      store
-    end
-  end
-
-  def secret_token
-    @secret_token ||= File.read config.secret_file
-  end
-
-  def read_timeout
-    config.http_settings['read_timeout'] || READ_TIMEOUT
-  end
 end

lib/http_helper.rb

+module HTTPHelper
+  READ_TIMEOUT = 300
+
+  protected
+
+  def config
+    @config ||= GitlabConfig.new
+  end
+
+  def base_api_endpoint
+    "#{config.gitlab_url}/api/v4"
+  end
+
+  def internal_api_endpoint
+    "#{base_api_endpoint}/internal"
+  end
+
+  def http_client_for(uri, options = {})
+    http = if uri.is_a?(URI::HTTPUNIX)
+             Net::HTTPUNIX.new(uri.hostname)
+           else
+             Net::HTTP.new(uri.host, uri.port)
+           end
+
+    http.read_timeout = options[:read_timeout] || read_timeout
+
+    if uri.is_a?(URI::HTTPS)
+      http.use_ssl = true
+      http.cert_store = cert_store
+      http.verify_mode = OpenSSL::SSL::VERIFY_NONE if config.http_settings['self_signed_cert']
+    end
+
+    http
+  end
+
+  def http_request_for(method, uri, params: {}, headers: {}, options: {})
+    request_klass = method == :get ? Net::HTTP::Get : Net::HTTP::Post
+    request = request_klass.new(uri.request_uri, headers)
+
+    user = config.http_settings['user']
+    password = config.http_settings['password']
+    request.basic_auth(user, password) if user && password
+
+    if options[:json]
+      request.body = options[:json].merge(secret_token: secret_token).to_json
+    else
+      request.set_form_data(params.merge(secret_token: secret_token))
+    end
+
+    if uri.is_a?(URI::HTTPUNIX)
+      # The HTTPUNIX HTTP client does not set a correct Host header. This can
+      # lead to 400 Bad Request responses.
+      request['Host'] = 'localhost'
+    end
+
+    request
+  end
+
+  def request(method, url, params: {}, headers: {}, options: {})
+    $logger.debug('Performing request', method: method.to_s.upcase, url: url)
+
+    uri = URI.parse(url)
+    http = http_client_for(uri, options)
+    request = http_request_for(method, uri,
+                               params: params,
+                               headers: headers,
+                               options: options)
+
+    begin
+      start_time = Time.new
+      response = http.start { http.request(request) }
+    rescue => e
+      $logger.warn('Failed to connect', method: method.to_s.upcase, url: url, error: e)
+      raise GitlabNet::ApiUnreachableError
+    ensure
+      $logger.info('finished HTTP request', method: method.to_s.upcase, url: url, duration: Time.new - start_time)
+    end
+
+    if response.code == "200"
+      $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)
+    end
+
+    response
+  end
+
+  def get(url, headers: {}, options: {})
+    request(:get, url, headers: headers, options: options)
+  end
+
+  def post(url, params, headers: {}, options: {})
+    request(:post, url, params: params, headers: headers, options: options)
+  end
+
+  def cert_store
+    @cert_store ||= begin
+      store = OpenSSL::X509::Store.new
+      store.set_default_paths
+
+      ca_file = config.http_settings['ca_file']
+      store.add_file(ca_file) if ca_file
+
+      ca_path = config.http_settings['ca_path']
+      store.add_path(ca_path) if ca_path
+
+      store
+    end
+  end
+
+  def secret_token
+    @secret_token ||= File.read config.secret_file
+  end
+
+  def read_timeout
+    config.http_settings['read_timeout'] || READ_TIMEOUT
+  end
+end

spec/gitlab_access_spec.rb

@@ -36,7 +36,7 @@ describe GitlabAccess do
     context "access is granted" do
 
       it "returns true" do
-        expect(subject.exec).to be_true
+        expect(subject.exec).to be_truthy
       end
     end
 
@@ -54,7 +54,7 @@ describe GitlabAccess do
       end
 
       it "returns false" do
-        expect(subject.exec).to be_false
+        expect(subject.exec).to be_falsey
       end
     end
 
@@ -65,7 +65,7 @@ describe GitlabAccess do
       end
 
       it "returns false" do
-        expect(subject.exec).to be_false
+        expect(subject.exec).to be_falsey
       end
     end
   end

spec/gitlab_keys_spec.rb

@@ -62,7 +62,7 @@ describe GitlabKeys do
       end
 
       it "should return true" do
-        gitlab_keys.send(:add_key).should be_true
+        gitlab_keys.send(:add_key).should be_truthy
       end
     end
   end
@@ -133,7 +133,7 @@ describe GitlabKeys do
       end
 
       it "should return true" do
-        gitlab_keys.send(:batch_add_keys).should be_true
+        gitlab_keys.send(:batch_add_keys).should be_truthy
       end
     end
   end
@@ -174,7 +174,7 @@ describe GitlabKeys do
       end
 
       it "should return true" do
-        gitlab_keys.send(:rm_key).should be_true
+        gitlab_keys.send(:rm_key).should be_truthy
       end
     end
 
@@ -201,7 +201,7 @@ describe GitlabKeys do
 
     it "should return true" do
       gitlab_keys.stub(:open)
-      gitlab_keys.send(:clear).should be_true
+      gitlab_keys.send(:clear).should be_truthy
     end
   end
 

spec/gitlab_metrics_spec.rb

@@ -3,6 +3,10 @@ require_relative '../lib/gitlab_metrics'
 
 describe GitlabMetrics do
   describe '.measure' do
+    before do
+      $logger = double('logger').as_null_object
+    end
+
     it 'returns the return value of the block' do
       val = described_class.measure('foo') { 10 }
 
@@ -10,7 +14,7 @@ describe GitlabMetrics do
     end
 
     it 'writes the metrics data to a log file' do
-      expect(described_class.logger).to receive(:debug).
+      expect($logger).to receive(:debug).
         with('metrics', a_metrics_log_message('foo'))
 
       described_class.measure('foo') { 10 }

spec/gitlab_net_spec.rb

@@ -3,20 +3,22 @@ require_relative '../lib/gitlab_net'
 require_relative '../lib/gitlab_access_status'
 
 describe GitlabNet, vcr: true do
-  let(:gitlab_net) { GitlabNet.new }
+  let(:gitlab_net) { described_class.new }
   let(:changes) { ['0000000000000000000000000000000000000000 92d0970eefd7acb6d548878925ce2208cfe2d2ec refs/heads/branch4'] }
-  let(:host) { 'http://localhost:3000/api/v4/internal' }
+  let(:base_api_endpoint) { 'http://localhost:3000/api/v4' }
+  let(:internal_api_endpoint) { 'http://localhost:3000/api/v4/internal' }
   let(:project) { 'gitlab-org/gitlab-test.git' }
   let(:key) { 'key-1' }
   let(:key2) { 'key-2' }
   let(:secret) { "0a3938d9d95d807e94d937af3a4fbbea\n" }
 
   before do
-    gitlab_net.stub(:host).and_return(host)
+    $logger = double('logger').as_null_object
+    gitlab_net.stub(:base_api_endpoint).and_return(base_api_endpoint)
     gitlab_net.stub(:secret_token).and_return(secret)
   end
 
-  describe :check do
+  describe '#check' do
     it 'should return 200 code for gitlab check' do
       VCR.use_cassette("check-ok") do
         result = gitlab_net.check
@@ -37,7 +39,7 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :discover do
+  describe '#discover' do
     it 'should return user has based on key id' do
       VCR.use_cassette("discover-ok") do
         user = gitlab_net.discover(key)
@@ -68,13 +70,13 @@ describe GitlabNet, vcr: true do
           lfs_access = gitlab_net.lfs_authenticate(key, project)
           lfs_access.username.should == 'root'
           lfs_access.lfs_token.should == 'Hyzhyde_wLUeyUQsR3tHGTG8eNocVQm4ssioTEsBSdb6KwCSzQ'
-          lfs_access.repository_http_path.should == URI.join(host.sub('api/v4', ''), project).to_s
+          lfs_access.repository_http_path.should == URI.join(internal_api_endpoint.sub('api/v4', ''), project).to_s
         end
       end
     end
   end
 
-  describe :broadcast_message do
+  describe '#broadcast_message' do
     context "broadcast message exists" do
       it 'should return message' do
         VCR.use_cassette("broadcast_message-ok") do
@@ -94,19 +96,19 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :merge_request_urls do
+  describe '#merge_request_urls' do
     let(:gl_repository) { "project-1" }
     let(:changes) { "123456 789012 refs/heads/test\n654321 210987 refs/tags/tag" }
     let(:encoded_changes) { "123456%20789012%20refs/heads/test%0A654321%20210987%20refs/tags/tag" }
 
     it "sends the given arguments as encoded URL parameters" do
-      gitlab_net.should_receive(:get).with("#{host}/merge_request_urls?project=#{project}&changes=#{encoded_changes}&gl_repository=#{gl_repository}")
+      gitlab_net.should_receive(:get).with("#{internal_api_endpoint}/merge_request_urls?project=#{project}&changes=#{encoded_changes}&gl_repository=#{gl_repository}")
 
       gitlab_net.merge_request_urls(gl_repository, project, changes)
     end
 
     it "omits the gl_repository parameter if it's nil" do
-      gitlab_net.should_receive(:get).with("#{host}/merge_request_urls?project=#{project}&changes=#{encoded_changes}")
+      gitlab_net.should_receive(:get).with("#{internal_api_endpoint}/merge_request_urls?project=#{project}&changes=#{encoded_changes}")
 
       gitlab_net.merge_request_urls(nil, project, changes)
     end
@@ -126,7 +128,7 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :pre_receive do
+  describe '#pre_receive' do
     let(:gl_repository) { "project-1" }
     let(:params) { { gl_repository: gl_repository } }
 
@@ -152,7 +154,7 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :post_receive do
+  describe '#post_receive' do
     let(:gl_repository) { "project-1" }
     let(:changes) { "123456 789012 refs/heads/test\n654321 210987 refs/tags/tag" }
     let(:params) do
@@ -192,7 +194,7 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :authorized_key do
+  describe '#authorized_key' do
     let (:ssh_key) { "rsa-key" }
 
     it "should return nil when the resource is not implemented" do
@@ -227,7 +229,7 @@ describe GitlabNet, vcr: true do
     it 'returns two factor recovery codes' do
       VCR.use_cassette('two-factor-recovery-codes') do
         result = gitlab_net.two_factor_recovery_codes(key)
-        expect(result['success']).to be_true
+        expect(result['success']).to be_truthy
         expect(result['recovery_codes']).to eq(['f67c514de60c4953','41278385fc00c1e0'])
       end
     end
@@ -235,7 +237,7 @@ describe GitlabNet, vcr: true do
     it 'returns false when recovery codes cannot be generated' do
       VCR.use_cassette('two-factor-recovery-codes-fail') do
         result = gitlab_net.two_factor_recovery_codes('key-777')
-        expect(result['success']).to be_false
+        expect(result['success']).to be_falsey
         expect(result['message']).to eq('Could not find the given key')
       end
     end
@@ -257,17 +259,17 @@ describe GitlabNet, vcr: true do
 
     it 'returns true if notification was succesful' do
       VCR.use_cassette('notify-post-receive') do
-        expect(gitlab_net.notify_post_receive(gl_repository, repo_path)).to be_true
+        expect(gitlab_net.notify_post_receive(gl_repository, repo_path)).to be_truthy
       end
     end
   end
 
-  describe :check_access do
+  describe '#check_access' do
     context 'ssh key with access nil, to project' do
       it 'should allow pull access for host' do
         VCR.use_cassette("allowed-pull") do
           access = gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh')
-          access.allowed?.should be_true
+          access.allowed?.should be_truthy
         end
       end
 
@@ -281,7 +283,7 @@ describe GitlabNet, vcr: true do
       it 'should allow push access for host' do
         VCR.use_cassette("allowed-push") do
           access = gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'ssh')
-          access.allowed?.should be_true
+          access.allowed?.should be_truthy
         end
       end
     end
@@ -290,7 +292,7 @@ describe GitlabNet, vcr: true do
       it 'should deny pull access for host' do
         VCR.use_cassette('ssh-pull-disabled') do
           access = gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'ssh')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
           access.message.should eq 'Git access over SSH is not allowed'
         end
       end
@@ -298,7 +300,7 @@ describe GitlabNet, vcr: true do
       it 'should deny push access for host' do
         VCR.use_cassette('ssh-push-disabled') do
           access = gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'ssh')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
           access.message.should eq 'Git access over SSH is not allowed'
         end
       end
@@ -308,7 +310,7 @@ describe GitlabNet, vcr: true do
       it 'should deny pull access for host' do
         VCR.use_cassette('http-pull-disabled') do
           access = gitlab_net.check_access('git-upload-pack', nil, project, key, changes, 'http')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
           access.message.should eq 'Pulling over HTTP is not allowed.'
         end
       end
@@ -316,7 +318,7 @@ describe GitlabNet, vcr: true do
       it 'should deny push access for host' do
         VCR.use_cassette("http-push-disabled") do
           access = gitlab_net.check_access('git-receive-pack', nil, project, key, changes, 'http')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
           access.message.should eq 'Pushing over HTTP is not allowed.'
         end
       end
@@ -326,21 +328,21 @@ describe GitlabNet, vcr: true do
       it 'should deny pull access for host' do
         VCR.use_cassette("ssh-pull-project-denied") do
           access = gitlab_net.check_access('git-receive-pack', nil, project, key2, changes, 'ssh')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
         end
       end
 
       it 'should deny push access for host' do
         VCR.use_cassette("ssh-push-project-denied") do
           access = gitlab_net.check_access('git-upload-pack', nil, project, key2, changes, 'ssh')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
         end
       end
 
       it 'should deny push access for host (with user)' do
         VCR.use_cassette("ssh-push-project-denied-with-user") do
           access = gitlab_net.check_access('git-upload-pack', nil, project, 'user-2', changes, 'ssh')
-          access.allowed?.should be_false
+          access.allowed?.should be_falsey
         end
       end
     end
@@ -353,16 +355,27 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :host do
-    let(:net) { GitlabNet.new }
-    subject { net.send :host }
+  describe '#base_api_endpoint' do
+    let(:net) { described_class.new }
 
-    it { should include(net.send(:config).gitlab_url) }
-    it("uses API version 4") { should include("api/v4") }
+    subject { net.send :base_api_endpoint }
+
+    it { is_expected.to include(net.send(:config).gitlab_url) }
+    it("uses API version 4") { should end_with("api/v4") }
+  end
+
+  describe '#internal_api_endpoint' do
+    let(:net) { described_class.new }
+
+    subject { net.send :internal_api_endpoint }
+
+    it { is_expected.to include(net.send(:config).gitlab_url) }
+    it("uses API version 4") { should end_with("api/v4/internal") }
   end
 
-  describe :http_client_for do
+  describe '#http_client_for' do
     subject { gitlab_net.send :http_client_for, URI('https://localhost/') }
+
     before do
       gitlab_net.stub :cert_store
       gitlab_net.send(:config).stub(:http_settings) { {'self_signed_cert' => true} }
@@ -371,26 +384,74 @@ describe GitlabNet, vcr: true do
     its(:verify_mode) { should eq(OpenSSL::SSL::VERIFY_NONE) }
   end
 
-  describe :http_request_for do
+  describe '#http_request_for' do
     context 'with stub' do
-      let(:get) do
-        double(Net::HTTP::Get).tap do |get|
-          Net::HTTP::Get.stub(:new) { get }
-        end
-      end
+      let(:get) { double(Net::HTTP::Get) }
       let(:user) { 'user' }
       let(:password) { 'password' }
       let(:url) { URI 'http://localhost/' }
-      subject { gitlab_net.send :http_request_for, :get, url }
+      let(:params) { { 'key1' => 'value1' } }
+      let(:headers) { { 'Content-Type' => 'application/json'} }
+      let(:options) { { json: { 'key2' => 'value2' } } }
+
+      context 'with no params, options or headers' do
+        subject { gitlab_net.send :http_request_for, :get, url }
+
+        before do
+          gitlab_net.send(:config).http_settings.stub(:[]).with('user') { user }
+          gitlab_net.send(:config).http_settings.stub(:[]).with('password') { password }
+          Net::HTTP::Get.should_receive(:new).with('/', {}).and_return(get)
+          get.should_receive(:basic_auth).with(user, password).once
+          get.should_receive(:set_form_data).with(hash_including(secret_token: secret)).once
+        end
 
-      before do
-        gitlab_net.send(:config).http_settings.stub(:[]).with('user') { user }
-        gitlab_net.send(:config).http_settings.stub(:[]).with('password') { password }
-        get.should_receive(:basic_auth).with(user, password).once
-        get.should_receive(:set_form_data).with(hash_including(secret_token: secret)).once
+        it { should_not be_nil }
       end
 
-      it { should_not be_nil }
+      context 'with params' do
+        subject { gitlab_net.send :http_request_for, :get, url, params: params, headers: headers }
+
+        before do
+          gitlab_net.send(:config).http_settings.stub(:[]).with('user') { user }
+          gitlab_net.send(:config).http_settings.stub(:[]).with('password') { password }
+          Net::HTTP::Get.should_receive(:new).with('/', headers).and_return(get)
+          get.should_receive(:basic_auth).with(user, password).once
+          get.should_receive(:set_form_data).with({ 'key1' => 'value1', secret_token: secret }).once
+        end
+
+        it { should_not be_nil }
+      end
+
+      context 'with headers' do
+        subject { gitlab_net.send :http_request_for, :get, url, headers: headers }
+
+        before do
+          gitlab_net.send(:config).http_settings.stub(:[]).with('user') { user }
+          gitlab_net.send(:config).http_settings.stub(:[]).with('password') { password }
+          Net::HTTP::Get.should_receive(:new).with('/', headers).and_return(get)
+          get.should_receive(:basic_auth).with(user, password).once
+          get.should_receive(:set_form_data).with(hash_including(secret_token: secret)).once
+        end
+
+        it { should_not be_nil }
+      end
+
+      context 'with options' do
+        context 'with json' do
+          subject { gitlab_net.send :http_request_for, :get, url, options: options }
+
+          before do
+            gitlab_net.send(:config).http_settings.stub(:[]).with('user') { user }
+            gitlab_net.send(:config).http_settings.stub(:[]).with('password') { password }
+            Net::HTTP::Get.should_receive(:new).with('/', {}).and_return(get)
+            get.should_receive(:basic_auth).with(user, password).once
+            get.should_receive(:body=).with({ 'key2' => 'value2', secret_token: secret }.to_json).once
+            get.should_not_receive(:set_form_data)
+          end
+
+          it { should_not be_nil }
+        end
+      end
     end
 
     context 'Unix socket' do
@@ -405,7 +466,7 @@ describe GitlabNet, vcr: true do
     end
   end
 
-  describe :cert_store do
+  describe '#cert_store' do
     let(:store) do
       double(OpenSSL::X509::Store).tap do |store|
         OpenSSL::X509::Store.stub(:new) { store }

spec/gitlab_shell_spec.rb

@@ -4,6 +4,7 @@ require_relative '../lib/gitlab_access_status'
 
 describe GitlabShell do
   before do
+    $logger = double('logger').as_null_object
     FileUtils.mkdir_p(tmp_repos_path)
   end
 
@@ -428,7 +429,7 @@ describe GitlabShell do
 
         it "refuses to assign the path" do
           $stderr.should_receive(:puts).with("GitLab: Invalid repository path")
-          expect(subject.exec(ssh_cmd)).to be_false
+          expect(subject.exec(ssh_cmd)).to be_falsey
         end
       end
     end

spec/spec_helper.rb

 ROOT_PATH = File.expand_path(File.join(File.dirname(__FILE__), ".."))
 
+require 'rspec/its'
+
 require 'simplecov'
 SimpleCov.start