Commit 07f517d4 authored by James Ramsay's avatar James Ramsay

Add new repository archive route

Repository archives are always named `<project>-<ref>-<sha>` even if
the ref is a commit. A consequence of always including the sha even
for tags is that packaging a release is more difficult because both
the ref and sha must be known by the packager.

- add `<project>/-/archive/<ref>/<filename>.<format>` route using the
`-` separator to prevent namespace collisions. If the filename is
`<project>-<ref>` or the ref is a sha, the sha will be omitted,
otherwise the default filename will be used.
- deprecate previous archive route `repository/<ref>/archive`
parent 0b1b9c40
class Projects::RepositoriesController < Projects::ApplicationController class Projects::RepositoriesController < Projects::ApplicationController
include ExtractsPath
# Authorize # Authorize
before_action :require_non_empty_project, except: :create before_action :require_non_empty_project, except: :create
before_action :assign_archive_vars, only: :archive
before_action :authorize_download_code! before_action :authorize_download_code!
before_action :authorize_admin_project!, only: :create before_action :authorize_admin_project!, only: :create
...@@ -11,9 +14,21 @@ class Projects::RepositoriesController < Projects::ApplicationController ...@@ -11,9 +14,21 @@ class Projects::RepositoriesController < Projects::ApplicationController
end end
def archive def archive
send_git_archive @repository, ref: params[:ref], format: params[:format] append_sha = params[:append_sha]
shortname = "#{@project.path}-#{@ref.tr('/', '-')}"
append_sha = false if @filename == shortname
send_git_archive @repository, ref: @ref, format: params[:format], append_sha: append_sha
rescue => ex rescue => ex
logger.error("#{self.class.name}: #{ex}") logger.error("#{self.class.name}: #{ex}")
return git_not_found! return git_not_found!
end end
def assign_archive_vars
@id = params[:id]
@ref, @filename = extract_ref(@id)
rescue InvalidPathError
render_404
end
end end
- pipeline = local_assigns.fetch(:pipeline) { project.latest_successful_pipeline_for(ref) } - pipeline = local_assigns.fetch(:pipeline) { project.latest_successful_pipeline_for(ref) }
- if !project.empty_repo? && can?(current_user, :download_code, project) - if !project.empty_repo? && can?(current_user, :download_code, project)
- archive_prefix = "#{project.path}-#{ref.tr('/', '-')}"
.project-action-button.dropdown.inline> .project-action-button.dropdown.inline>
%button.btn.has-tooltip{ title: s_('DownloadSource|Download'), 'data-toggle' => 'dropdown', 'aria-label' => s_('DownloadSource|Download') } %button.btn.has-tooltip{ title: s_('DownloadSource|Download'), 'data-toggle' => 'dropdown', 'aria-label' => s_('DownloadSource|Download') }
= sprite_icon('download') = sprite_icon('download')
...@@ -10,16 +11,16 @@ ...@@ -10,16 +11,16 @@
%li.dropdown-header %li.dropdown-header
#{ _('Source code') } #{ _('Source code') }
%li %li
= link_to archive_project_repository_path(project, ref: ref, format: 'zip'), rel: 'nofollow', download: '' do = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'zip'), rel: 'nofollow', download: '' do
%span= _('Download zip') %span= _('Download zip')
%li %li
= link_to archive_project_repository_path(project, ref: ref, format: 'tar.gz'), rel: 'nofollow', download: '' do = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.gz'), rel: 'nofollow', download: '' do
%span= _('Download tar.gz') %span= _('Download tar.gz')
%li %li
= link_to archive_project_repository_path(project, ref: ref, format: 'tar.bz2'), rel: 'nofollow', download: '' do = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.bz2'), rel: 'nofollow', download: '' do
%span= _('Download tar.bz2') %span= _('Download tar.bz2')
%li %li
= link_to archive_project_repository_path(project, ref: ref, format: 'tar'), rel: 'nofollow', download: '' do = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar'), rel: 'nofollow', download: '' do
%span= _('Download tar') %span= _('Download tar')
- if pipeline && pipeline.latest_builds_with_artifacts.any? - if pipeline && pipeline.latest_builds_with_artifacts.any?
......
---
title: Add alternate archive route for simplified packaging
merge_request: 17225
author:
type: added
...@@ -249,6 +249,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -249,6 +249,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
scope '-' do scope '-' do
get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive'
resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do
collection do collection do
post :cancel_all post :cancel_all
......
...@@ -2,10 +2,11 @@ ...@@ -2,10 +2,11 @@
resource :repository, only: [:create] do resource :repository, only: [:create] do
member do member do
get ':ref/archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex, ref: /.+/ }, action: 'archive', as: 'archive'
# deprecated since GitLab 9.5 # deprecated since GitLab 9.5
get 'archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex }, as: 'archive_alternative' get 'archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex }, as: 'archive_alternative', defaults: { append_sha: true }
# deprecated since GitLab 10.7
get ':id/archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+/ }, action: 'archive', as: 'archive_deprecated', defaults: { append_sha: true }
end end
end end
......
...@@ -406,7 +406,7 @@ module Gitlab ...@@ -406,7 +406,7 @@ module Gitlab
prefix_segments.join('-') prefix_segments.join('-')
end end
def archive_metadata(ref, storage_path, format = "tar.gz", append_sha: true) def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:)
ref ||= root_ref ref ||= root_ref
commit = Gitlab::Git::Commit.find(self, ref) commit = Gitlab::Git::Commit.find(self, ref)
return {} if commit.nil? return {} if commit.nil?
......
...@@ -63,10 +63,10 @@ module Gitlab ...@@ -63,10 +63,10 @@ module Gitlab
] ]
end end
def send_git_archive(repository, ref:, format:, append_sha: true) def send_git_archive(repository, ref:, format:, append_sha:)
format ||= 'tar.gz' format ||= 'tar.gz'
format.downcase! format.downcase!
params = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format, append_sha) params = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format, append_sha: append_sha)
raise "Repository or ref not found" if params.empty? raise "Repository or ref not found" if params.empty?
if Gitlab::GitalyClient.feature_enabled?(:workhorse_archive, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) if Gitlab::GitalyClient.feature_enabled?(:workhorse_archive, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT)
......
...@@ -6,7 +6,7 @@ describe Projects::RepositoriesController do ...@@ -6,7 +6,7 @@ describe Projects::RepositoriesController do
describe "GET archive" do describe "GET archive" do
context 'as a guest' do context 'as a guest' do
it 'responds with redirect in correct format' do it 'responds with redirect in correct format' do
get :archive, namespace_id: project.namespace, project_id: project, format: "zip", ref: 'master' get :archive, namespace_id: project.namespace, project_id: project, id: "master", format: "zip"
expect(response.header["Content-Type"]).to start_with('text/html') expect(response.header["Content-Type"]).to start_with('text/html')
expect(response).to be_redirect expect(response).to be_redirect
...@@ -22,18 +22,25 @@ describe Projects::RepositoriesController do ...@@ -22,18 +22,25 @@ describe Projects::RepositoriesController do
end end
it "uses Gitlab::Workhorse" do it "uses Gitlab::Workhorse" do
get :archive, namespace_id: project.namespace, project_id: project, ref: "master", format: "zip" get :archive, namespace_id: project.namespace, project_id: project, id: "master", format: "zip"
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:")
end end
it 'responds with redirect to the short name archive if fully qualified' do
get :archive, namespace_id: project.namespace, project_id: project, id: "master/#{project.path}-master", format: "zip"
expect(assigns(:ref)).to eq("master")
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:")
end
context "when the service raises an error" do context "when the service raises an error" do
before do before do
allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed") allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed")
end end
it "renders Not Found" do it "renders Not Found" do
get :archive, namespace_id: project.namespace, project_id: project, ref: "master", format: "zip" get :archive, namespace_id: project.namespace, project_id: project, id: "master", format: "zip"
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
......
...@@ -247,13 +247,13 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -247,13 +247,13 @@ describe Gitlab::Git::Repository, seed_helper: true do
end end
it 'returns parameterised string for a ref containing slashes' do it 'returns parameterised string for a ref containing slashes' do
prefix = repository.archive_prefix('test/branch', 'SHA', append_sha: true) prefix = repository.archive_prefix('test/branch', 'SHA', append_sha: nil)
expect(prefix).to eq("#{project_name}-test-branch-SHA") expect(prefix).to eq("#{project_name}-test-branch-SHA")
end end
it 'returns correct string for a ref containing dots' do it 'returns correct string for a ref containing dots' do
prefix = repository.archive_prefix('test.branch', 'SHA', append_sha: true) prefix = repository.archive_prefix('test.branch', 'SHA', append_sha: nil)
expect(prefix).to eq("#{project_name}-test.branch-SHA") expect(prefix).to eq("#{project_name}-test.branch-SHA")
end end
...@@ -266,25 +266,25 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -266,25 +266,25 @@ describe Gitlab::Git::Repository, seed_helper: true do
end end
describe '#archive' do describe '#archive' do
let(:metadata) { repository.archive_metadata('master', '/tmp') } let(:metadata) { repository.archive_metadata('master', '/tmp', append_sha: true) }
it_should_behave_like 'archive check', '.tar.gz' it_should_behave_like 'archive check', '.tar.gz'
end end
describe '#archive_zip' do describe '#archive_zip' do
let(:metadata) { repository.archive_metadata('master', '/tmp', 'zip') } let(:metadata) { repository.archive_metadata('master', '/tmp', 'zip', append_sha: true) }
it_should_behave_like 'archive check', '.zip' it_should_behave_like 'archive check', '.zip'
end end
describe '#archive_bz2' do describe '#archive_bz2' do
let(:metadata) { repository.archive_metadata('master', '/tmp', 'tbz2') } let(:metadata) { repository.archive_metadata('master', '/tmp', 'tbz2', append_sha: true) }
it_should_behave_like 'archive check', '.tar.bz2' it_should_behave_like 'archive check', '.tar.bz2'
end end
describe '#archive_fallback' do describe '#archive_fallback' do
let(:metadata) { repository.archive_metadata('master', '/tmp', 'madeup') } let(:metadata) { repository.archive_metadata('master', '/tmp', 'madeup', append_sha: true) }
it_should_behave_like 'archive check', '.tar.gz' it_should_behave_like 'archive check', '.tar.gz'
end end
......
...@@ -16,7 +16,7 @@ describe Gitlab::Workhorse do ...@@ -16,7 +16,7 @@ describe Gitlab::Workhorse do
let(:ref) { 'master' } let(:ref) { 'master' }
let(:format) { 'zip' } let(:format) { 'zip' }
let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path } let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path }
let(:base_params) { repository.archive_metadata(ref, storage_path, format) } let(:base_params) { repository.archive_metadata(ref, storage_path, format, append_sha: nil) }
let(:gitaly_params) do let(:gitaly_params) do
base_params.merge( base_params.merge(
'GitalyServer' => { 'GitalyServer' => {
...@@ -29,7 +29,7 @@ describe Gitlab::Workhorse do ...@@ -29,7 +29,7 @@ describe Gitlab::Workhorse do
let(:cache_disabled) { false } let(:cache_disabled) { false }
subject do subject do
described_class.send_git_archive(repository, ref: ref, format: format) described_class.send_git_archive(repository, ref: ref, format: format, append_sha: nil)
end end
before do before do
......
...@@ -164,20 +164,36 @@ describe 'project routing' do ...@@ -164,20 +164,36 @@ describe 'project routing' do
# archive_project_repository GET /:project_id/repository/archive(.:format) projects/repositories#archive # archive_project_repository GET /:project_id/repository/archive(.:format) projects/repositories#archive
# edit_project_repository GET /:project_id/repository/edit(.:format) projects/repositories#edit # edit_project_repository GET /:project_id/repository/edit(.:format) projects/repositories#edit
describe Projects::RepositoriesController, 'routing' do describe Projects::RepositoriesController, 'routing' do
it 'to #archive' do
expect(get('/gitlab/gitlabhq/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', ref: 'master')
end
it 'to #archive format:zip' do it 'to #archive format:zip' do
expect(get('/gitlab/gitlabhq/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', ref: 'master') expect(get('/gitlab/gitlabhq/-/archive/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', id: 'master/archive')
end end
it 'to #archive format:tar.bz2' do it 'to #archive format:tar.bz2' do
expect(get('/gitlab/gitlabhq/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', ref: 'master') expect(get('/gitlab/gitlabhq/-/archive/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', id: 'master/archive')
end end
it 'to #archive with "/" in route' do it 'to #archive with "/" in route' do
expect(get('/gitlab/gitlabhq/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', ref: 'improve/awesome') expect(get('/gitlab/gitlabhq/-/archive/improve/awesome/gitlabhq-improve-awesome.tar.gz')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.gz', id: 'improve/awesome/gitlabhq-improve-awesome')
end
it 'to #archive_alternative' do
expect(get('/gitlab/gitlabhq/repository/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', append_sha: true)
end
it 'to #archive_deprecated' do
expect(get('/gitlab/gitlabhq/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master', append_sha: true)
end
it 'to #archive_deprecated format:zip' do
expect(get('/gitlab/gitlabhq/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', id: 'master', append_sha: true)
end
it 'to #archive_deprecated format:tar.bz2' do
expect(get('/gitlab/gitlabhq/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', id: 'master', append_sha: true)
end
it 'to #archive_deprecated with "/" in route' do
expect(get('/gitlab/gitlabhq/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'improve/awesome', append_sha: true)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment