Merge branch '48773-gitlab-project-import-should-use-object-storage' into 'master'

Resolve "GitLab Project Import should use object storage"

Closes #48773

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

app/models/import_export_upload.rb

@@ -6,6 +6,7 @@ class ImportExportUpload < ActiveRecord::Base
 
   belongs_to :project
 
+  # These hold the project Import/Export archives (.tar.gz files)
   mount_uploader :import_file, ImportExportUploader
   mount_uploader :export_file, ImportExportUploader
 

app/services/projects/gitlab_projects_import_service.rb

@@ -15,7 +15,7 @@ module Projects
     end
 
     def execute
-      prepare_template_environment(template_file&.path)
+      prepare_template_environment(template_file)
 
       prepare_import_params
 
@@ -61,7 +61,6 @@ module Projects
 
       if template_file
         params[:import_type] = 'gitlab_project'
-        params[:import_source] = import_upload_path
       end
 
       params[:import_data] = { data: data } if data.present?

app/uploaders/import_export_uploader.rb

 # frozen_string_literal: true
 
 class ImportExportUploader < AttachmentUploader
-  EXTENSION_WHITELIST = %w[tar.gz].freeze
+  EXTENSION_WHITELIST = %w[tar.gz gz].freeze
 
   def extension_whitelist
     EXTENSION_WHITELIST

changelogs/unreleased/48773-gitlab-project-import-should-use-object-storage.yml

+---
+title: Add object storage logic to project import
+merge_request: 20773
+author:
+type: added

lib/gitlab/import_export/command_line_util.rb

@@ -18,6 +18,21 @@ module Gitlab
 
       private
 
