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 warning about ExternalName not working with TLS/HTTPS #16627

Closed
wants to merge 3 commits into from

Conversation

jimmyjones2
Copy link
Contributor

As per original discussion in kubernetes/kubernetes#13748

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
  • Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 30, 2019
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Sep 30, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign zacharysarah
You can assign the PR to them by writing /assign @zacharysarah in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@netlify
Copy link

netlify bot commented Sep 30, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 2968b3f

https://deploy-preview-16627--kubernetes-io-master-staging.netlify.com

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 30, 2019
@@ -913,6 +913,9 @@ forwarding. Should you later decide to move your database into your cluster, you
can start its Pods, add appropriate selectors or endpoints, and change the
Service's `type`.

{{< warning >}}
ExternalName many not work with services protected with TLS SNI and/or HTTPS.
Copy link
Contributor

Choose a reason for hiding this comment

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

@jimmyjones2 This doesn't feel like enough of an explanation (I had to go and read the previous issue).

  • I'm not sure that SNI or not is relevant. If the remote service uses TLS but not SNI, the client will still see a hostname mismatch when they connect to (eg) my-service.prod or my.service.prod.svc.cluster.local.
  • HTTPS might be useful in case readers haven't heard of TLS though.

How much is this different to the challenge with using TLS with a Service within your cluster? In both cases it feels like you need to make sure that the TLS server can provide a valid certificate and that if you don't the clients won't be happy.

It's a point worth making but it's also important to be clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sftim Absolutely, I agree it's not really an explanation. I can think of 3 issues:

  • Client: certificate CN/SAN won't match
  • Server: if using SNI to route traffic, it won't know where to send it
  • Server: with HTTP/HTTPS, the Host field will be wrong, so might not know which website to serve

I think the challenge is different to internal services. With internal services you can use the internal Kubernetes CA. For services outside the cluster, no public CA will sign certificates with internal cluster hostnames, so your only option is a corporate CA, but even then it's probably against policy as two clusters could have the same internal service name.

The other challenge is you may not control the external service, so are unable to encourage them to add support for the hostname you internally refer to it as. Even if you could, they might not even be able to help you due to the reasons above, and embedding internal cluster configuration into an external service seems nasty.

How about:

"ExternalName might not work for services using protocols such as as TLS or HTTP. These use the hostname to secure and/or route traffic and may not be expecting the internal cluster name"

@sftim
Copy link
Contributor

sftim commented Oct 3, 2019

"ExternalName might not work for services using protocols such as as TLS or HTTP. These use the hostname to secure and/or route traffic and may not be expecting the internal cluster name"

How about:

You may have trouble using ExternalName for common protocols, including HTTP and
HTTPS.
If you use ExternalName then the DNS name seen by clients inside your cluster is
different from the name that the ExternalName references.

For protocols that use hostnames this difference may lead to errors or unexpected
responses.
HTTP requests will have a `Host:` header that the origin server does not
recognize; TLS servers will not be able to provide a certificate matching the
hostname that the client indicates.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Oct 5, 2019
@k8s-ci-robot
Copy link
Contributor

Keywords which can automatically close issues and at(@) mentions are not allowed in commit messages.

The list of commits with invalid commit messages:

  • 53044e1 Updated ExternalName warning with @sftim suggested wording

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jimmyjones2
Copy link
Contributor Author

@sftim Moved to #16704, sorry!

@jimmyjones2 jimmyjones2 closed this Oct 5, 2019
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. do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants