FIX: S3 endpoint broke bucket creation in non-default region

FIX: S3 endpoint broke bucket creation in non-default region

diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb
index 0bdf7b3..49b9281 100644
--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@@ -152,7 +152,7 @@ class SiteSetting < ActiveRecord::Base
       bucket = SiteSetting.enable_s3_uploads ? Discourse.store.s3_bucket_name : GlobalSetting.s3_bucket_name
 
       # cf. http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region
-      if SiteSetting.s3_endpoint == "https://s3.amazonaws.com"
+      if SiteSetting.s3_endpoint.blank? || SiteSetting.s3_endpoint.end_with?("amazonaws.com")
         if SiteSetting.Upload.s3_region.start_with?("cn-")
           "//#{bucket}.s3.#{SiteSetting.Upload.s3_region}.amazonaws.com.cn"
         else
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 386b06b..286a8f2 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1485,7 +1485,7 @@ en:
     automatic_backups_enabled: "Run automatic backups as defined in backup frequency"
     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: Use default if using AWS S3"
+    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."
@@ -1542,7 +1542,7 @@ en:
     s3_upload_bucket: "The Amazon S3 bucket name that files will be uploaded into. WARNING: must be lowercase, no periods, no underscores."
     s3_access_key_id: "The Amazon S3 access key id that will be used to upload images."
     s3_secret_access_key: "The Amazon S3 secret access key that will be used to upload images."
-    s3_region: "The Amazon S3 region name that will be used to upload images."
+    s3_region: "The Amazon S3 region name that will be used to upload images and backups."
     s3_cdn_url: "The CDN URL to use for all s3 assets (for example: https://cdn.somewhere.com). WARNING: after changing this setting you must rebake all old posts."
 
     avatar_sizes: "List of automatically generated avatar sizes."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 5a1fcd1..7b13e73 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1038,7 +1038,7 @@ files:
     default: ""
     regex: '^[a-z0-9\-\/]+$' # can't use '.' when using HTTPS
   s3_endpoint:
-    default: "https://s3.amazonaws.com"
+    default: ""
     regex: '^https?:\/\/.+[^\/]$'
     shadowed_by_global: true
   s3_cdn_url:
diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb
index 380ba86..3e7b6c8 100644
--- a/lib/s3_helper.rb
+++ b/lib/s3_helper.rb
@@ -193,10 +193,11 @@ class S3Helper
   def self.s3_options(obj)
     opts = {
       region: obj.s3_region,
-      endpoint: SiteSetting.s3_endpoint,
       force_path_style: SiteSetting.s3_force_path_style
     }
 
+    opts[:endpoint] = SiteSetting.s3_endpoint if SiteSetting.s3_endpoint.present?
+
     unless obj.s3_use_iam_profile
       opts[:access_key_id] = obj.s3_access_key_id
       opts[:secret_access_key] = obj.s3_secret_access_key

GitHub sha: ba724d7f

Discourse failed to create a missing bucket when the region wasn’t the default “US East (N. Virginia)” because the S3 client ignores the region option when an endpoint is provided.

From the gem’s documentation:

The client endpoint is normally constructed from the :region option. You should only configure an :endpoint when connecting to test endpoints. This should be a valid HTTP(S) URI.

2 Likes

Wow, this clarifies everything. I could never find that detail in the docs and I spent too much time debugging this behavior here :cold_sweat:

2 Likes