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

Add Users and Groups permissions filters to DaskWorker profiles #2391

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

viniciusdc
Copy link
Contributor

@viniciusdc viniciusdc commented Apr 11, 2024

Reference Issues or PRs

#979

What does this implement/fix?

This PR includes the ability to restrict users from selecting dask worker profiles based on two factors:

  • Explicitly define which user or group will have access to the profile under nebari-config.yaml file:
profiles:
  ...
  dask_worker:
    "Small Worker":
     # New fields (Optional)
      access: yaml # Explicitly defines permission filtering from the yaml
      users: <test-user> # A list of users who will have access to this profile
      groups: <my-group> # A list of groups who will have access to this profile
      ....
      worker_cores_limit: 1
      worker_cores: 1
      worker_memory_limit: 1G
      worker_memory: 1G
...
  • Or, by including the dask_profiles attribute for the user or group on Keycloal
profiles:
  ...
  dask_worker:
    "Small Worker":
     # New fields (Optional)
      access: keycloak # Explicitly defines permission filtering from keycloak
      ....

bellow an example of such an attribute for a given user:

Captura de Tela 2024-04-11 às 11 52 53

This follows the same model that is already in place for jupyterhub profiles.

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Any other comments?

@viniciusdc
Copy link
Contributor Author

viniciusdc commented Apr 11, 2024

A few remarks:

  1. Deployment Testing Pending: The implementation must still be tested in a live deployment. (I will test on local soon)
  2. Inconsistency in Profile Object Standards: There must be more consistency in how profile objects are handled across different environments. For instance, Dask treats profiles as a dict entity, Terraform expects them as a list, and Jupyterlab also uses a list but with unconventional handling. This could be an excellent first issue.
  3. Mapper Functionality Check: I confirmed through Postman that the mapper is operational. However, there is an observable delay in updating the scopes.
  4. Need for Unit Testing in Configuration Files: Developing unit tests, or mock-ups, for gateway-config.py and jupyterhub python files is essential. These tests should ensure that the configuration objects and methods function as expected, at least from a Python perspective.
  5. Upcoming Refactoring for Keycloak Attribute Mapper: The Keycloak attribute mapper will likely undergo refactoring in light of the anticipated new permissions system.

@marcelovilla
Copy link
Member

@viniciusdc what's the status of this PR?

@viniciusdc
Copy link
Contributor Author

Hi, @marcelovilla; thanks for the ping. This was ready for review back then, but there are some changes that I need to make for this work again. As you said, the first step is fixing the conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New 🚦
Development

Successfully merging this pull request may close these issues.

2 participants