[NOT NEEDED ANYMORE] Remove wmic dependency

changelog/66959.fixed.md

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

salt/grains/disks.py

@@ -19,17 +19,20 @@
 import salt.modules.cmdmod
 
 __salt__ = {
-    'cmd.run': salt.modules.cmdmod._run_quiet,
-    'cmd.run_all': salt.modules.cmdmod._run_all_quiet
+    "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__)
 
 
 def disks():
-    '''
+    """
     Return list of disk devices
-    '''
+    """
     if salt.utils.platform.is_freebsd():
         return _freebsd_geom()
     elif salt.utils.platform.is_linux():
@@ -126,10 +129,10 @@ def parse_geom_attribs(device):
 
 
 def _linux_disks():
-    '''
+    """
     Return list of disk devices and work out if they are SSD or HDD.
-    '''
-    ret = {'disks': [], 'SSDs': []}
+    """
+    ret = {"disks": [], "SSDs": []}
 
     for entry in glob.glob('/sys/block/*/queue/rotational'):
         try:
@@ -153,39 +156,27 @@ def _linux_disks():
 
 
 def _windows_disks():
-    wmic = salt.utils.path.which('wmic')
-
-    namespace = r'\\root\microsoft\windows\storage'
-    path = 'MSFT_PhysicalDisk'
-    get = 'DeviceID,MediaType'
-
-    ret = {'disks': [], 'SSDs': []}
-
-    cmdret = __salt__['cmd.run_all'](
-        '{0} /namespace:{1} path {2} get {3} /format:table'.format(
-            wmic, namespace, path, get))
-
-    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 = r'\\.\PhysicalDrive{0}'.format(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)
+    cmd = "Get-PhysicalDisk | Select DeviceID, MediaType"
+    ret = {"disks": [], "SSDs": []}
+
+    drive_info = __salt__["cmd.powershell"](cmd)
+
+    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

salt/grains/iscsi.py

@@ -88,27 +88,26 @@ def _aix_iqn():
 
 
 def _windows_iqn():
-    '''
-    Return iSCSI IQN from a Windows host.
-    '''
+    """
+    Return iSCSI nodes from a Windows host.
+    """
+    cmd = "Get-InitiatorPort | Select NodeAddress"
     ret = []
 
-    wmic = salt.utils.path.which('wmic')
+    nodes = salt.modules.cmdmod.powershell(cmd)
 
-    if not wmic:
+    if not nodes:
+        log.trace("No iSCSI nodes found")
         return ret
 
-    namespace = r'\\root\WMI'
-    path = 'MSiSCSIInitiator_MethodClass'
-    get = 'iSCSINodeName'
-
-    cmd_ret = salt.modules.cmdmod.run_all(
-        '{0} /namespace:{1} path {2} get {3} /format:table'
-        ''.format(wmic, namespace, path, get))
+    # A single node will return a dictionary with a single entry
+    # {"NodeAddress": "iqn.1991-05.com.microsoft:johnj99-pc2.contoso.com"}
+    # Multiple nodes will return a list of single entry dicts
+    # We need a list of dict
+    if isinstance(nodes, dict):
+        nodes = [nodes]
 
-    for line in cmd_ret['stdout'].splitlines():
-        if line.startswith('iqn.'):
-            line = line.rstrip()
-            ret.append(line.rstrip())
+    for node in nodes:
+        ret.append(node["NodeAddress"])
 
     return ret

salt/utils/path.py

@@ -282,7 +282,7 @@ def has_executable_ext(path, ext_membership):
 
     # 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/unit/grains/test_disks.py

@@ -1,10 +1,8 @@
-# -*- coding: utf-8 -*-
-'''
+"""
     :codeauthor: :email:`Shane Lee <slee@saltstack.com>`
-'''
+"""
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
-import textwrap
 
 # Import Salt Testing Libs
 from tests.support.mixins import LoaderModuleMockMixin
@@ -16,6 +14,7 @@
 
 # Import Salt Libs
 import salt.grains.disks as disks
+from tests.support.mock import MagicMock, mock_open, patch
 
 
 class IscsiGrainsTestCase(TestCase, LoaderModuleMockMixin):
@@ -29,56 +28,100 @@ def setup_loader_modules(self):
             },
         }
 
-    def test__windows_disks(self):
-        '''
-        Test grains._windows_disks, normal return
-        Should return a populated dictionary
-        '''
-        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})
-
-        with patch('salt.utils.path.which', mock_which), \
-                patch.dict(disks.__salt__, {'cmd.run_all': mock_run_all}):
+
+    def test__windows_disks_dict():
+        """
+        Test grains._windows_disks with a single disk returned as a dict
+        Should return 1 disk and no ssds
+        """
+        devices = {"DeviceID": 0, "MediaType": "HDD"}
+        mock_powershell = MagicMock(return_value=devices)
+
+        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 of dictsmultiple disks and types as a list
+        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']}
-            self.assertDictEqual(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(self):
-        '''
-        Test grains._windows_disks, retcode 1
+                "disks": [
+                    "\\\\.\\PhysicalDrive0",
+                    "\\\\.\\PhysicalDrive1",
+                    "\\\\.\\PhysicalDrive2",
+                    "\\\\.\\PhysicalDrive3",
+                ],
+                "SSDs": ["\\\\.\\PhysicalDrive0"],
+            }
+            assert result == expected
+
+
+    def test__windows_disks_empty():
+        """
+        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': []}
-            self.assertDictEqual(result, expected)
+            assert result == expected
+
+
+    def test__linux_disks():
+        """
+        Test grains._linux_disks, normal return
+        Should return a populated dictionary
+        """
+
+        files = [
+            "/sys/block/asm!.asm_ctl_vbg0",
+            "/sys/block/dm-0",
+            "/sys/block/loop0",
+            "/sys/block/ram0",
+            "/sys/block/sda",
+            "/sys/block/sdb",
+            "/sys/block/vda",
+        ]
+        links = [
+            "../devices/virtual/block/asm!.asm_ctl_vbg0",
+            "../devices/virtual/block/dm-0",
+            "../devices/virtual/block/loop0",
+            "../devices/virtual/block/ram0",
+            "../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda",
+            "../devices/pci0000:35/0000:35:00.0/0000:36:00.0/host2/target2:1:0/2:1:0:0/block/sdb",
+            "../devices/pci0000L00:0000:00:05.0/virtio2/block/vda",
+        ]
+        contents = [
+            "1",
+            "1",
+            "1",
+            "0",
+            "1",
+            "1",
+            "1",
+        ]
+
+        patch_glob = patch("glob.glob", autospec=True, return_value=files)
+        patch_readlink = patch("salt.utils.path.readlink", autospec=True, side_effect=links)
+        patch_fopen = patch("salt.utils.files.fopen", mock_open(read_data=contents))
+        with patch_glob, patch_readlink, patch_fopen:
+            ret = disks._linux_disks()
+
+        assert ret == {"disks": ["sda", "sdb", "vda"], "SSDs": []}, ret