FEATURE: Support subfolders in SiteSetting.s3_backup_bucket.

This commit is contained in:
Guo Xiang Tan 2016-08-15 16:06:29 +08:00
parent fc095acaaa
commit 0433163866
5 changed files with 153 additions and 80 deletions

View File

@ -1013,7 +1013,7 @@ backups:
shadowed_by_global: true
s3_backup_bucket:
default: ''
regex: "^[a-z0-9-]+$" # can't use '.' when using HTTPS
regex: '^[a-z0-9\-\/]+$' # can't use '.' when using HTTPS
shadowed_by_global: true
s3_disable_cleanup:
default: false

View File

@ -7,33 +7,21 @@ require_dependency "file_helper"
module FileStore
class S3Store < BaseStore
attr_reader :s3_bucket, :s3_bucket_folder_path
attr_reader :s3_bucket
TOMBSTONE_PREFIX ||= "tombstone/"
def initialize(s3_helper=nil)
@s3_bucket, @s3_bucket_folder_path = begin
raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank?
SiteSetting.s3_upload_bucket.downcase.split("/".freeze, 2)
end
tombstone_prefix =
if @s3_bucket_folder_path
File.join(@s3_bucket_folder_path, TOMBSTONE_PREFIX)
else
TOMBSTONE_PREFIX
end
@s3_helper = s3_helper || S3Helper.new(s3_bucket, tombstone_prefix)
@s3_helper = s3_helper || S3Helper.new(s3_bucket, TOMBSTONE_PREFIX)
end
def store_upload(file, upload, content_type = nil)
path = get_path_for_s3_upload(get_path_for_upload(upload))
path = get_path_for_upload(upload)
store_file(file, path, filename: upload.original_filename, content_type: content_type, cache_locally: true)
end
def store_optimized_image(file, optimized_image)
path = get_path_for_s3_upload(get_path_for_optimized_image(optimized_image))
path = get_path_for_optimized_image(optimized_image)
store_file(file, path)
end
@ -53,7 +41,7 @@ module FileStore
# add a "content type" header when provided
options[:content_type] = content_type if content_type
# if this fails, it will throw an exception
@s3_helper.upload(file, path, options)
path = @s3_helper.upload(file, path, options)
# return the upload url
"#{absolute_base_url}/#{path}"
end
@ -61,7 +49,7 @@ module FileStore
def remove_file(url, path)
return unless has_been_uploaded?(url)
# copy the removed file to tombstone
@s3_helper.remove(get_path_for_s3_upload(path), path)
@s3_helper.remove(path, true)
end
def has_been_uploaded?(url)
@ -76,13 +64,15 @@ module FileStore
end
def absolute_base_url
bucket = s3_bucket.split("/".freeze, 2).first
# cf. http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region
@absolute_base_url ||= if SiteSetting.s3_region == "us-east-1"
"//#{s3_bucket}.s3.amazonaws.com"
"//#{bucket}.s3.amazonaws.com"
elsif SiteSetting.s3_region == 'cn-north-1'
"//#{s3_bucket}.s3.cn-north-1.amazonaws.com.cn"
"//#{bucket}.s3.cn-north-1.amazonaws.com.cn"
else
"//#{s3_bucket}.s3-#{SiteSetting.s3_region}.amazonaws.com"
"//#{bucket}.s3-#{SiteSetting.s3_region}.amazonaws.com"
end
end
@ -115,9 +105,11 @@ module FileStore
UserAvatar.external_avatar_url(user_id, avatar.upload_id, avatar.width)
end
def get_path_for_s3_upload(path)
path = File.join(@s3_bucket_folder_path, path) if @s3_bucket_folder_path
path
def s3_bucket
@s3_bucket ||= begin
raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank?
SiteSetting.s3_upload_bucket.downcase
end
end
end

View File

