diff --git a/src/query/management/src/role/role_api.rs b/src/query/management/src/role/role_api.rs index 6b46adfcdf946..ba4939d5414ab 100644 --- a/src/query/management/src/role/role_api.rs +++ b/src/query/management/src/role/role_api.rs @@ -32,7 +32,7 @@ pub trait RoleApi: Sync + Send { async fn get_raw_meta_roles(&self) -> Result; - async fn get_ownerships(&self) -> Result>>; + async fn list_ownerships(&self) -> Result>>; /// General role update. /// diff --git a/src/query/management/src/role/role_mgr.rs b/src/query/management/src/role/role_mgr.rs index 01d0c52cb9e0b..6b0c3662537a3 100644 --- a/src/query/management/src/role/role_mgr.rs +++ b/src/query/management/src/role/role_mgr.rs @@ -57,6 +57,7 @@ static TXN_MAX_RETRY_TIMES: u32 = 60; static BUILTIN_ROLE_ACCOUNT_ADMIN: &str = "account_admin"; +#[derive(Clone)] pub struct RoleMgr { kv_api: Arc + Send + Sync>, tenant: Tenant, @@ -210,7 +211,7 @@ impl RoleApi for RoleMgr { #[async_backtrace::framed] #[fastrace::trace] - async fn get_ownerships(&self) -> Result>, ErrorCode> { + async fn list_ownerships(&self) -> Result>, ErrorCode> { let object_owner_prefix = self.ownership_object_prefix(); let values = self .kv_api @@ -228,7 +229,7 @@ impl RoleApi for RoleMgr { // After rollback the old version, deserialize will return Err Ownership can not be none. // But get ownerships should try to ensure success because in this version. Err(err) => error!( - "deserialize key {} Got err {} while (get_ownerships)", + "deserialize key {} Got err {} while (list_ownerships)", &key, err ), } @@ -269,11 +270,11 @@ impl RoleApi for RoleMgr { /// Only drop role will call transfer. /// - /// If a role is dropped, but the owner object is exists, + /// If a role is dropped, but the owner object is existing, /// /// The owner role need to update to account_admin. /// - /// get_ownerships use prefix_list_kv that will generate once meta call + /// list_ownerships use prefix_list_kv that will generate once meta call /// /// According to Txn reduce meta call. If role own n objects, will generate once meta call. #[async_backtrace::framed] @@ -287,7 +288,7 @@ impl RoleApi for RoleMgr { trials.next().unwrap().map_err(AppError::from)?.await; let mut if_then = vec![]; let mut condition = vec![]; - let seq_owns = self.get_ownerships().await.map_err(|e| { + let seq_owns = self.list_ownerships().await.map_err(|e| { e.add_message_back("(while in transfer_ownership_to_admin get ownerships).") })?; let mut need_transfer = false; diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index 46c00e9ef5f19..eebe6def3010d 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -764,7 +764,7 @@ impl AccessChecker for PrivilegeAccess { let user_api = UserApiProvider::instance(); let ownerships = user_api .role_api(&tenant) - .get_ownerships() + .list_ownerships() .await?; let roles = self.ctx.get_all_effective_roles().await?; let roles_name: Vec = roles.iter().map(|role| role.name.to_string()).collect(); @@ -782,7 +782,7 @@ impl AccessChecker for PrivilegeAccess { let user_api = UserApiProvider::instance(); let ownerships = user_api .role_api(&tenant) - .get_ownerships() + .list_ownerships() .await?; let roles = self.ctx.get_all_effective_roles().await?; let roles_name: Vec = roles.iter().map(|role| role.name.to_string()).collect(); @@ -937,7 +937,7 @@ impl AccessChecker for PrivilegeAccess { let user_api = UserApiProvider::instance(); let ownerships = user_api .role_api(&tenant) - .get_ownerships() + .list_ownerships() .await?; let roles = self.ctx.get_all_effective_roles().await?; let roles_name: Vec = roles.iter().map(|role| role.name.to_string()).collect(); diff --git a/src/query/service/src/interpreters/interpreter_create_warehouses.rs b/src/query/service/src/interpreters/interpreter_create_warehouses.rs index 3cb1cf4930bf6..d75578c0a0e70 100644 --- a/src/query/service/src/interpreters/interpreter_create_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_create_warehouses.rs @@ -83,8 +83,8 @@ impl Interpreter for CreateWarehouseInterpreter { .await?; if let WarehouseInfo::SystemManaged(sw) = warehouse { - let role_api = UserApiProvider::instance().role_api(&tenant); if let Some(current_role) = self.ctx.get_current_role() { + let role_api = UserApiProvider::instance().role_api(&tenant); role_api .grant_ownership( &OwnershipObject::Warehouse { id: sw.role_id }, @@ -114,8 +114,8 @@ impl Interpreter for CreateWarehouseInterpreter { .await?; if let WarehouseInfo::SystemManaged(sw) = warehouse { - let role_api = UserApiProvider::instance().role_api(&tenant); if let Some(current_role) = self.ctx.get_current_role() { + let role_api = UserApiProvider::instance().role_api(&tenant); role_api .grant_ownership( &OwnershipObject::Warehouse { id: sw.role_id }, diff --git a/src/query/service/src/interpreters/interpreter_database_create.rs b/src/query/service/src/interpreters/interpreter_database_create.rs index 3bd38c2d67ff3..aeb9c90562b85 100644 --- a/src/query/service/src/interpreters/interpreter_database_create.rs +++ b/src/query/service/src/interpreters/interpreter_database_create.rs @@ -75,8 +75,8 @@ impl Interpreter for CreateDatabaseInterpreter { // Grant ownership as the current role. The above create_db_req.meta.owner could be removed in // the future. - let role_api = UserApiProvider::instance().role_api(&tenant); if let Some(current_role) = self.ctx.get_current_role() { + let role_api = UserApiProvider::instance().role_api(&tenant); role_api .grant_ownership( &OwnershipObject::Database { diff --git a/src/query/service/src/interpreters/interpreter_drop_warehouses.rs b/src/query/service/src/interpreters/interpreter_drop_warehouses.rs index dd3f9253e66a8..2d4d2bdce1010 100644 --- a/src/query/service/src/interpreters/interpreter_drop_warehouses.rs +++ b/src/query/service/src/interpreters/interpreter_drop_warehouses.rs @@ -72,8 +72,8 @@ impl Interpreter for DropWarehouseInterpreter { ); if let WarehouseInfo::SystemManaged(sw) = warehouse { - let role_api = UserApiProvider::instance().role_api(&tenant); if let Some(current_role) = self.ctx.get_current_role() { + let role_api = UserApiProvider::instance().role_api(&tenant); role_api .grant_ownership( &OwnershipObject::Warehouse { id: sw.role_id }, diff --git a/src/query/service/src/interpreters/interpreter_user_stage_create.rs b/src/query/service/src/interpreters/interpreter_user_stage_create.rs index 6597fed8fe7b1..d4d95aeb5e6f1 100644 --- a/src/query/service/src/interpreters/interpreter_user_stage_create.rs +++ b/src/query/service/src/interpreters/interpreter_user_stage_create.rs @@ -121,8 +121,8 @@ impl Interpreter for CreateUserStageInterpreter { // Grant ownership as the current role let tenant = self.ctx.get_tenant(); - let role_api = UserApiProvider::instance().role_api(&tenant); if let Some(current_role) = self.ctx.get_current_role() { + let role_api = UserApiProvider::instance().role_api(&tenant); role_api .grant_ownership( &OwnershipObject::Stage { diff --git a/src/query/service/src/sessions/session_privilege_mgr.rs b/src/query/service/src/sessions/session_privilege_mgr.rs index 381f2036db26a..383954862e8d4 100644 --- a/src/query/service/src/sessions/session_privilege_mgr.rs +++ b/src/query/service/src/sessions/session_privilege_mgr.rs @@ -397,7 +397,7 @@ impl SessionPrivilegeManager for SessionPrivilegeManagerImpl<'_> { let user_api = UserApiProvider::instance(); let ownerships = user_api .role_api(&self.session_ctx.get_current_tenant()) - .get_ownerships() + .list_ownerships() .await?; let mut ownership_objects = vec![]; for ownership in ownerships { diff --git a/src/query/service/src/table_functions/show_grants/show_grants_table.rs b/src/query/service/src/table_functions/show_grants/show_grants_table.rs index c4bb346ca1849..897f08469ad1e 100644 --- a/src/query/service/src/table_functions/show_grants/show_grants_table.rs +++ b/src/query/service/src/table_functions/show_grants/show_grants_table.rs @@ -495,7 +495,7 @@ async fn show_account_grants( // 2. not expand roles // So no need to get ownerships. if !roles.is_empty() { - let ownerships = user_api.role_api(&tenant).get_ownerships().await?; + let ownerships = user_api.role_api(&tenant).list_ownerships().await?; for ownership in ownerships { if roles.contains(&ownership.data.role) { match ownership.data.object { @@ -807,7 +807,7 @@ async fn show_object_grant( } } - let ownerships = user_api.role_api(&tenant).get_ownerships().await?; + let ownerships = user_api.role_api(&tenant).list_ownerships().await?; for ownership in ownerships { if ownership.data.object == owner_object { privileges.push("OWNERSHIP".to_string()); diff --git a/src/query/storages/system/src/streams_table.rs b/src/query/storages/system/src/streams_table.rs index f752186b84fea..7808d9ea5a439 100644 --- a/src/query/storages/system/src/streams_table.rs +++ b/src/query/storages/system/src/streams_table.rs @@ -157,7 +157,7 @@ impl AsyncSystemTable for StreamsTable { .collect::>(); let ownership = if T { - user_api.get_ownerships(&tenant).await.unwrap_or_default() + user_api.list_ownerships(&tenant).await.unwrap_or_default() } else { HashMap::new() }; diff --git a/src/query/storages/system/src/tables_table.rs b/src/query/storages/system/src/tables_table.rs index f97898328291e..004d43914a933 100644 --- a/src/query/storages/system/src/tables_table.rs +++ b/src/query/storages/system/src/tables_table.rs @@ -589,7 +589,7 @@ where TablesTable: HistoryAware dbs.clear(); let ownership = if get_ownership && default_catalog { - user_api.get_ownerships(&tenant).await.unwrap_or_default() + user_api.list_ownerships(&tenant).await.unwrap_or_default() } else { HashMap::new() }; diff --git a/src/query/users/src/role_mgr.rs b/src/query/users/src/role_mgr.rs index 4f3ca63ff630d..889a2dce73425 100644 --- a/src/query/users/src/role_mgr.rs +++ b/src/query/users/src/role_mgr.rs @@ -84,13 +84,13 @@ impl UserApiProvider { } #[async_backtrace::framed] - pub async fn get_ownerships( + pub async fn list_ownerships( &self, tenant: &Tenant, ) -> Result> { let seq_owns = self .role_api(tenant) - .get_ownerships() + .list_ownerships() .await .map_err(|e| e.add_message_back("(while get ownerships)."))?; @@ -172,7 +172,7 @@ impl UserApiProvider { if let Some(owner) = ownership { // if object has ownership, but the owner role is not exists, set owner role to ACCOUNT_ADMIN, // only account_admin can access this object. - // Note: get_ownerships no need to do this check. + // Note: list_ownerships no need to do this check. // Because this can cause system.table queries to slow down // The intention is that the account admin can grant ownership. // So system.tables will display dropped role. It's by design. diff --git a/src/query/users/src/user_api.rs b/src/query/users/src/user_api.rs index a5c4ef26dfb66..3aca8e7c93e7c 100644 --- a/src/query/users/src/user_api.rs +++ b/src/query/users/src/user_api.rs @@ -28,7 +28,6 @@ use databend_common_management::PasswordPolicyMgr; use databend_common_management::ProcedureMgr; use databend_common_management::QuotaApi; use databend_common_management::QuotaMgr; -use databend_common_management::RoleApi; use databend_common_management::RoleMgr; use databend_common_management::SettingMgr; use databend_common_management::StageApi; @@ -45,6 +44,7 @@ use databend_common_meta_store::MetaStore; use databend_common_meta_store::MetaStoreProvider; use databend_common_meta_types::MatchSeq; use databend_common_meta_types::MetaError; +use log::debug; use crate::builtin::BuiltIn; use crate::BUILTIN_ROLE_PUBLIC; @@ -65,6 +65,7 @@ impl UserApiProvider { ) -> Result<()> { GlobalInstance::set(Self::try_create(conf, builtin, tenant).await?); let user_mgr = UserApiProvider::instance(); + if let Some(q) = quota { let i = user_mgr.tenant_quota_api(tenant); let res = i.get_quota(MatchSeq::GE(0)).await?; @@ -125,13 +126,14 @@ impl UserApiProvider { Arc::new(user_mgr) } - pub fn role_api(&self, tenant: &Tenant) -> Arc { + pub fn role_api(&self, tenant: &Tenant) -> RoleMgr { let role_mgr = RoleMgr::create( self.client.clone(), tenant, GlobalConfig::instance().query.upgrade_to_pb, ); - Arc::new(role_mgr) + debug!("RoleMgr created"); + role_mgr } pub fn stage_api(&self, tenant: &Tenant) -> Arc {