diff --git a/pkg/kubelet/apis/config/validation/validation.go b/pkg/kubelet/apis/config/validation/validation.go index 83cf1989f8f..6fefe704f40 100644 --- a/pkg/kubelet/apis/config/validation/validation.go +++ b/pkg/kubelet/apis/config/validation/validation.go @@ -18,7 +18,6 @@ package validation import ( "fmt" - "path/filepath" "time" "unicode" @@ -33,6 +32,7 @@ import ( "k8s.io/kubernetes/pkg/features" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" + utilfs "k8s.io/kubernetes/pkg/util/filesystem" utiltaints "k8s.io/kubernetes/pkg/util/taints" "k8s.io/utils/cpuset" ) @@ -293,11 +293,11 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration, featur allErrors = append(allErrors, fmt.Errorf("invalid configuration: podLogsDir was not specified")) } - if !filepath.IsAbs(kc.PodLogsDir) { + if !utilfs.IsAbs(kc.PodLogsDir) { allErrors = append(allErrors, fmt.Errorf("invalid configuration: pod logs path %q must be absolute path", kc.PodLogsDir)) } - if filepath.Clean(kc.PodLogsDir) != kc.PodLogsDir { + if !utilfs.IsPathClean(kc.PodLogsDir) { allErrors = append(allErrors, fmt.Errorf("invalid configuration: pod logs path %q must be normalized", kc.PodLogsDir)) } diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 8ad7be79393..b93c8a3bd18 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -61,6 +61,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/status" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/kubelet/util" + utilfs "k8s.io/kubernetes/pkg/util/filesystem" utilpod "k8s.io/kubernetes/pkg/util/pod" volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/hostutil" @@ -199,7 +200,7 @@ func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVol var devices []kubecontainer.DeviceInfo for _, device := range container.VolumeDevices { // check path is absolute - if !filepath.IsAbs(device.DevicePath) { + if !utilfs.IsAbs(device.DevicePath) { return nil, fmt.Errorf("error DevicePath `%s` must be an absolute path", device.DevicePath) } vol, ok := podVolumes[device.Name] @@ -280,7 +281,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h } if subPath != "" { - if filepath.IsAbs(subPath) { + if utilfs.IsAbs(subPath) { return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", subPath) } @@ -332,7 +333,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h containerPath := mount.MountPath // IsAbs returns false for UNC path/SMB shares/named pipes in Windows. So check for those specifically and skip MakeAbsolutePath - if !volumeutil.IsWindowsUNCPath(runtime.GOOS, containerPath) && !filepath.IsAbs(containerPath) { + if !volumeutil.IsWindowsUNCPath(runtime.GOOS, containerPath) && !utilfs.IsAbs(containerPath) { containerPath = volumeutil.MakeAbsolutePath(runtime.GOOS, containerPath) } diff --git a/pkg/kubelet/kubeletconfig/configfiles/configfiles.go b/pkg/kubelet/kubeletconfig/configfiles/configfiles.go index 46423cb241d..4e8fd8ea021 100644 --- a/pkg/kubelet/kubeletconfig/configfiles/configfiles.go +++ b/pkg/kubelet/kubeletconfig/configfiles/configfiles.go @@ -100,7 +100,7 @@ func resolveRelativePaths(paths []*string, root string) { for _, path := range paths { // leave empty paths alone, "no path" is a valid input // do not attempt to resolve paths that are already absolute - if len(*path) > 0 && !filepath.IsAbs(*path) { + if len(*path) > 0 && !utilfs.IsAbs(*path) { *path = filepath.Join(root, *path) } } diff --git a/pkg/util/filesystem/util.go b/pkg/util/filesystem/util.go new file mode 100644 index 00000000000..d64fc37c71b --- /dev/null +++ b/pkg/util/filesystem/util.go @@ -0,0 +1,27 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package filesystem + +import ( + "path/filepath" +) + +// IsPathClean will replace slashes to Separator (which is OS-specific). +// This will make sure that all slashes are the same before comparing. +func IsPathClean(path string) bool { + return filepath.ToSlash(filepath.Clean(path)) == filepath.ToSlash(path) +} diff --git a/pkg/util/filesystem/util_test.go b/pkg/util/filesystem/util_test.go index 87ec73f067a..8b8cb5363d3 100644 --- a/pkg/util/filesystem/util_test.go +++ b/pkg/util/filesystem/util_test.go @@ -19,6 +19,7 @@ package filesystem import ( "net" "os" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -84,3 +85,48 @@ func TestIsUnixDomainSocket(t *testing.T) { assert.Equal(t, result, test.expectSocket, "Unexpected result from IsUnixDomainSocket: %v for %s", result, test.label) } } + +func TestIsCleanPath(t *testing.T) { + type Case struct { + path string + expected bool + } + + // Credits https://github.com/kubernetes/kubernetes/pull/124084/files#r1557566941 + cases := []Case{ + {path: "/logs", expected: true}, + {path: "/var/lib/something", expected: true}, + {path: "var/lib/something", expected: true}, + {path: "var\\lib\\something", expected: true}, + {path: "/", expected: true}, + {path: ".", expected: true}, + {path: "/var/../something", expected: false}, + {path: "/var//lib/something", expected: false}, + {path: "/var/./lib/something", expected: false}, + } + + // Additional cases applicable on Windows + if runtime.GOOS == "windows" { + cases = append(cases, []Case{ + {path: "\\", expected: true}, + {path: "C:/var/lib/something", expected: true}, + {path: "C:\\var\\lib\\something", expected: true}, + {path: "C:/", expected: true}, + {path: "C:\\", expected: true}, + {path: "C:/var//lib/something", expected: false}, + {path: "\\var\\\\lib\\something", expected: false}, + {path: "C:\\var\\\\lib\\something", expected: false}, + {path: "C:\\var\\..\\something", expected: false}, + {path: "\\var\\.\\lib\\something", expected: false}, + {path: "C:\\var\\.\\lib\\something", expected: false}, + }...) + } + + for _, tc := range cases { + actual := IsPathClean(tc.path) + if actual != tc.expected { + t.Errorf("actual: %t, expected: %t, for path: %s\n", actual, tc.expected, tc.path) + } + } + +} diff --git a/pkg/util/filesystem/util_unix.go b/pkg/util/filesystem/util_unix.go index df887f94508..863deb0f9cc 100644 --- a/pkg/util/filesystem/util_unix.go +++ b/pkg/util/filesystem/util_unix.go @@ -22,6 +22,7 @@ package filesystem import ( "fmt" "os" + "path/filepath" ) // IsUnixDomainSocket returns whether a given file is a AF_UNIX socket file @@ -35,3 +36,8 @@ func IsUnixDomainSocket(filePath string) (bool, error) { } return true, nil } + +// IsAbs is same as filepath.IsAbs on Unix. +func IsAbs(path string) bool { + return filepath.IsAbs(path) +} diff --git a/pkg/util/filesystem/util_windows.go b/pkg/util/filesystem/util_windows.go index cd6a11ed308..459477d36e7 100644 --- a/pkg/util/filesystem/util_windows.go +++ b/pkg/util/filesystem/util_windows.go @@ -23,6 +23,8 @@ import ( "fmt" "net" "os" + "path/filepath" + "strings" "time" "k8s.io/apimachinery/pkg/util/wait" @@ -85,3 +87,13 @@ func IsUnixDomainSocket(filePath string) (bool, error) { } return true, nil } + +// IsAbs returns whether the given path is absolute or not. +// On Windows, filepath.IsAbs will not return True for paths prefixed with a slash, even +// though they can be used as absolute paths (https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats). +// +// WARN: It isn't safe to use this for API values which will propagate across systems (e.g. REST API values +// that get validated on Unix, persisted, then consumed by Windows, etc). +func IsAbs(path string) bool { + return filepath.IsAbs(path) || strings.HasPrefix(path, `\`) || strings.HasPrefix(path, `/`) +} diff --git a/pkg/util/filesystem/util_windows_test.go b/pkg/util/filesystem/util_windows_test.go index f27880b28c9..4e87c3ddc22 100644 --- a/pkg/util/filesystem/util_windows_test.go +++ b/pkg/util/filesystem/util_windows_test.go @@ -89,3 +89,12 @@ func TestPendingUnixDomainSocket(t *testing.T) { wg.Wait() unixln.Close() } + +func TestAbsWithSlash(t *testing.T) { + // On Windows, filepath.IsAbs will not return True for paths prefixed with a slash + assert.True(t, IsAbs("/test")) + assert.True(t, IsAbs("\\test")) + + assert.False(t, IsAbs("./local")) + assert.False(t, IsAbs("local")) +}