Skip to content

Commit f052628

Browse files
farhan-syahfarhan-syah
authored
Merge pull request #132 from emanzx/fix/drop-tenant-by-name
fix(tenant): accept tenant name in DROP/ALTER/PURGE TENANT
2 parents 535021e + 39a23d2 commit f052628

6 files changed

Lines changed: 499 additions & 47 deletions

File tree

‎nodedb/src/control/server/pgwire/ddl/tenant/alter.rs‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3-
//! `ALTER TENANT <id> SET QUOTA <field> = <value>` handler.
3+
//! `ALTER TENANT <id|name> SET QUOTA <field> = <value>` handler.
44
//!
55
//! Tenant quotas live in the in-memory `TenantStore` and are not part
66
//! of `StoredTenant`. Quota replication is handled separately from
77
//! the tenant identity record.
8+
//!
9+
//! The tenant reference accepts either a numeric id or a tenant name
10+
//! (single-quoted optional), parallel to `CREATE TENANT <name>` and
11+
//! `SHOW TENANT <name|id>`.
812
913
use pgwire::api::results::{Response, Tag};
1014
use pgwire::error::PgWireResult;
1115

1216
use crate::control::security::audit::AuditEvent;
1317
use crate::control::security::identity::AuthenticatedIdentity;
1418
use crate::control::state::SharedState;
15-
use crate::types::TenantId;
1619

1720
use super::super::super::types::sqlstate_error;
1821

