-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Update TLS bootstrapping with 1.7 features #4208
Update TLS bootstrapping with 1.7 features #4208
Conversation
f6ed129
to
4b88f61
Compare
The controller categorizes CSRs into three subresources: | ||
|
||
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 re-requesting a client certificate with the same `O` and `CN`. |
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.
s/re-requesting/renewing/
|
||
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 re-requesting a client certificate with the same `O` and `CN`. | ||
3. `selfnodeserver` - a node requesting or re-requesting a serving 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.
s/re-requesting/renewing/
Why include 'requesting'?
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 indicate that this is for the initial serving cert too. Will change.
2. `selfnodeclient` - a node re-requesting a client certificate with the same `O` and `CN`. | ||
3. `selfnodeserver` - a node requesting or re-requesting a serving certificate. | ||
|
||
The checks to determine if a CSR is a `selfndeclient` or `selfnodeserver` request is currently tied to the kubelet's |
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.
s/selfndeclient/selfnodeclient/
4b88f61
to
24a0436
Compare
@@ -87,6 +177,19 @@ The flag to enable this bootstrapping when starting the kubelet is: | |||
--experimental-bootstrap-kubeconfig="/path/to/bootstrap/kubeconfig" | |||
``` | |||
|
|||
Additionally, in 1.7 the kubelet implements __alpha__ features for enabling rotation of both its client and/or serving certs. | |||
These can be enabled through the respective `RotateKubeletClientCertificate` and `RotateKubeletServerCertificate` feature | |||
flags on the kubelet, but may change in backward incompatible ways in future releases. |
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.
Also RotateKubeletServerCertificate
on the API server. The auto approver for the server certificate is feature gated.
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 CSR logic is in the API server? I would have expected that in the controller manager. Does the controller manager detect this based on the API server configuration?
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.
Sorry, I mis-spoke. It is indeed in the controller manager.
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.
Thanks for clarifying. Will update the doc.
BTW, I think we should turn this into a Task, which I can do in a follow up PR. Currently, it's not findable in the table of contents. |
24a0436
to
6bc0bb9
Compare
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.
Just some nits from tech review
|
||
The flag is: | ||
In 1.7 the experimenal "group auto approver" controller is dropped in favor of a new `csrapproving` controller |
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: experimental
would s/of a new/of the new/
make sense?
|
||
The flag is: | ||
In 1.7 the experimenal "group auto approver" controller is dropped in favor of a new `csrapproving` controller | ||
that ships as part of [kube-controller-manager](/docs/admin/kube-controller-manager/) and is enabled by default. |
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.
suggest: enabled by default
The flag is: | ||
In 1.7 the experimenal "group auto approver" controller is dropped in favor of a new `csrapproving` controller | ||
that ships as part of [kube-controller-manager](/docs/admin/kube-controller-manager/) and is enabled by default. | ||
The controller uses the [`SubjectAcessReview` API](/docs/admin/authorization/#checking-api-access) to determine |
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.
SubjectAccessReview
3. `selfnodeserver` - a node renewing a serving certificate. (ALPHA, requires feature gate) | ||
|
||
The checks to determine if a CSR is a `selfnodeserver` request is currently tied to the kubelet's credential rotation | ||
implementation, an __alpha__ feature. As such, the definition of `selfnodeserver` will likely change in a future and |
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.
link to the features issue?
# A ClusterRole which instructs the CSR approver to approve a user requesting | ||
# node client credentials. | ||
kind: ClusterRole | ||
apiVersion: rbac.authorization.k8s.io/v1beta1 |
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.
Are there auto-bootstrapped ClusterRoles for these?
I think that would make a lot of sense, at least in v1.8
name: auto-approve-csrs-for-group | ||
subjects: | ||
- kind: Group | ||
name: system:kubelet-bootstrap |
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 kind of group is this? Is it well-known or just something you wrote here?
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 was trying to provide a migration path for users who where using the deprecated approver. Will update the docs, thanks.
--insecure-experimental-approve-all-kubelet-csrs-for-group="system:kubelet-bootstrap"
--insecure-experimental-approve-all-kubelet-csrs-for-group="system:kubelet-bootstrap" | ||
``` | ||
|
||
To let a node renew its own credentials, an admin would construct a `ClusterRoleBinding` such as: |
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.
Is it worth showing how you can let all nodes in the cluster renew their CSRs automatically by binding to the system:nodes
group or should the user just understand 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.
I think it'd be best practice to have each node get a role binding so you can revoke individual node access, so I'd rather avoid suggesting that.
The controller categorizes CSRs into three subresources: | ||
|
||
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`. |
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 think this is pretty much alpha as well @jcbsmpsn, right?
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 "beta" and will hopefully not change. selfnodeserver
is the only recognizer that is alpha and will change
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.
Ok, cool, so it's just kubelet's mechanism of renewing those client certs that is alpha. Gotcha.
This includes documenting the new CSR approver built into the controller manager and the kubelet alpha features for certificate rotation. Since the CSR approver changed over the 1.7 release cycle we need to call out the migration steps for those using the alpha feature. This document as a whole could probably use some updates, but the main focus of this PR is just to get these features minimally documented before the release.
6bc0bb9
to
fbb1904
Compare
Thanks everyone for the reviews. I think this is good to go. |
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certificate
rotation.
Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.
ref:
cc @mikedanese @jcbsmpsn @luxas @liggitt
This change is