cloud-init-dev team mailing list archive

[Dimitri John Ledkov <launchpad@xxxxxxxxxxxx>]

Review: Needs Fixing

os.listdir is not safe to call, if directory does not exist.

Diff comments:

> diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py
> index 0535063..c1e5756 100644
> --- a/cloudinit/net/dhcp.py
> +++ b/cloudinit/net/dhcp.py
> @@ -118,4 +120,26 @@ def dhcp_discovery(dhclient_cmd_path, interface, cleandir):
>      return parse_dhcp_lease_file(lease_file)
>  
>  
> +def parse_systemd_lease(content):
> +    """Parse a systemd lease file content as in /run/systemd/netif/leases/
> +
> +    Parse this (almost) ini style file even though it says:
> +      # This is private data. Do not parse.
> +
> +    Simply return a dictionary of key/values."""
> +
> +    return dict(configobj.ConfigObj(StringIO(content), list_values=False))
> +
> +
> +def load_systemd_leases(leases_d="/run/systemd/netif/leases"):
> +    """Return a dictionary of dictionaries representing each lease
> +    found in lease_d.i
> +
> +    The top level key will be the filename, which is typically the ifindex."""
> +    ret = {}
> +    for lfile in os.listdir(leases_d):

Unlike glob.iglob this will bomb out with file not found exception when leases_d does not exist, like e.g. on xenial where systemd-networkd is not started by default. Either use glob.iglob, or test for the existence of the directory first. I wish os.listdir simply returned empty iterator in such cases.

> +        ret[lfile] = parse_systemd_lease(
> +            util.load_file(os.path.join(leases_d, lfile)))
> +    return ret
> +
>  # vi: ts=4 expandtab


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/331664
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/1718029-fix-dhcp-parsing-from-networkd into cloud-init:master.