Merge https://github.com/openstack/ironic:master (78201d7) into main

devstack/lib/ironic

@@ -1801,7 +1801,7 @@ EOF
             restart_libvirt
         fi
         # Standalone jobs may use some different paths, that is okay
-        iniset $IRONIC_CONF_FILE conductor file_url_allowed_paths /var/lib/ironic,/shared/html,/templates,/opt/cache/files,/vagrant,/opt/stack/ironic
+        iniset $IRONIC_CONF_FILE conductor file_url_allowed_paths /var/lib/ironic,/opt/stack/ironic
 
     fi
 

doc/source/contributor/dev-quickstart.rst

@@ -109,7 +109,7 @@ manage your locales, make sure you have enabled ``en_US.UTF8`` in
 Python Prerequisites
 ====================
 
-We suggest to use at least tox 3.9, if your distribution has an older version,
+Ironic requires at least tox 4.28.0, if your distribution has an older version,
 you can install it using pip system-wise or better per user using the --user
 option that by default will install the binary under $HOME/.local/bin, so you
 need to be sure to have that path in $PATH; for example::

doc/source/contributor/devstack-guide.rst

@@ -195,7 +195,7 @@ enabled and use the ``ipmi`` hardware type with this config::
     # Configure networking by disabling OVN and enabling Neutron w/OVS.
     disable_service ovn-controller
     disable_service ovn-northd
-    disable_service neutron-ovn-metadata-agent
+    disable_service neutron-ovn-agent
 
     enable_service neutron-agent q-agt
     enable_service neutron-dhcp q-dhcp

ironic/api/controllers/v1/node.py

@@ -1669,6 +1669,19 @@ def _get_fields_for_node_query(fields=None):
                 msg = 'Field %s is not a valid field.' % field
                 raise exception.Invalid(msg)
 
+        # NOTE(TheJulia): Bugfix for LP#2150573. owner and
+        # lessee must always be extracted from the RPC object
+        # so that node_sanitize() can populate target_dict
+        # with node.owner/node.lessee for RBAC policy checks.
+        # Without these, project-scoped owner/lessee policy
+        # rules always fail and fields like last_error get
+        # incorrectly redacted. sanitize_dict() will strip
+        # these from the response if not requested by the
+        # caller.
+        for rbac_field in ('owner', 'lessee'):
+            if rbac_field not in object_fields:
+                object_fields.append(rbac_field)
+
         return object_fields
 
 

ironic/common/image_service.py

@@ -392,8 +392,11 @@ class OciImageService(BaseImageService):
     _client = None
 
     def __init__(self):
-        verify = strutils.bool_from_string(CONF.webserver_verify_ca,
-                                           strict=True)
+        try:
+            verify = strutils.bool_from_string(CONF.webserver_verify_ca,
+                                               strict=True)
+        except ValueError:
+            verify = CONF.webserver_verify_ca
         # Creates a client which we can use for actions.
         # Note, this is not yet authenticated!
         self._client = oci_registry.OciClient(verify=verify)

ironic/common/utils.py

@@ -35,6 +35,7 @@
 import warnings
 
 import jinja2
+from jinja2 import sandbox as jinja2sandbox
 from oslo_concurrency import processutils
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
@@ -501,18 +502,18 @@ def render_template(template, params, is_file=True, strict=False):
     :param strict: Enable strict template rendering. Default is False
     :returns: Rendered template
     :raises: jinja2.exceptions.UndefinedError
+    :raises: jinja2.exceptions.SecurityError when the template has insecure
+             operations detected.
     """
     if is_file:
         tmpl_path, tmpl_name = os.path.split(template)
         loader = jinja2.FileSystemLoader(tmpl_path)
     else:
         tmpl_name = 'template'
         loader = jinja2.DictLoader({tmpl_name: template})
-    # NOTE(pas-ha) bandit does not seem to cope with such syntaxis
-    # and still complains with B701 for that line
     # NOTE(pas-ha) not using default_for_string=False as we set the name
     # of the template above for strings too.
-    env = jinja2.Environment(  # nosec B701
+    env = jinja2sandbox.SandboxedEnvironment(
         loader=loader,
         autoescape=jinja2.select_autoescape(),
         undefined=jinja2.StrictUndefined if strict else jinja2.Undefined

ironic/conf/conductor.py

@@ -635,8 +635,7 @@
                        'automatically applied by the conductor should a '
                        'compressed artifact be detected.')),
     cfg.ListOpt('file_url_allowed_paths',
-                default=['/var/lib/ironic', '/shared/html', '/templates',
-                         '/opt/cache/files', '/vagrant'],
+                default=['/var/lib/ironic'],
                 item_type=ir_types.ExplicitAbsolutePath(),
                 help=_(
                     'List of paths that are allowed to be used as file:// '

ironic/drivers/modules/redfish/firmware.py

@@ -45,6 +45,7 @@
 NIC_STARTING_TIMESTAMP = 'nic_starting_timestamp'
 NIC_REBOOT_TRIGGERED = 'nic_reboot_triggered'
 BIOS_REBOOT_TRIGGERED = 'bios_reboot_triggered'
+BMC_UPDATE_COMPLETED = 'bmc_update_completed'
 
 
 class RedfishFirmware(base.FirmwareInterface):
@@ -454,6 +455,9 @@ def _handle_bmc_update_completion(self, task, update_service,
         :param settings: firmware update settings
         :param current_update: the current firmware update being processed
         """
