Skip to content

Load balancer TLS termination [Reverted] #144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 21, 2021
Merged

Load balancer TLS termination [Reverted] #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b6e3ec
Allow user to set httpsToHttp for ALB tls
Aug 12, 2021
24119da
Add value to support httpsToHttp
Aug 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions templates/coderd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,20 +247,22 @@ spec:
selector:
coder.deployment: {{ include "coder.serviceName" . }}
ports:
- name: tcp-{{ include "coder.serviceName" . }}
port: 80
targetPort: 8080
protocol: TCP
{{ if .Values.coderd.serviceNodePorts.http }}
nodePort: {{ .Values.coderd.serviceNodePorts.http }}
{{ end }}
- name: tcp-{{ include "coder.serviceName" . }}-https
port: 443
{{- if .Values.coderd.httpsToHttp }}¬
targetPort: 8443
protocol: TCP
{{ if .Values.coderd.serviceNodePorts.https }}
nodePort: {{ .Values.coderd.serviceNodePorts.https }}
{{ end }}
- name: tcp-{{ include "coder.serviceName" . }}
port: 80
{{- end }}
targetPort: 8080
protocol: TCP
{{ if .Values.coderd.serviceNodePorts.http }}
nodePort: {{ .Values.coderd.serviceNodePorts.http }}
{{ end }}
{{- else }}
---
apiVersion: v1
Expand Down
5 changes: 5 additions & 0 deletions values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ coderd:
# coderd.serviceAnnotations -- Extra annotations to apply to the coderd service.
serviceAnnotations: {}

# coderd.httpsToHttp -- eliminates the external http port and routes traffic from
# the external https port to the internal http port. Useful for when the load balancer
# performs TLS termination (like Amazon's ACM)
httpsToHttp: false
Copy link

@jawnsy jawnsy Aug 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done a little bit of reading (How do I terminate HTTPS traffic on Amazon EKS workloads with ACM?
) but am by no means an expert on AWS.

It seems unusual to me that we'd be using a LoadBalancer spec when a frontend device is performing termination for us, and instead I'd have expected that we'd use an Ingress definition to do this? A Kubernetes LoadBalancer would be more like an Layer 3 (Network Load Balancer) and a Service with Ingress would be more like a Layer 7 device (Application Load Balancer)

Alternatively, is there a way to configure things manually so that we expose coderd as a ServiceIP (on the internal VPC network) and then manage the NLB/ALB outside of our Kube specs? Adding stuff to our Helm chart, especially in the absence of tests, increases complexity, so my preference is always to keep one-off customizations like this out of there, unless this is a very general thing with all of the clouds...

Copy link

@jawnsy jawnsy Aug 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like an alternative to this implementation would be to make the full service spec configurable as users want, instead of using conditionals based on this httpsToHttp setting


# coderd.trustProxyIP -- Whether Coder should trust X-Real-IP and/or
# X-Forwarded-For headers from your reverse proxy. This should only be turned
# on if you're using a reverse proxy that sets both of these headers. This is
Expand Down