Ensure desired permissions on all written pxe files

This change ensures all files written for pxe boot have
permissions determined by the [pxe]file_permission config option.

Change-Id: I1bc24e3871bae3ce070e7abe85fc4c48e844c317

5 files changed, 41 insertions, 27 deletions

diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index 02c9e9552..268c17846 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -348,7 +348,8 @@ def create_pxe_config(task, pxe_options, template=None, ipxe_enabled=False):
               'DISK_IDENTIFIER': pxe_config_disk_ident}
 
     pxe_config = utils.render_template(template, params)
-    utils.write_to_file(pxe_config_file_path, pxe_config)
+    utils.write_to_file(pxe_config_file_path, pxe_config,
+                        CONF.pxe.file_permission)
 
     # Always write the mac addresses
     _link_mac_pxe_configs(task, ipxe_enabled=ipxe_enabled)
@@ -381,7 +382,8 @@ def create_ipxe_boot_script():
     # which should be rather rare
     if (not os.path.isfile(bootfile_path)
             or not utils.file_has_content(bootfile_path, boot_script)):
-        utils.write_to_file(bootfile_path, boot_script)
+        utils.write_to_file(bootfile_path, boot_script,
+                            CONF.pxe.file_permission)
 
 
 def clean_up_pxe_config(task, ipxe_enabled=False):
@@ -1168,7 +1170,8 @@ def prepare_instance_kickstart_config(task, image_info, anaconda_boot=False):
     ks_config_drive = ks_utils.prepare_config_drive(task)
     if ks_config_drive:
         ks_cfg = ks_cfg + ks_config_drive
-    utils.write_to_file(image_info['ks_cfg'][1], ks_cfg)
+    utils.write_to_file(image_info['ks_cfg'][1], ks_cfg,
+                        CONF.pxe.file_permission)
 
 
 @image_cache.cleanup(priority=25)
@@ -1210,8 +1213,12 @@ def cache_ramdisk_kernel(task, pxe_info, ipxe_enabled=False):
 
     LOG.debug("Fetching necessary kernel and ramdisk for node %s",
               node.uuid)
-    deploy_utils.fetch_images(ctx, TFTPImageCache(), list(t_pxe_info.values()),
+    images_info = list(t_pxe_info.values())
+    deploy_utils.fetch_images(ctx, TFTPImageCache(), images_info,
                               CONF.force_raw_images)
+    if CONF.pxe.file_permission:
+        for info in images_info:
+            os.chmod(info[1], CONF.pxe.file_permission)
 
 
 def clean_up_pxe_env(task, images_info, ipxe_enabled=False):
diff --git a/ironic/common/utils.py b/ironic/common/utils.py
index e15083396..f37088c07 100644
--- a/ironic/common/utils.py
+++ b/ironic/common/utils.py
@@ -278,9 +278,11 @@ def rmtree_without_raise(path):
                     {'path': path, 'e': e})
 
 
-def write_to_file(path, contents):
+def write_to_file(path, contents, permission=None):
     with open(path, 'w') as f:
         f.write(contents)
+    if permission:
+        os.chmod(path, permission)
 
 
 def create_link_without_raise(source, link):
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index 161b85932..fbfe7f05b 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -476,7 +476,7 @@ class TestPXEUtils(db_base.DbTestCase):
 
         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)
+                                      render_mock.return_value, 0o644)
         self.assertTrue(mock_link_ip_addr.called)
 
     @mock.patch.object(pxe_utils, '_link_ip_address_pxe_configs',
@@ -509,7 +509,7 @@ class TestPXEUtils(db_base.DbTestCase):
 
         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)
+                                      render_mock.return_value, 0o644)
         self.assertTrue(mock_link_ip_addr.called)
 
     @mock.patch.object(pxe_utils, '_link_ip_address_pxe_configs',
@@ -540,7 +540,7 @@ class TestPXEUtils(db_base.DbTestCase):
         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)
+                                      render_mock.return_value, 0o644)
         self.assertTrue(mock_link_ip_address.called)
 
     @mock.patch.object(os.path, 'isdir', autospec=True)
