A secrets friendly version of extraEnv

[xrl]

I am looking for design feedback on a more general version of extraEnv. I have looked over #440 and it makes valid points but doesn't talk about a design to move forward.

Folks are going to be creating multi-user jupyter hubs where they'll want all environment to share some base secrets. I'm thinking AWS IAM credentials for reading from buckets or athena. We're not talking about "admin for everything" kind of credentials.

While these secrets are low impact I would rather not keep them in my values.yml used for the jhub release. What if I wanted to keep my kube config project as an open repo? I should be able to balance transparent kube objects and kube's secret system. Also, I want all my secrets in kubernetes so I can more easily audit and roll them later.

Could we convert extraEnv back to a more "raw" format? Instead of the current envKey: envValue key-pair, could we do a list of native kubernetes env values? We would absolutely be giving up the convenience in exchange for a more verbose but ultimately far more powerful format. A blind substition is also more the style of helm charts.

So envFrom would be something like:

  extraEnv:
    - name: AWS_ACCESS_KEY_ID
      value: 1234ABCD

and if they wanted to mount a secret instead:

  extraEnv:
    - name: AWS_ACCESS_KEY_ID
      valueFrom:
        secretKeyRef:
          name: jupyter-user
          key: AWS_ACCESS_KEY_ID

Perhaps we don't want to break folks values.yml, what would be another good values.yml key to use?

[minrk]

could we do a list of native kubernetes env values?

I definitely think we should move toward standard kube structures as much as possible. However, we've also learned that list config doesn't work well for hierarchical charts since they don't get merged (setting extraEnv in one chart that includes jupyterhub means deployments must be aware of and duplicate all of the extraEnv in that chart in their deployment if they want to set any env without clobbering). Dicts merge a lot nicer, so I'm somewhat inclined to deviate from the spec only in that the config ought to be a dict somehow. There are two ways I see to do that:

1. a dict of exactly kubernetes EnvVar arrays that just get concatenated, or
2. a dict keyed by env name where the body is either a simple env value (current), or an EnvVar struct.

We already do pattern number 1 where we've encountered this hierarchy before, in extraConfig. It's useful, but has also proven confusing to users who often omit the extra layer of keys, which doesn't make a lot of sense when writing your own config, since the key is arbitrary and any given deployment likely only has the one. Pattern 2 is the smallest deviation from the current spec, but is also a bit more custom and requires more logic (type checking) in the chart. I'm not sure which is best from a user perspective.

[MrSaints]

I'd be quite keen on injecting secrets as environment variables to user pods as well. As a workaround, there is https://kubernetes.io/docs/tasks/inject-data-application/podpreset/, but it is not available in most managed solutions.

EDIT: also, it'd be ideal if the imagePullSecrets could be defined outside of using kubespawner_override. Being able to inject secrets as environment variables will also open up the opportunity to avoid hardcoding credentials for nbgitpuller.

[sgloutnikov]

Also ran into the need to mount secrets as environment variables into the user pod and ended up with two options. For now, the first one looks easier to manage as there are less moving pieces.

1. Use hub.extraConfig to alter c.KubeSpawner.modify_pod_hook. The idea came from the following kubespawner issue. You would do something like this in your values.yaml file to append as many ENV variables as needed (in a loop if you have many):

hub:
  extraConfig:
    ...
    10-secretToEnv: |
      from kubernetes import client
      def modify_pod_hook(spawner, pod):
          pod.spec.containers[0].env.append(
              client.V1EnvVar(
                  name='YOUR_ENV_VARIABLE', 
                  value_from=client.V1EnvVarSource(
                      secret_key_ref=client.V1SecretKeySelector(
                          name='your-secret',
                          key='SECRET_KEY',
                      )
                  )
              )
          )
          return pod
      c.KubeSpawner.modify_pod_hook = modify_pod_hook
    ...

2. Mount the secret(s) to the singleuser.storage filesystem and add a singleuser.lifecycleHooks.podStart.exec.command (some bash script) to read over the appropriate files and create a .bash_profile with the ENV variables and values. You now have the secrets mounted as ENV variables in the login shell, but launching a new kernel will not source them, as discussed in the following JupyterHub issue. Add a simple kernel-helper.sh script to be executed as the first argv argument in your kernel(s) kernel.json. This will start the kernel in the same environment as the login shell.

