s3blob/blob: support additional endpoint query parameters #3486
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vangent
requested changes
Sep 11, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3486 +/- ##
==========================================
+ Coverage 73.14% 73.45% +0.30%
==========================================
Files 113 113
Lines 14983 15155 +172
==========================================
+ Hits 10960 11132 +172
+ Misses 3260 3258 -2
- Partials 763 765 +2 ☔ View full report in Codecov by Sentry. |
stanhu
force-pushed
the
sh-add-s3-endpoints-opts
branch
3 times, most recently
from
f1f85a3
to
7f41fbe
Compare
stanhu
force-pushed
the
sh-add-s3-endpoints-opts
branch
from
7f41fbe
to
d1a0e3d
Compare
vangent
requested changes
Sep 11, 2024
aws/aws.go
Outdated
| @@ -215,5 +246,12 @@ func V2ConfigFromURLParams(ctx context.Context, q url.Values) (awsv2.Config, err | |||
| }) | |||
| opts = append(opts, awsv2cfg.WithEndpointResolverWithOptions(customResolver)) | |||
| } | |||
| if dualStack { | |||
There was a problem hiding this comment.
Hmm if you're just going to add them down here, I think it was better before, doing it right after the parameter was processed. I thought maybe they needed to be set in the awsv2.Endpoint like HostnameImmutable, but if that's not the case then my apologies, please change it back.
There was a problem hiding this comment.
Yes, interestingly enough, they aren't passed in with the resolver.
This commit adds the following query parameters for AWS: 1. `dualstack` 2. `fips` 3. `accelerate` (S3-only) This avoids the need for users to specify `endpoint`. For example, if my AWS S3 bucket is `my-bucket` in `us-east-1`, and you want to enable transfer acceleration, dual-stack support, and/or FIPS, you would need to configure `endpoint` with one of the following: 1. `my-bucket.s3-accelerate.amazonaws.com` 2. `my-bucket.s3-accelerate.dualstack.amazonaws.com` 3. `my-bucket.s3-fips.us-gov-east-1.amazonaws.com` 4. `my-bucket.s3-fips.dualstack.us-east-1.amazonaws.com` For example, for the last option, users can use `s3://my-bucket?fips=true&dualstack=true`. Closes google#3484
stanhu
force-pushed
the
sh-add-s3-endpoints-opts
branch
from
d1a0e3d
to
602e83e
Compare
vangent
approved these changes
Sep 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds the following query parameters for AWS:
dualstackfipsaccelerate(S3-only)This avoids the need for users to specify
endpoint. For example, if my AWS S3 bucket ismy-bucketinus-east-1, and you want to enable transfer acceleration, dual-stack support, and/or FIPS, you would need to configureendpointwith one of the following:my-bucket.s3-accelerate.amazonaws.commy-bucket.s3-accelerate.dualstack.amazonaws.commy-bucket.s3-fips.us-gov-east-1.amazonaws.commy-bucket.s3-fips.dualstack.us-east-1.amazonaws.comFor example, for the last option, users can use
s3://my-bucket?fips=true&dualstack=true.Closes #3484