diff options
author | James Lopez <james@jameslopez.es> | 2018-06-25 15:10:26 +0200 |
---|---|---|
committer | James Lopez <james@jameslopez.es> | 2018-07-06 15:46:18 +0200 |
commit | a2bf1641546a1d3eeb3e9f44734854f655c0adef (patch) | |
tree | 0652c20a92513330aa09c4a2ec9adbfaeb3a6494 /spec | |
parent | b0fa01fce3822da94aee6264829841996beb6df3 (diff) | |
download | gitlab-ce-a2bf1641546a1d3eeb3e9f44734854f655c0adef.tar.gz |
Update Import/Export to use object storage (based on aa feature flag)
Diffstat (limited to 'spec')
16 files changed, 334 insertions, 16 deletions
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 34ed835a388..a2dfc43e9f7 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -790,23 +790,55 @@ describe ProjectsController do project.add_master(user) end - context 'when project export is enabled' do - it 'returns 302' do - get :download_export, namespace_id: project.namespace, id: project + context 'object storage disabled' do + before do + stub_feature_flags(import_export_object_storage: false) + end - expect(response).to have_gitlab_http_status(302) + context 'when project export is enabled' do + it 'returns 302' do + get :download_export, namespace_id: project.namespace, id: project + + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + get :download_export, namespace_id: project.namespace, id: project + + expect(response).to have_gitlab_http_status(404) + end end end - context 'when project export is disabled' do + context 'object storage enabled' do before do - stub_application_setting(project_export_enabled?: false) + stub_feature_flags(import_export_object_storage: true) end - it 'returns 404' do - get :download_export, namespace_id: project.namespace, id: project + context 'when project export is enabled' do + it 'returns 302' do + get :download_export, namespace_id: project.namespace, id: project - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + get :download_export, namespace_id: project.namespace, id: project + + expect(response).to have_gitlab_http_status(404) + end end end end diff --git a/spec/factories/import_export_uploads.rb b/spec/factories/import_export_uploads.rb new file mode 100644 index 00000000000..7750d49b1d0 --- /dev/null +++ b/spec/factories/import_export_uploads.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :import_export_upload do + project { create(:project) } + end +end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index f6b05bac0e8..f77ded23b18 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -103,6 +103,22 @@ FactoryBot.define do end trait :with_export do + before(:create) do |_project, _evaluator| + allow(Feature).to receive(:enabled?).with(:import_export_object_storage) { false } + allow(Feature).to receive(:enabled?).with('import_export_object_storage') { false } + end + + after(:create) do |project, _evaluator| + ProjectExportWorker.new.perform(project.creator.id, project.id) + end + end + + trait :with_object_export do + before(:create) do |_project, _evaluator| + allow(Feature).to receive(:enabled?).with(:import_export_object_storage) { true } + allow(Feature).to receive(:enabled?).with('import_export_object_storage') { true } + end + after(:create) do |project, evaluator| ProjectExportWorker.new.perform(project.creator.id, project.id) end diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb index 8a418356541..eb281cd2122 100644 --- a/spec/features/projects/import_export/export_file_spec.rb +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -25,6 +25,7 @@ describe 'Import/Export - project export integration test', :js do before do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + stub_feature_flags(import_export_object_storage: false) end after do diff --git a/spec/features/projects/import_export/namespace_export_file_spec.rb b/spec/features/projects/import_export/namespace_export_file_spec.rb index 7d056b0c140..9bb8a2063b5 100644 --- a/spec/features/projects/import_export/namespace_export_file_spec.rb +++ b/spec/features/projects/import_export/namespace_export_file_spec.rb @@ -5,6 +5,7 @@ describe 'Import/Export - Namespace export file cleanup', :js do before do allow(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + stub_feature_flags(import_export_object_storage: false) end after do diff --git a/spec/fixtures/project_export.tar.gz b/spec/fixtures/project_export.tar.gz Binary files differnew file mode 100644 index 00000000000..72ab2d71f35 --- /dev/null +++ b/spec/fixtures/project_export.tar.gz diff --git a/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_object_storage_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_object_storage_spec.rb new file mode 100644 index 00000000000..5059d68e54b --- /dev/null +++ b/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_object_storage_spec.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +describe Gitlab::ImportExport::AfterExportStrategies::BaseAfterExportStrategy do + let!(:service) { described_class.new } + let!(:project) { create(:project, :with_object_export) } + let(:shared) { project.import_export_shared } + let!(:user) { create(:user) } + + describe '#execute' do + before do + allow(service).to receive(:strategy_execute) + stub_feature_flags(import_export_object_storage: true) + end + + it 'returns if project exported file is not found' do + allow(project).to receive(:export_project_object_exists?).and_return(false) + + expect(service).not_to receive(:strategy_execute) + + service.execute(user, project) + end + + it 'creates a lock file in the export dir' do + allow(service).to receive(:delete_after_export_lock) + + service.execute(user, project) + + expect(lock_path_exist?).to be_truthy + end + + context 'when the method succeeds' do + it 'removes the lock file' do + service.execute(user, project) + + expect(lock_path_exist?).to be_falsey + end + end + + context 'when the method fails' do + before do + allow(service).to receive(:strategy_execute).and_call_original + end + + context 'when validation fails' do + before do + allow(service).to receive(:invalid?).and_return(true) + end + + it 'does not create the lock file' do + expect(service).not_to receive(:create_or_update_after_export_lock) + + service.execute(user, project) + end + + it 'does not execute main logic' do + expect(service).not_to receive(:strategy_execute) + + service.execute(user, project) + end + + it 'logs validation errors in shared context' do + expect(service).to receive(:log_validation_errors) + + service.execute(user, project) + end + end + + context 'when an exception is raised' do + it 'removes the lock' do + expect { service.execute(user, project) }.to raise_error(NotImplementedError) + + expect(lock_path_exist?).to be_falsey + end + end + end + end + + describe '#log_validation_errors' do + it 'add the message to the shared context' do + errors = %w(test_message test_message2) + + allow(service).to receive(:invalid?).and_return(true) + allow(service.errors).to receive(:full_messages).and_return(errors) + + expect(shared).to receive(:add_error_message).twice.and_call_original + + service.execute(user, project) + + expect(shared.errors).to eq errors + end + end + + describe '#to_json' do + it 'adds the current strategy class to the serialized attributes' do + params = { param1: 1 } + result = params.merge(klass: described_class.to_s).to_json + + expect(described_class.new(params).to_json).to eq result + end + end + + def lock_path_exist? + File.exist?(described_class.lock_file_path(project)) + end +end diff --git a/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb index ed54d87de4a..566b7f46c87 100644 --- a/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb +++ b/spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb @@ -9,6 +9,7 @@ describe Gitlab::ImportExport::AfterExportStrategies::BaseAfterExportStrategy do describe '#execute' do before do allow(service).to receive(:strategy_execute) + stub_feature_flags(import_export_object_storage: false) end it 'returns if project exported file is not found' do diff --git a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb index 5fe57d9987b..7f2e0a4ee2c 100644 --- a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb +++ b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb @@ -24,13 +24,34 @@ describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do end describe '#execute' do - it 'removes the exported project file after the upload' do - allow(strategy).to receive(:send_file) - allow(strategy).to receive(:handle_response_error) + context 'without object storage' do + before do + stub_feature_flags(import_export_object_storage: false) + end + + it 'removes the exported project file after the upload' do + allow(strategy).to receive(:send_file) + allow(strategy).to receive(:handle_response_error) + + expect(project).to receive(:remove_exported_project_file) + + strategy.execute(user, project) + end + end + + context 'with object storage' do + before do + stub_feature_flags(import_export_object_storage: true) + end - expect(project).to receive(:remove_exported_project_file) + it 'removes the exported project file after the upload' do + allow(strategy).to receive(:send_file) + allow(strategy).to receive(:handle_response_error) - strategy.execute(user, project) + expect(project).to receive(:remove_exported_project_file) + + strategy.execute(user, project) + end end end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 2ea66479c1b..084ce3066d6 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -293,6 +293,7 @@ project: - deploy_tokens - settings - ci_cd_settings +- import_export_upload award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/saver_spec.rb b/spec/lib/gitlab/import_export/saver_spec.rb new file mode 100644 index 00000000000..02f1a4b81aa --- /dev/null +++ b/spec/lib/gitlab/import_export/saver_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' +require 'fileutils' + +describe Gitlab::ImportExport::Saver do + let!(:project) { create(:project, :public, name: 'project') } + let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } + let(:shared) { project.import_export_shared } + subject { described_class.new(project: project, shared: shared) } + + before do + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + + FileUtils.mkdir_p(shared.export_path) + FileUtils.touch("#{shared.export_path}/tmp.bundle") + end + + after do + FileUtils.rm_rf(export_path) + end + + context 'local archive' do + it 'saves the repo to disk' do + stub_feature_flags(import_export_object_storage: false) + + subject.save + + expect(shared.errors).to be_empty + expect(Dir.empty?(shared.archive_path)).to be false + end + end + + context 'object storage' do + it 'saves the repo using object storage' do + stub_feature_flags(import_export_object_storage: true) + stub_uploads_object_storage(ImportExportUploader) + + subject.save + + expect(ImportExportUpload.find_by(project: project).export_file.url) + .to match(%r[\/uploads\/-\/system\/import_export_upload\/export_file.*]) + end + end +end diff --git a/spec/models/import_export_upload_spec.rb b/spec/models/import_export_upload_spec.rb new file mode 100644 index 00000000000..58af84b8a08 --- /dev/null +++ b/spec/models/import_export_upload_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe ImportExportUpload do + subject { described_class.new(project: create(:project)) } + + shared_examples 'stores the Import/Export file' do |method| + it 'stores the import file' do + subject.public_send("#{method}=", fixture_file_upload('spec/fixtures/project_export.tar.gz')) + + subject.save! + + url = "/uploads/-/system/import_export_upload/#{method}/#{subject.id}/project_export.tar.gz" + + expect(subject.public_send(method).url).to eq(url) + end + end + + context 'import' do + it_behaves_like 'stores the Import/Export file', :import_file + end + + context 'export' do + it_behaves_like 'stores the Import/Export file', :export_file + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c3aa6cd6fed..b9512b81678 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2782,6 +2782,10 @@ describe Project do let(:legacy_project) { create(:project, :legacy_storage, :with_export) } let(:project) { create(:project, :with_export) } + before do + stub_feature_flags(import_export_object_storage: false) + end + it 'removes the exports directory for the project' do expect(File.exist?(project.export_path)).to be_truthy @@ -2830,12 +2834,14 @@ describe Project do let(:project) { create(:project, :with_export) } it 'removes the exported project file' do + stub_feature_flags(import_export_object_storage: false) + exported_file = project.export_project_path expect(File.exist?(exported_file)).to be_truthy - allow(FileUtils).to receive(:rm_f).and_call_original - expect(FileUtils).to receive(:rm_f).with(exported_file).and_call_original + allow(FileUtils).to receive(:rm_rf).and_call_original + expect(FileUtils).to receive(:rm_rf).with(exported_file).and_call_original project.remove_exported_project_file diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 3834d27d0a9..a4615bd081f 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -192,6 +192,13 @@ describe API::ProjectExport do context 'when upload complete' do before do FileUtils.rm_rf(project_after_export.export_path) + + if project_after_export.export_project_object_exists? + upload = project_after_export.import_export_upload + + upload.remove_export_file! + upload.save + end end it_behaves_like '404 response' do @@ -261,6 +268,22 @@ describe API::ProjectExport do it_behaves_like 'get project export download not found' end end + + context 'when an uploader is used' do + before do + stub_uploads_object_storage(ImportExportUploader) + + [project, project_finished, project_after_export].each do |p| + p.add_master(user) + + upload = ImportExportUpload.new(project: p) + upload.export_file = fixture_file_upload('spec/fixtures/project_export.tar.gz', "`/tar.gz") + upload.save! + end + end + + it_behaves_like 'get project download by strategy' + end end describe 'POST /projects/:project_id/export' do diff --git a/spec/services/import_export_clean_up_service_spec.rb b/spec/services/import_export_clean_up_service_spec.rb index 1875d0448cd..de8ce9b6392 100644 --- a/spec/services/import_export_clean_up_service_spec.rb +++ b/spec/services/import_export_clean_up_service_spec.rb @@ -38,6 +38,24 @@ describe ImportExportCleanUpService do end end + context 'with uploader exports' do + it 'removes old files' do + upload = create(:import_export_upload, + updated_at: 2.days.ago, + export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz')) + + expect { service.execute }.to change { upload.reload.export_file.file.nil? }.to(true) + end + + it 'does not remove new files' do + upload = create(:import_export_upload, + updated_at: 1.hour.ago, + export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz')) + + expect { service.execute }.not_to change { upload.reload.export_file.file.nil? } + end + end + def in_directory_with_files(mtime:) Dir.mktmpdir do |tmpdir| stub_repository_downloads_path(tmpdir) diff --git a/spec/uploaders/import_export_uploader_spec.rb b/spec/uploaders/import_export_uploader_spec.rb new file mode 100644 index 00000000000..51b173b682d --- /dev/null +++ b/spec/uploaders/import_export_uploader_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe ImportExportUploader do + let(:model) { build_stubbed(:import_export_upload) } + let(:upload) { create(:upload, model: model) } + + subject { described_class.new(model, :import_file) } + + context "object_store is REMOTE" do + before do + stub_uploads_object_storage + end + + include_context 'with storage', described_class::Store::REMOTE + + it_behaves_like 'builds correct paths', + store_dir: %r[import_export_upload/import_file/], + upload_path: %r[import_export_upload/import_file/] + end +end |