diff --git a/app/services/import/gitlab_projects/create_project_from_remote_file_service.rb b/app/services/import/gitlab_projects/create_project_from_remote_file_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..bbfdaf692f97421d562dbc88f20feaafe8b6f2b6 --- /dev/null +++ b/app/services/import/gitlab_projects/create_project_from_remote_file_service.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module Import + module GitlabProjects + class CreateProjectFromRemoteFileService < CreateProjectFromUploadedFileService + FILE_SIZE_LIMIT = 10.gigabytes + ALLOWED_CONTENT_TYPES = ['application/gzip'].freeze + + validate :valid_remote_import_url? + validate :validate_file_size + validate :validate_content_type + + private + + def required_params + [:path, :namespace, :remote_import_url] + end + + def project_params + super + .except(:file) + .merge(import_export_upload: ::ImportExportUpload.new( + remote_import_url: params[:remote_import_url] + )) + end + + def valid_remote_import_url? + ::Gitlab::UrlBlocker.validate!( + params[:remote_import_url], + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + schemes: %w(http https) + ) + + true + rescue ::Gitlab::UrlBlocker::BlockedUrlError => e + errors.add(:base, e.message) + + false + end + + def allow_local_requests? + ::Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end + + def validate_content_type + if headers['content-type'].blank? + errors.add(:base, "Missing 'ContentType' header") + elsif !ALLOWED_CONTENT_TYPES.include?(headers['content-type']) + errors.add(:base, "Remote file content type '%{content_type}' not allowed. (Allowed content types: %{allowed})" % { + content_type: headers['content-type'], + allowed: ALLOWED_CONTENT_TYPES.join(',') + }) + end + end + + def validate_file_size + if headers['content-length'].to_i == 0 + errors.add(:base, "Missing 'ContentLength' header") + elsif headers['content-length'].to_i > FILE_SIZE_LIMIT + errors.add(:base, 'Remote file larger than limit. (limit %{limit})' % { + limit: ActiveSupport::NumberHelper.number_to_human_size(FILE_SIZE_LIMIT) + }) + end + end + + def headers + return {} if params[:remote_import_url].blank? || !valid_remote_import_url? + + @headers ||= Gitlab::HTTP.head(params[:remote_import_url]).headers + end + end + end +end diff --git a/app/services/import/gitlab_projects/create_project_from_uploaded_file_service.rb b/app/services/import/gitlab_projects/create_project_from_uploaded_file_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..35d52a112889d301eb84e52e0b9ec64fdb0bfc45 --- /dev/null +++ b/app/services/import/gitlab_projects/create_project_from_uploaded_file_service.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module Import + module GitlabProjects + class CreateProjectFromUploadedFileService + include ActiveModel::Validations + include ::Services::ReturnServiceResponses + + validate :required_params_presence + + def initialize(current_user, params = {}) + @current_user = current_user + @params = params.dup + end + + def execute + return error(errors.full_messages.first) unless valid? + return error(project.errors.full_messages&.first) unless project.saved? + + success(project) + rescue StandardError => e + error(e.message) + end + + private + + attr_reader :current_user, :params + + def error(message) + super(message, :bad_request) + end + + def project + @project ||= ::Projects::GitlabProjectsImportService.new( + current_user, + project_params, + params[:override] + ).execute + end + + def project_params + { + name: params[:name], + path: params[:path], + namespace_id: params[:namespace].id, + file: params[:file], + overwrite: params[:overwrite], + import_type: 'gitlab_project' + } + end + + def required_params + [:path, :namespace, :file] + end + + def required_params_presence + required_params + .select { |key| params[key].blank? } + .each do |missing_parameter| + errors.add(:base, "Parameter '#{missing_parameter}' is required") + end + end + end + end +end diff --git a/changelogs/unreleased/kassio-import-from-external-object-storage.yml b/changelogs/unreleased/kassio-import-from-external-object-storage.yml new file mode 100644 index 0000000000000000000000000000000000000000..ed50ae6221ff8e6b249f87245245875b37d3a8da --- /dev/null +++ b/changelogs/unreleased/kassio-import-from-external-object-storage.yml @@ -0,0 +1,7 @@ +--- +title: > + Create "projects/import-remote" to import a project using a remote object storage to fetch + the exported project +merge_request: 59033 +author: +type: added diff --git a/config/feature_flags/development/import_project_from_remote_file.yml b/config/feature_flags/development/import_project_from_remote_file.yml new file mode 100644 index 0000000000000000000000000000000000000000..9a44491172c0aa9f8e2dd968f514cd196695027d --- /dev/null +++ b/config/feature_flags/development/import_project_from_remote_file.yml @@ -0,0 +1,8 @@ +--- +name: import_project_from_remote_file +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59033 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330039 +milestone: '13.12' +type: development +group: group::import +default_enabled: false diff --git a/db/migrate/20210419203017_add_remote_import_url_to_import_export_upload.rb b/db/migrate/20210419203017_add_remote_import_url_to_import_export_upload.rb new file mode 100644 index 0000000000000000000000000000000000000000..00c5329406e8b057ff90fac15580067762eedfc4 --- /dev/null +++ b/db/migrate/20210419203017_add_remote_import_url_to_import_export_upload.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddRemoteImportUrlToImportExportUpload < ActiveRecord::Migration[6.0] + # limit is added in 20210419203018_add_remote_text_limit_to_import_url_in_import_export_upload.rb + def change + add_column :import_export_uploads, :remote_import_url, :text # rubocop:disable Migration/AddLimitToTextColumns + end +end diff --git a/db/migrate/20210419203018_add_remote_text_limit_to_import_url_in_import_export_upload.rb b/db/migrate/20210419203018_add_remote_text_limit_to_import_url_in_import_export_upload.rb new file mode 100644 index 0000000000000000000000000000000000000000..81b4e76b8d9060f409766194f0b63c10250c71b8 --- /dev/null +++ b/db/migrate/20210419203018_add_remote_text_limit_to_import_url_in_import_export_upload.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddRemoteTextLimitToImportUrlInImportExportUpload < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def up + add_text_limit :import_export_uploads, :remote_import_url, 512 + end + + def down + remove_text_limit :import_export_uploads, :remote_import_url + end +end diff --git a/db/schema_migrations/20210419203017 b/db/schema_migrations/20210419203017 new file mode 100644 index 0000000000000000000000000000000000000000..8421fd7c6d99a43a4b54584a94a826c3ceb51c43 --- /dev/null +++ b/db/schema_migrations/20210419203017 @@ -0,0 +1 @@ +1ca5f960c233be5d5a30632b8aaad9598c259154eee817f4d76e8f1bb3e95edb \ No newline at end of file diff --git a/db/schema_migrations/20210419203018 b/db/schema_migrations/20210419203018 new file mode 100644 index 0000000000000000000000000000000000000000..f28ebe78fc69ac4070afe1c79030b97af7b0a426 --- /dev/null +++ b/db/schema_migrations/20210419203018 @@ -0,0 +1 @@ +94404ed645a9c8a0ee462baff98cf2d0e50aecdb71bb1515fd3a82bf1a39dfda \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 7a8cd6e3072a7bc2ebe73b1fab25b51fe70363f7..2f833db4290d490be4fbb44897550e1c4f3baf26 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13565,7 +13565,9 @@ CREATE TABLE import_export_uploads ( project_id integer, import_file text, export_file text, - group_id bigint + group_id bigint, + remote_import_url text, + CONSTRAINT check_58f0d37481 CHECK ((char_length(remote_import_url) <= 512)) ); CREATE SEQUENCE import_export_uploads_id_seq diff --git a/lib/api/project_import.rb b/lib/api/project_import.rb index 5f3a574eeee0883e1d9a8569c4763a77992f6d1b..039f7b4be41286d04b15b2c0a4c22fad70ba604c 100644 --- a/lib/api/project_import.rb +++ b/lib/api/project_import.rb @@ -14,6 +14,21 @@ class ProjectImport < ::API::Base def import_params declared_params(include_missing: false) end + + def namespace_from(params, current_user) + if params[:namespace] + find_namespace!(params[:namespace]) + else + current_user.namespace + end + end + + def filtered_override_params(params) + override_params = params.delete(:override_params) + filter_attributes_using_license!(override_params) if override_params + + override_params + end end before do @@ -67,34 +82,25 @@ def import_params check_rate_limit! :project_import, [current_user, :project_import] - Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/20823') + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/21041') validate_file! - namespace = if import_params[:namespace] - find_namespace!(import_params[:namespace]) - else - current_user.namespace - end - - project_params = { - path: import_params[:path], - namespace_id: namespace.id, - name: import_params[:name], - file: import_params[:file], - overwrite: import_params[:overwrite] - } - - override_params = import_params.delete(:override_params) - filter_attributes_using_license!(override_params) if override_params - - project = ::Projects::GitlabProjectsImportService.new( - current_user, project_params, override_params + response = ::Import::GitlabProjects::CreateProjectFromUploadedFileService.new( + current_user, + path: import_params[:path], + namespace: namespace_from(import_params, current_user), + name: import_params[:name], + file: import_params[:file], + overwrite: import_params[:overwrite], + override: filtered_override_params(import_params) ).execute - render_api_error!(project.errors.full_messages&.first, 400) unless project.saved? - - present project, with: Entities::ProjectImportStatus + if response.success? + present(response.payload, with: Entities::ProjectImportStatus) + else + render_api_error!(response.message, response.http_status) + end end params do @@ -107,6 +113,44 @@ def import_params get ':id/import' do present user_project, with: Entities::ProjectImportStatus end + + params do + requires :url, type: String, desc: 'The URL for the file.' + requires :path, type: String, desc: 'The new project path and name' + optional :name, type: String, desc: 'The name of the project to be imported. Defaults to the path of the project if not provided.' + optional :namespace, type: String, desc: "The ID or name of the namespace that the project will be imported into. Defaults to the current user's namespace." + optional :overwrite, type: Boolean, default: false, desc: 'If there is a project in the same namespace and with the same name overwrite it' + optional :override_params, + type: Hash, + desc: 'New project params to override values in the export' do + use :optional_project_params + end + end + desc 'Create a new project import using a remote object storage path' do + detail 'This feature was introduced in GitLab 13.2.' + success Entities::ProjectImportStatus + end + post 'remote-import' do + not_found! unless ::Feature.enabled?(:import_project_from_remote_file) + + check_rate_limit! :project_import, [current_user, :project_import] + + response = ::Import::GitlabProjects::CreateProjectFromRemoteFileService.new( + current_user, + path: import_params[:path], + namespace: namespace_from(import_params, current_user), + name: import_params[:name], + remote_import_url: import_params[:url], + overwrite: import_params[:overwrite], + override: filtered_override_params(import_params) + ).execute + + if response.success? + present(response.payload, with: Entities::ProjectImportStatus) + else + render_api_error!(response.message, response.http_status) + end + end end end end diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb index af195fd4d11ed5cc005037d50cbb3d02368cc4e0..6f8cfec642515443ea3ad1514588ed16860028bb 100644 --- a/lib/gitlab/import_export/file_importer.rb +++ b/lib/gitlab/import_export/file_importer.rb @@ -67,7 +67,17 @@ def copy_archive @archive_file = File.join(@shared.archive_path, Gitlab::ImportExport.export_filename(exportable: @importable)) - download_or_copy_upload(@importable.import_export_upload.import_file, @archive_file) + remote_download_or_download_or_copy_upload + end + + def remote_download_or_download_or_copy_upload + import_export_upload = @importable.import_export_upload + + if import_export_upload.remote_import_url.present? + download(remote_import_url, @archive_file) + else + download_or_copy_upload(import_export_upload.import_file, @archive_file) + end end def remove_symlinks diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index f6cdf370e5cf801c589be83a6ea39b32c2d9f41c..d3b24eb38323d8ef1d96843ac1b6f03866fdbdff 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -4,6 +4,7 @@ RSpec.describe API::ProjectImport do include WorkhorseHelpers + include AfterNextHelpers include_context 'workhorse headers' @@ -31,6 +32,12 @@ allow(ImportExportUploader).to receive(:workhorse_upload_path).and_return('/') end + it 'executes a limited number of queries' do + control_count = ActiveRecord::QueryRecorder.new { subject }.count + + expect(control_count).to be <= 100 + end + it 'schedules an import using a namespace' do stub_import(namespace) params[:namespace] = namespace.id @@ -273,6 +280,75 @@ def stub_import(namespace) end end + describe 'POST /projects/remote-import' do + let(:params) do + { + path: 'test-import', + url: 'http://some.s3.url/file' + } + end + + it 'returns NOT FOUND when the feature is disabled' do + stub_feature_flags(import_project_from_remote_file: false) + + post api('/projects/remote-import', user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when the feature flag is enabled' do + before do + stub_feature_flags(import_project_from_remote_file: true) + end + + context 'when the response is successful' do + it 'schedules the import successfully' do + project = create( + :project, + namespace: user.namespace, + name: 'test-import', + path: 'test-import' + ) + + service_response = ServiceResponse.success(payload: project) + expect_next(::Import::GitlabProjects::CreateProjectFromRemoteFileService) + .to receive(:execute) + .and_return(service_response) + + post api('/projects/remote-import', user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include({ + 'id' => project.id, + 'name' => 'test-import', + 'name_with_namespace' => "#{user.namespace.name} / test-import", + 'path' => 'test-import', + 'path_with_namespace' => "#{user.namespace.path}/test-import" + }) + end + end + + context 'when the service returns an error' do + it 'fails to schedule the import' do + service_response = ServiceResponse.error( + message: 'Failed to import', + http_status: :bad_request + ) + expect_next(::Import::GitlabProjects::CreateProjectFromRemoteFileService) + .to receive(:execute) + .and_return(service_response) + + post api('/projects/remote-import', user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response).to eq({ + 'message' => 'Failed to import' + }) + end + end + end + end + describe 'GET /projects/:id/import' do it 'returns the import status' do project = create(:project, :import_started) diff --git a/spec/services/import/gitlab_projects/create_project_from_remote_file_service_spec.rb b/spec/services/import/gitlab_projects/create_project_from_remote_file_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3c461c91ff0ef7c4f22b3c210e3a6a4478b51f10 --- /dev/null +++ b/spec/services/import/gitlab_projects/create_project_from_remote_file_service_spec.rb @@ -0,0 +1,182 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Import::GitlabProjects::CreateProjectFromRemoteFileService do + let(:remote_url) { 'https://external.file.path/file' } + + let(:params) do + { + path: 'path', + namespace: user.namespace, + name: 'name', + remote_import_url: remote_url + } + end + + let_it_be(:user) { create(:user) } + + subject { described_class.new(user, params) } + + it 'creates a project and returns a successful response' do + stub_headers_for(remote_url, { + 'content-type' => 'application/gzip', + 'content-length' => '10' + }) + + response = nil + expect { response = subject.execute } + .to change(Project, :count).by(1) + + expect(response).to be_success + expect(response.http_status).to eq(:ok) + expect(response.payload).to be_instance_of(Project) + expect(response.payload.name).to eq('name') + expect(response.payload.path).to eq('path') + expect(response.payload.namespace).to eq(user.namespace) + end + + context 'when the file url is invalid' do + it 'returns an erred response with the reason of the failure' do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + + params[:remote_import_url] = 'https://localhost/file' + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message).to eq('Requests to localhost are not allowed') + end + end + + context 'validate file type' do + it 'returns erred response when the file type is not informed' do + stub_headers_for(remote_url, { 'content-length' => '10' }) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message) + .to eq("Missing 'ContentType' header") + end + + it 'returns erred response when the file type is not allowed' do + stub_headers_for(remote_url, { + 'content-type' => 'application/js', + 'content-length' => '10' + }) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message) + .to eq("Remote file content type 'application/js' not allowed. (Allowed content types: application/gzip)") + end + end + + context 'validate content type' do + it 'returns erred response when the file size is not informed' do + stub_headers_for(remote_url, { 'content-type' => 'application/gzip' }) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message) + .to eq("Missing 'ContentLength' header") + end + + it 'returns error response when the file size is a text' do + stub_headers_for(remote_url, { + 'content-type' => 'application/gzip', + 'content-length' => 'some text' + }) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message) + .to eq("Missing 'ContentLength' header") + end + + it 'returns erred response when the file is larger then allowed' do + stub_headers_for(remote_url, { + 'content-type' => 'application/gzip', + 'content-length' => 11.gigabytes.to_s + }) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message) + .to eq('Remote file larger than limit. (limit 10 GB)') + end + end + + context 'when required parameters are not provided' do + let(:params) { {} } + + it 'returns an erred response with the reason of the failure' do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message).to eq("Parameter 'path' is required") + + expect(subject.errors.full_messages).to match_array([ + "Missing 'ContentLength' header", + "Missing 'ContentType' header", + "Parameter 'namespace' is required", + "Parameter 'path' is required", + "Parameter 'remote_import_url' is required" + ]) + end + end + + context 'when the project is invalid' do + it 'returns an erred response with the reason of the failure' do + create(:project, namespace: user.namespace, path: 'path') + + stub_headers_for(remote_url, { + 'content-type' => 'application/gzip', + 'content-length' => '10' + }) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message).to eq('Path has already been taken') + end + end + + def stub_headers_for(url, headers = {}) + allow(Gitlab::HTTP) + .to receive(:head) + .with(url) + .and_return(double(headers: headers)) + end +end diff --git a/spec/services/import/gitlab_projects/create_project_from_uploaded_file_service_spec.rb b/spec/services/import/gitlab_projects/create_project_from_uploaded_file_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a0e04a9a69648277370655579310f4ea8d73055f --- /dev/null +++ b/spec/services/import/gitlab_projects/create_project_from_uploaded_file_service_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Import::GitlabProjects::CreateProjectFromUploadedFileService do + let(:file_upload) do + fixture_file_upload('spec/features/projects/import_export/test_project_export.tar.gz') + end + + let(:params) do + { + path: 'path', + namespace: user.namespace, + name: 'name', + file: file_upload + } + end + + let_it_be(:user) { create(:user) } + + subject { described_class.new(user, params) } + + it 'creates a project and returns a successful response' do + response = nil + expect { response = subject.execute } + .to change(Project, :count).by(1) + + expect(response).to be_success + expect(response.http_status).to eq(:ok) + expect(response.payload).to be_instance_of(Project) + expect(response.payload.name).to eq('name') + expect(response.payload.path).to eq('path') + expect(response.payload.namespace).to eq(user.namespace) + end + + context 'when required parameters are not provided' do + let(:params) { {} } + + it 'returns an erred response with the reason of the failure' do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message).to eq("Parameter 'path' is required") + + expect(subject.errors.full_messages).to match_array([ + "Parameter 'namespace' is required", + "Parameter 'path' is required", + "Parameter 'file' is required" + ]) + end + end + + context 'when the project is invalid' do + it 'returns an erred response with the reason of the failure' do + create(:project, namespace: user.namespace, path: 'path') + + response = nil + expect { response = subject.execute } + .not_to change(Project, :count) + + expect(response).not_to be_success + expect(response.http_status).to eq(:bad_request) + expect(response.message).to eq('Path has already been taken') + end + end +end