From 768439fe71d38fbe8944ae4e11799daddd980c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mina=20Gali=C4=87?= Date: Sat, 2 Dec 2023 00:20:21 +0000 Subject: [PATCH] dmi: Add support for OpenBSD (#4654) OpenBSD has the most important DMI data in sysctl's hw hierarchy. Extend dmi to read those, instead of falling back to dmidecode, since that requires setting kern.allowkmem=1 Extend FreeBSD support to include Dragonfly. Fix and add Unittests. Sponsored by: The FreeBSD Foundation --- cloudinit/dmi.py | 89 ++++++++++++++++++++++++++----------- tests/unittests/test_dmi.py | 37 ++++++++++++--- 2 files changed, 95 insertions(+), 31 deletions(-) diff --git a/cloudinit/dmi.py b/cloudinit/dmi.py index 3b8cca26c2ca..453e8510ab47 100644 --- a/cloudinit/dmi.py +++ b/cloudinit/dmi.py @@ -6,57 +6,74 @@ from typing import Optional from cloudinit import subp -from cloudinit.util import is_container, is_FreeBSD +from cloudinit.util import ( + is_container, + is_DragonFlyBSD, + is_FreeBSD, + is_OpenBSD, +) LOG = logging.getLogger(__name__) # Path for DMI Data DMI_SYS_PATH = "/sys/class/dmi/id" -KernelNames = namedtuple("KernelNames", ["linux", "freebsd"]) -KernelNames.__new__.__defaults__ = (None, None) +KernelNames = namedtuple("KernelNames", ["linux", "freebsd", "openbsd"]) +KernelNames.__new__.__defaults__ = (None, None, None) # FreeBSD's kenv(1) and Linux /sys/class/dmi/id/* both use different names from # dmidecode. The values are the same, and ultimately what we're interested in. +# Likewise, OpenBSD has the most commonly used things we need in sysctl(8)'s +# hw hierarchy. Moreover, this means it can be run without kern.allowkmem=1. # These tools offer a "cheaper" way to access those values over dmidecode. # This is our canonical translation table. If we add more tools on other # platforms to find dmidecode's values, their keys need to be put in here. DMIDECODE_TO_KERNEL = { - "baseboard-asset-tag": KernelNames("board_asset_tag", "smbios.planar.tag"), + "baseboard-asset-tag": KernelNames( + "board_asset_tag", "smbios.planar.tag", None + ), "baseboard-manufacturer": KernelNames( - "board_vendor", "smbios.planar.maker" + "board_vendor", "smbios.planar.maker", None ), "baseboard-product-name": KernelNames( - "board_name", "smbios.planar.product" + "board_name", "smbios.planar.product", None ), "baseboard-serial-number": KernelNames( - "board_serial", "smbios.planar.serial" + "board_serial", "smbios.planar.serial", None + ), + "baseboard-version": KernelNames( + "board_version", "smbios.planar.version", None ), - "baseboard-version": KernelNames("board_version", "smbios.planar.version"), - "bios-release-date": KernelNames("bios_date", "smbios.bios.reldate"), - "bios-vendor": KernelNames("bios_vendor", "smbios.bios.vendor"), - "bios-version": KernelNames("bios_version", "smbios.bios.version"), + "bios-release-date": KernelNames("bios_date", "smbios.bios.reldate", None), + "bios-vendor": KernelNames("bios_vendor", "smbios.bios.vendor", None), + "bios-version": KernelNames("bios_version", "smbios.bios.version", None), "chassis-asset-tag": KernelNames( - "chassis_asset_tag", "smbios.chassis.tag" + "chassis_asset_tag", "smbios.chassis.tag", None ), "chassis-manufacturer": KernelNames( - "chassis_vendor", "smbios.chassis.maker" + "chassis_vendor", "smbios.chassis.maker", "hw.vendor" ), "chassis-serial-number": KernelNames( - "chassis_serial", "smbios.chassis.serial" + "chassis_serial", "smbios.chassis.serial", "hw.uuid" ), "chassis-version": KernelNames( - "chassis_version", "smbios.chassis.version" + "chassis_version", "smbios.chassis.version", None + ), + "system-manufacturer": KernelNames( + "sys_vendor", "smbios.system.maker", "hw.vendor" ), - "system-manufacturer": KernelNames("sys_vendor", "smbios.system.maker"), "system-product-name": KernelNames( - "product_name", "smbios.system.product" + "product_name", "smbios.system.product", "hw.product" ), "system-serial-number": KernelNames( - "product_serial", "smbios.system.serial" + "product_serial", "smbios.system.serial", "hw.uuid" + ), + "system-uuid": KernelNames( + "product_uuid", "smbios.system.uuid", "hw.uuid" + ), + "system-version": KernelNames( + "product_version", "smbios.system.version", None ), - "system-uuid": KernelNames("product_uuid", "smbios.system.uuid"), - "system-version": KernelNames("product_version", "smbios.system.version"), } @@ -110,8 +127,7 @@ def _read_kenv(key: str) -> Optional[str]: try: cmd = ["kenv", "-q", kmap.freebsd] - (result, _err) = subp.subp(cmd) - result = result.strip() + result = subp.subp(cmd).stdout.strip() LOG.debug("kenv returned '%s' for '%s'", result, kmap.freebsd) return result except subp.ProcessExecutionError as e: @@ -120,6 +136,27 @@ def _read_kenv(key: str) -> Optional[str]: return None +def _read_sysctl(key: str) -> Optional[str]: + """ + Reads dmi data from OpenBSD's sysctl(8) + """ + kmap = DMIDECODE_TO_KERNEL.get(key) + if kmap is None or kmap.openbsd is None: + return None + + LOG.debug("querying dmi data %s", kmap.openbsd) + + try: + cmd = ["sysctl", "-qn", kmap.openbsd] + result = subp.subp(cmd).stdout.strip() + LOG.debug("sysctl returned '%s' for '%s'", result, kmap.openbsd) + return result + except subp.ProcessExecutionError as e: + LOG.debug("failed sysctl cmd: %s\n%s", cmd, e) + + return None + + def _call_dmidecode(key: str, dmidecode_path: str) -> Optional[str]: """ Calls out to dmidecode to get the data out. This is mostly for supporting @@ -127,8 +164,7 @@ def _call_dmidecode(key: str, dmidecode_path: str) -> Optional[str]: """ try: cmd = [dmidecode_path, "--string", key] - (result, _err) = subp.subp(cmd) - result = result.strip() + result = subp.subp(cmd).stdout.strip() LOG.debug("dmidecode returned '%s' for '%s'", result, key) if result.replace(".", "") == "": return "" @@ -159,9 +195,12 @@ def read_dmi_data(key: str) -> Optional[str]: if is_container(): return None - if is_FreeBSD(): + if is_FreeBSD() or is_DragonFlyBSD(): return _read_kenv(key) + if is_OpenBSD(): + return _read_sysctl(key) + syspath_value = _read_dmi_syspath(key) if syspath_value is not None: return syspath_value diff --git a/tests/unittests/test_dmi.py b/tests/unittests/test_dmi.py index 698e3df86f67..649706bb20fc 100644 --- a/tests/unittests/test_dmi.py +++ b/tests/unittests/test_dmi.py @@ -22,6 +22,10 @@ def setUp(self): self.addCleanup(p.stop) self._m_is_FreeBSD = p.start() + p = mock.patch("cloudinit.dmi.is_OpenBSD", return_value=False) + self.addCleanup(p.stop) + self._m_is_OpenBSD = p.start() + def _create_sysfs_parent_directory(self): util.ensure_dir(os.path.join("sys", "class", "dmi", "id")) @@ -40,7 +44,7 @@ def _configure_dmidecode_return(self, key, content, error=None): def _dmidecode_subp(cmd): if cmd[-1] != key: raise subp.ProcessExecutionError() - return (content, error) + return content, error self.patched_funcs.enter_context( mock.patch("cloudinit.dmi.subp.which", side_effect=lambda _: True) @@ -58,12 +62,27 @@ def _configure_kenv_return(self, key, content, error=None): def _kenv_subp(cmd): if cmd[-1] != dmi.DMIDECODE_TO_KERNEL[key].freebsd: raise subp.ProcessExecutionError() - return (content, error) + return content, error self.patched_funcs.enter_context( mock.patch("cloudinit.dmi.subp.subp", side_effect=_kenv_subp) ) + def _configure_sysctl_return(self, key, content, error=None): + """ + In order to test an OpenBSD system call outs to sysctl, this + function fakes the results of kenv to test the results. + """ + + def _sysctl_subp(cmd): + if cmd[-1] != dmi.DMIDECODE_TO_KERNEL[key].openbsd: + raise subp.ProcessExecutionError() + return content, error + + self.patched_funcs.enter_context( + mock.patch("cloudinit.dmi.subp.subp", side_effect=_sysctl_subp) + ) + def patch_mapping(self, new_mapping): self.patched_funcs.enter_context( mock.patch("cloudinit.dmi.DMIDECODE_TO_KERNEL", new_mapping) @@ -71,7 +90,7 @@ def patch_mapping(self, new_mapping): def test_sysfs_used_with_key_in_mapping_and_file_on_disk(self): self.patch_mapping( - {"mapped-key": dmi.KernelNames("mapped-value", None)} + {"mapped-key": dmi.KernelNames("mapped-value", None, None)} ) expected_dmi_value = "sys-used-correctly" self._create_sysfs_file("mapped-value", expected_dmi_value) @@ -149,7 +168,7 @@ def test_container_returns_none(self): # first verify we get the value if not in container self._m_is_container.return_value = False - key, val = ("system-product-name", "my_product") + key, val = "system-product-name", "my_product" self._create_sysfs_file("product_name", val) self.assertEqual(val, dmi.read_dmi_data(key)) @@ -167,13 +186,19 @@ def test_container_returns_none_on_unknown(self): def test_freebsd_uses_kenv(self): """On a FreeBSD system, kenv is called.""" self._m_is_FreeBSD.return_value = True - key, val = ("system-product-name", "my_product") + key, val = "system-product-name", "my_product" self._configure_kenv_return(key, val) self.assertEqual(dmi.read_dmi_data(key), val) + def test_openbsd_uses_kenv(self): + """On a OpenBSD system, sysctl is called.""" + self._m_is_OpenBSD.return_value = True + key, val = "system-product-name", "my_product" + self._configure_sysctl_return(key, val) + self.assertEqual(dmi.read_dmi_data(key), val) -class TestSubDMIVars: +class TestSubDMIVars: DMI_SRC = ( "dmi.nope__dmi.system-uuid__/__dmi.uuid____dmi.smbios.system.uuid__" )