github-actions[bot] commented on code in PR #63533:
URL: https://github.com/apache/doris/pull/63533#discussion_r3308912005


##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -325,6 +325,96 @@ static int find_cascade_instances(TxnKv* txn_kv, const 
std::string& root_instanc
     return 0;
 }
 
+static int find_storage_vault_position_by_id(const InstanceInfoPB& instance,
+                                             std::string_view vault_id) {
+    auto id_itr =
+            std::find(instance.resource_ids().begin(), 
instance.resource_ids().end(), vault_id);
+    if (id_itr == instance.resource_ids().end()) {
+        return -1;
+    }
+    return static_cast<int>(id_itr - instance.resource_ids().begin());
+}
+
+static int find_storage_vault_id_by_name(const InstanceInfoPB& instance,
+                                         std::string_view vault_name, 
std::string* vault_id) {
+    auto name_itr = std::find_if(
+            instance.storage_vault_names().begin(), 
instance.storage_vault_names().end(),
+            [&](const auto& current_name) { return current_name == vault_name; 
});
+    if (name_itr == instance.storage_vault_names().end()) {
+        return -1;
+    }
+    int pos = static_cast<int>(name_itr - 
instance.storage_vault_names().begin());
+    *vault_id = instance.resource_ids().Get(pos);
+    return 0;
+}
+
+static int alter_instance_obj_store_info_by_id(InstanceInfoPB& instance,
+                                               std::string_view target_obj_id, 
std::string_view ak,
+                                               std::string_view sk, 
std::string_view role_arn,
+                                               std::string_view external_id,
+                                               const EncryptionInfoPB& 
encryption_info,
+                                               MetaServiceCode& code, 
std::string& msg) {
+    auto& obj_info = 
const_cast<std::decay_t<decltype(instance.obj_info())>&>(instance.obj_info());
+    for (auto& it : obj_info) {
+        if (it.id() != target_obj_id) {
+            continue;
+        }
+
+        if (role_arn.empty()) {
+            if (it.ak() == ak && it.sk() == sk) {
+                code = MetaServiceCode::OK;
+                msg = "ak/sk not changed";
+                return 1;
+            }
+            it.clear_role_arn();
+            it.clear_external_id();
+            it.clear_cred_provider_type();
+
+            it.set_ak(std::string(ak));
+            it.set_sk(std::string(sk));
+            it.mutable_encryption_info()->CopyFrom(encryption_info);
+        } else {
+            if (!ak.empty() || !sk.empty()) {
+                code = MetaServiceCode::INVALID_ARGUMENT;
+                msg = "invaild argument, both set ak/sk and role_arn is not 
allowed";
+                LOG(INFO) << msg;
+                return -1;
+            }
+
+            if (it.provider() != ObjectStoreInfoPB::S3) {
+                code = MetaServiceCode::INVALID_ARGUMENT;
+                msg = "role_arn is only supported for s3 provider";
+                LOG(INFO) << msg << " provider=" << it.provider();
+                return -1;
+            }
+
+            if (it.role_arn() == role_arn && it.external_id() == external_id) {
+                code = MetaServiceCode::OK;
+                msg = "ak/sk not changed";
+                return 1;
+            }
+            it.clear_ak();
+            it.clear_sk();
+            it.clear_encryption_info();
+
+            it.set_role_arn(std::string(role_arn));
+            it.set_external_id(std::string(external_id));
+            it.set_cred_provider_type(CredProviderTypePB::INSTANCE_PROFILE);

Review Comment:
   This helper no longer has access to `request->obj()`, so role-based 
object-store updates now always persist `INSTANCE_PROFILE`. The old code copied 
`get_cred_provider_type(request->obj())` and included it in the no-op check, so 
a request that sets the same role/external_id but changes `cred_provider_type` 
(for example to `WEB_IDENTITY`, `ENV`, etc.) now returns `ak/sk not changed` 
without updating anything, and any non-default provider is downgraded to 
instance profile. Please pass the requested credential provider type into the 
helper and compare/store that value instead of hard-coding `INSTANCE_PROFILE`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to