@@ -31,19 +34,28 @@ pub fn alter_tenant(
3134
if parts.len() < 7 {
3235
return Err(sqlstate_error(
3336
"42601",
34-
"syntax: ALTER TENANT <id> SET QUOTA <field> = <value>",
37+
"syntax: ALTER TENANT <id|name> SET QUOTA <field> = <value>",
3538
));
3639
}
3740

38-
let tid: u64 = parts[2]
39-
.parse()
40-
.map_err(|_| sqlstate_error("42601", "TENANT ID must be a numeric value"))?;
41-
let tenant_id = TenantId::new(tid);
41+
// Accept either a numeric id or a tenant name (mirrors CREATE/SHOW/DROP).
42+
let tenant_id = super::resolve_tenant_ref(state, parts[2])?
43+
.ok_or_else(|| sqlstate_error("42704", &format!("tenant '{}' does not exist", parts[2])))?;
44+
45+
// Existence gate, uniform across numeric ids and resolved names: altering an
46+
// unknown tenant must error rather than silently seed a default quota for a
47+
// phantom id.
48+
if !super::tenant_exists(state, tenant_id)? {
49+
return Err(sqlstate_error(
50+
"42704",
51+
&format!("tenant '{}' does not exist", parts[2]),
52+
));
53+
}
4254

4355
if !parts[3].eq_ignore_ascii_case("SET") || !parts[4].eq_ignore_ascii_case("QUOTA") {
4456
return Err(sqlstate_error(
4557
"42601",
46-
"expected SET QUOTA after tenant ID",
58+
"expected SET QUOTA after tenant id or name",
4759
));
4860
}
4961

‎nodedb/src/control/server/pgwire/ddl/tenant/drop.rs‎

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3-
//! `DROP TENANT [IF EXISTS] <id>` handler. Migrated to
3+
//! `DROP TENANT [IF EXISTS] <id|name>` handler. Migrated to
44
//! `CatalogEntry::DeleteTenant` in phase 1k.6.
5+
//!
6+
//! Accepts either a numeric tenant id or a tenant name (single-quoted
7+
//! optional), parallel to the `CREATE TENANT <name>` and
8+
//! `SHOW TENANT <name|id>` paths.
59
610
use pgwire::api::results::{Response, Tag};
711
use pgwire::error::PgWireResult;
@@ -11,28 +15,10 @@ use crate::control::metadata_proposer::propose_catalog_entry;
1115
use crate::control::security::audit::AuditEvent;
1216
use crate::control::security::identity::AuthenticatedIdentity;
1317
use crate::control::state::SharedState;
14-
use crate::types::TenantId;
1518

1619
use super::super::super::types::sqlstate_error;
1720
use super::super::parse_utils::strip_if_exists;
18-
19-
/// Whether a tenant id currently exists, consulting the redb catalog when
20-
/// one is wired up and falling back to the in-memory quota table otherwise.
21-
fn tenant_exists(state: &SharedState, tenant_id: TenantId) -> PgWireResult<bool> {
22-
if let Some(catalog) = state.credentials.catalog() {
23-
let present = catalog
24-
.load_all_tenants()
25-
.map_err(|e| sqlstate_error("XX000", &format!("catalog read: {e}")))?
26-
.iter()
27-
.any(|t| t.tenant_id == tenant_id.as_u64());
28-
return Ok(present);
29-
}
30-
let tenants = match state.tenants.lock() {
31-
Ok(t) => t,
32-
Err(p) => p.into_inner(),
33-
};
34-
Ok(tenants.has_quota(tenant_id))
35-
}
21+
use super::tenant_exists;
3622

3723
pub fn drop_tenant(
3824
state: &SharedState,
@@ -51,22 +37,44 @@ pub fn drop_tenant(
5137
if parts.len() < 3 {
5238
return Err(sqlstate_error(
5339
"42601",
54-
"syntax: DROP TENANT [IF EXISTS] <id>",
40+
"syntax: DROP TENANT [IF EXISTS] <id|name>",
5541
));
5642
}
5743

58-
let tid: u64 = parts[2]
59-
.parse()
60-
.map_err(|_| sqlstate_error("42601", "TENANT ID must be a numeric value"))?;
61-
let tenant_id = TenantId::new(tid);
44+
// Accept either a numeric id or a tenant name; mirror the existing
45+
// CREATE TENANT name-resolution path. A name that matches no tenant yields
46+
// `None` here; an unknown numeric id resolves to a candidate that the
47+
// existence gate below rejects, so both forms behave identically.
48+
let tenant_id = match super::resolve_tenant_ref(state, parts[2])? {
49+
Some(tid) => tid,
50+
None => {
51+
// Name token did not resolve to any tenant.
52+
if if_exists {
53+
return Ok(vec![Response::Execution(Tag::new("DROP TENANT"))]);
54+
}
55+
return Err(sqlstate_error(
56+
"42704",
57+
&format!("tenant '{}' does not exist", parts[2]),
58+
));
59+
}
60+
};
61+
let tid = tenant_id.as_u64();
6262

6363
if tid == 0 {
6464
return Err(sqlstate_error("42501", "cannot drop system tenant (0)"));
6565
}
6666

67-
// `IF EXISTS`: dropping a tenant that does not exist is a no-op success.
68-
if if_exists && !tenant_exists(state, tenant_id)? {
69-
return Ok(vec![Response::Execution(Tag::new("DROP TENANT"))]);
67+
// Existence gate, uniform across numeric ids and resolved names: an unknown
68+
// tenant is a no-op under `IF EXISTS`, otherwise `42704` — never a silent
69+
// delete proposal for a tenant that does not exist.
70+
if !tenant_exists(state, tenant_id)? {
71+
if if_exists {
72+
return Ok(vec![Response::Execution(Tag::new("DROP TENANT"))]);
73+
}
74+
return Err(sqlstate_error(
75+
"42704",
76+
&format!("tenant '{}' does not exist", parts[2]),
77+
));
7078
}
7179

7280
let entry = CatalogEntry::DeleteTenant { tenant_id: tid };

‎nodedb/src/control/server/pgwire/ddl/tenant/mod.rs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,88 @@ pub use show::{show_tenant_quota, show_tenant_usage};
3131
pub use show_in_database::{
3232
handle_show_tenant_quota_in_database, handle_show_tenant_usage_in_database,
3333
};
34+
35+
use pgwire::error::PgWireResult;
36+
37+
use crate::control::state::SharedState;
38+
use crate::types::TenantId;
39+
40+
use super::super::types::sqlstate_error;
41+
42+
/// Resolve a tenant reference token to a [`TenantId`], accepting either a
43+
/// numeric id or a tenant name.
44+
///
45+
/// The numeric form is the legacy fast path and requires no catalog access;
46+
/// any `u64`-parseable token returns `Ok(Some(TenantId::new(id)))` whether or
47+
/// not that id currently exists. Resolving a token to an id does **not** assert
48+
/// the tenant exists — callers must gate the operation through [`tenant_exists`]
49+
/// so that an unknown numeric id and an unknown name behave identically (both
50+
/// `42704`, or an `IF EXISTS` no-op). Skipping that check reintroduces the
51+
/// id/name asymmetry where a bogus numeric id silently "succeeds".
52+
///
53+
/// A non-numeric token is treated as a tenant name and resolved via
54+
/// [`find_tenant_by_name`]. Single-quoted names are unwrapped, mirroring the
55+
/// AST `TenantSelector` behavior introduced for the CREATE/SHOW paths.
56+
/// `Ok(None)` is returned if the name does not match any tenant, so the
57+
/// caller can decide between `IF EXISTS` no-op success and an explicit
58+
/// `42704` error.
59+
///
60+
/// Errors:
61+
/// - `42601` — empty token (after quote stripping).
62+
/// - `42601` — non-numeric token but catalog is unavailable.
63+
/// - `XX000` — catalog read failure.
64+
///
65+
/// Used by `DROP TENANT`, `ALTER TENANT SET QUOTA`, and `PURGE TENANT` to
66+
/// accept names in addition to numeric ids, parallel to the existing
67+
/// `CREATE TENANT <name>` and `SHOW TENANT <name>` support.
68+
///
69+
/// [`find_tenant_by_name`]:
70+
/// crate::control::security::credential::store::CredentialStore::catalog
71+
pub(super) fn resolve_tenant_ref(
72+
state: &SharedState,
73+
token: &str,
74+
) -> PgWireResult<Option<TenantId>> {
75+
// Numeric id fast path — legacy compatible.
76+
if let Ok(id) = token.parse::<u64>() {
77+
return Ok(Some(TenantId::new(id)));
78+
}
79+
// Name resolution via catalog.
80+
let name = token.trim_matches('\'');
81+
if name.is_empty() {
82+
return Err(sqlstate_error(
83+
"42601",
84+
"TENANT reference must be a numeric id or a tenant name",
85+
));
86+
}
87+
let catalog = state.credentials.catalog().as_ref().ok_or_else(|| {
88+
sqlstate_error(
89+
"42601",
90+
"cannot resolve tenant by name: catalog unavailable; use numeric id",
91+
)
92+
})?;
93+
Ok(catalog
94+
.find_tenant_by_name(name)
95+
.map_err(|e| sqlstate_error("XX000", &format!("catalog read: {e}")))?
96+
.map(|stored| TenantId::new(stored.tenant_id)))
97+
}
98+
99+
/// Whether `tenant_id` currently exists, consulting the redb catalog when one
100+
/// is wired up and falling back to the in-memory quota table otherwise.
101+
///
102+
/// Shared by `DROP`, `ALTER`, and `PURGE TENANT` so existence is enforced the
103+
/// same way for numeric ids and resolved names — see [`resolve_tenant_ref`].
104+
pub(super) fn tenant_exists(state: &SharedState, tenant_id: TenantId) -> PgWireResult<bool> {
105+
if let Some(catalog) = state.credentials.catalog() {
106+
let present = catalog
107+
.load_all_tenants()
108+
.map_err(|e| sqlstate_error("XX000", &format!("catalog read: {e}")))?
109+
.iter()
110+
.any(|t| t.tenant_id == tenant_id.as_u64());
111+
return Ok(present);
112+
}
113+
let tenants = match state.tenants.lock() {
114+
Ok(t) => t,
115+
Err(p) => p.into_inner(),
116+
};
117+
Ok(tenants.has_quota(tenant_id))
118+
}

‎nodedb/src/control/server/pgwire/ddl/tenant/purge.rs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3-
//! `PURGE TENANT <id> CONFIRM` — Data Plane meta op that deletes
3+
//! `PURGE TENANT <id|name> CONFIRM` — Data Plane meta op that deletes
44
//! ALL tenant data across every engine. Superuser-only, requires
55
//! the literal `CONFIRM` keyword.
6+
//!
7+
//! The tenant reference accepts either a numeric id or a tenant name
8+
//! (single-quoted optional), parallel to `CREATE TENANT <name>` and
9+
//! `SHOW TENANT <name|id>`.
610
711
use pgwire::api::results::{Response, Tag};
812
use pgwire::error::PgWireResult;
913

1014
use crate::control::security::audit::AuditEvent;
1115
use crate::control::security::identity::AuthenticatedIdentity;
1216
use crate::control::state::SharedState;
13-
use crate::types::TenantId;
1417

1518
use super::super::super::types::sqlstate_error;
1619

@@ -27,26 +30,37 @@ pub async fn purge_tenant(
2730
}
2831

2932
if parts.len() < 4 {
30-
return Err(sqlstate_error("42601", "syntax: PURGE TENANT <id> CONFIRM"));
33+
return Err(sqlstate_error(
34+
"42601",
35+
"syntax: PURGE TENANT <id|name> CONFIRM",
36+
));
3137
}
3238

33-
let tid: u64 = parts[2]
34-
.parse()
35-
.map_err(|_| sqlstate_error("42601", "TENANT ID must be a numeric value"))?;
39+
// Accept either a numeric id or a tenant name (mirrors CREATE/SHOW/DROP).
40+
let tenant_id = super::resolve_tenant_ref(state, parts[2])?
41+
.ok_or_else(|| sqlstate_error("42704", &format!("tenant '{}' does not exist", parts[2])))?;
42+
let tid = tenant_id.as_u64();
3643

3744
if tid == 0 {
3845
return Err(sqlstate_error("42501", "cannot purge system tenant (0)"));
3946
}
4047

48+
// Existence gate, uniform across numeric ids and resolved names: refuse to
49+
// dispatch the destructive meta op for a tenant that does not exist.
50+
if !super::tenant_exists(state, tenant_id)? {
51+
return Err(sqlstate_error(
52+
"42704",
53+
&format!("tenant '{}' does not exist", parts[2]),
54+
));
55+
}
56+
4157
if !parts[3].eq_ignore_ascii_case("CONFIRM") {
4258
return Err(sqlstate_error(
4359
"42601",
4460
"PURGE TENANT requires CONFIRM keyword to prevent accidental data destruction",
4561
));
4662
}
4763

48-
let tenant_id = TenantId::new(tid);
49-
5064
state.audit_record(
5165
AuditEvent::AdminAction,
5266
Some(tenant_id),

‎nodedb/tests/pgwire_auth_tenants.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
//! Pgwire DDL coverage for the tenant surface. Split into submodules
44
//! under `pgwire_auth_tenants/`:
55
//!
6-
//! - `lifecycle` — CREATE / DROP TENANT, `IF [NOT] EXISTS`, `WITH ADMIN`
7-
//! - `introspection` — `SHOW TENANT[S]` by name / id + privilege gate
8-
//! - `grants` — `GRANT / REVOKE <perm> ON TENANT <name>`
6+
//! - `lifecycle` — CREATE / DROP TENANT, `IF [NOT] EXISTS`, `WITH ADMIN`
7+
//! - `introspection` — `SHOW TENANT[S]` by name / id + privilege gate
8+
//! - `grants` — `GRANT / REVOKE <perm> ON TENANT <name>`
9+
//! - `name_resolution` — DROP/ALTER/PURGE TENANT by name (mirroring CREATE/SHOW)
910
//!
1011
//! The entry file stays thin so future additions land in the topical
1112
//! submodule rather than bloating one ~500-line test file.
@@ -18,3 +19,5 @@ mod grants;
1819
mod introspection;
1920
#[path = "pgwire_auth_tenants/lifecycle.rs"]
2021
mod lifecycle;
22+
#[path = "pgwire_auth_tenants/name_resolution.rs"]
23+
mod name_resolution;

0 commit comments

Comments
 (0)