DEV: Remove SiteSetting.s3_force_path_style (#7210)

- s3_force_path_style was added as a Minio specific url scheme but it has never been well supported in our code base.
- Our new migrate_to_s3 rake task does not work reliably with path style urls too
- Minio has also added support for virtual style requests i.e the same scheme as AWS S3/DO Spaces so we can rely on that instead of using path style requests.
- Add migration to drop s3_force_path_style from the site_settings table

app/models/site_setting.rb

@@ -139,10 +139,6 @@ class SiteSetting < ActiveRecord::Base
       SiteSetting.enable_s3_uploads ? SiteSetting.s3_endpoint : GlobalSetting.s3_endpoint
     end
 
-    def self.s3_force_path_style
-      SiteSetting.enable_s3_uploads ? SiteSetting.s3_force_path_style : GlobalSetting.s3_force_path_style
-    end
-
     def self.enable_s3_uploads
       SiteSetting.enable_s3_uploads || GlobalSetting.use_s3?
     end
@@ -163,8 +159,6 @@ class SiteSetting < ActiveRecord::Base
         else
           "//#{bucket}.s3.dualstack.#{SiteSetting.Upload.s3_region}.amazonaws.com"
         end
-      elsif SiteSetting.s3_force_path_style
-        "//#{url_basename}/#{bucket}"
       else
         "//#{bucket}.#{url_basename}"
       end

config/discourse_defaults.conf

@@ -174,7 +174,6 @@ s3_secret_access_key =
 s3_use_iam_profile = false
 s3_cdn_url =
 s3_endpoint =
-s3_force_path_style =
 
 ### rate limits apply to all sites
 max_user_api_reqs_per_minute = 20

config/locales/server.en.yml

@@ -1507,7 +1507,6 @@ en:
     backup_frequency: "The number of days between backups."
     s3_backup_bucket: "The remote bucket to hold backups. WARNING: Make sure it is a private bucket."
     s3_endpoint: "The endpoint can be modified to backup to an S3 compatible service like DigitalOcean Spaces or Minio. WARNING: Leave blank if using AWS S3."
-    s3_force_path_style: "Enforce path-style addressing for your custom endpoint. IMPORTANT: Required for using Minio uploads and backups."
     s3_configure_tombstone_policy: "Enable automatic deletion policy for tombstone uploads. IMPORTANT: If disabled, no space will be reclaimed after uploads are deleted."
     s3_disable_cleanup: "Disable the removal of backups from S3 when removed locally."
     enable_s3_inventory: "Generate reports and verify uploads using Amazon S3 inventory. IMPORTANT: requires valid S3 credentials (both access key id & secret access key)."

config/site_settings.yml

@@ -1074,9 +1074,6 @@ files:
   s3_cdn_url:
     default: ""
     regex: '^https?:\/\/.+[^\/]$'
-  s3_force_path_style:
-    default: false
-    shadowed_by_global: true
   s3_configure_tombstone_policy:
     default: true
     shadowed_by_global: true

db/migrate/20190320104640_remove_s3_force_path_style.rb

@@ -0,0 +1,9 @@
+class RemoveS3ForcePathStyle < ActiveRecord::Migration[5.2]
+  def up
+    execute "DELETE FROM site_settings WHERE name = 's3_force_path_style'"
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/s3_helper.rb

@@ -192,8 +192,7 @@ class S3Helper
 
   def self.s3_options(obj)
     opts = {
-      region: obj.s3_region,
+      region: obj.s3_region
-      force_path_style: SiteSetting.s3_force_path_style
     }
 
     opts[:endpoint] = SiteSetting.s3_endpoint if SiteSetting.s3_endpoint.present?