Skip to content
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.

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

Add Backends.SSLPassthrough attribute #540

Merged
merged 1 commit into from
Apr 5, 2017

Conversation

jcmoraisjr
Copy link
Contributor

Add and configure Backends[].SSLPassthrough attribute based on server.SSLPassthrough attr and ingress.kubernetes.io/ssl-passthrough annotation.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 2, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 43.882% when pulling b59d49a on jcmoraisjr:jm-ssl-passthrough into 02cd3ce on kubernetes:master.

@aledbf
Copy link
Member

aledbf commented Apr 4, 2017

@jcmoraisjr why are you adding this information in the backend? (this is already present)

@jcmoraisjr
Copy link
Contributor Author

@aledbf this attribute is a single point of check if a backend is really a backend up to terminate SSL requests or not.

There are other two informations regarding SSL term:

  1. Backend.Secure means Ingress will terminate SSL and reencrypt the connection to the backend.
  2. Server.SSLPassthrough which willl also populate Configuration.PassthroughBackends is a server point of view that: a) doesn't validate a type mismatch - different servers with different SSLPassthrough configurations pointing to the same backend; and b) I cannot query the actual value from a backend iteration.

@aledbf
Copy link
Member

aledbf commented Apr 4, 2017

Backend.Secure means Ingress will terminate SSL and reencrypt the connection to the backend.

Right. This only applies to ingress controllers that require the protocol to reach the upstream (like nginx). This is not required in haproxy. Just in case this does not means TLS termination in the POD, just that the pod must be contacted in port 443

a) doesn't validate a type mismatch - different servers with different SSLPassthrough configurations pointing to the same backend

This is correct. SSLPassthrough means terminate TLS in the pod. You could have the same backend in different ingress with different behavior. I don't think this should be a mutually exclusive option

@jcmoraisjr
Copy link
Contributor Author

On HAProxy the backends (PODs) has two types of possible configurations: encrypt http data to https, or just proxy the request as is. I can use Backend.Secure to distinguish between them, however the proposal does a bit more:

  • Check if the same backend (pod) is receiving SSLPassthrough and plain http. This is what I called a type mismatch.
  • Avoid a HAProxy misconfiguration if the user configure both Backend.Secure and SSLPassthrough. If this wouldn't checked I'd connect a TCP frontend to a HTTP+TLS backend - the server would refuse to start.
  • Warn about SSLPassthrough servers without a valid default backend and with path that wouldn't be used.

@aledbf aledbf self-assigned this Apr 5, 2017
@aledbf
Copy link
Member

aledbf commented Apr 5, 2017

/lgtm

@aledbf aledbf merged commit ed6987e into kubernetes:master Apr 5, 2017
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 5, 2017
@jcmoraisjr jcmoraisjr deleted the jm-ssl-passthrough branch April 5, 2017 21:18
@aledbf aledbf mentioned this pull request Apr 17, 2017
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants