Skip to content

Commit e3e8b3f

Browse files
author
Kubernetes Submit Queue
authored
Merge pull request kubernetes#47438 from luxas/kubeadm_fix_v18alpha0_version
Automatic merge from submit-queue (batch tested with PRs 47523, 47438, 47550, 47450, 47612) kubeadm: Fix subtle versioning ordering issue with v1.8.0-alpha.0 **What this PR does / why we need it**: `--kubernetes-version latest` is broken since it evals to `v1.8.0-alpha.0` which actually is `v1.7.0-beta.0`, so kubeadm enables features that don't exist **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**: ```release-note NONE ``` @kubernetes/sig-cluster-lifecycle-pr-reviews
2 parents 6d6611c + 473bb9c commit e3e8b3f

File tree

8 files changed

+62
-4
lines changed

8 files changed

+62
-4
lines changed

cmd/kubeadm/app/cmd/defaults.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func setInitDynamicDefaults(cfg *kubeadmapi.MasterConfiguration) error {
8181
}
8282

8383
func defaultAuthorizationModes(authzModes []string, k8sVersion *version.Version) []string {
84-
if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) {
84+
if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
8585
strset := sets.NewString(authzModes...)
8686
if !strset.Has(authzmodes.ModeNode) {
8787
return append([]string{authzmodes.ModeNode}, authzModes...)

cmd/kubeadm/app/master/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ go_library(
2121
"//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
2222
"//cmd/kubeadm/app/constants:go_default_library",
2323
"//cmd/kubeadm/app/images:go_default_library",
24+
"//cmd/kubeadm/app/util:go_default_library",
2425
"//cmd/kubeadm/app/util/kubeconfig:go_default_library",
2526
"//pkg/bootstrap/api:go_default_library",
2627
"//pkg/kubeapiserver/authorizer/modes:go_default_library",

cmd/kubeadm/app/master/manifests.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
3535
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
3636
"k8s.io/kubernetes/cmd/kubeadm/app/images"
37+
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
3738
bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
3839
authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
3940
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -355,7 +356,7 @@ func getAPIServerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted bool, k
355356
defaultArguments["proxy-client-cert-file"] = filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyClientCertName)
356357
defaultArguments["proxy-client-key-file"] = filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyClientKeyName)
357358
}
358-
if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) {
359+
if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
359360
// enable the NodeRestriction admission plugin
360361
defaultArguments["admission-control"] = defaultv17AdmissionControl
361362
}

cmd/kubeadm/app/phases/apiconfig/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
tags = ["automanaged"],
1818
deps = [
1919
"//cmd/kubeadm/app/constants:go_default_library",
20+
"//cmd/kubeadm/app/util:go_default_library",
2021
"//pkg/bootstrap/api:go_default_library",
2122
"//pkg/kubelet/apis:go_default_library",
2223
"//pkg/util/node:go_default_library",

cmd/kubeadm/app/phases/apiconfig/clusterroles.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/client-go/pkg/api/v1"
2626
rbac "k8s.io/client-go/pkg/apis/rbac/v1beta1"
2727
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
28+
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
2829
bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
2930
"k8s.io/kubernetes/pkg/util/version"
3031
)
@@ -254,7 +255,7 @@ func deletePermissiveNodesBindingWhenUsingNodeAuthorization(clientset *clientset
254255

255256
// If the server version is higher than the Node Authorizer's minimum, try to delete the Group=system:nodes->ClusterRole=system:node binding
256257
// which is much more permissive than the Node Authorizer
257-
if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) {
258+
if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
258259

259260
nodesRoleBinding, err := clientset.RbacV1beta1().ClusterRoleBindings().Get(kubeadmconstants.NodesClusterRoleBinding, metav1.GetOptions{})
260261
if err != nil {

cmd/kubeadm/app/util/BUILD

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ go_library(
1717
],
1818
tags = ["automanaged"],
1919
deps = [
20+
"//cmd/kubeadm/app/constants:go_default_library",
2021
"//cmd/kubeadm/app/preflight:go_default_library",
22+
"//pkg/util/version:go_default_library",
2123
"//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
2224
],
2325
)
@@ -31,7 +33,10 @@ go_test(
3133
],
3234
library = ":go_default_library",
3335
tags = ["automanaged"],
34-
deps = ["//cmd/kubeadm/app/preflight:go_default_library"],
36+
deps = [
37+
"//cmd/kubeadm/app/preflight:go_default_library",
38+
"//pkg/util/version:go_default_library",
39+
],
3540
)
3641

