Merge branch 'kassio/import-from-external-object-storage' into 'master'

Enable Project Import to import from remote object storage

See merge request gitlab-org/gitlab!59033

app/services/import/gitlab_projects/create_project_from_remote_file_service.rb

+# 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

app/services/import/gitlab_projects/create_project_from_uploaded_file_service.rb

+# 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

changelogs/unreleased/kassio-import-from-external-object-storage.yml

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

config/feature_flags/development/import_project_from_remote_file.yml

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

db/migrate/20210419203017_add_remote_import_url_to_import_export_upload.rb

+# 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

db/migrate/20210419203018_add_remote_text_limit_to_import_url_in_import_export_upload.rb

+# 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

db/schema_migrations/20210419203017

+1ca5f960c233be5d5a30632b8aaad9598c259154eee817f4d76e8f1bb3e95edb
\ No newline at end of file

db/schema_migrations/20210419203018

+94404ed645a9c8a0ee462baff98cf2d0e50aecdb71bb1515fd3a82bf1a39dfda
\ No newline at end of file

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

lib/api/project_import.rb

@@ -14,6 +14,21 @@ module API
       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 @@ module API
 
         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 @@ module API
       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

lib/gitlab/import_export/file_importer.rb

@@ -67,7 +67,17 @@ module Gitlab
 
         @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

spec/requests/api/project_import_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe API::ProjectImport do
   include WorkhorseHelpers
+  include AfterNextHelpers
 
   include_context 'workhorse headers'
 
@@ -31,6 +32,12 @@ RSpec.describe API::ProjectImport do
       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 @@ RSpec.describe API::ProjectImport do
     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)

spec/services/import/gitlab_projects/create_project_from_remote_file_service_spec.rb

+# 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

spec/services/import/gitlab_projects/create_project_from_uploaded_file_service_spec.rb

+# 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