From caf9e357c80faaa59cc029aaa593dcbbeaa78124 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Mon, 3 Apr 2023 14:11:09 +0300 Subject: [PATCH] s3/client: Construct it with sstring endpoint Currently the client is constructed with socket_address which's prepared by the caller from the endpoint string. That's not flexible engouh, because s3 client needs to know the original endpoint string for two reasons. First, it needs to lookup endpoint config for potential AWS creds. Second, it needs this exact value as Host: header in its http requests. So this patch just relaxes the client constructor to accept the endpoint string and hard-code the 9000 port. The latter is temporary, this is how local tests' minio is started, but next patch will make it configurable. Signed-off-by: Pavel Emelyanov --- sstables/sstables.cc | 4 ++-- test/boost/s3_test.cc | 9 +++------ utils/s3/client.cc | 28 ++++++++++++++++------------ utils/s3/client.hh | 7 +++---- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/sstables/sstables.cc b/sstables/sstables.cc index 3e6cc61ed5..0ca9877d17 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -3298,8 +3298,8 @@ class sstable::s3_storage final : public sstable::storage { future<> ensure_remote_prefix(const sstable& sst); public: - s3_storage(sstring host, s3::endpoint_config_ptr cfg, sstring bucket, sstring dir) - : _client(s3::client::make(ipv4_addr(host))) + s3_storage(sstring endpoint, s3::endpoint_config_ptr cfg, sstring bucket, sstring dir) + : _client(s3::client::make(std::move(endpoint))) , _bucket(std::move(bucket)) , _location(std::move(dir)) { diff --git a/test/boost/s3_test.cc b/test/boost/s3_test.cc index 0e0819ebe7..a1ec96dd6f 100644 --- a/test/boost/s3_test.cc +++ b/test/boost/s3_test.cc @@ -26,11 +26,10 @@ */ SEASTAR_THREAD_TEST_CASE(test_client_put_get_object) { - const ipv4_addr s3_server(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST"), 9000); const sstring name(fmt::format("/{}/testobject-{}", tests::getenv_safe("S3_PUBLIC_BUCKET_FOR_TEST"), ::getpid())); testlog.info("Make client\n"); - auto cln = s3::client::make(s3_server); + auto cln = s3::client::make(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST")); testlog.info("Put object {}\n", name); temporary_buffer data = sstring("1234567890").release(); @@ -59,11 +58,10 @@ SEASTAR_THREAD_TEST_CASE(test_client_put_get_object) { } SEASTAR_THREAD_TEST_CASE(test_client_multipart_upload) { - const ipv4_addr s3_server(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST"), 9000); const sstring name(fmt::format("/{}/testlargeobject-{}", tests::getenv_safe("S3_PUBLIC_BUCKET_FOR_TEST"), ::getpid())); testlog.info("Make client\n"); - auto cln = s3::client::make(s3_server); + auto cln = s3::client::make(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST")); testlog.info("Upload object\n"); auto out = output_stream(cln->make_upload_sink(name)); @@ -110,11 +108,10 @@ SEASTAR_THREAD_TEST_CASE(test_client_multipart_upload) { } SEASTAR_THREAD_TEST_CASE(test_client_readable_file) { - const ipv4_addr s3_server(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST"), 9000); const sstring name(fmt::format("/{}/testroobject-{}", tests::getenv_safe("S3_PUBLIC_BUCKET_FOR_TEST"), ::getpid())); testlog.info("Make client\n"); - auto cln = s3::client::make(s3_server); + auto cln = s3::client::make(tests::getenv_safe("S3_SERVER_ADDRESS_FOR_TEST")); testlog.info("Put object {}\n", name); temporary_buffer data = sstring("1234567890ABCDEF").release(); diff --git a/utils/s3/client.cc b/utils/s3/client.cc index 584f55c115..aaa5a5ee45 100644 --- a/utils/s3/client.cc +++ b/utils/s3/client.cc @@ -8,8 +8,12 @@ #include #include +#include +#include #include #include +#include +#include #include #include #include "utils/s3/client.hh" @@ -39,15 +43,14 @@ future<> ignore_reply(const http::reply& rep, input_stream&& in_) { co_await util::skip_entire_stream(in); } -client::client(socket_address addr, private_tag) - : _addr(std::move(addr)) - , _host(to_sstring(_addr)) - , _http(_addr) +client::client(std::string host, private_tag) + : _host(std::move(host)) + , _http(ipv4_addr(_host, 9000 /* temporary hard-coded */)) { } -shared_ptr client::make(socket_address addr) { - return seastar::make_shared(std::move(addr), private_tag{}); +shared_ptr client::make(std::string endpoint) { + return seastar::make_shared(std::move(endpoint), private_tag{}); } future client::get_object_size(sstring object_name) { @@ -417,26 +420,27 @@ public: virtual future<> discard(uint64_t offset, uint64_t length) override { return make_ready_future<>(); } class readable_file_handle_impl final : public file_handle_impl { - socket_address _addr; + std::string _host; sstring _object_name; public: - readable_file_handle_impl(socket_address addr, sstring object_name) - : _addr(std::move(addr)) + readable_file_handle_impl(std::string host, sstring object_name) + : _host(std::move(host)) , _object_name(std::move(object_name)) {} virtual std::unique_ptr clone() const override { - return std::make_unique(_addr, _object_name); + return std::make_unique(_host, _object_name); } virtual shared_ptr to_file() && override { - return make_shared(client::make(std::move(_addr)), std::move(_object_name)); + auto client = seastar::make_shared(std::move(_host), client::private_tag{}); + return make_shared(std::move(client), std::move(_object_name)); } }; virtual std::unique_ptr dup() override { - return std::make_unique(_client->_addr, _object_name); + return std::make_unique(_client->_host, _object_name); } virtual future size(void) override { diff --git a/utils/s3/client.hh b/utils/s3/client.hh index 8ac527b9de..36407228f4 100644 --- a/utils/s3/client.hh +++ b/utils/s3/client.hh @@ -24,15 +24,14 @@ struct range { class client : public enable_shared_from_this { class upload_sink; class readable_file; - socket_address _addr; - sstring _host; + std::string _host; http::experimental::client _http; struct private_tag {}; public: - explicit client(socket_address addr, private_tag); - static shared_ptr make(socket_address addr); + explicit client(std::string host, private_tag); + static shared_ptr make(std::string endpoint); future get_object_size(sstring object_name); future> get_object_contiguous(sstring object_name, std::optional range = {});