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

Multi coupon support #21

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

Conversation

marcelovilla
Copy link
Member

@marcelovilla marcelovilla commented Jul 1, 2024

Description

This PR adds support for multiple coupons, so each can have independent configuration options. For example, having different coupons to add users to different keycloak groups.

Related Issue

Closes #14

Motivation and Context

We've been using this extension in training and tutorials where there's a clear benefit of having different coupons map to different groups to manage access. For example, you might have one coupon granting access to GPUs, while other coupon just grants access to regular instances. Or you might have yet another coupon which grants admin rights.

Having support for multiple coupons make it easy to handle these cases by assigning them to different keycloak groups and managing separate permissions for them.

How Has This Been Tested?

This has been tested by:

  1. Installing the extension from the multi-coupon branch:
pip install git+https://github.com/marcelovilla/nebari-self-registration@multi-coupon
  1. Set the following extension configuration in nebari-config.yaml and create the required groups in keycloak if they do not exist already:
self_registration:
  coupons:
    METROSTAR:
      account_expiration_days: 365
      approved_domains: ["*.mil"]
      registration_groups: ["metrostar"]
    QUANSIGHT:
      account_expiration_days: 31
      approved_domains: ["quansight.com"]
      registration_groups: ["quansight"]
    METROSTARADMIN:
      account_expiration_days: 365
      approved_domains: ["*.mil"]
      registration_groups: ["metrostar", "admin"]
  values:
    image:
      repository: ghcr.io/marcelovilla/nebari-self-registration
      tag: "20240701-1553"
  1. Re-deploy Nebari:
nebari deploy -c nebari-config.yaml
  1. Go to /registration and use the different coupons to create new accounts. If everything works as expected, you'll see the new users in keycloak and depending what coupon you used, you'll see they've been assigned to the corresponding groups.

Screenshots (if appropriate):

@kenafoster
Copy link
Contributor

This introduces a breaking change to the config schema (previous values like self_registration.approved_domains are no longer valid since they're under keys in the coupons dict now)

nebari upgrade doesn't have a hook in for updating extension config (this is not possible as things are currently implemented since upgrade checks for differences between installed nebari version and that specified in the config file.... and there are no specified versions of extensions in the config file to check against anyway)

@marcelovilla is there a way to add a more detailed error message than the below in the scenario where a user has the "old" config schema value(s) set?

I don't think this is necessary since there likely isn't much adoption of this extension outside of our own use cases (and we're still not on 1.0.0 semantic version anyway). Just asking in case there's an easy way to do this that you've encountered in similar situations before. Otherwise, I'd be good with just updating the README to mention this change.

ValidationError: 4 validation errors for ConfigSchema
self_registration.coupons
  Input should be a valid dictionary [type=dict_type, input_value=['C1i.3kg|p&v+', 'Bsn8PBdGW6o'], input_type=CommentedSeq]
    For further information visit https://errors.pydantic.dev/2.4/v/dict_type

@marcelovilla
Copy link
Member Author

Hey @kenafoster. Sorry for the late reply; I was out for two weeks and then I was catching up on other stuff.

is there a way to add a more detailed error message than the below in the scenario where a user has the "old" config schema value(s) set?

I don't think there's a straightforward way of adding detailed information on the error message right now. I can update the README on my PR to highlight these changes. Would you be OK with that?

@kenafoster
Copy link
Contributor

Hey @kenafoster. Sorry for the late reply; I was out for two weeks and then I was catching up on other stuff.

is there a way to add a more detailed error message than the below in the scenario where a user has the "old" config schema value(s) set?

I don't think there's a straightforward way of adding detailed information on the error message right now. I can update the README on my PR to highlight these changes. Would you be OK with that?

@marcelovilla A note in the README sounds good to me.

@marcelovilla
Copy link
Member Author

@kenafoster I just added a note on the README about the changes introduced here. Let me know if that makes sense

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.

Support for multiple coupons with specifc Keycloak group assignation
2 participants