From 2272c64e1552dae82d5ccad7a6e56fb77af4864c Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 04:40:51 +0800 Subject: [PATCH 01/12] [fix](vault) avoid encrypt twice when altering vault --- .../meta-service/meta_service_resource.cpp | 44 +++++--- .../vault_p0/alter/test_alter_s3_vault.groovy | 102 +++++++++++++++++- 2 files changed, 127 insertions(+), 19 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index cc459c090bfd28..df1546b1e7f655 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -648,10 +648,19 @@ static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptrbegin(), @@ -703,22 +712,25 @@ static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptrset_ak(cipher_ak_sk_pair.first); - new_vault.mutable_obj_info()->set_sk(cipher_ak_sk_pair.second); + + new_vault.mutable_obj_info()->set_ak(new_as_sk_pair.first); + new_vault.mutable_obj_info()->set_sk(new_as_sk_pair.second); new_vault.mutable_obj_info()->mutable_encryption_info()->CopyFrom(encryption_info); if (obj_info.has_use_path_style()) { new_vault.mutable_obj_info()->set_use_path_style(obj_info.use_path_style()); diff --git a/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy b/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy index 723422c6e0b84d..2d64e266798ee6 100644 --- a/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy +++ b/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy @@ -62,6 +62,25 @@ suite("test_alter_s3_vault", "nonConcurrent") { """ }, "Alter property") + expectExceptionLike({ + sql """ + ALTER STORAGE VAULT ${suiteName} + PROPERTIES ( + "type"="S3", + "s3.access_key" = "new_ak" + ); + """ + }, "Alter property") + + expectExceptionLike({ + sql """ + ALTER STORAGE VAULT ${suiteName} + PROPERTIES ( + "type"="S3", + "s3.secret_key" = "new_sk" + ); + """ + }, "Alter property") def vaultName = suiteName String properties; @@ -75,20 +94,97 @@ suite("test_alter_s3_vault", "nonConcurrent") { } } - def newVaultName = suiteName + "_new"; + // alter ak sk + sql """ + ALTER STORAGE VAULT ${vaultName} + PROPERTIES ( + "type"="S3", + "s3.access_key" = "${getS3AK()}", + "s3.secret_key" = "${getS3SK()}" + ); + """ + + vaultInfos = sql """SHOW STORAGE VAULT;""" + + for (int i = 0; i < vaultInfos.size(); i++) { + def name = vaultInfos[i][0] + logger.info("name is ${name}, info ${vaultInfos[i]}") + if (name.equals(vaultName)) { + assert properties == newProperties, "Properties are not the same" + } + } + + sql """insert into alter_s3_vault_tbl values("2", "2"); """ + + + // rename + newVaultName = vaultName + "_new"; + + sql """ + ALTER STORAGE VAULT ${vaultName} + PROPERTIES ( + "type"="S3", + "VAULT_NAME" = "${newVaultName}" + ); + """ + + vaultInfos = sql """SHOW STORAGE VAULT;""" + for (int i = 0; i < vaultInfos.size(); i++) { + def name = vaultInfos[i][0] + logger.info("name is ${name}, info ${vaultInfos[i]}") + if (name.equals(newVaultName)) { + assert properties == newProperties, "Properties are not the same" + } + if (name.equals(vaultName)) { + assertTrue(false); + } + } + + sql """insert into alter_s3_vault_tbl values("2", "2"); """ + + // rename + aksk + vaultName = newVaultName sql """ ALTER STORAGE VAULT ${vaultName} PROPERTIES ( "type"="S3", "VAULT_NAME" = "${newVaultName}", - "s3.access_key" = "new_ak" + "s3.access_key" = "${getS3AK()}", + "s3.secret_key" = "${getS3SK()}" ); """ + vaultInfos = sql """SHOW STORAGE VAULT;""" + for (int i = 0; i < vaultInfos.size(); i++) { + def name = vaultInfos[i][0] + logger.info("name is ${name}, info ${vaultInfos[i]}") + if (name.equals(newVaultName)) { + assert properties == newProperties, "Properties are not the same" + } + if (name.equals(vaultName)) { + assertTrue(false); + } + } + sql """insert into alter_s3_vault_tbl values("2", "2"); """ + + + vaultName = newVaultName; + newVaultName = vaultName + "_new"; + vaultInfos = sql """SHOW STORAGE VAULT;""" boolean exist = false + sql """ + ALTER STORAGE VAULT ${vaultName} + PROPERTIES ( + "type"="S3", + "VAULT_NAME" = "${newVaultName}", + "s3.access_key" = "new_ak_ak", + "s3.secret_key" = "sk" + ); + """ + for (int i = 0; i < vaultInfos.size(); i++) { def name = vaultInfos[i][0] logger.info("name is ${name}, info ${vaultInfos[i]}") @@ -96,7 +192,7 @@ suite("test_alter_s3_vault", "nonConcurrent") { assertTrue(false); } if (name.equals(newVaultName)) { - assertTrue(vaultInfos[i][2].contains("new_ak")) + assertTrue(vaultInfos[i][2].contains("new_ak_ak")) exist = true } } From e46a531ba36854fc47bb5fe2ef778cef3619d6cd Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 04:45:44 +0800 Subject: [PATCH 02/12] fix --- cloud/src/meta-service/meta_service_resource.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index df1546b1e7f655..db232eb5314783 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -715,12 +715,12 @@ static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptrset_ak(new_as_sk_pair.first); - new_vault.mutable_obj_info()->set_sk(new_as_sk_pair.second); + new_vault.mutable_obj_info()->set_ak(new_ak_sk_pair.first); + new_vault.mutable_obj_info()->set_sk(new_ak_sk_pair.second); new_vault.mutable_obj_info()->mutable_encryption_info()->CopyFrom(encryption_info); if (obj_info.has_use_path_style()) { new_vault.mutable_obj_info()->set_use_path_style(obj_info.use_path_style()); From ab163e1d977d41c8a4fcf99cee42722930660890 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 05:10:37 +0800 Subject: [PATCH 03/12] fix --- docker/runtime/doris-compose/command.py | 4 ++-- .../main/java/org/apache/doris/catalog/StorageVaultMgr.java | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docker/runtime/doris-compose/command.py b/docker/runtime/doris-compose/command.py index 7a2f3f3c195f18..5fd557336cd249 100644 --- a/docker/runtime/doris-compose/command.py +++ b/docker/runtime/doris-compose/command.py @@ -937,8 +937,8 @@ def run(self, args): metaServiceHttpAddress = "{ms_endpoint}" metaServiceToken = "greedisgood9999" recycleServiceHttpAddress = "{recycle_endpoint}" -instanceId = "default_instance_id" -multiClusterInstance = "default_instance_id" +instanceId = "12345678" +multiClusterInstance = "12345678" multiClusterBes = "{multi_cluster_bes}" cloudUniqueId= "{fe_cloud_unique_id}" ''' diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java index 762acc7bed28d0..4014219c5a49b6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java @@ -168,6 +168,9 @@ public void alterStorageVault(StorageVaultType type, Map propert } LOG.info("Succeed to alter storage vault {}, id {}, origin default vault replaced {}", name, response.getStorageVaultId(), response.getDefaultStorageVaultReplaced()); + + // Make BE eagerly fetch the storage vault info from Meta Service + ALTER_BE_SYNC_THREAD_POOL.execute(() -> alterSyncVaultTask()); } catch (RpcException e) { LOG.warn("failed to alter storage vault due to RpcException: {}", e); throw new DdlException(e.getMessage()); From bdece98507c9748f07e2a60e6e29231192d8b94b Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 05:19:12 +0800 Subject: [PATCH 04/12] fix --- cloud/src/meta-service/meta_service_resource.cpp | 2 +- .../suites/vault_p0/alter/test_alter_s3_vault.groovy | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index db232eb5314783..8e2925b391add9 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -656,7 +656,7 @@ static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptr Date: Sun, 8 Dec 2024 06:33:33 +0800 Subject: [PATCH 05/12] fix --- .../meta-service/meta_service_resource.cpp | 2 +- .../vault_p0/alter/test_alter_s3_vault.groovy | 70 +++++++++++++++---- 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index 8e2925b391add9..db232eb5314783 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -656,7 +656,7 @@ static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptr Date: Sun, 8 Dec 2024 14:27:39 +0800 Subject: [PATCH 06/12] fix --- .../suites/vault_p0/alter/test_alter_s3_vault.groovy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy b/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy index 8c802665d1eb81..73d007533f6c1c 100644 --- a/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy +++ b/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy @@ -112,10 +112,11 @@ suite("test_alter_s3_vault", "nonConcurrent") { def vaultName = suiteName def String properties; - def vaultInfos = try_sql """show storage vault""" + def vaultInfos = try_sql """show storage vaults""" for (int i = 0; i < vaultInfos.size(); i++) { def name = vaultInfos[i][0] + logger.info("name is ${name}, info ${vaultInfos[i]}") if (name.equals(vaultName)) { properties = vaultInfos[i][2] } From 7849280c64a2766176ea553aa52191bbbe16b6cb Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 15:09:27 +0800 Subject: [PATCH 07/12] [fix](vault) avoid duplicated name and id --- .../meta-service/meta_service_resource.cpp | 100 +++++++++++------- 1 file changed, 61 insertions(+), 39 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index db232eb5314783..9bddca90dc22b9 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -305,6 +305,22 @@ void MetaServiceImpl::get_obj_store_info(google::protobuf::RpcController* contro } } +static bool is_vault_id_not_used(const InstanceInfoPB& instance, const std::string& vault_id) { + if (std::find_if(instance.resource_ids().begin(), instance.resource_ids().end(), + [&vault_id](const auto& id) { return id == vault_id; }) != + instance.resource_ids().end()) { + return false; + } + + if (std::find_if(instance.obj_info().begin(), instance.obj_info().end(), + [&vault_id](const auto& obj) { return obj.id() == vault_id; }) != + instance.obj_info().end()) { + return false; + } + + return true; +} + // The next available vault id would be max(max(obj info id), max(vault id)) + 1. static std::string next_available_vault_id(const InstanceInfoPB& instance) { int vault_id = 0; @@ -328,6 +344,12 @@ static std::string next_available_vault_id(const InstanceInfoPB& instance) { instance.resource_ids().begin(), instance.resource_ids().end(), std::accumulate(instance.obj_info().begin(), instance.obj_info().end(), vault_id, max), max); + + // just add a defensive logic, we should use int64 in the future. + while (!is_vault_id_not_used(instance, std::to_string(prev + 1))) { + prev += 1; + } + return std::to_string(prev + 1); } @@ -454,14 +476,41 @@ static void create_object_info_with_encrypt(const InstanceInfoPB& instance, Obje obj->set_sse_enabled(sse_enabled); } -static int add_vault_into_instance(InstanceInfoPB& instance, Transaction* txn, - StorageVaultPB& vault_param, MetaServiceCode& code, - std::string& msg) { +static int check_duplicated_vault_name(const InstanceInfoPB& instance, const std::string& candidate_name, + MetaServiceCode& code, std::string& msg) { if (std::find_if(instance.storage_vault_names().begin(), instance.storage_vault_names().end(), - [&vault_param](const auto& name) { return name == vault_param.name(); }) != + [&candidate_name](const auto& name) { return name == candidate_name; }) != instance.storage_vault_names().end()) { code = MetaServiceCode::ALREADY_EXISTED; - msg = fmt::format("vault_name={} already created", vault_param.name()); + msg = fmt::format("candidate_name={} already created", candidate_name); + return -1; + } + + return 0; +} + +static int check_new_vault_name(const InstanceInfoPB& instance, const std::string& candidate_name, + MetaServiceCode& code, std::string& msg) { + if (!is_valid_storage_vault_name(candidate_name)) { + code = MetaServiceCode::INVALID_ARGUMENT; + std::stringstream ss; + ss << "invalid storage vault name =" << candidate_name << " the name must satisfy " + << pattern_str; + msg = ss.str(); + return -1; + } + + if (check_duplicated_vault_name(instance, candidate_name, code, msg) != 0) { + return -1; + } + + return 0; +} + +static int add_vault_into_instance(InstanceInfoPB& instance, Transaction* txn, + StorageVaultPB& vault_param, MetaServiceCode& code, + std::string& msg) { + if (check_new_vault_name(instance, vault_param.name(), code, msg) != 0) { return -1; } @@ -583,12 +632,7 @@ static int alter_hdfs_storage_vault(InstanceInfoPB& instance, std::unique_ptrput(vault_key, val); LOG(INFO) << "put vault_id=" << vault_id << ", vault_key=" << hex(vault_key) << ", origin vault=" << origin_vault_info << ", new_vault=" << new_vault_info; - err = txn->commit(); - if (err != TxnErrorCode::TXN_OK) { - code = cast_as(err); - msg = fmt::format("failed to commit kv txn, err={}", err); - LOG(WARNING) << msg; - } return 0; } @@ -700,14 +738,10 @@ static int alter_s3_storage_vault(InstanceInfoPB& instance, std::unique_ptrput(vault_key, val); LOG(INFO) << "put vault_id=" << vault_id << ", vault_key=" << hex(vault_key) << ", origin vault=" << origin_vault_info << ", new vault=" << new_vault_info; - err = txn->commit(); - if (err != TxnErrorCode::TXN_OK) { - code = cast_as(err); - msg = fmt::format("failed to commit kv txn, err={}", err); - LOG(WARNING) << msg; - } return 0; } @@ -998,16 +1026,10 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr ObjectStorageDesc {ak, sk, bucket, prefix, endpoint, external_endpoint, region}; ObjectStoreInfoPB last_item = object_info_pb_factory(tmp_tuple, obj, instance, encryption_info, cipher_ak_sk_pair); - if (instance.storage_vault_names().end() != - std::find_if(instance.storage_vault_names().begin(), - instance.storage_vault_names().end(), - [&](const std::string& candidate_name) { - return candidate_name == request->vault().name(); - })) { - code = MetaServiceCode::ALREADY_EXISTED; - msg = fmt::format("vault_name={} already created", request->vault().name()); + if (check_new_vault_name(instance, request->vault().name(), code, msg) != 0) { return; } + StorageVaultPB vault; vault.set_id(last_item.id()); vault.set_name(request->vault().name()); @@ -1097,11 +1119,11 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr } case AlterObjStoreInfoRequest::ALTER_S3_VAULT: { alter_s3_storage_vault(instance, std::move(txn), request->vault(), code, msg); - return; + break; } case AlterObjStoreInfoRequest::ALTER_HDFS_VAULT: { alter_hdfs_storage_vault(instance, std::move(txn), request->vault(), code, msg); - return; + break; } case AlterObjStoreInfoRequest::DROP_S3_VAULT: [[fallthrough]]; From 5d374f9e9b4e8b9917027d4c58ccb389f6a29b06 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 15:29:20 +0800 Subject: [PATCH 08/12] fix --- cloud/src/meta-service/meta_service_resource.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index 9bddca90dc22b9..034db4bdc5df0c 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -1118,11 +1118,17 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr break; } case AlterObjStoreInfoRequest::ALTER_S3_VAULT: { - alter_s3_storage_vault(instance, std::move(txn), request->vault(), code, msg); + int ret = alter_s3_storage_vault(instance, std::move(txn), request->vault(), code, msg); + if (ret != 0) { + return; + } break; } case AlterObjStoreInfoRequest::ALTER_HDFS_VAULT: { - alter_hdfs_storage_vault(instance, std::move(txn), request->vault(), code, msg); + int ret = alter_hdfs_storage_vault(instance, std::move(txn), request->vault(), code, msg); + if (ret != 0) { + return; + } break; } case AlterObjStoreInfoRequest::DROP_S3_VAULT: From 19044dda495c43bfe0309994511b628ad9330213 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 17:39:36 +0800 Subject: [PATCH 09/12] fix --- cloud/src/meta-service/meta_service_resource.cpp | 10 +++++----- .../vault_p0/alter/test_alter_s3_vault.groovy | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index 034db4bdc5df0c..63068c7124614f 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -482,7 +482,7 @@ static int check_duplicated_vault_name(const InstanceInfoPB& instance, const std [&candidate_name](const auto& name) { return name == candidate_name; }) != instance.storage_vault_names().end()) { code = MetaServiceCode::ALREADY_EXISTED; - msg = fmt::format("candidate_name={} already created", candidate_name); + msg = fmt::format("vault named {} already exists", candidate_name); return -1; } @@ -572,7 +572,7 @@ static void set_default_vault_log_helper(const InstanceInfoPB& instance, LOG(INFO) << vault_msg; } -static int alter_hdfs_storage_vault(InstanceInfoPB& instance, std::unique_ptr txn, +static int alter_hdfs_storage_vault(InstanceInfoPB& instance, Transaction* txn, const StorageVaultPB& vault, MetaServiceCode& code, std::string& msg) { if (!vault.has_hdfs_info()) { @@ -671,7 +671,7 @@ static int alter_hdfs_storage_vault(InstanceInfoPB& instance, std::unique_ptr txn, +static int alter_s3_storage_vault(InstanceInfoPB& instance, Transaction* txn, const StorageVaultPB& vault, MetaServiceCode& code, std::string& msg) { if (!vault.has_obj_info()) { @@ -1118,14 +1118,14 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr break; } case AlterObjStoreInfoRequest::ALTER_S3_VAULT: { - int ret = alter_s3_storage_vault(instance, std::move(txn), request->vault(), code, msg); + int ret = alter_s3_storage_vault(instance, txn.get(), request->vault(), code, msg); if (ret != 0) { return; } break; } case AlterObjStoreInfoRequest::ALTER_HDFS_VAULT: { - int ret = alter_hdfs_storage_vault(instance, std::move(txn), request->vault(), code, msg); + int ret = alter_hdfs_storage_vault(instance, txn.get(), request->vault(), code, msg); if (ret != 0) { return; } diff --git a/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy b/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy index 73d007533f6c1c..db692fc76ef6fc 100644 --- a/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy +++ b/regression-test/suites/vault_p0/alter/test_alter_s3_vault.groovy @@ -173,7 +173,7 @@ suite("test_alter_s3_vault", "nonConcurrent") { sql """insert into alter_s3_vault_tbl values("2", "2"); """ // rename + aksk - vaultName = newVaultName + vaultName = new String(newVaultName) newVaultName = vaultName + "_new"; sql """ @@ -200,14 +200,10 @@ suite("test_alter_s3_vault", "nonConcurrent") { } sql """insert into alter_s3_vault_tbl values("2", "2"); """ - - vaultName = newVaultName; + vaultName = new String(newVaultName) newVaultName = vaultName + "_new"; - vaultInfos = sql """SHOW STORAGE VAULT;""" - boolean exist = false - sql """ ALTER STORAGE VAULT ${vaultName} PROPERTIES ( @@ -218,8 +214,12 @@ suite("test_alter_s3_vault", "nonConcurrent") { ); """ + vaultInfos = sql """SHOW STORAGE VAULT;""" + def boolean exist = false + for (int i = 0; i < vaultInfos.size(); i++) { def name = vaultInfos[i][0] + logger.info("vaultName ${vaultName}"); logger.info("name is ${name}, info ${vaultInfos[i]}") if (name.equals(vaultName)) { assertTrue(false); @@ -231,7 +231,7 @@ suite("test_alter_s3_vault", "nonConcurrent") { } assertTrue(exist) - vaultName = newVaultName; + vaultName = new String(newVaultName) expectExceptionLike({ sql """ From 4892cddaff5515efecd26bea6ab0e97d0d9ee10f Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 21:54:25 +0800 Subject: [PATCH 10/12] fix --- .../meta-service/meta_service_resource.cpp | 4 +- cloud/test/meta_service_test.cpp | 377 ++++++++++++++++-- 2 files changed, 349 insertions(+), 32 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index 63068c7124614f..a31d866a8c902d 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -183,7 +183,7 @@ int decrypt_instance_info(InstanceInfoPB& instance, const std::string& instance_ return 0; } -static int decrypt_and_update_ak_sk(ObjectStoreInfoPB& obj_info, MetaServiceCode& code, +int decrypt_and_update_ak_sk(ObjectStoreInfoPB& obj_info, MetaServiceCode& code, std::string& msg) { if (obj_info.has_encryption_info()) { AkSkPair plain_ak_sk_pair; @@ -963,7 +963,7 @@ void MetaServiceImpl::alter_storage_vault(google::protobuf::RpcController* contr return; } err = txn->get(key, &val); - LOG(INFO) << "get instance_key=" << hex(key); + LOG(INFO) << "get instance_id=" << instance_id << ", instance_key=" << hex(key); if (err != TxnErrorCode::TXN_OK) { code = cast_as(err); diff --git a/cloud/test/meta_service_test.cpp b/cloud/test/meta_service_test.cpp index d2dd80f6871e3e..cdf5c833e53684 100644 --- a/cloud/test/meta_service_test.cpp +++ b/cloud/test/meta_service_test.cpp @@ -446,6 +446,64 @@ TEST(MetaServiceTest, CreateInstanceTest) { } } +static void get_all_vaults(const MetaServiceProxy* meta_service, const InstanceInfoPB& instance, + std::vector& all_vaults) { + LOG(INFO) << "get all vaults " << instance.DebugString(); + for (auto vault_id : instance.resource_ids()) { + std::unique_ptr txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + const std::string key = storage_vault_key({instance.instance_id(), vault_id}); + + LOG(INFO) << "vault key = " << hex(key); + + std::string value; + ASSERT_EQ(txn->get(key, &value), TxnErrorCode::TXN_OK); + + StorageVaultPB vault_pb; + ASSERT_TRUE(vault_pb.ParseFromArray(value.data(), value.size())); + + all_vaults.push_back(vault_pb); + LOG(INFO) << "get vault = " << vault_pb.DebugString(); + } + EXPECT_EQ(all_vaults.size(), instance.resource_ids().size()); +} + +static void check_vault_name_consistency(const MetaServiceProxy* meta_service, const InstanceInfoPB& instance) { + std::vector all_vaults; + + get_all_vaults(meta_service, instance, all_vaults); + LOG(INFO) << "All Vaults: "; + for (const auto& vault : all_vaults) { + LOG(INFO) << "Vault Name: " << vault.name(); + } + LOG(INFO) << "Instance Storage Vault Names: "; + for (const auto& vault_name : instance.storage_vault_names()) { + LOG(INFO) << "Vault Name: " << vault_name; + } + for (const auto& vault_name : instance.storage_vault_names()) { + int found = 0; + for (const auto& vault_pb : all_vaults) { + if (vault_pb.name() == vault_name) { + found++; + } + } + ASSERT_EQ(found, 1) << "vault_name " << vault_name << " does not exists in objinfo"; + } + + for (const auto& vault_pb : all_vaults) { + int found = 0; + for (const auto& vault_name : instance.storage_vault_names()) { + if (vault_pb.name() == vault_name) { + found++; + } + } + ASSERT_EQ(found, 1) << "vault_name " << vault_pb.name() << " does not exists in vault names"; + } +} + +extern int decrypt_and_update_ak_sk(ObjectStoreInfoPB& obj_info, MetaServiceCode& code, + std::string& msg); + TEST(MetaServiceTest, AlterS3StorageVaultTest) { auto meta_service = get_meta_service(); @@ -459,6 +517,14 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { auto* key_id = try_any_cast(args[2]); *key_id = 1; }); + sp->set_call_back("decrypt_ak_sk:get_encryption_key", [](auto&& args) { + auto* ret = try_any_cast(args[0]); + *ret = 0; + auto* key = try_any_cast(args[1]); + *key = "selectdbselectdbselectdbselectdb"; + auto* key_id = try_any_cast(args[2]); + *key_id = 1; + }); std::pair pair; sp->set_call_back("extract_object_storage_info:get_aksk_pair", [&](auto&& args) { auto* ret = try_any_cast*>(args[0]); @@ -469,7 +535,9 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); std::string key; std::string val; - InstanceKeyInfo key_info {"test_instance"}; + + const std::string instance_id = "test_instance"; + InstanceKeyInfo key_info {instance_id}; instance_key(key_info, &key); ObjectStoreInfoPB obj_info; @@ -484,29 +552,56 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { InstanceInfoPB instance; instance.add_storage_vault_names(vault.name()); instance.add_resource_ids(vault.id()); - instance.set_instance_id("GetObjStoreInfoTestInstance"); + instance.set_instance_id(instance_id); val = instance.SerializeAsString(); txn->put(key, val); - txn->put(storage_vault_key({instance.instance_id(), "2"}), vault.SerializeAsString()); + key.clear(); + storage_vault_key({instance.instance_id(), vault.id()}, &key); + LOG(INFO) << "vault key = " << hex(key); + txn->put(key, vault.SerializeAsString()); ASSERT_EQ(txn->commit(), TxnErrorCode::TXN_OK); txn = nullptr; + std::vector vaults; + get_all_vaults(meta_service.get(), instance, vaults); + auto get_test_instance = [&](InstanceInfoPB& i) { std::string key; std::string val; std::unique_ptr txn; ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); - InstanceKeyInfo key_info {"test_instance"}; + InstanceKeyInfo key_info {instance_id}; instance_key(key_info, &key); ASSERT_EQ(txn->get(key, &val), TxnErrorCode::TXN_OK); i.ParseFromString(val); }; + auto get_vault_pb = [&](std::string vault_id) { + InstanceInfoPB instance; + get_test_instance(instance); + + std::vector vaults; + get_all_vaults(meta_service.get(), instance, vaults); + + for (const auto& vault : vaults) { + if (vault.id() == vault_id) { + return vault; + } + } + + return StorageVaultPB(); + }; + + // can not change ak without sk { + + StorageVaultPB orig_vault = get_vault_pb("2"); + AlterObjStoreInfoRequest req; - req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_cloud_unique_id("1:" + instance_id + ":1"); req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); StorageVaultPB vault; + vault.mutable_obj_info()->set_ak("new_ak"); vault.set_name(vault_name); req.mutable_vault()->CopyFrom(vault); @@ -515,27 +610,50 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { AlterObjStoreInfoResponse res; meta_service->alter_storage_vault( reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); - ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); - InstanceInfoPB instance; - get_test_instance(instance); + ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) << res.status().msg(); - std::unique_ptr txn; - ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); - std::string val; - ASSERT_EQ(txn->get(storage_vault_key({instance.instance_id(), "2"}), &val), - TxnErrorCode::TXN_OK); - StorageVaultPB get_obj; - get_obj.ParseFromString(val); - ASSERT_EQ(get_obj.obj_info().ak(), "new_ak") << get_obj.obj_info().ak(); + StorageVaultPB new_vault = get_vault_pb("2"); + + ASSERT_EQ(orig_vault.SerializeAsString(), new_vault.SerializeAsString()); } + // can not change sk without ak { + + StorageVaultPB orig_vault = get_vault_pb("2"); + AlterObjStoreInfoRequest req; - req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_cloud_unique_id("1:" + instance_id + ":1"); + req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); + StorageVaultPB vault; + + vault.mutable_obj_info()->set_sk("new_sk"); + vault.set_name(vault_name); + req.mutable_vault()->CopyFrom(vault); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_storage_vault( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + ASSERT_EQ(orig_vault.SerializeAsString(), new_vault.SerializeAsString()); + } + + // alter not exists + { + StorageVaultPB orig_vault = get_vault_pb("2"); + + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("1:" + instance_id + ":1"); req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); StorageVaultPB vault; ObjectStoreInfoPB obj; - obj_info.set_ak("new_ak"); + obj.set_ak("new_ak"); + obj.set_sk("new_sk"); + vault.mutable_obj_info()->MergeFrom(obj); vault.set_name("test_alter_s3_vault_non_exist"); req.mutable_vault()->CopyFrom(vault); @@ -545,17 +663,76 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { meta_service->alter_storage_vault( reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); ASSERT_NE(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + EXPECT_EQ(new_vault.SerializeAsString(), orig_vault.SerializeAsString()); + } + + // alter ak sk + { + + LOG(INFO) << "alter ak sk"; + + StorageVaultPB orig_vault = get_vault_pb("2"); + + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("1:" + instance_id + ":1"); + req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); + StorageVaultPB vault; + ObjectStoreInfoPB obj; + obj.set_ak("new_ak"); + obj.set_sk("new_sk"); + + vault.mutable_obj_info()->MergeFrom(obj); + vault.set_name(vault_name); + req.mutable_vault()->CopyFrom(vault); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_storage_vault( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + // Check if all fields except ak and sk are the same for orig_vault and new_vault + EXPECT_EQ(orig_vault.name(), new_vault.name()); + EXPECT_EQ(orig_vault.obj_info().endpoint(), new_vault.obj_info().endpoint()); + EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); + EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); + EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); + ASSERT_NE(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); + ASSERT_NE(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); + + MetaServiceCode code; + std::string msg; + LOG(INFO) << "alter ak sk"; + + int ret = decrypt_and_update_ak_sk(*new_vault.mutable_obj_info(), code, msg); + ASSERT_EQ(ret, 0); + // Check if ak and sk of new vault are set + LOG(INFO) << "alter ak sk"; + + ASSERT_EQ(new_vault.obj_info().ak(), "new_ak"); + ASSERT_EQ(new_vault.obj_info().sk(), "new_sk"); } + // rename with invalid name { + LOG(INFO) << "rename with invalid name"; + + StorageVaultPB orig_vault = get_vault_pb("2"); + AlterObjStoreInfoRequest req; constexpr char new_vault_name[] = "@!#vault_name"; - req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_cloud_unique_id("1:" + instance_id + ":1"); req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); StorageVaultPB vault; vault.set_alter_name(new_vault_name); ObjectStoreInfoPB obj; - obj_info.set_ak("new_ak"); vault.mutable_obj_info()->MergeFrom(obj); vault.set_name(vault_name); req.mutable_vault()->CopyFrom(vault); @@ -567,17 +744,64 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { ASSERT_NE(res.status().code(), MetaServiceCode::OK) << res.status().msg(); ASSERT_TRUE(res.status().msg().find("invalid storage vault name") != std::string::npos) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + EXPECT_EQ(orig_vault.name(), new_vault.name()); + EXPECT_EQ(orig_vault.obj_info().endpoint(), new_vault.obj_info().endpoint()); + EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); + EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); + EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); + EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); + EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); + } + + // empty rename + { + StorageVaultPB orig_vault = get_vault_pb("2"); + + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("1:" + instance_id + ":1"); + req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); + StorageVaultPB vault; + ObjectStoreInfoPB obj; + vault.mutable_obj_info()->MergeFrom(obj); + vault.set_name(vault_name); + req.mutable_vault()->CopyFrom(vault); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_storage_vault( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + EXPECT_EQ(orig_vault.name(), new_vault.name()); + EXPECT_EQ(orig_vault.obj_info().endpoint(), new_vault.obj_info().endpoint()); + EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); + EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); + EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); + EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); + EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); } + // rename with normal name { + StorageVaultPB orig_vault = get_vault_pb("2"); + AlterObjStoreInfoRequest req; constexpr char new_vault_name[] = "new_test_alter_s3_vault"; - req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_cloud_unique_id("1:" + instance_id + ":1"); req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); StorageVaultPB vault; vault.set_alter_name(new_vault_name); ObjectStoreInfoPB obj; - obj_info.set_ak("new_ak"); + obj.set_ak("new_ak"); vault.mutable_obj_info()->MergeFrom(obj); vault.set_name(vault_name); req.mutable_vault()->CopyFrom(vault); @@ -587,17 +811,104 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { meta_service->alter_storage_vault( reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + EXPECT_EQ(vault.name(), new_vault.name()); + EXPECT_EQ(orig_vault.obj_info().endpoint(), new_vault.obj_info().endpoint()); + EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); + EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); + EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); + EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); + EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); + InstanceInfoPB instance; get_test_instance(instance); + check_vault_name_consistency(meta_service.get(), instance); + } - std::unique_ptr txn; - ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); - std::string val; - ASSERT_EQ(txn->get(storage_vault_key({instance.instance_id(), "2"}), &val), - TxnErrorCode::TXN_OK); - StorageVaultPB get_obj; - get_obj.ParseFromString(val); - ASSERT_EQ(get_obj.name(), new_vault_name) << get_obj.obj_info().ak(); + // rename with normal name + { + StorageVaultPB orig_vault = get_vault_pb("2"); + + constexpr char orig_vault_name[] = "new_test_alter_s3_vault"; + + AlterObjStoreInfoRequest req; + constexpr char new_vault_name[] = "new_test_alter_s3_vault_new"; + req.set_cloud_unique_id("1:" + instance_id + ":1"); + req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); + StorageVaultPB vault; + vault.set_alter_name(new_vault_name); + ObjectStoreInfoPB obj; + obj.set_ak("new_ak"); + vault.mutable_obj_info()->MergeFrom(obj); + vault.set_name(orig_vault_name); + req.mutable_vault()->CopyFrom(vault); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_storage_vault( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + EXPECT_EQ(vault.name(), new_vault.name()); + EXPECT_EQ(orig_vault.obj_info().endpoint(), new_vault.obj_info().endpoint()); + EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); + EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); + EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); + EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); + EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); + + InstanceInfoPB instance; + get_test_instance(instance); + check_vault_name_consistency(meta_service.get(), instance); + } + + // rename with same name + { + StorageVaultPB orig_vault = get_vault_pb("2"); + + AlterObjStoreInfoRequest req; + constexpr char new_vault_name[] = "new_test_alter_s3_vault_new"; + req.set_cloud_unique_id("1:" + instance_id + ":1"); + req.set_op(AlterObjStoreInfoRequest::ALTER_S3_VAULT); + StorageVaultPB vault; + vault.set_alter_name(new_vault_name); + ObjectStoreInfoPB obj; + obj.set_ak("new_ak"); + vault.mutable_obj_info()->MergeFrom(obj); + vault.set_name(new_vault_name); + req.mutable_vault()->CopyFrom(vault); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_storage_vault( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_NE(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + StorageVaultPB new_vault = get_vault_pb("2"); + + EXPECT_EQ(orig_vault.name(), new_vault.name()); + EXPECT_EQ(orig_vault.obj_info().endpoint(), new_vault.obj_info().endpoint()); + EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); + EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); + EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); + EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); + EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); + } + + { + InstanceInfoPB instance; + get_test_instance(instance); + check_vault_name_consistency(meta_service.get(), instance); } SyncPoint::get_instance()->disable_processing(); @@ -756,6 +1067,12 @@ TEST(MetaServiceTest, AlterHdfsStorageVaultTest) { ASSERT_EQ(get_obj.name(), new_vault_name) << get_obj.obj_info().ak(); } + { + InstanceInfoPB instance; + get_test_instance(instance); + check_vault_name_consistency(meta_service.get(), instance); + } + SyncPoint::get_instance()->disable_processing(); SyncPoint::get_instance()->clear_all_call_backs(); } From b05efbaa8a0cd597fe7417ae449930f0cf3ef9b8 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 22:06:35 +0800 Subject: [PATCH 11/12] fix --- .../meta-service/meta_service_resource.cpp | 10 +++---- cloud/test/meta_service_test.cpp | 27 +++++++++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index a31d866a8c902d..9364abdd02af07 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -183,8 +183,7 @@ int decrypt_instance_info(InstanceInfoPB& instance, const std::string& instance_ return 0; } -int decrypt_and_update_ak_sk(ObjectStoreInfoPB& obj_info, MetaServiceCode& code, - std::string& msg) { +int decrypt_and_update_ak_sk(ObjectStoreInfoPB& obj_info, MetaServiceCode& code, std::string& msg) { if (obj_info.has_encryption_info()) { AkSkPair plain_ak_sk_pair; if (int ret = decrypt_ak_sk_helper(obj_info.ak(), obj_info.sk(), obj_info.encryption_info(), @@ -476,8 +475,9 @@ static void create_object_info_with_encrypt(const InstanceInfoPB& instance, Obje obj->set_sse_enabled(sse_enabled); } -static int check_duplicated_vault_name(const InstanceInfoPB& instance, const std::string& candidate_name, - MetaServiceCode& code, std::string& msg) { +static int check_duplicated_vault_name(const InstanceInfoPB& instance, + const std::string& candidate_name, MetaServiceCode& code, + std::string& msg) { if (std::find_if(instance.storage_vault_names().begin(), instance.storage_vault_names().end(), [&candidate_name](const auto& name) { return name == candidate_name; }) != instance.storage_vault_names().end()) { @@ -495,7 +495,7 @@ static int check_new_vault_name(const InstanceInfoPB& instance, const std::strin code = MetaServiceCode::INVALID_ARGUMENT; std::stringstream ss; ss << "invalid storage vault name =" << candidate_name << " the name must satisfy " - << pattern_str; + << pattern_str; msg = ss.str(); return -1; } diff --git a/cloud/test/meta_service_test.cpp b/cloud/test/meta_service_test.cpp index cdf5c833e53684..6876bac959b0eb 100644 --- a/cloud/test/meta_service_test.cpp +++ b/cloud/test/meta_service_test.cpp @@ -468,7 +468,8 @@ static void get_all_vaults(const MetaServiceProxy* meta_service, const InstanceI EXPECT_EQ(all_vaults.size(), instance.resource_ids().size()); } -static void check_vault_name_consistency(const MetaServiceProxy* meta_service, const InstanceInfoPB& instance) { +static void check_vault_name_consistency(const MetaServiceProxy* meta_service, + const InstanceInfoPB& instance) { std::vector all_vaults; get_all_vaults(meta_service, instance, all_vaults); @@ -487,7 +488,8 @@ static void check_vault_name_consistency(const MetaServiceProxy* meta_service, c found++; } } - ASSERT_EQ(found, 1) << "vault_name " << vault_name << " does not exists in objinfo"; + ASSERT_EQ(found, 1) << "vault_name " << vault_name + << " does not exists in objinfo"; } for (const auto& vault_pb : all_vaults) { @@ -594,7 +596,6 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { // can not change ak without sk { - StorageVaultPB orig_vault = get_vault_pb("2"); AlterObjStoreInfoRequest req; @@ -619,7 +620,6 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { // can not change sk without ak { - StorageVaultPB orig_vault = get_vault_pb("2"); AlterObjStoreInfoRequest req; @@ -671,7 +671,6 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { // alter ak sk { - LOG(INFO) << "alter ak sk"; StorageVaultPB orig_vault = get_vault_pb("2"); @@ -702,7 +701,8 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); - EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), + new_vault.obj_info().external_endpoint()); EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); ASSERT_NE(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); ASSERT_NE(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); @@ -752,7 +752,8 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); - EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), + new_vault.obj_info().external_endpoint()); EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); @@ -784,7 +785,8 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); - EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), + new_vault.obj_info().external_endpoint()); EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); @@ -819,7 +821,8 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); - EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), + new_vault.obj_info().external_endpoint()); EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); @@ -860,7 +863,8 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); - EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), + new_vault.obj_info().external_endpoint()); EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); @@ -899,7 +903,8 @@ TEST(MetaServiceTest, AlterS3StorageVaultTest) { EXPECT_EQ(orig_vault.obj_info().region(), new_vault.obj_info().region()); EXPECT_EQ(orig_vault.obj_info().bucket(), new_vault.obj_info().bucket()); EXPECT_EQ(orig_vault.obj_info().prefix(), new_vault.obj_info().prefix()); - EXPECT_EQ(orig_vault.obj_info().external_endpoint(), new_vault.obj_info().external_endpoint()); + EXPECT_EQ(orig_vault.obj_info().external_endpoint(), + new_vault.obj_info().external_endpoint()); EXPECT_EQ(orig_vault.obj_info().provider(), new_vault.obj_info().provider()); EXPECT_EQ(orig_vault.obj_info().ak(), new_vault.obj_info().ak()); EXPECT_EQ(orig_vault.obj_info().sk(), new_vault.obj_info().sk()); From 3f47d20e7918da50962e93e511d9692280383f98 Mon Sep 17 00:00:00 2001 From: Yongqiang YANG Date: Sun, 8 Dec 2024 22:11:33 +0800 Subject: [PATCH 12/12] fix --- cloud/test/meta_service_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cloud/test/meta_service_test.cpp b/cloud/test/meta_service_test.cpp index 6876bac959b0eb..e256e59a571cb3 100644 --- a/cloud/test/meta_service_test.cpp +++ b/cloud/test/meta_service_test.cpp @@ -488,8 +488,7 @@ static void check_vault_name_consistency(const MetaServiceProxy* meta_service, found++; } } - ASSERT_EQ(found, 1) << "vault_name " << vault_name - << " does not exists in objinfo"; + ASSERT_EQ(found, 1) << "vault_name " << vault_name << " does not exists in objinfo"; } for (const auto& vault_pb : all_vaults) { @@ -499,7 +498,8 @@ static void check_vault_name_consistency(const MetaServiceProxy* meta_service, found++; } } - ASSERT_EQ(found, 1) << "vault_name " << vault_pb.name() << " does not exists in vault names"; + ASSERT_EQ(found, 1) << "vault_name " << vault_pb.name() + << " does not exists in vault names"; } }