From f8a69f10864c8936bfb652e8204e96a682f73f8f Mon Sep 17 00:00:00 2001 From: Deep Debroy Date: Fri, 12 Oct 2018 19:52:50 -0700 Subject: [PATCH] Broaden scope of host path types to skip processing in Windows Signed-off-by: Deep Debroy --- pkg/kubelet/kubelet_pods.go | 9 +++--- pkg/volume/util/util.go | 11 ++++++-- pkg/volume/util/util_test.go | 54 +++++++++++++++++++++--------------- 3 files changed, 46 insertions(+), 28 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 20f92997d80..0cda842d893 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -215,15 +215,16 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h } } - // Docker Volume Mounts fail on Windows if it is not of the form C:/ nor a named pipe starting with \\.\pipe\ + // Docker Volume Mounts fail on Windows if it is not of the form C:/ containerPath := mount.MountPath if runtime.GOOS == "windows" { - if !volumeutil.IsWindowsNamedPipe(runtime.GOOS, hostPath) && (strings.HasPrefix(hostPath, "/") || strings.HasPrefix(hostPath, "\\")) && !strings.Contains(hostPath, ":") { + // Append C: only if it looks like a local path. Do not process UNC path/SMB shares/named pipes + if (strings.HasPrefix(hostPath, "/") || strings.HasPrefix(hostPath, "\\")) && !strings.Contains(hostPath, ":") && !volumeutil.IsWindowsUNCPath(runtime.GOOS, hostPath) { hostPath = "c:" + hostPath } } - // IsAbs returns false for named pipes (\\.\pipe\...) in Windows. So check for it specifically and skip MakeAbsolutePath - if !volumeutil.IsWindowsNamedPipe(runtime.GOOS, containerPath) && !filepath.IsAbs(containerPath) { + // 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) { containerPath = volumeutil.MakeAbsolutePath(runtime.GOOS, containerPath) } diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 2426d06610f..e3ae1bf7004 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -945,8 +945,15 @@ func CheckPersistentVolumeClaimModeBlock(pvc *v1.PersistentVolumeClaim) bool { return utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == v1.PersistentVolumeBlock } -func IsWindowsNamedPipe(goos, path string) bool { - if goos == "windows" && strings.HasPrefix(path, `\\.\pipe\`) { +// IsWindowsUNCPath checks if path is prefixed with \\ +// This can be used to skip any processing of paths +// that point to SMB shares, local named pipes and local UNC path +func IsWindowsUNCPath(goos, path string) bool { + if goos != "windows" { + return false + } + // Check for UNC prefix \\ + if strings.HasPrefix(path, `\\`) { return true } return false diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 2cddc632bba..1f90fb67219 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -2373,43 +2373,53 @@ func TestGetWindowsPath(t *testing.T) { } } -func TestIsWindowsNamedPipe(t *testing.T) { +func TestIsWindowsUNCPath(t *testing.T) { tests := []struct { - goos string - path string - isNamedPipe bool + goos string + path string + isUNCPath bool }{ { - goos: "linux", - path: `/usr/bin`, - isNamedPipe: false, + goos: "linux", + path: `/usr/bin`, + isUNCPath: false, }, { - goos: "linux", - path: `\\.\pipe\foo`, - isNamedPipe: false, + goos: "linux", + path: `\\.\pipe\foo`, + isUNCPath: false, }, { - goos: "windows", - path: `C:\foo`, - isNamedPipe: false, + goos: "windows", + path: `C:\foo`, + isUNCPath: false, }, { - goos: "windows", - path: `\\.\invalid`, - isNamedPipe: false, + goos: "windows", + path: `\\server\share\foo`, + isUNCPath: true, }, { - goos: "windows", - path: `\\.\pipe\valid_pipe`, - isNamedPipe: true, + goos: "windows", + path: `\\?\server\share`, + isUNCPath: true, + }, + { + goos: "windows", + path: `\\?\c:\`, + isUNCPath: true, + }, + { + goos: "windows", + path: `\\.\pipe\valid_pipe`, + isUNCPath: true, }, } for _, test := range tests { - result := IsWindowsNamedPipe(test.goos, test.path) - if result != test.isNamedPipe { - t.Errorf("IsWindowsNamedPipe(%v) returned (%v), expected (%v)", test.path, result, test.isNamedPipe) + result := IsWindowsUNCPath(test.goos, test.path) + if result != test.isUNCPath { + t.Errorf("IsWindowsUNCPath(%v) returned (%v), expected (%v)", test.path, result, test.isUNCPath) } } }