@ -2,32 +2,41 @@ require "aws-sdk"
class S3Helper
def initialize(s3_bucket, tombstone_prefix=nil)
raise Discourse::InvalidParameters.new("s3_bucket") if s3_bucket.blank?
def initialize(s3_upload_bucket, tombstone_prefix='')
@s3_bucket, @s3_bucket_folder_path = begin
raise Discourse::InvalidParameters.new("s3_bucket") if s3_upload_bucket.blank?
s3_upload_bucket.downcase.split("/".freeze, 2)
end
@s3_bucket = s3_bucket
@tombstone_prefix = tombstone_prefix
@tombstone_prefix =
if @s3_bucket_folder_path
File.join(@s3_bucket_folder_path, tombstone_prefix)
else
tombstone_prefix
end
check_missing_site_settings
end
def upload(file, path, options={})
path = get_path_for_s3_upload(path)
obj = s3_bucket.object(path)
obj.upload_file(file, options)
path
end
def remove(s3_filename, tombstone_filename=false)
def remove(s3_filename, copy_to_tombstone=false)
bucket = s3_bucket
# copy the file in tombstone
if tombstone_filename && @tombstone_prefix.present?
if copy_to_tombstone && @tombstone_prefix.present?
bucket
.object(File.join(@tombstone_prefix, tombstone_filename))
.copy_from(copy_source: File.join(@s3_bucket, s3_filename))
.object(File.join(@tombstone_prefix, s3_filename))
.copy_from(copy_source: File.join(@s3_bucket, get_path_for_s3_upload(s3_filename)))
end
# delete the file
bucket.object(s3_filename).delete
bucket.object(get_path_for_s3_upload(s3_filename)).delete
rescue Aws::S3::Errors::NoSuchKey
end
@ -52,28 +61,32 @@ class S3Helper
private
def s3_resource
opts = { region: SiteSetting.s3_region }
def get_path_for_s3_upload(path)
path = File.join(@s3_bucket_folder_path, path) if @s3_bucket_folder_path
path
end
unless SiteSetting.s3_use_iam_profile
opts[:access_key_id] = SiteSetting.s3_access_key_id
opts[:secret_access_key] = SiteSetting.s3_secret_access_key
end
def s3_resource
opts = { region: SiteSetting.s3_region }
Aws::S3::Resource.new(opts)
unless SiteSetting.s3_use_iam_profile
opts[:access_key_id] = SiteSetting.s3_access_key_id
opts[:secret_access_key] = SiteSetting.s3_secret_access_key
end
def s3_bucket
bucket = s3_resource.bucket(@s3_bucket)
bucket.create unless bucket.exists?
bucket
end
Aws::S3::Resource.new(opts)
end
def check_missing_site_settings
unless SiteSetting.s3_use_iam_profile
raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank?
raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank?
end
end
def s3_bucket
bucket = s3_resource.bucket(@s3_bucket)
bucket.create unless bucket.exists?
bucket
end
def check_missing_site_settings
unless SiteSetting.s3_use_iam_profile
raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank?
raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank?
end
end
end

View File

