feat: Expose per-host permissions from the vfolder host list#12333
Open
rapsealk wants to merge 5 commits into
Open
feat: Expose per-host permissions from the vfolder host list#12333rapsealk wants to merge 5 commits into
rapsealk wants to merge 5 commits into
Conversation
The `GET /folders/_/hosts` response only returned `allowed` — the host names a user may use — discarding the per-host permission sets that `list_hosts` already computes. Clients therefore could not tell apart hosts the user may create vfolders on from read-only hosts, and would fail late when a create was attempted against a non-writable host. Carry the existing `VFolderHostPermissionMap` through `ListHostsActionResult` into `ListHostsResponse` as `allowed_permissions` (host -> permission names, e.g. `create-vfolder`). The field defaults to an empty mapping so existing clients keep parsing unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
comp:manager
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the vfolder REST host-list endpoint by exposing per-host permission sets alongside the existing allowed host list, enabling clients to distinguish create-capable hosts from read-only hosts before attempting vfolder creation.
Changes:
- Adds an
allowed_permissionsmapping (host → permission values) to thelist_hostsservice result and REST response DTO. - Populates the new mapping in
VFolderService.list_hosts()using the existingVFolderHostPermissionMapcomputed during host filtering. - Adds unit tests covering service behavior and DTO backward-compatibility (missing field defaults to
{}).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/services/vfolder/test_vfolder_storage_service.py | Adds a service-level test ensuring create-capable and read-only hosts are distinguished via allowed_permissions. |
| tests/unit/common/dto/manager/vfolder/test_response.py | Adds DTO tests for the new field and for default-empty backward compatibility. |
| src/ai/backend/manager/services/vfolder/services/vfolder.py | Includes allowed_permissions in the service action result returned by list_hosts(). |
| src/ai/backend/manager/services/vfolder/actions/storage_ops.py | Extends ListHostsActionResult with the new allowed_permissions field. |
| src/ai/backend/manager/api/rest/vfolder/handler.py | Passes allowed_permissions through into the REST response model. |
| src/ai/backend/common/dto/manager/vfolder/response.py | Adds allowed_permissions to ListHostsResponse with a {} default for compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| default=default_host, | ||
| allowed=sorted(allowed_hosts), | ||
| volume_info=volume_info, | ||
| allowed_permissions=allowed_hosts.to_json(), |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in c34ff84. allowed_permissions is now built with sorted hosts and sorted permission names instead of to_json()'s set ordering, so the REST response is deterministic. Added a test asserting each permission list is sorted.
`VFolderHostPermissionMap.to_json()` iterates over a set, so the per-host permission lists were emitted in nondeterministic order — unstable JSON once exposed in the REST response. Build `allowed_permissions` with sorted hosts and sorted permission names instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fregataa
reviewed
Jun 22, 2026
| "by host name (e.g. `create-vfolder`, `mount-in-session`). Lets " | ||
| "clients distinguish hosts the user may create folders on from " | ||
| "read-only hosts, since `allowed` carries only host names. " | ||
| "Added in 26.4.4." |
Member
Author
There was a problem hiding this comment.
Thanks — corrected to 26.4.5 (since 26.4.4 is already in RC). Updated in the latest push.
26.4.4 is already cutting RCs; a feature landing on main now ships in 26.4.5. Per reviewer feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /folders/_/hostsreturned onlyallowed— the list of host names a user may use — while discarding the per-host permission sets thatlist_hostsalready computes internally as aVFolderHostPermissionMap. As a result, a client cannot tell apart hosts where the user may create a vfolder (create-vfolder) from hosts where they only hold read/mount permissions (mount-in-session,download-file, …). The host list therefore advertises hosts that fail later when a create is attempted against them.This adds an
allowed_permissionsmapping (host → permission names) to the response, sourced from the permission map the service already builds — so clients can mark or hide non-writable hosts up front.Changes
ListHostsActionResult(services/vfolder/actions/storage_ops.py): addallowed_permissions: dict[str, list[str]].VFolderService.list_hosts(services/vfolder/services/vfolder.py): populate it fromallowed_hosts.to_json()(the same filteredVFolderHostPermissionMapused forallowed).ListHostsResponse(common/dto/manager/vfolder/response.py): addallowed_permissions, defaulting to{}so existing clients parse unchanged.VFolderHandler.list_hosts(api/rest/vfolder/handler.py): pass the field through.{}).Compatibility
Additive and backward-compatible:
allowedis unchanged andallowed_permissionsis optional with an empty-mapping default. Its keys mirrorallowedexactly. The per-host permission data was already exposed via the V2 GraphQL storage-host-permissions query; this closes the gap for the REST host-list endpoint.