Skip to content

Commit 6e79986

Browse files
committed
change silo_group index to "time_deleted IS NULL", add test for deleted groups
1 parent 9ee416f commit 6e79986

File tree

3 files changed

+72
-2
lines changed

3 files changed

+72
-2
lines changed

common/src/sql/dbinit.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ CREATE TABLE omicron.public.silo_group (
291291
CREATE INDEX ON omicron.public.silo_group (
292292
silo_id
293293
) WHERE
294-
time_deleted IS NOT NULL;
294+
time_deleted IS NULL;
295295

296296
CREATE UNIQUE INDEX ON omicron.public.silo_group (
297297
silo_id,

nexus/src/db/datastore/silo.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ impl DataStore {
276276
silo_group_membership::dsl::silo_group_id.eq_any(
277277
silo_group::dsl::silo_group
278278
.filter(silo_group::dsl::silo_id.eq(id))
279-
.filter(silo_group::dsl::time_deleted.is_not_null())
279+
.filter(silo_group::dsl::time_deleted.is_null())
280280
.select(silo_group::dsl::id),
281281
),
282282
)

nexus/tests/integration_tests/silos.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,3 +1293,73 @@ async fn test_silo_groups_remove_from_both_groups(
12931293

12941294
assert!(group_names.contains(&"c-group".to_string()));
12951295
}
1296+
1297+
// Test that silo delete cleans up associated groups
1298+
#[nexus_test]
1299+
async fn test_silo_delete_clean_up_groups(cptestctx: &ControlPlaneTestContext) {
1300+
let client = &cptestctx.external_client;
1301+
let nexus = &cptestctx.server.apictx.nexus;
1302+
1303+
// Create a silo
1304+
let silo =
1305+
create_silo(&client, "test-silo", true, shared::UserProvisionType::Jit)
1306+
.await;
1307+
1308+
let opctx = OpContext::for_tests(
1309+
cptestctx.logctx.log.new(o!()),
1310+
nexus.datastore().clone(),
1311+
);
1312+
1313+
// Create an admin user
1314+
let (authz_silo, db_silo) = LookupPath::new(&opctx, &nexus.datastore())
1315+
.silo_name(&silo.identity.name.into())
1316+
.fetch()
1317+
.await
1318+
.unwrap();
1319+
1320+
// Add a user with a group membership
1321+
let silo_user = nexus
1322+
.silo_user_from_authenticated_subject(
1323+
&nexus.opctx_external_authn(),
1324+
&authz_silo,
1325+
&db_silo,
1326+
&AuthenticatedSubject {
1327+
external_id: "user@company.com".into(),
1328+
groups: vec!["sre".into()],
1329+
},
1330+
)
1331+
.await
1332+
.expect("silo_user_from_authenticated_subject")
1333+
.unwrap();
1334+
1335+
// Delete the silo
1336+
NexusRequest::object_delete(&client, &"/silos/test-silo")
1337+
.authn_as(AuthnMode::PrivilegedUser)
1338+
.execute()
1339+
.await
1340+
.expect("failed to make request");
1341+
1342+
// Expect the group is gone
1343+
assert!(nexus
1344+
.datastore()
1345+
.silo_group_optional_lookup(&opctx, &authz_silo, "a-group".into(),)
1346+
.await
1347+
.expect("silo_group_optional_lookup")
1348+
.is_none());
1349+
1350+
// Expect the group membership is gone
1351+
let memberships = nexus
1352+
.datastore()
1353+
.silo_group_membership_for_user(&opctx, &authz_silo, silo_user.id())
1354+
.await
1355+
.expect("silo_group_membership_for_user");
1356+
1357+
assert!(memberships.is_empty());
1358+
1359+
// Expect the user is gone
1360+
LookupPath::new(&opctx, &nexus.datastore())
1361+
.silo_user_id(silo_user.id())
1362+
.fetch()
1363+
.await
1364+
.expect_err("user found");
1365+
}

0 commit comments

Comments
 (0)