DEV: Clean up S3 specs, stubs, and helpers

Extracted commonly used spec helpers into spec/support/uploads_helpers.rb, removed unused stubs and let definitions. Makes it easier to write new S3-related specs without copy and pasting setup steps from other specs.

spec/components/cooked_post_processor_spec.rb

@@ -4,17 +4,6 @@ require "rails_helper"
 require "cooked_post_processor"
 require "file_store/s3_store"
 
-def s3_setup
-  Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com")
-
-  SiteSetting.s3_upload_bucket = "some-bucket-on-s3"
-  SiteSetting.s3_access_key_id = "s3-access-key-id"
-  SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-  SiteSetting.s3_cdn_url = "https://s3.cdn.com"
-  SiteSetting.enable_s3_uploads = true
-  SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|"
-end
-
 describe CookedPostProcessor do
   fab!(:upload) { Fabricate(:upload) }
   let(:upload_path) { Discourse.store.upload_path }
@@ -483,17 +472,16 @@ describe CookedPostProcessor do
 
         context "s3_uploads" do
           let(:upload) { Fabricate(:secure_upload_s3) }
+
           before do
-            s3_setup
+            setup_s3
+            SiteSetting.s3_cdn_url = "https://s3.cdn.com"
+            SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|"
+
             stored_path = Discourse.store.get_path_for_upload(upload)
             upload.update_column(:url, "#{SiteSetting.Upload.absolute_base_url}/#{stored_path}")
 
-            stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
+            stub_upload(upload)
-            stub_request(
-              :put,
-              "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/optimized/1X/#{upload.sha1}_2_#{optimized_size}.#{upload.extension}"
-            )
-            stub_request(:get, /#{SiteSetting.s3_upload_bucket}\.s3\.amazonaws\.com/)
 
             SiteSetting.login_required = true
             SiteSetting.secure_media = true
@@ -1049,11 +1037,11 @@ describe CookedPostProcessor do
 
       context "when the post is with_secure_media and the upload is secure and secure media is enabled" do
         before do
+          setup_s3
           upload.update(secure: true)
+
           SiteSetting.login_required = true
-          s3_setup
           SiteSetting.secure_media = true
-          stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
         end
 
         it "does not use the direct URL, uses the cooked URL instead (because of the private ACL preventing w/h fetch)" do
@@ -1269,7 +1257,11 @@ describe CookedPostProcessor do
 
       context "s3_uploads" do
         before do
-          s3_setup
+          Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com")
+
+          setup_s3
+          SiteSetting.s3_cdn_url = "https://s3.cdn.com"
+          SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|"
 
           uploaded_file = file_from_fixtures("smallest.png")
           upload_sha1 = Digest::SHA1.hexdigest(File.read(uploaded_file))
@@ -1546,9 +1538,7 @@ describe CookedPostProcessor do
       end
 
       it "doesn't disable download_remote_images_to_local if site uses S3" do
-        SiteSetting.s3_access_key_id = "s3-access-key-id"
+        setup_s3
-        SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-        SiteSetting.enable_s3_uploads = true
         cpp.disable_if_low_on_disk_space
 
         expect(SiteSetting.download_remote_images_to_local).to eq(true)

spec/components/email/sender_spec.rb

@@ -408,25 +408,10 @@ describe Email::Sender do
     end
 
     context "when secure media enabled" do
-      def enable_s3_uploads
-        SiteSetting.enable_s3_uploads = true
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "some key"
-        SiteSetting.s3_secret_access_key = "some secrets3_region key"
-        stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-        stub_request(
-          :put,
-          "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image.sha1}.#{image.extension}?acl"
-        )
-        store = FileStore::S3Store.new
-        s3_helper = store.instance_variable_get(:@s3_helper)
-        client = Aws::S3::Client.new(stub_responses: true)
-        s3_helper.stubs(:s3_client).returns(client)
-        Discourse.stubs(:store).returns(store)
-      end
-
       before do
-        enable_s3_uploads
+        setup_s3
+        stub_s3_store
+
         SiteSetting.secure_media = true
         SiteSetting.login_required = true
         SiteSetting.email_total_attachment_size_limit_kb = 14_000

spec/components/file_store/base_store_spec.rb

@@ -47,12 +47,7 @@ RSpec.describe FileStore::BaseStore do
 
   describe '#download' do
     before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-      SiteSetting.s3_access_key_id = "some key"
-      SiteSetting.s3_secret_access_key = "some secret key"
-      SiteSetting.s3_region = "us-east-1"
-
       stub_request(:get, upload_s3.url).to_return(status: 200, body: "Hello world")
     end
 
@@ -78,7 +73,7 @@ RSpec.describe FileStore::BaseStore do
     end
 
     it "should return the file when s3 cdn enabled" do
-      SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com"
+      SiteSetting.s3_cdn_url = "https://cdn.s3.#{SiteSetting.s3_region}.amazonaws.com"
       stub_request(:get, Discourse.store.cdn_url(upload_s3.url)).to_return(status: 200, body: "Hello world")
 
       file = store.download(upload_s3)
@@ -90,7 +85,7 @@ RSpec.describe FileStore::BaseStore do
       SiteSetting.login_required = true
       SiteSetting.secure_media = true
 
-      stub_request(:head, "https://s3-upload-bucket.s3.amazonaws.com/")
+      stub_request(:head, "https://s3-upload-bucket.s3.#{SiteSetting.s3_region}.amazonaws.com/")
       signed_url = Discourse.store.signed_url_for_path(upload_s3.url)
       stub_request(:get, signed_url).to_return(status: 200, body: "Hello world")
 

spec/components/file_store/s3_store_spec.rb

@@ -5,42 +5,26 @@ require 'file_store/s3_store'
 require 'file_store/local_store'
 
 describe FileStore::S3Store do
-
   let(:store) { FileStore::S3Store.new }
   let(:s3_helper) { store.instance_variable_get(:@s3_helper) }
-  fab!(:upload) { Fabricate(:upload) }
+  let(:client) { Aws::S3::Client.new(stub_responses: true) }
-  let(:uploaded_file) { file_from_fixtures("logo.png") }
+  let(:resource) { Aws::S3::Resource.new(client: client) }
+  let(:s3_bucket) { resource.bucket("s3-upload-bucket") }
+  let(:s3_object) { stub }
 
   fab!(:optimized_image) { Fabricate(:optimized_image) }
   let(:optimized_image_file) { file_from_fixtures("logo.png") }
-
+  let(:uploaded_file) { file_from_fixtures("logo.png") }
-  before(:each) do
+  fab!(:upload) do
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+    Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string'))
-    SiteSetting.s3_access_key_id = "s3-access-key-id"
-    SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-    SiteSetting.enable_s3_uploads = true
   end
 
-  shared_context 's3 helpers' do
+  before do
-    fab!(:upload) do
+    setup_s3
-      Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string'))
+    SiteSetting.s3_region = 'us-west-1'
-    end
-
-    let(:store) { FileStore::S3Store.new }
-    let(:client) { Aws::S3::Client.new(stub_responses: true) }
-    let(:resource) { Aws::S3::Resource.new(client: client) }
-    let(:s3_bucket) { resource.bucket("s3-upload-bucket") }
-    let(:s3_helper) { store.instance_variable_get(:@s3_helper) }
-
-    before do
-      SiteSetting.s3_region = 'us-west-1'
-    end
   end
 
   context 'uploading to s3' do
-    include_context "s3 helpers"
-
-    let(:s3_object) { stub }
     let(:etag) { "etag" }
 
     describe "#store_upload" do
@@ -159,8 +143,6 @@ describe FileStore::S3Store do
   end
 
   context 'copying files in S3' do
-    include_context "s3 helpers"
-
     describe '#copy_file' do
       it "copies the from in S3 with the right paths" do
         s3_helper.expects(:s3_bucket).returns(s3_bucket)
@@ -186,8 +168,6 @@ describe FileStore::S3Store do
   end
 
   context 'removal from s3' do
-    include_context "s3 helpers"
-
     describe "#remove_upload" do
       it "removes the file from s3 with the right paths" do
         store.expects(:get_depth_for).with(upload.id).returns(0)
@@ -243,8 +223,6 @@ describe FileStore::S3Store do
     end
 
     describe "#remove_optimized_image" do
-      fab!(:optimized_image) { Fabricate(:optimized_image, upload: upload) }
-
       let(:image_path) do
         FileStore::BaseStore.new.get_path_for_optimized_image(optimized_image)
       end
@@ -303,23 +281,22 @@ describe FileStore::S3Store do
   end
 
   describe ".has_been_uploaded?" do
-
     it "doesn't crash for invalid URLs" do
       expect(store.has_been_uploaded?("https://site.discourse.com/#bad#6")).to eq(false)
     end
 
     it "doesn't crash if URL contains non-ascii characters" do
-      expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/漢1337.png")).to eq(true)
+      expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/漢1337.png")).to eq(true)
       expect(store.has_been_uploaded?("//s3-upload-bucket.s3.amazonaws.com/漢1337.png")).to eq(false)
     end
 
     it "identifies S3 uploads" do
-      expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/1337.png")).to eq(true)
+      expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/1337.png")).to eq(true)
     end
 
     it "does not match other s3 urls" do
       expect(store.has_been_uploaded?("//s3-upload-bucket.s3.amazonaws.com/1337.png")).to eq(false)
-      expect(store.has_been_uploaded?("//s3-upload-bucket.s3-us-east-1.amazonaws.com/1337.png")).to eq(false)
+      expect(store.has_been_uploaded?("//s3-upload-bucket.s3-us-west-1.amazonaws.com/1337.png")).to eq(false)
       expect(store.has_been_uploaded?("//s3.amazonaws.com/s3-upload-bucket/1337.png")).to eq(false)
       expect(store.has_been_uploaded?("//s4_upload_bucket.s3.amazonaws.com/1337.png")).to eq(false)
     end
@@ -328,7 +305,7 @@ describe FileStore::S3Store do
 
   describe ".absolute_base_url" do
     it "returns a lowercase schemaless absolute url" do
-      expect(store.absolute_base_url).to eq("//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com")
+      expect(store.absolute_base_url).to eq("//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com")
     end
 
     it "uses the proper endpoint" do
@@ -353,12 +330,10 @@ describe FileStore::S3Store do
   end
 
   describe ".purge_tombstone" do
-
     it "updates tombstone lifecycle" do
       s3_helper.expects(:update_tombstone_lifecycle)
       store.purge_tombstone(1.day)
     end
-
   end
 
   describe ".path_for" do
@@ -380,9 +355,6 @@ describe FileStore::S3Store do
   end
 
   context 'update ACL' do
-    include_context "s3 helpers"
-    let(:s3_object) { stub }
-
     before do
       SiteSetting.authorized_extensions = "pdf|png"
     end
@@ -411,7 +383,6 @@ describe FileStore::S3Store do
   end
 
   describe '.cdn_url' do
-
     it 'supports subfolder' do
       SiteSetting.s3_upload_bucket = 's3-upload-bucket/livechat'
       SiteSetting.s3_cdn_url = 'https://rainbow.com'
@@ -420,25 +391,19 @@ describe FileStore::S3Store do
       # subfolder should not leak into uploads
       set_subfolder "/community"
 
-      url = "//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/livechat/original/gif.png"
+      url = "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/livechat/original/gif.png"
 
       expect(store.cdn_url(url)).to eq("https://rainbow.com/original/gif.png")
     end
   end
 
   describe ".download_url" do
-    include_context "s3 helpers"
-    let(:s3_object) { stub }
-
     it "returns correct short URL with dl=1 param" do
       expect(store.download_url(upload)).to eq("#{upload.short_path}?dl=1")
     end
   end
 
   describe ".url_for" do
-    include_context "s3 helpers"
-    let(:s3_object) { stub }
-
     it "returns signed URL with content disposition when requesting to download image" do
       s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
       s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object)
