-
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
@uppy/companion: switch from aws-sdk v2 to @aws-sdk/client-s3 (v3) #4285
Conversation
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.
Reading through https://github.com/aws/aws-sdk-js-v3/blob/main/UPGRADING.md to check all the versions we are using:
signatureVersion
: is deprecated. We should probably remove it.endpoint
: no changes documented (except that HTTP may no longer be supported out of the box)region
: no change.useAccelerateEndpoint
: no changess3BucketEndpoint
: changed tobucketEndpoint
. We should update this.credentials
: no change (except I suppose the one you made in this PR).
Could you take care of the only two problematic items here?
5056483
to
a67c4a9
Compare
yup! pushed an update addressing those |
unfortunately it's not that easy because s3 v3 client API is completely different from v2. in v3 they use the new Command class format, and many things have been pulled out into separate packages. so this PR breaks all s3 code in https://github.com/transloadit/uppy/blob/main/packages/@uppy/companion/src/server/controllers/s3.js and https://github.com/transloadit/uppy/blob/main/packages/@uppy/companion/src/server/Uploader.js there's about 8 different s3 api calls that need to be refactored. anyone up for grabs?:) Note: aws sdk v3 has a lot of quirks and undocumented breaking changes, so we will want to test this refactor thoroughly. For example, we need to:
|
ah bummer. i'd be game but honestly my bigger issue is that we dont use AWS at all and end up with the massive aws-sdk dep. perhaps it would be easier/better to modularize that into an peer/optional dep in some way. is there an appetite for that? |
The team will discuss whether we can somehow modularize the upload destinations. Out of curiosity, why is it a problem to drag in unneeded dependencies? Security concerns? |
aws-sdk is one of, if not the, largest folders under node_modules for us. this has effects on devenv, as well as CI/CD. for example it accounts for 20% of size when bundling our api server. |
a67c4a9
to
91fe6b3
Compare
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is network access?This module accesses the network. Packages should remove all network access that isn't functionally unnecessary. Consumers should audit network access to ensure legitimate use. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎
Footnotes |
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 3.2.0 | @uppy/status-bar | 3.2.0 | | @uppy/aws-s3-multipart | 3.4.0 | @uppy/transloadit | 3.1.6 | | @uppy/companion | 4.5.1 | @uppy/tus | 3.1.1 | | @uppy/core | 3.2.1 | @uppy/url | 3.3.2 | | @uppy/dashboard | 3.4.1 | @uppy/utils | 5.4.0 | | @uppy/golden-retriever | 3.0.4 | @uppy/xhr-upload | 3.3.0 | | @uppy/locales | 3.2.2 | uppy | 3.10.0 | | @uppy/provider-views | 3.3.1 | | | - @uppy/aws-s3-multipart: fix the chunk size calculation (Antoine du Hamel / #4508) - @uppy/aws-s3: add `shouldUseMultipart` option (Antoine du Hamel / #4299) - @uppy/companion: switch from aws-sdk v2 to @aws-sdk/* (v3) (Scott Bessler / #4285) - @uppy/companion,@uppy/core,@uppy/dashboard,@uppy/golden-retriever,@uppy/status-bar,@uppy/utils: Migrate all lodash' per-method-packages usage to lodash. (LinusMain / #4274) - @uppy/core: Don't set late (throttled) progress event on a file that is 100% complete (Artur Paikin / #4507) - @uppy/companion: revert randomness from file names (Mikael Finstad / #4509) - @uppy/companion: Custom provider fixes (Mikael Finstad / #4498) - @uppy/transloadit: ensure `fields` is not nullish when there no uploaded files (Antoine du Hamel / #4487) - @uppy/aws-s3-multipart,@uppy/aws-s3,@uppy/tus,@uppy/utils,@uppy/xhr-upload: When file is removed (or all are canceled), controller.abort queued requests (Artur Paikin / #4504) - @uppy/provider-views: Fix range selection not resetting and computing correctly (Terence C / #4415) - meta: disallow use of `.only` in tests (Antoine du Hamel / #4494) - @uppy/companion: fix 500 when file name contains non-ASCII chars (Antoine du Hamel / #4493) - @uppy/locales: update `fr_FR.js` (Samuel De Backer / #4499) - @uppy/aws-s3-multipart,@uppy/tus,@uppy/xhr-upload: Don't close socket while upload is still in progress (Artur Paikin / #4479) - meta: bump `luxon` from 1.28.0 to 1.28.1 (dependabot[bot] / #4497) - @uppy/utils: rename `EventTracker` -> `EventManager` (Stephen Wooten / #4481) - meta: bump cookiejar from 2.1.3 to 2.1.4 (dependabot[bot] / #4496) - meta: make `pre-commit` use `corepack yarn` instead of `npm run` (Antoine du Hamel / #4495) - meta: bump ua-parser-js from 0.7.31 to 0.7.35 (dependabot[bot] / #4474) - meta: bump @sideway/formula from 3.0.0 to 3.0.1 (dependabot[bot] / #4473) - meta: bump http-cache-semantics from 4.1.0 to 4.1.1 (dependabot[bot] / #4472) - @uppy/companion: Use filename from content-disposition instead of relying on url, with fallback (Artur Paikin / #4489) - meta: bump `babel`, `esbuild`, and `vite` (dependabot[bot] / #4485) - @uppy/dashboard: include the old state when setting new (Artur Paikin / #4490) - @uppy/companion: fix companion implicitpath (Mikael Finstad / #4484) - @uppy/companion: fix undefined protocol and example page (Mikael Finstad / #4483) - meta: upgrade Cypress 12.9.0 -> 12.14.0 (Antoine du Hamel / #4491) - @uppy/core: remove `state` getter from types (Antoine du Hamel / #4477) - examples/php-xhr: Added filename sanitation and file size check before saving (neuronet77 / #4432) - examples/php-xhr: update PHP dependencies (dependabot[bot]) - @uppy/xhr-upload: add support for arrays in metadata (Vasiliy Matyushin / #4431) - @uppy/status-bar: Filtered ETA (stduhpf / #4458) - @uppy/aws-s3-multipart: fix `getUploadParameters` option (Antoine du Hamel / #4465)
* main: (61 commits) Release: uppy@3.10.0 (#4511) @uppy/aws-s3-multipart: fix the chunk size calculation (#4508) @uppy/aws-s3: add `shouldUseMultipart` option (#4299) @uppy/companion: switch from aws-sdk v2 to @aws-sdk/* (v3) (#4285) Migrate all lodash' per-method-packages usage to lodash. (#4274) @uppy/core: Don't set late (throttled) progress event on a file that is 100% complete (#4507) @uppy/companion: revert randomness from file names (#4509) Custom provider fixes (#4498) @uppy/transloadit: ensure `fields` is not nullish when there no uploaded files (#4487) When file is removed (or all are canceled), controller.abort queued requests (#4504) @uppy/provider-views: Fix range selection not resetting and computing correctly (#4415) meta: disallow use of `.only` in tests (#4494) @uppy/companion: fix 500 when file name contains non-ASCII chars (#4493) @uppy/locales: update `fr_FR.js` (#4499) Don't close socket while upload is still in progress (#4479) meta: bump `luxon` from 1.28.0 to 1.28.1 (#4497) @uppy/utils: rename `EventTracker` -> `EventManager` (#4481) meta: bump cookiejar from 2.1.3 to 2.1.4 (#4496) meta: make `pre-commit` use `corepack yarn` instead of `npm run` (#4495) meta: bump ua-parser-js from 0.7.31 to 0.7.35 (#4474) ...
* main: Release: uppy@3.10.0 (#4511) @uppy/aws-s3-multipart: fix the chunk size calculation (#4508) @uppy/aws-s3: add `shouldUseMultipart` option (#4299) @uppy/companion: switch from aws-sdk v2 to @aws-sdk/* (v3) (#4285) Migrate all lodash' per-method-packages usage to lodash. (#4274) @uppy/core: Don't set late (throttled) progress event on a file that is 100% complete (#4507) @uppy/companion: revert randomness from file names (#4509) Custom provider fixes (#4498) @uppy/transloadit: ensure `fields` is not nullish when there no uploaded files (#4487) When file is removed (or all are canceled), controller.abort queued requests (#4504) @uppy/provider-views: Fix range selection not resetting and computing correctly (#4415) meta: disallow use of `.only` in tests (#4494)
Upgrade to the more space-effecient aws-sdk v3 s3 client (@aws-sdk/client-s3).
This knocks down the included node_modules from the 76.4mb aws-sdk to a more modest 14.8mb. And its not just disk space and file count, but it significantly lowers the amount that gets bundled when esbuilding something that uses @uppy/companion.