From 3a1f57d87895f211d0d32cb9ba5f37f5e370395b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Feb 2026 12:22:03 +0000 Subject: [PATCH 1/6] Extend HeadBucket handler to include bucket region header - Add x-amz-bucket-region header to HeadBucket response - Use "us-east-1" as default region (Storj is a global service) - Add basic test to verify GetBucketInfo works correctly Co-authored-by: amwolff <30234756+amwolff@users.noreply.github.com> --- .gitignore | 1 + api/buckethandlers.go | 3 ++ testsuite/miniogw/head_bucket_test.go | 49 +++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 testsuite/miniogw/head_bucket_test.go diff --git a/.gitignore b/.gitignore index 18fd1e0..6f73efc 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ release/ # VS Code .vscode/ +/gateway diff --git a/api/buckethandlers.go b/api/buckethandlers.go index fff521b..b32170d 100644 --- a/api/buckethandlers.go +++ b/api/buckethandlers.go @@ -307,6 +307,9 @@ func (api *API) HeadBucketHandler(w http.ResponseWriter, r *http.Request) { return } + // Set the bucket region header. Storj is a global service, so we use the default AWS region. + w.Header().Set(xhttp.AmzBucketRegion, "us-east-1") + writeSuccessResponseHeadersOnly(w) } diff --git a/testsuite/miniogw/head_bucket_test.go b/testsuite/miniogw/head_bucket_test.go new file mode 100644 index 0000000..1afe170 --- /dev/null +++ b/testsuite/miniogw/head_bucket_test.go @@ -0,0 +1,49 @@ +// Copyright (C) 2026 Storj Labs, Inc. +// See LICENSE for copying information. + +package miniogw_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "storj.io/common/testcontext" + "storj.io/gateway/miniogw" + minio "storj.io/minio/cmd" + "storj.io/minio/pkg/auth" + "storj.io/storj/private/testplanet" +) + +func TestHeadBucketRegion(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 4, UplinkCount: 1, + }, func(t *testing.T, tCtx *testcontext.Context, planet *testplanet.Planet) { + project, err := planet.Uplinks[0].OpenProject(tCtx, planet.Satellites[0]) + require.NoError(t, err) + defer tCtx.Check(project.Close) + + // Establish new context with *uplink.Project for the gateway to pick up. + ctx := miniogw.WithCredentials(tCtx, project, miniogw.CredentialsInfo{}) + + layer, err := miniogw.NewStorjGateway(defaultS3CompatibilityConfig).NewGatewayLayer(auth.Credentials{}) + require.NoError(t, err) + + defer func() { require.NoError(t, layer.Shutdown(ctx)) }() + + // Create a test bucket + bucketName := "test-bucket-region" + err = layer.MakeBucketWithLocation(ctx, bucketName, minio.BucketOptions{}) + require.NoError(t, err) + + // Get bucket info + bucketInfo, err := layer.GetBucketInfo(ctx, bucketName) + require.NoError(t, err) + require.Equal(t, bucketName, bucketInfo.Name) + + // The HeadBucketHandler will set x-amz-bucket-region to "us-east-1" + // This test verifies that GetBucketInfo works correctly, which is + // required for HeadBucketHandler to succeed and set the region header. + t.Log("Bucket info retrieved successfully, HeadBucket handler will set x-amz-bucket-region header") + }) +} From b8ba902c64e8756f4a4a2d25d06c578da04e7b4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Feb 2026 12:24:29 +0000 Subject: [PATCH 2/6] Improve test documentation for HeadBucket region header - Add detailed comments explaining test purpose - Clarify that HTTP-level testing is done via integration tests - Document expected HeadBucketHandler behavior Co-authored-by: amwolff <30234756+amwolff@users.noreply.github.com> --- testsuite/miniogw/head_bucket_test.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/testsuite/miniogw/head_bucket_test.go b/testsuite/miniogw/head_bucket_test.go index 1afe170..993cf1c 100644 --- a/testsuite/miniogw/head_bucket_test.go +++ b/testsuite/miniogw/head_bucket_test.go @@ -15,6 +15,13 @@ import ( "storj.io/storj/private/testplanet" ) +// TestHeadBucketRegion tests that GetBucketInfo works correctly, which is the +// core dependency for the HeadBucketHandler. The HeadBucketHandler in the API +// package sets the x-amz-bucket-region header to "us-east-1" after successfully +// calling GetBucketInfo. +// +// Note: Full HTTP-level testing of the HeadBucket handler including header +// verification is done via integration tests (e.g., using rclone or AWS CLI). func TestHeadBucketRegion(t *testing.T) { testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, StorageNodeCount: 4, UplinkCount: 1, @@ -36,14 +43,13 @@ func TestHeadBucketRegion(t *testing.T) { err = layer.MakeBucketWithLocation(ctx, bucketName, minio.BucketOptions{}) require.NoError(t, err) - // Get bucket info + // Verify GetBucketInfo works - this is what HeadBucketHandler relies on bucketInfo, err := layer.GetBucketInfo(ctx, bucketName) require.NoError(t, err) require.Equal(t, bucketName, bucketInfo.Name) - // The HeadBucketHandler will set x-amz-bucket-region to "us-east-1" - // This test verifies that GetBucketInfo works correctly, which is - // required for HeadBucketHandler to succeed and set the region header. - t.Log("Bucket info retrieved successfully, HeadBucket handler will set x-amz-bucket-region header") + // If GetBucketInfo succeeds, HeadBucketHandler will: + // 1. Return 200 OK + // 2. Set the x-amz-bucket-region header to "us-east-1" }) } From 2012f8dec04a30ff8b02f183abc344071ce57a2f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Feb 2026 12:43:40 +0000 Subject: [PATCH 3/6] Fetch bucket location and include in HeadBucket response - Add BucketInfo struct with Location field to api/datatypes.go - Add GetBucketLocation method to gatewayLayer in miniogw/gateway.go - Add GetBucketInfoWithLocation helper in api/api.go that fetches both bucket info and location - Update HeadBucketHandler to use actual bucket location from libuplink - Falls back to "us-east-1" if location cannot be retrieved - Remove previous hardcoded implementation Co-authored-by: amwolff <30234756+amwolff@users.noreply.github.com> --- api/api.go | 31 +++++++++++++++ api/buckethandlers.go | 12 ++++-- api/datatypes.go | 9 +++++ miniogw/gateway.go | 27 +++++++++++-- testsuite/miniogw/head_bucket_test.go | 55 --------------------------- 5 files changed, 73 insertions(+), 61 deletions(-) delete mode 100644 testsuite/miniogw/head_bucket_test.go diff --git a/api/api.go b/api/api.go index cf7bc84..a76e7a8 100644 --- a/api/api.go +++ b/api/api.go @@ -48,6 +48,37 @@ type API struct { config Config } +// BucketLocationGetter is an interface for getting bucket location. +// This is implemented by the gateway layer to fetch bucket region/location. +type BucketLocationGetter interface { + GetBucketLocation(ctx context.Context, bucketName string) (string, error) +} + +// GetBucketInfoWithLocation fetches bucket info along with its location. +func (api *API) GetBucketInfoWithLocation(ctx context.Context, bucketName string) (BucketInfo, error) { + // Get basic bucket info + minioInfo, err := api.objectAPI.GetBucketInfo(ctx, bucketName) + if err != nil { + return BucketInfo{}, err + } + + // Try to get location if the objectAPI supports it + location := "" + if locationGetter, ok := api.objectAPI.(BucketLocationGetter); ok { + loc, err := locationGetter.GetBucketLocation(ctx, bucketName) + if err == nil { + location = loc + } + // If error, leave location empty - will default to us-east-1 + } + + return BucketInfo{ + Name: minioInfo.Name, + Created: minioInfo.Created, + Location: location, + }, nil +} + // New constructs a new S3-compatible HTTP API. func New(objectAPI cmd.ObjectLayer, credsProvider awsig.CredentialsProvider[AuthData], config Config) *API { v2v4 := awsig.NewV2V4(credsProvider, awsig.V4Config{ diff --git a/api/buckethandlers.go b/api/buckethandlers.go index b32170d..cc3f9d5 100644 --- a/api/buckethandlers.go +++ b/api/buckethandlers.go @@ -302,13 +302,19 @@ func (api *API) HeadBucketHandler(w http.ResponseWriter, r *http.Request) { return } - if _, err := api.objectAPI.GetBucketInfo(ctx, bucketName); err != nil { + bucketInfo, err := api.GetBucketInfoWithLocation(ctx, bucketName) + if err != nil { writeErrorResponseHeadersOnly(w, cmd.ToAPIError(ctx, err)) return } - // Set the bucket region header. Storj is a global service, so we use the default AWS region. - w.Header().Set(xhttp.AmzBucketRegion, "us-east-1") + // Set the bucket region header + if bucketInfo.Location != "" { + w.Header().Set(xhttp.AmzBucketRegion, bucketInfo.Location) + } else { + // Default to us-east-1 if location is not available + w.Header().Set(xhttp.AmzBucketRegion, "us-east-1") + } writeSuccessResponseHeadersOnly(w) } diff --git a/api/datatypes.go b/api/datatypes.go index f3d2269..0b799c4 100644 --- a/api/datatypes.go +++ b/api/datatypes.go @@ -22,10 +22,19 @@ package api import ( "encoding/xml" + "time" "storj.io/minio/cmd" ) +// BucketInfo represents bucket metadata. +// This struct extends the MinIO BucketInfo to include location/region information. +type BucketInfo struct { + Name string + Created time.Time + Location string +} + type grantee struct { XMLNS string `xml:"xmlns:xsi,attr"` XMLXSI string `xml:"xsi:type,attr"` diff --git a/miniogw/gateway.go b/miniogw/gateway.go index cccc98a..9a38a61 100644 --- a/miniogw/gateway.go +++ b/miniogw/gateway.go @@ -335,17 +335,38 @@ func (layer *gatewayLayer) GetBucketInfo(ctx context.Context, bucketName string) return minio.BucketInfo{}, err } - bucket, err := project.StatBucket(ctx, bucketName) + bucketObj, err := project.StatBucket(ctx, bucketName) if err != nil { return minio.BucketInfo{}, ConvertError(err, bucketName, "") } return minio.BucketInfo{ - Name: bucket.Name, - Created: bucket.Created, + Name: bucketObj.Name, + Created: bucketObj.Created, }, nil } +// GetBucketLocation returns the location/region of a bucket. +func (layer *gatewayLayer) GetBucketLocation(ctx context.Context, bucketName string) (location string, err error) { + defer mon.Task()(&ctx)(&err) + + if err := ValidateBucket(ctx, bucketName); err != nil { + return "", minio.BucketNameInvalid{Bucket: bucketName} + } + + project, err := projectFromContext(ctx, bucketName, "") + if err != nil { + return "", err + } + + location, err = bucket.GetBucketLocation(ctx, project, bucketName) + if err != nil { + return "", ConvertError(err, bucketName, "") + } + + return location, nil +} + func (layer *gatewayLayer) ListBuckets(ctx context.Context) (items []minio.BucketInfo, err error) { defer mon.Task()(&ctx)(&err) diff --git a/testsuite/miniogw/head_bucket_test.go b/testsuite/miniogw/head_bucket_test.go deleted file mode 100644 index 993cf1c..0000000 --- a/testsuite/miniogw/head_bucket_test.go +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright (C) 2026 Storj Labs, Inc. -// See LICENSE for copying information. - -package miniogw_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "storj.io/common/testcontext" - "storj.io/gateway/miniogw" - minio "storj.io/minio/cmd" - "storj.io/minio/pkg/auth" - "storj.io/storj/private/testplanet" -) - -// TestHeadBucketRegion tests that GetBucketInfo works correctly, which is the -// core dependency for the HeadBucketHandler. The HeadBucketHandler in the API -// package sets the x-amz-bucket-region header to "us-east-1" after successfully -// calling GetBucketInfo. -// -// Note: Full HTTP-level testing of the HeadBucket handler including header -// verification is done via integration tests (e.g., using rclone or AWS CLI). -func TestHeadBucketRegion(t *testing.T) { - testplanet.Run(t, testplanet.Config{ - SatelliteCount: 1, StorageNodeCount: 4, UplinkCount: 1, - }, func(t *testing.T, tCtx *testcontext.Context, planet *testplanet.Planet) { - project, err := planet.Uplinks[0].OpenProject(tCtx, planet.Satellites[0]) - require.NoError(t, err) - defer tCtx.Check(project.Close) - - // Establish new context with *uplink.Project for the gateway to pick up. - ctx := miniogw.WithCredentials(tCtx, project, miniogw.CredentialsInfo{}) - - layer, err := miniogw.NewStorjGateway(defaultS3CompatibilityConfig).NewGatewayLayer(auth.Credentials{}) - require.NoError(t, err) - - defer func() { require.NoError(t, layer.Shutdown(ctx)) }() - - // Create a test bucket - bucketName := "test-bucket-region" - err = layer.MakeBucketWithLocation(ctx, bucketName, minio.BucketOptions{}) - require.NoError(t, err) - - // Verify GetBucketInfo works - this is what HeadBucketHandler relies on - bucketInfo, err := layer.GetBucketInfo(ctx, bucketName) - require.NoError(t, err) - require.Equal(t, bucketName, bucketInfo.Name) - - // If GetBucketInfo succeeds, HeadBucketHandler will: - // 1. Return 200 OK - // 2. Set the x-amz-bucket-region header to "us-east-1" - }) -} From 16a6bd24ac7b36f91b2ea6290c3e8a5a3ff17659 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Feb 2026 13:23:13 +0000 Subject: [PATCH 4/6] Address PR review feedback - Remove unnecessary rename from bucket to bucketObj in GetBucketInfo - Remove fallback to "us-east-1" when location is not available - Simplify implementation: call GetBucketInfo and GetBucketLocation separately in HeadBucketHandler - Remove unused BucketInfo struct and GetBucketInfoWithLocation helper - Keep BucketLocationGetter interface for type assertion Co-authored-by: amwolff <30234756+amwolff@users.noreply.github.com> --- api/api.go | 25 ------------------------- api/buckethandlers.go | 14 ++++++-------- api/datatypes.go | 9 --------- miniogw/gateway.go | 6 +++--- 4 files changed, 9 insertions(+), 45 deletions(-) diff --git a/api/api.go b/api/api.go index a76e7a8..fa8479e 100644 --- a/api/api.go +++ b/api/api.go @@ -54,31 +54,6 @@ type BucketLocationGetter interface { GetBucketLocation(ctx context.Context, bucketName string) (string, error) } -// GetBucketInfoWithLocation fetches bucket info along with its location. -func (api *API) GetBucketInfoWithLocation(ctx context.Context, bucketName string) (BucketInfo, error) { - // Get basic bucket info - minioInfo, err := api.objectAPI.GetBucketInfo(ctx, bucketName) - if err != nil { - return BucketInfo{}, err - } - - // Try to get location if the objectAPI supports it - location := "" - if locationGetter, ok := api.objectAPI.(BucketLocationGetter); ok { - loc, err := locationGetter.GetBucketLocation(ctx, bucketName) - if err == nil { - location = loc - } - // If error, leave location empty - will default to us-east-1 - } - - return BucketInfo{ - Name: minioInfo.Name, - Created: minioInfo.Created, - Location: location, - }, nil -} - // New constructs a new S3-compatible HTTP API. func New(objectAPI cmd.ObjectLayer, credsProvider awsig.CredentialsProvider[AuthData], config Config) *API { v2v4 := awsig.NewV2V4(credsProvider, awsig.V4Config{ diff --git a/api/buckethandlers.go b/api/buckethandlers.go index cc3f9d5..c117e8f 100644 --- a/api/buckethandlers.go +++ b/api/buckethandlers.go @@ -302,18 +302,16 @@ func (api *API) HeadBucketHandler(w http.ResponseWriter, r *http.Request) { return } - bucketInfo, err := api.GetBucketInfoWithLocation(ctx, bucketName) - if err != nil { + if _, err := api.objectAPI.GetBucketInfo(ctx, bucketName); err != nil { writeErrorResponseHeadersOnly(w, cmd.ToAPIError(ctx, err)) return } - // Set the bucket region header - if bucketInfo.Location != "" { - w.Header().Set(xhttp.AmzBucketRegion, bucketInfo.Location) - } else { - // Default to us-east-1 if location is not available - w.Header().Set(xhttp.AmzBucketRegion, "us-east-1") + // Try to get bucket location if the objectAPI supports it + if locationGetter, ok := api.objectAPI.(BucketLocationGetter); ok { + if location, err := locationGetter.GetBucketLocation(ctx, bucketName); err == nil && location != "" { + w.Header().Set(xhttp.AmzBucketRegion, location) + } } writeSuccessResponseHeadersOnly(w) diff --git a/api/datatypes.go b/api/datatypes.go index 0b799c4..f3d2269 100644 --- a/api/datatypes.go +++ b/api/datatypes.go @@ -22,19 +22,10 @@ package api import ( "encoding/xml" - "time" "storj.io/minio/cmd" ) -// BucketInfo represents bucket metadata. -// This struct extends the MinIO BucketInfo to include location/region information. -type BucketInfo struct { - Name string - Created time.Time - Location string -} - type grantee struct { XMLNS string `xml:"xmlns:xsi,attr"` XMLXSI string `xml:"xsi:type,attr"` diff --git a/miniogw/gateway.go b/miniogw/gateway.go index 9a38a61..bf7207d 100644 --- a/miniogw/gateway.go +++ b/miniogw/gateway.go @@ -335,14 +335,14 @@ func (layer *gatewayLayer) GetBucketInfo(ctx context.Context, bucketName string) return minio.BucketInfo{}, err } - bucketObj, err := project.StatBucket(ctx, bucketName) + bucket, err := project.StatBucket(ctx, bucketName) if err != nil { return minio.BucketInfo{}, ConvertError(err, bucketName, "") } return minio.BucketInfo{ - Name: bucketObj.Name, - Created: bucketObj.Created, + Name: bucket.Name, + Created: bucket.Created, }, nil } From 9b39c69b634a7cd37ff0451e7400fc18c81744b3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Feb 2026 17:50:23 +0000 Subject: [PATCH 5/6] Optimize HeadBucket to use single call when possible - Add TODO comment about adding GetBucketLocation to object layer interface in storj/minio - Make only one call (GetBucketLocation) when objectAPI implements BucketLocationGetter - Fallback to GetBucketInfo only when GetBucketLocation is not available - This reduces two sequential calls to one call in the common case Co-authored-by: amwolff <30234756+amwolff@users.noreply.github.com> --- api/buckethandlers.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/api/buckethandlers.go b/api/buckethandlers.go index c117e8f..4d9ccbf 100644 --- a/api/buckethandlers.go +++ b/api/buckethandlers.go @@ -302,16 +302,24 @@ func (api *API) HeadBucketHandler(w http.ResponseWriter, r *http.Request) { return } - if _, err := api.objectAPI.GetBucketInfo(ctx, bucketName); err != nil { - writeErrorResponseHeadersOnly(w, cmd.ToAPIError(ctx, err)) - return - } - - // Try to get bucket location if the objectAPI supports it + // TODO: Add GetBucketLocation to the object layer interface in storj/minio + // instead of using type assertion. This would allow us to make a single call + // instead of two separate calls (GetBucketInfo + GetBucketLocation). if locationGetter, ok := api.objectAPI.(BucketLocationGetter); ok { - if location, err := locationGetter.GetBucketLocation(ctx, bucketName); err == nil && location != "" { + location, err := locationGetter.GetBucketLocation(ctx, bucketName) + if err != nil { + writeErrorResponseHeadersOnly(w, cmd.ToAPIError(ctx, err)) + return + } + if location != "" { w.Header().Set(xhttp.AmzBucketRegion, location) } + } else { + // Fallback to GetBucketInfo if GetBucketLocation is not available + if _, err := api.objectAPI.GetBucketInfo(ctx, bucketName); err != nil { + writeErrorResponseHeadersOnly(w, cmd.ToAPIError(ctx, err)) + return + } } writeSuccessResponseHeadersOnly(w) From 6731fb1ce2078b5cd6fca376bf6ab7427cf8ed74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Feb 2026 21:48:46 +0000 Subject: [PATCH 6/6] Remove unnecessary /gateway entry from .gitignore The /gateway entry was added unintentionally and is not needed for this PR. The repository's .gitignore already has appropriate patterns for ignoring build artifacts. Co-authored-by: halkyon <138450+halkyon@users.noreply.github.com> --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 6f73efc..18fd1e0 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,3 @@ release/ # VS Code .vscode/ -/gateway