diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-13 03:09:34 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-13 03:09:34 +0300 |
commit | 3cd08f4bf96cda3e9d3abf233095107832b17c20 (patch) | |
tree | dc09a618783a79d70f2a404374d4b850ccf9cc84 /spec | |
parent | dd4bee69b7d55620f7dc9db8c36b478bd4959755 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
25 files changed, 509 insertions, 156 deletions
diff --git a/spec/factories/x509_certificate.rb b/spec/factories/x509_certificate.rb index 819ad0704dc..37912548434 100644 --- a/spec/factories/x509_certificate.rb +++ b/spec/factories/x509_certificate.rb @@ -8,5 +8,6 @@ FactoryBot.define do email { 'gitlab@example.org' } serial_number { 278969561018901340486471282831158785578 } x509_issuer + certificate_status { :good } end end diff --git a/spec/fixtures/lib/elasticsearch/query_with_cursor.json b/spec/fixtures/lib/elasticsearch/query_with_cursor.json new file mode 100644 index 00000000000..1264fdb0322 --- /dev/null +++ b/spec/fixtures/lib/elasticsearch/query_with_cursor.json @@ -0,0 +1,43 @@ +{ + "query": { + "bool": { + "must": [ + { + "match_phrase": { + "kubernetes.pod.name": { + "query": "production-6866bc8974-m4sk4" + } + } + }, + { + "match_phrase": { + "kubernetes.namespace": { + "query": "autodevops-deploy-9-production" + } + } + } + ] + } + }, + "sort": [ + { + "@timestamp": { + "order": "desc" + } + }, + { + "offset": { + "order": "desc" + } + } + ], + "search_after": [ + 9999934, + 1572449784442 + ], + "_source": [ + "@timestamp", + "message" + ], + "size": 500 +} diff --git a/spec/frontend/monitoring/embed/mock_data.js b/spec/frontend/monitoring/embed/mock_data.js index 1dc31846034..da8eb8c0fc4 100644 --- a/spec/frontend/monitoring/embed/mock_data.js +++ b/spec/frontend/monitoring/embed/mock_data.js @@ -1,4 +1,4 @@ -export const metricsWithData = [15, 16]; +export const metricsWithData = ['15_metric_a', '16_metric_b']; export const groups = [ { @@ -7,41 +7,12 @@ export const groups = [ title: 'Memory Usage (Total)', type: 'area-chart', y_label: 'Total Memory Used', - weight: 4, - metrics: [ - { - id: 'system_metrics_kubernetes_container_memory_total', - metric_id: 15, - }, - ], - }, - { - title: 'Core Usage (Total)', - type: 'area-chart', - y_label: 'Total Cores', - weight: 3, - metrics: [ - { - id: 'system_metrics_kubernetes_container_cores_total', - metric_id: 16, - }, - ], + metrics: null, }, ], }, ]; -export const metrics = [ - { - id: 'system_metrics_kubernetes_container_memory_total', - metric_id: 15, - }, - { - id: 'system_metrics_kubernetes_container_cores_total', - metric_id: 16, - }, -]; - const result = [ { values: [ @@ -60,7 +31,7 @@ export const metricsData = [ { metrics: [ { - metric_id: 15, + metricId: '15_metric_a', result, }, ], @@ -68,7 +39,7 @@ export const metricsData = [ { metrics: [ { - metric_id: 16, + metricId: '16_metric_b', result, }, ], diff --git a/spec/frontend/monitoring/store/utils_spec.js b/spec/frontend/monitoring/store/utils_spec.js index 2bd8af9b7d5..1e5bbc9c113 100644 --- a/spec/frontend/monitoring/store/utils_spec.js +++ b/spec/frontend/monitoring/store/utils_spec.js @@ -213,20 +213,16 @@ describe('mapToDashboardViewModel', () => { expect(getMappedMetric(dashboard)).toEqual({ label: expect.any(String), metricId: expect.any(String), - metric_id: expect.any(String), }); }); - it('creates a metric with a correct ids', () => { + it('creates a metric with a correct id', () => { const dashboard = dashboardWithMetric({ id: 'http_responses', metric_id: 1, }); - expect(getMappedMetric(dashboard)).toMatchObject({ - metricId: '1_http_responses', - metric_id: '1_http_responses', - }); + expect(getMappedMetric(dashboard).metricId).toEqual('1_http_responses'); }); it('creates a metric with a default label', () => { diff --git a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb index 558209cc05c..ff82a6580df 100644 --- a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb @@ -149,10 +149,12 @@ describe Gitlab::BackgroundMigration::LegacyUploadMover, schema: :latest do context 'when an upload belongs to a legacy_diff_note' do let!(:merge_request) { create(:merge_request, source_project: project) } + let!(:note) do create(:legacy_diff_note_on_merge_request, note: 'some note', project: project, noteable: merge_request) end + let(:legacy_upload) do create(:upload, :with_file, :attachment_upload, path: "uploads/-/system/note/attachment/#{note.id}/#{filename}", model: note) @@ -193,6 +195,17 @@ describe Gitlab::BackgroundMigration::LegacyUploadMover, schema: :latest do it_behaves_like 'move error' end + context 'when upload has mount_point nil' do + let(:legacy_upload) do + create(:upload, :with_file, :attachment_upload, + path: "uploads/-/system/note/attachment/#{note.id}/#{filename}", model: note, mount_point: nil) + end + + it_behaves_like 'migrates the file correctly' + it_behaves_like 'legacy local file' + it_behaves_like 'legacy upload deletion' + end + context 'when the file can be handled correctly' do it_behaves_like 'migrates the file correctly' it_behaves_like 'legacy local file' diff --git a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb index 42e446c07c1..a273dcf9e5c 100644 --- a/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb +++ b/spec/lib/gitlab/background_migration/user_mentions/create_resource_user_mention_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require './db/post_migrate/20200128134110_migrate_commit_notes_mentions_to_db' require './db/post_migrate/20200211155539_migrate_merge_request_mentions_to_db' describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, schema: 20200211155539 do @@ -73,11 +74,36 @@ describe Gitlab::BackgroundMigration::UserMentions::CreateResourceUserMention, s it_behaves_like 'resource mentions migration', MigrateMergeRequestMentionsToDb, MergeRequest end + + context 'migrate commit mentions' do + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } + let(:commit) { Commit.new(RepoHelpers.sample_commit, project.becomes(Project)) } + let(:commit_user_mentions) { table(:commit_user_mentions) } + + let!(:note1) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: author.id, note: description_mentions) } + let!(:note2) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: author.id, note: 'sample note') } + let!(:note3) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: author.id, note: description_mentions, system: true) } + + # this not does not have actual mentions + let!(:note4) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: author.id, note: 'note for an email@somesite.com and some other random @ ref' ) } + # this should have pointed to an innexisted commit record in a commits table + # but because commit is not an AR we'll just make it so that it does not have mentions + let!(:note5) { notes.create!(commit_id: 'abc', noteable_type: 'Commit', project_id: project.id, author_id: author.id, note: 'note for an email@somesite.com and some other random @ ref') } + + let(:user_mentions) { commit_user_mentions } + let(:resource) { commit } + + it_behaves_like 'resource notes mentions migration', MigrateCommitNotesMentionsToDb, Commit + end end context 'checks no_quote_columns' do it 'has correct no_quote_columns' do expect(Gitlab::BackgroundMigration::UserMentions::Models::MergeRequest.no_quote_columns).to match([:note_id, :merge_request_id]) end + + it 'commit has correct no_quote_columns' do + expect(Gitlab::BackgroundMigration::UserMentions::Models::Commit.no_quote_columns).to match([:note_id]) + end end end diff --git a/spec/lib/gitlab/elasticsearch/logs_spec.rb b/spec/lib/gitlab/elasticsearch/logs_spec.rb index b2f23e30465..f82c4acb82b 100644 --- a/spec/lib/gitlab/elasticsearch/logs_spec.rb +++ b/spec/lib/gitlab/elasticsearch/logs_spec.rb @@ -20,6 +20,7 @@ describe Gitlab::Elasticsearch::Logs do let(:search) { "foo +bar "} let(:start_time) { "2019-12-13T14:35:34.034Z" } let(:end_time) { "2019-12-13T14:35:34.034Z" } + let(:cursor) { "9999934,1572449784442" } let(:body) { JSON.parse(fixture_file('lib/elasticsearch/query.json')) } let(:body_with_container) { JSON.parse(fixture_file('lib/elasticsearch/query_with_container.json')) } @@ -27,6 +28,7 @@ describe Gitlab::Elasticsearch::Logs do let(:body_with_times) { JSON.parse(fixture_file('lib/elasticsearch/query_with_times.json')) } let(:body_with_start_time) { JSON.parse(fixture_file('lib/elasticsearch/query_with_start_time.json')) } let(:body_with_end_time) { JSON.parse(fixture_file('lib/elasticsearch/query_with_end_time.json')) } + let(:body_with_cursor) { JSON.parse(fixture_file('lib/elasticsearch/query_with_cursor.json')) } RSpec::Matchers.define :a_hash_equal_to_json do |expected| match do |actual| @@ -39,42 +41,49 @@ describe Gitlab::Elasticsearch::Logs do expect(client).to receive(:search).with(body: a_hash_equal_to_json(body)).and_return(es_response) result = subject.pod_logs(namespace, pod_name) - expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1]) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) end it 'can further filter the logs by container name' do expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_container)).and_return(es_response) - result = subject.pod_logs(namespace, pod_name, container_name) - expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1]) + result = subject.pod_logs(namespace, pod_name, container_name: container_name) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) end it 'can further filter the logs by search' do expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_search)).and_return(es_response) - result = subject.pod_logs(namespace, pod_name, nil, search) - expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1]) + result = subject.pod_logs(namespace, pod_name, search: search) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) end it 'can further filter the logs by start_time and end_time' do expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_times)).and_return(es_response) - result = subject.pod_logs(namespace, pod_name, nil, nil, start_time, end_time) - expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1]) + result = subject.pod_logs(namespace, pod_name, start_time: start_time, end_time: end_time) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) end it 'can further filter the logs by only start_time' do expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_start_time)).and_return(es_response) - result = subject.pod_logs(namespace, pod_name, nil, nil, start_time) - expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1]) + result = subject.pod_logs(namespace, pod_name, start_time: start_time) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) end it 'can further filter the logs by only end_time' do expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_end_time)).and_return(es_response) - result = subject.pod_logs(namespace, pod_name, nil, nil, nil, end_time) - expect(result).to eq([es_message_4, es_message_3, es_message_2, es_message_1]) + result = subject.pod_logs(namespace, pod_name, end_time: end_time) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) + end + + it 'can search after a cursor' do + expect(client).to receive(:search).with(body: a_hash_equal_to_json(body_with_cursor)).and_return(es_response) + + result = subject.pod_logs(namespace, pod_name, cursor: cursor) + expect(result).to eq(logs: [es_message_4, es_message_3, es_message_2, es_message_1], cursor: cursor) end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d3daa7c0260..cfe73ead9cc 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -492,50 +492,6 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe '#fetch_repository_as_mirror' do - let(:new_repository) do - Gitlab::Git::Repository.new('default', 'my_project.git', '', 'group/project') - end - - subject { new_repository.fetch_repository_as_mirror(repository) } - - before do - new_repository.create_repository - end - - after do - new_repository.remove - end - - it 'fetches a repository as a mirror remote' do - subject - - expect(refs(new_repository_path)).to eq(refs(repository_path)) - end - - context 'with keep-around refs' do - let(:sha) { SeedRepo::Commit::ID } - let(:keep_around_ref) { "refs/keep-around/#{sha}" } - let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } - - before do - repository_rugged.references.create(keep_around_ref, sha, force: true) - repository_rugged.references.create(tmp_ref, sha, force: true) - end - - it 'includes the temporary and keep-around refs' do - subject - - expect(refs(new_repository_path)).to include(keep_around_ref) - expect(refs(new_repository_path)).to include(tmp_ref) - end - end - - def new_repository_path - File.join(TestEnv.repos_path, new_repository.relative_path) - end - end - describe '#fetch_remote' do it 'delegates to the gitaly RepositoryService' do ssh_auth = double(:ssh_auth) @@ -2181,4 +2137,49 @@ describe Gitlab::Git::Repository, :seed_helper do end end end + + describe '#replicate' do + let(:new_repository) do + Gitlab::Git::Repository.new('test_second_storage', TEST_REPO_PATH, '', 'group/project') + end + let(:new_repository_path) { File.join(TestEnv::SECOND_STORAGE_PATH, new_repository.relative_path) } + + subject { new_repository.replicate(repository) } + + before do + stub_storage_settings('test_second_storage' => { + 'gitaly_address' => Gitlab.config.repositories.storages.default.gitaly_address, + 'path' => TestEnv::SECOND_STORAGE_PATH + }) + Gitlab::Shell.new.create_repository('test_second_storage', TEST_REPO_PATH, 'group/project') + end + + after do + Gitlab::Shell.new.remove_repository('test_second_storage', TEST_REPO_PATH) + end + + it 'mirrors the source repository' do + subject + + expect(refs(new_repository_path)).to eq(refs(repository_path)) + end + + context 'with keep-around refs' do + let(:sha) { SeedRepo::Commit::ID } + let(:keep_around_ref) { "refs/keep-around/#{sha}" } + let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } + + before do + repository.write_ref(keep_around_ref, sha) + repository.write_ref(tmp_ref, sha) + end + + it 'includes the temporary and keep-around refs' do + subject + + expect(refs(new_repository_path)).to include(keep_around_ref) + expect(refs(new_repository_path)).to include(tmp_ref) + 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 73ae4cd95ce..2658414d9b0 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -34,19 +34,6 @@ describe Gitlab::GitalyClient::RemoteService do end end - describe '#fetch_internal_remote' do - let(:remote_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } - - it 'sends an fetch_internal_remote message and returns the result value' do - expect_any_instance_of(Gitaly::RemoteService::Stub) - .to receive(:fetch_internal_remote) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - .and_return(double(result: true)) - - expect(client.fetch_internal_remote(remote_repository)).to be(true) - end - end - describe '#find_remote_root_ref' do it 'sends an find_remote_root_ref message and returns the root ref' do expect_any_instance_of(Gitaly::RemoteService::Stub) diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index 503ac57ade6..5f4147f6ff6 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -275,7 +275,18 @@ describe Gitlab::GitalyClient::RepositoryService do end end - describe 'remove' do + describe '#rename' do + it 'sends a rename_repository message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:rename_repository) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_return(double(value: true)) + + client.rename('some/new/path') + end + end + + describe '#remove' do it 'sends a remove_repository message' do expect_any_instance_of(Gitaly::RepositoryService::Stub) .to receive(:remove_repository) @@ -286,14 +297,15 @@ describe Gitlab::GitalyClient::RepositoryService do end end - describe 'rename' do - it 'sends a rename_repository message' do + describe '#replicate' do + let(:source_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '', 'group/project') } + + it 'sends a replicate_repository message' do expect_any_instance_of(Gitaly::RepositoryService::Stub) - .to receive(:rename_repository) + .to receive(:replicate_repository) .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - .and_return(double(value: true)) - client.rename('some/new/path') + client.replicate(source_repository) end end end diff --git a/spec/lib/gitlab/x509/commit_spec.rb b/spec/lib/gitlab/x509/commit_spec.rb index c31e9e4b8e6..07d7eba6b9a 100644 --- a/spec/lib/gitlab/x509/commit_spec.rb +++ b/spec/lib/gitlab/x509/commit_spec.rb @@ -111,6 +111,22 @@ describe Gitlab::X509::Commit do expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) expect(signature.persisted?).to be_truthy end + + context 'revoked certificate' do + let(:x509_issuer) { create(:x509_issuer, user1_issuer_attributes) } + let!(:x509_certificate) { create(:x509_certificate, user1_certificate_attributes.merge(x509_issuer_id: x509_issuer.id, certificate_status: :revoked)) } + + it 'returns an unverified signature' do + expect(signature).to have_attributes( + commit_sha: commit_sha, + project: project, + verification_status: 'unverified' + ) + expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) + expect(signature.persisted?).to be_truthy + end + end end context 'without trusted certificate within store' do diff --git a/spec/migrations/cleanup_empty_commit_user_mentions_spec.rb b/spec/migrations/cleanup_empty_commit_user_mentions_spec.rb new file mode 100644 index 00000000000..7e6afbec520 --- /dev/null +++ b/spec/migrations/cleanup_empty_commit_user_mentions_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200128133510_cleanup_empty_commit_user_mentions') + +describe CleanupEmptyCommitUserMentions, :migration, :sidekiq do + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:notes) { table(:notes) } + + let(:user) { users.create!(name: 'root', email: 'root@example.com', username: 'root', projects_limit: 0) } + let(:group) { namespaces.create!(name: 'group1', path: 'group1', owner_id: user.id) } + let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) } + + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } + let(:commit) { Commit.new(RepoHelpers.sample_commit, project.becomes(Project)) } + let(:commit_user_mentions) { table(:commit_user_mentions) } + + let!(:resource1) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: user.id, note: 'note1 for @root to check') } + let!(:resource2) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: user.id, note: 'note1 for @root to check') } + let!(:resource3) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: user.id, note: 'note1 for @root to check', system: true) } + + # this note is already migrated, as it has a record in the commit_user_mentions table + let!(:resource4) { notes.create!(note: 'note3 for @root to check', commit_id: commit.id, noteable_type: 'Commit') } + let!(:user_mention) { commit_user_mentions.create!(commit_id: commit.id, note_id: resource4.id, mentioned_users_ids: [1]) } + + # these should get cleanup, by the migration + let!(:blank_commit_user_mention1) { commit_user_mentions.create!(commit_id: commit.id, note_id: resource1.id)} + let!(:blank_commit_user_mention2) { commit_user_mentions.create!(commit_id: commit.id, note_id: resource2.id)} + let!(:blank_commit_user_mention3) { commit_user_mentions.create!(commit_id: commit.id, note_id: resource3.id)} + + it 'cleanups blank user mentions' do + expect { migrate! }.to change { commit_user_mentions.count }.by(-3) + end +end diff --git a/spec/migrations/migrate_commit_notes_mentions_to_db_spec.rb b/spec/migrations/migrate_commit_notes_mentions_to_db_spec.rb new file mode 100644 index 00000000000..aa78381ba3a --- /dev/null +++ b/spec/migrations/migrate_commit_notes_mentions_to_db_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200128134110_migrate_commit_notes_mentions_to_db') + +describe MigrateCommitNotesMentionsToDb, :migration, :sidekiq do + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:notes) { table(:notes) } + + let(:user) { users.create!(name: 'root', email: 'root@example.com', username: 'root', projects_limit: 0) } + let(:group) { namespaces.create!(name: 'group1', path: 'group1', owner_id: user.id) } + let(:project) { projects.create!(name: 'gitlab1', path: 'gitlab1', namespace_id: group.id, visibility_level: 0) } + + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') } + let(:commit) { Commit.new(RepoHelpers.sample_commit, project.becomes(Project)) } + let(:commit_user_mentions) { table(:commit_user_mentions) } + + let!(:resource1) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: user.id, note: 'note1 for @root to check') } + let!(:resource2) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: user.id, note: 'note1 for @root to check') } + let!(:resource3) { notes.create!(commit_id: commit.id, noteable_type: 'Commit', project_id: project.id, author_id: user.id, note: 'note1 for @root to check', system: true) } + + # non-migrateable resources + # this note is already migrated, as it has a record in the commit_user_mentions table + let!(:resource4) { notes.create!(note: 'note3 for @root to check', commit_id: commit.id, noteable_type: 'Commit') } + let!(:user_mention) { commit_user_mentions.create!(commit_id: commit.id, note_id: resource4.id, mentioned_users_ids: [1]) } + # this should have pointed to an inexistent commit record in a commits table + # but because commit is not an AR, we'll just make it so that the note does not have mentions, i.e. no `@` char. + let!(:resource5) { notes.create!(note: 'note3 to check', commit_id: 'abc', noteable_type: 'Commit') } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + end + + it_behaves_like 'schedules resource mentions migration', Commit, true +end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index c1e7a1c2875..32e6b5afce5 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -440,6 +440,27 @@ describe JiraService do end end + context 'when Remote Link already exists' do + let(:remote_link) do + double( + 'remote link', + object: { + url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/#{commit_id}" + }.with_indifferent_access + ) + end + + it 'does not create comment' do + allow(JIRA::Resource::Remotelink).to receive(:all).and_return([remote_link]) + + expect(remote_link).to receive(:save!) + + @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) + + expect(WebMock).not_to have_requested(:post, @comment_url) + end + end + it 'does not send comment or remote links to issues already closed' do allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 7138305d7b1..8a64948d570 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -127,6 +127,36 @@ describe Upload do expect(uploader.mounted_as).to eq(subject.send(:mount_point)) expect(uploader.file).not_to be_nil end + + context 'when upload has mount_point nil' do + context 'when an upload belongs to a note' do + it 'mounts it as attachment' do + project = create(:project, :legacy_storage) + merge_request = create(:merge_request, source_project: project) + note = create(:legacy_diff_note_on_merge_request, note: 'some note', project: project, noteable: merge_request) + + subject = build(:upload, :with_file, :attachment_upload, model: note, mount_point: nil) + uploader = subject.retrieve_uploader + + expect(uploader.upload).to eq(subject) + expect(uploader.path).to include('attachment') + expect(uploader.file).not_to be_nil + end + end + + context 'when an upload does not belong to a note' do + it 'does not mount it as attachment' do + appearance = create(:appearance) + + subject = build(:upload, :with_file, :attachment_upload, model: appearance, mount_point: nil) + uploader = subject.retrieve_uploader + + expect(uploader.upload).to eq(subject) + expect(uploader.path).not_to include('attachment') + expect(uploader.file).not_to be_nil + end + end + end end describe '#needs_checksum?' do diff --git a/spec/models/x509_certificate_spec.rb b/spec/models/x509_certificate_spec.rb index 187d37334a1..880c5014a84 100644 --- a/spec/models/x509_certificate_spec.rb +++ b/spec/models/x509_certificate_spec.rb @@ -43,6 +43,28 @@ RSpec.describe X509Certificate do expect(certificate.subject).to eq(subject) expect(certificate.email).to eq(email) end + + it 'calls mark_commit_signatures_unverified' do + expect_any_instance_of(described_class).to receive(:mark_commit_signatures_unverified) + + described_class.safe_create!(attributes) + end + + context 'certificate revocation handling' do + let(:x509_certificate) { create(:x509_certificate) } + + it 'starts a revoke worker if certificate is revoked' do + expect(X509CertificateRevokeWorker).to receive(:perform_async).with(x509_certificate.id) + + x509_certificate.revoked! + end + + it 'does not starts a revoke worker for good certificates' do + expect(X509CertificateRevokeWorker).not_to receive(:perform_async).with(x509_certificate.id) + + x509_certificate + end + end end describe 'validators' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 1e8ab983b50..83f678ad2cb 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2455,7 +2455,7 @@ describe API::Projects do end it 'returns 200 when repository storage has changed' do - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/second_storage' }) + stub_storage_settings('test_second_storage' => { 'path' => TestEnv::SECOND_STORAGE_PATH }) expect do Sidekiq::Testing.fake! do diff --git a/spec/services/pod_logs/elasticsearch_service_spec.rb b/spec/services/pod_logs/elasticsearch_service_spec.rb index 0f0c36da56a..984a303e9e3 100644 --- a/spec/services/pod_logs/elasticsearch_service_spec.rb +++ b/spec/services/pod_logs/elasticsearch_service_spec.rb @@ -11,6 +11,7 @@ describe ::PodLogs::ElasticsearchService do let(:search) { 'foo -bar' } let(:start_time) { '2019-01-02T12:13:14+02:00' } let(:end_time) { '2019-01-03T12:13:14+02:00' } + let(:cursor) { '9999934,1572449784442' } let(:params) { {} } let(:expected_logs) do [ @@ -116,6 +117,36 @@ describe ::PodLogs::ElasticsearchService do end end + describe '#check_cursor' do + context 'with cursor provided and valid' do + let(:params) do + { + 'cursor' => cursor + } + end + + it 'returns success with cursor' do + result = subject.send(:check_cursor, {}) + + expect(result[:status]).to eq(:success) + expect(result[:cursor]).to eq(cursor) + end + end + + context 'with cursor not provided' do + let(:params) do + {} + end + + it 'returns success with nothing else' do + result = subject.send(:check_cursor, {}) + + expect(result.keys.length).to eq(1) + expect(result[:status]).to eq(:success) + end + end + end + describe '#pod_logs' do let(:result_arg) do { @@ -123,9 +154,11 @@ describe ::PodLogs::ElasticsearchService do container_name: container_name, search: search, start: start_time, - end: end_time + end: end_time, + cursor: cursor } end + let(:expected_cursor) { '9999934,1572449784442' } before do create(:clusters_applications_elastic_stack, :installed, cluster: cluster) @@ -137,13 +170,14 @@ describe ::PodLogs::ElasticsearchService do .and_return(Elasticsearch::Transport::Client.new) allow_any_instance_of(::Gitlab::Elasticsearch::Logs) .to receive(:pod_logs) - .with(namespace, pod_name, container_name, search, start_time, end_time) - .and_return(expected_logs) + .with(namespace, pod_name, container_name: container_name, search: search, start_time: start_time, end_time: end_time, cursor: cursor) + .and_return({ logs: expected_logs, cursor: expected_cursor }) result = subject.send(:pod_logs, result_arg) expect(result[:status]).to eq(:success) expect(result[:logs]).to eq(expected_logs) + expect(result[:cursor]).to eq(expected_cursor) end it 'returns an error when ES is unreachable' do @@ -170,5 +204,19 @@ describe ::PodLogs::ElasticsearchService do expect(result[:status]).to eq(:error) expect(result[:message]).to eq('Elasticsearch returned status code: ServiceUnavailable') end + + it 'handles cursor errors from elasticsearch' do + allow_any_instance_of(::Clusters::Applications::ElasticStack) + .to receive(:elasticsearch_client) + .and_return(Elasticsearch::Transport::Client.new) + allow_any_instance_of(::Gitlab::Elasticsearch::Logs) + .to receive(:pod_logs) + .and_raise(::Gitlab::Elasticsearch::Logs::InvalidCursor.new) + + result = subject.send(:pod_logs, result_arg) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('Invalid cursor value provided') + end end end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 4e5f10b3947..731febe75b3 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -311,9 +311,10 @@ describe Projects::ForkService do fork_before_move = fork_project(project) # Stub everything required to move a project to a Gitaly shard that does not exist - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/second_storage' }) - allow_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_repository_as_mirror).and_return(true) - allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum).and_return(::Gitlab::Git::BLANK_SHA) + stub_storage_settings('test_second_storage' => { 'path' => TestEnv::SECOND_STORAGE_PATH }) + allow_any_instance_of(Gitlab::Git::Repository).to receive(:replicate) + allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum) + .and_return(::Gitlab::Git::BLANK_SHA) Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage') fork_after_move = fork_project(project) diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index 2e9a4626abb..106a639ba28 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -32,8 +32,8 @@ describe Projects::UpdateRepositoryStorageService do project.repository.path_to_repo end - expect(project_repository_double).to receive(:fetch_repository_as_mirror) - .with(project.repository.raw).and_return(true) + expect(project_repository_double).to receive(:replicate) + .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return(checksum) @@ -49,16 +49,18 @@ describe Projects::UpdateRepositoryStorageService do context 'when the project is already on the target storage' do it 'bails out and does nothing' do - expect do - subject.execute(project.repository_storage) - end.to raise_error(described_class::RepositoryAlreadyMoved) + result = subject.execute(project.repository_storage) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to match(/repository and source have the same storage/) end end context 'when the move fails' do it 'unmarks the repository as read-only without updating the repository storage' do - expect(project_repository_double).to receive(:fetch_repository_as_mirror) - .with(project.repository.raw).and_return(false) + expect(project_repository_double).to receive(:replicate) + .with(project.repository.raw) + .and_raise(Gitlab::Git::CommandError) expect(GitlabShellWorker).not_to receive(:perform_async) result = subject.execute('test_second_storage') @@ -71,8 +73,8 @@ describe Projects::UpdateRepositoryStorageService do context 'when the checksum does not match' do it 'unmarks the repository as read-only without updating the repository storage' do - expect(project_repository_double).to receive(:fetch_repository_as_mirror) - .with(project.repository.raw).and_return(true) + expect(project_repository_double).to receive(:replicate) + .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return('not matching checksum') expect(GitlabShellWorker).not_to receive(:perform_async) @@ -89,8 +91,8 @@ describe Projects::UpdateRepositoryStorageService do let!(:pool) { create(:pool_repository, :ready, source_project: project) } it 'leaves the pool' do - expect(project_repository_double).to receive(:fetch_repository_as_mirror) - .with(project.repository.raw).and_return(true) + expect(project_repository_double).to receive(:replicate) + .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return(checksum) diff --git a/spec/services/x509_certificate_revoke_service_spec.rb b/spec/services/x509_certificate_revoke_service_spec.rb new file mode 100644 index 00000000000..ef76f616c93 --- /dev/null +++ b/spec/services/x509_certificate_revoke_service_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe X509CertificateRevokeService do + describe '#execute' do + let(:service) { described_class.new } + let!(:x509_signature_1) { create(:x509_commit_signature, x509_certificate: x509_certificate, verification_status: :verified ) } + let!(:x509_signature_2) { create(:x509_commit_signature, x509_certificate: x509_certificate, verification_status: :verified ) } + + context 'for revoked certificates' do + let(:x509_certificate) { create(:x509_certificate, certificate_status: :revoked ) } + + it 'update all commit signatures' do + expect do + service.execute(x509_certificate) + + x509_signature_1.reload + x509_signature_2.reload + end + .to change(x509_signature_1, :verification_status).from('verified').to('unverified') + .and change(x509_signature_2, :verification_status).from('verified').to('unverified') + end + end + + context 'for good certificates' do + RSpec::Matchers.define_negated_matcher :not_change, :change + + let(:x509_certificate) { create(:x509_certificate) } + + it 'do not update any commit signature' do + expect do + service.execute(x509_certificate) + + x509_signature_1.reload + x509_signature_2.reload + end + .to not_change(x509_signature_1, :verification_status) + .and not_change(x509_signature_2, :verification_status) + end + end + end +end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 0320f966c37..613535b6da5 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -84,6 +84,7 @@ module TestEnv TMP_TEST_PATH = Rails.root.join('tmp', 'tests', '**') REPOS_STORAGE = 'default'.freeze + SECOND_STORAGE_PATH = Rails.root.join('tmp', 'tests', 'second_storage') # Test environment # @@ -141,6 +142,7 @@ module TestEnv end FileUtils.mkdir_p(repos_path) + FileUtils.mkdir_p(SECOND_STORAGE_PATH) FileUtils.mkdir_p(backup_path) FileUtils.mkdir_p(pages_path) FileUtils.mkdir_p(artifacts_path) @@ -176,8 +178,6 @@ module TestEnv return end - FileUtils.mkdir_p("tmp/tests/second_storage") unless File.exist?("tmp/tests/second_storage") - spawn_script = Rails.root.join('scripts/gitaly-test-spawn').to_s Bundler.with_original_env do unless system(spawn_script) diff --git a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb index 6f83f52d54b..e30c620c4b1 100644 --- a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb +++ b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb @@ -22,14 +22,13 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| context 'when the move succeeds', :clean_gitlab_redis_shared_state do before do - allow(project_repository_double).to receive(:fetch_repository_as_mirror) + allow(project_repository_double).to receive(:replicate) .with(project.repository.raw) - .and_return(true) allow(project_repository_double).to receive(:checksum) .and_return(project_repository_checksum) - allow(repository_double).to receive(:fetch_repository_as_mirror) - .with(repository.raw).and_return(true) + allow(repository_double).to receive(:replicate) + .with(repository.raw) allow(repository_double).to receive(:checksum) .and_return(repository_checksum) end @@ -82,20 +81,23 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| context 'when the project is already on the target storage' do it 'bails out and does nothing' do - expect do - subject.execute(project.repository_storage) - end.to raise_error(described_class::RepositoryAlreadyMoved) + result = subject.execute(project.repository_storage) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to match(/repository and source have the same storage/) end end context "when the move of the #{repository_type} repository fails" do it 'unmarks the repository as read-only without updating the repository storage' do - allow(project_repository_double).to receive(:fetch_repository_as_mirror) - .with(project.repository.raw).and_return(true) + allow(project_repository_double).to receive(:replicate) + .with(project.repository.raw) allow(project_repository_double).to receive(:checksum) .and_return(project_repository_checksum) - allow(repository_double).to receive(:fetch_repository_as_mirror) - .with(repository.raw).and_return(false) + + allow(repository_double).to receive(:replicate) + .with(repository.raw) + .and_raise(Gitlab::Git::CommandError) expect(GitlabShellWorker).not_to receive(:perform_async) @@ -109,13 +111,13 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| context "when the checksum of the #{repository_type} repository does not match" do it 'unmarks the repository as read-only without updating the repository storage' do - allow(project_repository_double).to receive(:fetch_repository_as_mirror) - .with(project.repository.raw).and_return(true) + allow(project_repository_double).to receive(:replicate) + .with(project.repository.raw) allow(project_repository_double).to receive(:checksum) .and_return(project_repository_checksum) - allow(repository_double).to receive(:fetch_repository_as_mirror) - .with(repository.raw).and_return(true) + allow(repository_double).to receive(:replicate) + .with(repository.raw) allow(repository_double).to receive(:checksum) .and_return('not matching checksum') diff --git a/spec/workers/project_update_repository_storage_worker_spec.rb b/spec/workers/project_update_repository_storage_worker_spec.rb index aa6545f7f89..4cc44281a69 100644 --- a/spec/workers/project_update_repository_storage_worker_spec.rb +++ b/spec/workers/project_update_repository_storage_worker_spec.rb @@ -9,16 +9,11 @@ describe ProjectUpdateRepositoryStorageWorker do describe "#perform" do it "calls the update repository storage service" do - expect_any_instance_of(Projects::UpdateRepositoryStorageService) - .to receive(:execute).with('new_storage') + expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance| + expect(instance).to receive(:execute).with('new_storage') + end subject.perform(project.id, 'new_storage') end - - it 'catches and logs RepositoryAlreadyMoved' do - expect(Rails.logger).to receive(:info).with(/repository already moved/) - - expect { subject.perform(project.id, project.repository_storage) }.not_to raise_error - end end end diff --git a/spec/workers/x509_certificate_revoke_worker_spec.rb b/spec/workers/x509_certificate_revoke_worker_spec.rb new file mode 100644 index 00000000000..1e0cbf61267 --- /dev/null +++ b/spec/workers/x509_certificate_revoke_worker_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe X509CertificateRevokeWorker do + describe '#perform' do + context 'with a revoked certificate' do + subject { described_class.new } + + let(:x509_certificate) { create(:x509_certificate, certificate_status: :revoked) } + let(:job_args) { x509_certificate.id } + + include_examples 'an idempotent worker' do + it 'executes the revoke service' do + spy_service = X509CertificateRevokeService.new + + allow(X509CertificateRevokeService).to receive(:new) { spy_service } + + expect(spy_service).to receive(:execute) + .exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES).times + .with(x509_certificate) + .and_call_original + + subject + end + end + + it 'executes the revoke service' do + spy_service = X509CertificateRevokeService.new + + allow(X509CertificateRevokeService).to receive(:new) { spy_service } + + expect_next_instance_of(X509CertificateRevokeService) do |service| + expect(service).to receive(:execute).with(x509_certificate) + end + + subject + end + end + end +end |