diff options
Diffstat (limited to 'spec/services/projects')
14 files changed, 461 insertions, 138 deletions
diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb index 4b2569f6b2d..228dff6aa0b 100644 --- a/spec/services/projects/after_rename_service_spec.rb +++ b/spec/services/projects/after_rename_service_spec.rb @@ -26,6 +26,7 @@ RSpec.describe Projects::AfterRenameService, feature_category: :groups_and_proje let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) } let(:hashed_path) { File.join(hashed_prefix, hash) } + let(:message) { "Repository #{full_path_before_rename} could not be renamed to #{full_path_after_rename}" } before do # Project#gitlab_shell returns a new instance of Gitlab::Shell on every @@ -35,6 +36,15 @@ RSpec.describe Projects::AfterRenameService, feature_category: :groups_and_proje stub_application_setting(hashed_storage_enabled: true) end + shared_examples 'logging and raising a RenameFailedError' do + it 'logs raises a RenameFailedError' do + expect_any_instance_of(described_class).to receive(:log_error).with(message) + + expect { service_execute } + .to raise_error(described_class::RenameFailedError) + end + end + it 'renames a repository' do stub_container_registry_config(enabled: false) @@ -47,8 +57,21 @@ RSpec.describe Projects::AfterRenameService, feature_category: :groups_and_proje service_execute end + context 'when renaming or migrating fails' do + before do + allow_any_instance_of(::Projects::HashedStorage::MigrationService) + .to receive(:execute).and_return(false) + end + + it_behaves_like 'logging and raising a RenameFailedError' + end + context 'container registry with images' do let(:container_repository) { create(:container_repository) } + let(:message) do + "Project #{full_path_before_rename} cannot be renamed because images are " \ + "present in its container registry" + end before do stub_container_registry_config(enabled: true) @@ -56,9 +79,36 @@ RSpec.describe Projects::AfterRenameService, feature_category: :groups_and_proje project.container_repositories << container_repository end - it 'raises a RenameFailedError' do - expect { service_execute } - .to raise_error(described_class::RenameFailedError) + context 'when Gitlab API is not supported' do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(false) + end + + it_behaves_like 'logging and raising a RenameFailedError' + end + + context 'when Gitlab API Client is supported' do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true) + end + + it 'renames the base repository in the registry' do + expect(ContainerRegistry::GitlabApiClient).to receive(:rename_base_repository_path) + .with(full_path_before_rename, name: path_after_rename).and_return(:ok) + + service_execute + end + + context 'when the base repository rename in the registry fails' do + before do + allow(ContainerRegistry::GitlabApiClient) + .to receive(:rename_base_repository_path).and_return(:bad_request) + end + + let(:message) { 'Renaming the base repository in the registry failed with error bad_request.' } + + it_behaves_like 'logging and raising a RenameFailedError' + end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 899ed477180..5a7abf6cde8 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -812,6 +812,31 @@ RSpec.describe Projects::CreateService, '#execute', feature_category: :groups_an end end + context 'when SHA256 format is requested' do + let(:project) { create_project(user, opts) } + let(:opts) { super().merge(initialize_with_readme: true, repository_object_format: 'sha256') } + + before do + allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main') + end + + it 'creates a repository with SHA256 commit hashes', :aggregate_failures do + expect(project.repository.commit_count).to be(1) + expect(project.commit.id.size).to eq 64 + end + + context 'when "support_sha256_repositories" feature flag is disabled' do + before do + stub_feature_flags(support_sha256_repositories: false) + end + + it 'creates a repository with default SHA1 commit hash' do + expect(project.repository.commit_count).to be(1) + expect(project.commit.id.size).to eq 40 + end + end + end + describe 'create integration for the project' do subject(:project) { create_project(user, opts) } diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index a0064eadf13..3aea329a45f 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -454,6 +454,12 @@ RSpec.describe Projects::DestroyService, :aggregate_failures, :event_store_publi expect { destroy_project(forked_project, user) } .not_to raise_error end + + it 'does not update project statistics for the deleted project' do + expect(ProjectCacheWorker).not_to receive(:perform_async) + + destroy_project(forked_project, user) + end end context 'as the root of a fork network' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index ceb060445ad..e6418c7b4ea 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -387,7 +387,7 @@ RSpec.describe Projects::ForkService, feature_category: :source_code_management # Stub everything required to move a project to a Gitaly shard that does not exist allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid) - stub_storage_settings('test_second_storage' => { 'path' => TestEnv::SECOND_STORAGE_PATH }) + stub_storage_settings('test_second_storage' => {}) allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_repository) .and_return(true) allow_any_instance_of(Gitlab::Git::Repository).to receive(:replicate) diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb index e3f170ef3fe..6bc0b86545a 100644 --- a/spec/services/projects/group_links/create_service_spec.rb +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -103,11 +103,57 @@ RSpec.describe Projects::GroupLinks::CreateService, '#execute', feature_category it_behaves_like 'shareable' end end + + context 'when sharing it to a group with OWNER access' do + let(:opts) do + { + link_group_access: Gitlab::Access::OWNER, + expires_at: nil + } + end + + it 'does not share and returns a forbiden error' do + expect do + result = subject.execute + + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(403) + end.not_to change { project.reload.project_group_links.count } + end + end + end + + context 'when the user is an OWNER in the project' do + before do + project.add_owner(user) + end + + it_behaves_like 'shareable' + + context 'when sharing it to a group with OWNER access' do + let(:opts) do + { + link_group_access: Gitlab::Access::OWNER, + expires_at: nil + } + end + + it_behaves_like 'shareable' + end end end context 'when user does not have permissions to share the project with a group' do it_behaves_like 'not shareable' + + context 'when the user has less than MAINTAINER access in the project' do + before do + group.add_guest(user) + project.add_developer(user) + end + + it_behaves_like 'not shareable' + end end context 'when group is blank' do diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb index 0cd003f6142..4fc441c9e26 100644 --- a/spec/services/projects/group_links/destroy_service_spec.rb +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -19,8 +19,10 @@ RSpec.describe Projects::GroupLinks::DestroyService, '#execute', feature_categor end end - shared_examples_for 'returns not_found' do - it do + context 'if group_link is blank' do + let!(:group_link) { nil } + + it 'returns 404 not found' do expect do result = subject.execute(group_link) @@ -30,14 +32,15 @@ RSpec.describe Projects::GroupLinks::DestroyService, '#execute', feature_categor end end - context 'if group_link is blank' do - let!(:group_link) { nil } - - it_behaves_like 'returns not_found' - end - context 'if the user does not have access to destroy the link' do - it_behaves_like 'returns not_found' + it 'returns 404 not found' do + expect do + result = subject.execute(group_link) + + expect(result[:status]).to eq(:error) + expect(result[:reason]).to eq(:not_found) + end.not_to change { project.reload.project_group_links.count } + end end context 'when the user has proper permissions to remove a group-link from a project' do @@ -111,6 +114,41 @@ RSpec.describe Projects::GroupLinks::DestroyService, '#execute', feature_categor end end end + + context 'on trying to destroy a link with OWNER access' do + let(:group_access) { Gitlab::Access::OWNER } + + it 'does not remove the group from project' do + expect do + result = subject.execute(group_link) + + expect(result[:status]).to eq(:error) + expect(result[:reason]).to eq(:forbidden) + end.not_to change { project.reload.project_group_links.count } + end + + context 'if the user is an OWNER of the group' do + before do + group.add_owner(user) + end + + it_behaves_like 'removes group from project' + end + end + end + + context 'when the user is an OWNER in the project' do + before do + project.add_owner(user) + end + + it_behaves_like 'removes group from project' + + context 'on trying to destroy a link with OWNER access' do + let(:group_access) { Gitlab::Access::OWNER } + + it_behaves_like 'removes group from project' + end end end diff --git a/spec/services/projects/group_links/update_service_spec.rb b/spec/services/projects/group_links/update_service_spec.rb index b02614fa062..86ad1bcf286 100644 --- a/spec/services/projects/group_links/update_service_spec.rb +++ b/spec/services/projects/group_links/update_service_spec.rb @@ -20,8 +20,8 @@ RSpec.describe Projects::GroupLinks::UpdateService, '#execute', feature_category subject { described_class.new(link, user).execute(group_link_params) } - shared_examples_for 'returns not_found' do - it do + context 'when the user does not have proper permissions to update a project group link' do + it 'returns 404 not found' do result = subject expect(result[:status]).to eq(:error) @@ -29,10 +29,6 @@ RSpec.describe Projects::GroupLinks::UpdateService, '#execute', feature_category end end - context 'when the user does not have proper permissions to update a project group link' do - it_behaves_like 'returns not_found' - end - context 'when user has proper permissions to update a project group link' do context 'when the user is a MAINTAINER in the project' do before do @@ -92,6 +88,86 @@ RSpec.describe Projects::GroupLinks::UpdateService, '#execute', feature_category subject end end + + context 'updating a link with OWNER access' do + let(:group_access) { Gitlab::Access::OWNER } + + shared_examples_for 'returns :forbidden' do + it do + expect do + result = subject + + expect(result[:status]).to eq(:error) + expect(result[:reason]).to eq(:forbidden) + end.to not_change { link.expires_at }.and not_change { link.group_access } + end + end + + context 'updating expires_at' do + let(:group_link_params) do + { expires_at: 7.days.from_now } + end + + it_behaves_like 'returns :forbidden' + end + + context 'updating group_access' do + let(:group_link_params) do + { group_access: Gitlab::Access::MAINTAINER } + end + + it_behaves_like 'returns :forbidden' + end + + context 'updating both expires_at and group_access' do + it_behaves_like 'returns :forbidden' + end + end + end + + context 'when the user is an OWNER in the project' do + before do + project.add_owner(user) + end + + context 'updating expires_at' do + let(:group_link_params) do + { expires_at: 7.days.from_now.to_date } + end + + it 'updates existing link' do + expect do + result = subject + + expect(result[:status]).to eq(:success) + end.to change { link.reload.expires_at }.to(group_link_params[:expires_at]) + end + end + + context 'updating group_access' do + let(:group_link_params) do + { group_access: Gitlab::Access::MAINTAINER } + end + + it 'updates existing link' do + expect do + result = subject + + expect(result[:status]).to eq(:success) + end.to change { link.reload.group_access }.to(group_link_params[:group_access]) + end + end + + context 'updating both expires_at and group_access' do + it 'updates existing link' do + expect do + result = subject + + expect(result[:status]).to eq(:success) + end.to change { link.reload.group_access }.to(group_link_params[:group_access]) + .and change { link.reload.expires_at }.to(group_link_params[:expires_at]) + end + end end end end diff --git a/spec/services/projects/hashed_storage/base_attachment_service_spec.rb b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb index e32747ad907..4834af79225 100644 --- a/spec/services/projects/hashed_storage/base_attachment_service_spec.rb +++ b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Projects::HashedStorage::BaseAttachmentService, feature_category: describe '#move_folder!' do context 'when old_path is not a directory' do it 'adds information to the logger and returns true' do - Tempfile.create do |old_path| # rubocop:disable Rails/SaveBang + Tempfile.create do |old_path| new_path = "#{old_path}-new" expect(subject.send(:move_folder!, old_path, new_path)).to be_truthy diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index 16b9d2618ca..dd6e82b9ef2 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -183,81 +183,6 @@ RSpec.describe Projects::ImportService, feature_category: :importers do expect(result[:status]).to eq :error expect(result[:message]).to eq "Error importing repository #{project.safe_import_url} into #{project.full_path} - Failed to import the repository [FILTERED]" end - - context 'when bitbucket_parallel_importer feature flag is disabled' do - before do - stub_feature_flags(bitbucket_parallel_importer: false) - end - - it 'succeeds if repository import is successful' do - expect(project.repository).to receive(:import_repository).and_return(true) - expect_next_instance_of(Gitlab::BitbucketImport::Importer) do |importer| - expect(importer).to receive(:execute).and_return(true) - end - - expect_next_instance_of(Projects::LfsPointers::LfsImportService) do |service| - expect(service).to receive(:execute).and_return(status: :success) - end - - result = subject.execute - - expect(result[:status]).to eq :success - end - - it 'fails if repository import fails' do - expect(project.repository) - .to receive(:import_repository) - .with('https://bitbucket.org/vim/vim.git', resolved_address: '') - .and_raise(Gitlab::Git::CommandError, 'Failed to import the repository /a/b/c') - - result = subject.execute - - expect(result[:status]).to eq :error - expect(result[:message]).to eq "Error importing repository #{project.safe_import_url} into #{project.full_path} - Failed to import the repository [FILTERED]" - end - - context 'when lfs import fails' do - it 'logs the error' do - error_message = 'error message' - - expect(project.repository).to receive(:import_repository).and_return(true) - - expect_next_instance_of(Gitlab::BitbucketImport::Importer) do |importer| - expect(importer).to receive(:execute).and_return(true) - end - - expect_next_instance_of(Projects::LfsPointers::LfsImportService) do |service| - expect(service).to receive(:execute).and_return(status: :error, message: error_message) - end - - expect(Gitlab::AppLogger).to receive(:error).with("The Lfs import process failed. #{error_message}") - - subject.execute - end - end - - context 'when repository import scheduled' do - before do - expect(project.repository).to receive(:import_repository).and_return(true) - allow(subject).to receive(:import_data) - end - - it 'downloads lfs objects if lfs_enabled is enabled for project' do - allow(project).to receive(:lfs_enabled?).and_return(true) - - expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute) - - subject.execute - end - - it 'does not download lfs objects if lfs_enabled is not enabled for project' do - allow(project).to receive(:lfs_enabled?).and_return(false) - expect_any_instance_of(Projects::LfsPointers::LfsImportService).not_to receive(:execute) - - subject.execute - end - end - end end end end @@ -352,13 +277,53 @@ RSpec.describe Projects::ImportService, feature_category: :importers do end end + context 'when import is a local request' do + before do + project.import_url = "http://127.0.0.1/group/project" + end + + context 'when local network requests are enabled' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + end + + it 'returns an error' do + expect(project.repository).not_to receive(:import_repository) + expect(subject.execute).to include( + status: :error, + message: end_with('Requests to localhost are not allowed') + ) + end + + context 'when environment is development' do + before do + stub_rails_env('development') + end + + it 'imports successfully' do + expect(project.repository) + .to receive(:import_repository) + .and_return(true) + expect(subject.execute[:status]).to eq(:success) + end + end + end + end + context 'when DNS rebind protection is disabled' do before do allow(Gitlab::CurrentSettings).to receive(:dns_rebinding_protection_enabled?).and_return(false) project.import_url = "https://example.com/group/project" allow(Gitlab::UrlBlocker).to receive(:validate!) - .with(project.import_url, ports: Project::VALID_IMPORT_PORTS, schemes: Project::VALID_IMPORT_PROTOCOLS, dns_rebind_protection: false) + .with( + project.import_url, + ports: Project::VALID_IMPORT_PORTS, + schemes: Project::VALID_IMPORT_PROTOCOLS, + allow_local_network: false, + allow_localhost: false, + dns_rebind_protection: false + ) .and_return([Addressable::URI.parse("https://example.com/group/project"), nil]) end @@ -386,7 +351,14 @@ RSpec.describe Projects::ImportService, feature_category: :importers do project.import_url = "https://example.com/group/project" allow(Gitlab::UrlBlocker).to receive(:validate!) - .with(project.import_url, ports: Project::VALID_IMPORT_PORTS, schemes: Project::VALID_IMPORT_PROTOCOLS, dns_rebind_protection: true) + .with( + project.import_url, + ports: Project::VALID_IMPORT_PORTS, + schemes: Project::VALID_IMPORT_PROTOCOLS, + allow_local_network: false, + allow_localhost: false, + dns_rebind_protection: true + ) .and_return([Addressable::URI.parse("https://172.16.123.1/group/project"), 'example.com']) end @@ -407,7 +379,14 @@ RSpec.describe Projects::ImportService, feature_category: :importers do project.import_url = 'https://gitlab.com/gitlab-org/gitlab-development-kit' allow(Gitlab::UrlBlocker).to receive(:validate!) - .with(project.import_url, ports: Project::VALID_IMPORT_PORTS, schemes: Project::VALID_IMPORT_PROTOCOLS, dns_rebind_protection: true) + .with( + project.import_url, + ports: Project::VALID_IMPORT_PORTS, + schemes: Project::VALID_IMPORT_PROTOCOLS, + allow_local_network: false, + allow_localhost: false, + dns_rebind_protection: true + ) .and_return([Addressable::URI.parse('https://[2606:4700:90:0:f22e:fbec:5bed:a9b9]/gitlab-org/gitlab-development-kit'), 'gitlab.com']) end @@ -430,7 +409,14 @@ RSpec.describe Projects::ImportService, feature_category: :importers do project.import_url = "http://example.com/group/project" allow(Gitlab::UrlBlocker).to receive(:validate!) - .with(project.import_url, ports: Project::VALID_IMPORT_PORTS, schemes: Project::VALID_IMPORT_PROTOCOLS, dns_rebind_protection: true) + .with( + project.import_url, + ports: Project::VALID_IMPORT_PORTS, + schemes: Project::VALID_IMPORT_PROTOCOLS, + allow_local_network: false, + allow_localhost: false, + dns_rebind_protection: true + ) .and_return([Addressable::URI.parse("http://172.16.123.1/group/project"), 'example.com']) end @@ -452,7 +438,14 @@ RSpec.describe Projects::ImportService, feature_category: :importers do project.import_url = "git://example.com/group/project.git" allow(Gitlab::UrlBlocker).to receive(:validate!) - .with(project.import_url, ports: Project::VALID_IMPORT_PORTS, schemes: Project::VALID_IMPORT_PROTOCOLS, dns_rebind_protection: true) + .with( + project.import_url, + ports: Project::VALID_IMPORT_PORTS, + schemes: Project::VALID_IMPORT_PROTOCOLS, + allow_local_network: false, + allow_localhost: false, + dns_rebind_protection: true + ) .and_return([Addressable::URI.parse("git://172.16.123.1/group/project"), 'example.com']) end diff --git a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb index d3f053aaedc..5862ed15c2a 100644 --- a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb @@ -91,5 +91,23 @@ RSpec.describe Projects::LfsPointers::LfsLinkService, feature_category: :source_ # 3. Insert the lfs_objects_projects for that batch expect { subject.execute(new_oid_list.keys) }.not_to exceed_query_limit(3) end + + context 'when MAX_OIDS is 5' do + let(:max_oids) { 5 } + let(:oids) { Array.new(max_oids) { |i| "oid-#{i}" } } + + before do + stub_const("#{described_class}::MAX_OIDS", max_oids) + end + + it 'does not raise an error when trying to link exactly the OID limit' do + expect { subject.execute(oids) }.not_to raise_error + end + + it 'raises an error when trying to link more than OID limit' do + oids << 'the straw' + expect { subject.execute(oids) }.to raise_error(described_class::TooManyOidsError) + end + end end end diff --git a/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb b/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb index 591cd1cba8d..48cf963e6e1 100644 --- a/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb +++ b/spec/services/projects/refresh_build_artifacts_size_statistics_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitlab_redis_shared_state, feature_category: :build_artifacts do +RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitlab_redis_buffered_counter, feature_category: :build_artifacts do let(:service) { described_class.new } describe '#execute' do @@ -79,7 +79,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl end before do - allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(StandardError, 'error') + allow(Gitlab::Redis::BufferedCounter).to receive(:with).and_raise(StandardError, 'error') expect { service.execute }.to raise_error(StandardError) end diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index 872e38aba1d..2e1a6c03c90 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -58,6 +58,26 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin expect(source.forks_count).to be_zero end + it 'refreshes the project statistics of the forked project' do + expect(ProjectCacheWorker).to receive(:perform_async).with(forked_project.id, [], [:repository_size]) + + subject.execute + end + + it 'does not refresh project statistics when refresh_statistics is false' do + expect(ProjectCacheWorker).not_to receive(:perform_async) + + subject.execute(refresh_statistics: false) + end + + it 'does not refresh project statistics when the feature flag is disabled' do + stub_feature_flags(refresh_statistics_on_unlink_fork: false) + + expect(ProjectCacheWorker).not_to receive(:perform_async) + + subject.execute + end + context 'when the original project was deleted' do it 'does not fail when the original project is deleted' do source = forked_project.forked_from_project diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index b81fc8bf633..de7e6ae9a40 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -13,12 +13,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour before do allow(Time).to receive(:now).and_return(time) - stub_storage_settings( - 'test_second_storage' => { - 'gitaly_address' => Gitlab.config.repositories.storages.default.gitaly_address, - 'path' => TestEnv::SECOND_STORAGE_PATH - } - ) + stub_storage_settings('test_second_storage' => {}) end context 'without wiki and design repository' do @@ -76,6 +71,8 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('test_second_storage') expect(project.project_repository.shard_name).to eq('test_second_storage') + expect(repository_storage_move.reload).to be_finished + expect(repository_storage_move.error_message).to be_nil end end @@ -100,6 +97,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour expect(project).not_to be_repository_read_only expect(repository_storage_move.reload).to be_failed + expect(repository_storage_move.error_message).to eq('Boom') end end @@ -126,7 +124,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) - .and_raise(Gitlab::Git::CommandError) + .and_raise(Gitlab::Git::CommandError, 'Boom') expect(project_repository_double).to receive(:remove) expect do @@ -136,6 +134,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('default') expect(repository_storage_move).to be_failed + expect(repository_storage_move.error_message).to eq('Boom') end end @@ -216,29 +215,6 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour expect(object_pool_double).to have_received(:link).with(project.repository.raw) end - context 'when feature flag replicate_object_pool_on_move is disabled' do - before do - stub_feature_flags(replicate_object_pool_on_move: false) - end - - it 'just moves the repository without the object pool' do - result = subject.execute - expect(result).to be_success - - project.reload.cleanup - - new_pool_repository = project.pool_repository - - expect(new_pool_repository).to eq(pool_repository) - expect(new_pool_repository.shard).to eq(shard_default) - expect(new_pool_repository.state).to eq('ready') - expect(new_pool_repository.source_project).to eq(project) - - expect(object_pool_repository_double).not_to have_received(:replicate) - expect(object_pool_double).not_to have_received(:link) - end - end - context 'when new shard has a repository pool' do let!(:new_pool_repository) { create(:pool_repository, :ready, shard: shard_to, source_project: project) } diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 7ab85d8253a..a3f6c472f3b 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -356,7 +356,7 @@ RSpec.describe Projects::UpdateService, feature_category: :groups_and_projects d context 'when changes project features' do # Using some sample features for testing. # Not using all the features because some of them must be enabled/disabled together - %w[issues wiki forking model_experiments].each do |feature_name| + %w[issues wiki forking model_experiments model_registry].each do |feature_name| context "with feature_name:#{feature_name}" do let(:feature) { "#{feature_name}_access_level" } let(:params) do @@ -415,17 +415,92 @@ RSpec.describe Projects::UpdateService, feature_category: :groups_and_projects d end context 'when updating a project that contains container images' do + let(:new_name) { 'renamed' } + before do stub_container_registry_config(enabled: true) stub_container_registry_tags(repository: /image/, tags: %w[rc1]) create(:container_repository, project: project, name: :image) end - it 'does not allow to rename the project' do - result = update_project(project, admin, path: 'renamed') + shared_examples 'renaming the project fails with message' do |error_message| + it 'does not allow to rename the project' do + result = update_project(project, admin, path: new_name) + + expect(result).to include(status: :error) + expect(result[:message]).to match(error_message) + end + end + + context 'when the GitlabAPI is not supported' do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(false) + end + + it_behaves_like 'renaming the project fails with message', /contains container registry tags/ + end + + context 'when Gitlab API is supported' do + before do + allow(ContainerRegistry::GitlabApiClient).to receive(:supports_gitlab_api?).and_return(true) + end + + it 'executes a dry run of the project rename' do + stub_rename_base_repository_in_registry(dry_run: true) + + update_project(project, admin, path: new_name) + + expect_rename_of_base_repository_in_registry(dry_run: true) + end + + context 'when the dry run fails' do + before do + stub_rename_base_repository_in_registry(dry_run: true, result: :bad_request) + end + + it_behaves_like 'renaming the project fails with message', /container registry path rename validation failed/ + + it 'logs the error' do + expect_any_instance_of(described_class).to receive(:log_error).with("Dry run failed for renaming project with tags: #{project.full_path}, error: bad_request") + + update_project(project, admin, path: new_name) + end + end + + context 'when the dry run succeeds' do + before do + stub_rename_base_repository_in_registry(dry_run: true, result: :accepted) + end + + it 'continues with the project rename' do + stub_rename_base_repository_in_registry(dry_run: false, result: :ok) + old_project_full_path = project.full_path - expect(result).to include(status: :error) - expect(result[:message]).to match(/contains container registry tags/) + update_project(project, admin, path: new_name) + + expect_rename_of_base_repository_in_registry(dry_run: true, path: old_project_full_path) + expect_rename_of_base_repository_in_registry(dry_run: false, path: old_project_full_path) + end + end + + def stub_rename_base_repository_in_registry(dry_run:, result: nil) + options = { name: new_name } + options[:dry_run] = true if dry_run + + allow(ContainerRegistry::GitlabApiClient) + .to receive(:rename_base_repository_path) + .with(project.full_path, options) + .and_return(result) + end + + def expect_rename_of_base_repository_in_registry(dry_run:, path: nil) + options = { name: new_name } + options[:dry_run] = true if dry_run + + expect(ContainerRegistry::GitlabApiClient) + .to have_received(:rename_base_repository_path) + .with(path || project.full_path, options) + end end it 'allows to update other settings' do @@ -708,7 +783,7 @@ RSpec.describe Projects::UpdateService, feature_category: :groups_and_projects d let(:opts) { { repository_storage: 'test_second_storage' } } before do - stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + stub_storage_settings('test_second_storage' => {}) end shared_examples 'the transfer was not scheduled' do |