+      def download_or_copy_upload(uploader, upload_path)
+        if uploader.upload.local?
+          copy_files(uploader.path, upload_path)
+        else
+          download(uploader.url, upload_path)
+        end
+      end
+
+      def download(url, upload_path)
+        File.open(upload_path, 'w') do |file|
+          # Download (stream) file from the uploader's location
+          IO.copy_stream(URI.parse(url).open, file)
+        end
+      end
+
       def tar_with_options(archive:, dir:, options:)
         execute(%W(tar -#{options} #{archive} -C #{dir} .))
       end

lib/gitlab/import_export/file_importer.rb

@@ -10,15 +10,18 @@ module Gitlab
         new(*args).import
       end
 
-      def initialize(archive_file:, shared:)
+      def initialize(project:, archive_file:, shared:)
+        @project = project
         @archive_file = archive_file
         @shared = shared
       end
 
       def import
         mkdir_p(@shared.export_path)
+        mkdir_p(@shared.archive_path)
 
-        remove_symlinks!
+        remove_symlinks
+        copy_archive
 
         wait_for_archived_file do
           decompress_archive
@@ -27,7 +30,8 @@ module Gitlab
         @shared.error(e)
         false
       ensure
-        remove_symlinks!
+        remove_import_file
+        remove_symlinks
       end
 
       private
@@ -51,7 +55,15 @@ module Gitlab
         result
       end
 
-      def remove_symlinks!
+      def copy_archive
+        return if @archive_file
+
+        @archive_file = File.join(@shared.archive_path, Gitlab::ImportExport.export_filename(project: @project))
+
+        download_or_copy_upload(@project.import_export_upload.import_file, @archive_file)
+      end
+
+      def remove_symlinks
         extracted_files.each do |path|
           FileUtils.rm(path) if File.lstat(path).symlink?
         end
@@ -59,6 +71,10 @@ module Gitlab
         true
       end
 
+      def remove_import_file
+        FileUtils.rm_rf(@archive_file)
+      end
+
       def extracted_files
         Dir.glob("#{@shared.export_path}/**/*", File::FNM_DOTMATCH).reject { |f| IGNORED_FILENAMES.include?(File.basename(f)) }
       end

lib/gitlab/import_export/importer.rb

@@ -35,7 +35,8 @@ module Gitlab
       end
 
       def import_file
-        Gitlab::ImportExport::FileImporter.import(archive_file: @archive_file,
+        Gitlab::ImportExport::FileImporter.import(project: @project,
+                                                  archive_file: @archive_file,
                                                   shared: @shared)
       end
 
@@ -91,7 +92,14 @@ module Gitlab
       end
 
       def remove_import_file
-        FileUtils.rm_rf(@archive_file)
+        return unless Gitlab::ImportExport.object_storage?
+
+        upload = @project.import_export_upload
+
+        return unless upload&.import_file&.file
+
+        upload.remove_import_file!
+        upload.save!
       end
 
       def overwrite_project

lib/gitlab/import_export/uploads_manager.rb

@@ -91,10 +91,7 @@ module Gitlab
 
         mkdir_p(File.join(uploads_export_path, secret))
 
-        File.open(upload_path, 'w') do |file|
-          # Download (stream) file from the uploader's location
-          IO.copy_stream(URI.parse(upload.file.url).open, file)
-        end
+        download_or_copy_upload(upload, upload_path)
       end
     end
   end

lib/gitlab/template_helper.rb

@@ -2,11 +2,17 @@ module Gitlab
   module TemplateHelper
     include Gitlab::Utils::StrongMemoize
 
-    def prepare_template_environment(file_path)
-      return unless file_path.present?
+    def prepare_template_environment(file)
+      return unless file&.path.present?
 
-      FileUtils.mkdir_p(File.dirname(import_upload_path))
-      FileUtils.copy_entry(file_path, import_upload_path)
+      if Gitlab::ImportExport.object_storage?
+        params[:import_export_upload] = ImportExportUpload.new(import_file: file)
+      else
+        FileUtils.mkdir_p(File.dirname(import_upload_path))
+        FileUtils.copy_entry(file.path, import_upload_path)
+
+        params[:import_source] = import_upload_path
+      end
     end
 
     def import_upload_path

spec/features/projects/import_export/import_file_object_storage_spec.rb

+require 'spec_helper'
+
+describe 'Import/Export - project import integration test', :js do
+  include Select2Helper
+
+  let(:user) { create(:user) }
+  let(:file) { File.join(Rails.root, 'spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') }
+  let(:export_path) { "#{Dir.tmpdir}/import_file_spec" }
+
+  before do
+    stub_feature_flags(import_export_object_storage: true)
+    stub_uploads_object_storage(FileUploader)
+    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+    gitlab_sign_in(user)
+  end
+
+  after do
+    FileUtils.rm_rf(export_path, secure: true)
+  end
+
+  context 'when selecting the namespace' do
+    let(:user) { create(:admin) }
+    let!(:namespace) { user.namespace }
+    let(:project_path) { 'test-project-path' + SecureRandom.hex }
+
+    context 'prefilled the path' do
+      it 'user imports an exported project successfully' do
+        visit new_project_path
+
+        select2(namespace.id, from: '#project_namespace_id')
+        fill_in :project_path, with: project_path, visible: true
+        click_import_project_tab
+        click_link 'GitLab export'
+
+        expect(page).to have_content('Import an exported GitLab project')
+        expect(URI.parse(current_url).query).to eq("namespace_id=#{namespace.id}&path=#{project_path}")
+
+        attach_file('file', file)
+        click_on 'Import project'
+
+        expect(Project.count).to eq(1)
+
+        project = Project.last
+        expect(project).not_to be_nil
+        expect(project.description).to eq("Foo Bar")
+        expect(project.issues).not_to be_empty
+        expect(project.merge_requests).not_to be_empty
+        expect(project_hook_exists?(project)).to be true
+        expect(wiki_exists?(project)).to be true
+        expect(project.import_state.status).to eq('finished')
+      end
+    end
+
+    context 'path is not prefilled' do
+      it 'user imports an exported project successfully' do
+        visit new_project_path
+        click_import_project_tab
+        click_link 'GitLab export'
+
+        fill_in :path, with: 'test-project-path', visible: true
+        attach_file('file', file)
+
+        expect { click_on 'Import project' }.to change { Project.count }.by(1)
+
+        project = Project.last
+        expect(project).not_to be_nil
+        expect(page).to have_content("Project 'test-project-path' is being imported")
+      end
+    end
+  end
+
+  it 'invalid project' do
+    project = create(:project, namespace: user.namespace)
+
+    visit new_project_path
+
+    select2(user.namespace.id, from: '#project_namespace_id')
+    fill_in :project_path, with: project.name, visible: true
+    click_import_project_tab
+    click_link 'GitLab export'
+    attach_file('file', file)
+    click_on 'Import project'
+
+    page.within('.flash-container') do
+      expect(page).to have_content('Project could not be imported')
+    end
+  end
+
+  def wiki_exists?(project)
+    wiki = ProjectWiki.new(project)
+    wiki.repository.exists? && !wiki.repository.empty?
+  end
+
+  def project_hook_exists?(project)
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists?
+    end
+  end
+
+  def click_import_project_tab
+    find('#import-project-tab').click
+  end
+end

spec/features/projects/import_export/import_file_spec.rb

@@ -8,6 +8,7 @@ describe 'Import/Export - project import integration test', :js do
   let(:export_path) { "#{Dir.tmpdir}/import_file_spec" }
 
   before do
+    stub_feature_flags(import_export_object_storage: false)
     allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
     gitlab_sign_in(user)
   end

spec/lib/gitlab/import_export/file_importer_object_storage_spec.rb

+require 'spec_helper'
+
+describe Gitlab::ImportExport::FileImporter do
+  let(:shared) { Gitlab::ImportExport::Shared.new(nil) }
+  let(:storage_path) { "#{Dir.tmpdir}/file_importer_spec" }
+  let(:valid_file) { "#{shared.export_path}/valid.json" }
+  let(:symlink_file) { "#{shared.export_path}/invalid.json" }
+  let(:hidden_symlink_file) { "#{shared.export_path}/.hidden" }
+  let(:subfolder_symlink_file) { "#{shared.export_path}/subfolder/invalid.json" }
+  let(:evil_symlink_file) { "#{shared.export_path}/.\nevil" }
+
+  before do
+    stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0)
+    stub_feature_flags(import_export_object_storage: true)
+    stub_uploads_object_storage(FileUploader)
+
+    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(storage_path)
+    allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true)
+    allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:relative_archive_path).and_return('test')
+    allow(SecureRandom).to receive(:hex).and_return('abcd')
+    setup_files
+  end
+
+  after do
+    FileUtils.rm_rf(storage_path)
+  end
+
+  context 'normal run' do
+    before do
+      described_class.import(project: build(:project), archive_file: '', shared: shared)
+    end
+
+    it 'removes symlinks in root folder' do
+      expect(File.exist?(symlink_file)).to be false
+    end
+
+    it 'removes hidden symlinks in root folder' do
+      expect(File.exist?(hidden_symlink_file)).to be false
+    end
+
+    it 'removes evil symlinks in root folder' do
+      expect(File.exist?(evil_symlink_file)).to be false
+    end
+
+    it 'removes symlinks in subfolders' do
+      expect(File.exist?(subfolder_symlink_file)).to be false
+    end
+
+    it 'does not remove a valid file' do
+      expect(File.exist?(valid_file)).to be true
+    end
+
+    it 'creates the file in the right subfolder' do
+      expect(shared.export_path).to include('test/abcd')
+    end
+  end
+
+  context 'error' do
+    before do
+      allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError)
+      described_class.import(project: build(:project), archive_file: '', shared: shared)
+    end
+
+    it 'removes symlinks in root folder' do
+      expect(File.exist?(symlink_file)).to be false
+    end
+
+    it 'removes hidden symlinks in root folder' do
+      expect(File.exist?(hidden_symlink_file)).to be false
+    end
+
+    it 'removes symlinks in subfolders' do
+      expect(File.exist?(subfolder_symlink_file)).to be false
+    end
+
+    it 'does not remove a valid file' do
+      expect(File.exist?(valid_file)).to be true
+    end
+  end
+
+  def setup_files
+    FileUtils.mkdir_p("#{shared.export_path}/subfolder/")
+    FileUtils.touch(valid_file)
+    FileUtils.ln_s(valid_file, symlink_file)
+    FileUtils.ln_s(valid_file, subfolder_symlink_file)
+    FileUtils.ln_s(valid_file, hidden_symlink_file)
+    FileUtils.ln_s(valid_file, evil_symlink_file)
+  end
+end

