diff options
Diffstat (limited to 'spec/lib')
48 files changed, 1042 insertions, 394 deletions
diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index e1aea82653d..08165f147bb 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -179,7 +179,7 @@ describe API::Helpers do context 'when blob name is not null' do it 'returns disposition with the blob name' do - expect(send_git_blob['Content-Disposition']).to eq 'inline; filename="foobar"' + expect(send_git_blob['Content-Disposition']).to eq %q(inline; filename="foobar"; filename*=UTF-8''foobar) end end end diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index 7a457403b51..4972c4b4bd2 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -121,6 +121,13 @@ describe Banzai::Filter::AutolinkFilter do expect(doc.to_s).to eq("See #{link}") end + it 'does not autolink bad URLs after we remove trailing punctuation' do + link = 'http://]' + doc = filter("See #{link}") + + expect(doc.to_s).to eq("See #{link}") + end + it 'does not include trailing punctuation' do ['.', ', ok?', '...', '?', '!', ': is that ok?'].each do |trailing_punctuation| doc = filter("See #{link}#{trailing_punctuation}") @@ -188,6 +195,22 @@ describe Banzai::Filter::AutolinkFilter do expect(doc.at_css('a')['class']).to eq 'custom' end + it 'escapes RTLO and other characters' do + # rendered text looks like "http://example.com/evilexe.mp3" + evil_link = "#{link}evil\u202E3pm.exe" + doc = filter("#{evil_link}") + + expect(doc.at_css('a')['href']).to eq "http://about.gitlab.com/evil%E2%80%AE3pm.exe" + end + + it 'encodes international domains' do + link = "http://one😄two.com" + expected = "http://one%F0%9F%98%84two.com" + doc = filter(link) + + expect(doc.at_css('a')['href']).to eq expected + end + described_class::IGNORE_PARENTS.each do |elem| it "ignores valid links contained inside '#{elem}' element" do exp = act = "<#{elem}>See #{link}</#{elem}>" diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index e6dae8d5382..2acbe05f082 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -62,6 +62,13 @@ describe Banzai::Filter::ExternalLinkFilter do expect(doc.to_html).to eq(expected) end + + it 'adds rel and target to improperly formatted autolinks' do + doc = filter %q(<p><a href="mailto://jblogs@example.com">mailto://jblogs@example.com</a></p>) + expected = %q(<p><a href="mailto://jblogs@example.com" rel="nofollow noreferrer noopener" target="_blank">mailto://jblogs@example.com</a></p>) + + expect(doc.to_html).to eq(expected) + end end context 'for links with a username' do @@ -112,4 +119,62 @@ describe Banzai::Filter::ExternalLinkFilter do it_behaves_like 'an external link with rel attribute' end + + context 'links with RTLO character' do + # In rendered text this looks like "http://example.com/evilexe.mp3" + let(:doc) { filter %Q(<a href="http://example.com/evil%E2%80%AE3pm.exe">http://example.com/evil\u202E3pm.exe</a>) } + + it_behaves_like 'an external link with rel attribute' + + it 'escapes RTLO in link text' do + expected = %q(http://example.com/evil%E2%80%AE3pm.exe</a>) + + expect(doc.to_html).to include(expected) + end + + it 'does not mangle the link text' do + doc = filter %Q(<a href="http://example.com">One<span>and</span>\u202Eexe.mp3</a>) + + expect(doc.to_html).to include('One<span>and</span>%E2%80%AEexe.mp3</a>') + end + end + + context 'for generated autolinks' do + context 'with an IDN character' do + let(:doc) { filter(%q(<a href="http://exa%F0%9F%98%84mple.com">http://exa😄mple.com</a>)) } + let(:doc_email) { filter(%q(<a href="http://exa%F0%9F%98%84mple.com">http://exa😄mple.com</a>), emailable_links: true) } + + it_behaves_like 'an external link with rel attribute' + + it 'does not change the link text' do + expect(doc.to_html).to include('http://exa😄mple.com</a>') + end + + it 'uses punycode for emails' do + expect(doc_email.to_html).to include('http://xn--example-6p25f.com/</a>') + end + end + end + + context 'for links that look malicious' do + context 'with an IDN character' do + let(:doc) { filter %q(<a href="http://exa%F0%9F%98%84mple.com">http://exa😄mple.com</a>) } + + it 'adds a toolip with punycode' do + expect(doc.to_html).to include('http://exa😄mple.com</a>') + expect(doc.to_html).to include('class="has-tooltip"') + expect(doc.to_html).to include('title="http://xn--example-6p25f.com/"') + end + end + + context 'with RTLO character' do + let(:doc) { filter %q(<a href="http://example.com/evil%E2%80%AE3pm.exe">Evil Test</a>) } + + it 'adds a toolip with punycode' do + expect(doc.to_html).to include('Evil Test</a>') + expect(doc.to_html).to include('class="has-tooltip"') + expect(doc.to_html).to include('title="http://example.com/evil%E2%80%AE3pm.exe"') + end + end + end end diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb index 4c4e821deab..83fcda29680 100644 --- a/spec/lib/banzai/filter/markdown_filter_spec.rb +++ b/spec/lib/banzai/filter/markdown_filter_spec.rb @@ -10,12 +10,6 @@ describe Banzai::Filter::MarkdownFilter do filter('test') end - it 'uses Redcarpet' do - expect_any_instance_of(Banzai::Filter::MarkdownEngines::Redcarpet).to receive(:render).and_return('test') - - filter('test', { markdown_engine: :redcarpet }) - end - it 'uses CommonMark' do expect_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark).to receive(:render).and_return('test') @@ -47,24 +41,6 @@ describe Banzai::Filter::MarkdownFilter do expect(result).to start_with('<pre><code lang="日">') end end - - context 'using Redcarpet' do - before do - stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet) - end - - it 'adds language to lang attribute when specified' do - result = filter("```html\nsome code\n```") - - expect(result).to start_with("\n<pre><code lang=\"html\">") - end - - it 'does not add language to lang attribute when not specified' do - result = filter("```\nsome code\n```") - - expect(result).to start_with("\n<pre><code>") - end - end end describe 'source line position' do @@ -85,18 +61,6 @@ describe Banzai::Filter::MarkdownFilter do expect(result).to eq '<p>test</p>' end end - - context 'using Redcarpet' do - before do - stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet) - end - - it 'does not support data-sourcepos' do - result = filter('test') - - expect(result).to eq '<p>test</p>' - end - end end describe 'footnotes in tables' do diff --git a/spec/lib/banzai/filter/project_reference_filter_spec.rb b/spec/lib/banzai/filter/project_reference_filter_spec.rb index c68d49f9366..69f9c1ae829 100644 --- a/spec/lib/banzai/filter/project_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/project_reference_filter_spec.rb @@ -26,6 +26,12 @@ describe Banzai::Filter::ProjectReferenceFilter do expect(reference_filter(act).to_html).to eq(CGI.escapeHTML(exp)) end + it 'fails fast for long invalid string' do + expect do + Timeout.timeout(5.seconds) { reference_filter("A" * 50000).to_html } + end.not_to raise_error + end + it 'allows references with text after the > character' do doc = reference_filter("Hey #{reference}foo") expect(doc.css('a').first.attr('href')).to eq urls.project_url(subject) diff --git a/spec/lib/banzai/filter/spaced_link_filter_spec.rb b/spec/lib/banzai/filter/spaced_link_filter_spec.rb index 1ad7f3ff567..76d7644d76c 100644 --- a/spec/lib/banzai/filter/spaced_link_filter_spec.rb +++ b/spec/lib/banzai/filter/spaced_link_filter_spec.rb @@ -26,11 +26,6 @@ describe Banzai::Filter::SpacedLinkFilter do expect(doc.at_css('p')).to be_nil end - it 'does nothing when markdown_engine is redcarpet' do - exp = act = link - expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp - end - it 'does nothing with empty text' do link = '[](page slug)' doc = filter("See #{link}") diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index 209a547c3b3..3b52f6666d0 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -11,7 +11,7 @@ describe Banzai::ObjectRenderer do ) end - let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16) } describe '#render' do context 'with cache' do diff --git a/spec/lib/banzai/pipeline/email_pipeline_spec.rb b/spec/lib/banzai/pipeline/email_pipeline_spec.rb index 6a11ca2f9d5..b99161109eb 100644 --- a/spec/lib/banzai/pipeline/email_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/email_pipeline_spec.rb @@ -10,5 +10,19 @@ describe Banzai::Pipeline::EmailPipeline do expect(described_class.filters).not_to be_empty expect(described_class.filters).not_to include(Banzai::Filter::ImageLazyLoadFilter) end + + it 'shows punycode for autolinks' do + examples = %W[ + http://one😄two.com + http://\u0261itlab.com + ] + + examples.each do |markdown| + result = described_class.call(markdown, project: nil)[:output] + link = result.css('a').first + + expect(link.content).to include('http://xn--') + end + end end end diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb index aa503b6e1d5..3d3aa64d630 100644 --- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb @@ -59,4 +59,42 @@ describe Banzai::Pipeline::FullPipeline do expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote end end + + describe 'links are detected as malicious' do + it 'has tooltips for malicious links' do + examples = %W[ + http://example.com/evil\u202E3pm.exe + [evilexe.mp3](http://example.com/evil\u202E3pm.exe) + rdar://localhost.com/\u202E3pm.exe + http://one😄two.com + [Evil-Test](http://one😄two.com) + http://\u0261itlab.com + [Evil-GitLab-link](http://\u0261itlab.com) +  + ] + + examples.each do |markdown| + result = described_class.call(markdown, project: nil)[:output] + link = result.css('a').first + + expect(link[:class]).to include('has-tooltip') + end + end + + it 'has no tooltips for safe links' do + examples = %w[ + http://example.com + [Safe-Test](http://example.com) + https://commons.wikimedia.org/wiki/File:اسكرام_2_-_تمنراست.jpg + [Wikipedia-link](https://commons.wikimedia.org/wiki/File:اسكرام_2_-_تمنراست.jpg) + ] + + examples.each do |markdown| + result = described_class.call(markdown, project: nil)[:output] + link = result.css('a').first + + expect(link[:class]).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 236808c0b69..a4a6338961e 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::Auth do it 'optional_scopes contains all non-default scopes' do stub_container_registry_config(enabled: true) - expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid] + expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid profile email] end context 'registry_scopes' do diff --git a/spec/lib/gitlab/background_migration_spec.rb b/spec/lib/gitlab/background_migration_spec.rb index 8a83b76fd94..7d3d8a949ef 100644 --- a/spec/lib/gitlab/background_migration_spec.rb +++ b/spec/lib/gitlab/background_migration_spec.rb @@ -104,6 +104,38 @@ describe Gitlab::BackgroundMigration do end end end + + context 'when retry_dead_jobs is true', :sidekiq, :redis do + let(:retry_queue) do + [double(args: ['Object', [3]], queue: described_class.queue, delete: true)] + end + let(:dead_queue) do + [double(args: ['Object', [4]], queue: described_class.queue, delete: true)] + end + + before do + allow(Sidekiq::RetrySet).to receive(:new).and_return(retry_queue) + allow(Sidekiq::DeadSet).to receive(:new).and_return(dead_queue) + end + + it 'steals from the dead and retry queue' do + Sidekiq::Testing.disable! do + expect(described_class).to receive(:perform) + .with('Object', [1]).ordered + expect(described_class).to receive(:perform) + .with('Object', [2]).ordered + expect(described_class).to receive(:perform) + .with('Object', [3]).ordered + expect(described_class).to receive(:perform) + .with('Object', [4]).ordered + + BackgroundMigrationWorker.perform_async('Object', [2]) + BackgroundMigrationWorker.perform_in(10.minutes, 'Object', [1]) + + described_class.steal('Object', retry_dead_jobs: true) + end + end + end end describe '.perform' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index afd8f5da39f..a07c5371134 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -61,7 +61,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do let(:wiki_path) { File.join(root_path, "#{hashed_path}.wiki.git") } before do - gitlab_shell.create_repository(repository_storage, hashed_path) + gitlab_shell.create_repository(repository_storage, hashed_path, 'group/project') Gitlab::GitalyClient::StorageSettings.allow_disk_access do repository = Rugged::Repository.new(repo_path) repository.config['gitlab.fullpath'] = 'to/repo' diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index 0def685f177..c432cc223b9 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -218,7 +218,7 @@ describe Gitlab::BitbucketImport::Importer do describe 'wiki import' do it 'is skipped when the wiki exists' do expect(project.wiki).to receive(:repository_exists?) { true } - expect(importer.gitlab_shell).not_to receive(:import_repository) + expect(importer.gitlab_shell).not_to receive(:import_wiki_repository) importer.execute @@ -227,11 +227,7 @@ describe Gitlab::BitbucketImport::Importer do it 'imports to the project disk_path' do expect(project.wiki).to receive(:repository_exists?) { false } - expect(importer.gitlab_shell).to receive(:import_repository).with( - project.repository_storage, - project.wiki.disk_path, - project.import_url + '/wiki' - ) + expect(importer.gitlab_shell).to receive(:import_wiki_repository) importer.execute diff --git a/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb b/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb new file mode 100644 index 00000000000..795fd069ab2 --- /dev/null +++ b/spec/lib/gitlab/bitbucket_import/wiki_formatter_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::BitbucketImport::WikiFormatter do + let(:project) do + create(:project, + namespace: create(:namespace, path: 'gitlabhq'), + import_url: 'https://xxx@bitbucket.org/gitlabhq/sample.gitlabhq.git') + end + + subject(:wiki) { described_class.new(project) } + + describe '#disk_path' do + it 'appends .wiki to disk path' do + expect(wiki.disk_path).to eq project.wiki.disk_path + end + end + + describe '#full_path' do + it 'appends .wiki to project path' do + expect(wiki.full_path).to eq project.wiki.full_path + end + end + + describe '#import_url' do + it 'returns URL of the wiki repository' do + expect(wiki.import_url).to eq 'https://xxx@bitbucket.org/gitlabhq/sample.gitlabhq.git/wiki' + end + end +end diff --git a/spec/lib/gitlab/data_builder/push_spec.rb b/spec/lib/gitlab/data_builder/push_spec.rb index befdc18d1aa..0c4decc6518 100644 --- a/spec/lib/gitlab/data_builder/push_spec.rb +++ b/spec/lib/gitlab/data_builder/push_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::DataBuilder::Push do let(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { build(:user, public_email: 'public-email@example.com') } describe '.build_sample' do let(:data) { described_class.build_sample(project, user) } @@ -36,7 +36,7 @@ describe Gitlab::DataBuilder::Push do it { expect(data[:user_id]).to eq(user.id) } it { expect(data[:user_name]).to eq(user.name) } it { expect(data[:user_username]).to eq(user.username) } - it { expect(data[:user_email]).to eq(user.email) } + it { expect(data[:user_email]).to eq(user.public_email) } it { expect(data[:user_avatar]).to eq(user.avatar_url) } it { expect(data[:project_id]).to eq(project.id) } it { expect(data[:project]).to be_a(Hash) } diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index b1f48c15c21..50e473c459e 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -118,11 +118,44 @@ describe Gitlab::Email::Handler::CreateNoteHandler do end end - context "when everything is fine" do + shared_examples "checks permissions on noteable" do + context "when user has access" do + before do + project.add_reporter(user) + end + + it "creates a comment" do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + end + end + + context "when user does not have access" do + it "raises UserNotAuthorizedError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError) + end + end + end + + context "when discussion is locked" do before do - setup_attachment + noteable.update_attribute(:discussion_locked, true) + end + + it_behaves_like "checks permissions on noteable" + end + + context "when issue is confidential" do + let(:issue) { create(:issue, project: project) } + let(:note) { create(:note, noteable: issue, project: project) } + + before do + issue.update_attribute(:confidential, true) end + it_behaves_like "checks permissions on noteable" + end + + shared_examples 'a reply to existing comment' do it "creates a comment" do expect { receiver.execute }.to change { noteable.notes.count }.by(1) new_note = noteable.notes.last @@ -131,8 +164,22 @@ describe Gitlab::Email::Handler::CreateNoteHandler do expect(new_note.position).to eq(note.position) expect(new_note.note).to include("I could not disagree more.") expect(new_note.in_reply_to?(note)).to be_truthy + + if note.part_of_discussion? + expect(new_note.discussion_id).to eq(note.discussion_id) + else + expect(new_note.discussion_id).not_to eq(note.discussion_id) + end + end + end + + context "when everything is fine" do + before do + setup_attachment end + it_behaves_like 'a reply to existing comment' + it "adds all attachments" do receiver.execute @@ -170,4 +217,10 @@ describe Gitlab::Email::Handler::CreateNoteHandler do end end end + + context "when note is not a discussion" do + let(:note) { create(:note_on_merge_request, project: project) } + + it_behaves_like 'a reply to existing comment' + end end diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index e704d1c673c..0010c0304eb 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Blame, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 1bcec04d28f..a1b5cea88c0 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Blob, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged) do Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 0df282d0ae3..0764e525ede 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Branch, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged) do Rugged::Repository.new(File.join(TestEnv.repos_path, repository.relative_path)) end @@ -64,7 +64,7 @@ describe Gitlab::Git::Branch, :seed_helper do context 'with active, stale and future branches' do let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') end let(:user) { create(:user) } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index db68062e433..2611ebed25b 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Commit, :seed_helper do include GitHelpers - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:rugged_repo) do Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end @@ -146,7 +146,7 @@ describe Gitlab::Git::Commit, :seed_helper do end context 'with broken repo' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '', 'group/project') } it 'returns nil' do expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil diff --git a/spec/lib/gitlab/git/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index 771c71a16a9..65dfb93d0db 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Compare, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 8a4415506c4..1d22329b670 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Diff, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:gitaly_diff) do Gitlab::GitalyClient::Diff.new( from_path: '.gitmodules', diff --git a/spec/lib/gitlab/git/remote_repository_spec.rb b/spec/lib/gitlab/git/remote_repository_spec.rb index 53ed7c5a13a..e166628d4ca 100644 --- a/spec/lib/gitlab/git/remote_repository_spec.rb +++ b/spec/lib/gitlab/git/remote_repository_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper' describe Gitlab::Git::RemoteRepository, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } subject { described_class.new(repository) } describe '#empty?' do using RSpec::Parameterized::TableSyntax where(:repository, :result) do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') | false - Gitlab::Git::Repository.new('default', 'does-not-exist.git', '') | true + Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') | false + Gitlab::Git::Repository.new('default', 'does-not-exist.git', '', 'group/project') | true end with_them do @@ -44,11 +44,11 @@ describe Gitlab::Git::RemoteRepository, :seed_helper do using RSpec::Parameterized::TableSyntax where(:other_repository, :result) do - repository | true - Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '') | true - Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '') | false - Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '') | false - Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '') | false + repository | true + Gitlab::Git::Repository.new(repository.storage, repository.relative_path, '', 'group/project') | true + Gitlab::Git::Repository.new('broken', TEST_REPO_PATH, '', 'group/project') | false + Gitlab::Git::Repository.new(repository.storage, 'wrong/relative-path.git', '', 'group/project') | false + Gitlab::Git::Repository.new('broken', 'wrong/relative-path.git', '', 'group/project') | false end with_them do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index a19e3e84f83..cf9e0cccc71 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -19,8 +19,10 @@ describe Gitlab::Git::Repository, :seed_helper do end end - let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } + let(:mutable_repository_path) { File.join(TestEnv.repos_path, mutable_repository.relative_path) } + let(:mutable_repository_rugged) { Rugged::Repository.new(mutable_repository_path) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) } let(:repository_rugged) { Rugged::Repository.new(repository_path) } let(:storage_path) { TestEnv.repos_path } @@ -434,13 +436,13 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#fetch_repository_as_mirror' do let(:new_repository) do - Gitlab::Git::Repository.new('default', 'my_project.git', '') + Gitlab::Git::Repository.new('default', 'my_project.git', '', 'group/project') end subject { new_repository.fetch_repository_as_mirror(repository) } before do - Gitlab::Shell.new.create_repository('default', 'my_project') + Gitlab::Shell.new.create_repository('default', 'my_project', 'group/project') end after do @@ -497,6 +499,48 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#search_files_by_content' do + let(:repository) { mutable_repository } + let(:repository_rugged) { mutable_repository_rugged } + + before do + repository.create_branch('search-files-by-content-branch', 'master') + new_commit_edit_new_file_on_branch(repository_rugged, 'encoding/CHANGELOG', 'search-files-by-content-branch', 'committing something', 'search-files-by-content change') + new_commit_edit_new_file_on_branch(repository_rugged, 'anotherfile', 'search-files-by-content-branch', 'committing something', 'search-files-by-content change') + end + + after do + ensure_seeds + end + + shared_examples 'search files by content' do + it 'should have 2 items' do + expect(search_results.size).to eq(2) + end + + it 'should have the correct matching line' do + expect(search_results).to contain_exactly("search-files-by-content-branch:encoding/CHANGELOG\u00001\u0000search-files-by-content change\n", + "search-files-by-content-branch:anotherfile\u00001\u0000search-files-by-content change\n") + end + end + + it_should_behave_like 'search files by content' do + let(:search_results) do + repository.search_files_by_content('search-files-by-content', 'search-files-by-content-branch') + end + end + + it_should_behave_like 'search files by content' do + let(:search_results) do + repository.gitaly_repository_client.search_files_by_content( + 'search-files-by-content-branch', + 'search-files-by-content', + chunked_response: false + ) + end + end + end + describe '#find_remote_root_ref' do it 'gets the remote root ref from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::RemoteService) @@ -544,7 +588,7 @@ describe Gitlab::Git::Repository, :seed_helper do # Add new commits so that there's a renamed file in the commit history @commit_with_old_name_id = new_commit_edit_old_file(repository_rugged).oid @rename_commit_id = new_commit_move_file(repository_rugged).oid - @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged).oid + @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged, "encoding/CHANGELOG", "Edit encoding/CHANGELOG", "I'm a new changelog with different text").oid end after do @@ -1230,7 +1274,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#gitattribute' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } after do ensure_seeds @@ -1249,7 +1293,7 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'without gitattributes file' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } it 'returns nil' do expect(repository.gitattribute("README.md", 'gitlab-language')).to eq(nil) @@ -1513,7 +1557,7 @@ describe Gitlab::Git::Repository, :seed_helper do context 'repository does not exist' do it 'raises NoRepository and does not call Gitaly WriteConfig' do - repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '') + repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') expect(repository.gitaly_repository_client).not_to receive(:write_config) @@ -1803,7 +1847,7 @@ describe Gitlab::Git::Repository, :seed_helper do out: '/dev/null', err: '/dev/null') - empty_repo = described_class.new('default', 'empty-repo.git', '') + empty_repo = described_class.new('default', 'empty-repo.git', '', 'group/empty-repo') expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000' end @@ -1818,13 +1862,13 @@ describe Gitlab::Git::Repository, :seed_helper do File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0) - non_valid = described_class.new('default', 'non-valid.git', '') + non_valid = described_class.new('default', 'non-valid.git', '', 'a/non-valid') expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository) end it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git', '') + broken_repo = described_class.new('default', 'a/path.git', '', 'a/path') expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository) end @@ -1964,7 +2008,7 @@ describe Gitlab::Git::Repository, :seed_helper do end # Build the options hash that's passed to Rugged::Commit#create - def commit_options(repo, index, message) + def commit_options(repo, index, target, ref, message) options = {} options[:tree] = index.write_tree(repo) options[:author] = { @@ -1978,8 +2022,8 @@ describe Gitlab::Git::Repository, :seed_helper do time: Time.gm(2014, "mar", 3, 20, 15, 1) } options[:message] ||= message - options[:parents] = repo.empty? ? [] : [repo.head.target].compact - options[:update_ref] = "HEAD" + options[:parents] = repo.empty? ? [] : [target].compact + options[:update_ref] = ref options end @@ -1995,6 +2039,8 @@ describe Gitlab::Git::Repository, :seed_helper do options = commit_options( repo, index, + repo.head.target, + "HEAD", "Edit CHANGELOG in its original location" ) @@ -2003,19 +2049,24 @@ describe Gitlab::Git::Repository, :seed_helper do end # Writes a new commit to the repo and returns a Rugged::Commit. Replaces the - # contents of encoding/CHANGELOG with new text. - def new_commit_edit_new_file(repo) - oid = repo.write("I'm a new changelog with different text", :blob) + # contents of the specified file_path with new text. + def new_commit_edit_new_file(repo, file_path, commit_message, text, branch = repo.head) + oid = repo.write(text, :blob) index = repo.index - index.read_tree(repo.head.target.tree) - index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644) - - options = commit_options(repo, index, "Edit encoding/CHANGELOG") - + index.read_tree(branch.target.tree) + index.add(path: file_path, oid: oid, mode: 0100644) + options = commit_options(repo, index, branch.target, branch.canonical_name, commit_message) sha = Rugged::Commit.create(repo, options) repo.lookup(sha) end + # Writes a new commit to the repo and returns a Rugged::Commit. Replaces the + # contents of encoding/CHANGELOG with new text. + def new_commit_edit_new_file_on_branch(repo, file_path, branch_name, commit_message, text) + branch = repo.branches[branch_name] + new_commit_edit_new_file(repo, file_path, commit_message, text, branch) + end + # Writes a new commit to the repo and returns a Rugged::Commit. Moves the # CHANGELOG file to the encoding/ directory. def new_commit_move_file(repo) @@ -2027,7 +2078,7 @@ describe Gitlab::Git::Repository, :seed_helper do index.add(path: "encoding/CHANGELOG", oid: oid, mode: 0100644) index.remove("CHANGELOG") - options = commit_options(repo, index, "Move CHANGELOG to encoding/") + options = commit_options(repo, index, repo.head.target, "HEAD", "Move CHANGELOG to encoding/") sha = Rugged::Commit.create(repo, options) repo.lookup(sha) diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index b51e3879f49..4c0291f64f0 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tag, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } describe '#tags' do describe 'first tag' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index bec875fb03d..4a4d69490a3 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tree, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } context :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 3e34dd592f2..634c370d211 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -776,10 +776,13 @@ describe Gitlab::GitAccess do it "has the correct permissions for #{role}s" do if role == :admin user.update_attribute(:admin, true) + project.add_guest(user) else project.add_role(user, role) end + protected_branch.save + aggregate_failures do matrix.each do |action, allowed| check = -> { push_changes(changes[action]) } @@ -861,25 +864,19 @@ describe Gitlab::GitAccess do [%w(feature exact), ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type| context do - before do - create(:protected_branch, name: protected_branch_name, project: project) - end + let(:protected_branch) { create(:protected_branch, :maintainers_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix) end context "when developers are allowed to push into the #{protected_branch_type} protected branch" do - before do - create(:protected_branch, :developers_can_push, name: protected_branch_name, project: project) - end + let(:protected_branch) { create(:protected_branch, :developers_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) end context "developers are allowed to merge into the #{protected_branch_type} protected branch" do - before do - create(:protected_branch, :developers_can_merge, name: protected_branch_name, project: project) - end + let(:protected_branch) { create(:protected_branch, :developers_can_merge, name: protected_branch_name, project: project) } context "when a merge request exists for the given source/target branch" do context "when the merge request is in progress" do @@ -906,17 +903,13 @@ describe Gitlab::GitAccess do end context "when developers are allowed to push and merge into the #{protected_branch_type} protected branch" do - before do - create(:protected_branch, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) - end + let(:protected_branch) { create(:protected_branch, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) end context "when no one is allowed to push to the #{protected_branch_name} protected branch" do - before do - create(:protected_branch, :no_one_can_push, name: protected_branch_name, project: project) - end + let(:protected_branch) { build(:protected_branch, :no_one_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }, maintainer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }, diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index aff47599ad6..d5508dbff5d 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::GitalyClient::RemoteService do end describe '#fetch_internal_remote' do - let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } it 'sends an fetch_internal_remote message and returns the result value' do expect_any_instance_of(Gitaly::RemoteService::Stub) diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb index 550db6db6d9..78a5e195ad1 100644 --- a/spec/lib/gitlab/gitaly_client/util_spec.rb +++ b/spec/lib/gitlab/gitaly_client/util_spec.rb @@ -7,6 +7,7 @@ describe Gitlab::GitalyClient::Util do let(:gl_repository) { 'project-1' } let(:git_object_directory) { '.git/objects' } let(:git_alternate_object_directory) { ['/dir/one', '/dir/two'] } + let(:gl_project_path) { 'namespace/myproject' } let(:git_env) do { 'GIT_OBJECT_DIRECTORY_RELATIVE' => git_object_directory, @@ -15,7 +16,7 @@ describe Gitlab::GitalyClient::Util do end subject do - described_class.repository(repository_storage, relative_path, gl_repository) + described_class.repository(repository_storage, relative_path, gl_repository, gl_project_path) end it 'creates a Gitaly::Repository with the given data' do @@ -27,6 +28,7 @@ describe Gitlab::GitalyClient::Util do expect(subject.gl_repository).to eq(gl_repository) expect(subject.git_object_directory).to eq(git_object_directory) expect(subject.git_alternate_object_directories).to eq(git_alternate_object_directory) + expect(subject.gl_project_path).to eq(gl_project_path) end end end diff --git a/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb index 4857f2afbe2..8fd328d9c1e 100644 --- a/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/lfs_object_importer_spec.rb @@ -2,20 +2,26 @@ require 'spec_helper' describe Gitlab::GithubImport::Importer::LfsObjectImporter do let(:project) { create(:project) } - let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } - - let(:github_lfs_object) do - Gitlab::GithubImport::Representation::LfsObject.new( - oid: 'oid', download_link: download_link - ) + let(:lfs_attributes) do + { + oid: 'oid', + size: 1, + link: 'http://www.gitlab.com/lfs_objects/oid' + } end + let(:lfs_download_object) { LfsDownloadObject.new(lfs_attributes) } + let(:github_lfs_object) { Gitlab::GithubImport::Representation::LfsObject.new(lfs_attributes) } + let(:importer) { described_class.new(github_lfs_object, project, nil) } describe '#execute' do it 'calls the LfsDownloadService with the lfs object attributes' do - expect_any_instance_of(Projects::LfsPointers::LfsDownloadService) - .to receive(:execute).with('oid', download_link) + allow(importer).to receive(:lfs_download_object).and_return(lfs_download_object) + + service = double + expect(Projects::LfsPointers::LfsDownloadService).to receive(:new).with(project, lfs_download_object).and_return(service) + expect(service).to receive(:execute) importer.execute end diff --git a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb index 5f5c6b803c0..50442552eee 100644 --- a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb @@ -5,7 +5,15 @@ describe Gitlab::GithubImport::Importer::LfsObjectsImporter do let(:client) { double(:client) } let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } - let(:github_lfs_object) { ['oid', download_link] } + let(:lfs_attributes) do + { + oid: 'oid', + size: 1, + link: 'http://www.gitlab.com/lfs_objects/oid' + } + end + + let(:lfs_download_object) { LfsDownloadObject.new(lfs_attributes) } describe '#parallel?' do it 'returns true when running in parallel mode' do @@ -48,7 +56,7 @@ describe Gitlab::GithubImport::Importer::LfsObjectsImporter do allow(importer) .to receive(:each_object_to_import) - .and_yield(['oid', download_link]) + .and_yield(lfs_download_object) expect(Gitlab::GithubImport::Importer::LfsObjectImporter) .to receive(:new) @@ -71,7 +79,7 @@ describe Gitlab::GithubImport::Importer::LfsObjectsImporter do allow(importer) .to receive(:each_object_to_import) - .and_yield(github_lfs_object) + .and_yield(lfs_download_object) expect(Gitlab::GithubImport::ImportLfsObjectWorker) .to receive(:perform_async) diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index 77f5b2ffa37..47233ea6ee2 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -5,6 +5,14 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do let(:import_state) { double(:import_state) } let(:client) { double(:client) } + let(:wiki) do + double( + :wiki, + disk_path: 'foo.wiki', + full_path: 'group/foo.wiki' + ) + end + let(:project) do double( :project, @@ -15,7 +23,9 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do repository: repository, create_wiki: true, import_state: import_state, - lfs_enabled?: true + full_path: 'group/foo', + lfs_enabled?: true, + wiki: wiki ) end @@ -195,7 +205,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do it 'imports the wiki repository' do expect(importer.gitlab_shell) .to receive(:import_repository) - .with('foo', 'foo.wiki', 'foo.wiki.git') + .with('foo', 'foo.wiki', 'foo.wiki.git', 'group/foo.wiki') expect(importer.import_wiki_repository).to eq(true) end 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 242c16c4bdc..6084dc96410 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ] RSpec::Mocks.with_temporary_scope do - @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') + @project = create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') @shared = @project.import_export_shared allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/') @@ -40,7 +40,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do project = Project.find_by_path('project') expect(project.project_feature.issues_access_level).to eq(ProjectFeature::DISABLED) - expect(project.project_feature.builds_access_level).to eq(ProjectFeature::DISABLED) + expect(project.project_feature.builds_access_level).to eq(ProjectFeature::ENABLED) expect(project.project_feature.snippets_access_level).to eq(ProjectFeature::ENABLED) expect(project.project_feature.wiki_access_level).to eq(ProjectFeature::ENABLED) expect(project.project_feature.merge_requests_access_level).to eq(ProjectFeature::ENABLED) @@ -273,6 +273,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do it 'has group milestone' do expect(project.group.milestones.size).to eq(results.fetch(:milestones, 0)) end + + it 'has the correct visibility level' do + # INTERNAL in the `project.json`, group's is PRIVATE + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + end end context 'Light JSON' do @@ -347,7 +352,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do :issues_disabled, name: 'project', path: 'project', - group: create(:group)) + group: create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE)) end before do @@ -434,4 +439,58 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end end end + + describe '#restored_project' do + let(:project) { create(:project) } + let(:shared) { project.import_export_shared } + let(:tree_hash) { { 'visibility_level' => visibility } } + let(:restorer) { described_class.new(user: nil, shared: shared, project: project) } + + before do + restorer.instance_variable_set(:@tree_hash, tree_hash) + end + + context 'no group visibility' do + let(:visibility) { Gitlab::VisibilityLevel::PRIVATE } + + it 'uses the project visibility' do + expect(restorer.restored_project.visibility_level).to eq(visibility) + end + end + + context 'with group visibility' do + before do + group = create(:group, visibility_level: group_visibility) + + project.update(group: group) + end + + context 'private group visibility' do + let(:group_visibility) { Gitlab::VisibilityLevel::PRIVATE } + let(:visibility) { Gitlab::VisibilityLevel::PUBLIC } + + it 'uses the group visibility' do + expect(restorer.restored_project.visibility_level).to eq(group_visibility) + end + end + + context 'public group visibility' do + let(:group_visibility) { Gitlab::VisibilityLevel::PUBLIC } + let(:visibility) { Gitlab::VisibilityLevel::PRIVATE } + + it 'uses the project visibility' do + expect(restorer.restored_project.visibility_level).to eq(visibility) + end + end + + context 'internal group visibility' do + let(:group_visibility) { Gitlab::VisibilityLevel::INTERNAL } + let(:visibility) { Gitlab::VisibilityLevel::PUBLIC } + + it 'uses the group visibility' do + expect(restorer.restored_project.visibility_level).to eq(group_visibility) + end + end + end + end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index fe2087e8fc3..baca8f6d542 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -603,5 +603,6 @@ ResourceLabelEvent: ErrorTracking::ProjectErrorTrackingSetting: - id - api_url -- enabled - project_id +- project_name +- organization_name diff --git a/spec/lib/gitlab/import_export/shared_spec.rb b/spec/lib/gitlab/import_export/shared_spec.rb new file mode 100644 index 00000000000..f2d750c6595 --- /dev/null +++ b/spec/lib/gitlab/import_export/shared_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' +require 'fileutils' + +describe Gitlab::ImportExport::Shared do + let(:project) { build(:project) } + subject { project.import_export_shared } + + describe '#error' do + let(:error) { StandardError.new('Error importing into /my/folder Permission denied @ unlink_internal - /var/opt/gitlab/gitlab-rails/shared/a/b/c/uploads/file') } + + it 'filters any full paths' do + subject.error(error) + + expect(subject.errors).to eq(['Error importing into [FILTERED] Permission denied @ unlink_internal - [FILTERED]']) + end + + it 'calls the error logger with the full message' do + expect(subject).to receive(:log_error).with(hash_including(message: error.message)) + + subject.error(error) + end + + it 'calls the debug logger with a backtrace' do + error.set_backtrace('backtrace') + + expect(subject).to receive(:log_debug).with(hash_including(backtrace: 'backtrace')) + + subject.error(error) + end + end +end diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb index 49d857d9483..76f8253ec9b 100644 --- a/spec/lib/gitlab/import_export/version_checker_spec.rb +++ b/spec/lib/gitlab/import_export/version_checker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' include ImportExport::CommonUtil describe Gitlab::ImportExport::VersionChecker do - let(:shared) { Gitlab::ImportExport::Shared.new(nil) } + let!(:shared) { Gitlab::ImportExport::Shared.new(nil) } describe 'bundle a project Git repo' do let(:version) { Gitlab::ImportExport.version } diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb index c7f92cbb143..8433d40b2ea 100644 --- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb @@ -171,51 +171,6 @@ describe Gitlab::Kubernetes::Helm::Api do end end - describe '#update' do - let(:rbac) { false } - - let(:command) do - Gitlab::Kubernetes::Helm::UpgradeCommand.new( - application_name, - chart: 'chart-name', - files: files, - rbac: rbac - ) - end - - before do - allow(namespace).to receive(:ensure_exists!).once - - allow(client).to receive(:update_config_map).and_return(nil) - allow(client).to receive(:create_pod).and_return(nil) - allow(client).to receive(:delete_pod).and_return(nil) - end - - it 'ensures the namespace exists before creating the pod' do - expect(namespace).to receive(:ensure_exists!).once.ordered - expect(client).to receive(:create_pod).once.ordered - - subject.update(command) - end - - it 'removes an existing pod before updating' do - expect(client).to receive(:delete_pod).with('upgrade-app-name', 'gitlab-managed-apps').once.ordered - expect(client).to receive(:create_pod).once.ordered - - subject.update(command) - end - - it 'updates the config map on kubeclient when one exists' do - resource = Gitlab::Kubernetes::ConfigMap.new( - application_name, files - ).generate - - expect(client).to receive(:update_config_map).with(resource).once - - subject.update(command) - end - end - describe '#status' do let(:phase) { Gitlab::Kubernetes::Pod::RUNNING } let(:pod) { Kubeclient::Resource.new(status: { phase: phase }) } # partial representation diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 82ed4d47857..db76d5d207e 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -21,6 +21,15 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do ) end + let(:tls_flags) do + <<~EOS.squish + --tls + --tls-ca-cert /data/helm/app-name/config/ca.pem + --tls-cert /data/helm/app-name/config/cert.pem + --tls-key /data/helm/app-name/config/key.pem + EOS + end + subject { install_command } it_behaves_like 'helm commands' do @@ -36,12 +45,10 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:helm_install_comand) do <<~EOS.squish - helm install chart-name - --name app-name - --tls - --tls-ca-cert /data/helm/app-name/config/ca.pem - --tls-cert /data/helm/app-name/config/cert.pem - --tls-key /data/helm/app-name/config/key.pem + helm upgrade app-name chart-name + --install + --reset-values + #{tls_flags} --version 1.2.3 --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps @@ -66,12 +73,10 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:helm_install_command) do <<~EOS.squish - helm install chart-name - --name app-name - --tls - --tls-ca-cert /data/helm/app-name/config/ca.pem - --tls-cert /data/helm/app-name/config/cert.pem - --tls-key /data/helm/app-name/config/key.pem + helm upgrade app-name chart-name + --install + --reset-values + #{tls_flags} --version 1.2.3 --set rbac.create\\=true,rbac.enabled\\=true --namespace gitlab-managed-apps @@ -95,12 +100,10 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:helm_install_command) do <<~EOS.squish - helm install chart-name - --name app-name - --tls - --tls-ca-cert /data/helm/app-name/config/ca.pem - --tls-cert /data/helm/app-name/config/cert.pem - --tls-key /data/helm/app-name/config/key.pem + helm upgrade app-name chart-name + --install + --reset-values + #{tls_flags} --version 1.2.3 --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps @@ -120,15 +123,22 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add app-name https://repository.example.com helm repo update + /bin/date + /bin/true #{helm_install_command} EOS end let(:helm_install_command) do - <<~EOS.strip - /bin/date - /bin/true - helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --version 1.2.3 --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml + <<~EOS.squish + helm upgrade app-name chart-name + --install + --reset-values + #{tls_flags} + --version 1.2.3 + --set rbac.create\\=false,rbac.enabled\\=false + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml EOS end end @@ -145,14 +155,21 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do helm repo add app-name https://repository.example.com helm repo update #{helm_install_command} + /bin/date + /bin/false EOS end let(:helm_install_command) do - <<~EOS.strip - helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --version 1.2.3 --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml - /bin/date - /bin/false + <<~EOS.squish + helm upgrade app-name chart-name + --install + --reset-values + #{tls_flags} + --version 1.2.3 + --set rbac.create\\=false,rbac.enabled\\=false + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml EOS end end @@ -174,8 +191,9 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:helm_install_command) do <<~EOS.squish - helm install chart-name - --name app-name + helm upgrade app-name chart-name + --install + --reset-values --version 1.2.3 --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps @@ -201,12 +219,10 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:helm_install_command) do <<~EOS.squish - helm install chart-name - --name app-name - --tls - --tls-ca-cert /data/helm/app-name/config/ca.pem - --tls-cert /data/helm/app-name/config/cert.pem - --tls-key /data/helm/app-name/config/key.pem + helm upgrade app-name chart-name + --install + --reset-values + #{tls_flags} --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml diff --git a/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb deleted file mode 100644 index 9b201dae417..00000000000 --- a/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb +++ /dev/null @@ -1,140 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Gitlab::Kubernetes::Helm::UpgradeCommand do - let(:application) { build(:clusters_applications_prometheus) } - let(:files) { { 'ca.pem': 'some file content' } } - let(:namespace) { ::Gitlab::Kubernetes::Helm::NAMESPACE } - let(:rbac) { false } - let(:upgrade_command) do - described_class.new( - application.name, - chart: application.chart, - files: files, - rbac: rbac - ) - end - - subject { upgrade_command } - - it_behaves_like 'helm commands' do - let(:commands) do - <<~EOS - helm init --upgrade - for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done - helm upgrade #{application.name} #{application.chart} --tls --tls-ca-cert /data/helm/#{application.name}/config/ca.pem --tls-cert /data/helm/#{application.name}/config/cert.pem --tls-key /data/helm/#{application.name}/config/key.pem --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml - EOS - end - end - - context 'rbac is true' do - let(:rbac) { true } - - it_behaves_like 'helm commands' do - let(:commands) do - <<~EOS - helm init --upgrade - for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done - helm upgrade #{application.name} #{application.chart} --tls --tls-ca-cert /data/helm/#{application.name}/config/ca.pem --tls-cert /data/helm/#{application.name}/config/cert.pem --tls-key /data/helm/#{application.name}/config/key.pem --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml - EOS - end - end - end - - context 'with an application with a repository' do - let(:ci_runner) { create(:ci_runner) } - let(:application) { build(:clusters_applications_runner, runner: ci_runner) } - let(:upgrade_command) do - described_class.new( - application.name, - chart: application.chart, - files: files, - rbac: rbac, - repository: application.repository - ) - end - - it_behaves_like 'helm commands' do - let(:commands) do - <<~EOS - helm init --upgrade - for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done - helm repo add #{application.name} #{application.repository} - helm upgrade #{application.name} #{application.chart} --tls --tls-ca-cert /data/helm/#{application.name}/config/ca.pem --tls-cert /data/helm/#{application.name}/config/cert.pem --tls-key /data/helm/#{application.name}/config/key.pem --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml - EOS - end - end - end - - context 'when there is no ca.pem file' do - let(:files) { { 'file.txt': 'some content' } } - - it_behaves_like 'helm commands' do - let(:commands) do - <<~EOS - helm init --upgrade - for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done - helm upgrade #{application.name} #{application.chart} --reset-values --install --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml - EOS - end - end - end - - describe '#pod_resource' do - subject { upgrade_command.pod_resource } - - context 'rbac is enabled' do - let(:rbac) { true } - - it 'generates a pod that uses the tiller serviceAccountName' do - expect(subject.spec.serviceAccountName).to eq('tiller') - end - end - - context 'rbac is not enabled' do - let(:rbac) { false } - - it 'generates a pod that uses the default serviceAccountName' do - expect(subject.spec.serviceAcccountName).to be_nil - end - end - end - - describe '#config_map_resource' do - let(:metadata) do - { - name: "values-content-configuration-#{application.name}", - namespace: namespace, - labels: { name: "values-content-configuration-#{application.name}" } - } - end - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: files) } - - it 'returns a KubeClient resource with config map content for the application' do - expect(subject.config_map_resource).to eq(resource) - end - end - - describe '#rbac?' do - subject { upgrade_command.rbac? } - - context 'rbac is enabled' do - let(:rbac) { true } - - it { is_expected.to be_truthy } - end - - context 'rbac is not enabled' do - let(:rbac) { false } - - it { is_expected.to be_falsey } - end - end - - describe '#pod_name' do - it 'returns the pod name' do - expect(subject.pod_name).to eq("upgrade-#{application.name}") - end - end -end diff --git a/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb index 7723533aee2..7519707293c 100644 --- a/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/wiki_formatter_spec.rb @@ -10,11 +10,17 @@ describe Gitlab::LegacyGithubImport::WikiFormatter do subject(:wiki) { described_class.new(project) } describe '#disk_path' do - it 'appends .wiki to project path' do + it 'appends .wiki to disk path' do expect(wiki.disk_path).to eq project.wiki.disk_path end end + describe '#full_path' do + it 'appends .wiki to project path' do + expect(wiki.full_path).to eq project.wiki.full_path + end + end + describe '#import_url' do it 'returns URL of the wiki repository' do expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/sample.gitlabhq.wiki.git' diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index 771b633a2b9..4b03f3c2532 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -37,7 +37,7 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do end it 'updates metrics type unix and with addr' do - labels = { type: 'unix', address: socket_address } + 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') @@ -69,7 +69,7 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do end it 'updates metrics type unix and with addr' do - labels = { type: 'tcp', address: tcp_socket_address } + 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') diff --git a/spec/lib/gitlab/project_template_spec.rb b/spec/lib/gitlab/project_template_spec.rb index 57b0ef8d1ad..1cc2bde50e9 100644 --- a/spec/lib/gitlab/project_template_spec.rb +++ b/spec/lib/gitlab/project_template_spec.rb @@ -6,7 +6,12 @@ describe Gitlab::ProjectTemplate do expected = [ described_class.new('rails', 'Ruby on Rails', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/rails'), described_class.new('spring', 'Spring', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/spring'), - described_class.new('express', 'NodeJS Express', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/express') + described_class.new('express', 'NodeJS Express', 'Includes an MVC structure, .gitignore, Gemfile, and more great stuff', 'https://gitlab.com/gitlab-org/project-templates/express'), + described_class.new('hugo', 'Pages/Hugo', 'Everything you need to get started using a Hugo Pages site.', 'https://gitlab.com/pages/hugo'), + described_class.new('jekyll', 'Pages/Jekyll', 'Everything you need to get started using a Jekyll Pages site.', 'https://gitlab.com/pages/jekyll'), + described_class.new('plainhtml', 'Pages/Plain HTML', 'Everything you need to get started using a plain HTML Pages site.', 'https://gitlab.com/pages/plain-html'), + described_class.new('gitbook', 'Pages/GitBook', 'Everything you need to get started using a GitBook Pages site.', 'https://gitlab.com/pages/gitbook'), + described_class.new('hexo', 'Pages/Hexo', 'Everything you need to get started using a plan Hexo Pages site.', 'https://gitlab.com/pages/hexo') ] expect(described_class.all).to be_an(Array) diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 6ce9d515a0f..033e1bf81a1 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -412,7 +412,7 @@ describe Gitlab::Shell do end it 'creates a repository' do - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_truthy + expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_truthy expect(File.stat(created_path).mode & 0o777).to eq(0o770) @@ -427,7 +427,7 @@ describe Gitlab::Shell do # should cause #create_repository to fail. FileUtils.touch(created_path) - expect(gitlab_shell.create_repository(repository_storage, repo_name)).to be_falsy + expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_falsy end end @@ -474,13 +474,10 @@ describe Gitlab::Shell do end describe '#fork_repository' do + let(:target_project) { create(:project) } + subject do - gitlab_shell.fork_repository( - project.repository_storage, - project.disk_path, - 'nfs-file05', - 'fork/path' - ) + gitlab_shell.fork_repository(project, target_project) end it 'returns true when the command succeeds' do @@ -505,7 +502,7 @@ describe Gitlab::Shell do it 'returns true when the command succeeds' do expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:import_repository).with(import_url) - result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) expect(result).to be_truthy end @@ -516,7 +513,7 @@ describe Gitlab::Shell do expect_any_instance_of(Gitlab::Shell::GitalyGitlabProjects).to receive(:output) { 'error'} expect do - gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) + gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url, project.full_path) end.to raise_error(Gitlab::Shell::Error, "error") end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 2a09f581f68..4f5993ba226 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -26,6 +26,8 @@ describe Gitlab::UsageData do create(:clusters_applications_prometheus, :installed, cluster: gcp_cluster) create(:clusters_applications_runner, :installed, cluster: gcp_cluster) create(:clusters_applications_knative, :installed, cluster: gcp_cluster) + + ProjectFeature.first.update_attribute('repository_access_level', 0) end subject { described_class.data } @@ -112,6 +114,7 @@ describe Gitlab::UsageData do projects_slack_notifications_active projects_slack_slash_active projects_prometheus_active + projects_with_repositories_enabled pages_domains protected_branches releases @@ -134,6 +137,7 @@ describe Gitlab::UsageData do expect(count_data[:projects_jira_cloud_active]).to eq(1) expect(count_data[:projects_slack_notifications_active]).to eq(2) expect(count_data[:projects_slack_slash_active]).to eq(1) + expect(count_data[:projects_with_repositories_enabled]).to eq(2) expect(count_data[:clusters_enabled]).to eq(7) expect(count_data[:project_clusters_enabled]).to eq(6) diff --git a/spec/lib/safe_zip/entry_spec.rb b/spec/lib/safe_zip/entry_spec.rb new file mode 100644 index 00000000000..115e28c5994 --- /dev/null +++ b/spec/lib/safe_zip/entry_spec.rb @@ -0,0 +1,196 @@ +require 'spec_helper' + +describe SafeZip::Entry do + let(:target_path) { Dir.mktmpdir('safe-zip') } + let(:directories) { %w(public folder/with/subfolder) } + let(:params) { SafeZip::ExtractParams.new(directories: directories, to: target_path) } + + let(:entry) { described_class.new(zip_archive, zip_entry, params) } + let(:entry_name) { 'public/folder/index.html' } + let(:entry_path_dir) { File.join(target_path, File.dirname(entry_name)) } + let(:entry_path) { File.join(target_path, entry_name) } + let(:zip_archive) { double } + + let(:zip_entry) do + double( + name: entry_name, + file?: false, + directory?: false, + symlink?: false) + end + + after do + FileUtils.remove_entry_secure(target_path) + end + + context '#path_dir' do + subject { entry.path_dir } + + it { is_expected.to eq(target_path + '/public/folder') } + end + + context '#exist?' do + subject { entry.exist? } + + context 'when entry does not exist' do + it { is_expected.not_to be_truthy } + end + + context 'when entry does exist' do + before do + create_entry + end + + it { is_expected.to be_truthy } + end + end + + describe '#extract' do + subject { entry.extract } + + context 'when entry does not match the filtered directories' do + using RSpec::Parameterized::TableSyntax + + where(:entry_name) do + [ + 'assets/folder/index.html', + 'public/../folder/index.html', + 'public/../../../../../index.html', + '../../../../../public/index.html', + '/etc/passwd' + ] + end + + with_them do + it 'does not extract file' do + is_expected.to be_falsey + end + end + end + + context 'when entry does exist' do + before do + create_entry + end + + it 'raises an exception' do + expect { subject }.to raise_error(SafeZip::Extract::AlreadyExistsError) + end + end + + context 'when entry type is unknown' do + it 'raises an exception' do + expect { subject }.to raise_error(SafeZip::Extract::UnsupportedEntryError) + end + end + + context 'when entry is valid' do + shared_examples 'secured symlinks' do + context 'when we try to extract entry into symlinked folder' do + before do + FileUtils.mkdir_p(File.join(target_path, "source")) + File.symlink("source", File.join(target_path, "public")) + end + + it 'raises an exception' do + expect { subject }.to raise_error(SafeZip::Extract::PermissionDeniedError) + end + end + end + + context 'and is file' do + before do + allow(zip_entry).to receive(:file?) { true } + end + + it 'does extract file' do + expect(zip_archive).to receive(:extract) + .with(zip_entry, entry_path) + .and_return(true) + + is_expected.to be_truthy + end + + it_behaves_like 'secured symlinks' + end + + context 'and is directory' do + let(:entry_name) { 'public/folder/assets' } + + before do + allow(zip_entry).to receive(:directory?) { true } + end + + it 'does create directory' do + is_expected.to be_truthy + + expect(File.exist?(entry_path)).to eq(true) + end + + it_behaves_like 'secured symlinks' + end + + context 'and is symlink' do + let(:entry_name) { 'public/folder/assets' } + + before do + allow(zip_entry).to receive(:symlink?) { true } + allow(zip_archive).to receive(:read).with(zip_entry) { entry_symlink } + end + + shared_examples 'a valid symlink' do + it 'does create symlink' do + is_expected.to be_truthy + + expect(File.exist?(entry_path)).to eq(true) + end + end + + context 'when source is within target' do + let(:entry_symlink) { '../images' } + + context 'but does not exist' do + it 'raises an exception' do + expect { subject }.to raise_error(SafeZip::Extract::SymlinkSourceDoesNotExistError) + end + end + + context 'and does exist' do + before do + FileUtils.mkdir_p(File.join(target_path, 'public', 'images')) + end + + it_behaves_like 'a valid symlink' + end + end + + context 'when source points outside of target' do + let(:entry_symlink) { '../../images' } + + before do + FileUtils.mkdir(File.join(target_path, 'images')) + end + + it 'raises an exception' do + expect { subject }.to raise_error(SafeZip::Extract::PermissionDeniedError) + end + end + + context 'when source points to /etc/passwd' do + let(:entry_symlink) { '/etc/passwd' } + + it 'raises an exception' do + expect { subject }.to raise_error(SafeZip::Extract::PermissionDeniedError) + end + end + end + end + end + + private + + def create_entry + FileUtils.mkdir_p(entry_path_dir) + FileUtils.touch(entry_path) + end +end diff --git a/spec/lib/safe_zip/extract_params_spec.rb b/spec/lib/safe_zip/extract_params_spec.rb new file mode 100644 index 00000000000..85e22cfa495 --- /dev/null +++ b/spec/lib/safe_zip/extract_params_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe SafeZip::ExtractParams do + let(:target_path) { Dir.mktmpdir("safe-zip") } + let(:params) { described_class.new(directories: directories, to: target_path) } + let(:directories) { %w(public folder/with/subfolder) } + + after do + FileUtils.remove_entry_secure(target_path) + end + + describe '#extract_path' do + subject { params.extract_path } + + it { is_expected.to eq(target_path) } + end + + describe '#matching_target_directory' do + using RSpec::Parameterized::TableSyntax + + subject { params.matching_target_directory(target_path + path) } + + where(:path, :result) do + '/public/index.html' | '/public/' + '/non/existing/path' | nil + '/public' | nil + '/folder/with/index.html' | nil + end + + with_them do + it { is_expected.to eq(result ? target_path + result : nil) } + end + end + + describe '#target_directories' do + subject { params.target_directories } + + it 'starts with target_path' do + is_expected.to all(start_with(target_path + '/')) + end + + it 'ends with / for all paths' do + is_expected.to all(end_with('/')) + end + end + + describe '#directories_wildcard' do + subject { params.directories_wildcard } + + it 'adds * for all paths' do + is_expected.to all(end_with('/*')) + end + end +end diff --git a/spec/lib/safe_zip/extract_spec.rb b/spec/lib/safe_zip/extract_spec.rb new file mode 100644 index 00000000000..b75a8fede00 --- /dev/null +++ b/spec/lib/safe_zip/extract_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe SafeZip::Extract do + let(:target_path) { Dir.mktmpdir('safe-zip') } + let(:directories) { %w(public) } + let(:object) { described_class.new(archive) } + let(:archive) { Rails.root.join('spec', 'fixtures', 'safe_zip', archive_name) } + + after do + FileUtils.remove_entry_secure(target_path) + end + + context '#extract' do + subject { object.extract(directories: directories, to: target_path) } + + shared_examples 'extracts archive' do |param| + before do + stub_feature_flags(safezip_use_rubyzip: param) + end + + it 'does extract archive' do + subject + + expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) + expect(File.exist?(File.join(target_path, 'source'))).to eq(false) + end + end + + shared_examples 'fails to extract archive' do |param| + before do + stub_feature_flags(safezip_use_rubyzip: param) + end + + it 'does not extract archive' do + expect { subject }.to raise_error(SafeZip::Extract::Error) + end + end + + %w(valid-simple.zip valid-symlinks-first.zip valid-non-writeable.zip).each do |name| + context "when using #{name} archive" do + let(:archive_name) { name } + + context 'for RubyZip' do + it_behaves_like 'extracts archive', true + end + + context 'for UnZip' do + it_behaves_like 'extracts archive', false + end + end + end + + %w(invalid-symlink-does-not-exist.zip invalid-symlinks-outside.zip).each do |name| + context "when using #{name} archive" do + let(:archive_name) { name } + + context 'for RubyZip' do + it_behaves_like 'fails to extract archive', true + end + + context 'for UnZip (UNSAFE)' do + it_behaves_like 'extracts archive', false + end + end + end + + context 'when no matching directories are found' do + let(:archive_name) { 'valid-simple.zip' } + let(:directories) { %w(non/existing) } + + context 'for RubyZip' do + it_behaves_like 'fails to extract archive', true + end + + context 'for UnZip' do + it_behaves_like 'fails to extract archive', false + end + end + end +end diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb index b36be0fd9c1..6fbf60a6222 100644 --- a/spec/lib/sentry/client_spec.rb +++ b/spec/lib/sentry/client_spec.rb @@ -3,30 +3,76 @@ require 'spec_helper' describe Sentry::Client do - let(:issue_status) { 'unresolved' } - let(:limit) { 20 } let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } let(:token) { 'test-token' } - let(:sample_response) do + let(:issues_sample_response) do Gitlab::Utils.deep_indifferent_access( - JSON.parse(File.read(Rails.root.join('spec/fixtures/sentry/issues_sample_response.json'))) + JSON.parse(fixture_file('sentry/issues_sample_response.json')) + ) + end + + let(:projects_sample_response) do + Gitlab::Utils.deep_indifferent_access( + JSON.parse(fixture_file('sentry/list_projects_sample_response.json')) ) end subject(:client) { described_class.new(sentry_url, token) } - describe '#list_issues' do - subject { client.list_issues(issue_status: issue_status, limit: limit) } + # Requires sentry_api_url and subject to be defined + shared_examples 'no redirects' do + let(:redirect_to) { 'https://redirected.example.com' } + let(:other_url) { 'https://other.example.org' } + + let!(:redirected_req_stub) { stub_sentry_request(other_url) } + + let!(:redirect_req_stub) do + stub_sentry_request( + sentry_api_url, + status: 302, + headers: { location: redirect_to } + ) + end - before do - stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: sample_response) + it 'does not follow redirects' do + expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302') + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested end + end - it 'returns objects of type ErrorTracking::Error' do - expect(subject.length).to eq(1) - expect(subject[0]).to be_a(Gitlab::ErrorTracking::Error) + shared_examples 'has correct return type' do |klass| + it "returns objects of type #{klass}" do + expect(subject).to all( be_a(klass) ) end + end + + shared_examples 'has correct length' do |length| + it { expect(subject.length).to eq(length) } + end + + # Requires sentry_api_request and subject to be defined + shared_examples 'calls sentry api' do + it 'calls sentry api' do + subject + + expect(sentry_api_request).to have_been_requested + end + end + + describe '#list_issues' do + let(:issue_status) { 'unresolved' } + let(:limit) { 20 } + + let!(:sentry_api_request) { stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: issues_sample_response) } + + subject { client.list_issues(issue_status: issue_status, limit: limit) } + + it_behaves_like 'calls sentry api' + + it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Error + it_behaves_like 'has correct length', 1 context 'error object created from sentry response' do using RSpec::Parameterized::TableSyntax @@ -50,7 +96,7 @@ describe Sentry::Client do end with_them do - it { expect(subject[0].public_send(error_object)).to eq(sample_response[0].dig(*sentry_response)) } + it { expect(subject[0].public_send(error_object)).to eq(issues_sample_response[0].dig(*sentry_response)) } end context 'external_url' do @@ -61,24 +107,9 @@ describe Sentry::Client do end context 'redirects' do - let(:redirect_to) { 'https://redirected.example.com' } - let(:other_url) { 'https://other.example.org' } - - let!(:redirected_req_stub) { stub_sentry_request(other_url) } - - let!(:redirect_req_stub) do - stub_sentry_request( - sentry_url + '/issues/?limit=20&query=is:unresolved', - status: 302, - headers: { location: redirect_to } - ) - end + let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } - it 'does not follow redirects' do - expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302') - expect(redirect_req_stub).to have_been_requested - expect(redirected_req_stub).not_to have_been_requested - end + it_behaves_like 'no redirects' end # Sentry API returns 404 if there are extra slashes in the URL! @@ -99,7 +130,75 @@ describe Sentry::Client do anything ).and_call_original - client.list_issues(issue_status: issue_status, limit: limit) + subject + + expect(valid_req_stub).to have_been_requested + end + end + end + + describe '#list_projects' do + let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' } + + let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: projects_sample_response) } + + subject { client.list_projects } + + it_behaves_like 'calls sentry api' + + it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project + it_behaves_like 'has correct length', 2 + + context 'keys missing in API response' do + it 'raises exception' do + projects_sample_response[0].delete(:slug) + + stub_sentry_request(sentry_list_projects_url, body: projects_sample_response) + + expect { subject }.to raise_error(Sentry::Client::SentryError, 'Sentry API response is missing keys. key not found: "slug"') + end + end + + context 'error object created from sentry response' do + using RSpec::Parameterized::TableSyntax + + where(:sentry_project_object, :sentry_response) do + :id | :id + :name | :name + :status | :status + :slug | :slug + :organization_name | [:organization, :name] + :organization_id | [:organization, :id] + :organization_slug | [:organization, :slug] + end + + with_them do + it { expect(subject[0].public_send(sentry_project_object)).to eq(projects_sample_response[0].dig(*sentry_response)) } + end + end + + context 'redirects' do + let(:sentry_api_url) { sentry_list_projects_url } + + it_behaves_like 'no redirects' + end + + # Sentry API returns 404 if there are extra slashes in the URL! + context 'extra slashes in URL' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api//0/projects//' } + let(:client) { described_class.new(sentry_url, token) } + + let!(:valid_req_stub) do + stub_sentry_request(sentry_list_projects_url) + end + + it 'removes extra slashes in api url' do + expect(Gitlab::HTTP).to receive(:get).with( + URI(sentry_list_projects_url), + anything + ).and_call_original + + subject expect(valid_req_stub).to have_been_requested end |