FIX: Remap old S3 endpoints during backup restore (#12276)

It also starts outputting exceptions on the console.

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

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

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/",
+                from: s3_url_regex("old-bucket", "/"),
-                to: uploads_path(target_site_name)
+                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/",
+                from: s3_url_regex("old-bucket", "/"),
-                to: "/"
+                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/",
+                from: s3_url_regex("old-bucket", "/"),
-                to: uploads_path(target_site_name)
+                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/",
+                from: s3_url_regex("old-bucket", "/"),
-                to: "/"
+                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)