Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify and update permissions #1538

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions controllers/services/auth/resources/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Permissions For Users

When adding permissions for users, it's best to consider the "[reasonable level](#reasonable-level)" of permissions.

## Reasonable Level

> **Note:** This is a recommendation -- a good starting spot -- not a hard-fast rule.

When granting access to a bot (like a service account) you'd aim to grant specific actions the bot would directly do -- need patch? give patch; need read? give read. However, when granting permissions to users, you often want to take a wider approach -- need patch? give update & patch; need read? give get, list, and watch.

Admins should be considered as people with a desire of a clean user experience, and not a means to an end to complete a development flow. From there, we can look at if it is appropriate for us to be granting such wide permissions.

Consider the CRUD actions an admin or allowed user gets... as a user is granted access over resources, there a logical good UX extension to grant them inverse or complementary access. Consider the following CRUD layout as it is related to each k8s verbs:
* C - Create
* R - Get, List, Watch
* U - Update, Patch
* D - Delete

Furthermore, when a user gets “C” (create) actions... we should consider them having the ability to delete them, as to clean up their mistakes. Naturally they’ll need read permissions too so they can interact with them. The mapping looks something like this:
* Need **C** => Gets **CRUD**
* Need **R** => Gets **R**
* Need **U** => Gets **RU**
* Need **D** => Gets **CRUD**

The guidance would be for you to start with the idea that if you need one of the CRUD actions, you get the corresponding CRUD relationship. It’s all additive. If you need just “R” – you only get to read (which boils down to get, list, & watch powers). If you need Create? You’d look to get the full CRUD gambit.

It’s important to note, this is a guidance and recommendation for the user experience of the admin behind the permissions. Not a hard-fast rule. Consider the ramifications of broadly granting access to the resource in question you’re looking at.
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,3 @@ rules:
- get
- list
- watch
- apiGroups:
- nim.opendatahub.io
resources:
- accounts
verbs:
- watch
- update
- get
- list
- create
- patch
- delete
24 changes: 23 additions & 1 deletion controllers/services/auth/resources/admingroup-role.tmpl.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ rules:
- list
- watch
- patch
- update
- apiGroups:
- services.opendatahub.io
resources:
Expand All @@ -27,7 +28,9 @@ rules:
- create
- get
- list
- watch
- patch
- update
- delete
- apiGroups:
- route.openshift.io
Expand All @@ -54,6 +57,7 @@ rules:
- get
- list
- patch
- update
- delete
- watch
- apiGroups:
Expand All @@ -63,6 +67,8 @@ rules:
- buildconfigs
verbs:
- list
- get
- watch
- apiGroups:
- apps
resources:
Expand All @@ -76,6 +82,7 @@ rules:
- odhdashboardconfigs
verbs:
- get
- list
- watch
- create
- update
Expand All @@ -96,14 +103,16 @@ rules:
- odhdocuments
verbs:
- get
- list
- list
- watch
- apiGroups:
- console.openshift.io
resources:
- odhquickstarts
verbs:
- get
- list
- watch
- apiGroups:
- template.openshift.io
resources:
Expand All @@ -114,10 +123,23 @@ rules:
- watch
- create
- patch
- update
- delete
- apiGroups:
- serving.kserve.io
resources:
- servingruntimes
verbs:
- create
- apiGroups:
- nim.opendatahub.io
resources:
- accounts
verbs:
- watch
- update
- get
- list
- create
- patch
- delete
Loading