Skip to content

Fix for CILogon to provide a default idp to select #3267

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

Closed
consideRatio opened this issue Oct 13, 2023 · 5 comments · Fixed by #3454 or #3463
Closed

Fix for CILogon to provide a default idp to select #3267

consideRatio opened this issue Oct 13, 2023 · 5 comments · Fixed by #3454 or #3463
Assignees
Labels
nominated-to-be-resolved-during-q4-2023 Nomination to be resolved during q4 goal of reducing the technical debt

Comments

@consideRatio
Copy link
Contributor

consideRatio commented Oct 13, 2023

https://2i2c.freshdesk.com/a/tickets/1038 reported what is tracked in jupyterhub/oauthenticator#690 which should be easy to fix with the planning done already.

I think we should try to resolve this quickly without waiting for / rushing an oauthenticator release by:

  1. develop a fix, trial the fix, open a PR to oauthenticator
  2. update our default hub image to reference PR branch
    • I think we shouldn't use an experiment because its a regression impacting many hubs rather than a feature we need feedback and look to iterate on
  3. report to Robert in https://2i2c.freshdesk.com/a/tickets/1038 that this is now resolved
@yuvipanda
Copy link
Member

Looking at the generated values.yaml used with k -n ucmerced get secret hub -o json | jq -r '.data."values.yaml" ' | base64 -D

    CILogonOAuthenticator:
      allowed_idps:
        http://google.com/accounts/o8/id:
          username_derivation:
            username_claim: email
        urn:mace:incommon:ucmerced.edu:
          allow_all: true
          username_derivation:
            username_claim: eppn

So helm is doing something with the values.yaml that's causing this.

@yuvipanda
Copy link
Member

And we use the helm toYaml function to write this out, and it sorts keys lexicographically. And since 'u' comes after 'h' we end up in this situation.

yuvipanda added a commit to yuvipanda/pilot-hubs that referenced this issue Nov 7, 2023
@damianavila
Copy link
Contributor

damianavila commented Nov 14, 2023

Given that #3385 (workaround) was merged (and communicated via FD), I am gonna to close this one for now.
Feel free to re-open if you disagree.

@github-project-automation github-project-automation bot moved this from Todo 👍 to Done 🎉 in Sprint Board Nov 14, 2023
@github-project-automation github-project-automation bot moved this from Needs Shaping / Refinement to Complete in DEPRECATED Engineering and Product Backlog Nov 14, 2023
@yuvipanda yuvipanda reopened this Nov 14, 2023
@yuvipanda
Copy link
Member

I think we should keep this open until the upstream issue is sorted

@damianavila damianavila moved this from Done 🎉 to Waiting 🕛 in Sprint Board Nov 15, 2023
@github-project-automation github-project-automation bot moved this from Waiting 🕛 to Done 🎉 in Sprint Board Nov 27, 2023
@consideRatio consideRatio reopened this Nov 27, 2023
@consideRatio
Copy link
Contributor Author

Requires oauthenticator 16.2.1 with a bugfix in jupyterhub/oauthenticator#705.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nominated-to-be-resolved-during-q4-2023 Nomination to be resolved during q4 goal of reducing the technical debt
Projects
No open projects
Status: Done 🎉
3 participants