diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 405a66003f..32be0cb3a5 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -3309,6 +3309,17 @@ # caching. (string value) #tftp_master_path = /tftpboot/master_images +# The permission that will be applied to the TFTP folders upon +# creation. This should be set to the permission such that the +# tftpserver has access to read the contents of the configured +# TFTP folder. This setting is only required when the +# operating system's umask is restrictive such that ironic- +# conductor is creating files that cannot be read by the TFTP +# server. Setting to will result in the operating +# system's umask to be utilized for the creation of new tftp +# folders. (integer value) +#dir_permission = + # Bootfile DHCP parameter. (string value) #pxe_bootfile_name = pxelinux.0 diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index 5779d21990..bd57f6ef47 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -49,8 +49,17 @@ def _ensure_config_dirs_exist(node_uuid): """ root_dir = get_root_dir() - fileutils.ensure_tree(os.path.join(root_dir, node_uuid)) - fileutils.ensure_tree(os.path.join(root_dir, PXE_CFG_DIR_NAME)) + node_dir = os.path.join(root_dir, node_uuid) + pxe_dir = os.path.join(root_dir, PXE_CFG_DIR_NAME) + # NOTE: We should only change the permissions if the folder + # does not exist. i.e. if defined, an operator could have + # already created it and placed specific ACLs upon the folder + # which may not recurse downward. + for directory in (node_dir, pxe_dir): + if not os.path.isdir(directory): + fileutils.ensure_tree(directory) + if CONF.pxe.dir_permission: + os.chmod(directory, CONF.pxe.dir_permission) def _link_mac_pxe_configs(task): diff --git a/ironic/conf/pxe.py b/ironic/conf/pxe.py index 355c45f104..c8c02a8f9d 100644 --- a/ironic/conf/pxe.py +++ b/ironic/conf/pxe.py @@ -77,6 +77,17 @@ opts = [ help=_('On ironic-conductor node, directory where master TFTP ' 'images are stored on disk. ' 'Setting to disables image caching.')), + cfg.IntOpt('dir_permission', + help=_("The permission that will be applied to the TFTP " + "folders upon creation. This should be set to the " + "permission such that the tftpserver has access to " + "read the contents of the configured TFTP folder. This " + "setting is only required when the operating system's " + "umask is restrictive such that ironic-conductor is " + "creating files that cannot be read by the TFTP server. " + "Setting to will result in the operating " + "system's umask to be utilized for the creation of new " + "tftp folders.")), cfg.StrOpt('pxe_bootfile_name', default='pxelinux.0', help=_('Bootfile DHCP parameter.')), diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 668cac7ad0..830ce2f37c 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -277,11 +277,12 @@ class TestPXEUtils(db_base.DbTestCase): unlink_mock.assert_called_once_with('/tftpboot/10.10.0.1.conf') create_link_mock.assert_has_calls(create_link_calls) + @mock.patch.object(os, 'chmod', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True) @mock.patch('ironic.common.utils.render_template', autospec=True) @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) def test_create_pxe_config(self, ensure_tree_mock, render_mock, - write_mock): + write_mock, chmod_mock): with task_manager.acquire(self.context, self.node.uuid) as task: pxe_utils.create_pxe_config(task, self.pxe_options, CONF.pxe.pxe_config_template) @@ -291,23 +292,84 @@ class TestPXEUtils(db_base.DbTestCase): 'ROOT': '{{ ROOT }}', 'DISK_IDENTIFIER': '{{ DISK_IDENTIFIER }}'} ) + node_dir = os.path.join(CONF.pxe.tftp_root, self.node.uuid) + pxe_dir = os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg') ensure_calls = [ - mock.call(os.path.join(CONF.pxe.tftp_root, self.node.uuid)), - mock.call(os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg')) + mock.call(node_dir), mock.call(pxe_dir), ] ensure_tree_mock.assert_has_calls(ensure_calls) + chmod_mock.assert_not_called() pxe_cfg_file_path = pxe_utils.get_pxe_config_file_path(self.node.uuid) write_mock.assert_called_with(pxe_cfg_file_path, render_mock.return_value) + @mock.patch.object(os, 'chmod', autospec=True) + @mock.patch('ironic.common.utils.write_to_file', autospec=True) + @mock.patch('ironic.common.utils.render_template', autospec=True) + @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) + def test_create_pxe_config_set_dir_permission(self, ensure_tree_mock, + render_mock, + write_mock, chmod_mock): + self.config(dir_permission=0o755, group='pxe') + with task_manager.acquire(self.context, self.node.uuid) as task: + pxe_utils.create_pxe_config(task, self.pxe_options, + CONF.pxe.pxe_config_template) + render_mock.assert_called_with( + CONF.pxe.pxe_config_template, + {'pxe_options': self.pxe_options, + 'ROOT': '{{ ROOT }}', + 'DISK_IDENTIFIER': '{{ DISK_IDENTIFIER }}'} + ) + node_dir = os.path.join(CONF.pxe.tftp_root, self.node.uuid) + pxe_dir = os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg') + ensure_calls = [ + mock.call(node_dir), mock.call(pxe_dir), + ] + ensure_tree_mock.assert_has_calls(ensure_calls) + chmod_calls = [mock.call(node_dir, 0o755), mock.call(pxe_dir, 0o755)] + chmod_mock.assert_has_calls(chmod_calls) + + pxe_cfg_file_path = pxe_utils.get_pxe_config_file_path(self.node.uuid) + write_mock.assert_called_with(pxe_cfg_file_path, + render_mock.return_value) + + @mock.patch.object(os.path, 'isdir', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + @mock.patch('ironic.common.utils.write_to_file', autospec=True) + @mock.patch('ironic.common.utils.render_template', autospec=True) + @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) + def test_create_pxe_config_existing_dirs(self, ensure_tree_mock, + render_mock, + write_mock, chmod_mock, + isdir_mock): + self.config(dir_permission=0o755, group='pxe') + with task_manager.acquire(self.context, self.node.uuid) as task: + isdir_mock.return_value = True + pxe_utils.create_pxe_config(task, self.pxe_options, + CONF.pxe.pxe_config_template) + render_mock.assert_called_with( + CONF.pxe.pxe_config_template, + {'pxe_options': self.pxe_options, + 'ROOT': '{{ ROOT }}', + 'DISK_IDENTIFIER': '{{ DISK_IDENTIFIER }}'} + ) + ensure_tree_mock.assert_has_calls([]) + chmod_mock.assert_not_called() + isdir_mock.assert_has_calls([]) + pxe_cfg_file_path = pxe_utils.get_pxe_config_file_path(self.node.uuid) + write_mock.assert_called_with(pxe_cfg_file_path, + render_mock.return_value) + + @mock.patch.object(os, 'chmod', autospec=True) @mock.patch('ironic.common.pxe_utils._link_ip_address_pxe_configs', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True) @mock.patch('ironic.common.utils.render_template', autospec=True) @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) def test_create_pxe_config_uefi_elilo(self, ensure_tree_mock, render_mock, - write_mock, link_ip_configs_mock): + write_mock, link_ip_configs_mock, + chmod_mock): self.config( uefi_pxe_config_template=('ironic/drivers/modules/' 'elilo_efi_pxe_config.template'), @@ -320,9 +382,10 @@ class TestPXEUtils(db_base.DbTestCase): ensure_calls = [ mock.call(os.path.join(CONF.pxe.tftp_root, self.node.uuid)), - mock.call(os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg')) + mock.call(os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg')), ] ensure_tree_mock.assert_has_calls(ensure_calls) + chmod_mock.assert_not_called() render_mock.assert_called_with( CONF.pxe.uefi_pxe_config_template, {'pxe_options': self.pxe_options, @@ -334,13 +397,15 @@ class TestPXEUtils(db_base.DbTestCase): write_mock.assert_called_with(pxe_cfg_file_path, render_mock.return_value) + @mock.patch.object(os, 'chmod', autospec=True) @mock.patch('ironic.common.pxe_utils._link_ip_address_pxe_configs', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True) @mock.patch('ironic.common.utils.render_template', autospec=True) @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) def test_create_pxe_config_uefi_grub(self, ensure_tree_mock, render_mock, - write_mock, link_ip_configs_mock): + write_mock, link_ip_configs_mock, + chmod_mock): grub_tmplte = "ironic/drivers/modules/pxe_grub_config.template" with task_manager.acquire(self.context, self.node.uuid) as task: task.node.properties['capabilities'] = 'boot_mode:uefi' @@ -349,9 +414,10 @@ class TestPXEUtils(db_base.DbTestCase): ensure_calls = [ mock.call(os.path.join(CONF.pxe.tftp_root, self.node.uuid)), - mock.call(os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg')) + mock.call(os.path.join(CONF.pxe.tftp_root, 'pxelinux.cfg')), ] ensure_tree_mock.assert_has_calls(ensure_calls) + chmod_mock.assert_not_called() render_mock.assert_called_with( grub_tmplte, {'pxe_options': self.pxe_options, @@ -363,13 +429,15 @@ class TestPXEUtils(db_base.DbTestCase): write_mock.assert_called_with(pxe_cfg_file_path, render_mock.return_value) + @mock.patch.object(os, 'chmod', autospec=True) @mock.patch('ironic.common.pxe_utils._link_mac_pxe_configs', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True) @mock.patch('ironic.common.utils.render_template', autospec=True) @mock.patch('oslo_utils.fileutils.ensure_tree', autospec=True) def test_create_pxe_config_uefi_ipxe(self, ensure_tree_mock, render_mock, - write_mock, link_mac_pxe_mock): + write_mock, link_mac_pxe_mock, + chmod_mock): self.config(ipxe_enabled=True, group='pxe') ipxe_template = "ironic/drivers/modules/ipxe_config.template" with task_manager.acquire(self.context, self.node.uuid) as task: @@ -379,9 +447,10 @@ class TestPXEUtils(db_base.DbTestCase): ensure_calls = [ mock.call(os.path.join(CONF.deploy.http_root, self.node.uuid)), - mock.call(os.path.join(CONF.deploy.http_root, 'pxelinux.cfg')) + mock.call(os.path.join(CONF.deploy.http_root, 'pxelinux.cfg')), ] ensure_tree_mock.assert_has_calls(ensure_calls) + chmod_mock.assert_not_called() render_mock.assert_called_with( ipxe_template, {'pxe_options': self.ipxe_options, diff --git a/releasenotes/notes/fix-dir-permissions-bc56e83a651bbdb0.yaml b/releasenotes/notes/fix-dir-permissions-bc56e83a651bbdb0.yaml new file mode 100644 index 0000000000..e36b4bb219 --- /dev/null +++ b/releasenotes/notes/fix-dir-permissions-bc56e83a651bbdb0.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - Adds the capability for an operator to explicitly define the permission + for created tftpboot folders. This provides the ability for ironic to be + utilized with a restrictive umask, where the tftp server may not be able + to read the file. Introduced a new config option ``[pxe]/dir_permission`` + to specify the permission for the tftpboot directories to be created with.