@@ -454,9 +419,6 @@ describe FileStore::S3Store do
   end
 
   describe ".signed_url_for_path" do
-    include_context "s3 helpers"
-    let(:s3_object) { stub }
-
     it "returns signed URL for a given path" do
       s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
       s3_bucket.expects(:object).with("special/optimized/file.png").returns(s3_object)
@@ -469,5 +431,4 @@ describe FileStore::S3Store do
       expect(store.signed_url_for_path("special/optimized/file.png")).not_to eq(upload.url)
     end
   end
-
 end

spec/components/post_creator_spec.rb

@@ -1591,20 +1591,10 @@ describe PostCreator do
     fab!(:public_topic) { Fabricate(:topic) }
 
     before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
       SiteSetting.authorized_extensions = "png|jpg|gif|mp4"
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-      SiteSetting.s3_access_key_id = "some key"
-      SiteSetting.s3_secret_access_key = "some secret key"
-      SiteSetting.s3_region = "us-east-1"
       SiteSetting.secure_media = true
-
+      stub_upload(image_upload)
-      stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-
-      stub_request(
-        :put,
-        "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image_upload.sha1}.#{image_upload.extension}?acl"
-      )
     end
 
     it "links post uploads" do

spec/components/pretty_text_spec.rb

@@ -902,11 +902,8 @@ describe PrettyText do
 
     describe "#strip_secure_media" do
       before do
-        SiteSetting.s3_upload_bucket = "some-bucket-on-s3"
+        setup_s3
-        SiteSetting.s3_access_key_id = "s3-access-key-id"
-        SiteSetting.s3_secret_access_key = "s3-secret-access-key"
         SiteSetting.s3_cdn_url = "https://s3.cdn.com"
-        SiteSetting.enable_s3_uploads = true
         SiteSetting.secure_media = true
         SiteSetting.login_required = true
       end

spec/components/s3_helper_spec.rb

@@ -6,11 +6,8 @@ require "rails_helper"
 describe "S3Helper" do
   let(:client) { Aws::S3::Client.new(stub_responses: true) }
 
