Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm: Fix subtle versioning ordering issue with v1.8.0-alpha.0 #47438

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/defaults.go
Expand Up @@ -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...)
Expand Down
1 change: 1 addition & 0 deletions cmd/kubeadm/app/master/BUILD
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion cmd/kubeadm/app/master/manifests.go
Expand Up @@ -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"
Expand Down Expand Up @@ -355,7 +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)
}
if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) {
if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {
// enable the NodeRestriction admission plugin
defaultArguments["admission-control"] = defaultv17AdmissionControl
}
Expand Down
1 change: 1 addition & 0 deletions cmd/kubeadm/app/phases/apiconfig/BUILD
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion cmd/kubeadm/app/phases/apiconfig/clusterroles.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -254,7 +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
if k8sVersion.AtLeast(kubeadmconstants.MinimumNodeAuthorizerVersion) {
if kubeadmutil.IsNodeAuthorizerSupported(k8sVersion) {

nodesRoleBinding, err := clientset.RbacV1beta1().ClusterRoleBindings().Get(kubeadmconstants.NodesClusterRoleBinding, metav1.GetOptions{})
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion cmd/kubeadm/app/util/BUILD
Expand Up @@ -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",
],
)
Expand All @@ -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(
Expand Down
12 changes: 12 additions & 0 deletions cmd/kubeadm/app/util/version.go
Expand Up @@ -22,6 +22,9 @@ import (
"net/http"
"regexp"
"strings"

kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/pkg/util/version"
)

var (
Expand Down Expand Up @@ -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"
}
37 changes: 37 additions & 0 deletions cmd/kubeadm/app/util/version_test.go
Expand Up @@ -22,6 +22,8 @@ import (
"path"
"strings"
"testing"

"k8s.io/kubernetes/pkg/util/version"
)

func TestEmptyVersion(t *testing.T) {
Expand Down Expand Up @@ -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)
}
}
}