Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: getBucketLocation using virtual hosted style for Ksyun KS3 #1784

Closed
wants to merge 2 commits into from

Conversation

ChaosBee
Copy link

@ChaosBee ChaosBee commented Mar 3, 2023

Ksyun KS3 —— Object Storage Service by Kingsoft Cloud.

The getBucketLocation API is accessed using virtual hosted style in KS3 by default.

refer: #1253

@harshavardhana
Copy link
Member

We are not going to add more endpoints here. Aliyun is where we will stop.

@ChaosBee
Copy link
Author

ChaosBee commented Mar 4, 2023

OK. Thank you for your response.
What a pity. I believe that host-style access is better than path-style access. So is it possible that minio-sdk or minio will add a variable changing the style of get-bucket-location?

@harshavardhana
Copy link
Member

All you need to do is tell how the bucket lookups must happen

https://github.com/minio/minio-go/blob/master/api.go#LL107C6-L107C6

There is no reason to have new magic added into codebase. However I would perhaps go ahead and allow extending the checks to ensure users pass their own functions here instead of polluting the SDK further.

@ChaosBee
Copy link
Author

ChaosBee commented Mar 4, 2023

As your mention, I tried the BucketLoookup Variable before, but it does no effect on the urlStr that get_bucket_location use.

https://github.com/minio/minio-go/blob/master/bucket-cache.go#L201

@harshavardhana
Copy link
Member

Yes that is correct because an S3 service must implement both path and dns style. The region is not known prior to getBucketLocation() - however can fix the behavior here. to use DNS style for GetBucketLocation() requests if it works?

@harshavardhana
Copy link
Member

#1785 this should work for you @ChaosBee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants