kubeadm: Fix subtle versioning ordering issue

Author: luxas

cmd/kubeadm/app/cmd/defaults.go

@@ -81,7 +81,7 @@ func setInitDynamicDefaults(cfg *kubeadmapi.MasterConfiguration) error {
 }
 
 func defaultAuthorizationModes(authzModes []string, k8sVersion *version.Version) []string {
-	if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) {
+	if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
 		strset := sets.NewString(authzModes...)
 		if !strset.Has(authzmodes.ModeNode) {
 			return append([]string{authzmodes.ModeNode}, authzModes...)

cmd/kubeadm/app/master/BUILD

@@ -21,6 +21,7 @@ go_library(
         "//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/images:go_default_library",
+        "//cmd/kubeadm/app/util:go_default_library",
         "//cmd/kubeadm/app/util/kubeconfig:go_default_library",
         "//pkg/bootstrap/api:go_default_library",
         "//pkg/kubeapiserver/authorizer/modes:go_default_library",

cmd/kubeadm/app/master/manifests.go

@@ -34,6 +34,7 @@ import (
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/images"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
 	authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -355,9 +356,7 @@ func getAPIServerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted bool, k
 		defaultArguments["proxy-client-cert-file"] = filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyClientCertName)
 		defaultArguments["proxy-client-key-file"] = filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyClientKeyName)
 	}
-	// TODO: That we have to exclude v1.8.0-alpha.0 here is based on the way kubernetes branches work
-	// v1.7.0-beta.0 == v1.8.0.alpha.0 but they are sorted/compared differently
-	if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) && cfg.KubernetesVersion != "v1.8.0-alpha.0" {
+	if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
 		// enable the NodeRestriction admission plugin
 		defaultArguments["admission-control"] = defaultv17AdmissionControl
 	}

cmd/kubeadm/app/phases/apiconfig/BUILD

@@ -17,6 +17,7 @@ go_library(
     tags = ["automanaged"],
     deps = [
         "//cmd/kubeadm/app/constants:go_default_library",
+        "//cmd/kubeadm/app/util:go_default_library",
         "//pkg/bootstrap/api:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/util/node:go_default_library",

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

@@ -25,6 +25,7 @@ import (
 	"k8s.io/client-go/pkg/api/v1"
 	rbac "k8s.io/client-go/pkg/apis/rbac/v1beta1"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
 	"k8s.io/kubernetes/pkg/util/version"
 )
@@ -254,9 +255,7 @@ func deletePermissiveNodesBindingWhenUsingNodeAuthorization(clientset *clientset
 
 	// If the server version is higher than the Node Authorizer's minimum, try to delete the Group=system:nodes->ClusterRole=system:node binding
 	// which is much more permissive than the Node Authorizer
-	// TODO: That we have to exclude v1.8.0-alpha.0 here is based on the way kubernetes branches work
-	// v1.7.0-beta.0 == v1.8.0.alpha.0 but they are sorted/compared differently
-	if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) && cfg.KubernetesVersion != "v1.8.0-alpha.0" {
+	if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
 
 		nodesRoleBinding, err := clientset.RbacV1beta1().ClusterRoleBindings().Get(kubeadmconstants.NodesClusterRoleBinding, metav1.GetOptions{})
 		if err != nil {

cmd/kubeadm/app/util/BUILD

@@ -17,7 +17,9 @@ go_library(
     ],
     tags = ["automanaged"],
     deps = [
+        "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/preflight:go_default_library",
+        "//pkg/util/version:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
     ],
 )
@@ -31,7 +33,10 @@ go_test(
     ],
     library = ":go_default_library",
     tags = ["automanaged"],
-    deps = ["//cmd/kubeadm/app/preflight:go_default_library"],
+    deps = [
+        "//cmd/kubeadm/app/preflight:go_default_library",
+        "//pkg/util/version:go_default_library",
+    ],
 )
 
 filegroup(

cmd/kubeadm/app/util/version.go

@@ -22,6 +22,9 @@ import (
 	"net/http"
 	"regexp"
 	"strings"
+
+	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	"k8s.io/kubernetes/pkg/util/version"
 )
 
 var (
@@ -69,3 +72,12 @@ func KubernetesReleaseVersion(version string) (string, error) {
 	}
 	return "", fmt.Errorf("version %q doesn't match patterns for neither semantic version nor labels (stable, latest, ...)", version)
 }
+
+// IsNodeAuthorizerSupported returns true if the provided version of kubernetes is able to use the Node Authorizer feature.
+// There is a really nasty problem with the branching here and the timing of this feature implementation. When the release-1.7 branch was
+// 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
+// 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
+// exclude v1.8.0-alpha.0 from this condition. v1.8.0-alpha.1 will indeed contain the patch.
+func IsNodeAuthorizerSupported(k8sVersion *version.Version) bool {
+	return k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) && k8sVersion.String() != "1.8.0-alpha.0"
+}

cmd/kubeadm/app/util/version_test.go

@@ -22,6 +22,8 @@ import (
 	"path"
 	"strings"
 	"testing"
+
+	"k8s.io/kubernetes/pkg/util/version"
 )
 
 func TestEmptyVersion(t *testing.T) {
@@ -120,3 +122,38 @@ func TestVersionFromNetwork(t *testing.T) {
 		}
 	}
 }
+
+func TestIsNodeAuthorizerSupported(t *testing.T) {
+	versionsSupported := map[string]bool{
+		"v1.6.0":         false,
+		"v1.6.9":         false,
+		"v1.7.0-alpha.1": false,
+		"v1.7.0-alpha.2": false,
+		"v1.7.0-alpha.3": false,
+		"v1.7.0-alpha.4": false,
+		"v1.7.0-beta.0":  false,
+		"v1.7.0-beta.1":  true, // BREAKPOINT!
+		"v1.7.0-beta.2":  true,
+		"v1.7.0-rc.0":    true,
+		"v1.7.0":         true,
+		"v1.7.3":         true,
+		"v1.8.0-alpha.0": false, // EXCEPTION!
+		"v1.8.0-alpha.1": true,
+		"v1.8.0-alpha.2": true,
+		"v1.8.0-beta.0":  true,
+		"v1.8.0-beta.1":  true,
+		"v1.8.0-rc.0":    true,
+		"v1.8.0":         true,
+		"v1.8.6":         true,
+	}
+	for ver, expected := range versionsSupported {
+
+		parsedVersion, err := version.ParseSemantic(ver)
+		if err != nil {
+			t.Fatalf("version %s must parse", ver)
+		}
+		if actual := IsNodeAuthorizerSupported(parsedVersion); actual != expected {
+			t.Errorf("IsNodeAuthorizerSupported: unexpected result for version %s, expected %t but got %t", ver, expected, actual)
+		}
+	}
+}