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.

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

Jump to bottom

Update documentation to include multiple-TLS support. #278

Merged
merged 1 commit into from
May 22, 2018
Merged

Update documentation to include multiple-TLS support. #278

merged 1 commit into from
May 22, 2018

Conversation

rramkumar1
Copy link
Contributor

/assign @nicksardo

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 20, 2018
@rramkumar1 rramkumar1 mentioned this pull request May 20, 2018
Merged
nicksardo
nicksardo reviewed May 21, 2018
View reviewed changes
README.md Outdated
For encrypted communication between the client to the load balancer, you need to specify a TLS private key and certificate to be used by the ingress controller.
For encrypted communication between the client to the load balancer, you need to specify a TLS private key and certificate to be used by the ingress controller.

Version 1.1 of GLBC now supports (as a beta feature) using more than one SSL certificate in a single Ingress for request termination (aka Multiple-TLS). With this change, keep in mind that the upper limit is 10.
Copy link
Contributor

Choose a reason for hiding this comment

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

"GCP's limit is 10". Could you also link to the GCP page that says that, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

kimroen
kimroen reviewed May 21, 2018
View reviewed changes
README.md Outdated
For encrypted communication between the client to the load balancer, you need to specify a TLS private key and certificate to be used by the ingress controller.
For encrypted communication between the client to the load balancer, you need to specify a TLS private key and certificate to be used by the ingress controller.

Version 1.1 of GLBC now supports (as a beta feature) using more than one SSL certificate in a single Ingress for request termination (aka Multiple-TLS). With this change, keep in mind that the upper limit is 10.
Copy link

Choose a reason for hiding this comment

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

How can a user of GKE know what version of the load balancer they are using to realise whether or not this information is relevant to them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/manifests/glbc.manifest

stevedomin
stevedomin reviewed May 21, 2018
View reviewed changes
README.md Outdated
metadata:
name: no-rules-map
spec:
tls:
Copy link

Choose a reason for hiding this comment

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

How would you specify which certificate to use in an example with a rules map?

apiVersion: extensions/v1beta1
kind: Ingress
spec:
  tls:
  - secretName: svc1-certificate
  - secretName: svc2-certificate
  backend:
    serviceName: svc1
    servicePort: svc1-port
  rules:
  - host: svc1.example.com
    http:
      paths:
      - path: /*
        backend:
          serviceName: svc1
          servicePort: svc1-port
  - host: svc2.example.com
    http:
      paths:
      - path: /*
        backend:
          serviceName: svc2
          servicePort: svc2-port

Copy link
Contributor Author

@rramkumar1 rramkumar1 May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You should depend on the hostname provided in your cert to do this. For example, if the client request indicates a hostname of svc1.example.com and your svc1-certificate is for that same hostname, then svc1-certificate will be served. I will update the PR with your example to make this clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stevedomin Should be updated now, PTAL!

Copy link

Choose a reason for hiding this comment

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

@rramkumar1 thanks fo the updated doc. How does this work with wildcard certificate? Let's say that I'm using a EV certificate for a specific subdomain and a wildcard one for others (see example below). Is it always going to pick up the most specific certificate first?

apiVersion: extensions/v1beta1
kind: Ingress
spec:
  tls:
  - secretName: ev-certificate
  - secretName: wildcard-certificate
  backend:
    serviceName: svc1
    servicePort: svc1-port
  rules:
  - host: ev-domain.example.com
     http:
      paths:
      - path: /*
        backend:
          serviceName: svc1
          servicePort: svc1-port
  - host: sub.example.com
    http:
      paths:
      - path: /*
        backend:
          serviceName: svc2
          servicePort: svc2-port

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is straight from the GCP docs https://cloud.google.com/compute/docs/load-balancing/http/ssl-certificates:

"When the load balancer selects a certificate, it always prefers to match a hostname to certificates without wildcards over certificates with wildcards"

Copy link

Choose a reason for hiding this comment

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

Looks great, got it working in my project. Thanks again!

Copy link

@kimroen kimroen May 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

In my project I've also specified the tls[].hosts[] parameter for my certificates, but it doesn't look like that has any effect with this Ingress Controller. Would that assumption be right?

Something like this:

spec:
  tls:
  - hosts:
    - 'ev-domain.example.com'
    secretName: ev-certificate
  - hosts:
    - '*.example.com'
    secretName: wildcard-certificate

Copy link
Contributor Author

@rramkumar1 rramkumar1 May 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Correct, this controller ignores the hosts parameter.

Copy link

Choose a reason for hiding this comment

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

Perfect, thank you. Running this in production now, so it's working as documented here ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update the documentation to include this point in a separate PR.

@nicksardo
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2018
@nicksardo nicksardo merged commit 3ce055c into kubernetes:master May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicksardo nicksardo nicksardo left review comments

@stevedomin stevedomin stevedomin left review comments

@kimroen kimroen kimroen approved these changes

Assignees

@nicksardo nicksardo

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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rramkumar1 @nicksardo @stevedomin @kimroen @k8s-ci-robot