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

Fix issue with namespaces permission scopes #2946

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

viniciusdc
Copy link
Contributor

@viniciusdc viniciusdc commented Feb 7, 2025

Reference Issues or PRs

What does this implement/fix?

I found a minor issue with the current conda-store UI check in the playwright tests; if the user has admin access over more than its namespace, the current locator (HTML ARIA tagging) returns multiple "create envs" buttons, which breaks the rest of the current logic.:

playwright._impl._errors.Error: Locator.click: Error: strict mode violation: get_by_label("Create a new environment in") resolved to 3 elements:
    1) <a tabindex="0" aria-disabled="false" href="/testuser/new-environment" aria-label="Create a new environment in the testuser namespace" class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-colorPrimary MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-colorPrimary  css-3b4swr"></a> aka get_by_role("link", name="Create a new environment in the testuser namespace")
    2) <a tabindex="0" aria-disabled="false" href="/custom1/new-environment" aria-label="Create a new environment in the custom1 namespace" class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-colorPrimary MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-colorPrimary  css-3b4swr"></a> aka get_by_role("link", name="Create a new environment in the custom1 namespace")
    3) <a tabindex="0" aria-disabled="false" href="/custom2/new-environment" aria-label="Create a new environment in the custom2 namespace" class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-colorPrimary MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-colorPrimary  css-3b4swr"></a> aka get_by_role("link", name="Create a new environment in the custom2 namespace")

Call log:
  - waiting for get_by_label("Create a new environment in")

This PR filters out to only the user namespace while also loosening the assertion with the expected default namespace against the cluster's current namespaces (if the user has more groups than the default, this returns an error in the assert right now):

AssertionError: assert ['analyst', 'custom1', 'custom2', 'developer', 'global', 'nebari-git', 'testuser', 'users'] == ['analyst', 'developer', 'global', 'nebari-git', 'testuser', 'users']
  
  At index 1 diff: 'custom1' != 'developer'
  Left contains 2 more items, first extra item: 'testuser'
  
  Full diff:
    [
        'analyst',
  +     'custom1',
  +     'custom2',
        'developer',
        'global',
        'nebari-git',
        'testuser',
        'users',
    ]

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?

How to test this PR?

  • Deploy nebari locally and create at least one custom namespace besides the default ones, then create a new environment there;
  • Run the playwright test:
    • follow the readme instruction inside /tests/.../playwright/README for setup (make setup is your friend),
    • run pytest test_playwright.py::test_conda_store_ui --numprocesses auto -vvv --slowmo 300

Any other comments?

@viniciusdc viniciusdc requested a review from a team as a code owner February 7, 2025 22:09
@viniciusdc viniciusdc requested review from dcmcand and marcelovilla and removed request for a team February 7, 2025 22:09
@dcmcand
Copy link
Contributor

dcmcand commented Feb 11, 2025

@viniciusdc can you fill out the "How to test" section for this pr?

@viniciusdc
Copy link
Contributor Author

@viniciusdc can you fill out the "How to test" section for this pr?

Hey @dcmcand thanks for the ping, I've updated the description.

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