+        # Upgrade the lock to ensure we are using the latest info from
+        # the node.
+        task.upgrade_lock()
         node = task.node
 
         # Try to get current BMC version
@@ -466,15 +470,41 @@ def _handle_bmc_update_completion(self, task, update_service,
         if (current_version is not None
                 and version_before is not None
                 and current_version != version_before):
-            LOG.info(
-                'BMC firmware version for node %(node)s changed from '
-                '%(old)s to %(new)s. Update complete. Continuing without '
-                'reboot.',
-                {'node': node.uuid, 'old': version_before,
-                 'new': current_version})
             node.del_driver_internal_info(BMC_FW_VERSION_BEFORE_UPDATE)
-            node.save()
-            self._continue_updates(task, update_service, settings)
+
+            # Check if more components are pending updates after BMC update
+            if len(settings) > 1:
+                # Upgrade the lock to ensure we are using the latest info from
+                # the node.
+                task.upgrade_lock()
+                # More components to update - trigger reboot before continuing
+                #  Some hardware can only execute NIC firmware updates after
+                # the host reboots following the BMC firmware update.
+
+                LOG.info('BMC firmware update complete for node %(node)s. '
+                         'More components pending - triggering reboot before '
+                         'continuing to next component.',
+                         {'node': node.uuid})
+                # Set flag to indicate reboot completed, ready to continue
+                # This ensures we reboot and continue with the next component
+                # update, this is required because we saw cases where NIC
+                # updates were not being executed after the BMC update.
+                current_update[BMC_UPDATE_COMPLETED] = True
+                node.set_driver_internal_info('redfish_fw_updates', settings)
+                node.save()
+
+                manager_utils.node_power_action(task, states.REBOOT)
+                return
+            else:
+                # Last component - no reboot needed
+                # Servicing/Cleaning will trigger one.
+                LOG.info('BMC firmware version for node %(node)s changed '
+                         'from %(old)s to %(new)s.  Update complete last '
+                         'component',
+                         {'node': node.uuid, 'old': version_before,
+                          'new': current_version})
+                node.save()
+                self._continue_updates(task, update_service, settings)
             return
 
         # Check if we've been checking for too long
@@ -1366,7 +1396,6 @@ def _handle_firmware_update_task(self, task, node, current_update,
                     if msg:
                         messages.append(msg)
 
-            task.upgrade_lock()
             self._handle_task_completion(task, sushy_task, messages,
                                          update_service, settings,
                                          current_update)
@@ -1403,7 +1432,9 @@ def _handle_firmware_update_task(self, task, node, current_update,
     @METRICS.timer('RedfishFirmware._check_node_redfish_firmware_update')
     def _check_node_redfish_firmware_update(self, task):
         """Check the progress of running firmware update on a node."""
-
+        # Upgrade the lock to ensure we are using the latest info from
+        # the node.
+        task.upgrade_lock()
         node = task.node
 
         # Check overall timeout for firmware update operation
@@ -1424,6 +1455,20 @@ def _check_node_redfish_firmware_update(self, task):
                         {'node': node.uuid, 'error': e})
             return
 
+
+        # Check if BMC update just completed and node rebooted
+        # If so, continue with next component update
+        if current_update.get(BMC_UPDATE_COMPLETED):
+            LOG.info('BMC firmware update completed and node %(node)s has '
+                     'rebooted. Continuing with next component.',
+                     {'node': node.uuid})
+            current_update.pop(BMC_UPDATE_COMPLETED, None)
+            node.set_driver_internal_info('redfish_fw_updates', settings)
+            node.save()
+
+            self._continue_updates(task, update_service, settings)
+            return
+
         # Touch provisioning to indicate progress is being monitored.
         # This prevents heartbeat timeout from triggering for steps that
         # don't require the ramdisk agent (requires_ramdisk=False).

ironic/drivers/modules/redfish/management.py

@@ -908,8 +908,7 @@ def _process_storage_sensors(self, node, system):
         storage_sensors = {'Drive': {}}
 
         # Get system identity from driver info
-        driver_info = redfish_utils.parse_driver_info(node)
-        system_identity = driver_info['system_id'].split('/')[-1]
+        system_identity = system.path.split('/')[-1]
 
         try:
             drives = {}
@@ -954,8 +953,7 @@ def _process_simple_storage_sensors(self, node, system):
         simple_storage_sensors = {'Drive': {}}
 
         # Get system identity from driver info
-        driver_info = redfish_utils.parse_driver_info(node)
-        system_identity = driver_info['system_id'].split('/')[-1]
+        system_identity = system.path.split('/')[-1]
 
         try:
             drives = {}

ironic/tests/unit/api/controllers/v1/test_node.py

@@ -177,6 +177,100 @@ def test_one_field_specific_santization(self, mock_check_policy,
             mock.call().__bool__(),
         ])
 
+    @mock.patch.object(policy, 'check', autospec=True)
+    @mock.patch.object(policy, 'check_policy', autospec=True)
+    def test_field_redaction_with_owner_not_in_fields(
+            self, mock_check_policy, mock_check):
+        """Regression test for LP#2150573.
+
+        When a node owner requests specific fields without including
+        'owner' in the list, the RBAC policy checks for fields like
+        last_error must still have access to the node's owner in the
+        target_dict. Otherwise the project-scoped ownership rule
+        (project_id == node.owner) can never match and the fields
+        are incorrectly redacted.
+        """
+        owner_id = 'test-project-id'
+        obj_utils.create_test_node(self.context,
+                                   chassis_id=self.chassis.id,
+                                   owner=owner_id,
+                                   last_error='meow',
+                                   reservation='fake-conductor')
+        # filter_threshold=False triggers extended sanitization
+        mock_check_policy.return_value = False
+
+        def check_side_effect(rule, target, creds):
+            if rule in ('baremetal:node:get:last_error',
+                        'baremetal:node:get:reservation'):
+                # The target_dict (first positional arg after rule)
+                # must contain node.owner for ownership policy rules
+                # to evaluate correctly even though 'owner' was not
+                # in the requested field list.
+                assert 'node.owner' in target, (
+                    'node.owner missing from target_dict for '
+                    'policy rule %s' % rule)
+                assert target['node.owner'] == owner_id
+                return True
+            return True
+
+        mock_check.side_effect = check_side_effect
+
+        # Request fields WITHOUT 'owner' - the bug scenario
+        data = self.get_json(
+            '/nodes?fields=uuid,last_error,reservation',
+            headers={api_base.Version.string:
+                     str(api_v1.max_version())})
+
+        # Fields must contain actual values, not redaction text
+        self.assertEqual('meow',
+                         data['nodes'][0]['last_error'])
+        self.assertEqual('fake-conductor',
+                         data['nodes'][0]['reservation'])
+        # Owner must NOT be in the response since it was not
+        # requested, ensuring no information leak.
+        self.assertNotIn('owner', data['nodes'][0])
+
+    @mock.patch.object(policy, 'check', autospec=True)
+    @mock.patch.object(policy, 'check_policy', autospec=True)
+    def test_field_redaction_get_one_owner_not_in_fields(
+            self, mock_check_policy, mock_check):
+        """Regression test for LP#2150573 on single-node GET.
+
+        Same scenario as the list test above but exercising the
+        get_one code path through node_convert_with_links with
+        sanitize=True.
+        """
+        owner_id = 'test-project-id'
+        node = obj_utils.create_test_node(
+            self.context,
+            chassis_id=self.chassis.id,
+            owner=owner_id,
+            last_error='meow',
+            reservation='fake-conductor')
+        mock_check_policy.return_value = False
+
+        def check_side_effect(rule, target, creds):
+            if rule in ('baremetal:node:get:last_error',
+                        'baremetal:node:get:reservation'):
+                assert 'node.owner' in target, (
+                    'node.owner missing from target_dict for '
+                    'policy rule %s' % rule)
+                assert target['node.owner'] == owner_id
+                return True
+            return True
+
+        mock_check.side_effect = check_side_effect
+
+        data = self.get_json(
+            '/nodes/%s?fields=uuid,last_error,reservation'
+            % node.uuid,
+            headers={api_base.Version.string:
+                     str(api_v1.max_version())})
+
+        self.assertEqual('meow', data['last_error'])
+        self.assertEqual('fake-conductor', data['reservation'])
+        self.assertNotIn('owner', data)
+
     def test_instance_name_field_with_api_version(self):
         instance_name = 'test-instance-name'
         obj_utils.create_test_node(self.context,

ironic/tests/unit/common/test_image_service.py

@@ -1538,6 +1538,28 @@ def test__validate_url_is_specific_not_specific(self):
                           self.service._validate_url_is_specific,
                           'oci://foo/bar@baz:meow')
 
+    @mock.patch.object(ociclient, '__init__',
+                       return_value=None, autospec=True)
+    def test_init_verify_ca_path(self, mock_client):
+        cfg.CONF.set_override('webserver_verify_ca', '/some/path')
+        image_service.OciImageService()
+        mock_client.assert_called_with(
+            mock.ANY, verify='/some/path')
+
+    @mock.patch.object(ociclient, '__init__',
+                       return_value=None, autospec=True)
+    def test_init_verify_ca_true(self, mock_client):
+        cfg.CONF.set_override('webserver_verify_ca', 'True')
+        image_service.OciImageService()
+        mock_client.assert_called_with(mock.ANY, verify=True)
+
+    @mock.patch.object(ociclient, '__init__',
+                       return_value=None, autospec=True)
+    def test_init_verify_ca_false(self, mock_client):
+        cfg.CONF.set_override('webserver_verify_ca', 'False')
+        image_service.OciImageService()
+        mock_client.assert_called_with(mock.ANY, verify=False)
+
 
 class ServiceGetterTestCase(base.TestCase):