mirror of
https://github.com/discourse/discourse.git
synced 2024-12-30 15:16:09 +08:00
cf42466dea
This commit adds some system specs to test uploads with direct to S3 single and multipart uploads via uppy. This is done with minio as a local S3 replacement. We are doing this to catch regressions when uppy dependencies need to be upgraded or we change uppy upload code, since before this there was no way to know outside manual testing whether these changes would cause regressions. Minio's server lifecycle and the installed binaries are managed by the https://github.com/discourse/minio_runner gem, though the binaries are already installed on the discourse_test image we run GitHub CI from. These tests will only run in CI unless you specifically use the CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars. For a history of experimentation here see https://github.com/discourse/discourse/pull/22381 Related PRs: * https://github.com/discourse/minio_runner/pull/1 * https://github.com/discourse/minio_runner/pull/2 * https://github.com/discourse/minio_runner/pull/3
447 lines
17 KiB
Ruby
447 lines
17 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
require "file_store/s3_store"
|
|
|
|
RSpec.describe "Multisite s3 uploads", type: :multisite do
|
|
let(:original_filename) { "smallest.png" }
|
|
let(:uploaded_file) { file_from_fixtures(original_filename) }
|
|
let(:upload_sha1) { Digest::SHA1.hexdigest(File.read(uploaded_file)) }
|
|
let(:upload_path) { Discourse.store.upload_path }
|
|
|
|
def build_upload(secure: false)
|
|
Fabricate.build(
|
|
:upload,
|
|
sha1: upload_sha1,
|
|
id: 1,
|
|
original_filename: original_filename,
|
|
secure: secure,
|
|
)
|
|
end
|
|
|
|
describe "uploading to s3" do
|
|
before(:each) { setup_s3 }
|
|
|
|
describe "#store_upload" do
|
|
let(:s3_client) { Aws::S3::Client.new(stub_responses: true) }
|
|
let(:s3_helper) { S3Helper.new(SiteSetting.s3_upload_bucket, "", client: s3_client) }
|
|
let(:store) { FileStore::S3Store.new(s3_helper) }
|
|
let(:upload_opts) do
|
|
{
|
|
acl: "public-read",
|
|
cache_control: "max-age=31556952, public, immutable",
|
|
content_type: "image/png",
|
|
}
|
|
end
|
|
|
|
it "does not provide a content_disposition for images" do
|
|
s3_helper
|
|
.expects(:upload)
|
|
.with(uploaded_file, kind_of(String), upload_opts)
|
|
.returns(%w[path etag])
|
|
upload = build_upload
|
|
store.store_upload(uploaded_file, upload)
|
|
end
|
|
|
|
context "when the file is a PDF" do
|
|
let(:original_filename) { "small.pdf" }
|
|
let(:uploaded_file) { file_from_fixtures("small.pdf", "pdf") }
|
|
|
|
it "adds an attachment content-disposition with the original filename" do
|
|
disp_opts = {
|
|
content_disposition:
|
|
"attachment; filename=\"#{original_filename}\"; filename*=UTF-8''#{original_filename}",
|
|
content_type: "application/pdf",
|
|
}
|
|
s3_helper
|
|
.expects(:upload)
|
|
.with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts))
|
|
.returns(%w[path etag])
|
|
upload = build_upload
|
|
store.store_upload(uploaded_file, upload)
|
|
end
|
|
end
|
|
|
|
context "when the file is a video" do
|
|
let(:original_filename) { "small.mp4" }
|
|
let(:uploaded_file) { file_from_fixtures("small.mp4", "media") }
|
|
|
|
it "adds an attachment content-disposition with the original filename" do
|
|
disp_opts = {
|
|
content_disposition:
|
|
"attachment; filename=\"#{original_filename}\"; filename*=UTF-8''#{original_filename}",
|
|
content_type: "application/mp4",
|
|
}
|
|
s3_helper
|
|
.expects(:upload)
|
|
.with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts))
|
|
.returns(%w[path etag])
|
|
upload = build_upload
|
|
store.store_upload(uploaded_file, upload)
|
|
end
|
|
end
|
|
|
|
context "when the file is audio" do
|
|
let(:original_filename) { "small.mp3" }
|
|
let(:uploaded_file) { file_from_fixtures("small.mp3", "media") }
|
|
|
|
it "adds an attachment content-disposition with the original filename" do
|
|
disp_opts = {
|
|
content_disposition:
|
|
"attachment; filename=\"#{original_filename}\"; filename*=UTF-8''#{original_filename}",
|
|
content_type: "audio/mpeg",
|
|
}
|
|
s3_helper
|
|
.expects(:upload)
|
|
.with(uploaded_file, kind_of(String), upload_opts.merge(disp_opts))
|
|
.returns(%w[path etag])
|
|
upload = build_upload
|
|
store.store_upload(uploaded_file, upload)
|
|
end
|
|
end
|
|
|
|
it "returns the correct url for default and second multisite db" 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-west-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png",
|
|
)
|
|
expect(upload.etag).to eq("ETag")
|
|
end
|
|
|
|
test_multisite_connection("second") 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-west-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png",
|
|
)
|
|
expect(upload.etag).to eq("ETag")
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "removal from s3" do
|
|
before { setup_s3 }
|
|
|
|
describe "#remove_upload" do
|
|
let(:store) { FileStore::S3Store.new }
|
|
|
|
let(:upload) { build_upload }
|
|
let(:upload_key) { "#{upload_path}/original/1X/#{upload.sha1}.png" }
|
|
|
|
def prepare_fake_s3
|
|
@fake_s3 = FakeS3.create
|
|
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
|
|
bucket.put_object(key: upload_key, size: upload.filesize, last_modified: upload.created_at)
|
|
bucket
|
|
end
|
|
|
|
it "removes the file from s3 on multisite" do
|
|
test_multisite_connection("default") do
|
|
upload.update!(
|
|
url:
|
|
"//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.png",
|
|
)
|
|
tombstone_key = "uploads/tombstone/default/original/1X/#{upload.sha1}.png"
|
|
bucket = prepare_fake_s3
|
|
|
|
expect(bucket.find_object(upload_key)).to be_present
|
|
expect(bucket.find_object(tombstone_key)).to be_nil
|
|
|
|
store.remove_upload(upload)
|
|
|
|
expect(bucket.find_object(upload_key)).to be_nil
|
|
expect(bucket.find_object(tombstone_key)).to be_present
|
|
end
|
|
end
|
|
|
|
it "removes the file from s3 on another multisite db" do
|
|
test_multisite_connection("second") do
|
|
upload.update!(
|
|
url:
|
|
"//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.png",
|
|
)
|
|
tombstone_key = "uploads/tombstone/second/original/1X/#{upload.sha1}.png"
|
|
bucket = prepare_fake_s3
|
|
|
|
expect(bucket.find_object(upload_key)).to be_present
|
|
expect(bucket.find_object(tombstone_key)).to be_nil
|
|
|
|
store.remove_upload(upload)
|
|
|
|
expect(bucket.find_object(upload_key)).to be_nil
|
|
expect(bucket.find_object(tombstone_key)).to be_present
|
|
end
|
|
end
|
|
|
|
describe "when s3_upload_bucket includes folders path" do
|
|
let(:upload_key) { "discourse-uploads/#{upload_path}/original/1X/#{upload.sha1}.png" }
|
|
|
|
before { SiteSetting.s3_upload_bucket = "s3-upload-bucket/discourse-uploads" }
|
|
|
|
it "removes the file from s3 on multisite" do
|
|
test_multisite_connection("default") do
|
|
upload.update!(
|
|
url:
|
|
"//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/discourse-uploads/#{upload_path}/original/1X/#{upload.sha1}.png",
|
|
)
|
|
tombstone_key =
|
|
"discourse-uploads/uploads/tombstone/default/original/1X/#{upload.sha1}.png"
|
|
bucket = prepare_fake_s3
|
|
|
|
expect(bucket.find_object(upload_key)).to be_present
|
|
expect(bucket.find_object(tombstone_key)).to be_nil
|
|
|
|
store.remove_upload(upload)
|
|
|
|
expect(bucket.find_object(upload_key)).to be_nil
|
|
expect(bucket.find_object(tombstone_key)).to be_present
|
|
end
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "secure uploads" do
|
|
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("some-really-cool-bucket") }
|
|
let(:s3_helper) { store.s3_helper }
|
|
let(:s3_object) { stub }
|
|
|
|
before(:each) do
|
|
setup_s3
|
|
SiteSetting.s3_upload_bucket = "some-really-cool-bucket"
|
|
SiteSetting.authorized_extensions = "pdf|png|jpg|gif"
|
|
end
|
|
|
|
before { s3_object.stubs(:put).returns(Aws::S3::Types::PutObjectOutput.new(etag: "etag")) }
|
|
|
|
describe "when secure attachments are enabled" do
|
|
it "returns signed URL with correct path" do
|
|
test_multisite_connection("default") do
|
|
upload =
|
|
Fabricate(:upload, original_filename: "small.pdf", extension: "pdf", secure: true)
|
|
path = Discourse.store.get_path_for_upload(upload)
|
|
|
|
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
|
|
s3_bucket.expects(:object).with("#{upload_path}/#{path}").returns(s3_object).at_least_once
|
|
s3_object.expects(:presigned_url).with(
|
|
:get,
|
|
{ expires_in: SiteSetting.s3_presigned_get_url_expires_after_seconds },
|
|
)
|
|
|
|
upload.url = store.store_upload(uploaded_file, upload)
|
|
expect(upload.url).to eq(
|
|
"//some-really-cool-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/#{path}",
|
|
)
|
|
expect(store.url_for(upload)).not_to eq(upload.url)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "when secure uploads are enabled" do
|
|
before do
|
|
SiteSetting.login_required = true
|
|
SiteSetting.secure_uploads = true
|
|
s3_helper.stubs(:s3_client).returns(client)
|
|
Discourse.stubs(:store).returns(store)
|
|
end
|
|
|
|
it "returns signed URL with correct path" do
|
|
test_multisite_connection("default") do
|
|
upload = Fabricate.build(:upload_s3, sha1: upload_sha1, id: 1)
|
|
|
|
signed_url = Discourse.store.signed_url_for_path(upload.url)
|
|
expect(signed_url).to match(/Amz-Expires/)
|
|
expect(signed_url).to match("#{upload_path}")
|
|
end
|
|
|
|
test_multisite_connection("second") do
|
|
upload_path = Discourse.store.upload_path
|
|
upload = Fabricate.build(:upload_s3, sha1: upload_sha1, id: 1)
|
|
|
|
signed_url = Discourse.store.signed_url_for_path(upload.url)
|
|
expect(signed_url).to match(/Amz-Expires/)
|
|
expect(signed_url).to match("#{upload_path}")
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#update_upload_ACL" do
|
|
it "updates correct file for default and second multisite db" do
|
|
test_multisite_connection("default") do
|
|
upload = build_upload(secure: true)
|
|
upload.update!(original_filename: "small.pdf", extension: "pdf")
|
|
|
|
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
|
|
expect_upload_acl_update(upload, upload_path)
|
|
|
|
expect(store.update_upload_ACL(upload)).to be_truthy
|
|
end
|
|
|
|
test_multisite_connection("second") do
|
|
upload_path = Discourse.store.upload_path
|
|
upload = build_upload(secure: true)
|
|
upload.update!(original_filename: "small.pdf", extension: "pdf")
|
|
|
|
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
|
|
expect_upload_acl_update(upload, upload_path)
|
|
|
|
expect(store.update_upload_ACL(upload)).to be_truthy
|
|
end
|
|
end
|
|
|
|
describe "optimized images" do
|
|
it "updates correct file for default and second multisite DB" do
|
|
test_multisite_connection("default") do
|
|
upload = build_upload(secure: true)
|
|
upload_path = Discourse.store.upload_path
|
|
optimized_image = Fabricate(:optimized_image, upload: upload)
|
|
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
|
|
expect_upload_acl_update(upload, upload_path)
|
|
expect_optimized_image_acl_update(optimized_image, upload_path)
|
|
|
|
expect(store.update_upload_ACL(upload)).to be_truthy
|
|
end
|
|
|
|
test_multisite_connection("second") do
|
|
upload = build_upload(secure: true)
|
|
upload_path = Discourse.store.upload_path
|
|
optimized_image = Fabricate(:optimized_image, upload: upload)
|
|
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
|
|
expect_upload_acl_update(upload, upload_path)
|
|
expect_optimized_image_acl_update(optimized_image, upload_path)
|
|
|
|
expect(store.update_upload_ACL(upload)).to be_truthy
|
|
end
|
|
end
|
|
end
|
|
|
|
def expect_upload_acl_update(upload, upload_path)
|
|
s3_bucket
|
|
.expects(:object)
|
|
.with("#{upload_path}/original/1X/#{upload.sha1}.#{upload.extension}")
|
|
.returns(s3_object)
|
|
s3_object.expects(:acl).returns(s3_object)
|
|
s3_object.expects(:put).with(acl: "private").returns(s3_object)
|
|
end
|
|
|
|
def expect_optimized_image_acl_update(optimized_image, upload_path)
|
|
path = Discourse.store.get_path_for_optimized_image(optimized_image)
|
|
s3_bucket.expects(:object).with("#{upload_path}/#{path}").returns(s3_object)
|
|
s3_object.expects(:acl).returns(s3_object)
|
|
s3_object.expects(:put).with(acl: "private").returns(s3_object)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#has_been_uploaded?" do
|
|
before do
|
|
setup_s3
|
|
SiteSetting.s3_upload_bucket = "s3-upload-bucket/test"
|
|
end
|
|
|
|
let(:store) { FileStore::S3Store.new }
|
|
|
|
it "returns false for blank urls and bad urls" do
|
|
expect(store.has_been_uploaded?("")).to eq(false)
|
|
expect(store.has_been_uploaded?("http://test@test.com:test/test.git")).to eq(false)
|
|
expect(store.has_been_uploaded?("http:///+test@test.com/test.git")).to eq(false)
|
|
end
|
|
|
|
it "returns true if the base hostname is the same for both urls" do
|
|
url =
|
|
"https://s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/test/original/2X/d/dd7964f5fd13e1103c5244ca30abe1936c0a4b88.png"
|
|
expect(store.has_been_uploaded?(url)).to eq(true)
|
|
end
|
|
|
|
it "returns false if the base hostname is the same for both urls BUT the bucket name is different in the path" do
|
|
bucket = "someotherbucket"
|
|
url =
|
|
"https://s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/#{bucket}/original/2X/d/dd7964f5fd13e1103c5244ca30abe1936c0a4b88.png"
|
|
expect(store.has_been_uploaded?(url)).to eq(false)
|
|
end
|
|
|
|
it "returns false if the hostnames do not match and the s3_cdn_url is blank" do
|
|
url =
|
|
"https://www.someotherhostname.com/test/original/2X/d/dd7964f5fd13e1103c5244ca30abe1936c0a4b88.png"
|
|
expect(store.has_been_uploaded?(url)).to eq(false)
|
|
end
|
|
|
|
it "returns true if the s3_cdn_url is present and matches the url hostname" do
|
|
SiteSetting.s3_cdn_url = "https://www.someotherhostname.com"
|
|
url =
|
|
"https://www.someotherhostname.com/test/original/2X/d/dd7964f5fd13e1103c5244ca30abe1936c0a4b88.png"
|
|
expect(store.has_been_uploaded?(url)).to eq(true)
|
|
end
|
|
|
|
it "returns false if the URI is an invalid mailto link" do
|
|
link = "mailto: roman;@test.com"
|
|
|
|
expect(store.has_been_uploaded?(link)).to eq(false)
|
|
end
|
|
end
|
|
|
|
describe "#signed_request_for_temporary_upload" do
|
|
before { setup_s3 }
|
|
|
|
let(:store) { FileStore::S3Store.new }
|
|
|
|
context "for a bucket with no folder path" do
|
|
before { SiteSetting.s3_upload_bucket = "s3-upload-bucket" }
|
|
|
|
it "returns a presigned url and headers with the correct params and the key for the temporary file" do
|
|
url, signed_headers = store.signed_request_for_temporary_upload("test.png")
|
|
key = store.s3_helper.path_from_url(url)
|
|
expect(signed_headers).to eq("x-amz-acl" => "private")
|
|
expect(url).to match(/Amz-Expires/)
|
|
expect(key).to match(
|
|
/temp\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/[a-zA-z0-9]{0,32}.png/,
|
|
)
|
|
end
|
|
|
|
it "presigned url headers contains the metadata when provided" do
|
|
url, signed_headers =
|
|
store.signed_request_for_temporary_upload(
|
|
"test.png",
|
|
metadata: {
|
|
"test-meta": "testing",
|
|
},
|
|
)
|
|
expect(signed_headers).to eq("x-amz-acl" => "private", "x-amz-meta-test-meta" => "testing")
|
|
expect(url).not_to include("&x-amz-meta-test-meta=testing")
|
|
end
|
|
end
|
|
|
|
context "for a bucket with a folder path" do
|
|
before { SiteSetting.s3_upload_bucket = "s3-upload-bucket/site" }
|
|
|
|
it "returns a presigned url with the correct params and the key for the temporary file" do
|
|
url, _signed_headers = store.signed_request_for_temporary_upload("test.png")
|
|
key = store.s3_helper.path_from_url(url)
|
|
expect(url).to match(/Amz-Expires/)
|
|
expect(key).to match(
|
|
/temp\/site\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/[a-zA-z0-9]{0,32}.png/,
|
|
)
|
|
end
|
|
end
|
|
|
|
context "for a multisite site" do
|
|
before { SiteSetting.s3_upload_bucket = "s3-upload-bucket/standard99" }
|
|
|
|
it "returns a presigned url with the correct params and the key for the temporary file" do
|
|
test_multisite_connection("second") do
|
|
url, _signed_headers = store.signed_request_for_temporary_upload("test.png")
|
|
key = store.s3_helper.path_from_url(url)
|
|
expect(url).to match(/Amz-Expires/)
|
|
expect(key).to match(
|
|
/temp\/standard99\/uploads\/second\/test_[0-9]\/[a-zA-z0-9]{0,32}\/[a-zA-z0-9]{0,32}.png/,
|
|
)
|
|
end
|
|
end
|
|
end
|
|
end
|
|
end
|