@ -20,6 +20,10 @@ describe FileStore::S3Store do
end
shared_context 's3 helpers' do
let(:upload) do
Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string'))
end
let(:store) { FileStore::S3Store.new }
let(:client) { Aws::S3::Client.new(stub_responses: true) }
let(:resource) { Aws::S3::Resource.new(client: client) }
@ -36,6 +40,7 @@ describe FileStore::S3Store do
describe "#store_upload" do
it "returns an absolute schemaless url" do
store.expects(:get_depth_for).with(upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
@ -53,6 +58,7 @@ describe FileStore::S3Store do
end
it "returns an absolute schemaless url" do
store.expects(:get_depth_for).with(upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
@ -68,6 +74,7 @@ describe FileStore::S3Store do
describe "#store_optimized_image" do
it "returns an absolute schemaless url" do
store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
path = "optimized/1X/#{optimized_image.upload.sha1}_#{OptimizedImage::VERSION}_100x200.png"
@ -86,6 +93,7 @@ describe FileStore::S3Store do
end
it "returns an absolute schemaless url" do
store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
path = "discourse-uploads/optimized/1X/#{optimized_image.upload.sha1}_#{OptimizedImage::VERSION}_100x200.png"
@ -106,6 +114,7 @@ describe FileStore::S3Store do
describe "#remove_upload" do
it "removes the file from s3 with the right paths" do
store.expects(:get_depth_for).with(upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
upload.update_attributes!(url: "//s3-upload-bucket.s3-us-west-1.amazonaws.com/original/1X/#{upload.sha1}.png")
s3_object = stub
@ -124,6 +133,7 @@ describe FileStore::S3Store do
end
it "removes the file from s3 with the right paths" do
store.expects(:get_depth_for).with(upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
upload.update_attributes!(url: "//s3-upload-bucket.s3-us-west-1.amazonaws.com/discourse-uploads/original/1X/#{upload.sha1}.png")
s3_object = stub
@ -147,6 +157,7 @@ describe FileStore::S3Store do
end
it "removes the file from s3 with the right paths" do
store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
@ -164,6 +175,7 @@ describe FileStore::S3Store do
end
it "removes the file from s3 with the right paths" do
store.expects(:get_depth_for).with(optimized_image.upload.id).returns(0)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
@ -249,25 +261,5 @@ describe FileStore::S3Store do
it "should return the right bucket name" do
expect(store.s3_bucket).to eq('s3-upload-bucket')
end
it "should return the right bucket name when folders is included" do
SiteSetting.s3_upload_bucket = 's3-upload-bucket/this-site-upload-folder'
expect(store.s3_bucket).to eq('s3-upload-bucket')
end
end
describe "s3_bucket_folder_path" do
context "when no folder path is specified" do
it "should return the right folder path" do
expect(store.s3_bucket_folder_path).to eq(nil)
end
end
context "when site setting contains a folder path" do
it "should return the right folder path" do
SiteSetting.s3_upload_bucket = 's3-upload-bucket/this-site-upload-folder'
expect(store.s3_bucket_folder_path).to eq("this-site-upload-folder")
end
end
end
end

View File

@ -1,4 +1,5 @@
require 'rails_helper'
require "s3_helper"
require_dependency 'backup'
@ -28,11 +29,58 @@ describe Backup do
end
end
shared_context 's3 helpers' do
let(:client) { Aws::S3::Client.new(stub_responses: true) }
let(:resource) { Aws::S3::Resource.new(client: client) }
let!(:s3_bucket) { resource.bucket("s3-upload-bucket") }
let(:s3_helper) { b1.s3 }
before(:each) do
SiteSetting.s3_backup_bucket = "s3-upload-bucket"
SiteSetting.s3_access_key_id = "s3-access-key-id"
SiteSetting.s3_secret_access_key = "s3-secret-access-key"
end
end
context ".after_create_hook" do
it "calls upload_to_s3 if the SiteSetting is true" do
SiteSetting.enable_s3_backups = true
b1.expects(:upload_to_s3).once
b1.after_create_hook
context "when SiteSetting is true" do
include_context "s3 helpers"
before do
SiteSetting.enable_s3_backups = true
end
it "should upload the backup to S3 with the right paths" do
b1.path = 'some/path/backup.gz'
File.expects(:open).with(b1.path).yields(stub)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
s3_bucket.expects(:object).with(b1.filename).returns(s3_object)
s3_object.expects(:upload_file)
b1.after_create_hook
end
context "when s3_backup_bucket includes folders path" do
before do
SiteSetting.s3_backup_bucket = "s3-upload-bucket/discourse-backups"
end
it "should upload the backup to S3 with the right paths" do
b1.path = 'some/path/backup.gz'
File.expects(:open).with(b1.path).yields(stub)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
s3_bucket.expects(:object).with("discourse-backups/#{b1.filename}").returns(s3_object)
s3_object.expects(:upload_file)
b1.after_create_hook
end
end
end
it "calls upload_to_s3 if the SiteSetting is false" do
@ -43,10 +91,38 @@ describe Backup do
end
context ".after_remove_hook" do
it "calls remove_from_s3 if the SiteSetting is true" do
SiteSetting.enable_s3_backups = true
b1.expects(:remove_from_s3).once
b1.after_remove_hook
include_context "s3 helpers"
context "when SiteSetting is true" do
before do
SiteSetting.enable_s3_backups = true
end
it "should upload the backup to S3 with the right paths" do
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
s3_bucket.expects(:object).with(b1.filename).returns(s3_object)
s3_object.expects(:delete)
b1.after_remove_hook
end
context "when s3_backup_bucket includes folders path" do
before do
SiteSetting.s3_backup_bucket = "s3-upload-bucket/discourse-backups"
end
it "should upload the backup to S3 with the right paths" do
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_object = stub
s3_bucket.expects(:object).with("discourse-backups/#{b1.filename}").returns(s3_object)
s3_object.expects(:delete)
b1.after_remove_hook
end
end
end
it "calls remove_from_s3 if the SiteSetting is false" do