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

WP-371: Use set-facl job for workspace ACLs #967

Merged
merged 2 commits into from
Sep 16, 2024
Merged

Conversation

chandra-tacc
Copy link
Collaborator

@chandra-tacc chandra-tacc commented Sep 15, 2024

Overview

Uses setfacl job (see PR) to set ACLs on project folder during add/remove/role change operations

Related

WP-371
SetFacl app PR: TACC/WMA-Tapis-Templates#55

Changes

  1. Added setfacl job in apps for wma_prtl.
  2. Add settings to toggle between setfacl job and using tapis
  3. pass path, operation(add/remove), role(reader/writer/none)
  4. Add new workspace

Differences from DesignSafe

Passing role to the setfacl app. Based on the role, the permission bits are adjusted.

Testing

For quicker testing, the feature is deployed to https://dev.cep.tacc.utexas.edu/

  1. Add user to a project
  2. Adjust user role to read only (guest)
  3. Remove user
  4. Added unit tests

Verification

Add:

core portal logs

core_portal_django         | [DJANGO] INFO 2024-09-15 16:31:36,190 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_acls:63: Using setfacl job to submit ACL change for project: CEP-111, username: gedmonds, operation: add, role: writer
core_portal_django         | [DJANGO] INFO 2024-09-15 16:31:39,166 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_acls:65: Submitted workspace ACL job setfacl-project-CEP-111-gedmonds-add-writer with UUID 3197680e-92e0-4fc1-93c7-f4250153bc03-007
core_portal_django         | [DJANGO] INFO 2024-09-15 16:31:39,273 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_permissions:27: Adding gedmonds permissions to Tapis system cep.project.CEP-111

tapis job output

+ chmod +x ./getUID.sh
+ chmod +x ./JSON.sh
+ IFS=,
+ read -ra USERNAMES
+ ACL_COMMANDS=()
+ for delimitedUser in ${USERNAMES[@]}
++ ./getUID.sh gedmonds
+ USER_ID=835219
+ echo 'user id: 835219'
user id: 835219
+ case ${role} in
+ ACL_COMMANDS+=("d:u:${USER_ID}:rwX,u:${USER_ID}:rwX")
++ IFS=,
++ echo d:u:835219:rwX,u:835219:rwX
+ ACL_STRING=d:u:835219:rwX,u:835219:rwX
+ '[' add == add ']'
+ echo 'Running: setfacl -R -m d:u:835219:rwX,u:835219:rwX /corral-repl/tacc/aci/CEP/projects/CEP-111'
Running: setfacl -R -m d:u:835219:rwX,u:835219:rwX /corral-repl/tacc/aci/CEP/projects/CEP-111
+ setfacl -R -m d:u:835219:rwX,u:835219:rwX /corral-repl/tacc/aci/CEP/projects/CEP-111
+ '[' add == remove ']'
+ '[' add == dryrun ']'

Update role

core portal logs

core_portal_django         | [DJANGO] INFO 2024-09-15 16:32:38,684 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_acls:63: Using setfacl job to submit ACL change for project: CEP-111, username: gedmonds, operation: add, role: reader
core_portal_django         | [DJANGO] INFO 2024-09-15 16:32:41,704 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_acls:65: Submitted workspace ACL job setfacl-project-CEP-111-gedmonds-add-reader with UUID 7f77e037-69bd-4a02-b854-99fee2a8cf38-007
core_portal_django         | [DJANGO] INFO 2024-09-15 16:32:41,705 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_permissions:27: Adding gedmonds permissions to Tapis system cep.project.CEP-111

tapis job output

+ chmod +x ./getUID.sh
+ chmod +x ./JSON.sh
+ IFS=,
+ read -ra USERNAMES
+ ACL_COMMANDS=()
+ for delimitedUser in ${USERNAMES[@]}
++ ./getUID.sh gedmonds
+ USER_ID=835219
+ echo 'user id: 835219'
user id: 835219
+ case ${role} in
+ ACL_COMMANDS+=("d:u:${USER_ID}:rX,u:${USER_ID}:rX")
++ IFS=,
++ echo d:u:835219:rX,u:835219:rX
+ ACL_STRING=d:u:835219:rX,u:835219:rX
+ '[' add == add ']'
+ echo 'Running: setfacl -R -m d:u:835219:rX,u:835219:rX /corral-repl/tacc/aci/CEP/projects/CEP-111'
Running: setfacl -R -m d:u:835219:rX,u:835219:rX /corral-repl/tacc/aci/CEP/projects/CEP-111
+ setfacl -R -m d:u:835219:rX,u:835219:rX /corral-repl/tacc/aci/CEP/projects/CEP-111
+ '[' add == remove ']'
+ '[' add == dryrun ']'

