feat: allow users with role Viewer and above to view resource quotas #9822

salonig23 · 2024-08-14T21:48:17Z

Ticket

Description

Allowed users with Viewer, Editor, Editor Restricted, Editor Project Restricted, Gen AI and Workspace Admin to be able to view resource quotas in the WebUI when RBAC is enabled. Cluster Admin can still modify and view resource quotas. Allowed all users to view quotas when RBAC is disabled. This involved allowing users who cannot modify the workspaces to also see the Edit Workspace Modal but they will be seeing a "Config" option instead of "Edit Workspace".

Test Plan

Set up minikube and create a namespace "test-ns" with a resource quota of 2.

Login as Cluster Admin and create a workspace "w1" with no namespace binding, "w2" with auto-created namespace binding and some resource quota. and "w3" with "test-ns" as the namespace.
Edit "w1" and change the binding to an auto-created namespace with some resource quota. Make sure it saves correctly.
Edit "w2" and change the binding to "test-ns". Make sure that the updated resource quota shows up as 2.
Edit "w3" and change the binding to an auto-created namespace with a resource quota of 2, and make sure that it saves correctly.
login with Workspace Admin, and try to make changes to some of the workspaces and make sure that resource quota is not editable and there are no errors while saving the workspaces.
log in as workspace creator and make sure that viewing resource quota is non visible.
log in with the rest of the roles and make sure that resource quotas show up correctly and noneditable for:

Workspace with no binding
workspace with binding but no quota
workspace with autocreated binding and quota
workspace with other namespace binding and quota

Checklist

Changes have been manually QA'd
New features have been approved by the corresponding PM
User-facing API changes have the "User-facing API Change" label
Release notes have been added as a separate file under docs/release-notes/
See Release Note for details.
Licenses have been included for new code which was copied and/or modified from any external code

codecov · 2024-08-14T21:48:25Z

Codecov Report

Attention: Patch coverage is 13.48315% with 77 lines in your changes missing coverage. Please review.

Project coverage is 54.35%. Comparing base (91d0b67) to head (1981395).
Report is 11 commits behind head on main.

Files	Patch %	Lines
...ebui/react/src/components/WorkspaceCreateModal.tsx	0.00%	56 Missing ⚠️
master/internal/workspace/authz_rbac.go	0.00%	9 Missing ⚠️
master/internal/api_workspace.go	0.00%	5 Missing ⚠️
master/internal/workspace/authz_permissive.go	0.00%	3 Missing ⚠️
master/internal/workspace/authz_basic_impl.go	0.00%	2 Missing ⚠️
...rc/pages/WorkspaceList/WorkspaceActionDropdown.tsx	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9822      +/-   ##
==========================================
- Coverage   54.38%   54.35%   -0.03%     
==========================================
  Files        1261     1261              
  Lines      155770   155842      +72     
  Branches     3540     3542       +2     
==========================================
- Hits        84711    84710       -1     
- Misses      70921    70994      +73     
  Partials      138      138

