Added proxy.chp.extraEnv and proxy.traefik.extraEnv configuration

[agrahamlincoln]

the values.yaml implies that this option is available, but it is not. this change adds the template to the proxy deployment so .Values.proxy.extraEnv is available

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[agrahamlincoln]

The travisCI build failed because CoreDNS was unable to start up:

Waiting for deployment "coredns" rollout to finish: 0 of 1 updated replicas are available...
error: timed out waiting for the condition
Startup of Kubernetes DNS server failed!
...
Warning FailedScheduling 5m6s 0/1 nodes are available: 1 node(s) had taint {node.cloudprovider.kubernetes.io/uninitialized: true}, that the pod didn't tolerate.

Any way to restart the build?

[yuvipanda]

This is great, @agrahamlincoln! Thank you.

Do you think you can add an entry documenting this to https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/master/jupyterhub/schema.yaml? I see extraEnv is documented for hub & user pods, but not for proxy. That would make the config option easier to discover for others.

[consideRatio]

Thanks @agrahamlincoln! @yuvipanda would you say we want this to be nested under proxy.chp.extraEnv rather than proxy.extraEnv? I'm not 100% but I think that makes the most sense.

We both have proxy.chp and proxy.traefik, where chp is the jupyterhub's controlled proxy, and proxy.traefik refers to the TLS termination proxy, and setting extra env should be specific to the pod rather than generic.

Hmm yes I think we should configure those separately! @agrahamlincoln could you implement that change? If you want, feel free to add proxy.traefik.extraEnv to the autohttps deployment pod running traefik as well.

[agrahamlincoln]

Do you think you can add an entry documenting this to https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/master/jupyterhub/schema.yaml? I see extraEnv is documented for hub & user pods, but not for proxy. That would make the config option easier to discover for others.

Sure, though it seems like schema.yaml does not currently validate against the existing values.yaml 🙃
I tried to run validate.py and found numerous violations of jsonschema named types such as int instead of integer and bool instead of boolean. After fixing these, the values.yaml provided doesn't validate due to many instances where values in values.yaml specifies None instead of strings, etc.

Initially I was concerned with adding it because jsonschema does not allow specification of multiple possible types, which is the case for extraEnv. The template for this will accept either an object or a list, depending on how the user wishes to define their environment variables. Because it does not currently validate, the fact that multiple types aren't supported in the jsonschema validate won't break any validation workflows worse than they might be already. I am happy to add it as an additional source of documentation with the above multi-type behavior documented in the property description.

would you say we want this to be nested under proxy.chp.extraEnv rather than proxy.extraEnv? I'm not 100% but I think that makes the most sense.

I agree with you here since it is effectively modifying only the chp pod. I can also add this to the traefik pod as well.

[consideRatio]

Thank you for excellent work and documenting these in schema.yaml!

I'm a bit worried that the examples provided may indicate that its the only way to specify something, which could exclude the ability to define an environment variable in a k8s native syntax for example. The examples in hub.extraEnv and singleuser.extraEnv are more complete, perhaps you could copy paste from them, reference them, or extend your examples to cover the various options?

Feel free to update those examples as well if you like to change something for cohesiveness.

Thank you for your work on this @agrahamlincoln !!

[consideRatio]

Sure, though it seems like schema.yaml does not currently validate against the existing values.yaml upside_down_face
I tried to run validate.py and found numerous violations of jsonschema named types such as int instead of integer and bool instead of boolean.

Ah, yes we have not maintained the validate.py script well =/ I'm not familiar with it myself actually. I tried helm lint --strict which could do a schema validation like I believe validate.py does. It will be work for another PR to resolve those parts. I've tried a bit in the past #1681.

I agree with you here since it is effectively modifying only the chp pod. I can also add this to the traefik pod as well.

Wieee thank you!

[agrahamlincoln]

I honestly didn't see the examples for extraEnv elsewhere, and they're pretty good so I copied them in and modified them. Let me know if you disagree with any of the "usual use-cases" for modifying the environment variables in schema.yaml

The initial commit message is no longer correct now that we're enabling .Values.proxy.chp.extraEnv and .Values.proxy.traefik.extraEnv but that can be re-written on the squash/rebase 👍

Feel free to adjust this how you like, but if I were to rebase/squash this whole branch now, I'd name the commit as follows

enable extraEnv parameter for chp and traefik proxies

[consideRatio]

LGTM @agrahamlincoln! Thank you for your thorough work on this! I'll go ahead and merge this as it is. I think since the chain of commits are associated under the merge its fine to have a commit that is a bit off.

🎉 ❤️!

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️