-
Notifications
You must be signed in to change notification settings - Fork 180
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
First pass at enabling s3 path style config #317
base: master
Are you sure you want to change the base?
First pass at enabling s3 path style config #317
Conversation
This looks pretty good. Any reason to keep it draft? |
Only that I didn't have time to sufficiently test it. For me, improving my homelab to be able to handle virtual-host routing was ultimately preferable. I did deploy it a few times and it seemed to work, but I'm not convinced just yet. |
Once you move it to ready, I will kick off CI on it. Is there any way to add it to the integration tests? Those start a fakes3 server. Is there any way to get the tests to work? Or does it depend on DNS? I guess in theory we could add a DNS server that responds there, and have the http client use that DNS server? |
Hmm, so I haven't had a look at the available test suite here, but effectively this enables the legacy path-based routing for s3 buckets. For example: New way (virtual host routing): This PR enables the user to configure the underlying s3 client to use the path-based bucket routing. To answer your question of whether this would require a DNS server, that depends entirely upon what is faking S3. In the past, I've used Localstack to test path vs. virtual-host bucket routing config, but again I haven't had a chance to look at what's used here, so I cannot say whether the lack of DNS is blocking. I may have a chance to look at the available test suite this weekend sometime. |
What is the use case for when it is necessary? You can configure the s3 region and endpoint independently. When would we need it? |
When would I need what? Path style bucket routing? As mentioned it can be useful if you use a tool with an s3 compliant API that either does not support or you not want to bother with the infra required for virtual host routing. |
Yeah, I am trying to understand if it is not already covered? I guess if the target's OK, I get it. I will wait to hear what you say about being able to put it into the testing. |
Started working on this as I self-host Minio, and after updating mysql-backup I see you all switched to the aws-sdk-go-v2 lib which defaults to virtual-host style routing.
Didn't end up needing it as I instead opted to just set up wildcard dns resolution and certificates for my Minio instance, so the current impl works fine.
Opening the PR in case it may help somebody else and they want to finish it off (if it even needs further work).