3742
filegroup(

cmd/kubeadm/app/util/version.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import (
2222
"net/http"
2323
"regexp"
2424
"strings"
25+
26+
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
27+
"k8s.io/kubernetes/pkg/util/version"
2528
)
2629

2730
var (
@@ -69,3 +72,12 @@ func KubernetesReleaseVersion(version string) (string, error) {
6972
}
7073
return "", fmt.Errorf("version %q doesn't match patterns for neither semantic version nor labels (stable, latest, ...)", version)
7174
}
75+
76+
// IsNodeAuthorizerSupported returns true if the provided version of kubernetes is able to use the Node Authorizer feature.
77+
// There is a really nasty problem with the branching here and the timing of this feature implementation. When the release-1.7 branch was
78+
// cut, two new tags were made: v1.7.0-beta.0 and v1.8.0-alpha.0. The Node Authorizer feature merged _after those cuts_. This means the minimum
79+
// version we have to use is v1.7.0-beta.1. BUT since v1.8.0-alpha.0 sorts higher than v1.7.0-beta.1 (the actual version gate), we have to manually
80+
// exclude v1.8.0-alpha.0 from this condition. v1.8.0-alpha.1 will indeed contain the patch.
81+
func IsNodeAuthorizerSupported(k8sVersion *version.Version) bool {
82+
return k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) && k8sVersion.String() != "1.8.0-alpha.0"
83+
}

cmd/kubeadm/app/util/version_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
"path"
2323
"strings"
2424
"testing"
25+
26+
"k8s.io/kubernetes/pkg/util/version"
2527
)
2628

2729
func TestEmptyVersion(t *testing.T) {
@@ -120,3 +122,38 @@ func TestVersionFromNetwork(t *testing.T) {
120122
}
121123
}
122124
}
125+
126+
func TestIsNodeAuthorizerSupported(t *testing.T) {
127+
versionsSupported := map[string]bool{
128+
"v1.6.0": false,
129+
"v1.6.9": false,
130+
"v1.7.0-alpha.1": false,
131+
"v1.7.0-alpha.2": false,
132+
"v1.7.0-alpha.3": false,
133+
"v1.7.0-alpha.4": false,
134+
"v1.7.0-beta.0": false,
135+
"v1.7.0-beta.1": true, // BREAKPOINT!
136+
"v1.7.0-beta.2": true,
137+
"v1.7.0-rc.0": true,
138+
"v1.7.0": true,
139+
"v1.7.3": true,
140+
"v1.8.0-alpha.0": false, // EXCEPTION!
141+
"v1.8.0-alpha.1": true,
142+
"v1.8.0-alpha.2": true,
143+
"v1.8.0-beta.0": true,
144+
"v1.8.0-beta.1": true,
145+
"v1.8.0-rc.0": true,
146+
"v1.8.0": true,
147+
"v1.8.6": true,
148+
}
149+
for ver, expected := range versionsSupported {
150+
151+
parsedVersion, err := version.ParseSemantic(ver)
152+
if err != nil {
153+
t.Fatalf("version %s must parse", ver)
154+
}
155+
if actual := IsNodeAuthorizerSupported(parsedVersion); actual != expected {
156+
t.Errorf("IsNodeAuthorizerSupported: unexpected result for version %s, expected %t but got %t", ver, expected, actual)
157+
}
158+
}
159+
}

0 commit comments

Comments
 (0)