Minio S3 backend (#4) #7
Conversation
backends/s3/s3.go
Outdated
| @@ -48,8 +46,8 @@ type Options struct { | |||
| // CreateBucket tells us to try to create the bucket | |||
| CreateBucket bool `yaml:"create_bucket"` | |||
|
|
|||
| // EndpointURL can be set to something like "http://localhost:9000" when using Minio | |||
| // instead of AWS S3. | |||
| // EndpointURL can be set to something like "localhost:9000" when using Minio | |||
There was a problem hiding this comment.
Please don't change the configuration format if not strictly necessary. This seemingly innocent change would break all existing configurations.
There was a problem hiding this comment.
Tested this against a configuration I had used prior and was presented with the following error (thrown by minIO client):
"Endpoint url cannot have fully qualified paths"
Endpoint used: https://s3.eu-central-1.amazonaws.com/
See suggestion in the 'Secure' flag comment, perhaps that can be used to prevent/solve both.
There was a problem hiding this comment.
This error was the reason for this change in the first place, but indeed the configuration format should not have been changed.
There was a problem hiding this comment.
Retested; Error is no longer reproducible and my prior used configurations all seem to work. Nice to have the format of this config option stay the same 👍
backends/s3/s3.go
Outdated
| s3config.WithHTTPClient(hc)) | ||
| cfg := &minio.Options{ | ||
| Creds: credentials.NewStaticV4(opt.AccessKey, opt.SecretKey, ""), | ||
| //Secure: true, |
There was a problem hiding this comment.
I guess this needs to depend on if the URL starts with http or https?
There was a problem hiding this comment.
Seems so indeed, I tested against an AWS s3 bucket with the following endpoint variations:
- s3.eu-central-1.amazonaws.com:443/
- s3.eu-central-1.amazonaws.com:80/
- s3.eu-central-1.amazonaws.com/
Worked fine over 'no port' + port 80, but seemed to stop functioning with port 443 specified. Likely due to a protocol mismatch.
Perhaps enforce or allow endpoints in opt.EndpointURL to have protocols specified, then set the minIO Client's 'Secure' option based on the protocol identified in the opt.EndpointURL?
There was a problem hiding this comment.
I updated the file to read the URL's scheme to determine the value of Secure. I then pass the URL with https?:(//)? stripped out to Minio.
There was a problem hiding this comment.
Retested; The 'Secure' flag now indeed reflects the scheme of the configured Endpoint
Minio only accepts the endpoint URL's host value as its endoint URL. Providing the scheme along throws an error. Now the scheme and host are split from the endpoint URL in config file. Scheme is used to determine whether the client should use a secure connection.
There was a problem hiding this comment.
Looks good to me. Both findings encountered earlier have been resolved and prior configurations seem to work fine.
And on an extra positive side-note: An issue I had earlier with simpleblob (while it was using the AWS SDK) related to the AWS SDK retry budget when SSL certificate errors were encountered is now also resolved with this switch to the minIO Client 👍
Use Minio client for S3 instead of AWS SDK