-  before(:each) do
+  before do
-    SiteSetting.enable_s3_uploads = true
+    setup_s3
-    SiteSetting.s3_access_key_id = "abc"
-    SiteSetting.s3_secret_access_key = "def"
-    SiteSetting.s3_region = "us-east-1"
 
     @lifecycle = <<~XML
       <?xml version="1.0" encoding="UTF-8"?>
@@ -41,10 +38,10 @@ describe "S3Helper" do
     stub_request(:get, "http://169.254.169.254/latest/meta-data/iam/security-credentials/").
       to_return(status: 404, body: "", headers: {})
 
-    stub_request(:get, "https://bob.s3.amazonaws.com/?lifecycle").
+    stub_request(:get, "https://bob.s3.#{SiteSetting.s3_region}.amazonaws.com/?lifecycle").
       to_return(status: 200, body: @lifecycle, headers: {})
 
-    stub_request(:put, "https://bob.s3.amazonaws.com/?lifecycle").
+    stub_request(:put, "https://bob.s3.#{SiteSetting.s3_region}.amazonaws.com/?lifecycle").
       with do |req|
 
       hash = Hash.from_xml(req.body.to_s)

spec/components/s3_inventory_spec.rb

@@ -12,9 +12,7 @@ describe "S3Inventory" do
   let(:csv_filename) { "#{Rails.root}/spec/fixtures/csv/s3_inventory.csv" }
 
   before do
