-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add companion option for S3 ServerSideEncryption and update docs with missing options #5348
base: main
Are you sure you want to change the base?
Add companion option for S3 ServerSideEncryption and update docs with missing options #5348
Conversation
Diff output filesdiff --git a/packages/@uppy/companion/lib/server/controllers/s3.js b/packages/@uppy/companion/lib/server/controllers/s3.js
index 7526f30..f108410 100644
--- a/packages/@uppy/companion/lib/server/controllers/s3.js
+++ b/packages/@uppy/companion/lib/server/controllers/s3.js
@@ -120,6 +120,9 @@ module.exports = function s3(config) {
if (config.acl != null) {
params.ACL = config.acl;
}
+ if (config.serverSideEncryption != null) {
+ params.ServerSideEncryption = config.serverSideEncryption;
+ }
client.send(new CreateMultipartUploadCommand(params)).then((data) => {
res.json({
key: data.Key, |
@@ -563,6 +563,17 @@ app.use( | |||
When signing on the client, this function will only be called for multipart | |||
uploads. | |||
|
|||
##### `s3.acl` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The companion docs put the env var and JS option in the same header. This option already exists: https://uppy.io/docs/companion/#companion_aws_acl so you should add the JS var in front of it (see other options).
Or maybe the env var should be moved here?
##### `s3.serverSideEncryption` | ||
|
||
Specify the | ||
[Server-Side Encryption](https://docs.aws.amazon.com/AmazonS3/latest/dev/serv-side-encryption.html) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we maybe also link to https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-s3/Variable/ServerSideEncryption/ - so the user knows which values are valid?
also I think we need a corresponding env variable. maybe COMPANION_AWS_S3_SERVER_SIDE_ENCRYPTION
ping @kengoldfarb |
No description provided.