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

kubeadm should leverage kubelet automatic client cert rotation on nodes created with kubeadm init #1753

Closed
lnovara opened this issue Aug 26, 2019 · 18 comments · Fixed by kubernetes/kubernetes#83339 or kubernetes/kubernetes#84118
Assignees
Labels
area/security kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@lnovara
Copy link

lnovara commented Aug 26, 2019

Is this a BUG REPORT or FEATURE REQUEST?

FEATURE REQUEST

Versions

kubeadm version kubeadm version: &version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-26T15:59:52Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"darwin/amd64"}

What happened?

On master nodes, /etc/kubernetes/kubelet.conf gets created with "hardcoded" client-certificate/client-key instead of pointing to /var/lib/kubelet/pki/kubelet-client-current.pem as done on minions node.

What you expected to happen?

I expected /etc/kubernetes/kubelet.conf to point to /var/lib/kubelet/pki/kubelet-client-current.pem to leverage automatic kubelet client certificate rotation that is configured by kubeadm

How to reproduce it (as minimally and precisely as possible)?

kubeadm init && cat /etc/kubernetes/kubelet.conf

Anything else we need to know?

I already know this a chicken-and-egg problem but I think it would be really nice if the first master, after initialising the control plane, could make use of /var/lib/kubelet/pki/kubelet-client-current.pem to further streamline the certificates rotation process and avoid having to use kubeadm init kubeconfig kubelet just on the master nodes to renew kubelet's client certificate.

@lnovara
Copy link
Author

lnovara commented Aug 26, 2019

/assign fabriziopandini

@neolit123
Copy link
Member

neolit123 commented Aug 26, 2019

@lnovara i think the request is a reasonable one.
@fabriziopandini want me to take this?
(although probably needs some discussion / decision making first)

@neolit123 neolit123 added area/security priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Aug 26, 2019
@neolit123 neolit123 added this to the v1.16 milestone Aug 26, 2019
@neolit123
Copy link
Member

neolit123 commented Aug 26, 2019

so i did some investigation here and there are couple of options.

let the kubelet manage kubelet.conf

this means that:

  • we need to write the client certificates in boostrap-kubelet.conf
  • let the kubelet write kubelet.conf post-CSR, which may or may not point to rotatable files, depending on --rotate-certificates
  • wait for the control-plane to boot up.
  • manually create a Node object populated with node-role labels, NoSchedule taint and CRISocket, because the Node object of this node will not available yet and cannot be "patched". later this same object will be populated properly by the kubelet.

pros:

  • we don't have to track if the user decided to disable kubelet client cert rotation