Remove

core portal logs

core_portal_django         | [DJANGO] INFO 2024-09-15 17:49:53,935 UTC shared_workspace_operations portal.apps.projects.workspace_operations.shared_workspace_operations.set_workspace_acls:65: Submitted workspace ACL job setfacl-project-CEP-111-gedmonds-remove-none with UUID 0ad9209d-bfa5-468e-bace-09b9084a63e6-007
core_portal_django         | [DJANGO] INFO 2024-09-15 17:49:54,673 UTC basehttp django.server.log_message:212: "PATCH /api/projects/CEP-111/members/ HTTP/1.0" 200 295

tapis job output

+ chmod +x ./getUID.sh
+ chmod +x ./JSON.sh
+ IFS=,
+ read -ra USERNAMES
+ ACL_COMMANDS=()
+ for delimitedUser in ${USERNAMES[@]}
++ ./getUID.sh gedmonds
+ USER_ID=835219
+ echo 'user id: 835219'
user id: 835219
+ case ${role} in
+ ACL_COMMANDS+=("d:u:${USER_ID},u:${USER_ID}")
++ IFS=,
++ echo d:u:835219,u:835219
+ ACL_STRING=d:u:835219,u:835219
+ '[' remove == add ']'
+ '[' remove == remove ']'
+ echo 'Running: setfacl -R -x d:u:835219,u:835219 /corral-repl/tacc/aci/CEP/projects/CEP-111'
Running: setfacl -R -x d:u:835219,u:835219 /corral-repl/tacc/aci/CEP/projects/CEP-111
+ setfacl -R -x d:u:835219,u:835219 /corral-repl/tacc/aci/CEP/projects/CEP-111
+ '[' remove == dryrun ']'

UI

Notes

Copy link

codecov bot commented Sep 15, 2024

Codecov Report

Attention: Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 65.29%. Comparing base (4c21040) to head (6d02baf).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/portal/settings/settings.py 0.00% 1 Missing ⚠️
server/portal/settings/settings_default.py 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
+ Coverage   65.26%   65.29%   +0.02%     
==========================================
  Files         438      438              
  Lines       12688    12703      +15     
  Branches     2669     2638      -31     
==========================================
+ Hits         8281     8294      +13     
- Misses       4171     4173       +2     
  Partials      236      236              
Flag Coverage Δ
javascript 70.34% <ø> (ø)
unittests 60.12% <86.66%> (+0.06%) ⬆️

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

Files with missing lines Coverage Δ
...orkspace_operations/shared_workspace_operations.py 80.76% <100.00%> (+1.95%) ⬆️
server/portal/settings/unit_test_settings.py 99.08% <100.00%> (+<0.01%) ⬆️
server/portal/settings/settings.py 0.00% <0.00%> (ø)
server/portal/settings/settings_default.py 0.00% <0.00%> (ø)

@@ -194,6 +194,7 @@
_PORTAL_PROJECTS_ROOT_HOST = 'cloud.data.tacc.utexas.edu'
_PORTAL_PROJECTS_SYSTEM_PORT = "22"
_PORTAL_PROJECTS_PEMS_APP_ID = "" # Defunct in v3
_PORTAL_PROJECTS_USE_SET_FACL_JOB = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we enable this by default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll make sure to create credentials for cloud.data (wma_prtl) on all tenants

Copy link
Member

@rstijerina rstijerina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works great!

Copy link
Contributor

@fnets fnets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and works locally

@chandra-tacc chandra-tacc merged commit 2da9991 into main Sep 16, 2024
6 checks passed
@chandra-tacc chandra-tacc deleted the tasks/WP-371-set-facl branch September 16, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants