Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cloud/src/recycler/s3_accessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions cloud/src/recycler/s3_accessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions cloud/src/recycler/s3_obj_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_)
Expand Down
68 changes: 68 additions & 0 deletions cloud/test/s3_accessor_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Loading