From 2c764c4b7ff691123d316c49458f9c007c0b74cb Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Fri, 6 Jul 2018 14:06:24 +0300 Subject: [PATCH] fix CRI socket validatioin CRI socket doesn't have to be an absolute path, it should be an url. However, attempt to use it as an url in 'kubeadm init' command line causes this validation error: $ sudo ./kubeadm init --cri-socket unix:///var/run/crio/crio.sock nodeRegistration.criSocket: Invalid value: "unix:///var/run/crio/crio.sock": path is not absolute Fixed by adding ValidateSocket function and using it in the ValidateNodeRegistrationOptions check instead of ValidateAbsolutePath. --- .../apis/kubeadm/v1alpha3/defaults_unix.go | 3 +++ .../apis/kubeadm/v1alpha3/defaults_windows.go | 3 +++ cmd/kubeadm/app/apis/kubeadm/validation/BUILD | 2 ++ .../app/apis/kubeadm/validation/validation.go | 23 ++++++++++++++++++- .../kubeadm/validation/validation_test.go | 18 ++++++++------- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_unix.go b/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_unix.go index aad0f9a772e..8aab5c6ed9d 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_unix.go +++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_unix.go @@ -20,3 +20,6 @@ package v1alpha3 // DefaultCACertPath defines default location of CA certificate on Linux const DefaultCACertPath = "/etc/kubernetes/pki/ca.crt" + +// DefaultSocketUrlScheme defines default socket url prefix +const DefaultUrlScheme = "unix" diff --git a/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_windows.go b/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_windows.go index 4449239aa40..76562023489 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_windows.go +++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha3/defaults_windows.go @@ -20,3 +20,6 @@ package v1alpha3 // DefaultCACertPath defines default location of CA certificate on Windows const DefaultCACertPath = "C:/etc/kubernetes/pki/ca.crt" + +// DefaultSocketUrlScheme defines default socket url prefix +const DefaultUrlScheme = "tcp" diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/BUILD b/cmd/kubeadm/app/apis/kubeadm/validation/BUILD index bd6c109e183..1fd221cb0ed 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/BUILD +++ b/cmd/kubeadm/app/apis/kubeadm/validation/BUILD @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", + "//cmd/kubeadm/app/apis/kubeadm/v1alpha3:go_default_library", "//cmd/kubeadm/app/componentconfigs:go_default_library", "//cmd/kubeadm/app/constants:go_default_library", "//cmd/kubeadm/app/features:go_default_library", @@ -28,6 +29,7 @@ go_test( embed = [":go_default_library"], deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", + "//cmd/kubeadm/app/apis/kubeadm/v1alpha3:go_default_library", "//pkg/proxy/apis/kubeproxyconfig:go_default_library", "//pkg/util/pointer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go index 179b3f57e69..d83019ca50b 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go @@ -32,6 +32,7 @@ import ( bootstrapapi "k8s.io/client-go/tools/bootstrap/token/api" bootstraputil "k8s.io/client-go/tools/bootstrap/token/util" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + kubeadmapiv1alpha3 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha3" "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/features" @@ -75,7 +76,7 @@ func ValidateNodeRegistrationOptions(nro *kubeadm.NodeRegistrationOptions, fldPa } else { allErrs = append(allErrs, apivalidation.ValidateDNS1123Subdomain(nro.Name, field.NewPath("name"))...) } - allErrs = append(allErrs, ValidateAbsolutePath(nro.CRISocket, fldPath.Child("criSocket"))...) + allErrs = append(allErrs, ValidateSocketPath(nro.CRISocket, fldPath.Child("criSocket"))...) // TODO: Maybe validate .Taints as well in the future using something like validateNodeTaints() in pkg/apis/core/validation return allErrs } @@ -407,3 +408,23 @@ func ValidateIgnorePreflightErrors(ignorePreflightErrors []string, skipPreflight return ignoreErrors, allErrs.ToAggregate() } + +// ValidateSocketPath validates format of socket path or url +func ValidateSocketPath(socket string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + u, err := url.Parse(socket) + if err != nil { + return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("url parsing error: %v", err))) + } + + if u.Scheme == "" { + if !filepath.IsAbs(u.Path) { + return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("path is not absolute: %s", socket))) + } + } else if u.Scheme != kubeadmapiv1alpha3.DefaultUrlScheme { + return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("url scheme %s is not supported", u.Scheme))) + } + + return allErrs +} diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index fae40f5a054..2641f1ea158 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + kubeadmapiv1alpha3 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha3" "k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig" utilpointer "k8s.io/kubernetes/pkg/util/pointer" ) @@ -108,14 +109,15 @@ func TestValidateNodeRegistrationOptions(t *testing.T) { criSocket string expectedErrors bool }{ - {"", "/some/path", true}, // node name can't be empty - {"valid-nodename", "", true}, // crisocket can't be empty - {"INVALID-NODENAME", "/some/path", true}, // Upper cases is invalid - {"invalid-nodename-", "/some/path", true}, // Can't have trailing dashes - {"invalid-node?name", "/some/path", true}, // Unsupported characters - {"valid-nodename", "relative/path", true}, // crisocket must be an absolute path - {"valid-nodename", "/some/path", false}, // supported - {"valid-nodename-with-numbers01234", "/some/path/with/numbers/01234/", false}, // supported, with numbers as well + {"", "/some/path", true}, // node name can't be empty + {"INVALID-NODENAME", "/some/path", true}, // Upper cases is invalid + {"invalid-nodename-", "/some/path", true}, // Can't have trailing dashes + {"invalid-node?name", "/some/path", true}, // Unsupported characters + {"valid-nodename", "/some/path", false}, // supported + {"valid-nodename-with-numbers01234", "/some/path/with/numbers/01234/", false}, // supported, with numbers as well + {"valid-nodename", kubeadmapiv1alpha3.DefaultUrlScheme + "://" + "/some/path", false}, // supported, with socket url + {"valid-nodename", "bla:///some/path", true}, // unsupported url scheme + {"valid-nodename", ":::", true}, // unparseable url } for _, rt := range tests { nro := kubeadm.NodeRegistrationOptions{Name: rt.nodeName, CRISocket: rt.criSocket}