cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #02400
Re: [Merge] ~smoser/cloud-init:cleanup/mask2cidr into cloud-init:master
looks much better thanks for this. minor inlines below
Diff comments:
> diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
> index db3c357..ef000ae 100644
> --- a/cloudinit/net/network_state.py
> +++ b/cloudinit/net/network_state.py
> @@ -692,6 +670,125 @@ class NetworkStateInterpreter(object):
> return subnets
>
>
> +def _normalize_subnet(subnet):
> + # Prune all keys with None values.
> + subnet = copy.deepcopy(subnet)
> + normal_subnet = dict((k, v) for k, v in subnet.items() if v)
> +
> + if subnet.get('type') == 'static':
> + normal_subnet.update(
> + _normalize_net_keys(normal_subnet, address_keys=('address',)))
> + normal_subnet['routes'] = [_normalize_route(r)
> + for r in subnet.get('routes', [])]
> + return normal_subnet
> +
> +
> +def _normalize_net_keys(network, address_keys=()):
> +
no need for extra lines.
> + """Normalize dictionary network keys returning prefix and address keys.
> +
> + @param network: A dict of network-related definition containing prefix,
> + netmask and address_keys.
If we don't need netmask anymore in normalized dict, let's drop it here too.
> + @param address_keys: A tuple of keys to search for representing the address
> + or cidr. The first address_key discovered will be used for
> + normalization.
> +
> + @returns: A dict containing normalized prefix and matching addr_key.
> + """
> + net = dict((k, v) for k, v in network.items() if v)
> + addr_key = None
> + for key in address_keys:
> + if net.get(key):
> + addr_key = key
> + break
> + if not addr_key:
> + message = (
> + 'No config network address keys [%s] found in %s' %
> + (','.join(address_keys), network))
> + LOG.error(message)
> + raise ValueError(message)
> +
> + addr = net.get(addr_key)
> + ipv6 = is_ipv6_addr(addr)
> + netmask = net.get('netmask')
> + if "/" in addr:
> + toks = addr.split("/", 2)
> + net[addr_key] = toks[0]
> + # If prefix is defined by the user but addr_key is a CIDR, we
> + # silently overwrite the user's original prefix here. We should
> + # log a warning.
> + net['prefix'] = toks[1]
Agreed w/ Ryan here, can we log this warning?
> + try:
> + net['prefix'] = int(toks[1])
> + except ValueError:
> + # this supports input of <address>/255.255.255.0
> + net['prefix'] = mask_to_net_prefix(toks[1])
> +
> + elif netmask:
> + net['prefix'] = mask_to_net_prefix(netmask)
> + else:
> + net['prefix'] = 64 if ipv6 else 24
> +
> + if ipv6:
> + # TODO: we could/maybe should add this back with the very uncommon
> + # 'netmask' for ipv6. We need a 'net_prefix_to_ipv6_mask' for that.
> + if 'netmask' in net:
> + del net['netmask']
> + else:
> + net['netmask'] = net_prefix_to_ipv4_mask(net['prefix'])
Ok I'm being dumb, why do we want netmask key around in the normalized structure, don't we get all we want from our normalized prefix plus the function net_prefix_to_ipv4_mask?
> +
> + prefix = net['prefix']
> + if prefix:
> + try:
> + net['prefix'] = int(prefix)
> + except ValueError:
> + raise TypeError(
> + 'Network config prefix {} is not an integer'.format(prefix))
> + return net
> +
> +
> +def _normalize_route(route):
> + """normalize a route.
> + return a dictionary with only:
> + 'type': 'route' (only present if it was present in input)
> + 'network': the network portion of the route as a string.
> + 'prefix': the network prefix for address as an integer.
> + 'metric': integer metric (only if present in input).
> + 'netmask': netmask (string) equivalent to prefix iff network is ipv4.
s/iff/if/
Also: If we don't need netmask anymore in normalized dict, let's drop it here too.
> + """
> + # Prune None-value keys. Specifically allow 0 (a valid metric).
> + normal_route = dict((k, v) for k, v in route.items()
> + if v not in ("", None))
> + if 'destination' in normal_route:
> + normal_route['network'] = normal_route['destination']
> + del normal_route['destination']
> +
> + normal_route.update(
> + _normalize_net_keys(
> + normal_route, address_keys=('network', 'destination')))
> +
> + metric = normal_route.get('metric')
> + if metric:
> + try:
> + normal_route['metric'] = int(metric)
> + except ValueError:
> + raise TypeError(
> + 'Route config metric {} is not an integer'.format(metric))
> + return normal_route
> +
> +
> +def _normalize_subnets(subnets):
> + if not subnets:
> + subnets = []
> + return [_normalize_subnet(s) for s in subnets]
> +
> +
> +def is_ipv6_addr(address):
> + if not address:
> + return False
> + return ":" in str(address)
> +
> +
> def subnet_is_ipv6(subnet):
> """Common helper for checking network_state subnets for ipv6."""
> # 'static6' or 'dhcp6'
> @@ -703,42 +800,86 @@ def subnet_is_ipv6(subnet):
> return False
>
>
> -def cidr2mask(cidr):
> +def net_prefix_to_ipv4_mask(prefix):
> + """Convert a network prefix to an ipv4 netmask.
> +
> + This is the inverse of ipv4_mask_to_net_prefix.
> + 24 -> "255.255.255.0"
> + Also supports input as a string."""
> +
> mask = [0, 0, 0, 0]
> - for i in list(range(0, cidr)):
> + for i in list(range(0, int(prefix))):
> idx = int(i / 8)
> mask[idx] = mask[idx] + (1 << (7 - i % 8))
> return ".".join([str(x) for x in mask])
>
>
> -def ipv4mask2cidr(mask):
> - if '.' not in mask:
> +def ipv4_mask_to_net_prefix(mask):
> + """Convert an ipv4 netmask into a network prefix length.
> + "255.255.255.0" => 24
> + """
> + if isinstance(mask, int):
> return mask
> - return sum([bin(int(x)).count('1') for x in mask.split('.')])
> + elif isinstance(mask, six.string_types):
> + try:
> + return int(mask)
> + except ValueError:
> + pass
> + else:
> + raise TypeError("mask '%s' is not a string or int")
>
> + if '.' not in mask:
> + raise ValueError("netmask '%s' does not contain a '.'" % mask)
>
> -def ipv6mask2cidr(mask):
> - if ':' not in mask:
> + toks = mask.split(".")
> + if len(toks) != 4:
> + raise ValueError("netmask '%s' had only %d parts" % (mask, len(toks)))
> +
> + return sum([bin(int(x)).count('1') for x in toks])
> +
> +
> +def ipv6_mask_to_net_prefix(mask):
> + """Convert an ipv6 netmask (very uncommon) or prefix (64) to prefix."""
> +
> + if isinstance(mask, int):
> return mask
> + elif isinstance(mask, six.string_types):
You don't need an elif here are you just returned above.
> + try:
> + return int(mask)
> + except ValueError:
> + pass
> + else:
> + raise TypeError("mask '%s' is not a string or int")
> +
> + if ':' not in mask:
> + raise ValueError("mask '%s' does not have a ':'")
>
> bitCount = [0, 0x8000, 0xc000, 0xe000, 0xf000, 0xf800, 0xfc00, 0xfe00,
> 0xff00, 0xff80, 0xffc0, 0xffe0, 0xfff0, 0xfff8, 0xfffc,
> 0xfffe, 0xffff]
> - cidr = 0
> + prefix = 0
> for word in mask.split(':'):
> if not word or int(word, 16) == 0:
> break
> - cidr += bitCount.index(int(word, 16))
> + prefix += bitCount.index(int(word, 16))
> +
> + return prefix
>
> - return cidr
>
> +def mask_to_net_prefix(mask):
> + """Return the network prefix for the netmask provided.
>
> -def mask2cidr(mask):
> - if ':' in mask:
> - return ipv6mask2cidr(mask)
> - elif '.' in mask:
> - return ipv4mask2cidr(mask)
> + Supports ipv4 or ipv6 netmasks."""
> + try:
> + # if 'mask' is a prefix that is an integer.
> + # then just return it.
> + return int(mask)
> + except ValueError:
> + pass
> + if is_ipv6_addr(mask):
> + return ipv6_mask_to_net_prefix(mask)
> else:
> - return mask
> + return ipv4_mask_to_net_prefix(mask)
> +
>
> # vi: ts=4 expandtab
--
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324677
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/mask2cidr into cloud-init:master.
References