diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 027a6f3ad4..01edf16930 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -269,6 +269,13 @@ def _parse_driver_info(node): protocol_version = str(info.get('ipmi_protocol_version', '2.0')) force_boot_device = info.get('ipmi_force_boot_device', False) + if not username: + LOG.warning(_LW('ipmi_username is not defined or empty for node %s: ' + 'NULL user will be utilized.') % node.uuid) + if not password: + LOG.warning(_LW('ipmi_password is not defined or empty for node %s: ' + 'NULL password will be utilized.') % node.uuid) + if protocol_version not in VALID_PROTO_VERSIONS: valid_versions = ', '.join(VALID_PROTO_VERSIONS) raise exception.InvalidParameterValue(_( diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index d2ad41a87c..251de52db6 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -617,6 +617,33 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): ret = ipmi._parse_driver_info(node) self.assertEqual(623, ret['dest_port']) + @mock.patch.object(ipmi.LOG, 'warning', spec_set=True, autospec=True) + def test__parse_driver_info_undefined_credentials( + self, mock_log, mock_sleep): + info = dict(INFO_DICT) + del info['ipmi_username'] + del info['ipmi_password'] + node = obj_utils.get_test_node(self.context, driver_info=info) + ipmi._parse_driver_info(node) + calls = [ + mock.call(u'ipmi_username is not defined or empty for node ' + u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123: NULL user will ' + u'be utilized.'), + mock.call(u'ipmi_password is not defined or empty for node ' + u'1be26c0b-03f2-4d2e-ae87-c02d7f33c123: NULL password ' + u'will be utilized.'), + ] + mock_log.assert_has_calls(calls) + + @mock.patch.object(ipmi.LOG, 'warning', spec_set=True, autospec=True) + def test__parse_driver_info_have_credentials( + self, mock_log, mock_sleep): + """Ensure no warnings generated if have credentials""" + info = dict(INFO_DICT) + node = obj_utils.get_test_node(self.context, driver_info=info) + ipmi._parse_driver_info(node) + self.assertFalse(mock_log.called) + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(ipmi, '_make_password_file', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) @@ -845,6 +872,8 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_without_username( self, mock_exec, mock_pwf, mock_support, mock_sleep): + # An undefined username is treated the same as an empty username and + # will cause no user (-U) to be specified. self.info['username'] = None pw_file_handle = tempfile.NamedTemporaryFile() pw_file = pw_file_handle.name @@ -866,6 +895,94 @@ class IPMIToolPrivateMethodTestCase(db_base.DbTestCase): self.assertTrue(mock_pwf.called) mock_exec.assert_called_once_with(*args) + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_empty_username( + self, mock_exec, mock_pwf, mock_support, mock_sleep): + # An empty username is treated the same as an undefined username and + # will cause no user (-U) to be specified. + self.info['username'] = "" + pw_file_handle = tempfile.NamedTemporaryFile() + pw_file = pw_file_handle.name + file_handle = open(pw_file, "w") + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-f', file_handle, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_pwf.return_value = file_handle + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C') + mock_support.assert_called_once_with('timing') + self.assertTrue(mock_pwf.called) + mock_exec.assert_called_once_with(*args) + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_without_password( + self, mock_exec, mock_pwf, mock_support, mock_sleep): + # 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 + pw_file_handle = tempfile.NamedTemporaryFile() + pw_file = pw_file_handle.name + file_handle = open(pw_file, "w") + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-f', file_handle, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_pwf.return_value = file_handle + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C') + mock_support.assert_called_once_with('timing') + self.assertTrue(mock_pwf.called) + mock_exec.assert_called_once_with(*args) + mock_pwf.assert_called_once_with('\0') + + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) + @mock.patch.object(ipmi, '_make_password_file', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test__exec_ipmitool_with_empty_password( + self, mock_exec, mock_pwf, mock_support, mock_sleep): + # An empty password is treated the same as an undefined password and + # will cause a NULL (\0) password to be used""" + self.info['password'] = "" + pw_file_handle = tempfile.NamedTemporaryFile() + pw_file = pw_file_handle.name + file_handle = open(pw_file, "w") + args = [ + 'ipmitool', + '-I', 'lanplus', + '-H', self.info['address'], + '-L', self.info['priv_level'], + '-U', self.info['username'], + '-f', file_handle, + 'A', 'B', 'C', + ] + + mock_support.return_value = False + mock_pwf.return_value = file_handle + mock_exec.return_value = (None, None) + ipmi._exec_ipmitool(self.info, 'A B C') + mock_support.assert_called_once_with('timing') + self.assertTrue(mock_pwf.called) + mock_exec.assert_called_once_with(*args) + mock_pwf.assert_called_once_with('\0') + @mock.patch.object(ipmi, '_is_option_supported', autospec=True) @mock.patch.object(ipmi, '_make_password_file', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)