diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index b3020484738..99dbd4fbacf 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -34,6 +34,8 @@ module Storage # So we basically we mute exceptions in next actions begin send_update_instructions + write_projects_repository_config + true rescue # Returning false does not rollback after_* transaction but gives diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 0ff169d4531..bdcc9159d26 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -268,4 +268,11 @@ class Namespace < ActiveRecord::Base def namespace_previously_created_with_same_path? RedirectRoute.permanent.exists?(path: path) end + + def write_projects_repository_config + all_projects.find_each do |project| + project.expires_full_path_cache # we need to clear cache to validate renames correctly + project.write_repository_config + end + end end diff --git a/app/models/project.rb b/app/models/project.rb index 6ebb083aeb4..9c0bbf697e2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1420,6 +1420,8 @@ class Project < ActiveRecord::Base end def after_rename_repo + write_repository_config + path_before_change = previous_changes['path'].first # We need to check if project had been rolled out to move resource to hashed storage or not and decide @@ -1432,6 +1434,16 @@ class Project < ActiveRecord::Base Gitlab::PagesTransfer.new.rename_project(path_before_change, self.path, namespace.full_path) end + def write_repository_config(gl_full_path: full_path) + # We'd need to keep track of project full path otherwise directory tree + # created with hashed storage enabled cannot be usefully imported using + # the import rake task. + repo.config['gitlab.fullpath'] = gl_full_path + rescue Gitlab::Git::Repository::NoRepository => e + Rails.logger.error("Error writing to .git/config for project #{full_path} (#{id}): #{e.message}.") + nil + end + def rename_repo_notify! send_move_instructions(full_path_was) expires_full_path_cache diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index dc7b1f1f5cc..01838ec6b5d 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -88,6 +88,7 @@ module Projects log_info("#{@project.owner.name} created a new project \"#{@project.name_with_namespace}\"") unless @project.gitlab_project_import? + @project.write_repository_config @project.create_wiki unless skip_wiki? create_services_from_active_templates(@project) diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb index 7212e7524ab..67178de75de 100644 --- a/app/services/projects/hashed_storage/migrate_repository_service.rb +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -27,7 +27,9 @@ module Projects result &&= move_repository("#{@old_wiki_disk_path}", "#{@new_disk_path}.wiki") end - unless result + if result + project.write_repository_config + else rollback_folder_move project.storage_version = nil end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index e5cd6fcdfe3..26765e5c3f3 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -75,6 +75,8 @@ module Projects project.old_path_with_namespace = @old_path project.expires_full_path_cache + write_repository_config(@new_path) + execute_system_hooks end rescue Exception # rubocop:disable Lint/RescueException @@ -98,6 +100,10 @@ module Projects project.save! end + def write_repository_config(full_path) + project.write_repository_config(gl_full_path: full_path) + end + def refresh_permissions # This ensures we only schedule 1 job for every user that has access to # the namespaces. @@ -110,6 +116,7 @@ module Projects def rollback_side_effects rollback_folder_move update_namespace_and_visibility(@old_namespace) + write_repository_config(@old_path) end def rollback_folder_move diff --git a/changelogs/unreleased/da-handle-hashed-storage-repos-using-repo-import-task.yml b/changelogs/unreleased/da-handle-hashed-storage-repos-using-repo-import-task.yml new file mode 100644 index 00000000000..74a00d49ab3 --- /dev/null +++ b/changelogs/unreleased/da-handle-hashed-storage-repos-using-repo-import-task.yml @@ -0,0 +1,5 @@ +--- +title: Handle GitLab hashed storage repositories using the repo import task +merge_request: +author: +type: added diff --git a/lib/gitlab/bare_repository_import/importer.rb b/lib/gitlab/bare_repository_import/importer.rb index 64e41d42709..709a901aa77 100644 --- a/lib/gitlab/bare_repository_import/importer.rb +++ b/lib/gitlab/bare_repository_import/importer.rb @@ -14,7 +14,7 @@ module Gitlab repos_to_import.each do |repo_path| bare_repo = Gitlab::BareRepositoryImport::Repository.new(import_path, repo_path) - if bare_repo.hashed? || bare_repo.wiki? + unless bare_repo.processable? log " * Skipping repo #{bare_repo.repo_path}".color(:yellow) next @@ -62,6 +62,8 @@ module Gitlab if project.persisted? && mv_repo(project) log " * Created #{project.name} (#{project_full_path})".color(:green) + project.write_repository_config + ProjectCacheWorker.perform_async(project.id) else log " * Failed trying to create #{project.name} (#{project_full_path})".color(:red) diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb index fa7891c8dcc..85b79362196 100644 --- a/lib/gitlab/bare_repository_import/repository.rb +++ b/lib/gitlab/bare_repository_import/repository.rb @@ -6,39 +6,56 @@ module Gitlab def initialize(root_path, repo_path) @root_path = root_path @repo_path = repo_path - @root_path << '/' unless root_path.ends_with?('/') + full_path = + if hashed? && !wiki? + repository.config.get('gitlab.fullpath') + else + repo_relative_path + end + # Split path into 'all/the/namespaces' and 'project_name' - @group_path, _, @project_name = repo_relative_path.rpartition('/') + @group_path, _, @project_name = full_path.to_s.rpartition('/') end def wiki_exists? File.exist?(wiki_path) end - def wiki? - @wiki ||= repo_path.end_with?('.wiki.git') - end - def wiki_path @wiki_path ||= repo_path.sub(/\.git$/, '.wiki.git') end - def hashed? - @hashed ||= group_path.start_with?('@hashed') - end - def project_full_path @project_full_path ||= "#{group_path}/#{project_name}" end + def processable? + return false if wiki? + return false if hashed? && (group_path.blank? || project_name.blank?) + + true + end + private + def wiki? + @wiki ||= repo_path.end_with?('.wiki.git') + end + + def hashed? + @hashed ||= repo_relative_path.include?('@hashed') + end + def repo_relative_path # Remove root path and `.git` at the end repo_path[@root_path.size...-4] end + + def repository + @repository ||= Rugged::Repository.new(repo_path) + end end end end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 82ac3c424d4..b5d86df09d2 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -75,7 +75,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do end it 'creates the Git repo in disk' do - FileUtils.mkdir_p(File.join(base_dir, "#{project_path}.git")) + create_bare_repository("#{project_path}.git") importer.create_project_if_needed @@ -130,7 +130,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do end it 'creates the Git repo in disk' do - FileUtils.mkdir_p(File.join(base_dir, "#{project_path}.git")) + create_bare_repository("#{project_path}.git") importer.create_project_if_needed @@ -165,8 +165,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do it_behaves_like 'importing a repository' it 'creates the Wiki git repo in disk' do - FileUtils.mkdir_p(File.join(base_dir, "#{project_path}.git")) - FileUtils.mkdir_p(File.join(base_dir, "#{project_path}.wiki.git")) + create_bare_repository("#{project_path}.git") + create_bare_repository("#{project_path}.wiki.git") expect(Projects::CreateService).to receive(:new).with(admin, hash_including(skip_wiki: true, import_type: 'bare_repository')).and_call_original @@ -192,4 +192,9 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do end end end + + def create_bare_repository(project_path) + repo_path = File.join(base_dir, project_path) + Gitlab::Git::Repository.create(repo_path, bare: true) + end end diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 61b73abcba4..9f42cf1dfca 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -1,58 +1,122 @@ require 'spec_helper' describe ::Gitlab::BareRepositoryImport::Repository do - let(:project_repo_path) { described_class.new('/full/path/', '/full/path/to/repo.git') } + context 'legacy storage' do + subject { described_class.new('/full/path/', '/full/path/to/repo.git') } - it 'stores the repo path' do - expect(project_repo_path.repo_path).to eq('/full/path/to/repo.git') - end - - it 'stores the group path' do - expect(project_repo_path.group_path).to eq('to') - end - - it 'stores the project name' do - expect(project_repo_path.project_name).to eq('repo') - end - - it 'stores the wiki path' do - expect(project_repo_path.wiki_path).to eq('/full/path/to/repo.wiki.git') - end - - describe '#wiki?' do - it 'returns true if it is a wiki' do - wiki_path = described_class.new('/full/path/', '/full/path/to/a/b/my.wiki.git') - - expect(wiki_path.wiki?).to eq(true) + it 'stores the repo path' do + expect(subject.repo_path).to eq('/full/path/to/repo.git') end - it 'returns false if it is not a wiki' do - expect(project_repo_path.wiki?).to eq(false) + it 'stores the group path' do + expect(subject.group_path).to eq('to') + end + + it 'stores the project name' do + expect(subject.project_name).to eq('repo') + end + + it 'stores the wiki path' do + expect(subject.wiki_path).to eq('/full/path/to/repo.wiki.git') + end + + describe '#processable?' do + it 'returns false if it is a wiki' do + subject = described_class.new('/full/path/', '/full/path/to/a/b/my.wiki.git') + + expect(subject).not_to be_processable + end + + it 'returns true if group path is missing' do + subject = described_class.new('/full/path/', '/full/path/repo.git') + + expect(subject).to be_processable + end + + it 'returns true when group path and project name are present' do + expect(subject).to be_processable + end + end + + describe '#project_full_path' do + it 'returns the project full path with trailing slash in the root path' do + expect(subject.project_full_path).to eq('to/repo') + end + + it 'returns the project full path with no trailing slash in the root path' do + subject = described_class.new('/full/path', '/full/path/to/repo.git') + + expect(subject.project_full_path).to eq('to/repo') + end end end - describe '#hashed?' do - it 'returns true if it is a hashed folder' do - path = described_class.new('/full/path/', '/full/path/@hashed/my.repo.git') + context 'hashed storage' do + let(:gitlab_shell) { Gitlab::Shell.new } + let(:repository_storage) { 'default' } + let(:root_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + let(:hashed_path) { "@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b" } + let(:repo_path) { File.join(root_path, "#{hashed_path}.git") } + let(:wiki_path) { File.join(root_path, "#{hashed_path}.wiki.git") } - expect(path.hashed?).to eq(true) + before do + gitlab_shell.add_repository(repository_storage, hashed_path) + repository = Rugged::Repository.new(repo_path) + repository.config['gitlab.fullpath'] = 'to/repo' end - it 'returns false if it is not a hashed folder' do - expect(project_repo_path.hashed?).to eq(false) - end - end - - describe '#project_full_path' do - it 'returns the project full path' do - expect(project_repo_path.repo_path).to eq('/full/path/to/repo.git') - expect(project_repo_path.project_full_path).to eq('to/repo') + after do + gitlab_shell.remove_repository(root_path, hashed_path) end - it 'with no trailing slash in the root path' do - repo_path = described_class.new('/full/path', '/full/path/to/repo.git') + subject { described_class.new(root_path, repo_path) } - expect(repo_path.project_full_path).to eq('to/repo') + it 'stores the repo path' do + expect(subject.repo_path).to eq(repo_path) + end + + it 'stores the wiki path' do + expect(subject.wiki_path).to eq(wiki_path) + end + + it 'reads the group path from .git/config' do + expect(subject.group_path).to eq('to') + end + + it 'reads the project name from .git/config' do + expect(subject.project_name).to eq('repo') + end + + describe '#processable?' do + it 'returns false if it is a wiki' do + subject = described_class.new(root_path, wiki_path) + + expect(subject).not_to be_processable + end + + it 'returns false when group and project name are missing' do + repository = Rugged::Repository.new(repo_path) + repository.config.delete('gitlab.fullpath') + + expect(subject).not_to be_processable + end + + it 'returns true when group path and project name are present' do + expect(subject).to be_processable + end + end + + describe '#project_full_path' do + it 'returns the project full path with trailing slash in the root path' do + expect(subject.project_full_path).to eq('to/repo') + end + + it 'returns the project full path with no trailing slash in the root path' do + subject = described_class.new(root_path[0...-1], repo_path) + + expect(subject.project_full_path).to eq('to/repo') + end end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index b7c6286fd83..0678cae9b93 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -203,7 +203,7 @@ describe Namespace do context 'with subgroups' do let(:parent) { create(:group, name: 'parent', path: 'parent') } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } - let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) } + let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child, skip_disk_validation: true) } let(:uploads_dir) { File.join(CarrierWave.root, FileUploader.base_dir) } let(:pages_dir) { File.join(TestEnv.pages_path) } @@ -240,6 +240,20 @@ describe Namespace do end end end + + it 'updates project full path in .git/config for each project inside namespace' do + parent = create(:group, name: 'mygroup', path: 'mygroup') + subgroup = create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) + project_in_parent_group = create(:project, :repository, namespace: parent, name: 'foo1') + hashed_project_in_subgroup = create(:project, :repository, :hashed, namespace: subgroup, name: 'foo2') + legacy_project_in_subgroup = create(:project, :repository, namespace: subgroup, name: 'foo3') + + parent.update(path: 'mygroup_new') + + expect(project_in_parent_group.repo.config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" + expect(hashed_project_in_subgroup.repo.config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" + expect(legacy_project_in_subgroup.repo.config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" + end end describe '#rm_dir', 'callback' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 7338e341359..cea22bbd184 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2626,6 +2626,14 @@ describe Project do project.rename_repo end end + + it 'updates project full path in .git/config' do + allow(project_storage).to receive(:rename_repo).and_return(true) + + project.rename_repo + + expect(project.repo.config['gitlab.fullpath']).to eq(project.full_path) + end end describe '#pages_path' do @@ -2668,14 +2676,12 @@ describe Project do end context 'hashed storage' do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, skip_disk_validation: true) } let(:gitlab_shell) { Gitlab::Shell.new } - let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } before do stub_application_setting(hashed_storage_enabled: true) - allow(Digest::SHA2).to receive(:hexdigest) { hash } - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) end describe '#legacy_storage?' do @@ -2698,13 +2704,13 @@ describe Project do describe '#base_dir' do it 'returns base_dir based on hash of project id' do - expect(project.base_dir).to eq('@hashed/6b/86') + expect(project.base_dir).to eq("@hashed/#{hash[0..1]}/#{hash[2..3]}") end end describe '#disk_path' do it 'returns disk_path based on hash of project id' do - hashed_path = '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' + hashed_path = "@hashed/#{hash[0..1]}/#{hash[2..3]}/#{hash}" expect(project.disk_path).to eq(hashed_path) end @@ -2712,7 +2718,9 @@ describe Project do describe '#ensure_storage_path_exists' do it 'delegates to gitlab_shell to ensure namespace is created' do - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, '@hashed/6b/86') + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, "@hashed/#{hash[0..1]}/#{hash[2..3]}") project.ensure_storage_path_exists end @@ -2772,7 +2780,7 @@ describe Project do end context 'when not rolled out' do - let(:project) { create(:project, :repository, storage_version: 1) } + let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } it 'moves pages folder to new location' do expect_any_instance_of(Gitlab::UploadsTransfer).to receive(:rename_project) @@ -2781,6 +2789,12 @@ describe Project do end end end + + it 'updates project full path in .git/config' do + project.rename_repo + + expect(project.repo.config['gitlab.fullpath']).to eq(project.full_path) + end end describe '#pages_path' do @@ -3141,4 +3155,26 @@ describe Project do it { is_expected.to eq(platform_kubernetes) } end end + + describe '#write_repository_config' do + set(:project) { create(:project, :repository) } + + it 'writes full path in .git/config when key is missing' do + project.write_repository_config + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end + + it 'updates full path in .git/config when key is present' do + project.write_repository_config(gl_full_path: 'old/path') + + expect { project.write_repository_config }.to change { project.repo.config['gitlab.fullpath'] }.from('old/path').to(project.full_path) + end + + it 'does not raise an error with an empty repository' do + project = create(:project_empty_repo) + + expect { project.write_repository_config }.not_to raise_error + end + end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index dc89fdebce7..1833078f37c 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -252,6 +252,12 @@ describe Projects::CreateService, '#execute' do end end + it 'writes project full path to .git/config' do + project = create_project(user, opts) + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end + def create_project(user, opts) Projects::CreateService.new(user, opts).execute end diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 3a3e47fd9c0..ded864beb1d 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::HashedStorage::MigrateRepositoryService do let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :empty_repo, :wiki_repo) } + let(:project) { create(:project, :repository, :wiki_repo) } let(:service) { described_class.new(project) } let(:legacy_storage) { Storage::LegacyProject.new(project) } let(:hashed_storage) { Storage::HashedProject.new(project) } @@ -33,6 +33,12 @@ describe Projects::HashedStorage::MigrateRepositoryService do service.execute end + + it 'writes project full path to .git/config' do + service.execute + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end end context 'when one move fails' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 2b1337bee7e..7377c748698 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -54,6 +54,12 @@ describe Projects::TransferService do expect(project.disk_path).not_to eq(old_path) expect(project.disk_path).to start_with(group.path) end + + it 'updates project full path in .git/config' do + transfer_project(project, user, group) + + expect(project.repo.config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" + end end context 'when transfer fails' do @@ -86,6 +92,12 @@ describe Projects::TransferService do expect(original_path).to eq current_path end + it 'rolls back project full path in .git/config' do + attempt_project_transfer + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end + it "doesn't send move notifications" do expect_any_instance_of(NotificationService).not_to receive(:project_was_moved) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d887f70efae..fc6aa713d6f 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -61,7 +61,7 @@ describe Projects::UpdateService do end end - context 'When project visibility is higher than parent group' do + context 'when project visibility is higher than parent group' do let(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } before do diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index ffc051a3fff..1d99746b09f 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -215,7 +215,7 @@ module TestEnv end def copy_repo(project, bare_repo:, refs:) - target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.full_path}.git") + target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.disk_path}.git") FileUtils.mkdir_p(target_repo_path) FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path) FileUtils.chmod_R 0755, target_repo_path