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

Make sure that Kubelet Certificate Rotation is enabled in v1.8 #386

Closed
luxas opened this issue Aug 14, 2017 · 6 comments · Fixed by kubernetes/kubernetes#52196
Closed

Make sure that Kubelet Certificate Rotation is enabled in v1.8 #386

luxas opened this issue Aug 14, 2017 · 6 comments · Fixed by kubernetes/kubernetes#52196
Assignees
@luxas
Labels
area/security priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone
v1.8

Comments

@luxas
Copy link
Member

luxas commented Aug 14, 2017

As kubernetes/enhancements#266 and kubernetes/enhancements#267 mature, we should enable them in the kubeadm-specific kubelet arg list (https://github.com/kubernetes/release/blob/master/debian/xenial/kubeadm/channel/stable/etc/systemd/system/kubelet.service.d/10-kubeadm.conf)

At least client cert rotation will be beta in v1.8. We should enable that behavior by default.

cc @kubernetes/sig-auth-feature-requests @jcbsmpsn
@luxas luxas self-assigned this Aug 14, 2017
@luxas luxas added area/security kind/enhancement priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Aug 14, 2017
@luxas luxas added this to the v1.8 milestone Aug 14, 2017
@luxas luxas mentioned this issue Sep 7, 2017
Closed
@ericchiang
Copy link

Since this is part of the 1.8 milestone, going to note that this feature will not be turned on by default for the kubelet.

kubernetes/kubernetes#51045 (comment)

CSRs aren't properly garbage collected as of 1.8, so having the kubelets create some arbitrary number of CSRs on clusters that don't have an external controller to clean them up is problematic. Hoping to address this in 1.9.

@luxas
Copy link
Member Author

luxas commented Sep 8, 2017

We're planning to enable it via the feature gate. I saw that CSRs aren't GC'd yet, but if I understood it correctly, the expiration time is one year, right?
It makes a lot of sense to enable this now that we're migrating from kubeadm join doing the TLS bootstrap => /etc/kubernetes/kubelet.conf in v1.7 to kubeadm join writing /etc/kubernetes/bootstrap-kubelet.conf in v1.8, which kubelet uses to do the TLS bootstrapping itself. As we're doing this transition anyway, I think it's reasonable to enable now. (Also see kubernetes/kubernetes#52188)

@ericchiang
Copy link

We're planning to enable it via the feature gate.

Feature gate is enabled by default. There was a new flag added as part of kubernetes/kubernetes#51045

--rotate-certificates

This defaults to false because the actual CSR kubernetes resources in the certificates API are never removed, even if they're expired, denied, or even ignored for some period of time.

@luxas
Copy link
Member Author

luxas commented Sep 8, 2017

@ericchiang Gotcha. Then we're planning to set --rotate-certificates for the reasons I mentioned above :)
Makes sense?

@ericchiang
Copy link

Yep!

@luxas luxas mentioned this issue Sep 8, 2017
Merged
@mattmoyer
Copy link

Just to clarify because I wasn't familiar with the 1.8 behavior...

CSRs aren't garbage collected in 1.8, but the default lifetime is 1 year and rotation only happens after between 70% (~8 months) and 90% (~10 months) of the lifetime of the certificate. So in the worst case a cluster of N nodes will accumulate N CSRs every 8.4 months or so. This means after 3 years you still only have around 4N CSRs.

This is where the default certificate lifetime is set:
https://github.com/kubernetes/kubernetes/blob/b2b079b95a867b560fd327cba56e7d3878f43dc3/cmd/kube-controller-manager/app/options/options.go#L113

This is where the 70%-90% calculation is done:
https://github.com/kubernetes/kubernetes/blob/4d409a4d9e9772193cb24944bd76b90aaca01473/pkg/kubelet/certificate/certificate_manager.go#L311-L319

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Sep 12, 2017
Merge pull request #52196 from luxas/kubeadm_enable_rotation
63b3eea 
Automatic merge from submit-queue (batch tested with PRs 52007, 52196, 52169, 52263, 52291)

kubeadm: Enable certificate rotation

**What this PR does / why we need it**:

Enables cert rotation as planned for the v1.8 cycle in kubernetes/kubeadm#386
Can now be done as everything's in place in the code now that beta.1 is released with all the necessary features (Kubelet clientcert rotation now beta, woot!)

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

fixes: kubernetes/kubeadm#386

**Special notes for your reviewer**:

This file does _ONLY_ affect the kubeadm e2e CI.
What will actually end up in the debs/rpms is going into kubernetes/release right before v1.8 is released (due to how those scripts work, not optimal :/ )

**Release note**:

```release-note
kubeadm: Enable kubelet client certificate rotation
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @kubernetes/sig-auth-pr-reviews
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@luxas luxas

Labels
area/security priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Milestone
v1.8
Development

Successfully merging a pull request may close this issue.

kubeadm: Enable certificate rotation luxas/kubernetes
3 participants
@ericchiang @mattmoyer @luxas