Skip to content
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

S3 upload and region error #515

Closed
cmillet2127 opened this issue Mar 26, 2024 · 5 comments · Fixed by #565
Closed

S3 upload and region error #515

cmillet2127 opened this issue Mar 26, 2024 · 5 comments · Fixed by #565

Comments

@cmillet2127
Copy link

cmillet2127 commented Mar 26, 2024

Hello,
I made some tests with S3 upload. It works very well with a bucket stored in us-east-1, but doesn't work for other regions, in my case eu-west-3.
I try to analyze the code, and it seems it failed in MinIO call in util/storage.ts line 90:

    await this.client.fPutObject(
      this.bucketName,
      this.objectPrefix + targetFilename,
      srcFilename,
    );

where the region can optionnally be provided, otherwise us-east-1 is used by default.

The region can be extracted from the STORE_ENDPOINT_URL. example : https://<bucket>.s3.<region>.amazonaws.com/<bucket>/

Best regards

@ikreymer
Copy link
Member

We are planning to switch to AWS CLI SDK, I think this will be addressed with that change. See: #479

@tw4l
Copy link
Member

tw4l commented Apr 15, 2024

Hi @cmillet2127, based on a discussion in the minio-js repo I think the crawler should work as-is and minio-js will autodiscover the bucket if you use s3.amazonaws.com as the STORE_ENDPOINT_URL. Want to try that out and let us know if it works?

@tw4l tw4l closed this as completed Apr 15, 2024
@github-project-automation github-project-automation bot moved this from Triage to Done! in Webrecorder Projects Apr 15, 2024
@tw4l tw4l reopened this Apr 15, 2024
@github-project-automation github-project-automation bot moved this from Done! to Todo in Webrecorder Projects Apr 15, 2024
ikreymer pushed a commit that referenced this issue Apr 17, 2024
This should address the issue of connecting to buckets stored outside
us-east-1
(#515) while
the switch from Minio client to AWS SDK is being worked on
(#479)

Co-authored-by: Mattia <[email protected]>
@RomanSmolka
Copy link

The change in #543 has broken it for me in the 'eu-central-1' region. Now I'm getting an error:

S3Error: The authorization header is malformed; the region 'auto' is wrong; expecting 'eu-central-1'

After going back to 1.1.0 Beta 5, it's working fine. This is my env configuration:

environment: [
  { name: 'STORE_ENDPOINT_URL', value: 'https://s3.amazonaws.com/' + process.env.S3_BUCKET },
  { name: 'STORE_PATH', value: '/' },
  { name: 'STORE_FILENAME', value: item.s3_key },
  { name: 'STORE_ACCESS_KEY', value: process.env.S3_ACCESS_KEY },
  { name: 'STORE_SECRET_KEY', value: process.env.S3_SECRET },
  { name: 'CRAWL_ID', value: crawlId },
  { name: 'WEBHOOK_URL', value: process.env.WEBRECORDER_HOOK }
]

Perhaps just adding a STORE_REGION variable would be a better solution? @ikreymer @tw4l

@ikreymer
Copy link
Member

ikreymer commented May 7, 2024

@RomanSmolka agreed, that seems like a safer solution. This probably needs testing with a lot more S3 providers, which we just don't have resources to do. Making this customizable seems like the most flexible/safe option. @cmillet2127 @mguella would that work for you also?

@cmillet2127
Copy link
Author

@RomanSmolka agreed, that seems like a safer solution. This probably needs testing with a lot more S3 providers, which we just don't have resources to do. Making this customizable seems like the most flexible/safe option. @cmillet2127 @mguella would that work for you also?

Yes, it seems good solution.
Even if we would be able to use the generic url to access Amazon S3 url, it could make additional charge for the routing between AWS region. So it would be defintiely better to be able to specify the region.

ikreymer added a commit that referenced this issue May 7, 2024
…-east-1 by default

fixes #515
dependency: update get-folder-size in storage.js to latest, fix type errors
ikreymer added a commit that referenced this issue May 12, 2024
defaults to us-east-1 for minio compatibility
fixes #515
@github-project-automation github-project-automation bot moved this from Todo to Done! in Webrecorder Projects May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done!
Development

Successfully merging a pull request may close this issue.

4 participants