Cluster-api: refresh behavior of kubeadmconfig with machinepool
What steps did you take and what happened:
I took a critical read of the kubeadm config controller code with an eye toward machine pool after this slack thread: https://kubernetes.slack.com/archives/CEX9HENG7/p1594849365190200
I'm curious about the expected behavior of token refresh: https://github.com/kubernetes-sigs/cluster-api/blob/19afe6cc4abec48f16211617d7f8a6571f6a2997/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go#L243-L267
MachinePool and MachineDeployment both can scale up and it seems would reuse the existing kubeadmconfig/bootstrap secret/join token. Refresh works well as-is here for the most part, except if scale up occurs after token expiration/deletion by TokenCleaner
Machine maps 1:1 with KubeadmConfig, but MachinePool also maps 1:1. If scale up happens after token deletion, we don't attempt to recreate the token for machine pools -- this feels incorrect. We could simply check if the owner type is a MachinePool and recreate the token -- that seems like a fairly simply fix, if not particularly elegant. I don't see any immediate problems with that, but figured I'd open an issue for discussion.
cc @rudoi @CecileRobertMichon @devigned
What did you expect to happen:
Bootstrap data should be fresh for scale up on machine pools, even after longs periods where the original join token was deleted.
/kind bug
alexeldeib
All 16 comments
My only concern in the suggested behavior is about maintaining a long-lived token in the cluster, with the potential security implications. I'm wondering if, as mitigation of this problem, it possible to recreate/externd TTL only when scaling up...
fabriziopandini
on 17 Jul 2020
My only concern in the suggested behavior is about maintaining a long-lived token in the cluster, with the potential security implications. I'm wondering if, as mitigation of this problem, it possible to recreate/externd TTL only when scaling up...
One potential gotcha there is that there have been discussions around the potential to defer scaling decisions for a MachinePool to the cloud provider. If we end up supporting that, then it will be hard to know when we would need to recreate/extend the token.
Another potential solution, which might help with both the validity of the token and potentially having a long lived token is that we could do automated regeneration of the token for MachinePool owned KubeadmConfigs.
- For MachinePool owned KubeadmConfigs:
- Generate the initial bootstrap token with a TTL of n
- Return with a requeueAfter time shorter than n (to be able to act before the ttl expires)
- When "should rotate": create a new bootstrap token with a TTL of n
- Do not delete the previous bootstrap token to avoid interrupting any MachinePool actions in flight
- Return with a requeueAfter time shorter than n
- MachinePool provider controller:
- Detect the update to linked KubeadmConfig and update the bootstrap token for the backing configuration
detiber
on 17 Jul 2020
I like the idea to explore token rotation.
The tricky part is that token is embedded in the data secret, so probably we should rotate data secrets as well
fabriziopandini
on 17 Jul 2020
The token rotation solution sounds good to me. From capz perspective, I think that flow would be really smooth.
alexeldeib
on 17 Jul 2020
The tricky part is that token is embedded in the data secret, so probably we should rotate data secrets as well
Agreed -- do we see any issue with this? I don't, I think the token is the main stateful component. But I wondered if other people had different views.
alexeldeib
on 17 Jul 2020
+1 to what Jason suggested, there should be some ways we could do rotation without having breaking changes in behavior or APIs.
One clarification, for MachineDeployments the token is actually created when the MachineSet creates a Machine and clones the BootstrapConfigTemplate associated.
vincepri
on 20 Jul 2020
/milestone v0.3.x
/priority important-longterm
vincepri
on 20 Jul 2020
/milestone v0.4.0
vincepri
on 3 Aug 2020
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
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 stale
fejta-bot
on 1 Nov 2020
/remove-lifecycle stale
fabriziopandini
on 1 Nov 2020
/help
vincepri
on 2 Nov 2020
@mboersma @devigned are you working on this one?
CecileRobertMichon
on 2 Nov 2020
/assign
Yes indeed.
mboersma
on 2 Nov 2020
/remove help
CecileRobertMichon
on 2 Nov 2020
/remove-help
CecileRobertMichon
on 2 Nov 2020
/assign
CecileRobertMichon
on 20 Nov 2020
Related issues
jsturtevant
路
5Comments
detiber
路
5Comments
gyliu513
路
6Comments
vincepri
路
4Comments
dlipovetsky
路
5Comments
Most helpful comment
One potential gotcha there is that there have been discussions around the potential to defer scaling decisions for a MachinePool to the cloud provider. If we end up supporting that, then it will be hard to know when we would need to recreate/extend the token.
Another potential solution, which might help with both the validity of the token and potentially having a long lived token is that we could do automated regeneration of the token for MachinePool owned KubeadmConfigs.