diff options
author | Robert Speicher <robert@gitlab.com> | 2018-07-23 19:03:55 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2018-07-23 19:03:55 +0000 |
commit | f59cd678e22c02a8f5a474bb982c22cdde0134c1 (patch) | |
tree | 09b5c83caadd778e4173257469ade986665ce12e | |
parent | 6413c4bec177d8d2f10fe4513cf7ab339aeb68f0 (diff) | |
parent | c92bfcefece5eea1fa30f75f7e99212c09b5792e (diff) | |
download | gitlab-shell-f59cd678e22c02a8f5a474bb982c22cdde0134c1.tar.gz |
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
-rw-r--r-- | .rubocop.yml | 3 | ||||
-rw-r--r-- | .ruby-version | 1 | ||||
-rw-r--r-- | Gemfile | 14 | ||||
-rw-r--r-- | Gemfile.lock | 79 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 129 | ||||
-rw-r--r-- | lib/http_helper.rb | 118 | ||||
-rw-r--r-- | spec/gitlab_access_spec.rb | 6 | ||||
-rw-r--r-- | spec/gitlab_keys_spec.rb | 8 | ||||
-rw-r--r-- | spec/gitlab_metrics_spec.rb | 6 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 153 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 3 | ||||
-rw-r--r-- | spec/spec_helper.rb | 2 |
12 files changed, 310 insertions, 212 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 7ea677e..66b7951 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -67,3 +67,6 @@ Style/RegexpLiteral: Style/SpecialGlobalVars: Enabled: false + +Style/FrozenStringLiteralComment: + Enabled: false diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 0000000..00355e2 --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +2.3.7 @@ -1,11 +1,13 @@ 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 diff --git a/Gemfile.lock b/Gemfile.lock index 49a08e8..88fc8c0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,11 +1,13 @@ 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 diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index c93cf9a..584dd93 100644 --- a/lib/gitlab_net.rb +++ b/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 diff --git a/lib/http_helper.rb b/lib/http_helper.rb new file mode 100644 index 0000000..62d0c51 --- /dev/null +++ b/lib/http_helper.rb @@ -0,0 +1,118 @@ +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 diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index c8660a8..8882e01 100644 --- a/spec/gitlab_access_spec.rb +++ b/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 diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb index c4e617c..d6583b8 100644 --- a/spec/gitlab_keys_spec.rb +++ b/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 diff --git a/spec/gitlab_metrics_spec.rb b/spec/gitlab_metrics_spec.rb index 024a85a..00e94b5 100644 --- a/spec/gitlab_metrics_spec.rb +++ b/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 } diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 8e06fa8..be2f4ba 100644 --- a/spec/gitlab_net_spec.rb +++ b/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 } diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index eef0caf..af84b29 100644 --- a/spec/gitlab_shell_spec.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d93f88d..bce3eff 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,7 @@ ROOT_PATH = File.expand_path(File.join(File.dirname(__FILE__), "..")) +require 'rspec/its' + require 'simplecov' SimpleCov.start |