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

Revamp TLS bootstrapping doc #11181

Merged
merged 3 commits into from
Nov 23, 2018

Conversation

deitch
Copy link
Contributor

@deitch deitch commented Nov 22, 2018

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.

@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 email the CNCF helpdesk: [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/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 22, 2018
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Nov 22, 2018
@deitch
Copy link
Contributor Author

deitch commented Nov 22, 2018

I signed it

@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 Nov 22, 2018
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Nov 22, 2018

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

Built with commit 4385d51

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

@deitch deitch force-pushed the improve-tls-bootstrapping-docs branch from 3faf00c to 734e12c Compare November 22, 2018 13:43
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Kubernetes cluster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean capitalization?

Copy link
Contributor

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)?

Copy link
Contributor Author

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
Copy link
Contributor

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"

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.
Copy link
Contributor

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."

Copy link
Contributor Author

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.

@tengqm
Copy link
Contributor

tengqm commented Nov 23, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 23, 2018
@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2018
@k8s-ci-robot k8s-ci-robot merged commit 4d30a79 into kubernetes:master Nov 23, 2018
@deitch deitch deleted the improve-tls-bootstrapping-docs branch November 23, 2018 12:54
Copy link
Member

@liggitt liggitt left a 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
Copy link
Member

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)

Copy link
Contributor Author

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`.
Copy link
Member

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)

Copy link
Contributor Author

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
Copy link
Member

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 #?

Copy link
Contributor Author

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
Copy link
Member

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+

Copy link
Contributor Author

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
Copy link
Member

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?

Copy link
Member

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.

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 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".

Copy link
Member

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
Copy link
Member

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

Copy link
Contributor Author

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
Copy link
Member

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

Copy link
Contributor Author

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.
Copy link
Member

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

Copy link
Contributor Author

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
Copy link
Member

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

Copy link
Contributor Author

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
Copy link
Member

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.

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 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."

Copy link
Member

Choose a reason for hiding this comment

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

A few thoughts:

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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).
Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

@deitch
Copy link
Contributor Author

deitch commented Nov 24, 2018

Sure @liggitt , we can do another one.

@deitch
Copy link
Contributor Author

deitch commented Nov 24, 2018

@liggitt I think got all your comments. Take a look at #11258

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants