Skip to content
This repository has been archived by the owner on Jan 4, 2022. It is now read-only.

kubeadm join: fix TLS on Kubernetes >=1.9 #222

Merged
merged 1 commit into from
Nov 26, 2017
Merged

Conversation

alban
Copy link
Member

@alban alban commented Nov 25, 2017

kubeadm got a new option in version 1.8: --discovery-token-unsafe-skip-ca-verification
See: kubernetes/kubernetes#49520

Since version 1.9, it became mandatory to either use it to skip verification, or to use --discovery-token-ca-cert-hash=...
See: kubernetes/kubernetes#55468

Since kube-spawn is a developer tool used on one physical machine, use the former.

@alban alban requested review from nhlfr and dongsupark November 25, 2017 17:23
@alban alban force-pushed the alban/fix-tls-k8s-1.9 branch from 1d98f4a to 7221e73 Compare November 25, 2017 17:25
@alban alban mentioned this pull request Nov 26, 2017
2 tasks
@blixtra
Copy link
Member

blixtra commented Nov 26, 2017

Can you provide a description?

kubeadm got a new option in version 1.8:
--discovery-token-unsafe-skip-ca-verification
See: kubernetes/kubernetes#49520

Since version 1.9, it became mandatory to either use it to skip
verification, or to use --discovery-token-ca-cert-hash=...
See: kubernetes/kubernetes#55468

Since kube-spawn is a developer tool used on one physical machine, use
the former.
@alban alban force-pushed the alban/fix-tls-k8s-1.9 branch from 7221e73 to 0edaa40 Compare November 26, 2017 16:38
@alban
Copy link
Member Author

alban commented Nov 26, 2017

@blixtra updated.

Copy link
Member

@dongsupark dongsupark left a comment

Choose a reason for hiding this comment

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

It's indeed a good catch!
I just tested this PR, and I confirm that it works fine with both k8s 1.8 and 1.9.
Just a minor comment below:

// It is mandatory since Kubernetes 1.9
// See: https://github.com/kubernetes/kubernetes/pull/55468
// Test is !<1.8 instead of >=1.8 in order to handle non-semver version 'latest'
if !utils.CheckVersionConstraint(cfg.KubernetesVersion, "<1.8") {
Copy link
Member

Choose a reason for hiding this comment

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

FYI, similar version checks are done in pkg/config/defaults.go, either if cfg.DevCluster || utils.CheckVersionConstraint(cfg.KubernetesVersion, ">=1.8.0") or if !cfg.DevCluster && utils.CheckVersionConstraint(cfg.KubernetesVersion, "<1.8.0"). In this file the check is now used a little differently. Basically this pattern is kind of error-prone, so I think we should be somehow consistent. We could make a helper to simplify it in another PR.

@alban
Copy link
Member Author

alban commented Nov 26, 2017

Thanks for the review & test!

@alban alban merged commit 491e219 into master Nov 26, 2017
@alban alban deleted the alban/fix-tls-k8s-1.9 branch November 29, 2017 14:47
@dongsupark dongsupark added this to the v0.2.1 milestone Dec 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants