Commit e32069ef authored by Alessio Caiazza's avatar Alessio Caiazza

Process workhorse accelerated wiki uploads

Wiki attachments can be workhorse accelerated.
This commit is backward compatible with older workhorse
parent 222d9e62
---
title: Preprocess wiki attachments with GitLab-Workhorse
merge_request: 32663
author:
type: performance
# frozen_string_literal: true
module API
module Validations
module Types
class WorkhorseFile < Virtus::Attribute
def coerce(input)
# Processing of multipart file objects
# is already taken care of by Gitlab::Middleware::Multipart.
# Nothing to do here.
input
end
def value_coerced?(value)
value.is_a?(::UploadedFile)
end
end
end
end
end
...@@ -4,11 +4,21 @@ module API ...@@ -4,11 +4,21 @@ module API
class Wikis < Grape::API class Wikis < Grape::API
helpers do helpers do
def commit_params(attrs) def commit_params(attrs)
{ # In order to avoid service disruption this can work with an old workhorse without the acceleration
file_name: attrs[:file][:filename], # the first branch of this if must be removed when we drop support for non accelerated uploads
file_content: attrs[:file][:tempfile].read, if attrs[:file].is_a?(Hash)
branch_name: attrs[:branch] {
} file_name: attrs[:file][:filename],
file_content: attrs[:file][:tempfile].read,
branch_name: attrs[:branch]
}
else
{
file_name: attrs[:file].original_filename,
file_content: attrs[:file].read,
branch_name: attrs[:branch]
}
end
end end
params :common_wiki_page_params do params :common_wiki_page_params do
...@@ -106,7 +116,7 @@ module API ...@@ -106,7 +116,7 @@ module API
success Entities::WikiAttachment success Entities::WikiAttachment
end end
params do params do
requires :file, type: ::API::Validations::Types::SafeFile, desc: 'The attachment file to be uploaded' requires :file, types: [::API::Validations::Types::SafeFile, ::API::Validations::Types::WorkhorseFile], desc: 'The attachment file to be uploaded'
optional :branch, type: String, desc: 'The name of the branch' optional :branch, type: String, desc: 'The name of the branch'
end end
post ":id/wikis/attachments" do post ":id/wikis/attachments" do
......
...@@ -32,7 +32,7 @@ module Gitlab ...@@ -32,7 +32,7 @@ module Gitlab
class Handler class Handler
def initialize(env, message) def initialize(env, message)
@request = ActionDispatch::Request.new(env) @request = Rack::Request.new(env)
@rewritten_fields = message['rewritten_fields'] @rewritten_fields = message['rewritten_fields']
@open_files = [] @open_files = []
end end
...@@ -50,7 +50,7 @@ module Gitlab ...@@ -50,7 +50,7 @@ module Gitlab
value = decorate_params_value(value, @request.params[key]) value = decorate_params_value(value, @request.params[key])
end end
@request.update_param(key, value) update_param(key, value)
end end
yield yield
...@@ -92,6 +92,20 @@ module Gitlab ...@@ -92,6 +92,20 @@ module Gitlab
::UploadedFile.from_params(params, key, allowed_paths) ::UploadedFile.from_params(params, key, allowed_paths)
end end
# update_params ensures that both rails controllers and rack middleware can find
# workhorse accelerate files in the request
def update_param(key, value)
# we make sure we have key in POST otherwise update_params will add it in GET
@request.POST[key] ||= value
# this will force Rack::Request to properly update env keys
@request.update_param(key, value)
# ActionDispatch::Request is based on Rack::Request but it caches params
# inside other env keys, here we ensure everything is updated correctly
ActionDispatch::Request.new(@request.env).update_param(key, value)
end
end end
def initialize(app) def initialize(app)
......
...@@ -11,6 +11,8 @@ require 'spec_helper' ...@@ -11,6 +11,8 @@ require 'spec_helper'
# because they are 3 edge cases of using wiki pages. # because they are 3 edge cases of using wiki pages.
describe API::Wikis do describe API::Wikis do
include WorkhorseHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:group) { create(:group).tap { |g| g.add_owner(user) } } let(:group) { create(:group).tap { |g| g.add_owner(user) } }
let(:project_wiki) { create(:project_wiki, project: project, user: user) } let(:project_wiki) { create(:project_wiki, project: project, user: user) }
...@@ -155,7 +157,7 @@ describe API::Wikis do ...@@ -155,7 +157,7 @@ describe API::Wikis do
it 'pushes attachment to the wiki repository' do it 'pushes attachment to the wiki repository' do
allow(SecureRandom).to receive(:hex).and_return('fixed_hex') allow(SecureRandom).to receive(:hex).and_return('fixed_hex')
post(api(url, user), params: payload) workhorse_post_with_file(api(url, user), file_key: :file, params: payload)
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response).to eq result_hash.deep_stringify_keys expect(json_response).to eq result_hash.deep_stringify_keys
...@@ -180,6 +182,15 @@ describe API::Wikis do ...@@ -180,6 +182,15 @@ describe API::Wikis do
expect(json_response.size).to eq(1) expect(json_response.size).to eq(1)
expect(json_response['error']).to eq('file is invalid') expect(json_response['error']).to eq('file is invalid')
end end
it 'is backward compatible with regular multipart uploads' do
allow(SecureRandom).to receive(:hex).and_return('fixed_hex')
post(api(url, user), params: payload)
expect(response).to have_gitlab_http_status(201)
expect(json_response).to eq result_hash.deep_stringify_keys
end
end end
describe 'GET /projects/:id/wikis' do describe 'GET /projects/:id/wikis' do
......
...@@ -17,7 +17,36 @@ module WorkhorseHelpers ...@@ -17,7 +17,36 @@ module WorkhorseHelpers
end end
def workhorse_internal_api_request_header def workhorse_internal_api_request_header
jwt_token = JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256')
{ 'HTTP_' + Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER.upcase.tr('-', '_') => jwt_token } { 'HTTP_' + Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER.upcase.tr('-', '_') => jwt_token }
end end
# workhorse_post_with_file will transform file_key inside params as if it was disk accelerated by workhorse
def workhorse_post_with_file(url, file_key:, params:)
workhorse_params = params.dup
file = workhorse_params.delete(file_key)
workhorse_params.merge!(workhorse_disk_accelerated_file_params(file_key, file))
post(url,
params: workhorse_params,
headers: workhorse_rewritten_fields_header('file' => file.path)
)
end
private
def jwt_token(data = {})
JWT.encode({ 'iss' => 'gitlab-workhorse' }.merge(data), Gitlab::Workhorse.secret, 'HS256')
end
def workhorse_rewritten_fields_header(fields)
{ Gitlab::Middleware::Multipart::RACK_ENV_KEY => jwt_token('rewritten_fields' => fields) }
end
def workhorse_disk_accelerated_file_params(key, file)
{
"#{key}.name" => file.original_filename,
"#{key}.path" => file.path
}
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