From 60c0c0f3d08aa2c2a5be68aa784a86304fdb9c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 28 Mar 2017 17:27:44 +0000 Subject: Merge branch '29843-project-subgroup-transfer' into 'security' Use full path for moving directories when changing namespace path See merge request !2078 --- app/models/namespace.rb | 18 ++--- .../unreleased/29843-project-subgroup-transfer.yml | 4 ++ config/gitlab.yml.example | 2 + lib/gitlab/uploads_transfer.rb | 2 +- spec/models/namespace_spec.rb | 79 +++++++++++++++++++--- spec/support/stored_repositories.rb | 5 ++ spec/support/test_env.rb | 13 ++-- 7 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 changelogs/unreleased/29843-project-subgroup-transfer.yml create mode 100644 spec/support/stored_repositories.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 826ded22ae5..1d4b1f7d590 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -120,10 +120,10 @@ class Namespace < ActiveRecord::Base # Move the namespace directory in all storages paths used by member projects repository_storage_paths.each do |repository_storage_path| # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, path_was) + gitlab_shell.add_namespace(repository_storage_path, full_path_was) - unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path) - Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}" + unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" # if we cannot move namespace directory we should rollback # db changes in order to prevent out of sync between db and fs @@ -131,8 +131,8 @@ class Namespace < ActiveRecord::Base end end - Gitlab::UploadsTransfer.new.rename_namespace(path_was, path) - Gitlab::PagesTransfer.new.rename_namespace(path_was, path) + Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path) + Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path) remove_exports! @@ -155,7 +155,7 @@ class Namespace < ActiveRecord::Base def send_update_instructions projects.each do |project| - project.send_move_instructions("#{path_was}/#{project.path}") + project.send_move_instructions("#{full_path_was}/#{project.path}") end end @@ -230,10 +230,10 @@ class Namespace < ActiveRecord::Base old_repository_storage_paths.each do |repository_storage_path| # Move namespace directory into trash. # We will remove it later async - new_path = "#{path}+#{id}+deleted" + new_path = "#{full_path}+#{id}+deleted" - if gitlab_shell.mv_namespace(repository_storage_path, path, new_path) - message = "Namespace directory \"#{path}\" moved to \"#{new_path}\"" + if gitlab_shell.mv_namespace(repository_storage_path, full_path, new_path) + message = "Namespace directory \"#{full_path}\" moved to \"#{new_path}\"" Gitlab::AppLogger.info message # Remove namespace directroy async with delay so diff --git a/changelogs/unreleased/29843-project-subgroup-transfer.yml b/changelogs/unreleased/29843-project-subgroup-transfer.yml new file mode 100644 index 00000000000..1cf83517591 --- /dev/null +++ b/changelogs/unreleased/29843-project-subgroup-transfer.yml @@ -0,0 +1,4 @@ +--- +title: Correctly update paths when changing a child group +merge_request: +author: diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index d89e25c0959..bd27f01c872 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -570,6 +570,8 @@ test: # In order to setup it correctly you need to specify # your system username you use to run GitLab # user: YOUR_USERNAME + pages: + path: tmp/tests/pages repositories: storages: default: diff --git a/lib/gitlab/uploads_transfer.rb b/lib/gitlab/uploads_transfer.rb index 81701831a6a..7d0c47c5361 100644 --- a/lib/gitlab/uploads_transfer.rb +++ b/lib/gitlab/uploads_transfer.rb @@ -1,7 +1,7 @@ module Gitlab class UploadsTransfer < ProjectTransfer def root_dir - File.join(Rails.root, "public", "uploads") + File.join(CarrierWave.root, GitlabUploader.base_dir) end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 09aa6e9337f..ccaf0d7abc7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -129,10 +129,10 @@ describe Namespace, models: true do end end - describe '#move_dir' do + describe '#move_dir', repository: true do before do @namespace = create :namespace - @project = create(:empty_project, namespace: @namespace) + @project = create(:project_empty_repo, namespace: @namespace) allow(@namespace).to receive(:path_changed?).and_return(true) end @@ -141,9 +141,9 @@ describe Namespace, models: true do end it "moves dir if path changed" do - new_path = @namespace.path + "_new" - allow(@namespace).to receive(:path_was).and_return(@namespace.path) - allow(@namespace).to receive(:path).and_return(new_path) + new_path = @namespace.full_path + "_new" + allow(@namespace).to receive(:full_path_was).and_return(@namespace.full_path) + allow(@namespace).to receive(:full_path).and_return(new_path) expect(@namespace).to receive(:remove_exports!) expect(@namespace.move_dir).to be_truthy end @@ -161,16 +161,75 @@ describe Namespace, models: true do it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has tags in container registry') } end + + context 'renaming a sub-group' 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(:uploads_dir) { File.join(CarrierWave.root, 'uploads', 'parent') } + let(:pages_dir) { File.join(TestEnv.pages_path, 'parent') } + + before do + FileUtils.mkdir_p(File.join(uploads_dir, 'child', 'the-project')) + FileUtils.mkdir_p(File.join(pages_dir, 'child', 'the-project')) + end + + it 'correctly moves the repository, uploads and pages' do + expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git') + expected_upload_path = File.join(uploads_dir, 'renamed', 'the-project') + expected_pages_path = File.join(pages_dir, 'renamed', 'the-project') + + child.update_attributes!(path: 'renamed') + + expect(File.directory?(expected_repository_path)).to be(true) + expect(File.directory?(expected_upload_path)).to be(true) + expect(File.directory?(expected_pages_path)).to be(true) + end + end end - describe '#rm_dir', 'callback' do - let!(:project) { create(:empty_project, namespace: namespace) } - let!(:path) { File.join(Gitlab.config.repositories.storages.default['path'], namespace.full_path) } + describe '#rm_dir', 'callback', repository: true do + let!(:project) { create(:project_empty_repo, namespace: namespace) } + let(:repository_storage_path) { Gitlab.config.repositories.storages.default['path'] } + let(:path_in_dir) { File.join(repository_storage_path, namespace.full_path) } + let(:deleted_path) { namespace.full_path.gsub(namespace.path, "#{namespace.full_path}+#{namespace.id}+deleted") } + let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) } + + it 'renames its dirs when deleted' do + allow(GitlabShellWorker).to receive(:perform_in) - it "removes its dirs when deleted" do namespace.destroy - expect(File.exist?(path)).to be(false) + expect(File.exist?(deleted_path_in_dir)).to be(true) + end + + it 'schedules the namespace for deletion' do + expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path) + + namespace.destroy + end + + context 'in sub-groups' do + let(:parent) { create(:namespace, path: 'parent') } + let(:child) { create(:namespace, parent: parent, path: 'child') } + let!(:project) { create(:project_empty_repo, namespace: child) } + let(:path_in_dir) { File.join(repository_storage_path, 'parent', 'child') } + let(:deleted_path) { File.join('parent', "child+#{child.id}+deleted") } + let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) } + + it 'renames its dirs when deleted' do + allow(GitlabShellWorker).to receive(:perform_in) + + child.destroy + + expect(File.exist?(deleted_path_in_dir)).to be(true) + end + + it 'schedules the namespace for deletion' do + expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path) + + child.destroy + end end it 'removes the exports folder' do diff --git a/spec/support/stored_repositories.rb b/spec/support/stored_repositories.rb new file mode 100644 index 00000000000..df18926d58c --- /dev/null +++ b/spec/support/stored_repositories.rb @@ -0,0 +1,5 @@ +RSpec.configure do |config| + config.before(:each, :repository) do + TestEnv.clean_test_path + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 648b0380f18..54e222e44bf 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -61,9 +61,6 @@ module TestEnv clean_test_path - FileUtils.mkdir_p(repos_path) - FileUtils.mkdir_p(backup_path) - # Setup GitLab shell for test instance setup_gitlab_shell @@ -95,10 +92,14 @@ module TestEnv tmp_test_path = Rails.root.join('tmp', 'tests', '**') Dir[tmp_test_path].each do |entry| - unless File.basename(entry) =~ /\Agitlab-(shell|test|test-fork)\z/ + unless File.basename(entry) =~ /\Agitlab-(shell|test|test_bare|test-fork)\z/ FileUtils.rm_rf(entry) end end + + FileUtils.mkdir_p(repos_path) + FileUtils.mkdir_p(backup_path) + FileUtils.mkdir_p(pages_path) end def setup_gitlab_shell @@ -151,6 +152,10 @@ module TestEnv Gitlab.config.backup.path end + def pages_path + Gitlab.config.pages.path + end + def copy_forked_repo_with_submodules(project) base_repo_path = File.expand_path(forked_repo_path_bare) target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.full_path}.git") -- cgit v1.2.1