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 compatibility prompt and notes for shared group mounting #2739

Merged
merged 7 commits into from
Sep 27, 2024

Conversation

viniciusdc
Copy link
Contributor

Reference Issues or PRs

closes #2736

What does this implement/fix?

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?

Any other comments?

@viniciusdc viniciusdc linked an issue Sep 24, 2024 that may be closed by this pull request
@viniciusdc
Copy link
Contributor Author

Tested locally:

From a fresh deployment
Captura de tela de 2024-09-24 18 59 29

inspecting the keycloak UI we can assert the role is now present there

Captura de tela de 2024-09-24 18 57 44
Captura de tela de 2024-09-24 18 58 10

Using a custom role created from the UI:

Captura de tela de 2024-09-24 18 58 38
Captura de tela de 2024-09-24 18 59 13
Captura de tela de 2024-09-24 18 59 56

@viniciusdc viniciusdc added the needs: review 👀 This PR is complete and ready for reviewing label Sep 24, 2024
@viniciusdc
Copy link
Contributor Author

viniciusdc commented Sep 24, 2024

For the failling tests, I think the our keycloak module when imported ends up initializing a connection by default with the keycloak server, which leads to issues. Will address these soon

@viniciusdc viniciusdc force-pushed the 2736-upgrade-prompt-shared-groups branch from 612574d to 329e902 Compare September 24, 2024 23:52
@viniciusdc
Copy link
Contributor Author

viniciusdc commented Sep 24, 2024

For the failling tests, I think the our keycloak module when imported ends up initializing a connection by default with the keycloak server, which leads to issues. Will address these soon

I found the root issue. In the affected tests, we have a whitelist of prompt messages and their expected responses. If the prompt is not added there, it defaults to yes. That's why it skipped my default N value in the upgrade step and reached out to the keycloak endpoint.


text = textwrap.dedent(
"""
Nebari version [green]2024.9.1[/green] introduces changes to how group
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could use a little more detail explaining exactly what's going on here and specifying that the two options here are either all groups or just admin/analyst/developer. Suggested rewording below (please review for accuracy as well as grammar!)

Nebari version [green]2024.9.1[/green] introduces changes to how group directories are mounted in JupyterLab pods.

In previous versions of Nebari, every Keycloak group in the Nebari realm initiated the creation of a shared file directory that all group members could access in their JupyterLab pod at ~/shared/[group-name].

As of Nebari version [green]2024.9.1[/green], only groups that are assigned the jupyterhub Client Role allow-group-directory-creation will

You will be asked to confirm whether Nebari should now assign this role to all of your current groups. If not, only the admin, analyst, and developer group directories will be mounted in JupyterHub pods.

Regardless of your choice now, no data will be lost during this operation. You can re-mount group directories at any time by adding or removing the allow-group-directory-creation-role from your groups in the Keycloak UI.

For more information, please see the [green][link=https://www.nebari.dev/docs/how-tos/group-directory-creation]documentation[/link][/green].

rich.print(text)

confirm = Prompt.ask(
"[bold]Would you like Nebari to update your group permissions now?[/bold]",
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are clear with what the feature is actually doing in the intro text, then I think we can change this to:

Would you like Nebari to assign allow-group-directory-creation-role to all of your groups now?

@viniciusdc
Copy link
Contributor Author

Captura de tela de 2024-09-25 16 27 12

src/_nebari/upgrade.py Outdated Show resolved Hide resolved
@viniciusdc viniciusdc merged commit e2a53e2 into develop Sep 27, 2024
28 checks passed
@viniciusdc viniciusdc deleted the 2736-upgrade-prompt-shared-groups branch September 27, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: review 👀 This PR is complete and ready for reviewing
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

[ENH] - Update upgrade prompt regarding shared group mounting
3 participants