refactor

Author: huerni

src/CraneCtld/AccountManager.cpp

@@ -1014,6 +1014,8 @@ AccountManager::CraneExpected<void> AccountManager::CheckIfUidHasPermOnUser(
   auto user_result = GetUserInfoByUidNoLock_(uid);
   if (!user_result) return std::unexpected(user_result.error());
   const User* op_user = user_result.value();
+  result = CheckIfUserHasHigherPrivThan_(*op_user, User::None);
+  if (!result) return result;
 
   for (const auto& account_name : accounts) {
     const Account* account = GetAccountInfoNoLock_(account_name);

src/CraneCtld/CraneCtld.cpp

@@ -460,15 +460,17 @@ void ParseConfig(int argc, char** argv) {
 
           if (partition["AllowedAccounts"] &&
               !partition["AllowedAccounts"].IsNull()) {
-            std::string allowed_accounts_str =
+            auto allowed_accounts_str =
                 partition["AllowedAccounts"].as<std::string>();
             std::vector<std::string> allowed_accounts =
                 absl::StrSplit(absl::StripAsciiWhitespace(allowed_accounts_str).data(), ",");
             for (const auto& account_name : allowed_accounts) {
               part.allowed_accounts.insert(account_name);
             }
-          } else if (partition["DeniedAccounts"] && !partition["DeniedAccounts"].IsNull()) {
-            std::string denied_accounts_str = partition["DeniedAccounts"].as<std::string>();
+          }
+
+          if (partition["DeniedAccounts"] && !partition["DeniedAccounts"].IsNull()) {
+            auto denied_accounts_str = partition["DeniedAccounts"].as<std::string>();
             std::vector<std::string> denied_accounts = absl::StrSplit(absl::StripAsciiWhitespace(denied_accounts_str).data(), ",");
             for (const auto& account_name : denied_accounts) {
               part.denied_accounts.insert(account_name);

src/CraneCtld/CranedMetaContainer.cpp

@@ -608,33 +608,35 @@ crane::grpc::ModifyCranedStateReply CranedMetaContainer::ChangeNodeState(
 
   if (is_modify_allowed) {
     allowed_accounts = accounts;
-    denied_accounts.clear();
-  } else if (allowed_accounts.empty()) {
-    // When allowed_accounts is in use, denied_accounts cannot be modified.
+  } else {
     denied_accounts = accounts;
   }
 
   return result;
 }
 
-bool CranedMetaContainer::CheckIfAccountIsAllowedInPartition(
+std::expected<void, std::string> CranedMetaContainer::CheckIfAccountIsAllowedInPartition(
     const std::string& partition_name, const std::string& account_name) {
   auto part_metas_map = partition_metas_map_.GetMapSharedPtr();
 
-  if (!part_metas_map->contains(partition_name)) return false;
+  if (!part_metas_map->contains(partition_name)) return std::unexpected("Partition does not exist.");
 
   auto part_meta = part_metas_map->at(partition_name).GetExclusivePtr();
   const auto& allowed_accounts = part_meta->partition_global_meta.allowed_accounts;
   if (!allowed_accounts.empty()) {
     if (!allowed_accounts.contains(account_name))
-      return false;
+      return std::unexpected(
+        "The account is not in the AllowedAccounts of the partition "
+        "specified for the task, submission of the task is prohibited.");
   } else {
     const auto& denied_accounts = part_meta->partition_global_meta.denied_accounts;
     if (denied_accounts.contains(account_name))
-      return false;
+      return std::unexpected(
+        "The account is in the DeniedAccounts of the partition "
+        "specified for the task, submission of the task is prohibited.");
   }
 
-  return true;
+  return {};
 }
 
 void CranedMetaContainer::AddDedicatedResource(

src/CraneCtld/CranedMetaContainer.h

@@ -95,7 +95,7 @@ class CranedMetaContainer final {
       bool is_modify_allowed,
       const std::unordered_set<std::string>& accounts);
 
-  bool CheckIfAccountIsAllowedInPartition(const std::string& partition_name,
+  std::expected<void, std::string> CheckIfAccountIsAllowedInPartition(const std::string& partition_name,
                                           const std::string& account_name);
 
   void CranedUp(const CranedId& craned_id);

src/CraneCtld/CtldGrpcServer.cpp

@@ -1014,12 +1014,10 @@ CtldServer::SubmitTaskToScheduler(std::unique_ptr<TaskInCtld> task) {
     return std::unexpected(enable_res.error());
   }
 
-  if (!g_meta_container->CheckIfAccountIsAllowedInPartition(task->partition_id,
-                                                            task->account))
-    return std::unexpected(
-        "The account is not in the AllowedAccounts of the partition "
-        "specified "
-        "for the task, submission of the task is prohibited.");
+  auto result = g_meta_container->CheckIfAccountIsAllowedInPartition(task->partition_id,
+                                                            task->account);
+  if (!result)
+    return std::unexpected(result.error());
 
   err = g_task_scheduler->AcquireTaskAttributes(task.get());