google / go-cloud

The Go Cloud Development Kit (Go CDK): A library and tools for open cloud development in Go.
https://gocloud.dev/
Apache License 2.0
9.57k stars 812 forks source link

s3blob/blob: support additional endpoint parameters: UseDualStack, UseFips, and UseAccelerate options #3484

Closed stanhu closed 2 months ago

stanhu commented 2 months ago

Is your feature request related to a problem? Please describe.

https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/endpoints/ mentions additional parameters that can be configured:

These were added upstream in https://github.com/aws/aws-sdk-go-v2/pull/836.

Describe the solution you'd like

The AWS SDK v2 URL probably should accept the following query parameters:

  1. dualstack
  2. fips
  3. accelerate

As described in https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/endpoints/#migration, we probably should migrate to v2 of the endpoint resolution. Note that EndpointResolverWithOptionsFunc is deprecated and should likely be replaced with the v2 mechanism: https://github.com/google/go-cloud/blob/d2adbc5c0014d2b7de556e837a508ba999b08f51/aws/aws.go#L207

Describe alternatives you've considered

While the endpoint can probably be used to support this functionality, users would have to know their region-specific endpoints. For example, if my AWS S3 bucket is my-bucket in us-east-1, and I want to enable transfer acceleration, dual-stack support, and FIPS, I 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

This forces the application to build the hostname, when this seems more appropriately handled by the SDK.

References:

  1. https://docs.aws.amazon.com/AmazonS3/latest/userguide/transfer-acceleration.html
  2. https://aws.amazon.com/compliance/fips/

Additional context

I believe UseAccelerate can be added via something like:

diff --git a/blob/s3blob/s3blob.go b/blob/s3blob/s3blob.go
index d3e80cf0..7353c6db 100644
--- a/blob/s3blob/s3blob.go
+++ b/blob/s3blob/s3blob.go
@@ -145,8 +145,9 @@ type URLOpener struct {
 }

 const (
-   sseTypeParamKey  = "ssetype"
-   kmsKeyIdParamKey = "kmskeyid"
+   sseTypeParamKey    = "ssetype"
+   kmsKeyIdParamKey   = "kmskeyid"
+   accelerateParamKey = "accelerate"
 )

 func toServerSideEncryptionType(value string) (typesv2.ServerSideEncryption, error) {
@@ -178,12 +179,24 @@ func (o *URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket
        o.Options.KMSEncryptionID = kmsKeyID
    }

+   accelerate := false
+   if accelerateParam := q.Get(accelerateParamKey); accelerateParam != "" {
+       q.Del(accelerateParamKey)
+       var err error
+       accelerate, err = strconv.ParseBool(accelerateParam)
+       if err != nil {
+           return nil, fmt.Errorf("invalid value for %q: %v", accelerateParamKey, err)
+       }
+   }
+
    if o.UseV2 {
        cfg, err := gcaws.V2ConfigFromURLParams(ctx, q)
        if err != nil {
            return nil, fmt.Errorf("open bucket %v: %v", u, err)
        }
-       clientV2 := s3v2.NewFromConfig(cfg)
+       clientV2 := s3v2.NewFromConfig(cfg, func(o *s3v2.Options) {
+           o.UseAccelerate = accelerate
+       })

        return OpenBucketV2(ctx, clientV2, u.Host, &o.Options)
    }
vangent commented 2 months ago

IIUC, most (all?) of these options are not blob-specific, so they are currently implemented in aws/aws.go. There is a V2-specific function there (V2ConfigFromURLParams). TBH it is not obvious to me how to migrate this to "V2" (it is already V2, but I guess not the "real" V2 approach). Contributions welcome.

stanhu commented 2 months ago

Yeah, I was trying to add the fips and dualstack options in the global settings and accelerate in the S3 settings. https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/aws#EndpointResolver mentions:

Deprecated: The global endpoint resolution interface is deprecated. The API for endpoint resolution is now unique to each service and is set via the EndpointResolverV2 field on service client options. Setting a value for EndpointResolver on aws.Config or service client options will prevent you from using any endpoint-related service features released after the introduction of EndpointResolverV2. You may also encounter broken or unexpected behavior when using the old global interface with services that use many endpoint-related customizations such as S3.

I'm also seeing warnings like this in the tests:

2024-09-10 15:49:28.071 [info] /Users/stanhu/go-cloud/aws/aws_test.go:220:18 got.EndpointResolverWithOptions is deprecated: with the release of endpoint resolution v2 in API clients, EndpointResolver and EndpointResolverWithOptions are deprecated. Providing a value for this field will likely prevent you from using newer endpoint-related service features. See API client options EndpointResolverV2 and BaseEndpoint.  (SA1019)

I think this means setting the resolver must be pushed into each service-specific endpoint, such as:

        cfg, err := gcaws.V2ConfigFromURLParams(ctx, q)
        if err != nil {
            return nil, fmt.Errorf("open bucket %v: %v", u, err)
        }
        clientV2 := s3v2.NewFromConfig(cfg, func(o *s3v2.Options) {
            o.EndpointResolverV2 = <set the service-specific resolver here>
        })

This can probably done as a separate issue.