Repository: google/go-cloud
Pull request #3491 (2024-10-04 22:15:16)

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.

Please use a title starting with the name of the affected package, or \"all\",
followed by a colon, followed by a short summary of the issue. Example:
`blob/gcsblob: fix typo in documentation`.

Fixes: https://github.com/google/go-cloud/issues/3490


khrm: /assign @vangent 
khrm: @vangent Can you review this again?
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/google/go-cloud/pull/3491?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
Attention: Patch coverage is `80.95238%` with `4 lines` in your changes missing coverage. Please review.
> Project coverage is 73.24%. Comparing base [(`e5b1bc6`)](https://app.codecov.io/gh/google/go-cloud/commit/e5b1bc66f5c42c0a4bb43d179cefdab454559325?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) to head [(`0e94b90`)](https://app.codecov.io/gh/google/go-cloud/commit/0e94b903b8131c144583980e728a953eadcbbb3c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).
> Report is 2 commits behind head on master.

| [Files with missing lines](https://app.codecov.io/gh/google/go-cloud/pull/3491?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) | Patch % | Lines |
|---|---|---|
| [blob/s3blob/s3blob.go](https://app.codecov.io/gh/google/go-cloud/pull/3491?src=pr&el=tree&filepath=blob%2Fs3blob%2Fs3blob.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-YmxvYi9zM2Jsb2IvczNibG9iLmdv) | 80.95% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/google/go-cloud/pull/3491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) |

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master    #3491   +/-   ##
=======================================
  Coverage   73.23%   73.24%           
=======================================
  Files         113      113           
  Lines       15018    15037   +19     
=======================================
+ Hits        10999    11014   +15     
- Misses       3257     3259    +2     
- Partials      762      764    +2     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/google/go-cloud/pull/3491?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).


Patch:
diff --git a/blob/s3blob/s3blob.go b/blob/s3blob/s3blob.go
index 7ed47e44..f575649b 100644
--- a/blob/s3blob/s3blob.go
+++ b/blob/s3blob/s3blob.go
@@ -150,9 +150,11 @@ type URLOpener struct {
 }
 
 const (
-	sseTypeParamKey    = "ssetype"
-	kmsKeyIdParamKey   = "kmskeyid"
-	accelerateParamKey = "accelerate"
+	sseTypeParamKey      = "ssetype"
+	kmsKeyIdParamKey     = "kmskeyid"
+	accelerateParamKey   = "accelerate"
+	usePathStyleParamkey = "use_path_style"
+	disableHTTPSParamKey = "disable_https"
 )
 
 func toServerSideEncryptionType(value string) (typesv2.ServerSideEncryption, error) {
@@ -195,13 +197,37 @@ func (o *URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket
 	}
 
 	if o.UseV2 {
+		opts := []func(*s3v2.Options){
+			func(o *s3v2.Options) {
+				o.UseAccelerate = accelerate
+			},
+		}
+		if disableHTTPSParam := q.Get(disableHTTPSParamKey); disableHTTPSParam != "" {
+			q.Del(disableHTTPSParamKey)
+			value, err := strconv.ParseBool(disableHTTPSParam)
+			if err != nil {
+				return nil, fmt.Errorf("invalid value for %q: %v", disableHTTPSParamKey, err)
+			}
+			opts = append(opts, func(o *s3v2.Options) {
+				o.EndpointOptions.DisableHTTPS = value
+			})
+		}
+		if usePathStyleParam := q.Get(usePathStyleParamkey); usePathStyleParam != "" {
+			q.Del(usePathStyleParamkey)
+			value, err := strconv.ParseBool(usePathStyleParam)
+			if err != nil {
+				return nil, fmt.Errorf("invalid value for %q: %v", usePathStyleParamkey, err)
+			}
+			opts = append(opts, func(o *s3v2.Options) {
+				o.UsePathStyle = value
+			})
+		}
+
 		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.UseAccelerate = accelerate
-		})
+		clientV2 := s3v2.NewFromConfig(cfg, opts...)
 
 		return OpenBucketV2(ctx, clientV2, u.Host, &o.Options)
 	}


Test patch:
diff --git a/blob/s3blob/s3blob_test.go b/blob/s3blob/s3blob_test.go
index 8d54c521..2639b74d 100644
--- a/blob/s3blob/s3blob_test.go
+++ b/blob/s3blob/s3blob_test.go
@@ -482,6 +482,10 @@ func TestOpenBucketFromURL(t *testing.T) {
 		{"s3://mybucket?fips=true", false},
 		// OK, use S3 Transfer accleration and dual stack endpoints (v1)
 		{"s3://mybucket?awssdk=v1&accelerate=true&dualstack=true", false},
+		// OK, use use_path_style
+		{"s3://mybucket?use_path_style=true", false},
+		// OK, use disable_https
+		{"s3://mybucket?disable_https=true", false},
 		// OK, use FIPS endpoints (v1)
 		{"s3://mybucket?awssdk=v1&fips=true", false},
 		// Invalid accelerate (v1)
@@ -500,6 +504,10 @@ func TestOpenBucketFromURL(t *testing.T) {
 		{"s3://mybucket?ssetype=aws:notkmsoraes&kmskeyid=arn:aws:us-east-1:12345:key/1-a-2-b", true},
 		// Invalid parameter together with a valid one.
 		{"s3://mybucket?profile=main&param=value", true},
+		// Invalid use_path_style (v1)
+		{"s3://mybucket?awssdk=v1&usePathStyle=bad", true},
+		// Invalid disable_https (v2)
+		{"s3://mybucket?usePathStyle=bad", true},
 		// Invalid parameter.
 		{"s3://mybucket?param=value", true},
 	}