From 836b004d4521ce83027bfb70068182560c1e8278 Mon Sep 17 00:00:00 2001 From: Alexandru Dan Duna Date: Tue, 30 Sep 2025 16:10:50 +0300 Subject: [PATCH 1/4] Temporarily move disks and iscsi tests for cherry-picking from the Salt 3006.x branch --- tests/{ => pytests}/unit/grains/test_disks.py | 0 tests/{ => pytests}/unit/grains/test_iscsi.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/{ => pytests}/unit/grains/test_disks.py (100%) rename tests/{ => pytests}/unit/grains/test_iscsi.py (100%) diff --git a/tests/unit/grains/test_disks.py b/tests/pytests/unit/grains/test_disks.py similarity index 100% rename from tests/unit/grains/test_disks.py rename to tests/pytests/unit/grains/test_disks.py diff --git a/tests/unit/grains/test_iscsi.py b/tests/pytests/unit/grains/test_iscsi.py similarity index 100% rename from tests/unit/grains/test_iscsi.py rename to tests/pytests/unit/grains/test_iscsi.py From e0a7ba6b27c1b94f9ed2c126e83acd6d5b807b98 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 14 Oct 2024 16:11:55 -0600 Subject: [PATCH 2/4] Remove wmic usage from disk grains on Windows # Conflicts: # salt/grains/disks.py # salt/modules/cmdmod.py # tests/pytests/unit/grains/test_disks.py --- changelog/66959.fixed.md | 2 + salt/grains/disks.py | 73 +++++------- salt/utils/path.py | 2 +- tests/pytests/unit/grains/test_disks.py | 149 +++++++++++++++--------- 4 files changed, 131 insertions(+), 95 deletions(-) create mode 100644 changelog/66959.fixed.md diff --git a/changelog/66959.fixed.md b/changelog/66959.fixed.md new file mode 100644 index 000000000000..ad9878c2577a --- /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 38fb7755d853..8a2dca5c3fb8 100644 --- a/salt/grains/disks.py +++ b/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 diff --git a/salt/utils/path.py b/salt/utils/path.py index 38c5a08ae927..cc30654b903e 100644 --- a/salt/utils/path.py +++ b/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: diff --git a/tests/pytests/unit/grains/test_disks.py b/tests/pytests/unit/grains/test_disks.py index 9b23f1e8bc85..a31e01a7a87f 100644 --- a/tests/pytests/unit/grains/test_disks.py +++ b/tests/pytests/unit/grains/test_disks.py @@ -1,10 +1,8 @@ -# -*- coding: utf-8 -*- -''' +""" :codeauthor: :email:`Shane Lee ` -''' +""" # 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 From 8c6a7b70d41fde0afc94547e0744dbbe7a7b8c98 Mon Sep 17 00:00:00 2001 From: twangboy Date: Tue, 15 Oct 2024 11:10:51 -0600 Subject: [PATCH 3/4] Remove wmic from iscsi grains # Conflicts: # salt/grains/iscsi.py # tests/pytests/unit/grains/test_iscsi.py --- changelog/66959.fixed.md | 4 +- salt/grains/iscsi.py | 31 +++--- tests/pytests/unit/grains/test_iscsi.py | 119 ++++++++++++++---------- 3 files changed, 88 insertions(+), 66 deletions(-) diff --git a/changelog/66959.fixed.md b/changelog/66959.fixed.md index ad9878c2577a..69e40d664798 100644 --- a/changelog/66959.fixed.md +++ b/changelog/66959.fixed.md @@ -1,2 +1,2 @@ -Removed the usage of wmic to get the disk grains for Windows. The wmic binary is -being deprecated. +Removed the usage of wmic to get the disk and iscsi grains for Windows. The wmic +binary is being deprecated. diff --git a/salt/grains/iscsi.py b/salt/grains/iscsi.py index 56b6601439ad..a14586794f49 100644 --- a/salt/grains/iscsi.py +++ b/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 diff --git a/tests/pytests/unit/grains/test_iscsi.py b/tests/pytests/unit/grains/test_iscsi.py index 168ea0746e42..720b618653bf 100644 --- a/tests/pytests/unit/grains/test_iscsi.py +++ b/tests/pytests/unit/grains/test_iscsi.py @@ -9,85 +9,108 @@ # Import Salt Testing Libs from tests.support.unit import TestCase -from tests.support.mock import ( - patch, - mock_open, - MagicMock, -) +from tests.support.mock import MagicMock, mock_open, patch # Import Salt Libs import salt.grains.iscsi as iscsi - class IscsiGrainsTestCase(TestCase): ''' Test cases for iscsi grains ''' - def test_windows_iscsi_iqn_grains(self): - cmd_run_mock = MagicMock( - return_value={'stdout': 'iSCSINodeName\n' - 'iqn.1991-05.com.microsoft:simon-x1\n'} - ) - _grains = {} - with patch('salt.utils.path.which', MagicMock(return_value=True)): - with patch('salt.modules.cmdmod.run_all', cmd_run_mock): - _grains['iscsi_iqn'] = iscsi._windows_iqn() + def test_windows_iscsi_iqn_grains_empty(): + nodes_dict = {} + cmd_powershell_mock = MagicMock(return_value=nodes_dict) + with patch("salt.modules.cmdmod.powershell", cmd_powershell_mock): + result = iscsi._windows_iqn() + expected = [] + assert result == expected + + + def test_windows_iscsi_iqn_grains_single(): + nodes_dict = {"NodeAddress": "iqn.1991-05.com.microsoft:simon-x1"} + cmd_powershell_mock = MagicMock(return_value=nodes_dict) + with patch("salt.modules.cmdmod.powershell", cmd_powershell_mock): + result = iscsi._windows_iqn() + expected = ["iqn.1991-05.com.microsoft:simon-x1"] + assert result == expected + - self.assertEqual(_grains.get('iscsi_iqn'), - ['iqn.1991-05.com.microsoft:simon-x1']) + def test_windows_iscsi_iqn_grains_multiple(): + nodes_list = [ + {"NodeAddress": "iqn.1991-05.com.microsoft:simon-x1"}, + {"NodeAddress": "iqn.1991-05.com.microsoft:simon-x2"}, + {"NodeAddress": "iqn.1991-05.com.microsoft:simon-x3"}, + ] + cmd_powershell_mock = MagicMock(return_value=nodes_list) + with patch("salt.modules.cmdmod.powershell", cmd_powershell_mock): + result = iscsi._windows_iqn() + expected = [ + "iqn.1991-05.com.microsoft:simon-x1", + "iqn.1991-05.com.microsoft:simon-x2", + "iqn.1991-05.com.microsoft:simon-x3", + ] + assert result == expected - def test_aix_iscsi_iqn_grains(self): + + def test_aix_iscsi_iqn_grains(): cmd_run_mock = MagicMock( - return_value='initiator_name iqn.localhost.hostid.7f000001' + return_value="initiator_name iqn.localhost.hostid.7f000001" ) _grains = {} - with patch('salt.modules.cmdmod.run', cmd_run_mock): - _grains['iscsi_iqn'] = iscsi._aix_iqn() + with patch("salt.modules.cmdmod.run", cmd_run_mock): + _grains["iscsi_iqn"] = iscsi._aix_iqn() + + assert _grains.get("iscsi_iqn") == ["iqn.localhost.hostid.7f000001"] - self.assertEqual(_grains.get('iscsi_iqn'), - ['iqn.localhost.hostid.7f000001']) - def test_linux_iscsi_iqn_grains(self): - _iscsi_file = textwrap.dedent('''\ + def test_linux_iscsi_iqn_grains(): + _iscsi_file = textwrap.dedent( + """\ ## DO NOT EDIT OR REMOVE THIS FILE! ## If you remove this file, the iSCSI daemon will not start. ## If you change the InitiatorName, existing access control lists ## may reject this initiator. The InitiatorName must be unique ## for each iSCSI initiator. Do NOT duplicate iSCSI InitiatorNames. InitiatorName=iqn.1993-08.org.debian:01:d12f7aba36 - ''') + """ + ) - with patch('salt.utils.files.fopen', mock_open(read_data=_iscsi_file)): + with patch("salt.utils.files.fopen", mock_open(read_data=_iscsi_file)): iqn = iscsi._linux_iqn() assert isinstance(iqn, list) assert len(iqn) == 1 - assert iqn == ['iqn.1993-08.org.debian:01:d12f7aba36'] + assert iqn == ["iqn.1993-08.org.debian:01:d12f7aba36"] - @patch('salt.utils.files.fopen', MagicMock(side_effect=IOError(errno.EPERM, - 'The cables are not the same length.'))) - @patch('salt.grains.iscsi.log', MagicMock()) - def test_linux_iqn_non_root(self): - ''' + + def test_linux_iqn_non_root(): + """ Test if linux_iqn is running on salt-master as non-root and handling access denial properly. :return: - ''' - assert iscsi._linux_iqn() == [] - iscsi.log.debug.assert_called() - assert 'Error while accessing' in iscsi.log.debug.call_args[0][0] - assert 'cables are not the same' in iscsi.log.debug.call_args[0][2].strerror - assert iscsi.log.debug.call_args[0][2].errno == errno.EPERM - assert iscsi.log.debug.call_args[0][1] == '/etc/iscsi/initiatorname.iscsi' - - @patch('salt.utils.files.fopen', MagicMock(side_effect=IOError(errno.ENOENT, ''))) - @patch('salt.grains.iscsi.log', MagicMock()) - def test_linux_iqn_no_iscsii_initiator(self): - ''' + """ + with patch( + "salt.utils.files.fopen", + side_effect=IOError(errno.EPERM, "The cables are not the same length."), + ): + with patch("salt.grains.iscsi.log"): + assert iscsi._linux_iqn() == [] + iscsi.log.debug.assert_called() + assert "Error while accessing" in iscsi.log.debug.call_args[0][0] + assert "cables are not the same" in iscsi.log.debug.call_args[0][2].strerror + assert iscsi.log.debug.call_args[0][2].errno == errno.EPERM + assert iscsi.log.debug.call_args[0][1] == "/etc/iscsi/initiatorname.iscsi" + + + def test_linux_iqn_no_iscsii_initiator(): + """ Test if linux_iqn is running on salt-master as root. iscsii initiator is not there accessible or is not supported. :return: - ''' - assert iscsi._linux_iqn() == [] - iscsi.log.debug.assert_not_called() + """ + with patch("salt.utils.files.fopen", side_effect=IOError(errno.ENOENT, "")): + with patch("salt.grains.iscsi.log"): + assert iscsi._linux_iqn() == [] + iscsi.log.debug.assert_not_called() From 88e732dead820030071b39597a0875ef10aa07d3 Mon Sep 17 00:00:00 2001 From: Alexandru Dan Duna Date: Wed, 1 Oct 2025 12:36:31 +0300 Subject: [PATCH 4/4] Move disks and iscsi tests back after cherry-picks --- tests/{pytests => }/unit/grains/test_disks.py | 0 tests/{pytests => }/unit/grains/test_iscsi.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/{pytests => }/unit/grains/test_disks.py (100%) rename tests/{pytests => }/unit/grains/test_iscsi.py (100%) diff --git a/tests/pytests/unit/grains/test_disks.py b/tests/unit/grains/test_disks.py similarity index 100% rename from tests/pytests/unit/grains/test_disks.py rename to tests/unit/grains/test_disks.py diff --git a/tests/pytests/unit/grains/test_iscsi.py b/tests/unit/grains/test_iscsi.py similarity index 100% rename from tests/pytests/unit/grains/test_iscsi.py rename to tests/unit/grains/test_iscsi.py