Merge branch '42593-restore-rake-task' into 'master'

Tweak restore rake task to provide an option to set alternate temporary backup directory

Closes #42593

See merge request gitlab-org/gitlab-ce!17516

11 files changed, 128 insertions, 34 deletions

diff --git a/lib/backup/artifacts.rb b/lib/backup/artifacts.rb
index 4383124d150..6a5a223a614 100644
--- a/lib/backup/artifacts.rb
+++ b/lib/backup/artifacts.rb
@@ -5,9 +5,5 @@ module Backup
     def initialize
       super('artifacts', JobArtifactUploader.root)
     end
-
-    def create_files_dir
-      Dir.mkdir(app_files_dir, 0700)
-    end
   end
 end
diff --git a/lib/backup/builds.rb b/lib/backup/builds.rb
index 635967f4bd4..f869916e199 100644
--- a/lib/backup/builds.rb
+++ b/lib/backup/builds.rb
@@ -5,9 +5,5 @@ module Backup
     def initialize
       super('builds', Settings.gitlab_ci.builds_path)
     end
-
-    def create_files_dir
-      Dir.mkdir(app_files_dir, 0700)
-    end
   end
 end
diff --git a/lib/backup/files.rb b/lib/backup/files.rb
index 287d591e88d..88cb7e7b5a4 100644
--- a/lib/backup/files.rb
+++ b/lib/backup/files.rb
@@ -1,7 +1,10 @@
 require 'open3'
+require_relative 'helper'
 
 module Backup
   class Files
+    include Backup::Helper
+
     attr_reader :name, :app_files_dir, :backup_tarball, :files_parent_dir
 
     def initialize(name, app_files_dir)
@@ -35,15 +38,22 @@ module Backup
 
     def restore
       backup_existing_files_dir
-      create_files_dir
 
