CORE-2157: cloud_storage_clients: add support for path-style addressing #17806
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,20 +73,26 @@ request_creator::request_creator( | |
| const s3_configuration& conf, | ||
| ss::lw_shared_ptr<const cloud_roles::apply_credentials> apply_credentials) | ||
| : _ap(conf.uri) | ||
| , _ap_style(conf.url_style) | ||
| , _apply_credentials{std::move(apply_credentials)} {} | ||
|
|
||
| result<http::client::request_header> request_creator::make_get_object_request( | ||
| bucket_name const& name, | ||
| object_key const& key, | ||
| std::optional<http_byte_range> byte_range) { | ||
| http::client::request_header header{}; | ||
| // Virtual Style: | ||
| // GET /{object-id} HTTP/1.1 | ||
| // Host: {bucket-name}.s3.{region}.amazonaws.com | ||
| // Path Style: | ||
| // GET /{bucket-name}/{object-id} HTTP/1.1 | ||
| // Host: s3.{region}.amazonaws.com | ||
| // | ||
| // x-amz-date:{req-datetime} | ||
| // Authorization:{signature} | ||
| // x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 | ||
| auto host = make_host(name); | ||
| auto target = make_target(name, key); | ||
| header.method(boost::beast::http::verb::get); | ||
| header.target(target); | ||
| header.insert( | ||
|
|
@@ -111,13 +117,18 @@ result<http::client::request_header> request_creator::make_get_object_request( | |
| result<http::client::request_header> request_creator::make_head_object_request( | ||
| bucket_name const& name, object_key const& key) { | ||
| http::client::request_header header{}; | ||
| // Virtual Style: | ||
| // HEAD /{object-id} HTTP/1.1 | ||
| // Host: {bucket-name}.s3.{region}.amazonaws.com | ||
| // Path Style: | ||
| // HEAD /{bucket-name}/{object-id} HTTP/1.1 | ||
| // Host: s3.{region}.amazonaws.com | ||
| // | ||
| // x-amz-date:{req-datetime} | ||
| // Authorization:{signature} | ||
| // x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 | ||
| auto host = make_host(name); | ||
| auto target = make_target(name, key); | ||
| header.method(boost::beast::http::verb::head); | ||
| header.target(target); | ||
| header.insert( | ||
|
|
@@ -134,8 +145,13 @@ result<http::client::request_header> request_creator::make_head_object_request( | |
| result<http::client::request_header> | ||
| request_creator::make_unsigned_put_object_request( | ||
| bucket_name const& name, object_key const& key, size_t payload_size_bytes) { | ||
| // Virtual Style: | ||
| // PUT /my-image.jpg HTTP/1.1 | ||
| // Host: {bucket-name}.s3.{region}.amazonaws.com | ||
| // Path Style: | ||
| // PUT /{bucket-name}/{object-id} HTTP/1.1 | ||
| // Host: s3.{region}.amazonaws.com | ||
| // | ||
| // Date: Wed, 12 Oct 2009 17:50:00 GMT | ||
| // Authorization: authorization string | ||
| // Content-Type: text/plain | ||
|
|
@@ -144,8 +160,8 @@ request_creator::make_unsigned_put_object_request( | |
| // Expect: 100-continue | ||
| // [11434 bytes of object data] | ||
| http::client::request_header header{}; | ||
| auto host = make_host(name); | ||
| auto target = make_target(name, key); | ||
| header.method(boost::beast::http::verb::put); | ||
| header.target(target); | ||
| header.insert( | ||
|
|
@@ -172,19 +188,25 @@ request_creator::make_list_objects_v2_request( | |
| std::optional<size_t> max_keys, | ||
| std::optional<ss::sstring> continuation_token, | ||
| std::optional<char> delimiter) { | ||
| // Virtual Style: | ||
| // GET /?list-type=2&prefix=photos/2006/&delimiter=/ HTTP/1.1 | ||
| // Host: {bucket-name}.s3.{region}.amazonaws.com | ||
| // Path Style: | ||
| // GET /{bucket-name}/?list-type=2&prefix=photos/2006/&delimiter=/ HTTP/1.1 | ||
| // Host: s3.{region}.amazonaws.com | ||
| // | ||
| // x-amz-date: 20160501T000433Z | ||
| // Authorization: authorization string | ||
| http::client::request_header header{}; | ||
| auto host = make_host(name); | ||
| auto key = fmt::format("?list-type=2"); | ||
| if (prefix.has_value()) { | ||
| key = fmt::format("{}&prefix={}", key, (*prefix)().string()); | ||
| } | ||
| if (delimiter.has_value()) { | ||
| key = fmt::format("{}&delimiter={}", key, *delimiter); | ||
| } | ||
| auto target = make_target(name, object_key{key}); | ||
| header.method(boost::beast::http::verb::get); | ||
| header.target(target); | ||
| header.insert( | ||
|
|
@@ -224,15 +246,20 @@ result<http::client::request_header> | |
| request_creator::make_delete_object_request( | ||
| bucket_name const& name, object_key const& key) { | ||
| http::client::request_header header{}; | ||
| // Virtual Style: | ||
| // DELETE /{object-id} HTTP/1.1 | ||
| // Host: {bucket-name}.s3.amazonaws.com | ||
| // Path Style: | ||
| // DELETE /{bucket-name}/{object-id} HTTP/1.1 | ||
| // Host: s3.{region}.amazonaws.com | ||
| // | ||
| // x-amz-date:{req-datetime} | ||
| // Authorization:{signature} | ||
| // x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 | ||
| // | ||
| // NOTE: x-amz-mfa, x-amz-bypass-governance-retention are not used for now | ||
| auto host = make_host(name); | ||
| auto target = make_target(name, key); | ||
| header.method(boost::beast::http::verb::delete_); | ||
| header.target(target); | ||
| header.insert( | ||
|
|
@@ -262,8 +289,13 @@ request_creator::make_delete_objects_request( | |
| // https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html | ||
| // will generate this request: | ||
| // | ||
| // Virtual Style: | ||
| // POST /?delete HTTP/1.1 | ||
| // Host: {bucket-name}.s3.{region}.amazonaws.com | ||
| // Path Style: | ||
| // POST /{bucket-name}/?delete HTTP/1.1 | ||
| // Host: s3.{region}.amazonaws.com | ||
| // | ||
| // Content-MD5: <Computer from body> | ||
| // Authorization: <applied by _requestor> | ||
| // Content-Length: <...> | ||
|
|
@@ -317,11 +349,12 @@ request_creator::make_delete_objects_request( | |
| return bytes_to_base64(to_bytes_view(bin_digest)); | ||
| }(); | ||
|
|
||
| http::client::request_header header{}; | ||
| header.method(boost::beast::http::verb::post); | ||
| auto host = make_host(name); | ||
| auto target = make_target(name, object_key{"?delete"}); | ||
| header.target(target); | ||
| header.insert(boost::beast::http::field::host, host); | ||
| // from experiments, minio is sloppy in checking this field. It will check | ||
| // that it's valid base64, but seems not to actually check the value | ||
| header.insert( | ||
|
|
@@ -343,6 +376,29 @@ request_creator::make_delete_objects_request( | |
| std::make_unique<delete_objects_body>(std::move(body))}}}; | ||
| } | ||
|
|
||
| std::string request_creator::make_host(const bucket_name& name) const { | ||
| switch (_ap_style) { | ||
| case s3_url_style::virtual_host: | ||
| // Host: bucket-name.s3.region-code.amazonaws.com | ||
| return fmt::format("{}.{}", name(), _ap()); | ||
| case s3_url_style::path: | ||
| // Host: s3.region-code.amazonaws.com | ||
| return fmt::format("{}", _ap()); | ||
|
There was a problem hiding this comment. why the bucket name is not included? There was a problem hiding this comment. Following the S3 User Guide's examples of path-style requests, this seems to be the proper form for the host in the header. The bucket name is instead used in the target of the header. In commit 8edf7d5, I added some comments in the various There was a problem hiding this comment.
I was also confused here. But the answer is that this is There was a problem hiding this comment. Yes, correct. This is |
||
| } | ||
| } | ||
|
|
||
| std::string request_creator::make_target( | ||
| const bucket_name& name, const object_key& key) const { | ||
| switch (_ap_style) { | ||
| case s3_url_style::virtual_host: | ||
| // Target: /homepage.html | ||
| return fmt::format("/{}", key().string()); | ||
| case s3_url_style::path: | ||
| // Target: /example.com/homepage.html | ||
| return fmt::format("/{}/{}", name(), key().string()); | ||
| } | ||
| } | ||
|
|
||
| // client // | ||
|
|
||
| inline cloud_storage_clients::s3_error_code | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| #include "bytes/iobuf.h" | ||
| #include "cloud_storage_clients/s3_client.h" | ||
| #include "cloud_storage_clients/s3_error.h" | ||
| #include "cloud_storage_clients/types.h" | ||
| #include "http/client.h" | ||
| #include "syschecks/syschecks.h" | ||
|
|
||
|
|
@@ -76,6 +77,11 @@ void cli_opts(boost::program_options::options_description_easy_init opt) { | |
| po::value<std::string>()->default_value("us-east-1"), | ||
| "aws region"); | ||
|
|
||
| opt( | ||
| "url_style", | ||
| po::value<std::string>()->default_value("virtual_host"), | ||
|
There was a problem hiding this comment. This is good for development but we also need a proper unit-test. In the The motivation here is that some cloud storage providers may not support one style or the other. In this situation we don't want to accidentally use the wrong style. There was a problem hiding this comment. Looks like all existing REST API's that we're using are supported. But we can add new request in the future so some check is nice to have. There was a problem hiding this comment. Could be done in a followup. There was a problem hiding this comment.
@Lazin Yes, and we are also going to wire up ducktape wiht minio running in both modes (pretty sure it supports it). You can see all teh follow up work in CORE-725. @WillemKauf can you capture Evgeny's suggestions about additional unit testing in a new or existing jira ticket as a subtask for the overall project? |
||
| "aws addressing style"); | ||
|
|
||
| opt( | ||
| "in", | ||
| po::value<std::string>()->default_value(""), | ||
|
|
@@ -120,9 +126,10 @@ struct fmt::formatter<test_conf> : public fmt::formatter<std::string_view> { | |
| // make the output json-able so we can consume it in python for analysis | ||
| return formatter<std::string_view>::format( | ||
| fmt::format( | ||
| "[ 'bucket': '{}', 'objects': ['{}'], 'path style': {} ]", | ||
| cfg.bucket, | ||
| fmt::join(cfg.objects, "', '"), | ||
| cfg.client_cfg.url_style), | ||
| ctx); | ||
| } | ||
| }; | ||
|
|
@@ -163,6 +170,13 @@ test_conf cfg_from(boost::program_options::variables_map& m) { | |
| } | ||
| return std::nullopt; | ||
| }(), | ||
| .url_style = [&]() -> cloud_storage_clients::s3_url_style { | ||
| if (m["url_style"].as<std::string>() == "virtual_host") { | ||
| return cloud_storage_clients::s3_url_style::virtual_host; | ||
| } else { | ||
| return cloud_storage_clients::s3_url_style::path; | ||
| } | ||
| }(), | ||
| .disable_tls = m.contains("disable-tls") > 0, | ||
| }) | ||
| .get0(); | ||
|
|
@@ -358,7 +372,7 @@ int main(int args, char** argv, char** env) { | |
| } else { | ||
| vlog( | ||
| test_log.error, | ||
| "DeleteObject request failed: {}", | ||
| undeleted.error()); | ||
| } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comments!