Skip to content

Commit 16f1b2c

Browse files
authored
client-api: Deny changing the parent of an existing database (clockworklabs#3837)
Mainly a smoketest to exercise the intended behaviour. Also return an error if we end up delegating to the reset database endpoint, which itself doesn't accept a `parent` parameter.
1 parent 7e367b3 commit 16f1b2c

2 files changed

Lines changed: 43 additions & 5 deletions

File tree

crates/client-api/src/routes/database.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -711,14 +711,21 @@ pub async fn publish<S: NodeDelegate + ControlStateDelegate + Authorization>(
711711
let name_or_identity = name_or_identity
712712
.as_ref()
713713
.ok_or_else(|| bad_request("Clear database requires database name or identity".into()))?;
714-
if let Ok(identity) = name_or_identity.try_resolve(&ctx).await.map_err(log_and_500)? {
715-
if ctx
714+
let database_identity = name_or_identity.try_resolve(&ctx).await.map_err(log_and_500)?;
715+
if let Ok(identity) = database_identity {
716+
let exists = ctx
716717
.get_database_by_identity(&identity)
717718
.await
718719
.map_err(log_and_500)?
719-
.is_some()
720-
{
721-
return reset(
720+
.is_some();
721+
if exists {
722+
if parent.is_some() {
723+
return Err(bad_request(
724+
"Setting the parent of an existing database is not supported".into(),
725+
));
726+
}
727+
728+
return self::reset(
722729
State(ctx),
723730
Path(ResetDatabaseParams {
724731
name_or_identity: name_or_identity.clone(),

smoketests/tests/teams.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,37 @@ def query_controldb(self, parent, child):
3636
)
3737
return parse_sql_result(str(res))
3838

39+
40+
class ChangeDatabaseHierarchy(Smoketest):
41+
AUTOPUBLISH = False
42+
43+
def test_change_database_hierarchy(self):
44+
"""
45+
Test that changing the hierarchy of an existing database is not
46+
supported.
47+
"""
48+
49+
parent_name = f"parent-{random_string()}"
50+
sibling_name = f"sibling-{random_string()}"
51+
child_name = f"child-{random_string()}"
52+
53+
self.publish_module(parent_name)
54+
self.publish_module(sibling_name)
55+
56+
# Publish as a child of 'parent_name'.
57+
self.publish_module(f"{parent_name}/{child_name}")
58+
59+
# Publishing again with a different parent is rejected...
60+
with self.assertRaises(Exception):
61+
self.publish_module(f"{sibling_name}/{child_name}", clear = False)
62+
# ..even if `clear = True`
63+
with self.assertRaises(Exception):
64+
self.publish_module(f"{sibling_name}/{child_name}", clear = True)
65+
66+
# Publishing again with the same parent is ok.
67+
self.publish_module(f"{parent_name}/{child_name}", clear = False)
68+
69+
3970
class PermissionsTest(Smoketest):
4071
AUTOPUBLISH = False
4172

0 commit comments

Comments
 (0)