Skip to content

[2.x] Fixes support for S3-Compatible Storage like Herd minio #190

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

pintend
Copy link
Contributor

@pintend pintend commented May 29, 2025

Currently if you use vapor.js for file uploads, during local development you need to use a live aws account. There is no way to use cloudflare r2, minio, etc. for local development.

I created a minio service in herd to use for local storage development and was having weird issues where i either couldn't upload with vapor.js or render a public url for already uploaded files which these two issues would basically be toggled depending on my env configuration


Reproduce: create a minio service in herd and copy the env

Create a bucket in the MinIO dashboard

AWS_BUCKET=herd-bucket
AWS_ACCESS_KEY_ID=herd
AWS_USE_PATH_STYLE_ENDPOINT=true
AWS_SECRET_ACCESS_KEY=secretkey
AWS_DEFAULT_REGION=us-east-1
AWS_URL=https://minio.herd.test/herd-bucket
AWS_ENDPOINT=https://minio.herd.test

Problem:

  • With minio AWS_URL must include the bucket name (e.g., https://minio.herd.test/herd-bucket) to make public URLs work.
  • However, when AWS_URL includes a path, that path is duplicated in the signed upload URL generated by Vapor

Examples:

AWS_BUCKET=herd-bucket
AWS_URL=https://minio.herd.test/herd-bucket
AWS_ENDPOINT=https://minio.herd.test

Yields:

https://minio.herd.test/herd-bucket/herd-bucket/herd-bucket/tmp/UUID?query

AWS_BUCKET=herd-bucket
AWS_URL=https://minio.herd.test/laravel
AWS_ENDPOINT=https://minio.herd.test

Yields:

https://minio.herd.test/laravel/laravel/herd-bucket/tmp/UUID?query

AWS_BUCKET=herd-bucket
#AWS_URL=https://minio.herd.test/laravel
AWS_ENDPOINT=https://minio.herd.test

Yields:

https://s3.amazonaws.com/herd-bucket/tmp/UUID?query

ignoring the AWS_ENDPOINT


Fix:

The issue originates from how the S3 client is constructed in Vapor Core. It does not fully respect the configured disk settings and misuses AWS_URL as both the url and SDK endpoint.

I've updated the controller to use AWS_ENDPOINT if it exists which seems to make vapor core work with herd minio now.

@pintend
Copy link
Contributor Author

pintend commented May 29, 2025

Is there a specific reason why vapor users are "forced" into using s3 specifically? If an app requires multiple disks which may be on different accounts or regions the current implementation doesn't support that. Additionally if an app wants to use a 3rd party object storage like Cloudflare R2 (which has "free" egress) that wont either work.

Why dont we use the s3 client from Storage? e.g. Storage::disk('s3')->getClient() Wouldn't that be created by the same ENV keys anyway? then we don't even need the checks for AWS_LAMBDA_FUNCTION_VERSION

@pintend pintend changed the title Support S3 compatible storage - e.g. herd minio [2.x] Fixes support for S3-Compatible Storage like Herd minio May 29, 2025
@jbrooksuk jbrooksuk requested a review from themsaid May 29, 2025 20:41
@taylorotwell taylorotwell merged commit ded3cbc into laravel:2.0 Jun 2, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants