Add disableHTTPS and usePathStyle s3v2.Options as query param

This ensures that we can use blob/blob with s3 compatible storage like minio. Pass `usePathStyle=true` to force PathStyle for s3. Pass `disableHTTPS=true` to disable tls for endpoint.
google · Oct 7, 2024 · ba017d3 · ba017d3
1 parent e5b1bc6
commit ba017d3
Show file tree

Hide file tree

Showing 4 changed files with 178 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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&param=value", true},
 		// Invalid parameter.