From 4d52131dcf1a50142d4c1df557bd9e02b2cb2485 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 11 Apr 2019 15:33:27 +0100 Subject: Add a feature flag for subdirectory archives --- app/views/projects/buttons/_download.html.haml | 2 +- lib/gitlab/workhorse.rb | 50 +++++++++++----- spec/lib/gitlab/workhorse_spec.rb | 83 ++++++++++++++++++-------- 3 files changed, 94 insertions(+), 41 deletions(-) diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml index acd63de2277..df1cedba0e4 100644 --- a/app/views/projects/buttons/_download.html.haml +++ b/app/views/projects/buttons/_download.html.haml @@ -11,7 +11,7 @@ %li.dropdown-bold-header= _('Download source code') %li.dropdown-menu-content = render 'projects/buttons/download_links', project: project, ref: ref, archive_prefix: archive_prefix, path: nil - - if directory? + - if directory? && Feature.enabled?(:git_archive_path, default_enabled: true) %li.separator %li.dropdown-bold-header= _('Download this directory') %li.dropdown-menu-content diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 2a8b1568f8b..46a7b5b982a 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -64,28 +64,33 @@ module Gitlab end def send_git_archive(repository, ref:, format:, append_sha:, path: nil) + path_enabled = Feature.enabled?(:git_archive_path, default_enabled: true) + path = nil unless path_enabled + format ||= 'tar.gz' format = format.downcase - metadata = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format, append_sha: append_sha, path: path) + + metadata = repository.archive_metadata( + ref, + Gitlab.config.gitlab.repository_downloads_path, + format, + append_sha: append_sha, + path: path + ) raise "Repository or ref not found" if metadata.empty? - params = { - 'GitalyServer' => gitaly_server_hash(repository), - 'ArchivePath' => metadata['ArchivePath'], - 'GetArchiveRequest' => encode_binary( - Gitaly::GetArchiveRequest.new( - repository: repository.gitaly_repository, - commit_id: metadata['CommitId'], - prefix: metadata['ArchivePrefix'], - format: archive_format(format), - path: path.presence || "" - ).to_proto - ) - } + params = + if path_enabled + send_git_archive_params(repository, metadata, path, archive_format(format)) + else + metadata + end - # If present DisableCache must be a Boolean. Otherwise workhorse ignores it. + # If present, DisableCache must be a Boolean. Otherwise + # workhorse ignores it. params['DisableCache'] = true if git_archive_cache_disabled? + params['GitalyServer'] = gitaly_server_hash(repository) [ SEND_DATA_HEADER, @@ -273,6 +278,21 @@ module Gitlab Gitaly::GetArchiveRequest::Format::TAR_GZ end end + + def send_git_archive_params(repository, metadata, path, format) + { + 'ArchivePath' => metadata['ArchivePath'], + 'GetArchiveRequest' => encode_binary( + Gitaly::GetArchiveRequest.new( + repository: repository.gitaly_repository, + commit_id: metadata['CommitId'], + prefix: metadata['ArchivePrefix'], + format: format, + path: path.presence || "" + ).to_proto + ) + } + end end end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 6f591ffaab0..f8332757fcd 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::Workhorse do let(:ref) { 'master' } let(:format) { 'zip' } let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path } - let(:path) { 'some/path' } + let(:path) { 'some/path' if Feature.enabled?(:git_archive_path, default_enabled: true) } let(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: nil, path: path) } let(:cache_disabled) { false } @@ -28,35 +28,68 @@ describe Gitlab::Workhorse do allow(described_class).to receive(:git_archive_cache_disabled?).and_return(cache_disabled) end - it 'sets the header correctly' do - key, command, params = decode_workhorse_header(subject) + context 'feature flag disabled' do + before do + stub_feature_flags(git_archive_path: false) + end - expect(key).to eq('Gitlab-Workhorse-Send-Data') - expect(command).to eq('git-archive') - expect(params).to eq({ - 'GitalyServer' => { - address: Gitlab::GitalyClient.address(project.repository_storage), - token: Gitlab::GitalyClient.token(project.repository_storage) - }, - 'ArchivePath' => metadata['ArchivePath'], - 'GetArchiveRequest' => Base64.encode64( - Gitaly::GetArchiveRequest.new( - repository: repository.gitaly_repository, - commit_id: metadata['CommitId'], - prefix: metadata['ArchivePrefix'], - format: Gitaly::GetArchiveRequest::Format::ZIP, - path: path - ).to_proto + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expected_params = metadata.merge( + 'GitalyRepository' => repository.gitaly_repository.to_h, + 'GitalyServer' => { + address: Gitlab::GitalyClient.address(project.repository_storage), + token: Gitlab::GitalyClient.token(project.repository_storage) + } ) - }.deep_stringify_keys) + + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to eq(expected_params.deep_stringify_keys) + end + + context 'when archive caching is disabled' do + let(:cache_disabled) { true } + + it 'tells workhorse not to use the cache' do + _, _, params = decode_workhorse_header(subject) + expect(params).to include({ 'DisableCache' => true }) + end + end end - context 'when archive caching is disabled' do - let(:cache_disabled) { true } + context 'feature flag enabled' do + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to eq({ + 'GitalyServer' => { + address: Gitlab::GitalyClient.address(project.repository_storage), + token: Gitlab::GitalyClient.token(project.repository_storage) + }, + 'ArchivePath' => metadata['ArchivePath'], + 'GetArchiveRequest' => Base64.encode64( + Gitaly::GetArchiveRequest.new( + repository: repository.gitaly_repository, + commit_id: metadata['CommitId'], + prefix: metadata['ArchivePrefix'], + format: Gitaly::GetArchiveRequest::Format::ZIP, + path: path + ).to_proto + ) + }.deep_stringify_keys) + end - it 'tells workhorse not to use the cache' do - _, _, params = decode_workhorse_header(subject) - expect(params).to include({ 'DisableCache' => true }) + context 'when archive caching is disabled' do + let(:cache_disabled) { true } + + it 'tells workhorse not to use the cache' do + _, _, params = decode_workhorse_header(subject) + expect(params).to include({ 'DisableCache' => true }) + end end end -- cgit v1.2.1