Skip to content
Snippets Groups Projects
Verified Commit 7c4a069b authored by Moaz Khalifa's avatar Moaz Khalifa Committed by GitLab
Browse files

Avoid copy operation in object store during Generic Packages upload

parent 4f7c1f2d
No related branches found
No related tags found
1 merge request!147454Avoid copy operation in object store during Generic Packages upload
---
name: skip_copy_operation_in_generic_packages_upload
feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/429060
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147454
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/450992
milestone: '16.11'
group: group::package registry
type: gitlab_com_derisk
default_enabled: false
# frozen_string_literal: true
class AddFileFinalPathToPackagesPackageFiles < Gitlab::Database::Migration[2.2]
disable_ddl_transaction!
milestone '16.11'
def up
with_lock_retries do
add_column :packages_package_files, :file_final_path, :text, if_not_exists: true
end
add_text_limit :packages_package_files, :file_final_path, 1024
end
def down
with_lock_retries do
remove_column :packages_package_files, :file_final_path, if_exists: true
end
end
end
3fdf05335113f176717a30d28a2e60c55b677e4518d1445706827750b7d85c0e
\ No newline at end of file
......@@ -12847,6 +12847,8 @@ CREATE TABLE packages_package_files (
verification_state smallint DEFAULT 0 NOT NULL,
verification_started_at timestamp with time zone,
status smallint DEFAULT 0 NOT NULL,
file_final_path text,
CONSTRAINT check_0f29938b18 CHECK ((char_length(file_final_path) <= 1024)),
CONSTRAINT check_4c5e6bb0b3 CHECK ((file_store IS NOT NULL))
);
 
......@@ -47,9 +47,7 @@ class GenericPackages < ::API::Base
end
put 'authorize' do
project = authorized_user_project
authorize_workhorse!(subject: project, maximum_size: project.actual_limits.generic_packages_max_file_size)
authorize_workhorse!(**authorize_workhorse_params)
end
desc 'Upload package file' do
......@@ -146,6 +144,21 @@ class GenericPackages < ::API::Base
def max_file_size_exceeded?
authorized_user_project.actual_limits.exceeded?(:generic_packages_max_file_size, params[:file].size)
end
def authorize_workhorse_params
project = authorized_user_project
params = {
subject: project,
maximum_size: project.actual_limits.generic_packages_max_file_size
}
if ::Feature.enabled?(:skip_copy_operation_in_generic_packages_upload, project)
params[:use_final_store_path] = true
end
params
end
end
end
end
......@@ -40,7 +40,12 @@ def authorize_packages_access!(subject = user_project, required_permission = :re
end
end
def authorize_workhorse!(subject: user_project, has_length: true, maximum_size: MAX_PACKAGE_FILE_SIZE)
def authorize_workhorse!(
subject: user_project,
has_length: true,
maximum_size: MAX_PACKAGE_FILE_SIZE,
use_final_store_path: false)
authorize_upload!(subject)
Gitlab::Workhorse.verify_api_request!(headers)
......@@ -48,8 +53,9 @@ def authorize_workhorse!(subject: user_project, has_length: true, maximum_size:
status 200
content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
params = { has_length: has_length }
params = { has_length: has_length, use_final_store_path: use_final_store_path }
params[:maximum_size] = maximum_size unless has_length
params[:final_store_path_root_id] = subject.id if use_final_store_path
::Packages::PackageFileUploader.workhorse_authorize(**params)
end
......
......@@ -104,34 +104,45 @@
describe '#authorize_workhorse!' do
let_it_be(:headers) { {} }
let_it_be(:params) { { subject: project } }
subject { helper.authorize_workhorse!(subject: project) }
subject { helper.authorize_workhorse!(**params) }
shared_examples 'workhorse authorize' do
it 'authorizes workhorse' do
expect(helper).to receive(:authorize_upload!).with(project)
expect(helper).to receive(:status).with(200)
expect(helper).to receive(:content_type).with(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
expect(Gitlab::Workhorse).to receive(:verify_api_request!).with(headers)
expect(::Packages::PackageFileUploader).to receive(:workhorse_authorize).with(workhorse_authorize_params)
expect(subject).to eq nil
end
end
before do
allow(helper).to receive(:headers).and_return(headers)
end
it 'authorizes workhorse' do
expect(helper).to receive(:authorize_upload!).with(project)
expect(helper).to receive(:status).with(200)
expect(helper).to receive(:content_type).with(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
expect(Gitlab::Workhorse).to receive(:verify_api_request!).with(headers)
expect(::Packages::PackageFileUploader).to receive(:workhorse_authorize).with(has_length: true)
expect(subject).to eq nil
it_behaves_like 'workhorse authorize' do
let(:workhorse_authorize_params) { { has_length: true, use_final_store_path: false } }
end
context 'without length' do
subject { helper.authorize_workhorse!(subject: project, has_length: false) }
let(:params) { super().merge(has_length: false) }
it 'authorizes workhorse' do
expect(helper).to receive(:authorize_upload!).with(project)
expect(helper).to receive(:status).with(200)
expect(helper).to receive(:content_type).with(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
expect(Gitlab::Workhorse).to receive(:verify_api_request!).with(headers)
expect(::Packages::PackageFileUploader).to receive(:workhorse_authorize).with(has_length: false, maximum_size: ::API::Helpers::PackagesHelpers::MAX_PACKAGE_FILE_SIZE)
it_behaves_like 'workhorse authorize' do
let(:workhorse_authorize_params) do
{ has_length: false, maximum_size: ::API::Helpers::PackagesHelpers::MAX_PACKAGE_FILE_SIZE, use_final_store_path: false }
end
end
end
expect(subject).to eq nil
context 'when use_final_store_path is true' do
let(:params) { super().merge(use_final_store_path: true) }
it_behaves_like 'workhorse authorize' do
let(:workhorse_authorize_params) { { has_length: true, use_final_store_path: true, final_store_path_root_id: project.id } }
end
end
end
......
......@@ -170,6 +170,34 @@ def deploy_token_header(value)
end
end
context 'for use_final_store_path' do
before do
project.add_developer(user)
end
it 'sends use_final_store_path with true' do
expect(::Packages::PackageFileUploader).to receive(:workhorse_authorize).with(
hash_including(use_final_store_path: true, final_store_path_root_id: project.id)
).and_call_original
authorize_upload_file(workhorse_headers.merge(personal_access_token_header))
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(skip_copy_operation_in_generic_packages_upload: false)
end
it 'sends use_final_store_path with false' do
expect(::Packages::PackageFileUploader).to receive(:workhorse_authorize).with(
hash_including(use_final_store_path: false)
).and_call_original
authorize_upload_file(workhorse_headers.merge(personal_access_token_header))
end
end
end
def authorize_upload_file(request_headers, package_name: 'mypackage', file_name: 'myfile.tar.gz')
url = "/projects/#{project.id}/packages/generic/#{package_name}/0.0.1/#{file_name}/authorize"
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment