Skip to content

Commit 1c41631

Browse files
TCeasonTCeason
committed
1. Normalize the code
2. grant ownership on warehouse object only supported for warehouses managed by the system
1 parent ce9a14f commit 1c41631

File tree

14 files changed

+42
-26
lines changed

14 files changed

+42
-26
lines changed

‎src/meta/app/src/principal/ownership_object.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub enum OwnershipObject {
5454
},
5555

5656
Warehouse {
57-
uid: String,
57+
id: String,
5858
},
5959
}
6060

@@ -84,7 +84,7 @@ impl fmt::Display for OwnershipObject {
8484
}
8585
OwnershipObject::UDF { name } => write!(f, "UDF {name}"),
8686
OwnershipObject::Stage { name } => write!(f, "STAGE {name}"),
87-
OwnershipObject::Warehouse { uid } => write!(f, "Warehouse {uid}"),
87+
OwnershipObject::Warehouse { id: uid } => write!(f, "Warehouse {uid}"),
8888
}
8989
}
9090
}
@@ -124,7 +124,7 @@ impl KeyCodec for OwnershipObject {
124124
}
125125
OwnershipObject::Stage { name } => b.push_raw("stage-by-name").push_str(name),
126126
OwnershipObject::UDF { name } => b.push_raw("udf-by-name").push_str(name),
127-
OwnershipObject::Warehouse { uid } => b.push_raw("warehouse-by-uid").push_str(uid),
127+
OwnershipObject::Warehouse { id: uid } => b.push_raw("warehouse-by-uid").push_str(uid),
128128
}
129129
}
130130

@@ -173,7 +173,7 @@ impl KeyCodec for OwnershipObject {
173173
}
174174
"warehouse-by-uid" => {
175175
let uid = p.next_str()?;
176-
Ok(OwnershipObject::Warehouse { uid })
176+
Ok(OwnershipObject::Warehouse { id: uid })
177177
}
178178
_ => Err(kvapi::KeyError::InvalidSegment {
179179
i: p.index(),

‎src/meta/app/src/principal/tenant_ownership_object_ident.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ mod tests {
260260
let role_grantee = TenantOwnershipObjectIdent::new_unchecked(
261261
Tenant::new_literal("test"),
262262
OwnershipObject::Warehouse {
263-
uid: "n87s".to_string(),
263+
id: "n87s".to_string(),
264264
},
265265
);
266266

‎src/meta/app/src/principal/user_privilege.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,9 @@ impl UserPrivilegeSet {
207207
let stage_privs_without_ownership = Self::available_privileges_on_stage(false);
208208
let udf_privs_without_ownership = Self::available_privileges_on_udf(false);
209209
let wh_privs_without_ownership = Self::available_privileges_on_warehouse(false);
210-
let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask | CreateWarehouse });
210+
// TODO : The warehouse functionality is not yet fully integrated. Therefore, the CreateWarehouse permission must be granted separately
211+
// If self-created user or configured user wants to create warehouse in system-manage cluster must execute: grant create warehouse on *.* to <user_name>;
212+
let privs = make_bitflags!(UserPrivilegeType::{ Usage | Super | CreateUser | DropUser | CreateRole | DropRole | CreateDatabase | Grant | CreateDataMask });
211213
(database_privs.privileges
212214
| privs
213215
| stage_privs_without_ownership.privileges

‎src/meta/proto-conv/src/ownership_from_to_protobuf_impl.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ impl FromToProto for mt::principal::OwnershipObject {
9090
}) => Ok(mt::principal::OwnershipObject::Stage { name: stage }),
9191
pb::ownership_object::Object::Warehouse(
9292
pb::ownership_object::OwnershipWarehouseObject { uid },
93-
) => Ok(mt::principal::OwnershipObject::Warehouse { uid }),
93+
) => Ok(mt::principal::OwnershipObject::Warehouse { id: uid }),
9494
}
9595
}
9696

