From 3442f7838f7eb217f0949d82954228e79ac16abb Mon Sep 17 00:00:00 2001
From: Marco Braga <mrbraga@redhat.com>
Date: Thu, 29 Feb 2024 17:59:27 -0300
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20s3=20bucket=20in=20us-eas?=
 =?UTF-8?q?t-1=20should=20not=20set=20locations?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`us-east-1` is invalid as a Location Contraint when creating a bucket.

"Buckets in Region us-east-1 have a LocationConstraint of null."[1]

[1] https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLocation.html#API_GetBucketLocation_ResponseSyntax
---
 pkg/cloud/services/s3/s3.go      | 13 +++++++++----
 pkg/cloud/services/s3/s3_test.go | 17 +++++++++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/pkg/cloud/services/s3/s3.go b/pkg/cloud/services/s3/s3.go
index a6bbf26b86..b7e9a658ac 100644
--- a/pkg/cloud/services/s3/s3.go
+++ b/pkg/cloud/services/s3/s3.go
@@ -38,6 +38,9 @@ import (
 	"sigs.k8s.io/cluster-api-provider-aws/v2/util/system"
 )
 
+// AWSDefaultRegion is the default AWS region.
+const AWSDefaultRegion string = "us-east-1"
+
 // Service holds a collection of interfaces.
 // The interfaces are broken down like this to group functions together.
 // One alternative is to have a large list of functions from the ec2 client.
@@ -223,11 +226,13 @@ func (s *Service) Delete(m *scope.MachineScope) error {
 }
 
 func (s *Service) createBucketIfNotExist(bucketName string) error {
-	input := &s3.CreateBucketInput{
-		Bucket: aws.String(bucketName),
-		CreateBucketConfiguration: &s3.CreateBucketConfiguration{
+	input := &s3.CreateBucketInput{Bucket: aws.String(bucketName)}
+
+	// See https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateBucket.html#AmazonS3-CreateBucket-request-LocationConstraint.
+	if s.scope.Region() != AWSDefaultRegion {
+		input.CreateBucketConfiguration = &s3.CreateBucketConfiguration{
 			LocationConstraint: aws.String(s.scope.Region()),
-		},
+		}
 	}
 
 	_, err := s.S3Client.CreateBucket(input)
diff --git a/pkg/cloud/services/s3/s3_test.go b/pkg/cloud/services/s3/s3_test.go
index 7ce58d19d0..afaa5fdc99 100644
--- a/pkg/cloud/services/s3/s3_test.go
+++ b/pkg/cloud/services/s3/s3_test.go
@@ -307,6 +307,23 @@ func TestReconcileBucket(t *testing.T) {
 				t.Fatalf("Expected error")
 			}
 		})
+
+		t.Run("creates_bucket_without_location", func(t *testing.T) {
+			t.Parallel()
+
+			svc, s3Mock := testService(t, &infrav1.S3Bucket{})
+			input := &s3svc.CreateBucketInput{
+				CreateBucketConfiguration: &s3svc.CreateBucketConfiguration{
+					LocationConstraint: nil,
+				},
+			}
+
+			s3Mock.EXPECT().CreateBucket(gomock.Eq(input)).Return(nil, nil).Times(1)
+
+			if err := svc.ReconcileBucket(); err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+		})
 	})
 }