Fetch bucket location and include in HeadBucket response#107
Fetch bucket location and include in HeadBucket response#107
Conversation
- 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>
- 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>
amwolff
left a comment
There was a problem hiding this comment.
no, this won't cut it. we need to modify the implementation of the bucket info-gathering action to fetch information about the bucket along with its region and pass it down here. find out if there's a libuplink method that will allow us to do it in one libuplink call instead of two (bucket info + bucket location).
- 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>
Done in d743731. The implementation now:
There's no single libuplink method that returns both bucket info and location, so the implementation uses two separate calls as you suggested to investigate. |
- 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>
- 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>
|
@copilot CI is still failing on this (https://build.dev.storj.tools/job/gateway-st/job/PR-107/6/display/redirect) |
I've verified the code locally - it builds successfully, all tests pass, and linting checks (go vet, check-imports, check-copyright, check-mod-tidy) all pass. Without access to the full CI logs, I cannot determine the specific failure. Could you share the error message from the CI failure so I can address it? |
HeadBucket now returns the
x-amz-bucket-regionheader per AWS S3 specification with the actual bucket location fetched from libuplink.Changes
GetBucketLocationmethod that usesbucket.GetBucketLocationfrom libuplink to fetch the actual bucket locationBucketLocationGetterinterface for type assertion to access the gateway layer'sGetBucketLocationmethodHeadBucketHandlerto fetch actual bucket location and setx-amz-bucket-regionheader only when location is available. Optimized to use a single call toGetBucketLocationwhen available instead of two separate calls.Implementation Details
The implementation is optimized to use a single libuplink call when possible. When the object layer implements
BucketLocationGetter, it makes only one call toGetBucketLocation(which validates bucket exists AND returns location). It falls back toGetBucketInfoonly whenGetBucketLocationis not available. The handler uses type assertion to check if the object layer supports location fetching and only sets the header when a location is successfully retrieved. No default fallback value is used.A TODO comment has been added to track future work: adding
GetBucketLocationto the object layer interface in storj/minio would eliminate the need for type assertion and further streamline the implementation.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.