kubeadm: change the default bootstrap token TTL to 24 hours #48783
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @mattmoyer. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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. I understand the commands that are listed here. |
k8s-ci-robot
added
the
needs-ok-to-test
k8s-github-robot
added
size/XS
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
resouer
reviewed
Jul 12, 2017
| // Default behaviour is "never expire" == 0 | ||
| DefaultTokenDuration = 0 | ||
| // Default behaviour is 2 hours | ||
| DefaultTokenDuration = 2 * time.Hour |
There was a problem hiding this comment.
As you said this have possibility to break existing users, @luxas Do we have any rollout plan to avoid this?
|
@mattmoyer Please cc me on kubeadm PRs so it's easier for me to notice ;) @jbeda @mattmoyer Could we make this a day (24 hrs) instead? |
mattmoyer
force-pushed
the
change-kubeadm-default-token-ttl
branch
from
7b5f534
to
59f9841
Compare
mattmoyer
changed the title
mattmoyer
added a commit
to mattmoyer/kubernetes
that referenced
this pull request
Jul 14, 2017
This adds a warning to `kubeadm init` and `kubeadm token create` if they are run without the `--token-ttl` / `--ttl` flags. In 1.7 and before, the tokens generated by these commands defaulted to an infinite TTL (no expiration) in 1.8, they will generate a token with a 24 hour TTL. The actual default change is in kubernetes#48783. This change is separate so we can cherry pick the warning into the release-1.7 branch.
luxas
added
release-note-action-required
|
/lgtm Thanks @mattmoyer! |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: luxas, mattmoyer Associated issue: 343 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
Automatic merge from submit-queue (batch tested with PRs 48572, 48838, 48931, 48783, 47090) |
hh
pushed a commit
to ii/kubernetes
that referenced
this pull request
Jul 14, 2017
…t-token-ttl-warning Automatic merge from submit-queue (batch tested with PRs 48572, 48838, 48931, 48783, 47090) kubeadm: add a warning about the default token TTL changing in 1.8 **What this PR does / why we need it**: This adds a warning to `kubeadm init` and `kubeadm token create` if they are run without the `--token-ttl` / `--ttl` flags. In 1.7 and before, the tokens generated by these commands defaulted to an infinite TTL (no expiration) in 1.8, they will generate a token with a 24 hour TTL. The actual default change is in kubernetes#48783. This change is separate so we can cherry pick the warning into the `release-1.7` branch. **Which issue this PR fixes**: ref kubernetes/kubeadm#343 **Special notes for your reviewer**: This change is blocked on kubernetes/kubeadm#343. These warnings should probably be removed in the 1.9 cycle. **Release note**: ```release-note Add a runtime warning about the kubeadm default token TTL changes in 1.8. ``` /assign @luxas
mattmoyer
added a commit
to mattmoyer/kubernetes
that referenced
this pull request
Jul 14, 2017
This adds a warning to `kubeadm init` and `kubeadm token create` if they are run without the `--token-ttl` / `--ttl` flags. In 1.7 and before, the tokens generated by these commands defaulted to an infinite TTL (no expiration) in 1.8, they will generate a token with a 24 hour TTL. The actual default change is in kubernetes#48783. This change is separate so we can cherry pick the warning into the release-1.7 branch.
This was referenced Oct 26, 2017
mattmoyer
added a commit
to mattmoyer/kubernetes
that referenced
this pull request
Oct 26, 2017
This was broken because the API machinery defaulting mechanism couldn't differentiate between an unset value (which should default to 24 hours) and a value explicitly set to 0 (which should mean infinite). The fix is to change `TokenTTL` from a `metav1.Duration` to `*metav1.Duration` so that `nil` can represent the unspecified value. This bug was introduced in kubernetes#48783.
mattmoyer
added a commit
to mattmoyer/kubernetes
that referenced
this pull request
Oct 26, 2017
This was broken because the API machinery defaulting mechanism couldn't differentiate between an unset value (which should default to 24 hours) and a value explicitly set to 0 (which should mean infinite). The fix is to change `TokenTTL` from a `metav1.Duration` to `*metav1.Duration` so that `nil` can represent the unspecified value. This bug was introduced in kubernetes#48783.
mattmoyer
added a commit
to mattmoyer/kubernetes
that referenced
this pull request
Nov 7, 2017
This was broken because the API machinery defaulting mechanism couldn't differentiate between an unset value (which should default to 24 hours) and a value explicitly set to 0 (which should mean infinite). The fix is to change `TokenTTL` from a `metav1.Duration` to `*metav1.Duration` so that `nil` can represent the unspecified value. This bug was introduced in kubernetes#48783.
adnavare
pushed a commit
to adnavare/kubernetes
that referenced
this pull request
Nov 13, 2017
This was broken because the API machinery defaulting mechanism couldn't differentiate between an unset value (which should default to 24 hours) and a value explicitly set to 0 (which should mean infinite). The fix is to change `TokenTTL` from a `metav1.Duration` to `*metav1.Duration` so that `nil` can represent the unspecified value. This bug was introduced in kubernetes#48783.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR changes the TTL for the default bootstrap token generated by
kubeadm init(without the--token-ttlparameter) andkubeadm token create(without the--ttlflag). Previously, the default TTL was infinite. After this change it is 24 hours.The reasoning for 2 hours as a default is that it's 1) long enough that someone manually using kubeadm (copy-pasting) shouldn't have any issues and 2) short enough that if something is going to break, it should break while the user/admin is still paying attention to the cluster. I'm open to bikeshedding about the exact value, 2 hours is a bit of a strawman.Edit: updated this to 24 hours instead of 2 hours.
This is a breaking change if you rely on infinite TTL tokens (e.g., if you had an ASG group of worker nodes). The old behavior is easily restored by passing
--token-ttl 0tokubeadm initor the--ttl 0flag tokubeadm token create.Which issue this PR fixes: fixes kubernetes/kubeadm#343
Special notes for your reviewer:
This was discussed earlier today in SIG-cluster-lifecycle
Release note:
cc @jbeda