diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 69fe75d710..2716b95f71 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -30,6 +30,7 @@ DRIVER. """ import contextlib +import ipaddress import os import re import subprocess @@ -324,6 +325,29 @@ def is_bridging_enabled(node): return node.driver_info.get("ipmi_bridging", "no") != "no" +def _validate_ipmi_address(address, node_uuid): + """Validate that the IPMI address is a valid IP or hostname. + + :param address: The IPMI address to validate + :param node_uuid: Node UUID for error messages + :raises: InvalidParameterValue if the address is invalid + """ + if not address: + return + + try: + ipaddress.ip_address(address) + return # Valid IP address + except ValueError: + pass # Not an IP, check if it's a valid hostname + + if not utils.is_hostname_safe(address): + raise exception.InvalidParameterValue( + _('Invalid IPMI address "%(address)s" for node %(node)s. ' + 'Must be a valid IP address or hostname.') % + {'address': address, 'node': node_uuid}) + + def _parse_driver_info(node): """Gets the parameters required for ipmitool to access the node. @@ -1040,7 +1064,12 @@ class IPMIPower(base.PowerInterface): :raises: MissingParameterValue if a required parameter is missing. """ - _parse_driver_info(task.node) + driver_info = _parse_driver_info(task.node) + + ipmi_address = driver_info.get('address') + if ipmi_address: + _validate_ipmi_address(ipmi_address, task.node.uuid) + # NOTE(tenbrae): don't actually touch the BMC in validate because it is # called too often, and BMCs are too fragile. # This is a temporary measure to mitigate problems while diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 9f524dc41f..dd46cb6811 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -2745,6 +2745,29 @@ class IPMIToolDriverTestCase(Base): task.driver.power.validate(task) mock_parse.assert_called_once_with(mock.ANY) + @mock.patch.object(ipmi, "_validate_ipmi_address", autospec=True) + @mock.patch.object(ipmi, "_parse_driver_info", autospec=True) + def test_validate_fail_invalid_ipaddress(self, mock_parse, mock_validate): + """Test that validation fails with invalid IP address.""" + # Test invalid IP address (out of range octet) + mock_parse.return_value = {'address': '123.233.342.123'} + mock_validate.side_effect = exception.InvalidParameterValue('Invalid') + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises(exception.InvalidParameterValue, + self.power.validate, task) + mock_validate.assert_called_with('123.233.342.123', self.node.uuid) + + @mock.patch.object(ipmi, "_validate_ipmi_address", autospec=True) + @mock.patch.object(ipmi, "_parse_driver_info", autospec=True) + def test_validate_fail_invalid_hostname(self, mock_parse, mock_validate): + """Test that validation fails with invalid hostname.""" + mock_parse.return_value = {'address': 'claw-me_ow..com'} + mock_validate.side_effect = exception.InvalidParameterValue('Invalid') + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises(exception.InvalidParameterValue, + self.power.validate, task) + mock_validate.assert_called_with('claw-me_ow..com', self.node.uuid) + def test_get_properties(self): expected = ipmi.COMMON_PROPERTIES self.assertEqual(expected, self.power.get_properties()) diff --git a/releasenotes/notes/validate-ipmi-address-d7b41fa7c8ad47f6.yaml b/releasenotes/notes/validate-ipmi-address-d7b41fa7c8ad47f6.yaml new file mode 100644 index 0000000000..070241594a --- /dev/null +++ b/releasenotes/notes/validate-ipmi-address-d7b41fa7c8ad47f6.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Adds validation for the ``ipmi_address`` field during node validation + to ensure it contains a valid IP address or hostname.