@@ -569,7 +569,7 @@ class TestPXEUtils(db_base.DbTestCase):
         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)
+                                      render_mock.return_value, 0o644)
 
     @mock.patch.object(os, 'makedirs', autospec=True)
     @mock.patch('ironic.common.pxe_utils._link_ip_address_pxe_configs',
@@ -601,7 +601,7 @@ class TestPXEUtils(db_base.DbTestCase):
 
         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)
+                                      render_mock.return_value, 0o644)
 
     @mock.patch.object(os, 'makedirs', autospec=True)
     @mock.patch('ironic.common.pxe_utils._link_mac_pxe_configs',
@@ -642,7 +642,7 @@ class TestPXEUtils(db_base.DbTestCase):
 
         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)
+                                      render_mock.return_value, 0o644)
 
     @mock.patch.object(os, 'makedirs', autospec=True)
     @mock.patch('ironic.common.pxe_utils._link_mac_pxe_configs', autospec=True)
@@ -675,7 +675,7 @@ class TestPXEUtils(db_base.DbTestCase):
         pxe_cfg_file_path = pxe_utils.get_pxe_config_file_path(
             self.node.uuid, ipxe_enabled=True)
         write_mock.assert_called_with(pxe_cfg_file_path,
-                                      render_mock.return_value)
+                                      render_mock.return_value, 0o644)
 
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.get_ip_addresses',
                 autospec=True)
