grafana / k6-jslib-aws

Javascript Library allowing to interact with AWS resources from k6 scripts
Apache License 2.0
18 stars 29 forks source link

Add Ceph RGW S3 compatibility. #21

Closed ismaelpuerto closed 1 year ago

ismaelpuerto commented 1 year ago

Support to use ceph o minio as S3 provider

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

oleiade commented 1 year ago

Hi @ismaelpuerto

Thanks a lot for your contribution 🙇🏻 🎉

First and foremost, could you give me a quick run-through of what Ceph is as I'm not familiar with it?

I see most of the changes in your PR are related to having the ability to set the target host manually. We have been working on improving the signature code, and have introduced a settable host for all AWSClient inherited classes, such as S3Client. I believe this would address what you're trying to achieve.

This is a matter of days before we merge it to master, thus, I'd like for us to wait until it's the case to reevaluate this PR. I think once the changes are merged, you might be able to perform the actions you want to perform, without any custom code in the library.

Let me know what you think 🙇🏻

ismaelpuerto commented 1 year ago

Hello @oleiade

Thanks for your fast reply, Ceph it's a software-defined storage with the capacity to expose S3. The structure of url in ceph is:

http(s)?://hostname/bucketname/file1.txt

It can serve this at amazon style but require additional configuration in DNS. More info in this link https://docs.ceph.com/en/quincy/radosgw/s3/commons/

Happen the same with minio

I didn't see the branch when I did this PR, but I thing that cover my case. I suppose that the new branch it's a WIP but I can test and give you feedback. Be free if you want close this PR.

Thanks

jayush12 commented 1 year ago

Thanks for the PR @ismaelpuerto! 🙇

I would prefer making these changes agnostic to any S3-compatible API, so that we could support MinIO as well, like you mention. And it should also have documentation in the README for how to use it, in a separate section (e.g. "Support for S3-compatible APIs"), with examples for Ceph and MinIO.

EDIT: Apologies, I didn't read @oleiade's response above in detail. In that case, if host can be manually set, then it should work for all S3-compatible APIs. So, indeed, let's wait until those changes are merged.

Hello @imiric ,

Any update when the new changes with capability to set host will be available for public use. And are you considering about giving capability to set endpoint and port also.

Also for setting host externally, will it work for min.io if we pass host url along with port

oleiade commented 1 year ago

Hey folks @jayush12 @ismaelpuerto

Sorry for the lack of recent updates on this. Although we initially intended to merge the signature rewrite sooner we weren't able to prioritize it yet. I expect we should be able to move forward with this in the next couple of weeks 👍🏻

oleiade commented 1 year ago

Hi folks, we have published version v0.7.0 of the sdk, which supports settings a client's host directly. We do it in the context of our tests [here]().

@ismaelpuerto could you please rebase your PR on master, and let me know if your need is addressed by our recent changes? Otherwise I'm happy to discuss this PR further 🙇🏻

ismaelpuerto commented 1 year ago

Hello @oleiade

Thanks, I will work with the last version and come back with a new PR, additionally I will change the name of variable: rgw for forcePathStyle to be more generic as propose @imiric.

oleiade commented 1 year ago

As detailled in #26 I believe this has been addressed in #44.

Closing. Feel free to reopen if you're still experiencing the issue.