Merge branch '46246-gitlab-project-export-should-use-object-storage' into 'master'

Resolve "GitLab Project export should use object storage"

Closes #46246

See merge request gitlab-org/gitlab-ce!20105
This commit is contained in:
Sean McGivern 2018-07-06 18:57:58 +00:00
commit e0c0ce28ea
31 changed files with 509 additions and 46 deletions

View file

@ -2,6 +2,7 @@ class ProjectsController < Projects::ApplicationController
include IssuableCollections
include ExtractsPath
include PreviewMarkdown
include SendFileUpload
before_action :whitelist_query_limiting, only: [:create]
before_action :authenticate_user!, except: [:index, :show, :activity, :refs]
@ -188,9 +189,9 @@ class ProjectsController < Projects::ApplicationController
end
def download_export
export_project_path = @project.export_project_path
if export_project_path
if export_project_object_storage?
send_upload(@project.import_export_upload.export_file)
elsif export_project_path
send_file export_project_path, disposition: 'attachment'
else
redirect_to(
@ -265,8 +266,6 @@ class ProjectsController < Projects::ApplicationController
render json: options.to_json
end
private
# Render project landing depending of which features are available
# So if page is not availble in the list it renders the next page
#
@ -424,4 +423,12 @@ class ProjectsController < Projects::ApplicationController
def whitelist_query_limiting
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42440')
end
def export_project_path
@export_project_path ||= @project.export_project_path
end
def export_project_object_storage?
@project.export_project_object_exists?
end
end

View file

@ -0,0 +1,13 @@
class ImportExportUpload < ActiveRecord::Base
include WithUploads
include ObjectStorage::BackgroundMove
belongs_to :project
mount_uploader :import_file, ImportExportUploader
mount_uploader :export_file, ImportExportUploader
def retrieve_upload(_identifier, paths)
Upload.find_by(model: self, path: paths)
end
end

View file

@ -171,6 +171,7 @@ class Project < ActiveRecord::Base
has_one :fork_network, through: :fork_network_member
has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project
has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
# Merge Requests for target project should be removed with it
has_many :merge_requests, foreign_key: 'target_project_id'
@ -1712,7 +1713,7 @@ class Project < ActiveRecord::Base
:started
elsif after_export_in_progress?
:after_export_action
elsif export_project_path
elsif export_project_path || export_project_object_exists?
:finished
else
:none
@ -1727,16 +1728,21 @@ class Project < ActiveRecord::Base
import_export_shared.after_export_in_progress?
end
def remove_exports
return nil unless export_path.present?
FileUtils.rm_rf(export_path)
def remove_exports(path = export_path)
if path.present?
FileUtils.rm_rf(path)
elsif export_project_object_exists?
import_export_upload.remove_export_file!
import_export_upload.save
end
end
def remove_exported_project_file
return unless export_project_path.present?
remove_exports(export_project_path)
end
FileUtils.rm_f(export_project_path)
def export_project_object_exists?
Gitlab::ImportExport.object_storage? && import_export_upload&.export_file&.file
end
def full_path_slug

View file

@ -10,7 +10,9 @@ class ImportExportCleanUpService
def execute
Gitlab::Metrics.measure(:import_export_clean_up) do
next unless File.directory?(path)
clean_up_export_object_files
break unless File.directory?(path)
clean_up_export_files
end
@ -21,4 +23,11 @@ class ImportExportCleanUpService
def clean_up_export_files
Gitlab::Popen.popen(%W(find #{path} -not -path #{path} -mmin +#{mmin} -delete))
end
def clean_up_export_object_files
ImportExportUpload.where('updated_at < ?', mmin.minutes.ago).each do |upload|
upload.remove_export_file!
upload.save!
end
end
end

View file

@ -0,0 +1,15 @@
class ImportExportUploader < AttachmentUploader
EXTENSION_WHITELIST = %w[tar.gz].freeze
def extension_whitelist
EXTENSION_WHITELIST
end
def move_to_store
true
end
def move_to_cache
false
end
end

View file

@ -31,7 +31,7 @@
%li Any encrypted tokens
%p
Once the exported file is ready, you will receive a notification email with a download link, or you can download it from this page.
- if project.export_project_path
- if project.export_status == :finished
= link_to 'Download export', download_export_project_path(project),
rel: 'nofollow', download: '', method: :get, class: "btn btn-default"
= link_to 'Generate new export', generate_new_export_project_path(project),

View file

@ -0,0 +1,5 @@
---
title: Add Object Storage to project export
merge_request: 20105
author:
type: added

View file

@ -0,0 +1,16 @@
class CreateImportExportUploads < ActiveRecord::Migration
DOWNTIME = false
def change
create_table :import_export_uploads do |t|
t.datetime_with_timezone :updated_at, null: false
t.references :project, index: true, foreign_key: { on_delete: :cascade }, unique: true
t.text :import_file
t.text :export_file
end
add_index :import_export_uploads, :updated_at
end
end

View file

@ -949,6 +949,16 @@ ActiveRecord::Schema.define(version: 20180702120647) do
add_index "identities", ["user_id"], name: "index_identities_on_user_id", using: :btree
create_table "import_export_uploads", force: :cascade do |t|
t.datetime_with_timezone "updated_at", null: false
t.integer "project_id"
t.text "import_file"
t.text "export_file"
end
add_index "import_export_uploads", ["project_id"], name: "index_import_export_uploads_on_project_id", using: :btree
add_index "import_export_uploads", ["updated_at"], name: "index_import_export_uploads_on_updated_at", using: :btree
create_table "internal_ids", id: :bigserial, force: :cascade do |t|
t.integer "project_id"
t.integer "usage", null: false

View file

@ -30,5 +30,12 @@ sudo gitlab-rake gitlab:import_export:data
bundle exec rake gitlab:import_export:data RAILS_ENV=production
```
In order to enable Object Storage on the Export, you can use the [feature flag][feature-flags]:
```
import_export_object_storage
```
[ce-3050]: https://gitlab.com/gitlab-org/gitlab-ce/issues/3050
[feature-flags]: https://docs.gitlab.com/ee/api/features.html
[tmp]: ../../development/shared_files.md

View file

@ -23,9 +23,13 @@ module API
get ':id/export/download' do
path = user_project.export_project_path
render_api_error!('404 Not found or has expired', 404) unless path
present_disk_file!(path, File.basename(path), 'application/gzip')
if path
present_disk_file!(path, File.basename(path), 'application/gzip')
elsif user_project.export_project_object_exists?
present_carrierwave_file!(user_project.import_export_upload.export_file)
else
render_api_error!('404 Not found or has expired', 404)
end
end
desc 'Start export' do

View file

@ -40,6 +40,10 @@ module Gitlab
"#{basename[0..FILENAME_LIMIT]}_export.tar.gz"
end
def object_storage?
Feature.enabled?(:import_export_object_storage)
end
def version
VERSION
end

View file

@ -2,6 +2,7 @@ module Gitlab
module ImportExport
module AfterExportStrategies
class BaseAfterExportStrategy
extend Gitlab::ImportExport::CommandLineUtil
include ActiveModel::Validations
extend Forwardable
@ -24,9 +25,10 @@ module Gitlab
end
def execute(current_user, project)
return unless project&.export_project_path
@project = project
return unless @project.export_status == :finished
@current_user = current_user
if invalid?
@ -51,9 +53,12 @@ module Gitlab
end
def self.lock_file_path(project)
return unless project&.export_path
return unless project.export_path || object_storage?
File.join(project.export_path, AFTER_EXPORT_LOCK_FILE_NAME)
lock_path = project.import_export_shared.archive_path
mkdir_p(lock_path)
File.join(lock_path, AFTER_EXPORT_LOCK_FILE_NAME)
end
protected
@ -77,6 +82,10 @@ module Gitlab
def log_validation_errors
errors.full_messages.each { |msg| project.import_export_shared.add_error_message(msg) }
end
def object_storage?
project.export_project_object_exists?
end
end
end
end

View file

@ -38,14 +38,20 @@ module Gitlab
private
def send_file
export_file = File.open(project.export_project_path)
Gitlab::HTTP.public_send(http_method.downcase, url, send_file_options(export_file)) # rubocop:disable GitlabSecurity/PublicSend
Gitlab::HTTP.public_send(http_method.downcase, url, send_file_options) # rubocop:disable GitlabSecurity/PublicSend
ensure
export_file.close if export_file
export_file.close if export_file && !object_storage?
end
def send_file_options(export_file)
def export_file
if object_storage?
project.import_export_upload.export_file.file.open
else
File.open(project.export_project_path)
end
end
def send_file_options
{
body_stream: export_file,
headers: headers
@ -53,7 +59,15 @@ module Gitlab
end
def headers
{ 'Content-Length' => File.size(project.export_project_path).to_s }
{ 'Content-Length' => export_size.to_s }
end
def export_size
if object_storage?
project.import_export_upload.export_file.file.size
else
File.size(project.export_project_path)
end
end
end
end

View file

@ -15,15 +15,22 @@ module Gitlab
def save
if compress_and_save
remove_export_path
Rails.logger.info("Saved project export #{archive_file}")
archive_file
save_on_object_storage if use_object_storage?
else
@shared.error(Gitlab::ImportExport::Error.new("Unable to save #{archive_file} into #{@shared.export_path}"))
@shared.error(Gitlab::ImportExport::Error.new(error_message))
false
end
rescue => e
@shared.error(e)
false
ensure
if use_object_storage?
remove_archive
remove_export_path
end
end
private
@ -36,9 +43,29 @@ module Gitlab
FileUtils.rm_rf(@shared.export_path)
end
def remove_archive
FileUtils.rm_rf(@shared.archive_path)
end
def archive_file
@archive_file ||= File.join(@shared.archive_path, Gitlab::ImportExport.export_filename(project: @project))
end
def save_on_object_storage
upload = ImportExportUpload.find_or_initialize_by(project: @project)
File.open(archive_file) { |file| upload.export_file = file }
upload.save!
end
def use_object_storage?
Gitlab::ImportExport.object_storage?
end
def error_message
"Unable to save #{archive_file} into #{@shared.export_path}. Object storage enabled: #{use_object_storage?}"
end
end
end
end

View file

@ -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

View file

@ -0,0 +1,5 @@
FactoryBot.define do
factory :import_export_upload do
project { create(:project) }
end
end

View file

@ -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

View file

@ -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

View file

@ -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

BIN
spec/fixtures/project_export.tar.gz vendored Normal file

Binary file not shown.

View file

@ -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

View file

@ -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

View file

@ -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
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
context 'with object storage' do
before do
stub_feature_flags(import_export_object_storage: true)
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
end
end

View file

@ -293,6 +293,7 @@ project:
- deploy_tokens
- settings
- ci_cd_settings
- import_export_upload
award_emoji:
- awardable
- user

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -11,7 +11,6 @@ describe ImportExportCleanUpService do
path = '/invalid/path/'
stub_repository_downloads_path(path)
expect(File).to receive(:directory?).with(path + tmp_import_export_folder).and_return(false).at_least(:once)
expect(service).not_to receive(:clean_up_export_files)
service.execute
@ -38,6 +37,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)

View file

@ -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