service/s3/s3manager: Clarify documentation and behavior of GetBucketRegion

CHANGELOG_PENDING.md

@@ -1,5 +1,9 @@
 ### SDK Features
 
 ### SDK Enhancements
+* `service/s3/s3manager`: Clarify documentation and behavior of GetBucketRegion ([#3428](https://github.com/aws/aws-sdk-go/pull/3428))
+  * Updates the documentation for GetBucketRegion's behavior with regard to default configuration for path style addressing. Provides examples how to override this behavior.
+  * Updates the GetBucketRegion utility to not require a region hint when the session or client was configured with a custom endpoint URL.
+  * Related to [#3115](https://github.com/aws/aws-sdk-go/issues/3115)
 
 ### SDK Bugs

service/s3/s3manager/bucket_region.go

@@ -3,6 +3,7 @@ package s3manager
 import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/client"
+	"github.com/aws/aws-sdk-go/aws/corehandlers"
 	"github.com/aws/aws-sdk-go/aws/credentials"
 	"github.com/aws/aws-sdk-go/aws/request"
 	"github.com/aws/aws-sdk-go/service/s3"
@@ -35,6 +36,30 @@ import (
 //    }
 //    fmt.Printf("Bucket %s is in %s region\n", bucket, region)
 //
+// By default the request will be made to the Amazon S3 endpoint using the Path
+// style addressing.
+//
+//    s3.us-west-2.amazonaws.com/bucketname
+//
+// This is not compatible with Amazon S3's FIPS endpoints. To override this
+// behavior to use Virtual Host style addressing, provide a functional option
+// that will set the Request's Config.S3ForcePathStyle to aws.Bool(false).
+//
+//    region, err := s3manager.GetBucketRegion(ctx, sess, "bucketname", "us-west-2", func(r *request.Request) {
+//        r.S3ForcePathStyle = aws.Bool(false)
+//    })
+//
+// To configure the GetBucketRegion to make a request via the Amazon
+// S3 FIPS endpoints directly when a FIPS region name is not available, (e.g.
+// fips-us-gov-west-1) set the Config.Endpoint on the Session, or client the
+// utility is called with. The hint region will be ignored if an endpoint URL
+// is configured on the session or client.
+//
+//    sess, err := session.NewSession(&aws.Config{
+//        Endpoint: aws.String("https://s3-fips.us-west-2.amazonaws.com"),
+//    })
+//
+//    region, err := s3manager.GetBucketRegion(context.Background(), sess, "bucketname", "")
 func GetBucketRegion(ctx aws.Context, c client.ConfigProvider, bucket, regionHint string, opts ...request.Option) (string, error) {
 	var cfg aws.Config
 	if len(regionHint) != 0 {
@@ -50,12 +75,38 @@ const bucketRegionHeader = "X-Amz-Bucket-Region"
 // that it takes a S3 service client instead of a Session. The regionHint is
 // derived from the region the S3 service client was created in.
 //
+// By default the request will be made to the Amazon S3 endpoint using the Path
+// style addressing.
+//
+//    s3.us-west-2.amazonaws.com/bucketname
+//
+// This is not compatible with Amazon S3's FIPS endpoints. To override this
+// behavior to use Virtual Host style addressing, provide a functional option
+// that will set the Request's Config.S3ForcePathStyle to aws.Bool(false).
+//
+//    region, err := s3manager.GetBucketRegionWithClient(ctx, client, "bucketname", func(r *request.Request) {
+//        r.S3ForcePathStyle = aws.Bool(false)
+//    })
+//
+// To configure the GetBucketRegion to make a request via the Amazon
+// S3 FIPS endpoints directly when a FIPS region name is not available, (e.g.
+// fips-us-gov-west-1) set the Config.Endpoint on the Session, or client the
+// utility is called with. The hint region will be ignored if an endpoint URL
+// is configured on the session or client.
+//
+//    region, err := s3manager.GetBucketRegionWithClient(context.Background(),
+//    s3.New(sess, &aws.Config{
+//        Endpoint: aws.String("https://s3-fips.us-west-2.amazonaws.com"),
+//    }),
+//    "bucketname")
+//
 // See GetBucketRegion for more information.
 func GetBucketRegionWithClient(ctx aws.Context, svc s3iface.S3API, bucket string, opts ...request.Option) (string, error) {
 	req, _ := svc.HeadBucketRequest(&s3.HeadBucketInput{
 		Bucket: aws.String(bucket),
 	})
 	req.Config.S3ForcePathStyle = aws.Bool(true)
+
 	req.Config.Credentials = credentials.AnonymousCredentials
 	req.SetContext(ctx)
 
@@ -75,6 +126,16 @@ func GetBucketRegionWithClient(ctx aws.Context, svc s3iface.S3API, bucket string
 		r.HTTPResponse.Status = "OK"
 		r.Error = nil
 	})
+	// Replace the endpoint validation handler to not require a region if an
+	// endpoint URL was specified. Since these requests are not authenticated,
+	// requiring a region is not needed when an endpoint URL is provided.
+	req.Handlers.Validate.Swap(
+		corehandlers.ValidateEndpointHandler.Name,
+		request.NamedHandler{
+			Name: "validateEndpointWithoutRegion",
+			Fn:   validateEndpointWithoutRegion,
+		},
+	)
 
 	req.ApplyOptions(opts...)
 
@@ -86,3 +147,10 @@ func GetBucketRegionWithClient(ctx aws.Context, svc s3iface.S3API, bucket string
 
 	return bucketRegion, nil
 }
+
+func validateEndpointWithoutRegion(r *request.Request) {
+	if len(aws.StringValue(r.Config.Endpoint)) == 0 {
+		corehandlers.ValidateEndpointHandler.Fn(r)
+	}
+
+}

service/s3/s3manager/bucket_region_test.go

@@ -97,3 +97,25 @@ func TestGetBucketRegionWithClient(t *testing.T) {
 		}
 	}
 }
+
+func TestGetBucketRegionWithClientWithoutRegion(t *testing.T) {
+	for i, c := range testGetBucketRegionCases {
+		server := testSetupGetBucketRegionServer(c.RespRegion, c.StatusCode, true)
+		defer server.Close()
+
+		svc := s3.New(unit.Session, &aws.Config{
+			Endpoint:   aws.String(server.URL),
+			DisableSSL: aws.Bool(true),
+		})
+
+		ctx := aws.BackgroundContext()
+
+		region, err := GetBucketRegionWithClient(ctx, svc, "bucket")
+		if err != nil {
+			t.Fatalf("%d, expect no error, got %v", i, err)
+		}
+		if e, a := c.RespRegion, region; e != a {
+			t.Errorf("%d, expect %q region, got %q", i, e, a)
+		}
+	}
+}