FEATURE: Multisite support for S3 backup store (#6700)

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}")
 

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)
+    def self.base_directory(db: nil, root_directory: nil)
-      current_db ||= RailsMultisite::ConnectionManagement.current_db
+      current_db = db || RailsMultisite::ConnectionManagement.current_db
-      base_directory = File.join(Rails.root, "public", "backups", 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?

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

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(db_name: "default", 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(db_name: "default", 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(db_name: "default", 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: "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:)
+  def create_file(db_name:, filename:, last_modified:, size_in_bytes:)
-    path = File.join(@base_directory, filename)
+    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)
+  def source_regex(db_name, filename, multisite:)
-    path = File.join(@base_directory, filename)
+    path = File.join(@root_directory, db_name, filename)
     /^#{Regexp.escape(path)}$/
   end
 end

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
+    def expected_prefix
-      expect(context.params[:bucket]).to eq(SiteSetting.s3_backup_bucket)
+      Rails.configuration.multisite ? "backups/#{RailsMultisite::ConnectionManagement.current_db}/" : ""
-      expect(context.params[:prefix]).to be_blank
+    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

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