@@ -126,7 +126,7 @@ impl FromToProto for mt::principal::OwnershipObject {
126126
stage: name.clone(),
127127
}),
128128
),
129-
mt::principal::OwnershipObject::Warehouse { uid } => {
129+
mt::principal::OwnershipObject::Warehouse { id: uid } => {
130130
Some(pb::ownership_object::Object::Warehouse(
131131
pb::ownership_object::OwnershipWarehouseObject { uid: uid.clone() },
132132
))

‎src/meta/proto-conv/tests/it/v120_warehouse_ownershipobject.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ fn test_decode_v120_ownership() -> anyhow::Result<()> {
106106
let want = || mt::principal::OwnershipInfo {
107107
role: "r1".to_string(),
108108
object: OwnershipObject::Warehouse {
109-
uid: "auniqueid".to_string(),
109+
id: "auniqueid".to_string(),
110110
},
111111
};
112112
common::test_pb_from_to(func_name!(), want())?;

‎src/query/management/src/role/role_mgr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ fn convert_to_grant_obj(owner_obj: &OwnershipObject) -> GrantObject {
504504
} => GrantObject::TableById(catalog_name.to_string(), *db_id, *table_id),
505505
OwnershipObject::Stage { name } => GrantObject::Stage(name.to_string()),
506506
OwnershipObject::UDF { name } => GrantObject::UDF(name.to_string()),
507-
OwnershipObject::Warehouse { uid } => GrantObject::Warehouse(uid.to_string()),
507+
OwnershipObject::Warehouse { id: uid } => GrantObject::Warehouse(uid.to_string()),
508508
}
509509
}
510510

‎src/query/service/src/interpreters/access/privilege_access.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ impl PrivilegeAccess {
155155
name: name.to_string(),
156156
},
157157
GrantObject::Warehouse(uid) => OwnershipObject::Warehouse {
158-
uid: uid.to_string(),
158+
id: uid.to_string(),
159159
},
160160
GrantObject::Global => return Ok(None),
161161
};

