diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-06 18:18:36 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-06 18:18:36 +0300 |
commit | 6503150dcdf03a450b58a2d264735393e8728879 (patch) | |
tree | 8900805eefd8474454b295ced683d667f33c4f98 | |
parent | 81a855a7df8cf24195b11dbec8f296b0bae440bb (diff) |
Add latest changes from gitlab-org/gitlab@16-0-stable-ee
-rw-r--r-- | app/controllers/repositories/lfs_api_controller.rb | 56 | ||||
-rw-r--r-- | config/feature_flags/development/lfs_batch_direct_downloads.yml | 8 | ||||
-rw-r--r-- | spec/requests/lfs_http_spec.rb | 56 |
3 files changed, 117 insertions, 3 deletions
diff --git a/app/controllers/repositories/lfs_api_controller.rb b/app/controllers/repositories/lfs_api_controller.rb index d52ae723eee..32119ddf89e 100644 --- a/app/controllers/repositories/lfs_api_controller.rb +++ b/app/controllers/repositories/lfs_api_controller.rb @@ -6,6 +6,10 @@ module Repositories include Gitlab::Utils::StrongMemoize LFS_TRANSFER_CONTENT_TYPE = 'application/octet-stream' + # Downloading directly with presigned URLs via batch requests + # require longer expire time. + # The 1h should be enough to download 100 objects. + LFS_DIRECT_BATCH_EXPIRE_IN = 3600.seconds skip_before_action :lfs_check_access!, only: [:deprecated] before_action :lfs_check_batch_operation!, only: [:batch] @@ -22,7 +26,11 @@ module Repositories end if download_request? - render json: { objects: download_objects! }, content_type: LfsRequest::CONTENT_TYPE + if Feature.enabled?(:lfs_batch_direct_downloads, project) + render json: { objects: download_objects! }, content_type: LfsRequest::CONTENT_TYPE + else + render json: { objects: legacy_download_objects! }, content_type: LfsRequest::CONTENT_TYPE + end elsif upload_request? render json: { objects: upload_objects! }, content_type: LfsRequest::CONTENT_TYPE else @@ -52,11 +60,34 @@ module Repositories end def download_objects! + existing_oids = project.lfs_objects + .for_oids(objects_oids) + .index_by(&:oid) + + objects.each do |object| + if lfs_object = existing_oids[object[:oid]] + object[:actions] = download_actions(object, lfs_object) + + if Guest.can?(:download_code, project) + object[:authenticated] = true + end + else + object[:error] = { + code: 404, + message: _("Object does not exist on the server or you don't have permissions to access it") + } + end + end + + objects + end + + def legacy_download_objects! existing_oids = project.lfs_objects_oids(oids: objects_oids) objects.each do |object| if existing_oids.include?(object[:oid]) - object[:actions] = download_actions(object) + object[:actions] = proxy_download_actions(object) if Guest.can?(:download_code, project) object[:authenticated] = true @@ -85,7 +116,26 @@ module Repositories objects end - def download_actions(object) + def download_actions(object, lfs_object) + if lfs_object.file.file_storage? || lfs_object.file.class.proxy_download_enabled? + proxy_download_actions(object) + else + direct_download_actions(lfs_object) + end + end + + def direct_download_actions(lfs_object) + { + download: { + href: lfs_object.file.url( + content_type: "application/octet-stream", + expire_at: LFS_DIRECT_BATCH_EXPIRE_IN.since + ) + } + } + end + + def proxy_download_actions(object) { download: { href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}", diff --git a/config/feature_flags/development/lfs_batch_direct_downloads.yml b/config/feature_flags/development/lfs_batch_direct_downloads.yml new file mode 100644 index 00000000000..71634fc1d63 --- /dev/null +++ b/config/feature_flags/development/lfs_batch_direct_downloads.yml @@ -0,0 +1,8 @@ +--- +name: lfs_batch_direct_downloads +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122221 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/413684 +milestone: '16.1' +type: development +group: group::tenant scale +default_enabled: false diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index e5f03d16dda..b07296a0df2 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -323,6 +323,62 @@ RSpec.describe 'Git LFS API and storage', feature_category: :source_code_managem it_behaves_like 'process authorization header', renew_authorization: renew_authorization end + + context 'when downloading an LFS object that is stored on object storage' do + before do + stub_lfs_object_storage + lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) + end + + context 'when lfs.object_store.proxy_download=true' do + before do + stub_lfs_object_storage(proxy_download: true) + end + + it_behaves_like 'LFS http 200 response' + + it 'does return proxied address URL' do + expect(json_response['objects'].first).to include(sample_object) + expect(json_response['objects'].first['actions']['download']['href']).to eq(objects_url(project, sample_oid)) + end + end + + context 'when "lfs.object_store.proxy_download" is "false"' do + before do + stub_lfs_object_storage(proxy_download: false) + end + + it_behaves_like 'LFS http 200 response' + + it 'does return direct object storage URL' do + expect(json_response['objects'].first).to include(sample_object) + expect(json_response['objects'].first['actions']['download']['href']).to start_with("https://lfs-objects.s3.amazonaws.com/") + expect(json_response['objects'].first['actions']['download']['href']).to include("X-Amz-Expires=3600&") + end + + context 'when feature flag "lfs_batch_direct_downloads" is "false"' do + before do + stub_feature_flags(lfs_batch_direct_downloads: false) + end + + it_behaves_like 'LFS http 200 response' + + it 'does return proxied address URL' do + expect(json_response['objects'].first).to include(sample_object) + expect(json_response['objects'].first['actions']['download']['href']).to eq(objects_url(project, sample_oid)) + end + end + end + end + + context 'when sending objects=[]' do + let(:body) { download_body([]) } + + it_behaves_like 'LFS http expected response code and message' do + let(:response_code) { 404 } + let(:message) { 'Not found.' } + end + end end context 'when user is authenticated' do |