kubeadm: delete bootstrap-kubelet.conf after TLS bootstrap

[fabriziopandini]

What type of PR is this?
/kind bug

What this PR does / why we need it:
#66482 enabled kubeadm join --discovery-file with a discovery file with embedded credentials. This PR makes sure that kubeadm removes the bootstrap-kubelet.conf after completing the TLS bootstrap process, so copies of the above credentials won't be left around.

Which issue(s) this PR fixes:
Fixes #kubernetes/kubeadm#1689

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

ACTION REQUIRED:
kubeadm now deletes the bootstrap-kubelet.conf file after TLS bootstrap
User relying on bootstrap-kubelet.conf should switch to kubelet.conf that contains node credentials

/sig cluster-lifecycle
/priority important-soon
/assign @timothysc
/assign @neolit123

[neolit123]

i think this is mostly OK, but i recommend adding a ACTION REQUIRED: prefix to the release note.
the boostrap credential can be bound to existing infrastructures, which was probably not a great idea in the first place, but we should visibly notify about such changes.

/lgtm
/hold
for other reviews.

[neolit123]

if we really want to be on the safe side there should be a grace period of say 1 release while kubeadm join supports a knob for this that defaults to false.

EDIT and of course another option is not add this change and leave it to user infrastructure to call rm /etc/kubernetes/kubelet-bootstrap.conf.

[timothysc]

/lgtm
/approve
/hold cancel

Given that this is a security hole I'd be ok with a backport of this.
I don't think an action is required on the release note.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, timothysc

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:

• cmd/kubeadm/OWNERS [fabriziopandini,timothysc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[fabriziopandini]

/test pull-kubernetes-dependencies

[neolit123]

Given that this is a security hole I'd be ok with a backport of this.

i'm -1 given the workaround is very simple:
kubeadm join .... && sudo rm /etc/kubernetes/kubelet-bootstrap.conf

I don't think an action is required on the release note.

i worry about somebody is copying or doing something with the bootstrap credentials.
with kubeadm being GA we should avoid such changes without notification.

[fabriziopandini]

AFAIK there is no risk in backporting this fix

• if users are using token discovery, then bootstrap-kubelet can’t be used because it expires
• if users are using file discovery, there is no way to get other credentials in until Fix kubeadm file discovery #80675 merge

But I leave the final call on this on @timothysc / @neolit123

[neolit123]

/retest

[neolit123]

/retest

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

[yagonobre]

LGTM

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

[stealthybox]

the delete should be moved to defer here:
https://github.com/kubernetes/kubernetes/pull/80676/files#diff-3c566a0c1e3b213a34b39503abf4fc11R104

otherwise it's possible for the token to be left behind when the function errors

[neolit123]

yep, +1 to use defer.
let me log a ticket.