diff options
Diffstat (limited to 'spec/lib/gitlab')
56 files changed, 1778 insertions, 323 deletions
diff --git a/spec/lib/gitlab/background_migration/reset_merge_status_spec.rb b/spec/lib/gitlab/background_migration/reset_merge_status_spec.rb new file mode 100644 index 00000000000..740781f1aa5 --- /dev/null +++ b/spec/lib/gitlab/background_migration/reset_merge_status_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Gitlab::BackgroundMigration::ResetMergeStatus, :migration, schema: 20190528180441 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') } + let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') } + let(:merge_requests) { table(:merge_requests) } + + def create_merge_request(id, extra_params = {}) + params = { + id: id, + target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: 'mr name', + title: "mr name#{id}" + }.merge(extra_params) + + merge_requests.create!(params) + end + + it 'correctly updates opened mergeable MRs to unchecked' do + create_merge_request(1, state: 'opened', merge_status: 'can_be_merged') + create_merge_request(2, state: 'opened', merge_status: 'can_be_merged') + create_merge_request(3, state: 'opened', merge_status: 'can_be_merged') + create_merge_request(4, state: 'merged', merge_status: 'can_be_merged') + create_merge_request(5, state: 'opened', merge_status: 'cannot_be_merged') + + subject.perform(1, 5) + + expected_rows = [ + { id: 1, state: 'opened', merge_status: 'unchecked' }, + { id: 2, state: 'opened', merge_status: 'unchecked' }, + { id: 3, state: 'opened', merge_status: 'unchecked' }, + { id: 4, state: 'merged', merge_status: 'can_be_merged' }, + { id: 5, state: 'opened', merge_status: 'cannot_be_merged' } + ] + + rows = merge_requests.order(:id).map do |row| + row.attributes.slice('id', 'state', 'merge_status').symbolize_keys + end + + expect(rows).to eq(expected_rows) + end +end diff --git a/spec/lib/gitlab/background_migration/schedule_calculate_wiki_sizes_spec.rb b/spec/lib/gitlab/background_migration/schedule_calculate_wiki_sizes_spec.rb new file mode 100644 index 00000000000..d494ce68c5b --- /dev/null +++ b/spec/lib/gitlab/background_migration/schedule_calculate_wiki_sizes_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20190527194900_schedule_calculate_wiki_sizes.rb') + +describe ScheduleCalculateWikiSizes, :migration, :sidekiq do + let(:migration_class) { Gitlab::BackgroundMigration::CalculateWikiSizes } + let(:migration_name) { migration_class.to_s.demodulize } + + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:project_statistics) { table(:project_statistics) } + + context 'when missing wiki sizes exist' do + before do + namespaces.create!(id: 1, name: 'wiki-migration', path: 'wiki-migration') + projects.create!(id: 1, name: 'wiki-project-1', path: 'wiki-project-1', namespace_id: 1) + projects.create!(id: 2, name: 'wiki-project-2', path: 'wiki-project-2', namespace_id: 1) + projects.create!(id: 3, name: 'wiki-project-3', path: 'wiki-project-3', namespace_id: 1) + project_statistics.create!(id: 1, project_id: 1, namespace_id: 1, wiki_size: 1000) + project_statistics.create!(id: 2, project_id: 2, namespace_id: 1, wiki_size: nil) + project_statistics.create!(id: 3, project_id: 3, namespace_id: 1, wiki_size: nil) + end + + it 'schedules a background migration' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(migration_name).to be_scheduled_delayed_migration(5.minutes, 2, 3) + expect(BackgroundMigrationWorker.jobs.size).to eq 1 + end + end + end + + it 'calculates missing wiki sizes' do + expect(project_statistics.find_by(id: 2).wiki_size).to be_nil + expect(project_statistics.find_by(id: 3).wiki_size).to be_nil + + migrate! + + expect(project_statistics.find_by(id: 2).wiki_size).not_to be_nil + expect(project_statistics.find_by(id: 3).wiki_size).not_to be_nil + end + end + + context 'when missing wiki sizes do not exist' do + before do + namespaces.create!(id: 1, name: 'wiki-migration', path: 'wiki-migration') + projects.create!(id: 1, name: 'wiki-project-1', path: 'wiki-project-1', namespace_id: 1) + project_statistics.create!(id: 1, project_id: 1, namespace_id: 1, wiki_size: 1000) + end + + it 'does not schedule a background migration' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(BackgroundMigrationWorker.jobs.size).to eq 0 + end + end + end + end +end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index a02c00e3340..2e90f6c7f71 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -5,6 +5,7 @@ describe Gitlab::BitbucketImport::Importer do before do stub_omniauth_provider('bitbucket') + stub_feature_flags(stricter_mr_branch_name: false) end let(:statuses) do diff --git a/spec/lib/gitlab/ci/config/entry/service_spec.rb b/spec/lib/gitlab/ci/config/entry/service_spec.rb index d5bd139b5f1..d31866a1987 100644 --- a/spec/lib/gitlab/ci/config/entry/service_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/service_spec.rb @@ -112,6 +112,16 @@ describe Gitlab::Ci::Config::Entry::Service do it 'is valid' do expect(entry).to be_valid end + + context 'when unknown port keys detected' do + let(:ports) { [{ number: 80, invalid_key: 'foo' }] } + + it 'is not valid' do + expect(entry).not_to be_valid + expect(entry.errors.first) + .to match /port config contains unknown keys: invalid_key/ + end + end end describe '#ports' do diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index d8a61618e77..46d68097fff 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Remote do + include StubRequests + let(:context) { described_class::Context.new(nil, '12345', nil, Set.new) } let(:params) { { remote: location } } let(:remote_file) { described_class.new(params, context) } @@ -46,7 +48,7 @@ describe Gitlab::Ci::Config::External::File::Remote do describe "#valid?" do context 'when is a valid remote url' do before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content) + stub_full_request(location).to_return(body: remote_file_content) end it 'returns true' do @@ -92,7 +94,7 @@ describe Gitlab::Ci::Config::External::File::Remote do describe "#content" do context 'with a valid remote file' do before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content) + stub_full_request(location).to_return(body: remote_file_content) end it 'returns the content of the file' do @@ -114,7 +116,7 @@ describe Gitlab::Ci::Config::External::File::Remote do let(:location) { 'https://asdasdasdaj48ggerexample.com' } before do - WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error')) + stub_full_request(location).to_raise(SocketError.new('Some HTTP error')) end it 'is nil' do @@ -144,7 +146,7 @@ describe Gitlab::Ci::Config::External::File::Remote do context 'when timeout error has been raised' do before do - WebMock.stub_request(:get, location).to_timeout + stub_full_request(location).to_timeout end it 'returns error message about a timeout' do @@ -154,7 +156,7 @@ describe Gitlab::Ci::Config::External::File::Remote do context 'when HTTP error has been raised' do before do - WebMock.stub_request(:get, location).to_raise(Gitlab::HTTP::Error) + stub_full_request(location).to_raise(Gitlab::HTTP::Error) end it 'returns error message about a HTTP error' do @@ -164,7 +166,7 @@ describe Gitlab::Ci::Config::External::File::Remote do context 'when response has 404 status' do before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content, status: 404) + stub_full_request(location).to_return(body: remote_file_content, status: 404) end it 'returns error message about a timeout' do diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index 136974569de..e068b786b02 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Mapper do + include StubRequests + set(:project) { create(:project, :repository) } set(:user) { create(:user) } @@ -18,7 +20,7 @@ describe Gitlab::Ci::Config::External::Mapper do end before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) + stub_full_request(remote_url).to_return(body: file_content) end describe '#process' do diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 0f58a4f1d44..856187371e1 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Processor do + include StubRequests + set(:project) { create(:project, :repository) } set(:another_project) { create(:project, :repository) } set(:user) { create(:user) } @@ -42,7 +44,7 @@ describe Gitlab::Ci::Config::External::Processor do let(:values) { { include: remote_file, image: 'ruby:2.2' } } before do - WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error')) + stub_full_request(remote_file).and_raise(SocketError.new('Some HTTP error')) end it 'raises an error' do @@ -75,7 +77,7 @@ describe Gitlab::Ci::Config::External::Processor do end before do - WebMock.stub_request(:get, remote_file).to_return(body: external_file_content) + stub_full_request(remote_file).to_return(body: external_file_content) end it 'appends the file to the values' do @@ -145,7 +147,7 @@ describe Gitlab::Ci::Config::External::Processor do allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) .to receive(:fetch_local_content).and_return(local_file_content) - WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) + stub_full_request(remote_file).to_return(body: remote_file_content) end it 'appends the files to the values' do @@ -191,7 +193,8 @@ describe Gitlab::Ci::Config::External::Processor do end it 'takes precedence' do - WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) + stub_full_request(remote_file).to_return(body: remote_file_content) + expect(processor.perform[:image]).to eq('ruby:2.2') end end @@ -231,7 +234,8 @@ describe Gitlab::Ci::Config::External::Processor do HEREDOC end - WebMock.stub_request(:get, 'http://my.domain.com/config.yml').to_return(body: 'remote_build: { script: echo Hello World }') + stub_full_request('http://my.domain.com/config.yml') + .to_return(body: 'remote_build: { script: echo Hello World }') end context 'when project is public' do @@ -273,8 +277,10 @@ describe Gitlab::Ci::Config::External::Processor do context 'when config includes an external configuration file via SSL web request' do before do - stub_request(:get, 'https://sha256.badssl.com/fake.yml').to_return(body: 'image: ruby:2.6', status: 200) - stub_request(:get, 'https://self-signed.badssl.com/fake.yml') + stub_full_request('https://sha256.badssl.com/fake.yml', ip_address: '8.8.8.8') + .to_return(body: 'image: ruby:2.6', status: 200) + + stub_full_request('https://self-signed.badssl.com/fake.yml', ip_address: '8.8.8.9') .to_raise(OpenSSL::SSL::SSLError.new('SSL_connect returned=1 errno=0 state=error: certificate verify failed (self signed certificate)')) end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index fd2a29e4ddb..7f336ee853e 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Config do + include StubRequests + set(:user) { create(:user) } let(:config) do @@ -190,7 +192,6 @@ describe Gitlab::Ci::Config do let(:remote_file_content) do <<~HEREDOC variables: - AUTO_DEVOPS_DOMAIN: domain.example.com POSTGRES_USER: user POSTGRES_PASSWORD: testing-password POSTGRES_ENABLED: "true" @@ -217,8 +218,7 @@ describe Gitlab::Ci::Config do end before do - WebMock.stub_request(:get, remote_location) - .to_return(body: remote_file_content) + stub_full_request(remote_location).to_return(body: remote_file_content) allow(project.repository) .to receive(:blob_data_at).and_return(local_file_content) @@ -232,7 +232,6 @@ describe Gitlab::Ci::Config do "bundle install --jobs $(nproc) \"${FLAGS[@]}\"" ] variables = { - AUTO_DEVOPS_DOMAIN: "domain.example.com", POSTGRES_USER: "user", POSTGRES_PASSWORD: "testing-password", POSTGRES_ENABLED: "true", diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 0d998d89d73..29276d5b686 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' module Gitlab module Ci describe YamlProcessor do + include StubRequests + subject { described_class.new(config, user: nil) } describe '#build_attributes' do @@ -648,7 +650,7 @@ module Gitlab end before do - WebMock.stub_request(:get, 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml') + stub_full_request('https://gitlab.com/awesome-project/raw/master/.before-script-template.yml') .to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 32b90041c64..f7642182a17 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -2,7 +2,6 @@ require 'fast_spec_helper' require 'rspec-parameterized' -require 'webmock/rspec' require 'gitlab/danger/helper' @@ -19,39 +18,6 @@ describe Gitlab::Danger::Helper do end end - let(:teammate_json) do - <<~JSON - [ - { - "username": "in-gitlab-ce", - "name": "CE maintainer", - "projects":{ "gitlab-ce": "maintainer backend" } - }, - { - "username": "in-gitlab-ee", - "name": "EE reviewer", - "projects":{ "gitlab-ee": "reviewer frontend" } - } - ] - JSON - end - - let(:ce_teammate_matcher) do - satisfy do |teammate| - teammate.username == 'in-gitlab-ce' && - teammate.name == 'CE maintainer' && - teammate.projects == { 'gitlab-ce' => 'maintainer backend' } - end - end - - let(:ee_teammate_matcher) do - satisfy do |teammate| - teammate.username == 'in-gitlab-ee' && - teammate.name == 'EE reviewer' && - teammate.projects == { 'gitlab-ee' => 'reviewer frontend' } - end - end - let(:fake_git) { double('fake-git') } subject(:helper) { FakeDanger.new(git: fake_git) } @@ -119,69 +85,6 @@ describe Gitlab::Danger::Helper do end end - describe '#team' do - subject(:team) { helper.team } - - context 'HTTP failure' do - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(status: 404) - end - - it 'raises a pretty error' do - expect { team }.to raise_error(/Failed to read/) - end - end - - context 'JSON failure' do - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(body: 'INVALID JSON') - end - - it 'raises a pretty error' do - expect { team }.to raise_error(/Failed to parse/) - end - end - - context 'success' do - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(body: teammate_json) - end - - it 'returns an array of teammates' do - is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher) - end - - it 'memoizes the result' do - expect(team.object_id).to eq(helper.team.object_id) - end - end - end - - describe '#project_team' do - subject { helper.project_team } - - before do - WebMock - .stub_request(:get, 'https://about.gitlab.com/roulette.json') - .to_return(body: teammate_json) - end - - it 'filters team by project_name' do - expect(helper) - .to receive(:project_name) - .at_least(:once) - .and_return('gitlab-ce') - - is_expected.to contain_exactly(ce_teammate_matcher) - end - end - describe '#changes_by_category' do it 'categorizes changed files' do expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo qa/foo ee/changelogs/foo.yml] } diff --git a/spec/lib/gitlab/danger/roulette_spec.rb b/spec/lib/gitlab/danger/roulette_spec.rb new file mode 100644 index 00000000000..40dce0c5378 --- /dev/null +++ b/spec/lib/gitlab/danger/roulette_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'webmock/rspec' + +require 'gitlab/danger/roulette' + +describe Gitlab::Danger::Roulette do + let(:teammate_json) do + <<~JSON + [ + { + "username": "in-gitlab-ce", + "name": "CE maintainer", + "projects":{ "gitlab-ce": "maintainer backend" } + }, + { + "username": "in-gitlab-ee", + "name": "EE reviewer", + "projects":{ "gitlab-ee": "reviewer frontend" } + } + ] + JSON + end + + let(:ce_teammate_matcher) do + satisfy do |teammate| + teammate.username == 'in-gitlab-ce' && + teammate.name == 'CE maintainer' && + teammate.projects == { 'gitlab-ce' => 'maintainer backend' } + end + end + + let(:ee_teammate_matcher) do + satisfy do |teammate| + teammate.username == 'in-gitlab-ee' && + teammate.name == 'EE reviewer' && + teammate.projects == { 'gitlab-ee' => 'reviewer frontend' } + end + end + + subject(:roulette) { Object.new.extend(described_class) } + + describe '#team' do + subject(:team) { roulette.team } + + context 'HTTP failure' do + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(status: 404) + end + + it 'raises a pretty error' do + expect { team }.to raise_error(/Failed to read/) + end + end + + context 'JSON failure' do + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: 'INVALID JSON') + end + + it 'raises a pretty error' do + expect { team }.to raise_error(/Failed to parse/) + end + end + + context 'success' do + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: teammate_json) + end + + it 'returns an array of teammates' do + is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher) + end + + it 'memoizes the result' do + expect(team.object_id).to eq(roulette.team.object_id) + end + end + end + + describe '#project_team' do + subject { roulette.project_team('gitlab-ce') } + + before do + WebMock + .stub_request(:get, described_class::ROULETTE_DATA_URL) + .to_return(body: teammate_json) + end + + it 'filters team by project_name' do + is_expected.to contain_exactly(ce_teammate_matcher) + end + end +end diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb index 4bc0a4c1398..753c74ff814 100644 --- a/spec/lib/gitlab/danger/teammate_spec.rb +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true +require 'fast_spec_helper' + +require 'gitlab/danger/teammate' + describe Gitlab::Danger::Teammate do subject { described_class.new({ 'projects' => projects }) } let(:projects) { { project => capabilities } } @@ -9,15 +13,15 @@ describe Gitlab::Danger::Teammate do let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer database'] } it '#reviewer? supports multiple roles per project' do - expect(subject.reviewer?(project, 'backend')).to be_truthy + expect(subject.reviewer?(project, :backend)).to be_truthy end it '#traintainer? supports multiple roles per project' do - expect(subject.traintainer?(project, 'database')).to be_truthy + expect(subject.traintainer?(project, :database)).to be_truthy end it '#maintainer? supports multiple roles per project' do - expect(subject.maintainer?(project, 'frontend')).to be_truthy + expect(subject.maintainer?(project, :frontend)).to be_truthy end end @@ -25,15 +29,15 @@ describe Gitlab::Danger::Teammate do let(:capabilities) { 'reviewer backend' } it '#reviewer? supports one role per project' do - expect(subject.reviewer?(project, 'backend')).to be_truthy + expect(subject.reviewer?(project, :backend)).to be_truthy end it '#traintainer? supports one role per project' do - expect(subject.traintainer?(project, 'database')).to be_falsey + expect(subject.traintainer?(project, :database)).to be_falsey end it '#maintainer? supports one role per project' do - expect(subject.maintainer?(project, 'frontend')).to be_falsey + expect(subject.maintainer?(project, :frontend)).to be_falsey end end end diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb index 9ef987a0826..1f36fd5c6ef 100644 --- a/spec/lib/gitlab/data_builder/pipeline_spec.rb +++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb @@ -50,5 +50,14 @@ describe Gitlab::DataBuilder::Pipeline do it { expect(attributes[:variables]).to be_a(Array) } it { expect(attributes[:variables]).to contain_exactly({ key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1' }) } end + + context 'when pipeline is a detached merge request pipeline' do + let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } + let(:pipeline) { merge_request.all_pipelines.first } + + it 'returns a source ref' do + expect(attributes[:ref]).to eq(merge_request.source_branch) + end + end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 7644d83992f..cb4701e8edc 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -29,51 +29,6 @@ describe Gitlab::Git::Repository, :seed_helper do let(:storage_path) { TestEnv.repos_path } let(:user) { build(:user) } - describe '.create_hooks' do - let(:repo_path) { File.join(storage_path, 'hook-test.git') } - let(:hooks_dir) { File.join(repo_path, 'hooks') } - let(:target_hooks_dir) { Gitlab::Shell.new.hooks_path } - let(:existing_target) { File.join(repo_path, 'foobar') } - - before do - FileUtils.rm_rf(repo_path) - FileUtils.mkdir_p(repo_path) - end - - context 'hooks is a directory' do - let(:existing_file) { File.join(hooks_dir, 'my-file') } - - before do - FileUtils.mkdir_p(hooks_dir) - FileUtils.touch(existing_file) - described_class.create_hooks(repo_path, target_hooks_dir) - end - - it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } - it { expect(Dir[File.join(repo_path, "hooks.old.*/my-file")].count).to eq(1) } - end - - context 'hooks is a valid symlink' do - before do - FileUtils.mkdir_p existing_target - File.symlink(existing_target, hooks_dir) - described_class.create_hooks(repo_path, target_hooks_dir) - end - - it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } - end - - context 'hooks is a broken symlink' do - before do - FileUtils.rm_f(existing_target) - File.symlink(existing_target, hooks_dir) - described_class.create_hooks(repo_path, target_hooks_dir) - end - - it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) } - end - end - describe "Respond to" do subject { repository } @@ -1959,13 +1914,6 @@ describe Gitlab::Git::Repository, :seed_helper do expect { imported_repo.fsck }.not_to raise_exception end - it 'creates a symlink to the global hooks dir' do - imported_repo.create_from_bundle(valid_bundle_path) - hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') } - - expect(File.readlink(hooks_path)).to eq(Gitlab::Shell.new.hooks_path) - end - it 'raises an error if the bundle is an attempted malicious payload' do expect do imported_repo.create_from_bundle(malicious_bundle_path) diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb index 3ab04a1c46d..b63389af29f 100644 --- a/spec/lib/gitlab/git_ref_validator_spec.rb +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -1,31 +1,69 @@ require 'spec_helper' describe Gitlab::GitRefValidator do - it { expect(described_class.validate('feature/new')).to be_truthy } - it { expect(described_class.validate('implement_@all')).to be_truthy } - it { expect(described_class.validate('my_new_feature')).to be_truthy } - it { expect(described_class.validate('my-branch')).to be_truthy } - it { expect(described_class.validate('#1')).to be_truthy } - it { expect(described_class.validate('feature/refs/heads/foo')).to be_truthy } - it { expect(described_class.validate('feature/~new/')).to be_falsey } - it { expect(described_class.validate('feature/^new/')).to be_falsey } - it { expect(described_class.validate('feature/:new/')).to be_falsey } - it { expect(described_class.validate('feature/?new/')).to be_falsey } - it { expect(described_class.validate('feature/*new/')).to be_falsey } - it { expect(described_class.validate('feature/[new/')).to be_falsey } - it { expect(described_class.validate('feature/new/')).to be_falsey } - it { expect(described_class.validate('feature/new.')).to be_falsey } - it { expect(described_class.validate('feature\@{')).to be_falsey } - it { expect(described_class.validate('feature\new')).to be_falsey } - it { expect(described_class.validate('feature//new')).to be_falsey } - it { expect(described_class.validate('feature new')).to be_falsey } - it { expect(described_class.validate('refs/heads/')).to be_falsey } - it { expect(described_class.validate('refs/remotes/')).to be_falsey } - it { expect(described_class.validate('refs/heads/feature')).to be_falsey } - it { expect(described_class.validate('refs/remotes/origin')).to be_falsey } - it { expect(described_class.validate('-')).to be_falsey } - it { expect(described_class.validate('-branch')).to be_falsey } - it { expect(described_class.validate('.tag')).to be_falsey } - it { expect(described_class.validate('my branch')).to be_falsey } - it { expect(described_class.validate("\xA0\u0000\xB0")).to be_falsey } + using RSpec::Parameterized::TableSyntax + + context '.validate' do + it { expect(described_class.validate('feature/new')).to be true } + it { expect(described_class.validate('implement_@all')).to be true } + it { expect(described_class.validate('my_new_feature')).to be true } + it { expect(described_class.validate('my-branch')).to be true } + it { expect(described_class.validate('#1')).to be true } + it { expect(described_class.validate('feature/refs/heads/foo')).to be true } + it { expect(described_class.validate('feature/~new/')).to be false } + it { expect(described_class.validate('feature/^new/')).to be false } + it { expect(described_class.validate('feature/:new/')).to be false } + it { expect(described_class.validate('feature/?new/')).to be false } + it { expect(described_class.validate('feature/*new/')).to be false } + it { expect(described_class.validate('feature/[new/')).to be false } + it { expect(described_class.validate('feature/new/')).to be false } + it { expect(described_class.validate('feature/new.')).to be false } + it { expect(described_class.validate('feature\@{')).to be false } + it { expect(described_class.validate('feature\new')).to be false } + it { expect(described_class.validate('feature//new')).to be false } + it { expect(described_class.validate('feature new')).to be false } + it { expect(described_class.validate('refs/heads/')).to be false } + it { expect(described_class.validate('refs/remotes/')).to be false } + it { expect(described_class.validate('refs/heads/feature')).to be false } + it { expect(described_class.validate('refs/remotes/origin')).to be false } + it { expect(described_class.validate('-')).to be false } + it { expect(described_class.validate('-branch')).to be false } + it { expect(described_class.validate('+foo:bar')).to be false } + it { expect(described_class.validate('foo:bar')).to be false } + it { expect(described_class.validate('.tag')).to be false } + it { expect(described_class.validate('my branch')).to be false } + it { expect(described_class.validate("\xA0\u0000\xB0")).to be false } + end + + context '.validate_merge_request_branch' do + it { expect(described_class.validate_merge_request_branch('HEAD')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/new')).to be true } + it { expect(described_class.validate_merge_request_branch('implement_@all')).to be true } + it { expect(described_class.validate_merge_request_branch('my_new_feature')).to be true } + it { expect(described_class.validate_merge_request_branch('my-branch')).to be true } + it { expect(described_class.validate_merge_request_branch('#1')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/refs/heads/foo')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/~new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/^new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/:new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/?new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/*new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/[new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/new.')).to be false } + it { expect(described_class.validate_merge_request_branch('feature\@{')).to be false } + it { expect(described_class.validate_merge_request_branch('feature\new')).to be false } + it { expect(described_class.validate_merge_request_branch('feature//new')).to be false } + it { expect(described_class.validate_merge_request_branch('feature new')).to be false } + it { expect(described_class.validate_merge_request_branch('refs/heads/master')).to be true } + it { expect(described_class.validate_merge_request_branch('refs/heads/')).to be false } + it { expect(described_class.validate_merge_request_branch('refs/remotes/')).to be false } + it { expect(described_class.validate_merge_request_branch('-')).to be false } + it { expect(described_class.validate_merge_request_branch('-branch')).to be false } + it { expect(described_class.validate_merge_request_branch('+foo:bar')).to be false } + it { expect(described_class.validate_merge_request_branch('foo:bar')).to be false } + it { expect(described_class.validate_merge_request_branch('.tag')).to be false } + it { expect(described_class.validate_merge_request_branch('my branch')).to be false } + it { expect(described_class.validate_merge_request_branch("\xA0\u0000\xB0")).to be false } + end end diff --git a/spec/lib/gitlab/github_import/parallel_importer_spec.rb b/spec/lib/gitlab/github_import/parallel_importer_spec.rb index f5df38c9aaf..ecab64a372a 100644 --- a/spec/lib/gitlab/github_import/parallel_importer_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_importer_spec.rb @@ -25,18 +25,9 @@ describe Gitlab::GithubImport::ParallelImporter do end it 'sets the JID in Redis' do - expect(Gitlab::SidekiqStatus) - .to receive(:set) - .with("github-importer/#{project.id}", StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) - .and_call_original + expect(Gitlab::Import::SetAsyncJid).to receive(:set_jid).with(project).and_call_original importer.execute end - - it 'updates the import JID of the project' do - importer.execute - - expect(project.import_state.reload.jid).to eq("github-importer/#{project.id}") - end end end diff --git a/spec/lib/gitlab/graphql/loaders/batch_project_statistics_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_project_statistics_loader_spec.rb new file mode 100644 index 00000000000..ec2fcad31e5 --- /dev/null +++ b/spec/lib/gitlab/graphql/loaders/batch_project_statistics_loader_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::Loaders::BatchProjectStatisticsLoader do + describe '#find' do + it 'only queries once for project statistics' do + stats = create_list(:project_statistics, 2) + project1 = stats.first.project + project2 = stats.last.project + + expect do + described_class.new(project1.id).find + described_class.new(project2.id).find + end.not_to exceed_query_limit(1) + end + end +end diff --git a/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb b/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb new file mode 100644 index 00000000000..66033736e01 --- /dev/null +++ b/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do + subject { described_class.new } + + describe '#analyze?' do + context 'feature flag disabled' do + before do + stub_feature_flags(graphql_logging: false) + end + + it 'disables the analyzer' do + expect(subject.analyze?(anything)).to be_falsey + end + end + + context 'feature flag enabled by default' do + it 'enables the analyzer' do + expect(subject.analyze?(anything)).to be_truthy + end + end + end +end diff --git a/spec/lib/gitlab/graphql_logger_spec.rb b/spec/lib/gitlab/graphql_logger_spec.rb new file mode 100644 index 00000000000..4977f98b83e --- /dev/null +++ b/spec/lib/gitlab/graphql_logger_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::GraphqlLogger do + subject { described_class.new('/dev/null') } + + let(:now) { Time.now } + + it 'builds a logger once' do + expect(::Logger).to receive(:new).and_call_original + + subject.info('hello world') + subject.error('hello again') + end + + context 'logging a GraphQL query' do + let(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) } + + it 'logs a query from JSON' do + analyzer_memo = { + query_string: query, + variables: {}, + complexity: 181, + depth: 0, + duration: 7 + } + + output = subject.format_message('INFO', now, 'test', analyzer_memo) + + data = JSON.parse(output) + expect(data['severity']).to eq('INFO') + expect(data['time']).to eq(now.utc.iso8601(3)) + expect(data['complexity']).to eq(181) + expect(data['variables']).to eq({}) + expect(data['depth']).to eq(0) + expect(data['duration']).to eq(7) + end + end +end diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb new file mode 100644 index 00000000000..930d1f62272 --- /dev/null +++ b/spec/lib/gitlab/http_connection_adapter_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::HTTPConnectionAdapter do + describe '#connection' do + context 'when local requests are not allowed' do + it 'sets up the connection' do + uri = URI('https://example.org') + + connection = described_class.new(uri).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + + it 'raises error when it is a request to local address' do + uri = URI('http://172.16.0.0/12') + + expect { described_class.new(uri).connection } + .to raise_error(Gitlab::HTTP::BlockedUrlError, + "URL 'http://172.16.0.0/12' is blocked: Requests to the local network are not allowed") + end + + it 'raises error when it is a request to localhost address' do + uri = URI('http://127.0.0.1') + + expect { described_class.new(uri).connection } + .to raise_error(Gitlab::HTTP::BlockedUrlError, + "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed") + end + + context 'when port different from URL scheme is used' do + it 'sets up the addr_port accordingly' do + uri = URI('https://example.org:8080') + + connection = described_class.new(uri).connection + + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org:8080') + expect(connection.port).to eq(8080) + end + end + end + + context 'when DNS rebinding protection is disabled' do + it 'sets up the connection' do + stub_application_setting(dns_rebinding_protection_enabled: false) + + uri = URI('https://example.org') + + connection = described_class.new(uri).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('example.org') + expect(connection.hostname_override).to eq(nil) + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + end + + context 'when http(s) environment variable is set' do + it 'sets up the connection' do + stub_env('https_proxy' => 'https://my.proxy') + + uri = URI('https://example.org') + + connection = described_class.new(uri).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('example.org') + expect(connection.hostname_override).to eq(nil) + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + end + + context 'when local requests are allowed' do + it 'sets up the connection' do + uri = URI('https://example.org') + + connection = described_class.new(uri, allow_local_requests: true).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + + it 'sets up the connection when it is a local network' do + uri = URI('http://172.16.0.0/12') + + connection = described_class.new(uri, allow_local_requests: true).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('172.16.0.0') + expect(connection.hostname_override).to be(nil) + expect(connection.addr_port).to eq('172.16.0.0') + expect(connection.port).to eq(80) + end + + it 'sets up the connection when it is localhost' do + uri = URI('http://127.0.0.1') + + connection = described_class.new(uri, allow_local_requests: true).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('127.0.0.1') + expect(connection.hostname_override).to be(nil) + expect(connection.addr_port).to eq('127.0.0.1') + expect(connection.port).to eq(80) + end + end + end +end diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb index 6c37c157f5d..158f77cab2c 100644 --- a/spec/lib/gitlab/http_spec.rb +++ b/spec/lib/gitlab/http_spec.rb @@ -1,6 +1,28 @@ require 'spec_helper' describe Gitlab::HTTP do + include StubRequests + + context 'when allow_local_requests' do + it 'sends the request to the correct URI' do + stub_full_request('https://example.org:8080', ip_address: '8.8.8.8').to_return(status: 200) + + described_class.get('https://example.org:8080', allow_local_requests: false) + + expect(WebMock).to have_requested(:get, 'https://8.8.8.8:8080').once + end + end + + context 'when not allow_local_requests' do + it 'sends the request to the correct URI' do + stub_full_request('https://example.org:8080') + + described_class.get('https://example.org:8080', allow_local_requests: true) + + expect(WebMock).to have_requested(:get, 'https://8.8.8.9:8080').once + end + end + describe 'allow_local_requests_from_hooks_and_services is' do before do WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') @@ -21,6 +43,8 @@ describe Gitlab::HTTP do context 'if allow_local_requests set to true' do it 'override the global value and allow requests to localhost or private network' do + stub_full_request('http://localhost:3003') + expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error end end @@ -32,6 +56,8 @@ describe Gitlab::HTTP do end it 'allow requests to localhost' do + stub_full_request('http://localhost:3003') + expect { described_class.get('http://localhost:3003') }.not_to raise_error end @@ -49,7 +75,7 @@ describe Gitlab::HTTP do describe 'handle redirect loops' do before do - WebMock.stub_request(:any, "http://example.org").to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep")) + stub_full_request("http://example.org", method: :any).to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep")) end it 'handles GET requests' do diff --git a/spec/lib/gitlab/import/set_async_jid_spec.rb b/spec/lib/gitlab/import/set_async_jid_spec.rb new file mode 100644 index 00000000000..51397280138 --- /dev/null +++ b/spec/lib/gitlab/import/set_async_jid_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::Import::SetAsyncJid do + describe '.set_jid', :clean_gitlab_redis_shared_state do + let(:project) { create(:project, :import_scheduled) } + + it 'sets the JID in Redis' do + expect(Gitlab::SidekiqStatus) + .to receive(:set) + .with("async-import/#{project.id}", StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) + .and_call_original + + described_class.set_jid(project) + end + + it 'updates the import JID of the project' do + described_class.set_jid(project) + + expect(project.import_state.reload.jid).to eq("async-import/#{project.id}") + end + end +end diff --git a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb index 7c4ac62790e..21a227335cd 100644 --- a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb +++ b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do + include StubRequests + let(:example_url) { 'http://www.example.com' } let(:strategy) { subject.new(url: example_url, http_method: 'post') } let!(:project) { create(:project, :with_export) } @@ -35,7 +37,7 @@ describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do context 'when upload fails' do it 'stores the export error' do - stub_request(:post, example_url).to_return(status: [404, 'Page not found']) + stub_full_request(example_url, method: :post).to_return(status: [404, 'Page not found']) strategy.execute(user, project) diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index 536cc359d39..99669285d5b 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do 'notid' => 99, 'import_source' => 'whatever', 'import_type' => 'whatever', - 'non_existent_attr' => 'whatever' + 'non_existent_attr' => 'whatever', + 'some_html' => '<p>dodgy html</p>', + 'legit_html' => '<p>legit html</p>', + '_html' => '<p>perfectly ordinary html</p>', + 'cached_markdown_version' => 12345 } end diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 67e4c289906..b95b5dfe791 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -12,7 +12,6 @@ describe Gitlab::ImportExport::MembersMapper do "access_level" => 40, "source_id" => 14, "source_type" => "Project", - "user_id" => 19, "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", @@ -25,7 +24,8 @@ describe Gitlab::ImportExport::MembersMapper do "id" => exported_user_id, "email" => user2.email, "username" => 'test' - } + }, + "user_id" => 19 }, { "id" => 3, @@ -73,6 +73,22 @@ describe Gitlab::ImportExport::MembersMapper do expect(user2.authorized_project?(project)).to be true end + it 'maps an owner as a maintainer' do + exported_members.first['access_level'] = ProjectMember::OWNER + + expect(members_mapper.map[exported_user_id]).to eq(user2.id) + expect(ProjectMember.find_by_user_id(user2.id).access_level).to eq(ProjectMember::MAINTAINER) + end + + it 'removes old user_id from member_hash to avoid conflict with user key' do + expect(ProjectMember).to receive(:create) + .twice + .with(hash_excluding('user_id')) + .and_call_original + + members_mapper.map + end + context 'user is not an admin' do let(:user) { create(:user) } diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 4a7accc4c52..fb7bddb386c 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -158,6 +158,8 @@ { "id": 351, "note": "Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi.", + "note_html": "<p>something else entirely</p>", + "cached_markdown_version": 917504, "noteable_type": "Issue", "author_id": 26, "created_at": "2016-06-14T15:02:47.770Z", @@ -2363,6 +2365,8 @@ { "id": 671, "note": "Sit voluptatibus eveniet architecto quidem.", + "note_html": "<p>something else entirely</p>", + "cached_markdown_version": 917504, "noteable_type": "MergeRequest", "author_id": 26, "created_at": "2016-06-14T15:02:56.632Z", diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 6084dc96410..ca46006ea58 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -58,6 +58,26 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(Milestone.find_by_description('test milestone').issues.count).to eq(2) end + context 'when importing a project with cached_markdown_version and note_html' do + context 'for an Issue' do + it 'does not import note_html' do + note_content = 'Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi' + issue_note = Issue.find_by(description: 'Aliquam enim illo et possimus.').notes.select { |n| n.note.match(/#{note_content}/)}.first + + expect(issue_note.note_html).to match(/#{note_content}/) + end + end + + context 'for a Merge Request' do + it 'does not import note_html' do + note_content = 'Sit voluptatibus eveniet architecto quidem' + merge_request_note = MergeRequest.find_by(title: 'MR1').notes.select { |n| n.note.match(/#{note_content}/)}.first + + expect(merge_request_note.note_html).to match(/#{note_content}/) + end + end + end + it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end @@ -328,6 +348,19 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end context 'when the project has overridden params in import data' do + it 'handles string versions of visibility_level' do + # Project needs to be in a group for visibility level comparison + # to happen + group = create(:group) + project.group = group + + project.create_import_data(data: { override_params: { visibility_level: Gitlab::VisibilityLevel::INTERNAL.to_s } }) + + restored_project_json + + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + it 'overwrites the params stored in the JSON' do project.create_import_data(data: { override_params: { description: "Overridden" } }) diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 8a699eb1461..e2ffb2adb9b 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -34,11 +34,5 @@ describe Gitlab::ImportExport::RepoRestorer do it 'restores the repo successfully' do expect(restorer.restore).to be_truthy end - - it 'has the webhooks' do - restorer.restore - - expect(project_hook_exists?(project)).to be true - end end end diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index 94abf9679c4..8060b5d4448 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -14,7 +14,8 @@ describe Gitlab::ImportSources do 'Repo by URL' => 'git', 'GitLab export' => 'gitlab_project', 'Gitea' => 'gitea', - 'Manifest file' => 'manifest' + 'Manifest file' => 'manifest', + 'Phabricator' => 'phabricator' } expect(described_class.options).to eq(expected) @@ -35,6 +36,7 @@ describe Gitlab::ImportSources do gitlab_project gitea manifest + phabricator ) expect(described_class.values).to eq(expected) @@ -53,6 +55,7 @@ describe Gitlab::ImportSources do fogbugz gitlab_project gitea + phabricator ) expect(described_class.importer_names).to eq(expected) @@ -70,7 +73,8 @@ describe Gitlab::ImportSources do 'git' => nil, 'gitlab_project' => Gitlab::ImportExport::Importer, 'gitea' => Gitlab::LegacyGithubImport::Importer, - 'manifest' => nil + 'manifest' => nil, + 'phabricator' => Gitlab::PhabricatorImport::Importer } import_sources.each do |name, klass| @@ -91,7 +95,8 @@ describe Gitlab::ImportSources do 'git' => 'Repo by URL', 'gitlab_project' => 'GitLab export', 'gitea' => 'Gitea', - 'manifest' => 'Manifest file' + 'manifest' => 'Manifest file', + 'phabricator' => 'Phabricator' } import_sources.each do |name, title| @@ -102,7 +107,7 @@ describe Gitlab::ImportSources do end describe 'imports_repository? checker' do - let(:allowed_importers) { %w[github gitlab_project bitbucket_server] } + let(:allowed_importers) { %w[github gitlab_project bitbucket_server phabricator] } it 'fails if any importer other than the allowed ones implements this method' do current_importers = described_class.values.select { |kind| described_class.importer(kind).try(:imports_repository?) } diff --git a/spec/lib/gitlab/lets_encrypt/client_spec.rb b/spec/lib/gitlab/lets_encrypt/client_spec.rb index 16a16acfd25..5454d9c1af4 100644 --- a/spec/lib/gitlab/lets_encrypt/client_spec.rb +++ b/spec/lib/gitlab/lets_encrypt/client_spec.rb @@ -26,6 +26,36 @@ describe ::Gitlab::LetsEncrypt::Client do ) end + it 'generates and stores private key and initialize acme client with it' do + expect(Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(nil) + + subject + + saved_private_key = Gitlab::CurrentSettings.lets_encrypt_private_key + + expect(saved_private_key).to be + expect(Acme::Client).to have_received(:new).with( + hash_including(private_key: eq_pem(saved_private_key)) + ) + end + + context 'when private key is saved in settings' do + let!(:saved_private_key) do + key = OpenSSL::PKey::RSA.new(4096).to_pem + Gitlab::CurrentSettings.current_application_settings.update(lets_encrypt_private_key: key) + key + end + + it 'uses current value of private key' do + subject + + expect(Acme::Client).to have_received(:new).with( + hash_including(private_key: eq_pem(saved_private_key)) + ) + expect(Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(saved_private_key) + end + end + context 'when acme integration is disabled' do before do stub_application_setting(lets_encrypt_terms_of_service_accepted: false) @@ -92,6 +122,18 @@ describe ::Gitlab::LetsEncrypt::Client do context 'when terms of service are accepted' do it { is_expected.to eq(true) } + context "when private_key isn't present and database is read only" do + before do + allow(::Gitlab::Database).to receive(:read_only?).and_return(true) + end + + it 'returns false' do + expect(::Gitlab::CurrentSettings.lets_encrypt_private_key).to eq(nil) + + is_expected.to eq(false) + end + end + context 'when feature flag is disabled' do before do stub_feature_flags(pages_auto_ssl: false) diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb index 8961ecc4be0..701ed1f3a1b 100644 --- a/spec/lib/gitlab/lfs_token_spec.rb +++ b/spec/lib/gitlab/lfs_token_spec.rb @@ -77,96 +77,42 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do let(:actor) { create(:user, username: 'test_user_lfs_1') } let(:lfs_token) { described_class.new(actor) } - context 'for an HMAC token' do - before do - # We're not interested in testing LegacyRedisDeviseToken here - allow(Gitlab::LfsToken::LegacyRedisDeviseToken).to receive_message_chain(:new, :token_valid?).and_return(false) - end - - context 'where the token is invalid' do - context "because it's junk" do - it 'returns false' do - expect(lfs_token.token_valid?('junk')).to be_falsey - end - end - - context "because it's been fiddled with" do - it 'returns false' do - fiddled_token = lfs_token.token.tap { |token| token[0] = 'E' } - expect(lfs_token.token_valid?(fiddled_token)).to be_falsey - end - end - - context "because it was generated with a different secret" do - it 'returns false' do - different_actor = create(:user, username: 'test_user_lfs_2') - different_secret_token = described_class.new(different_actor).token - expect(lfs_token.token_valid?(different_secret_token)).to be_falsey - end - end - - context "because it's expired" do - it 'returns false' do - expired_token = lfs_token.token - # Needs to be at least 1860 seconds, because the default expiry is - # 1800 seconds with an additional 60 second leeway. - Timecop.freeze(Time.now + 1865) do - expect(lfs_token.token_valid?(expired_token)).to be_falsey - end - end + context 'where the token is invalid' do + context "because it's junk" do + it 'returns false' do + expect(lfs_token.token_valid?('junk')).to be_falsey end end - context 'where the token is valid' do - it 'returns true' do - expect(lfs_token.token_valid?(lfs_token.token)).to be_truthy + context "because it's been fiddled with" do + it 'returns false' do + fiddled_token = lfs_token.token.tap { |token| token[0] = 'E' } + expect(lfs_token.token_valid?(fiddled_token)).to be_falsey end end - end - - context 'for a LegacyRedisDevise token' do - before do - # We're not interested in testing HMACToken here - allow_any_instance_of(Gitlab::LfsToken::HMACToken).to receive(:token_valid?).and_return(false) - end - - context 'where the token is invalid' do - context "because it's junk" do - it 'returns false' do - expect(lfs_token.token_valid?('junk')).to be_falsey - end - end - context "because it's been fiddled with" do - it 'returns false' do - generated_token = Gitlab::LfsToken::LegacyRedisDeviseToken.new(actor).store_new_token - fiddled_token = generated_token.tap { |token| token[0] = 'E' } - expect(lfs_token.token_valid?(fiddled_token)).to be_falsey - end - end - - context "because it was generated with a different secret" do - it 'returns false' do - different_actor = create(:user, username: 'test_user_lfs_2') - different_secret_token = described_class.new(different_actor).token - expect(lfs_token.token_valid?(different_secret_token)).to be_falsey - end + context "because it was generated with a different secret" do + it 'returns false' do + different_actor = create(:user, username: 'test_user_lfs_2') + different_secret_token = described_class.new(different_actor).token + expect(lfs_token.token_valid?(different_secret_token)).to be_falsey end + end - context "because it's expired" do - it 'returns false' do - generated_token = Gitlab::LfsToken::LegacyRedisDeviseToken.new(actor).store_new_token(1) - # We need a real sleep here because we need to wait for redis to expire the key. - sleep(0.01) - expect(lfs_token.token_valid?(generated_token)).to be_falsey + context "because it's expired" do + it 'returns false' do + expired_token = lfs_token.token + # Needs to be at least 1860 seconds, because the default expiry is + # 1800 seconds with an additional 60 second leeway. + Timecop.freeze(Time.now + 1865) do + expect(lfs_token.token_valid?(expired_token)).to be_falsey end end end context 'where the token is valid' do it 'returns true' do - generated_token = Gitlab::LfsToken::LegacyRedisDeviseToken.new(actor).store_new_token - expect(lfs_token.token_valid?(generated_token)).to be_truthy + expect(lfs_token.token_valid?(lfs_token.token)).to be_truthy end end end diff --git a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb new file mode 100644 index 00000000000..f4a6e1fc7d9 --- /dev/null +++ b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::Samplers::PumaSampler do + subject { described_class.new(5) } + let(:null_metric) { double('null_metric', set: nil, observe: nil) } + + before do + allow(Gitlab::Metrics::NullMetric).to receive(:instance).and_return(null_metric) + end + + describe '#sample' do + before do + expect(subject).to receive(:puma_stats).and_return(puma_stats) + end + + context 'in cluster mode' do + let(:puma_stats) do + <<~EOS + { + "workers": 2, + "phase": 2, + "booted_workers": 2, + "old_workers": 0, + "worker_status": [{ + "pid": 32534, + "index": 0, + "phase": 1, + "booted": true, + "last_checkin": "2019-05-15T07:57:55Z", + "last_status": { + "backlog":0, + "running":1, + "pool_capacity":4, + "max_threads": 4 + } + }] + } + EOS + end + + it 'samples master statistics' do + labels = { worker: 'master' } + + expect(subject.metrics[:puma_workers]).to receive(:set).with(labels, 2) + expect(subject.metrics[:puma_running_workers]).to receive(:set).with(labels, 2) + expect(subject.metrics[:puma_stale_workers]).to receive(:set).with(labels, 0) + expect(subject.metrics[:puma_phase]).to receive(:set).once.with(labels, 2) + expect(subject.metrics[:puma_phase]).to receive(:set).once.with({ worker: 'worker_0' }, 1) + + subject.sample + end + + it 'samples worker statistics' do + labels = { worker: 'worker_0' } + + expect_worker_stats(labels) + + subject.sample + end + end + + context 'with empty worker stats' do + let(:puma_stats) do + <<~EOS + { + "workers": 2, + "phase": 2, + "booted_workers": 2, + "old_workers": 0, + "worker_status": [{ + "pid": 32534, + "index": 0, + "phase": 1, + "booted": true, + "last_checkin": "2019-05-15T07:57:55Z", + "last_status": {} + }] + } + EOS + end + + it 'does not log worker stats' do + expect(subject).not_to receive(:set_worker_metrics) + + subject.sample + end + end + + context 'in single mode' do + let(:puma_stats) do + <<~EOS + { + "backlog":0, + "running":1, + "pool_capacity":4, + "max_threads": 4 + } + EOS + end + + it 'samples worker statistics' do + labels = {} + + expect(subject.metrics[:puma_workers]).to receive(:set).with(labels, 1) + expect(subject.metrics[:puma_running_workers]).to receive(:set).with(labels, 1) + expect_worker_stats(labels) + + subject.sample + end + end + end + + def expect_worker_stats(labels) + expect(subject.metrics[:puma_queued_connections]).to receive(:set).with(labels, 0) + expect(subject.metrics[:puma_active_connections]).to receive(:set).with(labels, 0) + expect(subject.metrics[:puma_running]).to receive(:set).with(labels, 1) + expect(subject.metrics[:puma_pool_capacity]).to receive(:set).with(labels, 4) + expect(subject.metrics[:puma_max_threads]).to receive(:set).with(labels, 4) + expect(subject.metrics[:puma_idle_threads]).to receive(:set).with(labels, 1) + end +end diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 7972ff253fe..aaf8c9fa2a0 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -10,17 +10,20 @@ describe Gitlab::Metrics::Samplers::RubySampler do describe '#sample' do it 'samples various statistics' do - expect(Gitlab::Metrics::System).to receive(:memory_usage) + expect(Gitlab::Metrics::System).to receive(:cpu_time) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) + expect(Gitlab::Metrics::System).to receive(:memory_usage) + expect(Gitlab::Metrics::System).to receive(:process_start_time) + expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors) expect(sampler).to receive(:sample_gc) sampler.sample end - it 'adds a metric containing the memory usage' do + it 'adds a metric containing the process resident memory bytes' do expect(Gitlab::Metrics::System).to receive(:memory_usage).and_return(9000) - expect(sampler.metrics[:memory_usage]).to receive(:set).with({}, 9000) + expect(sampler.metrics[:process_resident_memory_bytes]).to receive(:set).with({}, 9000) sampler.sample end @@ -34,6 +37,27 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end + it 'adds a metric containing the process total cpu time' do + expect(Gitlab::Metrics::System).to receive(:cpu_time).and_return(0.51) + expect(sampler.metrics[:process_cpu_seconds_total]).to receive(:set).with({}, 0.51) + + sampler.sample + end + + it 'adds a metric containing the process start time' do + expect(Gitlab::Metrics::System).to receive(:process_start_time).and_return(12345) + expect(sampler.metrics[:process_start_time_seconds]).to receive(:set).with({}, 12345) + + sampler.sample + end + + it 'adds a metric containing the process max file descriptors' do + expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors).and_return(1024) + expect(sampler.metrics[:process_max_fds]).to receive(:set).with({}, 1024) + + sampler.sample + end + it 'clears any GC profiles' do expect(GC::Profiler).to receive(:clear) diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index 4b03f3c2532..090e456644f 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -39,8 +39,8 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do it 'updates metrics type unix and with addr' do labels = { socket_type: 'unix', socket_address: socket_address } - expect(subject).to receive_message_chain(:unicorn_active_connections, :set).with(labels, 'active') - expect(subject).to receive_message_chain(:unicorn_queued_connections, :set).with(labels, 'queued') + expect(subject.metrics[:unicorn_active_connections]).to receive(:set).with(labels, 'active') + expect(subject.metrics[:unicorn_queued_connections]).to receive(:set).with(labels, 'queued') subject.sample end @@ -50,7 +50,6 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do context 'unicorn listens on tcp sockets' do let(:tcp_socket_address) { '0.0.0.0:8080' } let(:tcp_sockets) { [tcp_socket_address] } - before do allow(unicorn).to receive(:listener_names).and_return(tcp_sockets) end @@ -71,13 +70,29 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do it 'updates metrics type unix and with addr' do labels = { socket_type: 'tcp', socket_address: tcp_socket_address } - expect(subject).to receive_message_chain(:unicorn_active_connections, :set).with(labels, 'active') - expect(subject).to receive_message_chain(:unicorn_queued_connections, :set).with(labels, 'queued') + expect(subject.metrics[:unicorn_active_connections]).to receive(:set).with(labels, 'active') + expect(subject.metrics[:unicorn_queued_connections]).to receive(:set).with(labels, 'queued') subject.sample end end end + + context 'additional metrics' do + let(:unicorn_workers) { 2 } + + before do + allow(unicorn).to receive(:listener_names).and_return([""]) + allow(::Gitlab::Metrics::System).to receive(:cpu_time).and_return(3.14) + allow(subject).to receive(:unicorn_workers_count).and_return(unicorn_workers) + end + + it "sets additional metrics" do + expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, unicorn_workers) + + subject.sample + end + end end describe '#start' do diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb index 14afcdf5daa..b0603d96eb2 100644 --- a/spec/lib/gitlab/metrics/system_spec.rb +++ b/spec/lib/gitlab/metrics/system_spec.rb @@ -13,6 +13,18 @@ describe Gitlab::Metrics::System do expect(described_class.file_descriptor_count).to be > 0 end end + + describe '.max_open_file_descriptors' do + it 'returns the max allowed open file descriptors' do + expect(described_class.max_open_file_descriptors).to be > 0 + end + end + + describe '.process_start_time' do + it 'returns the process start time' do + expect(described_class.process_start_time).to be > 0 + end + end else describe '.memory_usage' do it 'returns 0.0' do @@ -25,6 +37,18 @@ describe Gitlab::Metrics::System do expect(described_class.file_descriptor_count).to eq(0) end end + + describe '.max_open_file_descriptors' do + it 'returns 0' do + expect(described_class.max_open_file_descriptors).to eq(0) + end + end + + describe 'process_start_time' do + it 'returns 0' do + expect(described_class.process_start_time).to eq(0) + end + end end describe '.cpu_time' do diff --git a/spec/lib/gitlab/omniauth_initializer_spec.rb b/spec/lib/gitlab/omniauth_initializer_spec.rb index d808b4d49e0..f9c0daf1ef1 100644 --- a/spec/lib/gitlab/omniauth_initializer_spec.rb +++ b/spec/lib/gitlab/omniauth_initializer_spec.rb @@ -38,6 +38,28 @@ describe Gitlab::OmniauthInitializer do subject.execute([hash_config]) end + it 'normalizes a String strategy_class' do + hash_config = { 'name' => 'hash', 'args' => { strategy_class: 'OmniAuth::Strategies::OAuth2Generic' } } + + expect(devise_config).to receive(:omniauth).with(:hash, strategy_class: OmniAuth::Strategies::OAuth2Generic) + + subject.execute([hash_config]) + end + + it 'allows a class to be specified in strategy_class' do + hash_config = { 'name' => 'hash', 'args' => { strategy_class: OmniAuth::Strategies::OAuth2Generic } } + + expect(devise_config).to receive(:omniauth).with(:hash, strategy_class: OmniAuth::Strategies::OAuth2Generic) + + subject.execute([hash_config]) + end + + it 'throws an error for an invalid strategy_class' do + hash_config = { 'name' => 'hash', 'args' => { strategy_class: 'OmniAuth::Strategies::Bogus' } } + + expect { subject.execute([hash_config]) }.to raise_error(NameError) + end + it 'configures fail_with_empty_uid for shibboleth' do shibboleth_config = { 'name' => 'shibboleth', 'args' => {} } diff --git a/spec/lib/gitlab/phabricator_import/base_worker_spec.rb b/spec/lib/gitlab/phabricator_import/base_worker_spec.rb new file mode 100644 index 00000000000..d46d908a3e3 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/base_worker_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::BaseWorker do + let(:subclass) do + # Creating an anonymous class for a worker is complicated, as we generate the + # queue name from the class name. + Gitlab::PhabricatorImport::ImportTasksWorker + end + + describe '.schedule' do + let(:arguments) { %w[project_id the_next_page] } + + it 'schedules the job' do + expect(subclass).to receive(:perform_async).with(*arguments) + + subclass.schedule(*arguments) + end + + it 'counts the scheduled job', :clean_gitlab_redis_shared_state do + state = Gitlab::PhabricatorImport::WorkerState.new('project_id') + + allow(subclass).to receive(:remove_job) # otherwise the job is removed before we saw it + + expect { subclass.schedule(*arguments) }.to change { state.running_count }.by(1) + end + end + + describe '#perform' do + let(:project) { create(:project, :import_started, import_url: "https://a.phab.instance") } + let(:worker) { subclass.new } + let(:state) { Gitlab::PhabricatorImport::WorkerState.new(project.id) } + + before do + allow(worker).to receive(:import) + end + + it 'does not break for a non-existing project' do + expect { worker.perform('not a thing') }.not_to raise_error + end + + it 'does not do anything when the import is not in progress' do + project = create(:project, :import_failed) + + expect(worker).not_to receive(:import) + + worker.perform(project.id) + end + + it 'calls import for the project' do + expect(worker).to receive(:import).with(project, 'other_arg') + + worker.perform(project.id, 'other_arg') + end + + it 'marks the project as imported if there was only one job running' do + worker.perform(project.id) + + expect(project.import_state.reload).to be_finished + end + + it 'does not mark the job as finished when there are more scheduled jobs' do + 2.times { state.add_job } + + worker.perform(project.id) + + expect(project.import_state.reload).to be_in_progress + end + + it 'decrements the job counter' do + expect { worker.perform(project.id) }.to change { state.running_count }.by(-1) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/cache/map_spec.rb b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb new file mode 100644 index 00000000000..52c7a02219f --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Cache::Map, :clean_gitlab_redis_cache do + set(:project) { create(:project) } + let(:redis) { Gitlab::Redis::Cache } + subject(:map) { described_class.new(project) } + + describe '#get_gitlab_model' do + it 'returns nil if there was nothing cached for the phabricator id' do + expect(map.get_gitlab_model('does not exist')).to be_nil + end + + it 'returns the object if it was set in redis' do + issue = create(:issue, project: project) + set_in_redis('exists', issue) + + expect(map.get_gitlab_model('exists')).to eq(issue) + end + + it 'extends the TTL for the cache key' do + set_in_redis('extend', create(:issue, project: project)) do |redis| + redis.expire(cache_key('extend'), 10.seconds.to_i) + end + + map.get_gitlab_model('extend') + + ttl = redis.with { |redis| redis.ttl(cache_key('extend')) } + + expect(ttl).to be > 10.seconds + end + end + + describe '#set_gitlab_model' do + around do |example| + Timecop.freeze { example.run } + end + + it 'sets the class and id in redis with a ttl' do + issue = create(:issue, project: project) + + map.set_gitlab_model(issue, 'it is set') + + set_data, ttl = redis.with do |redis| + redis.pipelined do |p| + p.mapped_hmget(cache_key('it is set'), :classname, :database_id) + p.ttl(cache_key('it is set')) + end + end + + expect(set_data).to eq({ classname: 'Issue', database_id: issue.id.to_s }) + expect(ttl).to be_within(1.second).of(StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION) + end + end + + def set_in_redis(key, object) + redis.with do |redis| + redis.mapped_hmset(cache_key(key), + { classname: object.class, database_id: object.id }) + yield(redis) if block_given? + end + end + + def cache_key(phabricator_id) + subject.__send__(:cache_key_for_phabricator_id, phabricator_id) + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb new file mode 100644 index 00000000000..542b3cd060f --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::Client do + let(:client) do + described_class.new('https://see-ya-later.phabricator', 'api-token') + end + + describe '#get' do + it 'performs and parses a request' do + params = { some: 'extra', values: %w[are passed] } + stub_valid_request(params) + + response = client.get('test', params: params) + + expect(response).to be_a(Gitlab::PhabricatorImport::Conduit::Response) + expect(response).to be_success + end + + it 'wraps request errors in an `ApiError`' do + stub_timeout + + expect { client.get('test') }.to raise_error(Gitlab::PhabricatorImport::Conduit::ApiError) + end + + it 'raises response error' do + stub_error_response + + expect { client.get('test') } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /has the wrong length/) + end + end + + def stub_valid_request(params = {}) + WebMock.stub_request( + :get, 'https://see-ya-later.phabricator/api/test' + ).with( + body: CGI.unescape(params.reverse_merge('api.token' => 'api-token').to_query) + ).and_return( + status: 200, + body: fixture_file('phabricator_responses/maniphest.search.json') + ) + end + + def stub_timeout + WebMock.stub_request( + :get, 'https://see-ya-later.phabricator/api/test' + ).to_timeout + end + + def stub_error_response + WebMock.stub_request( + :get, 'https://see-ya-later.phabricator/api/test' + ).and_return( + status: 200, + body: fixture_file('phabricator_responses/auth_failed.json') + ) + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb new file mode 100644 index 00000000000..0d7714649b9 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::Maniphest do + let(:maniphest) do + described_class.new(phabricator_url: 'https://see-ya-later.phabricator', api_token: 'api-token') + end + + describe '#tasks' do + let(:fake_client) { double('Phabricator client') } + + before do + allow(maniphest).to receive(:client).and_return(fake_client) + end + + it 'calls the api with the correct params' do + expected_params = { + after: '123', + attachments: { + projects: 1, subscribers: 1, columns: 1 + } + } + + expect(fake_client).to receive(:get).with('maniphest.search', + params: expected_params) + + maniphest.tasks(after: '123') + end + + it 'returns a parsed response' do + response = Gitlab::PhabricatorImport::Conduit::Response + .new(fixture_file('phabricator_responses/maniphest.search.json')) + + allow(fake_client).to receive(:get).and_return(response) + + expect(maniphest.tasks).to be_a(Gitlab::PhabricatorImport::Conduit::TasksResponse) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb new file mode 100644 index 00000000000..a8596968f14 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::Response do + let(:response) { described_class.new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json')))} + let(:error_response) { described_class.new(JSON.parse(fixture_file('phabricator_responses/auth_failed.json'))) } + + describe '.parse!' do + it 'raises a ResponseError if the http response was not successfull' do + fake_response = double(:http_response, success?: false, status: 401) + + expect { described_class.parse!(fake_response) } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /responded with 401/) + end + + it 'raises a ResponseError if the response contained a Phabricator error' do + fake_response = double(:http_response, + success?: true, + status: 200, + body: fixture_file('phabricator_responses/auth_failed.json')) + + expect { described_class.parse!(fake_response) } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /ERR-INVALID-AUTH: API token/) + end + + it 'raises a ResponseError if JSON parsing failed' do + fake_response = double(:http_response, + success?: true, + status: 200, + body: 'This is no JSON') + + expect { described_class.parse!(fake_response) } + .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected token at/) + end + + it 'returns a parsed response for valid input' do + fake_response = double(:http_response, + success?: true, + status: 200, + body: fixture_file('phabricator_responses/maniphest.search.json')) + + expect(described_class.parse!(fake_response)).to be_a(described_class) + end + end + + describe '#success?' do + it { expect(response).to be_success } + it { expect(error_response).not_to be_success } + end + + describe '#error_code' do + it { expect(error_response.error_code).to eq('ERR-INVALID-AUTH') } + it { expect(response.error_code).to be_nil } + end + + describe '#error_info' do + it 'returns the correct error info' do + expected_message = 'API token "api-token" has the wrong length. API tokens should be 32 characters long.' + + expect(error_response.error_info).to eq(expected_message) + end + + it { expect(response.error_info).to be_nil } + end + + describe '#data' do + it { expect(error_response.data).to be_nil } + it { expect(response.data).to be_an(Array) } + end + + describe '#pagination' do + it { expect(error_response.pagination).to be_nil } + + it 'builds the pagination correctly' do + expect(response.pagination).to be_a(Gitlab::PhabricatorImport::Conduit::Pagination) + expect(response.pagination.next_page).to eq('284') + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb new file mode 100644 index 00000000000..4b4c2a6276e --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Conduit::TasksResponse do + let(:conduit_response) do + Gitlab::PhabricatorImport::Conduit::Response + .new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json'))) + end + + subject(:response) { described_class.new(conduit_response) } + + describe '#pagination' do + it 'delegates to the conduit reponse' do + expect(response.pagination).to eq(conduit_response.pagination) + end + end + + describe '#tasks' do + it 'builds the correct tasks representation' do + tasks = response.tasks + + titles = tasks.map(&:issue_attributes).map { |attrs| attrs[:title] } + + expect(titles).to contain_exactly('Things are slow', 'Things are broken') + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb b/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb new file mode 100644 index 00000000000..1e38ef8aaa5 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::ImportTasksWorker do + describe '#perform' do + it 'calls the correct importer' do + project = create(:project, :import_started, import_url: "https://the.phab.ulr") + fake_importer = instance_double(Gitlab::PhabricatorImport::Issues::Importer) + + expect(Gitlab::PhabricatorImport::Issues::Importer).to receive(:new).with(project).and_return(fake_importer) + expect(fake_importer).to receive(:execute) + + described_class.new.perform(project.id) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/importer_spec.rb b/spec/lib/gitlab/phabricator_import/importer_spec.rb new file mode 100644 index 00000000000..bf14010a187 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/importer_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Importer do + it { expect(described_class).to be_async } + + it "acts like it's importing repositories" do + expect(described_class).to be_imports_repository + end + + describe '#execute' do + let(:project) { create(:project, :import_scheduled) } + subject(:importer) { described_class.new(project) } + + it 'sets a custom jid that will be kept up to date' do + expect { importer.execute }.to change { project.import_state.reload.jid } + end + + it 'starts importing tasks' do + expect(Gitlab::PhabricatorImport::ImportTasksWorker).to receive(:schedule).with(project.id) + + importer.execute + end + + it 'marks the import as failed when something goes wrong' do + allow(importer).to receive(:schedule_first_tasks_page).and_raise('Stuff is broken') + + importer.execute + + expect(project.import_state).to be_failed + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb new file mode 100644 index 00000000000..2412cf76f79 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Issues::Importer do + set(:project) { create(:project) } + + let(:response) do + Gitlab::PhabricatorImport::Conduit::TasksResponse.new( + Gitlab::PhabricatorImport::Conduit::Response + .new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json'))) + ) + end + + subject(:importer) { described_class.new(project, nil) } + + before do + client = instance_double(Gitlab::PhabricatorImport::Conduit::Maniphest) + + allow(client).to receive(:tasks).and_return(response) + allow(importer).to receive(:client).and_return(client) + end + + describe '#execute' do + it 'imports each task in the response' do + response.tasks.each do |task| + task_importer = instance_double(Gitlab::PhabricatorImport::Issues::TaskImporter) + + expect(task_importer).to receive(:execute) + expect(Gitlab::PhabricatorImport::Issues::TaskImporter) + .to receive(:new).with(project, task) + .and_return(task_importer) + end + + importer.execute + end + + it 'schedules the next batch if there is one' do + expect(Gitlab::PhabricatorImport::ImportTasksWorker) + .to receive(:schedule).with(project.id, response.pagination.next_page) + + importer.execute + end + + it 'does not reschedule when there is no next page' do + allow(response.pagination).to receive(:has_next_page?).and_return(false) + + expect(Gitlab::PhabricatorImport::ImportTasksWorker) + .not_to receive(:schedule) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb new file mode 100644 index 00000000000..1625604e754 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Issues::TaskImporter do + set(:project) { create(:project) } + let(:task) do + Gitlab::PhabricatorImport::Representation::Task.new( + { + 'phid' => 'the-phid', + 'fields' => { + 'name' => 'Title', + 'description' => { + 'raw' => '# This is markdown\n it can contain more text.' + }, + 'dateCreated' => '1518688921', + 'dateClosed' => '1518789995' + } + } + ) + end + + describe '#execute' do + it 'creates the issue with the expected attributes' do + issue = described_class.new(project, task).execute + + expect(issue.project).to eq(project) + expect(issue).to be_persisted + expect(issue.author).to eq(User.ghost) + expect(issue.title).to eq('Title') + expect(issue.description).to eq('# This is markdown\n it can contain more text.') + expect(issue).to be_closed + expect(issue.created_at).to eq(Time.at(1518688921)) + expect(issue.closed_at).to eq(Time.at(1518789995)) + end + + it 'does not recreate the issue when called multiple times' do + expect { described_class.new(project, task).execute } + .to change { project.issues.reload.size }.from(0).to(1) + expect { described_class.new(project, task).execute } + .not_to change { project.issues.reload.size } + end + + it 'does not trigger a save when the object did not change' do + existing_issue = create(:issue, + task.issue_attributes.merge(author: User.ghost)) + importer = described_class.new(project, task) + allow(importer).to receive(:issue).and_return(existing_issue) + + expect(existing_issue).not_to receive(:save!) + + importer.execute + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/project_creator_spec.rb b/spec/lib/gitlab/phabricator_import/project_creator_spec.rb new file mode 100644 index 00000000000..e9455b866ac --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/project_creator_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::PhabricatorImport::ProjectCreator do + let(:user) { create(:user) } + let(:params) do + { path: 'new-phab-import', + phabricator_server_url: 'http://phab.example.com', + api_token: 'the-token' } + end + subject(:creator) { described_class.new(user, params) } + + describe '#execute' do + it 'creates a project correctly and schedule an import' do + expect_next_instance_of(Gitlab::PhabricatorImport::Importer) do |importer| + expect(importer).to receive(:execute) + end + + project = creator.execute + + expect(project).to be_persisted + expect(project).to be_import + expect(project.import_type).to eq('phabricator') + expect(project.import_data.credentials).to match(a_hash_including(api_token: 'the-token')) + expect(project.import_data.data).to match(a_hash_including('phabricator_url' => 'http://phab.example.com')) + expect(project.import_url).to eq(Project::UNKNOWN_IMPORT_URL) + expect(project.namespace).to eq(user.namespace) + end + + context 'when import params are missing' do + let(:params) do + { path: 'new-phab-import', + phabricator_server_url: 'http://phab.example.com', + api_token: '' } + end + + it 'returns nil' do + expect(creator.execute).to be_nil + end + end + + context 'when import params are invalid' do + let(:params) do + { path: 'new-phab-import', + namespace_id: '-1', + phabricator_server_url: 'http://phab.example.com', + api_token: 'the-token' } + end + + it 'returns an unpersisted project' do + project = creator.execute + + expect(project).not_to be_persisted + expect(project).not_to be_valid + end + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/representation/task_spec.rb b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb new file mode 100644 index 00000000000..dfbd8c546eb --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::Representation::Task do + subject(:task) do + described_class.new( + { + 'phid' => 'the-phid', + 'fields' => { + 'name' => 'Title'.ljust(257, '.'), # A string padded to 257 chars + 'description' => { + 'raw' => '# This is markdown\n it can contain more text.' + }, + 'dateCreated' => '1518688921', + 'dateClosed' => '1518789995' + } + } + ) + end + + describe '#issue_attributes' do + it 'contains the expected values' do + expected_attributes = { + title: 'Title'.ljust(255, '.'), + description: '# This is markdown\n it can contain more text.', + state: :closed, + created_at: Time.at(1518688921), + closed_at: Time.at(1518789995) + } + + expect(task.issue_attributes).to eq(expected_attributes) + end + end +end diff --git a/spec/lib/gitlab/phabricator_import/worker_state_spec.rb b/spec/lib/gitlab/phabricator_import/worker_state_spec.rb new file mode 100644 index 00000000000..a44947445c9 --- /dev/null +++ b/spec/lib/gitlab/phabricator_import/worker_state_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Gitlab::PhabricatorImport::WorkerState, :clean_gitlab_redis_shared_state do + subject(:state) { described_class.new('weird-project-id') } + let(:key) { 'phabricator-import/jobs/project-weird-project-id/job-count' } + + describe '#add_job' do + it 'increments the counter for jobs' do + set_value(3) + + expect { state.add_job }.to change { get_value }.from('3').to('4') + end + end + + describe '#remove_job' do + it 'decrements the counter for jobs' do + set_value(3) + + expect { state.remove_job }.to change { get_value }.from('3').to('2') + end + end + + describe '#running_count' do + it 'reads the value' do + set_value(9) + + expect(state.running_count).to eq(9) + end + + it 'returns 0 when nothing was set' do + expect(state.running_count).to eq(0) + end + end + + def set_value(value) + redis.with { |r| r.set(key, value) } + end + + def get_value + redis.with { |r| r.get(key) } + end + + def redis + Gitlab::Redis::SharedState + end +end diff --git a/spec/lib/gitlab/prometheus/query_variables_spec.rb b/spec/lib/gitlab/prometheus/query_variables_spec.rb index 048f4af6020..6dc99ef26ec 100644 --- a/spec/lib/gitlab/prometheus/query_variables_spec.rb +++ b/spec/lib/gitlab/prometheus/query_variables_spec.rb @@ -23,7 +23,7 @@ describe Gitlab::Prometheus::QueryVariables do context 'with deployment platform' do context 'with project cluster' do - let(:kube_namespace) { environment.deployment_platform.actual_namespace } + let(:kube_namespace) { environment.deployment_platform.cluster.kubernetes_namespace_for(project) } before do create(:cluster, :project, :provided_by_user, projects: [project]) diff --git a/spec/lib/gitlab/rack_timeout_observer_spec.rb b/spec/lib/gitlab/rack_timeout_observer_spec.rb new file mode 100644 index 00000000000..3dc1a8b68fb --- /dev/null +++ b/spec/lib/gitlab/rack_timeout_observer_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::RackTimeoutObserver do + let(:counter) { Gitlab::Metrics::NullMetric.instance } + + before do + allow(Gitlab::Metrics).to receive(:counter) + .with(any_args) + .and_return(counter) + end + + describe '#callback' do + context 'when request times out' do + let(:env) do + { + ::Rack::Timeout::ENV_INFO_KEY => double(state: :timed_out), + 'action_dispatch.request.parameters' => { + 'controller' => 'foo', + 'action' => 'bar' + } + } + end + + subject { described_class.new } + + it 'increments timeout counter' do + expect(counter) + .to receive(:increment) + .with({ controller: 'foo', action: 'bar', route: nil, state: :timed_out }) + + subject.callback.call(env) + end + end + + context 'when request expires' do + let(:endpoint) { double } + let(:env) do + { + ::Rack::Timeout::ENV_INFO_KEY => double(state: :expired), + Grape::Env::API_ENDPOINT => endpoint + } + end + + subject { described_class.new } + + it 'increments timeout counter' do + allow(endpoint).to receive_message_chain('route.pattern.origin') { 'foobar' } + expect(counter) + .to receive(:increment) + .with({ controller: nil, action: nil, route: 'foobar', state: :expired }) + + subject.callback.call(env) + end + end + end +end diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 312aa3be490..3d27156b356 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -256,4 +256,28 @@ describe Gitlab::SearchResults do expect(results.objects('merge_requests')).not_to include merge_request end + + context 'milestones' do + it 'returns correct set of milestones' do + private_project_1 = create(:project, :private) + private_project_2 = create(:project, :private) + internal_project = create(:project, :internal) + public_project_1 = create(:project, :public) + public_project_2 = create(:project, :public, :issues_disabled, :merge_requests_disabled) + private_project_1.add_developer(user) + # milestones that should not be visible + create(:milestone, project: private_project_2, title: 'Private project without access milestone') + create(:milestone, project: public_project_2, title: 'Public project with milestones disabled milestone') + # milestones that should be visible + milestone_1 = create(:milestone, project: private_project_1, title: 'Private project with access milestone', state: 'closed') + milestone_2 = create(:milestone, project: internal_project, title: 'Internal project milestone') + milestone_3 = create(:milestone, project: public_project_1, title: 'Public project with milestones enabled milestone') + # Global search scope takes user authorized projects, internal projects and public projects. + limit_projects = ProjectsFinder.new(current_user: user).execute + + milestones = described_class.new(user, limit_projects, 'milestone').objects('milestones') + + expect(milestones).to match_array([milestone_1, milestone_2, milestone_3]) + end + end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index e2f09de2808..bce2e754176 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -612,16 +612,6 @@ describe Gitlab::Shell do FileUtils.rm_rf(created_path) end - it 'creates a repository' do - expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_truthy - - expect(File.stat(created_path).mode & 0o777).to eq(0o770) - - hooks_path = File.join(created_path, 'hooks') - expect(File.lstat(hooks_path)).to be_symlink - expect(File.realpath(hooks_path)).to eq(gitlab_shell_hooks_path) - end - it 'returns false when the command fails' do FileUtils.mkdir_p(File.dirname(created_path)) # This file will block the creation of the repo's .git directory. That diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 445a56ab0d8..253366e0789 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -2,6 +2,87 @@ require 'spec_helper' describe Gitlab::UrlBlocker do + describe '#validate!' do + context 'when URI is nil' do + let(:import_url) { nil } + + it 'returns no URI and hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to be(nil) + expect(hostname).to be(nil) + end + end + + context 'when URI is internal' do + let(:import_url) { 'http://localhost' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to eq(Addressable::URI.parse('http://[::1]')) + expect(hostname).to eq('localhost') + end + end + + context 'when the URL hostname is a domain' do + let(:import_url) { 'https://example.org' } + + it 'returns URI and hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34')) + expect(hostname).to eq('example.org') + end + end + + context 'when the URL hostname is an IP address' do + let(:import_url) { 'https://93.184.216.34' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34')) + expect(hostname).to be(nil) + end + end + + context 'disabled DNS rebinding protection' do + context 'when URI is internal' do + let(:import_url) { 'http://localhost' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false) + + expect(uri).to eq(Addressable::URI.parse('http://localhost')) + expect(hostname).to be(nil) + end + end + + context 'when the URL hostname is a domain' do + let(:import_url) { 'https://example.org' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false) + + expect(uri).to eq(Addressable::URI.parse('https://example.org')) + expect(hostname).to eq(nil) + end + end + + context 'when the URL hostname is an IP address' do + let(:import_url) { 'https://93.184.216.34' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false) + + expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34')) + expect(hostname).to be(nil) + end + end + end + end + describe '#blocked_url?' do let(:ports) { Project::VALID_IMPORT_PORTS } @@ -208,7 +289,7 @@ describe Gitlab::UrlBlocker do end def stub_domain_resolv(domain, ip) - address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false) + address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false, ipv4?: false) allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address]) allow(address).to receive(:ipv6_v4mapped?).and_return(false) end diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index 9f495a5d50b..bbcb92608d8 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -32,7 +32,7 @@ describe Gitlab::UrlBuilder do url = described_class.build(milestone) - expect(url).to eq "#{Settings.gitlab['url']}/#{milestone.project.full_path}/milestones/#{milestone.iid}" + expect(url).to eq "#{Settings.gitlab['url']}/#{milestone.project.full_path}/-/milestones/#{milestone.iid}" end end diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 5861e6955a6..7242255d535 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -115,6 +115,40 @@ describe Gitlab::UrlSanitizer do end end + describe '#user' do + context 'credentials in hash' do + it 'overrides URL-provided user' do + sanitizer = described_class.new('http://a:b@example.com', credentials: { user: 'c', password: 'd' }) + + expect(sanitizer.user).to eq('c') + end + end + + context 'credentials in URL' do + where(:url, :user) do + 'http://foo:bar@example.com' | 'foo' + 'http://foo:bar:baz@example.com' | 'foo' + 'http://:bar@example.com' | nil + 'http://foo:@example.com' | 'foo' + 'http://foo@example.com' | 'foo' + 'http://:@example.com' | nil + 'http://@example.com' | nil + 'http://example.com' | nil + + # Other invalid URLs + nil | nil + '' | nil + 'no' | nil + end + + with_them do + subject { described_class.new(url).user } + + it { is_expected.to eq(user) } + end + end + end + describe '#full_url' do context 'credentials in hash' do where(:credentials, :userinfo) do |