From da46c60ef2313514dac75c6b02e13d04cae2c0e4 Mon Sep 17 00:00:00 2001 From: a-dubs Date: Fri, 12 Jul 2024 01:52:33 -0400 Subject: [PATCH 1/7] mypy'd all over ibm datasource --- cloudinit/sources/DataSourceIBMCloud.py | 103 ++++++++++++++---------- cloudinit/sources/__init__.py | 2 +- pyproject.toml | 1 - 3 files changed, 60 insertions(+), 46 deletions(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index 89edd79f7ea..c86d685162f 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -96,6 +96,7 @@ import json import logging import os +from typing import Any, Callable, Dict, Optional, Tuple from cloudinit import atomic_helper, sources, subp, util from cloudinit.sources.helpers import openstack @@ -176,7 +177,7 @@ def network_config(self): # environment handles networking configuration. Not cloud-init. return {"config": "disabled", "version": 1} if self._network_config is None: - if self.network_json is not None: + if self.network_json not in (sources.UNSET, None): LOG.debug("network config provided via network_json") self._network_config = openstack.convert_net_json( self.network_json, known_macs=None @@ -186,7 +187,12 @@ def network_config(self): return self._network_config -def _read_system_uuid(): +def _read_system_uuid() -> Optional[str]: + """ + Read the system uuid. + + @return: the system uuid or None if not available. + """ uuid_path = "/sys/hypervisor/uuid" if not os.path.isfile(uuid_path): return None @@ -194,6 +200,9 @@ def _read_system_uuid(): def _is_xen(): + """ + Return boolean indicating if this is a xen hypervisor. + """ return os.path.exists("/proc/xen") @@ -201,7 +210,7 @@ def _is_ibm_provisioning( prov_cfg="/root/provisioningConfiguration.cfg", inst_log="/root/swinstall.log", boot_ref="/proc/1/environ", -): +) -> bool: """Return boolean indicating if this boot is ibm provisioning boot.""" if os.path.exists(prov_cfg): msg = "config '%s' exists." % prov_cfg @@ -229,7 +238,7 @@ def _is_ibm_provisioning( return result -def get_ibm_platform(): +def get_ibm_platform() -> Tuple[Optional[str], Optional[str]]: """Return a tuple (Platform, path) If this is Not IBM cloud, then the return value is (None, None). @@ -242,7 +251,7 @@ def get_ibm_platform(): return not_found # fslabels contains only the first entry with a given label. - fslabels = {} + fslabels: Dict[str, Dict] = {} try: devs = util.blkid() except subp.ProcessExecutionError as e: @@ -289,7 +298,7 @@ def get_ibm_platform(): return not_found -def read_md(): +def read_md() -> Optional[Dict[str, Any]]: """Read data from IBM Cloud. @return: None if not running on IBM Cloud. @@ -325,27 +334,39 @@ def read_md(): return ret -def metadata_from_dir(source_dir): +def metadata_from_dir(source_dir: str) -> Dict[str, Any]: """Walk source_dir extracting standardized metadata. Certain metadata keys are renamed to present a standardized set of metadata keys. This function has a lot in common with ConfigDriveReader.read_v2 but - there are a number of inconsistencies, such key renames and as only - presenting a 'latest' version which make it an unlikely candidate to share + there are a number of inconsistencies, such as key renames and only + presenting a 'latest' version, which make it an unlikely candidate to share code. @return: Dict containing translated metadata, userdata, vendordata, networkdata as present. """ - def opath(fname): + def opath(fname: str) -> str: return os.path.join("openstack", "latest", fname) - def load_json_bytes(blob): + def load_json_bytes(blob: bytes) -> Dict[str, Any]: return json.loads(blob.decode("utf-8")) + def load_file( + path: str, translator: Optional[Callable[[bytes], Any]] = None + ) -> Any: + try: + raw = util.load_binary_file(path) + return translator(raw) if translator else raw + except IOError as e: + LOG.debug("Failed reading path '%s': %s", path, e) + return None + except Exception as e: + raise sources.BrokenMetadata(f"Failed decoding {path}: {e}") + files = [ # tuples of (results_name, path, translator) ("metadata_raw", opath("meta_data.json"), load_json_bytes), @@ -354,54 +375,48 @@ def load_json_bytes(blob): ("networkdata", opath("network_data.json"), load_json_bytes), ] - results = {} - for (name, path, transl) in files: - fpath = os.path.join(source_dir, path) - raw = None - try: - raw = util.load_binary_file(fpath) - except IOError as e: - LOG.debug("Failed reading path '%s': %s", fpath, e) + raw_results: Dict[str, Optional[bytes]] = {} + translated_results: Dict[str, Any] = {} - if raw is None or transl is None: - data = raw + for name, path, transl in files: + fpath = os.path.join(source_dir, path) + raw_results[name] = load_file(fpath) + if raw_results[name] is not None and transl is not None: + translated_results[name] = load_file(fpath, transl) else: - try: - data = transl(raw) - except Exception as e: - raise sources.BrokenMetadata( - "Failed decoding %s: %s" % (path, e) - ) + translated_results[name] = raw_results[name] - results[name] = data - - if results.get("metadata_raw") is None: + if raw_results["metadata_raw"] is None: raise sources.BrokenMetadata( - "%s missing required file 'meta_data.json'" % source_dir + "%s missing required file 'meta_data.json'", + source_dir, ) - results["metadata"] = {} + translated_results["metadata"] = {} + + md_raw = translated_results["metadata_raw"] + md = translated_results["metadata"] - md_raw = results["metadata_raw"] - md = results["metadata"] if "random_seed" in md_raw: try: md["random_seed"] = base64.b64decode(md_raw["random_seed"]) except (ValueError, TypeError) as e: raise sources.BrokenMetadata( - "Badly formatted metadata random_seed entry: %s" % e + "Badly formatted metadata random_seed entry: %s", + e, ) - renames = ( - ("public_keys", "public-keys"), - ("hostname", "local-hostname"), - ("uuid", "instance-id"), - ) - for mdname, newname in renames: - if mdname in md_raw: - md[newname] = md_raw[mdname] + renames = { + "public_keys": "public-keys", + "hostname": "local-hostname", + "uuid": "instance-id", + } + + for old_key, new_key in renames.items(): + if old_key in md_raw: + md[new_key] = md_raw[old_key] - return results + return translated_results # Used to match classes to dependencies diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 27c37ee1e13..391131edc96 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -325,7 +325,7 @@ def __init__(self, sys_cfg, distro: Distro, paths: Paths, ud_proc=None): self.vendordata_raw = None self.vendordata2_raw = None self.metadata_address = None - self.network_json = UNSET + self.network_json: Optional[str] = UNSET self.ec2_metadata = UNSET self.ds_cfg = util.get_cfg_by_path( diff --git a/pyproject.toml b/pyproject.toml index d5578c1379b..df969290451 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -95,7 +95,6 @@ module = [ "cloudinit.sources.DataSourceExoscale", "cloudinit.sources.DataSourceGCE", "cloudinit.sources.DataSourceHetzner", - "cloudinit.sources.DataSourceIBMCloud", "cloudinit.sources.DataSourceMAAS", "cloudinit.sources.DataSourceNoCloud", "cloudinit.sources.DataSourceOVF", From 93c82a5a72bf8b945d3033574d96d0fa9b95e0ee Mon Sep 17 00:00:00 2001 From: a-dubs Date: Tue, 16 Jul 2024 10:06:08 -0400 Subject: [PATCH 2/7] applied james' patch! thanks pookie <3 --- cloudinit/sources/DataSourceIBMCloud.py | 27 ++++++++++--------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index c86d685162f..05ca8f056c0 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -341,7 +341,7 @@ def metadata_from_dir(source_dir: str) -> Dict[str, Any]: keys. This function has a lot in common with ConfigDriveReader.read_v2 but - there are a number of inconsistencies, such as key renames and only + there are a number of inconsistencies, such as key renames and only presenting a 'latest' version, which make it an unlikely candidate to share code. @@ -356,11 +356,11 @@ def load_json_bytes(blob: bytes) -> Dict[str, Any]: return json.loads(blob.decode("utf-8")) def load_file( - path: str, translator: Optional[Callable[[bytes], Any]] = None + path: str, translator: Callable[[bytes], Any] ) -> Any: try: raw = util.load_binary_file(path) - return translator(raw) if translator else raw + return translator(raw) except IOError as e: LOG.debug("Failed reading path '%s': %s", path, e) return None @@ -370,32 +370,27 @@ def load_file( files = [ # tuples of (results_name, path, translator) ("metadata_raw", opath("meta_data.json"), load_json_bytes), - ("userdata", opath("user_data"), None), + ("userdata", opath("user_data"), lambda x: x), ("vendordata", opath("vendor_data.json"), load_json_bytes), ("networkdata", opath("network_data.json"), load_json_bytes), ] - raw_results: Dict[str, Optional[bytes]] = {} - translated_results: Dict[str, Any] = {} + results: Dict[str, Any] = {} for name, path, transl in files: fpath = os.path.join(source_dir, path) - raw_results[name] = load_file(fpath) - if raw_results[name] is not None and transl is not None: - translated_results[name] = load_file(fpath, transl) - else: - translated_results[name] = raw_results[name] + results[name] = load_file(fpath, transl) - if raw_results["metadata_raw"] is None: + if results["metadata_raw"] is None: raise sources.BrokenMetadata( "%s missing required file 'meta_data.json'", source_dir, ) - translated_results["metadata"] = {} + results["metadata"] = {} - md_raw = translated_results["metadata_raw"] - md = translated_results["metadata"] + md_raw = results["metadata_raw"] + md = results["metadata"] if "random_seed" in md_raw: try: @@ -416,7 +411,7 @@ def load_file( if old_key in md_raw: md[new_key] = md_raw[old_key] - return translated_results + return results # Used to match classes to dependencies From c8bc38f06ee8ee64ccd47f4eecbefa0ddae85a5b Mon Sep 17 00:00:00 2001 From: a-dubs Date: Tue, 16 Jul 2024 10:10:04 -0400 Subject: [PATCH 3/7] convert dict to tuples as per james' comment --- cloudinit/sources/DataSourceIBMCloud.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index 05ca8f056c0..7b9f8995555 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -401,13 +401,13 @@ def load_file( e, ) - renames = { - "public_keys": "public-keys", - "hostname": "local-hostname", - "uuid": "instance-id", - } + renames = ( + ("public_keys", "public-keys"), + ("hostname", "local-hostname"), + ("uuid", "instance-id"), + ) - for old_key, new_key in renames.items(): + for old_key, new_key in renames: if old_key in md_raw: md[new_key] = md_raw[old_key] From e084b1fe5784340b2f684496e4a38718ad93dbae Mon Sep 17 00:00:00 2001 From: a-dubs Date: Wed, 24 Jul 2024 09:39:08 -0400 Subject: [PATCH 4/7] fixing james' requested changes --- cloudinit/sources/DataSourceIBMCloud.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index 7b9f8995555..459436f88fa 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -191,7 +191,7 @@ def _read_system_uuid() -> Optional[str]: """ Read the system uuid. - @return: the system uuid or None if not available. + :return: the system uuid or None if not available. """ uuid_path = "/sys/hypervisor/uuid" if not os.path.isfile(uuid_path): @@ -202,6 +202,8 @@ def _read_system_uuid() -> Optional[str]: def _is_xen(): """ Return boolean indicating if this is a xen hypervisor. + + :return: True if this is a xen hypervisor, False otherwise. """ return os.path.exists("/proc/xen") @@ -301,7 +303,7 @@ def get_ibm_platform() -> Tuple[Optional[str], Optional[str]]: def read_md() -> Optional[Dict[str, Any]]: """Read data from IBM Cloud. - @return: None if not running on IBM Cloud. + :return: None if not running on IBM Cloud. dictionary with guaranteed fields: metadata, version and optional fields: userdata, vendordata, networkdata. Also includes the system uuid from /sys/hypervisor/uuid.""" @@ -345,7 +347,7 @@ def metadata_from_dir(source_dir: str) -> Dict[str, Any]: presenting a 'latest' version, which make it an unlikely candidate to share code. - @return: Dict containing translated metadata, userdata, vendordata, + :return: Dict containing translated metadata, userdata, vendordata, networkdata as present. """ @@ -353,6 +355,15 @@ def opath(fname: str) -> str: return os.path.join("openstack", "latest", fname) def load_json_bytes(blob: bytes) -> Dict[str, Any]: + """ + Load JSON from a byte string. + + This technically could return a list or a str, but we are only + assuming a dict here. + + :param blob: The byte string to load JSON from. + :return: The loaded JSON object. + """ return json.loads(blob.decode("utf-8")) def load_file( @@ -383,8 +394,7 @@ def load_file( if results["metadata_raw"] is None: raise sources.BrokenMetadata( - "%s missing required file 'meta_data.json'", - source_dir, + f"{source_dir} missing required file 'meta_data.json'", ) results["metadata"] = {} @@ -397,8 +407,7 @@ def load_file( md["random_seed"] = base64.b64decode(md_raw["random_seed"]) except (ValueError, TypeError) as e: raise sources.BrokenMetadata( - "Badly formatted metadata random_seed entry: %s", - e, + f"Badly formatted metadata random_seed entry: {e}" ) renames = ( From e37b220d3a87fa2713f7efee18413190f76e77e3 Mon Sep 17 00:00:00 2001 From: a-dubs Date: Mon, 29 Jul 2024 21:09:58 -0400 Subject: [PATCH 5/7] fixed linting errors --- cloudinit/sources/DataSourceIBMCloud.py | 6 ++---- tests/unittests/sources/test_ibmcloud.py | 6 ++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index 459436f88fa..c0dd7ba3316 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -308,7 +308,7 @@ def read_md() -> Optional[Dict[str, Any]]: and optional fields: userdata, vendordata, networkdata. Also includes the system uuid from /sys/hypervisor/uuid.""" platform, path = get_ibm_platform() - if platform is None: + if platform is None or path is None: LOG.debug("This is not an IBMCloud platform.") return None elif platform in PROVISIONING: @@ -366,9 +366,7 @@ def load_json_bytes(blob: bytes) -> Dict[str, Any]: """ return json.loads(blob.decode("utf-8")) - def load_file( - path: str, translator: Callable[[bytes], Any] - ) -> Any: + def load_file(path: str, translator: Callable[[bytes], Any]) -> Any: try: raw = util.load_binary_file(path) return translator(raw) diff --git a/tests/unittests/sources/test_ibmcloud.py b/tests/unittests/sources/test_ibmcloud.py index bee486f4dd5..37c2594dce3 100644 --- a/tests/unittests/sources/test_ibmcloud.py +++ b/tests/unittests/sources/test_ibmcloud.py @@ -272,6 +272,8 @@ def test_template_live(self, m_platform, m_sysuuid): ) ret = ibm.read_md() + if ret is None: # this is needed for mypy - ensures ret is not None + self.fail("read_md returned None unexpectedly") self.assertEqual(ibm.Platforms.TEMPLATE_LIVE_METADATA, ret["platform"]) self.assertEqual(tmpdir, ret["source"]) self.assertEqual(self.userdata, ret["userdata"]) @@ -298,6 +300,8 @@ def test_os_code_live(self, m_platform, m_sysuuid): ) ret = ibm.read_md() + if ret is None: # this is needed for mypy - ensures ret is not None + self.fail("read_md returned None unexpectedly") self.assertEqual(ibm.Platforms.OS_CODE, ret["platform"]) self.assertEqual(tmpdir, ret["source"]) self.assertEqual(self.userdata, ret["userdata"]) @@ -320,6 +324,8 @@ def test_os_code_live_no_userdata(self, m_platform, m_sysuuid): ) ret = ibm.read_md() + if ret is None: # this is needed for mypy - ensures ret is not None + self.fail("read_md returned None unexpectedly") self.assertEqual(ibm.Platforms.OS_CODE, ret["platform"]) self.assertEqual(tmpdir, ret["source"]) self.assertIsNone(ret["userdata"]) From da433d2acf18372c7dbf9d029c49dc33d10a84bb Mon Sep 17 00:00:00 2001 From: a-dubs Date: Thu, 1 Aug 2024 14:45:48 -0400 Subject: [PATCH 6/7] should be good to go --- cloudinit/sources/DataSourceIBMCloud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index c0dd7ba3316..830cb728e41 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -308,10 +308,10 @@ def read_md() -> Optional[Dict[str, Any]]: and optional fields: userdata, vendordata, networkdata. Also includes the system uuid from /sys/hypervisor/uuid.""" platform, path = get_ibm_platform() - if platform is None or path is None: + if platform is None: LOG.debug("This is not an IBMCloud platform.") return None - elif platform in PROVISIONING: + elif platform in PROVISIONING or path is None: LOG.debug("Cloud-init is disabled during provisioning: %s.", platform) return None From 4ecc3d034b5af4d7f54e4bf4f578b166853f084a Mon Sep 17 00:00:00 2001 From: a-dubs Date: Tue, 6 Aug 2024 10:11:24 -0400 Subject: [PATCH 7/7] RAHHHH STUPID TRAILING WHITESPACE --- cloudinit/sources/DataSourceIBMCloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index 830cb728e41..9b3ce8d9f83 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -311,7 +311,7 @@ def read_md() -> Optional[Dict[str, Any]]: if platform is None: LOG.debug("This is not an IBMCloud platform.") return None - elif platform in PROVISIONING or path is None: + elif platform in PROVISIONING or path is None: LOG.debug("Cloud-init is disabled during provisioning: %s.", platform) return None