Allow setting s3 forcepathstyle without regionendpoint #4291
Conversation
2ad47ee to
34084bc
Compare
|
To retain the current behaviour, we could keep a default of |
|
Any chance getting this in, maybe in v3.0.0? @milosgajdos @thaJeztah @Jamstah |
Can you point me somewhere where I can read about this? I understand the reasons, just never came across AWS discouraging users from doing this |
|
That's stated in the distribution s3 driver docs here https://github.com/distribution/distribution/blob/main/docs/content/storage-drivers/s3.md?plain=1#L44
|
|
Anyone? |
milosgajdos
left a comment
There was a problem hiding this comment.
Is there a reason why we've changed the bool flag/option into a string? It's not entirely clear to me why we do that
|
This was to allow the value to be "unset", so we fall back to the aws-sdk default behavior (like we do now if we don't set the regionendpoint). |
milosgajdos
left a comment
There was a problem hiding this comment.
I see no reason why forcepathstyle can't remain a boolean and has to be changed to a string 🤷♂️
We will also need to update the docs explaining that the value takes effect regardless of the region settings.
Currently, the `forcepathstyle` parameter for the s3 storage driver is considered only if the `regionendpoint` parameter is set. Since setting a region endpoint explicitly is discouraged with AWS s3, it is not clear how to enforce path style URLs with AWS s3. This also means, that the default value (true) only applies if a region endpoint is configured. This change makes sure we always forward the `forcepathstyle` parameter to the aws-sdk if present in the config. This is a breaking change where a `regionendpoint` is configured but no explicit `forcepathstyle` value is set. Signed-off-by: Benjamin Schanzel <benjamin.schanzel@bmw.de>
34084bc to
8654a0e
Compare
|
Thanks for the feedback. I updated the change accordingly, always setting |
milosgajdos
left a comment
There was a problem hiding this comment.
LGTM, PTAL @Jamstah @thaJeztah
What are the drawbacks of retaining the current |
|
That would probably work but would also mean we'd be using pathstyle URLs for aws s3 per default, too. The awssdk uses virutal-hosted-style URLs per default and pathstyle urls are ought to be deprecated with aws s3 at some point (cf. https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html). |
1. update distribution dep to support S3-compatible services (ref: distribution/distribution#4291) 2. update the github.com/gorilla/mux: the issue was solved. Signed-off-by: wuhuizuo <wuhuizuo@126.com>
1. update distribution dep to support S3-compatible services (ref: distribution/distribution#4291) 2. update the github.com/gorilla/mux: the issue was solved. Signed-off-by: wuhuizuo <wuhuizuo@126.com>
1. update distribution dep to support S3-compatible services (ref: distribution/distribution#4291) 2. update the github.com/gorilla/mux: the issue was solved. Signed-off-by: wuhuizuo <wuhuizuo@126.com>
1. update distribution dep to support S3-compatible services (ref: distribution/distribution#4291) 2. update the github.com/gorilla/mux: the issue was solved. Signed-off-by: wuhuizuo <wuhuizuo@126.com>
1. update distribution dep to support S3-compatible services (ref: distribution/distribution#4291) 2. update the github.com/gorilla/mux: the issue was solved. Signed-off-by: wuhuizuo <wuhuizuo@126.com>
Currently, the
forcepathstyleparameter for the s3 storage driver isconsidered only if the
regionendpointparameter is set. Since settinga region endpoint explicitly is discouraged with AWS s3, it is not clear
how to enforce path style URLs with AWS s3.
This also means, that the default value (true) only applies if a region
endpoint is configured.
This change makes sure we always forward the
forcepathstyleparameterto the aws-sdk if present in the config. This is a breaking change where
a
regionendpointis configured but no explicitforcepathstylevalueis set.
Signed-off-by: Benjamin Schanzel benjamin.schanzel@bmw.de