Hub existingSecret functionality not working as expected

[zarrarrana]

Bug description

Hi, I am trying to update jupyterhub to helm chart 0.10.6 from 0.9.0, and with the recent change of moving values.yaml only to hub-secret and removing it from hub-config is breaking the Hub existingSecret functionality. In our existing deployment on EKS we use gitops/flux/helm operator for our deployments and we didn't want to add secrets to git, so we created hub-secret which only includes secrets(proxy token, db creds, crypto-key, cookie-secret, etc) using external secrets operator and set the name as existingSecret. Now when the existingSecret is set, the secret is not created and values.yaml doesn't make it to the hub pod and it doesn't start properly.

Expected behaviour

The previous behaviour of existingSecret should continue to work without having to add values.yaml to existingSecret.

[consideRatio]

Yeah =/ This is the kind of experience I feared from accepting this feature in the first place. The documentation has a note about you needing to keep it updated.

Short term

Short term, what you are required to do is to specify your passwords etc without hub.existingSecret set, then re-create the k8s secret, and then set hub.existingSecret again and remove your specification of passwords etc.

Long term

My understanding of the feature wanted is: to be able to extract all secret information from a helm configuration file, and letting the Helm chart pick up on those from a k8s Secret.

Currently this works, but, the major drawback is that one need to manually manage the k8s Secret even though the only change was in default configuration between Helm chart versions rather than the user provided passwords.

I think it is possible to implement a better solution. Such solution should improve the user experience but also not add too fragile logic that is hard to maintain.

Brainstorming long term solution

• proxy.secretToken is special a special case because its used outside the hub pod, but we can manage this with Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated #1993.
• We could make hub.existingSecret be an optionally mounted k8s secret to the hub pod, which then is only supposed to contain a values.yaml entry that we then load and merge into other values.

Related

• Functionality added in Add support for existing secret for hub #1142
• Requested in Support "existingSecret" for secrets used in jupyterhub chart #1622
• Upcoming functionality to not need to specify proxy.secretToken / hub.cookieSecret / hub.config.CryptKeeper.keys in Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated #1993

[consideRatio]

@minrk @manics @yuvipanda - what do you think about the brainstormed solution above to making hub.existingSecret a sustainable practice for users/maintainers?

Sounds to me like a potential 1.0.0 enhancement to aim for.

[meeseeksmachine]

This issue has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/how-am-i-supposed-to-use-existingsecret/5097/4

[consideRatio]

Reimplementation of hub.existingSecret

This is my preliminary strategy planned to make hub.existingSecret sustainable to use to manage sensitive configuration needed by the Helm chart.

Use of hub.existingSecret hasn't been a good experience for users because they end up needing to manually update it whenever they change something passed to the hub pod through values.yaml to be read in jupyterhub_config.py.

My idea is to solve this by allowing hub.existingSecret be mounted in parallel to the default k8s Secret resource we make use of and then merging values. For keys such as hub.db.password and hub.config.JupyterHub.proxy_auth_token that are root level keys in the secrets, the idea is to always favor reference to an hub.existingSecret if there is one set.

Implementation challenges

1. JupyterHub.proxy_auth_token and hub.db.password are used from helm templates (hub / proxy deployment env vars). Even if we have a existingSecret, we can't inspect it. and must reference one or the other secret - there is no merge functionality we can use here as we don't actually probe the content but instead only declare where it resides. This forces those fields to be pinned in one or the other Secret. Due to this, we should always favor existingSecret content over defaultSecret content whenever it could be defined in either.

   Since we need to favor looking for hub.db.password and JupyterHub.proxy_auth_token in the existingSecret, usage of it will require it to be found there.

2. The Helm lookup function that can be used during template rendering is only able to lookup chart managed resources, so not the existingSecret even if we wanted to.

   This is causing a problem for the automatic generation of the proxy_auth_token secret because we can't lookup the existingSecret while also giving priority to existingSecret, and it would force the user to declare something there that otherwise can be autogenerated since Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated #1993.

   Due to this, I suggest we lookup JupyterHub.proxy_auth_token in the defaultSecret at all time and write an exception notice regarding that in the docs of hub.existingSecret.

Implementation steps

1. Let hub.existingSecret mount in parallel to the defaultSecret in the hub pod.
2. Merge in existingSecret values with defaultSecret values, and let top level keys override defaultSecret values if they are defined. This can be handled in z2jh.py.
3. Create three separate _helpers-names.tpl templates and make use of them where appropriate.
   • hub used for: volume declaration hub pod, mounting env var from JupyterHub.proxy_auth_token
   • hub-existing-secret used for: volume declaration hub pod (conditional)
   • hub-existing-secret-or-default used for: mounting of hub.db.password env var
4. Update configuration reference
5. changelog strategy: point to docs

---

Feedback on the strategy appreciated of course.

[bbockelm]

@consideRatio -

I realize the whole approach is being reworked for 1.0.0 but is it possible / is there interest to backport a simpler approach into 0.11.x and 0.10.x?

Basically, prior to 0.10.x (and specifically, #1682), if existingSecret is used, then all chart values were available in the config map and hence usable as part of jupyterhub_config.py. Starting in 0.10.0, no chart values were available to the jupyterhub_config.py.

So folks who are waiting on 1.0.0 and want to use existingSecret have to copy/paste all changes to values.yaml into a separate secret -- or have to put the token into the helm chart.

This is a pretty dramatic difference - and not all that obvious from the documentation.

@zarrarrana - we are in the exact same boat as you. How do you plan on solving this problem until 1.0.0?

[consideRatio]

Starting in 0.10.0, no chart values were available to the jupyterhub_config.py

Hmmmm... This would work... jupyterhub_config.py have access to everything that is mounted, which still is quite a lot of the Helm chart values afaik - because it mounts the k8s Secret for example.

hub:
  extraConfig:
    myExtraConfig:
      from z2jh import get_config
      a_lot_of_things = get_config("hub.something")

The short answer with regards to backporting is that unless it is a security fix, we don't put in effort in doing it to reduce maintenance burden. This specific feature would also be quite complicated to backport actually because it relies on some other refactoring PRs.

If you want this feature before next release, you can use the dev-release though. They are supposed to work at all time, and an actual release is pretty much just a snapshot. For the dev release version, see/click the badges in this projects README file.

For reference, here is a version that can be used right now: 0.11.1-n302.h2b488a3a, and documented in here.

[bbockelm]

Hmmmm... This would work... jupyterhub_config.py have access to everything that is mounted, which still is quite a lot of the Helm chart values afaik - because it mounts the k8s Secret for example.

I don't think this is correct; the secret is dropped completely if the existingSecret is set. Regardless...

The short answer with regards to backporting is that unless it is a security fix, we don't put in effort in doing it to reduce maintenance burden

... ok, that's what I was looking for! Many thanks for your work; I do understand the concerns of maintenance burden.

@zarrarrana - one workaround I found is this:

https://docs.fluxcd.io/projects/helm-operator/en/stable/helmrelease-guide/values/#secrets

That is, we can locally stop using existingSecret and embed part of values in a secret referred to from the HelmRelease.