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

Kubelet Server TLS Certificate Rotation #267

Open
jcbsmpsn opened this Issue Apr 24, 2017 · 26 comments

Comments

Projects
@jcbsmpsn
Member

jcbsmpsn commented Apr 24, 2017

Feature Description

  • One-line feature description (can be used as a release note):
    Rotation of the server TLS certificate on the kubelet.

  • Primary contact (assignee):
    @jcbsmpsn

  • Responsible SIGs:
    sig-auth

  • Design proposal link (community repo): kubernetes/community#602

  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred:
    @mikedanese @awly

  • Approver (likely from SIG/area to which feature belongs):
    @tallclair

  • Initial target stage (alpha/beta/stable) and release (x.y):

    • alpha 1.7
    • beta 1.12
@ericchiang

This comment has been minimized.

Member

ericchiang commented Apr 25, 2017

I'm assuming this encompasses initial bootstrapping of the serving cert too?

@jcbsmpsn

This comment has been minimized.

Member

jcbsmpsn commented Apr 25, 2017

Yes, initial bootstrapping of the server cert will be covered by this. Would you like a separate one, or just checking that feature wasn't lost in the shuffle?

@ericchiang

This comment has been minimized.

Member

ericchiang commented Apr 25, 2017

Just checking, thanks.

@idvoretskyi

This comment has been minimized.

Member

idvoretskyi commented May 3, 2017

@jcbsmpsn please, provide us with the design proposal link

@idvoretskyi idvoretskyi added this to Action required in Kubernetes 1.7 features May 3, 2017

@jcbsmpsn

This comment has been minimized.

Member

jcbsmpsn commented May 8, 2017

Design proposal: kubernetes/community#602

@idvoretskyi

This comment has been minimized.

Member

idvoretskyi commented May 9, 2017

@jcbsmpsn updated the feature description with the link, thanks.

@idvoretskyi idvoretskyi removed the help wanted label May 9, 2017

@idvoretskyi idvoretskyi moved this from Action required to In Progress in Kubernetes 1.7 features May 9, 2017

@jcbsmpsn jcbsmpsn changed the title from Server TLS Certificate Rotation to Kubelet Server TLS Certificate Rotation Jun 5, 2017

@ericchiang

This comment has been minimized.

Member

ericchiang commented Jun 22, 2017

Going to merge this into #266

@ericchiang ericchiang closed this Jun 22, 2017

@liggitt

This comment has been minimized.

Member

liggitt commented Jun 22, 2017

Actually, I wouldn't. There's more work to do here with determining which SANs a node is allowed to serve.

@liggitt liggitt reopened this Jun 22, 2017

@apsinha

This comment has been minimized.

apsinha commented Jun 27, 2017

@jcbsmpsn
What is the link for the documentation update for certificate rotation? Is there no docs that need update?

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certifiate
rotation.

ref:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certifiate
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:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267
@jcbsmpsn

This comment has been minimized.

Member

jcbsmpsn commented Jun 27, 2017

@apsinha Some documentation for this is included in kubernetes/website#4208

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
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:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
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:

- kubernetes/kubernetes#45030
- kubernetes/enhancements#266
- kubernetes/enhancements#267
@luxas

This comment has been minimized.

Member

luxas commented Aug 14, 2017

@jcbsmpsn Can you please update this feature's status for v1.8?
Is beta targeted or will this be still in alpha?

@fejta-bot

This comment has been minimized.

fejta-bot commented May 17, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented May 29, 2018

k8s-merge-robot added a commit to kubernetes/kubernetes that referenced this issue Jul 12, 2018

Merge pull request #65594 from liggitt/node-csr-addresses-2
Automatic merge from submit-queue (batch tested with PRs 65052, 65594). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Derive kubelet serving certificate CSR template from node status addresses

xref kubernetes/enhancements#267
fixes #55633

Builds on #65587

* Makes the cloud provider authoritative when recording node status addresses
* Makes the node status addresses authoritative for the kube-apiserver determining how to speak to a kubelet (stops paying attention to the hostname label when determining how to reach a kubelet, which was only done to support kubelets < 1.5)
* Updates kubelet certificate rotation to be driven from node status
  * Avoids needing to compute node addresses a second time, and differently, in order to request serving certificates.
  * Allows the kubelet to react to changes in its status addresses by updating its serving certificate
  * Allows the kubelet to be driven by external cloud providers recording node addresses on the node status

test procedure:
```sh
# setup
export FEATURE_GATES=RotateKubeletServerCertificate=true
export KUBELET_FLAGS="--rotate-server-certificates=true --cloud-provider=external"

# cleanup from previous runs
sudo rm -fr /var/lib/kubelet/pki/

# startup
hack/local-up-cluster.sh

# wait for a node to register, verify it didn't set addresses
kubectl get nodes 
kubectl get node/127.0.0.1 -o jsonpath={.status.addresses}

# verify the kubelet server isn't available, and that it didn't populate a serving certificate
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
ls -la /var/lib/kubelet/pki

# set an address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname, but NOT the IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki

# set an hostname and IP address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"},{"type":"InternalIP","address":"127.0.0.1"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname AND IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki
```

