From 2548f022c511c228250625822dfe9c29425fd6a5 Mon Sep 17 00:00:00 2001 From: cid Date: Fri, 26 Apr 2024 11:46:14 +0100 Subject: [PATCH] Flexible IPMI credential persistence method configuration Instead of only file-based persistence which leaves files with credentials on the conductor disk for the duration of the session. User can now pass ``True`` to the ``store_cred_in_env`` parameter which instead stores IPMI password as an environment variable, still for the duration of the session, but limiting exposure to just the user session of ironic and anyone that has access to it. Defaults to ``False``. Closes-Bug: #2058749 Change-Id: Icd91e969e5c58bf42fc50958c3cd1acabd36ccdf --- doc/source/admin/drivers/ipmitool.rst | 22 + ironic/conf/ipmi.py | 5 + ironic/drivers/modules/ipmitool.py | 110 +- .../unit/drivers/modules/test_ipmitool.py | 1026 +++++++++++++++-- ...method_configuration-e5ed052576576d71.yaml | 6 + 5 files changed, 1015 insertions(+), 154 deletions(-) create mode 100644 releasenotes/notes/flexible_ipmi_credential_persistence_method_configuration-e5ed052576576d71.yaml diff --git a/doc/source/admin/drivers/ipmitool.rst b/doc/source/admin/drivers/ipmitool.rst index 6b472a3507..2b110f7396 100644 --- a/doc/source/admin/drivers/ipmitool.rst +++ b/doc/source/admin/drivers/ipmitool.rst @@ -82,6 +82,28 @@ with an IPMItool-based driver. For example:: --driver-info ipmi_username= \ --driver-info ipmi_password= + +Changing The Default IPMI Credential Persistence Method +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- ``store_cred_in_env``: :oslo.config:option:`ipmi.store_cred_in_env`. + +The `store_cred_in_env` configuration option allow users to switch +between file-based and environment variable persistence methods for +IPMI password. + +For the temporary file option, long lived IPMI sessions, such as those for +console support, leave files with credentials on the conductor disk for the +duration of the session. + +To switch to environment variable persistence, set the +``store_cred_in_env`` parameter to ``True`` in the configuration file: + +.. code-block:: ini + + [ipmi] + store_cred_in_env = True + Advanced configuration ====================== diff --git a/ironic/conf/ipmi.py b/ironic/conf/ipmi.py index 1ea9aa4a6e..873f69cd91 100644 --- a/ironic/conf/ipmi.py +++ b/ironic/conf/ipmi.py @@ -73,6 +73,11 @@ opts = [ 'additional debugging output. This is a separate ' 'option as ipmitool can log a substantial amount ' 'of misleading text when in this mode.')), + cfg.BoolOpt('store_cred_in_env', + default=False, + help=_('Boolean flag to determine IPMI password persistence ' + 'method. Defaults to False (file-based persistence). ' + )), cfg.ListOpt('cipher_suite_versions', default=[], help=_('List of possible cipher suites versions that can ' diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index e5259f7bcd..76059f900e 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -71,9 +71,9 @@ REQUIRED_PROPERTIES = { } OPTIONAL_PROPERTIES = { 'ipmi_password': _("password. Optional."), - 'ipmi_hex_kg_key': _('Kg key for IPMIv2 authentication. ' - 'The key is expected in hexadecimal format. ' - 'Optional.'), + 'ipmi_hex_kg_key': _("Kg key for IPMIv2 authentication. " + "The key is expected in hexadecimal format. " + "Optional."), 'ipmi_port': _("remote IPMI RMCP port. Optional."), 'ipmi_priv_level': _("privilege level; default is ADMINISTRATOR. One of " "%s. Optional.") % ', '.join(VALID_PRIV_LEVELS), @@ -280,37 +280,42 @@ def _console_pwfile_path(uuid): @contextlib.contextmanager -def _make_password_file(password): - """Makes a temporary file that contains the password. +def _prepare_ipmi_password(driver_info): + """Prepares the IPMI password by either setting it in the environment - :param password: the password - :returns: the absolute pathname of the temporary file + or creating a temporary file. + + :param driver_info: the ipmitool parameters for accessing a node. + :returns: the absolute pathname of the password :raises: PasswordFileFailedToCreate from creating or writing to the temporary file """ - f = None - try: - f = tempfile.NamedTemporaryFile(mode='w', dir=CONF.tempdir) - f.write(str(password)) - f.flush() - except (IOError, OSError) as exc: - if f is not None: - f.close() - raise exception.PasswordFileFailedToCreate(error=exc) - except Exception: - with excutils.save_and_reraise_exception(): + if CONF.ipmi.store_cred_in_env: + yield _persist_ipmi_password(driver_info) + else: + f = None + try: + f = tempfile.NamedTemporaryFile(mode='w', dir=CONF.tempdir) + f.write(str(driver_info['password'] or '\0')) + f.flush() + except (IOError, OSError) as exc: if f is not None: f.close() + raise exception.PasswordFileFailedToCreate(error=exc) + except Exception: + with excutils.save_and_reraise_exception(): + if f is not None: + f.close() - try: - # NOTE(jlvillal): This yield can not be in the try/except block above - # because an exception by the caller of this function would then get - # changed to a PasswordFileFailedToCreate exception which would mislead - # about the problem and its cause. - yield f.name - finally: - if f is not None: - f.close() + try: + # NOTE(jlvillal): This yield can not be in the try/except block + # above because an exception by the caller of this function would + # then get changed to a PasswordFileFailedToCreate exception which + # would mislead about the problem and its cause. + yield ('-f', f.name) + finally: + if f is not None: + f.close() def is_bridging_enabled(node): @@ -495,6 +500,25 @@ def _get_ipmitool_args(driver_info, pw_file=None): return args +def _persist_ipmi_password(driver_info): + """Persists IPMI password for passing to the ipmitool + + :param driver_info: driver info with the ipmitool parameters + """ + env = {} + if CONF.ipmi.store_cred_in_env: + password = driver_info.get('password') + if password: + env = {'IPMI_PASSWORD': password} + return '-E', env + + path = _console_pwfile_path(driver_info['uuid']) + pw_file = console_utils.make_persistent_password_file( + path, driver_info['password'] or '\0') + + return '-f', pw_file + + def _ipmitool_timing_args(): if not _is_option_supported('timing'): return [] @@ -644,10 +668,15 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None, # 'ipmitool' command will prompt password if there is no '-f' # option, we set it to '\0' to write a password file to support # empty password - with _make_password_file(driver_info['password'] or '\0') as pw_file: - cmd_args.append('-f') - cmd_args.append(pw_file) + + with _prepare_ipmi_password(driver_info) as (flag, env_path): + cmd_args.append(flag) + if CONF.ipmi.store_cred_in_env: + extra_args['env_variables'] = env_path + else: + cmd_args.append(env_path) cmd_args.extend(command.split(" ")) + try: out, err = utils.execute(*cmd_args, **extra_args) return out, err @@ -679,6 +708,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None, 'Error: %(error)s', {'node': driver_info['uuid'], 'cmd': e.cmd, 'error': e}) + finally: LAST_CMD_TIME[driver_info['address']] = time.time() @@ -1543,17 +1573,17 @@ class IPMIConsole(base.ConsoleInterface): created :raises: ConsoleSubprocessFailed when invoking the subprocess failed """ - path = _console_pwfile_path(driver_info['uuid']) - pw_file = console_utils.make_persistent_password_file( - path, driver_info['password'] or '\0') - ipmi_cmd = self._get_ipmi_cmd(driver_info, pw_file) - ipmi_cmd += ' sol activate' - try: - start_method(driver_info['uuid'], driver_info['port'], ipmi_cmd) - except (exception.ConsoleError, exception.ConsoleSubprocessFailed): - with excutils.save_and_reraise_exception(): - ironic_utils.unlink_without_raise(path) + cmd = self._get_ipmi_cmd(driver_info, pw_file=None) + cmd += ' sol activate' + start_method(driver_info['uuid'], driver_info['port'], cmd) + except (exception.ConsoleError, + exception.ConsoleSubprocessFailed) as e: + LOG.exception('IPMI Error while attempting "%(cmd)s" ' + 'for node %(node)s. Error: %(error)s', + {'node': driver_info['uuid'], + 'cmd': cmd, 'error': e}) + raise class IPMIShellinaboxConsole(IPMIConsole): diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index aa5da4697b..9ed9f5db8e 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -33,6 +33,8 @@ from unittest import mock import fixtures from ironic_lib import utils as ironic_utils from oslo_concurrency import processutils +from oslo_config import cfg +from oslo_config import fixture as cfg_fixture from oslo_utils import uuidutils from ironic.common import boot_devices @@ -356,8 +358,11 @@ awesome_password_filename = 'awesome_password_filename' @contextlib.contextmanager -def _make_password_file_stub(password): - yield awesome_password_filename +def _prepare_ipmi_password_stub(driver_info): + if CONF.ipmi.store_cred_in_env: + yield ipmi._persist_ipmi_password(driver_info) + else: + yield '-f', awesome_password_filename class IPMIToolPrivateMethodTestCaseMeta(type): @@ -528,65 +533,80 @@ class IPMIToolPrivateMethodTestCase( m.start() self.addCleanup(m.stop) - def _test__make_password_file(self, input_password, - exception_to_raise=None): + def _test__prepare_ipmi_password(self, info, exception_to_raise=None): pw_file = None try: - with ipmi._make_password_file(input_password) as pw_file: + with ipmi._prepare_ipmi_password(info) as (_, pw_file): if exception_to_raise is not None: raise exception_to_raise self.assertTrue(os.path.isfile(pw_file)) self.assertEqual(0o600, os.stat(pw_file)[stat.ST_MODE] & 0o777) with open(pw_file, "r") as f: password = f.read() - self.assertEqual(str(input_password), password) + self.assertEqual(str(info['password']), password) finally: if pw_file is not None: self.assertFalse(os.path.isfile(pw_file)) - def test__make_password_file_str_password(self): - self._test__make_password_file(self.info['password']) + def test__prepare_ipmi_password_str_password(self): + self._test__prepare_ipmi_password(self.info) - def test__make_password_file_with_numeric_password(self): - self._test__make_password_file(12345) + def test__prepare_ipmi_password_with_numeric_password(self): + info_copy = dict(self.info) + info_copy['password'] = 12345 + self._test__prepare_ipmi_password(self.info) - def test__make_password_file_caller_exception(self): + def test__prepare_ipmi_password_caller_exception(self): # Test caller raising exception + info_copy = dict(self.info) + info_copy['password'] = 12345 + result = self.assertRaises( ValueError, - self._test__make_password_file, - 12345, ValueError('we should fail')) + self._test__prepare_ipmi_password, + info_copy, ValueError('we should fail')) self.assertEqual('we should fail', str(result)) @mock.patch.object(tempfile, 'NamedTemporaryFile', new=mock.MagicMock(side_effect=OSError('Test Error'))) - def test__make_password_file_tempfile_known_exception(self): - # Test OSError exception in _make_password_file for + def test__prepare_ipmi_password_tempfile_known_exception(self): + # Test OSError exception in _prepare_ipmi_password for # tempfile.NamedTemporaryFile + info_copy = dict(self.info) + info_copy['password'] = 12345 + self.assertRaises( exception.PasswordFileFailedToCreate, - self._test__make_password_file, 12345) + self._test__prepare_ipmi_password, info_copy) @mock.patch.object( tempfile, 'NamedTemporaryFile', new=mock.MagicMock(side_effect=OverflowError('Test Error'))) - def test__make_password_file_tempfile_unknown_exception(self): - # Test exception in _make_password_file for tempfile.NamedTemporaryFile + def test__prepare_ipmi_password_tempfile_unknown_exception(self): + # Test exception in _prepare_ipmi_password for + # tempfile.NamedTemporaryFile + info_copy = dict(self.info) + info_copy['password'] = 12345 + result = self.assertRaises( OverflowError, - self._test__make_password_file, 12345) + self._test__prepare_ipmi_password, info_copy) self.assertEqual('Test Error', str(result)) - def test__make_password_file_write_exception(self): - # Test exception in _make_password_file for write() + def test__prepare_ipmi_password_write_exception(self): + # Test exception in _prepare_ipmi_password for write() mock_namedtemp = mock.mock_open(mock.MagicMock(name='JLV')) with mock.patch('tempfile.NamedTemporaryFile', mock_namedtemp): mock_filehandle = mock_namedtemp.return_value mock_write = mock_filehandle.write mock_write.side_effect = OSError('Test 2 Error') + + info_copy = dict(self.info) + info_copy['password'] = 12345 + self.assertRaises( exception.PasswordFileFailedToCreate, - self._test__make_password_file, 12345) + self._test__prepare_ipmi_password, info_copy) def test__parse_driver_info(self): # make sure we get back the expected things @@ -895,7 +915,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(driver_info['port'], 10001) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_first_call_to_address(self, mock_exec, mock_support): @@ -921,7 +942,8 @@ class IPMIToolPrivateMethodTestCase( self.assertFalse(self.mock_sleep.called) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_second_call_to_address_sleep( self, mock_exec, mock_support): @@ -960,7 +982,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_with(*args[1]) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_second_call_to_address_no_sleep( self, mock_exec, mock_support): @@ -1001,7 +1024,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_with(*args[1]) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_two_calls_to_diff_address( self, mock_exec, mock_support): @@ -1040,7 +1064,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_with(*args[1]) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_without_timing( self, mock_exec, mock_support): @@ -1064,7 +1089,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_timing( self, mock_exec, mock_support): @@ -1092,7 +1118,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_ironic_retries( self, mock_exec, mock_support): @@ -1120,7 +1147,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_timeout( self, mock_exec, mock_support): @@ -1147,7 +1175,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args, timeout=60) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_ironic_retries_multiple( self, mock_exec, mock_support): @@ -1176,7 +1205,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(3, mock_exec.call_count) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_without_username( self, mock_exec, mock_support): @@ -1200,7 +1230,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_empty_username( self, mock_exec, mock_support): @@ -1224,63 +1255,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object( - ipmi, '_make_password_file', wraps=_make_password_file_stub) - @mock.patch.object(utils, 'execute', autospec=True) - def test__exec_ipmitool_without_password(self, mock_exec, - _make_password_file_mock, - mock_support): - # An undefined password is treated the same as an empty password and - # will cause a NULL (\0) password to be used""" - self.info['password'] = None - args = [ - 'ipmitool', - '-I', 'lanplus', - '-H', self.info['address'], - '-L', self.info['priv_level'], - '-U', self.info['username'], - '-v', - '-f', awesome_password_filename, - 'A', 'B', 'C', - ] - - mock_support.return_value = False - mock_exec.return_value = (None, None) - ipmi._exec_ipmitool(self.info, 'A B C') - mock_support.assert_called_once_with('timing') - mock_exec.assert_called_once_with(*args) - _make_password_file_mock.assert_called_once_with('\0') - - @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object( - ipmi, '_make_password_file', wraps=_make_password_file_stub) - @mock.patch.object(utils, 'execute', autospec=True) - def test__exec_ipmitool_with_empty_password(self, mock_exec, - _make_password_file_mock, - mock_support): - # An empty password is treated the same as an undefined password and - # will cause a NULL (\0) password to be used""" - self.info['password'] = "" - args = [ - 'ipmitool', - '-I', 'lanplus', - '-H', self.info['address'], - '-L', self.info['priv_level'], - '-U', self.info['username'], - '-v', - '-f', awesome_password_filename, - 'A', 'B', 'C', - ] - - mock_support.return_value = False - mock_exec.return_value = (None, None) - ipmi._exec_ipmitool(self.info, 'A B C') - mock_support.assert_called_once_with('timing') - mock_exec.assert_called_once_with(*args) - _make_password_file_mock.assert_called_once_with('\0') - - @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_dual_bridging(self, mock_exec, @@ -1316,7 +1292,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(expected, mock_support.call_args_list) mock_exec.assert_called_once_with(*args) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_single_bridging(self, @@ -1355,7 +1332,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_exception(self, mock_exec, mock_support): args = [ @@ -1381,7 +1359,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(1, mock_exec.call_count) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_IPMI_version_1_5( self, mock_exec, mock_support): @@ -1405,7 +1384,8 @@ class IPMIToolPrivateMethodTestCase( mock_exec.assert_called_once_with(*args) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_port(self, mock_exec, mock_support): self.info['dest_port'] = '1623' @@ -1432,7 +1412,8 @@ class IPMIToolPrivateMethodTestCase( self.assertFalse(self.mock_sleep.called) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite(self, mock_exec, mock_support): self.info['cipher_suite'] = '3' @@ -1459,7 +1440,8 @@ class IPMIToolPrivateMethodTestCase( self.assertFalse(self.mock_sleep.called) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite_error_noconfig( @@ -1506,7 +1488,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(0, mock_check_cs.call_count) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite_set_with_error_noconfig( @@ -1555,7 +1538,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(0, mock_check_cs.call_count) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite_set_with_error_config( @@ -1604,7 +1588,8 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(0, mock_check_cs.call_count) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_try_different_cipher_suite( @@ -1748,7 +1733,8 @@ class IPMIToolPrivateMethodTestCase( self.assertTrue(ipmi.check_cipher_suite_errors(valid_err)) @mock.patch.object(ipmi, '_is_option_supported', autospec=True) - @mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_with_check_exit_code(self, mock_exec, mock_support): @@ -1922,6 +1908,812 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(['-R', '7', '-N', '1'], ipmi._ipmitool_timing_args()) +class IPMIToolPrivateMethodOnEnvPersistenceTestCase( + Base, + metaclass=IPMIToolPrivateMethodTestCaseMeta): + + def setUp(self): + self.conf_fixture = self.useFixture(cfg_fixture.Config(cfg.CONF)) + + # Override the 'store_cred_in_env' configuration option + self.conf_fixture.config(store_cred_in_env=True, group='ipmi') + + super(IPMIToolPrivateMethodOnEnvPersistenceTestCase, + self).setUp() + + self.env = {'IPMI_PASSWORD': self.info['password']} + + self._mock_system_random_distribution() + + mock_sleep_fixture = self.useFixture( + fixtures.MockPatchObject(time, 'sleep', autospec=True)) + self.mock_sleep = mock_sleep_fixture.mock + + # NOTE(etingof): besides the conventional unittest methods that follow, + # the metaclass will inject some more `test_` methods aimed at testing + # the handling of specific errors potentially returned by the `ipmitool` + + def _mock_system_random_distribution(self): + # random.SystemRandom with gauss distribution is used by oslo_service's + # BackoffLoopingCall, it multiplies default interval (equals to 1) by + # 2 * return_value, so if you want BackoffLoopingCall to "sleep" for + # 1 second, return_value should be 0.5. + m = mock.patch.object(random.SystemRandom, 'gauss', return_value=0.5, + autospec=True) + m.start() + self.addCleanup(m.stop) + + def _test__prepare_ipmi_password(self, info, exception_to_raise=None): + with ipmi._prepare_ipmi_password(info) as (_, env): + if exception_to_raise is not None: + raise exception_to_raise + password = env.get('IPMI_PASSWORD') + self.assertEqual(str(info['password']), password) + + def test__prepare_ipmi_password_str_password(self): + self._test__prepare_ipmi_password(self.info) + + def test__prepare_ipmi_password_with_numeric_password(self): + info_copy = dict(self.info) + info_copy['password'] = 12345 + self._test__prepare_ipmi_password(self.info) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_first_call_to_address(self, mock_exec, + mock_support): + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + self.assertFalse(self.mock_sleep.called) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_second_call_to_address_sleep( + self, mock_exec, mock_support): + ipmi.LAST_CMD_TIME = {} + args = [[ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ], [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'D', 'E', 'F', + ]] + + expected = [mock.call('timing'), + mock.call('timing')] + mock_support.return_value = False + mock_exec.side_effect = [(None, None), (None, None)] + + ipmi._exec_ipmitool(self.info, 'A B C') + mock_exec.assert_called_with(*args[0], env_variables=self.env) + + ipmi._exec_ipmitool(self.info, 'D E F') + self.assertTrue(self.mock_sleep.called) + self.assertEqual(expected, mock_support.call_args_list) + mock_exec.assert_called_with(*args[1], env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_second_call_to_address_no_sleep( + self, mock_exec, mock_support): + ipmi.LAST_CMD_TIME = {} + args = [[ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ], [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'D', 'E', 'F', + ]] + + expected = [mock.call('timing'), + mock.call('timing')] + mock_support.return_value = False + mock_exec.side_effect = [(None, None), (None, None)] + + ipmi._exec_ipmitool(self.info, 'A B C') + mock_exec.assert_called_with(*args[0], env_variables=self.env) + # act like enough time has passed + ipmi.LAST_CMD_TIME[self.info['address']] = ( + time.time() - CONF.ipmi.min_command_interval) + ipmi._exec_ipmitool(self.info, 'D E F') + self.assertFalse(self.mock_sleep.called) + self.assertEqual(expected, mock_support.call_args_list) + mock_exec.assert_called_with(*args[1], env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_two_calls_to_diff_address( + self, mock_exec, mock_support): + ipmi.LAST_CMD_TIME = {} + args = [[ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ], [ + 'ipmitool', + '-I', 'lanplus', + '-H', '127.127.127.127', + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'D', 'E', 'F', + ]] + + expected = [mock.call('timing'), + mock.call('timing')] + mock_support.return_value = False + mock_exec.side_effect = [(None, None), (None, None)] + + ipmi._exec_ipmitool(self.info, 'A B C') + mock_exec.assert_called_with(*args[0], env_variables=self.env) + self.info['address'] = '127.127.127.127' + ipmi._exec_ipmitool(self.info, 'D E F') + self.assertFalse(self.mock_sleep.called) + self.assertEqual(expected, mock_support.call_args_list) + mock_exec.assert_called_with(*args[1], env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_without_timing( + self, mock_exec, mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_timing( + self, mock_exec, mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-R', '7', + '-N', '5', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = True + mock_exec.return_value = (None, None) + + self.config(use_ipmitool_retries=True, group='ipmi') + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_ironic_retries( + self, mock_exec, mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-R', '1', + '-N', '5', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = True + mock_exec.return_value = (None, None) + + self.config(use_ipmitool_retries=False, group='ipmi') + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_timeout( + self, mock_exec, mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-R', '7', + '-N', '5', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = True + mock_exec.return_value = (None, None) + + self.config(use_ipmitool_retries=True, group='ipmi') + ipmi._exec_ipmitool(self.info, 'A B C', kill_on_timeout=True) + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env, + timeout=60) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_without_username( + self, mock_exec, mock_support): + # An undefined username is treated the same as an empty username and + # will cause no user (-U) to be specified. + self.info['username'] = None + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C') + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_empty_username( + self, mock_exec, mock_support): + # An empty username is treated the same as an undefined username and + # will cause no user (-U) to be specified. + self.info['username'] = "" + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C') + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_dual_bridging(self, + mock_exec, + mock_support): + + single_bridge_info = dict(BRIDGE_INFO_DICT) + single_bridge_info['ipmi_store_cred_in_env'] = True + node = obj_utils.get_test_node(self.context, + driver_info=single_bridge_info) + # when support for dual bridge command is called returns True + mock_support.return_value = True + info = ipmi._parse_driver_info(node) + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', info['address'], + '-L', info['priv_level'], + '-U', info['username'], + '-m', info['local_address'], + '-B', info['transit_channel'], + '-T', info['transit_address'], + '-b', info['target_channel'], + '-t', info['target_address'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + expected = [mock.call('dual_bridge'), + mock.call('timing')] + # When support for timing command is called returns False + mock_support.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(info, 'A B C') + self.assertEqual(expected, mock_support.call_args_list) + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_single_bridging(self, + mock_exec, + mock_support): + single_bridge_info = dict(BRIDGE_INFO_DICT) + single_bridge_info['ipmi_bridging'] = 'single' + single_bridge_info['ipmi_store_cred_in_env'] = True + node = obj_utils.get_test_node(self.context, + driver_info=single_bridge_info) + # when support for single bridge command is called returns True + mock_support.return_value = True + info = ipmi._parse_driver_info(node) + info['transit_channel'] = info['transit_address'] = None + + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', info['address'], + '-L', info['priv_level'], + '-U', info['username'], + '-m', info['local_address'], + '-b', info['target_channel'], + '-t', info['target_address'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + expected = [mock.call('single_bridge'), + mock.call('timing')] + # When support for timing command is called returns False + mock_support.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(info, 'A B C') + self.assertEqual(expected, mock_support.call_args_list) + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_exception(self, mock_exec, mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + self.config(use_ipmitool_retries=True, group='ipmi') + + mock_support.return_value = False + mock_exec.side_effect = processutils.ProcessExecutionError("x") + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + self.assertEqual(1, mock_exec.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_IPMI_version_1_5( + self, mock_exec, mock_support): + self.info['protocol_version'] = '1.5' + # Assert it uses "-I lan" (1.5) instead of "-I lanplus" (2.0) + args = [ + 'ipmitool', + '-I', 'lan', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C') + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_port(self, mock_exec, mock_support): + self.info['dest_port'] = '1623' + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-p', '1623', + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + self.assertFalse(self.mock_sleep.called) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite(self, mock_exec, mock_support): + self.info['cipher_suite'] = '3' + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-C', '3', + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.return_value = (None, None) + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env) + self.assertFalse(self.mock_sleep.called) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite_error_noconfig( + self, mock_exec, mock_check_cs, mock_support): + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ + 'Unable to establish IPMI v2 / RMCP+ session\n' + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + self.config(cipher_suite_versions=[], group='ipmi') + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + ] + mock_check_cs.return_value = False + + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + calls = [mock.call(*args, env_variables=self.env), + mock.call(*args, env_variables=self.env)] + mock_exec.assert_has_calls(calls) + self.assertEqual(2, mock_exec.call_count) + self.assertEqual(0, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite_set_with_error_noconfig( + self, mock_exec, mock_check_cs, mock_support): + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ + 'Unable to establish IPMI v2 / RMCP+ session\n' + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + self.config(cipher_suite_versions=[], group='ipmi') + ipmi.LAST_CMD_TIME = {} + self.info['cipher_suite'] = '17' + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-C', '17', + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + ] + mock_check_cs.return_value = False + + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + calls = [mock.call(*args, env_variables=self.env), + mock.call(*args, env_variables=self.env)] + mock_exec.assert_has_calls(calls) + self.assertEqual(2, mock_exec.call_count) + self.assertEqual(0, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_cipher_suite_set_with_error_config( + self, mock_exec, mock_check_cs, mock_support): + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ + 'Unable to establish IPMI v2 / RMCP+ session\n' + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + self.config(cipher_suite_versions=[0, 1, 2, 3], group='ipmi') + ipmi.LAST_CMD_TIME = {} + self.info['cipher_suite'] = '17' + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-C', '17', + '-v', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + ] + mock_check_cs.return_value = False + + self.assertRaises(processutils.ProcessExecutionError, + ipmi._exec_ipmitool, + self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + calls = [mock.call(*args, env_variables=self.env), + mock.call(*args, env_variables=self.env)] + mock_exec.assert_has_calls(calls) + self.assertEqual(2, mock_exec.call_count) + self.assertEqual(0, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(ipmi, 'check_cipher_suite_errors', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_try_different_cipher_suite( + self, mock_exec, mock_check_cs, mock_support): + + self.config(min_command_interval=1, group='ipmi') + self.config(command_retry_timeout=4, group='ipmi') + self.config(use_ipmitool_retries=False, group='ipmi') + cs_list = ['0', '1', '2', '3', '17'] + self.config(cipher_suite_versions=cs_list, group='ipmi') + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ + 'Unable to establish IPMI v2 / RMCP+ session\n' + unsupported_error = 'Unsupported cipher suite ID : 17\n\n' \ + 'Error: Unable to establish IPMI v2 / RMCP+ session\n' + ipmi.LAST_CMD_TIME = {} + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + + args_cs17 = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-C', '17', + '-E', + 'A', 'B', 'C', + ] + + args_cs3 = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-C', '3', + '-E', + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_exec.side_effect = [ + processutils.ProcessExecutionError( + stdout='', + stderr=no_matching_error), + processutils.ProcessExecutionError( + stdout='', + stderr=unsupported_error), + processutils.ProcessExecutionError(stderr="Unknown"), + ('', ''), + ] + mock_check_cs.side_effect = [True, True, False] + + ipmi._exec_ipmitool(self.info, 'A B C') + + mock_support.assert_called_once_with('timing') + + execute_calls = [mock.call(*args, env_variables=self.env), + mock.call(*args_cs17, env_variables=self.env), + mock.call(*args_cs3, env_variables=self.env), + mock.call(*args_cs3, env_variables=self.env)] + mock_exec.assert_has_calls(execute_calls) + self.assertEqual(4, mock_exec.call_count) + check_calls = [mock.call(no_matching_error), + mock.call(unsupported_error), mock.call('Unknown')] + mock_check_cs.assert_has_calls(check_calls) + self.assertEqual(3, mock_check_cs.call_count) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_prepare_ipmi_password', + _prepare_ipmi_password_stub) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_check_exit_code(self, mock_exec, + mock_support): + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-v', + '-E', + 'A', 'B', 'C', + ] + mock_support.return_value = False + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C', check_exit_code=[0, 1]) + mock_support.assert_called_once_with('timing') + mock_exec.assert_called_once_with(*args, env_variables=self.env, + check_exit_code=[0, 1]) + + class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, "_parse_driver_info", autospec=True) @@ -3369,23 +4161,20 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): console_utils.start_shellinabox_console) mock_start.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) - @mock.patch.object(console_utils, 'make_persistent_password_file', - autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test__start_console_empty_password(self, mock_start, mock_pass): + def test__start_console_empty_password(self, mock_start): driver_info = self.node.driver_info del driver_info['ipmi_password'] self.node.driver_info = driver_info self.node.save() - with task_manager.acquire(self.context, - self.node.uuid) as task: + with task_manager.acquire(self.context, self.node.uuid) as task: driver_info = ipmi._parse_driver_info(task.node) self.console._start_console( - driver_info, console_utils.start_shellinabox_console) + driver_info, + console_utils.start_shellinabox_console) - mock_pass.assert_called_once_with(mock.ANY, '\0') mock_start.assert_called_once_with(self.info['uuid'], self.info['port'], mock.ANY) @@ -3534,12 +4323,15 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): console_utils.start_socat_console) mock_alloc.assert_called_once_with(mock.ANY, host='2001:dead:beef::1') + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(ipmi.IPMISocatConsole, '_get_ipmi_cmd', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console(self, mock_start, mock_ipmi_cmd): + def test__start_console(self, mock_start, mock_ipmi_cmd, mock_exec): mock_start.return_value = None + mock_exec.return_value = ('output', 'error') + with task_manager.acquire(self.context, self.node.uuid) as task: driver_info = ipmi._parse_driver_info(task.node) @@ -3552,12 +4344,15 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): mock_ipmi_cmd.assert_called_once_with(self.console, driver_info, mock.ANY) + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console_fail(self, mock_start): + def test__start_console_fail(self, mock_start, mock_exec): mock_start.side_effect = exception.ConsoleSubprocessFailed( error='error') + mock_exec.return_value = ('output', 'error') + with task_manager.acquire(self.context, self.node.uuid) as task: driver_info = ipmi._parse_driver_info(task.node) @@ -3570,11 +4365,14 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): self.info['port'], mock.ANY) + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console_fail_nodir(self, mock_start): + def test__start_console_fail_nodir(self, mock_start, mock_exec): mock_start.side_effect = exception.ConsoleError() + mock_exec.return_value = ('output', 'error') + with task_manager.acquire(self.context, self.node.uuid) as task: driver_info = ipmi._parse_driver_info(task.node) @@ -3584,23 +4382,23 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): console_utils.start_socat_console) mock_start.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) - @mock.patch.object(console_utils, 'make_persistent_password_file', - autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console_empty_password(self, mock_start, mock_pass): + def test__start_console_empty_password(self, mock_start, mock_exec): driver_info = self.node.driver_info del driver_info['ipmi_password'] self.node.driver_info = driver_info self.node.save() + mock_exec.return_value = ('output', 'error') + with task_manager.acquire(self.context, self.node.uuid) as task: driver_info = ipmi._parse_driver_info(task.node) self.console._start_console( driver_info, console_utils.start_socat_console) - mock_pass.assert_called_once_with(mock.ANY, '\0') mock_start.assert_called_once_with(self.info['uuid'], self.info['port'], mock.ANY) diff --git a/releasenotes/notes/flexible_ipmi_credential_persistence_method_configuration-e5ed052576576d71.yaml b/releasenotes/notes/flexible_ipmi_credential_persistence_method_configuration-e5ed052576576d71.yaml new file mode 100644 index 0000000000..1ecba03021 --- /dev/null +++ b/releasenotes/notes/flexible_ipmi_credential_persistence_method_configuration-e5ed052576576d71.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds a new configuration option ``store_cred_in_env`` to allow + switching between file-based and environment variable persistence for + IPMI credentials. Defaults to ``False``.