@@ -714,7 +714,7 @@ class TestPXEUtils(db_base.DbTestCase):
         write_mock.assert_called_once_with(
             os.path.join(CONF.deploy.http_root,
                          os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
         render_mock.assert_called_once_with(
             CONF.pxe.ipxe_boot_script,
             {'ipxe_for_mac_uri': 'pxelinux.cfg/',
@@ -736,7 +736,7 @@ class TestPXEUtils(db_base.DbTestCase):
         write_mock.assert_called_once_with(
             os.path.join(CONF.deploy.http_root,
                          os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
         render_mock.assert_called_once_with(
             CONF.pxe.ipxe_boot_script,
             {'ipxe_for_mac_uri': 'pxelinux.cfg/',
@@ -755,7 +755,7 @@ class TestPXEUtils(db_base.DbTestCase):
         write_mock.assert_called_once_with(
             os.path.join(CONF.deploy.http_root,
                          os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
         render_mock.assert_called_once_with(
             CONF.pxe.ipxe_boot_script,
             {'ipxe_for_mac_uri': 'pxelinux.cfg/',
@@ -1317,8 +1317,10 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
                 task, ipxe_enabled=False
             )
 
+    @mock.patch.object(os, 'chmod', autospec=True)
     @mock.patch.object(deploy_utils, 'fetch_images', autospec=True)
-    def test__cache_tftp_images_master_path(self, mock_fetch_image):
+    def test__cache_tftp_images_master_path(self, mock_fetch_image,
+                                            mock_chmod):
         temp_dir = tempfile.mkdtemp()
         self.config(tftp_root=temp_dir, group='pxe')
         self.config(tftp_master_path=os.path.join(temp_dir,
@@ -1338,11 +1340,13 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
                                                    image_path)],
                                                  True)
 
+    @mock.patch.object(os, 'chmod', autospec=True)
     @mock.patch.object(pxe_utils, 'TFTPImageCache', lambda: None)
     @mock.patch.object(pxe_utils, 'ensure_tree', autospec=True)
     @mock.patch.object(deploy_utils, 'fetch_images', autospec=True)
-    def test_cache_ramdisk_kernel(self, mock_fetch_image, mock_ensure_tree):
-        fake_pxe_info = {'foo': 'bar'}
+    def test_cache_ramdisk_kernel(self, mock_fetch_image, mock_ensure_tree,
+                                  mock_chmod):
+        fake_pxe_info = pxe_utils.get_image_info(self.node)
         expected_path = os.path.join(CONF.pxe.tftp_root, self.node.uuid)
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
@@ -1351,12 +1355,13 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
         mock_fetch_image.assert_called_once_with(
             self.context, mock.ANY, list(fake_pxe_info.values()), True)
 
+    @mock.patch.object(os, 'chmod', autospec=True)
     @mock.patch.object(pxe_utils, 'TFTPImageCache', lambda: None)
     @mock.patch.object(pxe_utils, 'ensure_tree', autospec=True)
     @mock.patch.object(deploy_utils, 'fetch_images', autospec=True)
     def test_cache_ramdisk_kernel_ipxe(self, mock_fetch_image,
-                                       mock_ensure_tree):
-        fake_pxe_info = {'foo': 'bar'}
+                                       mock_ensure_tree, mock_chmod):
+        fake_pxe_info = pxe_utils.get_image_info(self.node)
         expected_path = os.path.join(CONF.deploy.http_root,
                                      self.node.uuid)
         with task_manager.acquire(self.context, self.node.uuid,
@@ -1475,7 +1480,7 @@ class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase):
                 image_info['ks_template'][1], {'ks_options': params}
             )
             write_mock.assert_called_with(image_info['ks_cfg'][1],
-                                          render_mock.return_value)
+                                          render_mock.return_value, 0o644)
 
     def test_validate_kickstart_template(self):
         self.config_temp_dir('http_root', group='deploy')
diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py
index 2254a2c72..085c96c41 100644
--- a/ironic/tests/unit/drivers/modules/test_ipxe.py
+++ b/ironic/tests/unit/drivers/modules/test_ipxe.py
@@ -392,7 +392,7 @@ class iPXEBootTestCase(db_base.DbTestCase):
             os.path.join(
                 CONF.deploy.http_root,
                 os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
         render_mock.assert_called_once_with(
             CONF.pxe.ipxe_boot_script,
             {'ipxe_for_mac_uri': 'pxelinux.cfg/',
@@ -413,7 +413,7 @@ class iPXEBootTestCase(db_base.DbTestCase):
             os.path.join(
                 CONF.deploy.http_root,
                 os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
         render_mock.assert_called_once_with(
             CONF.pxe.ipxe_boot_script,
             {'ipxe_for_mac_uri': 'pxelinux.cfg/',
@@ -441,7 +441,7 @@ class iPXEBootTestCase(db_base.DbTestCase):
             os.path.join(
                 CONF.deploy.http_root,
                 os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
 
     @mock.patch.object(common_utils, 'render_template', lambda *args: 'foo')
     @mock.patch('ironic.common.utils.write_to_file', autospec=True)
@@ -455,7 +455,7 @@ class iPXEBootTestCase(db_base.DbTestCase):
             os.path.join(
                 CONF.deploy.http_root,
                 os.path.basename(CONF.pxe.ipxe_boot_script)),
-            'foo')
+            'foo', 0o644)
 
     def test_prepare_ramdisk_cleaning(self):
         self.node.provision_state = states.CLEANING
diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py
index ce40b8dd8..e1559b6f6 100644
--- a/ironic/tests/unit/drivers/modules/test_pxe.py
+++ b/ironic/tests/unit/drivers/modules/test_pxe.py
@@ -862,7 +862,7 @@ class PXEBootTestCase(db_base.DbTestCase):
             provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts)
             render_mock.assert_called()
             write_file_mock.assert_called_with(
-                '/path/to/ks.cfg', render_mock.return_value
+                '/path/to/ks.cfg', render_mock.return_value, 0o644
             )
             create_pxe_config_mock.assert_called_once_with(
                 task, mock.ANY, CONF.pxe.uefi_pxe_config_template,
@@ -926,7 +926,7 @@ class PXEBootTestCase(db_base.DbTestCase):
             provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts)
             render_mock.assert_called()
             write_file_mock.assert_called_with(
-                '/path/to/ks.cfg', render_mock.return_value
+                '/path/to/ks.cfg', render_mock.return_value, 0o644
             )
             create_pxe_config_mock.assert_called_once_with(
                 task, mock.ANY, CONF.pxe.pxe_config_template,