Merge branch '208674-use-wh-accel-only-for-ui-imports' into 'master'

Enable workhorse upload acceleration for Project Import uploads via UI

See merge request gitlab-org/gitlab!27332

GITLAB_WORKHORSE_VERSION

-8.25.0
+8.26.0

app/controllers/import/gitlab_projects_controller.rb

@@ -48,14 +48,7 @@ class Import::GitlabProjectsController < Import::BaseController
   private
 
   def file_is_valid?
-    # TODO: remove the condition and the private method after the WH version including
-    # https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/470
-    # is released and GITLAB_WORKHORSE_VERSION is updated accordingly.
-    if with_workhorse_upload_acceleration?
-      return false unless project_params[:file].is_a?(::UploadedFile)
-    else
-      return false unless project_params[:file] && project_params[:file].respond_to?(:read)
-    end
+    return false unless project_params[:file].is_a?(::UploadedFile)
 
     filename = project_params[:file].original_filename
 
@@ -75,8 +68,4 @@ class Import::GitlabProjectsController < Import::BaseController
   def whitelist_query_limiting
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42437')
   end
-
-  def with_workhorse_upload_acceleration?
-    request.headers[Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER].present?
-  end
 end

changelogs/unreleased/208674-use-wh-accel-only-for-ui-imports.yml

+---
+title: Enable Workhorse upload acceleration for Project Import uploads via UI
+merge_request: 27332
+author:
+type: performance

spec/controllers/import/gitlab_projects_controller_spec.rb → spec/requests/import/gitlab_projects_controller_spec.rb

@@ -3,62 +3,101 @@
 require 'spec_helper'
 
 describe Import::GitlabProjectsController do
+  include WorkhorseHelpers
+
+  let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
+  let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
+
   let_it_be(:namespace) { create(:namespace) }
   let_it_be(:user) { namespace.owner }
-  let(:file) { fixture_file_upload('spec/fixtures/project_export.tar.gz', 'text/plain') }
 
   before do
-    sign_in(user)
+    login_as(user)
   end
 
   describe 'POST create' do
-    context 'with an invalid path' do
-      it 'redirects with an error' do
-        post :create, params: { namespace_id: namespace.id, path: '/test', file: file }
+    subject { upload_archive(file_upload, workhorse_headers, params) }
+
+    let(:file) { File.join('spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') }
+    let(:file_upload) { fixture_file_upload(file) }
+    let(:params) { { namespace_id: namespace.id, path: 'test' } }
+
+    before do
+      allow(ImportExportUploader).to receive(:workhorse_upload_path).and_return('/')
+    end
 
-        expect(flash[:alert]).to start_with('Project could not be imported')
+    context 'with a valid path' do
+      it 'schedules an import and redirects to the new project path' do
+        stub_import(namespace)
+
+        subject
+
+        expect(flash[:notice]).to include('is being imported')
         expect(response).to have_gitlab_http_status(:found)
       end
+    end
+
+    context 'with an invalid path' do
+      ['/test', '../test'].each do |invalid_path|
+        it "redirects with an error when path is `#{invalid_path}`" do
+          params[:path] = invalid_path
 
-      it 'redirects with an error when a relative path is used' do
-        post :create, params: { namespace_id: namespace.id, path: '../test', file: file }
+          subject
 
-        expect(flash[:alert]).to start_with('Project could not be imported')
-        expect(response).to have_gitlab_http_status(:found)
+          expect(flash[:alert]).to start_with('Project could not be imported')
+          expect(response).to have_gitlab_http_status(:found)
+        end
       end
     end
 
-    context 'with a valid path' do
-      it 'redirects to the new project path' do
-        post :create, params: { namespace_id: namespace.id, path: 'test', file: file }
+    context 'when request exceeds the rate limit' do
+      before do
+        allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
+      end
 
-        expect(flash[:notice]).to include('is being imported')
+      it 'prevents users from importing projects' do
+        subject
+
+        expect(flash[:alert]).to eq('This endpoint has been requested too many times. Try again later.')
         expect(response).to have_gitlab_http_status(:found)
       end
     end
 
-    it_behaves_like 'project import rate limiter'
+    def upload_archive(file, headers = {}, params = {})
+      workhorse_finalize(
+        import_gitlab_project_path,
+        method: :post,
+        file_key: :file,
+        params: params.merge(file: file),
+        headers: headers,
+        send_rewritten_field: true
+      )
+    end
+
+    def stub_import(namespace)
+      expect_any_instance_of(ProjectImportState).to receive(:schedule)
+      expect(::Projects::CreateService)
+        .to receive(:new)
+        .with(user, instance_of(ActionController::Parameters))
+        .and_call_original
+    end
   end
 
   describe 'POST authorize' do
-    let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
-
-    before do
-      request.headers['GitLab-Workhorse'] = '1.0'
-      request.headers[Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER] = workhorse_token
-    end
+    subject { post authorize_import_gitlab_project_path, headers: workhorse_headers }
 
     it 'authorizes importing project with workhorse header' do
-      post :authorize, format: :json
+      subject
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+      expect(json_response['TempPath']).to eq(ImportExportUploader.workhorse_local_upload_path)
     end
 
-    it 'rejects requests that bypassed gitlab-workhorse or have invalid header' do
-      request.headers[Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER] = 'INVALID_HEADER'
+    it 'rejects requests that bypassed gitlab-workhorse' do
+      workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
 
-      expect { post :authorize, format: :json }.to raise_error(JWT::DecodeError)
+      expect { subject }.to raise_error(JWT::DecodeError)
     end
 
     context 'when using remote storage' do
@@ -68,7 +107,7 @@ describe Import::GitlabProjectsController do
         end
 
         it 'responds with status 200, location of file remote store and object details' do
-          post :authorize, format: :json
+          subject
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
@@ -87,7 +126,7 @@ describe Import::GitlabProjectsController do
         end
 
         it 'handles as a local file' do
-          post :authorize, format: :json
+          subject
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)