‎src/query/service/src/interpreters/interpreter_create_warehouses.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ impl Interpreter for CreateWarehouseInterpreter {
8787
if let Some(current_role) = self.ctx.get_current_role() {
8888
role_api
8989
.grant_ownership(
90-
&OwnershipObject::Warehouse { uid: sw.role_id },
90+
&OwnershipObject::Warehouse { id: sw.role_id },
9191
&current_role.name,
9292
)
9393
.await?;
@@ -118,7 +118,7 @@ impl Interpreter for CreateWarehouseInterpreter {
118118
if let Some(current_role) = self.ctx.get_current_role() {
119119
role_api
120120
.grant_ownership(
121-
&OwnershipObject::Warehouse { uid: sw.role_id },
121+
&OwnershipObject::Warehouse { id: sw.role_id },
122122
&current_role.name,
123123
)
124124
.await?;

‎src/query/service/src/interpreters/interpreter_drop_warehouses.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ impl Interpreter for DropWarehouseInterpreter {
7676
if let Some(current_role) = self.ctx.get_current_role() {
7777
role_api
7878
.grant_ownership(
79-
&OwnershipObject::Warehouse { uid: sw.role_id },
79+
&OwnershipObject::Warehouse { id: sw.role_id },
8080
&current_role.name,
8181
)
8282
.await?;

‎src/query/service/src/interpreters/interpreter_privilege_grant.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
use std::sync::Arc;
1616

17+
use databend_common_base::base::GlobalInstance;
1718
use databend_common_exception::ErrorCode;
1819
use databend_common_exception::Result;
1920
use databend_common_meta_app::principal::GrantObject;
@@ -25,6 +26,7 @@ use databend_common_meta_app::tenant::Tenant;
2526
use databend_common_sql::plans::GrantPrivilegePlan;
2627
use databend_common_users::RoleCacheManager;
2728
use databend_common_users::UserApiProvider;
29+
use databend_enterprise_resources_management::ResourcesManagement;
2830
use log::debug;
2931
use log::error;
3032
use log::info;
@@ -102,7 +104,7 @@ impl GrantPrivilegeInterpreter {
102104
name: name.to_string(),
103105
}),
104106
GrantObject::Warehouse(uid) => Ok(OwnershipObject::Warehouse {
105-
uid: uid.to_string(),
107+
id: uid.to_string(),
106108
}),
107109
GrantObject::Global => Err(ErrorCode::IllegalGrant(
108110
"Illegal GRANT/REVOKE command; please consult the manual to see which privileges can be used",
@@ -203,6 +205,17 @@ impl Interpreter for GrantPrivilegeInterpreter {
203205
.convert_to_ownerobject(&tenant, &plan.on, plan.on.catalog())
204206
.await?;
205207
if self.ctx.get_current_role().is_some() {
208+
if let OwnershipObject::Warehouse { .. } = owner_object {
209+
let warehouse_mgr =
210+
GlobalInstance::get::<Arc<dyn ResourcesManagement>>();
211+
212+
// Only support grant ownership when support_forward_warehouse_request is true
213+
if !warehouse_mgr.support_forward_warehouse_request() {
214+
return Err(ErrorCode::IllegalGrant(
215+
"Illegal GRANT/REVOKE command; only supported for warehouses managed by the system",
216+
));
217+
}
218+
}
206219
self.grant_ownership(&self.ctx, &tenant, &owner_object, &role)
207220
.await?;
208221
} else {

‎src/query/service/src/table_functions/show_grants/show_grants_table.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ async fn show_account_grants(
516516
privileges.push("OWNERSHIP".to_string());
517517
grant_list.push(format!("GRANT OWNERSHIP ON UDF {} TO {}", name, identity));
518518
}
519-
OwnershipObject::Warehouse { uid } => {
519+
OwnershipObject::Warehouse { id: uid } => {
520520
if let Some(sw) = warehouses
521521
.iter()
522522
.filter_map(|w| {
@@ -739,27 +739,25 @@ async fn show_object_grant(
739739
return Err(ErrorCode::InvalidArgument("The 'SHOW GRANTS ON <warehouse_name>' only supported for warehouses managed by the system. Please verify that you are using a system-managed warehouse".to_string()));
740740
}
741741
let warehouses = warehouse_mgr.list_warehouses().await?;
742-
let mut uid = String::new();
742+
let mut id = String::new();
743743
for w in warehouses {
744744
if let WarehouseInfo::SystemManaged(rw) = w {
745745
if rw.id == name {
746-
uid = rw.role_id.to_string();
746+
id = rw.role_id.to_string();
747747
break;
748748
}
749749
}
750750
}
751-
if !visibility_checker.check_warehouse_visibility(&uid) {
751+
if !visibility_checker.check_warehouse_visibility(&id) {
752752
return Err(ErrorCode::PermissionDenied(format!(
753753
"Permission denied: No privilege on warehouse {} for user {}.",
754754
name, current_user
755755
)));
756756
}
757757
(
758-
GrantObject::Warehouse(uid.to_string()),
759-
OwnershipObject::Warehouse {
760-
uid: uid.to_string(),
761-
},
762-
Some(uid),
758+
GrantObject::Warehouse(id.to_string()),
759+
OwnershipObject::Warehouse { id: id.to_string() },
760+
Some(id),
763761
name,
764762
)
765763
}

‎src/query/users/src/user_mgr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ impl UserApiProvider {
233233
}
234234
if let GrantObject::Warehouse(_) = object {
235235
return Err(ErrorCode::IllegalUser(format!(
236-
"Cannot grant warehouse privileges to user `{}`",
236+
"Cannot revoke warehouse privileges to user `{}`",
237237
user.username
238238
)));
239239
}

‎src/query/users/src/visibility_checker.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ impl GrantObjectVisibilityChecker {
200200
OwnershipObject::UDF { name } => {
201201
granted_udfs.insert(name.to_string());
202202
}
203-
OwnershipObject::Warehouse { uid } => {
203+
OwnershipObject::Warehouse { id: uid } => {
204204
granted_ws.insert(uid.to_string());
205205
}
206206
}

‎tests/sqllogictests/suites/base/05_ddl/05_0017_ddl_grant_role.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,6 @@ drop role if exists role1;
264264

265265
statement ok
266266
drop role if exists role2;
267+
268+
statement error 1061
269+
GRANT ownership on warehouse w1 to role 'role1';

0 commit comments

Comments
 (0)