Skip to content

Conversation

@mterhar
Copy link
Contributor

@mterhar mterhar commented Aug 12, 2021

If TLS is terminated by an ALB, the service port 80 routes traffic without redirecting
to HTTPS and the HTTPS port presents an error since it's pointing to another HTTPS
port of the node.

This change allows the load balancer to expose only port 443 externally which routes to
the HTTP port of the coderd pod.

If TLS is terminated by an ALB, the service port 80 routes traffic without redirecting
to HTTPS and the HTTPS port presents an error since it's pointing to another HTTPS
port of the node.

This change allows the load balancer to expose only port 443 externally which routes to
the HTTP port of the coderd pod.
@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #15687: Support TLS termination at the Load Balancer.

Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

I don't have much context on this, but it seems reasonable.

@mterhar mterhar merged commit be5b7f6 into main Aug 21, 2021
@mterhar mterhar deleted the mterhar/ch15687/lb-tls-termination branch August 21, 2021 22:27
# 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

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

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

jawnsy pushed a commit that referenced this pull request Aug 24, 2021
jawnsy pushed a commit that referenced this pull request Aug 24, 2021
@mterhar mterhar changed the title Load balancer TLS termination Load balancer TLS termination [Reverted] Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants