chore: Move away from microsoft/wmi by laozc · Pull Request #3096 · kubernetes-sigs/azurefile-csi-driver

laozc · 2026-04-19T08:31:19Z

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
Use go-ole directly for all the WMI invocation.
Remove the heavy dependency on microsoft/wmi to avoid concerns on supply chain risks on related projects.

Which issue(s) this PR fixes:
Fixes #

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Special notes for your reviewer:
Refer to kubernetes-csi/csi-proxy#415

Release note:

none

k8s-ci-robot · 2026-04-19T08:31:23Z

@laozc: The label(s) kind/refactor cannot be applied, because the repository doesn't have them.

Details

In response to this:

What type of PR is this?
/kind refactor

What this PR does / why we need it:
Use go-ole directly for all the WMI invocation.
Remove the heavy dependency on microsoft/wmi to avoid concerns on supply chain risks on related projects.

Which issue(s) this PR fixes:
Fixes #

Requirements:

uses conventional commit messages

includes documentation

adds unit tests

tested upgrade from previous version

Special notes for your reviewer:
Refer to kubernetes-csi/csi-proxy#415

Release note:
none

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2026-04-19T08:31:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: laozc
Once this PR has been reviewed and has the lgtm label, please assign feiskyer for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

This PR removes the github.com/microsoft/wmi dependency and replaces its usage with a new in-repo WMI helper package built directly on go-ole, updating SMB global mapping operations accordingly.

Changes:

Remove vendored github.com/microsoft/wmi and drop it from module dependencies.
Add pkg/os/wmi helpers for COM-thread setup, WMI querying, enumeration, and class method invocation via go-ole.
Migrate SMB global mapping operations to the new WMI helpers and switch Windows filesystem code from github.com/pkg/errors to stdlib errors.

Reviewed changes

Copilot reviewed 6 out of 36 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`pkg/os/wmi/wmi.go`	New core WMI/COM helper layer (query builder, enumeration, method invocation, scoped COM object tracking).
`pkg/os/wmi/smb.go`	Port SMB global mapping query/remove/create to the new WMI helper APIs.
`pkg/os/smb/smb_cim.go`	Update SMB API implementation to use `pkg/os/wmi` (WithCOMThread + WithScope).
`pkg/os/filesystem/filesystem.go`	Replace `github.com/pkg/errors` with stdlib `errors` usage.
`go.mod` / `go.sum` / `vendor/modules.txt`	Remove `github.com/microsoft/wmi` and adjust `github.com/pkg/errors` dependency classification.
`vendor/github.com/microsoft/wmi/**`	Remove vendored Microsoft WMI library sources.
`pkg/os/cim/wmi.go`	Remove previous CIM/WMI wrapper that depended on `github.com/microsoft/wmi`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Fix WQL escaping: use doubled single quotes ('') and backslash escaping in formatValue - Remove escapeQueryParameter: centralize WQL escaping in formatValue to avoid double-escaping - Fix Query: propagate ItemIndex errors instead of silently continuing - Fix PutProperty: capture and Clear() returned VARIANT to prevent COM resource leaks

Copilot

Pull request overview

Copilot reviewed 6 out of 36 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

pkg/os/wmi/smb.go:80

statusProp is an *ole.VARIANT returned from GetProperty and should be cleared after converting/reading its value. Currently it isn’t cleared, which can leak VARIANT/COM resources on repeated status checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 6 out of 36 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

pkg/os/wmi/smb.go:80

The VARIANT returned from GetProperty("Status") is not cleared after converting it to a uint32. Please ensure statusProp.Clear() is called (e.g., via defer) to avoid leaking COM resources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Fix VARIANT leaks: defer Clear() on serviceRaw, resultRaw, countVar, classRaw - Fix WMIError: store Target as string to avoid dangling IDispatch pointer - Add unit tests for formatValue and QueryBuilder.Build - CoInitializeSecurity: acknowledged, deferred to follow-up

Copilot

Pull request overview

Copilot reviewed 7 out of 37 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Fix double-release: remove defer Release() when defer Clear() already handles it - Fix test: correct selector separator to match Build() output (comma+space) - Add CoInitializeSecurity via ole32.dll syscall with RPC_E_TOO_LATE suppression - Build tag already present on wmi_test.go (confirmed)

Copilot

Pull request overview

Copilot reviewed 7 out of 37 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

