From 025d1941bcdd3da211e05b073c0e8d2a5a8f280b Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Thu, 28 Jan 2016 21:42:54 -0800 Subject: [PATCH] Log warning if ipmi_username/ipmi_password missing Even though an empty ipmi_username and/or ipmi_password is valid, it is often an error in configuration. Log a warning if either value is missing. Add unit tests showing that an unspecified username or an empty string username are treated the same and will cause no user to be specified. Add unit tests showing that an unspecified password or an empty string password are treated the same and will cause a NULL (\0) password to be used. Change-Id: I384e210da3216fb9335c4c782d69e606026d4b97 --- ironic/drivers/modules/ipmitool.py | 7 ++ .../unit/drivers/modules/test_ipmitool.py | 117 ++++++++++++++++++ 2 files changed, 124 insertions(+) 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)