kernel-helper.sh:

#!/bin/bash -il
exec "$@"

singleuser:
  storage:
    extraVolumes:
      - name: mysecrets
        secret:
          secretName: your-secret
    extraVolumeMounts:
      - name: mysecrets
        mountPath: /etc/secrets

[jtlz2]

@sgloutnikov Did (1) work? :\

[sgloutnikov]

@jtlz2 yes, both worked. Not in the example above, but I went with a loop in there to minimize copy-pasting a bunch of the same code since ENV variable and key were the same names for me.

[chrisroat]

Is there a specific proposal here? I believe this is a very nice idea.

[consideRatio]

This should now be supported, as documented in the configuration reference both for hub.extraEnv and singleuser.extraEnv.

Thanks for reporting and thumbing up interacting etc, it was very useful to see 17 thumbs up on the issue to help prioritize this issue!

Btw any confirmations that this did indeed solve the issue would be appreciated, especially for singleuser.extraEnv configuration.

[peterrmah]

@consideRatio I can confirm the extraEnv worked successfully on my deployment. Thanks for the help!

[MattiasVahlberg]

@peterrmah What version of helm chart did you use when you got this to work? I've tried with 0.9.0 but did not get it to work..
Reference to secrets:

ENV_VAR:
  valueFrom:
    secretKeyRef:
      name: mysecret
      key: secret1

But getting errors when trying to spawn a new pod.

[consideRatio]

@MattiasVahlberg try 0.9.0-n251.h9594ed30 for example, its the latest as seen from pressing the orange badge in the repo's README.md and pressing "development releases" on the site you arrive to.

[peterrmah]

@MattiasVahlberg I was using version 0.9.0

[consideRatio]

@peterrmah hmmm, support for this syntax was introduced after 0.9.0, in 0.9.0-nXXX.hXXXXXXX something. I'd recommend not trying going for the latest current development version: 0.9.0-n251.h9594ed30

ENV_VAR:
  valueFrom:
    secretKeyRef:
      name: mysecret
      key: secret1

[metasim]

I'm also not able to get this to work in the hub section, version 0.9.1. The secret gets expanded like this.

name: "CLIENT_SECRET"
value: "map[name:CLIENT_SECRET valueFrom:map[secretKeyRef:map[key:CLIENT_SECRET name:app-secrets]]]"

[consideRatio]

@metasim sorry, the 0.9.1 is confusingly older than the latest development version prefixed 0.9.0.

This is of course unwanted and cause confusion. The technical mumbo-jumbo relates to 0.9.1 was a single-commit security patch to 0.9.0 while 0.9.0-nXXX.hXXXXXXX are all the commits since 0.9.0, which also includes the single commit patch released as 0.9.1.

So, try with 0.9.0-n251.h9594ed30.

[kyprifog]

@consideRatio Is there a verison of 0.9.1 where this works? I had downgraded to the version you had stated, but it breaks with other components i'm using (namely dask-gateway) which seems to only work with 0.9.1 and up on my testing.

[kamac]

@kyprifog Can you upgrade to 0.10.2? I believe that's the newest version, and it works for me there.

[kyprifog]

Thanks, @kamac I confirmed that upgrading to 0.10.2 did indeed fix the problem. Now I have to deal with the fact that upgrade broke daskhub for me, but thats a different issue (specifically this one: dask/helm-chart#130 ) :)

[consideRatio]

Thanks, @kamac I confirmed that upgrading to 0.10.2 did indeed fix the problem. Now I have to deal with the fact that upgrade broke daskhub for me, but thats a different issue (specifically this one: dask/helm-chart#130 ) :)

@kyprifog if something broke when upgrading to 0.10.2, I guess it relates to networkpolicy enforcement. Did some pod try to access the hub pod directly without a hub.jupyter.org/network-access-hub: "true" label in the same namespace for example?

You could conclude if its about netwrok policies by disabling them for the. See the z2jh configuration reference about that. At this point, I want our discussion in this issue to stop entirely now though, because this is no longer at all related to the topic and it will be hard to find for anyone that would benefit from reading this conversation.

[kyprifog]

@consideRatio, apologies, I didn't actually mean to continue the conversation beyond that comment (thought I made that clear by saying its a different issue) but thanks for the reply! I'll pursue any follow ups in the linked issue.