Merge branch 'fix-data-integrity-issue-with-repository-downloads-path' into 'master'

Avoid data-integrity issue when cleaning up repository archive cache

## What does this MR do?
Sets the default value for `repository_downloads_path` if someone has it configured incorrectly, and it points to the path where repositories are stored. It's also replace invocation of `find` with Ruby code that matches old cached files in a better, and safe way to avoid data-integrity issues.

## Why was this MR needed?
The `repository_downloads_path` is used by the `RepositoryArchiveCacheWorker` to remove outdated repository archives, if it points to the wrong directory can cause some data-integrity issue.

## What are the relevant issue numbers?
Closes #14222

See merge request !5285

3 files changed, 34 insertions, 11 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 511df2d67c6..a6580e85498 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -11,16 +11,6 @@ class Repository
 
   attr_accessor :path_with_namespace, :project
 
-  def self.clean_old_archives
-    Gitlab::Metrics.measure(:clean_old_archives) do
-      repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path
-
-      return unless File.directory?(repository_downloads_path)
-
-      Gitlab::Popen.popen(%W(find #{repository_downloads_path} -not -path #{repository_downloads_path} -mmin +120 -delete))
-    end
-  end
-
   def initialize(path_with_namespace, project)
     @path_with_namespace = path_with_namespace
     @project = project
diff --git a/app/services/repository_archive_clean_up_service.rb b/app/services/repository_archive_clean_up_service.rb
new file mode 100644
index 00000000000..0b56b09738d
--- /dev/null
+++ b/app/services/repository_archive_clean_up_service.rb
@@ -0,0 +1,33 @@
+class RepositoryArchiveCleanUpService
+  LAST_MODIFIED_TIME_IN_MINUTES = 120
+
+  def initialize(mmin = LAST_MODIFIED_TIME_IN_MINUTES)
+    @mmin = mmin
+    @path = Gitlab.config.gitlab.repository_downloads_path
+  end
+
+  def execute
+    Gitlab::Metrics.measure(:repository_archive_clean_up) do
+      return unless File.directory?(path)
+
+      clean_up_old_archives
+      clean_up_empty_directories
+    end
+  end
+
+  private
+
+  attr_reader :mmin, :path
+
+  def clean_up_old_archives
+    run(%W(find #{path} -not -path #{path} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -maxdepth 2 -mmin +#{mmin} -delete))
+  end
+
+  def clean_up_empty_directories
+    run(%W(find #{path} -not -path #{path} -type d -empty -name \*.git -maxdepth 1 -delete))
+  end
+
+  def run(cmd)
+    Gitlab::Popen.popen(cmd)
+  end
+end
diff --git a/app/workers/repository_archive_cache_worker.rb b/app/workers/repository_archive_cache_worker.rb
index 47c5a670ed4..a2e49c61f59 100644
--- a/app/workers/repository_archive_cache_worker.rb
+++ b/app/workers/repository_archive_cache_worker.rb
@@ -4,6 +4,6 @@ class RepositoryArchiveCacheWorker
   sidekiq_options queue: :default
 
   def perform
-    Repository.clean_old_archives
+    RepositoryArchiveCleanUpService.new.execute
   end
 end