Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/concerns/uploads_actions.rb10
-rw-r--r--app/controllers/groups/uploads_controller.rb4
-rw-r--r--app/controllers/projects/uploads_controller.rb4
-rw-r--r--changelogs/unreleased/jprovazn-direct-upload.yml5
-rw-r--r--config/routes/group.rb1
-rw-r--r--config/routes/project.rb1
-rw-r--r--doc/administration/uploads.md3
-rw-r--r--lib/gitlab/middleware/multipart.rb16
-rw-r--r--spec/controllers/groups/uploads_controller_spec.rb8
-rw-r--r--spec/controllers/projects/uploads_controller_spec.rb8
-rw-r--r--spec/lib/gitlab/middleware/multipart_spec.rb73
-rw-r--r--spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb79
12 files changed, 170 insertions, 42 deletions
diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index 16374146ae4..434459a225a 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -45,6 +45,16 @@ module UploadsActions
send_upload(uploader, attachment: uploader.filename, disposition: disposition)
end
+ def authorize
+ set_workhorse_internal_api_content_type
+
+ authorized = uploader_class.workhorse_authorize(
+ has_length: false,
+ maximum_size: Gitlab::CurrentSettings.max_attachment_size.megabytes.to_i)
+
+ render json: authorized
+ end
+
private
# Explicitly set the format.
diff --git a/app/controllers/groups/uploads_controller.rb b/app/controllers/groups/uploads_controller.rb
index f1578f75e88..74760194a1f 100644
--- a/app/controllers/groups/uploads_controller.rb
+++ b/app/controllers/groups/uploads_controller.rb
@@ -1,9 +1,11 @@
class Groups::UploadsController < Groups::ApplicationController
include UploadsActions
+ include WorkhorseRequest
skip_before_action :group, if: -> { action_name == 'show' && image_or_video? }
- before_action :authorize_upload_file!, only: [:create]
+ before_action :authorize_upload_file!, only: [:create, :authorize]
+ before_action :verify_workhorse_api!, only: [:authorize]
private
diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb
index f5cf089ad98..7a85046164c 100644
--- a/app/controllers/projects/uploads_controller.rb
+++ b/app/controllers/projects/uploads_controller.rb
@@ -1,11 +1,13 @@
class Projects::UploadsController < Projects::ApplicationController
include UploadsActions
+ include WorkhorseRequest
# These will kick you out if you don't have access.
skip_before_action :project, :repository,
if: -> { action_name == 'show' && image_or_video? }
- before_action :authorize_upload_file!, only: [:create]
+ before_action :authorize_upload_file!, only: [:create, :authorize]
+ before_action :verify_workhorse_api!, only: [:authorize]
private
diff --git a/changelogs/unreleased/jprovazn-direct-upload.yml b/changelogs/unreleased/jprovazn-direct-upload.yml
new file mode 100644
index 00000000000..57f6d1e07c3
--- /dev/null
+++ b/changelogs/unreleased/jprovazn-direct-upload.yml
@@ -0,0 +1,5 @@
+---
+title: Support direct_upload for generic uploads
+merge_request:
+author:
+type: added
diff --git a/config/routes/group.rb b/config/routes/group.rb
index b09eb3c1b5b..25fbb38ba87 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -55,6 +55,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
resources :uploads, only: [:create] do
collection do
get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} }
+ post :authorize
end
end
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 6dfbd7ecd1f..b689b9800e6 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -406,6 +406,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
resources :uploads, only: [:create] do
collection do
get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} }
+ post :authorize
end
end
diff --git a/doc/administration/uploads.md b/doc/administration/uploads.md
index 6688181c5a8..85eca403253 100644
--- a/doc/administration/uploads.md
+++ b/doc/administration/uploads.md
@@ -52,6 +52,7 @@ _The uploads are stored by default in
>**Notes:**
- [Introduced][ee-3867] in [GitLab Enterprise Edition Premium][eep] 10.5.
+- Since version 11.1, we support direct_upload to S3.
If you don't want to use the local disk where GitLab is installed to store the
uploads, you can use an object storage provider like AWS S3 instead.
@@ -65,7 +66,7 @@ For source installations the following settings are nested under `uploads:` and
|---------|-------------|---------|
| `enabled` | Enable/disable object storage | `false` |
| `remote_directory` | The bucket name where Uploads will be stored| |
-| `direct_upload` | Set to true to enable direct upload of Uploads without the need of local shared storage. Option may be removed once we decide to support only single storage for all files. This is beta option as it uses inefficient way of uploading data (via Unicorn). The accelerated uploads gonna be implemented in future releases | `false` |
+| `direct_upload` | Set to true to enable direct upload of Uploads without the need of local shared storage. Option may be removed once we decide to support only single storage for all files. | `false` |
| `background_upload` | Set to false to disable automatic upload. Option may be removed once upload is direct to S3 | `true` |
| `proxy_download` | Set to true to enable proxying all files served. Option allows to reduce egress traffic as this allows clients to download directly from remote storage instead of proxying all data | `false` |
| `connection` | Various connection options described below | |
diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb
index a5f5d719cc1..9753be6d5c3 100644
--- a/lib/gitlab/middleware/multipart.rb
+++ b/lib/gitlab/middleware/multipart.rb
@@ -42,10 +42,10 @@ module Gitlab
key, value = parsed_field.first
if value.nil?
- value = open_file(tmp_path, @request.params["#{key}.name"])
+ value = open_file(@request.params, key)
@open_files << value
else
- value = decorate_params_value(value, @request.params[key], tmp_path)
+ value = decorate_params_value(value, @request.params[key])
end
@request.update_param(key, value)
@@ -57,7 +57,7 @@ module Gitlab
end
# This function calls itself recursively
- def decorate_params_value(path_hash, value_hash, tmp_path)
+ def decorate_params_value(path_hash, value_hash)
unless path_hash.is_a?(Hash) && path_hash.count == 1
raise "invalid path: #{path_hash.inspect}"
end
@@ -70,19 +70,21 @@ module Gitlab
case path_value
when nil
- value_hash[path_key] = open_file(tmp_path, value_hash.dig(path_key, '.name'))
+ value_hash[path_key] = open_file(value_hash.dig(path_key), '')
@open_files << value_hash[path_key]
value_hash
when Hash
- decorate_params_value(path_value, value_hash[path_key], tmp_path)
+ decorate_params_value(path_value, value_hash[path_key])
value_hash
else
raise "unexpected path value: #{path_value.inspect}"
end
end
- def open_file(path, name)
- ::UploadedFile.new(path, filename: name || File.basename(path), content_type: 'application/octet-stream')
+ def open_file(params, key)
+ ::UploadedFile.from_params(
+ params, key,
+ Gitlab.config.uploads.storage_path)
end
end
diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb
index 6a1869d1a48..5a7281ed704 100644
--- a/spec/controllers/groups/uploads_controller_spec.rb
+++ b/spec/controllers/groups/uploads_controller_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Groups::UploadsController do
+ include WorkhorseHelpers
+
let(:model) { create(:group, :public) }
let(:params) do
{ group_id: model }
@@ -9,4 +11,10 @@ describe Groups::UploadsController do
it_behaves_like 'handle uploads' do
let(:uploader_class) { NamespaceFileUploader }
end
+
+ def post_authorize(verified: true)
+ request.headers.merge!(workhorse_internal_api_request_header) if verified
+
+ post :authorize, group_id: model.full_path, format: :json
+ end
end
diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index eca9baed9c9..325ee53aafb 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Projects::UploadsController do
+ include WorkhorseHelpers
+
let(:model) { create(:project, :public) }
let(:params) do
{ namespace_id: model.namespace.to_param, project_id: model }
@@ -15,4 +17,10 @@ describe Projects::UploadsController do
expect(response).to redirect_to(new_user_session_path)
end
end
+
+ def post_authorize(verified: true)
+ request.headers.merge!(workhorse_internal_api_request_header) if verified
+
+ post :authorize, namespace_id: model.namespace, project_id: model.path, format: :json
+ end
end
diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb
index a2ba91dae80..b4837a1689a 100644
--- a/spec/lib/gitlab/middleware/multipart_spec.rb
+++ b/spec/lib/gitlab/middleware/multipart_spec.rb
@@ -7,18 +7,47 @@ describe Gitlab::Middleware::Multipart do
let(:middleware) { described_class.new(app) }
let(:original_filename) { 'filename' }
- it 'opens top-level files' do
- Tempfile.open('top-level') do |tempfile|
- env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+ shared_examples_for 'multipart upload files' do
+ it 'opens top-level files' do
+ Tempfile.open('top-level') do |tempfile|
+ env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path, 'file.remote_id' => remote_id }, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+ expect_uploaded_file(tempfile, %w(file))
+
+ middleware.call(env)
+ end
+ end
+
+ it 'opens files one level deep' do
+ Tempfile.open('one-level') do |tempfile|
+ in_params = { 'user' => { 'avatar' => { '.name' => original_filename, '.path' => tempfile.path, '.remote_id' => remote_id } } }
+ env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+
+ expect_uploaded_file(tempfile, %w(user avatar))
+
+ middleware.call(env)
+ end
+ end
+
+ it 'opens files two levels deep' do
+ Tempfile.open('two-levels') do |tempfile|
+ in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename, '.path' => tempfile.path, '.remote_id' => remote_id } } } }
+ env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+
+ expect_uploaded_file(tempfile, %w(project milestone themesong))
+
+ middleware.call(env)
+ end
+ end
+
+ def expect_uploaded_file(tempfile, path, remote: false)
expect(app).to receive(:call) do |env|
- file = Rack::Request.new(env).params['file']
+ file = Rack::Request.new(env).params.dig(*path)
expect(file).to be_a(::UploadedFile)
expect(file.path).to eq(tempfile.path)
expect(file.original_filename).to eq(original_filename)
+ expect(file.remote_id).to eq(remote_id)
end
-
- middleware.call(env)
end
end
@@ -34,36 +63,16 @@ describe Gitlab::Middleware::Multipart do
expect { middleware.call(env) }.to raise_error(JWT::InvalidIssuerError)
end
- it 'opens files one level deep' do
- Tempfile.open('one-level') do |tempfile|
- in_params = { 'user' => { 'avatar' => { '.name' => original_filename } } }
- env = post_env({ 'user[avatar]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+ context 'with remote file' do
+ let(:remote_id) { 'someid' }
- expect(app).to receive(:call) do |env|
- file = Rack::Request.new(env).params['user']['avatar']
- expect(file).to be_a(::UploadedFile)
- expect(file.path).to eq(tempfile.path)
- expect(file.original_filename).to eq(original_filename)
- end
-
- middleware.call(env)
- end
+ it_behaves_like 'multipart upload files'
end
- it 'opens files two levels deep' do
- Tempfile.open('two-levels') do |tempfile|
- in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename } } } }
- env = post_env({ 'project[milestone][themesong]' => tempfile.path }, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse')
+ context 'with local file' do
+ let(:remote_id) { nil }
- expect(app).to receive(:call) do |env|
- file = Rack::Request.new(env).params['project']['milestone']['themesong']
- expect(file).to be_a(::UploadedFile)
- expect(file.path).to eq(tempfile.path)
- expect(file.original_filename).to eq(original_filename)
- end
-
- middleware.call(env)
- end
+ it_behaves_like 'multipart upload files'
end
def post_env(rewritten_fields, params, secret, issuer)
diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
index bbbad86dcd5..7088fb1e5fb 100644
--- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
@@ -260,4 +260,83 @@ shared_examples 'handle uploads' do
end
end
end
+
+ describe "POST #authorize" do
+ context 'when a user is not authorized to upload a file' do
+ it 'returns 404 status' do
+ post_authorize
+
+ expect(response.status).to eq(404)
+ end
+ end
+
+ context 'when a user can upload a file' do
+ before do
+ sign_in(user)
+ model.add_developer(user)
+ end
+
+ context 'and the request bypassed workhorse' do
+ it 'raises an exception' do
+ expect { post_authorize(verified: false) }.to raise_error JWT::DecodeError
+ end
+ end
+
+ context 'and request is sent by gitlab-workhorse to authorize the request' do
+ shared_examples 'a valid response' do
+ before do
+ post_authorize
+ end
+
+ it 'responds with status 200' do
+ expect(response).to have_gitlab_http_status(200)
+ end
+
+ it 'uses the gitlab-workhorse content type' do
+ expect(response.headers["Content-Type"]).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+ end
+ end
+
+ shared_examples 'a local file' do
+ it_behaves_like 'a valid response' do
+ it 'responds with status 200, location of uploads store and object details' do
+ expect(json_response['TempPath']).to eq(uploader_class.workhorse_local_upload_path)
+ expect(json_response['RemoteObject']).to be_nil
+ end
+ end
+ end
+
+ context 'when using local storage' do
+ it_behaves_like 'a local file'
+ end
+
+ context 'when using remote storage' do
+ context 'when direct upload is enabled' do
+ before do
+ stub_uploads_object_storage(uploader_class, direct_upload: true)
+ end
+
+ it_behaves_like 'a valid response' do
+ it 'responds with status 200, location of uploads remote store and object details' do
+ expect(json_response['TempPath']).to eq(uploader_class.workhorse_local_upload_path)
+ expect(json_response['RemoteObject']).to have_key('ID')
+ expect(json_response['RemoteObject']).to have_key('GetURL')
+ expect(json_response['RemoteObject']).to have_key('StoreURL')
+ expect(json_response['RemoteObject']).to have_key('DeleteURL')
+ expect(json_response['RemoteObject']).to have_key('MultipartUpload')
+ end
+ end
+ end
+
+ context 'when direct upload is disabled' do
+ before do
+ stub_uploads_object_storage(uploader_class, direct_upload: false)
+ end
+
+ it_behaves_like 'a local file'
+ end
+ end
+ end
+ end
+ end
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-08-01 09:38:26 +0300