pkg/os/wmi/smb.go:81

statusProp is an *ole.VARIANT returned from GetProperty and must be Clear()'d to avoid leaking COM resources. Add a defer statusProp.Clear() (after the nil/error check) before converting it with NewSafeVariant.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T10:48:28Z

+// GetProperty gets the property of the given name from the given object.
+func GetProperty(item *ole.IDispatch, name string) (*ole.VARIANT, error) {
+	return oleutil.GetProperty(item, name)
+}


GetProperty returns a raw *ole.VARIANT that requires callers to Clear() it. This is easy to misuse (and already causes a leak in smb.go). Consider either documenting this requirement in the function comment, or providing typed helpers (e.g., GetUint32Property/GetStringProperty) that clear the variant internally.

Copilot

Pull request overview

Copilot reviewed 7 out of 37 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

pkg/os/wmi/smb.go:81

smb.GetProperty("Status") returns an *ole.VARIANT that must be cleared to avoid leaking COM allocations/refs. Right now statusProp is never Clear()'d before returning, which can cause per-call memory/handle leaks in long-running processes. Consider defer statusProp.Clear() after the nil/error check (or change COMDispatchObject.GetProperty to return a helper that auto-clears).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T11:24:17Z

+	if err := initializeSecurity(); err != nil {
+		klog.V(4).Infof("CoInitializeSecurity failed (non-fatal): %v", err)
+	}


WithCOMThread currently logs and continues when initializeSecurity() fails. If COM security initialization fails (other than the already-handled RPC_E_TOO_LATE case inside initializeSecurity), subsequent WMI calls may fail in confusing ways or run with unexpected security settings. Consider returning this error to the caller instead of treating it as non-fatal (or at least make the behavior explicit/configurable).

Copilot

Pull request overview

Copilot reviewed 7 out of 37 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

pkg/os/wmi/smb.go:129

On non-zero WMI return codes, NewWMIError is constructed with a nil target, which produces errors like target=<nil> and loses useful context. Consider including the relevant dispatch target (e.g., the class/service dispatch from CallMethodOnWMIClass) and/or the remotePath in the returned error to aid debugging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T12:15:39Z

+		if length == 0 {
+			break
+		}
+		if err != nil {
+			return err
+		}


Enumerate() breaks on length==0 before checking err. go-ole's IEnumVARIANT.Next returns a non-nil error for any HRESULT!=0 (including but not limited to S_FALSE), so this will silently swallow real COM/WMI failures that also return length==0. Handle err first and only treat S_FALSE as end-of-enumeration; return other errors.

Suggested change

if length == 0 {

break

}

if err != nil {

return err

}

if err != nil {

var oleErr *ole.OleError

if errors.As(err, &oleErr) && oleErr.Code() == ole.S_FALSE && length == 0 {

break

}

return err

}

if length == 0 {

break

}

Copilot · 2026-04-20T12:15:39Z