Flag	Coverage Δ
backend	`45.20% <0.00%> (-0.04%)`	⬇️
harness	`72.61% <ø> (ø)`
web	`53.67% <17.14%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
webui/react/src/hooks/usePermissions.ts	`77.17% <100.00%> (+0.35%)`	⬆️
master/internal/workspace/authz_basic_impl.go	`1.61% <0.00%> (-0.06%)`	⬇️
...rc/pages/WorkspaceList/WorkspaceActionDropdown.tsx	`0.00% <0.00%> (ø)`
master/internal/workspace/authz_permissive.go	`1.56% <0.00%> (-0.08%)`	⬇️
master/internal/api_workspace.go	`63.49% <0.00%> (-0.14%)`	⬇️
master/internal/workspace/authz_rbac.go	`0.35% <0.00%> (-0.02%)`	⬇️
...ebui/react/src/components/WorkspaceCreateModal.tsx	`0.00% <0.00%> (ø)`

... and 6 files with indirect coverage changes

netlify · 2024-08-14T21:48:34Z

✅ Deploy Preview for determined-ui ready!

Name	Link
🔨 Latest commit	`1981395`
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/66be7f32c69af60008bfd4c1
😎 Deploy Preview	https://deploy-preview-9822--determined-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

johnkim-det

looks like the e2e testing issue is related to a bug in this PR when editing an existing workspace -- the toggles don't seem to work.

webui/react/src/components/WorkspaceCreateModal.tsx

master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql

amandavialva01 · 2024-08-15T19:31:19Z

master/internal/api_workspace.go

-	err = workspace.AuthZProvider.Get().CanGetWorkspaceID(
-		ctx, *curUser, req.Id,
+	err = workspace.AuthZProvider.Get().CanViewResourceQuotas(
+		ctx, *curUser,


Instead of removing the CanGetWorkspaceID check, should we maybe do
a.getWorkspaceAndCheckCanDoActions(ctx, req.Id, false, workspace.AuthZProvider.Get().CanGetWorkspace, workspace.AuthZProvider.Get().CanViewResourceQuotas) so that we perform both of these checks? Or do we not need the CanGetWorkspaceID check here?

I don't think we can check 2 permissions at a time, so I added both checks in a different way

amandavialva01

This LGTM modulo one comment!
Can we maybe add some unit tests in postgres_rbac_intg_test.go just to make sure the permissions are properly added to these respective roles?

…rq API req

webui/react/src/components/WorkspaceCreateModal.tsx

johnkim-det

web LGTM

webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx

master/internal/db/postgres_rbac_intg_test.go

amandavialva01

LGTM, awesome work!

feat: allow users with role Viewer and above to view resource quotas

5821b4d

cla-bot bot added the cla-signed label Aug 14, 2024

salonig23 requested review from keita-determined and johnkim-det August 14, 2024 21:48

salonig23 requested review from amandavialva01 and ShreyaLnuHpe August 14, 2024 21:49

allow non admin user to view quotas when RBAC is not enabled

627e42b

salonig23 marked this pull request as ready for review August 15, 2024 16:32

salonig23 requested review from a team as code owners August 15, 2024 16:32

salonig23 removed the request for review from ShreyaLnuHpe August 15, 2024 17:18

johnkim-det reviewed Aug 15, 2024

View reviewed changes

ashtonG reviewed Aug 15, 2024

View reviewed changes

webui/react/src/components/WorkspaceCreateModal.tsx Outdated Show resolved Hide resolved

amandavialva01 reviewed Aug 15, 2024

View reviewed changes

master/static/migrations/20240814123944_add-view-resource-quotas-permissions.tx.up.sql Show resolved Hide resolved

amandavialva01 reviewed Aug 15, 2024

View reviewed changes

revert some earlier changes and use permissions to avoid sending set …

4b0d1cd

…rq API req

salonig23 force-pushed the allow-viewers-resource-quota branch from a888f16 to 4b0d1cd Compare August 15, 2024 20:07

ashtonG reviewed Aug 15, 2024

View reviewed changes

webui/react/src/components/WorkspaceCreateModal.tsx Outdated Show resolved Hide resolved

johnkim-det approved these changes Aug 15, 2024

View reviewed changes

webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx Outdated Show resolved Hide resolved

webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx Outdated Show resolved Hide resolved

added postgres test

e81eab4

salonig23 requested a review from amandavialva01 August 15, 2024 21:03

amandavialva01 reviewed Aug 15, 2024

View reviewed changes

master/internal/db/postgres_rbac_intg_test.go Outdated Show resolved Hide resolved

amandavialva01 reviewed Aug 15, 2024

View reviewed changes

master/internal/db/postgres_rbac_intg_test.go Outdated Show resolved Hide resolved

addressing comments

c4de4a9

salonig23 requested a review from amandavialva01 August 15, 2024 21:15

addressing more comments

8690ffb

amandavialva01 approved these changes Aug 15, 2024

View reviewed changes

salonig23 added 2 commits August 15, 2024 14:34

add back disabled={true}

1105cdf

addressed nits

1981395

salonig23 merged commit f7846cb into main Aug 15, 2024
82 of 96 checks passed

salonig23 deleted the allow-viewers-resource-quota branch August 15, 2024 23:05

salonig23 mentioned this pull request Aug 19, 2024

chore: add release notes for PR 9822 #9837

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow users with role Viewer and above to view resource quotas #9822

feat: allow users with role Viewer and above to view resource quotas #9822

salonig23 commented Aug 14, 2024 •

edited

Loading

codecov bot commented Aug 14, 2024 •

edited

Loading

netlify bot commented Aug 14, 2024 •

edited

Loading

johnkim-det left a comment

amandavialva01 Aug 15, 2024

salonig23 Aug 15, 2024

amandavialva01 left a comment

johnkim-det left a comment

amandavialva01 left a comment

feat: allow users with role Viewer and above to view resource quotas #9822

feat: allow users with role Viewer and above to view resource quotas #9822

Conversation

salonig23 commented Aug 14, 2024 • edited Loading

Ticket

Description

Test Plan

Checklist

codecov bot commented Aug 14, 2024 • edited Loading

Codecov Report

netlify bot commented Aug 14, 2024 • edited Loading

✅ Deploy Preview for determined-ui ready!

johnkim-det left a comment

Choose a reason for hiding this comment

amandavialva01 Aug 15, 2024

Choose a reason for hiding this comment

salonig23 Aug 15, 2024

Choose a reason for hiding this comment

amandavialva01 left a comment

Choose a reason for hiding this comment

johnkim-det left a comment

Choose a reason for hiding this comment

amandavialva01 left a comment

Choose a reason for hiding this comment

salonig23 commented Aug 14, 2024 •

edited

Loading

codecov bot commented Aug 14, 2024 •

edited

Loading

netlify bot commented Aug 14, 2024 •

edited

Loading