Commit fb12eaef authored by Micaël Bergeron's avatar Micaël Bergeron

enforce the use of hashed storage for remote file uploads

parent fb0b98a4
...@@ -51,17 +51,15 @@ class FileUploader < GitlabUploader ...@@ -51,17 +51,15 @@ class FileUploader < GitlabUploader
# #
# Returns a String without a trailing slash # Returns a String without a trailing slash
def self.model_path_segment(model) def self.model_path_segment(model)
if model.hashed_storage?(:attachments) case model
model.disk_path when Storage::HashedProject then model.disk_path
when Project then
model.hashed_storage?(:attachments) ? model.disk_path : model.full_path
else else
model.full_path model.full_path
end end
end end
def self.upload_path(secret, identifier)
File.join(secret, identifier)
end
def self.generate_secret def self.generate_secret
SecureRandom.hex SecureRandom.hex
end end
...@@ -75,8 +73,12 @@ class FileUploader < GitlabUploader ...@@ -75,8 +73,12 @@ class FileUploader < GitlabUploader
apply_context!(uploader_context) apply_context!(uploader_context)
end end
# enforce the usage of Hashed storage when storing to
# remote store as the FileMover doesn't support OS
def base_dir def base_dir
self.class.base_dir(@model) model = file_storage? ? @model : Storage::HashedProject.new(@model)
self.class.base_dir(model)
end end
# we don't need to know the actual path, an uploader instance should be # we don't need to know the actual path, an uploader instance should be
...@@ -86,15 +88,18 @@ class FileUploader < GitlabUploader ...@@ -86,15 +88,18 @@ class FileUploader < GitlabUploader
end end
def upload_path def upload_path
self.class.upload_path(dynamic_segment, identifier) if file_storage? # Legacy
end File.join(dynamic_segment, identifier)
else
def model_path_segment File.join(store_dir, identifier)
self.class.model_path_segment(@model) end
end end
def store_dir def store_dirs
File.join(base_dir, dynamic_segment) {
Store::LOCAL => local_store_dir = File.join(base_dir, dynamic_segment),
Store::REMOTE => local_store_dir
}
end end
def markdown_link def markdown_link
...@@ -134,6 +139,9 @@ class FileUploader < GitlabUploader ...@@ -134,6 +139,9 @@ class FileUploader < GitlabUploader
private private
def hashed_model
end
def apply_context!(uploader_context) def apply_context!(uploader_context)
@secret, @identifier = uploader_context.values_at(:secret, :identifier) @secret, @identifier = uploader_context.values_at(:secret, :identifier)
......
...@@ -11,34 +11,30 @@ describe FileUploader do ...@@ -11,34 +11,30 @@ describe FileUploader do
shared_examples 'builds correct legacy storage paths' do shared_examples 'builds correct legacy storage paths' do
include_examples 'builds correct paths', include_examples 'builds correct paths',
store_dir: %r{awesome/project/\h+}, store_dir: %r{awesome/project/\h+},
upload_path: %r{\h+/<filename>},
absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg} absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg}
end end
shared_examples 'uses hashed storage' do context 'legacy storage' do
context 'when rolled out attachments' do it_behaves_like 'builds correct legacy storage paths'
let(:project) { build_stubbed(:project, namespace: group, name: 'project') }
before do context 'uses hashed storage' do
allow(project).to receive(:disk_path).and_return('ca/fe/fe/ed') context 'when rolled out attachments' do
end let(:project) { build_stubbed(:project, namespace: group, name: 'project') }
it_behaves_like 'builds correct paths', include_examples 'builds correct paths',
store_dir: %r{ca/fe/fe/ed/\h+}, store_dir: %r{@hashed/\h{2}/\h{2}/\h+},
absolute_path: %r{#{described_class.root}/ca/fe/fe/ed/secret/foo.jpg} upload_path: %r{\h+/<filename>}
end end
context 'when only repositories are rolled out' do context 'when only repositories are rolled out' do
let(:project) { build_stubbed(:project, namespace: group, name: 'project', storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) } let(:project) { build_stubbed(:project, namespace: group, name: 'project', storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) }
it_behaves_like 'builds correct legacy storage paths' it_behaves_like 'builds correct legacy storage paths'
end
end end
end end
context 'legacy storage' do
it_behaves_like 'builds correct legacy storage paths'
include_examples 'uses hashed storage'
end
context 'object store is remote' do context 'object store is remote' do
before do before do
stub_uploads_object_storage stub_uploads_object_storage
...@@ -46,8 +42,10 @@ describe FileUploader do ...@@ -46,8 +42,10 @@ describe FileUploader do
include_context 'with storage', described_class::Store::REMOTE include_context 'with storage', described_class::Store::REMOTE
it_behaves_like 'builds correct legacy storage paths' # always use hashed storage path for remote uploads
include_examples 'uses hashed storage' it_behaves_like 'builds correct paths',
store_dir: %r{@hashed/\h{2}/\h{2}/\h+},
upload_path: %r{@hashed/\h{2}/\h{2}/\h+/\h+/<filename>}
end end
describe 'initialize' do describe 'initialize' do
......
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