diff --git a/changelog/66959.fixed.md b/changelog/66959.fixed.md new file mode 100644 index 00000000000..ad9878c2577 --- /dev/null +++ b/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. diff --git a/salt/grains/disks.py b/salt/grains/disks.py index f61ad6d6b34..6b288706036 100644 --- a/salt/grains/disks.py +++ b/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"]( - "{} /namespace:{} path {} get {} /format:table".format( - wmic, namespace, path, get - ) - ) + drive_info = __salt__["cmd.powershell"](cmd) - if cmdret["retcode"] != 0: - log.trace("Disk grain does not support this version of Windows") - else: - for line in cmdret["stdout"].splitlines(): - info = line.split() - if len(info) != 2 or not info[0].isdigit() or not info[1].isdigit(): - continue - device = rf"\\.\PhysicalDrive{info[0]}" - mediatype = info[1] - if mediatype == "3": - log.trace("Device %s reports itself as an HDD", device) - ret["disks"].append(device) - elif mediatype == "4": - log.trace("Device %s reports itself as an SSD", device) - ret["ssds"].append(device) - ret["disks"].append(device) - elif mediatype == "5": - 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) + if not drive_info: + log.trace("No physical discs found") + return ret + + # We need a list of dict + if isinstance(drive_info, dict): + drive_info = [drive_info] + + for drive in drive_info: + # Make sure we have a valid drive type + if drive["MediaType"].lower() not in ["hdd", "ssd", "scm", "unspecified"]: + log.trace(f'Unknown media type: {drive["MediaType"]}') + continue + device = rf'\\.\PhysicalDrive{drive["DeviceID"]}' + ret["disks"].append(device) + if drive["MediaType"].lower() == "ssd": + ret["ssds"].append(device) return ret diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 1891c044fb0..e708c10c0b0 100644 --- a/salt/modules/cmdmod.py +++ b/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"] diff --git a/salt/utils/path.py b/salt/utils/path.py index edaab010c06..297433665c3 100644 --- a/salt/utils/path.py +++ b/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: diff --git a/tests/pytests/unit/grains/test_disks.py b/tests/pytests/unit/grains/test_disks.py index a0d6d1030e7..b2a2d5fbd2f 100644 --- a/tests/pytests/unit/grains/test_disks.py +++ b/tests/pytests/unit/grains/test_disks.py @@ -2,8 +2,6 @@ :codeauthor: :email:`Shane Lee ` """ -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 - Should return a populated dictionary + Test grains._windows_disks with a single disk returned as a dict + Should return 1 disk and no ssds """ - mock_which = MagicMock(return_value="C:\\Windows\\System32\\wbem\\WMIC.exe") - wmic_result = textwrap.dedent( - """ - DeviceId MediaType - 0 4 - 1 0 - 2 3 - 3 5 - """ - ) - mock_run_all = MagicMock(return_value={"stdout": wmic_result, "retcode": 0}) + devices = {"DeviceID": 0, "MediaType": "HDD"} + 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}): + 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") - mock_run_all = MagicMock(return_value={"stdout": "", "retcode": 1}) - with patch("salt.utils.path.which", mock_which), patch.dict( - disks.__salt__, {"cmd.run_all": mock_run_all} - ): + devices = {} + mock_powershell = MagicMock(return_value=devices) + + with patch.dict(disks.__salt__, {"cmd.powershell": mock_powershell}): + expected = {"disks": [], "ssds": []} result = disks._windows_disks() - expected = {"ssds": [], "disks": []} assert result == expected