[den-dw]

Fixes #276

Requests to storages were signed with us-east-1 regardless of the storage defaultRegion. Providers that validate the SigV4 credential scope (e.g. Cloud.ru Object Storage) answer AuthorizationHeaderMalformed: unexpected region, expecting '<region>' (or a bare 400 Bad Request for HEAD), so replication to such a storage fails: first bucket:create, then object:sync (unable to get to object stat: 400 Bad Request) and tag sync.

Three places:

• the AWS SDK client in pkg/s3client/aws_wrapper.go (worker HeadBucket/CreateBucket, ACL, tagging) — hardcoded us-east-1;
• proxy request forwarding in pkg/s3client/http_forward.go — hardcoded us-east-1;
• the minio client in pkg/s3client/client.go — created without Region, so minio-go looks the region up with GetBucketLocation signed with us-east-1, which those providers reject.

Changes:

• signingRegion() returns the storage defaultRegion, or us-east-1 when it is empty; the AWS SDK client and the proxy forwarding use it;
• the minio client gets Region: defaultRegion; when it is empty the lookup works as before (no us-east-1 is forced, so AWS buckets in other regions keep working);
• isOnline uses BucketExists (HEAD): with a configured region GetBucketLocation answers locally and would no longer check that the storage is reachable.

Tests: region choice, signature scope of the AWS client, and an httptest storage asserting that with defaultRegion the minio client signs with that region, never calls ?location, and the online check still reaches the storage (fails without the change). go test ./pkg/s3client/... ./pkg/s3/... ./pkg/objstore/... ./service/proxy/... ./service/worker/handler/..., go vet, gofmt.

[arttor]

please also rebase onto main to make tests pass on CI

[arttor]

Thanks for adding tests. In this repo storage behaviour is tested with e2e tests under test/ against real storages running in containers, not with a hand-written fake S3 server. test/env/components.go already provides Garage, SeaweedFS, MinIO, gofakes3 and Ceph RGW. test/minio/presign_test.go is an example of such a test.

Can you replace region_test.go with an e2e test against one of them configured with a region? You'll probably need to add an option in test/env to set the region on the container (e.g. via an env var). The test should cover both cases from the issue - replication to that storage and requests through the proxy - and fail on main.

[arttor]

Let's not switch the health check to HEAD: HEAD errors have no body, so wrong credentials come back as a plain 403 which will now make the check pass for AccessDenied. Am I correct that the problem with keeping GetBucketLocation is that minio answers it from the configured region without sending a request?

Can you use GetBucketVersioning("probe-health-test") instead, which always reaches the storage? Also, to be safe with storages that don't implement every API, can the check treat any S3 error response as online, and fail only on network errors and credential/signature errors (InvalidAccessKeyId, SignatureDoesNotMatch, AuthorizationHeaderMalformed)?

[den-dw]

Thanks for the review!

Yes, that's right: when the client has a region, minio-go's getBucketLocation returns c.region without sending a request, so the online check never reached the storage.

• Online check: it now calls GetBucketVersioning("probe-health-test"). Network errors, and InvalidAccessKeyId / SignatureDoesNotMatch / AuthorizationHeaderMalformed, mean the storage is offline. Any other S3 error response means it is online. A storage that requires a region but has no defaultRegion is now reported offline at startup, instead of failing on every request later.

• Tests: I removed region_test.go. test/env has a new WithRegion option that starts MinIO with MINIO_SITE_REGION, so it rejects requests signed for any other region. The new Test_e2e_storage_region in test/minio covers:

  • replication to that storage;
  • requests through the proxy with it as main;
  • the online check with valid and wrong credentials.

  On main the replication case fails: bucket:create keeps being retried with AuthorizationHeaderMalformed: ... expecting 'test-region-1'. The proxy case fails too. Both pass with this PR.

I also rebased the branch onto main.