set region from s3 endpoint if present #2234
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2234 +/- ##
=========================================
+ Coverage 8.87% 8.94% +0.06%
=========================================
Files 93 93
Lines 9033 9048 +15
=========================================
+ Hits 802 809 +7
- Misses 8104 8110 +6
- Partials 127 129 +2
Continue to review full report at Codecov.
|
cmd/client-s3.go
Outdated
| // getRegionFromHost - parse region from host name if present. | ||
| func getRegionFromHost(host string) (region string) { | ||
| region = "" | ||
| if isGoogle(host) { |
There was a problem hiding this comment.
You can use s3utils.IsGoogleCloudEndpoint() - perhaps combine below if and Google in a switch case and return?
cmd/mb-main.go
Outdated
| @@ -29,7 +29,7 @@ var ( | |||
| mbFlags = []cli.Flag{ | |||
| cli.StringFlag{ | |||
| Name: "region", | |||
| Value: "us-east-1", | |||
| Value: "", | |||
There was a problem hiding this comment.
Leave this as is.. since we don't expect users to set this value.. if its empty its used as us-east-1 in MakeBucket(). Keeping it explicit serves as a way to document..
There was a problem hiding this comment.
Using empty string deliberately here - minio-go overrides region specified in client constructor only if location string is empty. If mb call passes us-east-1 as region, all mc mb calls without region will fail with header mismatch and confusing error message that the location should not be us-east-1.
There was a problem hiding this comment.
@harshavardhana, overriding region inside client-s3 so that default is not changed
cmd/mb-main.go
Outdated
| @@ -124,7 +122,11 @@ func mainMakeBucket(ctx *cli.Context) error { | |||
| cErr = exitStatus(globalErrorExitStatus) | |||
| continue | |||
| } | |||
|
|
|||
| // Save region. | |||
There was a problem hiding this comment.
This comment can be better.. save region doesn't mean anything here.
poornas
force-pushed
the
issue/2224
branch
2 times, most recently
from
e53aa04
to
bcb68bb
Compare
Fixes #2224
Currently list bucket fails on mc if the endpoint entered in mc config has a region specific endpoint such as s3.eu-west-1.amazonaws.com. This PR parses the region from endpoint and instantiates the client with region if specified.