cons:

  • disruptive to the existing kubeadm workflow (although i'm starting to think this should have been the original workflow in kubeadm).

let the kubeadm manage kubelet.conf (current behavior)

  • let kubeadm write kubelet.conf
  • wait for the kubelet to write certificates in /var/lib/kubelet/pki
  • if kubelet client cert rotation is enabled:
    updated kubelet.conf to point to /var/lib/kubelet/pki/kubelet-client-current.pem
  • if kubelet client cert rotation is disabled continue using the embedded client certificates

pros:

  • preserves the existing kubeadm workflow

cons:

  • kubeadm has to guess if the kubelet is configured without client cert rotation, possibly based on contents in /var/lib/kubelet/pki/

@neolit123 neolit123 added the kind/design Categorizes issue or PR as related to design. label Aug 26, 2019
@neolit123 neolit123 self-assigned this Aug 26, 2019
@neolit123 neolit123 added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 26, 2019
@fabriziopandini
Copy link
Member

@neolit123 feel free to work on this issue
Some initial thought:

  1. AFAIK the problem exists only on the first bootstrap node because of both join and join --control plane should layout things as expected
  2. On the first bootstrap node, we can't leverage on CSR (or am I wrong?)
  3. I think that the easiest approach is to Pivot the first node to use the /var/lib/kubelet/pki/ immediately after kubelet start
  4. I think we should pivot no matter if certificate rotation is enabled or not, so we are consistent in how kubelet is configured across all nodes

@lnovara
Copy link
Author

lnovara commented Aug 27, 2019

@fabriziopandini if we pivot no matter if certificate rotation is enabled or not, we need to dig into the kubelet configuration to understand if client cert rotation is enabled or not when a cert rotation is needed.

What's kubelet behaviour when TLS bootstrap is enabled but client cert rotation is not? Is it even possibile to do TLS bootstrap without enabling client cert rotation?

I think we should be consistent with the kubelet behaviour in this case.

Just my 2 cents.

@neolit123
Copy link
Member

neolit123 commented Aug 27, 2019

@fabriziopandini

AFAIK the problem exists only on the first bootstrap node because of both join and join --control plane should layout things as expected

true.

On the first bootstrap node, we can't leverage on CSR (or am I wrong?)

we can - i've tried it, but it will result in delay of the automatic creation of the Node object for this node. can be mitigated if the object is created by kubeadm.

I think that the easiest approach is to Pivot the first node to use the /var/lib/kubelet/pki/ immediately after kubelet start

from my experiments yesterday this is not possible. the kubelet needs valid client certs to authenticate with the api-server first, and only then we can use /var/lib/kubelet/pki/kubelet-client-current.pem
or do you mean something else?

EDIT: actually reading this again, yes this is what we want to do but then there is a problem:
#1753 (comment)

I think we should pivot no matter if certificate rotation is enabled or not, so we are consistent in how kubelet is configured across all nodes

/var/lib/kubelet/pki/kubelet-client-current.pem will be absent if cert rotation is disabled, so we can only pivot based on the kubelet --rotate-certificates or KubeletConfiguration equivalent.

@neolit123
Copy link
Member

@lnovara

What's kubelet behaviour when TLS bootstrap is enabled but client cert rotation is not? Is it even possibile to do TLS bootstrap without enabling client cert rotation?

if the kubelet client cert rotation is disabled TLS boostrap still works.

@neolit123
Copy link
Member

neolit123 commented Aug 27, 2019

if we look at the approach let the kubeadm manage kubelet.conf the biggest blocker here is external kubelet.conf vs cert rotation.

if the user feeds their external kubelet.conf before kubeadm init, kubeadm does not touch the file during the kubeconfig phase.

and given we can only update this kubelet.conf file after the rotation PEM files are on disk, and given kubeadm phases are granular, we don't have a way to know at that point if the kubelet.conf file was external, unless we:

  • add more state on disk or in the cluster.
  • dissect the kubelet.conf trying to determine if kubeadm generated it and not the user.

both options seem not ideal.

@neolit123 neolit123 modified the milestones: v1.16, v1.17 Aug 29, 2019
@neolit123
Copy link
Member

had a discussion about this with @fabriziopandini
we are going to try to go with the first option from here:
#1753 (comment)

but first discuss it during a kubeadm meeting with the wider group.

@neolit123
Copy link
Member

a PR is up for this:
kubernetes/kubernetes#83339

@neolit123 neolit123 added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Oct 5, 2019
@neolit123 neolit123 changed the title kubeadm should leverage kubelet automatic client cert rotation on master nodes kubeadm should leverage kubelet automatic client cert rotation on nodes created with kubeadm init Oct 7, 2019
@neolit123 neolit123 added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Oct 7, 2019
@neolit123 neolit123 reopened this Oct 16, 2019
@neolit123
Copy link
Member

neolit123 commented Oct 16, 2019

we reverted the PR that merged for this with option1 from here:
#1753 (comment)
kubernetes/kubernetes#84012

i will send a new PR when i have the time with option2.

@jfbai
Copy link

jfbai commented Oct 17, 2019

we reverted the PR that merged for this with option1 from here:
#1753 (comment)
kubernetes/kubernetes#84012

i will send a new PR when i have the time with option2.

@neolit123 May I take this task?

@neolit123
Copy link
Member

neolit123 commented Oct 17, 2019

@jfbai this one is tricky and needs a lot of discussion.
please take a look at some of our help-wanted tickets.

@jfbai
Copy link

jfbai commented Oct 17, 2019

@jfbai this one is tricky and needs a lot of discussion.
please take a look at some of our help-wanted tickets.

IC, thanks a lot.

@jenting
Copy link

jenting commented Sep 21, 2020

@neolit123, is it possible that implements auto-update the kubelet.conf to points to the rotated kubelet client certificates, not the embedded base64 encoded certificates during kubeadm upgrade?

@neolit123
Copy link
Member

neolit123 commented Sep 21, 2020 via email

@neolit123
Copy link
Member

neolit123 commented Sep 21, 2020 via email

@jenting
Copy link

jenting commented Sep 22, 2020

Sorry, missed the part about upgrade. I think its a bit tricky to do this on upgrade because the user might have disabled cert rotation. You can still execute the "kubelet-finalize" phase on demand.

I ran through the code and I think kubeadm want to decouple to kubelet so kubeadm guess the client cert rotation enabled or not by checking the kubelet-client-current.pem exists, but not checking the kubetlet config.yaml.

The command kubeadm init phase kubelet-finalize experimental-cert-rotation is useful to update local kubelet.conf, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
5 participants