diff --git a/cloud/src/recycler/s3_accessor.cpp b/cloud/src/recycler/s3_accessor.cpp index 2c983a5fa0617b..1aca88d2d1161d 100644 --- a/cloud/src/recycler/s3_accessor.cpp +++ b/cloud/src/recycler/s3_accessor.cpp @@ -205,6 +205,7 @@ std::optional 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( 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(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 6886ee5e7c5640..e9640b5693a1f5 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 53fa821c7e5503..c8dcdad18d7115 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 0dd51b749d86e2..c19f5f6a1dfdfb 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 #include #include +#include #include #include @@ -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 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> 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(args[0]); + EXPECT_EQ(std::get<2>(inputs[case_idx]), static_cast(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 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