Remove wmic usage from disk grains on Windows

changelog/66959.fixed.md

@@ -0,0 +1,2 @@
+Removed the usage of wmic to get the disk grains for Windows. The wmic binary is
+being deprecated.

salt/grains/disks.py

@@ -16,8 +16,11 @@ import salt.utils.platform
 __salt__ = {
     "cmd.run": salt.modules.cmdmod._run_quiet,
     "cmd.run_all": salt.modules.cmdmod._run_all_quiet,
+    "cmd.powershell": salt.modules.cmdmod.powershell,
 }
 
+from salt.exceptions import CommandExecutionError
+
 log = logging.getLogger(__name__)
 
 
@@ -153,41 +156,28 @@ def _linux_disks():
 
 
 def _windows_disks():
-    wmic = salt.utils.path.which("wmic")
-
-    namespace = r"\\root\microsoft\windows\storage"
-    path = "MSFT_PhysicalDisk"
-    get = "DeviceID,MediaType"
 
+    cmd = "Get-PhysicalDisk | Select DeviceID, MediaType"
     ret = {"disks": [], "ssds": []}
 
-    cmdret = __salt__["cmd.run_all"](
+    drive_info = __salt__["cmd.powershell"](cmd)
-        "{} /namespace:{} path {} get {} /format:table".format(
-            wmic, namespace, path, get
-        )
-    )
 
-    if cmdret["retcode"] != 0:
+    if not drive_info:
-        log.trace("Disk grain does not support this version of Windows")
+        log.trace("No physical discs found")
-    else:
+        return ret
-        for line in cmdret["stdout"].splitlines():
+
-            info = line.split()
+    # We need a list of dict
-            if len(info) != 2 or not info[0].isdigit() or not info[1].isdigit():
+    if isinstance(drive_info, dict):
-                continue
+        drive_info = [drive_info]
-            device = rf"\\.\PhysicalDrive{info[0]}"
+
-            mediatype = info[1]
+    for drive in drive_info:
-            if mediatype == "3":
+        # Make sure we have a valid drive type
-                log.trace("Device %s reports itself as an HDD", device)
+        if drive["MediaType"].lower() not in ["hdd", "ssd", "scm", "unspecified"]:
-                ret["disks"].append(device)
+            log.trace(f'Unknown media type: {drive["MediaType"]}')
-            elif mediatype == "4":
+            continue
-                log.trace("Device %s reports itself as an SSD", device)
+        device = rf'\\.\PhysicalDrive{drive["DeviceID"]}'
-                ret["ssds"].append(device)
+        ret["disks"].append(device)
-                ret["disks"].append(device)
+        if drive["MediaType"].lower() == "ssd":
-            elif mediatype == "5":
+            ret["ssds"].append(device)
-                log.trace("Device %s reports itself as an SCM", device)
-                ret["disks"].append(device)
-            else:
-                log.trace("Device %s reports itself as Unspecified", device)
-                ret["disks"].append(device)
 
     return ret

salt/modules/cmdmod.py

@@ -266,7 +266,7 @@ def _prep_powershell_cmd(win_shell, cmd, encoded_cmd):
     win_shell = salt.utils.path.which(win_shell)
 
     if not win_shell:
-        raise CommandExecutionError("PowerShell binary not found")
+        raise CommandExecutionError(f"PowerShell binary not found: {win_shell}")
 
     new_cmd = [win_shell, "-NonInteractive", "-NoProfile", "-ExecutionPolicy", "Bypass"]
 

salt/utils/path.py

@@ -203,7 +203,7 @@ def which(exe=None):
 
     # now to search through our system_path
     for path in system_path:
-        p = join(path, exe)
+        p = join(os.path.expandvars(path), exe)
 
         # iterate through all extensions to see which one is executable
         for ext in pathext:

tests/pytests/unit/grains/test_disks.py

@@ -2,8 +2,6 @@
     :codeauthor: :email:`Shane Lee <slee@saltstack.com>`
 """
 
-import textwrap
-
 import pytest
 
 import salt.grains.disks as disks
@@ -17,63 +15,58 @@ def configure_loader_modules():
     }
 
 
-def test__windows_disks():
+def test__windows_disks_dict():
     """
-    Test grains._windows_disks, normal return
+    Test grains._windows_disks with a single disk returned as a dict
-    Should return a populated dictionary
+    Should return 1 disk and no ssds
     """
-    mock_which = MagicMock(return_value="C:\\Windows\\System32\\wbem\\WMIC.exe")
+    devices = {"DeviceID": 0, "MediaType": "HDD"}
-    wmic_result = textwrap.dedent(
+    mock_powershell = MagicMock(return_value=devices)
-        """
-        DeviceId  MediaType
-        0         4
-        1         0
-        2         3
-        3         5
-    """
-    )
-    mock_run_all = MagicMock(return_value={"stdout": wmic_result, "retcode": 0})
 
-    with patch("salt.utils.path.which", mock_which), patch.dict(
+    with patch.dict(disks.__salt__, {"cmd.powershell": mock_powershell}):
-        disks.__salt__, {"cmd.run_all": mock_run_all}
+        result = disks._windows_disks()
-    ):
+        expected = {"disks": ["\\\\.\\PhysicalDrive0"], "ssds": []}
+        assert result == expected
+
+
+def test__windows_disks_list():
+    """
+    test grains._windows_disks with multiple disks and types as a list of dicts
+    Should return 4 disks and 1 ssd
+    """
+    devices = [
+        {"DeviceID": 0, "MediaType": "SSD"},
+        {"DeviceID": 1, "MediaType": "HDD"},
+        {"DeviceID": 2, "MediaType": "HDD"},
+        {"DeviceID": 3, "MediaType": "HDD"},
+    ]
+    mock_powershell = MagicMock(return_value=devices)
+
+    with patch.dict(disks.__salt__, {"cmd.powershell": mock_powershell}):
         result = disks._windows_disks()
         expected = {
-            "ssds": ["\\\\.\\PhysicalDrive0"],
             "disks": [
                 "\\\\.\\PhysicalDrive0",
                 "\\\\.\\PhysicalDrive1",
                 "\\\\.\\PhysicalDrive2",
                 "\\\\.\\PhysicalDrive3",
             ],
+            "ssds": ["\\\\.\\PhysicalDrive0"],
         }
         assert result == expected
-        cmd = " ".join(
-            [
-                "C:\\Windows\\System32\\wbem\\WMIC.exe",
-                "/namespace:\\\\root\\microsoft\\windows\\storage",
-                "path",
-                "MSFT_PhysicalDisk",
-                "get",
-                "DeviceID,MediaType",
-                "/format:table",
-            ]
-        )
-        mock_run_all.assert_called_once_with(cmd)
 
 
-def test__windows_disks_retcode():
+def test__windows_disks_empty():
     """
-    Test grains._windows_disks, retcode 1
+    Test grains._windows_disks when nothing is returned
     Should return empty lists
     """
-    mock_which = MagicMock(return_value="C:\\Windows\\System32\\wbem\\WMIC.exe")
+    devices = {}
-    mock_run_all = MagicMock(return_value={"stdout": "", "retcode": 1})
+    mock_powershell = MagicMock(return_value=devices)
-    with patch("salt.utils.path.which", mock_which), patch.dict(
+
-        disks.__salt__, {"cmd.run_all": mock_run_all}
+    with patch.dict(disks.__salt__, {"cmd.powershell": mock_powershell}):
-    ):
+        expected = {"disks": [], "ssds": []}
         result = disks._windows_disks()
-        expected = {"ssds": [], "disks": []}
         assert result == expected