Skip to content

Commit 7b59d98

Browse files
committed
fix: correct boolean comparisons and add keypair active check in host permission filter
1 parent 7eb8696 commit 7b59d98

File tree

5 files changed

+42
-9
lines changed

5 files changed

+42
-9
lines changed

docs/manager/graphql-reference/supergraph.graphql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5608,7 +5608,7 @@ input HostPermissionFilter
56085608
in: [VFolderHostPermission!] = null
56095609

56105610
"""
5611-
Include only vfolders on hosts where the user LACKS the listed permissions.
5611+
Include only vfolders on hosts where the user does NOT have all of the listed permissions (i.e., lacks at least one of them).
56125612
"""
56135613
notIn: [VFolderHostPermission!] = null
56145614
}

docs/manager/graphql-reference/v2-schema.graphql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3336,7 +3336,7 @@ input HostPermissionFilter {
33363336
in: [VFolderHostPermission!] = null
33373337

33383338
"""
3339-
Include only vfolders on hosts where the user LACKS the listed permissions.
3339+
Include only vfolders on hosts where the user does NOT have all of the listed permissions (i.e., lacks at least one of them).
33403340
"""
33413341
notIn: [VFolderHostPermission!] = null
33423342
}

src/ai/backend/manager/api/gql/vfolder_v2/types/filters.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ class HostPermissionFilterGQL(PydanticInputMixin[HostPermissionFilter]):
8080
default=None,
8181
)
8282
not_in: list[VFolderHostPermissionGQL] | None = gql_field(
83-
description="Include only vfolders on hosts where the user LACKS the listed permissions.",
83+
description=(
84+
"Include only vfolders on hosts where the user does NOT have all of the listed "
85+
"permissions (i.e., lacks at least one of them)."
86+
),
8487
default=None,
8588
)
8689

