From 5656e346c764cc765fc2c2361b3c7e17034869cc Mon Sep 17 00:00:00 2001
From: andyzhangx <xiazhang@microsoft.com>
Date: Thu, 15 Oct 2020 07:02:12 +0000
Subject: [PATCH] fix: smb valid path error

---
 .../src/k8s.io/mount-utils/mount_windows.go   | 41 +++++++++++--------
 .../k8s.io/mount-utils/mount_windows_test.go  | 26 ++++++++++++
 2 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/staging/src/k8s.io/mount-utils/mount_windows.go b/staging/src/k8s.io/mount-utils/mount_windows.go
index 316bf15d7ee..f4573f264f7 100644
--- a/staging/src/k8s.io/mount-utils/mount_windows.go
+++ b/staging/src/k8s.io/mount-utils/mount_windows.go
@@ -29,6 +29,10 @@ import (
 	"k8s.io/utils/keymutex"
 )
 
+const (
+	accessDenied string = "access is denied"
+)
+
 // Mounter provides the default implementation of mount.Interface
 // for the windows platform.  This implementation assumes that the
 // kubelet is running in the host's root mount namespace.
@@ -103,32 +107,29 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri
 		getSMBMountMutex.LockKey(source)
 		defer getSMBMountMutex.UnlockKey(source)
 
-		var output string
-		var err error
 		username := allOptions[0]
 		password := allOptions[1]
-		if output, err = newSMBMapping(username, password, source); err != nil {
+		if output, err := newSMBMapping(username, password, source); err != nil {
 			klog.Warningf("SMB Mapping(%s) returned with error(%v), output(%s)", source, err, string(output))
 			if isSMBMappingExist(source) {
-				valid, errPath := isValidPath(source)
-				if errPath != nil {
-					return errPath
-				}
-				if valid {
-					err = nil
-					klog.V(2).Infof("SMB Mapping(%s) already exists and is still valid, skip error", source)
-				} else {
-					klog.V(2).Infof("SMB Mapping(%s) already exists while it's not valid, now begin to remove and remount", source)
-					if output, err = removeSMBMapping(source); err != nil {
-						return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output)
+				valid, err := isValidPath(source)
+				if !valid {
+					if err == nil || isAccessDeniedError(err) {
+						klog.V(2).Infof("SMB Mapping(%s) already exists while it's not valid, return error: %v, now begin to remove and remount", source, err)
+						if output, err = removeSMBMapping(source); err != nil {
+							return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output)
+						}
+						if output, err := newSMBMapping(username, password, source); err != nil {
+							return fmt.Errorf("New-SmbGlobalMapping(%s) failed: %v, output: %q", source, err, output)
+						}
 					}
-					output, err = newSMBMapping(username, password, source)
+				} else {
+					klog.V(2).Infof("SMB Mapping(%s) already exists and is still valid, skip error(%v)", source, err)
 				}
+			} else {
+				return fmt.Errorf("New-SmbGlobalMapping(%s) failed: %v, output: %q", source, err, output)
 			}
 		}
-		if err != nil {
-			return fmt.Errorf("New-SmbGlobalMapping(%s) failed: %v, output: %q", source, err, output)
-		}
 	}
 
 	output, err := exec.Command("cmd", "/c", "mklink", "/D", target, bindSource).CombinedOutput()
@@ -184,6 +185,10 @@ func isValidPath(remotepath string) (bool, error) {
 	return strings.HasPrefix(strings.ToLower(string(output)), "true"), nil
 }
 
+func isAccessDeniedError(err error) bool {
+	return err != nil && strings.Contains(strings.ToLower(err.Error()), accessDenied)
+}
+
 // remove SMB mapping
 func removeSMBMapping(remotepath string) (string, error) {
 	cmd := exec.Command("powershell", "/c", `Remove-SmbGlobalMapping -RemotePath $Env:smbremotepath -Force`)
diff --git a/staging/src/k8s.io/mount-utils/mount_windows_test.go b/staging/src/k8s.io/mount-utils/mount_windows_test.go
index 584a5e7e591..b87e3430126 100644
--- a/staging/src/k8s.io/mount-utils/mount_windows_test.go
+++ b/staging/src/k8s.io/mount-utils/mount_windows_test.go
@@ -361,3 +361,29 @@ func TestIsValidPath(t *testing.T) {
 		}
 	}
 }
+
+func TestIsAccessDeniedError(t *testing.T) {
+	tests := []struct {
+		err            error
+		expectedResult bool
+	}{
+		{
+			nil,
+			false,
+		},
+		{
+			fmt.Errorf("other error message"),
+			false,
+		},
+		{
+			fmt.Errorf(`PathValid(\\xxx\share) failed with returned output: Test-Path : Access is denied`),
+			true,
+		},
+	}
+
+	for _, test := range tests {
+		result := isAccessDeniedError(test.err)
+		assert.Equal(t, result, test.expectedResult, "Expect result not equal with isAccessDeniedError(%v) return: %q, expected: %q",
+			test.err, result, test.expectedResult)
+	}
+}