Separate automatic HTTPS to its own deployment #592
Conversation
|
I've tested this without HTTPS, but need to test it with HTTPS. |
| @@ -86,7 +89,6 @@ spec: | |||
| - name: https | |||
| containerPort: 443 | |||
| protocol: TCP | |||
| {{ if and .Values.proxy.https.hosts (and .Values.proxy.https.enabled (eq .Values.proxy.https.type "letsencrypt" ) ) -}} | |||
There was a problem hiding this comment.
I just realized this is a rat's nest of conditionals. I'm just going to split the autoHTTPS part into its own deployment, which should make things easier for us to maintain for now. Longer term, we can replace CHP with something that does ACME (or add a sidecar that does ACME and stores the certificates in a kubernetes Secret object)
|
Makes sense. This would affect Binder, right? as we do our own https there, so the internal nginx would be disabled? |
|
@minrk yep, this would make life simpler in Binder too. |
We use an nginx in front of CHP just for HTTPS auto setup. However, in cases where we don't use HTTPS, the extra nginx layer complicates things. Adds an additional layer of timeouts and connection pools for us to think about. In the multiple-hubs setup, I have two nginx layers in front of hubs already, and adding another layer without need makes things complicated.
The extra network hop is worth the following advantages: 1. The code is much easier for us to understand and reason about. 2. We have pod affinity set up to try to place all three components (hub, proxy, autohttps) in the same node if possible. This should cut down on hops! 3. For users with at least one proxy in front of the hub, this is a much cleaner way to disable the extra nginx cost - both in terms of resources and extra processing overhead. Longer term, we should build / use a proxy implementation that can automatically get certificates from ACME and store them in Kubernetes Secret objects.
No need for the extra hop here!
yuvipanda
changed the title
|
@minrk I've pretty much redone this entire PR. Have also updated description. I've tested both manual and automatic let's encrypt modes. LMK what you think! |
|
This is also the last change I need for my EdX launch I think :) |
| labels: | ||
| name: proxy | ||
| component: proxy | ||
| release: {{ .Release.Name }} | ||
| heritage: {{ .Release.Service }} | ||
| {{- if eq .Values.proxy.https.type "letsencrypt" }} | ||
| # required for kube-lego to work | ||
| app: kube-lego |
There was a problem hiding this comment.
We could add app: jupyterhub as a label now that this hack is out of the way :)
There was a problem hiding this comment.
I get a warm feeling about this PR! I don't grasp whats done fully, but it seems like you are successfully separating concerns and making the code more understandable!
|
Nice improvement! I'll let @consideRatio update the |
The extra network hop is worth the following advantages:
(hub, proxy, autohttps) in the same node if possible. This
should cut down on hops!
this is a much cleaner way to disable the extra nginx cost -
both in terms of resources and extra processing overhead.
Longer term, we should build / use a proxy implementation that
can automatically get certificates from ACME and store them
in Kubernetes Secret objects.
We also terminate HTTPS in CHP when using manual HTTPS, avoiding
an extra hop unnecessarily. IIRC this means we lose http/2 when
using manual HTTPS, but that feels fine for now.