```release-note
* kubelets that specify `--cloud-provider` now only report addresses in Node status as determined by the cloud provider
* kubelet serving certificate rotation now reacts to changes in reported node addresses, and will request certificates for addresses set by an external cloud provider
```

k8s-publishing-bot added a commit to kubernetes/client-go that referenced this issue Jul 12, 2018

Merge pull request #65594 from liggitt/node-csr-addresses-2
Automatic merge from submit-queue (batch tested with PRs 65052, 65594). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Derive kubelet serving certificate CSR template from node status addresses

xref kubernetes/enhancements#267
fixes #55633

Builds on kubernetes/kubernetes#65587

* Makes the cloud provider authoritative when recording node status addresses
* Makes the node status addresses authoritative for the kube-apiserver determining how to speak to a kubelet (stops paying attention to the hostname label when determining how to reach a kubelet, which was only done to support kubelets < 1.5)
* Updates kubelet certificate rotation to be driven from node status
  * Avoids needing to compute node addresses a second time, and differently, in order to request serving certificates.
  * Allows the kubelet to react to changes in its status addresses by updating its serving certificate
  * Allows the kubelet to be driven by external cloud providers recording node addresses on the node status

test procedure:
```sh
# setup
export FEATURE_GATES=RotateKubeletServerCertificate=true
export KUBELET_FLAGS="--rotate-server-certificates=true --cloud-provider=external"

# cleanup from previous runs
sudo rm -fr /var/lib/kubelet/pki/

# startup
hack/local-up-cluster.sh

# wait for a node to register, verify it didn't set addresses
kubectl get nodes
kubectl get node/127.0.0.1 -o jsonpath={.status.addresses}

# verify the kubelet server isn't available, and that it didn't populate a serving certificate
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
ls -la /var/lib/kubelet/pki

# set an address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname, but NOT the IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki

# set an hostname and IP address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"},{"type":"InternalIP","address":"127.0.0.1"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname AND IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki
```

```release-note
* kubelets that specify `--cloud-provider` now only report addresses in Node status as determined by the cloud provider
* kubelet serving certificate rotation now reacts to changes in reported node addresses, and will request certificates for addresses set by an external cloud provider
```

Kubernetes-commit: 337dfe0a9cde3894eb6a26f9184df659d54007c6

@liggitt liggitt added stage/beta and removed stage/alpha labels Jul 25, 2018

@liggitt liggitt added this to the v1.12 milestone Jul 25, 2018

@liggitt liggitt self-assigned this Jul 25, 2018

@liggitt

This comment has been minimized.

Member

liggitt commented Jul 25, 2018

@mikedanese I updated this to beta for 1.12, just for the rotation/kubelet bits, since the approval piece was moved out of scope

@justaugustus

This comment has been minimized.

Member

justaugustus commented Jul 26, 2018

Thanks for the update! This has been added to the 1.12 Tracking sheet.

k8s-merge-robot added a commit to kubernetes/kubernetes that referenced this issue Jul 28, 2018

Merge pull request #66726 from liggitt/kubelet-server
Automatic merge from submit-queue (batch tested with PRs 62444, 66358, 66724, 66726). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Move kubelet serving cert rotation to beta

xref kubernetes/enhancements#267

This is exercised in the alpha gke e2es, and can be enabled in the non-alpha gke e2es once it no longer requires an alpha feature gate.

```release-note
Kubelet serving certificate bootstrapping and rotation has been promoted to beta status.
```
@zparnold

This comment has been minimized.

Member

zparnold commented Aug 20, 2018

Hey there! @jcbsmpsn I'm the wrangler for the Docs this release. Is there any chance I could have you open up a docs PR against the release-1.12 branch as a placeholder? That gives us more confidence in the feature shipping in this release and gives me something to work with when we start doing reviews/edits. Thanks! If this feature does not require docs, could you please update the features tracking spreadsheet to reflect it?

@mikedanese

This comment has been minimized.

Member

mikedanese commented Sep 7, 2018

Beta docs PR here:

kubernetes/website#10232

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Sep 28, 2018

@Marcel-Lambacher: cat image

In response to this:

/meow

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.

@ameukam

This comment has been minimized.

Contributor

ameukam commented Oct 5, 2018

Hi folks,
Kubernetes 1.13 is going to be a 'stable' release since the cycle is only 10 weeks. We encourage no big alpha features and only consider adding this feature if you have a high level of confidence it will make code slush by 11/09. Are there plans for this enhancement to graduate to beta/stable within the 1.13 release cycle? If not, can you please remove it from the 1.12 milestone or add it to 1.13?

We are also now encouraging that every new enhancement aligns with a KEP. If a KEP has been created, please link to it in the original post. Please take the opportunity to develop a KEP.

@kacole2

This comment has been minimized.

Contributor

kacole2 commented Oct 8, 2018

@liggitt @mikedanese @jcbsmpsn and plans to graduate for 1.13?

@kacole2 kacole2 added tracked/no and removed tracked/yes labels Oct 8, 2018

@mikedanese mikedanese removed this from the v1.12 milestone Oct 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment