From 96b948c89818e03d7ce99d0c20a7ad2e86ad546b Mon Sep 17 00:00:00 2001 From: Gavin Chou Date: Sun, 8 Dec 2024 04:26:23 +0800 Subject: [PATCH 1/2] [fix](storage vault) Fix missing use_path_style when create storage vault --- .../meta-service/meta_service_resource.cpp | 27 ++++++----- .../apache/doris/catalog/StorageVault.java | 4 ++ .../property/constants/S3Properties.java | 3 ++ .../suites/vault_p0/create/load.groovy | 48 +++++++++++++++++++ 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index cc459c090bfd28..f26c2d26200dd3 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -753,6 +753,7 @@ struct ObjectStorageDesc { std::string& endpoint; std::string& external_endpoint; std::string& region; + bool& use_path_style; }; static int extract_object_storage_info(const AlterObjStoreInfoRequest* request, @@ -765,7 +766,7 @@ static int extract_object_storage_info(const AlterObjStoreInfoRequest* request, msg = "s3 obj info err " + proto_to_json(*request); return -1; } - auto& [ak, sk, bucket, prefix, endpoint, external_endpoint, region] = obj_desc; + auto& [ak, sk, bucket, prefix, endpoint, external_endpoint, region, use_path_style] = obj_desc; const auto& obj = request->has_obj() ? request->obj() : request->vault().obj_info(); // Prepare data if (!obj.has_ak() || !obj.has_sk()) { @@ -791,6 +792,7 @@ static int extract_object_storage_info(const AlterObjStoreInfoRequest* request, endpoint = obj.has_endpoint() ? obj.endpoint() : ""; external_endpoint = obj.has_external_endpoint() ? obj.external_endpoint() : ""; region = obj.has_region() ? obj.region() : ""; + use_path_style = obj.use_path_style(); // obj size > 1k, refuse if (obj.ByteSizeLong() > 1024) { code = MetaServiceCode::INVALID_ARGUMENT; @@ -800,13 +802,13 @@ static int extract_object_storage_info(const AlterObjStoreInfoRequest* request, return 0; } -static ObjectStoreInfoPB object_info_pb_factory(ObjectStorageDesc& obj_info, +static ObjectStoreInfoPB object_info_pb_factory(ObjectStorageDesc& obj_desc, const ObjectStoreInfoPB& obj, InstanceInfoPB& instance, EncryptionInfoPB& encryption_info, AkSkPair& cipher_ak_sk_pair) { ObjectStoreInfoPB last_item; - auto& [ak, sk, bucket, prefix, endpoint, external_endpoint, region] = obj_info; + auto& [ak, sk, bucket, prefix, endpoint, external_endpoint, region, use_path_style] = obj_desc; auto now_time = std::chrono::system_clock::now(); uint64_t time = std::chrono::duration_cast(now_time.time_since_epoch()).count(); @@ -828,6 +830,7 @@ static ObjectStoreInfoPB object_info_pb_factory(ObjectStorageDesc& obj_info, last_item.set_region(region); last_item.set_provider(obj.provider()); last_item.set_sse_enabled(instance.sse_enabled()); + last_item.set_use_path_style(use_path_style); return last_item; } @@ -836,14 +839,15 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr AlterObjStoreInfoResponse* response, ::google::protobuf::Closure* done) { std::string ak, sk, bucket, prefix, endpoint, external_endpoint, region; + bool use_path_style; EncryptionInfoPB encryption_info; AkSkPair cipher_ak_sk_pair; RPC_PREPROCESS(alter_storage_vault); switch (request->op()) { case AlterObjStoreInfoRequest::ADD_S3_VAULT: case AlterObjStoreInfoRequest::DROP_S3_VAULT: { - auto tmp_desc = - ObjectStorageDesc {ak, sk, bucket, prefix, endpoint, external_endpoint, region}; + auto tmp_desc = ObjectStorageDesc { + ak, sk, bucket, prefix, endpoint, external_endpoint, region, use_path_style}; if (0 != extract_object_storage_info(request, code, msg, tmp_desc, encryption_info, cipher_ak_sk_pair)) { return; @@ -982,8 +986,8 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr } } // calc id - auto tmp_tuple = - ObjectStorageDesc {ak, sk, bucket, prefix, endpoint, external_endpoint, region}; + auto tmp_tuple = ObjectStorageDesc { + ak, sk, bucket, prefix, endpoint, external_endpoint, region, use_path_style}; ObjectStoreInfoPB last_item = object_info_pb_factory(tmp_tuple, obj, instance, encryption_info, cipher_ak_sk_pair); if (instance.storage_vault_names().end() != @@ -1126,6 +1130,7 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont AlterObjStoreInfoResponse* response, ::google::protobuf::Closure* done) { std::string ak, sk, bucket, prefix, endpoint, external_endpoint, region; + bool use_path_style; EncryptionInfoPB encryption_info; AkSkPair cipher_ak_sk_pair; RPC_PREPROCESS(alter_obj_store_info); @@ -1133,8 +1138,8 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont case AlterObjStoreInfoRequest::ADD_OBJ_INFO: case AlterObjStoreInfoRequest::LEGACY_UPDATE_AK_SK: case AlterObjStoreInfoRequest::UPDATE_AK_SK: { - auto tmp_desc = - ObjectStorageDesc {ak, sk, bucket, prefix, endpoint, external_endpoint, region}; + auto tmp_desc = ObjectStorageDesc { + ak, sk, bucket, prefix, endpoint, external_endpoint, region, use_path_style}; if (0 != extract_object_storage_info(request, code, msg, tmp_desc, encryption_info, cipher_ak_sk_pair)) { return; @@ -1273,8 +1278,8 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont } } // calc id - auto tmp_tuple = - ObjectStorageDesc {ak, sk, bucket, prefix, endpoint, external_endpoint, region}; + auto tmp_tuple = ObjectStorageDesc { + ak, sk, bucket, prefix, endpoint, external_endpoint, region, use_path_style}; ObjectStoreInfoPB last_item = object_info_pb_factory(tmp_tuple, obj, instance, encryption_info, cipher_ak_sk_pair); instance.add_obj_info()->CopyFrom(last_item); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java index df9310526e4757..9d45ce7bdd8f51 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java @@ -23,6 +23,7 @@ import org.apache.doris.common.AnalysisException; import org.apache.doris.common.DdlException; import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.PropertyConverter; import org.apache.doris.qe.ShowResultSetMetaData; import com.google.common.base.Strings; @@ -144,6 +145,9 @@ public void setId(String id) { vault.modifyProperties(stmt.getProperties()); break; case S3: + if (!stmt.getProperties().containsKey(PropertyConverter.USE_PATH_STYLE)) { + stmt.getProperties().put(PropertyConverter.USE_PATH_STYLE, "true"); + } CreateResourceStmt resourceStmt = new CreateResourceStmt(false, ifNotExists, name, stmt.getProperties()); resourceStmt.analyzeResourceType(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java index 7e94d81518ec0a..18286b2fbc3cb5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java @@ -343,6 +343,9 @@ public static Cloud.ObjectStoreInfoPB.Builder getObjStoreInfoPB(Map= 3); + for (int i = 0; i < result.size(); ++i) { + if (result[i][0].equals("create_s3_vault")) { + assertTrue(result[i][2].contains("use_path_style: true")); + } + if (result[i][0].equals("create_s3_vault1")) { + assertTrue(result[i][2].contains("use_path_style: true")); + } + if (result[i][0].equals("create_s3_vault2")) { + assertTrue(result[i][2].contains("use_path_style: false")); + } + } + expectExceptionLike({ sql """ CREATE STORAGE VAULT create_s3_vault From dbe00c233e8adff942410d6ebddbbbeb0af32ef1 Mon Sep 17 00:00:00 2001 From: Gavin Chou Date: Sun, 8 Dec 2024 05:25:40 +0800 Subject: [PATCH 2/2] Update fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java --- .../doris/datasource/property/constants/S3Properties.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java index 18286b2fbc3cb5..7e94d81518ec0a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java @@ -343,9 +343,6 @@ public static Cloud.ObjectStoreInfoPB.Builder getObjStoreInfoPB(Map