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

✨ KCP regenerates kubeconfigs before client certs expire #3140

Merged
merged 7 commits into from
Jun 11, 2020
Merged

✨ KCP regenerates kubeconfigs before client certs expire #3140

merged 7 commits into from
Jun 11, 2020

Conversation

benmoss
Copy link

@benmoss benmoss commented Jun 4, 2020

What this PR does / why we need it:
KCP will now automatically regenerate the Kubeconfig secret during cluster reconciliation. It will do so when the client certificate in it reaches it's half-life of 180 days.

Which issue(s) this PR fixes:
Fixes #3113

Side note: I'd like to propose deprecating kubeconfig.New as part of this, any thoughts? It's kind of confusing now where New is essentially a private method that only gets called by generateKubeconfig. Previously the only usage was from CreateSecretWithOwner.

/assign @nader-ziada @sedefsavas

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 4, 2020
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 4, 2020
@benmoss benmoss force-pushed the kcp-regenerate-kubeconfig branch from 1315126 to 85256b8 Compare June 4, 2020 13:55
vincepri
vincepri reviewed Jun 4, 2020
View reviewed changes
controlplane/kubeadm/controllers/helpers.go Show resolved Hide resolved
util/certs/consts.go Outdated Show resolved Hide resolved
@benmoss benmoss force-pushed the kcp-regenerate-kubeconfig branch 2 times, most recently from 9cd076f to 4b8eb85 Compare June 4, 2020 15:02
@benmoss benmoss force-pushed the kcp-regenerate-kubeconfig branch from 4b8eb85 to f9488ac Compare June 4, 2020 15:02
detiber
detiber reviewed Jun 4, 2020
View reviewed changes
controlplane/kubeadm/controllers/helpers.go Outdated Show resolved Hide resolved
controlplane/kubeadm/controllers/helpers.go Outdated Show resolved Hide resolved
controlplane/kubeadm/controllers/helpers.go Show resolved Hide resolved
util/kubeconfig/kubeconfig.go Outdated Show resolved Hide resolved
@benmoss benmoss force-pushed the kcp-regenerate-kubeconfig branch from 03f0bbb to 5e1cdf8 Compare June 4, 2020 18:18
@vincepri
Copy link
Member

vincepri commented Jun 5, 2020

Does this apply to secrets generated by CABPK when not using KCP?

@benmoss
Copy link
Author

benmoss commented Jun 5, 2020 •
edited
Loading

Does this apply to secrets generated by CABPK when not using KCP?

No, it only applies to KCP. Do we want to support this for non-KCP control planes? It'd just involve duplicating this code to the cluster controller AFAIK.

@vincepri
Copy link
Member

vincepri commented Jun 5, 2020

@detiber what do you think?

@detiber
Copy link
Member

detiber commented Jun 5, 2020

@vincepri spoke with @benmoss earlier about whether this should also be extended to non-KCP managed kubeconfigs, and I don't think we should. Since the non-KCP (or really non-control plane provider) Machine-based control plane support is in place to support backwards compatibility only.

@vincepri
Copy link
Member

vincepri commented Jun 5, 2020

SGTM

@benmoss
Copy link
Author

benmoss commented Jun 5, 2020

It does mean that this is another feature of the very wide and loosely defined set of expectations for a control plane provider to implement, but I think that's true of many features of KCP now

@benmoss benmoss force-pushed the kcp-regenerate-kubeconfig branch from ba24882 to 518efd9 Compare June 8, 2020 13:38
@detiber
Copy link
Member

detiber commented Jun 8, 2020

@benmoss should probably add some docs around the new behavior (and lack of support for the non-KCP case), otherwise lgtm

@detiber
Update docs/book/src/developer/architecture/controllers/control-plane.md
d70a45b 
Co-authored-by: Jason DeTiberus <detiberusj@vmware.com>
@vincepri
Copy link
Member

This PR looks ready to go, @detiber wdyt?

@vincepri
Copy link
Member

/milestone v0.3.7

@k8s-ci-robot k8s-ci-robot added this to the v0.3.7 milestone Jun 11, 2020
@detiber
Copy link
Member

detiber commented Jun 11, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 11, 2020
vincepri
vincepri approved these changes Jun 11, 2020
View reviewed changes
Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benmoss, vincepri

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2020
@k8s-ci-robot k8s-ci-robot merged commit 714b0a9 into kubernetes-sigs:master Jun 11, 2020
@benmoss benmoss deleted the kcp-regenerate-kubeconfig branch June 11, 2020 18:05
@fabriziopandini fabriziopandini mentioned this pull request Dec 11, 2020
Closed
@rsmitty rsmitty mentioned this pull request Jun 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123 neolit123 left review comments

@detiber detiber detiber left review comments

@sedefsavas sedefsavas sedefsavas left review comments

@vincepri vincepri vincepri approved these changes

@CecileRobertMichon CecileRobertMichon Awaiting requested review from CecileRobertMichon

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees

@detiber detiber

@nader-ziada nader-ziada

@sedefsavas sedefsavas

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v0.3.7
Development

Successfully merging this pull request may close these issues.

Kubeconfig secret is not rotated on cluster upgrade
7 participants
@benmoss @vincepri @detiber @k8s-ci-robot @neolit123 @sedefsavas @nader-ziada