backend.ai
backend.ai copied to clipboard
feat: implement atomic vfolder host permission
ref: https://github.com/lablup/backend.ai/issues/753#issuecomment-1260603825
- Migrate
domains.allowed_vfolder_hosts
,groups.allowed_vfolder_hosts
,keypair_resource_policies.allowed_vfolder_hosts
from ARRAY of string to JSONB
etc)
# Before
domains.allowed_vfolder_hosts = ["storage1"]
domains.allowed_vfolder_hosts = ["storage2"]
# After
domains.allowed_vfolder_hosts = {"storage1": ["mount", "create-vfolder"]}
domains.allowed_vfolder_hosts = {"storage2": ["*"]}
- Impl
PermissionListColumn
which can be used by other permission spec columns.
Please set PR status to "Ready for review" when you've done the final touches.
@yomybaby @lizable @Sujin-Kim1
There should be update in frontend since allowed_vfolder_hosts
field of Domain
, Group
,KeyPairResourcePolicy
graphene and db schema will change in this PR.
There is an alternative, just letting allowed_vfolder_hosts
as a legacy field and adding allowed_vfolder_host_map
JSON field.
Please give me your opinion.
IMO, there are a few requests written in the comments and also one more thing to be fixed before merging this PR. TL;DR: We need to change the code in here(
src/ai/backend/manager/model/gql.py#1428
)https://github.com/lablup/backend.ai/blob/40c04807d9b03040218b39ead81262ff483e79f2/src/ai/backend/manager/models/gql.py#L1415
to this
mutation_cls = getattr(Mutations, info.field_name).type
Considering the overall keyallowed_vfolder_hosts
are located in several policies (such asdomains
,groups(project groups
, andkeypair_resource_policies
), I think It's quite crucial to show the overview of access in those policies easily. In light of this, It's not surprising that users may want features for updating those access controls at once. Therefore, I decided to chain mutations so that It reduces(not completely, but some) repetitive requests to manager server. To make chained mutations, it needs aliasing. Unfortunately, In current code, It regards aliasing string as one of the variables in Mutations class, which would definitely throw an error. I checked many graphql requests and It doesn't affect any mutations at all. It would be appreciated you to apply the new code described above.
It's better to handle this in as a separate issue(#879), which is already opened by @yomybaby .
@fregataa Could you merge the diff from main branch?