diff --git a/lib/backup_restore/backuper.rb b/lib/backup_restore/backuper.rb index f03d3dd50f3..376c0a862c9 100644 --- a/lib/backup_restore/backuper.rb +++ b/lib/backup_restore/backuper.rb @@ -83,7 +83,7 @@ module BackupRestore @timestamp = Time.now.strftime("%Y-%m-%d-%H%M%S") @tmp_directory = File.join(Rails.root, "tmp", "backups", @current_db, @timestamp) @dump_filename = File.join(@tmp_directory, BackupRestore::DUMP_FILE) - @archive_directory = BackupRestore::LocalBackupStore.base_directory(@current_db) + @archive_directory = BackupRestore::LocalBackupStore.base_directory(db: @current_db) filename = @filename_override || "#{SiteSetting.title.parameterize}-#{@timestamp}" @archive_basename = File.join(@archive_directory, "#{filename}-#{BackupRestore::VERSION_PREFIX}#{BackupRestore.current_version}") diff --git a/lib/backup_restore/local_backup_store.rb b/lib/backup_restore/local_backup_store.rb index 8b0148ee3c3..fce72d27ecb 100644 --- a/lib/backup_restore/local_backup_store.rb +++ b/lib/backup_restore/local_backup_store.rb @@ -3,9 +3,11 @@ require_dependency "disk_space" module BackupRestore class LocalBackupStore < BackupStore - def self.base_directory(current_db = nil) - current_db ||= RailsMultisite::ConnectionManagement.current_db - base_directory = File.join(Rails.root, "public", "backups", current_db) + def self.base_directory(db: nil, root_directory: nil) + current_db = db || RailsMultisite::ConnectionManagement.current_db + root_directory ||= File.join(Rails.root, "public", "backups") + + base_directory = File.join(root_directory, current_db) FileUtils.mkdir_p(base_directory) unless Dir.exists?(base_directory) base_directory end @@ -15,7 +17,7 @@ module BackupRestore end def initialize(opts = {}) - @base_directory = opts[:base_directory] || LocalBackupStore.base_directory + @base_directory = LocalBackupStore.base_directory(root_directory: opts[:root_directory]) end def remote? diff --git a/lib/backup_restore/s3_backup_store.rb b/lib/backup_restore/s3_backup_store.rb index cbbc916df08..2fae590b70a 100644 --- a/lib/backup_restore/s3_backup_store.rb +++ b/lib/backup_restore/s3_backup_store.rb @@ -5,11 +5,12 @@ module BackupRestore class S3BackupStore < BackupStore DOWNLOAD_URL_EXPIRES_AFTER_SECONDS ||= 15 UPLOAD_URL_EXPIRES_AFTER_SECONDS ||= 21_600 # 6 hours + MULTISITE_PREFIX = "backups" def initialize(opts = {}) s3_options = S3Helper.s3_options(SiteSetting) s3_options.merge!(opts[:s3_options]) if opts[:s3_options] - @s3_helper = S3Helper.new(SiteSetting.s3_backup_bucket, '', s3_options) + @s3_helper = S3Helper.new(s3_bucket_name_with_prefix, '', s3_options) end def remote? @@ -91,5 +92,13 @@ module BackupRestore def cleanup_allowed? !SiteSetting.s3_disable_cleanup end + + def s3_bucket_name_with_prefix + if Rails.configuration.multisite + File.join(SiteSetting.s3_backup_bucket, MULTISITE_PREFIX, RailsMultisite::ConnectionManagement.current_db) + else + SiteSetting.s3_backup_bucket + end + end end end diff --git a/spec/lib/backup_restore/local_backup_store_spec.rb b/spec/lib/backup_restore/local_backup_store_spec.rb index efe87583d90..bd5f627c07a 100644 --- a/spec/lib/backup_restore/local_backup_store_spec.rb +++ b/spec/lib/backup_restore/local_backup_store_spec.rb @@ -4,19 +4,19 @@ require_relative 'shared_examples_for_backup_store' describe BackupRestore::LocalBackupStore do before(:all) do - @base_directory = Dir.mktmpdir + @root_directory = Dir.mktmpdir @paths = [] end after(:all) do - FileUtils.remove_dir(@base_directory, true) + FileUtils.remove_dir(@root_directory, true) end before do SiteSetting.backup_location = BackupLocationSiteSetting::LOCAL end - subject(:store) { BackupRestore::BackupStore.create(base_directory: @base_directory) } + subject(:store) { BackupRestore::BackupStore.create(root_directory: @root_directory) } let(:expected_type) { BackupRestore::LocalBackupStore } it_behaves_like "backup store" @@ -26,10 +26,13 @@ describe BackupRestore::LocalBackupStore do end def create_backups - create_file(filename: "b.tar.gz", last_modified: "2018-09-13T15:10:00Z", size_in_bytes: 17) - create_file(filename: "a.tgz", last_modified: "2018-02-11T09:27:00Z", size_in_bytes: 29) - create_file(filename: "r.sql.gz", last_modified: "2017-12-20T03:48:00Z", size_in_bytes: 11) - create_file(filename: "no-backup.txt", last_modified: "2018-09-05T14:27:00Z", size_in_bytes: 12) + create_file(db_name: "default", filename: "b.tar.gz", last_modified: "2018-09-13T15:10:00Z", size_in_bytes: 17) + create_file(db_name: "default", filename: "a.tgz", last_modified: "2018-02-11T09:27:00Z", size_in_bytes: 29) + create_file(db_name: "default", filename: "r.sql.gz", last_modified: "2017-12-20T03:48:00Z", size_in_bytes: 11) + create_file(db_name: "default", filename: "no-backup.txt", last_modified: "2018-09-05T14:27:00Z", size_in_bytes: 12) + + create_file(db_name: "second", filename: "multi-2.tar.gz", last_modified: "2018-11-27T03:16:54Z", size_in_bytes: 19) + create_file(db_name: "second", filename: "multi-1.tar.gz", last_modified: "2018-11-26T03:17:09Z", size_in_bytes: 22) end def remove_backups @@ -37,8 +40,11 @@ describe BackupRestore::LocalBackupStore do @paths.clear end - def create_file(filename:, last_modified:, size_in_bytes:) - path = File.join(@base_directory, filename) + def create_file(db_name:, filename:, last_modified:, size_in_bytes:) + path = File.join(@root_directory, db_name) + Dir.mkdir(path) unless Dir.exists?(path) + + path = File.join(path, filename) return if File.exists?(path) @paths << path @@ -49,8 +55,8 @@ describe BackupRestore::LocalBackupStore do File.utime(time, time, path) end - def source_regex(filename) - path = File.join(@base_directory, filename) + def source_regex(db_name, filename, multisite:) + path = File.join(@root_directory, db_name, filename) /^#{Regexp.escape(path)}$/ end end diff --git a/spec/lib/backup_restore/s3_backup_store_spec.rb b/spec/lib/backup_restore/s3_backup_store_spec.rb index 33cfa945b00..3020cabe0cc 100644 --- a/spec/lib/backup_restore/s3_backup_store_spec.rb +++ b/spec/lib/backup_restore/s3_backup_store_spec.rb @@ -9,22 +9,32 @@ describe BackupRestore::S3BackupStore do @objects = [] - @s3_client.stub_responses(:list_objects, -> (context) do - expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket) - expect(context.params[:prefix]).to be_blank + def expected_prefix + Rails.configuration.multisite ? "backups/#{RailsMultisite::ConnectionManagement.current_db}/" : "" + end - { contents: @objects } + def check_context(context) + expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket) + expect(context.params[:key]).to start_with(expected_prefix) if context.params.key?(:key) + expect(context.params[:prefix]).to eq(expected_prefix) if context.params.key?(:prefix) + end + + @s3_client.stub_responses(:list_objects, -> (context) do + check_context(context) + + { contents: objects_with_prefix(context) } end) @s3_client.stub_responses(:delete_object, -> (context) do - expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket) + check_context(context) + expect do @objects.delete_if { |obj| obj[:key] == context.params[:key] } end.to change { @objects } end) @s3_client.stub_responses(:head_object, -> (context) do - expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket) + check_context(context) if object = @objects.find { |obj| obj[:key] == context.params[:key] } { content_length: object[:size], last_modified: object[:last_modified] } @@ -34,7 +44,7 @@ describe BackupRestore::S3BackupStore do end) @s3_client.stub_responses(:get_object, -> (context) do - expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket) + check_context(context) if object = @objects.find { |obj| obj[:key] == context.params[:key] } { content_length: object[:size], body: "A" * object[:size] } @@ -44,7 +54,7 @@ describe BackupRestore::S3BackupStore do end) @s3_client.stub_responses(:put_object, -> (context) do - expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket) + check_context(context) @objects << { key: context.params[:key], @@ -79,31 +89,51 @@ describe BackupRestore::S3BackupStore do end end + def objects_with_prefix(context) + prefix = context.params[:prefix] + + if prefix.blank? + @objects.reject { |obj| obj[:key].include?("/") } + else + @objects.select { |obj| obj[:key].start_with?(prefix) } + end + end + def create_backups @objects.clear + @objects << { key: "b.tar.gz", size: 17, last_modified: Time.parse("2018-09-13T15:10:00Z") } @objects << { key: "a.tgz", size: 29, last_modified: Time.parse("2018-02-11T09:27:00Z") } @objects << { key: "r.sql.gz", size: 11, last_modified: Time.parse("2017-12-20T03:48:00Z") } @objects << { key: "no-backup.txt", size: 12, last_modified: Time.parse("2018-09-05T14:27:00Z") } + + @objects << { key: "backups/second/multi-2.tar.gz", size: 19, last_modified: Time.parse("2018-11-27T03:16:54Z") } + @objects << { key: "backups/second/multi-1.tar.gz", size: 22, last_modified: Time.parse("2018-11-26T03:17:09Z") } end def remove_backups @objects.clear end - def source_regex(filename) + def source_regex(db_name, filename, multisite:) bucket = Regexp.escape(SiteSetting.s3_backup_bucket) + prefix = file_prefix(db_name, multisite) filename = Regexp.escape(filename) expires = BackupRestore::S3BackupStore::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS - /\Ahttps:\/\/#{bucket}.*\/#{filename}\?.*X-Amz-Expires=#{expires}.*X-Amz-Signature=.*\z/ + /\Ahttps:\/\/#{bucket}.*#{prefix}\/#{filename}\?.*X-Amz-Expires=#{expires}.*X-Amz-Signature=.*\z/ end - def upload_url_regex(filename) + def upload_url_regex(db_name, filename, multisite:) bucket = Regexp.escape(SiteSetting.s3_backup_bucket) + prefix = file_prefix(db_name, multisite) filename = Regexp.escape(filename) expires = BackupRestore::S3BackupStore::UPLOAD_URL_EXPIRES_AFTER_SECONDS - /\Ahttps:\/\/#{bucket}.*\/#{filename}\?.*X-Amz-Expires=#{expires}.*X-Amz-Signature=.*\z/ + /\Ahttps:\/\/#{bucket}.*#{prefix}\/#{filename}\?.*X-Amz-Expires=#{expires}.*X-Amz-Signature=.*\z/ + end + + def file_prefix(db_name, multisite) + multisite ? "\\/#{db_name}" : "" end end diff --git a/spec/lib/backup_restore/shared_examples_for_backup_store.rb b/spec/lib/backup_restore/shared_examples_for_backup_store.rb index 2ab716b6030..9a66186bccb 100644 --- a/spec/lib/backup_restore/shared_examples_for_backup_store.rb +++ b/spec/lib/backup_restore/shared_examples_for_backup_store.rb @@ -2,9 +2,14 @@ shared_context "backups" do before { create_backups } after(:all) { remove_backups } + # default backup files let(:backup1) { BackupFile.new(filename: "b.tar.gz", size: 17, last_modified: Time.parse("2018-09-13T15:10:00Z")) } let(:backup2) { BackupFile.new(filename: "a.tgz", size: 29, last_modified: Time.parse("2018-02-11T09:27:00Z")) } let(:backup3) { BackupFile.new(filename: "r.sql.gz", size: 11, last_modified: Time.parse("2017-12-20T03:48:00Z")) } + + # backup files on another multisite + let(:backup4) { BackupFile.new(filename: "multi-1.tar.gz", size: 22, last_modified: Time.parse("2018-11-26T03:17:09Z")) } + let(:backup5) { BackupFile.new(filename: "multi-2.tar.gz", size: 19, last_modified: Time.parse("2018-11-27T03:16:54Z")) } end shared_examples "backup store" do @@ -39,6 +44,12 @@ shared_examples "backup store" do expect(files).to_not be_empty expect(files.map(&:filename)).to contain_exactly(backup1.filename, backup2.filename, backup3.filename) end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + expect(store.files).to eq([backup5, backup4]) + end + end end describe "#latest_file" do @@ -50,6 +61,12 @@ shared_examples "backup store" do store.files.each { |file| store.delete_file(file.filename) } expect(store.latest_file).to be_nil end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + expect(store.latest_file).to eq(backup5) + end + end end describe "#delete_old" do @@ -66,6 +83,15 @@ shared_examples "backup store" do store.delete_old expect(store.files).to eq([backup1]) end + + it "works with multisite", type: :multisite do + SiteSetting.maximum_backups = 1 + + RailsMultisite::ConnectionManagement.with_connection("second") do + store.delete_old + expect(store.files).to eq([backup5]) + end + end end describe "#file" do @@ -79,7 +105,14 @@ shared_examples "backup store" do it "includes the file's source location if it is requested" do file = store.file(backup1.filename, include_download_source: true) - expect(file.source).to match(source_regex(backup1.filename)) + expect(file.source).to match(source_regex("default", backup1.filename, multisite: false)) + end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + file = store.file(backup4.filename, include_download_source: true) + expect(file.source).to match(source_regex("second", backup4.filename, multisite: true)) + end end end @@ -95,6 +128,14 @@ shared_examples "backup store" do it "does nothing when the file doesn't exist" do expect { store.delete_file("foo.gz") }.to_not change { store.files } end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + expect(store.files).to include(backup5) + store.delete_file(backup5.filename) + expect(store.files).to_not include(backup5) + end + end end describe "#download_file" do @@ -116,6 +157,14 @@ shared_examples "backup store" do expect { store.download_file(filename, destination_path) }.to raise_exception(StandardError) end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + expect(store.files).to include(backup5) + store.delete_file(backup5.filename) + expect(store.files).to_not include(backup5) + end + end end end end @@ -129,7 +178,7 @@ shared_examples "remote backup store" do include_context "backups" describe "#upload_file" do - it "uploads file into store" do + def upload_file freeze_time backup = BackupFile.new( @@ -151,6 +200,16 @@ shared_examples "remote backup store" do expect(store.file(backup.filename)).to eq(backup) end + it "uploads file into store" do + upload_file + end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + upload_file + end + end + it "raises an exception when a file with same filename exists" do Tempfile.create(backup1.filename) do |file| expect { store.upload_file(backup1.filename, file.path, "application/gzip") } @@ -164,13 +223,22 @@ shared_examples "remote backup store" do filename = "foo.tar.gz" url = store.generate_upload_url(filename) - expect(url).to match(upload_url_regex(filename)) + expect(url).to match(upload_url_regex("default", filename, multisite: false)) end it "raises an exeption when a file with same filename exists" do expect { store.generate_upload_url(backup1.filename) } .to raise_exception(BackupRestore::BackupStore::BackupFileExists) end + + it "works with multisite", type: :multisite do + RailsMultisite::ConnectionManagement.with_connection("second") do + filename = "foo.tar.gz" + url = store.generate_upload_url(filename) + + expect(url).to match(upload_url_regex("second", filename, multisite: true)) + end + end end end end