-      run_pipeline!([%w(gzip -cd), %W(tar -C #{app_files_dir} -xf -)], in: backup_tarball)
+      run_pipeline!([%w(gzip -cd), %W(tar --unlink-first --recursive-unlink -C #{app_files_dir} -xf -)], in: backup_tarball)
     end
 
     def backup_existing_files_dir
-      timestamped_files_path = File.join(files_parent_dir, "#{name}.#{Time.now.to_i}")
+      timestamped_files_path = File.join(Gitlab.config.backup.path, "tmp", "#{name}.#{Time.now.to_i}")
       if File.exist?(app_files_dir)
-        FileUtils.mv(app_files_dir, File.expand_path(timestamped_files_path))
+        # Move all files in the existing repos directory except . and .. to
+        # repositories.old.<timestamp> directory
+        FileUtils.mkdir_p(timestamped_files_path, mode: 0700)
+        files = Dir.glob(File.join(app_files_dir, "*"), File::FNM_DOTMATCH) - [File.join(app_files_dir, "."), File.join(app_files_dir, "..")]
+        begin
+          FileUtils.mv(files, timestamped_files_path)
+        rescue Errno::EACCES
+          access_denied_error(app_files_dir)
+        end
       end
     end
 
diff --git a/lib/backup/helper.rb b/lib/backup/helper.rb
new file mode 100644
index 00000000000..a1ee0faefe9
--- /dev/null
+++ b/lib/backup/helper.rb
@@ -0,0 +1,17 @@
+module Backup
+  module Helper
+    def access_denied_error(path)
+      message = <<~EOS
+
+      ### NOTICE ###
+      As part of restore, the task tried to move existing content from #{path}.
+      However, it seems that directory contains files/folders that are not owned
+      by the user #{Gitlab.config.gitlab.user}. To proceed, please move the files
+      or folders inside #{path} to a secure location so that #{path} is empty and
+      run restore task again.
+
+      EOS
+      raise message
+    end
+  end
+end
diff --git a/lib/backup/lfs.rb b/lib/backup/lfs.rb
index 4153467fbee..4e234e50a7a 100644
--- a/lib/backup/lfs.rb
+++ b/lib/backup/lfs.rb
@@ -5,9 +5,5 @@ module Backup
     def initialize
       super('lfs', Settings.lfs.storage_path)
     end
-
-    def create_files_dir
-      Dir.mkdir(app_files_dir, 0700)
-    end
   end
 end
diff --git a/lib/backup/pages.rb b/lib/backup/pages.rb
index 215ded93bfe..5830b209d6e 100644
--- a/lib/backup/pages.rb
+++ b/lib/backup/pages.rb
@@ -5,9 +5,5 @@ module Backup
     def initialize
       super('pages', Gitlab.config.pages.path)
     end
-
-    def create_files_dir
-      Dir.mkdir(app_files_dir, 0700)
-    end
   end
 end
diff --git a/lib/backup/registry.rb b/lib/backup/registry.rb
index 67fe0231087..91698669402 100644
--- a/lib/backup/registry.rb
+++ b/lib/backup/registry.rb
@@ -5,9 +5,5 @@ module Backup
     def initialize
       super('registry', Settings.registry.path)
     end
-
-    def create_files_dir
-      Dir.mkdir(app_files_dir, 0700)
-    end
   end
 end
diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb
index 88a7f2a4235..89e3f1d9076 100644
--- a/lib/backup/repository.rb
+++ b/lib/backup/repository.rb
@@ -1,8 +1,11 @@
 require 'yaml'
+require_relative 'helper'
 
 module Backup
   class Repository
+    include Backup::Helper
     # rubocop:disable Metrics/AbcSize
+
     def dump
       prepare
 
@@ -63,18 +66,27 @@ module Backup
       end
     end
 
-    def restore
+    def prepare_directories
       Gitlab.config.repositories.storages.each do |name, repository_storage|
         path = repository_storage.legacy_disk_path
         next unless File.exist?(path)
 
-        # Move repos dir to 'repositories.old' dir
-        bk_repos_path = File.join(path, '..', 'repositories.old.' + Time.now.to_i.to_s)
-        FileUtils.mv(path, bk_repos_path)
-        # This is expected from gitlab:check
-        FileUtils.mkdir_p(path, mode: 02770)
+        # Move all files in the existing repos directory except . and .. to
+        # repositories.old.<timestamp> directory
+        bk_repos_path = File.join(Gitlab.config.backup.path, "tmp", "#{name}-repositories.old." + Time.now.to_i.to_s)
+        FileUtils.mkdir_p(bk_repos_path, mode: 0700)
+        files = Dir.glob(File.join(path, "*"), File::FNM_DOTMATCH) - [File.join(path, "."), File.join(path, "..")]
+
+        begin
+          FileUtils.mv(files, bk_repos_path)
+        rescue Errno::EACCES
+          access_denied_error(path)
+        end
       end
+    end
 
+    def restore
+      prepare_directories
       Project.find_each(batch_size: 1000) do |project|
         progress.print " * #{display_repo_path(project)} ... "
         path_to_project_repo = path_to_repo(project)
diff --git a/lib/backup/uploads.rb b/lib/backup/uploads.rb
index 35118375499..d46e2cd869d 100644
--- a/lib/backup/uploads.rb
+++ b/lib/backup/uploads.rb
@@ -5,9 +5,5 @@ module Backup
     def initialize
       super('uploads', Rails.root.join('public/uploads'))
     end
-
-    def create_files_dir
-      Dir.mkdir(app_files_dir)
-    end
   end
 end
diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb
new file mode 100644
index 00000000000..14d055cbcc1
--- /dev/null
+++ b/spec/lib/backup/files_spec.rb
@@ -0,0 +1,66 @@
+require 'spec_helper'
+
+describe Backup::Files do
+  let(:progress) { StringIO.new }
+  let!(:project) { create(:project) }
+
+  before do
+    allow(progress).to receive(:puts)
+    allow(progress).to receive(:print)
+    allow(FileUtils).to receive(:mkdir_p).and_return(true)
+    allow(FileUtils).to receive(:mv).and_return(true)
+    allow(File).to receive(:exist?).and_return(true)
+    allow(File).to receive(:realpath).with("/var/gitlab-registry").and_return("/var/gitlab-registry")
+    allow(File).to receive(:realpath).with("/var/gitlab-registry/..").and_return("/var")
+
+    allow_any_instance_of(String).to receive(:color) do |string, _color|
+      string
+    end
+
+    allow_any_instance_of(described_class).to receive(:progress).and_return(progress)
+  end
+
+  describe '#restore' do
+    subject { described_class.new('registry', '/var/gitlab-registry') }
+    let(:timestamp) { Time.utc(2017, 3, 22) }
+
+    around do |example|
+      Timecop.freeze(timestamp) { example.run }
+    end
+
+    describe 'folders with permission' do
+      before do
+        allow(subject).to receive(:run_pipeline!).and_return(true)
+        allow(subject).to receive(:backup_existing_files).and_return(true)
+        allow(Dir).to receive(:glob).with("/var/gitlab-registry/*", File::FNM_DOTMATCH).and_return(["/var/gitlab-registry/.", "/var/gitlab-registry/..", "/var/gitlab-registry/sample1"])
+      end
+
+      it 'moves all necessary files' do
+        allow(subject).to receive(:backup_existing_files).and_call_original
+        expect(FileUtils).to receive(:mv).with(["/var/gitlab-registry/sample1"], File.join(Gitlab.config.backup.path, "tmp", "registry.#{Time.now.to_i}"))
+        subject.restore
+      end
+
+      it 'raises no errors' do
+        expect { subject.restore }.not_to raise_error
+      end
+
+      it 'calls tar command with unlink' do
+        expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args)
+        subject.restore
+      end
+    end
+
+    describe 'folders without permissions' do
+      before do
+        allow(FileUtils).to receive(:mv).and_raise(Errno::EACCES)
+        allow(subject).to receive(:run_pipeline!).and_return(true)
+      end
+
+      it 'shows error message' do
+        expect(subject).to receive(:access_denied_error).with("/var/gitlab-registry")
+        subject.restore
+      end
+    end
+  end
+end
diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb
index 03573c304aa..e4c1c9bafc0 100644
--- a/spec/lib/backup/repository_spec.rb
+++ b/spec/lib/backup/repository_spec.rb
@@ -7,6 +7,8 @@ describe Backup::Repository do
   before do
     allow(progress).to receive(:puts)
     allow(progress).to receive(:print)
+    allow(FileUtils).to receive(:mkdir_p).and_return(true)
+    allow(FileUtils).to receive(:mv).and_return(true)
 
     allow_any_instance_of(String).to receive(:color) do |string, _color|
       string
@@ -68,6 +70,17 @@ describe Backup::Repository do
         end
       end
     end
+
+    describe 'folders without permissions' do
+      before do
+        allow(FileUtils).to receive(:mv).and_raise(Errno::EACCES)
+      end
+
+      it 'shows error message' do
+        expect(subject).to receive(:access_denied_error)
+        subject.restore
+      end
+    end
   end
 
   describe '#empty_repo?' do