python: fix type annotations (#3888)

Various type annotations were broken or half-broken. This prevented even
running the scripts under some setups.
In particular, the annotations for the ipaddress types used the
"construction functions" (e.g. `ip_address`) instead of the type of the
returned objects (e.g.  `IPv4Address`).

Relatedly, simplify the flake8 lint step and configuration and enable
style checks for acceptance tests and all python tools.

Additionally, fix up the last few remaining imports in order to simplify
the PYTHONPATH to only need the base directory, instead of the base
directory _and_ python/ -- mypy didn't like this.

Finally (and somewhat unrelatedly), revert some recently introduced use
of f-strings (new in python-3.6) back to plain %-interpolation. This was
breaking compatibility with python-3.5 without real benefit.

acceptance/cert_renewal_acceptance/test

@@ -68,7 +68,8 @@ class TestSetup(CmdBase):
         self.scion.topology('topology/tiny4.topo')
 
         logger.info('==> Remove client certificate for 112')
-        path = local.path('gen/ISD1/ASff00_0_110/cs1-ff00_0_110-1/crypto/ca/clients/ISD1-ASff00_0_112.pem')
+        path = local.path('gen/ISD1/ASff00_0_110/cs1-ff00_0_110-1/'
+                          'crypto/ca/clients/ISD1-ASff00_0_112.pem')
         path.delete()
 
         logger.info('==> Start the topology')
@@ -81,7 +82,8 @@ class TestSetup(CmdBase):
         cert = local.path('gen/certs/ISD1-ASff00_0_112.pem')
         cert.copy(path)
 
-        logger.info('==> Sleep thirty seconds to make sure the CS has picked up the client certificate')
+        logger.info('==> Sleep thirty seconds to make sure the CS '
+                    'has picked up the client certificate')
         time.sleep(30)
 
 

acceptance/common/base.py

@@ -29,7 +29,7 @@
 logger = logging.getLogger(__name__)
 
 
-def set_name(file: dir):
+def set_name(file: str):
     global NAME
     global DIR
     DIR = local.path(file).dirname.name
@@ -64,7 +64,7 @@ class TestBase(cli.Application):
     TestBase is used to implement the test entry point. Tests should
     sub-class it and only define the doc string.
     """
-    test_state = None
+    test_state = None  # type: TestState
 
     @cli.switch('disable-docker', envname='DISABLE_DOCKER',
                 help='Run in supervisor environment.')

acceptance/common/scion.py

@@ -15,7 +15,7 @@
 import logging
 import json
 from abc import ABC, abstractmethod
-from typing import Any, Dict, List
+from typing import Any, Dict, List, MutableMapping
 
 import toml
 from plumbum import local
@@ -187,7 +187,7 @@ def path_to_dict(path: str, val: Any) -> Dict:
     return d
 
 
-def merge_dict(change_dict: Dict[str, Any], orig_dict: Dict[str, Any]):
+def merge_dict(change_dict: Dict[str, Any], orig_dict: MutableMapping[str, Any]):
     """
     Merge changes into the original dictionary. Leaf values in the change dict
     overwrite the values in the original dictionary.

doc/conf.py

@@ -14,7 +14,7 @@
 # import sys
 # sys.path.insert(0, os.path.abspath('.'))
 
-import sphinx_rtd_theme
+import sphinx_rtd_theme  # noqa
 
 # -- Project information -----------------------------------------------------
 

python/topology/common.py

@@ -13,15 +13,15 @@
 # limitations under the License.
 
 # Stdlib
-from ipaddress import ip_address, ip_network
+from ipaddress import ip_address
 import os
 import subprocess
 from urllib.parse import urlsplit
-from typing import Mapping
+from typing import Mapping, Tuple
 
 # SCION
 from python.lib.scion_addr import ISD_AS
-from python.topology.net import AddressProxy, NetworkDescription
+from python.topology.net import AddressProxy, NetworkDescription, IPNetwork
 
 COMMON_DIR = 'endhost'
 
@@ -98,7 +98,7 @@ def prom_addr(addr: str, port: int) -> str:
     return join_host_port(ip, port)
 
 
-def split_host_port(addr: str) -> (str, int):
+def split_host_port(addr: str) -> Tuple[str, int]:
     parts = urlsplit('//' + addr)
     if parts.port is None:
         raise ValueError("missing port in addr: {}".format(addr))
@@ -114,7 +114,7 @@ def join_host_port(host: str, port: int) -> str:
     return '[{}]:{}'.format(host, port)
 
 
-def sciond_ip(docker, topo_id, networks: Mapping[ip_network, NetworkDescription]):
+def sciond_ip(docker, topo_id, networks: Mapping[IPNetwork, NetworkDescription]):
     for net_desc in networks.values():
         for prog, ip_net in net_desc.ip_net.items():
             if prog == 'sd%s' % topo_id.file_fmt():
@@ -123,7 +123,7 @@ def sciond_ip(docker, topo_id, networks: Mapping[ip_network, NetworkDescription]
 
 
 def prom_addr_dispatcher(docker, topo_id,
-                         networks: Mapping[ip_network, NetworkDescription],
+                         networks: Mapping[IPNetwork, NetworkDescription],
                          port, name):
     if not docker:
         return "[127.0.0.1]:%s" % port

python/topology/config.py

@@ -23,7 +23,6 @@
 import os
 import sys
 from io import StringIO
-from ipaddress import ip_network
 from typing import Mapping
 
 # SCION
@@ -44,6 +43,7 @@
 from python.topology.jaeger import JaegerGenArgs, JaegerGenerator
 from python.topology.net import (
     NetworkDescription,
+    IPNetwork,
     SubnetGenerator,
     DEFAULT_NETWORK,
 )
@@ -184,19 +184,19 @@ def _write_ca_files(self, topo_dicts, ca_files):
                 write_file(os.path.join(base, path), value.decode())
 
     def _write_networks_conf(self,
-                             networks: Mapping[ip_network, NetworkDescription],
+                             networks: Mapping[IPNetwork, NetworkDescription],
                              out_file: str):
         config = configparser.ConfigParser(interpolation=None)
         for net, net_desc in networks.items():
             sub_conf = {}
             for prog, ip_net in net_desc.ip_net.items():
-                sub_conf[prog] = ip_net.ip
-            config[net] = sub_conf
+                sub_conf[prog] = str(ip_net.ip)
+            config[str(net)] = sub_conf
         text = StringIO()
         config.write(text)
         write_file(os.path.join(self.args.output_dir, out_file), text.getvalue())
 
-    def _write_sciond_conf(self, networks: Mapping[ip_network, NetworkDescription], out_file: str):
+    def _write_sciond_conf(self, networks: Mapping[IPNetwork, NetworkDescription], out_file: str):
         d = dict()
         for net_desc in networks.values():
             for prog, ip_net in net_desc.ip_net.items():
@@ -207,8 +207,8 @@ def _write_sciond_conf(self, networks: Mapping[ip_network, NetworkDescription],
             json.dump(d, f, sort_keys=True, indent=4)
 
 
-def remove_v4_nets(nets: Mapping[ip_network, NetworkDescription]
-                   ) -> Mapping[ip_network, NetworkDescription]:
+def remove_v4_nets(nets: Mapping[IPNetwork, NetworkDescription]
+                   ) -> Mapping[IPNetwork, NetworkDescription]:
     res = {}
     for net, net_desc in nets.items():
         if net_desc.name.endswith('_v4'):

python/topology/docker.py

@@ -15,7 +15,6 @@
 # Stdlib
 import copy
 import os
-from ipaddress import ip_network
 from typing import Mapping
 # External packages
 import yaml
@@ -31,14 +30,14 @@
     sciond_svc_name
 )
 from python.topology.docker_utils import DockerUtilsGenArgs, DockerUtilsGenerator
-from python.topology.net import NetworkDescription
+from python.topology.net import NetworkDescription, IPNetwork
 from python.topology.sig import SIGGenArgs, SIGGenerator
 
 DOCKER_CONF = 'scion-dc.yml'
 
 
 class DockerGenArgs(ArgsTopoDicts):
-    def __init__(self, args, topo_dicts, networks: Mapping[ip_network, NetworkDescription]):
+    def __init__(self, args, topo_dicts, networks: Mapping[IPNetwork, NetworkDescription]):
         """
         :param object args: Contains the passed command line arguments as named attributes.
         :param dict topo_dicts: The generated topo dicts from TopoGenerator.

python/topology/go.py

@@ -20,7 +20,6 @@
 import os
 import toml
 import yaml
-from ipaddress import ip_network
 from typing import Mapping
 
 # SCION
@@ -41,7 +40,7 @@
     CO_CONFIG_NAME,
 )
 
-from python.topology.net import socket_address_str, NetworkDescription
+from python.topology.net import socket_address_str, NetworkDescription, IPNetwork
 
 from python.topology.prometheus import (
     CS_PROM_PORT,
@@ -57,7 +56,7 @@
 
 
 class GoGenArgs(ArgsTopoDicts):
-    def __init__(self, args, topo_dicts, networks: Mapping[ip_network, NetworkDescription]):
+    def __init__(self, args, topo_dicts, networks: Mapping[IPNetwork, NetworkDescription]):
         super().__init__(args, topo_dicts)
         self.networks = networks
 
@@ -78,7 +77,7 @@ def generate_br(self):
             for k, v in topo.get("border_routers", {}).items():
                 base = topo_id.base_dir(self.args.output_dir)
                 br_conf = self._build_br_conf(topo_id, topo["isd_as"], base, k, v)
-                write_file(os.path.join(base, f"{k}.toml"), toml.dumps(br_conf))
+                write_file(os.path.join(base, "%s.toml" % k), toml.dumps(br_conf))
 
     def _build_br_conf(self, topo_id, ia, base, name, v):
         config_dir = '/share/conf' if self.args.docker else base
@@ -104,7 +103,7 @@ def generate_control_service(self):
                     base = topo_id.base_dir(self.args.output_dir)
                     bs_conf = self._build_control_service_conf(
                         topo_id, topo["isd_as"], base, elem_id, elem, ca)
-                    write_file(os.path.join(base, f"{elem_id}.toml"),
+                    write_file(os.path.join(base, "%s.toml" % elem_id),
                                toml.dumps(bs_conf))
 
     def _build_control_service_conf(self, topo_id, ia, base, name, infra_elem, ca):
@@ -278,7 +277,7 @@ def _gen_disp_docker(self):
             for k in elem_ids:
                 disp_id = 'disp_%s' % k
                 disp_conf = self._build_disp_conf(disp_id, topo_id)
-                write_file(os.path.join(base, f'{disp_id}.toml'), toml.dumps(disp_conf))
+                write_file(os.path.join(base, '%s.toml' % disp_id), toml.dumps(disp_conf))
 
     def _build_disp_conf(self, name, topo_id=None):
         prometheus_addr = prom_addr_dispatcher(self.args.docker, topo_id,

python/topology/net.py

@@ -21,8 +21,17 @@
 import math
 import sys
 from collections import defaultdict
-from ipaddress import ip_interface, ip_network
-from typing import Mapping
+from ipaddress import (
+    ip_interface,
+    ip_network,
+    IPv4Address,
+    IPv6Address,
+    IPv4Network,
+    IPv6Network,
+    IPv4Interface,
+    IPv6Interface,
+)
+from typing import Mapping, Union
 
 # External packages
 import yaml
@@ -34,9 +43,13 @@
 DEFAULT_PRIV_NETWORK = "192.168.0.0/16"
 DEFAULT_SCN_DC_NETWORK = "172.20.0.0/20"
 
+IPAddress = Union[IPv4Address, IPv6Address]
+IPNetwork = Union[IPv4Network, IPv6Network]
+IPInterface = Union[IPv4Interface, IPv6Interface]
+
 
 class NetworkDescription(object):
-    def __init__(self, name: str, ip_net: Mapping[str, ip_interface]):
+    def __init__(self, name: str, ip_net: Mapping[str, IPInterface]):
         self.name = name
         self.ip_net = ip_net
 
@@ -68,7 +81,7 @@ def __init__(self, docker):
     def register(self, id_: str) -> AddressProxy:
         return self._addrs[id_]
 
-    def alloc_addrs(self, subnet) -> Mapping[str, ip_interface]:
+    def alloc_addrs(self, subnet) -> Mapping[str, IPInterface]:
         hosts = subnet.hosts()
         interfaces = {}
         # With the docker backend, docker itself claims the first ip of every network
@@ -97,7 +110,8 @@ def __init__(self, network: str, docker: bool):
         except ValueError:
             logging.critical("Invalid network '%s'", network)
             sys.exit(1)
-        self._subnets = defaultdict(lambda: AddressGenerator(self.docker))
+        self._subnets = defaultdict(lambda: AddressGenerator(self.docker)) \
+            # type: Mapping[str, AddressGenerator]
         self._allocations = defaultdict(list)
         # Initialise the allocations with the supplied network, making sure to
         # exclude 127.0.0.0/30 (for v4) and DEFAULT6_NETWORK_ADDR/126 (for v6)
@@ -119,7 +133,7 @@ def __init__(self, network: str, docker: bool):
     def register(self, location: str) -> AddressGenerator:
         return self._subnets[location]
 
-    def alloc_subnets(self) -> Mapping[ip_network, NetworkDescription]:
+    def alloc_subnets(self) -> Mapping[IPNetwork, NetworkDescription]:
         max_prefix = self._net.max_prefixlen
         networks = {}
         for topo, subnet in sorted(self._subnets.items(), key=lambda x: str(x)):
@@ -173,13 +187,13 @@ def register(self, id_: str) -> int:
         return p
 
 
-def socket_address_str(ip: str, port: int) -> str:
+def socket_address_str(ip: IPAddress, port: int) -> str:
     if ip.version == 4:
         return "%s:%d" % (ip, port)
     return "[%s]:%d" % (ip, port)
 
 
-def _workaround_ip_network_hosts_py35(net: ip_network) -> ip_network:
+def _workaround_ip_network_hosts_py35(net: IPNetwork) -> IPNetwork:
     """
     Returns an _identical_ ipaddress.ip_network for which hosts() which will work as it should.
 

python/topology/prometheus.py

@@ -19,7 +19,6 @@
 # Stdlib
 import os
 from collections import defaultdict
-from ipaddress import ip_network
 from typing import Mapping
 
 # External packages
@@ -35,7 +34,8 @@
     sciond_ip,
 )
 from python.topology.net import (
-    NetworkDescription
+    NetworkDescription,
+    IPNetwork,
 )
 
 CS_PROM_PORT = 30452
@@ -49,7 +49,7 @@
 
 
 class PrometheusGenArgs(ArgsTopoDicts):
-    def __init__(self, args, topo_dicts, networks: Mapping[ip_network, NetworkDescription]):
+    def __init__(self, args, topo_dicts, networks: Mapping[IPNetwork, NetworkDescription]):
         super().__init__(args, topo_dicts)
         self.networks = networks
 

python/topology/supervisor.py

@@ -73,7 +73,7 @@ def _as_entries(self, topo_id, topo):
     def _br_entries(self, topo, cmd, base):
         entries = []
         for k, v in topo.get("border_routers", {}).items():
-            conf = os.path.join(base, f"{k}.toml")
+            conf = os.path.join(base, "%s.toml" % k)
             prog = self._common_entry(k, [cmd, "--config", conf])
             prog['environment'] += ',GODEBUG="cgocheck=0"'
             entries.append((k, prog))
@@ -84,7 +84,7 @@ def _control_service_entries(self, topo, base):
         for k, v in topo.get("control_service", {}).items():
             # only a single control service instance per AS is currently supported
             if k.endswith("-1"):
-                conf = os.path.join(base, f"{k}.toml")
+                conf = os.path.join(base, "%s.toml" % k)
                 prog = self._common_entry(k, ["bin/cs", "--config", conf])
                 entries.append((k, prog))
         return entries
@@ -112,7 +112,7 @@ def _common_entry(self, name, cmd_args):
             'autostart': 'false',
             'autorestart': 'false',
             'environment': 'TZ=UTC',
-            'stdout_logfile': f"logs/{name}.log",
+            'stdout_logfile': "logs/%s.log" % name,
             'redirect_stderr': True,
             'startretries': 0,
             'startsecs': 5,