+const rpcETooLate = 0x80010119
+
+var (
+	modole32			= windows.NewLazySystemDLL("ole32.dll")
+	procCoInitializeSecurity	= modole32.NewProc("CoInitializeSecurity")
+)
+
+// initializeSecurity calls CoInitializeSecurity with default settings.
+// It is safe to call multiple times; RPC_E_TOO_LATE is suppressed.
+func initializeSecurity() error {
+	// Call CoInitializeSecurity with:
+	// pSecDesc=nil, cAuthSvc=-1(negotiate), asAuthSvc=nil, pReserved1=nil,
+	// dwAuthnLevel=DEFAULT(0), dwImpLevel=IMPERSONATE(3),
+	// pAuthList=nil, dwCapabilities=0, pReserved3=nil
+	hr, _, _ := procCoInitializeSecurity.Call(
+		0, uintptr(0xFFFFFFFF), 0, 0, 0, 3, 0, 0, 0,


initializeSecurity() calls CoInitializeSecurity with dwAuthnLevel=DEFAULT (0) and dwImpLevel=IMPERSONATE (3) via raw numeric args. This is weaker/less explicit than packet privacy and can change DCOM/WMI security behavior. Consider defining named constants and using RPC_C_AUTHN_LEVEL_PKT_PRIVACY (and optionally EOAC_DYNAMIC_CLOAKING) to preserve confidentiality for WMI calls.

Suggested change

const rpcETooLate = 0x80010119

var (

modole32 = windows.NewLazySystemDLL("ole32.dll")

procCoInitializeSecurity = modole32.NewProc("CoInitializeSecurity")

)

// initializeSecurity calls CoInitializeSecurity with default settings.

// It is safe to call multiple times; RPC_E_TOO_LATE is suppressed.

func initializeSecurity() error {

// Call CoInitializeSecurity with:

// pSecDesc=nil, cAuthSvc=-1(negotiate), asAuthSvc=nil, pReserved1=nil,

// dwAuthnLevel=DEFAULT(0), dwImpLevel=IMPERSONATE(3),

// pAuthList=nil, dwCapabilities=0, pReserved3=nil

hr, _, _ := procCoInitializeSecurity.Call(

0, uintptr(0xFFFFFFFF), 0, 0, 0, 3, 0, 0, 0,

const (

rpcETooLate = 0x80010119

rpcCAuthnSvcDefault = ^uintptr(0)

rpcCAuthnLevelPktPrivacy = 6

rpcCImpLevelImpersonate = 3

eoacDynamicCloaking = 0x40

)

var (

modole32 = windows.NewLazySystemDLL("ole32.dll")

procCoInitializeSecurity = modole32.NewProc("CoInitializeSecurity")

)

// initializeSecurity calls CoInitializeSecurity with explicit WMI/DCOM settings.

// It is safe to call multiple times; RPC_E_TOO_LATE is suppressed.

func initializeSecurity() error {

// Call CoInitializeSecurity with:

// pSecDesc=nil, cAuthSvc=-1(default authentication services), asAuthSvc=nil, pReserved1=nil,

// dwAuthnLevel=RPC_C_AUTHN_LEVEL_PKT_PRIVACY, dwImpLevel=RPC_C_IMP_LEVEL_IMPERSONATE,

// pAuthList=nil, dwCapabilities=EOAC_DYNAMIC_CLOAKING, pReserved3=nil

hr, _, _ := procCoInitializeSecurity.Call(

0, rpcCAuthnSvcDefault, 0, 0, rpcCAuthnLevelPktPrivacy, rpcCImpLevelImpersonate, 0, eoacDynamicCloaking, 0,

Copilot · 2026-04-20T12:15:39Z

+	if err := initializeSecurity(); err != nil {
+		klog.V(4).Infof("CoInitializeSecurity failed (non-fatal): %v", err)
+	}


WithCOMThread() logs CoInitializeSecurity failures as non-fatal and then proceeds. If COM security init fails, subsequent WMI calls are likely to fail with less actionable errors; returning the security init error (or at least surfacing it to callers) would make failures easier to diagnose and avoid partially-initialized COM usage.

k8s-ci-robot · 2026-04-20T14:14:40Z

@laozc: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-azurefile-csi-driver-external-e2e-nfs	`f61b68b`	link	true	`/test pull-azurefile-csi-driver-external-e2e-nfs`
pull-azurefile-csi-driver-e2e-capz	`f61b68b`	link	true	`/test pull-azurefile-csi-driver-e2e-capz`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

k8s-ci-robot · 2026-04-28T01:04:41Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot requested review from cvvz and gnufied April 19, 2026 08:31

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 19, 2026

laozc changed the title ~~refactor: Move away from micRemove ms wmi~~ refactor: Move away from microsoft/wmi Apr 19, 2026

laozc changed the title ~~refactor: Move away from microsoft/wmi~~ chore: Move away from microsoft/wmi Apr 19, 2026

laozc force-pushed the remove-ms-wmi branch from 6dae8a0 to 277736b Compare April 19, 2026 09:08

laozc added 3 commits April 19, 2026 17:12

Move away from microsoft/wmi

b49e406

Remove deprecated pkg/errors

02b7f7a

Remove microsoft/wmi from dependency

4d03d79

laozc force-pushed the remove-ms-wmi branch from 277736b to 4d03d79 Compare April 19, 2026 09:12

andyzhangx requested a review from Copilot April 19, 2026 14:15

Copilot started reviewing on behalf of andyzhangx April 19, 2026 14:16 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Comment thread pkg/os/wmi/wmi.go Outdated

Comment thread pkg/os/wmi/wmi.go

Comment thread pkg/os/wmi/wmi.go Outdated

andyzhangx requested a review from Copilot April 20, 2026 09:01

Copilot started reviewing on behalf of andyzhangx April 20, 2026 09:02 View session