-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Revamp TLS bootstrapping doc #11181
Revamp TLS bootstrapping doc #11181
Conversation
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.
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. |
I signed it |
Deploy preview for kubernetes-io-master-staging ready! Built with commit 4385d51 https://deploy-preview-11181--kubernetes-io-master-staging.netlify.com |
3faf00c
to
734e12c
Compare
kubelets. Kubernetes 1.4 introduced an API for requesting certificates from a | ||
cluster-level Certificate Authority (CA). The original intent of this API is to | ||
enable provisioning of TLS client certificates for kubelets. The proposal can be | ||
In a kubernetes cluster, the components on the worker nodes - kubelet and kube-proxy - need to communicate with Kubernetes master components, i.e. kube-apiserver. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kubernetes cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean capitalization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the style guide, let's try to avoid latin terms such as i.e. :) Maybe you can use 'such as' instead (felt it's more appropriate than 'that is' in this context)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aw, but I like id est
! :-)
Sure. Updating to capitalize now and removing id est
.
cluster-level Certificate Authority (CA). The original intent of this API is to | ||
enable provisioning of TLS client certificates for kubelets. The proposal can be | ||
In a kubernetes cluster, the components on the worker nodes - kubelet and kube-proxy - need to communicate with Kubernetes master components, i.e. kube-apiserver. | ||
In order to ensure that communication is kept private, not interfered with, and ensure that each is talking to a trusted element, we strongly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"that each is talking to a trusted " -> Seems to be missing a word..
Did you mean the following: "that each cluster is talking to a trusted"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, will fix that too. Updated commit coming momentarily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new commit out. I didn't squash them to make it easier for you to see changes.
|
||
The normal process of bootstrappinng these components, and especially worker nodes which need certificates so they can communicate safely with kube-apiserver, | ||
can be a challenging process, often outside of the scope of Kubernetes, and requiring significant additional work. This, in turn, can make it challenging to | ||
initialize or scale a cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Made a few minor tweaks
"The normal process of bootstrappinng these components, especially worker nodes that need certificates so they can communicate safely with kube-apiserver, can be a challenging process as it is often outside of the scope of Kubernetes and require significant additional work. This in turn, can make it challenging to initialize or scale a cluster."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Also cleaned up some minor typo and grammatical/syntactical issues.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tengqm The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you very much for the update! added a few comments for a follow-up
Bootstrap tokens are a simpler and more easily managed method to authenticate kubelets, and do not require any additional flags when starting kube-apiserver. | ||
Using bootstrap tokens is currently __beta__ as of Kubernetes version 1.12. | ||
|
||
Whichever method you choose, the requirement is that the kubelet be able to authenticate as a user in the `system:bootstrappers` group. This group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the system:bootstrappers
group is not a requirement... CSR creation permissions can be granted to any user or group. we don't want to encourage someone using an alternate auth method to include a system:bootstrappers
group just because of this statement.
We need to make it clear in this doc when we are using a specific group (like system:bootstrappers
) as a fixed example for the particular flow this document is walking you through, and when we are stating a requirement that any authentication method they chose to use must conform to.
Instead, focus on the authorization requirements (authorized to create the CSR, and authorized for approval)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do that. I get what you are saying, but I do think this makes it far easier to understand. Rather than, "you must set up a role with the right permissions and a group and... ", it is easier for people to understand, "make sure your user authenticates as system:bootstrappers
and this all will work."
Maybe something like:
Whichever method you choose, the requirement is that the kubelet be able to authenticate as a user who has the right to create and get CSRs. You can create such a group or user on your own. In the interest of standardization and simplicity, Kubernetes ships with bootstrap tokens authenticating to the `system:bootstrappers` group, which we strongly recommend you simply adopt.
@@ -69,61 +184,130 @@ systemd unit file perhaps) to enable the token file. See docs | |||
[here](/docs/reference/access-authn-authz/authentication/#static-token-file) for | |||
further details. | |||
|
|||
### Client certificate CA bundle | |||
### Authorize kubelet to create CSR | |||
Now that the bootstrapping node is _authenticated_ as part of the `system:bootstrappers` group, it needs to be _authorized_ to create a certificate signing request (CSR) as well as retrieve it when done. Fortunately, Kubernetes ships with a `ClusterRole` with precisely these (and just these) permissions, `system:node-bootstrappers`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, default role is system:node-bootstrapper
(applies below as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh!
management of the CA is beyond the scope of this document but it is recommended | ||
that you generate a dedicated CA for Kubernetes. Both certificate and key are | ||
assumed to be PEM-encoded. | ||
#### Access to key and certificate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: nesting is odd, should all these sections drop a #
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is. Maybe tidy up with the next one.
|
||
How you enable these permissions depends on which version of Kubernetes, you are running: v1.8+ or below v1.8. | ||
|
||
##### Approval 1.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the title, clarify the section applies to 1.8+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
apiGroup: rbac.authorization.k8s.io | ||
``` | ||
|
||
##### Approval Below 1.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kubernetes/sig-docs-en-reviews how many releases do we maintain legacy info like this on the website? current supported versions are 1.10+. Do we have a way to link to archived 1.8-era docs instead of including all this legacy info in a current docs page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we do end up needing to include this section, I would make it additive over the 1.8+ section and just include the role definitions for the missing system:certificates.k8s.io:certificatesigningrequests:nodeclient
and system:certificates.k8s.io:certificatesigningrequests:selfnodeclient
cluster roles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't understand this. Are you saying we should just say, "if you are running Kube <1.8, add the following roles..."? Can you create the system:*
roles? I never have tried, always treating it as "privileged".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. They're just roles
|
||
1. `nodeclient` - a request by a user for a client certificate with `O=system:nodes` and `CN=system:node:(node name)`. | ||
2. `selfnodeclient` - a node renewing a client certificate with the same `O` and `CN`. A node can use its existing client certificate to authenticate this request. | ||
To enable the kubelet to renew certificate, create a `ClusterRoleBinding` that binds the group in which the fully functioning node is a member `system:nodes` to the `ClusterRole` that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to renew its own client certificate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
will use to bootstrap its individual node identity. | ||
* A path to store the key and certificate it generates (optional, can use default) | ||
* A path to a `kubeconfig` file that does not yet exist; it will place the bootstrapped config file here | ||
* A path to a bootstrap `kubeconfig` file to provide the bootstrap token and URL for the server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to provide the bootstrap credentials
... don't imply the bootstrap credential must be a token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair.
* `server`: URL to kube-apiserver | ||
* `token`: the token to use | ||
|
||
The format of the token does not matter, as long as it matches what kube-apiserver expects. In the above example, we used a bootstrap token. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note here that any valid authentication method can be used, not just tokens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Additionally, in 1.7 the kubelet implements __beta__ features for enabling | ||
rotation of both its client and/or serving certs. These can be enabled through | ||
* use provided key and certificate, via the `--tls-private-key-file` and `--tls-cert-file` flags | ||
* create self-signed key and certificate, if a key and certificate are not provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this list is confusing in combination with the section below... I would include a third option: * request serving certificates from the cluster signer, via the CSR API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I struggled with a way to make this clear, and finally decided, let's get one in to capture the data and then improve it iteratively.
reasons](https://github.com/kubernetes/community/pull/1982). To use | ||
`RotateKubeletServerCertificate` operators need to run a custom approving | ||
controller, or manually approve the serving certificate requests. | ||
{{< /note >}} | ||
|
||
## kube-proxy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not include kube-proxy in this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only one on which I beg to differ.
I get that the intent of this page was to document how Kubelet bootstrapping works. But from an end-user perspective, someone coming to this page views it as, "oh look! A page describing how to bootstrap TLS for my workers!" The fact that kube-proxy is technically a separate component is immaterial (and confusing) to the end-user (speaking from experience here... :-) ).
My opinion, anyways, but my perspective, despite a few years of running kube in anger and digging into its myriad components, remains one of "components shmomponents, I only care about the end-use case."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few thoughts:
- kube-proxy is one of many things that can run on nodes... calling it out specifically here makes it seem more special than it is.
- I don't think we should advocate pointing kube-proxy at kubelet credentials on disk. cc @mikedanese, xref Clients using kubeconfigs that load certificates from disk should reload credentials on rotation kubernetes#70958 Guidelines for node-level plugin auth kubernetes#62747
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kube-proxy is one of many things that can run on nodes... calling it out specifically here makes it seem more special than it is.
If I look at the docs for installing kube, at a minimum, each node will have:
- container runtime (historically docker, although that can be anything with CRI, but it must exist). Either way, this does not have to authenticate to the master plane, since it is controlled by kubelet
- kubelet - covered by this doc
- kube-proxy - subject of this discussion
(do you know how many times I had to edit this because of the damn MacBook Pro keyboard? What really bad hardware engineering...)
kube-proxy is required to do things correctly, it is part of the core control plane, and the docs tell me, "install container runtime, kubelet, kube-proxy."
Doesn't that make it special? What else might I run on a node that would make it a worker and is part of the control plane?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, not trying to make a point "just because"; this really is how many of us users see it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What else might I run on a node that would make it a worker and is part of the control plane?
A network plugin that requires speaking to the API server comes to mind. If we want a section that describes approaches for running other things on the node, and kube-proxy is given as an example, that seems more reasonable. The two approaches I would highlight would be to manually provision and provide credentials, or to run as a daemonset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a network plugin be part of the official control plane?
If we want a section that describes approaches for running other things on the node, and kube-proxy is given as an example, that seems more reasonable.
Yeah, that works. Just mildly different from this one anyways. :-)
The two approaches I would highlight would be to manually provision and provide credentials, or to run as a daemonset.
In other words, get rid of the "shared credential" approach?
Let's move this all to the other open PR.
|
||
The reason for this limitation is that the kubelet attempts to bootstrap communication with kube-apiserver _before_ starting any pods, even static ones define on disk and referenced via the kubelet option `--pod-manifest-path=<PATH>`. Trying to do both TLS Bootstrapping and master components in kubelet leads to a race condition: kubelet needs to communicate to kube-apiserver to request certificates, yet requires those certificates to be available to start kube-apiserver. | ||
|
||
A fix for this issue is in progress [here](https://github.com/kubernetes/kubernetes/pull/71174). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer referencing the issue filed for this behavior (kubernetes/kubernetes#68686) over a specific pull request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
Sure @liggitt , we can do another one. |
As requested by @liggitt per Slack conversation, proposed update to TLS Bootstrapping doc to make it more of a how-to guide, while including design, with clearer steps and descriptions of options and limitations.