spec/lib/gitlab/import_export/file_importer_spec.rb

@@ -24,7 +24,7 @@ describe Gitlab::ImportExport::FileImporter do
 
   context 'normal run' do
     before do
-      described_class.import(archive_file: '', shared: shared)
+      described_class.import(project: nil, archive_file: '', shared: shared)
     end
 
     it 'removes symlinks in root folder' do
@@ -55,7 +55,7 @@ describe Gitlab::ImportExport::FileImporter do
   context 'error' do
     before do
       allow_any_instance_of(described_class).to receive(:wait_for_archived_file).and_raise(StandardError)
-      described_class.import(archive_file: '', shared: shared)
+      described_class.import(project: nil, archive_file: '', shared: shared)
     end
 
     it 'removes symlinks in root folder' do

spec/lib/gitlab/import_export/importer_object_storage_spec.rb

+require 'spec_helper'
+
+describe Gitlab::ImportExport::Importer do
+  let(:user) { create(:user) }
+  let(:test_path) { "#{Dir.tmpdir}/importer_spec" }
+  let(:shared) { project.import_export_shared }
+  let(:project) { create(:project) }
+  let(:import_file) { fixture_file_upload('spec/features/projects/import_export/test_project_export.tar.gz') }
+
+  subject(:importer) { described_class.new(project) }
+
+  before do
+    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(test_path)
+    allow_any_instance_of(Gitlab::ImportExport::FileImporter).to receive(:remove_import_file)
+    stub_feature_flags(import_export_object_storage: true)
+    stub_uploads_object_storage(FileUploader)
+
+    FileUtils.mkdir_p(shared.export_path)
+    ImportExportUpload.create(project: project, import_file: import_file)
+  end
+
+  after do
+    FileUtils.rm_rf(test_path)
+  end
+
+  describe '#execute' do
+    it 'succeeds' do
+      importer.execute
+
+      expect(shared.errors).to be_empty
+    end
+
+    it 'extracts the archive'  do
+      expect(Gitlab::ImportExport::FileImporter).to receive(:import).and_call_original
+
+      importer.execute
+    end
+
+    it 'checks the version' do
+      expect(Gitlab::ImportExport::VersionChecker).to receive(:check!).and_call_original
+
+      importer.execute
+    end
+
+    context 'all restores are executed' do
+      [
+        Gitlab::ImportExport::AvatarRestorer,
+        Gitlab::ImportExport::RepoRestorer,
+        Gitlab::ImportExport::WikiRestorer,
+        Gitlab::ImportExport::UploadsRestorer,
+        Gitlab::ImportExport::LfsRestorer,
+        Gitlab::ImportExport::StatisticsRestorer
+      ].each do |restorer|
+        it "calls the #{restorer}" do
+          fake_restorer = double(restorer.to_s)
+
+          expect(fake_restorer).to receive(:restore).and_return(true).at_least(1)
+          expect(restorer).to receive(:new).and_return(fake_restorer).at_least(1)
+
+          importer.execute
+        end
+      end
+
+      it 'restores the ProjectTree' do
+        expect(Gitlab::ImportExport::ProjectTreeRestorer).to receive(:new).and_call_original
+
+        importer.execute
+      end
+
+      it 'removes the import file' do
+        expect(importer).to receive(:remove_import_file).and_call_original
+
+        importer.execute
+
+        expect(project.import_export_upload.import_file&.file).to be_nil
+      end
+    end
+
+    context 'when project successfully restored' do
+      let!(:existing_project) { create(:project, namespace: user.namespace) }
+      let(:project) { create(:project, namespace: user.namespace, name: 'whatever', path: 'whatever') }
+
+      before do
+        restorers = double(:restorers, all?: true)
+
+        allow(subject).to receive(:import_file).and_return(true)
+        allow(subject).to receive(:check_version!).and_return(true)
+        allow(subject).to receive(:restorers).and_return(restorers)
+        allow(project).to receive(:import_data).and_return(double(data: { 'original_path' => existing_project.path }))
+      end
+
+      context 'when import_data' do
+        context 'has original_path' do
+          it 'overwrites existing project' do
+            expect_any_instance_of(::Projects::OverwriteProjectService).to receive(:execute).with(existing_project)
+
+            subject.execute
+          end
+        end
+
+        context 'has not original_path' do
+          before do
+            allow(project).to receive(:import_data).and_return(double(data: {}))
+          end
+
+          it 'does not call the overwrite service' do
+            expect_any_instance_of(::Projects::OverwriteProjectService).not_to receive(:execute).with(existing_project)
+
+            subject.execute
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/importer_spec.rb

@@ -10,9 +10,10 @@ describe Gitlab::ImportExport::Importer do
 
   before do
     allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(test_path)
+    allow_any_instance_of(Gitlab::ImportExport::FileImporter).to receive(:remove_import_file)
+
     FileUtils.mkdir_p(shared.export_path)
     FileUtils.cp(Rails.root.join('spec/features/projects/import_export/test_project_export.tar.gz'), test_path)
-    allow(subject).to receive(:remove_import_file)
   end
 
   after do
@@ -69,7 +70,7 @@ describe Gitlab::ImportExport::Importer do
       let(:project) { create(:project, namespace: user.namespace, name: 'whatever', path: 'whatever') }
 
       before do
-        restorers = double
+        restorers = double(:restorers, all?: true)
 
         allow(subject).to receive(:import_file).and_return(true)
         allow(subject).to receive(:check_version!).and_return(true)

spec/requests/api/project_import_spec.rb

@@ -7,6 +7,8 @@ describe API::ProjectImport do
   let(:namespace) { create(:group) }
   before do
     allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+    stub_feature_flags(import_export_object_storage: true)
+    stub_uploads_object_storage(FileUploader)
 
     namespace.add_owner(user)
   end

spec/services/projects/gitlab_projects_import_service_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Projects::GitlabProjectsImportService do
   set(:namespace) { create(:namespace) }
   let(:path) { 'test-path' }
-  let(:file) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') }
+  let(:file) { fixture_file_upload('spec/fixtures/project_export.tar.gz') }
   let(:overwrite) { false }
   let(:import_params) { { namespace_id: namespace.id, path: path, file: file, overwrite: overwrite } }