-    SiteSetting.enable_s3_uploads = true
+    setup_s3
-    SiteSetting.s3_access_key_id = "abc"
-    SiteSetting.s3_secret_access_key = "def"
     SiteSetting.enable_s3_inventory = true
 
     client.stub_responses(:list_objects, -> (context) {

spec/components/stylesheet/importer_spec.rb

@@ -23,13 +23,12 @@ describe Stylesheet::Importer do
   end
 
   it "applies S3 CDN to background category images" do
+    setup_s3
     SiteSetting.s3_use_iam_profile = true
     SiteSetting.s3_upload_bucket = 'test'
     SiteSetting.s3_region = 'ap-southeast-2'
     SiteSetting.s3_cdn_url = "https://s3.cdn"
 
-    SiteSetting.enable_s3_uploads = true
-
     background = Fabricate(:upload_s3)
     category = Fabricate(:category, uploaded_background: background)
 

spec/components/svg_sprite/svg_sprite_spec.rb

@@ -164,11 +164,7 @@ describe SvgSprite do
     let(:upload_s3) { Fabricate(:upload_s3) }
 
     before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_upload_bucket = "s3bucket"
-      SiteSetting.s3_access_key_id = "s3_access_key_id"
-      SiteSetting.s3_secret_access_key = "s3_secret_access_key"
-
       stub_request(:get, upload_s3.url).to_return(status: 200, body: "Hello world")
     end
 

spec/components/url_helper_spec.rb

@@ -156,16 +156,15 @@ describe UrlHelper do
   end
 
   describe "#cook_url" do
-    let(:url) { "//s3bucket.s3.dualstack.us-east-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg" }
+    let(:url) { "//s3bucket.s3.dualstack.us-west-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg" }
 
     before do
-      FileStore::S3Store.any_instance.stubs(:has_been_uploaded?).returns(true)
+      setup_s3
-      Rails.configuration.action_controller.asset_host = "https://test.some-cdn.com/dev"
-      SiteSetting.enable_s3_uploads = true
       SiteSetting.s3_upload_bucket = "s3bucket"
-      SiteSetting.s3_access_key_id = "s3_access_key_id"
-      SiteSetting.s3_secret_access_key = "s3_secret_access_key"
       SiteSetting.login_required = true
+      Rails.configuration.action_controller.asset_host = "https://test.some-cdn.com/dev"
+
+      FileStore::S3Store.any_instance.stubs(:has_been_uploaded?).returns(true)
     end
 
     def cooked
@@ -187,7 +186,7 @@ describe UrlHelper do
 
       it "returns the local_cdn_url" do
         expect(cooked).to eq(
-          "//s3bucket.s3.dualstack.us-east-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg"
+          "//s3bucket.s3.dualstack.us-west-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg"
         )
       end
     end

spec/helpers/user_notifications_helper_spec.rb

@@ -163,16 +163,13 @@ describe UserNotificationsHelper do
       let(:upload) { Fabricate(:upload_s3, sha1: "somesha1") }
 
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "some key"
-        SiteSetting.s3_secret_access_key = "some secret key"
         SiteSetting.logo = upload
       end
 
       it 'should return the right URL' do
         expect(helper.logo_url).to eq(
-          "http://s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/original/1X/somesha1.png"
+          "http://s3-upload-bucket.s3.dualstack.#{SiteSetting.s3_region}.amazonaws.com/original/1X/somesha1.png"
         )
       end
 
@@ -181,7 +178,7 @@ describe UserNotificationsHelper do
           GlobalSetting.stubs(:cdn_url).returns('https://some.cdn.com/cluster')
 
           expect(helper.logo_url).to eq(
-            "http://s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/original/1X/somesha1.png"
+            "http://s3-upload-bucket.s3.dualstack.#{SiteSetting.s3_region}.amazonaws.com/original/1X/somesha1.png"
           )
         end
       end

spec/jobs/correct_missing_dualstack_urls_spec.rb

@@ -3,33 +3,30 @@
 require 'rails_helper'
 
 describe Jobs::CorrectMissingDualstackUrls do
-
   it 'corrects the urls' do
-
+    setup_s3
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+    SiteSetting.s3_region = "us-east-1"
-    SiteSetting.s3_access_key_id = "s3-access-key-id"
+    SiteSetting.s3_upload_bucket = "custom-bucket"
-    SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-    SiteSetting.enable_s3_uploads = true
 
     # we will only correct for our base_url, random urls will be left alone
-    expect(Discourse.store.absolute_base_url).to eq('//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com')
+    expect(Discourse.store.absolute_base_url).to eq('//custom-bucket.s3.dualstack.us-east-1.amazonaws.com')
 
     current_upload = Upload.create!(
-      url: '//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png',
+      url: '//custom-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png',
       original_filename: 'a.png',
       filesize: 100,
       user_id: -1,
     )
 
     bad_upload = Upload.create!(
-      url: '//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png',
+      url: '//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png',
       original_filename: 'a.png',
       filesize: 100,
       user_id: -1,
     )
 
     current_optimized = OptimizedImage.create!(
-      url: '//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png',
+      url: '//custom-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png',
       filesize: 100,
       upload_id: current_upload.id,
       width: 100,
@@ -39,7 +36,7 @@ describe Jobs::CorrectMissingDualstackUrls do
     )
 
     bad_optimized = OptimizedImage.create!(
-      url: '//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png',
+      url: '//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png',
       filesize: 100,
       upload_id: current_upload.id,
       width: 110,
@@ -51,15 +48,15 @@ describe Jobs::CorrectMissingDualstackUrls do
     Jobs::CorrectMissingDualstackUrls.new.execute_onceoff(nil)
 
     bad_upload.reload
-    expect(bad_upload.url).to eq('//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png')
+    expect(bad_upload.url).to eq('//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png')
 
     current_upload.reload
-    expect(current_upload.url).to eq('//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png')
+    expect(current_upload.url).to eq('//custom-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png')
 
     bad_optimized.reload
-    expect(bad_optimized.url).to eq('//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png')
+    expect(bad_optimized.url).to eq('//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png')
 
     current_optimized.reload
-    expect(current_optimized.url).to eq('//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png')
+    expect(current_optimized.url).to eq('//custom-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png')
   end
 end

spec/jobs/ensure_s3_uploads_existence_spec.rb

@@ -5,9 +5,7 @@ require 'rails_helper'
 RSpec.describe Jobs::EnsureS3UploadsExistence do
   context "S3 inventory enabled" do
     before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_access_key_id = "abc"
-      SiteSetting.s3_secret_access_key = "def"
       SiteSetting.enable_s3_inventory = true
     end
 

spec/jobs/pull_hotlinked_images_spec.rb

@@ -186,7 +186,9 @@ describe Jobs::PullHotlinkedImages do
 
     context "when secure media enabled for an upload that has already been downloaded and exists" do
       it "doesnt redownload the secure upload" do
-        enable_secure_media
+        setup_s3
+        SiteSetting.secure_media = true
+
         upload = Fabricate(:secure_upload_s3, secure: true)
         stub_s3(upload)
         url = Upload.secure_media_url_from_upload_url(upload.url)
@@ -199,7 +201,9 @@ describe Jobs::PullHotlinkedImages do
 
       context "when the upload original_sha1 is missing" do
         it "redownloads the upload" do
-          enable_secure_media
+          setup_s3
+          SiteSetting.secure_media = true
+
           upload = Fabricate(:upload_s3, secure: true)
           stub_s3(upload)
           Upload.stubs(:signed_url_from_secure_media_url).returns(upload.url)
@@ -218,7 +222,9 @@ describe Jobs::PullHotlinkedImages do
 
       context "when the upload access_control_post is different to the current post" do
         it "redownloads the upload" do
-          enable_secure_media
+          setup_s3
+          SiteSetting.secure_media = true
+
           upload = Fabricate(:secure_upload_s3, secure: true)
           stub_s3(upload)
           Upload.stubs(:signed_url_from_secure_media_url).returns(upload.url)
@@ -395,7 +401,9 @@ describe Jobs::PullHotlinkedImages do
 
       context "when secure media enabled" do
         it 'should return false for secure-media-upload url' do
-          enable_secure_media
+          setup_s3
+          SiteSetting.secure_media = true
+
           upload = Fabricate(:upload_s3, secure: true)
           stub_s3(upload)
           url = Upload.secure_media_url_from_upload_url(upload.url)
@@ -415,12 +423,9 @@ describe Jobs::PullHotlinkedImages do
     end
 
     it "returns false for emoji when app and S3 CDNs configured" do
-      set_cdn_url "https://mydomain.cdn/test"
+      setup_s3
-      SiteSetting.s3_upload_bucket = "some-bucket-on-s3"
-      SiteSetting.s3_access_key_id = "s3-access-key-id"
-      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
       SiteSetting.s3_cdn_url = "https://s3.cdn.com"
-      SiteSetting.enable_s3_uploads = true
+      set_cdn_url "https://mydomain.cdn/test"
 
       src = UrlHelper.cook_url(Emoji.url_for("testemoji.png"))
       expect(subject.should_download_image?(src)).to eq(false)
@@ -503,22 +508,8 @@ describe Jobs::PullHotlinkedImages do
     end
   end
 
-  def enable_secure_media
-    SiteSetting.enable_s3_uploads = true
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "some key"
-    SiteSetting.s3_secret_access_key = "some secrets3_region key"
-    SiteSetting.secure_media = true
-  end
-
   def stub_s3(upload)
-    stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
+    stub_upload(upload)
-
-    stub_request(
-      :put,
-      "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl"
-    )
     stub_request(:get, "https:" + upload.url).to_return(status: 200, body: file_from_fixtures("smallest.png"))
-    # stub_request(:get, /#{SiteSetting.s3_upload_bucket}\.s3\.amazonaws\.com/)
   end
 end

spec/jobs/update_s3_inventory_spec.rb

@@ -5,9 +5,8 @@ require "file_store/s3_store"
 
 describe Jobs::UpdateS3Inventory do
   before do
-    SiteSetting.enable_s3_uploads = true
+    setup_s3
-    SiteSetting.s3_access_key_id = "abc"
+    SiteSetting.s3_upload_bucket = "special-bucket"
-    SiteSetting.s3_secret_access_key = "def"
     SiteSetting.enable_s3_inventory = true
 
     store = FileStore::S3Store.new
@@ -21,17 +20,17 @@ describe Jobs::UpdateS3Inventory do
     path = File.join(S3Inventory::INVENTORY_PREFIX, S3Inventory::INVENTORY_VERSION)
 
     @client.expects(:put_bucket_policy).with(
-      bucket: "bucket",
+      bucket: "special-bucket",
-      policy: %Q|{"Version":"2012-10-17","Statement":[{"Sid":"InventoryAndAnalyticsPolicy","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Action":["s3:PutObject"],"Resource":["arn:aws:s3:::bucket/#{path}/*"],"Condition":{"ArnLike":{"aws:SourceArn":"arn:aws:s3:::bucket"},"StringEquals":{"s3:x-amz-acl":"bucket-owner-full-control"}}}]}|
+      policy: %Q|{"Version":"2012-10-17","Statement":[{"Sid":"InventoryAndAnalyticsPolicy","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Action":["s3:PutObject"],"Resource":["arn:aws:s3:::special-bucket/#{path}/*"],"Condition":{"ArnLike":{"aws:SourceArn":"arn:aws:s3:::special-bucket"},"StringEquals":{"s3:x-amz-acl":"bucket-owner-full-control"}}}]}|
     )
     @client.expects(:put_bucket_inventory_configuration)
     @client.expects(:put_bucket_inventory_configuration).with(
-      bucket: "bucket",
+      bucket: "special-bucket",
       id: id,
       inventory_configuration: {
         destination: {
           s3_bucket_destination: {
-            bucket: "arn:aws:s3:::bucket",
+            bucket: "arn:aws:s3:::special-bucket",
             prefix: path,
             format: "CSV"
           }

spec/jobs/vacate_legacy_prefix_backups_spec.rb

@@ -18,9 +18,7 @@ describe Jobs::VacateLegacyPrefixBackups, type: :multisite do
   end
 
   before do
-    SiteSetting.enable_s3_uploads = true
+    setup_s3
-    SiteSetting.s3_access_key_id = "abc"
-    SiteSetting.s3_secret_access_key = "def"
     SiteSetting.s3_backup_bucket = bucket_name
     SiteSetting.backup_location = BackupLocationSiteSetting::S3
   end

spec/lib/backup_restore/uploads_restorer_spec.rb

@@ -369,10 +369,7 @@ describe BackupRestore::UploadsRestorer do
 
     context "currently stored on S3" do
       before do
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+        setup_s3
-        SiteSetting.s3_access_key_id = "s3-access-key-id"
-        SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-        SiteSetting.enable_s3_uploads = true
       end
 
       let!(:store_class) { FileStore::S3Store }

spec/lib/shrink_uploaded_image_spec.rb

@@ -72,15 +72,8 @@ describe ShrinkUploadedImage do
     let(:upload) { Fabricate(:s3_image_upload, width: 200, height: 200) }
 
     before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_access_key_id = "fakeid7974664"
+      stub_s3_store
-      SiteSetting.s3_secret_access_key = "fakesecretid7974664"
-
-      store = FileStore::S3Store.new
-      s3_helper = store.instance_variable_get(:@s3_helper)
-      client = Aws::S3::Client.new(stub_responses: true)
-      s3_helper.stubs(:s3_client).returns(client)
-      Discourse.stubs(:store).returns(store)
     end
 
     it "resizes the image" do

spec/lib/topic_upload_security_manager_spec.rb

@@ -30,7 +30,12 @@ describe TopicUploadSecurityManager do
       let(:category) { Fabricate(:private_category, group: group) }
 
       context "when secure media is enabled" do
-        before { enable_secure_media }
+        before do
+          setup_s3
+          SiteSetting.secure_media = true
+
+          [upload, upload2, upload3].each { |upl| stub_upload(upl) }
+        end
 
         it "does not change any upload statuses or update ACLs or rebake" do
           expect_upload_status_not_to_change
@@ -57,7 +62,12 @@ describe TopicUploadSecurityManager do
       let(:topic) { Fabricate(:private_message_topic, category: category, user: user) }
 
       context "when secure media is enabled" do
-        before { enable_secure_media }
+        before do
+          setup_s3
+          SiteSetting.secure_media = true
+
+          [upload, upload2, upload3].each { |upl| stub_upload(upl) }
+        end
 
         it "does not change any upload statuses or update ACLs or rebake" do
           expect_upload_status_not_to_change
@@ -82,7 +92,12 @@ describe TopicUploadSecurityManager do
 
     context "when the topic is public" do
       context "when secure media is enabled" do
-        before { enable_secure_media }
+        before do
+          setup_s3
+          SiteSetting.secure_media = true
+
+          [upload, upload2, upload3].each { |upl| stub_upload(upl) }
+        end
 
         context "when login required is enabled" do
           before do
@@ -111,7 +126,10 @@ describe TopicUploadSecurityManager do
       let!(:upload3) { Fabricate(:upload) }
 
       before do
-        enable_secure_media
+        setup_s3
+        SiteSetting.secure_media = true
+
+        [upload, upload2, upload3].each { |upl| stub_upload(upl) }
       end
 
       context "when this is the first post the upload has appeared in" do
@@ -155,24 +173,6 @@ describe TopicUploadSecurityManager do
     end
   end
 
-  def enable_secure_media
-    SiteSetting.enable_s3_uploads = true
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "some key"
-    SiteSetting.s3_secret_access_key = "some secrets3_region key"
-    SiteSetting.secure_media = true
-
-    stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-
-    # because the ACLs will be changing...
-    [upload, upload2, upload3].each do |upl|
-      stub_request(
-        :put,
-        "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upl.sha1}.#{upl.extension}?acl"
-      )
-    end
-  end
-
   def expect_upload_status_not_to_change
     Post.any_instance.expects(:rebake!).never
     subject.run

spec/lib/upload_creator_spec.rb

@@ -193,7 +193,9 @@ RSpec.describe UploadCreator do
       let(:opts) { { type: "composer" } }
 
       before do
-        enable_s3_uploads
+        setup_s3
+        stub_s3_store
+
         SiteSetting.secure_media = true
         SiteSetting.authorized_extensions = 'pdf|svg|jpg'
       end
@@ -221,7 +223,8 @@ RSpec.describe UploadCreator do
       let(:opts) { { type: "composer" } }
 
       before do
-        enable_s3_uploads
+        setup_s3
+        stub_s3_store
       end
 
       it 'should store the file and return etag' do
@@ -280,7 +283,9 @@ RSpec.describe UploadCreator do
 
       context "when SiteSetting.secure_media is enabled" do
         before do
-          enable_s3_uploads
+          setup_s3
+          stub_s3_store
+
           SiteSetting.secure_media = true
         end
 
@@ -298,7 +303,9 @@ RSpec.describe UploadCreator do
 
       context "when SiteSetting.secure_media enabled" do
         before do
-          enable_s3_uploads
+          setup_s3
+          stub_s3_store
+
           SiteSetting.secure_media = true
         end
 
@@ -434,18 +441,4 @@ RSpec.describe UploadCreator do
       end
     end
   end
-
-  def enable_s3_uploads
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "s3-access-key-id"
-    SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-    SiteSetting.s3_region = 'us-west-1'
-    SiteSetting.enable_s3_uploads = true
-
-    store = FileStore::S3Store.new
-    s3_helper = store.instance_variable_get(:@s3_helper)
-    client = Aws::S3::Client.new(stub_responses: true)
-    s3_helper.stubs(:s3_client).returns(client)
-    Discourse.stubs(:store).returns(store)
-  end
 end

spec/lib/upload_security_spec.rb

@@ -12,10 +12,7 @@ RSpec.describe UploadSecurity do
 
   context "when secure media is enabled" do
     before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-      SiteSetting.s3_access_key_id = "some key"
-      SiteSetting.s3_secret_access_key = "some secrets3_region key"
       SiteSetting.secure_media = true
     end
 

spec/models/optimized_image_spec.rb

@@ -293,11 +293,7 @@ describe OptimizedImage do
 
     describe "external store" do
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "some key"
-        SiteSetting.s3_secret_access_key = "some secret key"
-        SiteSetting.s3_region = "us-east-1"
       end
 
       context "when we have a bad file returned" do
@@ -318,8 +314,7 @@ describe OptimizedImage do
           before do
             stub_request(:head, "http://#{s3_upload.url}").to_return(status: 200)
             stub_request(:get, "http://#{s3_upload.url}").to_return(status: 200, body: file_from_fixtures("logo.png"))
-            stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
+            stub_request(:put, "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com#{optimized_path}")
-            stub_request(:put, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com#{optimized_path}")
               .to_return(status: 200, headers: { "ETag" => "someetag" })
           end
 
@@ -330,14 +325,14 @@ describe OptimizedImage do
             expect(oi.extension).to eq(".png")
             expect(oi.width).to eq(100)
             expect(oi.height).to eq(200)
-            expect(oi.url).to eq("//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com#{optimized_path}")
+            expect(oi.url).to eq("//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com#{optimized_path}")
             expect(oi.filesize).to be > 0
 
             oi.filesize = nil
 
             stub_request(
               :get,
-              "http://#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com#{optimized_path}"
+              "http://#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com#{optimized_path}"
             ).to_return(status: 200, body: file_from_fixtures("resized.png"))
 
             expect(oi.filesize).to be > 0

spec/models/post_spec.rb

@@ -164,7 +164,11 @@ describe Post do
     end
 
     context "when secure media is enabled" do
-      before { enable_secure_media_and_s3 }
+      before do
+        setup_s3
+        SiteSetting.authorized_extensions = "pdf|png|jpg|csv"
+        SiteSetting.secure_media = true
+      end
 
       context "if login_required" do
         before { SiteSetting.login_required = true }
@@ -1426,7 +1430,11 @@ describe Post do
       end
 
       context "when secure media is enabled" do
-        before { enable_secure_media_and_s3 }
+        before do
+          setup_s3
+        SiteSetting.authorized_extensions = "pdf|png|jpg|csv"
+        SiteSetting.secure_media = true
+        end
 
         it "sets the access_control_post_id on uploads in the post that don't already have the value set" do
           other_post = Fabricate(:post)
@@ -1464,19 +1472,14 @@ describe Post do
       end
 
       before do
-        enable_secure_media_and_s3
+        setup_s3
+        SiteSetting.authorized_extensions = "pdf|png|jpg|csv"
+        SiteSetting.secure_media = true
+
         attachment_upload.update!(original_filename: "hello.csv")
 
-        stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
+        stub_upload(attachment_upload)
-
+        stub_upload(image_upload)
-        stub_request(
-          :put,
-          "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{attachment_upload.sha1}.#{attachment_upload.extension}?acl"
-        )
-        stub_request(
-          :put,
-          "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image_upload.sha1}.#{image_upload.extension}?acl"
-        )
       end
 
       it "marks image and attachment uploads as secure in PMs when secure_media is ON" do
@@ -1643,7 +1646,10 @@ describe Post do
     end
 
     it "correctly identifies secure uploads" do
-      enable_secure_media_and_s3
+      setup_s3
+      SiteSetting.authorized_extensions = "pdf|png|jpg|csv"
+      SiteSetting.secure_media = true
+
       upload1 = Fabricate(:upload_s3, secure: true)
       upload2 = Fabricate(:upload_s3, secure: true)
 
@@ -1690,11 +1696,7 @@ describe Post do
     end
 
     it "should skip external urls with upload url in query string" do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-      SiteSetting.s3_access_key_id = "some key"
-      SiteSetting.s3_secret_access_key = "some secret key"
-      SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com"
 
       urls = []
       upload = Fabricate(:upload_s3)
@@ -1703,13 +1705,4 @@ describe Post do
       expect(urls).to be_empty
     end
   end
-
-  def enable_secure_media_and_s3
-    SiteSetting.authorized_extensions = "pdf|png|jpg|csv"
-    SiteSetting.enable_s3_uploads = true
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "some key"
-    SiteSetting.s3_secret_access_key = "some secret key"
-    SiteSetting.secure_media = true
-  end
 end

spec/models/topic_link_click_spec.rb

@@ -166,10 +166,8 @@ describe TopicLinkClick do
         context "s3 cdns" do
 
           it "works with s3 urls" do
+            setup_s3
             SiteSetting.s3_cdn_url = "https://discourse-s3-cdn.global.ssl.fastly.net"
-            SiteSetting.s3_access_key_id = 'X'
-            SiteSetting.s3_secret_access_key = 'X'
-            SiteSetting.enable_s3_uploads = true
 
             post = Fabricate(:post, topic: @topic, raw: "[test](//test.localhost/uploads/default/my-test-link)")
             TopicLink.extract_from(post)

spec/models/upload_spec.rb

@@ -179,10 +179,7 @@ describe Upload do
       let(:path) { upload.url.sub(SiteSetting.Upload.s3_base_url, '') }
 
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "some key"
-        SiteSetting.s3_secret_access_key = "some secret key"
       end
 
       it "should return the right upload when using base url (not CDN) for s3" do
@@ -407,22 +404,12 @@ describe Upload do
   end
 
   def enable_secure_media
-    SiteSetting.enable_s3_uploads = true
+    setup_s3
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "some key"
-    SiteSetting.s3_secret_access_key = "some secrets3_region key"
     SiteSetting.secure_media = true
-
+    stub_upload(upload)
-    stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-
-    stub_request(
-      :put,
-      "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl"
-    )
   end
 
   context '.destroy' do
-
     it "can correctly clear information when destroying an upload" do
       upload = Fabricate(:upload)
       user = Fabricate(:user)

spec/multisite/post_spec.rb

@@ -8,13 +8,11 @@ RSpec.describe 'Multisite Post', type: :multisite do
     let(:upload2) { Fabricate(:upload_s3) }
     let(:upload3) { Fabricate(:upload_s3) }
 
-    it "correctly identifies all upload urls" do
+    before do
-      SiteSetting.enable_s3_uploads = true
+      setup_s3
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+    end
-      SiteSetting.s3_access_key_id = "some key"
-      SiteSetting.s3_secret_access_key = "some secret key"
-      SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com"
 
+    it "correctly identifies all upload urls" do
       upload3.url.sub!(RailsMultisite::ConnectionManagement.current_db, "secondsite")
       upload3.save!
 

spec/multisite/s3_store_spec.rb

@@ -15,10 +15,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
 
   context 'uploading to s3' do
     before(:each) do
-      SiteSetting.s3_upload_bucket = "some-really-cool-bucket"
+      setup_s3
-      SiteSetting.s3_access_key_id = "s3-access-key-id"
-      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-      SiteSetting.enable_s3_uploads = true
     end
 
     describe "#store_upload" do
@@ -79,7 +76,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
         test_multisite_connection('default') do
           upload = build_upload
           expect(store.store_upload(uploaded_file, upload)).to eq(
-            "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
+            "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
           )
           expect(upload.etag).to eq("ETag")
         end
@@ -88,7 +85,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
           upload_path = Discourse.store.upload_path
           upload = build_upload
           expect(store.store_upload(uploaded_file, upload)).to eq(
-            "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
+            "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png"
           )
           expect(upload.etag).to eq("ETag")
         end
@@ -98,11 +95,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
 
   context 'removal from s3' do
     before do
-      SiteSetting.s3_region = 'us-west-1'
+      setup_s3
-      SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-      SiteSetting.s3_access_key_id = "s3-access-key-id"
-      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-      SiteSetting.enable_s3_uploads = true
     end
 
     describe "#remove_upload" do
@@ -180,10 +173,8 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
     let(:s3_object) { stub }
 
     before(:each) do
+      setup_s3
       SiteSetting.s3_upload_bucket = "some-really-cool-bucket"
-      SiteSetting.s3_access_key_id = "s3-access-key-id"
-      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-      SiteSetting.enable_s3_uploads = true
       SiteSetting.authorized_extensions = "pdf|png|jpg|gif"
     end
 
@@ -202,7 +193,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
           s3_object.expects(:presigned_url).with(:get, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS)
 
           expect(store.store_upload(uploaded_file, upload)).to eq(
-            "//some-really-cool-bucket.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.pdf"
+            "//some-really-cool-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.pdf"
           )
 
           expect(store.url_for(upload)).not_to eq(upload.url)
@@ -270,18 +261,11 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
 
   describe "#has_been_uploaded?" do
     before do
-      SiteSetting.s3_region = 'us-west-1'
+      setup_s3
       SiteSetting.s3_upload_bucket = "s3-upload-bucket/test"
-      SiteSetting.s3_access_key_id = "s3-access-key-id"
-      SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-      SiteSetting.enable_s3_uploads = true
     end
 
     let(:store) { FileStore::S3Store.new }
-    let(:client) { Aws::S3::Client.new(stub_responses: true) }
-    let(:resource) { Aws::S3::Resource.new(client: client) }
-    let(:s3_bucket) { resource.bucket(SiteSetting.s3_upload_bucket) }
-    let(:s3_helper) { store.s3_helper }
 
     it "returns false for blank urls and bad urls" do
       expect(store.has_been_uploaded?("")).to eq(false)

spec/rails_helper.rb

@@ -173,6 +173,7 @@ RSpec.configure do |config|
   config.include WebauthnIntegrationHelpers
   config.include SiteSettingsHelpers
   config.include SidekiqHelpers
+  config.include UploadsHelpers
   config.mock_framework = :mocha
   config.order = 'random'
   config.infer_spec_type_from_file_location!

spec/requests/session_controller_spec.rb

@@ -1092,23 +1092,20 @@ RSpec.describe SessionController do
 
       it 'handles non local content correctly' do
         SiteSetting.avatar_sizes = "100|49"
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_access_key_id = "XXX"
-        SiteSetting.s3_secret_access_key = "XXX"
-        SiteSetting.s3_upload_bucket = "test"
         SiteSetting.s3_cdn_url = "http://cdn.com"
 
-        stub_request(:any, /test.s3.dualstack.us-east-1.amazonaws.com/).to_return(status: 200, body: "", headers: { referer: "fgdfds" })
+        stub_request(:any, /s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/).to_return(status: 200, body: "", headers: { referer: "fgdfds" })
 
         @user.create_user_avatar!
-        upload = Fabricate(:upload, url: "//test.s3.dualstack.us-east-1.amazonaws.com/something")
+        upload = Fabricate(:upload, url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/something")
 
         Fabricate(:optimized_image,
           sha1: SecureRandom.hex << "A" * 8,
           upload: upload,
           width: 98,
           height: 98,
-          url: "//test.s3.amazonaws.com/something/else"
+          url: "//s3-upload-bucket.s3.amazonaws.com/something/else"
         )
 
         @user.update_columns(uploaded_avatar_id: upload.id)

spec/requests/static_controller_spec.rb

@@ -52,9 +52,7 @@ describe StaticController do
       end
 
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_access_key_id = 'X'
-        SiteSetting.s3_secret_access_key = 'X'
       end
 
       it 'can proxy a favicon correctly' do

spec/requests/uploads_controller_multisite_spec.rb

@@ -10,17 +10,15 @@ describe UploadsController do
       let(:upload) { Fabricate(:upload_s3) }
 
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_access_key_id = "fakeid7974664"
-        SiteSetting.s3_secret_access_key = "fakesecretid7974664"
       end
 
       context "when upload is secure and secure media enabled" do
         before do
           SiteSetting.secure_media = true
           upload.update(secure: true)
-          stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
         end
+
         context "when running on a multisite connection", type: :multisite do
           it "redirects to the signed_url_for_path with the multisite DB name in the url" do
             sign_in(user)

spec/requests/uploads_controller_spec.rb

@@ -240,9 +240,7 @@ describe UploadsController do
       fab!(:upload) { upload_file("small.pdf", "pdf") }
 
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_access_key_id = "fakeid7974664"
-        SiteSetting.s3_secret_access_key = "fakesecretid7974664"
       end
 
       it "returns 404 " do
@@ -383,9 +381,7 @@ describe UploadsController do
       let(:upload) { Fabricate(:upload_s3) }
 
       before do
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_access_key_id = "fakeid7974664"
-        SiteSetting.s3_secret_access_key = "fakesecretid7974664"
       end
 
       it "should redirect to the s3 URL" do
@@ -398,7 +394,6 @@ describe UploadsController do
         before do
           SiteSetting.secure_media = true
           upload.update(secure: true)
-          stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
         end
 
         it "redirects to the signed_url_for_path" do
@@ -448,22 +443,9 @@ describe UploadsController do
       let(:upload) { Fabricate(:upload_s3) }
       let(:secure_url) { upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") }
 
-      def sign_in_and_stub_head
-        sign_in(user)
-        stub_head
-      end
-
-      def stub_head
-        stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-      end
-
       before do
+        setup_s3
         SiteSetting.authorized_extensions = "*"
-        SiteSetting.enable_s3_uploads = true
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "fakeid7974664"
-        SiteSetting.s3_secret_access_key = "fakesecretid7974664"
-        SiteSetting.s3_region = "us-east-1"
         SiteSetting.secure_media = true
       end
 
@@ -473,8 +455,7 @@ describe UploadsController do
       end
 
       it "should return signed url for legitimate request" do
-        sign_in_and_stub_head
+        sign_in(user)
-
         get secure_url
 
         expect(response.status).to eq(302)
@@ -494,7 +475,7 @@ describe UploadsController do
 
       context "when the upload cannot be found from the URL" do
         it "returns a 404" do
-          sign_in_and_stub_head
+          sign_in(user)
           upload.update(sha1: 'test')
 
           get secure_url
@@ -507,13 +488,13 @@ describe UploadsController do
         let!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) }
 
         before do
-          sign_in_and_stub_head
+          sign_in(user)
           upload.update(access_control_post_id: post.id)
         end
 
         context "when the user has access to the post via guardian" do
           it "should return signed url for legitimate request" do
-            sign_in_and_stub_head
+            sign_in(user)
             get secure_url
             expect(response.status).to eq(302)
             expect(response.redirect_url).to match("Amz-Expires")
@@ -526,7 +507,7 @@ describe UploadsController do
           end
 
           it "returns a 403" do
-            sign_in_and_stub_head
+            sign_in(user)
             get secure_url
             expect(response.status).to eq(403)
           end
@@ -538,7 +519,7 @@ describe UploadsController do
           upload.update(original_filename: 'test.pdf')
         end
         it "redirects to the signed_url_for_path" do
-          sign_in_and_stub_head
+          sign_in(user)
           get secure_url
           expect(response.status).to eq(302)
           expect(response.redirect_url).to match("Amz-Expires")
@@ -554,7 +535,7 @@ describe UploadsController do
           end
 
           it "returns a 403" do
-            sign_in_and_stub_head
+            sign_in(user)
             get secure_url
             expect(response.status).to eq(403)
           end
@@ -564,8 +545,8 @@ describe UploadsController do
           before do
             SiteSetting.prevent_anons_from_downloading_files = true
           end
+
           it "returns a 404" do
-            stub_head
             delete "/session/#{user.username}.json"
             get secure_url
             expect(response.status).to eq(404)
@@ -586,8 +567,6 @@ describe UploadsController do
           it "should redirect to the regular show route" do
             secure_url = upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads")
             sign_in(user)
-            stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-
             get secure_url
 
             expect(response.status).to eq(302)
@@ -603,8 +582,6 @@ describe UploadsController do
           it "should redirect to the presigned URL still otherwise we will get a 403" do
             secure_url = upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads")
             sign_in(user)
-            stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-
             get secure_url
 
             expect(response.status).to eq(302)
@@ -632,11 +609,8 @@ describe UploadsController do
       let(:upload) { Fabricate(:upload_s3, secure: true) }
 
       before do
+        setup_s3
         SiteSetting.authorized_extensions = "pdf|png"
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "s3-access-key-id"
-        SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-        SiteSetting.enable_s3_uploads = true
         SiteSetting.secure_media = true
       end
 

spec/requests/user_avatars_controller_spec.rb

@@ -69,25 +69,22 @@ describe UserAvatarsController do
     end
 
     it 'handles non local content correctly' do
+      setup_s3
       SiteSetting.avatar_sizes = "100|49"
-      SiteSetting.enable_s3_uploads = true
-      SiteSetting.s3_access_key_id = "XXX"
-      SiteSetting.s3_secret_access_key = "XXX"
-      SiteSetting.s3_upload_bucket = "test"
-      SiteSetting.s3_cdn_url = "http://cdn.com"
       SiteSetting.unicode_usernames = true
+      SiteSetting.s3_cdn_url = "http://cdn.com"
 
-      stub_request(:get, "http://cdn.com/something/else").to_return(body: 'image')
+      stub_request(:get, "#{SiteSetting.s3_cdn_url}/something/else").to_return(body: 'image')
       set_cdn_url("http://awesome.com/boom")
 
-      upload = Fabricate(:upload, url: "//test.s3.dualstack.us-east-1.amazonaws.com/something")
+      upload = Fabricate(:upload, url: "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/something")
 
       optimized_image = Fabricate(:optimized_image,
         sha1: SecureRandom.hex << "A" * 8,
         upload: upload,
         width: 98,
         height: 98,
-        url: "//test.s3.dualstack.us-east-1.amazonaws.com/something/else",
+        url: "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/something/else",
         version: OptimizedImage::VERSION
       )
 

spec/serializers/upload_serializer_spec.rb

@@ -29,7 +29,7 @@ RSpec.describe UploadSerializer do
 
     context "when secure media is enabled" do
       before do
-        enable_s3_uploads
+        setup_s3
         SiteSetting.secure_media = true
       end
 
@@ -39,18 +39,4 @@ RSpec.describe UploadSerializer do
       end
     end
   end
-
-  def enable_s3_uploads
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "s3-access-key-id"
-    SiteSetting.s3_secret_access_key = "s3-secret-access-key"
-    SiteSetting.s3_region = 'us-west-1'
-    SiteSetting.enable_s3_uploads = true
-
-    store = FileStore::S3Store.new
-    s3_helper = store.instance_variable_get(:@s3_helper)
-    client = Aws::S3::Client.new(stub_responses: true)
-    s3_helper.stubs(:s3_client).returns(client)
-    Discourse.stubs(:store).returns(store)
-  end
 end

spec/services/inline_uploads_spec.rb

@@ -654,10 +654,7 @@ RSpec.describe InlineUploads do
 
       before do
         upload3
-        SiteSetting.enable_s3_uploads = true
+        setup_s3
-        SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-        SiteSetting.s3_access_key_id = "some key"
-        SiteSetting.s3_secret_access_key = "some secret key"
         SiteSetting.s3_cdn_url = "https://s3.cdn.com"
       end
 

spec/support/uploads_helpers.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module UploadsHelpers
+  def setup_s3
+    SiteSetting.enable_s3_uploads = true
+
+    SiteSetting.s3_region = 'us-west-1'
+    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
+
+    SiteSetting.s3_access_key_id = "some key"
+    SiteSetting.s3_secret_access_key = "some secrets3_region key"
+
+    stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com/")
+  end
+
+  def stub_upload(upload)
+    url = "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl"
+    stub_request(:put, url)
+  end
+
+  def stub_s3_store
+    store = FileStore::S3Store.new
+    client = Aws::S3::Client.new(stub_responses: true)
+    store.s3_helper.stubs(:s3_client).returns(client)
+    Discourse.stubs(:store).returns(store)
+  end
+end

spec/tasks/uploads_spec.rb

@@ -50,7 +50,8 @@ RSpec.describe "tasks/uploads" do
 
     context "when store is external" do
       before do
-        enable_s3_uploads(uploads)
+        setup_s3
+        uploads.each { |upload| stub_upload(upload) }
       end
 
       context "when secure media is enabled" do
@@ -125,7 +126,11 @@ RSpec.describe "tasks/uploads" do
             upload_to_mark_not_secure = Fabricate(:upload_s3, secure: true)
             post_for_upload = Fabricate(:post)
             PostUpload.create(post: post_for_upload, upload: upload_to_mark_not_secure)
-            enable_s3_uploads(uploads.concat([upload_to_mark_not_secure]))
+
+            setup_s3
+            uploads.each { |upload| stub_upload(upload) }
+            stub_upload(upload_to_mark_not_secure)
+
             invoke_task
             expect(upload_to_mark_not_secure.reload.secure).to eq(false)
           end
@@ -154,7 +159,10 @@ RSpec.describe "tasks/uploads" do
     before do
       global_setting :s3_bucket, 'file-uploads/folder'
       global_setting :s3_region, 'us-east-1'
-      enable_s3_uploads(uploads)
+
+      setup_s3
+      uploads.each { |upload| stub_upload(upload) }
+
       upload1.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload1.base62_sha1}.png"
       upload1.save!
       upload2.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload2.base62_sha1}.png"
@@ -206,7 +214,9 @@ RSpec.describe "tasks/uploads" do
     before do
       global_setting :s3_bucket, 'file-uploads/folder'
       global_setting :s3_region, 'us-east-1'
-      enable_s3_uploads(uploads)
+      setup_s3
+      uploads.each { |upload| stub_upload(upload) }
+
       upload1.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload1.base62_sha1}.png"
       upload1.save!
       upload2.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload2.base62_sha1}.png"
@@ -251,7 +261,9 @@ RSpec.describe "tasks/uploads" do
     end
 
     before do
-      enable_s3_uploads(uploads)
+      setup_s3
+      uploads.each { |upload| stub_upload(upload) }
+
       SiteSetting.secure_media = true
       PostUpload.create(post: post1, upload: upload1)
       PostUpload.create(post: post1, upload: upload2)
@@ -300,20 +312,4 @@ RSpec.describe "tasks/uploads" do
       invoke_task
     end
   end
-
-  def enable_s3_uploads(uploads)
-    SiteSetting.enable_s3_uploads = true
-    SiteSetting.s3_upload_bucket = "s3-upload-bucket"
-    SiteSetting.s3_access_key_id = "some key"
-    SiteSetting.s3_secret_access_key = "some secrets3_region key"
-
-    stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/")
-
-    uploads.each do |upload|
-      stub_request(
-        :put,
-        "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl"
-      )
-    end
-  end
 end