This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 1a03fed4199 [fix](recycler) Process self-defined domain names for s3
storage vault (#45072)
1a03fed4199 is described below
commit 1a03fed419975db54d417b71bc457667ab3184db
Author: Gavin Chou <[email protected]>
AuthorDate: Fri Dec 6 14:40:39 2024 +0800
[fix](recycler) Process self-defined domain names for s3 storage vault
(#45072)
---
cloud/src/recycler/s3_accessor.cpp | 3 +-
cloud/src/recycler/s3_accessor.h | 1 +
cloud/src/recycler/s3_obj_client.cpp | 1 +
cloud/test/s3_accessor_test.cpp | 68 ++++++++++++++++++++++++++++++++++++
4 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/cloud/src/recycler/s3_accessor.cpp
b/cloud/src/recycler/s3_accessor.cpp
index 2c983a5fa06..1aca88d2d11 100644
--- a/cloud/src/recycler/s3_accessor.cpp
+++ b/cloud/src/recycler/s3_accessor.cpp
@@ -205,6 +205,7 @@ std::optional<S3Conf> S3Conf::from_obj_store_info(const
ObjectStoreInfoPB& obj_i
s3_conf.region = obj_info.region();
s3_conf.bucket = obj_info.bucket();
s3_conf.prefix = obj_info.prefix();
+ s3_conf.use_virtual_addressing = !obj_info.use_path_style();
return s3_conf;
}
@@ -289,7 +290,7 @@ int S3Accessor::init() {
auto s3_client = std::make_shared<Aws::S3::S3Client>(
std::move(aws_cred), std::move(aws_config),
Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
- true /* useVirtualAddressing */);
+ conf_.use_virtual_addressing /* useVirtualAddressing */);
obj_client_ = std::make_shared<S3ObjClient>(std::move(s3_client),
conf_.endpoint);
return 0;
}
diff --git a/cloud/src/recycler/s3_accessor.h b/cloud/src/recycler/s3_accessor.h
index 6886ee5e7c5..e9640b5693a 100644
--- a/cloud/src/recycler/s3_accessor.h
+++ b/cloud/src/recycler/s3_accessor.h
@@ -69,6 +69,7 @@ struct S3Conf {
std::string region;
std::string bucket;
std::string prefix;
+ bool use_virtual_addressing {true};
enum Provider : uint8_t {
S3,
diff --git a/cloud/src/recycler/s3_obj_client.cpp
b/cloud/src/recycler/s3_obj_client.cpp
index 53fa821c7e5..c8dcdad18d7 100644
--- a/cloud/src/recycler/s3_obj_client.cpp
+++ b/cloud/src/recycler/s3_obj_client.cpp
@@ -284,6 +284,7 @@ ObjectStorageResponse
S3ObjClient::delete_object(ObjectStoragePathRef path) {
SCOPED_BVAR_LATENCY(s3_bvar::s3_delete_object_latency);
return s3_client_->DeleteObject(request);
});
+ TEST_SYNC_POINT_CALLBACK("S3ObjClient::delete_object", &outcome);
if (!outcome.IsSuccess()) {
LOG_WARNING("failed to delete object")
.tag("endpoint", endpoint_)
diff --git a/cloud/test/s3_accessor_test.cpp b/cloud/test/s3_accessor_test.cpp
index 0dd51b749d8..c19f5f6a1df 100644
--- a/cloud/test/s3_accessor_test.cpp
+++ b/cloud/test/s3_accessor_test.cpp
@@ -17,8 +17,10 @@
#include "recycler/s3_accessor.h"
+#include <aws/s3/S3Client.h>
#include <aws/s3/model/ListObjectsV2Request.h>
#include <butil/guid.h>
+#include <gen_cpp/cloud.pb.h>
#include <gtest/gtest.h>
#include <azure/storage/blobs/blob_options.hpp>
@@ -320,4 +322,70 @@ TEST(S3AccessorTest, gcs) {
test_s3_accessor(*accessor);
}
+TEST(S3AccessorTest, path_style_test) {
+ ObjectStoreInfoPB obj_info;
+ obj_info.set_prefix("doris-debug-instance-prefix");
+ obj_info.set_provider(ObjectStoreInfoPB_Provider_S3);
+ obj_info.set_ak("dummy_ak");
+ obj_info.set_sk("dummy_sk");
+ obj_info.set_endpoint("dummy-bucket");
+ obj_info.set_region("cn-north-1");
+ obj_info.set_bucket("dummy-bucket");
+ config::max_s3_client_retry = 0;
+
+ auto* sp = SyncPoint::get_instance();
+ sp->enable_processing();
+ std::vector<SyncPoint::CallbackGuard> guards;
+
+ std::string base_domain = "s3.cn-north-1.amazonaws.com.cn";
+ std::string domain_ip = "54.222.51.71"; // first ip of base_domain
+ // to test custom_domain, add ${domain_ip} ${custom_domain} to /etc/hosts
+ // otherwise the related cases will fail
+ std::string custom_domain = "gavin.s3.aws.com";
+ // clang-format off
+ // http code 403 means there is nothing wrong the given domain in objinfo
+ // domain, use_path_style, http_code
+ std::vector<std::tuple<std::string, bool, int>> inputs {
+ {base_domain , false , 403}, // works
+ {base_domain , true , 403}, // works
+ {"http://"; + base_domain , false , 403}, // works
+ {"http://"; + base_domain , true , 403}, // works
+ {"https://"; + base_domain , false , 403}, // works
+ {"https://"; + base_domain , true , 403}, // works
+ {"http://"; + domain_ip , false , 301}, // works, ip with virtual
addressing
+ {"http://"; + domain_ip , true , 301}, // works, ip with path style
+ {custom_domain , false , -1} , // custom_domain could not
resolve with virtual addressing
+ {custom_domain , true , 403}, // custom_domain working
with path style
+ {"http://"; + custom_domain , false , -1} , // custom_domain could not
resolve with virtual addressing
+ {"https://"; + custom_domain, true , -1}, // certificate issue,
custom_domain does not attached with any certs
+ // {"https://54.222.51.71"; , false , -1} , // certificate issue
+ // {"https://54.222.51.71"; , true , -1} , // certificate issue
+ };
+
+ int case_idx = 0;
+ sp->set_call_back("S3ObjClient::delete_object",
+ [&case_idx, &inputs](auto&& args) {
+ auto* res =
try_any_cast<Aws::S3::Model::DeleteObjectOutcome*>(args[0]);
+ EXPECT_EQ(std::get<2>(inputs[case_idx]),
static_cast<int>(res->GetError().GetResponseCode())) << "<<<<<<<<<<<<<<<<<<<<<
" << case_idx;
+ case_idx++;
+ },
+ &guards.emplace_back());
+ // clang-format on
+
+ for (auto& i : inputs) {
+ obj_info.set_endpoint(std::get<0>(i));
+ obj_info.set_use_path_style(std::get<1>(i));
+ auto s3_conf = S3Conf::from_obj_store_info(obj_info);
+ EXPECT_EQ(s3_conf->use_virtual_addressing, !obj_info.use_path_style())
<< case_idx;
+ std::shared_ptr<S3Accessor> accessor;
+ int ret = S3Accessor::create(*s3_conf, &accessor);
+ EXPECT_EQ(ret, 0) << "<<<<<<<<<<<<<<<<<<<<< " << case_idx;
+ ret = accessor->init();
+ EXPECT_EQ(ret, 0) << "<<<<<<<<<<<<<<<<<<<<< " << case_idx;
+ // this function call will trigger syncpoint callback to increment
case_idx
+ accessor->delete_file("abc"); // try to delete a nonexisted file,
ignore the result
+ // EXPECT_EQ(ret, exp) << "<<<<<<<<<<<<<<<<<<<<< " << case_idx << "
domain " << std::get<0>(i);
+ }
+}
+
} // namespace doris::cloud
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
