cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:cleanup/mask2cidr into cloud-init:master.

Commit message:
place holder

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
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.

diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index 9819d4f..6d02d22 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -46,6 +46,10 @@ def _iface_add_subnet(iface, subnet):
         'dns_nameservers',
     ]
     for key, value in subnet.items():
+        if key == 'netmask':
+            continue
+        if key == 'address':
+            value = "%s/%s" % (subnet['address'], subnet['prefix'])
         if value and key in valid_map:
             if type(value) == list:
                 value = " ".join(value)
diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index d7ddf0c..6754330 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -119,8 +119,8 @@ def _extract_addresses(config, entry):
             entry.update({sn_type: True})
         elif sn_type in ['static']:
             addr = "%s" % subnet.get('address')
-            if 'netmask' in subnet:
-                addr += "/%s" % subnet.get('netmask')
+            if 'prefix' in subnet:
+                addr += "/%d" % subnet.get('prefix')
             if 'gateway' in subnet and subnet.get('gateway'):
                 gateway = subnet.get('gateway')
                 if ":" in gateway:
@@ -138,7 +138,7 @@ def _extract_addresses(config, entry):
                 entry.update({mtukey: subnet.get('mtu')})
             for route in subnet.get('routes', []):
                 to_net = "%s/%s" % (route.get('network'),
-                                    route.get('netmask'))
+                                    route.get('prefix'))
                 route = {
                     'via': route.get('gateway'),
                     'to': to_net,
diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index db3c357..9def76d 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -289,19 +289,16 @@ class NetworkStateInterpreter(object):
             iface.update({param: val})
 
         # convert subnet ipv6 netmask to cidr as needed
-        subnets = command.get('subnets')
-        if subnets:
+        subnets = _normalize_subnets(command.get('subnets'))
+        print("normalized subnets: %s" % subnets)
+
+        # automatically set 'use_ipv6' if any addresses are ipv6
+        if not self.use_ipv6:
             for subnet in subnets:
-                if subnet['type'] == 'static':
-                    if ':' in subnet['address']:
-                        self.use_ipv6 = True
-                    if 'netmask' in subnet and ':' in subnet['address']:
-                        subnet['netmask'] = mask2cidr(subnet['netmask'])
-                        for route in subnet.get('routes', []):
-                            if 'netmask' in route:
-                                route['netmask'] = mask2cidr(route['netmask'])
-                elif subnet['type'].endswith('6'):
+                if (subnet.get('type').endswith('6') or
+                        is_ipv6_addr(subnet.get('address'))):
                     self.use_ipv6 = True
+                    break
 
         iface.update({
             'name': command.get('name'),
@@ -692,6 +689,122 @@ 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=()):
+
+    """Normalize dictionary network keys returning prefix and address keys.
+
+    @param network: A dict of network-related definition containing prefix,
+        netmask and address_keys.
+    @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.
+    """
+    network = copy.deepcopy(network)
+    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]
+        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:
+        if 'netmask' in net:
+            del net['netmask']
+    else:
+        net['netmask'] = net_prefix_to_ipv4_mask(net['prefix'])
+
+    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 if ipv4.
+       """
+    route = copy.deepcopy(route)
+    print("normalizing %s" % route)
+    # Prune None-value keys
+    normal_route = dict((k, v) for k, v in route.items() if v)
+    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))
+    print("normaled to %s" % normal_route)
+    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 address
+
+
 def subnet_is_ipv6(subnet):
     """Common helper for checking network_state subnets for ipv6."""
     # 'static6' or 'dhcp6'
@@ -703,22 +816,45 @@ 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):
+def ipv4_mask_to_net_prefix(mask, strict=True):
+    """Convert an ipv4 netmask into a network prefix length.
+       "255.255.255.0" => 24
+    """
+    if not isinstance(mask, six.string_types):
+        raise TypeError("netmask '%s' is not a string")
+
     if '.' not in mask:
+        if strict:
+            raise ValueError("netmask '%s' does not contain a '.'" % mask)
         return mask
-    return sum([bin(int(x)).count('1') for x in mask.split('.')])
+    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 ipv6mask2cidr(mask):
+
+def ipv6_mask_to_net_prefix(mask, strict=True):
+    """Convert an ipv6 netmask (very uncommon) into a network prefix length."""
+    if not isinstance(mask, six.string_types):
+        raise TypeError("mask '%s' is not a string")
     if ':' not in mask:
+        if strict:
+            raise ValueError("mask '%s' does not have a ':'")
         return mask
 
     bitCount = [0, 0x8000, 0xc000, 0xe000, 0xf000, 0xf800, 0xfc00, 0xfe00,
@@ -733,6 +869,28 @@ def ipv6mask2cidr(mask):
     return cidr
 
 
+def mask_to_net_prefix(mask):
+    """Return the network prefix for the netmask provided.
+
+    Supports ipv4 or ipv6 netmasks."""
+    if ':' in str(mask):
+        return ipv6_mask_to_net_prefix(mask)
+    elif '.' in str(mask):
+        return ipv4_mask_to_net_prefix(mask)
+
+
+def ipv4mask2cidr(mask):
+    return ipv4_mask_to_net_prefix(mask, strict=False)
+
+
+def cidr2mask(cidr):
+    return net_prefix_to_ipv4_mask(cidr)
+
+
+def ipv6mask2cidr(mask):
+    return ipv6_mask_to_net_prefix(mask, strict=False)
+
+
 def mask2cidr(mask):
     if ':' in mask:
         return ipv6mask2cidr(mask)
@@ -741,4 +899,5 @@ def mask2cidr(mask):
     else:
         return mask
 
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 58c5713..3f917ed 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -9,7 +9,7 @@ from cloudinit.distros.parsers import resolv_conf
 from cloudinit import util
 
 from . import renderer
-from .network_state import subnet_is_ipv6
+from .network_state import subnet_is_ipv6, net_prefix_to_ipv4_mask
 
 
 def _make_header(sep='#'):
@@ -26,11 +26,9 @@ def _make_header(sep='#'):
 
 
 def _is_default_route(route):
-    if route['network'] == '::' and route['netmask'] == 0:
-        return True
-    if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0':
-        return True
-    return False
+    print("checking idr route=%s" % route)
+    default_nets = ('::', '0.0.0.0')
+    return route['prefix'] == 0 and route['network'] in default_nets
 
 
 def _quote_value(value):
@@ -323,22 +321,18 @@ class Renderer(renderer.Renderer):
                             " " + ipv6_cidr)
                 else:
                     ipv4_index = ipv4_index + 1
-                    if ipv4_index == 0:
-                        iface_cfg['IPADDR'] = subnet['address']
-                        if 'netmask' in subnet:
-                            iface_cfg['NETMASK'] = subnet['netmask']
-                    else:
-                        iface_cfg['IPADDR' + str(ipv4_index)] = \
-                            subnet['address']
-                        if 'netmask' in subnet:
-                            iface_cfg['NETMASK' + str(ipv4_index)] = \
-                                subnet['netmask']
+                    suff = "" if ipv4_index == 0 else str(ipv4_index)
+                    iface_cfg['IPADDR' + suff] = subnet['address']
+                    iface_cfg['NETMASK' + suff] = \
+                        net_prefix_to_ipv4_mask(subnet['prefix'])
 
     @classmethod
     def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets):
         for i, subnet in enumerate(subnets, start=len(iface_cfg.children)):
+            print("subnet=%s" % subnet)
             for route in subnet.get('routes', []):
                 is_ipv6 = subnet.get('ipv6')
+                print("route=%s" % route)
 
                 if _is_default_route(route):
                     if (
diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index fd7c051..83580cc 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -92,10 +92,9 @@ iface lo inet loopback
 
 auto eth0
 iface eth0 inet static
-    address 192.168.1.5
+    address 192.168.1.5/24
     broadcast 192.168.1.0
     gateway 192.168.1.254
-    netmask 255.255.255.0
 
 auto eth1
 iface eth1 inet dhcp
@@ -127,7 +126,7 @@ network:
     ethernets:
         eth0:
             addresses:
-            - 192.168.1.5/255.255.255.0
+            - 192.168.1.5/24
             gateway4: 192.168.1.254
         eth1:
             dhcp4: true
@@ -156,7 +155,7 @@ network:
     ethernets:
         eth7:
             addresses:
-            - 192.168.1.5/255.255.255.0
+            - 192.168.1.5/24
             gateway4: 192.168.1.254
         eth9:
             dhcp4: true
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 5169821..a5a43bd 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -341,17 +341,15 @@ iface lo inet loopback
 
 auto eth0
 iface eth0 inet static
-    address 1.2.3.12
+    address 1.2.3.12/29
     broadcast 1.2.3.15
     dns-nameservers 69.9.160.191 69.9.191.4
     gateway 1.2.3.9
-    netmask 255.255.255.248
 
 auto eth1
 iface eth1 inet static
-    address 10.248.2.4
+    address 10.248.2.4/29
     broadcast 10.248.2.7
-    netmask 255.255.255.248
 """.lstrip()
 
 NETWORK_CONFIGS = {
@@ -406,7 +404,7 @@ NETWORK_CONFIGS = {
                             - sach.maas
                             - wark.maas
                         routes:
-                        -   to: 0.0.0.0/0.0.0.0
+                        -   to: 0.0.0.0/0
                             via: 65.61.151.37
                         set-name: eth99
         """).rstrip(' '),
@@ -890,6 +888,7 @@ USERCTL=no
         macs = {'fa:16:3e:ed:9a:59': 'eth0'}
         render_dir = self.tmp_dir()
         network_cfg = openstack.convert_net_json(net_json, known_macs=macs)
+        print("network_cfg=%s" % network_cfg)
         ns = network_state.parse_net_config_data(network_cfg,
                                                  skip_broken=False)
         renderer = sysconfig.Renderer()