Fix for self hosted S3 self signed URLs #499
Conversation
…d when all containers are running in docker and we are using internal docker service names.
…docker network. Added console logs for s3 configs to make it easier to debug.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
longshuicy
left a comment
longshuicy
left a comment
There was a problem hiding this comment.
What would be the best way to test this PR? Should I build this branch locally into an image? I recall the self-hosted setup is linked via git submodules just wonder if there's any tricks.
| console.log('[s3Client] AWS_SECRET:', process.env.AWS_SECRET ? '***' + '*'.repeat(Math.min(process.env.AWS_SECRET.length - 4, 8)) + process.env.AWS_SECRET.slice(-4) : 'undefined') | ||
| console.log('[s3Client] LOCAL_MINIO:', process.env.LOCAL_MINIO) | ||
| console.log('[s3Client] MINIO_ENDPOINT:', process.env.MINIO_ENDPOINT) | ||
| console.log('[vyriadMinioClient] S3_BUCKET_NAME:', process.env.S3_BUCKET_NAME) |
There was a problem hiding this comment.
Should we remove those logs that prints out secret?
| console.log('[vyriadMinioClient] MINIO_ENDPOINT:', process.env.MINIO_ENDPOINT) | ||
| console.log('[vyriadMinioClient] MINIO_REGION:', process.env.MINIO_REGION) | ||
| console.log('[vyriadMinioClient] S3_BUCKET_NAME:', process.env.S3_BUCKET_NAME) | ||
|
|
There was a problem hiding this comment.
Same should we remove those console logs?
There was a problem hiding this comment.
I left that there because they might be useful for debugging, but when I last was paying attention to the logs I think some of them show up multiple times (instead of just at startup). If they just show up at startup it might behelpful to leave them there.
This fixes the S3 self signed URLs when running all containers in docker compose for self hosted.
Before the URLs were being signed with the internal docker service name
minioand then they would failed when being validated against localhost. Now we can provide aMINIO_PUBLIC_ENDPOINTin cases where the internal and external urls are different.I am honestly not sure if this is the best strategy, but it seems to work locally. Not tested on dev yet and it could use some more testing on localhost using the current dev environment where
MINIO_PUBLIC_ENDPOINTshould not be necessary.