Package repository management UI for users with restricted permissions #5480
Conversation
Signed-off-by: Rafa Castelblanque <[email protected]>
Signed-off-by: Rafa Castelblanque <[email protected]>
Signed-off-by: Rafa Castelblanque <[email protected]>
Signed-off-by: Rafa Castelblanque <[email protected]>
✅ Deploy Preview for kubeapps-dev canceled.
|
Signed-off-by: Rafa Castelblanque <[email protected]>
Signed-off-by: Rafa Castelblanque <[email protected]>
Signed-off-by: Rafa Castelblanque <[email protected]>
|
Thanks for this enhancement to have better feedback from the UI according to your permission. |
| ), | ||
| ) | ||
| .filter(i => i !== undefined), | ||
| ).then(results => { |
There was a problem hiding this comment.
what about using the await keyword instead like in other places?
also, we should have a .catch to handle any promise error, shouldn't we?
There was a problem hiding this comment.
The reason to use Promise.all is to trigger an instance of .getRepositoriesPermissions per plugin, all three started at the same time.
What is being called underneath is a set of canI calls. There is a catch per canI invocation in getResourcePermissions, so we are handling the potential errors there.
| setReposRBAC(rbac); | ||
| }); | ||
|
|
||
| // Cluster-wide check | ||
| Kube.canI(cluster, "kubeapps.com", "apprepositories", "list", "") |
There was a problem hiding this comment.
unrelated to this PR, but just curious... what this canSetAllNS is for? It seems it's coupled with the helm plugin, no?
There was a problem hiding this comment.
It was a previously existing flag. It controls if the toggle to show repos from all namespaces is hidden.
Making this feature cross-plugin could be a task for the new UI #5515.
| config: IConfig, | ||
| plugin: Plugin, | ||
| ): IPackageRepositoryPermission | undefined => { | ||
| switch (plugin.name) { |
There was a problem hiding this comment.
I think the answer is no... but: can we obtain these strings from the plugin backend somehow? Like, what if packaging.carvel.dev get eventually renamed?
There was a problem hiding this comment.
There is a comment exactly about that a little bit below.
We could even add a new API endpoint to provide the permissions of the user for e.g. repos.
In this way the UI would not need to know anything about resource group, name, verb, etc.
Description of the change
This PR improves the repositories management screen for the cases of users with restricted permissions.
Before, there was a single
disableControlsflag for all repositories, regardless of the plugin.Now this PR introduces a function
PackageRepositoriesService.getRepositoriesPermissionsthat checks the permissions for repos CRDs per plugin. It does so by invoking multiplecan-irequests, which could be improved by adding a new, on purpose, backend API.This permissions check allows to enable/disable edit controls at the repo level and not globally as before.
For example, a user that:
will see a screen like this:
Also, after #5453 and #5460, users with no cluster-wide permissions but with permissions in a specific ns, can see the repositories list without an alert error.
Benefits
Possible drawbacks
N/A
Applicable issues