Upgrade to z2jh 3.0.2 from 3.0.0-beta.1 - oauthenticator 15.1 bumped to 16.0

[consideRatio]

• Fixes Upgrade z2jh from 3.0.0-beta.1 to 3.0.0-beta.3 (breaking: oauthenticator 15.1.0 is bumped to 16.0.2) #2774

This PR would benefit from a careful review as typos could break things not tested until actual users show up and use their identity provider to login.

I've tested this on a few staging hubs successfully so far.

Self review on the following commits

For each commit checked, I've checked for unelated changes to the commit
description.

• 18ddef9 - oauthenticator 16: remove shown_idps, allowed_idps now provides that effect
  For each idp removed in shown_idps, there looked to be an entry in
  allowed_idps.

• 640660d - oauthenticator 16: remove explicit scope, profile is included anyhow
  Assumes we upgrade to oauthenticator 16 in a subsequent commmit, because there
  "profile" and "email" scopes are included by default. In oauthenticator 16,
  there is also another scope included.

  Overall, this upgrade will cause scope changes, and we will end up requesting
  a bit more than we did before no matter what, but I think it should be fine.

• 435432d - oauthenticator 16: add allow_existing_users where allowed_users was configured
  Besides adding allow_existing_users=True where allowed_users was
  configured (that implied allow_existing_users=True in OAuthenticator <16),
  this commit also removes fixme comments related to the pending upgrade and
  sometimes relocates keys under hub.config to have a consistent ordering.

• 0da0b3e - oauthenticator 16: remove outdated comment about allowed_users
  The comments like below has been removed:

  This hub uses GitHub Org auth and so we don't set allowed_users in order to
  not deny access to valid members of the listed orgs.

  This comment related to oauthenticator<16 when it was unreasonable to
  combine allowed_users with allowing members of an external github org/team,
  because then you had to be listed in both (or be an existing user and in the
  github org/team) to gain access.

  With oauthenticator 16, this is no longer the case - you can be part of
  either allowed_users or allowed_organizations.

• 0c43b0c - auth config: remove outdated workaround setting empty admin_users
  The comments like below has been removed:

  You must always set admin_users, even if it is an empty list,
  otherwise add_staff_user_ids_to_admin_users: true will fail
  silently and no staff members will have admin access.

  They were a remnant since before Take into account all possible states of c.Authenticator.admin_users … #2299 that fixed this.

• 63eabbc - oauthenticator 16: remove redundant spec of allowed_users, add warnings
  Systematically removed specification of allowed_users in favor of
  admin_users only, its sufficient to specify just admin_users in
  OAuthenticator 16 to grant them access, and because a falsy or truthy
  allowed_users config doesn't matter now that the explicit configuration
  allow_existing_users has been introduced.

  • I'll iterate on the comment leading with To properly revoke access, as
    it doesn't mention admin status in any way which it should as well.

• 7201a88 - auth config: remove temporary config addition
  The pangeo-hubs/coessing hub had delete_invalid_users specified, but I'm
  almost 100% this isn't relevant and has either mistakenly been introduced or
  was just temporarily used to cleanup the database of user with a trick.

• a3bb003 - basehub: tweak values to avoid formatting conflicts
  When using chartpress I found that config was changed, and that pre-commit
  also could end up changing it back. To avoid a battle between autoformatting
  stuff, I updated the config to have the autoformatters settle.

  • This commit mistakenly includes a version bump of the hub image in
    basehub. This isn't much of an issue as fa134cea provides a final bump of
    all the hub images.

• 28cd4f5 - basehub: refactor, simplify chowning container's command
  This commit was added for consistency with the previous commit, where
  chartpress had modified syntax of a chown command in basehub's values
  specifically and I wanted all to look similar. As part of this I also reduced
  the complexity of the command by letting a single chown command update
  multiple folders at once.

• 4545ef6 - basehub: upgrade z2jh from 3.0.0-beta.1 to 3.0.2
  This commit include some unrelated refactoring for consistency in
  chartpress.yaml, and added a comment or two.

• 37d9911 - dynamic image building experiment: bump kubespawner's main branch further
  I had in another PR updated the unlisted_choice experiment image to have a
  more recent version of kubespawner with misc fixes, but I had missed out on
  that also the dynamic-image-building-experiment used kubespawner's main
  branch. Due to this, I ended up making sure they were both updated to the same
  version. This change is in other words quite unrelated (at least assuming the
  previous kubespawner commit was modern enough, a requirement for z2jh 3).

• fa134ce - basehub: update hub image's to a z2jh 3.0.2 derived image
  This is what

[GeorgianaElena]

@consideRatio, if it's oke, I will wait a bit reviewing this PR until #3114 is merged and this one rebased as you suggested in the top comment.

I started looking at the changes in this diff consideRatio/pilot-hubs@pr/auth-refactoring-and-fixes...consideRatio:pilot-hubs:pr/z2jh-3-upgrade, but I still need to get back to this PR, find the row and then leave the review comment here, so I'll instead wait.

[consideRatio]

Thank you @GeorgianaElena! I'll perform a self review commit by commit myself now and include comments about them.

[consideRatio]

@GeorgianaElena okay this is now self-reviewed with an updated PR description about that. I pushed a commit refining a warning comment and made a note in the PR about a mistake in a commit that is resolved by a later commit anyhow.

[GeorgianaElena]

@consideRatio, I reviewed each commit and everything looked fine to me! Thank you very much for taking the time to do this upgrade, with lots of breaking config changes <3

I only left a comment about the long warning comment that I believe belongs in the docs. But if you believe that might require more bandwidth from you that you don't have, please feel free to ignore it and don't block the merge because of it or open an issue to maybe address it later if you agree with it.

[GeorgianaElena]

Unfortunately I am not a fan of carrying this sort of long comments everywhere.

For example, this comment is not relevant when we use CILogon, because there is no concept of allowing groups of users there (but there is the allowing of a domain part though that still stands I believe).

My personal preference is to have such a comment in a documentation that we follow when we set up the authentication for a hub (or in a future template of some sort).

Though, I understand that we don't have yet templates for hub configs, so we end up copying config from one hub to another and such info might get lost in this process if we only have it in the docs.

Maybe, instead of having one long comment we could instead have a short one that links to this explanation that we put somewhere in the docs instead.

What do you think?

[consideRatio]

I opened #3134 for a short term fix as not addressing this right away makes me able to get this out the door before I take thursday/firday off in order to work on my move!

[yuvipanda]

A big plus one to what @GeorgianaElena says as well. I often find long repeated comments extremely distracting when working on the projects too.

[consideRatio]

Thank you soo much for your review @GeorgianaElena!!!

[github-actions]

🎉🎉🎉🎉

Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/6179150978