DEV: Introduce S3 transfer acceleration for uploads behind hidden setting (#24238)

This commit adds an `enable_s3_transfer_acceleration` site setting,
which is hidden to begin with. We are adding this because in certain
regions, using https://aws.amazon.com/s3/transfer-acceleration/ can
drastically speed up uploads, sometimes as much as 70% in certain
regions depending on the target bucket region. This is important for
us because we have direct S3 multipart uploads enabled everywhere
on our hosting.

To start, we only want this on the uploads bucket, not the backup one.
Also, this will accelerate both uploads **and** downloads, depending
on whether a presigned URL is used for downloading. This is the case
when secure uploads is enabled, not anywhere else at this time. To
enable the S3 acceleration on downloads more generally would be a
more in-depth change, since we currently store S3 Upload record URLs
like this:

```
 url: "//test.s3.dualstack.us-east-2.amazonaws.com/original/2X/6/123456.png"
```

For acceleration, `s3.dualstack` would need to be changed to `s3-accelerate.dualstack`
here.

Note that for this to have any effect, Transfer Acceleration must be enabled
on the S3 bucket used for uploads per https://docs.aws.amazon.com/AmazonS3/latest/userguide/transfer-acceleration-examples.html.

config/discourse_defaults.conf

@@ -225,6 +225,7 @@ s3_cdn_url =
 s3_endpoint =
 s3_http_continue_timeout =
 s3_install_cors_rule =
+s3_enable_transfer_acceleration =
 
 # Optionally, specify a separate CDN to be used for static JS assets stored on S3
 s3_asset_cdn_url =

config/site_settings.yml

@@ -1462,6 +1462,9 @@ files:
   enable_s3_uploads:
     default: false
     client: true
+  enable_s3_transfer_acceleration:
+    default: false
+    hidden: true
   s3_use_iam_profile:
     default: false
   s3_access_key_id:

lib/file_store/s3_store.rb

@@ -25,6 +25,7 @@ module FileStore
         S3Helper.new(
           s3_bucket,
           Rails.configuration.multisite ? multisite_tombstone_prefix : TOMBSTONE_PREFIX,
+          use_accelerate_endpoint: SiteSetting.enable_s3_transfer_acceleration,
         )
     end
 

lib/s3_helper.rb

@@ -47,6 +47,15 @@ class S3Helper
     setting_klass = use_db_s3_config ? SiteSetting : GlobalSetting
     options = S3Helper.s3_options(setting_klass)
     options[:client] = s3_client if s3_client.present?
+    use_accelerate_endpoint =
+      (
+        if use_db_s3_config
+          SiteSetting.enable_s3_transfer_acceleration
+        else
+          GlobalSetting.s3_enable_transfer_acceleration
+        end
+      )
+    options[:use_accelerate_endpoint] = !for_backup && use_accelerate_endpoint
 
     bucket =
       if for_backup
@@ -349,7 +358,12 @@ class S3Helper
   def presigned_url(key, method:, expires_in: S3Helper::UPLOAD_URL_EXPIRES_AFTER_SECONDS, opts: {})
     Aws::S3::Presigner.new(client: s3_client).presigned_url(
       method,
-      { bucket: s3_bucket_name, key: key, expires_in: expires_in }.merge(opts),
+      {
+        bucket: s3_bucket_name,
+        key: key,
+        expires_in: expires_in,
+        use_accelerate_endpoint: @s3_options[:use_accelerate_endpoint],
+      }.merge(opts),
     )
   end
 
@@ -362,7 +376,12 @@ class S3Helper
   )
     Aws::S3::Presigner.new(client: s3_client).presigned_request(
       method,
-      { bucket: s3_bucket_name, key: key, expires_in: expires_in }.merge(opts),
+      {
+        bucket: s3_bucket_name,
+        key: key,
+        expires_in: expires_in,
+        use_accelerate_endpoint: @s3_options[:use_accelerate_endpoint],
+      }.merge(opts),
     )
   end
 

spec/requests/uploads_controller_spec.rb

@@ -780,6 +780,29 @@ RSpec.describe UploadsController do
         )
       end
 
+      context "when enable_s3_transfer_acceleration is true" do
+        before { SiteSetting.enable_s3_transfer_acceleration = true }
+
+        it "uses the s3-accelerate endpoint for presigned URLs" do
+          post "/uploads/generate-presigned-put.json",
+               **{
+                 params: {
+                   file_name: "test.png",
+                   file_size: 1024,
+                   type: "card_background",
+                   metadata: {
+                     "sha1-checksum" => "testing",
+                     "blah" => "wontbeincluded",
+                   },
+                 },
+               }
+          expect(response.status).to eq(200)
+
+          result = response.parsed_body
+          expect(result["url"]).to include("s3-accelerate")
+        end
+      end
+
       describe "rate limiting" do
         before { RateLimiter.enable }
 
@@ -1045,7 +1068,7 @@ RSpec.describe UploadsController do
         XML
         stub_request(
           :get,
-          "https://s3-upload-bucket.s3.us-west-1.amazonaws.com/#{external_upload_stub.key}?max-parts=1&uploadId=#{mock_multipart_upload_id}",
+          "https://s3-upload-bucket.#{SiteSetting.enable_s3_transfer_acceleration ? "s3-accelerate" : "s3.us-west-1"}.amazonaws.com/#{external_upload_stub.key}?max-parts=1&uploadId=#{mock_multipart_upload_id}",
         ).to_return({ status: 200, body: list_multipart_result })
       end
 
@@ -1129,6 +1152,24 @@ RSpec.describe UploadsController do
         )
       end
 
+      context "when enable_s3_transfer_acceleration is true" do
+        before { SiteSetting.enable_s3_transfer_acceleration = true }
+
+        it "uses the s3-accelerate endpoint for presigned URLs" do
+          stub_list_multipart_request
+          post "/uploads/batch-presign-multipart-parts.json",
+               params: {
+                 unique_identifier: external_upload_stub.unique_identifier,
+                 part_numbers: [2, 3, 4],
+               }
+
+          expect(response.status).to eq(200)
+          result = response.parsed_body
+          expect(result["presigned_urls"].keys).to eq(%w[2 3 4])
+          expect(result["presigned_urls"]["2"]).to include("s3-accelerate")
+        end
+      end
+
       describe "rate limiting" do
         before { RateLimiter.enable }
 
@@ -1173,7 +1214,7 @@ RSpec.describe UploadsController do
 
   describe "#complete_multipart" do
     let(:upload_base_url) do
-      "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com"
+      "https://#{SiteSetting.s3_upload_bucket}.#{SiteSetting.enable_s3_transfer_acceleration ? "s3-accelerate" : "s3.#{SiteSetting.s3_region}"}.amazonaws.com"
     end
     let(:mock_multipart_upload_id) do
       "ibZBv_75gd9r8lH_gqXatLdxMVpAlj6CFTR.OwyF3953YdwbcQnMA2BLGn8Lx12fQNICtMw5KyteFeHw.Sjng--"
@@ -1396,7 +1437,7 @@ RSpec.describe UploadsController do
 
   describe "#abort_multipart" do
     let(:upload_base_url) do
-      "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com"
+      "https://#{SiteSetting.s3_upload_bucket}.#{SiteSetting.enable_s3_transfer_acceleration ? "s3-accelerate" : "s3.#{SiteSetting.s3_region}"}.amazonaws.com"
     end
     let(:mock_multipart_upload_id) do
       "ibZBv_75gd9r8lH_gqXatLdxMVpAlj6CFTR.OwyF3953YdwbcQnMA2BLGn8Lx12fQNICtMw5KyteFeHw.Sjng--"