diff --git a/aws/aws.go b/aws/aws.go index 2e4cbafdef..b43734a8fc 100644 --- a/aws/aws.go +++ b/aws/aws.go @@ -23,13 +23,14 @@ import ( awsv2 "github.com/aws/aws-sdk-go-v2/aws" awsv2cfg "github.com/aws/aws-sdk-go-v2/config" + s3v2 "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/session" "github.com/google/wire" -) + // DefaultSession is a Wire provider set that provides a *session.Session using // the default options. @@ -232,7 +233,7 @@ func V2ConfigFromURLParams(ctx context.Context, q url.Values) (awsv2.Config, err if fips { opts = append(opts, awsv2cfg.WithUseFIPSEndpoint(awsv2.FIPSEndpointStateEnabled)) } - case "awssdk": + case "awssdk", "usePathStyle", "disableHTTPS", "disableAccelerate": // ignore, should be handled before this default: return awsv2.Config{}, fmt.Errorf("unknown query parameter %q", param) @@ -253,3 +254,62 @@ func V2ConfigFromURLParams(ctx context.Context, q url.Values) (awsv2.Config, err return awsv2cfg.LoadDefaultConfig(ctx, opts...) } + +// V2s3OptionsFromURLParams returns a slice of s3 options to initialize +// an S3 client for AWS SDK v2 initialized based on the URL parameters in q. + +// It is intended to be used by URLOpeners for AWS services if +// UseV2 returns true. +// +// https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/s3#Options +// +// It returns an error if q contains any unknown query parameters; callers +// should remove any query parameters they know about from q before calling +// V2s3OptionsFromURLParams. +// +// The following query options are supported: +// - usePathStyle: A value of "true" forces the request to use path-style addressing; sets s3v2.Options.UsePathStyle. +// - disableHTTPS: A value of "true" disables SSL when sending requests; sets s3v2.Options.EndpointOptions.DisableHTTPS. +// - disableAccelerate: A value of "true" disables the use of S3 Transfer Acceleration; sets s3v2.Options.UseAccelerate. +func V2s3OptionsFromURLParams(q url.Values) ([]func(*s3v2.Options), error) { + opts := []func(*s3v2.Options){ + func(o *s3v2.Options) { + o.UseAccelerate = true + }, + } + for param, values := range q { + value := values[0] + switch param { + case "usePathStyle": + b, err := strconv.ParseBool(value) + if err != nil { + return nil, fmt.Errorf("invalid value for query parameter %q: %v", param, err) + } + opts = append(opts, func(o *s3v2.Options) { + o.UsePathStyle = b + }) + case "disableHTTPS": + b, err := strconv.ParseBool(value) + if err != nil { + return nil, fmt.Errorf("invalid value for query parameter %q: %v", param, err) + } + opts = append(opts, func(o *s3v2.Options) { + o.EndpointOptions.DisableHTTPS = b + }) + case "disableAccelerate": + b, err := strconv.ParseBool(value) + if err != nil { + return nil, fmt.Errorf("invalid value for query parameter %q: %v", param, err) + } + opts[0] = func(o *s3v2.Options) { + o.UseAccelerate = !b + } + case "awssdk", "region", "endpoint", "profile", "dualstack", "fips", "hostname_immutable": + // ignore, should be handled before this + + default: + return nil, fmt.Errorf("unknown query parameter %q", param) + } + } + return opts, nil +} diff --git a/aws/aws_test.go b/aws/aws_test.go index 8ad2984ed4..10e40f0321 100644 --- a/aws/aws_test.go +++ b/aws/aws_test.go @@ -21,6 +21,7 @@ import ( "testing" awsv2 "github.com/aws/aws-sdk-go-v2/aws" + s3v2 "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go/aws" "github.com/google/go-cmp/cmp" gcaws "gocloud.dev/aws" @@ -217,6 +218,107 @@ func TestV2ConfigFromURLParams(t *testing.T) { t.Errorf("got endpoint %+v, want %+v", gotE, *test.wantEndpoint) } } + // Test that params are added to the V2s3OptionsFromURLParams + if !test.wantErr { + opts, err := gcaws.V2s3OptionsFromURLParams(test.query) + if err != nil { + t.Errorf("got error %v, want nil", err) + } + if len(opts) != 1 { + t.Errorf("got %d options, want 1", len(opts)) + } + } + }) + } +} + +func TestV2s3OptionsFromURLParams(t *testing.T) { + opts := []func(*s3v2.Options){ + func(o *s3v2.Options) { + o.UseAccelerate = true + }, + } + tests := []struct { + name string + query url.Values + wantErr bool + wantOpts []func(*s3v2.Options) + wantOptions *s3v2.Options + }{ + { + name: "No overrides", + query: url.Values{}, + wantOpts: opts, + wantOptions: &s3v2.Options{UseAccelerate: true}, + }, + { + name: "Invalid query parameter", + query: url.Values{"foo": {"bar"}}, + wantErr: true, + wantOptions: &s3v2.Options{UseAccelerate: true}, + }, + { + name: "UsePathStyle true", + query: url.Values{"usePathStyle": {"true"}}, + wantOpts: append(opts, func(o *s3v2.Options) { + o.UsePathStyle = true + }), + wantOptions: &s3v2.Options{UsePathStyle: true, UseAccelerate: true}, + }, + { + name: "UsePathStyle false", + query: url.Values{"usePathStyle": {"false"}}, + wantOpts: append(opts, func(o *s3v2.Options) { + o.UsePathStyle = false + }), + wantOptions: &s3v2.Options{UsePathStyle: false, UseAccelerate: true}, + }, + { + name: "disableHTTPS true", + query: url.Values{"disableHTTPS": {"true"}}, + wantOpts: append(opts, func(o *s3v2.Options) { + o.EndpointOptions.DisableHTTPS = true + }), + wantOptions: &s3v2.Options{EndpointOptions: s3v2.EndpointResolverOptions{DisableHTTPS: true}, UseAccelerate: true}, + }, + { + name: "disableHTTPS false", + query: url.Values{"disableHTTPS": {"false"}}, + wantOpts: append(opts, func(o *s3v2.Options) { + o.EndpointOptions.DisableHTTPS = false + }), + wantOptions: &s3v2.Options{EndpointOptions: s3v2.EndpointResolverOptions{DisableHTTPS: false}, UseAccelerate: true}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + opts, err := gcaws.V2s3OptionsFromURLParams(test.query) + if (err != nil) != test.wantErr { + t.Errorf("got err %v want error %v", err, test.wantErr) + } + if err != nil { + return + } + if len(opts) != len(test.wantOpts) { + t.Errorf("got %d options, want %d", len(opts), len(test.wantOpts)) + } + + if test.wantOptions != nil { + options := &s3v2.Options{} + for _, opt := range opts { + opt(options) + } + if !reflect.DeepEqual(options, test.wantOptions) { + t.Errorf("got options %v, want %v", options, test.wantOptions) + } + } + if !test.wantErr { + _, err := gcaws.V2ConfigFromURLParams(context.Background(), test.query) + if err != nil { + t.Errorf("got error %v, want nil", err) + } + } }) } } diff --git a/blob/s3blob/s3blob.go b/blob/s3blob/s3blob.go index 7ed47e444e..8768e91e76 100644 --- a/blob/s3blob/s3blob.go +++ b/blob/s3blob/s3blob.go @@ -197,12 +197,14 @@ func (o *URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket if o.UseV2 { cfg, err := gcaws.V2ConfigFromURLParams(ctx, q) if err != nil { - return nil, fmt.Errorf("open bucket %v: %v", u, err) + return nil, fmt.Errorf("open bucket config %v: %v", u, err) + } + options, err := gcaws.V2s3OptionsFromURLParams(q) + if err != nil { + return nil, fmt.Errorf("open bucket options%v: %v", u, err) } - clientV2 := s3v2.NewFromConfig(cfg, func(o *s3v2.Options) { - o.UseAccelerate = accelerate - }) + clientV2 := s3v2.NewFromConfig(cfg, options...) return OpenBucketV2(ctx, clientV2, u.Host, &o.Options) } configProvider := &gcaws.ConfigOverrider{ diff --git a/blob/s3blob/s3blob_test.go b/blob/s3blob/s3blob_test.go index 8d54c521ca..190b9daedf 100644 --- a/blob/s3blob/s3blob_test.go +++ b/blob/s3blob/s3blob_test.go @@ -484,6 +484,10 @@ func TestOpenBucketFromURL(t *testing.T) { {"s3://mybucket?awssdk=v1&accelerate=true&dualstack=true", false}, // OK, use FIPS endpoints (v1) {"s3://mybucket?awssdk=v1&fips=true", false}, + // OK, use usePathStyle + {"s3://mybucket?usePathStyle=true", false}, + // OK, use disableHTTPS + {"s3://mybucket?disableHTTPS=true", false}, // Invalid accelerate (v1) {"s3://mybucket?awssdk=v1&accelerate=bogus", true}, // Invalid accelerate (v2) @@ -498,6 +502,10 @@ func TestOpenBucketFromURL(t *testing.T) { {"s3://mybucket?dualstack=bad", true}, // Invalid ssetype {"s3://mybucket?ssetype=aws:notkmsoraes&kmskeyid=arn:aws:us-east-1:12345:key/1-a-2-b", true}, + // Invalid usePathStyle (v1) + {"s3://mybucket?awssdk=v1&usePathStyle=bad", true}, + // Invalid usePathStyle (v2) + {"s3://mybucket?usePathStyle=bad", true}, // Invalid parameter together with a valid one. {"s3://mybucket?profile=main¶m=value", true}, // Invalid parameter.