diff options
Diffstat (limited to 'spec/lib/gitlab')
113 files changed, 2849 insertions, 1081 deletions
diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index 3a8667e434d..dcbd12fe190 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -498,7 +498,7 @@ describe Gitlab::Auth::OAuth::User do end end - describe 'ensure backwards compatibility with with sync email from provider option' do + describe 'ensure backwards compatibility with sync email from provider option' do let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') } before do diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index 242ab4a91dd..3d979132880 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -19,17 +19,17 @@ describe Gitlab::Auth::RequestAuthenticator do allow_any_instance_of(described_class).to receive(:find_sessionless_user).and_return(sessionless_user) allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user) - expect(subject.user).to eq sessionless_user + expect(subject.user([:api])).to eq sessionless_user end it 'returns session user if no sessionless user found' do allow_any_instance_of(described_class).to receive(:find_user_from_warden).and_return(session_user) - expect(subject.user).to eq session_user + expect(subject.user([:api])).to eq session_user end it 'returns nil if no user found' do - expect(subject.user).to be_blank + expect(subject.user([:api])).to be_blank end it 'bubbles up exceptions' do @@ -42,26 +42,26 @@ describe Gitlab::Auth::RequestAuthenticator do let!(:feed_token_user) { build(:user) } it 'returns access_token user first' do - allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_return(access_token_user) + allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_return(access_token_user) allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user) - expect(subject.find_sessionless_user).to eq access_token_user + expect(subject.find_sessionless_user([:api])).to eq access_token_user end it 'returns feed_token user if no access_token user found' do allow_any_instance_of(described_class).to receive(:find_user_from_feed_token).and_return(feed_token_user) - expect(subject.find_sessionless_user).to eq feed_token_user + expect(subject.find_sessionless_user([:api])).to eq feed_token_user end it 'returns nil if no user found' do - expect(subject.find_sessionless_user).to be_blank + expect(subject.find_sessionless_user([:api])).to be_blank end it 'rescue Gitlab::Auth::AuthenticationError exceptions' do - allow_any_instance_of(described_class).to receive(:find_user_from_access_token).and_raise(Gitlab::Auth::UnauthorizedError) + allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_raise(Gitlab::Auth::UnauthorizedError) - expect(subject.find_sessionless_user).to be_blank + expect(subject.find_sessionless_user([:api])).to be_blank end end end diff --git a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb index 76f49e778fb..3620e1afe25 100644 --- a/spec/lib/gitlab/auth/saml/auth_hash_spec.rb +++ b/spec/lib/gitlab/auth/saml/auth_hash_spec.rb @@ -82,6 +82,17 @@ describe Gitlab::Auth::Saml::AuthHash do end end + context 'with SAML 2.0 response_object' do + before do + auth_hash_data[:extra][:response_object] = { document: + saml_xml(File.read('spec/fixtures/authentication/saml2_response.xml')) } + end + + it 'can extract authn_context' do + expect(saml_auth_hash.authn_context).to eq 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport' + end + end + context 'without response_object' do it 'returns an empty string' do expect(saml_auth_hash.authn_context).to be_nil diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb index 454ad1589b9..4e4c8b215c2 100644 --- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Auth::UserAuthFinders do 'rack.input' => '' } end - let(:request) { Rack::Request.new(env)} + let(:request) { Rack::Request.new(env) } def set_param(key, value) request.update_param(key, value) @@ -49,6 +49,7 @@ describe Gitlab::Auth::UserAuthFinders do describe '#find_user_from_feed_token' do context 'when the request format is atom' do before do + env['SCRIPT_NAME'] = 'url.atom' env['HTTP_ACCEPT'] = 'application/atom+xml' end @@ -56,17 +57,17 @@ describe Gitlab::Auth::UserAuthFinders do it 'returns user if valid feed_token' do set_param(:feed_token, user.feed_token) - expect(find_user_from_feed_token).to eq user + expect(find_user_from_feed_token(:rss)).to eq user end it 'returns nil if feed_token is blank' do - expect(find_user_from_feed_token).to be_nil + expect(find_user_from_feed_token(:rss)).to be_nil end it 'returns exception if invalid feed_token' do set_param(:feed_token, 'invalid_token') - expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) end end @@ -74,34 +75,38 @@ describe Gitlab::Auth::UserAuthFinders do it 'returns user if valid rssd_token' do set_param(:rss_token, user.feed_token) - expect(find_user_from_feed_token).to eq user + expect(find_user_from_feed_token(:rss)).to eq user end it 'returns nil if rss_token is blank' do - expect(find_user_from_feed_token).to be_nil + expect(find_user_from_feed_token(:rss)).to be_nil end it 'returns exception if invalid rss_token' do set_param(:rss_token, 'invalid_token') - expect { find_user_from_feed_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) end end end context 'when the request format is not atom' do it 'returns nil' do + env['SCRIPT_NAME'] = 'json' + set_param(:feed_token, user.feed_token) - expect(find_user_from_feed_token).to be_nil + expect(find_user_from_feed_token(:rss)).to be_nil end end context 'when the request format is empty' do it 'the method call does not modify the original value' do + env['SCRIPT_NAME'] = 'url.atom' + env.delete('action_dispatch.request.formats') - find_user_from_feed_token + find_user_from_feed_token(:rss) expect(env['action_dispatch.request.formats']).to be_nil end @@ -111,8 +116,12 @@ describe Gitlab::Auth::UserAuthFinders do describe '#find_user_from_access_token' do let(:personal_access_token) { create(:personal_access_token, user: user) } + before do + env['SCRIPT_NAME'] = 'url.atom' + end + it 'returns nil if no access_token present' do - expect(find_personal_access_token).to be_nil + expect(find_user_from_access_token).to be_nil end context 'when validate_access_token! returns valid' do @@ -131,9 +140,59 @@ describe Gitlab::Auth::UserAuthFinders do end end + describe '#find_user_from_web_access_token' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + + before do + env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token + end + + it 'returns exception if token has no user' do + allow_any_instance_of(PersonalAccessToken).to receive(:user).and_return(nil) + + expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + + context 'no feed or API requests' do + it 'returns nil if the request is not RSS' do + expect(find_user_from_web_access_token(:rss)).to be_nil + end + + it 'returns nil if the request is not ICS' do + expect(find_user_from_web_access_token(:ics)).to be_nil + end + + it 'returns nil if the request is not API' do + expect(find_user_from_web_access_token(:api)).to be_nil + end + end + + it 'returns the user for RSS requests' do + env['SCRIPT_NAME'] = 'url.atom' + + expect(find_user_from_web_access_token(:rss)).to eq(user) + end + + it 'returns the user for ICS requests' do + env['SCRIPT_NAME'] = 'url.ics' + + expect(find_user_from_web_access_token(:ics)).to eq(user) + end + + it 'returns the user for API requests' do + env['SCRIPT_NAME'] = '/api/endpoint' + + expect(find_user_from_web_access_token(:api)).to eq(user) + end + end + describe '#find_personal_access_token' do let(:personal_access_token) { create(:personal_access_token, user: user) } + before do + env['SCRIPT_NAME'] = 'url.atom' + end + context 'passed as header' do it 'returns token if valid personal_access_token' do env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token @@ -220,5 +279,20 @@ describe Gitlab::Auth::UserAuthFinders do expect { validate_access_token!(scopes: [:sudo]) }.to raise_error(Gitlab::Auth::InsufficientScopeError) end end + + context 'with impersonation token' do + let(:personal_access_token) { create(:personal_access_token, :impersonation, user: user) } + + context 'when impersonation is disabled' do + before do + stub_config_setting(impersonation_enabled: false) + allow_any_instance_of(described_class).to receive(:access_token).and_return(personal_access_token) + end + + it 'returns Gitlab::Auth::ImpersonationDisabled' do + expect { validate_access_token! }.to raise_error(Gitlab::Auth::ImpersonationDisabled) + end + end + end end end diff --git a/spec/lib/gitlab/background_migration/backfill_hashed_project_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_hashed_project_repositories_spec.rb new file mode 100644 index 00000000000..b6c1edbbf8b --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_hashed_project_repositories_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::BackfillHashedProjectRepositories, :migration, schema: 20181130102132 do + let(:namespaces) { table(:namespaces) } + let(:project_repositories) { table(:project_repositories) } + let(:projects) { table(:projects) } + let(:shards) { table(:shards) } + let(:group) { namespaces.create!(name: 'foo', path: 'foo') } + let(:shard) { shards.create!(name: 'default') } + + describe described_class::ShardFinder do + describe '#find_shard_id' do + it 'creates a new shard when it does not exist yet' do + expect { subject.find_shard_id('other') }.to change(shards, :count).by(1) + end + + it 'returns the shard when it exists' do + shards.create(id: 5, name: 'other') + + shard_id = subject.find_shard_id('other') + + expect(shard_id).to eq(5) + end + + it 'only queries the database once to retrieve shards' do + subject.find_shard_id('default') + + expect { subject.find_shard_id('default') }.not_to exceed_query_limit(0) + end + end + end + + describe described_class::Project do + describe '.on_hashed_storage' do + it 'finds projects with repository on hashed storage' do + projects.create!(id: 1, name: 'foo', path: 'foo', namespace_id: group.id, storage_version: 1) + projects.create!(id: 2, name: 'bar', path: 'bar', namespace_id: group.id, storage_version: 2) + projects.create!(id: 3, name: 'baz', path: 'baz', namespace_id: group.id, storage_version: 0) + projects.create!(id: 4, name: 'zoo', path: 'zoo', namespace_id: group.id, storage_version: nil) + + expect(described_class.on_hashed_storage.pluck(:id)).to match_array([1, 2]) + end + end + + describe '.without_project_repository' do + it 'finds projects which do not have a projects_repositories entry' do + projects.create!(id: 1, name: 'foo', path: 'foo', namespace_id: group.id) + projects.create!(id: 2, name: 'bar', path: 'bar', namespace_id: group.id) + project_repositories.create!(project_id: 2, disk_path: '@phony/foo/bar', shard_id: shard.id) + + expect(described_class.without_project_repository.pluck(:id)).to contain_exactly(1) + end + end + end + + describe '#perform' do + it 'creates a project_repository row for projects on hashed storage that need one' do + projects.create!(id: 1, name: 'foo', path: 'foo', namespace_id: group.id, storage_version: 1) + projects.create!(id: 2, name: 'bar', path: 'bar', namespace_id: group.id, storage_version: 2) + + expect { described_class.new.perform(1, projects.last.id) }.to change(project_repositories, :count).by(2) + end + + it 'does nothing for projects on hashed storage that have already a project_repository row' do + projects.create!(id: 1, name: 'foo', path: 'foo', namespace_id: group.id, storage_version: 1) + project_repositories.create!(project_id: 1, disk_path: '@phony/foo/bar', shard_id: shard.id) + + expect { described_class.new.perform(1, projects.last.id) }.not_to change(project_repositories, :count) + end + + it 'does nothing for projects on legacy storage' do + projects.create!(name: 'foo', path: 'foo', namespace_id: group.id, storage_version: 0) + + expect { described_class.new.perform(1, projects.last.id) }.not_to change(project_repositories, :count) + end + + it 'inserts rows in a single query' do + projects.create!(name: 'foo', path: 'foo', namespace_id: group.id, storage_version: 1, repository_storage: shard.name) + + control_count = ActiveRecord::QueryRecorder.new { described_class.new.perform(1, projects.last.id) } + + projects.create!(name: 'bar', path: 'bar', namespace_id: group.id, storage_version: 1, repository_storage: shard.name) + projects.create!(name: 'zoo', path: 'zoo', namespace_id: group.id, storage_version: 1, repository_storage: shard.name) + + expect { described_class.new.perform(1, projects.last.id) }.not_to exceed_query_limit(control_count) + end + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb new file mode 100644 index 00000000000..c66d7cd6148 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig, :migration, schema: 20181010133639 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:group) { namespaces.create!(name: 'foo', path: 'foo') } + let(:subgroup) { namespaces.create!(name: 'bar', path: 'bar', parent_id: group.id) } + + describe described_class::Storage::HashedProject do + let(:project) { double(id: 555) } + subject(:project_storage) { described_class.new(project) } + + it 'has the correct disk_path' do + expect(project_storage.disk_path).to eq('@hashed/91/a7/91a73fd806ab2c005c13b4dc19130a884e909dea3f72d46e30266fe1a1f588d8') + end + end + + describe described_class::Storage::LegacyProject do + let(:project) { double(full_path: 'this/is/the/full/path') } + subject(:project_storage) { described_class.new(project) } + + it 'has the correct disk_path' do + expect(project_storage.disk_path).to eq('this/is/the/full/path') + end + end + + describe described_class::Project do + let(:project_record) { projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') } + subject(:project) { described_class.find(project_record.id) } + + describe '#full_path' do + it 'returns path containing all parent namespaces' do + expect(project.full_path).to eq('foo/bar/baz') + end + + it 'raises OrphanedNamespaceError when any parent namespace does not exist' do + subgroup.update_attribute(:parent_id, namespaces.maximum(:id).succ) + + expect { project.full_path }.to raise_error(Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig::OrphanedNamespaceError) + end + end + end + + describe described_class::Up do + describe '#perform' do + subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) } + + it 'asks the gitaly client to set config' do + projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') + projects.create!(namespace_id: subgroup.id, name: 'buzz', path: 'buzz', storage_version: 1) + + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| + allow(repository_service).to receive(:cleanup) + expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/baz') + end + + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| + allow(repository_service).to receive(:cleanup) + expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/buzz') + end + + migrate + end + end + end + + describe described_class::Down do + describe '#perform' do + subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) } + + it 'asks the gitaly client to set config' do + projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') + + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| + allow(repository_service).to receive(:cleanup) + expect(repository_service).to receive(:delete_config).with(['gitlab.fullpath']) + end + + migrate + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/encrypt_columns_spec.rb b/spec/lib/gitlab/background_migration/encrypt_columns_spec.rb index 2a869446753..1d9bac79dcd 100644 --- a/spec/lib/gitlab/background_migration/encrypt_columns_spec.rb +++ b/spec/lib/gitlab/background_migration/encrypt_columns_spec.rb @@ -65,5 +65,30 @@ describe Gitlab::BackgroundMigration::EncryptColumns, :migration, schema: 201809 expect(hook).to have_attributes(values) end + + it 'reloads the model column information' do + expect(model).to receive(:reset_column_information).and_call_original + expect(model).to receive(:define_attribute_methods).and_call_original + + subject.perform(model, [:token, :url], 1, 1) + end + + it 'fails if a source column is not present' do + columns = model.columns.reject { |c| c.name == 'url' } + allow(model).to receive(:columns) { columns } + + expect do + subject.perform(model, [:token, :url], 1, 1) + end.to raise_error(/source column: url is missing/) + end + + it 'fails if a destination column is not present' do + columns = model.columns.reject { |c| c.name == 'encrypted_url' } + allow(model).to receive(:columns) { columns } + + expect do + subject.perform(model, [:token, :url], 1, 1) + end.to raise_error(/destination column: encrypted_url is missing/) + end end end diff --git a/spec/lib/gitlab/background_migration/encrypt_runners_tokens_spec.rb b/spec/lib/gitlab/background_migration/encrypt_runners_tokens_spec.rb new file mode 100644 index 00000000000..9d4921968b3 --- /dev/null +++ b/spec/lib/gitlab/background_migration/encrypt_runners_tokens_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::EncryptRunnersTokens, :migration, schema: 20181121111200 do + let(:settings) { table(:application_settings) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:runners) { table(:ci_runners) } + + context 'when migrating application settings' do + before do + settings.create!(id: 1, runners_registration_token: 'plain-text-token1') + end + + it 'migrates runners registration tokens' do + migrate!(:settings, 1, 1) + + encrypted_token = settings.first.runners_registration_token_encrypted + decrypted_token = ::Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_token) + + expect(decrypted_token).to eq 'plain-text-token1' + expect(settings.first.runners_registration_token).to eq 'plain-text-token1' + end + end + + context 'when migrating namespaces' do + before do + namespaces.create!(id: 11, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token1') + namespaces.create!(id: 12, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token2') + namespaces.create!(id: 22, name: 'gitlab', path: 'gitlab-org', runners_token: 'my-token3') + end + + it 'migrates runners registration tokens' do + migrate!(:namespace, 11, 22) + + expect(namespaces.all.reload).to all( + have_attributes(runners_token: be_a(String), runners_token_encrypted: be_a(String)) + ) + end + end + + context 'when migrating projects' do + before do + namespaces.create!(id: 11, name: 'gitlab', path: 'gitlab-org') + projects.create!(id: 111, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token1') + projects.create!(id: 114, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token2') + projects.create!(id: 116, namespace_id: 11, name: 'gitlab', path: 'gitlab-ce', runners_token: 'my-token3') + end + + it 'migrates runners registration tokens' do + migrate!(:project, 111, 116) + + expect(projects.all.reload).to all( + have_attributes(runners_token: be_a(String), runners_token_encrypted: be_a(String)) + ) + end + end + + context 'when migrating runners' do + before do + runners.create!(id: 201, runner_type: 1, token: 'plain-text-token1') + runners.create!(id: 202, runner_type: 1, token: 'plain-text-token2') + runners.create!(id: 203, runner_type: 1, token: 'plain-text-token3') + end + + it 'migrates runners communication tokens' do + migrate!(:runner, 201, 203) + + expect(runners.all.reload).to all( + have_attributes(token: be_a(String), token_encrypted: be_a(String)) + ) + end + end + + def migrate!(model, from, to) + subject.perform(model, from, to) + end +end diff --git a/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_improved_spec.rb b/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_improved_spec.rb new file mode 100644 index 00000000000..d1d64574627 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_improved_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Gitlab::BackgroundMigration::PopulateMergeRequestMetricsWithEventsDataImproved, :migration, schema: 20181204154019 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:users) { table(:users) } + let(:events) { table(:events) } + + let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') } + + 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, params = {}) + params.merge!(id: id, + target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: 'mr name', + title: "mr name#{id}") + + merge_requests.create(params) + end + + def create_merge_request_event(id, params = {}) + params.merge!(id: id, + project_id: project.id, + author_id: user.id, + target_type: 'MergeRequest') + + events.create(params) + end + + describe '#perform' do + it 'creates and updates closed and merged events' do + timestamp = Time.new('2018-01-01 12:00:00').utc + + create_merge_request(1) + create_merge_request_event(1, target_id: 1, action: 3, updated_at: timestamp) + create_merge_request_event(2, target_id: 1, action: 3, updated_at: timestamp + 10.seconds) + + create_merge_request_event(3, target_id: 1, action: 7, updated_at: timestamp) + create_merge_request_event(4, target_id: 1, action: 7, updated_at: timestamp + 10.seconds) + + subject.perform(1, 1) + + merge_request = MergeRequest.first + + expect(merge_request.metrics).to have_attributes(latest_closed_by_id: user.id, + latest_closed_at: timestamp + 10.seconds, + merged_by_id: user.id) + end + end +end diff --git a/spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb b/spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb new file mode 100644 index 00000000000..1e969542975 --- /dev/null +++ b/spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BranchPushMergeCommitAnalyzer do + let(:project) { create(:project, :repository) } + let(:oldrev) { 'merge-commit-analyze-before' } + let(:newrev) { 'merge-commit-analyze-after' } + let(:commits) { project.repository.commits_between(oldrev, newrev).reverse } + + subject { described_class.new(commits) } + + describe '#get_merge_commit' do + let(:expected_merge_commits) do + { + '646ece5cfed840eca0a4feb21bcd6a81bb19bda3' => '646ece5cfed840eca0a4feb21bcd6a81bb19bda3', + '29284d9bcc350bcae005872d0be6edd016e2efb5' => '29284d9bcc350bcae005872d0be6edd016e2efb5', + '5f82584f0a907f3b30cfce5bb8df371454a90051' => '29284d9bcc350bcae005872d0be6edd016e2efb5', + '8a994512e8c8f0dfcf22bb16df6e876be7a61036' => '29284d9bcc350bcae005872d0be6edd016e2efb5', + '689600b91aabec706e657e38ea706ece1ee8268f' => '29284d9bcc350bcae005872d0be6edd016e2efb5', + 'db46a1c5a5e474aa169b6cdb7a522d891bc4c5f9' => 'db46a1c5a5e474aa169b6cdb7a522d891bc4c5f9' + } + end + + it 'returns correct merge commit SHA for each commit' do + expected_merge_commits.each do |commit, merge_commit| + expect(subject.get_merge_commit(commit)).to eq(merge_commit) + end + end + + context 'when one parent has two children' do + let(:oldrev) { '1adbdefe31288f3bbe4b614853de4908a0b6f792' } + let(:newrev) { '5f82584f0a907f3b30cfce5bb8df371454a90051' } + + let(:expected_merge_commits) do + { + '5f82584f0a907f3b30cfce5bb8df371454a90051' => '5f82584f0a907f3b30cfce5bb8df371454a90051', + '8a994512e8c8f0dfcf22bb16df6e876be7a61036' => '5f82584f0a907f3b30cfce5bb8df371454a90051', + '689600b91aabec706e657e38ea706ece1ee8268f' => '689600b91aabec706e657e38ea706ece1ee8268f' + } + end + + it 'returns correct merge commit SHA for each commit' do + expected_merge_commits.each do |commit, merge_commit| + expect(subject.get_merge_commit(commit)).to eq(merge_commit) + end + end + end + + context 'when relevant_commit_ids is provided' do + let(:relevant_commit_id) { '8a994512e8c8f0dfcf22bb16df6e876be7a61036' } + subject { described_class.new(commits, relevant_commit_ids: [relevant_commit_id]) } + + it 'returns correct merge commit' do + expected_merge_commits.each do |commit, merge_commit| + subject = described_class.new(commits, relevant_commit_ids: [commit]) + expect(subject.get_merge_commit(commit)).to eq(merge_commit) + end + end + end + end +end diff --git a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb index e5999a1c509..be9b2588c90 100644 --- a/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb +++ b/spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do let!(:project) { create(:project, :repository) } let(:pipeline_status) { described_class.new(project) } - let(:cache_key) { described_class.cache_key_for_project(project) } + let(:cache_key) { pipeline_status.cache_key } describe '.load_for_project' do it "loads the status" do @@ -14,94 +14,24 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do end describe 'loading in batches' do - let(:status) { 'success' } - let(:sha) { '424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6' } - let(:ref) { 'master' } - let(:pipeline_info) { { sha: sha, status: status, ref: ref } } - let!(:project_without_status) { create(:project, :repository) } - describe '.load_in_batch_for_projects' do - it 'preloads pipeline_status on projects' do + it 'loads pipeline_status on projects' do described_class.load_in_batch_for_projects([project]) # Don't call the accessor that would lazy load the variable - expect(project.instance_variable_get('@pipeline_status')).to be_a(described_class) - end - - describe 'without a status in redis_cache' do - it 'loads the status from a commit when it was not in redis_cache' do - empty_status = { sha: nil, status: nil, ref: nil } - fake_pipeline = described_class.new( - project_without_status, - pipeline_info: empty_status, - loaded_from_cache: false - ) - - expect(described_class).to receive(:new) - .with(project_without_status, - pipeline_info: empty_status, - loaded_from_cache: false) - .and_return(fake_pipeline) - expect(fake_pipeline).to receive(:load_from_project) - expect(fake_pipeline).to receive(:store_in_cache) - - described_class.load_in_batch_for_projects([project_without_status]) - end - - it 'only connects to redis twice' do - expect(Gitlab::Redis::Cache).to receive(:with).exactly(2).and_call_original - - described_class.load_in_batch_for_projects([project_without_status]) - - expect(project_without_status.pipeline_status).not_to be_nil - end - end - - describe 'when a status was cached in redis_cache' do - before do - Gitlab::Redis::Cache.with do |redis| - redis.mapped_hmset(cache_key, - { sha: sha, status: status, ref: ref }) - end - end - - it 'loads the correct status' do - described_class.load_in_batch_for_projects([project]) - - pipeline_status = project.instance_variable_get('@pipeline_status') - - expect(pipeline_status.sha).to eq(sha) - expect(pipeline_status.status).to eq(status) - expect(pipeline_status.ref).to eq(ref) - end - - it 'only connects to redis_cache once' do - expect(Gitlab::Redis::Cache).to receive(:with).exactly(1).and_call_original + project_pipeline_status = project.instance_variable_get('@pipeline_status') - described_class.load_in_batch_for_projects([project]) - - expect(project.pipeline_status).not_to be_nil - end - - it "doesn't load the status separatly" do - expect_any_instance_of(described_class).not_to receive(:load_from_project) - expect_any_instance_of(described_class).not_to receive(:load_from_cache) - - described_class.load_in_batch_for_projects([project]) - end + expect(project_pipeline_status).to be_a(described_class) + expect(project_pipeline_status).to be_loaded end - end - describe '.cached_results_for_projects' do - it 'loads a status from caching for all projects' do - Gitlab::Redis::Cache.with do |redis| - redis.mapped_hmset(cache_key, { sha: sha, status: status, ref: ref }) + it 'loads 10 projects without hitting Gitaly call limit', :request_store do + projects = Gitlab::GitalyClient.allow_n_plus_1_calls do + (1..10).map { create(:project, :repository) } end + Gitlab::GitalyClient.reset_counts - result = [{ loaded_from_cache: false, pipeline_info: { sha: nil, status: nil, ref: nil } }, - { loaded_from_cache: true, pipeline_info: pipeline_info }] - - expect(described_class.cached_results_for_projects([project_without_status, project])).to eq(result) + expect { described_class.load_in_batch_for_projects(projects) }.not_to raise_error end end end @@ -198,7 +128,9 @@ describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do status_for_empty_commit.load_status - expect(status_for_empty_commit).to be_loaded + expect(status_for_empty_commit.sha).to be_nil + expect(status_for_empty_commit.status).to be_nil + expect(status_for_empty_commit.ref).to be_nil end end diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb new file mode 100644 index 00000000000..77366e91dca --- /dev/null +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::BranchCheck do + include_context 'change access checks context' + + describe '#validate!' do + it 'does not raise any error' do + expect { subject.validate! }.not_to raise_error + end + + context 'trying to delete the default branch' do + let(:newrev) { '0000000000000000000000000000000000000000' } + let(:ref) { 'refs/heads/master' } + + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'The default branch of a project cannot be deleted.') + end + end + + context 'protected branches check' do + before do + allow(ProtectedBranch).to receive(:protected?).with(project, 'master').and_return(true) + allow(ProtectedBranch).to receive(:protected?).with(project, 'feature').and_return(true) + end + + it 'raises an error if the user is not allowed to do forced pushes to protected branches' do + expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true) + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to force push code to a protected branch on this project.') + end + + it 'raises an error if the user is not allowed to merge to protected branches' do + expect_any_instance_of(Gitlab::Checks::MatchingMergeRequest).to receive(:match?).and_return(true) + expect(user_access).to receive(:can_merge_to_branch?).and_return(false) + expect(user_access).to receive(:can_push_to_branch?).and_return(false) + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to merge code into protected branches on this project.') + end + + it 'raises an error if the user is not allowed to push to protected branches' do + expect(user_access).to receive(:can_push_to_branch?).and_return(false) + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.') + end + + context 'when project repository is empty' do + let(:project) { create(:project) } + + it 'raises an error if the user is not allowed to push to protected branches' do + expect(user_access).to receive(:can_push_to_branch?).and_return(false) + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /Ask a project Owner or Maintainer to create a default branch/) + end + end + + context 'branch deletion' do + let(:newrev) { '0000000000000000000000000000000000000000' } + let(:ref) { 'refs/heads/feature' } + + context 'if the user is not allowed to delete protected branches' do + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.') + end + end + + context 'if the user is allowed to delete protected branches' do + before do + project.add_maintainer(user) + end + + context 'through the web interface' do + let(:protocol) { 'web' } + + it 'allows branch deletion' do + expect { subject.validate! }.not_to raise_error + end + end + + context 'over SSH or HTTP' do + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only delete protected branches using the web interface.') + end + end + end + end + end + end +end diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index 81804ba5c76..45fb33e9e4a 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -2,245 +2,56 @@ require 'spec_helper' describe Gitlab::Checks::ChangeAccess do describe '#exec' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:user_access) { Gitlab::UserAccess.new(user, project: project) } - let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' } - let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } - let(:ref) { 'refs/heads/master' } - let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } } - let(:protocol) { 'ssh' } - let(:timeout) { Gitlab::GitAccess::INTERNAL_TIMEOUT } - let(:logger) { Gitlab::Checks::TimedLogger.new(timeout: timeout) } + include_context 'change access checks context' - subject(:change_access) do - described_class.new( - changes, - project: project, - user_access: user_access, - protocol: protocol, - logger: logger - ) - end - - before do - project.add_developer(user) - end + subject { change_access } context 'without failed checks' do it "doesn't raise an error" do expect { subject.exec }.not_to raise_error end - end - context 'when time limit was reached' do - it 'raises a TimeoutError' do - logger = Gitlab::Checks::TimedLogger.new(start_time: timeout.ago, timeout: timeout) - access = described_class.new(changes, - project: project, - user_access: user_access, - protocol: protocol, - logger: logger) + it 'calls pushes checks' do + expect_any_instance_of(Gitlab::Checks::PushCheck).to receive(:validate!) - expect { access.exec }.to raise_error(Gitlab::Checks::TimedLogger::TimeoutError) + subject.exec end - end - context 'when the user is not allowed to push to the repo' do - it 'raises an error' do - expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) - expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false) + it 'calls branches checks' do + expect_any_instance_of(Gitlab::Checks::BranchCheck).to receive(:validate!) - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') + subject.exec end - end - context 'tags check' do - let(:ref) { 'refs/tags/v1.0.0' } + it 'calls tags checks' do + expect_any_instance_of(Gitlab::Checks::TagCheck).to receive(:validate!) - it 'raises an error if the user is not allowed to update tags' do - allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true) - expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false) - - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to change existing tags on this project.') + subject.exec end - context 'with protected tag' do - let!(:protected_tag) { create(:protected_tag, project: project, name: 'v*') } - - context 'as maintainer' do - before do - project.add_maintainer(user) - end + it 'calls lfs checks' do + expect_any_instance_of(Gitlab::Checks::LfsCheck).to receive(:validate!) - context 'deletion' do - let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' } - let(:newrev) { '0000000000000000000000000000000000000000' } - - it 'is prevented' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be deleted/) - end - end - - context 'update' do - let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' } - let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } - - it 'is prevented' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be updated/) - end - end - end - - context 'creation' do - let(:oldrev) { '0000000000000000000000000000000000000000' } - let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } - let(:ref) { 'refs/tags/v9.1.0' } - - it 'prevents creation below access level' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /allowed to create this tag as it is protected/) - end - - context 'when user has access' do - let!(:protected_tag) { create(:protected_tag, :developers_can_create, project: project, name: 'v*') } - - it 'allows tag creation' do - expect { subject.exec }.not_to raise_error - end - end - end + subject.exec end - end - context 'branches check' do - context 'trying to delete the default branch' do - let(:newrev) { '0000000000000000000000000000000000000000' } - let(:ref) { 'refs/heads/master' } + it 'calls diff checks' do + expect_any_instance_of(Gitlab::Checks::DiffCheck).to receive(:validate!) - it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'The default branch of a project cannot be deleted.') - end - end - - context 'protected branches check' do - before do - allow(ProtectedBranch).to receive(:protected?).with(project, 'master').and_return(true) - allow(ProtectedBranch).to receive(:protected?).with(project, 'feature').and_return(true) - end - - it 'raises an error if the user is not allowed to do forced pushes to protected branches' do - expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true) - - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to force push code to a protected branch on this project.') - end - - it 'raises an error if the user is not allowed to merge to protected branches' do - expect_any_instance_of(Gitlab::Checks::MatchingMergeRequest).to receive(:match?).and_return(true) - expect(user_access).to receive(:can_merge_to_branch?).and_return(false) - expect(user_access).to receive(:can_push_to_branch?).and_return(false) - - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to merge code into protected branches on this project.') - end - - it 'raises an error if the user is not allowed to push to protected branches' do - expect(user_access).to receive(:can_push_to_branch?).and_return(false) - - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.') - end - - context 'when project repository is empty' do - let(:project) { create(:project) } - - it 'raises an error if the user is not allowed to push to protected branches' do - expect(user_access).to receive(:can_push_to_branch?).and_return(false) - - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /Ask a project Owner or Maintainer to create a default branch/) - end - end - - context 'branch deletion' do - let(:newrev) { '0000000000000000000000000000000000000000' } - let(:ref) { 'refs/heads/feature' } - - context 'if the user is not allowed to delete protected branches' do - it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to delete protected branches from this project. Only a project maintainer or owner can delete a protected branch.') - end - end - - context 'if the user is allowed to delete protected branches' do - before do - project.add_maintainer(user) - end - - context 'through the web interface' do - let(:protocol) { 'web' } - - it 'allows branch deletion' do - expect { subject.exec }.not_to raise_error - end - end - - context 'over SSH or HTTP' do - it 'raises an error' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only delete protected branches using the web interface.') - end - end - end - end + subject.exec end end - context 'LFS integrity check' do - it 'fails if any LFS blobs are missing' do - allow_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).and_return(true) - - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/) - end - - it 'succeeds if LFS objects have already been uploaded' do - allow_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).and_return(false) - - expect { subject.exec }.not_to raise_error - end - end - - context 'LFS file lock check' do - let(:owner) { create(:user) } - let!(:lock) { create(:lfs_file_lock, user: owner, project: project, path: 'README') } - - before do - allow(project.repository).to receive(:new_commits).and_return( - project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51') - ) - end - - context 'with LFS not enabled' do - it 'skips the validation' do - expect_any_instance_of(Gitlab::Checks::CommitCheck).not_to receive(:validate) - - subject.exec - end - end - - context 'with LFS enabled' do - before do - allow(project).to receive(:lfs_enabled?).and_return(true) - end - - context 'when change is sent by a different user' do - it 'raises an error if the user is not allowed to update the file' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked in Git LFS by #{lock.user.name}") - end - end - - context 'when change is sent by the author of the lock' do - let(:user) { owner } + context 'when time limit was reached' do + it 'raises a TimeoutError' do + logger = Gitlab::Checks::TimedLogger.new(start_time: timeout.ago, timeout: timeout) + access = described_class.new(changes, + project: project, + user_access: user_access, + protocol: protocol, + logger: logger) - it "doesn't raise any error" do - expect { subject.exec }.not_to raise_error - end - end + expect { access.exec }.to raise_error(Gitlab::Checks::TimedLogger::TimeoutError) end end end diff --git a/spec/lib/gitlab/checks/diff_check_spec.rb b/spec/lib/gitlab/checks/diff_check_spec.rb new file mode 100644 index 00000000000..eeec1e83179 --- /dev/null +++ b/spec/lib/gitlab/checks/diff_check_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::DiffCheck do + include_context 'change access checks context' + + describe '#validate!' do + let(:owner) { create(:user) } + let!(:lock) { create(:lfs_file_lock, user: owner, project: project, path: 'README') } + + before do + allow(project.repository).to receive(:new_commits).and_return( + project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51') + ) + end + + context 'with LFS not enabled' do + before do + allow(project).to receive(:lfs_enabled?).and_return(false) + end + + it 'skips the validation' do + expect(subject).not_to receive(:validate_diff) + expect(subject).not_to receive(:validate_file_paths) + + subject.validate! + end + end + + context 'with LFS enabled' do + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + context 'when change is sent by a different user' do + it 'raises an error if the user is not allowed to update the file' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "The path 'README' is locked in Git LFS by #{lock.user.name}") + end + end + + context 'when change is sent by the author of the lock' do + let(:user) { owner } + + it "doesn't raise any error" do + expect { subject.validate! }.not_to raise_error + end + end + end + end +end diff --git a/spec/lib/gitlab/checks/lfs_check_spec.rb b/spec/lib/gitlab/checks/lfs_check_spec.rb new file mode 100644 index 00000000000..35f8069c8a4 --- /dev/null +++ b/spec/lib/gitlab/checks/lfs_check_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::LfsCheck do + include_context 'change access checks context' + + let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') } + + before do + allow_any_instance_of(Gitlab::Git::LfsChanges).to receive(:new_pointers) do + [blob_object] + end + end + + describe '#validate!' do + context 'with LFS not enabled' do + it 'skips integrity check' do + expect_any_instance_of(Gitlab::Git::LfsChanges).not_to receive(:new_pointers) + + subject.validate! + end + end + + context 'with LFS enabled' do + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + context 'deletion' do + let(:changes) { { oldrev: oldrev, ref: ref } } + + it 'skips integrity check' do + expect(project.repository).not_to receive(:new_objects) + + subject.validate! + end + end + + it 'fails if any LFS blobs are missing' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/) + end + + it 'succeeds if LFS objects have already been uploaded' do + lfs_object = create(:lfs_object, oid: blob_object.lfs_oid) + create(:lfs_objects_project, project: project, lfs_object: lfs_object) + + expect { subject.validate! }.not_to raise_error + end + end + end +end diff --git a/spec/lib/gitlab/checks/push_check_spec.rb b/spec/lib/gitlab/checks/push_check_spec.rb new file mode 100644 index 00000000000..25f0d428cb9 --- /dev/null +++ b/spec/lib/gitlab/checks/push_check_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::PushCheck do + include_context 'change access checks context' + + describe '#validate!' do + it 'does not raise any error' do + expect { subject.validate! }.not_to raise_error + end + + context 'when the user is not allowed to push to the repo' do + it 'raises an error' do + expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) + expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false) + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') + end + end + end +end diff --git a/spec/lib/gitlab/checks/tag_check_spec.rb b/spec/lib/gitlab/checks/tag_check_spec.rb new file mode 100644 index 00000000000..b1258270611 --- /dev/null +++ b/spec/lib/gitlab/checks/tag_check_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::TagCheck do + include_context 'change access checks context' + + describe '#validate!' do + let(:ref) { 'refs/tags/v1.0.0' } + + it 'raises an error' do + allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true) + expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false) + + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to change existing tags on this project.') + end + + context 'with protected tag' do + let!(:protected_tag) { create(:protected_tag, project: project, name: 'v*') } + + context 'as maintainer' do + before do + project.add_maintainer(user) + end + + context 'deletion' do + let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' } + let(:newrev) { '0000000000000000000000000000000000000000' } + + it 'is prevented' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be deleted/) + end + end + + context 'update' do + let(:oldrev) { 'be93687618e4b132087f430a4d8fc3a609c9b77c' } + let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } + + it 'is prevented' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /cannot be updated/) + end + end + end + + context 'creation' do + let(:oldrev) { '0000000000000000000000000000000000000000' } + let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } + let(:ref) { 'refs/tags/v9.1.0' } + + it 'prevents creation below access level' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /allowed to create this tag as it is protected/) + end + + context 'when user has access' do + let!(:protected_tag) { create(:protected_tag, :developers_can_create, project: project, name: 'v*') } + + it 'allows tag creation' do + expect { subject.validate! }.not_to raise_error + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/policy/changes_spec.rb b/spec/lib/gitlab/ci/build/policy/changes_spec.rb index ab401108c84..5fee37bb43e 100644 --- a/spec/lib/gitlab/ci/build/policy/changes_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/changes_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Ci::Build::Policy::Changes do set(:project) { create(:project) } describe '#satisfied_by?' do - describe 'paths matching matching' do + describe 'paths matching' do let(:pipeline) do build(:ci_empty_pipeline, project: project, ref: 'master', @@ -49,6 +49,12 @@ describe Gitlab::Ci::Build::Policy::Changes do expect(policy).to be_satisfied_by(pipeline, seed) end + it 'is satisfied by matching a pattern with a glob' do + policy = described_class.new(%w[some/**/*.{rb,txt}]) + + expect(policy).to be_satisfied_by(pipeline, seed) + end + it 'is not satisfied when pattern does not match path' do policy = described_class.new(%w[some/*.rb]) @@ -61,6 +67,12 @@ describe Gitlab::Ci::Build::Policy::Changes do expect(policy).not_to be_satisfied_by(pipeline, seed) end + it 'is not satified when pattern with glob does not match' do + policy = described_class.new(%w[invalid/*.{md,rake}]) + + expect(policy).not_to be_satisfied_by(pipeline, seed) + end + context 'when pipelines does not run for a branch update' do before do pipeline.before_sha = Gitlab::Git::BLANK_SHA diff --git a/spec/lib/gitlab/ci/build/policy/refs_spec.rb b/spec/lib/gitlab/ci/build/policy/refs_spec.rb index 7211187e511..553fc0fb9bf 100644 --- a/spec/lib/gitlab/ci/build/policy/refs_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/refs_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::Ci::Build::Policy::Refs do end end - context 'when maching tags' do + context 'when matching tags' do context 'when pipeline runs for a tag' do let(:pipeline) do build_stubbed(:ci_pipeline, ref: 'feature', tag: true) @@ -56,10 +56,10 @@ describe Gitlab::Ci::Build::Policy::Refs do end end - context 'when maching a source' do + context 'when matching a source' do let(:pipeline) { build_stubbed(:ci_pipeline, source: :push) } - it 'is satisifed when provided source keyword matches' do + it 'is satisfied when provided source keyword matches' do expect(described_class.new(%w[pushes])) .to be_satisfied_by(pipeline) end diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index d48aac15f28..bd1f2c92844 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Ci::Config::Entry::Artifacts do let(:config) { { paths: %w[public/] } } describe '#value' do - it 'returns artifacs configuration' do + it 'returns artifacts configuration' do expect(entry.value).to eq config end end diff --git a/spec/lib/gitlab/ci/config/entry/except_policy_spec.rb b/spec/lib/gitlab/ci/config/entry/except_policy_spec.rb new file mode 100644 index 00000000000..d036bf2f4d1 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/except_policy_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::ExceptPolicy do + let(:entry) { described_class.new(config) } + + it_behaves_like 'correct only except policy' + + describe '.default' do + it 'does not have a default value' do + expect(described_class.default).to be_nil + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb index 7c18514934e..12f4b9dc624 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb @@ -160,7 +160,8 @@ describe Gitlab::Ci::Config::Entry::Global do cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push' }, variables: { 'VAR' => 'value' }, ignore: false, - after_script: ['make clean'] }, + after_script: ['make clean'], + only: { refs: %w[branches tags] } }, spinach: { name: :spinach, before_script: [], script: %w[spinach], @@ -171,7 +172,8 @@ describe Gitlab::Ci::Config::Entry::Global do cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push' }, variables: {}, ignore: false, - after_script: ['make clean'] } + after_script: ['make clean'], + only: { refs: %w[branches tags] } } ) end end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 57d4577a90c..c1f4a060063 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -258,7 +258,8 @@ describe Gitlab::Ci::Config::Entry::Job do commands: "ls\npwd\nrspec", stage: 'test', ignore: false, - after_script: %w[cleanup]) + after_script: %w[cleanup], + only: { refs: %w[branches tags] }) end end end diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb index c0a2b6517e3..2a753408f54 100644 --- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb @@ -67,12 +67,14 @@ describe Gitlab::Ci::Config::Entry::Jobs do script: %w[rspec], commands: 'rspec', ignore: false, - stage: 'test' }, + stage: 'test', + only: { refs: %w[branches tags] } }, spinach: { name: :spinach, script: %w[spinach], commands: 'spinach', ignore: false, - stage: 'test' }) + stage: 'test', + only: { refs: %w[branches tags] } }) end end diff --git a/spec/lib/gitlab/ci/config/entry/only_policy_spec.rb b/spec/lib/gitlab/ci/config/entry/only_policy_spec.rb new file mode 100644 index 00000000000..5518b68e51a --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/only_policy_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::OnlyPolicy do + let(:entry) { described_class.new(config) } + + it_behaves_like 'correct only except policy' + + describe '.default' do + it 'haa a default value' do + expect(described_class.default).to eq( { refs: %w[branches tags] } ) + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index bef93fe7af7..cf40a22af2e 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -1,173 +1,8 @@ -require 'fast_spec_helper' -require_dependency 'active_model' +require 'spec_helper' describe Gitlab::Ci::Config::Entry::Policy do let(:entry) { described_class.new(config) } - context 'when using simplified policy' do - describe 'validations' do - context 'when entry config value is valid' do - context 'when config is a branch or tag name' do - let(:config) { %w[master feature/branch] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - - describe '#value' do - it 'returns refs hash' do - expect(entry.value).to eq(refs: config) - end - end - end - - context 'when config is a regexp' do - let(:config) { ['/^issue-.*$/'] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - end - - context 'when config is a special keyword' do - let(:config) { %w[tags triggers branches] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - end - end - - context 'when entry value is not valid' do - let(:config) { [1] } - - describe '#errors' do - it 'saves errors' do - expect(entry.errors) - .to include /policy config should be an array of strings or regexps/ - end - end - end - end - end - - context 'when using complex policy' do - context 'when specifiying refs policy' do - let(:config) { { refs: ['master'] } } - - it 'is a correct configuraton' do - expect(entry).to be_valid - expect(entry.value).to eq(refs: %w[master]) - end - end - - context 'when specifying kubernetes policy' do - let(:config) { { kubernetes: 'active' } } - - it 'is a correct configuraton' do - expect(entry).to be_valid - expect(entry.value).to eq(kubernetes: 'active') - end - end - - context 'when specifying invalid kubernetes policy' do - let(:config) { { kubernetes: 'something' } } - - it 'reports an error about invalid policy' do - expect(entry.errors).to include /unknown value: something/ - end - end - - context 'when specifying valid variables expressions policy' do - let(:config) { { variables: ['$VAR == null'] } } - - it 'is a correct configuraton' do - expect(entry).to be_valid - expect(entry.value).to eq(config) - end - end - - context 'when specifying variables expressions in invalid format' do - let(:config) { { variables: '$MY_VAR' } } - - it 'reports an error about invalid format' do - expect(entry.errors).to include /should be an array of strings/ - end - end - - context 'when specifying invalid variables expressions statement' do - let(:config) { { variables: ['$MY_VAR =='] } } - - it 'reports an error about invalid statement' do - expect(entry.errors).to include /invalid expression syntax/ - end - end - - context 'when specifying invalid variables expressions token' do - let(:config) { { variables: ['$MY_VAR == 123'] } } - - it 'reports an error about invalid expression' do - expect(entry.errors).to include /invalid expression syntax/ - end - end - - context 'when using invalid variables expressions regexp' do - let(:config) { { variables: ['$MY_VAR =~ /some ( thing/'] } } - - it 'reports an error about invalid expression' do - expect(entry.errors).to include /invalid expression syntax/ - end - end - - context 'when specifying a valid changes policy' do - let(:config) { { changes: %w[some/* paths/**/*.rb] } } - - it 'is a correct configuraton' do - expect(entry).to be_valid - expect(entry.value).to eq(config) - end - end - - context 'when changes policy is invalid' do - let(:config) { { changes: [1, 2] } } - - it 'returns errors' do - expect(entry.errors).to include /changes should be an array of strings/ - end - end - - context 'when specifying unknown policy' do - let(:config) { { refs: ['master'], invalid: :something } } - - it 'returns error about invalid key' do - expect(entry.errors).to include /unknown keys: invalid/ - end - end - - context 'when policy is empty' do - let(:config) { {} } - - it 'is not a valid configuration' do - expect(entry.errors).to include /can't be blank/ - end - end - end - - context 'when policy strategy does not match' do - let(:config) { 'string strategy' } - - it 'returns information about errors' do - expect(entry.errors) - .to include /has to be either an array of conditions or a hash/ - end - end - describe '.default' do it 'does not have a default value' do expect(described_class.default).to be_nil diff --git a/spec/lib/gitlab/ci/config/entry/reports_spec.rb b/spec/lib/gitlab/ci/config/entry/reports_spec.rb index 1140bfdf6c3..38943138cbf 100644 --- a/spec/lib/gitlab/ci/config/entry/reports_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/reports_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::Ci::Config::Entry::Reports do shared_examples 'a valid entry' do |keyword, file| describe '#value' do - it 'returns artifacs configuration' do + it 'returns artifacts configuration' do expect(entry.value).to eq({ "#{keyword}": [file] } ) end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 2708d8d5b6b..541deb13b97 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -37,7 +37,7 @@ describe Gitlab::Ci::Config::External::File::Local do end describe '#content' do - context 'with a a valid file' do + context 'with a valid file' do let(:local_file_content) do <<~HEREDOC before_script: diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index 85d73e5c382..fab071405df 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -18,6 +18,7 @@ describe Gitlab::Ci::Pipeline::Chain::Build do before_sha: nil, trigger_request: nil, schedule: nil, + merge_request: nil, project: project, current_user: user, variables_attributes: variables_attributes) @@ -76,6 +77,7 @@ describe Gitlab::Ci::Pipeline::Chain::Build do before_sha: nil, trigger_request: nil, schedule: nil, + merge_request: nil, project: project, current_user: user) end @@ -90,4 +92,31 @@ describe Gitlab::Ci::Pipeline::Chain::Build do expect(pipeline).to be_tag end end + + context 'when pipeline is running for a merge request' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :merge_request, + origin_ref: 'feature', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + merge_request: merge_request, + project: project, + current_user: user) + end + + let(:merge_request) { build(:merge_request, target_project: project) } + + before do + step.perform! + end + + it 'correctly indicated that this is a merge request pipeline' do + expect(pipeline).to be_merge_request + expect(pipeline.merge_request).to eq(merge_request) + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index a8dc5356413..053bc421649 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -106,4 +106,34 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do expect(step.break?).to be false end end + + context 'when pipeline source is merge request' do + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + end + + let(:pipeline) { build_stubbed(:ci_pipeline, project: project) } + + let(:merge_request_pipeline) do + build(:ci_pipeline, source: :merge_request, project: project) + end + + let(:chain) { described_class.new(merge_request_pipeline, command).tap(&:perform!) } + + context "when config contains 'merge_requests' keyword" do + let(:config) { { rspec: { script: 'echo', only: ['merge_requests'] } } } + + it 'does not break the chain' do + expect(chain).not_to be_break + end + end + + context "when config contains 'merge_request' keyword" do + let(:config) { { rspec: { script: 'echo', only: ['merge_request'] } } } + + it 'does not break the chain' do + expect(chain).not_to be_break + end + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb index 1ccb792d1da..f54ef492e6d 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb @@ -93,7 +93,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::String do end describe '#evaluate' do - it 'returns string value it is is present' do + it 'returns string value if it is present' do string = described_class.new('my string') expect(string.evaluate).to eq 'my string' diff --git a/spec/lib/gitlab/ci/trace/chunked_io_spec.rb b/spec/lib/gitlab/ci/trace/chunked_io_spec.rb index 6259b952add..546a9e7d0cc 100644 --- a/spec/lib/gitlab/ci/trace/chunked_io_spec.rb +++ b/spec/lib/gitlab/ci/trace/chunked_io_spec.rb @@ -116,6 +116,19 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do chunked_io.each_line { |line| } end end + + context 'when buffer consist of many empty lines' do + let(:sample_trace_raw) { Array.new(10, " ").join("\n") } + + before do + build.trace.set(sample_trace_raw) + end + + it 'yields lines' do + expect { |b| chunked_io.each_line(&b) } + .to yield_successive_args(*string_io.each_line.to_a) + end + end end context "#read" do @@ -143,6 +156,22 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do end end + context 'when chunk is missing data' do + let(:length) { nil } + + before do + stub_buffer_size(1024) + build.trace.set(sample_trace_raw) + + # make second chunk to not have data + build.trace_chunks.second.append('', 0) + end + + it 'raises an error' do + expect { subject }.to raise_error described_class::FailedToGetChunkError + end + end + context 'when read only first 100 bytes' do let(:length) { 100 } @@ -266,6 +295,40 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do expect(chunked_io.readline).to eq(string_io.readline) end end + + context 'when chunk is missing data' do + let(:length) { nil } + + before do + build.trace.set(sample_trace_raw) + + # make first chunk to have invalid data + build.trace_chunks.first.append('data', 0) + end + + it 'raises an error' do + expect { subject }.to raise_error described_class::FailedToGetChunkError + end + end + + context 'when utf-8 is being used' do + let(:sample_trace_raw) { sample_trace_raw_utf8.force_encoding(Encoding::BINARY) } + let(:sample_trace_raw_utf8) { "😺\n😺\n😺\n😺" } + + before do + stub_buffer_size(3) # the utf-8 character has 4 bytes + + build.trace.set(sample_trace_raw_utf8) + end + + it 'has known length' do + expect(sample_trace_raw_utf8.bytesize).to eq(4 * 4 + 3 * 1) + expect(sample_trace_raw.bytesize).to eq(4 * 4 + 3 * 1) + expect(chunked_io.size).to eq(4 * 4 + 3 * 1) + end + + it_behaves_like 'all line matching' + end end context "#write" do diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb index 4f49958dd33..38626f728d7 100644 --- a/spec/lib/gitlab/ci/trace/stream_spec.rb +++ b/spec/lib/gitlab/ci/trace/stream_spec.rb @@ -257,7 +257,8 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do let!(:last_result) { stream.html_with_state } before do - stream.append("5678", 4) + data_stream.seek(4, IO::SEEK_SET) + data_stream.write("5678") stream.seek(0) end @@ -271,25 +272,29 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do end context 'when stream is StringIO' do + let(:data_stream) do + StringIO.new("1234") + end + let(:stream) do - described_class.new do - StringIO.new("1234") - end + described_class.new { data_stream } end it_behaves_like 'html_with_states' end context 'when stream is ChunkedIO' do - let(:stream) do - described_class.new do - Gitlab::Ci::Trace::ChunkedIO.new(build).tap do |chunked_io| - chunked_io.write("1234") - chunked_io.seek(0, IO::SEEK_SET) - end + let(:data_stream) do + Gitlab::Ci::Trace::ChunkedIO.new(build).tap do |chunked_io| + chunked_io.write("1234") + chunked_io.seek(0, IO::SEEK_SET) end end + let(:stream) do + described_class.new { data_stream } + end + it_behaves_like 'html_with_states' end end diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb index 46874662edd..8bf44acb228 100644 --- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb @@ -36,7 +36,7 @@ describe Gitlab::Ci::Variables::Collection::Item do shared_examples 'raises error for invalid type' do it do expect { described_class.new(key: variable_key, value: variable_value) } - .to raise_error ArgumentError, /`value` must be of type String, while it was:/ + .to raise_error ArgumentError, /`#{variable_key}` must be of type String or nil value, while it was:/ end end diff --git a/spec/lib/gitlab/ci/config/entry/attributable_spec.rb b/spec/lib/gitlab/config/entry/attributable_spec.rb index b028b771375..abb4fff3ad7 100644 --- a/spec/lib/gitlab/ci/config/entry/attributable_spec.rb +++ b/spec/lib/gitlab/config/entry/attributable_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Attributable do +describe Gitlab::Config::Entry::Attributable do let(:node) do Class.new do - include Gitlab::Ci::Config::Entry::Attributable + include Gitlab::Config::Entry::Attributable end end @@ -48,7 +48,7 @@ describe Gitlab::Ci::Config::Entry::Attributable do it 'raises an error' do expectation = expect do Class.new(String) do - include Gitlab::Ci::Config::Entry::Attributable + include Gitlab::Config::Entry::Attributable attributes :length end diff --git a/spec/lib/gitlab/ci/config/entry/boolean_spec.rb b/spec/lib/gitlab/config/entry/boolean_spec.rb index 5f067cad93c..1b7a3f850ec 100644 --- a/spec/lib/gitlab/ci/config/entry/boolean_spec.rb +++ b/spec/lib/gitlab/config/entry/boolean_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Boolean do +describe Gitlab::Config::Entry::Boolean do let(:entry) { described_class.new(config) } describe 'validations' do diff --git a/spec/lib/gitlab/ci/config/entry/configurable_spec.rb b/spec/lib/gitlab/config/entry/configurable_spec.rb index 088d4b472da..85a7cf1d241 100644 --- a/spec/lib/gitlab/ci/config/entry/configurable_spec.rb +++ b/spec/lib/gitlab/config/entry/configurable_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Configurable do +describe Gitlab::Config::Entry::Configurable do let(:entry) do - Class.new(Gitlab::Ci::Config::Entry::Node) do - include Gitlab::Ci::Config::Entry::Configurable + Class.new(Gitlab::Config::Entry::Node) do + include Gitlab::Config::Entry::Configurable end end @@ -39,7 +39,7 @@ describe Gitlab::Ci::Config::Entry::Configurable do it 'creates a node factory' do expect(entry.nodes[:object]) - .to be_an_instance_of Gitlab::Ci::Config::Entry::Factory + .to be_an_instance_of Gitlab::Config::Entry::Factory end it 'returns a duplicated factory object' do diff --git a/spec/lib/gitlab/ci/config/entry/factory_spec.rb b/spec/lib/gitlab/config/entry/factory_spec.rb index 8dd48e4efae..c29d17eaee3 100644 --- a/spec/lib/gitlab/ci/config/entry/factory_spec.rb +++ b/spec/lib/gitlab/config/entry/factory_spec.rb @@ -1,9 +1,17 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Factory do +describe Gitlab::Config::Entry::Factory do describe '#create!' do + class Script < Gitlab::Config::Entry::Node + include Gitlab::Config::Entry::Validatable + + validations do + validates :config, array_of_strings: true + end + end + + let(:entry) { Script } let(:factory) { described_class.new(entry) } - let(:entry) { Gitlab::Ci::Config::Entry::Script } context 'when setting a concrete value' do it 'creates entry with valid value' do @@ -54,7 +62,7 @@ describe Gitlab::Ci::Config::Entry::Factory do context 'when not setting a value' do it 'raises error' do expect { factory.create! }.to raise_error( - Gitlab::Ci::Config::Entry::Factory::InvalidFactory + Gitlab::Config::Entry::Factory::InvalidFactory ) end end diff --git a/spec/lib/gitlab/ci/config/entry/simplifiable_spec.rb b/spec/lib/gitlab/config/entry/simplifiable_spec.rb index 395062207a3..bc8387ada67 100644 --- a/spec/lib/gitlab/ci/config/entry/simplifiable_spec.rb +++ b/spec/lib/gitlab/config/entry/simplifiable_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Simplifiable do +describe Gitlab::Config::Entry::Simplifiable do describe '.strategy' do let(:entry) do Class.new(described_class) do diff --git a/spec/lib/gitlab/ci/config/entry/undefined_spec.rb b/spec/lib/gitlab/config/entry/undefined_spec.rb index fdf48d84192..48f9d276c95 100644 --- a/spec/lib/gitlab/ci/config/entry/undefined_spec.rb +++ b/spec/lib/gitlab/config/entry/undefined_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Undefined do +describe Gitlab::Config::Entry::Undefined do let(:entry) { described_class.new } describe '#leaf?' do diff --git a/spec/lib/gitlab/ci/config/entry/unspecified_spec.rb b/spec/lib/gitlab/config/entry/unspecified_spec.rb index 66f88fa35b6..64421824a12 100644 --- a/spec/lib/gitlab/ci/config/entry/unspecified_spec.rb +++ b/spec/lib/gitlab/config/entry/unspecified_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Unspecified do +describe Gitlab::Config::Entry::Unspecified do let(:unspecified) { described_class.new(entry) } let(:entry) { spy('Entry') } diff --git a/spec/lib/gitlab/ci/config/entry/validatable_spec.rb b/spec/lib/gitlab/config/entry/validatable_spec.rb index ae2a7a51ba6..5a8f9766d23 100644 --- a/spec/lib/gitlab/ci/config/entry/validatable_spec.rb +++ b/spec/lib/gitlab/config/entry/validatable_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Validatable do +describe Gitlab::Config::Entry::Validatable do let(:entry) do - Class.new(Gitlab::Ci::Config::Entry::Node) do - include Gitlab::Ci::Config::Entry::Validatable + Class.new(Gitlab::Config::Entry::Node) do + include Gitlab::Config::Entry::Validatable end end @@ -20,7 +20,7 @@ describe Gitlab::Ci::Config::Entry::Validatable do it 'returns validator' do expect(entry.validator.superclass) - .to be Gitlab::Ci::Config::Entry::Validator + .to be Gitlab::Config::Entry::Validator end it 'returns only one validator to mitigate leaks' do diff --git a/spec/lib/gitlab/ci/config/entry/validator_spec.rb b/spec/lib/gitlab/config/entry/validator_spec.rb index 172b6b47a4f..efa16c4265c 100644 --- a/spec/lib/gitlab/ci/config/entry/validator_spec.rb +++ b/spec/lib/gitlab/config/entry/validator_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Validator do +describe Gitlab::Config::Entry::Validator do let(:validator) { Class.new(described_class) } let(:validator_instance) { validator.new(node) } let(:node) { spy('node') } diff --git a/spec/lib/gitlab/ci/config/loader_spec.rb b/spec/lib/gitlab/config/loader/yaml_spec.rb index 590fc8904c1..44c9a3896a8 100644 --- a/spec/lib/gitlab/ci/config/loader_spec.rb +++ b/spec/lib/gitlab/config/loader/yaml_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Loader do +describe Gitlab::Config::Loader::Yaml do let(:loader) { described_class.new(yml) } context 'when yaml syntax is correct' do @@ -31,7 +31,7 @@ describe Gitlab::Ci::Config::Loader do describe '#load!' do it 'raises error' do expect { loader.load! }.to raise_error( - Gitlab::Ci::Config::Loader::FormatError, + Gitlab::Config::Loader::FormatError, 'Invalid configuration format' ) end @@ -43,7 +43,7 @@ describe Gitlab::Ci::Config::Loader do describe '#initialize' do it 'raises FormatError' do - expect { loader }.to raise_error(Gitlab::Ci::Config::Loader::FormatError, 'Unknown alias: bad_alias') + expect { loader }.to raise_error(Gitlab::Config::Loader::FormatError, 'Unknown alias: bad_alias') end end end diff --git a/spec/lib/gitlab/contributions_calendar_spec.rb b/spec/lib/gitlab/contributions_calendar_spec.rb index 6d29044ffd5..b7924302014 100644 --- a/spec/lib/gitlab/contributions_calendar_spec.rb +++ b/spec/lib/gitlab/contributions_calendar_spec.rb @@ -135,7 +135,7 @@ describe Gitlab::ContributionsCalendar do expect(calendar(contributor).events_by_date(today)).to contain_exactly(e1, e2, e3) end - context 'when the user cannot read read cross project' do + context 'when the user cannot read cross project' do before do allow(Ability).to receive(:allowed?).and_call_original expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } diff --git a/spec/lib/gitlab/correlation_id_spec.rb b/spec/lib/gitlab/correlation_id_spec.rb new file mode 100644 index 00000000000..584d1f48386 --- /dev/null +++ b/spec/lib/gitlab/correlation_id_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::CorrelationId do + describe '.use_id' do + it 'yields when executed' do + expect { |blk| described_class.use_id('id', &blk) }.to yield_control + end + + it 'stacks correlation ids' do + described_class.use_id('id1') do + described_class.use_id('id2') do |current_id| + expect(current_id).to eq('id2') + end + end + end + + it 'for missing correlation id it generates random one' do + described_class.use_id('id1') do + described_class.use_id(nil) do |current_id| + expect(current_id).not_to be_empty + expect(current_id).not_to eq('id1') + end + end + end + end + + describe '.current_id' do + subject { described_class.current_id } + + it 'returns last correlation id' do + described_class.use_id('id1') do + described_class.use_id('id2') do + is_expected.to eq('id2') + end + end + end + end + + describe '.current_or_new_id' do + subject { described_class.current_or_new_id } + + context 'when correlation id is set' do + it 'returns last correlation id' do + described_class.use_id('id1') do + is_expected.to eq('id1') + end + end + end + + context 'when correlation id is missing' do + it 'returns a new correlation id' do + expect(described_class).to receive(:new_id) + .and_call_original + + is_expected.not_to be_empty + end + end + end + + describe '.ids' do + subject { described_class.send(:ids) } + + it 'returns empty list if not correlation is used' do + is_expected.to be_empty + end + + it 'returns list if correlation ids are used' do + described_class.use_id('id1') do + described_class.use_id('id2') do + is_expected.to eq(%w(id1 id2)) + end + end + end + end +end diff --git a/spec/lib/gitlab/cross_project_access/check_info_spec.rb b/spec/lib/gitlab/cross_project_access/check_info_spec.rb index 239fa364f5e..ea7393a7006 100644 --- a/spec/lib/gitlab/cross_project_access/check_info_spec.rb +++ b/spec/lib/gitlab/cross_project_access/check_info_spec.rb @@ -50,7 +50,7 @@ describe Gitlab::CrossProjectAccess::CheckInfo do expect(info.should_run?(dummy_controller)).to be_truthy end - it 'returns the the opposite of #should_skip? when the check is a skip' do + it 'returns the opposite of #should_skip? when the check is a skip' do info = described_class.new({}, nil, nil, true) expect(info).to receive(:should_skip?).with(dummy_controller).and_return(false) @@ -101,7 +101,7 @@ describe Gitlab::CrossProjectAccess::CheckInfo do expect(info.should_skip?(dummy_controller)).to be_truthy end - it 'returns the the opposite of #should_run? when the check is not a skip' do + it 'returns the opposite of #should_run? when the check is not a skip' do info = described_class.new({}, nil, nil, false) expect(info).to receive(:should_run?).with(dummy_controller).and_return(false) diff --git a/spec/lib/gitlab/crypto_helper_spec.rb b/spec/lib/gitlab/crypto_helper_spec.rb new file mode 100644 index 00000000000..05cc6cf15de --- /dev/null +++ b/spec/lib/gitlab/crypto_helper_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::CryptoHelper do + describe '.sha256' do + it 'generates SHA256 digest Base46 encoded' do + digest = described_class.sha256('some-value') + + expect(digest).to match %r{\A[A-Za-z0-9+/=]+\z} + expect(digest).to eq digest.strip + end + end + + describe '.aes256_gcm_encrypt' do + it 'is Base64 encoded string without new line character' do + encrypted = described_class.aes256_gcm_encrypt('some-value') + + expect(encrypted).to match %r{\A[A-Za-z0-9+/=]+\z} + expect(encrypted).not_to include "\n" + end + end + + describe '.aes256_gcm_decrypt' do + let(:encrypted) { described_class.aes256_gcm_encrypt('some-value') } + + it 'correctly decrypts encrypted string' do + decrypted = described_class.aes256_gcm_decrypt(encrypted) + + expect(decrypted).to eq 'some-value' + end + + it 'decrypts a value when it ends with a new line character' do + decrypted = described_class.aes256_gcm_decrypt(encrypted + "\n") + + expect(decrypted).to eq 'some-value' + end + end +end diff --git a/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb new file mode 100644 index 00000000000..3991c737a26 --- /dev/null +++ b/spec/lib/gitlab/database/count/exact_count_strategy_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::Database::Count::ExactCountStrategy do + before do + create_list(:project, 3) + create(:identity) + end + + let(:models) { [Project, Identity] } + + subject { described_class.new(models).count } + + describe '#count' do + it 'counts all models' do + expect(models).to all(receive(:count).and_call_original) + + expect(subject).to eq({ Project => 3, Identity => 1 }) + end + + it 'returns default value if count times out' do + allow(models.first).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) + + expect(subject).to eq({}) + end + end + + describe '.enabled?' do + it 'is enabled for PostgreSQL' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + + expect(described_class.enabled?).to be_truthy + end + + it 'is enabled for MySQL' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + + expect(described_class.enabled?).to be_truthy + end + end +end diff --git a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb new file mode 100644 index 00000000000..b44e8c5a110 --- /dev/null +++ b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Gitlab::Database::Count::ReltuplesCountStrategy do + before do + create_list(:project, 3) + create(:identity) + end + + let(:models) { [Project, Identity] } + subject { described_class.new(models).count } + + describe '#count', :postgresql do + context 'when reltuples is up to date' do + before do + ActiveRecord::Base.connection.execute('ANALYZE projects') + ActiveRecord::Base.connection.execute('ANALYZE identities') + end + + it 'uses statistics to do the count' do + models.each { |model| expect(model).not_to receive(:count) } + + expect(subject).to eq({ Project => 3, Identity => 1 }) + end + end + + context 'insufficient permissions' do + it 'returns an empty hash' do + allow(ActiveRecord::Base).to receive(:transaction).and_raise(PG::InsufficientPrivilege) + + expect(subject).to eq({}) + end + end + end + + describe '.enabled?' do + it 'is enabled for PostgreSQL' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + + expect(described_class.enabled?).to be_truthy + end + + it 'is disabled for MySQL' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + + expect(described_class.enabled?).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb new file mode 100644 index 00000000000..203f9344a41 --- /dev/null +++ b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' + +describe Gitlab::Database::Count::TablesampleCountStrategy do + before do + create_list(:project, 3) + create(:identity) + end + + let(:models) { [Project, Identity] } + let(:strategy) { described_class.new(models) } + + subject { strategy.count } + + describe '#count', :postgresql do + let(:estimates) { { Project => threshold + 1, Identity => threshold - 1 } } + let(:threshold) { Gitlab::Database::Count::TablesampleCountStrategy::EXACT_COUNT_THRESHOLD } + + before do + allow(strategy).to receive(:size_estimates).with(check_statistics: false).and_return(estimates) + end + + context 'for tables with an estimated small size' do + it 'performs an exact count' do + expect(Identity).to receive(:count).and_call_original + + expect(subject).to include({ Identity => 1 }) + end + end + + context 'for tables with an estimated large size' do + it 'performs a tablesample count' do + expect(Project).not_to receive(:count) + + result = subject + expect(result[Project]).to eq(3) + end + end + + context 'insufficient permissions' do + it 'returns an empty hash' do + allow(strategy).to receive(:size_estimates).and_raise(PG::InsufficientPrivilege) + + expect(subject).to eq({}) + end + end + end + + describe '.enabled?' do + before do + stub_feature_flags(tablesample_counts: true) + end + + it 'is enabled for PostgreSQL' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + + expect(described_class.enabled?).to be_truthy + end + + it 'is disabled for MySQL' do + allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + + expect(described_class.enabled?).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/database/count_spec.rb b/spec/lib/gitlab/database/count_spec.rb index 407d9470785..1d096b8fa7c 100644 --- a/spec/lib/gitlab/database/count_spec.rb +++ b/spec/lib/gitlab/database/count_spec.rb @@ -8,63 +8,51 @@ describe Gitlab::Database::Count do let(:models) { [Project, Identity] } - describe '.approximate_counts' do - context 'with MySQL' do - context 'when reltuples have not been updated' do - it 'counts all models the normal way' do - expect(Gitlab::Database).to receive(:postgresql?).and_return(false) + context '.approximate_counts' do + context 'selecting strategies' do + let(:strategies) { [double('s1', enabled?: true), double('s2', enabled?: false)] } - expect(Project).to receive(:count).and_call_original - expect(Identity).to receive(:count).and_call_original + it 'uses only enabled strategies' do + expect(strategies[0]).to receive(:new).and_return(double('strategy1', count: {})) + expect(strategies[1]).not_to receive(:new) - expect(described_class.approximate_counts(models)).to eq({ Project => 3, Identity => 1 }) - end + described_class.approximate_counts(models, strategies: strategies) end end - context 'with PostgreSQL', :postgresql do - describe 'when reltuples have not been updated' do - it 'counts all models the normal way' do - expect(described_class).to receive(:reltuples_from_recently_updated).with(%w(projects identities)).and_return({}) + context 'fallbacks' do + subject { described_class.approximate_counts(models, strategies: strategies) } - expect(Project).to receive(:count).and_call_original - expect(Identity).to receive(:count).and_call_original - expect(described_class.approximate_counts(models)).to eq({ Project => 3, Identity => 1 }) - end + let(:strategies) do + [ + double('s1', enabled?: true, new: first_strategy), + double('s2', enabled?: true, new: second_strategy) + ] end - describe 'no permission' do - it 'falls back to standard query' do - allow(described_class).to receive(:postgresql_estimate_query).and_raise(PG::InsufficientPrivilege) + let(:first_strategy) { double('first strategy', count: {}) } + let(:second_strategy) { double('second strategy', count: {}) } - expect(Project).to receive(:count).and_call_original - expect(Identity).to receive(:count).and_call_original - expect(described_class.approximate_counts(models)).to eq({ Project => 3, Identity => 1 }) - end + it 'gets results from first strategy' do + expect(strategies[0]).to receive(:new).with(models).and_return(first_strategy) + expect(first_strategy).to receive(:count) + + subject end - describe 'when some reltuples have been updated' do - it 'counts projects in the fast way' do - expect(described_class).to receive(:reltuples_from_recently_updated).with(%w(projects identities)).and_return({ 'projects' => 3 }) + it 'gets more results from second strategy if some counts are missing' do + expect(first_strategy).to receive(:count).and_return({ Project => 3 }) + expect(strategies[1]).to receive(:new).with([Identity]).and_return(second_strategy) + expect(second_strategy).to receive(:count).and_return({ Identity => 1 }) - expect(Project).not_to receive(:count).and_call_original - expect(Identity).to receive(:count).and_call_original - expect(described_class.approximate_counts(models)).to eq({ Project => 3, Identity => 1 }) - end + expect(subject).to eq({ Project => 3, Identity => 1 }) end - describe 'when all reltuples have been updated' do - before do - ActiveRecord::Base.connection.execute('ANALYZE projects') - ActiveRecord::Base.connection.execute('ANALYZE identities') - end - - it 'counts models with the standard way' do - expect(Project).not_to receive(:count) - expect(Identity).not_to receive(:count) + it 'does not get more results as soon as all counts are present' do + expect(first_strategy).to receive(:count).and_return({ Project => 3, Identity => 1 }) + expect(strategies[1]).not_to receive(:new) - expect(described_class.approximate_counts(models)).to eq({ Project => 3, Identity => 1 }) - end + subject end end end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb index 0a8c77b0ad9..b6096d4faf6 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb @@ -165,7 +165,7 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, : end describe '#rename_namespace_dependencies' do - it "moves the the repository for a project in the namespace" do + it "moves the repository for a project in the namespace" do create(:project, :repository, :legacy_storage, namespace: namespace, path: "the-path-project") expected_repo = File.join(TestEnv.repos_path, "the-path0", "the-path-project.git") diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 7d76519dddd..fc295b2deff 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -443,11 +443,17 @@ describe Gitlab::Database do end end + describe '.read_only?' do + it 'returns false' do + expect(described_class.read_only?).to be_falsey + end + end + describe '.db_read_only?' do context 'when using PostgreSQL' do before do allow(ActiveRecord::Base.connection).to receive(:execute).and_call_original - expect(described_class).to receive(:postgresql?).and_return(true) + allow(described_class).to receive(:postgresql?).and_return(true) end it 'detects a read only database' do @@ -456,11 +462,25 @@ describe Gitlab::Database do expect(described_class.db_read_only?).to be_truthy end + # TODO: remove rails5-only tag after removing rails4 tests + it 'detects a read only database', :rails5 do + allow(ActiveRecord::Base.connection).to receive(:execute).with('SELECT pg_is_in_recovery()').and_return([{ "pg_is_in_recovery" => true }]) + + expect(described_class.db_read_only?).to be_truthy + end + it 'detects a read write database' do allow(ActiveRecord::Base.connection).to receive(:execute).with('SELECT pg_is_in_recovery()').and_return([{ "pg_is_in_recovery" => "f" }]) expect(described_class.db_read_only?).to be_falsey end + + # TODO: remove rails5-only tag after removing rails4 tests + it 'detects a read write database', :rails5 do + allow(ActiveRecord::Base.connection).to receive(:execute).with('SELECT pg_is_in_recovery()').and_return([{ "pg_is_in_recovery" => false }]) + + expect(described_class.db_read_only?).to be_falsey + end end context 'when using MySQL' do diff --git a/spec/lib/gitlab/diff/file_collection/commit_spec.rb b/spec/lib/gitlab/diff/file_collection/commit_spec.rb index 6d1b66deb6a..34ed22b8941 100644 --- a/spec/lib/gitlab/diff/file_collection/commit_spec.rb +++ b/spec/lib/gitlab/diff/file_collection/commit_spec.rb @@ -12,4 +12,8 @@ describe Gitlab::Diff::FileCollection::Commit do let(:diffable) { project.commit } let(:stub_path) { 'bar/branch-test.txt' } end + + it_behaves_like 'unfoldable diff' do + let(:diffable) { project.commit } + end end diff --git a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb index 4578da70bfc..256166dbad3 100644 --- a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb +++ b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb @@ -2,22 +2,29 @@ require 'spec_helper' describe Gitlab::Diff::FileCollection::MergeRequestDiff do let(:merge_request) { create(:merge_request) } - let(:diff_files) { described_class.new(merge_request.merge_request_diff, diff_options: nil).diff_files } + let(:subject) { described_class.new(merge_request.merge_request_diff, diff_options: nil) } + let(:diff_files) { subject.diff_files } - it 'does not highlight binary files' do - allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(false) + describe '#diff_files' do + it 'does not highlight binary files' do + allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(false) - expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines) + expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines) - diff_files - end + diff_files + end + + it 'does not highlight files marked as undiffable in .gitattributes' do + allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(false) - it 'does not highlight files marked as undiffable in .gitattributes' do - allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(false) + expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines) - expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines) + diff_files + end + end - diff_files + it_behaves_like 'unfoldable diff' do + let(:diffable) { merge_request.merge_request_diff } end it 'it uses a different cache key if diff line keys change' do @@ -37,17 +44,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do let(:stub_path) { '.gitignore' } end - shared_examples 'initializes a DiffCollection' do - it 'returns a valid instance of a DiffCollection' do - expect(diff_files).to be_a(Gitlab::Git::DiffCollection) - end - end - - context 'with Gitaly disabled', :disable_gitaly do - it_behaves_like 'initializes a DiffCollection' - end - - context 'with Gitaly enabled' do - it_behaves_like 'initializes a DiffCollection' + it 'returns a valid instance of a DiffCollection' do + expect(diff_files).to be_a(Gitlab::Git::DiffCollection) end end diff --git a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb index 7296bbf5df3..97e65318059 100644 --- a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::Diff::InlineDiffMarker do end end - context "when the text text is not html safe" do + context "when the text is not html safe" do let(:rich) { "abc 'def' differs" } it 'marks the range' do diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb index 0989188f7ee..376d3accd55 100644 --- a/spec/lib/gitlab/email/reply_parser_spec.rb +++ b/spec/lib/gitlab/email/reply_parser_spec.rb @@ -49,7 +49,7 @@ describe Gitlab::Email::ReplyParser do expect(test_parse_body(fixture_file("emails/paragraphs.eml"))) .to eq( <<-BODY.strip_heredoc.chomp - Is there any reason the *old* candy can't be be kept in silos while the new candy + Is there any reason the *old* candy can't be kept in silos while the new candy is imported into *new* silos? The thing about candy is it stays delicious for a long time -- we can just keep diff --git a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb index 2e3656b52fb..5107e1efbbd 100644 --- a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb @@ -11,6 +11,14 @@ describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state do let(:options) { {} } + context 'when unique key is not set' do + let(:unique_key) { } + + it 'raises an error' do + expect { subject }.to raise_error ArgumentError + end + end + context 'when the lease is not obtained yet' do before do stub_exclusive_lease(unique_key, 'uuid') diff --git a/spec/lib/gitlab/file_detector_spec.rb b/spec/lib/gitlab/file_detector_spec.rb index edab53247e9..4ba9094b24e 100644 --- a/spec/lib/gitlab/file_detector_spec.rb +++ b/spec/lib/gitlab/file_detector_spec.rb @@ -15,14 +15,22 @@ describe Gitlab::FileDetector do describe '.type_of' do it 'returns the type of a README file' do - %w[README readme INDEX index].each do |filename| + filenames = Gitlab::MarkupHelper::PLAIN_FILENAMES + Gitlab::MarkupHelper::PLAIN_FILENAMES.map(&:upcase) + extensions = Gitlab::MarkupHelper::EXTENSIONS + Gitlab::MarkupHelper::EXTENSIONS.map(&:upcase) + + filenames.each do |filename| expect(described_class.type_of(filename)).to eq(:readme) - %w[.md .adoc .rst].each do |extname| - expect(described_class.type_of(filename + extname)).to eq(:readme) + + extensions.each do |extname| + expect(described_class.type_of("#{filename}.#{extname}")).to eq(:readme) end end end + it 'returns nil for a README.rb file' do + expect(described_class.type_of('README.rb')).to be_nil + end + it 'returns nil for a README file in a directory' do expect(described_class.type_of('foo/README.md')).to be_nil end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index b243f0dacae..80dd3dcc58e 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -128,7 +128,7 @@ describe Gitlab::Git::Blob, :seed_helper do end end - shared_examples 'finding blobs by ID' do + describe '.raw' do let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) } let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) } @@ -166,16 +166,6 @@ describe Gitlab::Git::Blob, :seed_helper do end end - describe '.raw' do - context 'when the blob_raw Gitaly feature is enabled' do - it_behaves_like 'finding blobs by ID' - end - - context 'when the blob_raw Gitaly feature is disabled', :skip_gitaly_mock do - it_behaves_like 'finding blobs by ID' - end - end - describe '.batch' do let(:blob_references) do [ diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 9ef27081f98..db68062e433 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -94,7 +94,7 @@ describe Gitlab::Git::Commit, :seed_helper do context 'body_size less than threshold' do let(:body_size) { 123 } - it 'fetches commit message seperately' do + it 'fetches commit message separately' do expect(described_class).to receive(:get_message).with(repository, id) commit.safe_message @@ -183,110 +183,100 @@ describe Gitlab::Git::Commit, :seed_helper do end end - shared_examples '.where' do - context 'path is empty string' do - subject do - commits = described_class.where( - repo: repository, - ref: 'master', - path: '', - limit: 10 - ) - - commits.map { |c| c.id } - end + context 'path is empty string' do + subject do + commits = described_class.where( + repo: repository, + ref: 'master', + path: '', + limit: 10 + ) - it 'has 10 elements' do - expect(subject.size).to eq(10) - end - it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + commits.map { |c| c.id } end - context 'path is nil' do - subject do - commits = described_class.where( - repo: repository, - ref: 'master', - path: nil, - limit: 10 - ) - - commits.map { |c| c.id } - end - - it 'has 10 elements' do - expect(subject.size).to eq(10) - end - it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + it 'has 10 elements' do + expect(subject.size).to eq(10) end + it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + end - context 'ref is branch name' do - subject do - commits = described_class.where( - repo: repository, - ref: 'master', - path: 'files', - limit: 3, - offset: 1 - ) + context 'path is nil' do + subject do + commits = described_class.where( + repo: repository, + ref: 'master', + path: nil, + limit: 10 + ) - commits.map { |c| c.id } - end + commits.map { |c| c.id } + end - it 'has 3 elements' do - expect(subject.size).to eq(3) - end - it { is_expected.to include("d14d6c0abdd253381df51a723d58691b2ee1ab08") } - it { is_expected.not_to include("eb49186cfa5c4338011f5f590fac11bd66c5c631") } + it 'has 10 elements' do + expect(subject.size).to eq(10) end + it { is_expected.to include(SeedRepo::EmptyCommit::ID) } + end - context 'ref is commit id' do - subject do - commits = described_class.where( - repo: repository, - ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", - path: 'files', - limit: 3, - offset: 1 - ) + context 'ref is branch name' do + subject do + commits = described_class.where( + repo: repository, + ref: 'master', + path: 'files', + limit: 3, + offset: 1 + ) - commits.map { |c| c.id } - end + commits.map { |c| c.id } + end - it 'has 3 elements' do - expect(subject.size).to eq(3) - end - it { is_expected.to include("2f63565e7aac07bcdadb654e253078b727143ec4") } - it { is_expected.not_to include(SeedRepo::Commit::ID) } + it 'has 3 elements' do + expect(subject.size).to eq(3) end + it { is_expected.to include("d14d6c0abdd253381df51a723d58691b2ee1ab08") } + it { is_expected.not_to include("eb49186cfa5c4338011f5f590fac11bd66c5c631") } + end - context 'ref is tag' do - subject do - commits = described_class.where( - repo: repository, - ref: 'v1.0.0', - path: 'files', - limit: 3, - offset: 1 - ) + context 'ref is commit id' do + subject do + commits = described_class.where( + repo: repository, + ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", + path: 'files', + limit: 3, + offset: 1 + ) - commits.map { |c| c.id } - end + commits.map { |c| c.id } + end - it 'has 3 elements' do - expect(subject.size).to eq(3) - end - it { is_expected.to include("874797c3a73b60d2187ed6e2fcabd289ff75171e") } - it { is_expected.not_to include(SeedRepo::Commit::ID) } + it 'has 3 elements' do + expect(subject.size).to eq(3) end + it { is_expected.to include("2f63565e7aac07bcdadb654e253078b727143ec4") } + it { is_expected.not_to include(SeedRepo::Commit::ID) } end - describe '.where with gitaly' do - it_should_behave_like '.where' - end + context 'ref is tag' do + subject do + commits = described_class.where( + repo: repository, + ref: 'v1.0.0', + path: 'files', + limit: 3, + offset: 1 + ) + + commits.map { |c| c.id } + end - describe '.where without gitaly', :skip_gitaly_mock do - it_should_behave_like '.where' + it 'has 3 elements' do + expect(subject.size).to eq(3) + end + it { is_expected.to include("874797c3a73b60d2187ed6e2fcabd289ff75171e") } + it { is_expected.not_to include(SeedRepo::Commit::ID) } end describe '.between' do @@ -460,11 +450,17 @@ describe Gitlab::Git::Commit, :seed_helper do described_class.extract_signature_lazily(repository, commit_id) end + other_repository = double(:repository) + described_class.extract_signature_lazily(other_repository, commit_ids.first) + expect(described_class).to receive(:batch_signature_extraction) .with(repository, commit_ids) .once .and_return({}) + expect(described_class).not_to receive(:batch_signature_extraction) + .with(other_repository, commit_ids.first) + 2.times { signatures.each(&:itself) } end end @@ -508,7 +504,7 @@ describe Gitlab::Git::Commit, :seed_helper do end end - shared_examples '#stats' do + describe '#stats' do subject { commit.stats } describe '#additions' do @@ -527,14 +523,6 @@ describe Gitlab::Git::Commit, :seed_helper do end end - describe '#stats with gitaly on' do - it_should_behave_like '#stats' - end - - describe '#stats with gitaly disabled', :skip_gitaly_mock do - it_should_behave_like '#stats' - end - describe '#has_zero_stats?' do it { expect(commit.has_zero_stats?).to eq(false) } end @@ -577,25 +565,15 @@ describe Gitlab::Git::Commit, :seed_helper do commit_ids.map { |id| described_class.get_message(repository, id) } end - shared_examples 'getting commit messages' do - it 'gets commit messages' do - expect(subject).to contain_exactly( - "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", - "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n" - ) - end - end - - context 'when Gitaly commit_messages feature is enabled' do - it_behaves_like 'getting commit messages' - - it 'gets messages in one batch', :request_store do - expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) - end + it 'gets commit messages' do + expect(subject).to contain_exactly( + "Added contributing guide\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", + "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n" + ) end - context 'when Gitaly commit_messages feature is disabled', :disable_gitaly do - it_behaves_like 'getting commit messages' + it 'gets messages in one batch', :request_store do + expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) end end diff --git a/spec/lib/gitlab/git/merge_base_spec.rb b/spec/lib/gitlab/git/merge_base_spec.rb index 2f4e043a20f..8d16d451730 100644 --- a/spec/lib/gitlab/git/merge_base_spec.rb +++ b/spec/lib/gitlab/git/merge_base_spec.rb @@ -82,7 +82,7 @@ describe Gitlab::Git::MergeBase do end describe '#unknown_refs', :missing_ref do - it 'returns the the refs passed that are not part of the repository' do + it 'returns the refs passed that are not part of the repository' do expect(merge_base.unknown_refs).to contain_exactly('aaaa') end diff --git a/spec/lib/gitlab/git/object_pool_spec.rb b/spec/lib/gitlab/git/object_pool_spec.rb new file mode 100644 index 00000000000..363c2aa67af --- /dev/null +++ b/spec/lib/gitlab/git/object_pool_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::ObjectPool do + let(:pool_repository) { create(:pool_repository) } + let(:source_repository) { pool_repository.source_project.repository } + + subject { pool_repository.object_pool } + + describe '#storage' do + it "equals the pool repository's shard name" do + expect(subject.storage).not_to be_nil + expect(subject.storage).to eq(pool_repository.shard_name) + end + end + + describe '#create' do + before do + subject.create + end + + context "when the pool doesn't exist yet" do + it 'creates the pool' do + expect(subject.exists?).to be(true) + end + end + + context 'when the pool already exists' do + it 'raises an FailedPrecondition' do + expect do + subject.create + end.to raise_error(GRPC::FailedPrecondition) + end + end + end + + describe '#exists?' do + context "when the object pool doesn't exist" do + it 'returns false' do + expect(subject.exists?).to be(false) + end + end + + context 'when the object pool exists' do + let(:pool) { create(:pool_repository, :ready) } + + subject { pool.object_pool } + + it 'returns true' do + expect(subject.exists?).to be(true) + end + end + end + + describe '#link' do + let!(:pool_repository) { create(:pool_repository, :ready) } + + context 'when no remotes are set' do + it 'sets a remote' do + subject.link(source_repository) + + repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Rugged::Repository.new(subject.repository.path) + end + + expect(repo.remotes.count).to be(1) + expect(repo.remotes.first.name).to eq(source_repository.object_pool_remote_name) + end + end + + context 'when the remote is already set' do + before do + subject.link(source_repository) + end + + it "doesn't raise an error" do + subject.link(source_repository) + + repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Rugged::Repository.new(subject.repository.path) + end + + expect(repo.remotes.count).to be(1) + expect(repo.remotes.first.name).to eq(source_repository.object_pool_remote_name) + end + end + end +end diff --git a/spec/lib/gitlab/git/remote_mirror_spec.rb b/spec/lib/gitlab/git/remote_mirror_spec.rb new file mode 100644 index 00000000000..dc63eef7814 --- /dev/null +++ b/spec/lib/gitlab/git/remote_mirror_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::Git::RemoteMirror do + describe '#update' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:ref_name) { 'foo' } + let(:options) { { only_branches_matching: ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS' } } + + subject(:remote_mirror) { described_class.new(repository, ref_name, **options) } + + it 'delegates to the Gitaly client' do + expect(repository.gitaly_remote_client) + .to receive(:update_remote_mirror) + .with(ref_name, ['master'], ssh_key: 'KEY', known_hosts: 'KNOWN HOSTS') + + remote_mirror.update + end + + it 'wraps gitaly errors' do + expect(repository.gitaly_remote_client) + .to receive(:update_remote_mirror) + .and_raise(StandardError) + + expect { remote_mirror.update }.to raise_error(StandardError) + end + end +end diff --git a/spec/lib/gitlab/git/repository_cleaner_spec.rb b/spec/lib/gitlab/git/repository_cleaner_spec.rb new file mode 100644 index 00000000000..a9d9e67ef94 --- /dev/null +++ b/spec/lib/gitlab/git/repository_cleaner_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::Git::RepositoryCleaner do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:head_sha) { repository.head_commit.id } + + let(:object_map) { StringIO.new("#{head_sha} #{'0' * 40}") } + + subject(:cleaner) { described_class.new(repository.raw) } + + describe '#apply_bfg_object_map' do + it 'removes internal references pointing at SHAs in the object map' do + # Create some refs we expect to be removed + repository.keep_around(head_sha) + repository.create_ref(head_sha, 'refs/environments/1') + repository.create_ref(head_sha, 'refs/merge-requests/1') + repository.create_ref(head_sha, 'refs/heads/_keep') + repository.create_ref(head_sha, 'refs/tags/_keep') + + cleaner.apply_bfg_object_map(object_map) + + aggregate_failures do + expect(repository.kept_around?(head_sha)).to be_falsy + expect(repository.ref_exists?('refs/environments/1')).to be_falsy + expect(repository.ref_exists?('refs/merge-requests/1')).to be_falsy + expect(repository.ref_exists?('refs/heads/_keep')).to be_truthy + expect(repository.ref_exists?('refs/tags/_keep')).to be_truthy + end + end + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 1fe73c12fc0..852ee9c96af 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1469,6 +1469,19 @@ describe Gitlab::Git::Repository, :seed_helper do end end end + + it 'writes the HEAD' do + repository.write_ref('HEAD', 'refs/heads/feature') + + expect(repository.commit('HEAD')).to eq(repository.commit('feature')) + expect(repository.root_ref).to eq('feature') + end + + it 'writes other refs' do + repository.write_ref('refs/heads/feature', SeedRepo::Commit::ID) + + expect(repository.commit('feature').sha).to eq(SeedRepo::Commit::ID) + end end describe '#write_config' do diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index 2d9db576a6c..b51e3879f49 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Tag, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - shared_examples 'Gitlab::Git::Repository#tags' do + describe '#tags' do describe 'first tag' do let(:tag) { repository.tags.first } @@ -25,14 +25,6 @@ describe Gitlab::Git::Tag, :seed_helper do it { expect(repository.tags.size).to eq(SeedRepo::Repo::TAGS.size) } end - context 'when Gitaly tags feature is enabled' do - it_behaves_like 'Gitlab::Git::Repository#tags' - end - - context 'when Gitaly tags feature is disabled', :skip_gitaly_mock do - it_behaves_like 'Gitlab::Git::Repository#tags' - end - describe '.get_message' do let(:tag_ids) { %w[f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8 8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b] } @@ -40,23 +32,16 @@ describe Gitlab::Git::Tag, :seed_helper do tag_ids.map { |id| described_class.get_message(repository, id) } end - shared_examples 'getting tag messages' do - it 'gets tag messages' do - expect(subject[0]).to eq("Release\n") - expect(subject[1]).to eq("Version 1.1.0\n") - end + it 'gets tag messages' do + expect(subject[0]).to eq("Release\n") + expect(subject[1]).to eq("Version 1.1.0\n") end - context 'when Gitaly tag_messages feature is enabled' do - it_behaves_like 'getting tag messages' - - it 'gets messages in one batch', :request_store do - expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) - end - end + it 'gets messages in one batch', :request_store do + other_repository = double(:repository) + described_class.get_message(other_repository, tag_ids.first) - context 'when Gitaly tag_messages feature is disabled', :disable_gitaly do - it_behaves_like 'getting tag messages' + expect { subject.map(&:itself) }.to change { Gitlab::GitalyClient.get_request_count }.by(1) end end @@ -68,7 +53,7 @@ describe Gitlab::Git::Tag, :seed_helper do context 'message_size less than threshold' do let(:message_size) { 123 } - it 'fetches tag message seperately' do + it 'fetches tag message separately' do expect(described_class).to receive(:get_message).with(repository, gitaly_tag.id) tag.message diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 3792d6bf67b..bec875fb03d 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -80,18 +80,8 @@ describe Gitlab::Git::Tree, :seed_helper do end describe '#where' do - shared_examples '#where' do - it 'returns an empty array when called with an invalid ref' do - expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) - end - end - - context 'with gitaly' do - it_behaves_like '#where' - end - - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#where' + it 'returns an empty array when called with an invalid ref' do + expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) end end end diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb index ba7fb168a3b..3ab04a1c46d 100644 --- a/spec/lib/gitlab/git_ref_validator_spec.rb +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -27,4 +27,5 @@ describe Gitlab::GitRefValidator do 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 } end diff --git a/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb b/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb new file mode 100644 index 00000000000..369deff732a --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/cleanup_service_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::CleanupService do + let(:project) { create(:project) } + let(:storage_name) { project.repository_storage } + let(:relative_path) { project.disk_path + '.git' } + let(:client) { described_class.new(project.repository) } + + describe '#apply_bfg_object_map' do + it 'sends an apply_bfg_object_map message' do + expect_any_instance_of(Gitaly::CleanupService::Stub) + .to receive(:apply_bfg_object_map) + .with(kind_of(Enumerator), kind_of(Hash)) + .and_return(double) + + client.apply_bfg_object_map(StringIO.new) + end + end +end diff --git a/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb b/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb new file mode 100644 index 00000000000..149b7ec5bb0 --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::GitalyClient::ObjectPoolService do + let(:pool_repository) { create(:pool_repository) } + let(:project) { create(:project, :repository) } + let(:raw_repository) { project.repository.raw } + let(:object_pool) { pool_repository.object_pool } + + subject { described_class.new(object_pool) } + + before do + subject.create(raw_repository) + end + + describe '#create' do + it 'exists on disk' do + expect(object_pool.repository.exists?).to be(true) + end + + context 'when the pool already exists' do + it 'returns an error' do + expect do + subject.create(raw_repository) + end.to raise_error(GRPC::FailedPrecondition) + end + end + end + + describe '#delete' do + it 'removes the repository from disk' do + subject.delete + + expect(object_pool.repository.exists?).to be(false) + end + + context 'when called twice' do + it "doesn't raise an error" do + subject.delete + + expect { object_pool.delete }.not_to raise_error + end + end + end +end diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index 9030a49983d..aff47599ad6 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -68,6 +68,8 @@ describe Gitlab::GitalyClient::RemoteService do describe '#update_remote_mirror' do let(:ref_name) { 'remote_mirror_1' } let(:only_branches_matching) { ['my-branch', 'master'] } + let(:ssh_key) { 'KEY' } + let(:known_hosts) { 'KNOWN HOSTS' } it 'sends an update_remote_mirror message' do expect_any_instance_of(Gitaly::RemoteService::Stub) @@ -75,7 +77,7 @@ describe Gitlab::GitalyClient::RemoteService do .with(kind_of(Enumerator), kind_of(Hash)) .and_return(double(:update_remote_mirror_response)) - client.update_remote_mirror(ref_name, only_branches_matching) + client.update_remote_mirror(ref_name, only_branches_matching, ssh_key: ssh_key, known_hosts: known_hosts) end end diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index d605fcbafee..46ca2340389 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -130,7 +130,7 @@ describe Gitlab::GitalyClient::RepositoryService do end context 'SSH auth' do - where(:ssh_import, :ssh_key_auth, :ssh_private_key, :ssh_known_hosts, :expected_params) do + where(:ssh_mirror_url, :ssh_key_auth, :ssh_private_key, :ssh_known_hosts, :expected_params) do false | false | 'key' | 'known_hosts' | {} false | true | 'key' | 'known_hosts' | {} true | false | 'key' | 'known_hosts' | { known_hosts: 'known_hosts' } @@ -145,7 +145,7 @@ describe Gitlab::GitalyClient::RepositoryService do let(:ssh_auth) do double( :ssh_auth, - ssh_import?: ssh_import, + ssh_mirror_url?: ssh_mirror_url, ssh_key_auth?: ssh_key_auth, ssh_private_key: ssh_private_key, ssh_known_hosts: ssh_known_hosts diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 74831197bfb..36c9e9a72e9 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' # We stub Gitaly in `spec/support/gitaly.rb` for other tests. We don't want # those stubs while testing the GitalyClient itself. -describe Gitlab::GitalyClient, skip_gitaly_mock: true do +describe Gitlab::GitalyClient do describe '.stub_class' do it 'returns the gRPC health check stub' do expect(described_class.stub_class(:health_check)).to eq(::Grpc::Health::V1::Health::Stub) @@ -224,102 +224,13 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do let(:feature_name) { 'my_feature' } let(:real_feature_name) { "gitaly_#{feature_name}" } - context 'when Gitaly is disabled' do - before do - allow(described_class).to receive(:enabled?).and_return(false) - end - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name)).to be(false) - end - end - - context 'when the feature status is DISABLED' do - let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::DISABLED } - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context 'when the feature_status is OPT_IN' do - let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_IN } - - context "when the feature flag hasn't been set" do - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context "when the feature flag is set to disable" do - before do - Feature.get(real_feature_name).disable - end - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context "when the feature flag is set to enable" do - before do - Feature.get(real_feature_name).enable - end - - it 'returns true' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - end - end - - context "when the feature flag is set to a percentage of time" do - before do - Feature.get(real_feature_name).enable_percentage_of_time(70) - end - - it 'bases the result on pseudo-random numbers' do - expect(Random).to receive(:rand).and_return(0.3) - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - - expect(Random).to receive(:rand).and_return(0.8) - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end - - context "when a feature is not persisted" do - it 'returns false when opt_into_all_features is off' do - allow(Feature).to receive(:persisted?).and_return(false) - allow(described_class).to receive(:opt_into_all_features?).and_return(false) - - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - - it 'returns true when the override is on' do - allow(Feature).to receive(:persisted?).and_return(false) - allow(described_class).to receive(:opt_into_all_features?).and_return(true) - - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - end - end + before do + allow(Feature).to receive(:enabled?).and_return(false) end - context 'when the feature_status is OPT_OUT' do - let(:feature_status) { Gitlab::GitalyClient::MigrationStatus::OPT_OUT } - - context "when the feature flag hasn't been set" do - it 'returns true' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(true) - end - end - - context "when the feature flag is set to disable" do - before do - Feature.get(real_feature_name).disable - end - - it 'returns false' do - expect(described_class.feature_enabled?(feature_name, status: feature_status)).to be(false) - end - end + it 'returns false' do + expect(Feature).to receive(:enabled?).with(real_feature_name) + expect(described_class.feature_enabled?(feature_name)).to be(false) end end @@ -338,4 +249,29 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do end end end + + describe 'Peek Performance bar details' do + let(:gitaly_server) { Gitaly::Server.all.first } + + before do + Gitlab::SafeRequestStore[:peek_enabled] = true + end + + context 'when the request store is active', :request_store do + it 'records call details if a RPC is called' do + gitaly_server.server_version + + expect(described_class.list_call_details).not_to be_empty + expect(described_class.list_call_details.size).to be(1) + end + end + + context 'when no request store is active' do + it 'records nothing' do + gitaly_server.server_version + + expect(described_class.list_call_details).to be_empty + end + end + end end 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 d8f01dcb76b..77f5b2ffa37 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -218,7 +218,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do describe '#fail_import' do it 'marks the import as failed' do - expect(project).to receive(:mark_import_as_failed).with('foo') + expect(project.import_state).to receive(:mark_as_failed).with('foo') expect(importer.fail_import('foo')).to eq(false) end diff --git a/spec/lib/gitlab/github_import/parallel_importer_spec.rb b/spec/lib/gitlab/github_import/parallel_importer_spec.rb index 20b48c1de68..f5df38c9aaf 100644 --- a/spec/lib/gitlab/github_import/parallel_importer_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_importer_spec.rb @@ -36,7 +36,7 @@ describe Gitlab::GithubImport::ParallelImporter do it 'updates the import JID of the project' do importer.execute - expect(project.reload.import_jid).to eq("github-importer/#{project.id}") + expect(project.import_state.reload.jid).to eq("github-importer/#{project.id}") end end end diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index 8c6d673391b..8229f0eb794 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -26,6 +26,28 @@ describe Gitlab::Gpg::Commit do end end + context 'invalid signature' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } + + let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } + + before do + allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) + .with(Gitlab::Git::Repository, commit_sha) + .and_return( + [ + # Corrupt the key + GpgHelpers::User1.signed_commit_signature.tr('=', 'a'), + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns nil' do + expect(described_class.new(commit).signature).to be_nil + end + end + context 'known key' do context 'user matches the key uid' do context 'user email matches the email committer' do diff --git a/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb new file mode 100644 index 00000000000..4609593ef6a --- /dev/null +++ b/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gitlab::Graphql::Loaders::BatchModelLoader do + describe '#find' do + let(:issue) { create(:issue) } + let(:user) { create(:user) } + + it 'finds a model by id' do + issue_result = described_class.new(Issue, issue.id).find + user_result = described_class.new(User, user.id).find + + expect(issue_result.__sync).to eq(issue) + expect(user_result.__sync).to eq(user) + end + + it 'only queries once per model' do + other_user = create(:user) + user + issue + + expect do + [described_class.new(User, other_user.id).find, + described_class.new(User, user.id).find, + described_class.new(Issue, issue.id).find].map(&:__sync) + end.not_to exceed_query_limit(2) + end + end +end diff --git a/spec/lib/gitlab/group_hierarchy_spec.rb b/spec/lib/gitlab/group_hierarchy_spec.rb index 30686634af4..f3de7adcec7 100644 --- a/spec/lib/gitlab/group_hierarchy_spec.rb +++ b/spec/lib/gitlab/group_hierarchy_spec.rb @@ -34,6 +34,28 @@ describe Gitlab::GroupHierarchy, :postgresql do expect { relation.update_all(share_with_group_lock: false) } .to raise_error(ActiveRecord::ReadOnlyRecord) end + + describe 'hierarchy_order option' do + let(:relation) do + described_class.new(Group.where(id: child2.id)).base_and_ancestors(hierarchy_order: hierarchy_order) + end + + context ':asc' do + let(:hierarchy_order) { :asc } + + it 'orders by child to parent' do + expect(relation).to eq([child2, child1, parent]) + end + end + + context ':desc' do + let(:hierarchy_order) { :desc } + + it 'orders by parent to child' do + expect(relation).to eq([parent, child1, child2]) + end + end + end end describe '#base_and_descendants' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 1d184375a52..bae5b21c26f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -94,6 +94,7 @@ merge_requests: - timelogs - head_pipeline - latest_merge_request_diff +- merge_request_pipelines merge_request_diff: - merge_request - merge_request_diff_commits @@ -102,7 +103,7 @@ merge_request_diff_commits: - merge_request_diff merge_request_diff_files: - merge_request_diff -pipelines: +ci_pipelines: - project - user - stages @@ -121,6 +122,9 @@ pipelines: - artifacts - pipeline_schedule - merge_requests +- merge_request +- deployments +- environments pipeline_variables: - pipeline stages: @@ -245,6 +249,7 @@ project: - protected_branches - protected_tags - project_members +- project_repository - users - requesters - deploy_keys_projects @@ -260,7 +265,8 @@ project: - notification_settings - import_data - commit_statuses -- pipelines +- ci_pipelines +- all_pipelines - stages - builds - runner_projects @@ -281,6 +287,7 @@ project: - statistics - container_repositories - uploads +- file_uploads - import_state - members_and_requesters - build_trace_section_names @@ -301,6 +308,7 @@ project: - import_export_upload - repository_languages - pool_repository +- kubernetes_namespaces award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 3f2281f213f..58949f76bd6 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -2556,7 +2556,7 @@ "merge_request_diff_id": 27, "relative_order": 0, "sha": "bb5206fee213d983da88c47f9cf4cc6caf9c66dc", - "message": "Feature conflcit added\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", + "message": "Feature conflict added\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n", "authored_date": "2014-08-06T08:35:52.000+02:00", "author_name": "Dmitriy Zaporozhets", "author_email": "dmitriy.zaporozhets@gmail.com", @@ -3605,7 +3605,7 @@ "merge_request_diff_id": 14, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", @@ -4290,7 +4290,7 @@ "merge_request_diff_id": 13, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", @@ -4799,7 +4799,7 @@ "merge_request_diff_id": 12, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", @@ -5507,7 +5507,7 @@ "merge_request_diff_id": 10, "relative_order": 8, "sha": "08f22f255f082689c0d7d39d19205085311542bc", - "message": "remove emtpy file.(beacase git ignore empty file)\nadd whitespace test file.\n", + "message": "remove empty file.(beacase git ignore empty file)\nadd whitespace test file.\n", "authored_date": "2015-11-13T06:00:16.000+01:00", "author_name": "윤민식", "author_email": "minsik.yoon@samsung.com", diff --git a/spec/lib/gitlab/import_export/project.light.json b/spec/lib/gitlab/import_export/project.light.json index ba2248073f5..2971ca0f0f8 100644 --- a/spec/lib/gitlab/import_export/project.light.json +++ b/spec/lib/gitlab/import_export/project.light.json @@ -101,6 +101,28 @@ ] } ], + "services": [ + { + "id": 100, + "title": "JetBrains TeamCity CI", + "project_id": 5, + "created_at": "2016-06-14T15:01:51.315Z", + "updated_at": "2016-06-14T15:01:51.315Z", + "active": false, + "properties": {}, + "template": true, + "push_events": true, + "issues_events": true, + "merge_requests_events": true, + "tag_push_events": true, + "note_events": true, + "job_events": true, + "type": "TeamcityService", + "category": "ci", + "default": false, + "wiki_page_events": true + } + ], "snippets": [], "hooks": [] } 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 365bfae0d88..242c16c4bdc 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -197,9 +197,9 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end it 'has the correct number of pipelines and statuses' do - expect(@project.pipelines.size).to eq(5) + expect(@project.ci_pipelines.size).to eq(5) - @project.pipelines.zip([2, 2, 2, 2, 2]) + @project.ci_pipelines.zip([2, 2, 2, 2, 2]) .each do |(pipeline, expected_status_size)| expect(pipeline.statuses.size).to eq(expected_status_size) end @@ -297,7 +297,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do issues: 1, labels: 1, milestones: 1, - first_issue_labels: 1 + first_issue_labels: 1, + services: 1 context 'project.json file access check' do it 'does not read a symlink' do @@ -382,6 +383,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.light.json") end + it 'does not import any templated services' do + restored_project_json + + expect(project.services.where(template: true).count).to eq(0) + end + it 'imports labels' do create(:group_label, name: 'Another label', group: project.group) diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 5dc372263ad..46fdfba953b 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -119,16 +119,16 @@ describe Gitlab::ImportExport::ProjectTreeSaver do end it 'has pipeline stages' do - expect(saved_project_json.dig('pipelines', 0, 'stages')).not_to be_empty + expect(saved_project_json.dig('ci_pipelines', 0, 'stages')).not_to be_empty end it 'has pipeline statuses' do - expect(saved_project_json.dig('pipelines', 0, 'stages', 0, 'statuses')).not_to be_empty + expect(saved_project_json.dig('ci_pipelines', 0, 'stages', 0, 'statuses')).not_to be_empty end it 'has pipeline builds' do builds_count = saved_project_json - .dig('pipelines', 0, 'stages', 0, 'statuses') + .dig('ci_pipelines', 0, 'stages', 0, 'statuses') .count { |hash| hash['type'] == 'Ci::Build' } expect(builds_count).to eq(1) @@ -142,11 +142,11 @@ describe Gitlab::ImportExport::ProjectTreeSaver do end it 'has pipeline commits' do - expect(saved_project_json['pipelines']).not_to be_empty + expect(saved_project_json['ci_pipelines']).not_to be_empty end it 'has ci pipeline notes' do - expect(saved_project_json['pipelines'].first['notes']).not_to be_empty + expect(saved_project_json['ci_pipelines'].first['notes']).not_to be_empty end it 'has labels with no associations' do diff --git a/spec/lib/gitlab/import_export/relation_rename_service_spec.rb b/spec/lib/gitlab/import_export/relation_rename_service_spec.rb new file mode 100644 index 00000000000..a20a844a492 --- /dev/null +++ b/spec/lib/gitlab/import_export/relation_rename_service_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ImportExport::RelationRenameService do + let(:renames) do + { + 'example_relation1' => 'new_example_relation1', + 'example_relation2' => 'new_example_relation2' + } + end + + let(:user) { create(:admin) } + let(:group) { create(:group, :nested) } + let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') } + let(:shared) { project.import_export_shared } + + before do + stub_const("#{described_class}::RENAMES", renames) + end + + context 'when importing' do + let(:project_tree_restorer) { Gitlab::ImportExport::ProjectTreeRestorer.new(user: user, shared: shared, project: project) } + let(:import_path) { 'spec/lib/gitlab/import_export' } + let(:file_content) { IO.read("#{import_path}/project.json") } + let!(:json_file) { ActiveSupport::JSON.decode(file_content) } + let(:tree_hash) { project_tree_restorer.instance_variable_get(:@tree_hash) } + + before do + allow(shared).to receive(:export_path).and_return(import_path) + allow(ActiveSupport::JSON).to receive(:decode).with(file_content).and_return(json_file) + end + + context 'when the file has only old relationship names' do + # Configuring the json as an old version exported file, with only + # the previous association with the old name + before do + renames.each do |old_name, _| + json_file[old_name.to_s] = [] + end + end + + it 'renames old relationships to the new name' do + expect(json_file.keys).to include(*renames.keys) + + project_tree_restorer.restore + + expect(json_file.keys).to include(*renames.values) + expect(json_file.keys).not_to include(*renames.keys) + end + end + + context 'when the file has both the old and new relationships' do + # Configuring the json as the new version exported file, with both + # the old association name and the new one + before do + renames.each do |old_name, new_name| + json_file[old_name.to_s] = [1] + json_file[new_name.to_s] = [2] + end + end + + it 'uses the new relationships and removes the old ones from the hash' do + expect(json_file.keys).to include(*renames.keys) + + project_tree_restorer.restore + + expect(json_file.keys).to include(*renames.values) + expect(json_file.values_at(*renames.values).flatten.uniq.first).to eq 2 + expect(json_file.keys).not_to include(*renames.keys) + end + end + + context 'when the file has only new relationship names' do + # Configuring the json as the future version exported file, with only + # the new association name + before do + renames.each do |_, new_name| + json_file[new_name.to_s] = [] + end + end + + it 'uses the new relationships' do + expect(json_file.keys).not_to include(*renames.keys) + + project_tree_restorer.restore + + expect(json_file.keys).to include(*renames.values) + end + end + end + + context 'when exporting' do + let(:project_tree_saver) { Gitlab::ImportExport::ProjectTreeSaver.new(project: project, current_user: user, shared: shared) } + let(:project_tree) { project_tree_saver.send(:project_json) } + + it 'adds old relationships to the exported file' do + project_tree.merge!(renames.values.map { |new_name| [new_name, []] }.to_h) + + allow(project_tree_saver).to receive(:save) do |arg| + project_tree_saver.send(:project_json_tree) + end + + result = project_tree_saver.save + + saved_data = ActiveSupport::JSON.decode(result) + + expect(saved_data.keys).to include(*(renames.keys + renames.values)) + 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 f7935149b23..d3bfde181bc 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -243,6 +243,7 @@ Ci::Pipeline: - failure_reason - protected - iid +- merge_request_id Ci::Stage: - id - name diff --git a/spec/lib/gitlab/json_logger_spec.rb b/spec/lib/gitlab/json_logger_spec.rb index 0a62785f880..cff7dd58c8c 100644 --- a/spec/lib/gitlab/json_logger_spec.rb +++ b/spec/lib/gitlab/json_logger_spec.rb @@ -7,6 +7,10 @@ describe Gitlab::JsonLogger do let(:now) { Time.now } describe '#format_message' do + before do + allow(Gitlab::CorrelationId).to receive(:current_id).and_return('new-correlation-id') + end + it 'formats strings' do output = subject.format_message('INFO', now, 'test', 'Hello world') data = JSON.parse(output) @@ -14,6 +18,7 @@ describe Gitlab::JsonLogger do expect(data['severity']).to eq('INFO') expect(data['time']).to eq(now.utc.iso8601(3)) expect(data['message']).to eq('Hello world') + expect(data['correlation_id']).to eq('new-correlation-id') end it 'formats hashes' do @@ -24,6 +29,7 @@ describe Gitlab::JsonLogger do expect(data['time']).to eq(now.utc.iso8601(3)) expect(data['hello']).to eq(1) expect(data['message']).to be_nil + expect(data['correlation_id']).to eq('new-correlation-id') end end end diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb index 8bce7a4cdf5..c7f92cbb143 100644 --- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb @@ -40,6 +40,7 @@ describe Gitlab::Kubernetes::Helm::Api do allow(client).to receive(:create_config_map).and_return(nil) allow(client).to receive(:create_service_account).and_return(nil) allow(client).to receive(:create_cluster_role_binding).and_return(nil) + allow(client).to receive(:delete_pod).and_return(nil) allow(namespace).to receive(:ensure_exists!).once end @@ -50,6 +51,13 @@ describe Gitlab::Kubernetes::Helm::Api do subject.install(command) end + it 'removes an existing pod before installing' do + expect(client).to receive(:delete_pod).with('install-app-name', 'gitlab-managed-apps').once.ordered + expect(client).to receive(:create_pod).once.ordered + + subject.install(command) + end + context 'with a ConfigMap' do let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application_name, files).generate } @@ -180,6 +188,7 @@ describe Gitlab::Kubernetes::Helm::Api do 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 @@ -189,6 +198,13 @@ describe Gitlab::Kubernetes::Helm::Api do 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 @@ -224,9 +240,18 @@ describe Gitlab::Kubernetes::Helm::Api do describe '#delete_pod!' do it 'deletes the POD from kubernetes cluster' do - expect(client).to receive(:delete_pod).with(command.pod_name, gitlab_namespace).once + expect(client).to receive(:delete_pod).with('install-app-name', 'gitlab-managed-apps').once - subject.delete_pod!(command.pod_name) + subject.delete_pod!('install-app-name') + end + + context 'when the resource being deleted does not exist' do + it 'catches the error' do + expect(client).to receive(:delete_pod).with('install-app-name', 'gitlab-managed-apps') + .and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil)) + + subject.delete_pod!('install-app-name') + end end end diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 2b7e3ea6def..82ed4d47857 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -26,7 +26,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + 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 #{helm_install_comand} @@ -42,6 +43,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do --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 @@ -54,7 +56,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + 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 #{helm_install_command} @@ -84,7 +87,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done #{helm_install_command} EOS end @@ -98,6 +102,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do --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 @@ -111,7 +116,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + 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 #{helm_install_command} @@ -122,7 +128,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand 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 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml + 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 end end @@ -134,7 +140,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + 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 #{helm_install_command} @@ -143,7 +150,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do 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 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml + 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 @@ -157,7 +164,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + 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 #{helm_install_command} @@ -169,6 +177,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do helm install chart-name --name app-name --version 1.2.3 + --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml EOS @@ -182,7 +191,8 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + helm init --upgrade + 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 #{helm_install_command} @@ -197,6 +207,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do --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 + --set rbac.create\\=false,rbac.enabled\\=false --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml EOS diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index c92bc92c42d..2dd3a570a1d 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::Kubernetes::Helm::Pod do it 'should generate the appropriate specifications for the container' do container = subject.generate.spec.containers.first expect(container.name).to eq('helm') - expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.7.2-kube-1.11.0') + expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.11.0-kube-1.11.0') expect(container.env.count).to eq(3) expect(container.env.map(&:name)).to match_array([:HELM_VERSION, :TILLER_NAMESPACE, :COMMAND_SCRIPT]) expect(container.command).to match_array(["/bin/sh"]) diff --git a/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb index 9c9fc91ef3c..9b201dae417 100644 --- a/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/upgrade_command_spec.rb @@ -21,7 +21,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + 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 @@ -33,7 +34,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + 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 @@ -56,7 +58,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + 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 @@ -70,7 +73,8 @@ describe Gitlab::Kubernetes::Helm::UpgradeCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only + 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 diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index 3979a43216c..8fc85301304 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -99,6 +99,7 @@ describe Gitlab::Kubernetes::KubeClient do :create_secret, :create_service_account, :update_config_map, + :update_secret, :update_service_account ].each do |method| describe "##{method}" do @@ -174,6 +175,84 @@ describe Gitlab::Kubernetes::KubeClient do end end + shared_examples 'create_or_update method' do + let(:get_method) { "get_#{resource_type}" } + let(:update_method) { "update_#{resource_type}" } + let(:create_method) { "create_#{resource_type}" } + + context 'resource exists' do + before do + expect(client).to receive(get_method).and_return(resource) + end + + it 'calls the update method' do + expect(client).to receive(update_method).with(resource) + + subject + end + end + + context 'resource does not exist' do + before do + expect(client).to receive(get_method).and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil)) + end + + it 'calls the create method' do + expect(client).to receive(create_method).with(resource) + + subject + end + end + end + + describe '#create_or_update_cluster_role_binding' do + let(:resource_type) { 'cluster_role_binding' } + + let(:resource) do + ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' }) + end + + subject { client.create_or_update_cluster_role_binding(resource) } + + it_behaves_like 'create_or_update method' + end + + describe '#create_or_update_role_binding' do + let(:resource_type) { 'role_binding' } + + let(:resource) do + ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' }) + end + + subject { client.create_or_update_role_binding(resource) } + + it_behaves_like 'create_or_update method' + end + + describe '#create_or_update_service_account' do + let(:resource_type) { 'service_account' } + + let(:resource) do + ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' }) + end + + subject { client.create_or_update_service_account(resource) } + + it_behaves_like 'create_or_update method' + end + + describe '#create_or_update_secret' do + let(:resource_type) { 'secret' } + + let(:resource) do + ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' }) + end + + subject { client.create_or_update_secret(resource) } + + it_behaves_like 'create_or_update method' + end + describe 'methods that do not exist on any client' do it 'throws an error' do expect { client.non_existent_method }.to raise_error(NoMethodError) diff --git a/spec/lib/gitlab/kubernetes_spec.rb b/spec/lib/gitlab/kubernetes_spec.rb index 5c03a2ce7d3..f326d57e9c6 100644 --- a/spec/lib/gitlab/kubernetes_spec.rb +++ b/spec/lib/gitlab/kubernetes_spec.rb @@ -48,26 +48,30 @@ describe Gitlab::Kubernetes do end describe '#to_kubeconfig' do + let(:token) { 'TOKEN' } + let(:ca_pem) { 'PEM' } + subject do to_kubeconfig( url: 'https://kube.domain.com', namespace: 'NAMESPACE', - token: 'TOKEN', - ca_pem: ca_pem) + token: token, + ca_pem: ca_pem + ) end - context 'when CA PEM is provided' do - let(:ca_pem) { 'PEM' } - let(:path) { expand_fixture_path('config/kubeconfig.yml') } - - it { is_expected.to eq(YAML.load_file(path)) } - end + it { expect(YAML.safe_load(subject)).to eq(YAML.load_file(expand_fixture_path('config/kubeconfig.yml'))) } context 'when CA PEM is not provided' do let(:ca_pem) { nil } - let(:path) { expand_fixture_path('config/kubeconfig-without-ca.yml') } - it { is_expected.to eq(YAML.load_file(path)) } + it { expect(YAML.safe_load(subject)).to eq(YAML.load_file(expand_fixture_path('config/kubeconfig-without-ca.yml'))) } + end + + context 'when token is not provided' do + let(:token) { nil } + + it { is_expected.to be_nil } end end diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 20514486727..d2df21d7bb5 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -174,7 +174,7 @@ describe Gitlab::LegacyGithubImport::Importer do described_class.new(project).execute - expect(project.import_error).to eq error.to_json + expect(project.import_state.last_error).to eq error.to_json end end diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb index 3a20dad16d0..77ee30264bf 100644 --- a/spec/lib/gitlab/lfs_token_spec.rb +++ b/spec/lib/gitlab/lfs_token_spec.rb @@ -48,4 +48,59 @@ describe Gitlab::LfsToken do end end end + + describe '#deploy_key_pushable?' do + let(:lfs_token) { described_class.new(actor) } + + context 'when actor is not a DeployKey' do + let(:actor) { create(:user) } + let(:project) { create(:project) } + + it 'returns false' do + expect(lfs_token.deploy_key_pushable?(project)).to be_falsey + end + end + + context 'when actor is a DeployKey' do + let(:deploy_keys_project) { create(:deploy_keys_project, can_push: can_push) } + let(:project) { deploy_keys_project.project } + let(:actor) { deploy_keys_project.deploy_key } + + context 'but the DeployKey cannot push to the project' do + let(:can_push) { false } + + it 'returns false' do + expect(lfs_token.deploy_key_pushable?(project)).to be_falsey + end + end + + context 'and the DeployKey can push to the project' do + let(:can_push) { true } + + it 'returns true' do + expect(lfs_token.deploy_key_pushable?(project)).to be_truthy + end + end + end + end + + describe '#type' do + let(:lfs_token) { described_class.new(actor) } + + context 'when actor is not a User' do + let(:actor) { create(:deploy_key) } + + it 'returns false' do + expect(lfs_token.type).to eq(:lfs_deploy_token) + end + end + + context 'when actor is a User' do + let(:actor) { create(:user) } + + it 'returns false' do + expect(lfs_token.type).to eq(:lfs_token) + end + end + end end diff --git a/spec/lib/gitlab/multi_collection_paginator_spec.rb b/spec/lib/gitlab/multi_collection_paginator_spec.rb index 68bd4f93159..28cd704b05a 100644 --- a/spec/lib/gitlab/multi_collection_paginator_spec.rb +++ b/spec/lib/gitlab/multi_collection_paginator_spec.rb @@ -28,7 +28,7 @@ describe Gitlab::MultiCollectionPaginator do expect(paginator.paginate(1)).to eq(all_projects.take(3)) end - it 'fils the second page with a mixture of of the first & second collection' do + it 'fils the second page with a mixture of the first & second collection' do first_collection_element = all_projects.last second_collection_elements = all_groups.take(2) diff --git a/spec/lib/gitlab/private_commit_email_spec.rb b/spec/lib/gitlab/private_commit_email_spec.rb index bc86cd3842a..10bf624bbdd 100644 --- a/spec/lib/gitlab/private_commit_email_spec.rb +++ b/spec/lib/gitlab/private_commit_email_spec.rb @@ -4,6 +4,9 @@ require 'spec_helper' describe Gitlab::PrivateCommitEmail do let(:hostname) { Gitlab::CurrentSettings.current_application_settings.commit_email_hostname } + let(:id) { 1 } + let(:valid_email) { "#{id}-foo@#{hostname}" } + let(:invalid_email) { "#{id}-foo@users.noreply.bar.com" } context '.regex' do subject { described_class.regex } @@ -16,18 +19,25 @@ describe Gitlab::PrivateCommitEmail do end context '.user_id_for_email' do - let(:id) { 1 } - it 'parses user id from email' do - email = "#{id}-foo@#{hostname}" - - expect(described_class.user_id_for_email(email)).to eq(id) + expect(described_class.user_id_for_email(valid_email)).to eq(id) end it 'returns nil on invalid commit email' do - email = "#{id}-foo@users.noreply.bar.com" + expect(described_class.user_id_for_email(invalid_email)).to be_nil + end + end + + context '.user_ids_for_email' do + it 'returns deduplicated user IDs for each valid email' do + result = described_class.user_ids_for_emails([valid_email, valid_email, invalid_email]) + + expect(result).to eq([id]) + end - expect(described_class.user_id_for_email(email)).to be_nil + it 'returns an empty array with no valid emails' do + result = described_class.user_ids_for_emails([invalid_email]) + expect(result).to eq([]) end end diff --git a/spec/lib/gitlab/profiler_spec.rb b/spec/lib/gitlab/profiler_spec.rb index 4059188fba1..8bb0c1a0b8a 100644 --- a/spec/lib/gitlab/profiler_spec.rb +++ b/spec/lib/gitlab/profiler_spec.rb @@ -43,31 +43,16 @@ describe Gitlab::Profiler do it 'uses the user for auth if given' do user = double(:user) - user_token = 'user' - allow(user).to receive_message_chain(:personal_access_tokens, :active, :pluck, :first).and_return(user_token) - - expect(app).to receive(:get).with('/', nil, 'Private-Token' => user_token) - expect(app).to receive(:get).with('/api/v4/users') + expect(described_class).to receive(:with_user).with(user) described_class.profile('/', user: user) end - context 'when providing a user without a personal access token' do - it 'raises an error' do - user = double(:user) - allow(user).to receive_message_chain(:personal_access_tokens, :active, :pluck).and_return([]) - - expect { described_class.profile('/', user: user) }.to raise_error('Your user must have a personal_access_token') - end - end - it 'uses the private_token for auth if both it and user are set' do user = double(:user) - user_token = 'user' - - allow(user).to receive_message_chain(:personal_access_tokens, :active, :pluck, :first).and_return(user_token) + expect(described_class).to receive(:with_user).with(nil).and_call_original expect(app).to receive(:get).with('/', nil, 'Private-Token' => private_token) expect(app).to receive(:get).with('/api/v4/users') @@ -210,6 +195,29 @@ describe Gitlab::Profiler do end end + describe '.with_user' do + context 'when the user is set' do + let(:user) { double(:user) } + + it 'overrides auth in ApplicationController to use the given user' do + expect(described_class.with_user(user) { ApplicationController.new.current_user }).to eq(user) + end + + it 'cleans up ApplicationController afterwards' do + expect { described_class.with_user(user) { } } + .to not_change { ActionController.instance_methods(false) } + end + end + + context 'when the user is nil' do + it 'does not define methods on ApplicationController' do + expect(ApplicationController).not_to receive(:define_method) + + described_class.with_user(nil) { } + end + end + end + describe '.log_load_times_by_model' do it 'logs the model, query count, and time by slowest first' do expect(null_logger).to receive(:load_times_by_model).and_return( diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 4a0dc3686ec..6831274d37c 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -54,11 +54,18 @@ describe Gitlab::ProjectSearchResults do end it 'finds by name' do - expect(results.map(&:first)).to include(expected_file_by_name) + expect(results.map(&:filename)).to include(expected_file_by_name) + end + + it "loads all blobs for filename matches in single batch" do + expect(Gitlab::Git::Blob).to receive(:batch).once.and_call_original + + expected = project.repository.search_files_by_name(query, 'master') + expect(results.map(&:filename)).to include(*expected) end it 'finds by content' do - blob = results.select { |result| result.first == expected_file_by_content }.flatten.last + blob = results.select { |result| result.filename == expected_file_by_content }.flatten.last expect(blob.filename).to eq(expected_file_by_content) end @@ -122,126 +129,6 @@ describe Gitlab::ProjectSearchResults do let(:blob_type) { 'blobs' } let(:entity) { project } end - - describe 'parsing results' do - let(:results) { project.repository.search_files_by_content('feature', 'master') } - let(:search_result) { results.first } - - subject { described_class.parse_search_result(search_result) } - - it "returns a valid FoundBlob" do - is_expected.to be_an Gitlab::SearchResults::FoundBlob - expect(subject.id).to be_nil - expect(subject.path).to eq('CHANGELOG') - expect(subject.filename).to eq('CHANGELOG') - expect(subject.basename).to eq('CHANGELOG') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(188) - expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n") - end - - context 'when the matching filename contains a colon' do - let(:search_result) { "master:testdata/project::function1.yaml\x001\x00---\n" } - - it 'returns a valid FoundBlob' do - expect(subject.filename).to eq('testdata/project::function1.yaml') - expect(subject.basename).to eq('testdata/project::function1') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(1) - expect(subject.data).to eq("---\n") - end - end - - context 'when the matching content contains a number surrounded by colons' do - let(:search_result) { "master:testdata/foo.txt\x001\x00blah:9:blah" } - - it 'returns a valid FoundBlob' do - expect(subject.filename).to eq('testdata/foo.txt') - expect(subject.basename).to eq('testdata/foo') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(1) - expect(subject.data).to eq('blah:9:blah') - end - end - - context 'when the matching content contains multiple null bytes' do - let(:search_result) { "master:testdata/foo.txt\x001\x00blah\x001\x00foo" } - - it 'returns a valid FoundBlob' do - expect(subject.filename).to eq('testdata/foo.txt') - expect(subject.basename).to eq('testdata/foo') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(1) - expect(subject.data).to eq("blah\x001\x00foo") - end - end - - context 'when the search result ends with an empty line' do - let(:results) { project.repository.search_files_by_content('Role models', 'master') } - - it 'returns a valid FoundBlob that ends with an empty line' do - expect(subject.filename).to eq('files/markdown/ruby-style-guide.md') - expect(subject.basename).to eq('files/markdown/ruby-style-guide') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(1) - expect(subject.data).to eq("# Prelude\n\n> Role models are important. <br/>\n> -- Officer Alex J. Murphy / RoboCop\n\n") - end - end - - context 'when the search returns non-ASCII data' do - context 'with UTF-8' do - let(:results) { project.repository.search_files_by_content('файл', 'master') } - - it 'returns results as UTF-8' do - expect(subject.filename).to eq('encoding/russian.rb') - expect(subject.basename).to eq('encoding/russian') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(1) - expect(subject.data).to eq("Хороший файл\n") - end - end - - context 'with UTF-8 in the filename' do - let(:results) { project.repository.search_files_by_content('webhook', 'master') } - - it 'returns results as UTF-8' do - expect(subject.filename).to eq('encoding/テスト.txt') - expect(subject.basename).to eq('encoding/テスト') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(3) - expect(subject.data).to include('WebHookの確認') - end - end - - context 'with ISO-8859-1' do - let(:search_result) { "master:encoding/iso8859.txt\x001\x00\xC4\xFC\nmaster:encoding/iso8859.txt\x002\x00\nmaster:encoding/iso8859.txt\x003\x00foo\n".force_encoding(Encoding::ASCII_8BIT) } - - it 'returns results as UTF-8' do - expect(subject.filename).to eq('encoding/iso8859.txt') - expect(subject.basename).to eq('encoding/iso8859') - expect(subject.ref).to eq('master') - expect(subject.startline).to eq(1) - expect(subject.data).to eq("Äü\n\nfoo\n") - end - end - end - - context "when filename has extension" do - let(:search_result) { "master:CONTRIBUTE.md\x005\x00- [Contribute to GitLab](#contribute-to-gitlab)\n" } - - it { expect(subject.path).to eq('CONTRIBUTE.md') } - it { expect(subject.filename).to eq('CONTRIBUTE.md') } - it { expect(subject.basename).to eq('CONTRIBUTE') } - end - - context "when file under directory" do - let(:search_result) { "master:a/b/c.md\x005\x00a b c\n" } - - it { expect(subject.path).to eq('a/b/c.md') } - it { expect(subject.filename).to eq('a/b/c.md') } - it { expect(subject.basename).to eq('a/b/c') } - end - end end describe 'wiki search' do diff --git a/spec/lib/gitlab/prometheus/query_variables_spec.rb b/spec/lib/gitlab/prometheus/query_variables_spec.rb new file mode 100644 index 00000000000..78974cadb69 --- /dev/null +++ b/spec/lib/gitlab/prometheus/query_variables_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Prometheus::QueryVariables do + describe '.call' do + set(:environment) { create(:environment) } + let(:slug) { environment.slug } + + subject { described_class.call(environment) } + + it { is_expected.to include(ci_environment_slug: slug) } + + it do + is_expected.to include(environment_filter: + %{container_name!="POD",environment="#{slug}"}) + end + + context 'without deployment platform' do + it { is_expected.to include(kube_namespace: '') } + end + + context 'with deplyoment platform' do + let(:kube_namespace) { environment.deployment_platform.actual_namespace } + + before do + create(:cluster, :provided_by_user, projects: [environment.project]) + end + + it { is_expected.to include(kube_namespace: kube_namespace) } + end + end +end diff --git a/spec/lib/gitlab/repository_cache_spec.rb b/spec/lib/gitlab/repository_cache_spec.rb index 741ee12633f..1b9a8b4ab0d 100644 --- a/spec/lib/gitlab/repository_cache_spec.rb +++ b/spec/lib/gitlab/repository_cache_spec.rb @@ -4,14 +4,14 @@ describe Gitlab::RepositoryCache do let(:backend) { double('backend').as_null_object } let(:project) { create(:project) } let(:repository) { project.repository } - let(:namespace) { "#{repository.full_path}:#{project.id}" } + let(:namespace) { "project:#{project.id}" } let(:cache) { described_class.new(repository, backend: backend) } describe '#cache_key' do subject { cache.cache_key(:foo) } it 'includes the namespace' do - expect(subject).to eq "foo:#{namespace}" + expect(subject).to eq "#{namespace}:foo" end context 'with a given namespace' do @@ -22,7 +22,7 @@ describe Gitlab::RepositoryCache do end it 'includes the full namespace' do - expect(subject).to eq "foo:#{namespace}:#{extra_namespace}" + expect(subject).to eq "#{namespace}:#{extra_namespace}:foo" end end end @@ -30,21 +30,21 @@ describe Gitlab::RepositoryCache do describe '#expire' do it 'expires the given key from the cache' do cache.expire(:foo) - expect(backend).to have_received(:delete).with("foo:#{namespace}") + expect(backend).to have_received(:delete).with("#{namespace}:foo") end end describe '#fetch' do it 'fetches the given key from the cache' do cache.fetch(:bar) - expect(backend).to have_received(:fetch).with("bar:#{namespace}") + expect(backend).to have_received(:fetch).with("#{namespace}:bar") end it 'accepts a block' do p = -> {} cache.fetch(:baz, &p) - expect(backend).to have_received(:fetch).with("baz:#{namespace}", &p) + expect(backend).to have_received(:fetch).with("#{namespace}:baz", &p) end end @@ -67,7 +67,7 @@ describe Gitlab::RepositoryCache do end it 'caches the value' do - expect(backend).to receive(:write).with("#{key}:#{namespace}", true) + expect(backend).to receive(:write).with("#{namespace}:#{key}", true) cache.fetch_without_caching_false(key) { true } end @@ -83,7 +83,7 @@ describe Gitlab::RepositoryCache do end it 'does not cache the value' do - expect(backend).not_to receive(:write).with("#{key}:#{namespace}", true) + expect(backend).not_to receive(:write).with("#{namespace}:#{key}", true) cache.fetch_without_caching_false(key, &p) end @@ -92,7 +92,7 @@ describe Gitlab::RepositoryCache do context 'when the cached value is truthy' do before do - backend.write("#{key}:#{namespace}", true) + backend.write("#{namespace}:#{key}", true) end it 'returns the cached value' do @@ -116,7 +116,7 @@ describe Gitlab::RepositoryCache do context 'when the cached value is falsey' do before do - backend.write("#{key}:#{namespace}", false) + backend.write("#{namespace}:#{key}", false) end it 'returns the result of the block' do @@ -126,7 +126,7 @@ describe Gitlab::RepositoryCache do end it 'writes the truthy value to the cache' do - expect(backend).to receive(:write).with("#{key}:#{namespace}", 'block result') + expect(backend).to receive(:write).with("#{namespace}:#{key}", 'block result') cache.fetch_without_caching_false(key) { 'block result' } end diff --git a/spec/lib/gitlab/search/found_blob_spec.rb b/spec/lib/gitlab/search/found_blob_spec.rb new file mode 100644 index 00000000000..74157e5c67c --- /dev/null +++ b/spec/lib/gitlab/search/found_blob_spec.rb @@ -0,0 +1,138 @@ +# coding: utf-8 + +require 'spec_helper' + +describe Gitlab::Search::FoundBlob do + describe 'parsing results' do + let(:project) { create(:project, :public, :repository) } + let(:results) { project.repository.search_files_by_content('feature', 'master') } + let(:search_result) { results.first } + + subject { described_class.new(content_match: search_result, project: project) } + + it "returns a valid FoundBlob" do + is_expected.to be_an described_class + expect(subject.id).to be_nil + expect(subject.path).to eq('CHANGELOG') + expect(subject.filename).to eq('CHANGELOG') + expect(subject.basename).to eq('CHANGELOG') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(188) + expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n") + end + + it "doesn't parses content if not needed" do + expect(subject).not_to receive(:parse_search_result) + expect(subject.project_id).to eq(project.id) + expect(subject.binary_filename).to eq('CHANGELOG') + end + + it "parses content only once when needed" do + expect(subject).to receive(:parse_search_result).once.and_call_original + expect(subject.filename).to eq('CHANGELOG') + expect(subject.startline).to eq(188) + end + + context 'when the matching filename contains a colon' do + let(:search_result) { "master:testdata/project::function1.yaml\x001\x00---\n" } + + it 'returns a valid FoundBlob' do + expect(subject.filename).to eq('testdata/project::function1.yaml') + expect(subject.basename).to eq('testdata/project::function1') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(1) + expect(subject.data).to eq("---\n") + end + end + + context 'when the matching content contains a number surrounded by colons' do + let(:search_result) { "master:testdata/foo.txt\x001\x00blah:9:blah" } + + it 'returns a valid FoundBlob' do + expect(subject.filename).to eq('testdata/foo.txt') + expect(subject.basename).to eq('testdata/foo') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(1) + expect(subject.data).to eq('blah:9:blah') + end + end + + context 'when the matching content contains multiple null bytes' do + let(:search_result) { "master:testdata/foo.txt\x001\x00blah\x001\x00foo" } + + it 'returns a valid FoundBlob' do + expect(subject.filename).to eq('testdata/foo.txt') + expect(subject.basename).to eq('testdata/foo') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(1) + expect(subject.data).to eq("blah\x001\x00foo") + end + end + + context 'when the search result ends with an empty line' do + let(:results) { project.repository.search_files_by_content('Role models', 'master') } + + it 'returns a valid FoundBlob that ends with an empty line' do + expect(subject.filename).to eq('files/markdown/ruby-style-guide.md') + expect(subject.basename).to eq('files/markdown/ruby-style-guide') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(1) + expect(subject.data).to eq("# Prelude\n\n> Role models are important. <br/>\n> -- Officer Alex J. Murphy / RoboCop\n\n") + end + end + + context 'when the search returns non-ASCII data' do + context 'with UTF-8' do + let(:results) { project.repository.search_files_by_content('файл', 'master') } + + it 'returns results as UTF-8' do + expect(subject.filename).to eq('encoding/russian.rb') + expect(subject.basename).to eq('encoding/russian') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(1) + expect(subject.data).to eq("Хороший файл\n") + end + end + + context 'with UTF-8 in the filename' do + let(:results) { project.repository.search_files_by_content('webhook', 'master') } + + it 'returns results as UTF-8' do + expect(subject.filename).to eq('encoding/テスト.txt') + expect(subject.basename).to eq('encoding/テスト') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(3) + expect(subject.data).to include('WebHookの確認') + end + end + + context 'with ISO-8859-1' do + let(:search_result) { "master:encoding/iso8859.txt\x001\x00\xC4\xFC\nmaster:encoding/iso8859.txt\x002\x00\nmaster:encoding/iso8859.txt\x003\x00foo\n".force_encoding(Encoding::ASCII_8BIT) } + + it 'returns results as UTF-8' do + expect(subject.filename).to eq('encoding/iso8859.txt') + expect(subject.basename).to eq('encoding/iso8859') + expect(subject.ref).to eq('master') + expect(subject.startline).to eq(1) + expect(subject.data).to eq("Äü\n\nfoo\n") + end + end + end + + context "when filename has extension" do + let(:search_result) { "master:CONTRIBUTE.md\x005\x00- [Contribute to GitLab](#contribute-to-gitlab)\n" } + + it { expect(subject.path).to eq('CONTRIBUTE.md') } + it { expect(subject.filename).to eq('CONTRIBUTE.md') } + it { expect(subject.basename).to eq('CONTRIBUTE') } + end + + context "when file under directory" do + let(:search_result) { "master:a/b/c.md\x005\x00a b c\n" } + + it { expect(subject.path).to eq('a/b/c.md') } + it { expect(subject.filename).to eq('a/b/c.md') } + it { expect(subject.basename).to eq('a/b/c') } + end + end +end diff --git a/spec/lib/gitlab/sentry_spec.rb b/spec/lib/gitlab/sentry_spec.rb index d3b41b27b80..1128eaf8560 100644 --- a/spec/lib/gitlab/sentry_spec.rb +++ b/spec/lib/gitlab/sentry_spec.rb @@ -19,14 +19,15 @@ describe Gitlab::Sentry do end it 'raises the exception if it should' do - expect(described_class).to receive(:should_raise?).and_return(true) + expect(described_class).to receive(:should_raise_for_dev?).and_return(true) expect { described_class.track_exception(exception) } .to raise_error(RuntimeError) end context 'when exceptions should not be raised' do before do - allow(described_class).to receive(:should_raise?).and_return(false) + allow(described_class).to receive(:should_raise_for_dev?).and_return(false) + allow(Gitlab::CorrelationId).to receive(:current_id).and_return('cid') end it 'logs the exception with all attributes passed' do @@ -35,8 +36,14 @@ describe Gitlab::Sentry do issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1' } + expected_tags = { + correlation_id: 'cid' + } + expect(Raven).to receive(:capture_exception) - .with(exception, extra: a_hash_including(expected_extras)) + .with(exception, + tags: a_hash_including(expected_tags), + extra: a_hash_including(expected_extras)) described_class.track_exception( exception, @@ -58,6 +65,7 @@ describe Gitlab::Sentry do before do allow(described_class).to receive(:enabled?).and_return(true) + allow(Gitlab::CorrelationId).to receive(:current_id).and_return('cid') end it 'calls Raven.capture_exception' do @@ -66,8 +74,14 @@ describe Gitlab::Sentry do issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1' } + expected_tags = { + correlation_id: 'cid' + } + expect(Raven).to receive(:capture_exception) - .with(exception, extra: a_hash_including(expected_extras)) + .with(exception, + tags: a_hash_including(expected_tags), + extra: a_hash_including(expected_extras)) described_class.track_acceptable_exception( exception, diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 2421b1e5a1a..f773f370ee2 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -12,7 +12,8 @@ describe Gitlab::SidekiqLogging::StructuredLogger do "queue_namespace" => "cronjob", "jid" => "da883554ee4fe414012f5f42", "created_at" => timestamp.to_f, - "enqueued_at" => timestamp.to_f + "enqueued_at" => timestamp.to_f, + "correlation_id" => 'cid' } end let(:logger) { double() } diff --git a/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb b/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb new file mode 100644 index 00000000000..a138ad7c910 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/correlation_injector_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::CorrelationInjector do + class TestWorker + include ApplicationWorker + end + + before do |example| + Sidekiq.client_middleware do |chain| + chain.add described_class + end + end + + after do |example| + Sidekiq.client_middleware do |chain| + chain.remove described_class + end + + Sidekiq::Queues.clear_all + end + + around do |example| + Sidekiq::Testing.fake! do + example.run + end + end + + it 'injects into payload the correlation id' do + expect_any_instance_of(described_class).to receive(:call).and_call_original + + Gitlab::CorrelationId.use_id('new-correlation-id') do + TestWorker.perform_async(1234) + end + + expected_job_params = { + "class" => "TestWorker", + "args" => [1234], + "correlation_id" => "new-correlation-id" + } + + expect(Sidekiq::Queues.jobs_by_worker).to a_hash_including( + "TestWorker" => a_collection_containing_exactly( + a_hash_including(expected_job_params))) + end +end diff --git a/spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb b/spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb new file mode 100644 index 00000000000..94ae4ffa184 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/correlation_logger_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::CorrelationLogger do + class TestWorker + include ApplicationWorker + end + + before do |example| + Sidekiq::Testing.server_middleware do |chain| + chain.add described_class + end + end + + after do |example| + Sidekiq::Testing.server_middleware do |chain| + chain.remove described_class + end + end + + it 'injects into payload the correlation id' do + expect_any_instance_of(described_class).to receive(:call).and_call_original + + expect_any_instance_of(TestWorker).to receive(:perform).with(1234) do + expect(Gitlab::CorrelationId.current_id).to eq('new-correlation-id') + end + + Sidekiq::Client.push( + 'queue' => 'test', + 'class' => TestWorker, + 'args' => [1234], + 'correlation_id' => 'new-correlation-id') + end +end diff --git a/spec/lib/gitlab/template/finders/global_template_finder_spec.rb b/spec/lib/gitlab/template/finders/global_template_finder_spec.rb new file mode 100644 index 00000000000..c7f58fbd2a5 --- /dev/null +++ b/spec/lib/gitlab/template/finders/global_template_finder_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Gitlab::Template::Finders::GlobalTemplateFinder do + let(:base_dir) { Dir.mktmpdir } + + def create_template!(name_with_category) + full_path = File.join(base_dir, name_with_category) + FileUtils.mkdir_p(File.dirname(full_path)) + FileUtils.touch(full_path) + end + + after do + FileUtils.rm_rf(base_dir) + end + + subject(:finder) { described_class.new(base_dir, '', 'Foo' => '', 'Bar' => 'bar') } + + describe '.find' do + it 'finds a template in the Foo category' do + create_template!('test-template') + + expect(finder.find('test-template')).to be_present + end + + it 'finds a template in the Bar category' do + create_template!('bar/test-template') + + expect(finder.find('test-template')).to be_present + end + + it 'does not permit path traversal requests' do + expect { finder.find('../foo') }.to raise_error(/Invalid path/) + end + end +end diff --git a/spec/lib/gitlab/template/finders/repo_template_finders_spec.rb b/spec/lib/gitlab/template/finders/repo_template_finders_spec.rb index 2eabccd5dff..e329d55d837 100644 --- a/spec/lib/gitlab/template/finders/repo_template_finders_spec.rb +++ b/spec/lib/gitlab/template/finders/repo_template_finders_spec.rb @@ -25,6 +25,10 @@ describe Gitlab::Template::Finders::RepoTemplateFinder do expect(result).to eq('files/html/500.html') end + + it 'does not permit path traversal requests' do + expect { finder.find('../foo') }.to raise_error(/Invalid path/) + end end describe '#list_files_for' do diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 8df0facdab3..62970bd8cb6 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -10,8 +10,8 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?(import_url)).to be false end - it 'allows imports from configured SSH host and port' do - import_url = "http://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git" + it 'allows mirroring from configured SSH host and port' do + import_url = "ssh://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git" expect(described_class.blocked_url?(import_url)).to be false end @@ -29,24 +29,46 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true end + it 'returns true for bad protocol on configured web/SSH host and ports' do + web_url = "javascript://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git%0aalert(1)" + expect(described_class.blocked_url?(web_url)).to be true + + ssh_url = "javascript://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git%0aalert(1)" + expect(described_class.blocked_url?(ssh_url)).to be true + end + it 'returns true for localhost IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:0:0:0]/foo/foo.git')).to be true expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git')).to be true - expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true - expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::]/foo/foo.git')).to be true end it 'returns true for loopback IP' do expect(described_class.blocked_url?('https://127.0.0.2/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true end it 'returns true for alternative version of 127.0.0.1 (0177.1)' do expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (017700000001)' do + expect(described_class.blocked_url?('https://017700000001:65535/foo/foo.git')).to be true + end + it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (0x7f.0.0.1)' do + expect(described_class.blocked_url?('https://0x7f.0.0.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f000001)' do + expect(described_class.blocked_url?('https://0x7f000001:65535/foo/foo.git')).to be true + end + it 'returns true for alternative version of 127.0.0.1 (2130706433)' do expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true end @@ -55,6 +77,27 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (127.0.1)' do + expect(described_class.blocked_url?('https://127.0.1:65535/foo/foo.git')).to be true + end + + context 'with ipv6 mapped address' do + it 'returns true for localhost IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:0.0.0.0]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:0.0.0.0]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:0:0]/foo/foo.git')).to be true + end + + it 'returns true for loopback IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.1]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:127.0.0.1]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:7f00:1]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.2]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:127.0.0.2]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:7f00:2]/foo/foo.git')).to be true + end + end + it 'returns true for a non-alphanumeric hostname' do stub_resolv @@ -78,7 +121,22 @@ describe Gitlab::UrlBlocker do end context 'when allow_local_network is' do - let(:local_ips) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] } + let(:local_ips) do + [ + '192.168.1.2', + '[0:0:0:0:0:ffff:192.168.1.2]', + '[::ffff:c0a8:102]', + '10.0.0.2', + '[0:0:0:0:0:ffff:10.0.0.2]', + '[::ffff:a00:2]', + '172.16.0.2', + '[0:0:0:0:0:ffff:172.16.0.2]', + '[::ffff:ac10:20]', + '[feef::1]', + '[fee2::]', + '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]' + ] + end let(:fake_domain) { 'www.fakedomain.fake' } context 'true (default)' do @@ -109,10 +167,14 @@ describe Gitlab::UrlBlocker do expect(described_class).not_to be_blocked_url('http://169.254.168.100') end - # This is blocked due to the hostname check: https://gitlab.com/gitlab-org/gitlab-ce/issues/50227 - it 'blocks IPv6 link-local endpoints' do - expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]') - expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]') + it 'allows IPv6 link-local endpoints' do + expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]') + expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]') + expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]') + expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]') + expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]') + expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]') + expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]') end end @@ -135,14 +197,20 @@ describe Gitlab::UrlBlocker do end it 'blocks IPv6 link-local endpoints' do + expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]', allow_local_network: false) expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a9fe]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]', allow_local_network: false) expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]', allow_local_network: false) - expect(described_class).to be_blocked_url('http://[FE80::C800:EFF:FE74:8]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a864]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[fe80::c800:eff:fe74:8]', allow_local_network: false) end end def stub_domain_resolv(domain, ip) - allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([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) + allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address]) + allow(address).to receive(:ipv6_v4mapped?).and_return(false) end def unstub_domain_resolv @@ -181,6 +249,57 @@ describe Gitlab::UrlBlocker do end end end + + context 'when ascii_only is true' do + it 'returns true for unicode domain' do + expect(described_class.blocked_url?('https://𝕘itⅼαƄ.com/foo/foo.bar', ascii_only: true)).to be true + end + + it 'returns true for unicode tld' do + expect(described_class.blocked_url?('https://gitlab.ᴄοｍ/foo/foo.bar', ascii_only: true)).to be true + end + + it 'returns true for unicode path' do + expect(described_class.blocked_url?('https://gitlab.com/𝒇οο/𝒇οο.Ƅαꮁ', ascii_only: true)).to be true + end + + it 'returns true for IDNA deviations' do + expect(described_class.blocked_url?('https://mißile.com/foo/foo.bar', ascii_only: true)).to be true + expect(described_class.blocked_url?('https://miςςile.com/foo/foo.bar', ascii_only: true)).to be true + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.bar', ascii_only: true)).to be true + expect(described_class.blocked_url?('https://gitlab.com/foo/foo.bar', ascii_only: true)).to be true + end + end + end + + describe '#validate_hostname!' do + let(:ip_addresses) do + [ + '2001:db8:1f70::999:de8:7648:6e8', + 'FE80::C800:EFF:FE74:8', + '::ffff:127.0.0.1', + '::ffff:169.254.168.100', + '::ffff:7f00:1', + '0:0:0:0:0:ffff:0.0.0.0', + 'localhost', + '127.0.0.1', + '127.000.000.001', + '0x7f000001', + '0x7f.0.0.1', + '0x7f.0.0.1', + '017700000001', + '0177.1', + '2130706433', + '::', + '::1' + ] + end + + it 'does not raise error for valid Ip addresses' do + ip_addresses.each do |ip| + expect { described_class.send(:validate_hostname!, ip) }.not_to raise_error + end + end end # Resolv does not support resolving UTF-8 domain names diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index b41a81a8167..6e98a999766 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -41,6 +41,7 @@ describe Gitlab::UrlSanitizer do false | '123://invalid:url' false | 'valid@project:url.git' false | 'valid:pass@project:url.git' + false | %w(test array) true | 'ssh://example.com' true | 'ssh://:@example.com' true | 'ssh://foo@example.com' diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index b212d2b05f2..deb19fe1a4b 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -17,8 +17,12 @@ describe Gitlab::UsageData do gcp_cluster = create(:cluster, :provided_by_gcp) create(:cluster, :provided_by_user) create(:cluster, :provided_by_user, :disabled) + create(:cluster, :group) + create(:cluster, :group, :disabled) + create(:cluster, :group, :disabled) create(:clusters_applications_helm, :installed, cluster: gcp_cluster) create(:clusters_applications_ingress, :installed, cluster: gcp_cluster) + create(:clusters_applications_cert_managers, :installed, cluster: gcp_cluster) create(:clusters_applications_prometheus, :installed, cluster: gcp_cluster) create(:clusters_applications_runner, :installed, cluster: gcp_cluster) create(:clusters_applications_knative, :installed, cluster: gcp_cluster) @@ -76,11 +80,16 @@ describe Gitlab::UsageData do environments clusters clusters_enabled + project_clusters_enabled + group_clusters_enabled clusters_disabled + project_clusters_disabled + group_clusters_disabled clusters_platforms_gke clusters_platforms_user clusters_applications_helm clusters_applications_ingress + clusters_applications_cert_managers clusters_applications_prometheus clusters_applications_runner clusters_applications_knative @@ -125,12 +134,18 @@ describe Gitlab::UsageData do expect(count_data[:projects_slack_notifications_active]).to eq(2) expect(count_data[:projects_slack_slash_active]).to eq(1) - expect(count_data[:clusters_enabled]).to eq(6) - expect(count_data[:clusters_disabled]).to eq(1) + expect(count_data[:clusters_enabled]).to eq(7) + expect(count_data[:project_clusters_enabled]).to eq(6) + expect(count_data[:group_clusters_enabled]).to eq(1) + expect(count_data[:clusters_disabled]).to eq(3) + expect(count_data[:project_clusters_disabled]).to eq(1) + expect(count_data[:group_clusters_disabled]).to eq(2) + expect(count_data[:group_clusters_enabled]).to eq(1) expect(count_data[:clusters_platforms_gke]).to eq(1) expect(count_data[:clusters_platforms_user]).to eq(1) expect(count_data[:clusters_applications_helm]).to eq(1) expect(count_data[:clusters_applications_ingress]).to eq(1) + expect(count_data[:clusters_applications_cert_managers]).to eq(1) expect(count_data[:clusters_applications_prometheus]).to eq(1) expect(count_data[:clusters_applications_runner]).to eq(1) expect(count_data[:clusters_applications_knative]).to eq(1) @@ -198,4 +213,29 @@ describe Gitlab::UsageData do expect(described_class.count(relation, fallback: 15)).to eq(15) end end + + describe '#approximate_counts' do + it 'gets approximate counts for selected models' do + create(:label) + + expect(Gitlab::Database::Count).to receive(:approximate_counts) + .with(described_class::APPROXIMATE_COUNT_MODELS).once.and_call_original + + counts = described_class.approximate_counts.values + + expect(counts.count).to eq(described_class::APPROXIMATE_COUNT_MODELS.count) + expect(counts.any? { |count| count < 0 }).to be_falsey + end + + it 'returns default values if counts can not be retrieved' do + described_class::APPROXIMATE_COUNT_MODELS.map do |model| + model.name.underscore.pluralize.to_sym + end + + expect(Gitlab::Database::Count).to receive(:approximate_counts) + .and_return({}) + + expect(described_class.approximate_counts.values.uniq).to eq([-1]) + end + end end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index ad2c9d7f2af..f5a4b7e2ebf 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -2,7 +2,33 @@ require 'spec_helper' describe Gitlab::Utils do delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string, - :bytes_to_megabytes, :append_path, to: :described_class + :bytes_to_megabytes, :append_path, :check_path_traversal!, to: :described_class + + describe '.check_path_traversal!' do + it 'detects path traversal at the start of the string' do + expect { check_path_traversal!('../foo') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the start of the string, even to just the subdirectory' do + expect { check_path_traversal!('../') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal in the middle of the string' do + expect { check_path_traversal!('foo/../../bar') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string when slash-terminates' do + expect { check_path_traversal!('foo/../') }.to raise_error(/Invalid path/) + end + + it 'detects path traversal at the end of the string' do + expect { check_path_traversal!('foo/..') }.to raise_error(/Invalid path/) + end + + it 'does nothing for a safe string' do + expect(check_path_traversal!('./foo')).to eq('./foo') + end + end describe '.slugify' do { @@ -18,6 +44,12 @@ describe Gitlab::Utils do end end + describe '.nlbr' do + it 'replaces new lines with <br>' do + expect(described_class.nlbr("<b>hello</b>\n<i>world</i>".freeze)).to eq("hello<br>world") + end + end + describe '.remove_line_breaks' do using RSpec::Parameterized::TableSyntax @@ -127,4 +159,42 @@ describe Gitlab::Utils do end end end + + describe '.ensure_utf8_size' do + context 'string is has less bytes than expected' do + it 'backfills string with null characters' do + transformed = described_class.ensure_utf8_size('a' * 10, bytes: 32) + + expect(transformed.bytesize).to eq 32 + expect(transformed).to eq(('a' * 10) + ('0' * 22)) + end + end + + context 'string size is exactly the one that is expected' do + it 'returns original value' do + transformed = described_class.ensure_utf8_size('a' * 32, bytes: 32) + + expect(transformed).to eq 'a' * 32 + expect(transformed.bytesize).to eq 32 + end + end + + context 'when string contains a few multi-byte UTF characters' do + it 'backfills string with null characters' do + transformed = described_class.ensure_utf8_size('❤' * 6, bytes: 32) + + expect(transformed).to eq '❤❤❤❤❤❤' + ('0' * 14) + expect(transformed.bytesize).to eq 32 + end + end + + context 'when string has multiple multi-byte UTF chars exceeding 32 bytes' do + it 'truncates string to 32 characters and backfills it if needed' do + transformed = described_class.ensure_utf8_size('❤' * 18, bytes: 32) + + expect(transformed).to eq(('❤' * 10) + ('0' * 2)) + expect(transformed.bytesize).to eq 32 + end + end + end end |