Merge pull request #2609 from laozc/test-path-windows-call

feat: Migrate PathValid and SMB API to use WMI and Win32 API

Author: k8s-ci-robot

go.mod

@@ -24,6 +24,7 @@ require (
 	github.com/microsoft/wmi v0.31.1
 	github.com/onsi/ginkgo/v2 v2.23.4
 	github.com/onsi/gomega v1.37.0
+	github.com/pkg/errors v0.9.1
 	github.com/rubiojr/go-vhd v0.0.0-20200706105327-02e210299021
 	github.com/stretchr/testify v1.10.0
 	go.uber.org/goleak v1.3.0
@@ -121,7 +122,6 @@ require (
 	github.com/opencontainers/runtime-spec v1.2.0 // indirect
 	github.com/opencontainers/selinux v1.11.1 // indirect
 	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_golang v1.22.0 // indirect
 	github.com/prometheus/client_model v0.6.2 // indirect

pkg/azurefile/azurefile_options.go

@@ -86,7 +86,7 @@ func (o *DriverOptions) AddFlags() *flag.FlagSet {
 	fs.BoolVar(&o.AppendNoShareSockOption, "append-nosharesock-option", true, "Whether appending nosharesock option to smb mount command")
 	fs.BoolVar(&o.AppendNoResvPortOption, "append-noresvport-option", true, "Whether appending noresvport option to nfs mount command")
 	fs.BoolVar(&o.AppendActimeoOption, "append-actimeo-option", true, "Whether appending actimeo=0 option to nfs mount command")
-	fs.BoolVar(&o.UseWinCIMAPI, "use-win-cim-api", false, "Whether performing azure file operations using CIM API or Powershell command on Windows node")
+	fs.BoolVar(&o.UseWinCIMAPI, "use-win-cim-api", true, "Whether performing azure file operations using CIM API or Powershell command on Windows node")
 	fs.IntVar(&o.SkipMatchingTagCacheExpireInMinutes, "skip-matching-tag-cache-expire-in-minutes", 30, "The cache expire time in minutes for skipMatchingTagCache")
 	fs.IntVar(&o.VolStatsCacheExpireInMinutes, "vol-stats-cache-expire-in-minutes", 10, "The cache expire time in minutes for volume stats cache")
 	fs.BoolVar(&o.PrintVolumeStatsCallLogs, "print-volume-stats-call-logs", false, "Whether to print volume statfs call logs with log level 2")

pkg/mounter/safe_mounter_host_process_windows.go

@@ -87,6 +87,7 @@ func (mounter *winMounter) SMBMount(source, target, fsType string, mountOptions,
 
 	isMapped, err := mounter.smbAPI.IsSmbMapped(remotePath)
 	if err != nil {
+		klog.Errorf("IsSmbMapped(%s) failed with %v", remotePath, err)
 		isMapped = false
 	}
 

pkg/os/cim/smb.go

@@ -20,6 +20,8 @@ limitations under the License.
 package cim
 
 import (
+	"strings"
+
 	"github.com/microsoft/wmi/pkg/base/query"
 	cim "github.com/microsoft/wmi/pkg/wmiinstance"
 )
@@ -33,8 +35,24 @@ const (
 	SmbMappingStatusConnecting
 	SmbMappingStatusReconnecting
 	SmbMappingStatusUnavailable
+
+	credentialDelimiter = ":"
 )
 
+// escapeQueryParameter escapes a parameter for WMI Queries
+func escapeQueryParameter(s string) string {
+	s = strings.ReplaceAll(s, "'", "''")
+	s = strings.ReplaceAll(s, "\\", "\\\\")
+	return s
+}
+
+func escapeUserName(userName string) string {
+	// refer to https://github.com/PowerShell/PowerShell/blob/9303de597da55963a6e26a8fe164d0b256ca3d4d/src/Microsoft.PowerShell.Commands.Management/cimSupport/cmdletization/cim/cimConverter.cs#L169-L170
+	userName = strings.ReplaceAll(userName, "\\", "\\\\")
+	userName = strings.ReplaceAll(userName, credentialDelimiter, "\\"+credentialDelimiter)
+	return userName
+}
+
 // QuerySmbGlobalMappingByRemotePath retrieves the SMB global mapping from its remote path.
 //
 // The equivalent WMI query is:
@@ -44,7 +62,7 @@ const (
 // Refer to https://pkg.go.dev/github.com/microsoft/wmi/server2019/root/microsoft/windows/smb#MSFT_SmbGlobalMapping
 // for the WMI class definition.
 func QuerySmbGlobalMappingByRemotePath(remotePath string) (*cim.WmiInstance, error) {
-	smbQuery := query.NewWmiQuery("MSFT_SmbGlobalMapping", "RemotePath", remotePath)
+	smbQuery := query.NewWmiQuery("MSFT_SmbGlobalMapping", "RemotePath", escapeQueryParameter(remotePath))
 	instances, err := QueryInstances(WMINamespaceSmb, smbQuery)
 	if err != nil {
 		return nil, err
@@ -53,12 +71,22 @@ func QuerySmbGlobalMappingByRemotePath(remotePath string) (*cim.WmiInstance, err
 	return instances[0], err
 }
 
-// RemoveSmbGlobalMappingByRemotePath removes a SMB global mapping matching to the remote path.
+// GetSmbGlobalMappingStatus returns the status of an SMB global mapping.
+func GetSmbGlobalMappingStatus(inst *cim.WmiInstance) (int32, error) {
+	statusProp, err := inst.GetProperty("Status")
+	if err != nil {
+		return SmbMappingStatusUnavailable, err
+	}
+
+	return statusProp.(int32), nil
+}
+
+// RemoveSmbGlobalMappingByRemotePath removes an SMB global mapping matching to the remote path.
 //
 // Refer to https://pkg.go.dev/github.com/microsoft/wmi/server2019/root/microsoft/windows/smb#MSFT_SmbGlobalMapping
 // for the WMI class definition.
 func RemoveSmbGlobalMappingByRemotePath(remotePath string) error {
-	smbQuery := query.NewWmiQuery("MSFT_SmbGlobalMapping", "RemotePath", remotePath)
+	smbQuery := query.NewWmiQuery("MSFT_SmbGlobalMapping", "RemotePath", escapeQueryParameter(remotePath))
 	instances, err := QueryInstances(WMINamespaceSmb, smbQuery)
 	if err != nil {
 		return err
@@ -67,3 +95,22 @@ func RemoveSmbGlobalMappingByRemotePath(remotePath string) error {
 	_, err = instances[0].InvokeMethod("Remove", true)
 	return err
 }
+
+// NewSmbGlobalMapping creates a new SMB global mapping to the remote path.
+//
+// Refer to https://pkg.go.dev/github.com/microsoft/wmi/server2019/root/microsoft/windows/smb#MSFT_SmbGlobalMapping
+// for the WMI class definition.
+func NewSmbGlobalMapping(remotePath, username, password string, requirePrivacy bool) (int, error) {
+	params := map[string]interface{}{
+		"RemotePath":     remotePath,
+		"RequirePrivacy": requirePrivacy,
+	}
+	if username != "" {
+		// refer to https://github.com/PowerShell/PowerShell/blob/9303de597da55963a6e26a8fe164d0b256ca3d4d/src/Microsoft.PowerShell.Commands.Management/cimSupport/cmdletization/cim/cimConverter.cs#L166-L178
+		// on how SMB credential is handled in PowerShell
+		params["Credential"] = escapeUserName(username) + credentialDelimiter + password
+	}
+
+	result, _, err := InvokeCimMethod(WMINamespaceSmb, "MSFT_SmbGlobalMapping", "Create", params)
+	return result, err
+}

pkg/os/cim/wmi.go

@@ -20,32 +20,32 @@ limitations under the License.
 package cim
 
 import (
+	"errors"
 	"fmt"
+	"runtime"
 
 	"github.com/go-ole/go-ole"
 	"github.com/go-ole/go-ole/oleutil"
 	"github.com/microsoft/wmi/pkg/base/query"
-	"github.com/microsoft/wmi/pkg/errors"
+	wmierrors "github.com/microsoft/wmi/pkg/errors"
 	cim "github.com/microsoft/wmi/pkg/wmiinstance"
+	"golang.org/x/sys/windows"
 	"k8s.io/klog/v2"
 )
 
 const (
-	WMINamespaceRoot    = "Root\\CimV2"
+	WMINamespaceCimV2   = "Root\\CimV2"
 	WMINamespaceStorage = "Root\\Microsoft\\Windows\\Storage"
 	WMINamespaceSmb     = "Root\\Microsoft\\Windows\\Smb"
 )
 
 type InstanceHandler func(instance *cim.WmiInstance) (bool, error)
 
-// An InstanceIndexer provides index key to a WMI Instance in a map
-type InstanceIndexer func(instance *cim.WmiInstance) (string, error)
-
 // NewWMISession creates a new local WMI session for the given namespace, defaulting
 // to root namespace if none specified.
 func NewWMISession(namespace string) (*cim.WmiSession, error) {
 	if namespace == "" {
-		namespace = WMINamespaceRoot
+		namespace = WMINamespaceCimV2
 	}
 
 	sessionManager := cim.NewWmiSessionManager()
@@ -80,7 +80,7 @@ func QueryFromWMI(namespace string, query *query.WmiQuery, handler InstanceHandl
 	}
 
 	if len(instances) == 0 {
-		return errors.NotFound
+		return wmierrors.NotFound
 	}
 
 	var cont bool
@@ -114,7 +114,7 @@ func executeClassMethodParam(classInst *cim.WmiInstance, method *cim.WmiMethod,
 
 	iDispatchInstance := classInst.GetIDispatch()
 	if iDispatchInstance == nil {
-		return nil, errors.Wrapf(errors.InvalidInput, "InvalidInstance")
+		return nil, wmierrors.Wrapf(wmierrors.InvalidInput, "InvalidInstance")
 	}
 	rawResult, err := iDispatchInstance.GetProperty("Methods_")
 	if err != nil {
@@ -254,11 +254,52 @@ func InvokeCimMethod(namespace, class, methodName string, inputParameters map[st
 	return int(result.ReturnValue), outputParameters, nil
 }
 
+// IsNotFound returns true if it's a "not found" error.
+func IsNotFound(err error) bool {
+	return wmierrors.IsNotFound(err)
+}
+
 // IgnoreNotFound returns nil if the error is nil or a "not found" error,
 // otherwise returns the original error.
 func IgnoreNotFound(err error) error {
-	if err == nil || errors.IsNotFound(err) {
+	if err == nil || IsNotFound(err) {
 		return nil
 	}
 	return err
 }
+
+// WithCOMThread runs the given function `fn` on a locked OS thread
+// with COM initialized using COINIT_MULTITHREADED.
+//
+// This is necessary for using COM/OLE APIs directly (e.g., via go-ole),
+// because COM requires that initialization and usage occur on the same thread.
+//
+// It performs the following steps:
+//   - Locks the current goroutine to its OS thread
+//   - Calls ole.CoInitializeEx with COINIT_MULTITHREADED
+//   - Executes the user-provided function
+//   - Uninitializes COM
+//   - Unlocks the thread
+//
+// If COM initialization fails, or if the user's function returns an error,
+// that error is returned by WithCOMThread.
+func WithCOMThread(fn func() error) error {
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	if err := ole.CoInitializeEx(0, ole.COINIT_MULTITHREADED); err != nil {
+		var oleError *ole.OleError
+		if errors.As(err, &oleError) && oleError != nil && oleError.Code() == uintptr(windows.S_FALSE) {
+			klog.V(10).Infof("COM library has been already initialized for the calling thread, proceeding to the function with no error")
+			err = nil
+		}
+		if err != nil {
+			return err
+		}
+	} else {
+		klog.V(10).Infof("COM library is initialized for the calling thread")
+	}
+	defer ole.CoUninitialize()
+
+	return fn()
+}

pkg/os/filesystem/filesystem.go

@@ -26,6 +26,8 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/pkg/errors"
+	"golang.org/x/sys/windows"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/azurefile-csi-driver/pkg/util"
 )
@@ -84,14 +86,27 @@ func PathExists(path string) (bool, error) {
 }
 
 func PathValid(_ context.Context, path string) (bool, error) {
-	cmd := `Test-Path $Env:remotepath`
-	cmdEnv := fmt.Sprintf("remotepath=%s", path)
-	output, err := util.RunPowershellCmd(cmd, cmdEnv)
+	klog.V(6).Infof("PathValid called with path: %s", path)
+	pathString, err := windows.UTF16PtrFromString(path)
 	if err != nil {
-		return false, fmt.Errorf("returned output: %s, error: %v", string(output), err)
+		klog.V(6).Infof("failed to convert path %s to UTF16: %v", path, err)
+		return false, fmt.Errorf("invalid path: %w", err)
 	}
 
-	return strings.HasPrefix(strings.ToLower(string(output)), "true"), nil
+	attrs, err := windows.GetFileAttributes(pathString)
+	if err != nil {
+		klog.V(6).Infof("failed to get file attributes for path %s: %v", path, err)
+		if errors.Is(err, windows.ERROR_PATH_NOT_FOUND) || errors.Is(err, windows.ERROR_FILE_NOT_FOUND) || errors.Is(err, windows.ERROR_INVALID_NAME) {
+			klog.Warningf("path %s does not exist or is invalid, error: %v", path, err)
+			return false, nil
+		}
+
+		// GetFileAttribute returns user or password incorrect for a disconnected SMB connection after the password is changed
+		return false, fmt.Errorf("failed to get path %s attribute: %w", path, err)
+	}
+
+	klog.V(6).Infof("GetFileAttributes for path %s returned attributes: %d", path, attrs)
+	return attrs != windows.INVALID_FILE_ATTRIBUTES, nil
 }
 
 func ValidatePathWindows(path string) error {

pkg/os/smb/smb_cim.go

@@ -20,27 +20,10 @@ limitations under the License.
 package smb
 
 import (
-	"fmt"
-	"strings"
-
+	"k8s.io/klog/v2"
 	"sigs.k8s.io/azurefile-csi-driver/pkg/os/cim"
 )
 
-const (
-	credentialDelimiter = ":"
-)
-
-func remotePathForQuery(remotePath string) string {
-	return strings.ReplaceAll(remotePath, "\\", "\\\\")
-}
-
-func escapeUserName(userName string) string {
-	// refer to https://github.com/PowerShell/PowerShell/blob/9303de597da55963a6e26a8fe164d0b256ca3d4d/src/Microsoft.PowerShell.Commands.Management/cimSupport/cmdletization/cim/cimConverter.cs#L169-L170
-	escaped := strings.ReplaceAll(userName, "\\", "\\\\")
-	escaped = strings.ReplaceAll(escaped, credentialDelimiter, "\\"+credentialDelimiter)
-	return escaped
-}
-
 var _ SMBAPI = &cimSMBAPI{}
 
 type cimSMBAPI struct{}
@@ -50,42 +33,44 @@ func NewCimSMBAPI() *cimSMBAPI {
 }
 
 func (*cimSMBAPI) IsSmbMapped(remotePath string) (bool, error) {
-	inst, err := cim.QuerySmbGlobalMappingByRemotePath(remotePathForQuery(remotePath))
-	if err != nil {
-		return false, cim.IgnoreNotFound(err)
-	}
-
-	status, err := inst.GetProperty("Status")
-	if err != nil {
-		return false, err
-	}
-
-	return status.(int32) == cim.SmbMappingStatusOK, nil
+	var isMapped bool
+	err := cim.WithCOMThread(func() error {
+		inst, err := cim.QuerySmbGlobalMappingByRemotePath(remotePath)
+		if err != nil {
+			klog.V(6).Infof("error querying smb mapping for remote path %s. err: %v", remotePath, err)
+			return err
+		}
+
+		status, err := cim.GetSmbGlobalMappingStatus(inst)
+		if err != nil {
+			klog.V(6).Infof("error getting smb mapping status for remote path %s. err: %v", remotePath, err)
+			return err
+		}
+
+		isMapped = status == cim.SmbMappingStatusOK
+		return nil
+	})
+	return isMapped, cim.IgnoreNotFound(err)
 }
 
 func (*cimSMBAPI) NewSmbGlobalMapping(remotePath, username, password string) error {
-	params := map[string]interface{}{
-		"RemotePath":     remotePath,
-		"RequirePrivacy": true,
-	}
-	if username != "" {
-		// refer to https://github.com/PowerShell/PowerShell/blob/9303de597da55963a6e26a8fe164d0b256ca3d4d/src/Microsoft.PowerShell.Commands.Management/cimSupport/cmdletization/cim/cimConverter.cs#L166-L178
-		// on how SMB credential is handled in PowerShell
-		params["Credential"] = escapeUserName(username) + credentialDelimiter + password
-	}
-
-	result, _, err := cim.InvokeCimMethod(cim.WMINamespaceSmb, "MSFT_SmbGlobalMapping", "Create", params)
-	if err != nil {
-		return fmt.Errorf("NewSmbGlobalMapping failed. result: %d, err: %v", result, err)
-	}
-
-	return nil
+	return cim.WithCOMThread(func() error {
+		result, err := cim.NewSmbGlobalMapping(remotePath, username, password, true)
+		if err != nil {
+			klog.V(6).Infof("error creating smb mapping for remote path %s. result %d, err: %v", remotePath, result, err)
+			return err
+		}
+		return nil
+	})
 }
 
 func (*cimSMBAPI) RemoveSmbGlobalMapping(remotePath string) error {
-	err := cim.RemoveSmbGlobalMappingByRemotePath(remotePathForQuery(remotePath))
-	if err != nil {
-		return fmt.Errorf("error remove smb mapping '%s'. err: %v", remotePath, err)
-	}
-	return nil
+	return cim.WithCOMThread(func() error {
+		err := cim.RemoveSmbGlobalMappingByRemotePath(remotePath)
+		if err != nil {
+			klog.V(6).Infof("error removing smb mapping for remote path %s. err: %v", remotePath, err)
+			return err
+		}
+		return nil
+	})
 }