From f6bd23e199416d65a97f598c69a7493e7c78d8b1 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 30 Oct 2018 08:57:39 -0700 Subject: [PATCH] Fix ipv6 URL formatting for pxe/iPXE The first DHCPv6 related fix was not enough. In testing, it appears that we were not correctly returning the proper variables and formatting to enable the machine to boot via IPv6. Fixes values, wraps a bare IPv6 address as dicated by RFC standards, and enhances unit tests. Change-Id: I9ebfb1ff1ba53dd117988b5e2a7736e20d1041a0 --- ironic/common/pxe_utils.py | 26 ++-- ironic/tests/unit/common/test_pxe_utils.py | 118 ++++++++++++------ .../fix-boot-url-for-v6-802abde9de8ba455.yaml | 8 ++ 3 files changed, 105 insertions(+), 47 deletions(-) create mode 100644 releasenotes/notes/fix-boot-url-for-v6-802abde9de8ba455.yaml diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index bd29b947d4..8e6409e745 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -21,6 +21,7 @@ from oslo_log import log as logging from oslo_utils import excutils from oslo_utils import fileutils from oslo_utils import importutils +from oslo_utils import netutils from ironic.common import dhcp_factory from ironic.common import exception @@ -410,11 +411,12 @@ def _dhcp_option_file_or_url(task, urlboot=False): if not urlboot: return boot_file elif urlboot: - path_prefix = get_tftp_path_prefix() - if path_prefix == '': - path_prefix = '/' - return ("tftp://" + CONF.pxe.tftp_server - + path_prefix + boot_file) + if netutils.is_valid_ipv6(CONF.pxe.tftp_server): + host = "[%s]" % CONF.pxe.tftp_server + else: + host = CONF.pxe.tftp_server + return "tftp://{host}/{boot_file}".format(host=host, + boot_file=boot_file) def dhcp_options_for_instance(task, ipxe_enabled=False, url_boot=False): @@ -434,12 +436,17 @@ def dhcp_options_for_instance(task, ipxe_enabled=False, url_boot=False): """ dhcp_opts = [] ip_version = int(CONF.pxe.ip_version) + dhcp_provider_name = CONF.dhcp.dhcp_provider if ip_version == 4: boot_file_param = DHCP_BOOTFILE_NAME else: # NOTE(TheJulia): Booting with v6 means it is always # a URL reply. - boot_file_param = DHCPV6_BOOTFILE_NAME + if dhcp_provider_name == 'neutron': + # dnsmasq requires ipv6 options be explicitly flagged. :( + boot_file_param = "option6:{}".format(DHCPV6_BOOTFILE_NAME) + else: + boot_file_param = DHCPV6_BOOTFILE_NAME url_boot = True # NOTE(TheJulia): The ip_version value config from the PXE config is # guarded in the configuration, so there is no real sense in having @@ -449,8 +456,11 @@ def dhcp_options_for_instance(task, ipxe_enabled=False, url_boot=False): if ipxe_enabled: script_name = os.path.basename(CONF.pxe.ipxe_boot_script) + # TODO(TheJulia): We should make this smarter to handle unwrapped v6 + # addresses, since the format is http://[ff80::1]:80/boot.ipxe. + # As opposed to requiring configuraiton, we can eventually make this + # dynamic, and would need to do similar then. ipxe_script_url = '/'.join([CONF.deploy.http_url, script_name]) - dhcp_provider_name = CONF.dhcp.dhcp_provider # if the request comes from dumb firmware send them the iPXE # boot image. if dhcp_provider_name == 'neutron': @@ -458,7 +468,7 @@ def dhcp_options_for_instance(task, ipxe_enabled=False, url_boot=False): # to neutron "dhcp-match=set:ipxe,175" and use below option dhcp_opts.append({'opt_name': "tag:!ipxe,%s" % boot_file_param, 'opt_value': boot_file}) - dhcp_opts.append({'opt_name': "tag:ipxe,%s" % DHCP_BOOTFILE_NAME, + dhcp_opts.append({'opt_name': "tag:ipxe,%s" % boot_file_param, 'opt_value': ipxe_script_url}) else: # !175 == non-iPXE. diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 1298584330..12a80edabd 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -716,7 +716,10 @@ class TestPXEUtils(db_base.DbTestCase): def _dhcp_options_for_instance(self, ip_version=4): self.config(ip_version=ip_version, group='pxe') - self.config(tftp_server='192.0.2.1', group='pxe') + if ip_version == 4: + self.config(tftp_server='192.0.2.1', group='pxe') + elif ip_version == 6: + self.config(tftp_server='ff80::1', group='pxe') self.config(pxe_bootfile_name='fake-bootfile', group='pxe') self.config(tftp_root='/tftp-path/', group='pxe') @@ -725,9 +728,8 @@ class TestPXEUtils(db_base.DbTestCase): # options are not imported, although they may be supported # by vendors. The apparent proper option is to return a # URL in the field https://tools.ietf.org/html/rfc5970#section-3 - expected_info = [{'opt_name': '59', - 'opt_value': 'tftp://192.0.2.1/tftp-path' - '/fake-bootfile', + expected_info = [{'opt_name': 'option6:59', + 'opt_value': 'tftp://[ff80::1]/fake-bootfile', 'ip_version': ip_version}] elif ip_version == 4: expected_info = [{'opt_name': '67', @@ -754,6 +756,7 @@ class TestPXEUtils(db_base.DbTestCase): self._dhcp_options_for_instance(ip_version=4) def test_dhcp_options_for_instance_ipv6(self): + self.config(tftp_server='ff80::1', group='pxe') self._dhcp_options_for_instance(ip_version=6) def _test_get_kernel_ramdisk_info(self, expected_dir, mode='deploy'): @@ -803,69 +806,106 @@ class TestPXEUtils(db_base.DbTestCase): self.config(http_root=expected_dir, group='deploy') self._test_get_kernel_ramdisk_info(expected_dir, mode='rescue') - def _dhcp_options_for_instance_ipxe(self, task, boot_file): - self.config(tftp_server='192.0.2.1', group='pxe') + def _dhcp_options_for_instance_ipxe(self, task, boot_file, ip_version=4): self.config(ipxe_enabled=True, group='pxe') - self.config(http_url='http://192.0.3.2:1234', group='deploy') self.config(ipxe_boot_script='/test/boot.ipxe', group='pxe') + self.config(tftp_root='/tftp-path/', group='pxe') + if ip_version == 4: + self.config(tftp_server='192.0.2.1', group='pxe') + self.config(http_url='http://192.0.3.2:1234', group='deploy') + self.config(ipxe_boot_script='/test/boot.ipxe', group='pxe') + elif ip_version == 6: + self.config(tftp_server='ff80::1', group='pxe') + self.config(http_url='http://[ff80::1]:1234', group='deploy') self.config(dhcp_provider='isc', group='dhcp') - expected_boot_script_url = 'http://192.0.3.2:1234/boot.ipxe' - expected_info = [{'opt_name': '!175,67', - 'opt_value': boot_file, - 'ip_version': 4}, - {'opt_name': '66', - 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': '150', - 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': '67', - 'opt_value': expected_boot_script_url, - 'ip_version': 4}, - {'opt_name': 'server-ip-address', - 'opt_value': '192.0.2.1', - 'ip_version': 4}] + if ip_version == 6: + # NOTE(TheJulia): DHCPv6 RFCs seem to indicate that the prior + # options are not imported, although they may be supported + # by vendors. The apparent proper option is to return a + # URL in the field https://tools.ietf.org/html/rfc5970#section-3 + expected_boot_script_url = 'http://[ff80::1]:1234/boot.ipxe' + expected_info = [{'opt_name': '!175,59', + 'opt_value': 'tftp://[ff80::1]/fake-bootfile', + 'ip_version': ip_version}, + {'opt_name': '59', + 'opt_value': expected_boot_script_url, + 'ip_version': ip_version}] + + elif ip_version == 4: + expected_boot_script_url = 'http://192.0.3.2:1234/boot.ipxe' + expected_info = [{'opt_name': '!175,67', + 'opt_value': boot_file, + 'ip_version': ip_version}, + {'opt_name': '66', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}, + {'opt_name': '150', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}, + {'opt_name': '67', + 'opt_value': expected_boot_script_url, + 'ip_version': ip_version}, + {'opt_name': 'server-ip-address', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}] self.assertItemsEqual(expected_info, pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True)) self.config(dhcp_provider='neutron', group='dhcp') - expected_boot_script_url = 'http://192.0.3.2:1234/boot.ipxe' - expected_info = [{'opt_name': 'tag:!ipxe,67', - 'opt_value': boot_file, - 'ip_version': 4}, - {'opt_name': '66', - 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': '150', - 'opt_value': '192.0.2.1', - 'ip_version': 4}, - {'opt_name': 'tag:ipxe,67', - 'opt_value': expected_boot_script_url, - 'ip_version': 4}, - {'opt_name': 'server-ip-address', - 'opt_value': '192.0.2.1', - 'ip_version': 4}] + if ip_version == 6: + # Boot URL variable set from prior test of isc parameters. + expected_info = [{'opt_name': 'tag:!ipxe,option6:59', + 'opt_value': 'tftp://[ff80::1]/fake-bootfile', + 'ip_version': ip_version}, + {'opt_name': 'tag:ipxe,option6:59', + 'opt_value': expected_boot_script_url, + 'ip_version': ip_version}] + elif ip_version == 4: + expected_info = [{'opt_name': 'tag:!ipxe,67', + 'opt_value': boot_file, + 'ip_version': ip_version}, + {'opt_name': '66', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}, + {'opt_name': '150', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}, + {'opt_name': 'tag:ipxe,67', + 'opt_value': expected_boot_script_url, + 'ip_version': ip_version}, + {'opt_name': 'server-ip-address', + 'opt_value': '192.0.2.1', + 'ip_version': ip_version}] self.assertItemsEqual(expected_info, pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True)) def test_dhcp_options_for_instance_ipxe_bios(self): + self.config(ip_version=4, group='pxe') boot_file = 'fake-bootfile-bios' self.config(pxe_bootfile_name=boot_file, group='pxe') with task_manager.acquire(self.context, self.node.uuid) as task: self._dhcp_options_for_instance_ipxe(task, boot_file) def test_dhcp_options_for_instance_ipxe_uefi(self): + self.config(ip_version=4, group='pxe') boot_file = 'fake-bootfile-uefi' self.config(uefi_pxe_bootfile_name=boot_file, group='pxe') with task_manager.acquire(self.context, self.node.uuid) as task: task.node.properties['capabilities'] = 'boot_mode:uefi' self._dhcp_options_for_instance_ipxe(task, boot_file) + def test_dhcp_options_for_ipxe_ipv6(self): + self.config(ip_version=6, group='pxe') + boot_file = 'fake-bootfile' + self.config(pxe_bootfile_name=boot_file, group='pxe') + with task_manager.acquire(self.context, self.node.uuid) as task: + self._dhcp_options_for_instance_ipxe(task, boot_file, ip_version=6) + @mock.patch('ironic.common.utils.rmtree_without_raise', autospec=True) @mock.patch('ironic_lib.utils.unlink_without_raise', autospec=True) @mock.patch('ironic.common.dhcp_factory.DHCPFactory.provider', diff --git a/releasenotes/notes/fix-boot-url-for-v6-802abde9de8ba455.yaml b/releasenotes/notes/fix-boot-url-for-v6-802abde9de8ba455.yaml new file mode 100644 index 0000000000..0a6f7ac9b9 --- /dev/null +++ b/releasenotes/notes/fix-boot-url-for-v6-802abde9de8ba455.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes the URL generation with TFTP URLs does not account for IPv6 addresses + needing to be wrapped in '[]' in order to be parsed. + - | + Fixes DHCP option parameter generation to correctly return the proper + values for IPv6 based booting when iPXE is in use.