Version in base suite: 29.0.0-7 Base version: ironic_29.0.0-7 Target version: ironic_29.0.5-0+debu13u1 Base file: /srv/ftp-master.debian.org/ftp/pool/main/i/ironic/ironic_29.0.0-7.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/i/ironic/ironic_29.0.5-0+debu13u1.dsc .gitreview | 1 debian/changelog | 25 debian/patches/CVE-2025-44021_OSSA-2025-001_Disallow+unsafe_image_file_paths.patch | 395 ---------- debian/patches/CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch | 141 +++ debian/patches/CVE-2026-44919_move_file_url_validation_up_into_deploy_utils_main_path.patch | 197 ++++ debian/patches/series | 3 devstack/lib/ironic | 13 doc/source/install/standalone/enrollment.rst | 17 ironic/api/controllers/v1/inspection_rule.py | 20 ironic/api/controllers/v1/node.py | 3 ironic/common/auth_basic.py | 13 ironic/common/checksum_utils.py | 7 ironic/common/exception.py | 7 ironic/common/glance_service/image_service.py | 73 + ironic/common/glance_service/service_utils.py | 51 + ironic/common/image_service.py | 373 ++++++--- ironic/common/images.py | 16 ironic/common/inspection_rules/actions.py | 29 ironic/common/inspection_rules/base.py | 87 +- ironic/common/inspection_rules/engine.py | 4 ironic/common/inspection_rules/operators.py | 82 +- ironic/common/inspection_rules/validation.py | 13 ironic/common/molds.py | 23 ironic/common/oci_registry.py | 85 +- ironic/common/pxe_utils.py | 39 ironic/common/states.py | 5 ironic/conductor/manager.py | 11 ironic/conf/conductor.py | 15 ironic/conf/default.py | 6 ironic/conf/types.py | 55 + ironic/drivers/modules/agent_base.py | 15 ironic/drivers/modules/console_utils.py | 25 ironic/drivers/modules/image_cache.py | 13 ironic/drivers/modules/inspect_utils.py | 5 ironic/drivers/modules/inspector/agent.py | 13 ironic/drivers/modules/inspector/hooks/local_link_connection.py | 6 ironic/drivers/modules/inspector/hooks/validate_interfaces.py | 5 ironic/drivers/modules/ipmitool.py | 56 + ironic/drivers/modules/network/flat.py | 5 ironic/drivers/modules/redfish/inspect.py | 3 ironic/drivers/modules/redfish/management.py | 110 ++ ironic/drivers/modules/redfish/power.py | 41 + ironic/tests/unit/api/controllers/v1/test_inspection_rule.py | 8 ironic/tests/unit/api/controllers/v1/test_node.py | 51 + ironic/tests/unit/api/test_middleware.py | 19 ironic/tests/unit/common/test_glance_service.py | 68 + ironic/tests/unit/common/test_image_service.py | 251 ++++-- ironic/tests/unit/common/test_images.py | 14 ironic/tests/unit/common/test_inspection_rule.py | 308 ++++++- ironic/tests/unit/common/test_molds.py | 47 + ironic/tests/unit/common/test_oci_registry.py | 55 + ironic/tests/unit/common/test_pxe_utils.py | 77 + ironic/tests/unit/conductor/test_manager.py | 32 ironic/tests/unit/conf/test_conductor.py | 34 ironic/tests/unit/conf/test_types.py | 63 + ironic/tests/unit/drivers/modules/inspector/hooks/test_local_link_connection.py | 3 ironic/tests/unit/drivers/modules/network/test_flat.py | 15 ironic/tests/unit/drivers/modules/redfish/test_inspect.py | 23 ironic/tests/unit/drivers/modules/redfish/test_management.py | 158 ++++ ironic/tests/unit/drivers/modules/redfish/test_power.py | 187 ++++ ironic/tests/unit/drivers/modules/test_agent_base.py | 38 ironic/tests/unit/drivers/modules/test_console_utils.py | 82 ++ ironic/tests/unit/drivers/modules/test_deploy_utils.py | 14 ironic/tests/unit/drivers/modules/test_image_cache.py | 29 ironic/tests/unit/drivers/modules/test_inspect_utils.py | 7 ironic/tests/unit/drivers/modules/test_ipmitool.py | 101 ++ ironic/tests/unit/stubs.py | 18 releasenotes/notes/add_allow_image_access_via_auth_token-1b5869f1c0999bea.yaml | 19 releasenotes/notes/agent-inspect-hooks-cleanup-error-c8901a7f8ad0dfd3.yaml | 7 releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml | 7 releasenotes/notes/bootloader-paths-creation-b5097003f25a18ad.yaml | 5 releasenotes/notes/catch-redfish-409-0819174174245ade.yaml | 11 releasenotes/notes/config-redfish-compatible-bmc-3c54a945a7aa2a7f.yaml | 8 releasenotes/notes/control-pxe-enabled-field-inspection-206f67c6638a0bdb.yaml | 7 releasenotes/notes/escape-socat-console-string-arguments-555388ab8dcb8cc3.yaml | 5 releasenotes/notes/fix-bug-2148317-471adcfac69791dc.yaml | 19 releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml | 6 releasenotes/notes/fix-loop-functionality-in-inspection-rules-9bf61e8355297804.yaml | 7 releasenotes/notes/fix-order-of-disable-ramdisk-validation-for-runbooks-e32617f1e9227e65.yaml | 6 releasenotes/notes/fix-redfish-async-updates-servicewait-e834ae30c5f72641.yaml | 9 releasenotes/notes/fix-redfish-boot-device-full-request-6ff0ee231ee6e663.yaml | 11 releasenotes/notes/fix-swift-for-inventory-c371da65dd20fc74.yaml | 7 releasenotes/notes/fixes-inspection-rules-schema-validation-5cac6058d12ce030.yaml | 9 releasenotes/notes/flat-driver-rebind-no-vifs-192c9be8e6962d46.yaml | 6 releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml | 11 releasenotes/notes/image-download-performance-0bf1af5556c1adbf.yaml | 9 releasenotes/notes/inspect-hook-local-link-connection-crash-394edb1c35354968.yaml | 6 releasenotes/notes/oci-fixes-bbbcc633394252f6.yaml | 5 releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml | 29 releasenotes/notes/permit-retries-with-agent-startup-aebfc36a775794c3.yaml | 11 releasenotes/notes/redfish-full-boot-payload-asrockrack-79a9291da8ec5a50.yaml | 14 releasenotes/notes/service-wait-unprovision-dacfa468824335b7.yaml | 5 releasenotes/notes/validate-interfaces-hook-49d7d6c57929a8cd.yaml | 5 tox.ini | 16 zuul.d/ironic-jobs.yaml | 4 zuul.d/project.yaml | 4 96 files changed, 3141 insertions(+), 935 deletions(-) dpkg-source: warning: cannot verify inline signature for /srv/release.debian.org/tmp/tmpepvfq7vr/ironic_29.0.0-7.dsc: no acceptable signature found dpkg-source: warning: cannot verify inline signature for /srv/release.debian.org/tmp/tmpepvfq7vr/ironic_29.0.5-0+debu13u1.dsc: no acceptable signature found diff -Nru ironic-29.0.0/.gitreview ironic-29.0.5/.gitreview --- ironic-29.0.0/.gitreview 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/.gitreview 2026-04-30 14:30:48.000000000 +0000 @@ -2,3 +2,4 @@ host=review.opendev.org port=29418 project=openstack/ironic.git +defaultbranch=stable/2025.1 diff -Nru ironic-29.0.0/debian/changelog ironic-29.0.5/debian/changelog --- ironic-29.0.0/debian/changelog 2025-07-11 11:58:33.000000000 +0000 +++ ironic-29.0.5/debian/changelog 2026-04-30 08:05:36.000000000 +0000 @@ -1,3 +1,28 @@ +ironic (1:29.0.5-0+debu13u1) trixie; urgency=medium + + * New upstream release. Include fix for: + - CVE-2026-42997 / OSSA-2026-010: Credential Forwarding to Arbitrary + Endpoints via Ironic’s idrac Configuration molds Feature + (Closes: #1135898). + - CVE-2026-42510 / OSSA-2026-008: Command Injection in Ironic IPMI Console + Implementations. Applied upstream patch: "Shell-quote console command + passed to socat" (Closes: #1135255). + * CVE-2026-44916: instance_info['ks_template'] is rendered without + sandboxing. An attacker with sufficient access, an ironic deployment with + the anaconda deploy interface, a node with the anaconda deployment + interface set by an admin, and a malicious template could result in + conductor internal data being rendered and if the infrastucture operator is + allowing traffic egress for the provisioning network, could have sensitive + internal data exfiled out of the environment. Applied upstream patch: + - CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch + (Closes: #1136005). + * CVE-2026-44919: during image handling, an infinite loop in checksum + calculations can occur via the file:///dev/zero URL. Add upstream patch: + move_file_url_validation_up_into_deploy_utils_main_path.patch. + (Closes: #1136655). + + -- Thomas Goirand Thu, 30 Apr 2026 10:05:36 +0200 + ironic (1:29.0.0-7) unstable; urgency=medium * export OS_OSLO_MESSAGING_RABBIT__PROCESSNAME for all daemons. diff -Nru ironic-29.0.0/debian/patches/CVE-2025-44021_OSSA-2025-001_Disallow+unsafe_image_file_paths.patch ironic-29.0.5/debian/patches/CVE-2025-44021_OSSA-2025-001_Disallow+unsafe_image_file_paths.patch --- ironic-29.0.0/debian/patches/CVE-2025-44021_OSSA-2025-001_Disallow+unsafe_image_file_paths.patch 2025-07-11 11:58:33.000000000 +0000 +++ ironic-29.0.5/debian/patches/CVE-2025-44021_OSSA-2025-001_Disallow+unsafe_image_file_paths.patch 1970-01-01 00:00:00.000000000 +0000 @@ -1,395 +0,0 @@ -Author: Jay Faulkner -Date: Mon, 21 Apr 2025 15:57:37 -0700 -Description: CVE-2025-44021 / OSSA-2025-001: Disallow unsafe image file:// paths - Before this change, Ironic did not filter file:// paths when used as an - image source except to ensure they were a file (and not, e.g. a - character device). This is problematic from a security perspective - because you could end up with config files from well-known paths being - written to disk on a node. - . - The allowlist default list is huge, but it includes all known usages of - file:// URLs across Bifrost, Ironic, Metal3, and OpenShift in both CI - and default configuration. - . - For the backportable version of this patch for stable branches, we have - omitted the unconditional block of system paths in order to permit - operators using those branches to fully disable the new security - functionality. -Generated-by: Jetbrains Junie -Bug: https://launchpad.net/bugs/2107847 -Bug-Debian: https://bugs.debian.org/1104964 -Change-Id: I2fa995439ee500f9dd82ec8ccfa1a25ee8e1179c - -diff --git a/doc/source/install/standalone/enrollment.rst b/doc/source/install/standalone/enrollment.rst -index 4fd9ebb..78e5d2f 100644 ---- a/doc/source/install/standalone/enrollment.rst -+++ b/doc/source/install/standalone/enrollment.rst -@@ -32,13 +32,20 @@ - $ sha256sum image.qcow2 - 9f6c942ad81690a9926ff530629fb69a82db8b8ab267e2cbd59df417c1a28060 image.qcow2 - --* :ref:`direct-deploy` started supporting ``file://`` images in the Victoria -- release cycle, before that only HTTP(s) had been supported. -+* If you're using :ref:`direct-deploy` with ``file://`` URLs, you have to -+ ensure the images meet all requirements: -+ -+ * File images must be accessible to every conductor -+ * File images must be located in a path listed in -+ :oslo_config:option:`conductor.file_url_allowed_paths` -+ * File images must not be located in ``/dev``, ``/sys``, ``/proc``, -+ ``/etc``, ``/boot``, ``/run`` or other system paths starting with the -+ Ironic 2025.2 release. - - .. warning:: -- File images must be accessible to every conductor! Use a shared file -- system if you have more than one conductor. The ironic CLI tool will not -- transfer the file from a local machine to the conductor(s). -+ The Ironic CLI tool will not transfer the file from a local machine to the -+ conductor(s). Operators should use shared file systems or configuration -+ management to ensure consistent availability of images. - - .. note:: - The Bare Metal service tracks content changes for non-Glance images by -diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py -index f656f18..99ac390 100644 ---- a/ironic/common/image_service.py -+++ b/ironic/common/image_service.py -@@ -813,14 +813,32 @@ - - :param image_href: Image reference. - :raises: exception.ImageRefValidationFailed if source image file -- doesn't exist. -- :returns: Path to image file if it exists. -+ doesn't exist, is in a blocked path, or is not in an allowed path. -+ :returns: Path to image file if it exists and is allowed. - """ - image_path = urlparse.urlparse(image_href).path -+ -+ # Check if the path is in the blocklist -+ rpath = os.path.abspath(image_path) -+ -+ # Check if the path is in the allowlist -+ for allowed in CONF.conductor.file_url_allowed_paths: -+ if rpath == allowed or rpath.startswith(allowed + os.sep): -+ break -+ else: -+ raise exception.ImageRefValidationFailed( -+ image_href=image_href, -+ reason=_( -+ "Security: Path %s is not allowed for image source " -+ "file URLs" % image_path) -+ ) -+ -+ # Check if the file exists - if not os.path.isfile(image_path): - raise exception.ImageRefValidationFailed( - image_href=image_href, - reason=_("Specified image file not found.")) -+ - return image_path - - def download(self, image_href, image_file): -diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py -index 43aab0f..a361889 100644 ---- a/ironic/conf/conductor.py -+++ b/ironic/conf/conductor.py -@@ -21,6 +21,7 @@ - from ironic.common import boot_modes - from ironic.common.i18n import _ - from ironic.common import lessee_sources -+from ironic.conf import types as ir_types - - - opts = [ -@@ -531,6 +532,20 @@ - 'wish to disable this functionality, otherwise it is ' - 'automaticly 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'], -+ item_type=ir_types.ExplicitAbsolutePath(), -+ help=_( -+ 'List of paths that are allowed to be used as file:// ' -+ 'URLs. Files in /boot, /dev, /etc, /proc, /sys and other' -+ 'system paths are always disallowed for security reasons. ' -+ 'Any files in this path readable by ironic may be used as ' -+ 'an image source when deploying. Setting this value to ' -+ '"" (empty) disables file:// URL support. Paths listed ' -+ 'here are validated as absolute paths and will be rejected' -+ 'if they contain path traversal mechanisms, such as "..".' -+ )), - ] - - -diff --git a/ironic/conf/types.py b/ironic/conf/types.py -new file mode 100644 -index 0000000..b9003d8 ---- /dev/null -+++ b/ironic/conf/types.py -@@ -0,0 +1,55 @@ -+# Licensed under the Apache License, Version 2.0 (the "License"); you may not -+# use this file except in compliance with the License. You may obtain a copy -+# of the License at -+# -+# http://www.apache.org/licenses/LICENSE-2.0 -+# -+# Unless required by applicable law or agreed to in writing, software -+# distributed under the License is distributed on an "AS IS" BASIS, -+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -+# See the License for the specific language governing permissions and -+# limitations under the License. -+import os -+ -+from oslo_config import types -+ -+ -+class ExplicitAbsolutePath(types.ConfigType): -+ """Explicit Absolute path type. -+ -+ Absolute path values do not get transformed and are returned as -+ strings. They are validated to ensure they are absolute paths and are equal -+ to os.path.abspath(value) -- protecting from path traversal issues. -+ -+ Python path libraries generally define "absolute path" as anything -+ starting with a /, so tools like path.PurePath(str).is_absolute() is not -+ useful, because it will happily return that /tmp/../etc/resolv.conf is -+ absolute. This type is to be used in cases where we require the path to -+ be explicitly absolute. -+ -+ :param type_name: Type name to be used in the sample config file. -+ """ -+ -+ def __init__(self, type_name='explicit absolute path'): -+ super().__init__(type_name=type_name) -+ -+ def __call__(self, value): -+ value = str(value) -+ -+ # NOTE(JayF): This removes trailing / if provided, since -+ # os.path.abspath will not return a trailing slash. -+ if len(value) > 1: -+ value = value.rstrip('/') -+ absvalue = os.path.abspath(value) -+ if value != absvalue: -+ raise ValueError('Value must be an absolute path ' -+ 'containing no path traversal mechanisms. Config' -+ f'item was: {value}, but resolved to {absvalue}') -+ -+ return value -+ -+ def __repr__(self): -+ return 'explicit absolute path' -+ -+ def _formatter(self, value): -+ return self.quote_trailing_and_leading_space(value) -diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py -index 2623614..f614ce9 100644 ---- a/ironic/tests/unit/common/test_image_service.py -+++ b/ironic/tests/unit/common/test_image_service.py -@@ -615,20 +615,58 @@ - def setUp(self): - super(FileImageServiceTestCase, self).setUp() - self.service = image_service.FileImageService() -- self.href = 'file:///home/user/image.qcow2' -- self.href_path = '/home/user/image.qcow2' -+ self.href = 'file:///var/lib/ironic/images/image.qcow2' -+ self.href_path = '/var/lib/ironic/images/image.qcow2' - - @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) - def test_validate_href(self, path_exists_mock): - self.service.validate_href(self.href) - path_exists_mock.assert_called_once_with(self.href_path) - -- @mock.patch.object(os.path, 'isfile', return_value=False, autospec=True) -+ @mock.patch.object(os.path, 'isfile', return_value=False, -+ autospec=True) - def test_validate_href_path_not_found_or_not_file(self, path_exists_mock): - self.assertRaises(exception.ImageRefValidationFailed, - self.service.validate_href, self.href) - path_exists_mock.assert_called_once_with(self.href_path) - -+ @mock.patch.object(os.path, 'abspath', autospec=True) -+ def test_validate_href_empty_allowlist(self, abspath_mock): -+ abspath_mock.return_value = self.href_path -+ cfg.CONF.set_override('file_url_allowed_paths', [], 'conductor') -+ self.assertRaisesRegex(exception.ImageRefValidationFailed, -+ "is not allowed for image source file URLs", -+ self.service.validate_href, self.href) -+ -+ @mock.patch.object(os.path, 'abspath', autospec=True) -+ def test_validate_href_not_in_allowlist(self, abspath_mock): -+ href = "file:///var/is/allowed/not/this/path/image.qcow2" -+ href_path = "/var/is/allowed/not/this/path/image.qcow2" -+ abspath_mock.side_effect = ['/var/lib/ironic', href_path] -+ cfg.CONF.set_override('file_url_allowed_paths', ['/var/lib/ironic'], -+ 'conductor') -+ self.assertRaisesRegex(exception.ImageRefValidationFailed, -+ "is not allowed for image source file URLs", -+ self.service.validate_href, href) -+ -+ @mock.patch.object(os.path, 'abspath', autospec=True) -+ @mock.patch.object(os.path, 'isfile', -+ return_value=True, autospec=True) -+ def test_validate_href_in_allowlist(self, -+ path_exists_mock, -+ abspath_mock): -+ href_dir = '/var/lib' # self.href_path is in /var/lib/ironic/images/ -+ # First call is ironic.conf.types.ExplicitAbsolutePath -+ # Second call is in validate_href() -+ abspath_mock.side_effect = [href_dir, self.href_path] -+ cfg.CONF.set_override('file_url_allowed_paths', [href_dir], -+ 'conductor') -+ result = self.service.validate_href(self.href) -+ self.assertEqual(self.href_path, result) -+ path_exists_mock.assert_called_once_with(self.href_path) -+ abspath_mock.assert_has_calls( -+ [mock.call(href_dir), mock.call(self.href_path)]) -+ - @mock.patch.object(os.path, 'getmtime', return_value=1431087909.1641912, - autospec=True) - @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True) -diff --git a/ironic/tests/unit/conf/test_conductor.py b/ironic/tests/unit/conf/test_conductor.py -new file mode 100644 -index 0000000..abc583c ---- /dev/null -+++ b/ironic/tests/unit/conf/test_conductor.py -@@ -0,0 +1,34 @@ -+# Licensed under the Apache License, Version 2.0 (the "License"); you may -+# not use this file except in compliance with the License. You may obtain -+# a copy of the License at -+# -+# http://www.apache.org/licenses/LICENSE-2.0 -+# -+# Unless required by applicable law or agreed to in writing, software -+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -+# License for the specific language governing permissions and limitations -+# under the License. -+ -+from ironic.conf import CONF -+from ironic.tests.base import TestCase -+ -+ -+class ValidateConductorAllowedPaths(TestCase): -+ def test_abspath_validation_bad_path_raises(self): -+ """Verifies setting a relative path raises an error via oslo.config.""" -+ self.assertRaises( -+ ValueError, -+ CONF.set_override, -+ 'file_url_allowed_paths', -+ ['bad/path'], -+ 'conductor' -+ ) -+ -+ def test_abspath_validation_good_paths(self): -+ """Verifies setting an absolute path works via oslo.config.""" -+ CONF.set_override('file_url_allowed_paths', ['/var'], 'conductor') -+ -+ def test_abspath_validation_good_paths_trailing_slash(self): -+ """Verifies setting an absolute path works via oslo.config.""" -+ CONF.set_override('file_url_allowed_paths', ['/var/'], 'conductor') -diff --git a/ironic/tests/unit/conf/test_types.py b/ironic/tests/unit/conf/test_types.py -new file mode 100644 -index 0000000..889638c ---- /dev/null -+++ b/ironic/tests/unit/conf/test_types.py -@@ -0,0 +1,63 @@ -+# Licensed under the Apache License, Version 2.0 (the "License"); you may -+# not use this file except in compliance with the License. You may obtain -+# a copy of the License at -+# -+# http://www.apache.org/licenses/LICENSE-2.0 -+# -+# Unless required by applicable law or agreed to in writing, software -+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -+# License for the specific language governing permissions and limitations -+# under the License. -+ -+from ironic.conf import types -+from ironic.tests.base import TestCase -+ -+ -+class ExplicitAbsolutePath(TestCase): -+ def test_explicit_absolute_path(self): -+ """Verifies the Opt subclass used to validate absolute paths.""" -+ good_paths = [ -+ '/etc/passwd', # Valid -+ '/usr/bin/python', # Valid -+ '/home/user/file.txt', # Valid - dot in filename allowed -+ '/var/lib/ironic/.secretdir', # Valid - hidden directory allowed -+ '/var/lib/ironic/oslo.config', # Valid - dots in filename allowed -+ '/tmp/', # Valid -+ '/', # Valid (root directory) -+ '/.hidden_root_file', # Valid -+ '/path/including/a/numb3r', # Valid -+ '/a/path/with/a/trailing/slash/' # Valid -+ ] -+ bad_paths = [ -+ 'relative/path', # Invalid - no leading slash -+ './file.txt', # Invalid - relative path -+ '../file.txt', # Invalid - relative path -+ 'file.txt', # Invalid - no leading slash -+ '', # Invalid - empty string -+ '/var/lib/ironic/../../../etc/passwd', # Invalid - path traversal -+ '/etc/../etc/passwd', # Invalid - path traversal -+ '/home/user/./config', # Invalid - contains current dir reference -+ '/home/user/../user/config', # Invalid - path traversal -+ '/../etc/passwd', # Invalid - path traversal at beginning -+ '/.', # Invalid - just current directory -+ '/..' # Invalid - just parent directory -+ ] -+ -+ eap = types.ExplicitAbsolutePath() -+ -+ def _trypath(tpath): -+ try: -+ eap(tpath) -+ except ValueError: -+ return False -+ else: -+ return True -+ -+ for path in good_paths: -+ self.assertTrue(_trypath(path), -+ msg=f"Improperly disallowed path: {path}") -+ -+ for path in bad_paths: -+ self.assertFalse(_trypath(path), -+ msg=f"Improperly allowed path: {path}") -diff --git a/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml b/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml -new file mode 100644 -index 0000000..2e14820 ---- /dev/null -+++ b/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml -@@ -0,0 +1,29 @@ -+--- -+security: -+ - | -+ Fixes OSSA-2025-001, where Ironic did not properly filter file:// paths -+ when used as image sources. This would permit any file accessible by the -+ conductor to be used as an image to attempt deployment. -+ -+ Adds ``CONF.conductor.file_url_allowed_paths``, an allowlist configuration -+ defaulting to ``/var/lib/ironic``, ``/shared/html``, -+ ``/opt/cache/files``, ``/vagrant``, and ``/templates``, -+ permits operators to further restrict where the conductor will fetch -+ images for when provided a file:// URL. This default value was chosen -+ based on known usage by projects downstream of Ironic, including Metal3, -+ Bifrost, and OpenShift. These defaults may change to be more restrictive -+ at a later date. Operators using file:// URLs are encouraged to explicitly -+ set this value even if the current default is sufficient. Operators wishing -+ to fully disable the ability to deploy with a file:// URL should set this -+ configuration to "" (empty). -+ -+ Operators wishing to restore the original insecure behavior should set -+ ``CONF.conductor.file_url_allowed_paths`` to ``/``. Take note that in the -+ 2025.2 release and later, ``/dev``, ``/sys``, ``/proc``, ``/run``, and -+ ``/etc`` will be unconditionally blocked as a security measure. -+ -+ This issue only poses a significant security risk when Ironic's -+ automated cleaning process is disabled and the service is configured in -+ such a way that permits direct deployment by an untrusted API user, such as -+ standalone Ironic installations or environments granting ownership of nodes -+ to projects. diff -Nru ironic-29.0.0/debian/patches/CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch ironic-29.0.5/debian/patches/CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch --- ironic-29.0.0/debian/patches/CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/debian/patches/CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch 2026-04-30 08:05:36.000000000 +0000 @@ -0,0 +1,141 @@ +Author: Julia Kreger +Date: Tue, 28 Apr 2026 11:11:22 -0700 +Description: CVE-2026-44916: security: Use sandbox rendering for jinja2 + Analysis revealed that a malicious attacker with sufficent + access to request a node to be provisioned could supply a + maliciously crafted kickstart template configuration, + which would then be rendered in an unsafe form ultimately. + . + This is because the underlying render utility was modeled + for rendering only admin-suppied files or the in-code tree + files. Anaconda had to take this further by allowing the + jinja utilized to be user supplied. + . + Anyhow, an attacker with sufficient access, an ironic deployment + with the anaconda deploy interface, a node with the anaconda + deployment interface set by an admin, and a malicious template + could result in conductor internal data being rendered and if + the infrastucture operator is allowing traffic egress for the + provisioning network, could have sensitive internal data + exfiled out of the environment. + . + The render helper has been changed to utilize a sandboxed + environment. Attacks such as this now internally raise + a Jinja2 SecurityError. +Bug: https://launchpad.net/bugs/2148307 +Change-Id: Ie85357166fafca0acd9d852fe05ce34818d2b366 +Signed-off-by: Julia Kreger +Origin: upstream, https://review.opendev.org/c/openstack/ironic/+/987776 +Last-Update: 2026-05-08 + +diff --git a/ironic/common/utils.py b/ironic/common/utils.py +index b126668..0182173 100644 +--- a/ironic/common/utils.py ++++ b/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 +@@ -495,6 +496,8 @@ + :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) +@@ -502,11 +505,9 @@ + 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 +diff --git a/ironic/tests/unit/common/test_utils.py b/ironic/tests/unit/common/test_utils.py +index 2da8f05..cf8e62d 100644 +--- a/ironic/tests/unit/common/test_utils.py ++++ b/ironic/tests/unit/common/test_utils.py +@@ -555,6 +555,11 @@ + self.template = '{{ foo }} {{ bar }}' + self.params = {'foo': 'spam', 'bar': 'ham'} + self.expected = 'spam ham' ++ self.bogus_template = ("{{ foo }}{{ ''.__class__.__mro__[1]." ++ "__subclasses__() }} {{ bar }}") ++ self.bogus_template2 = ("{{ foo }}{{ ''|attr('__class__')|attr('__mro" ++ "__')|attr('__getitem__')(1)|attr('__subclass" ++ "es__')() }} {{ bar }}") + + def test_render_string(self): + self.assertEqual(self.expected, +@@ -581,6 +586,46 @@ + self.params)) + jinja_fsl_mock.assert_called_once_with('/path/to') + ++ def test_render_bogus_string(self): ++ """Jinja explicitly blocks access access to .__ delimited items.""" ++ self.assertRaises(jinja2.exceptions.SecurityError, ++ utils.render_template, ++ self.bogus_template, ++ self.params, ++ is_file=False) ++ ++ @mock.patch('ironic.common.utils.jinja2.FileSystemLoader', autospec=True) ++ def test_render_bogus_file(self, jinja_fsl_mock): ++ """Jinja explicitly blocks access access to .__ delimited items.""" ++ path = '/path/to/template.j2' ++ jinja_fsl_mock.return_value = jinja2.DictLoader( ++ {'template.j2': self.bogus_template}) ++ self.assertRaises(jinja2.exceptions.SecurityError, ++ utils.render_template, ++ path, ++ self.params) ++ jinja_fsl_mock.assert_called_once_with('/path/to') ++ ++ def test_render_bogus_string_attr(self): ++ """Jinja explicitly blocks access access to .__ delimited items.""" ++ self.assertRaises(jinja2.exceptions.SecurityError, ++ utils.render_template, ++ self.bogus_template2, ++ self.params, ++ is_file=False) ++ ++ @mock.patch('ironic.common.utils.jinja2.FileSystemLoader', autospec=True) ++ def test_render_bogus_file_attr(self, jinja_fsl_mock): ++ """Jinja explicitly blocks access access to .__ delimited items.""" ++ path = '/path/to/template.j2' ++ jinja_fsl_mock.return_value = jinja2.DictLoader( ++ {'template.j2': self.bogus_template2}) ++ self.assertRaises(jinja2.exceptions.SecurityError, ++ utils.render_template, ++ path, ++ self.params) ++ jinja_fsl_mock.assert_called_once_with('/path/to') ++ + + class ValidateConductorGroupTestCase(base.TestCase): + def test_validate_conductor_group_success(self): +diff --git a/releasenotes/notes/fix-bug-2148307-ddf0b4d69244e86e.yaml b/releasenotes/notes/fix-bug-2148307-ddf0b4d69244e86e.yaml +new file mode 100644 +index 0000000..c905254 +--- /dev/null ++++ b/releasenotes/notes/fix-bug-2148307-ddf0b4d69244e86e.yaml +@@ -0,0 +1,8 @@ ++--- ++fixes: ++ - | ++ Fixes security issue identified in ++ `bug 2148307 `_ ++ where users of the anaconda deployment interface could supply ++ a malicious template. The conductor process now sandboxes all ++ jinja2 rendering operations. diff -Nru ironic-29.0.0/debian/patches/CVE-2026-44919_move_file_url_validation_up_into_deploy_utils_main_path.patch ironic-29.0.5/debian/patches/CVE-2026-44919_move_file_url_validation_up_into_deploy_utils_main_path.patch --- ironic-29.0.0/debian/patches/CVE-2026-44919_move_file_url_validation_up_into_deploy_utils_main_path.patch 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/debian/patches/CVE-2026-44919_move_file_url_validation_up_into_deploy_utils_main_path.patch 2026-04-30 08:05:36.000000000 +0000 @@ -0,0 +1,197 @@ +Author: Julia Kreger +Date: Thu, 30 Apr 2026 15:45:54 -0700 +Description: CVE-2026-44919: move file url validation up into deploy_utils main path + An issue was discovered where we were executing checksums + prior to doing file path guard logic. We've moved the check + into the same area of the code where we do all other url checks + for consistency. + . + This issue is tracked as CVE-2026-44919. + . +Bug: https://launchpad.net/bugs/2150332 +Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1136655 +Change-Id: I09fa51801cf40da6f7be31014cca63ad1c253a5a +Signed-off-by: Julia Kreger +Origin: upstream, https://review.opendev.org/c/openstack/ironic/+/988357 +Last-Update: 2026-05-16 + +diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py +index c68be56..209f636 100644 +--- a/ironic/common/image_service.py ++++ b/ironic/common/image_service.py +@@ -909,8 +909,9 @@ + doesn't exist, is in a blocked path, or is not in an allowed path. + :returns: Path to image file if it exists and is allowed. + """ ++ # TODO(TheJulia): Validate there are *THREE* slashes in the file path ++ # URL, otherwise urlparse doesn't split it properly. + image_path = urlparse.urlparse(image_href).path +- + # Check if the path is in the blocklist + rpath = os.path.abspath(image_path) + +diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py +index c1d3de0..e0b2a61 100644 +--- a/ironic/drivers/modules/deploy_utils.py ++++ b/ironic/drivers/modules/deploy_utils.py +@@ -1254,6 +1254,11 @@ + def _cache_and_convert_image(task, instance_info, image_info=None): + """Cache an image locally and convert it to RAW if needed. + ++ Caches the supplied image as defined in the request, and converts it ++ to raw if required. This method should only be called once initial ++ first pass validation has been performed and can be called on multiple ++ code paths where the file contents must be downloaded. ++ + :param task: The Taskmanager object related to this action. + :param instance_info: The instance_info field being used in + association with this method call. +@@ -1515,8 +1520,8 @@ + di_info['image_source'] = image_source + node.driver_internal_info = di_info + if not is_glance_image: +- if (image_source.startswith('file://') +- or image_download_source == 'local'): ++ is_file_url = image_source.startswith('file://') ++ if (is_file_url or image_download_source == 'local'): + # In this case, we're explicitly downloading (or copying a file) + # hosted locally so IPA can download it directly from Ironic. + +@@ -1524,7 +1529,14 @@ + # based deploy source since we don't want to, nor should we be in + # in the business of copying large numbers of files as it is a + # huge performance impact. +- ++ if is_file_url: ++ # In this case, we need to validate the URL first before ++ # moving on to _cache_and_convert_image, because it's whole ++ # existence is to download, checksum, convert, etc. ++ image_service.FileImageService().validate_href( ++ image_href=image_source) ++ # Either the file is local, or the file needs to be downloaded. ++ # _cache_and_convert_image handles both cases + _cache_and_convert_image(task, instance_info) + else: + # This is the "all other cases" logic for aspects like the user +diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py +index 69c0cce..fb8d643 100644 +--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py ++++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py +@@ -2664,6 +2664,54 @@ + self.assertEqual('https://image-url/file', + task.node.instance_info['image_source']) + ++ @mock.patch.object(os.path, 'isfile', autospec=True) ++ @mock.patch.object(utils, '_cache_and_convert_image', autospec=True) ++ def test_build_instance_info_for_deploy_file_url_valid( ++ self, mock_cache_image, mock_isfile): ++ i_info = self.node.instance_info ++ driver_internal_info = self.node.driver_internal_info ++ i_info['image_source'] = 'file:///var/lib/ironic/files/foo/bar' ++ driver_internal_info['is_whole_disk_image'] = True ++ self.node.instance_info = i_info ++ self.node.driver_internal_info = driver_internal_info ++ self.node.save() ++ mock_isfile.return_value = True ++ with task_manager.acquire( ++ self.context, self.node.uuid, shared=False) as task: ++ ++ utils.build_instance_info_for_deploy(task) ++ mock_cache_image.assert_called_once_with( ++ mock.ANY, ++ {'configdrive': 'TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQ=', ++ 'image_url': None, ++ 'foo': 'bar', ++ 'image_source': 'file:///var/lib/ironic/files/foo/bar', ++ 'image_type': 'whole-disk'}) ++ ++ @mock.patch.object(utils, 'cache_instance_image', autospec=True) ++ def test_build_instance_info_for_deploy_file_url_invalid( ++ self, mock_cache_image): ++ mock_cache_image.return_value = ('fake', '/tmp/foo', 'qcow2') ++ i_info = self.node.instance_info ++ driver_internal_info = self.node.driver_internal_info ++ url = 'file:///dev/zero' ++ i_info['image_source'] = url ++ self.node.instance_info = i_info ++ driver_internal_info['is_whole_disk_image'] = True ++ self.node.driver_internal_info = driver_internal_info ++ self.node.save() ++ ++ with task_manager.acquire( ++ self.context, self.node.uuid, shared=False) as task: ++ self.assertRaisesRegex( ++ exception.ImageRefValidationFailed, ++ 'Validation of image href file:///dev/zero failed, reason: ' ++ 'Security: Path /dev/zero is not allowed for image source ' ++ 'file URLs', ++ utils.build_instance_info_for_deploy, task) ++ ++ mock_cache_image.assert_not_called() ++ + + class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): + def setUp(self): +@@ -2806,41 +2854,6 @@ + + @mock.patch.object(image_service.HttpImageService, 'validate_href', + autospec=True) +- def test_build_instance_info_file_image(self, validate_href_mock): +- i_info = self.node.instance_info +- driver_internal_info = self.node.driver_internal_info +- i_info['image_source'] = 'file://image-ref' +- i_info['image_checksum'] = 'aa' +- i_info['root_gb'] = 10 +- driver_internal_info['is_whole_disk_image'] = True +- self.node.instance_info = i_info +- self.node.driver_internal_info = driver_internal_info +- self.node.save() +- +- expected_url = ( +- 'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid) +- +- with task_manager.acquire( +- self.context, self.node.uuid, shared=False) as task: +- +- info = utils.build_instance_info_for_deploy(task) +- +- self.assertEqual(expected_url, info['image_url']) +- self.assertEqual('sha256', info['image_os_hash_algo']) +- self.assertEqual('fake-checksum', info['image_os_hash_value']) +- self.assertEqual('raw', info['image_disk_format']) +- self.cache_image_mock.assert_called_once_with( +- task.context, task.node, force_raw=True, +- expected_format=None, +- expected_checksum='aa', +- expected_checksum_algo=None) +- self.checksum_mock.assert_called_once_with( +- self.fake_path, algorithm='sha256') +- validate_href_mock.assert_called_once_with( +- mock.ANY, expected_url, False) +- +- @mock.patch.object(image_service.HttpImageService, 'validate_href', +- autospec=True) + def test_build_instance_info_local_image(self, validate_href_mock): + cfg.CONF.set_override('image_download_source', 'local', group='agent') + i_info = self.node.instance_info +diff --git a/releasenotes/notes/fix-file-url-validation-5fb819ef5ae74ddc.yaml b/releasenotes/notes/fix-file-url-validation-5fb819ef5ae74ddc.yaml +new file mode 100644 +index 0000000..0b7b49a +--- /dev/null ++++ b/releasenotes/notes/fix-file-url-validation-5fb819ef5ae74ddc.yaml +@@ -0,0 +1,15 @@ ++--- ++fixes: ++ - | ++ Fixes an issue in the url handling logic of instance deploy logic where ++ file paths validity was not checked upfront, and the conductor service ++ would directly begin calculating checksums. This highlighted a DoS issue ++ where an attacker could exhaust conductor threads by attempting to request, ++ "file:///dev/zero", which would never return the thread. This is because ++ file URLs don't require downloading first, and validity checking logic ++ was all in the file access logic, not checksum logic. ++ ++ Ironic now explicitly invokes the URL/validity checking logic prior to ++ calling the checksum logic which effectively prevents this issue. ++ ++ This issue is tracked as CVE-2026-44919. diff -Nru ironic-29.0.0/debian/patches/series ironic-29.0.5/debian/patches/series --- ironic-29.0.0/debian/patches/series 2025-07-11 11:58:33.000000000 +0000 +++ ironic-29.0.5/debian/patches/series 2026-04-30 08:05:36.000000000 +0000 @@ -1,4 +1,5 @@ adds-alembic.ini-in-MANIFEST.in.patch fix-initial_grub_cfg.template.patch do-not-print.patch -CVE-2025-44021_OSSA-2025-001_Disallow+unsafe_image_file_paths.patch +CVE-2026-44916_Use_sandbox_rendering_for_jinja2.patch +CVE-2026-44919_move_file_url_validation_up_into_deploy_utils_main_path.patch diff -Nru ironic-29.0.0/devstack/lib/ironic ironic-29.0.5/devstack/lib/ironic --- ironic-29.0.0/devstack/lib/ironic 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/devstack/lib/ironic 2026-04-30 14:30:48.000000000 +0000 @@ -36,10 +36,6 @@ # Set up default directories GITDIR["python-ironicclient"]=$DEST/python-ironicclient -GITREPO["pyghmi"]=${PYGHMI_REPO:-${GIT_BASE}/x/pyghmi} -GITBRANCH["pyghmi"]=${PYGHMI_BRANCH:-master} -GITDIR["pyghmi"]=$DEST/pyghmi - GITREPO["virtualbmc"]=${VIRTUALBMC_REPO:-${GIT_BASE}/openstack/virtualbmc.git} GITBRANCH["virtualbmc"]=${VIRTUALBMC_BRANCH:-master} GITDIR["virtualbmc"]=$DEST/virtualbmc @@ -337,7 +333,7 @@ IRONIC_EFIBOOT=${IRONIC_EFIBOOT:-$TOP_DIR/files/ir-deploy-$IRONIC_DEPLOY_DRIVER.efiboot} # NOTE(jroll) this needs to be updated when stable branches are cut -IPA_DOWNLOAD_BRANCH=${IPA_DOWNLOAD_BRANCH:-${TARGET_BRANCH:-master}} +IPA_DOWNLOAD_BRANCH=${IPA_DOWNLOAD_BRANCH:-${TARGET_BRANCH:-stable/2025.1}} IPA_DOWNLOAD_BRANCH=$(echo $IPA_DOWNLOAD_BRANCH | tr / -) IPA_DOWNLOAD_SOURCE=ironic-python-agent @@ -612,7 +608,7 @@ TEMPEST_BAREMETAL_MIN_MICROVERSION=${TEMPEST_BAREMETAL_MIN_MICROVERSION:-} # Define baremetal max_microversion in tempest config. No default value means that it is picked from tempest. -TEMPEST_BAREMETAL_MAX_MICROVERSION=${TEMPEST_BAREMETAL_MAX_MICROVERSION:-} +TEMPEST_BAREMETAL_MAX_MICROVERSION=${TEMPEST_BAREMETAL_MAX_MICROVERSION:-1.96} # TODO(TheJulia): This PHYSICAL_NETWORK needs to be refactored in # our devstack plugin. It is used by the neutron integration, @@ -1810,6 +1806,9 @@ 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 + fi if [[ "$IRONIC_IS_HARDWARE" == "False" ]]; then @@ -3734,7 +3733,7 @@ # NOTE(JayF) Cannot shallow clone; need to checkout a commit that # is not part of a named tag DNSMASQ_DIR=$(mktemp --tmpdir -u dnsmasq.XXXXXX) - git clone http://thekelleys.org.uk/git/dnsmasq.git $DNSMASQ_DIR + git clone git://thekelleys.org.uk/dnsmasq.git $DNSMASQ_DIR pushd $DNSMASQ_DIR # https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commitdiff;h=f006be7842104a9f86fbf419326b7aad08ade61d # above is the specific patch we need; but using the 2.91 RC which diff -Nru ironic-29.0.0/doc/source/install/standalone/enrollment.rst ironic-29.0.5/doc/source/install/standalone/enrollment.rst --- ironic-29.0.0/doc/source/install/standalone/enrollment.rst 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/doc/source/install/standalone/enrollment.rst 2026-04-30 14:30:48.000000000 +0000 @@ -32,13 +32,20 @@ $ sha256sum image.qcow2 9f6c942ad81690a9926ff530629fb69a82db8b8ab267e2cbd59df417c1a28060 image.qcow2 -* :ref:`direct-deploy` started supporting ``file://`` images in the Victoria - release cycle, before that only HTTP(s) had been supported. +* If you're using :ref:`direct-deploy` with ``file://`` URLs, you have to + ensure the images meet all requirements: + + * File images must be accessible to every conductor + * File images must be located in a path listed in + :oslo_config:option:`conductor.file_url_allowed_paths` + * File images must not be located in ``/dev``, ``/sys``, ``/proc``, + ``/etc``, ``/boot``, ``/run`` or other system paths starting with the + Ironic 2025.2 release. .. warning:: - File images must be accessible to every conductor! Use a shared file - system if you have more than one conductor. The ironic CLI tool will not - transfer the file from a local machine to the conductor(s). + The Ironic CLI tool will not transfer the file from a local machine to the + conductor(s). Operators should use shared file systems or configuration + management to ensure consistent availability of images. .. note:: The Bare Metal service tracks content changes for non-Glance images by diff -Nru ironic-29.0.0/ironic/api/controllers/v1/inspection_rule.py ironic-29.0.5/ironic/api/controllers/v1/inspection_rule.py --- ironic-29.0.0/ironic/api/controllers/v1/inspection_rule.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/api/controllers/v1/inspection_rule.py 2026-04-30 14:30:48.000000000 +0000 @@ -41,11 +41,18 @@ def convert_actions(rpc_actions): - return [{ - 'op': action['op'], - 'args': action['args'], - 'loop': action.get('loop', []) - } for action in rpc_actions] + converted_actions = [] + for action in rpc_actions: + result = { + 'op': action['op'], + 'args': action['args'] + } + + if action.get('loop', []): + result['loop'] = action['loop'] + + converted_actions.append(result) + return converted_actions def convert_conditions(rpc_conditions): @@ -205,6 +212,7 @@ @METRICS.timer('InspectionRuleController.post') @method.expose(status_code=http_client.CREATED) @method.body('inspection_rule') + @args.validate(inspection_rule=validation.VALIDATOR) def post(self, inspection_rule): """Create a new inspection rule. @@ -231,7 +239,7 @@ @METRICS.timer('InspectionRuleController.patch') @method.expose() @method.body('patch') - @args.validate(inspection_rule_uuid=args.uuid) + @args.validate(inspection_rule_uuid=args.uuid, patch=args.patch) def patch(self, inspection_rule_uuid, patch=None): """Update an existing inspection rule. diff -Nru ironic-29.0.0/ironic/api/controllers/v1/node.py ironic-29.0.5/ironic/api/controllers/v1/node.py --- ironic-29.0.0/ironic/api/controllers/v1/node.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/api/controllers/v1/node.py 2026-04-30 14:30:48.000000000 +0000 @@ -1254,13 +1254,14 @@ state=rpc_node.provision_state) api_utils.check_allow_configdrive(target, configdrive) - api_utils.check_allow_clean_disable_ramdisk(target, disable_ramdisk) if runbook: clean_steps, service_steps, disable_ramdisk = self._handle_runbook( rpc_node, target, runbook, clean_steps, service_steps ) + api_utils.check_allow_clean_disable_ramdisk(target, disable_ramdisk) + if clean_steps and target != ir_states.VERBS['clean']: msg = (_('"clean_steps" is only valid when setting target ' 'provision state to %s') % ir_states.VERBS['clean']) diff -Nru ironic-29.0.0/ironic/common/auth_basic.py ironic-29.0.5/ironic/common/auth_basic.py --- ironic-29.0.0/ironic/common/auth_basic.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/auth_basic.py 2026-04-30 14:30:48.000000000 +0000 @@ -15,6 +15,7 @@ import base64 import binascii +import functools import logging import bcrypt @@ -91,6 +92,16 @@ unauthorized() +@functools.lru_cache(maxsize=256) +def _checkpw(password, hashed): + """Wrapped bcrypt.checkpw for caching + + Keep an in-memory cache of bcrypt.checkpw responses to avoid the + high CPU cost of repeatedly checking the same values + """ + return bcrypt.checkpw(password, hashed) + + def auth_entry(entry, password): """Compare a password with a single user auth file entry @@ -102,7 +113,7 @@ """ username, encrypted = parse_entry(entry) - if not bcrypt.checkpw(password, encrypted): + if not _checkpw(password, encrypted): LOG.info('Password for %s does not match', username) unauthorized() diff -Nru ironic-29.0.0/ironic/common/checksum_utils.py ironic-29.0.5/ironic/common/checksum_utils.py --- ironic-29.0.0/ironic/common/checksum_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/checksum_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -272,7 +272,8 @@ class TransferHelper(object): - def __init__(self, response, checksum_algo, expected_checksum): + def __init__(self, response, checksum_algo, expected_checksum, + chunk_size=1024 * 1024): """Helper class to drive data download with concurrent checksum. The TransferHelper can be used to help retrieve data from a @@ -284,6 +285,8 @@ :param checksum_algo: The expected checksum algorithm. :param expected_checksum: The expected checksum of the data being transferred. + :param chunk_size: Size of chunks to read during transfer. + Defaults to 1MB. """ # NOTE(TheJulia): Similar code exists in IPA in regards to @@ -297,7 +300,7 @@ # This may artificially throttle transfer speeds a little in # high performance environments as the data may get held up # in the kernel limiting the window from scaling. - self._chunk_size = 1024 * 1024 # 1MB + self._chunk_size = chunk_size self._last_check_time = time.time() self._request = response self._bytes_transferred = 0 diff -Nru ironic-29.0.0/ironic/common/exception.py ironic-29.0.5/ironic/common/exception.py --- ironic-29.0.0/ironic/common/exception.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/exception.py 2026-04-30 14:30:48.000000000 +0000 @@ -526,7 +526,7 @@ class Forbidden(IronicException): - _msg_fmt = _("Requested OpenStack Images API is forbidden") + _msg_fmt = _("Requested OpenStack Baremetal API is forbidden") # TODO(dtantsur): leave only one variant @@ -1100,6 +1100,11 @@ "utilized for the file download.") +class OciImageTagNotFound(ImageNotFound): + _msg_fmt = _("Image tag %(image_url)s could not be found, " + "available tags are %(tags)s") + + class ImageServiceAuthenticationRequired(ImageUnacceptable): _msg_fmt = _("The requested image %(image_ref)s requires " "authentication which has not been provided. " diff -Nru ironic-29.0.0/ironic/common/glance_service/image_service.py ironic-29.0.5/ironic/common/glance_service/image_service.py --- ironic-29.0.0/ironic/common/glance_service/image_service.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/glance_service/image_service.py 2026-04-30 14:30:48.000000000 +0000 @@ -28,6 +28,7 @@ from oslo_utils import uuidutils import tenacity +from ironic.common import checksum_utils from ironic.common import exception from ironic.common.glance_service import service_utils from ironic.common.i18n import _ @@ -43,6 +44,15 @@ LOG = log.getLogger(__name__) _GLANCE_SESSION = None +# Chunk size for streaming image downloads. This MUST be specified explicitly +# when iterating over a requests.Response object. The default chunk size when +# iterating directly over a Response (i.e., `for chunk in resp:`) is only +# 128 bytes, which for a 40GB image results in ~335 million iterations. +# This causes severe CPU starvation and memory pressure, leading to conductor +# heartbeat failures and system unresponsiveness. +# See: https://bugs.launchpad.net/ironic/+bug/2129260 +IMAGE_CHUNK_SIZE = 1024 * 1024 # 1 MiB + def _translate_image_exception(image_id, exc_value): if isinstance(exc_value, (openstack_exc.ForbiddenException)): @@ -113,6 +123,7 @@ self.client = client self.context = context self.endpoint = None + self._transfer_verified_checksum = None @tenacity.retry( retry=tenacity.retry_if_exception_type( @@ -178,14 +189,22 @@ return base_image_meta @check_image_service - def download(self, image_href, data=None): + def download(self, image_href, data=None, checksum=None, + checksum_algo=None): """Calls out to Glance for data and writes data. :param image_href: The opaque image identifier. :param data: (Optional) File object to write data to. + :param checksum: Expected checksum value for validation during + transfer. + :param checksum_algo: Algorithm for checksum (e.g., 'md5', + 'sha256'). """ image_id = service_utils.parse_image_id(image_href) + # Reset transfer checksum for new download + self._transfer_verified_checksum = None + if 'file' in CONF.glance.allowed_direct_url_schemes: location = self._get_location(image_id) url = urlparse.urlparse(location) @@ -198,10 +217,50 @@ image_size = 0 image_data = None if data: - image_chunks = self.call('download_image', image_id, stream=True) - for chunk in image_chunks: - data.write(chunk) - image_size += len(chunk) + # Get streaming response from glance + resp = self.call('download_image', image_id, stream=True) + + # If checksum validation is requested, use TransferHelper + # to calculate checksum during download + if checksum and checksum_algo: + try: + download_helper = checksum_utils.TransferHelper( + resp, checksum_algo, checksum, + chunk_size=IMAGE_CHUNK_SIZE) + for chunk in download_helper: + data.write(chunk) + image_size += len(chunk) + + # Verify checksum matches + if download_helper.checksum_matches: + # Store verified checksum in algo:value format + self._transfer_verified_checksum = ( + f"{checksum_algo}:{checksum}") + LOG.debug("Verified checksum during download of image " + "%(image)s: %(algo)s:%(checksum)s", + {'image': image_href, 'algo': checksum_algo, + 'checksum': checksum}) + else: + raise exception.ImageChecksumError() + except (exception.ImageChecksumAlgorithmFailure, ValueError): + # Fall back to download without checksumming + LOG.warning("Checksum algorithm %(algo)s not available, " + "downloading without incremental checksum for " + "image %(image)s", + {'algo': checksum_algo, 'image': image_href}) + # NOTE(JayF): Must use iter_content with explicit chunk + # size. See IMAGE_CHUNK_SIZE definition for details. + for chunk in resp.iter_content( + chunk_size=IMAGE_CHUNK_SIZE): + data.write(chunk) + image_size += len(chunk) + else: + # No checksum requested, just stream and write. + # NOTE(JayF): Must use iter_content with explicit chunk + # size. See IMAGE_CHUNK_SIZE definition for details. + for chunk in resp.iter_content(chunk_size=IMAGE_CHUNK_SIZE): + data.write(chunk) + image_size += len(chunk) else: image_data = self.call('download_image', image_id).content image_size = len(image_data) @@ -431,6 +490,4 @@ @property def transfer_verified_checksum(self): """The transferred artifact checksum.""" - # FIXME(TheJulia): We should look at and see if we wire - # this up in a future change. - return None + return self._transfer_verified_checksum diff -Nru ironic-29.0.0/ironic/common/glance_service/service_utils.py ironic-29.0.5/ironic/common/glance_service/service_utils.py --- ironic-29.0.0/ironic/common/glance_service/service_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/glance_service/service_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -22,6 +22,7 @@ from oslo_utils import uuidutils from ironic.common import exception +from ironic.common import keystone from ironic.conf import CONF _IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', @@ -34,6 +35,7 @@ LOG = log.getLogger(__name__) +_GLANCE_SESSION = None def _extract_attributes(image): @@ -123,28 +125,40 @@ This check is needed in case Nova and Glance are deployed without authentication turned on. """ + # NOTE: Any support for private/shared images in Ironic requires a secure + # way for ironic to know the original requester: + # - If we trust node[instance_info][project_id], we are susceptible to a + # node.owner stealing another project's private image by lying in + # instance_info. + # - As of 2025.1, the project_id attached to the auth context at this + # point is more likely to be the nova-computes service user rather + # than the original requester. This is a missing feature from the + # Ironic/Nova virt driver. + auth_token = getattr(context, 'auth_token', None) + conductor_project_id = get_conductor_project_id() image_visibility = getattr(image, 'visibility', None) image_owner = getattr(image, 'owner', None) image_id = getattr(image, 'id', 'unknown') is_admin = 'admin' in getattr(context, 'roles', []) - project_id = getattr(context, 'project_id', None) project = getattr(context, 'project', 'unknown') - # The presence of an auth token implies this is an authenticated - # request and we need not handle the noauth use-case. - if auth_token: + # If an auth token is present and the config allows access via auth token, + # allow image access. + if CONF.allow_image_access_via_auth_token and auth_token: # We return true here since we want the *user* request context to # be able to be used. return True - - if image_visibility == 'public': - return True - - if project_id and image_owner == project_id: + # If the image visibility is public or community, allow access. + if image_visibility in ['public', 'community']: return True - + # If the user is an admin and the config allows ignoring project checks for + # admin tasks, allow access. if is_admin and CONF.ignore_project_check_for_admin_tasks: return True + # If the image is private and the owner is the conductor project, + # allow access. + if image_visibility == 'private' and image_owner == conductor_project_id: + return True LOG.info( 'Access to %s owned by %s denied to requester %s', @@ -167,3 +181,20 @@ return False return (image_href.startswith('glance://') or uuidutils.is_uuid_like(image_href)) + + +def get_conductor_project_id(): + global _GLANCE_SESSION + if not _GLANCE_SESSION: + _GLANCE_SESSION = keystone.get_session('glance') + session = _GLANCE_SESSION + service_auth = keystone.get_auth('glance') + + try: + if service_auth and hasattr(service_auth, 'get_project_id'): + return service_auth.get_project_id(session) + elif hasattr(session, 'get_project_id') and session.auth: + return session.get_project_id() + except Exception as e: + LOG.debug("Error getting conductor project ID: %s", str(e)) + return None diff -Nru ironic-29.0.0/ironic/common/image_service.py ironic-29.0.5/ironic/common/image_service.py --- ironic-29.0.0/ironic/common/image_service.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/image_service.py 2026-04-30 14:30:48.000000000 +0000 @@ -28,6 +28,7 @@ from oslo_utils import uuidutils import requests +from ironic.common import checksum_utils from ironic.common import exception from ironic.common.glance_service.image_service import GlanceImageService from ironic.common.i18n import _ @@ -52,11 +53,16 @@ """ @abc.abstractmethod - def download(self, image_href, image_file): + def download(self, image_href, image_file, checksum=None, + checksum_algo=None): """Downloads image to specified location. :param image_href: Image reference. :param image_file: File object to write data to. + :param checksum: Expected checksum value for validation during + transfer. + :param checksum_algo: Algorithm for checksum (e.g., 'md5', + 'sha256'). :raises: exception.ImageRefValidationFailed. :raises: exception.ImageDownloadFailed. """ @@ -86,6 +92,14 @@ class HttpImageService(BaseImageService): """Provides retrieval of disk images using HTTP.""" + def __init__(self): + self._transfer_verified_checksum = None + + @property + def transfer_verified_checksum(self): + """The transferred artifact checksum.""" + return self._transfer_verified_checksum + @staticmethod def gen_auth_from_conf_user_pass(image_href): """This function is used to pass the credentials to the chosen @@ -173,7 +187,10 @@ response = requests.head(image_href, verify=verify, timeout=CONF.webserver_connection_timeout, auth=auth) - if response.status_code == http_client.MOVED_PERMANENTLY: + if (response.status_code == http_client.MOVED_PERMANENTLY + or response.status_code == http_client.FOUND + or response.status_code == http_client.TEMPORARY_REDIRECT + or response.status_code == http_client.PERMANENT_REDIRECT): # NOTE(TheJulia): In the event we receive a redirect, we need # to notify the caller. Before this we would just fail, # but a url which is missing a trailing slash results in a @@ -215,20 +232,27 @@ reason=str(e)) return response - def download(self, image_href, image_file): + def download(self, image_href, image_file, checksum=None, + checksum_algo=None): """Downloads image to specified location. :param image_href: Image reference. :param image_file: File object to write data to. + :param checksum: Expected checksum value for validation during + transfer. + :param checksum_algo: Algorithm for checksum (e.g., 'md5', + 'sha256'). :raises: exception.ImageRefValidationFailed if GET request returned response code not equal to 200. :raises: exception.ImageDownloadFailed if: * IOError happened during file write; * GET request failed. + :raises: exception.ImageChecksumError if checksum validation fails. """ + # Reset transfer checksum for new download + self._transfer_verified_checksum = None try: - verify = strutils.bool_from_string(CONF.webserver_verify_ca, strict=True) except ValueError: @@ -245,8 +269,45 @@ reason=_("Got HTTP code %s instead of 200 in response " "to GET request.") % response.status_code) - with response.raw as input_img: - shutil.copyfileobj(input_img, image_file, IMAGE_CHUNK_SIZE) + # If checksum validation is requested, use TransferHelper + # to calculate checksum during download + if checksum and checksum_algo: + try: + download_helper = checksum_utils.TransferHelper( + response, checksum_algo, checksum, + chunk_size=IMAGE_CHUNK_SIZE) + for chunk in download_helper: + image_file.write(chunk) + + # Verify checksum matches + if download_helper.checksum_matches: + # Store verified checksum in algo:value format + self._transfer_verified_checksum = ( + f"{checksum_algo}:{checksum}") + LOG.debug("Verified checksum during download of image " + "%(image)s: %(algo)s:%(checksum)s", + {'image': image_href, 'algo': checksum_algo, + 'checksum': checksum}) + else: + raise exception.ImageChecksumError() + except (exception.ImageChecksumAlgorithmFailure, ValueError): + # Fall back to download without checksumming + LOG.warning("Checksum algorithm %(algo)s not available, " + "downloading without incremental checksum for " + "image %(image)s", + {'algo': checksum_algo, 'image': image_href}) + # NOTE(JayF): Must use iter_content with explicit chunk + # size. See IMAGE_CHUNK_SIZE definition for details. + for chunk in response.iter_content( + chunk_size=IMAGE_CHUNK_SIZE): + image_file.write(chunk) + else: + # No checksum requested, just stream and write. + # NOTE(JayF): Must use iter_content with explicit chunk + # size. See IMAGE_CHUNK_SIZE definition for details. + for chunk in response.iter_content( + chunk_size=IMAGE_CHUNK_SIZE): + image_file.write(chunk) except (OSError, requests.ConnectionError, requests.RequestException, IOError) as e: @@ -372,7 +433,7 @@ if the URL is specific to to a specific manifest, or is otherwise generalized and needs to be identified. - :raises: OciImageNotSpecifc if the supplied image_href lacks a + :raises: OciImageNotSpecific if the supplied image_href lacks a required manifest digest value, or if the digest value is not understood. :raises: ImageRefValidationFailed if the supplied image_href @@ -442,11 +503,18 @@ return self.show(image_href) - def download(self, image_href, image_file): + def download(self, image_href, image_file, checksum=None, + checksum_algo=None): """Downloads image to specified location. :param image_href: Image reference. :param image_file: File object to write data to. + :param checksum: Expected checksum value for validation during + transfer. (Currently unused, OCI validates via + manifest digest.) + :param checksum_algo: Algorithm for checksum (e.g., 'md5', + 'sha256'). (Currently unused, OCI validates + via manifest digest.) :raises: exception.ImageRefValidationFailed. :raises: exception.ImageDownloadFailed. :raises: exception.OciImageNotSpecific. @@ -530,6 +598,73 @@ password = None self._client.authenticate(image_url, username, password) + def _filter_manifests(self, manifests, image_download_source=None, + cpu_arch=None): + # Determine our preferences for matching + if image_download_source == 'local': + # When the image is served from the conductor, it makes sense to + # download the smaller variant (i.e. qcow2) and convert locally. + # Raw images are used when compressed ones are unavailable. + disk_format_priority = {'qcow2': 1, + 'qemu': 2, + 'raw': 3, + 'applehv': 4} + else: + # When IPA downloads the image directly, it is preferred to let it + # download a raw image because it enables direct streaming to the + # target block device. Compressed images are also possible. + # Note: applehv appears to be a raw image. + disk_format_priority = {'qcow2': 3, + 'qemu': 4, + 'raw': 1, + 'applehv': 2} + + # First thing to do, filter by disk types + # and assign a selection priority... since Ironic can handle + # several different formats without issue. + new_manifests = [] + for manifest in manifests: + # First evaluate the architecture because ironic can operated in + # an architecture agnostic mode... and we *can* match on it, but + # it is one of the most constraining factors. + if cpu_arch: + # NOTE(TheJulia): amd64 is the noted standard format in the + # API for x86_64. One thing, at least observing quay.io hosted + # artifacts is that there is heavy use of x86_64 as instead + # of amd64 as expected by the specification. This same sort + # of pattern extends to arm64/aarch64. + if cpu_arch in ['x86_64', 'amd64']: + possible_cpu_arch = ['x86_64', 'amd64'] + elif cpu_arch in ['arm64', 'aarch64']: + possible_cpu_arch = ['aarch64', 'arm64'] + else: + possible_cpu_arch = [cpu_arch] + # Extract what the architecture is noted for the image, from + # the platform field. + architecture = manifest.get('platform', {}).get('architecture') + if architecture and architecture not in possible_cpu_arch: + # skip onward, we don't have a localized match + continue + + disktype = manifest.get('annotations', {}).get('disktype') + if disktype in disk_format_priority: + manifest['_priority'] = disk_format_priority[disktype] + elif not disktype: + manifest['_priority'] = 100 + else: + continue + + # Normalize and cache the disk type for further processing + if disktype == 'applehv': + disktype = 'raw' + elif disktype and disktype != 'raw': + disktype = 'qcow2' + + manifest['_disktype'] = disktype + new_manifests.append(manifest) + + return sorted(new_manifests, key=itemgetter('_priority')) + def identify_specific_image(self, image_href, image_download_source=None, cpu_arch=None): """Identify a specific OCI Registry Artifact. @@ -635,132 +770,87 @@ artifact_index = self._client.get_artifact_index(image_href) manifests = artifact_index.get('manifests', []) if len(manifests) < 1: - # This is likely not going to happen, but we have nothing - # to identify and deploy based upon, so nothing found - # for user consistency. - raise exception.ImageNotFound(image_id=image_href) + mediaType = artifact_index.get('mediaType') or 'unknown' + if mediaType == oci_registry.MEDIA_OCI_MANIFEST_V1: + LOG.debug('The artifact index for image %s is a single ' + 'manifest, using its layers') + manifests = [artifact_index] + else: + LOG.error('Cannot use image %s: the artifact index of type %s ' + 'does not contain a list of manifests: %s', + image_href, mediaType, artifact_index) + # This is likely not going to happen, but we have nothing + # to identify and deploy based upon, so nothing found + # for user consistency. + raise exception.InvalidImageRef(image_href=image_href) if image_download_source == 'swift': raise exception.InvalidParameterValue( err="An image_download_source of swift is incompatible with " "retrieval of artifacts from an OCI container registry.") - # Determine our preferences for matching - if image_download_source == 'local': - # These types are qcow2 images, we can download these and convert - # them, but it is okay for us to match a raw appearing image - # if we don't have a qcow available. - disk_format_priority = {'qcow2': 1, - 'qemu': 2, - 'raw': 3, - 'applehv': 4} - else: - # applehv appears to be a raw image, - # raw is the Ironic community preference. - disk_format_priority = {'qcow2': 3, - 'qemu': 4, - 'raw': 1, - 'applehv': 2} - - # First thing to do, filter by disk types - # and assign a selection priority... since Ironic can handle - # several different formats without issue. - new_manifests = [] - for manifest in manifests: - artifact_format = manifest.get('annotations', {}).get('disktype') - if artifact_format in disk_format_priority.keys(): - manifest['_priority'] = disk_format_priority[artifact_format] - else: - manifest['_priority'] = 100 - new_manifests.append(manifest) - - sorted_manifests = sorted(new_manifests, key=itemgetter('_priority')) + sorted_manifests = self._filter_manifests( + manifests, image_download_source, cpu_arch) + LOG.debug('Using manifests %s for image %s', + sorted_manifests, image_href) # Iterate through the entries of manifests and evaluate them # one by one to identify a likely item. for manifest in sorted_manifests: - # First evaluate the architecture because ironic can operated in - # an architecture agnostic mode... and we *can* match on it, but - # it is one of the most constraining factors. - if cpu_arch: - # NOTE(TheJulia): amd64 is the noted standard format in the - # API for x86_64. One thing, at least observing quay.io hosted - # artifacts is that there is heavy use of x86_64 as instead - # of amd64 as expected by the specification. This same sort - # of pattern extends to arm64/aarch64. - if cpu_arch in ['x86_64', 'amd64']: - possible_cpu_arch = ['x86_64', 'amd64'] - elif cpu_arch in ['arm64', 'aarch64']: - possible_cpu_arch = ['aarch64', 'arm64'] - else: - possible_cpu_arch = [cpu_arch] - # Extract what the architecture is noted for the image, from - # the platform field. - architecture = manifest.get('platform', {}).get('architecture') - if architecture and architecture not in possible_cpu_arch: - # skip onward, we don't have a localized match - continue - - # One thing podman is doing, and an ORAS client can set for - # upload, is annotations. This is ultimately the first point - # where we can identify likely artifacts. - # We also pre-sorted on disktype earlier, so in theory based upon - # preference, we should have the desired result as our first - # matching hint which meets the criteria. - disktype = manifest.get('annotations', {}).get('disktype') - if disktype: - if disktype in disk_format_priority.keys(): - identified_manifest_digest = manifest.get('digest') - blob_manifest = self._client.get_manifest( - image_href, identified_manifest_digest) - layers = blob_manifest.get('layers', []) - if len(layers) != 1: - # This is a *multilayer* artifact, meaning a container - # construction, not a blob artifact in the OCI - # container registry. Odds are we're at the end of - # the references for what the user has requested - # consideration of as well, so it is good to log here. - LOG.info('Skipping consideration of container ' - 'registry manifest %s as it has multiple' - 'layers.', - identified_manifest_digest) - continue - - # NOTE(TheJulia): The resulting layer contents, has a - # mandatory mediaType value, which may be something like - # application/zstd or application/octet-stream and the - # an optional org.opencontainers.image.title annotation - # which would contain the filename the file was stored - # with in alignment with OARS annotations. Furthermore, - # there is an optional artifactType value with OCI - # distribution spec 1.1 (mid-2024) which could have - # been stored when the artifact was uploaded, - # but is optional. In any event, this is only available - # on the manifest contents, not further up unless we have - # the newer referrers API available. As of late 2024, - # quay.io did not offer the referrers API. - chosen_layer = layers[0] - blob_digest = chosen_layer.get('digest') - - # Use the client helper to assemble a blob url, so we - # have consistency with what we expect and what we parse. - image_url = self._client.get_blob_url(image_href, - blob_digest) - image_size = chosen_layer.get('size') - chosen_original_filename = chosen_layer.get( - 'annotations', {}).get( - 'org.opencontainers.image.title') - manifest_digest = manifest.get('digest') - media_type = chosen_layer.get('mediaType') - is_raw_image = disktype in ['raw', 'applehv'] - break - else: - # The case of there being no disk type in the entry. - # The only option here is to query the manifest contents out - # and based decisions upon that. :\ - # We could look at the layers, count them, and maybe look at - # artifact types. + if not manifest['_disktype']: + # If we got here, it means that the disk type is not set, and + # we need to detect it down the road. + LOG.warning('Image %s does not have a suitable disk type set ' + 'on any of its manifests, will use the first ' + 'manifest without a disk type', image_href) + + identified_manifest_digest = manifest.get('digest') + layers = manifest.get('layers') + if not layers: + blob_manifest = self._client.get_manifest( + image_href, identified_manifest_digest) + layers = blob_manifest.get('layers', []) + if len(layers) != 1: + # This is a *multilayer* artifact, meaning a container + # construction, not a blob artifact in the OCI + # container registry. Odds are we're at the end of + # the references for what the user has requested + # consideration of as well, so it is good to log here. + LOG.info('Skipping consideration of container ' + 'registry manifest %s for image %s as it has ' + 'multiple layers', + identified_manifest_digest, image_href) continue + + # NOTE(TheJulia): The resulting layer contents, has a + # mandatory mediaType value, which may be something like + # application/zstd or application/octet-stream and the + # an optional org.opencontainers.image.title annotation + # which would contain the filename the file was stored + # with in alignment with OARS annotations. Furthermore, + # there is an optional artifactType value with OCI + # distribution spec 1.1 (mid-2024) which could have + # been stored when the artifact was uploaded, + # but is optional. In any event, this is only available + # on the manifest contents, not further up unless we have + # the newer referrers API available. As of late 2024, + # quay.io did not offer the referrers API. + chosen_layer = layers[0] + blob_digest = chosen_layer.get('digest') + + # Use the client helper to assemble a blob url, so we + # have consistency with what we expect and what we parse. + image_url = self._client.get_blob_url(image_href, blob_digest) + image_size = chosen_layer.get('size') + chosen_original_filename = chosen_layer.get( + 'annotations', {}).get( + 'org.opencontainers.image.title') + manifest_digest = (manifest.get('digest') + or manifest.get('dockerContentDigest')) + media_type = chosen_layer.get('mediaType') + disktype = manifest['_disktype'] + break + if image_url: # NOTE(TheJulia): Doing the final return dict generation as a # last step in order to leave the door open to handling other @@ -783,7 +873,10 @@ url = urlparse.urlparse(image_href) # Drop any trailing content indicating a tag image_path = url.path.split(':')[0] - manifest = f'{url.scheme}://{url.netloc}{image_path}@{manifest_digest}' # noqa + if manifest_digest: + manifest = f'{url.scheme}://{url.netloc}{image_path}@{manifest_digest}' # noqa + else: + manifest = None return { 'image_url': image_url, 'image_size': image_size, @@ -792,7 +885,7 @@ 'image_container_manifest_digest': manifest_digest, 'image_media_type': media_type, 'image_compression_type': compression_type, - 'image_disk_format': 'raw' if is_raw_image else 'qcow2', + 'image_disk_format': disktype, 'image_request_authorization_secret': cached_auth, 'oci_image_manifest_url': manifest, } @@ -813,21 +906,45 @@ :param image_href: Image reference. :raises: exception.ImageRefValidationFailed if source image file - doesn't exist. - :returns: Path to image file if it exists. + doesn't exist, is in a blocked path, or is not in an allowed path. + :returns: Path to image file if it exists and is allowed. """ image_path = urlparse.urlparse(image_href).path + + # Check if the path is in the blocklist + rpath = os.path.abspath(image_path) + + # Check if the path is in the allowlist + for allowed in CONF.conductor.file_url_allowed_paths: + if rpath == allowed or rpath.startswith(allowed + os.sep): + break + else: + raise exception.ImageRefValidationFailed( + image_href=image_href, + reason=_( + "Security: Path %s is not allowed for image source " + "file URLs" % image_path) + ) + + # Check if the file exists if not os.path.isfile(image_path): raise exception.ImageRefValidationFailed( image_href=image_href, reason=_("Specified image file not found.")) + return image_path - def download(self, image_href, image_file): + def download(self, image_href, image_file, checksum=None, + checksum_algo=None): """Downloads image to specified location. :param image_href: Image reference. :param image_file: File object to write data to. + :param checksum: Expected checksum value for validation during + transfer. (Currently unused, for API compatibility.) + :param checksum_algo: Algorithm for checksum (e.g., 'md5', + 'sha256'). (Currently unused, for API + compatibility.) :raises: exception.ImageRefValidationFailed if source image file doesn't exist. :raises: exception.ImageDownloadFailed if exceptions were raised while diff -Nru ironic-29.0.0/ironic/common/images.py ironic-29.0.5/ironic/common/images.py --- ironic-29.0.0/ironic/common/images.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/images.py 2026-04-30 14:30:48.000000000 +0000 @@ -365,7 +365,7 @@ def fetch_into(context, image_href, image_file, - image_auth_data=None): + image_auth_data=None, checksum=None, checksum_algo=None): """Fetches image file contents into a file. :param context: A context object. @@ -375,6 +375,8 @@ :param image_auth_data: Optional dictionary for credentials to be conveyed from the original task to the image download process, if required. + :param checksum: Expected checksum value for validation during transfer. + :param checksum_algo: Algorithm for checksum (e.g., 'md5', 'sha256'). :returns: If a value is returned, that value was validated as the checksum. Otherwise None indicating the process had been completed. """ @@ -399,9 +401,13 @@ if isinstance(image_file, str): with open(image_file, "wb") as image_file_obj: - image_service.download(image_href, image_file_obj) + image_service.download(image_href, image_file_obj, + checksum=checksum, + checksum_algo=checksum_algo) else: - image_service.download(image_href, image_file) + image_service.download(image_href, image_file, + checksum=checksum, + checksum_algo=checksum_algo) LOG.debug("Image %(image_href)s downloaded in %(time).2f seconds.", {'image_href': image_href, 'time': time.time() - start}) @@ -450,7 +456,9 @@ image_auth_data=None): with fileutils.remove_path_on_error(path): transfer_checksum = fetch_into(context, image_href, path, - image_auth_data) + image_auth_data, + checksum=checksum, + checksum_algo=checksum_algo) if (not transfer_checksum and not CONF.conductor.disable_file_checksum and checksum): diff -Nru ironic-29.0.0/ironic/common/inspection_rules/actions.py ironic-29.0.5/ironic/common/inspection_rules/actions.py --- ironic-29.0.0/ironic/common/inspection_rules/actions.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/inspection_rules/actions.py 2026-04-30 14:30:48.000000000 +0000 @@ -44,6 +44,10 @@ def get_action(op_name): """Get operator class by name.""" + if op_name not in ACTIONS: + raise exception.Invalid( + _("Unsupported action '%s'.") % op_name + ) class_name = ACTIONS[op_name] return globals()[class_name] @@ -68,20 +72,29 @@ """Run action on successful rule match.""" def execute_with_loop(self, task, action, inventory, plugin_data): - loop_items = action.get('loop', []) - results = [] + loop_items = None + if action.get('loop', []): + loop_items = action['loop'] if isinstance(loop_items, (list, dict)): + if isinstance(loop_items, dict): + loop_context = {'item': loop_items} + action_copy = action.copy() + self.execute_action(task, action_copy, inventory, plugin_data, + loop_context) + return + for item in loop_items: + loop_context = {'item': item} action_copy = action.copy() - action_copy['args'] = item - results.append(self.execute_action(task, action_copy, - inventory, plugin_data)) - return results + self.execute_action(task, action_copy, inventory, plugin_data, + loop_context) - def execute_action(self, task, action, inventory, plugin_data): + def execute_action(self, task, action, inventory, plugin_data, + loop_context=None): processed_args = self._process_args(task, action, inventory, - plugin_data) + plugin_data, loop_context) + return self(task, **processed_args) diff -Nru ironic-29.0.0/ironic/common/inspection_rules/base.py ironic-29.0.5/ironic/common/inspection_rules/base.py --- ironic-29.0.0/ironic/common/inspection_rules/base.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/inspection_rules/base.py 2026-04-30 14:30:48.000000000 +0000 @@ -30,7 +30,7 @@ REQUIRES_PLUGIN_DATA = False """Flag to indicate if this action needs plugin_data as an arg.""" - def _get_validation_signature(self): + def get_validation_signature(self): """Get the signature to validate against.""" signature = inspect.signature(self.__call__) @@ -79,7 +79,7 @@ :param op_args: Operator args as a dictionary :raises: InspectionRuleValidationFailure on validation failure """ - required_args, optional_args = self._get_validation_signature() + required_args, optional_args = self.get_validation_signature() normalized_args = self._normalize_list_args( required_args=required_args, optional_args=optional_args, op_args=op_args) @@ -105,30 +105,70 @@ raise exception.InspectionRuleValidationFailure( _("args must be either a list or dictionary")) - def interpolate_variables(value, node, inventory, plugin_data): + def interpolate_variables(value, node, inventory, plugin_data, + loop_context=None): + loop_context = loop_context or {} + format_context = { + 'node': node, + 'inventory': inventory, + 'plugin_data': plugin_data, + **loop_context + } + + def safe_format(val, context): + if isinstance(val, str): + try: + return val.format(**context) + except (AttributeError, KeyError, ValueError, IndexError, + TypeError) as e: + LOG.warning( + "Interpolation failed: %(value)s: %(error_class)s, " + "%(error)s", {'value': val, + 'error_class': e.__class__.__name__, + 'error': e}) + return val + return val + + if loop_context: + # Format possible dictionary loop item containing replacement + # fields. + # + # E.g: + # 'args': {'path': '{item[path]}', 'value': '{item[value]}'}, + # 'loop': [ + # { + # 'path': 'driver_info/ipmi_address', + # 'value': '{inventory[bmc_address]}' + # } + # ] + # or a dict loop (which seems to defeat the purpose of a loop + # field, but is supported all the same): + # 'args': {'path': '{item[path]}', 'value': '{item[value]}'}, + # 'loop': { + # 'path': 'driver_info/ipmi_address', + # 'value': '{inventory[bmc_address]}' + # } + value = safe_format(value, format_context) + if isinstance(value, str): - try: - return value.format(node=node, inventory=inventory, - plugin_data=plugin_data) - except (AttributeError, KeyError, ValueError, IndexError, - TypeError) as e: - LOG.warning( - "Interpolation failed: %(value)s: %(error_class)s, " - "%(error)s", {'value': value, - 'error_class': e.__class__.__name__, - 'error': e}) - return value + return safe_format(value, format_context) elif isinstance(value, dict): return { - Base.interpolate_variables(k, node, inventory, plugin_data): - Base.interpolate_variables(v, node, inventory, plugin_data) - for k, v in value.items()} + safe_format(k, format_context): Base.interpolate_variables( + v, node, inventory, plugin_data, loop_context) + for k, v in value.items() + } elif isinstance(value, list): - return [Base.interpolate_variables( - v, node, inventory, plugin_data) for v in value] + return [ + safe_format(v, format_context) if isinstance(v, str) + else Base.interpolate_variables( + v, node, inventory, plugin_data, loop_context) + for v in value + ] return value - def _process_args(self, task, operation, inventory, plugin_data): + def _process_args(self, task, operation, inventory, plugin_data, + loop_context=None): "Normalize and process args based on the operator." op = operation.get('op') @@ -136,7 +176,7 @@ raise exception.InspectionRuleExecutionFailure( _("Operation must contain 'op' key")) - required_args, optional_args = self._get_validation_signature() + required_args, optional_args = self.get_validation_signature() op, invtd = utils.parse_inverted_operator(op) dict_args = self._normalize_list_args( @@ -151,7 +191,8 @@ node = task.node formatted_args = getattr(self, 'FORMATTED_ARGS', []) return { - k: (Base.interpolate_variables(v, node, inventory, plugin_data) - if k in formatted_args else v) + k: (Base.interpolate_variables( + v, node, inventory, plugin_data, loop_context) + if (k in formatted_args or loop_context) else v) for k, v in dict_args.items() } diff -Nru ironic-29.0.0/ironic/common/inspection_rules/engine.py ironic-29.0.5/ironic/common/inspection_rules/engine.py --- ironic-29.0.0/ironic/common/inspection_rules/engine.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/inspection_rules/engine.py 2026-04-30 14:30:48.000000000 +0000 @@ -90,7 +90,7 @@ raise ValueError(msg) plugin = operators.get_operator(op) - if 'loop' in condition: + if condition.get('loop', []): result = plugin().check_with_loop(task, condition, inventory, plugin_data) else: @@ -123,7 +123,7 @@ raise ValueError(msg) plugin = actions.get_action(op) - if 'loop' in action: + if action.get('loop', []): plugin().execute_with_loop(task, action, inventory, plugin_data) else: diff -Nru ironic-29.0.0/ironic/common/inspection_rules/operators.py ironic-29.0.5/ironic/common/inspection_rules/operators.py --- ironic-29.0.0/ironic/common/inspection_rules/operators.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/inspection_rules/operators.py 2026-04-30 14:30:48.000000000 +0000 @@ -41,19 +41,14 @@ def get_operator(op_name): """Get operator class by name.""" + if op_name not in OPERATORS: + raise exception.Invalid( + _("Unsupported operator '%s'.") % op_name + ) class_name = OPERATORS[op_name] return globals()[class_name] -def coerce(value, expected): - if isinstance(expected, float): - return float(value) - elif isinstance(expected, int): - return int(value) - else: - return value - - class OperatorBase(base.Base, metaclass=abc.ABCMeta): """Abstract base class for rule condition plugins.""" @@ -62,22 +57,36 @@ """Checks if condition holds for a given field.""" def check_with_loop(self, task, condition, inventory, plugin_data): - loop_items = condition.get('loop', []) - multiple = condition.get('multiple', 'any') - results = [] + loop_items = None + if condition.get('loop', []): + loop_items = condition['loop'] + multiple = condition.get('multiple', 'any') + results = [] if isinstance(loop_items, (list, dict)): - for item in loop_items: + if isinstance(loop_items, dict): + loop_context = {'item': loop_items} condition_copy = condition.copy() - condition_copy['args'] = item result = self.check_condition(task, condition_copy, - inventory, plugin_data) + inventory, plugin_data, + loop_context) results.append(result) + if multiple in ('first', 'last'): + return result - if multiple == 'first' and result: - return True - elif multiple == 'last': - results = [result] + elif isinstance(loop_items, list): + for item in loop_items: + loop_context = {'item': item} + condition_copy = condition.copy() + result = self.check_condition(task, condition_copy, + inventory, plugin_data, + loop_context) + results.append(result) + + if multiple == 'first' and result: + return True + elif multiple == 'last': + results = [result] if multiple == 'any': return any(results) @@ -86,14 +95,15 @@ return results[0] if results else False return self.check_condition(task, condition, inventory, plugin_data) - def check_condition(self, task, condition, inventory, plugin_data): + def check_condition(self, task, condition, inventory, plugin_data, + loop_context=None): """Process condition arguments and apply the check logic. :param task: TaskManger instance :param condition: condition to check - :param args: parameters as a dictionary, changing it here will change - what will be stored in database - :param kwargs: used for extensibility without breaking existing plugins + :param inventory: Node inventory data with hardware information + :param plugin_data: Data from inspection plugins + :param loop_context: Current loop item when called from check_with_loop :raises InspectionRuleExecutionFailure: on unacceptable field value :returns: True if check succeeded, otherwise False """ @@ -101,7 +111,15 @@ condition['op']) processed_args = self._process_args(task, condition, inventory, - plugin_data) + plugin_data, loop_context) + + required_args, optional_args = self.get_validation_signature() + if loop_context and 'force_strings' in optional_args: + # When in a loop context, variable interpolation might convert + # numbers to strings. Setting force_strings ensures consistent + # comparison by converting all values to strings before + # comparison. + processed_args['force_strings'] = True result = self(task, **processed_args) return not result if is_inverted else result @@ -125,12 +143,24 @@ return True if force_strings: - values = [coerce(value, str) for value in values] + values = [str(value) for value in values] return all(self.op(values[i], values[i + 1]) for i in range(len(values) - 1)) +class LeOperator(SimpleOperator): + op = operator.le + + +class GeOperator(SimpleOperator): + op = operator.ge + + +class NeOperator(SimpleOperator): + op = operator.ne + + class EqOperator(SimpleOperator): op = operator.eq @@ -197,7 +227,7 @@ FORMATTED_ARGS = ['value'] def __call__(self, task, value): - return str(value) == 'None' + return value is None class OneOfOperator(OperatorBase): diff -Nru ironic-29.0.0/ironic/common/inspection_rules/validation.py ironic-29.0.5/ironic/common/inspection_rules/validation.py --- ironic-29.0.0/ironic/common/inspection_rules/validation.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/inspection_rules/validation.py 2026-04-30 14:30:48.000000000 +0000 @@ -128,14 +128,13 @@ :param rule: The inspection rule to validate. :raises: Invalid if the rule is invalid. """ - if not rule.get('conditions'): - rule['conditions'] = [] - - errors = [] try: jsonschema.validate(rule, SCHEMA) except jsonschema.ValidationError as e: - errors.append(_('Validation failed for inspection rule: %s') % e) + raise exception.Invalid( + _('Validation failed for inspection rule: %s') % e) + + errors = [] phase = rule.get('phase', InspectionPhase.MAIN.value) if phase not in (p.value for p in InspectionPhase): @@ -146,10 +145,10 @@ }) priority = rule.get('priority', 0) - if priority < 0 and not rule['built_in']: + if priority < 0 and not rule.get('built_in'): errors.append( _("Priority cannot be negative for user-defined rules.")) - if priority > 9999 and not rule['built_in']: + if priority > 9999 and not rule.get('built_in'): errors.append( _("Priority must be between 0 and 9999 for user-defined rules.")) diff -Nru ironic-29.0.0/ironic/common/molds.py ironic-29.0.5/ironic/common/molds.py --- ironic-29.0.0/ironic/common/molds.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/molds.py 2026-04-30 14:30:48.000000000 +0000 @@ -13,6 +13,7 @@ # under the License. import json +from urllib.parse import urlparse from oslo_config import cfg from oslo_log import log as logging @@ -22,6 +23,7 @@ from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import keystone from ironic.common import swift LOG = logging.getLogger(__name__) @@ -55,7 +57,7 @@ url, data=json.dumps(data, indent=2), headers=auth_header, timeout=CONF.webserver_connection_timeout) - auth_header = _get_auth_header(task) + auth_header = _get_auth_header(task, url) response = _request(url, data, auth_header) response.raise_for_status() @@ -86,7 +88,7 @@ return requests.get(url, headers=auth_header, timeout=CONF.webserver_connection_timeout) - auth_header = _get_auth_header(task) + auth_header = _get_auth_header(task, url) response = _request(url, auth_header) if response.status_code == requests.codes.ok: if not response.content: @@ -107,7 +109,7 @@ # NOTE(TheJulia): Deprecated after the 2024.1 PTG in favor of # a future step templating interface. -def _get_auth_header(task): +def _get_auth_header(task, url): """Based on setup of configuration mold storage gets authentication header :param task: A TaskManager instance. @@ -116,8 +118,21 @@ """ auth_header = None if CONF.molds.storage == 'swift': + swift_session = swift.get_swift_session() + # NOTE(TheJulia): First validate the URL. + if url: + endpoint = keystone.get_endpoint('swift', + session=swift_session) + if (not endpoint + or urlparse(endpoint).netloc != urlparse(url).netloc): + # Raise an InvalidParameterValue should the value not + # match swift configuration. + raise exception.InvalidParameterValue( + _('Supplied URL for a mold target does not match the ' + 'swift configuration.')) + # TODO(ajya) Need to update to use Swift client and context session - auth_token = swift.get_swift_session().get_token() + auth_token = swift_session.get_token() if auth_token: auth_header = {'X-Auth-Token': auth_token} else: diff -Nru ironic-29.0.0/ironic/common/oci_registry.py ironic-29.0.5/ironic/common/oci_registry.py --- ironic-29.0.0/ironic/common/oci_registry.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/oci_registry.py 2026-04-30 14:30:48.000000000 +0000 @@ -29,6 +29,7 @@ from ironic.common import checksum_utils from ironic.common import exception +from ironic.common.i18n import _ from ironic.conf import CONF LOG = logging.getLogger(__name__) @@ -471,6 +472,33 @@ response.encoding = encoding return response.text + @staticmethod + def _get_response_json(response, encoding='utf-8', force_encoding=False, + validate_digest=None): + """Return request response as JSON. + + We need to set the encoding for the response other wise it + will attempt to detect the encoding which is very time consuming. + See https://github.com/psf/requests/issues/4235 for additional + context. + + The docker-content-digest header is added as a dockerContentDigest + field to the response. + + :param: response: requests Respoinse object + :param: encoding: encoding to set if not currently set + :param: force_encoding: set response encoding always + :param: validate_digest: checksum to validate the text against + """ + text = OciClient._get_response_text(response, encoding, force_encoding) + if validate_digest: + checksum_utils.validate_text_checksum(text, validate_digest) + resource = json.loads(text) + contentDigest = response.headers.get('docker-content-digest') + if contentDigest: + resource['dockerContentDigest'] = contentDigest + return resource + @classmethod def _build_url(cls, url, path): """Build an HTTPS URL from the input urlparse data. @@ -507,6 +535,9 @@ timeout=CONF.webserver_connection_timeout ) except requests.exceptions.HTTPError as e: + LOG.error('Encountered error while attempting to download ' + 'manifest %s for image %s: %s', + manifest_url, image_url, e) if e.response.status_code == 401: # Authorization Required. raise exception.ImageServiceAuthenticationRequired( @@ -517,9 +548,8 @@ if e.response.status_code >= 500: raise exception.TemporaryFailure() raise - manifest_str = self._get_response_text(manifest_r) - checksum_utils.validate_text_checksum(manifest_str, digest) - return json.loads(manifest_str) + return self._get_response_json( + manifest_r, validate_digest=digest) def _get_artifact_index(self, image_url): LOG.debug('Attempting to get the artifact index for: %s', @@ -528,8 +558,9 @@ index_url = self._build_url( image_url, CALL_MANIFEST % parts ) - # Explicitly ask for the OCI artifact index - index_headers = {'Accept': ", ".join([MEDIA_OCI_INDEX_V1])} + # Explicitly ask for the OCI artifact index, fall back to manifest + index_headers = {'Accept': ", ".join([MEDIA_OCI_INDEX_V1, + MEDIA_OCI_MANIFEST_V1])} try: index_r = RegistrySessionHelper.get( @@ -539,6 +570,9 @@ timeout=CONF.webserver_connection_timeout ) except requests.exceptions.HTTPError as e: + LOG.error('Encountered error while attempting to download ' + 'artifact index %s for image %s: %s', + index_url, image_url, e) if e.response.status_code == 401: # Authorization Required. raise exception.ImageServiceAuthenticationRequired( @@ -549,10 +583,9 @@ if e.response.status_code >= 500: raise exception.TemporaryFailure() raise - index_str = self._get_response_text(index_r) # Return a dictionary to the caller so it can house the # filtering/sorting application logic. - return json.loads(index_str) + return self._get_response_json(index_r) def _resolve_tag(self, image_url): """Attempts to resolve tags from a container URL.""" @@ -573,6 +606,9 @@ headers=tag_headers, timeout=CONF.webserver_connection_timeout) except requests.exceptions.HTTPError as e: + LOG.error('Encountered error while attempting to download ' + 'the tag list %s for image %s: %s', + tags_url, image_url, e) if e.response.status_code == 401: # Authorization Required. raise exception.ImageServiceAuthenticationRequired( @@ -583,14 +619,24 @@ tags = tags_r.json()['tags'] while 'next' in tags_r.links: next_url = parse.urljoin(tags_url, tags_r.links['next']['url']) - tags_r = RegistrySessionHelper.get( - self.session, next_url, - headers=tag_headers, - timeout=CONF.webserver_connection_timeout) + try: + tags_r = RegistrySessionHelper.get( + self.session, next_url, + headers=tag_headers, + timeout=CONF.webserver_connection_timeout) + except requests.exceptions.HTTPError as e: + LOG.error('Encountered error while attempting to download ' + 'the next tag list %s for image %s: %s', + next_url, image_url, e) + raise tags.extend(tags_r.json()['tags']) + if not tags: + raise exception.InvalidImageRef( + _("Image %s does not have any tags") % image_url.geturl()) if tag not in tags: - raise exception.ImageNotFound( - image_id=image_url.geturl()) + raise exception.OciImageTagNotFound( + image_url=image_url.geturl(), + tags=', '.join(tags)) return parts def get_artifact_index(self, image): @@ -649,15 +695,16 @@ """ if not blob_digest and '@' in image: split_url = image.split('@') - image_url = parse.urlparse(split_url[0]) + image_url = split_url[0] blob_digest = split_url[1] elif blob_digest and '@' in image: split_url = image.split('@') - image_url = parse.urlparse(split_url[0]) + image_url = split_url[0] # The caller likely has a bug or bad pattern # which needs to be fixed else: - image_url = parse.urlparse(image) + image_url = image + image_url = self._image_to_url(image_url) # just in caes, split out the tag since it is not # used for a blob manifest lookup. image_path = image_url.path.split(':')[0] @@ -738,8 +785,8 @@ raise exception.ImageChecksumError() except requests.exceptions.HTTPError as e: - LOG.debug('Encountered error while attempting to download %s', - blob_url) + LOG.error('Encountered error while attempting to download %s: %s', + blob_url, e) # Stream changes the behavior, so odds of hitting # this area area a bit low unless an actual exception # is raised. @@ -755,6 +802,8 @@ except (OSError, requests.ConnectionError, requests.RequestException, IOError) as e: + LOG.error('Encountered error while attempting to download %s: %s', + blob_url, e) raise exception.ImageDownloadFailed(image_href=blob_url, reason=str(e)) diff -Nru ironic-29.0.0/ironic/common/pxe_utils.py ironic-29.0.5/ironic/common/pxe_utils.py --- ironic-29.0.0/ironic/common/pxe_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/pxe_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -16,6 +16,7 @@ import copy import os +import pathlib import shutil import tempfile from urllib import parse as urlparse @@ -1373,29 +1374,27 @@ if not loaders or not base_path: # Do nothing, return. return + dir_mode = CONF.pxe.dir_permission or 0o755 + + base_path = pathlib.Path(base_path) for dest, src in loaders.items(): - (head, _tail) = os.path.split(dest) - if head: - if head.startswith('/'): - # NOTE(TheJulia): The intent here is to error if the operator - # has put absolute paths in place, as we can and likely should - # copy to multiple folders based upon the protocol operation - # being used. Absolute paths are problematic there, where - # as a relative path is more a "well, that is silly, but okay - # misconfiguration. - msg = ('File paths configured for [pxe]loader_file_paths ' - 'must be relative paths. Entry: %s') % dest - raise exception.IncorrectConfiguration(msg) - else: - try: - ensure_tree(os.path.join(base_path, head)) - except OSError as e: - msg = ('Failed to create supplied directories in ' - 'asset copy paths. Error: %s') % e - raise exception.IncorrectConfiguration(msg) + dest = pathlib.Path(dest) + if dest.is_absolute(): + msg = ('File paths configured for [pxe]loader_file_paths ' + 'must be relative paths. Entry: %s') % dest + raise exception.IncorrectConfiguration(msg) + + full_dest = base_path.joinpath(dest) + try: + # ensure that we have somewhere to copy the files to + # notice the use of the parent path of the file + full_dest.parent.mkdir(mode=dir_mode, parents=True, exist_ok=True) + except OSError as e: + msg = (_('Failed to create bootloader destination path. Error: %s') + % e) + raise exception.IncorrectConfiguration(msg) - full_dest = os.path.join(base_path, dest) LOG.debug('Copying bootloader %(dest)s from %(src)s.', {'src': src, 'dest': full_dest}) try: diff -Nru ironic-29.0.0/ironic/common/states.py ironic-29.0.5/ironic/common/states.py --- ironic-29.0.0/ironic/common/states.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/common/states.py 2026-04-30 14:30:48.000000000 +0000 @@ -667,5 +667,8 @@ # A node in service fail can be rescued machine.add_transition(SERVICEFAIL, RESCUING, 'rescue') -# A no in service fail may be deleted. +# A node in service fail may be deleted. machine.add_transition(SERVICEFAIL, DELETING, 'delete') + +# A node in service wait may be deleted. +machine.add_transition(SERVICEWAIT, DELETING, 'delete') diff -Nru ironic-29.0.0/ironic/conductor/manager.py ironic-29.0.5/ironic/conductor/manager.py --- ironic-29.0.0/ironic/conductor/manager.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/conductor/manager.py 2026-04-30 14:30:48.000000000 +0000 @@ -4024,9 +4024,14 @@ utils.node_history_record(task.node, event=msg, event_type=states.MONITORING, error=True) - node.maintenance = True - node.maintenance_reason = msg - node.fault = faults.POWER_FAILURE + + # Only set maintenance and fault due to power sync failure if the node + # was not already placed into maintenance (e.g., by an administrator). + # This avoids auto-clearing admin-set maintenance during recovery. + if not node.maintenance: + node.maintenance = True + node.maintenance_reason = msg + node.fault = faults.POWER_FAILURE node.save() if old_power_state != actual_power_state: if node.instance_uuid: diff -Nru ironic-29.0.0/ironic/conf/conductor.py ironic-29.0.5/ironic/conf/conductor.py --- ironic-29.0.0/ironic/conf/conductor.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/conf/conductor.py 2026-04-30 14:30:48.000000000 +0000 @@ -21,6 +21,7 @@ from ironic.common import boot_modes from ironic.common.i18n import _ from ironic.common import lessee_sources +from ironic.conf import types as ir_types opts = [ @@ -531,6 +532,20 @@ 'wish to disable this functionality, otherwise it is ' 'automaticly 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'], + item_type=ir_types.ExplicitAbsolutePath(), + help=_( + 'List of paths that are allowed to be used as file:// ' + 'URLs. Files in /boot, /dev, /etc, /proc, /sys and other' + 'system paths are always disallowed for security reasons. ' + 'Any files in this path readable by ironic may be used as ' + 'an image source when deploying. Setting this value to ' + '"" (empty) disables file:// URL support. Paths listed ' + 'here are validated as absolute paths and will be rejected' + 'if they contain path traversal mechanisms, such as "..".' + )), ] diff -Nru ironic-29.0.0/ironic/conf/default.py ironic-29.0.5/ironic/conf/default.py --- ironic-29.0.0/ironic/conf/default.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/conf/default.py 2026-04-30 14:30:48.000000000 +0000 @@ -69,6 +69,12 @@ default='/etc/ironic/htpasswd', help=_('Path to Apache format user authentication file used ' 'when auth_strategy=http_basic')), + cfg.BoolOpt('allow_image_access_via_auth_token', + default=True, + deprecated_for_removal=True, + help=_('If True, Ironic allows access to Glance images if an ' + 'auth_token is present in the request context.') + ), cfg.BoolOpt( 'ignore_project_check_for_admin_tasks', default=True, diff -Nru ironic-29.0.0/ironic/conf/types.py ironic-29.0.5/ironic/conf/types.py --- ironic-29.0.0/ironic/conf/types.py 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/ironic/conf/types.py 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,55 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may not +# use this file except in compliance with the License. You may obtain a copy +# of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os + +from oslo_config import types + + +class ExplicitAbsolutePath(types.ConfigType): + """Explicit Absolute path type. + + Absolute path values do not get transformed and are returned as + strings. They are validated to ensure they are absolute paths and are equal + to os.path.abspath(value) -- protecting from path traversal issues. + + Python path libraries generally define "absolute path" as anything + starting with a /, so tools like path.PurePath(str).is_absolute() is not + useful, because it will happily return that /tmp/../etc/resolv.conf is + absolute. This type is to be used in cases where we require the path to + be explicitly absolute. + + :param type_name: Type name to be used in the sample config file. + """ + + def __init__(self, type_name='explicit absolute path'): + super().__init__(type_name=type_name) + + def __call__(self, value): + value = str(value) + + # NOTE(JayF): This removes trailing / if provided, since + # os.path.abspath will not return a trailing slash. + if len(value) > 1: + value = value.rstrip('/') + absvalue = os.path.abspath(value) + if value != absvalue: + raise ValueError('Value must be an absolute path ' + 'containing no path traversal mechanisms. Config' + f'item was: {value}, but resolved to {absvalue}') + + return value + + def __repr__(self): + return 'explicit absolute path' + + def _formatter(self, value): + return self.quote_trailing_and_leading_space(value) diff -Nru ironic-29.0.0/ironic/drivers/modules/agent_base.py ironic-29.0.5/ironic/drivers/modules/agent_base.py --- ironic-29.0.0/ironic/drivers/modules/agent_base.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/agent_base.py 2026-04-30 14:30:48.000000000 +0000 @@ -224,11 +224,16 @@ agent executed will be get_XXX_steps. For later reboots the list of commands will be empty. """ - return ( - not commands - or (len(commands) == 1 - and commands[0]['command_name'] == 'get_%s_steps' % step_type) - ) + if not commands: + # Empty list, most likely unit testing or immediately after a reboot. + return True + step_name = 'get_%s_steps' % step_type + # Make a list of resulting commands which do not match the expected + # get_XXX_steps command. + result = [x for x in commands if not x['command_name'] == step_name] + # If the length of the result is greater than 0, then this is not a freshly + # booted agent. + return not len(result) > 0 def _get_completed_command(task, commands, step_type): diff -Nru ironic-29.0.0/ironic/drivers/modules/console_utils.py ironic-29.0.5/ironic/drivers/modules/console_utils.py --- ironic-29.0.0/ironic/drivers/modules/console_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/console_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -23,6 +23,7 @@ import fcntl import ipaddress import os +import shlex import signal import socket import subprocess @@ -262,12 +263,15 @@ return self._finished -def start_shellinabox_console(node_uuid, port, console_cmd): +def start_shellinabox_console(node_uuid, port, console_cmd, + env_variables=None): """Open the serial console for a node. :param node_uuid: the uuid for the node. :param port: the terminal port for the node. :param console_cmd: the shell command that gets the console. + :param env_variables: optional dict of environment variables to pass to + the subprocess (e.g. IPMI_PASSWORD for ipmitool -E). :raises: ConsoleError if the directory for the PID file cannot be created or an old process cannot be stopped. :raises: ConsoleSubprocessFailed when invoking the subprocess failed. @@ -302,9 +306,10 @@ LOG.debug('Running subprocess: %s', ' '.join(args)) # use pipe here to catch the error in case shellinaboxd # failed to start. - obj = subprocess.Popen(args, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + popen_kwargs = {'stdout': subprocess.PIPE, 'stderr': subprocess.PIPE} + if env_variables is not None: + popen_kwargs['env'] = dict(os.environ, **env_variables) + obj = subprocess.Popen(args, **popen_kwargs) except (OSError, ValueError) as e: error = _("%(exec_error)s\n" "Command: %(command)s") % {'exec_error': str(e), @@ -383,13 +388,15 @@ 'port': port} -def start_socat_console(node_uuid, port, console_cmd): +def start_socat_console(node_uuid, port, console_cmd, env_variables=None): """Open the serial console for a node. :param node_uuid: the uuid of the node :param port: the terminal port for the node :param console_cmd: the shell command that will be executed by socat to establish console to the node + :param env_variables: optional dict of environment variables to pass to + the subprocess (e.g. IPMI_PASSWORD for ipmitool -E). :raises ConsoleError: if the directory for the PID file or the PID file cannot be created :raises ConsoleSubprocessFailed: when invoking the subprocess failed @@ -423,7 +430,8 @@ args.append(arg % {'host': console_host, 'port': port}) - args.append('EXEC:"%s",pty,stderr' % console_cmd) + quoted_cmd = shlex.quote(console_cmd) + args.append('EXEC:"%s",pty,stderr' % quoted_cmd) # run the command as a subprocess try: @@ -431,7 +439,10 @@ # Use pipe here to catch the error in case socat # fails to start. Note that socat uses stdout as transferring # data, so we only capture stderr for checking if it fails. - obj = subprocess.Popen(args, stderr=subprocess.PIPE) + popen_kwargs = {'stderr': subprocess.PIPE} + if env_variables is not None: + popen_kwargs['env'] = dict(os.environ, **env_variables) + obj = subprocess.Popen(args, **popen_kwargs) except (OSError, ValueError) as e: error = _("%(exec_error)s\n" "Command: %(command)s") % {'exec_error': str(e), diff -Nru ironic-29.0.0/ironic/drivers/modules/image_cache.py ironic-29.0.5/ironic/drivers/modules/image_cache.py --- ironic-29.0.0/ironic/drivers/modules/image_cache.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/image_cache.py 2026-04-30 14:30:48.000000000 +0000 @@ -19,6 +19,7 @@ """ import os +import shutil import tempfile import threading import time @@ -167,7 +168,17 @@ if cache_up_to_date: # NOTE(dtantsur): ensure we're not in the middle of clean up with lockutils.lock('master_image'): - os.link(master_path, dest_path) + try: + os.link(master_path, dest_path) + except OSError as exc: + LOG.debug( + "Could not hardlink image file %(image)s to " + "the cache location %(dest_path)s (will copy it " + "over): %(error)s", { + 'image': master_path, + 'dest_path': dest_path, + 'error': exc}) + shutil.copyfile(master_path, dest_path) LOG.debug("Master cache hit for image %(href)s", {'href': href}) return diff -Nru ironic-29.0.0/ironic/drivers/modules/inspect_utils.py ironic-29.0.5/ironic/drivers/modules/inspect_utils.py --- ironic-29.0.0/ironic/drivers/modules/inspect_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/inspect_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -14,6 +14,7 @@ # under the License. from http import client as http_client +import json import socket import typing import urllib @@ -189,10 +190,10 @@ swift_object_name = f'{_OBJECT_NAME_PREFIX}-{node_uuid}' container = CONF.inventory.swift_data_container swift_api.create_object_from_data(f'{swift_object_name}-inventory', - inventory_data, + json.dumps(inventory_data), container) swift_api.create_object_from_data(f'{swift_object_name}-plugin', - plugin_data, + json.dumps(plugin_data), container) return swift_object_name diff -Nru ironic-29.0.0/ironic/drivers/modules/inspector/agent.py ironic-29.0.5/ironic/drivers/modules/inspector/agent.py --- ironic-29.0.0/ironic/drivers/modules/inspector/agent.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/inspector/agent.py 2026-04-30 14:30:48.000000000 +0000 @@ -90,9 +90,16 @@ :param inventory: hardware inventory from the node. :param plugin_data: optional plugin-specific data. """ - # Run the inspection hooks - inspect_utils.run_inspection_hooks(task, inventory, plugin_data, - self.hooks, _store_logs) + + try: + # Run the inspection hooks + inspect_utils.run_inspection_hooks(task, inventory, plugin_data, + self.hooks, _store_logs) + except Exception as exc: + error = _("failed to run inspection hooks: %s") % exc + common.inspection_error_handler(task, error, raise_exc=True, + clean_up=True) + if CONF.agent.deploy_logs_collect == 'always': _store_logs(plugin_data, task.node) common.clean_up(task, finish=False, always_power_off=True) diff -Nru ironic-29.0.0/ironic/drivers/modules/inspector/hooks/local_link_connection.py ironic-29.0.5/ironic/drivers/modules/inspector/hooks/local_link_connection.py --- ironic-29.0.0/ironic/drivers/modules/inspector/hooks/local_link_connection.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/inspector/hooks/local_link_connection.py 2026-04-30 14:30:48.000000000 +0000 @@ -106,8 +106,10 @@ continue mac_address = iface['mac_address'] - port = ironic_port.Port.get_by_address(task.context, mac_address) - if not port: + try: + port = ironic_port.Port.get_by_address(task.context, + mac_address) + except exception.PortNotFound: LOG.debug('Skipping LLDP processing for interface %s of node ' '%s: matching port not found in Ironic.', mac_address, task.node.uuid) diff -Nru ironic-29.0.0/ironic/drivers/modules/inspector/hooks/validate_interfaces.py ironic-29.0.5/ironic/drivers/modules/inspector/hooks/validate_interfaces.py --- ironic-29.0.0/ironic/drivers/modules/inspector/hooks/validate_interfaces.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/inspector/hooks/validate_interfaces.py 2026-04-30 14:30:48.000000000 +0000 @@ -49,6 +49,11 @@ result = {} pxe_mac = get_pxe_mac(inventory) + if 'interfaces' not in inventory: + raise exception.InvalidNodeInventory( + node=node.uuid, + reason=_('no interfaces present in inventory')) + for iface in inventory['interfaces']: name = iface.get('name') mac = iface.get('mac_address') diff -Nru ironic-29.0.0/ironic/drivers/modules/ipmitool.py ironic-29.0.5/ironic/drivers/modules/ipmitool.py --- ironic-29.0.0/ironic/drivers/modules/ipmitool.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/ipmitool.py 2026-04-30 14:30:48.000000000 +0000 @@ -462,7 +462,7 @@ } -def _get_ipmitool_args(driver_info, pw_file=None): +def _get_ipmitool_args(driver_info, pw_file=None, use_pwd_env=False): ipmi_version = ('lanplus' if driver_info['protocol_version'] == '2.0' else 'lan') @@ -489,6 +489,8 @@ if pw_file: args.append('-f') args.append(pw_file) + elif use_pwd_env: + args.append('-E') if CONF.ipmi.debug: args.append('-v') @@ -1569,14 +1571,17 @@ "Check the 'ipmi_protocol_version' parameter in " "node's driver_info")) - def _get_ipmi_cmd(self, driver_info, pw_file): + def _get_ipmi_cmd(self, driver_info, pw_file=None, use_pwd_env=False): """Get ipmi command for ipmitool usage. :param driver_info: driver info with the ipmitool parameters :param pw_file: password file to be used in ipmitool command + :param use_pwd_env: when True, add -E so ipmitool reads password + from IPMI_PASSWORD environment variable :returns: returns a command string for ipmitool """ - return ' '.join(_get_ipmitool_args(driver_info, pw_file=pw_file)) + return ' '.join(_get_ipmitool_args(driver_info, pw_file=pw_file, + use_pwd_env=use_pwd_env)) def _start_console(self, driver_info, start_method): """Start a remote console for the node. @@ -1590,17 +1595,36 @@ created :raises: ConsoleSubprocessFailed when invoking the subprocess failed """ - try: - cmd = self._get_ipmi_cmd(driver_info, pw_file=None) + flag, path_or_env = _persist_ipmi_password(driver_info) + if flag == '-f': + pw_file = path_or_env + cmd = self._get_ipmi_cmd(driver_info, pw_file=pw_file) cmd += ' sol activate' - start_method(driver_info['uuid'], driver_info['port'], cmd) - except (exception.ConsoleError, - exception.ConsoleSubprocessFailed) as e: - LOG.exception('IPMI Error while attempting "%(cmd)s" ' - 'for node %(node)s. Error: %(error)s', - {'node': driver_info['uuid'], - 'cmd': cmd, 'error': e}) - raise + try: + start_method(driver_info['uuid'], driver_info['port'], cmd) + except (exception.ConsoleError, + exception.ConsoleSubprocessFailed) as e: + utils.unlink_without_raise(pw_file) + LOG.exception('IPMI Error while attempting "%(cmd)s" ' + 'for node %(node)s. Error: %(error)s', + {'node': driver_info['uuid'], + 'cmd': cmd, 'error': e}) + raise + else: + # flag == '-E', use password from environment + cmd = self._get_ipmi_cmd(driver_info, pw_file=None, + use_pwd_env=True) + cmd += ' sol activate' + try: + start_method(driver_info['uuid'], driver_info['port'], cmd, + env_variables=path_or_env) + except (exception.ConsoleError, + exception.ConsoleSubprocessFailed) as e: + LOG.exception('IPMI Error while attempting "%(cmd)s" ' + 'for node %(node)s. Error: %(error)s', + {'node': driver_info['uuid'], + 'cmd': cmd, 'error': e}) + raise class IPMIShellinaboxConsole(IPMIConsole): @@ -1614,15 +1638,17 @@ supported = False - def _get_ipmi_cmd(self, driver_info, pw_file): + def _get_ipmi_cmd(self, driver_info, pw_file=None, use_pwd_env=False): """Get ipmi command for ipmitool usage. :param driver_info: driver info with the ipmitool parameters :param pw_file: password file to be used in ipmitool command + :param use_pwd_env: when True, add -E so ipmitool reads password + from IPMI_PASSWORD environment variable :returns: returns a command string for ipmitool """ command = super(IPMIShellinaboxConsole, self)._get_ipmi_cmd( - driver_info, pw_file) + driver_info, pw_file, use_pwd_env=use_pwd_env) return ("/:%(uid)s:%(gid)s:HOME:%(basic_command)s" % {'uid': os.getuid(), 'gid': os.getgid(), diff -Nru ironic-29.0.0/ironic/drivers/modules/network/flat.py ironic-29.0.5/ironic/drivers/modules/network/flat.py --- ironic-29.0.0/ironic/drivers/modules/network/flat.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/network/flat.py 2026-04-30 14:30:48.000000000 +0000 @@ -75,6 +75,11 @@ '%(err)s') % {'port_id': vif_port_id, 'err': e}) LOG.exception(msg) raise exception.NetworkError(msg) + if not bound_ports: + msg = (_('Failed to bind any flat network ports for node %s') % + task.node.uuid) + LOG.error(msg) + raise exception.NetworkError(msg) LOG.debug("Finished binding flat network ports, attached: %s", ' '.join(bound_ports)) diff -Nru ironic-29.0.0/ironic/drivers/modules/redfish/inspect.py ironic-29.0.5/ironic/drivers/modules/redfish/inspect.py --- ironic-29.0.0/ironic/drivers/modules/redfish/inspect.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/redfish/inspect.py 2026-04-30 14:30:48.000000000 +0000 @@ -22,6 +22,7 @@ from ironic.common.i18n import _ from ironic.common import states from ironic.common import utils +from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import inspect_utils from ironic.drivers.modules.redfish import utils as redfish_utils @@ -183,7 +184,7 @@ if pxe_port_macs is None: LOG.warning("No PXE enabled NIC was found for node " "%(node_uuid)s.", {'node_uuid': task.node.uuid}) - else: + elif CONF.inspector.update_pxe_enabled: pxe_port_macs = [macs.lower() for macs in pxe_port_macs] ports = objects.Port.list_by_node_id(task.context, task.node.id) diff -Nru ironic-29.0.0/ironic/drivers/modules/redfish/management.py ironic-29.0.5/ironic/drivers/modules/redfish/management.py --- ironic-29.0.0/ironic/drivers/modules/redfish/management.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/redfish/management.py 2026-04-30 14:30:48.000000000 +0000 @@ -89,6 +89,13 @@ BOOT_DEVICE_PERSISTENT_MAP_REV = {v: k for k, v in BOOT_DEVICE_PERSISTENT_MAP.items()} +VENDORS_REQUIRING_FULL_BOOT_REQUEST = [ + "american megatrends international", + "ami", + "asrockrack" + "redfish_compatible" +] + INDICATOR_MAP = { sushy.INDICATOR_LED_LIT: indicator_states.ON, sushy.INDICATOR_LED_OFF: indicator_states.OFF, @@ -121,6 +128,10 @@ :param http_boot_url: A string value to be sent to the sushy library, which is sent to the BMC as the url to boot from. :raises: SushyError on an error from the Sushy library + Vendor-specific logic: + - Vendors listed in VENDORS_REQUIRING_FULL_BOOT_REQUEST: + Require setting full boot parameters + (mode, enabled, target) even if unchanged. """ # The BMC handling of the persistent setting is vendor specific. @@ -130,6 +141,13 @@ # persistent setting must be set when setting the boot device # (see https://storyboard.openstack.org/#!/story/2008547). vendor = task.node.properties.get('vendor', None) + LOG.debug("Vendor : %(vendor)s node %(uuid)s", + {'vendor': vendor, 'uuid': task.node.uuid}) + requires_full_boot_request = ( + vendor and any(vendor_id in vendor.lower() + for vendor_id in + VENDORS_REQUIRING_FULL_BOOT_REQUEST) + ) if vendor and vendor.lower() == 'supermicro': enabled = BOOT_DEVICE_PERSISTENT_MAP_REV[persistent] LOG.debug('Setting BootSourceOverrideEnable to %(enable)s ' @@ -145,8 +163,35 @@ try: # NOTE(TheJulia): In sushy, it is uri, due to the convention used # in the standard. URL is used internally in ironic. - system.set_system_boot_options(device, enabled=enabled, - http_boot_uri=http_boot_url) + if requires_full_boot_request: + # Some vendors require sending all boot parameters every time + desired_mode = system.boot.get('mode') \ + or sushy.BOOT_SOURCE_MODE_UEFI + desired_enabled = BOOT_DEVICE_PERSISTENT_MAP_REV[persistent] + current_enabled = system.boot.get('enabled') \ + or sushy.BOOT_SOURCE_ENABLED_ONCE + current_target = system.boot.get('target') \ + or sushy.BOOT_SOURCE_TARGET_NONE + + LOG.debug('Vendor "%(vendor)s" requires full boot settings. ' + 'Sending: mode=%(mode)s, enabled=%(enabled)s, ' + 'target=%(target)s for node %(node)s', + {'vendor': vendor, 'mode': desired_mode, + 'enabled': current_enabled, 'target': current_target, + 'node': task.node.uuid}) + + system.set_system_boot_options( + device, + mode=desired_mode, + enabled=desired_enabled, + http_boot_uri=http_boot_url + ) + else: + LOG.debug('Sending minimal Redfish boot device' + ' change for node %(node)s', + {'node': task.node.uuid}) + system.set_system_boot_options(device, enabled=enabled, + http_boot_uri=http_boot_url) except sushy.exceptions.SushyError as e: if enabled == sushy.BOOT_SOURCE_ENABLED_CONTINUOUS: # NOTE(dtantsur): continuous boot device settings have been @@ -334,13 +379,54 @@ """ system = redfish_utils.get_system(task.node) + vendor = task.node.properties.get('vendor', None) + requires_full_boot_request = ( + vendor and any(vendor_id in vendor.lower() + for vendor_id in + VENDORS_REQUIRING_FULL_BOOT_REQUEST) + ) + + LOG.debug('Requested %(vendor)s to set boot mode' + ' to "%(mode)s" for node %(node)s', + {'mode': mode, 'node': task.node.uuid, 'vendor': vendor}) + # NOTE(dtantsur): check the readability of the current mode before # modifying anything. I suspect it can become None transiently after # the update, while we need to know if it is supported *at all*. get_mode_unsupported = (system.boot.get('mode') is None) + desired_mode = BOOT_MODE_MAP_REV[mode] + LOG.debug('Current boot mode read from Redfish for ' + 'node %(node)s is: %(mode)s', + {'node': task.node.uuid, + 'mode': desired_mode}) + try: - system.set_system_boot_options(mode=BOOT_MODE_MAP_REV[mode]) + + if requires_full_boot_request: + current_enabled = system.boot.get('enable') \ + or sushy.BOOT_SOURCE_ENABLED_ONCE + current_target = system.boot.get('enable') \ + or sushy.BOOT_SOURCE_TARGET_PXE + + LOG.debug('Vendor "%(vendor)s" requires full boot settings. ' + 'Sending: mode=%(mode)s, enabled=%(enabled)s, ' + 'target=%(target)s for node %(node)s', + {'vendor': vendor, 'mode': desired_mode, + 'enabled': current_enabled, 'node': task.node.uuid, + 'target': current_target + }) + + system.set_system_boot_options( + mode=desired_mode, + enabled=current_enabled, + target=current_target + ) + else: + LOG.debug('Sending minimal Redfish boot mode ' + 'change for node %(node)s', + {'node': task.node.uuid}) + system.set_system_boot_options(mode=desired_mode) except sushy.exceptions.SushyError as e: error_msg = (_('Setting boot mode to %(mode)s ' 'failed for node %(node)s. ' @@ -938,8 +1024,12 @@ LOG.info('Firmware updates completed for node %(node)s', {'node': node.uuid}) - - manager_utils.notify_conductor_resume_clean(task) + if task.node.clean_step: + manager_utils.notify_conductor_resume_clean(task) + elif task.node.service_step: + manager_utils.notify_conductor_resume_service(task) + elif task.node.deploy_step: + manager_utils.notify_conductor_resume_deploy(task) else: firmware_updates.pop(0) self._apply_firmware_update(node, @@ -967,7 +1057,10 @@ @periodics.node_periodic( purpose='checking if async firmware update failed', spacing=CONF.redfish.firmware_update_fail_interval, - filters={'reserved': False, 'provision_state': states.CLEANFAIL, + filters={'reserved': False, + 'provision_state_in': {states.CLEANFAIL, + states.SERVICEFAIL, + states.DEPLOYFAIL}, 'maintenance': True}, predicate_extra_fields=['driver_internal_info'], predicate=lambda n: n.driver_internal_info.get('firmware_updates'), @@ -989,7 +1082,10 @@ @periodics.node_periodic( purpose='checking async firmware update tasks', spacing=CONF.redfish.firmware_update_status_interval, - filters={'reserved': False, 'provision_state': states.CLEANWAIT}, + filters={'reserved': False, + 'provision_state_in': {states.CLEANWAIT, + states.SERVICEWAIT, + states.DEPLOYWAIT}}, predicate_extra_fields=['driver_internal_info'], predicate=lambda n: n.driver_internal_info.get('firmware_updates'), ) diff -Nru ironic-29.0.0/ironic/drivers/modules/redfish/power.py ironic-29.0.5/ironic/drivers/modules/redfish/power.py --- ironic-29.0.0/ironic/drivers/modules/redfish/power.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/drivers/modules/redfish/power.py 2026-04-30 14:30:48.000000000 +0000 @@ -123,6 +123,47 @@ try: _set_power_state(task, system, power_state, timeout=timeout) + except sushy.exceptions.HTTPError as e: + # Handle HTTP 400 (BadRequest) and 409 (Conflict) errors where + # the BMC indicates the node is already in the desired state + if e.status_code in (400, 409): + target_state = TARGET_STATE_MAP.get(power_state, power_state) + + # Refresh system state to get current power state from BMC + # instead of using potentially stale cached data + try: + system.refresh() + except Exception as refresh_error: + LOG.warning('Failed to refresh system state for node ' + '%(node)s: %(error)s', + {'node': task.node.uuid, + 'error': refresh_error}) + + current_state = GET_POWER_STATE_MAP.get(system.power_state) + + # If current state is undetermined, check for hints in the + # error message. + if current_state is None: + error_msg = str(e).lower() + if (target_state == states.POWER_OFF + and ('host is powered off' in error_msg + or 'already powered off' in error_msg)): + current_state = states.POWER_OFF + elif (target_state == states.POWER_ON + and ('host is powered on' in error_msg + or 'already powered on' in error_msg)): + current_state = states.POWER_ON + + if current_state == target_state: + LOG.info('Node %(node)s power operation (%(requested)s) ' + 'failed with HTTP %(code)s, but node is already ' + 'in the expected final state (%(final)s). ' + 'Treating as successful. Error was: %(error)s', + {'node': task.node.uuid, 'requested': power_state, + 'code': e.status_code, 'final': target_state, + 'error': e}) + return # Success - already in desired final state + raise except sushy.exceptions.SushyError as e: error_msg = (_('Setting power state to %(state)s failed for node ' '%(node)s. Error: %(error)s') % diff -Nru ironic-29.0.0/ironic/tests/unit/api/controllers/v1/test_inspection_rule.py ironic-29.0.5/ironic/tests/unit/api/controllers/v1/test_inspection_rule.py --- ironic-29.0.0/ironic/tests/unit/api/controllers/v1/test_inspection_rule.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/api/controllers/v1/test_inspection_rule.py 2026-04-30 14:30:48.000000000 +0000 @@ -198,6 +198,14 @@ headers=self.headers) self.assertEqual(201, response.status_int) + def test_create_rule_with_invalid_priority_fails(self): + idict = test_api_utils.post_get_test_inspection_rule() + idict['priority'] = -1 + response = self.post_json('/inspection_rules', idict, + headers=self.headers, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + class TestPatch(BaseInspectionRulesAPITest): def test_patch_invalid_api_version(self): diff -Nru ironic-29.0.0/ironic/tests/unit/api/controllers/v1/test_node.py ironic-29.0.5/ironic/tests/unit/api/controllers/v1/test_node.py --- ironic-29.0.0/ironic/tests/unit/api/controllers/v1/test_node.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/api/controllers/v1/test_node.py 2026-04-30 14:30:48.000000000 +0000 @@ -6229,6 +6229,23 @@ self.assertEqual(urlparse.urlparse(ret.location).path, expected_location) + def test_provision_with_unprovision_in_service_wait(self): + node = self.node + node.provision_state = states.SERVICEWAIT + node.target_provision_state = states.ACTIVE + node.save() + ret = self.put_json('/nodes/%s/states/provision' % node.uuid, + {'target': states.DELETED}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dntd.assert_called_once_with( + mock.ANY, mock.ANY, node.uuid, 'test-topic') + # Check location header + self.assertIsNotNone(ret.location) + expected_location = '/v1/nodes/%s/states' % node.uuid + self.assertEqual(urlparse.urlparse(ret.location).path, + expected_location) + @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action', autospec=True) def test_provide_from_manage(self, mock_dpa): @@ -6553,6 +6570,36 @@ expect_errors=True) self.assertEqual(http_client.NOT_ACCEPTABLE, ret.status_code) + @mock.patch.object(api_utils, 'check_runbook_policy_and_retrieve', + autospec=True) + @mock.patch.object(rpcapi.ConductorAPI, 'do_node_clean', autospec=True) + @mock.patch.object(api_node, '_check_clean_steps', autospec=True) + def test_clean_with_runbook_disable_ramdisk(self, mock_check, + mock_rpcapi, mock_policy): + objects.TraitList.create(self.context, self.node.id, ['CUSTOM_1']) + self.node.refresh() + + self.node.provision_state = states.MANAGEABLE + self.node.save() + + runbook = mock.Mock() + runbook.name = 'CUSTOM_1' + runbook.steps = [{"step": "erase_devices", "interface": "deploy", + "args": {}}] + runbook.disable_ramdisk = True + mock_policy.return_value = runbook + + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['clean'], + 'runbook': runbook.name}, + headers={api_base.Version.string: "1.106"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + mock_check.assert_called_once_with(runbook.steps) + mock_rpcapi.assert_called_once_with(mock.ANY, mock.ANY, + self.node.uuid, runbook.steps, + True, topic='test-topic') + def test_adopt_raises_error_before_1_17(self): """Test that a lower API client cannot use the adopt verb""" ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, @@ -7232,12 +7279,12 @@ runbook.name = 'CUSTOM_1' runbook.steps = [{"step": "upgrade_firmware", "interface": "deploy", "args": {}}] + runbook.disable_ramdisk = None mock_policy.return_value = runbook ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, {'target': states.VERBS['service'], 'runbook': runbook.name}, - headers={api_base.Version.string: - str(api_v1.max_version())}) + headers={api_base.Version.string: '1.106'}) self.assertEqual(http_client.ACCEPTED, ret.status_code) self.assertEqual(b'', ret.body) mock_policy.assert_has_calls([mock.call('baremetal:runbook:use', diff -Nru ironic-29.0.0/ironic/tests/unit/api/test_middleware.py ironic-29.0.5/ironic/tests/unit/api/test_middleware.py --- ironic-29.0.0/ironic/tests/unit/api/test_middleware.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/api/test_middleware.py 2026-04-30 14:30:48.000000000 +0000 @@ -15,15 +15,16 @@ Tests to assert that various incorporated middleware works as expected. """ +import bcrypt from http import client as http_client import os import tempfile +from unittest import mock from oslo_config import cfg import oslo_middleware.cors as cors_middleware from ironic.tests.unit.api import base -from ironic.tests.unit.api import utils from ironic.tests.unit.db import utils as db_utils @@ -132,7 +133,7 @@ def setUp(self): super(TestBasicAuthMiddleware, self).setUp() - self.environ = {'fake.cache': utils.FakeMemcache()} + self.environ = {} self.fake_db_node = db_utils.get_test_node(chassis_id=None) def test_not_authenticated(self): @@ -148,6 +149,20 @@ response = self.get_json('/chassis', headers=auth_header) self.assertEqual({'chassis': []}, response) + @mock.patch.object(bcrypt, 'checkpw', autospec=True) + def test_authenticated_cached(self, mock_checkpw): + def checkpw(password, hashed): + return password == b'myPassword' + mock_checkpw.side_effect = checkpw + + auth_header = {'Authorization': 'Basic bXlOYW1lOm15UGFzc3dvcmQ='} + for i in range(10): + response = self.get_json('/chassis', headers=auth_header) + self.assertEqual({'chassis': []}, response) + # call count will be zero or one, depending on already cached + # results from other tests + self.assertLessEqual(mock_checkpw.call_count, 1) + def test_public_unauthenticated(self): response = self.get_json('/') self.assertEqual('v1', response['id']) diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_glance_service.py ironic-29.0.5/ironic/tests/unit/common/test_glance_service.py --- ironic-29.0.0/ironic/tests/unit/common/test_glance_service.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_glance_service.py 2026-04-30 14:30:48.000000000 +0000 @@ -102,7 +102,8 @@ fixture = {'name': None, 'owner': None, 'properties': {}, - 'status': "active"} + 'status': "active", + 'visibility': "public"} fixture.update(kwargs) return openstack.image.v2.image.Image.new(**fixture) @@ -146,7 +147,7 @@ 'status': "active", 'tags': [], 'updated_at': None, - 'visibility': None, + 'visibility': "public", 'os_hash_algo': None, 'os_hash_value': None, } @@ -188,6 +189,7 @@ class MyGlanceStubClient(stubs.StubGlanceClient): """A client that fails the first time, then succeeds.""" + def get_image(self, image_id): if tries[0] == 0: tries[0] = 1 @@ -229,10 +231,12 @@ 'image contains no data', self.service.download, image_id) + @mock.patch.object(service_utils, '_GLANCE_SESSION', autospec=True) @mock.patch('os.sendfile', autospec=True) @mock.patch('os.path.getsize', autospec=True) @mock.patch('%s.open' % __name__, new=mock.mock_open(), create=True) - def test_download_file_url(self, mock_getsize, mock_sendfile): + def test_download_file_url(self, mock_getsize, mock_sendfile, + mock_serviceutils_glance): # NOTE: only in v2 API class MyGlanceStubClient(stubs.StubGlanceClient): @@ -241,8 +245,9 @@ s_tmpfname = '/whatever/source' def get_image(self, image_id): + direct_url = "file://%s" + self.s_tmpfname return type('GlanceTestDirectUrlMeta', (object,), - {'direct_url': 'file://%s' + self.s_tmpfname}) + dict(visibility='public', direct_url=direct_url)) stub_context = context.RequestContext(auth_token=True) stub_context.user_id = 'fake' @@ -251,6 +256,7 @@ stub_service = image_service.GlanceImageService(stub_client, context=stub_context) + mock_serviceutils_glance.return_value = stub_service image_id = uuidutils.generate_uuid() self.config(allowed_direct_url_schemes=['file'], group='glance') @@ -278,6 +284,7 @@ def test_client_forbidden_converts_to_imagenotauthed(self): class MyGlanceStubClient(stubs.StubGlanceClient): """A client that raises a Forbidden exception.""" + def get_image(self, image_id): raise openstack_exc.ForbiddenException() @@ -295,6 +302,7 @@ def test_client_notfound_converts_to_imagenotfound(self): class MyGlanceStubClient(stubs.StubGlanceClient): """A client that raises a NotFound exception.""" + def get_image(self, image_id): raise openstack_exc.NotFoundException() @@ -995,3 +1003,55 @@ self.assertFalse(service_utils.is_glance_image(image_href)) image_href = None self.assertFalse(service_utils.is_glance_image(image_href)) + + +class TestIsImageAvailable(base.TestCase): + + def setUp(self): + super(TestIsImageAvailable, self).setUp() + self.image = mock.Mock() + self.context = context.RequestContext() + self.context.roles = [] + + def test_allow_access_via_auth_token_enabled(self): + self.context.auth_token = 'fake-token' + self.config(allow_image_access_via_auth_token=True) + self.assertTrue(service_utils.is_image_available( + self.context, self.image)) + + def test_allow_public_image(self): + self.image.visibility = 'public' + self.assertTrue(service_utils.is_image_available( + self.context, self.image)) + + def test_allow_community_image(self): + self.image.visibility = 'community' + self.assertTrue(service_utils.is_image_available( + self.context, self.image)) + + def test_allow_admin_if_config_enabled(self): + self.context.roles = ['admin'] + self.config(ignore_project_check_for_admin_tasks=True) + self.assertTrue(service_utils.is_image_available( + self.context, self.image)) + + def test_allow_private_image_owned_by_conductor(self): + self.image.visibility = 'private' + self.image.owner = service_utils.get_conductor_project_id() + self.assertTrue(service_utils.is_image_available( + self.context, self.image)) + + def test_deny_private_image_different_owner(self): + self.config(allow_image_access_via_auth_token=False) + self.config(ignore_project_check_for_admin_tasks=False) + + self.image.visibility = 'private' + self.image.owner = 'other-owner' + self.image.id = 'fake-id' + + self.context.project = 'test-project' + self.context.roles = [] + self.context.auth_token = None + + result = service_utils.is_image_available(self.context, self.image) + self.assertFalse(result) diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_image_service.py ironic-29.0.5/ironic/tests/unit/common/test_image_service.py --- ironic-29.0.0/ironic/tests/unit/common/test_image_service.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_image_service.py 2026-04-30 14:30:48.000000000 +0000 @@ -245,7 +245,7 @@ self.assertEqual(http_client.FORBIDDEN, resp.status_code) @mock.patch.object(requests, 'head', autospec=True) - def test_validate_href_path_redirected(self, head_mock): + def test_validate_href_path_moved_permanently(self, head_mock): cfg.CONF.set_override('webserver_verify_ca', 'True') response = head_mock.return_value @@ -260,6 +260,54 @@ head_mock.assert_called_once_with(url, verify=True, timeout=60, auth=None) + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_path_found(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.FOUND + url = self.href + '/' + new_url = 'http://new-url' + response.headers = {'location': new_url} + exc = self.assertRaises(exception.ImageRefIsARedirect, + self.service.validate_href, + url) + self.assertEqual(new_url, exc.redirect_url) + head_mock.assert_called_once_with(url, verify=True, + timeout=60, auth=None) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_path_temporary_redirect(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.TEMPORARY_REDIRECT + url = self.href + '/' + new_url = 'http://new-url' + response.headers = {'location': new_url} + exc = self.assertRaises(exception.ImageRefIsARedirect, + self.service.validate_href, + url) + self.assertEqual(new_url, exc.redirect_url) + head_mock.assert_called_once_with(url, verify=True, + timeout=60, auth=None) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_path_permanent_redirect(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.PERMANENT_REDIRECT + url = self.href + '/' + new_url = 'http://new-url' + response.headers = {'location': new_url} + exc = self.assertRaises(exception.ImageRefIsARedirect, + self.service.validate_href, + url) + self.assertEqual(new_url, exc.redirect_url) + head_mock.assert_called_once_with(url, verify=True, + timeout=60, auth=None) + def test_verify_basic_auth_cred_format(self): self.assertIsNone(self .service @@ -377,45 +425,39 @@ head_mock.assert_called_with(self.href, verify=True, timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success_http_scheme(self, req_get_mock, shutil_mock): + def test_download_success_http_scheme(self, req_get_mock): self.href = 'http://127.0.0.1:12345/fedora.qcow2' response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) self.service.download(self.href, file_mock) - shutil_mock.assert_called_once_with( - response_mock.raw.__enter__(), file_mock, - image_service.IMAGE_CHUNK_SIZE - ) + response_mock.iter_content.assert_called_once_with( + chunk_size=image_service.IMAGE_CHUNK_SIZE) + self.assertEqual(2, file_mock.write.call_count) req_get_mock.assert_called_once_with(self.href, stream=True, verify=True, timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success_verify_false( - self, req_get_mock, shutil_mock): + def test_download_success_verify_false(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', 'False') response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) self.service.download(self.href, file_mock) - shutil_mock.assert_called_once_with( - response_mock.raw.__enter__(), file_mock, - image_service.IMAGE_CHUNK_SIZE - ) + response_mock.iter_content.assert_called_once_with( + chunk_size=image_service.IMAGE_CHUNK_SIZE) + self.assertEqual(2, file_mock.write.call_count) req_get_mock.assert_called_once_with(self.href, stream=True, verify=False, timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) def test_download_success_verify_false_basic_auth_sucess( - self, req_get_mock, shutil_mock): + self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', 'False') cfg.CONF.set_override('image_server_auth_strategy', 'http_basic', @@ -427,13 +469,12 @@ auth_creds = requests.auth.HTTPBasicAuth(user, password) response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) self.service.download(self.href, file_mock) - shutil_mock.assert_called_once_with( - response_mock.raw.__enter__(), file_mock, - image_service.IMAGE_CHUNK_SIZE - ) + response_mock.iter_content.assert_called_once_with( + chunk_size=image_service.IMAGE_CHUNK_SIZE) + self.assertEqual(2, file_mock.write.call_count) req_get_mock.assert_called_once_with(self.href, stream=True, verify=False, timeout=60, auth=auth_creds) @@ -447,76 +488,61 @@ self.assertRaises(exception.ImageRefValidationFailed, self.service.download, self.href, file_mock) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success_verify_true( - self, req_get_mock, shutil_mock): + def test_download_success_verify_true(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', 'True') response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) self.service.download(self.href, file_mock) - shutil_mock.assert_called_once_with( - response_mock.raw.__enter__(), file_mock, - image_service.IMAGE_CHUNK_SIZE - ) + response_mock.iter_content.assert_called_once_with( + chunk_size=image_service.IMAGE_CHUNK_SIZE) + self.assertEqual(2, file_mock.write.call_count) req_get_mock.assert_called_once_with(self.href, stream=True, verify=True, timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success_verify_path( - self, req_get_mock, shutil_mock): + def test_download_success_verify_path(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', '/some/path') response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) self.service.download(self.href, file_mock) - shutil_mock.assert_called_once_with( - response_mock.raw.__enter__(), file_mock, - image_service.IMAGE_CHUNK_SIZE - ) + response_mock.iter_content.assert_called_once_with( + chunk_size=image_service.IMAGE_CHUNK_SIZE) + self.assertEqual(2, file_mock.write.call_count) req_get_mock.assert_called_once_with(self.href, stream=True, verify='/some/path', timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_fail_verify_false_connerror( - self, req_get_mock, shutil_mock): + def test_download_fail_verify_false_connerror(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', False) req_get_mock.side_effect = requests.ConnectionError() file_mock = mock.Mock(spec=io.BytesIO) self.assertRaises(exception.ImageDownloadFailed, self.service.download, self.href, file_mock) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_fail_verify_false_ioerror( - self, req_get_mock, shutil_mock): + def test_download_fail_verify_false_ioerror(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', False) response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) - shutil_mock.side_effect = IOError + file_mock.write.side_effect = IOError self.assertRaises(exception.ImageDownloadFailed, self.service.download, self.href, file_mock) req_get_mock.assert_called_once_with(self.href, stream=True, verify=False, timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success_verify_true_connerror( - self, req_get_mock, shutil_mock): + def test_download_success_verify_true_connerror(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', '/some/path') - response_mock = mock.Mock() - response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) req_get_mock.side_effect = requests.ConnectionError file_mock = mock.Mock(spec=io.BytesIO) @@ -526,52 +552,45 @@ verify='/some/path', timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_fail_verify_true_ioerror( - self, req_get_mock, shutil_mock): + def test_download_fail_verify_true_ioerror(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', '/some/path') response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) - shutil_mock.side_effect = IOError + file_mock.write.side_effect = IOError self.assertRaises(exception.ImageDownloadFailed, self.service.download, self.href, file_mock) req_get_mock.assert_called_once_with(self.href, stream=True, verify='/some/path', timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_fail_verify_true_oserror( - self, req_get_mock, shutil_mock): + def test_download_fail_verify_true_oserror(self, req_get_mock): cfg.CONF.set_override('webserver_verify_ca', '/some/path') response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) - shutil_mock.side_effect = OSError() + file_mock.write.side_effect = OSError() self.assertRaises(exception.ImageDownloadFailed, self.service.download, self.href, file_mock) req_get_mock.assert_called_once_with(self.href, stream=True, verify='/some/path', timeout=60, auth=None) - @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success_custom_timeout( - self, req_get_mock, shutil_mock): + def test_download_success_custom_timeout(self, req_get_mock): cfg.CONF.set_override('webserver_connection_timeout', 15) response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK - response_mock.raw = mock.MagicMock(spec=io.BytesIO) + response_mock.iter_content.return_value = [b'chunk1', b'chunk2'] file_mock = mock.Mock(spec=io.BytesIO) self.service.download(self.href, file_mock) - shutil_mock.assert_called_once_with( - response_mock.raw.__enter__(), file_mock, - image_service.IMAGE_CHUNK_SIZE - ) + response_mock.iter_content.assert_called_once_with( + chunk_size=image_service.IMAGE_CHUNK_SIZE) + self.assertEqual(2, file_mock.write.call_count) req_get_mock.assert_called_once_with(self.href, stream=True, verify=True, timeout=15, auth=None) @@ -615,20 +634,58 @@ def setUp(self): super(FileImageServiceTestCase, self).setUp() self.service = image_service.FileImageService() - self.href = 'file:///home/user/image.qcow2' - self.href_path = '/home/user/image.qcow2' + self.href = 'file:///var/lib/ironic/images/image.qcow2' + self.href_path = '/var/lib/ironic/images/image.qcow2' @mock.patch.object(os.path, 'isfile', return_value=True, autospec=True) def test_validate_href(self, path_exists_mock): self.service.validate_href(self.href) path_exists_mock.assert_called_once_with(self.href_path) - @mock.patch.object(os.path, 'isfile', return_value=False, autospec=True) + @mock.patch.object(os.path, 'isfile', return_value=False, + autospec=True) def test_validate_href_path_not_found_or_not_file(self, path_exists_mock): self.assertRaises(exception.ImageRefValidationFailed, self.service.validate_href, self.href) path_exists_mock.assert_called_once_with(self.href_path) + @mock.patch.object(os.path, 'abspath', autospec=True) + def test_validate_href_empty_allowlist(self, abspath_mock): + abspath_mock.return_value = self.href_path + cfg.CONF.set_override('file_url_allowed_paths', [], 'conductor') + self.assertRaisesRegex(exception.ImageRefValidationFailed, + "is not allowed for image source file URLs", + self.service.validate_href, self.href) + + @mock.patch.object(os.path, 'abspath', autospec=True) + def test_validate_href_not_in_allowlist(self, abspath_mock): + href = "file:///var/is/allowed/not/this/path/image.qcow2" + href_path = "/var/is/allowed/not/this/path/image.qcow2" + abspath_mock.side_effect = ['/var/lib/ironic', href_path] + cfg.CONF.set_override('file_url_allowed_paths', ['/var/lib/ironic'], + 'conductor') + self.assertRaisesRegex(exception.ImageRefValidationFailed, + "is not allowed for image source file URLs", + self.service.validate_href, href) + + @mock.patch.object(os.path, 'abspath', autospec=True) + @mock.patch.object(os.path, 'isfile', + return_value=True, autospec=True) + def test_validate_href_in_allowlist(self, + path_exists_mock, + abspath_mock): + href_dir = '/var/lib' # self.href_path is in /var/lib/ironic/images/ + # First call is ironic.conf.types.ExplicitAbsolutePath + # Second call is in validate_href() + abspath_mock.side_effect = [href_dir, self.href_path] + cfg.CONF.set_override('file_url_allowed_paths', [href_dir], + 'conductor') + result = self.service.validate_href(self.href) + self.assertEqual(self.href_path, result) + path_exists_mock.assert_called_once_with(self.href_path) + abspath_mock.assert_has_calls( + [mock.call(href_dir), mock.call(self.href_path)]) + @mock.patch.object(os.path, 'getmtime', return_value=1431087909.1641912, autospec=True) @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True) @@ -790,6 +847,20 @@ 'mediaType': 'application/vnd.oci.image.index.v1+json', 'manifests': [] } + self.single_manifest = { + 'mediaType': 'application/vnd.oci.image.manifest.v1+json', + 'artifactType': 'application/vnd.unknown.artifact.v1', + 'layers': [ + {'mediaType': 'application/vnd.oci.image.layer.v1.tar', + 'digest': 'sha256:7d6355852aeb6dbcd191bcda7cd74f1536cfe5cbf' + '8a10495a7283a8396e4b75b', + 'size': 21692416, + 'annotations': { + 'org.opencontainers.image.title': + 'cirros-0.6.3-x86_64-disk.img' + }}, + ] + } @mock.patch.object(ociclient, 'get_manifest', autospec=True) @mock.patch.object(ociclient, 'get_artifact_index', @@ -1007,7 +1078,7 @@ mock_get_artifact_index, mock_get_manifest): mock_get_artifact_index.return_value = self.empty_artifact_index - self.assertRaises(exception.ImageNotFound, + self.assertRaises(exception.InvalidImageRef, self.service.identify_specific_image, self.href) mock_get_artifact_index.assert_called_once_with(mock.ANY, self.href) @@ -1109,6 +1180,32 @@ self.assertEqual('sha256:' + csum, self.service.transfer_verified_checksum) + @mock.patch.object(ociclient, 'get_artifact_index', autospec=True) + def test_identify_specific_image_single_manifest( + self, mock_get_artifact_index): + + self.single_manifest['dockerContentDigest'] = \ + 'sha256:9d046091b3dbeda26e1f4364a116ca8d94284000f103da7310e3a4703df1d3e4' # noqa + + mock_get_artifact_index.return_value = self.single_manifest + + expected_data = { + 'image_checksum': '7d6355852aeb6dbcd191bcda7cd74f1536cfe5cbf8a10495a7283a8396e4b75b', # noqa + 'image_compression_type': None, + 'image_container_manifest_digest': 'sha256:9d046091b3dbeda26e1f4364a116ca8d94284000f103da7310e3a4703df1d3e4', # noqa + 'image_filename': 'cirros-0.6.3-x86_64-disk.img', + 'image_disk_format': None, + 'image_media_type': 'application/vnd.oci.image.layer.v1.tar', + 'image_request_authorization_secret': None, + 'image_size': 21692416, + 'image_url': 'https://localhost/v2/podman/machine-os/blobs/sha256:7d6355852aeb6dbcd191bcda7cd74f1536cfe5cbf8a10495a7283a8396e4b75b', # noqa + 'oci_image_manifest_url': 'oci://localhost/podman/machine-os@sha256:9d046091b3dbeda26e1f4364a116ca8d94284000f103da7310e3a4703df1d3e4' # noqa + } + img_data = self.service.identify_specific_image( + self.href, cpu_arch='amd64') + self.assertEqual(expected_data, img_data) + mock_get_artifact_index.assert_called_once_with(mock.ANY, self.href) + @mock.patch.object(ociclient, '_get_manifest', autospec=True) def test_show(self, mock_get_manifest): layer_csum = ('96f33f01d5347424f947e43ff05634915f422debc' @@ -1179,14 +1276,14 @@ def test_get_http_image_service(self, http_service_mock): image_href = 'http://127.0.0.1/image.qcow2' image_service.get_image_service(image_href) - http_service_mock.assert_called_once_with() + http_service_mock.assert_called_once() @mock.patch.object(image_service.HttpImageService, '__init__', return_value=None, autospec=True) def test_get_https_image_service(self, http_service_mock): image_href = 'https://127.0.0.1/image.qcow2' image_service.get_image_service(image_href) - http_service_mock.assert_called_once_with() + http_service_mock.assert_called_once() @mock.patch.object(image_service.FileImageService, '__init__', return_value=None, autospec=True) diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_images.py ironic-29.0.5/ironic/tests/unit/common/test_images.py --- ironic-29.0.0/ironic/tests/unit/common/test_images.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_images.py 2026-04-30 14:30:48.000000000 +0000 @@ -57,7 +57,7 @@ image_service_mock.assert_called_once_with('image_href', context='context') image_service_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum=None, checksum_algo=None) mock_zstd.assert_called_once_with('path') @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @@ -76,7 +76,7 @@ open_mock.assert_called_once_with('path', 'wb') image_service_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum=None, checksum_algo=None) image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') mock_zstd.assert_called_once_with('path') @@ -102,7 +102,7 @@ mock_checksum.assert_called_once_with('path', algorithm='sha256') open_mock.assert_called_once_with('path', 'wb') image_service_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum='f00', checksum_algo='sha256') image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') mock_zstd.assert_called_once_with('path') @@ -131,7 +131,7 @@ mock_checksum.assert_called_once_with('path', algorithm='sha256') open_mock.assert_called_once_with('path', 'wb') image_service_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum='f00', checksum_algo='sha256') # If the checksum fails, then we don't attempt to convert the image. image_to_raw_mock.assert_not_called() mock_zstd.assert_not_called() @@ -158,7 +158,7 @@ mock_checksum.assert_called_once_with('path', algorithm='md5') open_mock.assert_called_once_with('path', 'wb') image_service_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum='f00', checksum_algo=None) image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') mock_zstd.assert_called_once_with('path') @@ -185,7 +185,7 @@ mock_checksum.assert_called_once_with('path', algorithm='sha512') open_mock.assert_called_once_with('path', 'wb') image_service_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum='sha512:f00', checksum_algo=None) image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') mock_zstd.assert_called_once_with('path') @@ -215,7 +215,7 @@ mock_checksum.assert_not_called() open_mock.assert_called_once_with('path', 'wb') svc_mock.return_value.download.assert_called_once_with( - 'image_href', 'file') + 'image_href', 'file', checksum='sha512:f00', checksum_algo=None) image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') svc_mock.return_value.set_image_auth.assert_called_once_with( diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_inspection_rule.py ironic-29.0.5/ironic/tests/unit/common/test_inspection_rule.py --- ironic-29.0.0/ironic/tests/unit/common/test_inspection_rule.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_inspection_rule.py 2026-04-30 14:30:48.000000000 +0000 @@ -16,8 +16,10 @@ from ironic.common import exception from ironic.common import inspection_rules +from ironic.common.inspection_rules import base from ironic.common.inspection_rules import engine from ironic.common.inspection_rules import utils +from ironic.common.inspection_rules import validation from ironic.conductor import task_manager from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils @@ -27,7 +29,10 @@ def setUp(self): super(TestInspectionRules, self).setUp() self.node = obj_utils.create_test_node(self.context, - driver='fake-hardware') + driver='fake-hardware', + driver_info={}, + extra={}) + self.sensitive_fields = ['password', 'auth_token', 'bmc_password'] self.test_data = { 'username': 'testuser', @@ -378,40 +383,78 @@ def test_operator_with_loop(self): """Test operator check_with_loop method.""" - condition = { + eq_condition = { 'op': 'eq', - 'loop': [ - {'values': [1, 1]}, - {'values': [2, 2]}, - {'values': [3, 4]} - ], + 'args': {'values': [1, '{item}']}, + 'loop': [1, 2, 3, 4], + 'multiple': 'any' + } + + contains_condition = { + 'op': 'contains', + 'args': {'value': '{item}', 'regex': '4'}, + 'loop': ['test4', 'value5', 'string6'], 'multiple': 'any' } - inventory = {'data': 'test'} - plugin_data = {'plugin': 'data'} + oneof_condition = { + 'op': 'one-of', + 'args': {'value': '{inventory[cpu][architecture]}', + 'values': ['{item}']}, + 'loop': ['x86_64', 'aarch64', 'ppc64le'], + 'multiple': 'any' + } with task_manager.acquire(self.context, self.node.uuid) as task: - op = inspection_rules.operators.EqOperator() + eq_op = inspection_rules.operators.EqOperator() + contains_op = inspection_rules.operators.ContainsOperator() + oneof_op = inspection_rules.operators.OneOfOperator() # 'any' multiple (should return True) - self.assertTrue(op.check_with_loop(task, condition, inventory, - plugin_data)) + self.assertTrue(eq_op.check_with_loop( + task, eq_condition, self.inventory, self.plugin_data)) + self.assertTrue(contains_op.check_with_loop( + task, contains_condition, self.inventory, self.plugin_data)) + self.assertTrue(oneof_op.check_with_loop( + task, oneof_condition, self.inventory, self.plugin_data)) # 'all' multiple (should return False) - condition['multiple'] = 'all' - self.assertFalse(op.check_with_loop(task, condition, inventory, - plugin_data)) + eq_condition['multiple'] = 'all' + contains_condition['multiple'] = 'all' + oneof_condition['multiple'] = 'all' + + self.assertFalse(eq_op.check_with_loop( + task, eq_condition, self.inventory, self.plugin_data)) + self.assertFalse(contains_op.check_with_loop( + task, contains_condition, self.inventory, self.plugin_data)) + self.assertFalse(oneof_op.check_with_loop( + task, oneof_condition, self.inventory, self.plugin_data)) # 'first' multiple (should return True) - condition['multiple'] = 'first' - self.assertTrue(op.check_with_loop(task, condition, inventory, - plugin_data)) - - # 'last' multiple (should return False) - condition['multiple'] = 'last' - self.assertFalse(op.check_with_loop(task, condition, inventory, - plugin_data)) + eq_condition['multiple'] = 'first' + contains_condition['multiple'] = 'first' + oneof_condition['multiple'] = 'first' + + self.assertTrue(eq_op.check_with_loop( + task, eq_condition, self.inventory, self.plugin_data)) + self.assertTrue(contains_op.check_with_loop( + task, contains_condition, self.inventory, self.plugin_data)) + self.assertTrue(oneof_op.check_with_loop( + task, oneof_condition, self.inventory, self.plugin_data)) + + # 'last' multiple (should return False for eq, True for others) + eq_condition['multiple'] = 'last' + contains_condition['multiple'] = 'last' + oneof_condition['multiple'] = 'last' + + self.assertFalse(eq_op.check_with_loop(task, eq_condition, + self.inventory, + self.plugin_data)) + self.assertFalse(contains_op.check_with_loop( + task, contains_condition, self.inventory, self.plugin_data)) + # This should be False since 'ppc64le' doesn't match 'x86_64' + self.assertFalse(oneof_op.check_with_loop( + task, oneof_condition, self.inventory, self.plugin_data)) def test_rule_operators(self): """Test all inspection_rules.operators with True and False cases.""" @@ -420,14 +463,26 @@ {'values': [5, 5]}, {'values': [5, 10]} ], + inspection_rules.operators.NeOperator: [ + {'values': [5, 10]}, + {'values': [5, 5]} + ], inspection_rules.operators.LtOperator: [ {'values': [5, 10]}, {'values': [10, 5]} ], + inspection_rules.operators.LeOperator: [ + {'values': [5, 10]}, + {'values': [10, 5]} + ], inspection_rules.operators.GtOperator: [ {'values': [10, 5]}, {'values': [5, 10]} ], + inspection_rules.operators.GeOperator: [ + {'values': [10, 5]}, + {'values': [5, 10]} + ], inspection_rules.operators.EmptyOperator: [ {'value': ''}, {'value': 'not empty'} @@ -449,7 +504,7 @@ {'value': 'z', 'values': ['a', 'b', 'c']} ], inspection_rules.operators.IsNoneOperator: [ - {'value': 'None'}, + {'value': None}, {'value': 'something'} ], inspection_rules.operators.IsTrueOperator: [ @@ -492,12 +547,34 @@ self.assertRaises(exception.HardwareInspectionFailure, action, task, msg=error_msg) + def test_action_path_dot_slash_notation(self): + with task_manager.acquire(self.context, self.node.uuid) as task: + action = inspection_rules.actions.SetAttributeAction() + + # slash notation + action( + task, path='driver_info/new', value={'new_key': 'test_value'}) + + # dot notation + action(task, path='driver_info.next.nested.deeper', + value={'next_key': 'test_value'}) + + self.assertEqual( + {'new_key': 'test_value'}, task.node.driver_info['new']) + self.assertEqual( + {'nested': {'deeper': {'next_key': 'test_value'}}}, + task.node.driver_info['next']) + def test_set_attribute_action(self): """Test SetAttributeAction sets node attribute.""" with task_manager.acquire(self.context, self.node.uuid) as task: action = inspection_rules.actions.SetAttributeAction() - action(task, path='extra', value={'test_key': 'test_value'}) - self.assertEqual({'test_key': 'test_value'}, task.node.extra) + + action( + task, path='driver_info/new', value={'new_key': 'test_value'}) + + self.assertEqual( + {'new_key': 'test_value'}, task.node.driver_info['new']) def test_extend_attribute_action(self): """Test ExtendAttributeAction extends a list attribute.""" @@ -667,31 +744,50 @@ log_action, task, msg='test message', level='invalid_level' ) - def test_action_with_loop(self): + def test_action_with_list_loop(self): """Test action execute_with_loop method.""" - action_data = { + list_loop_data = { 'op': 'set-attribute', + 'args': {'path': '{item[path]}', 'value': '{item[value]}'}, 'loop': [ - {'path': 'extra/test1', 'value': 'value1'}, - {'path': 'extra/test2', 'value': 'value2'} + {'path': 'driver_info/ipmi_username', 'value': 'cidadmin'}, + {'path': 'driver_info/ipmi_password', 'value': 'cidpassword'}, + { + 'path': 'driver_info/ipmi_address', + 'value': '{inventory[bmc_address]}' + } ] } - inventory = {'data': 'test'} - plugin_data = {'plugin': 'data'} - with task_manager.acquire(self.context, self.node.uuid) as task: - task.node.extra = {} + action = inspection_rules.actions.SetAttributeAction() + action.execute_with_loop(task, list_loop_data, self.inventory, + self.plugin_data) + self.assertEqual('cidadmin', + task.node.driver_info['ipmi_username']) + self.assertEqual('cidpassword', + task.node.driver_info['ipmi_password']) + self.assertEqual('192.168.1.100', + task.node.driver_info['ipmi_address']) + + def test_action_with_dict_loop(self): + """Test action execute_with_loop method.""" + dict_loop_data = { + 'op': 'set-attribute', + 'args': {'path': '{item[path]}', 'value': '{item[value]}'}, + 'loop': { + 'path': 'driver_info/ipmi_address', + 'value': '{inventory[bmc_address]}' + } + } - # execute_with_loop + with task_manager.acquire(self.context, self.node.uuid) as task: action = inspection_rules.actions.SetAttributeAction() - results = action.execute_with_loop(task, action_data, inventory, - plugin_data) + action.execute_with_loop(task, dict_loop_data, self.inventory, + self.plugin_data) - # verify both loop items were processed - self.assertEqual(2, len(results)) - self.assertEqual('value1', task.node.extra['test1']) - self.assertEqual('value2', task.node.extra['test2']) + self.assertEqual('192.168.1.100', + task.node.driver_info['ipmi_address']) class TestShallowMask(TestInspectionRules): @@ -806,3 +902,133 @@ self.assertEqual('new_value', original_data['new_key']) self.assertEqual([1, 2, 3, 4], original_data['data']) + + +class TestInterpolation(TestInspectionRules): + def setUp(self): + super(TestInterpolation, self).setUp() + + def test_variable_interpolation(self): + """Test variable interpolation.""" + loop_context = { + 'item': { + 'key': 'value', + 'nested': {'deep': 'nested_value'} + } + } + + with task_manager.acquire(self.context, self.node.uuid) as task: + value = "{inventory[cpu][architecture]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, self.plugin_data) + self.assertEqual("x86_64", result) + + value = "{inventory[interfaces][0][mac_address]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, self.plugin_data) + self.assertEqual("2a:03:9c:53:4e:46", result) + + value = "{plugin_data[plugin]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, self.plugin_data) + self.assertEqual("data", result) + + value = "{node.driver}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, self.plugin_data) + self.assertEqual("fake-hardware", result) + + value = "{item}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, + self.plugin_data, loop_context) + + self.assertEqual( + "{'key': 'value', 'nested': {'deep': 'nested_value'}}", + result) + + value = "{item[key]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, + self.plugin_data, loop_context) + self.assertEqual("value", result) + + value = "{item[nested][deep]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, + self.plugin_data, loop_context) + self.assertEqual("nested_value", result) + + value = "CPU: {inventory[cpu][count]}, Item: {item[key]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, + self.plugin_data, loop_context) + self.assertEqual("CPU: 4, Item: value", result) + + dict_value = { + "normal_key": "normal_value", + "interpolated_key": "{inventory[cpu][architecture]}", + "nested": { + "item_key": "{item[key]}", + "inventory_key": "{inventory[bmc_address]}" + } + } + result = base.Base.interpolate_variables( + dict_value, task.node, self.inventory, + self.plugin_data, loop_context) + self.assertEqual({ + "normal_key": "normal_value", + "interpolated_key": "x86_64", + "nested": { + "item_key": "value", + "inventory_key": "192.168.1.100" + } + }, result) + + list_value = [ + "normal_value", + "{inventory[cpu][architecture]}", + "{item[key]}", + ["{inventory[bmc_address]}", "{item[nested][deep]}"] + ] + result = base.Base.interpolate_variables( + list_value, task.node, self.inventory, + self.plugin_data, loop_context) + self.assertEqual([ + "normal_value", + "x86_64", + "value", + ["192.168.1.100", "nested_value"] + ], result) + + value = "{inventory[missing][key]}" + result = base.Base.interpolate_variables( + value, task.node, self.inventory, self.plugin_data) + self.assertEqual(value, result) + + +class TestValidation(TestInspectionRules): + def test_unsupported_operator_rejected(self): + """Unsupported operator (even inverted) must raise Invalid.""" + rule = { + 'actions': [{'op': 'noop', 'args': {}}], + 'conditions': [{'op': '!unknown', 'args': {}}] + } + + self.assertRaises(exception.Invalid, validation.validate_rule, rule) + + def test_conditions_not_list_raises_invalid(self): + rule = { + 'actions': [{'op': 'noop', 'args': {}}], + 'conditions': {'op': 'eq', 'args': [1, 1]} # not a list + } + + self.assertRaises(exception.Invalid, validation.validate_rule, rule) + + def test_missing_actions_key_raises_invalid(self): + rule = { + 'conditions': [{'op': 'eq', 'args': [1, 1]}] + # 'actions' is missing + } + + self.assertRaises(exception.Invalid, validation.validate_rule, rule) diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_molds.py ironic-29.0.5/ironic/tests/unit/common/test_molds.py --- ironic-29.0.0/ironic/tests/unit/common/test_molds.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_molds.py 2026-04-30 14:30:48.000000000 +0000 @@ -19,6 +19,7 @@ import requests from ironic.common import exception +from ironic.common import keystone from ironic.common import molds from ironic.common import swift from ironic.conductor import task_manager @@ -32,15 +33,18 @@ super(ConfigurationMoldTestCase, self).setUp() self.node = obj_utils.create_test_node(self.context) + @mock.patch.object(keystone, 'get_endpoint', autospec=True) @mock.patch.object(swift, 'get_swift_session', autospec=True) @mock.patch.object(requests, 'put', autospec=True) - def test_save_configuration_swift(self, mock_put, mock_swift): + def test_save_configuration_swift(self, mock_put, mock_swift, + mock_endpoint): mock_session = mock.Mock() mock_session.get_token.return_value = 'token' mock_swift.return_value = mock_session cfg.CONF.set_override('storage', 'swift', 'molds') url = 'https://example.com/file1' data = {'key': 'value'} + mock_endpoint.return_value = 'https://example.com/v1' with task_manager.acquire(self.context, self.node.uuid) as task: molds.save_configuration(task, url, data) @@ -49,15 +53,18 @@ headers={'X-Auth-Token': 'token'}, timeout=60) + @mock.patch.object(keystone, 'get_endpoint', autospec=True) @mock.patch.object(swift, 'get_swift_session', autospec=True) @mock.patch.object(requests, 'put', autospec=True) - def test_save_configuration_swift_noauth(self, mock_put, mock_swift): + def test_save_configuration_swift_noauth(self, mock_put, mock_swift, + mock_endpoint): mock_session = mock.Mock() mock_session.get_token.return_value = None mock_swift.return_value = mock_session cfg.CONF.set_override('storage', 'swift', 'molds') url = 'https://example.com/file1' data = {'key': 'value'} + mock_endpoint.return_value = 'https://example.com/v1' with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises( @@ -164,9 +171,11 @@ timeout=60) self.assertEqual(mock_put.call_count, 2) + @mock.patch.object(keystone, 'get_endpoint', autospec=True) @mock.patch.object(swift, 'get_swift_session', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_get_configuration_swift(self, mock_get, mock_swift): + def test_get_configuration_swift(self, mock_get, mock_swift, + mock_endpoint): mock_session = mock.Mock() mock_session.get_token.return_value = 'token' mock_swift.return_value = mock_session @@ -177,6 +186,7 @@ response.json.return_value = {'key': 'value'} mock_get.return_value = response url = 'https://example.com/file1' + mock_endpoint.return_value = 'https://example.com/v1' with task_manager.acquire(self.context, self.node.uuid) as task: result = molds.get_configuration(task, url) @@ -185,15 +195,44 @@ url, headers={'X-Auth-Token': 'token'}, timeout=60) self.assertJsonEqual({'key': 'value'}, result) + mock_endpoint.assert_called_once_with('swift', session=mock_session) + @mock.patch.object(keystone, 'get_endpoint', autospec=True) @mock.patch.object(swift, 'get_swift_session', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_get_configuration_swift_noauth(self, mock_get, mock_swift): + def test_get_configuration_swift_url_mismatch( + self, mock_get, mock_swift, mock_endpoint): + mock_session = mock.Mock() + mock_session.get_token.return_value = 'token' + mock_swift.return_value = mock_session + cfg.CONF.set_override('storage', 'swift', 'molds') + response = mock.MagicMock() + response.status_code = 200 + response.content = "{'key': 'value'}" + response.json.return_value = {'key': 'value'} + mock_get.return_value = response + url = 'https://example.com/file1' + mock_endpoint.return_value = 'https://cloud.foo/v1' + + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises(exception.InvalidParameterValue, + molds.get_configuration, + task, url) + + mock_endpoint.assert_called_once_with('swift', session=mock_session) + mock_get.assert_not_called() + + @mock.patch.object(keystone, 'get_endpoint', autospec=True) + @mock.patch.object(swift, 'get_swift_session', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_get_configuration_swift_noauth(self, mock_get, mock_swift, + mock_endpoint): mock_session = mock.Mock() mock_session.get_token.return_value = None mock_swift.return_value = mock_session cfg.CONF.set_override('storage', 'swift', 'molds') url = 'https://example.com/file1' + mock_endpoint.return_value = 'https://example.com/v1' with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises( diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_oci_registry.py ironic-29.0.5/ironic/tests/unit/common/test_oci_registry.py --- ironic-29.0.0/ironic/tests/unit/common/test_oci_registry.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_oci_registry.py 2026-04-30 14:30:48.000000000 +0000 @@ -28,6 +28,12 @@ CONF = cfg.CONF +ACCEPT_INDEX_OR_MANIFEST = ( + 'application/vnd.oci.image.index.v1+json, ' + 'application/vnd.oci.image.manifest.v1+json' +) + + class OciClientTestCase(base.TestCase): def setUp(self): @@ -110,6 +116,7 @@ '60f61caaff8a') get_mock.return_value.status_code = 200 get_mock.return_value.text = '{}' + get_mock.return_value.headers = {} res = self.client.get_manifest( 'oci://localhost/local@sha256:' + csum) self.assertEqual({}, res) @@ -122,6 +129,24 @@ headers={'Accept': 'application/vnd.oci.image.manifest.v1+json'}, timeout=60) + def test_get_manifest_with_content_digest(self, get_mock): + csum = ('44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c0' + '60f61caaff8a') + get_mock.return_value.status_code = 200 + get_mock.return_value.text = '{}' + get_mock.return_value.headers = {'docker-content-digest': 'abcd'} + res = self.client.get_manifest( + 'oci://localhost/local@sha256:' + csum) + self.assertEqual({'dockerContentDigest': 'abcd'}, res) + get_mock.return_value.assert_has_calls([ + mock.call.raise_for_status(), + mock.call.encoding.__bool__()]) + get_mock.assert_called_once_with( + mock.ANY, + 'https://localhost/v2/local/manifests/sha256:' + csum, + headers={'Accept': 'application/vnd.oci.image.manifest.v1+json'}, + timeout=60) + def test_get_manifest_auth_required(self, get_mock): fake_csum = 'f' * 64 response = mock.Mock() @@ -208,6 +233,7 @@ } get_mock.return_value.status_code = 200 get_mock.return_value.text = '{}' + get_mock.return_value.headers = {} res = self.client.get_artifact_index( 'oci://localhost/local:tag') self.assertEqual({}, res) @@ -220,7 +246,7 @@ get_mock.assert_called_once_with( mock.ANY, 'https://localhost/v2/local/manifests/tag', - headers={'Accept': 'application/vnd.oci.image.index.v1+json'}, + headers={'Accept': ACCEPT_INDEX_OR_MANIFEST}, timeout=60) @mock.patch.object(oci_registry.OciClient, '_resolve_tag', @@ -245,7 +271,7 @@ call_mock = mock.call( mock.ANY, 'https://localhost/v2/local/manifests/tag', - headers={'Accept': 'application/vnd.oci.image.index.v1+json'}, + headers={'Accept': ACCEPT_INDEX_OR_MANIFEST}, timeout=60) get_mock.assert_has_calls([call_mock]) @@ -272,7 +298,7 @@ call_mock = mock.call( mock.ANY, 'https://localhost/v2/local/manifests/tag', - headers={'Accept': 'application/vnd.oci.image.index.v1+json'}, + headers={'Accept': ACCEPT_INDEX_OR_MANIFEST}, timeout=60) # Automatic retry to authenticate get_mock.assert_has_calls([call_mock, call_mock]) @@ -300,7 +326,7 @@ call_mock = mock.call( mock.ANY, 'https://localhost/v2/local/manifests/tag', - headers={'Accept': 'application/vnd.oci.image.index.v1+json'}, + headers={'Accept': ACCEPT_INDEX_OR_MANIFEST}, timeout=60) get_mock.assert_has_calls([call_mock]) @@ -327,7 +353,7 @@ call_mock = mock.call( mock.ANY, 'https://localhost/v2/local/manifests/tag', - headers={'Accept': 'application/vnd.oci.image.index.v1+json'}, + headers={'Accept': ACCEPT_INDEX_OR_MANIFEST}, timeout=60) get_mock.assert_has_calls([call_mock]) @@ -354,7 +380,24 @@ response.links = {} get_mock.return_value = response self.assertRaises( - exception.ImageNotFound, + exception.OciImageTagNotFound, + self.client._resolve_tag, + parse.urlparse('oci://localhost/local')) + call_mock = mock.call( + mock.ANY, + 'https://localhost/v2/local/tags/list', + headers={'Accept': 'application/vnd.oci.image.index.v1+json'}, + timeout=60) + get_mock.assert_has_calls([call_mock]) + + def test__resolve_tag_if_no_tags(self, get_mock): + response = mock.Mock() + response.json.return_value = {'tags': []} + response.status_code = 200 + response.links = {} + get_mock.return_value = response + self.assertRaises( + exception.InvalidImageRef, self.client._resolve_tag, parse.urlparse('oci://localhost/local')) call_mock = mock.call( diff -Nru ironic-29.0.0/ironic/tests/unit/common/test_pxe_utils.py ironic-29.0.5/ironic/tests/unit/common/test_pxe_utils.py --- ironic-29.0.0/ironic/tests/unit/common/test_pxe_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/common/test_pxe_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -16,6 +16,7 @@ import collections import os +import pathlib import shutil import tempfile from unittest import mock @@ -2666,7 +2667,7 @@ self.assertEqual(30 * 60, cache._cache_ttl) -@mock.patch.object(os, 'makedirs', autospec=True) +@mock.patch.object(pathlib.Path, 'mkdir', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(shutil, 'copy2', autospec=True) class TestPXEUtilsBootloader(db_base.DbTestCase): @@ -2675,80 +2676,83 @@ super(TestPXEUtilsBootloader, self).setUp() def test_place_loaders_for_boot_default_noop(self, mock_copy2, - mock_chmod, mock_makedirs): + mock_chmod, mock_mkdir): res = pxe_utils.place_loaders_for_boot('/httpboot') self.assertIsNone(res) self.assertFalse(mock_copy2.called) self.assertFalse(mock_chmod.called) - self.assertFalse(mock_makedirs.called) - self.assertFalse(mock_makedirs.called) + self.assertFalse(mock_mkdir.called) def test_place_loaders_for_boot_no_source(self, mock_copy2, - mock_chmod, mock_makedirs): + mock_chmod, mock_mkdir): self.config(loader_file_paths='grubaa64.efi:/path/to/file', group='pxe') mock_copy2.side_effect = FileNotFoundError('No such file or directory') self.assertRaises(exception.IncorrectConfiguration, pxe_utils.place_loaders_for_boot, '/tftpboot') + self.assertTrue(mock_copy2.called) self.assertFalse(mock_chmod.called) - self.assertFalse(mock_makedirs.called) + mock_mkdir.assert_called() def test_place_loaders_for_boot_two_files(self, mock_copy2, - mock_chmod, mock_makedirs): + mock_chmod, mock_mkdir): self.config(loader_file_paths='bootx64.efi:/path/to/shimx64.efi,' 'grubx64.efi:/path/to/grubx64.efi', group='pxe') self.config(file_permission=420, group='pxe') - res = pxe_utils.place_loaders_for_boot('/tftpboot') + dest = pathlib.Path('/tftpboot') + res = pxe_utils.place_loaders_for_boot(dest) self.assertIsNone(res) mock_copy2.assert_has_calls([ - mock.call('/path/to/shimx64.efi', '/tftpboot/bootx64.efi'), - mock.call('/path/to/grubx64.efi', '/tftpboot/grubx64.efi') + mock.call('/path/to/shimx64.efi', dest.joinpath('bootx64.efi')), + mock.call('/path/to/grubx64.efi', dest.joinpath('grubx64.efi')) ]) mock_chmod.assert_has_calls([ - mock.call('/tftpboot/bootx64.efi', CONF.pxe.file_permission), - mock.call('/tftpboot/grubx64.efi', CONF.pxe.file_permission) + mock.call(dest.joinpath('bootx64.efi'), CONF.pxe.file_permission), + mock.call(dest.joinpath('grubx64.efi'), CONF.pxe.file_permission) ]) - self.assertFalse(mock_makedirs.called) + self.assertTrue(mock_mkdir.called) def test_place_loaders_for_boot_two_files_exception_on_copy( - self, mock_copy2, mock_chmod, mock_makedirs): + self, mock_copy2, mock_chmod, mock_mkdir): self.config(loader_file_paths='bootx64.efi:/path/to/shimx64.efi,' 'grubx64.efi:/path/to/grubx64.efi', group='pxe') self.config(file_permission=420, group='pxe') mock_copy2.side_effect = PermissionError('Permission denied') + dest = pathlib.Path('/tftpboot') self.assertRaises(exception.IncorrectConfiguration, pxe_utils.place_loaders_for_boot, - '/tftpboot') + dest) mock_copy2.assert_has_calls([ - mock.call('/path/to/shimx64.efi', '/tftpboot/bootx64.efi'), + mock.call('/path/to/shimx64.efi', dest.joinpath('bootx64.efi')), ]) mock_chmod.assert_not_called() - self.assertFalse(mock_makedirs.called) + mock_mkdir.assert_called() def test_place_loaders_for_boot_two_files_exception_on_chmod( - self, mock_copy2, mock_chmod, mock_makedirs): + self, mock_copy2, mock_chmod, mock_mkdir): self.config(loader_file_paths='bootx64.efi:/path/to/shimx64.efi,' 'grubx64.efi:/path/to/grubx64.efi', group='pxe') self.config(file_permission=420, group='pxe') mock_chmod.side_effect = OSError('Chmod not permitted') + dest = pathlib.Path('/tftpboot') self.assertRaises(exception.IncorrectConfiguration, pxe_utils.place_loaders_for_boot, - '/tftpboot') + dest) mock_copy2.assert_has_calls([ - mock.call('/path/to/shimx64.efi', '/tftpboot/bootx64.efi'), + mock.call('/path/to/shimx64.efi', dest.joinpath('bootx64.efi')), ]) mock_chmod.assert_has_calls([ - mock.call('/tftpboot/bootx64.efi', CONF.pxe.file_permission), + mock.call(dest.joinpath('bootx64.efi'), CONF.pxe.file_permission), ]) - self.assertFalse(mock_makedirs.called) + mock_mkdir.assert_called() def test_place_loaders_for_boot_raises_exception_with_absolute_path( - self, mock_copy2, mock_chmod, mock_makedirs): + self, mock_copy2, mock_chmod, mock_mkdir): self.config( loader_file_paths='/tftpboot/bootx64.efi:/path/to/shimx64.efi', group='pxe') @@ -2760,25 +2764,30 @@ '/tftpboot/bootx64.efi', str(exc)) def test_place_loaders_for_boot_two_files_relative_path( - self, mock_copy2, mock_chmod, mock_makedirs): + self, mock_copy2, mock_chmod, mock_mkdir): self.config(loader_file_paths='grub/bootx64.efi:/path/to/shimx64.efi,' 'grub/grubx64.efi:/path/to/grubx64.efi', group='pxe') self.config(dir_permission=484, group='pxe') self.config(file_permission=420, group='pxe') - res = pxe_utils.place_loaders_for_boot('/tftpboot') + dest = pathlib.Path('/tftpboot') + res = pxe_utils.place_loaders_for_boot(dest) self.assertIsNone(res) mock_copy2.assert_has_calls([ - mock.call('/path/to/shimx64.efi', '/tftpboot/grub/bootx64.efi'), - mock.call('/path/to/grubx64.efi', '/tftpboot/grub/grubx64.efi') + mock.call('/path/to/shimx64.efi', + dest.joinpath('grub/bootx64.efi')), + mock.call('/path/to/grubx64.efi', + dest.joinpath('grub/grubx64.efi')) ]) mock_chmod.assert_has_calls([ - mock.call('/tftpboot/grub/bootx64.efi', CONF.pxe.file_permission), - mock.call('/tftpboot/grub/grubx64.efi', CONF.pxe.file_permission) + mock.call(dest.joinpath('grub/bootx64.efi'), + CONF.pxe.file_permission), + mock.call(dest.joinpath('grub/grubx64.efi'), + CONF.pxe.file_permission) ]) - mock_makedirs.assert_has_calls([ - mock.call('/tftpboot/grub', mode=CONF.pxe.dir_permission, - exist_ok=True), - mock.call('/tftpboot/grub', mode=CONF.pxe.dir_permission, - exist_ok=True), + mock_mkdir.assert_has_calls([ + mock.call(dest.joinpath('grub'), mode=CONF.pxe.dir_permission, + parents=True, exist_ok=True), + mock.call(dest.joinpath('grub'), mode=CONF.pxe.dir_permission, + parents=True, exist_ok=True), ]) diff -Nru ironic-29.0.0/ironic/tests/unit/conductor/test_manager.py ironic-29.0.5/ironic/tests/unit/conductor/test_manager.py --- ironic-29.0.0/ironic/tests/unit/conductor/test_manager.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/conductor/test_manager.py 2026-04-30 14:30:48.000000000 +0000 @@ -5723,6 +5723,38 @@ mock_power_update.assert_called_once_with( self.task.context, self.node.instance_uuid, states.POWER_OFF) + @mock.patch.object(nova, 'power_update', autospec=True) + def test_max_retries_exceeded_preserve_admin_intent(self, + mock_power_update, + node_power_action): + self.config(force_power_state_during_sync=True, group='conductor') + self.config(power_state_sync_max_retries=1, group='conductor') + + self.node.maintenance = True + self.node.maintenance_reason = 'admin set maintenance' + self.node.fault = None + + self._do_sync_power_state( + states.POWER_ON, + [exception.IronicException('foo'), + exception.IronicException('foo')] + ) + + self.assertFalse(self.power.validate.called) + power_exp_calls = [mock.call(self.task)] * 2 + self.assertEqual(power_exp_calls, + self.power.get_power_state.call_args_list) + node_power_action.assert_not_called() + self.assertIsNone(self.node.power_state) + self.assertEqual(2, + self.service.power_state_sync_count[self.node.uuid]) + + self.assertTrue(self.node.maintenance) + self.assertEqual('admin set maintenance', self.node.maintenance_reason) + self.assertIsNone(self.node.fault) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, None) + @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification', autospec=True) @mock.patch.object(nova, 'power_update', autospec=True) diff -Nru ironic-29.0.0/ironic/tests/unit/conf/test_conductor.py ironic-29.0.5/ironic/tests/unit/conf/test_conductor.py --- ironic-29.0.0/ironic/tests/unit/conf/test_conductor.py 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/conf/test_conductor.py 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,34 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic.conf import CONF +from ironic.tests.base import TestCase + + +class ValidateConductorAllowedPaths(TestCase): + def test_abspath_validation_bad_path_raises(self): + """Verifies setting a relative path raises an error via oslo.config.""" + self.assertRaises( + ValueError, + CONF.set_override, + 'file_url_allowed_paths', + ['bad/path'], + 'conductor' + ) + + def test_abspath_validation_good_paths(self): + """Verifies setting an absolute path works via oslo.config.""" + CONF.set_override('file_url_allowed_paths', ['/var'], 'conductor') + + def test_abspath_validation_good_paths_trailing_slash(self): + """Verifies setting an absolute path works via oslo.config.""" + CONF.set_override('file_url_allowed_paths', ['/var/'], 'conductor') diff -Nru ironic-29.0.0/ironic/tests/unit/conf/test_types.py ironic-29.0.5/ironic/tests/unit/conf/test_types.py --- ironic-29.0.0/ironic/tests/unit/conf/test_types.py 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/conf/test_types.py 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,63 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic.conf import types +from ironic.tests.base import TestCase + + +class ExplicitAbsolutePath(TestCase): + def test_explicit_absolute_path(self): + """Verifies the Opt subclass used to validate absolute paths.""" + good_paths = [ + '/etc/passwd', # Valid + '/usr/bin/python', # Valid + '/home/user/file.txt', # Valid - dot in filename allowed + '/var/lib/ironic/.secretdir', # Valid - hidden directory allowed + '/var/lib/ironic/oslo.config', # Valid - dots in filename allowed + '/tmp/', # Valid + '/', # Valid (root directory) + '/.hidden_root_file', # Valid + '/path/including/a/numb3r', # Valid + '/a/path/with/a/trailing/slash/' # Valid + ] + bad_paths = [ + 'relative/path', # Invalid - no leading slash + './file.txt', # Invalid - relative path + '../file.txt', # Invalid - relative path + 'file.txt', # Invalid - no leading slash + '', # Invalid - empty string + '/var/lib/ironic/../../../etc/passwd', # Invalid - path traversal + '/etc/../etc/passwd', # Invalid - path traversal + '/home/user/./config', # Invalid - contains current dir reference + '/home/user/../user/config', # Invalid - path traversal + '/../etc/passwd', # Invalid - path traversal at beginning + '/.', # Invalid - just current directory + '/..' # Invalid - just parent directory + ] + + eap = types.ExplicitAbsolutePath() + + def _trypath(tpath): + try: + eap(tpath) + except ValueError: + return False + else: + return True + + for path in good_paths: + self.assertTrue(_trypath(path), + msg=f"Improperly disallowed path: {path}") + + for path in bad_paths: + self.assertFalse(_trypath(path), + msg=f"Improperly allowed path: {path}") diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/inspector/hooks/test_local_link_connection.py ironic-29.0.5/ironic/tests/unit/drivers/modules/inspector/hooks/test_local_link_connection.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/inspector/hooks/test_local_link_connection.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/inspector/hooks/test_local_link_connection.py 2026-04-30 14:30:48.000000000 +0000 @@ -15,6 +15,7 @@ from oslo_utils import uuidutils +from ironic.common import exception from ironic.conductor import task_manager from ironic.conf import CONF from ironic.drivers.modules.inspector.hooks import local_link_connection as \ @@ -86,7 +87,7 @@ @mock.patch.object(port.Port, 'get_by_address', autospec=True) @mock.patch.object(port.Port, 'save', autospec=True) def test_no_port_in_ironic(self, mock_port_save, mock_get_port, mock_log): - mock_get_port.return_value = None + mock_get_port.side_effect = exception.PortNotFound with task_manager.acquire(self.context, self.node.id) as task: hook.LocalLinkConnectionHook().__call__(task, self.inventory, self.plugin_data) diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/network/test_flat.py ironic-29.0.5/ironic/tests/unit/drivers/modules/network/test_flat.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/network/test_flat.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/network/test_flat.py 2026-04-30 14:30:48.000000000 +0000 @@ -249,6 +249,21 @@ self.assertRaises(exception.NetworkError, self.interface._bind_flat_ports, task) + def test__bind_flat_ports_no_vifs_bound_raise(self): + utils.create_test_port(self.context, node_id=self.node.id, + address='52:54:00:cf:2d:33', + internal_info={}, + uuid=uuidutils.generate_uuid()) + internal_info = {'cleaning_vif_port_id': uuidutils.generate_uuid()} + utils.create_test_port(self.context, node_id=self.node.id, + address='52:54:00:cf:2d:34', + internal_info=internal_info, + uuid=uuidutils.generate_uuid()) + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegex(exception.NetworkError, + 'Failed to bind any flat network ports', + self.interface._bind_flat_ports, task) + @mock.patch.object(flat_interface.FlatNetwork, '_bind_flat_ports', autospec=True) def test_add_rescuing_network(self, bind_mock): diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/redfish/test_inspect.py ironic-29.0.5/ironic/tests/unit/drivers/modules/redfish/test_inspect.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/redfish/test_inspect.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/redfish/test_inspect.py 2026-04-30 14:30:48.000000000 +0000 @@ -475,6 +475,29 @@ port = mock_list_by_node_id.return_value self.assertFalse(port[0].pxe_enabled) + @mock.patch.object(objects.Port, 'list_by_node_id') # noqa + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_inspect_hardware_with_conf_update_pxe_disabled_false( + self, mock_get_system, mock_list_by_node_id): + self.init_system_mock(mock_get_system.return_value) + + pxe_enabled_port = obj_utils.create_test_port( + self.context, uuid=self.node.uuid, + node_id=self.node.id, address='24:6E:96:70:49:01', + pxe_enabled=True) + mock_list_by_node_id.return_value = [pxe_enabled_port] + + self.config(update_pxe_enabled=False, group='inspector') + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.inspect._get_pxe_port_macs = mock.Mock() + task.driver.inspect._get_pxe_port_macs.return_value = \ + ['24:6E:96:70:49:00'] + task.driver.inspect.inspect_hardware(task) + port = mock_list_by_node_id.return_value + self.assertTrue(port[0].pxe_enabled) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_inspect_hardware_with_no_mac(self, mock_get_system): self.init_system_mock(mock_get_system.return_value) diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/redfish/test_management.py ironic-29.0.5/ironic/tests/unit/drivers/modules/redfish/test_management.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/redfish/test_management.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/redfish/test_management.py 2026-04-30 14:30:48.000000000 +0000 @@ -1243,6 +1243,102 @@ management._check_node_firmware_update.assert_not_called() + @mock.patch.object(task_manager, 'acquire', autospec=True) + def test__query_firmware_update_failed_deployfail(self, mock_acquire): + driver_internal_info = { + 'firmware_updates': [ + {'task_monitor': '/task/123', + 'url': 'test1'}]} + self.node.driver_internal_info = driver_internal_info + self.node.provision_state = states.DEPLOYFAIL + self.node.save() + management = redfish_mgmt.RedfishManagement() + mock_manager = mock.Mock() + node_list = [(self.node.uuid, 'redfish', '', driver_internal_info)] + mock_manager.iter_nodes.return_value = node_list + task = mock.Mock(node=self.node, + driver=mock.Mock(management=management)) + mock_acquire.return_value = mock.MagicMock( + __enter__=mock.MagicMock(return_value=task)) + management._clear_firmware_updates = mock.Mock() + + management._query_firmware_update_failed(mock_manager, + self.context) + + management._clear_firmware_updates.assert_called_once_with(self.node) + + @mock.patch.object(task_manager, 'acquire', autospec=True) + def test__query_firmware_update_failed_servicefail(self, mock_acquire): + driver_internal_info = { + 'firmware_updates': [ + {'task_monitor': '/task/123', + 'url': 'test1'}]} + self.node.driver_internal_info = driver_internal_info + self.node.provision_state = states.SERVICEFAIL + self.node.save() + management = redfish_mgmt.RedfishManagement() + mock_manager = mock.Mock() + node_list = [(self.node.uuid, 'redfish', '', driver_internal_info)] + mock_manager.iter_nodes.return_value = node_list + task = mock.Mock(node=self.node, + driver=mock.Mock(management=management)) + mock_acquire.return_value = mock.MagicMock( + __enter__=mock.MagicMock(return_value=task)) + management._clear_firmware_updates = mock.Mock() + + management._query_firmware_update_failed(mock_manager, + self.context) + + management._clear_firmware_updates.assert_called_once_with(self.node) + + @mock.patch.object(task_manager, 'acquire', autospec=True) + def test__query_firmware_update_status_deploywait(self, mock_acquire): + driver_internal_info = { + 'firmware_updates': [ + {'task_monitor': '/task/123', + 'url': 'test1'}]} + self.node.driver_internal_info = driver_internal_info + self.node.provision_state = states.DEPLOYWAIT + self.node.save() + management = redfish_mgmt.RedfishManagement() + mock_manager = mock.Mock() + node_list = [(self.node.uuid, 'redfish', '', driver_internal_info)] + mock_manager.iter_nodes.return_value = node_list + task = mock.Mock(node=self.node, + driver=mock.Mock(management=management)) + mock_acquire.return_value = mock.MagicMock( + __enter__=mock.MagicMock(return_value=task)) + management._check_node_firmware_update = mock.Mock() + + management._query_firmware_update_status(mock_manager, + self.context) + + management._check_node_firmware_update.assert_called_once_with(task) + + @mock.patch.object(task_manager, 'acquire', autospec=True) + def test__query_firmware_update_status_servicewait(self, mock_acquire): + driver_internal_info = { + 'firmware_updates': [ + {'task_monitor': '/task/123', + 'url': 'test1'}]} + self.node.driver_internal_info = driver_internal_info + self.node.provision_state = states.SERVICEWAIT + self.node.save() + management = redfish_mgmt.RedfishManagement() + mock_manager = mock.Mock() + node_list = [(self.node.uuid, 'redfish', '', driver_internal_info)] + mock_manager.iter_nodes.return_value = node_list + task = mock.Mock(node=self.node, + driver=mock.Mock(management=management)) + mock_acquire.return_value = mock.MagicMock( + __enter__=mock.MagicMock(return_value=task)) + management._check_node_firmware_update = mock.Mock() + + management._query_firmware_update_status(mock_manager, + self.context) + + management._check_node_firmware_update.assert_called_once_with(task) + @mock.patch.object(redfish_mgmt.LOG, 'warning', autospec=True) @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) def test__check_node_firmware_update_redfish_conn_error( @@ -1482,6 +1578,8 @@ 'firmware_updates': [ {'task_monitor': '/task/123', 'url': 'test1'}]} self.node.driver_internal_info = driver_internal_info + self.node.clean_step = {'interface': 'management', + 'step': 'update_firmware'} self.node.save() management = redfish_mgmt.RedfishManagement() @@ -1497,6 +1595,66 @@ self.assertEqual({'something': 'else'}, task.node.driver_internal_info) + @mock.patch.object(redfish_mgmt.LOG, 'info', autospec=True) + @mock.patch.object(manager_utils, 'notify_conductor_resume_service', + autospec=True) + def test__continue_firmware_updates_last_update_service( + self, + mock_notify_conductor_resume_service, + mock_log): + mock_update_service = mock.Mock() + driver_internal_info = { + 'something': 'else', + 'firmware_updates': [ + {'task_monitor': '/task/123', 'url': 'test1'}]} + self.node.driver_internal_info = driver_internal_info + self.node.service_step = {'interface': 'management', + 'step': 'update_firmware'} + self.node.save() + + management = redfish_mgmt.RedfishManagement() + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + management._continue_firmware_updates( + task, + mock_update_service, + [{'task_monitor': '/task/123', 'url': 'test1'}]) + + self.assertTrue(mock_log.called) + mock_notify_conductor_resume_service.assert_called_once_with(task) + self.assertEqual({'something': 'else'}, + task.node.driver_internal_info) + + @mock.patch.object(redfish_mgmt.LOG, 'info', autospec=True) + @mock.patch.object(manager_utils, 'notify_conductor_resume_deploy', + autospec=True) + def test__continue_firmware_updates_last_update_deploy( + self, + mock_notify_conductor_resume_deploy, + mock_log): + mock_update_service = mock.Mock() + driver_internal_info = { + 'something': 'else', + 'firmware_updates': [ + {'task_monitor': '/task/123', 'url': 'test1'}]} + self.node.driver_internal_info = driver_internal_info + self.node.deploy_step = {'interface': 'management', + 'step': 'update_firmware'} + self.node.save() + + management = redfish_mgmt.RedfishManagement() + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + management._continue_firmware_updates( + task, + mock_update_service, + [{'task_monitor': '/task/123', 'url': 'test1'}]) + + self.assertTrue(mock_log.called) + mock_notify_conductor_resume_deploy.assert_called_once_with(task) + self.assertEqual({'something': 'else'}, + task.node.driver_internal_info) + @mock.patch.object(redfish_mgmt.LOG, 'debug', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) def test__continue_firmware_updates_more_updates(self, diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/redfish/test_power.py ironic-29.0.5/ironic/tests/unit/drivers/modules/redfish/test_power.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/redfish/test_power.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/redfish/test_power.py 2026-04-30 14:30:48.000000000 +0000 @@ -176,6 +176,193 @@ sushy.RESET_ON) mock_get_system.assert_called_once_with(task.node) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_power_state_race_condition_handling( + self, mock_get_system): + + fake_system = mock_get_system.return_value + mock_response = mock.Mock(status_code=400) + mock_response.json.return_value = { + 'error': { + 'message': 'Host is powered OFF. Power On host and try again', + } + } + + fake_system.reset_system.side_effect = ( + sushy.exceptions.BadRequestError( + method='POST', url='test', response=mock_response)) + + success_scenarios = [ + ( + states.SOFT_POWER_OFF, + states.POWER_OFF, + sushy.RESET_GRACEFUL_SHUTDOWN, + ), + ( + states.SOFT_REBOOT, + states.POWER_ON, + sushy.RESET_GRACEFUL_RESTART + ), + (states.POWER_OFF, states.POWER_OFF, sushy.RESET_FORCE_OFF), + (states.POWER_ON, states.POWER_ON, sushy.RESET_ON), + (states.REBOOT, states.POWER_ON, sushy.RESET_FORCE_RESTART), + (states.POWER_OFF, None, sushy.RESET_FORCE_OFF), + ] + + failure_scenarios = [ + (states.POWER_OFF, states.POWER_ON), + (states.POWER_ON, states.POWER_OFF), + ] + + for target_state, final_state, expected_reset in success_scenarios: + fake_system.power_state = None + if final_state == states.POWER_OFF: + fake_system.power_state = sushy.SYSTEM_POWER_STATE_OFF + elif final_state == states.POWER_ON: + fake_system.power_state = sushy.SYSTEM_POWER_STATE_ON + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.power.set_power_state(task, target_state) + + fake_system.reset_system.assert_called_with(expected_reset) + + fake_system.reset_mock() + + for target_state, actual_state in failure_scenarios: + fake_system.power_state = None + if actual_state == states.POWER_OFF: + fake_system.power_state = sushy.SYSTEM_POWER_STATE_OFF + elif actual_state == states.POWER_ON: + fake_system.power_state = sushy.SYSTEM_POWER_STATE_ON + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises( + sushy.exceptions.BadRequestError, + task.driver.power.set_power_state, task, target_state) + + fake_system.reset_mock() + + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_power_state_conflict_error_handling( + self, mock_get_system): + """Test HTTP 409 Conflict error handling when node already in state.""" + fake_system = mock_get_system.return_value + + # Simulate HTTP 409 Conflict error with "already powered off" message + mock_response_off = mock.Mock(status_code=409) + mock_response_off.json.return_value = { + 'error': { + 'message': 'Server is already powered OFF.', + } + } + error_409_off = sushy.exceptions.HTTPError( + method='POST', url='test', response=mock_response_off) + + # Simulate HTTP 409 Conflict error with "already powered on" message + mock_response_on = mock.Mock(status_code=409) + mock_response_on.json.return_value = { + 'error': { + 'message': 'Server is already powered ON.', + } + } + error_409_on = sushy.exceptions.HTTPError( + method='POST', url='test', response=mock_response_on) + + # Test scenarios where node is already in target state (should succeed) + success_scenarios = [ + ( + states.POWER_OFF, + sushy.SYSTEM_POWER_STATE_OFF, + error_409_off, + sushy.RESET_FORCE_OFF, + ), + ( + states.SOFT_POWER_OFF, + sushy.SYSTEM_POWER_STATE_OFF, + error_409_off, + sushy.RESET_GRACEFUL_SHUTDOWN, + ), + ( + states.POWER_ON, + sushy.SYSTEM_POWER_STATE_ON, + error_409_on, + sushy.RESET_ON, + ), + ] + + for (target_state, power_state, error, + expected_reset) in success_scenarios: + fake_system.reset_system.side_effect = error + fake_system.power_state = power_state + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # Should succeed without raising exception + task.driver.power.set_power_state(task, target_state) + fake_system.reset_system.assert_called_with(expected_reset) + # Verify that refresh() was called to get current state + fake_system.refresh.assert_called_once() + + fake_system.reset_mock() + + # Test scenarios where node is NOT in target state (should fail) + failure_scenarios = [ + (states.POWER_OFF, sushy.SYSTEM_POWER_STATE_ON, error_409_off), + (states.POWER_ON, sushy.SYSTEM_POWER_STATE_OFF, error_409_on), + ] + + for target_state, power_state, error in failure_scenarios: + fake_system.reset_system.side_effect = error + fake_system.power_state = power_state + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # Should raise HTTPError since state doesn't match + self.assertRaises( + sushy.exceptions.HTTPError, + task.driver.power.set_power_state, task, target_state) + + fake_system.reset_mock() + + @mock.patch.object(redfish_power.LOG, 'warning', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_power_state_conflict_error_refresh_fails( + self, mock_get_system, mock_log): + """Test HTTP 409 handling when refresh() fails but error msg helps.""" + fake_system = mock_get_system.return_value + + # Simulate HTTP 409 Conflict error + mock_response = mock.Mock(status_code=409) + mock_response.json.return_value = { + 'error': { + 'message': 'Server is already powered OFF.', + } + } + error_409 = sushy.exceptions.HTTPError( + method='POST', url='test', response=mock_response) + + fake_system.reset_system.side_effect = error_409 + # Make refresh() raise an exception + fake_system.refresh.side_effect = sushy.exceptions.ConnectionError( + 'Connection failed') + # Set power_state to None to force fallback to error message + fake_system.power_state = None + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # Should succeed by falling back to error message parsing + task.driver.power.set_power_state(task, states.POWER_OFF) + fake_system.reset_system.assert_called_with( + sushy.RESET_FORCE_OFF) + # Verify that refresh() was attempted + fake_system.refresh.assert_called_once() + # Verify that warning was logged about refresh failure + self.assertTrue(mock_log.called) + log_msg = mock_log.call_args[0][0] + self.assertIn('Failed to refresh system state', log_msg) + @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/test_agent_base.py ironic-29.0.5/ironic/tests/unit/drivers/modules/test_agent_base.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/test_agent_base.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/test_agent_base.py 2026-04-30 14:30:48.000000000 +0000 @@ -1689,3 +1689,41 @@ response = agent_base.execute_step( task, self.clean_steps['deploy'][0], 'clean') self.assertEqual(states.CLEANWAIT, response) + + +class FreshlyBootedTestCase(db_base.DbTestCase): + + def setUp(self): + super(FreshlyBootedTestCase, self).setUp() + + def test__freshly_booted_empty_result(self): + commands = [] + self.assertTrue(agent_base._freshly_booted(commands, 'deploy')) + + def test__freshly_booted_single_command(self): + commands = [{'command_name': 'get_deploy_steps'}] + self.assertTrue(agent_base._freshly_booted(commands, 'deploy')) + + def test__freshly_booted_single_command_mismatch(self): + commands = [{'command_name': 'get_service_steps'}] + self.assertFalse(agent_base._freshly_booted(commands, 'deploy')) + + def test__freshly_booted_has_retries(self): + # NOTE(TheJulia): this is just an arbitrary number + # of retires to account for lossy/problematic networks + commands = [ + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}] + self.assertTrue(agent_base._freshly_booted(commands, 'deploy')) + + def test__freshly_booted_multi_command(self): + commands = [ + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_deploy_steps'}, + {'command_name': 'get_service_steps'}] + self.assertFalse(agent_base._freshly_booted(commands, 'deploy')) diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/test_console_utils.py ironic-29.0.5/ironic/tests/unit/drivers/modules/test_console_utils.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/test_console_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/test_console_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -313,6 +313,45 @@ stderr=subprocess.PIPE) mock_popen.return_value.poll.assert_called_once_with() + @mock.patch.object(console_utils, 'open', + mock.mock_open(read_data='12345\n')) + @mock.patch.object(os.path, 'exists', autospec=True) + @mock.patch.object(subprocess, 'Popen', autospec=True) + @mock.patch.object(psutil, 'pid_exists', autospec=True) + @mock.patch.object(console_utils, '_get_console_pid_file', autospec=True) + @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', + autospec=True) + @mock.patch.object(console_utils, '_stop_console', autospec=True) + @mock.patch.object(loopingcall.FixedIntervalLoopingCall, 'start', + autospec=True) + def test_start_shellinabox_console_with_env_variables( + self, mock_timer_start, mock_stop, mock_dir_exists, + mock_get_pid, mock_pid_exists, mock_popen, mock_path_exists): + """Test start_shellinabox_console passes env_variables to Popen.""" + mock_timer_start.return_value = mock.Mock() + mock_get_pid.return_value = '/tmp/%s.pid' % self.info['uuid'] + mock_popen.return_value.poll.return_value = 0 + mock_popen.return_value.stdout = self.mock_stdout + self.mock_stdout.seek(0) + mock_popen.return_value.stderr = self.mock_stderr + self.mock_stderr.seek(0) + mock_pid_exists.return_value = True + mock_path_exists.return_value = True + + console_utils.start_shellinabox_console( + self.info['uuid'], self.info['port'], 'ls&', + env_variables={'SOME_VAR': 'value'}) + + mock_popen.assert_called_once() + call_kwargs = mock_popen.call_args[1] + self.assertIn('env', call_kwargs) + env = call_kwargs['env'] + self.assertEqual(env.get('SOME_VAR'), 'value') + # Should be a copy of os.environ plus our var + for key, value in os.environ.items(): + self.assertEqual(env.get(key), value, + 'env should contain os.environ') + @mock.patch.object(fcntl, 'fcntl', autospec=True) @mock.patch.object(console_utils, 'open', mock.mock_open(read_data='12345\n')) @@ -534,13 +573,14 @@ autospec=True) def _test_start_socat_console_check_arg(self, mock_timer_start, mock_stop, mock_dir_exists, - mock_get_pid, mock_popen): + mock_get_pid, mock_popen, + console_cmd='ls&'): mock_timer_start.return_value = mock.Mock() mock_get_pid.return_value = '/tmp/%s.pid' % self.info['uuid'] console_utils.start_socat_console(self.info['uuid'], self.info['port'], - 'ls&') + console_cmd) mock_stop.assert_called_once_with(self.info['uuid']) mock_dir_exists.assert_called_once_with() @@ -549,6 +589,12 @@ mock_popen.assert_called_once_with(mock.ANY, stderr=subprocess.PIPE) return mock_popen.call_args[0][0] + def test_escape_start_socat_console_command(self): + command = ";cat /etc/passwd; && echo it\'s tricky" + quoted_command = ';cat /etc/passwd; && echo it\'"\'"\'s tricky' + args = self._test_start_socat_console_check_arg(console_cmd=command) + self.assertIn(quoted_command, args[-1]) + def test_start_socat_console_check_arg_default_timeout(self): args = self._test_start_socat_console_check_arg() self.assertIn('-T600', args) @@ -609,6 +655,38 @@ @mock.patch.object(os.path, 'exists', autospec=True) @mock.patch.object(subprocess, 'Popen', autospec=True) + @mock.patch.object(psutil, 'pid_exists', autospec=True) + @mock.patch.object(console_utils, '_get_console_pid', autospec=True) + @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', + autospec=True) + @mock.patch.object(console_utils, '_stop_console', autospec=True) + def test_start_socat_console_with_env_variables( + self, mock_stop, mock_dir_exists, mock_get_pid, + mock_pid_exists, mock_popen, mock_path_exists): + """Test start_socat_console passes env_variables to Popen.""" + mock_get_pid.return_value = 23456 + mock_popen.return_value.pid = 23456 + mock_popen.return_value.poll.return_value = None + mock_popen.return_value.communicate.return_value = (None, None) + mock_pid_exists.return_value = True + mock_path_exists.return_value = True + + console_utils.start_socat_console( + self.info['uuid'], self.info['port'], 'ls&', + env_variables={'SOME_VAR': 'value'}) + + mock_popen.assert_called_once() + call_kwargs = mock_popen.call_args[1] + self.assertIn('env', call_kwargs) + env = call_kwargs['env'] + self.assertEqual(env.get('SOME_VAR'), 'value') + # Should be a copy of os.environ plus our var + for key, value in os.environ.items(): + self.assertEqual(env.get(key), value, + 'env should contain os.environ') + + @mock.patch.object(os.path, 'exists', autospec=True) + @mock.patch.object(subprocess, 'Popen', autospec=True) @mock.patch.object(psutil, 'pid_exists', autospec=True) @mock.patch.object(console_utils, '_get_console_pid', autospec=True) @mock.patch.object(console_utils, '_ensure_console_pid_dir_exists', diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/test_deploy_utils.py ironic-29.0.5/ironic/tests/unit/drivers/modules/test_deploy_utils.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/test_deploy_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/test_deploy_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -1120,15 +1120,20 @@ @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' 'remove_servicing_network', autospec=True) + @mock.patch('ironic.common.neutron.update_neutron_port', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_tear_down_inband_service( - self, power_mock, remove_service_network_mock, + self, power_mock, update_neutron_port_mock, + remove_service_network_mock, clean_up_ramdisk_mock, prepare_instance_mock): # NOTE(TheJulia): This should be back to servicing upon a heartbeat # operation, before we go back to WAIT. We wouldn't know to teardown # in a wait state anyway. self.node.provision_state = states.SERVICING self.node.save() + + self.ports[0].internal_info = {'tenant_vif_port_id': 'fake-vif-id'} + self.ports[0].save() with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: utils.tear_down_inband_service(task) @@ -1163,9 +1168,11 @@ @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' 'remove_servicing_network', autospec=True) + @mock.patch('ironic.common.neutron.update_neutron_port', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_tear_down_inband_service_disable_power_off( - self, power_mock, remove_service_network_mock, + self, power_mock, update_neutron_port_mock, + remove_service_network_mock, clean_up_ramdisk_mock, prepare_instance_mock): # NOTE(TheJulia): This should be back to servicing upon a heartbeat # operation, before we go back to WAIT. We wouldn't know to teardown @@ -1173,6 +1180,9 @@ self.node.provision_state = states.SERVICING self.node.disable_power_off = True self.node.save() + + self.ports[0].internal_info = {'tenant_vif_port_id': 'fake-vif-id'} + self.ports[0].save() with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: utils.tear_down_inband_service(task) diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/test_image_cache.py ironic-29.0.5/ironic/tests/unit/drivers/modules/test_image_cache.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/test_image_cache.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/test_image_cache.py 2026-04-30 14:30:48.000000000 +0000 @@ -18,6 +18,7 @@ import datetime import os +import shutil import tempfile import time from unittest import mock @@ -238,6 +239,34 @@ mock_clean_up.assert_not_called() mock_image_service.assert_not_called() + @mock.patch.object(shutil, 'copyfile', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(image_cache, '_delete_dest_path_if_stale', + return_value=False, autospec=True) + @mock.patch.object(image_cache, '_delete_master_path_if_stale', + return_value=True, autospec=True) + def test_fetch_image_hardlink_fails_fallback_to_copy( + self, mock_cache_upd, mock_dest_upd, mock_link, mock_copyfile, + mock_download, mock_clean_up, mock_image_service): + mock_link.side_effect = OSError("Invalid cross-device link") + + self.cache.fetch_image(self.uuid, self.dest_path) + + mock_link.assert_called_once_with(self.master_path, self.dest_path) + + mock_copyfile.assert_called_once_with(self.master_path, self.dest_path) + + mock_cache_upd.assert_called_once_with( + self.master_path, self.uuid, + mock_image_service.return_value.show.return_value) + mock_dest_upd.assert_called_once_with(self.master_path, self.dest_path) + + mock_download.assert_not_called() + mock_clean_up.assert_not_called() + + mock_image_service.assert_called_once_with(self.uuid, context=None) + mock_image_service.return_value.show.assert_called_once_with(self.uuid) + @mock.patch.object(image_cache, '_fetch', autospec=True) class TestImageCacheDownload(BaseTest): diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/test_inspect_utils.py ironic-29.0.5/ironic/tests/unit/drivers/modules/test_inspect_utils.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/test_inspect_utils.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/test_inspect_utils.py 2026-04-30 14:30:48.000000000 +0000 @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import json import socket from unittest import mock @@ -255,9 +256,11 @@ swift_obj_mock = swift_api_mock.return_value object_name = 'inspector_data-' + str(self.node.uuid) swift_obj_mock.create_object_from_data.assert_has_calls([ - mock.call(object_name + '-inventory', self.fake_inventory_data, + mock.call(object_name + '-inventory', + json.dumps(self.fake_inventory_data), container), - mock.call(object_name + '-plugin', self.fake_plugin_data, + mock.call(object_name + '-plugin', + json.dumps(self.fake_plugin_data), container)]) @mock.patch.object(swift, 'SwiftAPI', autospec=True) diff -Nru ironic-29.0.0/ironic/tests/unit/drivers/modules/test_ipmitool.py ironic-29.0.5/ironic/tests/unit/drivers/modules/test_ipmitool.py --- ironic-29.0.0/ironic/tests/unit/drivers/modules/test_ipmitool.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/drivers/modules/test_ipmitool.py 2026-04-30 14:30:48.000000000 +0000 @@ -4154,11 +4154,13 @@ mock_exec.assert_called_once_with( driver_info, 'sol deactivate', check_exit_code=[0, 1]) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(ipmi.IPMIConsole, '_get_ipmi_cmd', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test__start_console(self, mock_start, mock_ipmi_cmd): + def test__start_console(self, mock_start, mock_ipmi_cmd, mock_persist): mock_start.return_value = None + mock_persist.return_value = ('-f', '/tmp/console.pw') with task_manager.acquire(self.context, self.node.uuid) as task: @@ -4166,17 +4168,21 @@ self.console._start_console( driver_info, console_utils.start_shellinabox_console) + mock_persist.assert_called_once_with(driver_info) mock_start.assert_called_once_with(self.info['uuid'], self.info['port'], mock.ANY) - mock_ipmi_cmd.assert_called_once_with(self.console, - driver_info, mock.ANY) + mock_ipmi_cmd.assert_called_once_with( + self.console, driver_info, pw_file='/tmp/console.pw', + use_pwd_env=False) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test__start_console_fail(self, mock_start): + def test__start_console_fail(self, mock_start, mock_persist): mock_start.side_effect = exception.ConsoleSubprocessFailed( error='error') + mock_persist.return_value = ('-f', '/tmp/console.pw') with task_manager.acquire(self.context, self.node.uuid) as task: @@ -4186,10 +4192,12 @@ driver_info, console_utils.start_shellinabox_console) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test__start_console_fail_nodir(self, mock_start): + def test__start_console_fail_nodir(self, mock_start, mock_persist): mock_start.side_effect = exception.ConsoleError() + mock_persist.return_value = ('-f', '/tmp/console.pw') with task_manager.acquire(self.context, self.node.uuid) as task: @@ -4200,9 +4208,11 @@ console_utils.start_shellinabox_console) mock_start.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) - def test__start_console_empty_password(self, mock_start): + def test__start_console_empty_password(self, mock_start, mock_persist): + mock_persist.return_value = ('-f', '/tmp/console.pw') driver_info = self.node.driver_info del driver_info['ipmi_password'] self.node.driver_info = driver_info @@ -4218,6 +4228,31 @@ self.info['port'], mock.ANY) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) + @mock.patch.object(ipmi.IPMIConsole, '_get_ipmi_cmd', autospec=True) + @mock.patch.object(console_utils, 'start_shellinabox_console', + autospec=True) + def test__start_console_store_cred_in_env(self, mock_start, mock_ipmi_cmd, + mock_persist): + """Test _start_console with store_cred_in_env=True uses env.""" + mock_start.return_value = None + mock_persist.return_value = ('-E', {'IPMI_PASSWORD': 'secret'}) + + self.config(store_cred_in_env=True, group='ipmi') + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.console._start_console( + driver_info, console_utils.start_shellinabox_console) + + mock_persist.assert_called_once_with(driver_info) + mock_ipmi_cmd.assert_called_once_with( + self.console, driver_info, pw_file=None, + use_pwd_env=True) + mock_start.assert_called_once_with( + self.info['uuid'], self.info['port'], mock.ANY, + env_variables={'IPMI_PASSWORD': 'secret'}) + @mock.patch.object(ipmi, '_release_allocated_port', autospec=True) @mock.patch.object(console_utils, 'stop_shellinabox_console', autospec=True) @@ -4363,12 +4398,14 @@ mock_alloc.assert_called_once_with(mock.ANY, host='2001:dead:beef::1') @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(ipmi.IPMISocatConsole, '_get_ipmi_cmd', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console(self, mock_start, mock_ipmi_cmd, mock_exec): + def test__start_console(self, mock_start, mock_ipmi_cmd, mock_persist, + mock_exec): mock_start.return_value = None - + mock_persist.return_value = ('-f', '/tmp/console.pw') mock_exec.return_value = ('output', 'error') with task_manager.acquire(self.context, @@ -4377,19 +4414,21 @@ self.console._start_console( driver_info, console_utils.start_socat_console) + mock_persist.assert_called_once_with(driver_info) mock_start.assert_called_once_with(self.info['uuid'], self.info['port'], mock.ANY) - mock_ipmi_cmd.assert_called_once_with(self.console, - driver_info, mock.ANY) + mock_ipmi_cmd.assert_called_once_with( + self.console, driver_info, pw_file='/tmp/console.pw') @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console_fail(self, mock_start, mock_exec): + def test__start_console_fail(self, mock_start, mock_persist, mock_exec): mock_start.side_effect = exception.ConsoleSubprocessFailed( error='error') - + mock_persist.return_value = ('-f', '/tmp/console.pw') mock_exec.return_value = ('output', 'error') with task_manager.acquire(self.context, @@ -4405,11 +4444,13 @@ mock.ANY) @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console_fail_nodir(self, mock_start, mock_exec): + def test__start_console_fail_nodir(self, mock_start, mock_persist, + mock_exec): mock_start.side_effect = exception.ConsoleError() - + mock_persist.return_value = ('-f', '/tmp/console.pw') mock_exec.return_value = ('output', 'error') with task_manager.acquire(self.context, @@ -4422,9 +4463,12 @@ mock_start.assert_called_once_with(self.node.uuid, mock.ANY, mock.ANY) @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) @mock.patch.object(console_utils, 'start_socat_console', autospec=True) - def test__start_console_empty_password(self, mock_start, mock_exec): + def test__start_console_empty_password(self, mock_start, mock_persist, + mock_exec): + mock_persist.return_value = ('-f', '/tmp/console.pw') driver_info = self.node.driver_info del driver_info['ipmi_password'] self.node.driver_info = driver_info @@ -4442,6 +4486,33 @@ self.info['port'], mock.ANY) + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(ipmi, '_persist_ipmi_password', autospec=True) + @mock.patch.object(ipmi.IPMISocatConsole, '_get_ipmi_cmd', autospec=True) + @mock.patch.object(console_utils, 'start_socat_console', + autospec=True) + def test__start_console_store_cred_in_env(self, mock_start, mock_ipmi_cmd, + mock_persist, mock_exec): + """Test _start_console with store_cred_in_env=True uses env.""" + mock_start.return_value = None + mock_persist.return_value = ('-E', {'IPMI_PASSWORD': 'secret'}) + mock_exec.return_value = ('output', 'error') + + self.config(store_cred_in_env=True, group='ipmi') + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.console._start_console( + driver_info, console_utils.start_socat_console) + + mock_persist.assert_called_once_with(driver_info) + mock_ipmi_cmd.assert_called_once_with( + self.console, driver_info, pw_file=None, + use_pwd_env=True) + mock_start.assert_called_once_with( + self.info['uuid'], self.info['port'], mock.ANY, + env_variables={'IPMI_PASSWORD': 'secret'}) + @mock.patch.object(ipmi, '_release_allocated_port', autospec=True) @mock.patch.object(ipmi.IPMISocatConsole, '_exec_stop_console', autospec=True) diff -Nru ironic-29.0.0/ironic/tests/unit/stubs.py ironic-29.0.5/ironic/tests/unit/stubs.py --- ironic-29.0.0/ironic/tests/unit/stubs.py 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/ironic/tests/unit/stubs.py 2026-04-30 14:30:48.000000000 +0000 @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import io from openstack.connection import exceptions as openstack_exc @@ -38,7 +37,7 @@ def download_image(self, image_id, stream=False): self.get_image(image_id) if stream: - return io.BytesIO(self.image_data) + return FakeStreamingResponse(self.image_data) else: return FakeImageDownload(self.image_data) @@ -51,6 +50,21 @@ self.content = content +class FakeStreamingResponse(object): + """Mimics a requests.Response object for streaming downloads.""" + + def __init__(self, content): + self._content = content + self._pos = 0 + + def iter_content(self, chunk_size=1): + """Yield chunks of content mimicking requests.Response.iter_content.""" + while self._pos < len(self._content): + chunk = self._content[self._pos:self._pos + chunk_size] + self._pos += chunk_size + yield chunk + + class FakeNeutronPort(dict): def __init__(self, **attrs): PORT_ATTRS = ['admin_state_up', diff -Nru ironic-29.0.0/releasenotes/notes/add_allow_image_access_via_auth_token-1b5869f1c0999bea.yaml ironic-29.0.5/releasenotes/notes/add_allow_image_access_via_auth_token-1b5869f1c0999bea.yaml --- ironic-29.0.0/releasenotes/notes/add_allow_image_access_via_auth_token-1b5869f1c0999bea.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/add_allow_image_access_via_auth_token-1b5869f1c0999bea.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,19 @@ +--- +features: + - | + If `allow_image_access_via_auth_token` is set to `True`, Ironic allows access to + Glance images if an auth_token is present in the request context. +upgrade: + - | + In this release, the default value of `allow_image_access_via_auth_token` has been + changed to `True`. This enables Ironic to access Glance images when an auth token + is present in the request context. + + This change was backported from the master branch but with a modified default value + for this stable release. OpenStack integrated operators should ensure images for + Ironic use are using image visibility "public" or "community" for the most reliable + results. +deprecation: + - | + CONF.allow_image_access_via_auth_token is deprecated, and will be removed, + along with legacy image access logic, in or after the OpenStack 2026.2 release. \ No newline at end of file diff -Nru ironic-29.0.0/releasenotes/notes/agent-inspect-hooks-cleanup-error-c8901a7f8ad0dfd3.yaml ironic-29.0.5/releasenotes/notes/agent-inspect-hooks-cleanup-error-c8901a7f8ad0dfd3.yaml --- ironic-29.0.0/releasenotes/notes/agent-inspect-hooks-cleanup-error-c8901a7f8ad0dfd3.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/agent-inspect-hooks-cleanup-error-c8901a7f8ad0dfd3.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`Bug 2135265 `_] + Fixes an issue where an exception in one of the inspection hooks when + using the agent inspector interface would result in the node not being + cleaned up. diff -Nru ironic-29.0.0/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml ironic-29.0.5/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml --- ironic-29.0.0/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/bcrypt_cache-d78775ff02f2d970.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,7 @@ +--- +fixes: + - | + Performance of Basic HTTP authentication has been improved by keeping a + memory cache of bcrypt password checks. This improves the performance of + Ironic conductor with JSON-RPC, and API access when using Basic HTTP + authentication. \ No newline at end of file diff -Nru ironic-29.0.0/releasenotes/notes/bootloader-paths-creation-b5097003f25a18ad.yaml ironic-29.0.5/releasenotes/notes/bootloader-paths-creation-b5097003f25a18ad.yaml --- ironic-29.0.0/releasenotes/notes/bootloader-paths-creation-b5097003f25a18ad.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/bootloader-paths-creation-b5097003f25a18ad.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,5 @@ +--- +fixes: + - | + Ensure that the path to where we are copying the bootloader exists before + attempting to copy the bootloader in. diff -Nru ironic-29.0.0/releasenotes/notes/catch-redfish-409-0819174174245ade.yaml ironic-29.0.5/releasenotes/notes/catch-redfish-409-0819174174245ade.yaml --- ironic-29.0.0/releasenotes/notes/catch-redfish-409-0819174174245ade.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/catch-redfish-409-0819174174245ade.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixes a race condition where the Redfish power interface could fail with + HTTP 400 (BadRequest) or HTTP 409 (Conflict) errors during power + operations. These errors are now treated as success when the node is + already in the target power state (either on or off), preventing + deployment failures when power state changes complete after Ironic's + state verification times out. + Also refresh system state to get current power state from BMC + instead of using potentially stale cached data. diff -Nru ironic-29.0.0/releasenotes/notes/config-redfish-compatible-bmc-3c54a945a7aa2a7f.yaml ironic-29.0.5/releasenotes/notes/config-redfish-compatible-bmc-3c54a945a7aa2a7f.yaml --- ironic-29.0.0/releasenotes/notes/config-redfish-compatible-bmc-3c54a945a7aa2a7f.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/config-redfish-compatible-bmc-3c54a945a7aa2a7f.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,8 @@ +--- +other: + - | + Addition of an additional vendor variable for + ``VENDORS_REQUIRING_FULL_BOOT_REQUEST``. This is because there are + some recognised vendors, i.e. Lenovo, which use various BMC for + their hardware, some of which require a full boot request; in such + cases the ``vendor`` can now be changed to ``redfish_compatible``. diff -Nru ironic-29.0.0/releasenotes/notes/control-pxe-enabled-field-inspection-206f67c6638a0bdb.yaml ironic-29.0.5/releasenotes/notes/control-pxe-enabled-field-inspection-206f67c6638a0bdb.yaml --- ironic-29.0.0/releasenotes/notes/control-pxe-enabled-field-inspection-206f67c6638a0bdb.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/control-pxe-enabled-field-inspection-206f67c6638a0bdb.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,7 @@ +--- +fixes: + - | + In the redfish inspector, use condition to control whether the pxe_enabled field of a port is updated during inspection. + [inspector]update_pxe_enabled is used to control this so it behaves like all other inspection interfaces. + The default value for this configuration is True. + diff -Nru ironic-29.0.0/releasenotes/notes/escape-socat-console-string-arguments-555388ab8dcb8cc3.yaml ironic-29.0.5/releasenotes/notes/escape-socat-console-string-arguments-555388ab8dcb8cc3.yaml --- ironic-29.0.0/releasenotes/notes/escape-socat-console-string-arguments-555388ab8dcb8cc3.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/escape-socat-console-string-arguments-555388ab8dcb8cc3.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes an issue where console command passed to `socat`'s `EXEC:` was not + quoted which could have security implications. diff -Nru ironic-29.0.0/releasenotes/notes/fix-bug-2148317-471adcfac69791dc.yaml ironic-29.0.5/releasenotes/notes/fix-bug-2148317-471adcfac69791dc.yaml --- ironic-29.0.0/releasenotes/notes/fix-bug-2148317-471adcfac69791dc.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-bug-2148317-471adcfac69791dc.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,19 @@ +--- +fixes: + - | + Fixes a security issue where the deprecated configuration molds feature + would allow an user invoking molds to request authorization + to be sent to a remote endpoint. This user supplied URL could be a + ``swift`` or ``http`` url. While when used with ``http``, the feature + was explicitly designed around a concept of just publishing to a file + in a limited context with authentication details provided by the + conductor, where as with ``swift`` the impact is greater because the + time limited session token for Ironic's access of swift resources could + be leaked, captured, and used. + + The configuration molds feature now explicitly checks the swift endpoint + URL and raises an exception when the URL does not match the user supplied + the configured Swift endpoint. + + More information can be found in + `bug 2148317 `_. diff -Nru ironic-29.0.0/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml ironic-29.0.5/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml --- ironic-29.0.0/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-cache-hardlink-66a8b2302abde76d.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,6 @@ +--- +fixes: + - | + When caching an image between different file systems, the hard link + operation would fail. This is fixed by falling back to a copy + operation. diff -Nru ironic-29.0.0/releasenotes/notes/fix-loop-functionality-in-inspection-rules-9bf61e8355297804.yaml ironic-29.0.5/releasenotes/notes/fix-loop-functionality-in-inspection-rules-9bf61e8355297804.yaml --- ironic-29.0.0/releasenotes/notes/fix-loop-functionality-in-inspection-rules-9bf61e8355297804.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-loop-functionality-in-inspection-rules-9bf61e8355297804.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes loop functionality to align more closely with the spec where, + with `loop` present, `args` reference loop items using '{item}' + placeholder to support direct array iteration; plus, + separately handle list and dict loop item types. diff -Nru ironic-29.0.0/releasenotes/notes/fix-order-of-disable-ramdisk-validation-for-runbooks-e32617f1e9227e65.yaml ironic-29.0.5/releasenotes/notes/fix-order-of-disable-ramdisk-validation-for-runbooks-e32617f1e9227e65.yaml --- ironic-29.0.0/releasenotes/notes/fix-order-of-disable-ramdisk-validation-for-runbooks-e32617f1e9227e65.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-order-of-disable-ramdisk-validation-for-runbooks-e32617f1e9227e65.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where ``disable_ramdisk`` could bypass API validation when + supplied via a runbook because validation happened before runbook + resolution. diff -Nru ironic-29.0.0/releasenotes/notes/fix-redfish-async-updates-servicewait-e834ae30c5f72641.yaml ironic-29.0.5/releasenotes/notes/fix-redfish-async-updates-servicewait-e834ae30c5f72641.yaml --- ironic-29.0.0/releasenotes/notes/fix-redfish-async-updates-servicewait-e834ae30c5f72641.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-redfish-async-updates-servicewait-e834ae30c5f72641.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes an issue with the redfish firmware updates attached to the + ``management`` interface where firmware updates performed as part of the + SERVICE steps workflow could end up stuck in ``service wait``. + This issue has been corrected and the nodes should now properly have their + state checked. For more information, please consult + `bug 2136895 `_. diff -Nru ironic-29.0.0/releasenotes/notes/fix-redfish-boot-device-full-request-6ff0ee231ee6e663.yaml ironic-29.0.5/releasenotes/notes/fix-redfish-boot-device-full-request-6ff0ee231ee6e663.yaml --- ironic-29.0.0/releasenotes/notes/fix-redfish-boot-device-full-request-6ff0ee231ee6e663.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-redfish-boot-device-full-request-6ff0ee231ee6e663.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed an issue in the Redfish management driver where boot device setting + failed for vendors requiring full boot parameter requests (such as American + Megatrends International, ASRock Rack, and Redfish compatible systems). The + driver was incorrectly using a parameter that could be None, but + these vendors require explicit values for all boot parameters. This fix ensures + that vendors in ``VENDORS_REQUIRING_FULL_BOOT_REQUEST`` always receive + explicit boot configuration values, resolving boot device configuration + failures on affected systems. diff -Nru ironic-29.0.0/releasenotes/notes/fix-swift-for-inventory-c371da65dd20fc74.yaml ironic-29.0.5/releasenotes/notes/fix-swift-for-inventory-c371da65dd20fc74.yaml --- ironic-29.0.0/releasenotes/notes/fix-swift-for-inventory-c371da65dd20fc74.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fix-swift-for-inventory-c371da65dd20fc74.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes storage of inventory and plugin data in Swift. + Previously, the data has not been stored as JSON. + This meant that the data was stored in an odd format + with part of the fields missing. diff -Nru ironic-29.0.0/releasenotes/notes/fixes-inspection-rules-schema-validation-5cac6058d12ce030.yaml ironic-29.0.5/releasenotes/notes/fixes-inspection-rules-schema-validation-5cac6058d12ce030.yaml --- ironic-29.0.0/releasenotes/notes/fixes-inspection-rules-schema-validation-5cac6058d12ce030.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/fixes-inspection-rules-schema-validation-5cac6058d12ce030.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes schema validation by raising formatting and schema errors + early during inspection rule creation, updates and execution. + + - | + Adds support for standard comparison operators (`le`, `ge`, `ne`) + to extend inspection rules capabilities for common logical conditions. diff -Nru ironic-29.0.0/releasenotes/notes/flat-driver-rebind-no-vifs-192c9be8e6962d46.yaml ironic-29.0.5/releasenotes/notes/flat-driver-rebind-no-vifs-192c9be8e6962d46.yaml --- ironic-29.0.0/releasenotes/notes/flat-driver-rebind-no-vifs-192c9be8e6962d46.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/flat-driver-rebind-no-vifs-192c9be8e6962d46.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,6 @@ +--- +fixes: + - | + The flat network driver now raises ``NetworkError`` when rebind operations + fail to bind any VIFs, instead of silently continuing. This prevents + unexpected behavior when all port bindings fail during node rebind. diff -Nru ironic-29.0.0/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml ironic-29.0.5/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml --- ironic-29.0.0/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/handle-http-multiple-redirection-cfa2b4693e1db82f.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed HttpImageService.validate_href() ImageRefValidationFailed exception + if protocol is HTTP/HTTPS and the HTTP header response is a redirection + other then 301 (MOVED_PERMANENTLY). HTTP/HTTPS protocol is often used + under standalone Ironic configuration to identify an image source (e.g + --instance-info image_source=). The HTTP server may use redirection + to load balance or geographically distribute the requests, or simply point + to the correct URL. The redirection may vary from 301 (MOVED_PERMANENTLY), + to 302 (FOUND), or 307 (TEMPORARY_REDIRECT), and 308 (PERMANENT_REDIRECT). diff -Nru ironic-29.0.0/releasenotes/notes/image-download-performance-0bf1af5556c1adbf.yaml ironic-29.0.5/releasenotes/notes/image-download-performance-0bf1af5556c1adbf.yaml --- ironic-29.0.0/releasenotes/notes/image-download-performance-0bf1af5556c1adbf.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/image-download-performance-0bf1af5556c1adbf.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,9 @@ +fixes: + - | + Extremely large instance images, post eventlet removal, were causing CPU + spikes and the conductor process to hang while the image was being + fetched and validated. This optimizes the instance fetch and validation + in two ways -- first, we now calculate the image checksum while the + file is being fetched instead of as a separate step. Secondly, we were, + in some cases, using the default chunk size of 128 bytes during downloads. + Now, we use a more standard and reasonable value of 1 megabyte. diff -Nru ironic-29.0.0/releasenotes/notes/inspect-hook-local-link-connection-crash-394edb1c35354968.yaml ironic-29.0.5/releasenotes/notes/inspect-hook-local-link-connection-crash-394edb1c35354968.yaml --- ironic-29.0.0/releasenotes/notes/inspect-hook-local-link-connection-crash-394edb1c35354968.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/inspect-hook-local-link-connection-crash-394edb1c35354968.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,6 @@ +--- +fixes: + - | + Avoid an unhandled exception in the local_link_connection inspection hook + which would cause inspection to fail instead of skipping past missing ports + as originally intended. diff -Nru ironic-29.0.0/releasenotes/notes/oci-fixes-bbbcc633394252f6.yaml ironic-29.0.5/releasenotes/notes/oci-fixes-bbbcc633394252f6.yaml --- ironic-29.0.0/releasenotes/notes/oci-fixes-bbbcc633394252f6.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/oci-fixes-bbbcc633394252f6.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes deploying OCI artifacts uploaded by ORAS to Quay.io (and potentially + other registries) as a single manifest. diff -Nru ironic-29.0.0/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml ironic-29.0.5/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml --- ironic-29.0.0/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/ossa-2025-001-disallow-unsafe-image-paths-670fdcfe3e4647d4.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,29 @@ +--- +security: + - | + Fixes OSSA-2025-001, where Ironic did not properly filter file:// paths + when used as image sources. This would permit any file accessible by the + conductor to be used as an image to attempt deployment. + + Adds ``CONF.conductor.file_url_allowed_paths``, an allowlist configuration + defaulting to ``/var/lib/ironic``, ``/shared/html``, + ``/opt/cache/files``, ``/vagrant``, and ``/templates``, + permits operators to further restrict where the conductor will fetch + images for when provided a file:// URL. This default value was chosen + based on known usage by projects downstream of Ironic, including Metal3, + Bifrost, and OpenShift. These defaults may change to be more restrictive + at a later date. Operators using file:// URLs are encouraged to explicitly + set this value even if the current default is sufficient. Operators wishing + to fully disable the ability to deploy with a file:// URL should set this + configuration to "" (empty). + + Operators wishing to restore the original insecure behavior should set + ``CONF.conductor.file_url_allowed_paths`` to ``/``. Take note that in the + 2025.2 release and later, ``/dev``, ``/sys``, ``/proc``, ``/run``, and + ``/etc`` will be unconditionally blocked as a security measure. + + This issue only poses a significant security risk when Ironic's + automated cleaning process is disabled and the service is configured in + such a way that permits direct deployment by an untrusted API user, such as + standalone Ironic installations or environments granting ownership of nodes + to projects. diff -Nru ironic-29.0.0/releasenotes/notes/permit-retries-with-agent-startup-aebfc36a775794c3.yaml ironic-29.0.5/releasenotes/notes/permit-retries-with-agent-startup-aebfc36a775794c3.yaml --- ironic-29.0.0/releasenotes/notes/permit-retries-with-agent-startup-aebfc36a775794c3.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/permit-retries-with-agent-startup-aebfc36a775794c3.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixes an issue with agent startup where the workflow from the first + agent heartbeat interaction could fail due to a transient networking + issue leaving the Agent and Ironic in a state where the node cannot be + deployed and continues to record errors upon each additional heartbeat + operation. Logic to check the state of the agent has been adjusted to + ignore retry operations which were recorded by the agent. + More information on this issue can be found in + `bug 2110698 `_. diff -Nru ironic-29.0.0/releasenotes/notes/redfish-full-boot-payload-asrockrack-79a9291da8ec5a50.yaml ironic-29.0.5/releasenotes/notes/redfish-full-boot-payload-asrockrack-79a9291da8ec5a50.yaml --- ironic-29.0.0/releasenotes/notes/redfish-full-boot-payload-asrockrack-79a9291da8ec5a50.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/redfish-full-boot-payload-asrockrack-79a9291da8ec5a50.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,14 @@ +--- +fixes: + - | + Improved Redfish compatibility with ASRock Rack servers by updating how + boot mode and boot device settings are applied. + + Previously, calls to `set_boot_device` and `set_boot_mode` only included + minimal parameters (e.g., `BootOverrideTarget` and `BootOverrideMode`), which + were insufficient for certain vendor implementations like ASRockRack. + + This fix updates the Redfish driver to send the full payload, including + `BootSourceOverrideEnabled`, `BootSourceOverrideTarget`, `BootSourceOverrideMode`, + ensuring better compliance and reliability on these platforms. + diff -Nru ironic-29.0.0/releasenotes/notes/service-wait-unprovision-dacfa468824335b7.yaml ironic-29.0.5/releasenotes/notes/service-wait-unprovision-dacfa468824335b7.yaml --- ironic-29.0.0/releasenotes/notes/service-wait-unprovision-dacfa468824335b7.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/service-wait-unprovision-dacfa468824335b7.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,5 @@ +--- +features: + - | + Add support for a node in ``service wait`` state can be unprovisioned + via the ``delete`` provision action. diff -Nru ironic-29.0.0/releasenotes/notes/validate-interfaces-hook-49d7d6c57929a8cd.yaml ironic-29.0.5/releasenotes/notes/validate-interfaces-hook-49d7d6c57929a8cd.yaml --- ironic-29.0.0/releasenotes/notes/validate-interfaces-hook-49d7d6c57929a8cd.yaml 1970-01-01 00:00:00.000000000 +0000 +++ ironic-29.0.5/releasenotes/notes/validate-interfaces-hook-49d7d6c57929a8cd.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -0,0 +1,5 @@ +--- +fixes: + - | + Report a better error message than a KeyError when there are no interfaces + in the inspection data when running the validate-interfaces inspection hook. diff -Nru ironic-29.0.0/tox.ini ironic-29.0.5/tox.ini --- ironic-29.0.0/tox.ini 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/tox.ini 2026-04-30 14:30:48.000000000 +0000 @@ -16,7 +16,7 @@ OS_STDERR_CAPTURE={env:OS_STDERR_CAPTURE:true} PYTHONUNBUFFERED=1 deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2025.1} -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt commands = @@ -85,9 +85,10 @@ allowlist_externals = dot # NOTE(dtantsur): documentation building process requires importing ironic deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2025.1} -r{toxinidir}/requirements.txt -r{toxinidir}/doc/requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = sphinx-build -b html doc/source doc/build/html [testenv:pdf-docs] @@ -103,9 +104,10 @@ # NOTE(Mahnoor): documentation building process requires importing ironic API modules usedevelop = False deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2025.1} -r{toxinidir}/requirements.txt -r{toxinidir}/doc/requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT allowlist_externals = bash commands = bash -c 'rm -rf api-ref/build' @@ -114,8 +116,9 @@ [testenv:releasenotes] usedevelop = False deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2025.1} -r{toxinidir}/doc/requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html @@ -126,9 +129,10 @@ # environment. allowlist_externals = * deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2025.1} -r{toxinidir}/test-requirements.txt -r{toxinidir}/doc/requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = {posargs} [testenv:local-ironic-dev] @@ -140,7 +144,7 @@ PYTHONUNBUFFERED=1 SQLALCHEMY_WARN_20=true deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2025.1} -r{toxinidir}/requirements.txt -r{toxinidir}/driver-requirements.txt python-ironicclient diff -Nru ironic-29.0.0/zuul.d/ironic-jobs.yaml ironic-29.0.5/zuul.d/ironic-jobs.yaml --- ironic-29.0.0/zuul.d/ironic-jobs.yaml 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/zuul.d/ironic-jobs.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -50,7 +50,7 @@ IRONIC_TEMPEST_WHOLE_DISK_IMAGE: False IRONIC_VM_COUNT: 2 IRONIC_VM_EPHEMERAL_DISK: 1 - IRONIC_VM_SPECS_RAM: 2600 + IRONIC_VM_SPECS_RAM: 2750 IRONIC_VM_LOG_DIR: '{{ devstack_base_dir }}/ironic-bm-logs' # NOTE(dtantsur): in some jobs we end up with 12 disks total, so reduce # each of them. For don't need all 10 GiB for CirrOS anyway. @@ -307,7 +307,7 @@ IRONIC_ENABLED_DEPLOY_INTERFACES: "anaconda" IRONIC_VM_COUNT: 2 IRONIC_VM_VOLUME_COUNT: 1 - IRONIC_VM_SPECS_RAM: 3192 + IRONIC_VM_SPECS_RAM: 4096 IRONIC_VM_SPECS_CPU: 3 IRONIC_ENFORCE_SCOPE: True # We're using a lot of disk space in this job. Some testing nodes have diff -Nru ironic-29.0.0/zuul.d/project.yaml ironic-29.0.5/zuul.d/project.yaml --- ironic-29.0.0/zuul.d/project.yaml 2025-03-19 21:03:23.000000000 +0000 +++ ironic-29.0.5/zuul.d/project.yaml 2026-04-30 14:30:48.000000000 +0000 @@ -52,7 +52,7 @@ # NOTE(TheJulia): At present, metal3 doesn't leverage # stable branches, and as far as we are aware these jobs # can be removed once this branch is made stable. - - metal3-integration + # - metal3-integration # Non-voting jobs - ironic-inspector-tempest: voting: false @@ -95,7 +95,7 @@ # NOTE(TheJulia): At present, metal3 doesn't leverage # stable branches, and as far as we are aware these jobs # can be removed once this branch is made stable. - - metal3-integration + # - metal3-integration experimental: jobs: # NOTE(dtantsur): this job is rarely used, no need to run it always.