From 58c218a4bfd17a38b3770664ca49eb95891571cd Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Wed, 3 Mar 2021 21:10:09 +0100 Subject: [PATCH] FIX: Remap old S3 endpoints during backup restore (#12276) It also starts outputting exceptions on the console. --- lib/backup_restore/logger.rb | 7 ++- lib/backup_restore/uploads_restorer.rb | 26 +++++++- .../backup_restore/uploads_restorer_spec.rb | 61 ++++++++++++++++--- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/lib/backup_restore/logger.rb b/lib/backup_restore/logger.rb index f33acbfd46d..fe777d389bc 100644 --- a/lib/backup_restore/logger.rb +++ b/lib/backup_restore/logger.rb @@ -19,7 +19,12 @@ module BackupRestore puts(message) publish_log(message, timestamp) save_log(message, timestamp) - Rails.logger.error("#{ex}\n" + ex.backtrace.join("\n")) if ex + + if ex + formatted_ex = "#{ex}\n" + ex.backtrace.join("\n") + puts formatted_ex + Rails.logger.error(formatted_ex) + end end protected diff --git a/lib/backup_restore/uploads_restorer.rb b/lib/backup_restore/uploads_restorer.rb index ae986bc1648..0cf18a0558d 100644 --- a/lib/backup_restore/uploads_restorer.rb +++ b/lib/backup_restore/uploads_restorer.rb @@ -6,6 +6,20 @@ module BackupRestore class UploadsRestorer delegate :log, to: :@logger, private: true + S3_ENDPOINT_REGEX = /\.s3(?:\.dualstack\.[a-z0-9\-]+?|[.\-][a-z0-9\-]+?)?\.amazonaws\.com/ + + def self.s3_regex_string(s3_base_url) + clean_url = s3_base_url.sub(S3_ENDPOINT_REGEX, ".s3.amazonaws.com") + + regex_string = clean_url + .split(".s3.amazonaws.com") + .map { |s| Regexp.escape(s) } + .insert(1, S3_ENDPOINT_REGEX.source) + .join("") + + [regex_string, clean_url] + end + def initialize(logger) @logger = logger end @@ -77,7 +91,7 @@ module BackupRestore current_s3_base_url = SiteSetting::Upload.enable_s3_uploads ? SiteSetting::Upload.s3_base_url : nil if (old_s3_base_url = BackupMetadata.value_for("s3_base_url")) && old_s3_base_url != current_s3_base_url - remap("#{old_s3_base_url}/", uploads_folder) + remap_s3("#{old_s3_base_url}/", uploads_folder) end current_s3_cdn_url = SiteSetting::Upload.enable_s3_uploads ? SiteSetting::Upload.s3_cdn_url : nil @@ -112,6 +126,16 @@ module BackupRestore DbHelper.remap(from, to, verbose: true, excluded_tables: ["backup_metadata"]) end + def remap_s3(old_s3_base_url, uploads_folder) + if old_s3_base_url.include?("amazonaws.com") + from_regex, from_clean_url = self.class.s3_regex_string(old_s3_base_url) + log "Remapping with regex from '#{from_clean_url}' to '#{uploads_folder}'" + DbHelper.regexp_replace(from_regex, uploads_folder, verbose: true, excluded_tables: ["backup_metadata"]) + else + remap(old_s3_base_url, uploads_folder) + end + end + def generate_optimized_images log "Optimizing site icons..." DB.exec("TRUNCATE TABLE optimized_images") diff --git a/spec/lib/backup_restore/uploads_restorer_spec.rb b/spec/lib/backup_restore/uploads_restorer_spec.rb index 3a3729bed2d..f4fb16ec48a 100644 --- a/spec/lib/backup_restore/uploads_restorer_spec.rb +++ b/spec/lib/backup_restore/uploads_restorer_spec.rb @@ -25,17 +25,20 @@ describe BackupRestore::UploadsRestorer do ) end - def expect_remap(source_site_name: nil, target_site_name:, metadata: [], from:, to:, &block) + def expect_remap(source_site_name: nil, target_site_name:, metadata: [], from:, to:, regex: false, &block) expect_remaps( source_site_name: source_site_name, target_site_name: target_site_name, metadata: metadata, - remaps: [{ from: from, to: to }], + remaps: [{ from: from, to: to, regex: regex }], &block ) end def expect_remaps(source_site_name: nil, target_site_name:, metadata: [], remaps: [], &block) + regex_remaps = remaps.select { |r| r[:regex] } + remaps.delete_if { |r| r.delete(:regex) } + source_site_name ||= metadata.find { |d| d[:name] == "db_name" }&.dig(:value) || "default" if source_site_name != target_site_name @@ -57,6 +60,15 @@ describe BackupRestore::UploadsRestorer do end.times(remaps.size) end + if regex_remaps.blank? + DbHelper.expects(:regexp_replace).never + else + DbHelper.expects(:regexp_replace).with do |from, to, args| + args[:excluded_tables]&.include?("backup_metadata") + regex_remaps.shift == { from: from, to: to } + end.times(regex_remaps.size) + end + if target_site_name == "default" setup_and_restore(directory, metadata) else @@ -78,6 +90,10 @@ describe BackupRestore::UploadsRestorer do "/#{path}/" end + def s3_url_regex(bucket, path) + Regexp.escape("//#{bucket}") + %q*\.s3(?:\.dualstack\.[a-z0-9\-]+?|[.\-][a-z0-9\-]+?)?\.amazonaws\.com* + Regexp.escape(path) + end + context "uploads" do let!(:multisite) { { name: "multisite", value: true } } let!(:no_multisite) { { name: "multisite", value: false } } @@ -289,8 +305,9 @@ describe BackupRestore::UploadsRestorer do expect_remap( target_site_name: target_site_name, metadata: [no_multisite, s3_base_url], - from: "//old-bucket.s3-us-east-1.amazonaws.com/", - to: uploads_path(target_site_name) + from: s3_url_regex("old-bucket", "/"), + to: uploads_path(target_site_name), + regex: true ) end @@ -311,8 +328,9 @@ describe BackupRestore::UploadsRestorer do expect_remap( target_site_name: target_site_name, metadata: [source_db_name, multisite, s3_base_url], - from: "//old-bucket.s3-us-east-1.amazonaws.com/", - to: "/" + from: s3_url_regex("old-bucket", "/"), + to: "/", + regex: true ) end @@ -430,8 +448,9 @@ describe BackupRestore::UploadsRestorer do expect_remap( target_site_name: target_site_name, metadata: [no_multisite, s3_base_url], - from: "//old-bucket.s3-us-east-1.amazonaws.com/", - to: uploads_path(target_site_name) + from: s3_url_regex("old-bucket", "/"), + to: uploads_path(target_site_name), + regex: true ) end @@ -454,8 +473,9 @@ describe BackupRestore::UploadsRestorer do expect_remap( target_site_name: target_site_name, metadata: [source_db_name, multisite, s3_base_url], - from: "//old-bucket.s3-us-east-1.amazonaws.com/", - to: "/" + from: s3_url_regex("old-bucket", "/"), + to: "/", + regex: true ) end @@ -532,6 +552,27 @@ describe BackupRestore::UploadsRestorer do end end + describe ".s3_regex_string" do + def regex_matches(s3_base_url) + regex, _ = BackupRestore::UploadsRestorer.s3_regex_string(s3_base_url) + expect(Regexp.new(regex)).to match(s3_base_url) + end + + it "correctly matches different S3 base URLs" do + regex_matches("//some-bucket.s3.amazonaws.com/") + regex_matches("//some-bucket.s3.us-west-2.amazonaws.com/") + regex_matches("//some-bucket.s3-us-west-2.amazonaws.com/") + regex_matches("//some-bucket.s3.dualstack.us-west-2.amazonaws.com/") + regex_matches("//some-bucket.s3.cn-north-1.amazonaws.com.cn/") + + regex_matches("//some-bucket.s3.amazonaws.com/foo/") + regex_matches("//some-bucket.s3.us-east-2.amazonaws.com/foo/") + regex_matches("//some-bucket.s3-us-east-2.amazonaws.com/foo/") + regex_matches("//some-bucket.s3.dualstack.us-east-2.amazonaws.com/foo/") + regex_matches("//some-bucket.s3.cn-north-1.amazonaws.com.cn/foo/") + end + end + it "raises an exception when the store doesn't support the copy_from method" do Discourse.stubs(:store).returns(Object.new)