src/ai/backend/manager/models/vfolder/conditions.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ def _build_source_query(
238238
domain_pairs = _build_source_query(
239239
DomainRow.allowed_vfolder_hosts,
240240
DomainRow.__table__,
241-
(DomainRow.name == requester.domain_name) & (DomainRow.is_active),
241+
(DomainRow.name == requester.domain_name) & (DomainRow.is_active.is_(True)),
242242
)
243243

244244
# 2. User's group hosts
@@ -251,7 +251,7 @@ def _build_source_query(
251251
group_from,
252252
(AssocGroupUserRow.user_id == requester.user_id)
253253
& (GroupRow.domain_name == requester.domain_name)
254-
& (GroupRow.is_active),
254+
& (GroupRow.is_active.is_(True)),
255255
)
256256

257257
# 3. Keypair resource policy hosts
@@ -262,7 +262,7 @@ def _build_source_query(
262262
keypair_pairs = _build_source_query(
263263
KeyPairResourcePolicyRow.allowed_vfolder_hosts,
264264
krp_from,
265-
KeyPairRow.user == requester.user_id,
265+
(KeyPairRow.user == requester.user_id) & (KeyPairRow.is_active.is_(True)),
266266
)
267267

268268
# UNION ALL (host, perm) from all sources, then GROUP BY host

tests/unit/manager/repositories/vfolder/test_vfolder_host_permission_filter.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ class HostPermissionTestData:
7373
# VFolder owned by User C
7474
vf_alpha_c2_id: uuid.UUID # host="host-c"
7575

76+
# VFolder on keypair resource policy host
77+
vf_alpha_krp_id: uuid.UUID # host="host-krp" (from keypair resource policy)
78+
7679
# Shared vfolders (owned by B/C, shared to A via VFolderPermissionRow)
7780
vf_shared_to_a_id: uuid.UUID # owned by B, host="host-shared", shared to A
7881
vf_shared_on_c_id: uuid.UUID # owned by C, host="host-c", shared to A
@@ -175,6 +178,7 @@ async def test_data(
175178
vf_beta_a_id = uuid.uuid4()
176179
vf_alpha_c2_id = uuid.uuid4()
177180
vf_shared_to_a_id = uuid.uuid4()
181+
vf_alpha_krp_id = uuid.uuid4()
178182
vf_shared_on_c_id = uuid.uuid4()
179183

180184
async with db_with_cleanup.begin_session() as db_sess:
@@ -233,6 +237,9 @@ async def test_data(
233237
max_concurrent_sftp_sessions=5,
234238
max_containers_per_session=1,
235239
idle_timeout=3600,
240+
allowed_vfolder_hosts={
241+
"host-krp": ["create-vfolder", "mount-in-session"],
242+
},
236243
)
237244
)
238245
await db_sess.flush()
@@ -363,6 +370,7 @@ def _make_vfolder(
363370
(vf_alpha_b_id, "vf-alpha-b", "host-b"),
364371
(vf_alpha_c_id, "vf-alpha-c", "host-c"),
365372
(vf_alpha_orphan_id, "vf-alpha-orphan", "host-nowhere"),
373+
(vf_alpha_krp_id, "vf-alpha-krp", "host-krp"),
366374
]:
367375
db_sess.add(_make_vfolder(vid, name, host, "alpha", user_a_id))
368376

@@ -413,6 +421,7 @@ def _make_vfolder(
413421
vf_alpha_b_id=vf_alpha_b_id,
414422
vf_alpha_c_id=vf_alpha_c_id,
415423
vf_alpha_orphan_id=vf_alpha_orphan_id,
424+
vf_alpha_krp_id=vf_alpha_krp_id,
416425
vf_beta_shared_id=vf_beta_shared_id,
417426
vf_beta_x_id=vf_beta_x_id,
418427
vf_beta_y_id=vf_beta_y_id,
@@ -438,6 +447,7 @@ def _make_vfolder(
438447
"vf_alpha_shared_id",
439448
"vf_alpha_a_id",
440449
"vf_alpha_b_id",
450+
"vf_alpha_krp_id",
441451
"vf_shared_to_a_id",
442452
},
443453
),
@@ -511,6 +521,7 @@ def _make_vfolder(
511521
"vf_alpha_shared_id",
512522
"vf_alpha_a_id",
513523
"vf_alpha_b_id",
524+
"vf_alpha_krp_id",
514525
"vf_shared_to_a_id",
515526
},
516527
excluded_vf_keys={"vf_shared_on_c_id"},
@@ -531,6 +542,7 @@ def _make_vfolder(
531542
"vf_alpha_shared_id",
532543
"vf_alpha_a_id", # CREATE from domain + MOUNT from group (cross-source)
533544
"vf_alpha_b_id",
545+
"vf_alpha_krp_id", # both CREATE + MOUNT from keypair policy
534546
"vf_shared_to_a_id",
535547
},
536548
),
@@ -550,6 +562,7 @@ def _make_vfolder(
550562
"vf_alpha_shared_id",
551563
"vf_alpha_a_id",
552564
"vf_alpha_b_id",
565+
"vf_alpha_krp_id",
553566
"vf_shared_to_a_id",
554567
},
555568
excluded_vf_keys={
@@ -559,6 +572,23 @@ def _make_vfolder(
559572
),
560573
id="cross-source-union",
561574
),
575+
pytest.param(
576+
HostPermFilterCase(
577+
description="keypair resource policy host included in filter results",
578+
user_id_field="user_a_id",
579+
domain_name="alpha",
580+
permissions=[VFolderHostPermission.CREATE],
581+
negate=False,
582+
expected_vf_keys={
583+
"vf_alpha_shared_id",
584+
"vf_alpha_a_id",
585+
"vf_alpha_b_id",
586+
"vf_alpha_krp_id", # from keypair resource policy
587+
"vf_shared_to_a_id",
588+
},
589+
),
590+
id="keypair-source",
591+
),
562592
],
563593
)
564594
async def test_host_permission_filter(
@@ -612,8 +642,8 @@ async def test_no_filter_returns_all_user_vfolders(
612642

613643
result = await vfolder_repository.search_user_vfolders(querier, scope)
614644

615-
# User A owns 5 vfolders + 2 shared = 7
616-
assert result.total_count == 7
645+
# User A owns 6 vfolders + 2 shared = 8
646+
assert result.total_count == 8
617647

618648
# ── H-12: pagination compatibility ──
619649

@@ -645,7 +675,7 @@ async def test_pagination_with_host_permission_filter(
645675

646676
result = await vfolder_repository.search_user_vfolders(querier, scope)
647677

648-
assert result.total_count == 4
678+
assert result.total_count == 5
649679
assert len(result.items) == 2
650680
assert result.has_next_page is True
651681

0 commit comments

Comments
 (0)