Add a feature flag for subdirectory archives

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

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

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