Fix redfish-virtual-media file permission

When not using swift with the redfish virtual media boot interface,
in other words local file storage, the file permissions for the
/httpboot/redfish folder was incorrect upon initially being created,
and new file ISOs were being created with permissions based upon the
conductor process umask value which is OS environment dependent.

Change-Id: I038ca335efa9b5443469ab8c8af12863deea0e38

ironic/conf/redfish.py

@@ -72,6 +72,14 @@ opts = [
                       '/proc/cmdline. Mind severe cmdline size limit! Can be '
                       'overridden by `instance_info/kernel_append_params` '
                       'property.')),
+    cfg.IntOpt('file_permission',
+               default=0o644,
+               help=_('File permission for swift-less image hosting with the '
+                      'octal permission representation of file access '
+                      'permissions. This setting defaults to ``644``, '
+                      'or as the octal number ``0o644`` in Python. '
+                      'This setting must be set to the octal number '
+                      'representation, meaning starting with ``0o``.')),
 ]
 
 

ironic/drivers/modules/redfish/boot.py

@@ -394,12 +394,13 @@ def _publish_image(image_file, object_name):
         public_dir = os.path.join(CONF.deploy.http_root, IMAGE_SUBDIR)
 
         if not os.path.exists(public_dir):
-            os.mkdir(public_dir, 0x755)
+            os.mkdir(public_dir, 0o755)
 
         published_file = os.path.join(public_dir, object_name)
 
         try:
             os.link(image_file, published_file)
+            os.chmod(image_file, CONF.redfish.file_permission)
 
         except OSError as exc:
             LOG.debug(
@@ -410,6 +411,7 @@ def _publish_image(image_file, object_name):
                               'error': exc})
 
             shutil.copyfile(image_file, published_file)
+            os.chmod(published_file, CONF.redfish.file_permission)
 
         image_url = os.path.join(
             CONF.deploy.http_url, IMAGE_SUBDIR, object_name)

ironic/tests/unit/drivers/modules/redfish/test_boot.py

@@ -227,11 +227,12 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
             mock_swift_api.delete_object.assert_called_once_with(
                 'ironic_redfish_container', object_name)
 
+    @mock.patch.object(os, 'chmod', autospec=True)
     @mock.patch.object(redfish_boot, 'shutil', autospec=True)
     @mock.patch.object(os, 'link', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     def test__publish_image_local_link(
-            self, mock_mkdir, mock_link, mock_shutil):
+            self, mock_mkdir, mock_link, mock_shutil, mock_chmod):
         self.config(use_swift=False, group='redfish')
         self.config(http_url='http://localhost', group='deploy')
 
@@ -240,15 +241,17 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
         self.assertEqual(
             'http://localhost/redfish/boot.iso?filename=file.iso', url)
 
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0x755)
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
         mock_link.assert_called_once_with(
             'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('file.iso', 0o644)
 
+    @mock.patch.object(os, 'chmod', autospec=True)
     @mock.patch.object(redfish_boot, 'shutil', autospec=True)
     @mock.patch.object(os, 'link', autospec=True)
     @mock.patch.object(os, 'mkdir', autospec=True)
     def test__publish_image_local_copy(
-            self, mock_mkdir, mock_link, mock_shutil):
+            self, mock_mkdir, mock_link, mock_shutil, mock_chmod):
         self.config(use_swift=False, group='redfish')
         self.config(http_url='http://localhost', group='deploy')
 
@@ -259,10 +262,12 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
         self.assertEqual(
             'http://localhost/redfish/boot.iso?filename=file.iso', url)
 
-        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0x755)
+        mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755)
 
         mock_shutil.copyfile.assert_called_once_with(
             'file.iso', '/httpboot/redfish/boot.iso')
+        mock_chmod.assert_called_once_with('/httpboot/redfish/boot.iso',
+                                           0o644)
 
     @mock.patch.object(redfish_boot, 'ironic_utils', autospec=True)
     def test__unpublish_image_local(self, mock_ironic_utils):

releasenotes/notes/redfish-virtual-media-permission-fix-1909b9cdbbbf9fd1.yaml

@@ -0,0 +1,25 @@
+---
+upgrade:
+  - |
+    Operators may need to check their ``/httpboot/redfish`` folder permissions
+    if using ``redfish-virtual-media``. The conductor was previously creating
+    the folder with incorrect permissions.
+  - |
+    A permission setting has been added for ``redfish-virtual-media`` boot
+    interface, which allows for explicit file permission setting when the
+    driver is being used. The default for the new ``[redfish]file_permission
+    setting is ``0u644``, or 644 if manually changed using ``chmod`` on the
+    command line. Operators MAY need to adjust this if they were running the
+    conductor with a specific ``umask`` to work around the permission setting
+    defect.
+fixes:
+  - |
+    Fixes the ``redfish-virtual-media`` and related based drivers to utilize
+    an explicit file permission instead of rely upon the ironic-conductor
+    umask, which may be incorrect. This can be tuned with the
+    ``[redfish]file_permission`` setting.
+  - |
+    Fixes an issue where the default folder permission for the
+    ``redfish-virtual-media`` driver where the folder permissions for the
+    ``/httpboot/redfish`` folder was being created with incorrect
+    permissions.