From c91f86165a6de369ae50e97463ad0268fb16e6a1 Mon Sep 17 00:00:00 2001
From: roos <roos@anapaya.net>
Date: Mon, 5 Nov 2018 17:52:57 +0100
Subject: [PATCH 1/2] gen: Refactor command line arguments

Make command line arguments more modular
---
 python/topology/ca.py         |  15 ++-
 python/topology/cert.py       |  17 ++-
 python/topology/common.py     |  26 +++++
 python/topology/config.py     | 196 ++++++++++++++++++++--------------
 python/topology/docker.py     |  55 +++++-----
 python/topology/generator.py  |  52 +--------
 python/topology/go.py         |  45 ++++----
 python/topology/prometheus.py |  25 +++--
 python/topology/supervisor.py |  55 +++++-----
 python/topology/topo.py       |  91 +++++++++-------
 python/topology/utils.py      |  14 ++-
 11 files changed, 330 insertions(+), 261 deletions(-)

diff --git a/python/topology/ca.py b/python/topology/ca.py
index 22602ae170..02168d6bdb 100755
--- a/python/topology/ca.py
+++ b/python/topology/ca.py
@@ -27,11 +27,20 @@
     get_ca_cert_file_path,
     get_ca_private_key_file_path,
 )
+from topology.common import ArgsTopoConfig
+
+
+class CAGenArgs(ArgsTopoConfig):
+    pass
 
 
 class CAGenerator(object):
-    def __init__(self, topo_config):
-        self.topo_config = topo_config
+    def __init__(self, args):
+        """
+        :param CAGenArgs args: Contains the passed command line
+        arguments and the topo config.
+        """
+        self.args = args
         self.ca_key_pairs = {}
         self.ca_certs = defaultdict(dict)
         self.ca_private_key_files = defaultdict(dict)
@@ -45,7 +54,7 @@ def generate(self):
         return self.ca_private_key_files, self.ca_cert_files, self.ca_certs
 
     def _iterate(self, f):
-        for ca_name, ca_config in self.topo_config["CAs"].items():
+        for ca_name, ca_config in self.args.config["CAs"].items():
             f(ca_name, ca_config)
 
     def _gen_ca_key(self, ca_name, ca_config):
diff --git a/python/topology/cert.py b/python/topology/cert.py
index f7666543aa..0ea3ecce20 100755
--- a/python/topology/cert.py
+++ b/python/topology/cert.py
@@ -44,7 +44,7 @@
     get_online_key_file_path,
 )
 from lib.errors import SCIONParseError
-from topology.common import TopoID
+from topology.common import ArgsTopoConfig, TopoID
 
 INITIAL_CERT_VERSION = 1
 INITIAL_TRC_VERSION = 1
@@ -60,11 +60,18 @@
 DEFAULT_KEYGEN_ALG = 'ed25519'
 
 
+class CertGenArgs(ArgsTopoConfig):
+    pass
+
+
 class CertGenerator(object):
-    def __init__(self, topo_config, ca_certs):
-        self.topo_config = topo_config
+    def __init__(self, args):
+        """
+        :param CertGenArgs args: Contains the passed command line
+        arguments and the parsed topo config.
+        """
+        self.args = args
         self.core_count = defaultdict(int)
-        self.ca_certs = ca_certs
         self.sig_priv_keys = {}
         self.sig_pub_keys = {}
         self.enc_priv_keys = {}
@@ -99,7 +106,7 @@ def _self_sign_keys(self):
         self.enc_pub_keys[topo_id], self.enc_priv_keys[topo_id] = generate_enc_keypair()
 
     def _iterate(self, f):
-        for isd_as, as_conf in self.topo_config["ASes"].items():
+        for isd_as, as_conf in self.args.config["ASes"].items():
             f(TopoID(isd_as), as_conf)
 
     def _count_cores(self, topo_id, as_conf):
diff --git a/python/topology/common.py b/python/topology/common.py
index c8ea2176a6..de96f4bfd5 100644
--- a/python/topology/common.py
+++ b/python/topology/common.py
@@ -29,6 +29,32 @@
 )
 
 
+class ArgsBase:
+    def __init__(self, args):
+        for k, v in vars(args).items():
+            setattr(self, k, v)
+
+
+class ArgsTopoConfig(ArgsBase):
+    def __init__(self, args, topo_config):
+        """
+        :param ArgsBase args: Contains the passed command line arguments.
+        :param dict topo_config: The parsed topology config.
+        """
+        super().__init__(args)
+        self.config = topo_config
+
+
+class ArgsTopoDicts(ArgsBase):
+    def __init__(self, args, topo_dicts):
+        """
+        :param ArgsBase args: Contains the passed command line arguments.
+        :param dict topo_dicts: The generated topo dicts from TopoGenerator.
+        """
+        super().__init__(args)
+        self.topo_dicts = topo_dicts
+
+
 class TopoID(ISD_AS):
     def ISD(self):
         return "ISD%s" % self.isd_str()
diff --git a/python/topology/config.py b/python/topology/config.py
index e7b1df0adf..0afbfb1263 100755
--- a/python/topology/config.py
+++ b/python/topology/config.py
@@ -17,6 +17,7 @@
 =============================================
 """
 # Stdlib
+import argparse
 import base64
 import configparser
 import logging
@@ -39,9 +40,9 @@
     AS_CONF_FILE,
     DEFAULT_MTU,
     DEFAULT_SEGMENT_TTL,
-    GEN_PATH,
     DEFAULT6_NETWORK,
     DEFAULT6_PRIV_NETWORK,
+    GEN_PATH,
     NETWORKS_FILE,
     PATH_POLICY_FILE,
     PRV_NETWORKS_FILE,
@@ -53,15 +54,15 @@
     load_yaml_file,
     write_file,
 )
-from topology.ca import CAGenerator
-from topology.cert import CertGenerator
-from topology.common import _srv_iter
-from topology.docker import DockerGenerator
-from topology.go import GoGenerator
+from topology.ca import CAGenArgs, CAGenerator
+from topology.cert import CertGenArgs, CertGenerator
+from topology.common import _srv_iter, ArgsBase
+from topology.docker import DockerGenArgs, DockerGenerator
+from topology.go import GoGenArgs, GoGenerator
 from topology.net import PortGenerator, SubnetGenerator
-from topology.prometheus import PrometheusGenerator
-from topology.supervisor import SupervisorGenerator
-from topology.topo import TopoGenerator
+from topology.prometheus import PrometheusGenArgs, PrometheusGenerator
+from topology.supervisor import SupervisorGenArgs, SupervisorGenerator
+from topology.topo import TopoGenArgs, TopoGenerator
 
 DEFAULT_TOPOLOGY_FILE = "topology/Default.topo"
 DEFAULT_PATH_POLICY_FILE = "topology/PathPolicy.yml"
@@ -78,53 +79,62 @@
 GENERATE_BIND_ADDRESS = False
 
 
+class ConfigGenArgs(ArgsBase):
+
+    @classmethod
+    def create_parser(cls):
+        parser = argparse.ArgumentParser()
+        parser.add_argument('-6', '--ipv6', action='store_true',
+                            help='Generate IPv6 addresses')
+        parser.add_argument('-c', '--topo-config', default=DEFAULT_TOPOLOGY_FILE,
+                            help='Default topology config')
+        parser.add_argument('-p', '--path-policy', default=DEFAULT_PATH_POLICY_FILE,
+                            help='Path policy file')
+        parser.add_argument('-m', '--mininet', action='store_true',
+                            help='Use Mininet to create a virtual network topology')
+        parser.add_argument('-d', '--docker', action='store_true',
+                            help='Create a docker-compose configuration')
+        parser.add_argument('-n', '--network',
+                            help='Network to create subnets in (E.g. "127.0.0.0/8"')
+        parser.add_argument('-o', '--output-dir', default=GEN_PATH,
+                            help='Output directory')
+        parser.add_argument('-z', '--zk-config', default=DEFAULT_ZK_CONFIG,
+                            help='Zookeeper configuration file')
+        parser.add_argument('-b', '--bind-addr', default=GENERATE_BIND_ADDRESS,
+                            help='Generate bind addresses (E.g. "192.168.0.0/16"')
+        parser.add_argument('--pseg-ttl', type=int, default=DEFAULT_SEGMENT_TTL,
+                            help='Path segment TTL (in seconds)')
+        parser.add_argument('-cs', '--cert-server', default=DEFAULT_CERTIFICATE_SERVER,
+                            help='Certificate Server implementation to use ("go" or "py")')
+        parser.add_argument('-sd', '--sciond', default=DEFAULT_SCIOND,
+                            help='SCIOND implementation to use ("go" or "py")')
+        parser.add_argument('-ps', '--path-server', default=DEFAULT_PATH_SERVER,
+                            help='Path Server implementation to use ("go or "py")')
+        parser.add_argument('-ds', '--discovery', action='store_true',
+                            help='Generate discovery service')
+        return parser
+
+
 class ConfigGenerator(object):
     """
     Configuration and/or topology generator.
     """
-    def __init__(self, ipv6=False, out_dir=GEN_PATH, topo_file=DEFAULT_TOPOLOGY_FILE,
-                 path_policy_file=DEFAULT_PATH_POLICY_FILE,
-                 zk_config_file=DEFAULT_ZK_CONFIG, network=None,
-                 use_mininet=False, use_docker=False, bind_addr=GENERATE_BIND_ADDRESS,
-                 pseg_ttl=DEFAULT_SEGMENT_TTL, cs=DEFAULT_CERTIFICATE_SERVER,
-                 sd=DEFAULT_SCIOND, ps=DEFAULT_PATH_SERVER, ds=False):
+    def __init__(self, args):
         """
         Initialize an instance of the class ConfigGenerator.
 
-        :param string out_dir: path to the topology folder.
-        :param string topo_file: path to topology config
-        :param string path_policy_file: path to PathPolicy.yml
-        :param string zk_config_file: path to Zookeeper.yml
-        :param string network:
-            Network to create subnets in, of the form x.x.x.x/y
-        :param bool use_mininet: Use Mininet
-        :param bool use_docker: Create a docker-compose config
-        :param int pseg_ttl: The TTL for path segments (in seconds)
-        :param string cs: Use go or python implementation of certificate server
-        :param string sd: Use go or python implementation of SCIOND
-        :param string ps: Use go or python implementation of path server
-        :param bool ds: Use discovery service
+        :param ConfigGenArgs args: Contains the passed command line arguments.
         """
-        self.ipv6 = ipv6
-        self.out_dir = out_dir
-        self.topo_config = load_yaml_file(topo_file)
-        self.zk_config = load_yaml_file(zk_config_file)
-        self.path_policy_file = path_policy_file
-        self.mininet = use_mininet
-        self.docker = use_docker
-        if self.docker and self.mininet:
+        self.args = args
+        self.topo_config = load_yaml_file(args.topo_config)
+        self.zk_config = load_yaml_file(self.args.zk_config)
+        if self.args.docker and self.args.mininet:
             logging.critical("Cannot use mininet with docker!")
             sys.exit(1)
         self.default_mtu = None
-        self.gen_bind_addr = bind_addr
-        self.pseg_ttl = pseg_ttl
-        self._read_defaults(network)
+        self._read_defaults(args.network)
         self.port_gen = PortGenerator()
-        self.cs = cs
-        self.sd = sd
-        self.ps = ps
-        self.ds = ds
-        if self.docker and self.cs is not DEFAULT_CERTIFICATE_SERVER:
+        if self.args.docker and self.args.cert_server is not DEFAULT_CERTIFICATE_SERVER:
             logging.critical("Cannot use non-default CS with docker!")
             sys.exit(1)
 
@@ -137,21 +147,21 @@ def _read_defaults(self, network):
         if not def_network:
             def_network = defaults.get("subnet")
         if not def_network:
-            if self.ipv6:
+            if self.args.ipv6:
                 def_network = DEFAULT6_NETWORK
             else:
-                if self.mininet:
+                if self.args.mininet:
                     def_network = DEFAULT_MININET_NETWORK
                 else:
                     def_network = DEFAULT_NETWORK
-        if self.ipv6:
+        if self.args.ipv6:
             priv_net = DEFAULT6_PRIV_NETWORK
         else:
             priv_net = DEFAULT_PRIV_NETWORK
         self.subnet_gen = SubnetGenerator(def_network)
         self.prvnet_gen = SubnetGenerator(priv_net)
         for key, val in defaults.get("zookeepers", {}).items():
-            if self.mininet and val['addr'] == "127.0.0.1":
+            if self.args.mininet and val['addr'] == "127.0.0.1":
                 val['addr'] = "169.254.0.1"
         self.default_mtu = defaults.get("mtu", DEFAULT_MTU)
 
@@ -161,14 +171,9 @@ def generate_all(self):
         """
         self._ensure_uniq_ases()
         ca_private_key_files, ca_cert_files, ca_certs = self._generate_cas()
-        cert_files, trc_files, cust_files = self._generate_certs_trcs(ca_certs)
+        cert_files, trc_files, cust_files = self._generate_certs_trcs()
         topo_dicts, zookeepers, networks, prv_networks = self._generate_topology()
-        self._generate_go(topo_dicts)
-        if self.docker:
-            self._generate_docker(topo_dicts)
-        else:
-            self._generate_supervisor(topo_dicts)
-        self._generate_prom_conf(topo_dicts)
+        self._generate_with_topo(topo_dicts)
         self._write_ca_files(topo_dicts, ca_private_key_files)
         self._write_ca_files(topo_dicts, ca_cert_files)
         self._write_trust_files(topo_dicts, cert_files)
@@ -177,7 +182,7 @@ def generate_all(self):
         self._write_conf_policies(topo_dicts)
         self._write_master_keys(topo_dicts)
         self._write_networks_conf(networks, NETWORKS_FILE)
-        if self.gen_bind_addr:
+        if self.args.bind_addr:
             self._write_networks_conf(prv_networks, PRV_NETWORKS_FILE)
 
     def _ensure_uniq_ases(self):
@@ -189,62 +194,91 @@ def _ensure_uniq_ases(self):
                 sys.exit(1)
             seen.add(ia[1])
 
+    def _generate_with_topo(self, topo_dicts):
+        self._generate_go(topo_dicts)
+        if self.args.docker:
+            self._generate_docker(topo_dicts)
+        else:
+            self._generate_supervisor(topo_dicts)
+        self._generate_prom_conf(topo_dicts)
+
     def _generate_cas(self):
-        ca_gen = CAGenerator(self.topo_config)
+        ca_gen = CAGenerator(self._ca_args())
         return ca_gen.generate()
 
-    def _generate_certs_trcs(self, ca_certs):
-        certgen = CertGenerator(self.topo_config, ca_certs)
+    def _ca_args(self):
+        return CAGenArgs(self.args, self.topo_config)
+
+    def _generate_certs_trcs(self):
+        certgen = CertGenerator(self._cert_args())
         return certgen.generate()
 
+    def _cert_args(self):
+        return CertGenArgs(self.args, self.topo_config)
+
     def _generate_go(self, topo_dicts):
-        go_gen = GoGenerator(self.out_dir, topo_dicts, self.docker)
-        if self.cs == "go":
+        args = self._go_args(topo_dicts)
+        go_gen = GoGenerator(args)
+        if self.args.cert_server == "go":
             go_gen.generate_cs()
-        if self.sd == "go":
+        if self.args.sciond == "go":
             go_gen.generate_sciond()
-        if self.ps == "go":
+        if self.args.path_server == "go":
             go_gen.generate_ps()
 
+    def _go_args(self, topo_dicts):
+        return GoGenArgs(self.args, topo_dicts)
+
     def _generate_topology(self):
-        topo_gen = TopoGenerator(
-            self.topo_config, self.out_dir, self.subnet_gen, self.prvnet_gen, self.zk_config,
-            self.default_mtu, self.gen_bind_addr, self.docker, self.ipv6, self.cs, self.ps,
-            self.ds, self.port_gen)
+        topo_gen = TopoGenerator(self._topo_args())
         return topo_gen.generate()
 
+    def _topo_args(self):
+        return TopoGenArgs(self.args, self.topo_config, self.zk_config, self.subnet_gen,
+                           self.prvnet_gen, self.default_mtu, self.port_gen)
+
     def _generate_supervisor(self, topo_dicts):
-        super_gen = SupervisorGenerator(
-            self.out_dir, topo_dicts, self.mininet, self.cs, self.sd, self.ps, self.port_gen)
+        args = self._supervisor_args(topo_dicts)
+        super_gen = SupervisorGenerator(args)
         super_gen.generate()
 
+    def _supervisor_args(self, topo_dicts):
+        return SupervisorGenArgs(self.args, topo_dicts, self.port_gen)
+
     def _generate_docker(self, topo_dicts):
-        docker_gen = DockerGenerator(
-            self.out_dir, topo_dicts, self.sd, self.ps, self.port_gen)
+        args = self._docker_args(topo_dicts)
+        docker_gen = DockerGenerator(args)
         docker_gen.generate()
 
+    def _docker_args(self, topo_dicts):
+        return DockerGenArgs(self.args, topo_dicts, self.port_gen)
+
     def _generate_prom_conf(self, topo_dicts):
-        prom_gen = PrometheusGenerator(self.out_dir, topo_dicts, self.port_gen)
+        args = self._prometheus_args(topo_dicts)
+        prom_gen = PrometheusGenerator(args)
         prom_gen.generate()
 
+    def _prometheus_args(self, topo_dicts):
+        return PrometheusGenArgs(self.args, topo_dicts, self.port_gen)
+
     def _write_ca_files(self, topo_dicts, ca_files):
         isds = set()
         for topo_id, as_topo in topo_dicts.items():
             isds.add(topo_id[0])
         for isd in isds:
-            base = os.path.join(self.out_dir, "CAS")
+            base = os.path.join(self.args.output_dir, "CAS")
             for path, value in ca_files[int(isd)].items():
                 write_file(os.path.join(base, path), value.decode())
 
     def _write_trust_files(self, topo_dicts, cert_files):
         for topo_id, as_topo, base in _srv_iter(
-                topo_dicts, self.out_dir, common=True):
+                topo_dicts, self.args.output_dir, common=True):
             for path, value in cert_files[topo_id].items():
                 write_file(os.path.join(base, path), value + '\n')
 
     def _write_cust_files(self, topo_dicts, cust_files):
         for topo_id, as_topo in topo_dicts.items():
-            base = topo_id.base_dir(self.out_dir)
+            base = topo_id.base_dir(self.args.output_dir)
             for elem in as_topo["CertificateService"]:
                 for path, value in cust_files[topo_id].items():
                     write_file(os.path.join(base, elem, path), value)
@@ -255,17 +289,17 @@ def _write_conf_policies(self, topo_dicts):
         """
         as_confs = {}
         for topo_id, as_topo, base in _srv_iter(
-                topo_dicts, self.out_dir, common=True):
+                topo_dicts, self.args.output_dir, common=True):
             as_confs.setdefault(topo_id, yaml.dump(
                 self._gen_as_conf(as_topo), default_flow_style=False))
             conf_file = os.path.join(base, AS_CONF_FILE)
             write_file(conf_file, as_confs[topo_id])
             # Confirm that config parses cleanly.
             Config.from_file(conf_file)
-            copy_file(self.path_policy_file,
+            copy_file(self.args.path_policy,
                       os.path.join(base, PATH_POLICY_FILE))
         # Confirm that parser actually works on path policy file
-        PathPolicy.from_file(self.path_policy_file)
+        PathPolicy.from_file(self.args.path_policy)
 
     def _gen_as_conf(self, as_topo):
         return {
@@ -274,7 +308,7 @@ def _gen_as_conf(self, as_topo):
             'CertChainVersion': 0,
             # FIXME(kormat): This seems to always be true..:
             'RegisterPath': True if as_topo["PathService"] else False,
-            'PathSegmentTTL': self.pseg_ttl,
+            'PathSegmentTTL': self.args.pseg_ttl,
         }
 
     def _write_master_keys(self, topo_dicts):
@@ -283,7 +317,7 @@ def _write_master_keys(self, topo_dicts):
         """
         master_keys = {}
         for topo_id, as_topo, base in _srv_iter(
-                topo_dicts, self.out_dir, common=True):
+                topo_dicts, self.args.output_dir, common=True):
 
             master_keys.setdefault(topo_id, self._gen_master_keys())
             write_file(get_master_key_file_path(base, MASTER_KEY_0),
@@ -306,4 +340,4 @@ def _write_networks_conf(self, networks, out_file):
             config[net] = sub_conf
         text = StringIO()
         config.write(text)
-        write_file(os.path.join(self.out_dir, out_file), text.getvalue())
+        write_file(os.path.join(self.args.output_dir, out_file), text.getvalue())
diff --git a/python/topology/docker.py b/python/topology/docker.py
index eb7ecca843..e457359b6e 100644
--- a/python/topology/docker.py
+++ b/python/topology/docker.py
@@ -27,21 +27,24 @@
     read_file,
     write_file,
 )
-from topology.common import _prom_addr_br, _prom_addr_infra
-from topology.utils import TesterGenerator
+from topology.common import _prom_addr_br, _prom_addr_infra, ArgsTopoDicts
+from topology.utils import TesterGenArgs, TesterGenerator
 
 DOCKER_BASE_CONF = 'base-dc.yml'
 DOCKER_SCION_CONF = 'scion-dc.yml'
 DEFAULT_DOCKER_NETWORK = "172.18.0.0/24"
 
 
+class DockerGenArgs(ArgsTopoDicts):
+    pass
+
+
 class DockerGenerator(object):
-    def __init__(self, out_dir, topo_dicts, sd, ps, port_gen):
-        self.out_dir = out_dir
-        self.topo_dicts = topo_dicts
-        self.sd = sd
-        self.ps = ps
-        self.port_gen = port_gen
+    def __init__(self, args):
+        """
+        :param DockerGenArgs args: Contains the passed command line arguments and topo dicts.
+        """
+        self.args = args
         self.dc_base_conf = {'version': '3', 'networks': {}}
         self.dc_conf = {'version': '3', 'services': {}}
         self.dc_util_conf = {'version': '3', 'services': {}}
@@ -52,17 +55,20 @@ def generate(self):
         self._base_conf()
         self._zookeeper_conf()
         self._dispatcher_conf()
-        for topo_id, topo in self.topo_dicts.items():
-            base = os.path.join(self.output_base, topo_id.base_dir(self.out_dir))
+        for topo_id, topo in self.args.topo_dicts.items():
+            base = os.path.join(self.output_base, topo_id.base_dir(self.args.output_dir))
             self._gen_topo(topo_id, topo, base)
-        write_file(os.path.join(self.out_dir, DOCKER_SCION_CONF),
+        write_file(os.path.join(self.args.output_dir, DOCKER_SCION_CONF),
                    yaml.dump(self.dc_conf, default_flow_style=False))
-        write_file(os.path.join(self.out_dir, DOCKER_BASE_CONF),
+        write_file(os.path.join(self.args.output_dir, DOCKER_BASE_CONF),
                    yaml.dump(self.dc_base_conf, default_flow_style=False))
 
-        tester_gen = TesterGenerator(self.out_dir)
+        tester_gen = TesterGenerator(self._tester_args())
         tester_gen.generate()
 
+    def _tester_args(self):
+        return TesterGenArgs(self.args)
+
     def _base_conf(self):
         default_net = {'ipam': {'config': [{'subnet': DEFAULT_DOCKER_NETWORK}]}}
         self.dc_base_conf['networks']['default'] = default_net
@@ -97,7 +103,7 @@ def _br_conf(self, topo, base):
             entry['container_name'] = k
             entry['volumes'].append('%s:/share/conf:ro' % os.path.join(base, k))
             entry['command'].append('-id=%s' % k)
-            entry['command'].append('-prom=%s' % _prom_addr_br(k, v, self.port_gen))
+            entry['command'].append('-prom=%s' % _prom_addr_br(k, v, self.args.port_gen))
             self.dc_conf['services'][k] = entry
 
     def _cs_conf(self, topo_id, topo, base):
@@ -127,7 +133,7 @@ def _cs_conf(self, topo_id, topo, base):
             entry = copy.deepcopy(raw_entry)
             entry['container_name'] = k
             entry['volumes'].append('%s:/share/conf:ro' % os.path.join(base, k))
-            entry['command'].append('--prom=%s' % _prom_addr_infra(k, v, self.port_gen))
+            entry['command'].append('--prom=%s' % _prom_addr_infra(k, v, self.args.port_gen))
             entry['command'].append('--sciond_path=%s' %
                                     get_default_sciond_path(ISD_AS(topo["ISD_AS"])))
             entry['command'].append(k)
@@ -161,7 +167,7 @@ def _bs_conf(self, topo_id, topo, base):
             entry = copy.deepcopy(raw_entry)
             entry['container_name'] = k
             entry['volumes'].append('%s:/share/conf:ro' % os.path.join(base, k))
-            entry['command'].append('--prom=%s' % _prom_addr_infra(k, v, self.port_gen))
+            entry['command'].append('--prom=%s' % _prom_addr_infra(k, v, self.args.port_gen))
             entry['command'].append('--sciond_path=%s' %
                                     get_default_sciond_path(ISD_AS(topo["ISD_AS"])))
             entry['command'].append(k)
@@ -169,7 +175,7 @@ def _bs_conf(self, topo_id, topo, base):
             self.dc_conf['services'][k] = entry
 
     def _ps_conf(self, topo_id, topo, base):
-        image = 'scion_path_py' if self.ps == 'py' else 'scion_path'
+        image = 'scion_path_py' if self.args.path_server == 'py' else 'scion_path'
         raw_entry = {
             'image': image,
             'depends_on': [
@@ -194,9 +200,9 @@ def _ps_conf(self, topo_id, topo, base):
             entry = copy.deepcopy(raw_entry)
             entry['container_name'] = k
             entry['volumes'].append('%s:/share/conf:ro' % os.path.join(base, k))
-            if self.ps == 'py':
+            if self.args.path_server == 'py':
                 entry['command'].append('--spki_cache_dir=cache')
-                entry['command'].append('--prom=%s' % _prom_addr_infra(k, v, self.port_gen))
+                entry['command'].append('--prom=%s' % _prom_addr_infra(k, v, self.args.port_gen))
                 entry['command'].append('--sciond_path=%s' %
                                         get_default_sciond_path(ISD_AS(topo["ISD_AS"])))
                 entry['command'].append(k)
@@ -216,7 +222,8 @@ def _zookeeper_conf(self):
             'volumes': [
                 '/etc/passwd:/etc/passwd:ro',
                 '/etc/group:/etc/group:ro',
-                os.path.join(self.output_base, self.out_dir, cfg_file) + ':/conf/zoo.cfg:rw',
+                os.path.join(
+                    self.output_base, self.args.output_dir, cfg_file) + ':/conf/zoo.cfg:rw',
                 '/var/lib/docker-zk:/var/lib/zookeeper:rw',
                 '/run/shm/docker-zk:/dev/shm/zookeeper:rw'
             ],
@@ -225,7 +232,7 @@ def _zookeeper_conf(self):
             ]
         }
         self.dc_conf['services']['zookeeper'] = entry
-        cfg_path = os.path.join(self.out_dir, cfg_file)
+        cfg_path = os.path.join(self.args.output_dir, cfg_file)
         os.makedirs(os.path.dirname(cfg_path))
         copyfile(os.path.join(os.environ['PWD'], cfg_file), cfg_path)
 
@@ -249,12 +256,12 @@ def _dispatcher_conf(self):
 
         # Create dispatcher config
         tmpl = Template(read_file("topology/zlog.tmpl"))
-        cfg = self.out_dir + "/dispatcher/dispatcher.zlog.conf"
+        cfg = self.args.output_dir + "/dispatcher/dispatcher.zlog.conf"
         write_file(cfg, tmpl.substitute(name="dispatcher", elem="dispatcher"))
 
     def _sciond_conf(self, topo_id, base):
         name = self._sciond_name(topo_id)
-        image = 'scion_sciond_py' if self.sd == 'py' else 'scion_sciond'
+        image = 'scion_sciond_py' if self.args.sciond == 'py' else 'scion_sciond'
         entry = {
             'image': image,
             'container_name': name,
@@ -274,7 +281,7 @@ def _sciond_conf(self, topo_id, base):
                 self.output_base + '/logs:/share/logs:rw'
             ],
         }
-        if self.sd == 'py':
+        if self.args.sciond == 'py':
             entry['command'] = [
                 '--api-addr=%s' % os.path.join(SCIOND_API_SOCKDIR, "%s.sock" % name),
                 '--log_dir=logs',
diff --git a/python/topology/generator.py b/python/topology/generator.py
index 6aac188bd9..6908266ae2 100755
--- a/python/topology/generator.py
+++ b/python/topology/generator.py
@@ -17,23 +17,10 @@
 :mod:`generator` --- SCION topology generator
 =============================================
 """
-# Stdlib
-import argparse
-
 # SCION
-from lib.defines import (
-    DEFAULT_SEGMENT_TTL,
-    GEN_PATH,
-)
 from topology.config import (
     ConfigGenerator,
-    DEFAULT_CERTIFICATE_SERVER,
-    DEFAULT_SCIOND,
-    DEFAULT_PATH_POLICY_FILE,
-    DEFAULT_PATH_SERVER,
-    DEFAULT_TOPOLOGY_FILE,
-    DEFAULT_ZK_CONFIG,
-    GENERATE_BIND_ADDRESS,
+    ConfigGenArgs,
 )
 
 
@@ -41,40 +28,9 @@ def main():
     """
     Main function.
     """
-    parser = argparse.ArgumentParser()
-    parser.add_argument('-6', '--ipv6', action='store_true',
-                        help='Generate IPv6 addresses')
-    parser.add_argument('-c', '--topo-config', default=DEFAULT_TOPOLOGY_FILE,
-                        help='Default topology config')
-    parser.add_argument('-p', '--path-policy', default=DEFAULT_PATH_POLICY_FILE,
-                        help='Path policy file')
-    parser.add_argument('-m', '--mininet', action='store_true',
-                        help='Use Mininet to create a virtual network topology')
-    parser.add_argument('-d', '--docker', action='store_true',
-                        help='Create a docker-compose configuration')
-    parser.add_argument('-n', '--network',
-                        help='Network to create subnets in (E.g. "127.0.0.0/8"')
-    parser.add_argument('-o', '--output-dir', default=GEN_PATH,
-                        help='Output directory')
-    parser.add_argument('-z', '--zk-config', default=DEFAULT_ZK_CONFIG,
-                        help='Zookeeper configuration file')
-    parser.add_argument('-b', '--bind-addr', default=GENERATE_BIND_ADDRESS,
-                        help='Generate bind addresses (E.g. "192.168.0.0/16"')
-    parser.add_argument('--pseg-ttl', type=int, default=DEFAULT_SEGMENT_TTL,
-                        help='Path segment TTL (in seconds)')
-    parser.add_argument('-cs', '--cert-server', default=DEFAULT_CERTIFICATE_SERVER,
-                        help='Certificate Server implementation to use ("go" or "py")')
-    parser.add_argument('-sd', '--sciond', default=DEFAULT_SCIOND,
-                        help='SCIOND implementation to use ("go" or "py")')
-    parser.add_argument('-ps', '--path-server', default=DEFAULT_PATH_SERVER,
-                        help='Path Server implementation to use ("go or "py")')
-    parser.add_argument('-ds', '--discovery', action='store_true',
-                        help='Generate discovery service')
-    args = parser.parse_args()
-    confgen = ConfigGenerator(
-        args.ipv6, args.output_dir, args.topo_config, args.path_policy, args.zk_config,
-        args.network, args.mininet, args.docker, args.bind_addr, args.pseg_ttl, args.cert_server,
-        args.sciond, args.path_server, args.discovery)
+    parser = ConfigGenArgs.create_parser()
+    args = ConfigGenArgs(parser.parse_args())
+    confgen = ConfigGenerator(args)
     confgen.generate_all()
 
 
diff --git a/python/topology/go.py b/python/topology/go.py
index 7a67b0d92f..ee4a76754a 100755
--- a/python/topology/go.py
+++ b/python/topology/go.py
@@ -24,28 +24,33 @@
 from lib.app.sciond import get_default_sciond_path
 from lib.defines import SCIOND_API_SOCKDIR
 from lib.util import write_file
-from topology.common import COMMON_DIR
+from topology.common import ArgsTopoDicts, COMMON_DIR
+
+
+class GoGenArgs(ArgsTopoDicts):
+    pass
 
 
 class GoGenerator(object):
-    def __init__(self, out_dir, topo_dicts, docker):
-        self.out_dir = out_dir
-        self.topo_dicts = topo_dicts
-        self.docker = docker
+    def __init__(self, args):
+        """
+        :param GoGenArgs args: Contains the passed command line arguments and topo dicts.
+        """
+        self.args = args
 
     def generate_ps(self):
-        for topo_id, topo in self.topo_dicts.items():
+        for topo_id, topo in self.args.topo_dicts.items():
             for k, v in topo.get("PathService", {}).items():
                 # only a single Go-PS per AS is currently supported
                 if k.endswith("-1"):
-                    base = topo_id.base_dir(self.out_dir)
+                    base = topo_id.base_dir(self.args.output_dir)
                     ps_conf = self._build_ps_conf(topo_id, topo["ISD_AS"], base, k)
                     write_file(os.path.join(base, k, "psconfig.toml"), toml.dumps(ps_conf))
 
     def _build_ps_conf(self, topo_id, ia, base, name):
-        config_dir = '/share/conf' if self.docker else os.path.join(base, name)
-        log_dir = '/share/logs' if self.docker else 'logs'
-        db_dir = '/share/cache' if self.docker else 'gen-cache'
+        config_dir = '/share/conf' if self.args.docker else os.path.join(base, name)
+        log_dir = '/share/logs' if self.args.docker else 'logs'
+        db_dir = '/share/cache' if self.args.docker else 'gen-cache'
         raw_entry = {
             'general': {
                 'ID': name,
@@ -78,16 +83,16 @@ def _build_ps_conf(self, topo_id, ia, base, name):
         return raw_entry
 
     def generate_sciond(self):
-        for topo_id, topo in self.topo_dicts.items():
-            base = topo_id.base_dir(self.out_dir)
+        for topo_id, topo in self.args.topo_dicts.items():
+            base = topo_id.base_dir(self.args.output_dir)
             sciond_conf = self._build_sciond_conf(topo_id, topo["ISD_AS"], base)
             write_file(os.path.join(base, COMMON_DIR, "sciond.toml"), toml.dumps(sciond_conf))
 
     def _build_sciond_conf(self, topo_id, ia, base):
         name = self._sciond_name(topo_id)
-        config_dir = '/share/conf' if self.docker else os.path.join(base, COMMON_DIR)
-        log_dir = '/share/logs' if self.docker else 'logs'
-        db_dir = '/share/cache' if self.docker else 'gen-cache'
+        config_dir = '/share/conf' if self.args.docker else os.path.join(base, COMMON_DIR)
+        log_dir = '/share/logs' if self.args.docker else 'logs'
+        db_dir = '/share/cache' if self.args.docker else 'gen-cache'
         raw_entry = {
             'general': {
                 'ID': name,
@@ -121,18 +126,18 @@ def _sciond_name(self, topo_id):
         return 'sd' + topo_id.file_fmt()
 
     def generate_cs(self):
-        for topo_id, topo in self.topo_dicts.items():
+        for topo_id, topo in self.args.topo_dicts.items():
             for k, v in topo.get("CertificateService", {}).items():
                 # only a single Go-CS per AS is currently supported
                 if k.endswith("-1"):
-                    base = topo_id.base_dir(self.out_dir)
+                    base = topo_id.base_dir(self.args.output_dir)
                     cs_conf = self._build_cs_conf(topo_id, topo["ISD_AS"], base, k)
                     write_file(os.path.join(base, k, "csconfig.toml"), toml.dumps(cs_conf))
 
     def _build_cs_conf(self, topo_id, ia, base, name):
-        config_dir = '/share/conf' if self.docker else os.path.join(base, name)
-        log_dir = '/share/logs' if self.docker else 'logs'
-        db_dir = '/share/cache' if self.docker else 'gen-cache'
+        config_dir = '/share/conf' if self.args.docker else os.path.join(base, name)
+        log_dir = '/share/logs' if self.args.docker else 'logs'
+        db_dir = '/share/cache' if self.args.docker else 'gen-cache'
         raw_entry = {
             'general': {
                 'ID': name,
diff --git a/python/topology/prometheus.py b/python/topology/prometheus.py
index 8dd8687a12..1f2407aa41 100755
--- a/python/topology/prometheus.py
+++ b/python/topology/prometheus.py
@@ -26,7 +26,11 @@
 # SCION
 from lib.defines import PROM_FILE
 from lib.util import write_file
-from topology.common import _prom_addr_br, _prom_addr_infra
+from topology.common import _prom_addr_br, _prom_addr_infra, ArgsTopoDicts
+
+
+class PrometheusGenArgs(ArgsTopoDicts):
+    pass
 
 
 class PrometheusGenerator(object):
@@ -44,27 +48,28 @@ class PrometheusGenerator(object):
         "PathService": "PS",
     }
 
-    def __init__(self, out_dir, topo_dicts, port_gen):
-        self.out_dir = out_dir
-        self.topo_dicts = topo_dicts
-        self.port_gen = port_gen
+    def __init__(self, args):
+        """
+        :param PrometheusGenArgs args: Contains the passed command line arguments and topo dicts.
+        """
+        self.args = args
 
     def generate(self):
         config_dict = {}
-        for topo_id, as_topo in self.topo_dicts.items():
+        for topo_id, as_topo in self.args.topo_dicts.items():
             ele_dict = defaultdict(list)
             for br_id, br_ele in as_topo["BorderRouters"].items():
-                ele_dict["BorderRouters"].append(_prom_addr_br(br_id, br_ele, self.port_gen))
+                ele_dict["BorderRouters"].append(_prom_addr_br(br_id, br_ele, self.args.port_gen))
             for svc_type in ["BeaconService", "PathService", "CertificateService"]:
                 for elem_id, elem in as_topo[svc_type].items():
-                    ele_dict[svc_type].append(_prom_addr_infra(elem_id, elem, self.port_gen))
+                    ele_dict[svc_type].append(_prom_addr_infra(elem_id, elem, self.args.port_gen))
             config_dict[topo_id] = ele_dict
         self._write_config_files(config_dict)
 
     def _write_config_files(self, config_dict):
         targets_paths = defaultdict(list)
         for topo_id, ele_dict in config_dict.items():
-            base = topo_id.base_dir(self.out_dir)
+            base = topo_id.base_dir(self.args.output_dir)
             as_local_targets_path = {}
             for ele_type, target_list in ele_dict.items():
                 targets_path = os.path.join(base, self.PROM_DIR, self.TARGET_FILES[ele_type])
@@ -72,7 +77,7 @@ def _write_config_files(self, config_dict):
                 as_local_targets_path[self.JOB_NAMES[ele_type]] = [targets_path]
                 self._write_target_file(base, target_list, ele_type)
             self._write_config_file(os.path.join(base, PROM_FILE), as_local_targets_path)
-        self._write_config_file(os.path.join(self.out_dir, PROM_FILE), targets_paths)
+        self._write_config_file(os.path.join(self.args.output_dir, PROM_FILE), targets_paths)
 
     def _write_config_file(self, config_path, job_dict):
         scrape_configs = []
diff --git a/python/topology/supervisor.py b/python/topology/supervisor.py
index ed284197d7..80f8c4c506 100755
--- a/python/topology/supervisor.py
+++ b/python/topology/supervisor.py
@@ -28,25 +28,26 @@
 from lib.defines import SCIOND_API_SOCKDIR
 from lib.packet.scion_addr import ISD_AS
 from lib.util import read_file, write_file
-from topology.common import _prom_addr_br, _prom_addr_infra, COMMON_DIR
+from topology.common import _prom_addr_br, _prom_addr_infra, ArgsTopoDicts, COMMON_DIR
 
 SUPERVISOR_CONF = 'supervisord.conf'
 
 
+class SupervisorGenArgs(ArgsTopoDicts):
+    pass
+
+
 class SupervisorGenerator(object):
-    def __init__(self, out_dir, topo_dicts, mininet, cs, sd, ps, port_gen):
-        self.out_dir = out_dir
-        self.topo_dicts = topo_dicts
-        self.mininet = mininet
-        self.cs = cs
-        self.sd = sd
-        self.ps = ps
-        self.port_gen = port_gen
+    def __init__(self, args):
+        """
+        :param SupervisorGenArgs args: Contains the passed command line arguments and topo dicts.
+        """
+        self.args = args
 
     def generate(self):
         self._write_dispatcher_conf()
-        for topo_id, topo in self.topo_dicts.items():
-            base = topo_id.base_dir(self.out_dir)
+        for topo_id, topo in self.args.topo_dicts.items():
+            base = topo_id.base_dir(self.args.output_dir)
             entries = self._as_conf(topo, base)
             self._write_as_conf(topo_id, entries)
 
@@ -62,7 +63,7 @@ def _std_entries(self, topo, topo_key, cmd, base):
         entries = []
         for elem_id, elem in topo.get(topo_key, {}).items():
             conf_dir = os.path.join(base, elem_id)
-            prom_addr = _prom_addr_infra(elem_id, elem, self.port_gen)
+            prom_addr = _prom_addr_infra(elem_id, elem, self.args.port_gen)
             entries.append((elem_id, [cmd, "--prom", prom_addr, "--sciond_path",
                                       get_default_sciond_path(ISD_AS(topo["ISD_AS"])),
                                       elem_id, conf_dir]))
@@ -73,14 +74,14 @@ def _br_entries(self, topo, cmd, base):
         for k, v in topo.get("BorderRouters", {}).items():
             conf_dir = os.path.join(base, k)
             entries.append((k, [cmd, "-id=%s" % k, "-confd=%s" % conf_dir,
-                                "-prom=%s" % _prom_addr_br(k, v, self.port_gen)]))
+                                "-prom=%s" % _prom_addr_br(k, v, self.args.port_gen)]))
         return entries
 
     def _bs_entries(self, topo, base):
         return self._std_entries(topo, "BeaconService", "python/bin/beacon_server", base)
 
     def _cs_entries(self, topo, base):
-        if self.cs == "py":
+        if self.args.cert_server == "py":
             return self._std_entries(topo, "CertificateService", "python/bin/cert_server", base)
         entries = []
         for k, v in topo.get("CertificateService", {}).items():
@@ -91,7 +92,7 @@ def _cs_entries(self, topo, base):
         return entries
 
     def _ps_entries(self, topo, base):
-        if self.ps == "py":
+        if self.args.path_server == "py":
             return self._std_entries(topo, "PathService", "python/bin/path_server", base)
         entries = []
         for k, v in topo.get("PathService", {}).items():
@@ -103,7 +104,7 @@ def _ps_entries(self, topo, base):
 
     def _sciond_entry(self, name, conf_dir):
         path = self._sciond_path(name)
-        if self.sd == "py":
+        if self.args.sciond == "py":
             return self._common_entry(
                 name, ["python/bin/sciond", "--api-addr", path, name, conf_dir])
         return self._common_entry(
@@ -115,15 +116,15 @@ def _sciond_path(self, name):
     def _write_as_conf(self, topo_id, entries):
         config = configparser.ConfigParser(interpolation=None)
         names = []
-        base = topo_id.base_dir(self.out_dir)
+        base = topo_id.base_dir(self.args.output_dir)
         for elem, entry in sorted(entries, key=lambda x: x[0]):
             names.append(elem)
             elem_dir = os.path.join(base, elem)
             self._write_elem_conf(elem, entry, elem_dir, topo_id)
-            if self.mininet:
+            if self.args.mininet:
                 self._write_elem_mininet_conf(elem, elem_dir)
         # Mininet runs sciond per element, and not at an AS level.
-        if not self.mininet:
+        if not self.args.mininet:
             sd_name = "sd%s" % topo_id.file_fmt()
             names.append(sd_name)
             conf_dir = os.path.join(base, COMMON_DIR)
@@ -132,14 +133,14 @@ def _write_as_conf(self, topo_id, entries):
         config["group:as%s" % topo_id.file_fmt()] = {"programs": ",".join(names)}
         text = StringIO()
         config.write(text)
-        conf_path = os.path.join(topo_id.base_dir(self.out_dir), SUPERVISOR_CONF)
+        conf_path = os.path.join(topo_id.base_dir(self.args.output_dir), SUPERVISOR_CONF)
         write_file(conf_path, text.getvalue())
 
     def _write_elem_conf(self, elem, entry, elem_dir, topo_id=None):
         config = configparser.ConfigParser(interpolation=None)
         prog = self._common_entry(elem, entry, elem_dir)
         self._write_zlog_cfg(os.path.basename(entry[0]), elem, elem_dir)
-        if self.mininet and not elem.startswith("br"):
+        if self.args.mininet and not elem.startswith("br"):
             # Start a dispatcher for every non-BR element under mininet.
             prog['environment'] += ',DISPATCHER_ID="%s"' % elem
             dp_name = "dp-" + elem
@@ -148,7 +149,7 @@ def _write_elem_conf(self, elem, entry, elem_dir, topo_id=None):
             config["program:%s" % dp_name] = dp
             self._write_zlog_cfg("dispatcher", dp_name, elem_dir)
         if elem.startswith("cs"):
-            if self.mininet:
+            if self.args.mininet:
                 # Start a sciond for every CS element under mininet.
                 sd_name = "sd-" + elem
                 config["program:%s" % sd_name] = self._sciond_entry(
@@ -162,10 +163,10 @@ def _write_elem_conf(self, elem, entry, elem_dir, topo_id=None):
 
     def _write_elem_mininet_conf(self, elem, elem_dir):
         tmpl = Template(read_file("python/mininet/supervisord.conf"))
-        mn_conf_path = os.path.join(self.out_dir, "mininet", "%s.conf" % elem)
+        mn_conf_path = os.path.join(self.args.output_dir, "mininet", "%s.conf" % elem)
         rel_conf_path = os.path.relpath(
             os.path.join(elem_dir, SUPERVISOR_CONF),
-            os.path.join(self.out_dir, "mininet")
+            os.path.join(self.args.output_dir, "mininet")
         )
         write_file(mn_conf_path,
                    tmpl.substitute(elem=elem, conf_path=rel_conf_path,
@@ -178,12 +179,12 @@ def _write_zlog_cfg(self, name, elem, elem_dir):
 
     def _write_dispatcher_conf(self):
         elem = "dispatcher"
-        elem_dir = os.path.join(self.out_dir, elem)
+        elem_dir = os.path.join(self.args.output_dir, elem)
         self._write_elem_conf(elem, ["bin/dispatcher"], elem_dir)
 
     def _common_entry(self, name, cmd_args, elem_dir=None):
         entry = {
-            'autostart': 'false' if self.mininet else 'false',
+            'autostart': 'false' if self.args.mininet else 'false',
             'autorestart': 'false',
             'environment': 'PYTHONPATH=python/:.,TZ=UTC',
             'stdout_logfile': "NONE",
@@ -199,7 +200,7 @@ def _common_entry(self, name, cmd_args, elem_dir=None):
         if name == "dispatcher":
             entry['startsecs'] = 1
             entry['priority'] = 50
-        if self.mininet:
+        if self.args.mininet:
             entry['autostart'] = 'true'
         return entry
 
diff --git a/python/topology/topo.py b/python/topology/topo.py
index ab1a447b26..f231a0b9a9 100755
--- a/python/topology/topo.py
+++ b/python/topology/topo.py
@@ -39,7 +39,7 @@
 from lib.topology import Topology
 from lib.types import LinkType
 from lib.util import write_file
-from topology.common import _srv_iter, TopoID, SCION_SERVICE_NAMES
+from topology.common import _srv_iter, ArgsBase, TopoID, SCION_SERVICE_NAMES
 from topology.net import AddressProxy
 
 DEFAULT_LINK_BW = 1000
@@ -53,17 +53,32 @@
 ZOOKEEPER_ADDR = "172.18.0.1"
 
 
-class TopoGenerator(object):
-    def __init__(self, topo_config, out_dir, subnet_gen, prvnet_gen, zk_config,
-                 default_mtu, gen_bind_addr, docker, ipv6, cs, ps, ds, port_gen):
-        self.topo_config = topo_config
-        self.out_dir = out_dir
+class TopoGenArgs(ArgsBase):
+    def __init__(self, args, topo_config, zk_config, subnet_gen, privnet_gen, default_mtu, port_gen):
+        """
+        :param ArgsBase args: Contains the passed command line arguments.
+        :param dict topo_config: The parsed topology config.
+        :param dict zk_config: The parsed zookeeper config.
+        :param SubnetGenerator subnet_gen: The default network generator.
+        :param SubnetGenerator privnet_gen: The private network generator.
+        :param dict default_mtu: The default mtu.
+        :param PortGenerator port_gen: The port generator
+        """
+        super().__init__(args)
+        self.topo_config_dict = topo_config
+        self.zk_config_dict = zk_config
         self.subnet_gen = subnet_gen
-        self.prvnet_gen = prvnet_gen
-        self.zk_config = zk_config
+        self.privnet_gen = privnet_gen
         self.default_mtu = default_mtu
-        self.gen_bind_addr = gen_bind_addr
-        self.docker = docker
+        self.port_gen = port_gen
+
+
+class TopoGenerator(object):
+    def __init__(self, args):
+        """
+        :param TopoGenArgs args: Contains the passed command line arguments.
+        """
+        self.args = args
         self.topo_dicts = {}
         self.hosts = []
         self.zookeepers = defaultdict(dict)
@@ -71,41 +86,37 @@ def __init__(self, topo_config, out_dir, subnet_gen, prvnet_gen, zk_config,
         self.as_list = defaultdict(list)
         self.links = defaultdict(list)
         self.ifid_map = {}
-        if ipv6:
+        if args.ipv6:
             self.overlay = "UDP/IPv6"
             self.addr_type = "IPv6"
         else:
             self.overlay = "UDP/IPv4"
             self.addr_type = "IPv4"
-        self.cs = cs
-        self.ps = ps
-        self.ds = ds
-        self.port_gen = port_gen
 
     def _reg_addr(self, topo_id, elem_id):
-        subnet = self.subnet_gen.register(topo_id)
+        subnet = self.args.subnet_gen.register(topo_id)
         return subnet.register(elem_id)
 
     def _reg_bind_addr(self, topo_id, elem_id):
-        prvnet = self.prvnet_gen.register(topo_id)
+        prvnet = self.args.privnet_gen.register(topo_id)
         return prvnet.register(elem_id)
 
     def _reg_link_addrs(self, local_br, remote_br, local_ifid, remote_ifid):
         link_name = str(sorted((local_br, remote_br)))
         link_name += str(sorted((local_ifid, remote_ifid)))
-        subnet = self.subnet_gen.register(link_name)
+        subnet = self.args.subnet_gen.register(link_name)
         return subnet.register(local_br), subnet.register(remote_br)
 
     def _iterate(self, f):
-        for isd_as, as_conf in self.topo_config["ASes"].items():
+        for isd_as, as_conf in self.args.topo_config_dict["ASes"].items():
             f(TopoID(isd_as), as_conf)
 
     def generate(self):
         self._read_links()
         self._iterate(self._generate_as_topo)
         self._iterate(self._generate_as_list)
-        networks = self.subnet_gen.alloc_subnets()
-        prv_networks = self.prvnet_gen.alloc_subnets()
+        networks = self.args.subnet_gen.alloc_subnets()
+        prv_networks = self.args.privnet_gen.alloc_subnets()
         self._write_as_topos()
         self._write_as_list()
         self._write_ifids()
@@ -133,9 +144,9 @@ def _read_links(self):
         assigned_br_id = {}
         br_ids = defaultdict(int)
         if_ids = defaultdict(lambda: IFIDGenerator())
-        if not self.topo_config.get("links", None):
+        if not self.args.topo_config_dict.get("links", None):
             return
-        for attrs in self.topo_config["links"]:
+        for attrs in self.args.topo_config_dict["links"]:
             a = LinkEP(attrs.pop("a"))
             b = LinkEP(attrs.pop("b"))
             linkto = linkto_a = linkto_b = attrs.pop("linkAtoB")
@@ -154,7 +165,7 @@ def _read_links(self):
             self.ifid_map[str(b)][b_desc] = a_desc
 
     def _generate_as_topo(self, topo_id, as_conf):
-        mtu = as_conf.get('mtu', self.default_mtu)
+        mtu = as_conf.get('mtu', self.args.default_mtu)
         assert mtu >= SCION_MIN_MTU, mtu
         self.topo_dicts[topo_id] = {
             'Core': as_conf.get('core', False), 'ISD_AS': str(topo_id),
@@ -182,10 +193,10 @@ def _gen_srv_entry(self, topo_id, as_conf, conf_key, def_num, nick,
                        topo_key):
         count = as_conf.get(conf_key, def_num)
         # only a single Go-PS/Go-CS per AS is currently supported
-        if ((conf_key == "path_servers" and self.ps == "go") or
-           (conf_key == "certificate_servers" and self.cs == "go")):
+        if ((conf_key == "path_servers" and self.args.path_server == "go") or
+           (conf_key == "certificate_servers" and self.args.cert_server == "go")):
             count = 1
-        if conf_key == "discovery_servers" and not self.ds:
+        if conf_key == "discovery_servers" and not self.args.discovery:
             count = 0
         for i in range(1, count + 1):
             elem_id = "%s%s-%s" % (nick, topo_id.file_fmt(), i)
@@ -194,15 +205,15 @@ def _gen_srv_entry(self, topo_id, as_conf, conf_key, def_num, nick,
                     self.addr_type: {
                         'Public': {
                             'Addr': self._reg_addr(topo_id, elem_id),
-                            'L4Port': self.port_gen.register(elem_id),
+                            'L4Port': self.args.port_gen.register(elem_id),
                         }
                     }
                 }
             }
-            if self.gen_bind_addr:
+            if self.args.bind_addr:
                 d['Addrs'][self.addr_type]['Bind'] = {
                     'Addr': self._reg_bind_addr(topo_id, elem_id),
-                    'L4Port': self.port_gen.register(elem_id),
+                    'L4Port': self.args.port_gen.register(elem_id),
                 }
             self.topo_dicts[topo_id][topo_key][elem_id] = d
 
@@ -221,7 +232,7 @@ def _gen_br_entry(self, local, l_ifid, remote, r_ifid, remote_type, attrs,
                     self.addr_type: {
                         'Public': {
                             'Addr': int_addr,
-                            'L4Port': self.port_gen.register(local_br),
+                            'L4Port': self.args.port_gen.register(local_br),
                         }
                     }
                 },
@@ -229,7 +240,7 @@ def _gen_br_entry(self, local, l_ifid, remote, r_ifid, remote_type, attrs,
                     self.addr_type: {
                         'PublicOverlay': {
                             'Addr': int_addr,
-                            'OverlayPort': self.port_gen.register(local_br),
+                            'OverlayPort': self.args.port_gen.register(local_br),
                         }
                     }
                 },
@@ -261,15 +272,15 @@ def _gen_br_intf(self, remote, public_addr, remote_addr, attrs, remote_type):
 
     def _gen_zk_entries(self, topo_id, as_conf):
         zk_conf = {}
-        if "zookeepers" in self.topo_config.get("defaults", {}):
-            zk_conf = self.topo_config["defaults"]["zookeepers"]
-        if self.docker:
+        if "zookeepers" in self.args.topo_config_dict.get("defaults", {}):
+            zk_conf = self.args.topo_config_dict["defaults"]["zookeepers"]
+        if self.args.docker:
             zk_conf[1] = {'addr': ZOOKEEPER_ADDR}
         for key, val in zk_conf.items():
             self._gen_zk_entry(topo_id, key, val)
 
     def _gen_zk_entry(self, topo_id, zk_id, zk_conf):
-        zk = ZKTopo(zk_conf, self.zk_config)
+        zk = ZKTopo(zk_conf, self.args.zk_config_dict)
         addr = str(zk.addr)
         self.topo_dicts[topo_id]["ZookeeperService"][zk_id] = {
             'Addr': addr,
@@ -285,7 +296,7 @@ def _generate_as_list(self, topo_id, as_conf):
 
     def _write_as_topos(self):
         for topo_id, as_topo, base in _srv_iter(
-                self.topo_dicts, self.out_dir, common=True):
+                self.topo_dicts, self.args.output_dir, common=True):
             path = os.path.join(base, TOPO_FILE)
             contents_json = json.dumps(self.topo_dicts[topo_id],
                                        default=_json_default, indent=2)
@@ -294,16 +305,16 @@ def _write_as_topos(self):
             Topology.from_file(path)
 
     def _write_as_list(self):
-        list_path = os.path.join(self.out_dir, AS_LIST_FILE)
+        list_path = os.path.join(self.args.output_dir, AS_LIST_FILE)
         write_file(list_path, yaml.dump(dict(self.as_list)))
 
     def _write_ifids(self):
-        list_path = os.path.join(self.out_dir, IFIDS_FILE)
+        list_path = os.path.join(self.args.output_dir, IFIDS_FILE)
         write_file(list_path, yaml.dump(self.ifid_map,
                                         default_flow_style=False))
 
     def _write_overlay(self):
-        file_path = os.path.join(self.out_dir, OVERLAY_FILE)
+        file_path = os.path.join(self.args.output_dir, OVERLAY_FILE)
         write_file(file_path, self.overlay + '\n')
 
 
diff --git a/python/topology/utils.py b/python/topology/utils.py
index 37637a09b8..c789ccebb4 100644
--- a/python/topology/utils.py
+++ b/python/topology/utils.py
@@ -18,19 +18,27 @@
 import yaml
 # SCION
 from lib.util import write_file
+from topology.common import ArgsBase
 
 DOCKER_TESTER_CONF = 'testers-dc.yml'
 
 
+class TesterGenArgs(ArgsBase):
+    pass
+
+
 class TesterGenerator(object):
-    def __init__(self, out_dir):
-        self.out_dir = out_dir
+    def __init__(self, args):
+        """
+        :param TesterGenArgs args: Contains the passed command line arguments.
+        """
+        self.args = args
         self.dc_tester_conf = {'version': '3', 'services': {}}
         self.output_base = os.environ.get('SCION_OUTPUT_BASE', os.getcwd())
 
     def generate(self):
         self._test_conf()
-        write_file(os.path.join(self.out_dir, DOCKER_TESTER_CONF),
+        write_file(os.path.join(self.args.output_dir, DOCKER_TESTER_CONF),
                    yaml.dump(self.dc_tester_conf, default_flow_style=False))
 
     def _test_conf(self):

From 848fb2d18b05ea68a6181128a4cecb4bfcddd68d Mon Sep 17 00:00:00 2001
From: roos <roos@anapaya.net>
Date: Thu, 8 Nov 2018 09:26:43 +0100
Subject: [PATCH 2/2] feedback 1

---
 python/topology/common.py    |  8 +++---
 python/topology/config.py    | 45 ++++-----------------------------
 python/topology/generator.py | 49 +++++++++++++++++++++++++++++++++++-
 python/topology/topo.py      |  3 ++-
 4 files changed, 60 insertions(+), 45 deletions(-)

diff --git a/python/topology/common.py b/python/topology/common.py
index de96f4bfd5..1a5a411438 100644
--- a/python/topology/common.py
+++ b/python/topology/common.py
@@ -38,7 +38,7 @@ def __init__(self, args):
 class ArgsTopoConfig(ArgsBase):
     def __init__(self, args, topo_config):
         """
-        :param ArgsBase args: Contains the passed command line arguments.
+        :param object args: Contains the passed command line arguments as named attributes.
         :param dict topo_config: The parsed topology config.
         """
         super().__init__(args)
@@ -46,13 +46,15 @@ def __init__(self, args, topo_config):
 
 
 class ArgsTopoDicts(ArgsBase):
-    def __init__(self, args, topo_dicts):
+    def __init__(self, args, topo_dicts, port_gen=None):
         """
-        :param ArgsBase args: Contains the passed command line arguments.
+        :param object args: Contains the passed command line arguments as named attributes.
         :param dict topo_dicts: The generated topo dicts from TopoGenerator.
+        :param PortGenerator port_gen: The port generator
         """
         super().__init__(args)
         self.topo_dicts = topo_dicts
+        self.port_gen = port_gen
 
 
 class TopoID(ISD_AS):
diff --git a/python/topology/config.py b/python/topology/config.py
index 0afbfb1263..45310fbfa1 100755
--- a/python/topology/config.py
+++ b/python/topology/config.py
@@ -17,7 +17,6 @@
 =============================================
 """
 # Stdlib
-import argparse
 import base64
 import configparser
 import logging
@@ -39,10 +38,8 @@
 from lib.defines import (
     AS_CONF_FILE,
     DEFAULT_MTU,
-    DEFAULT_SEGMENT_TTL,
     DEFAULT6_NETWORK,
     DEFAULT6_PRIV_NETWORK,
-    GEN_PATH,
     NETWORKS_FILE,
     PATH_POLICY_FILE,
     PRV_NETWORKS_FILE,
@@ -80,39 +77,7 @@
 
 
 class ConfigGenArgs(ArgsBase):
-
-    @classmethod
-    def create_parser(cls):
-        parser = argparse.ArgumentParser()
-        parser.add_argument('-6', '--ipv6', action='store_true',
-                            help='Generate IPv6 addresses')
-        parser.add_argument('-c', '--topo-config', default=DEFAULT_TOPOLOGY_FILE,
-                            help='Default topology config')
-        parser.add_argument('-p', '--path-policy', default=DEFAULT_PATH_POLICY_FILE,
-                            help='Path policy file')
-        parser.add_argument('-m', '--mininet', action='store_true',
-                            help='Use Mininet to create a virtual network topology')
-        parser.add_argument('-d', '--docker', action='store_true',
-                            help='Create a docker-compose configuration')
-        parser.add_argument('-n', '--network',
-                            help='Network to create subnets in (E.g. "127.0.0.0/8"')
-        parser.add_argument('-o', '--output-dir', default=GEN_PATH,
-                            help='Output directory')
-        parser.add_argument('-z', '--zk-config', default=DEFAULT_ZK_CONFIG,
-                            help='Zookeeper configuration file')
-        parser.add_argument('-b', '--bind-addr', default=GENERATE_BIND_ADDRESS,
-                            help='Generate bind addresses (E.g. "192.168.0.0/16"')
-        parser.add_argument('--pseg-ttl', type=int, default=DEFAULT_SEGMENT_TTL,
-                            help='Path segment TTL (in seconds)')
-        parser.add_argument('-cs', '--cert-server', default=DEFAULT_CERTIFICATE_SERVER,
-                            help='Certificate Server implementation to use ("go" or "py")')
-        parser.add_argument('-sd', '--sciond', default=DEFAULT_SCIOND,
-                            help='SCIOND implementation to use ("go" or "py")')
-        parser.add_argument('-ps', '--path-server', default=DEFAULT_PATH_SERVER,
-                            help='Path Server implementation to use ("go or "py")')
-        parser.add_argument('-ds', '--discovery', action='store_true',
-                            help='Generate discovery service')
-        return parser
+    pass
 
 
 class ConfigGenerator(object):
@@ -126,15 +91,15 @@ def __init__(self, args):
         :param ConfigGenArgs args: Contains the passed command line arguments.
         """
         self.args = args
-        self.topo_config = load_yaml_file(args.topo_config)
+        self.topo_config = load_yaml_file(self.args.topo_config)
         self.zk_config = load_yaml_file(self.args.zk_config)
         if self.args.docker and self.args.mininet:
             logging.critical("Cannot use mininet with docker!")
             sys.exit(1)
         self.default_mtu = None
-        self._read_defaults(args.network)
+        self._read_defaults(self.args.network)
         self.port_gen = PortGenerator()
-        if self.args.docker and self.args.cert_server is not DEFAULT_CERTIFICATE_SERVER:
+        if self.args.docker and (self.args.cert_server != DEFAULT_CERTIFICATE_SERVER):
             logging.critical("Cannot use non-default CS with docker!")
             sys.exit(1)
 
@@ -305,7 +270,7 @@ def _gen_as_conf(self, as_topo):
         return {
             'RegisterTime': 5,
             'PropagateTime': 5,
-            'CertChainVersion': 0,
+            'CertChainVersion': 1,
             # FIXME(kormat): This seems to always be true..:
             'RegisterPath': True if as_topo["PathService"] else False,
             'PathSegmentTTL': self.args.pseg_ttl,
diff --git a/python/topology/generator.py b/python/topology/generator.py
index 6908266ae2..c02dab4fe9 100755
--- a/python/topology/generator.py
+++ b/python/topology/generator.py
@@ -17,18 +17,65 @@
 :mod:`generator` --- SCION topology generator
 =============================================
 """
+# Stdlib
+import argparse
+
 # SCION
+from lib.defines import (
+    DEFAULT_SEGMENT_TTL,
+    GEN_PATH,
+)
 from topology.config import (
     ConfigGenerator,
     ConfigGenArgs,
+    DEFAULT_CERTIFICATE_SERVER,
+    DEFAULT_PATH_POLICY_FILE,
+    DEFAULT_PATH_SERVER,
+    DEFAULT_SCIOND,
+    DEFAULT_TOPOLOGY_FILE,
+    DEFAULT_ZK_CONFIG,
+    GENERATE_BIND_ADDRESS,
 )
 
 
+def add_arguments(parser):
+    parser.add_argument('-6', '--ipv6', action='store_true',
+                        help='Generate IPv6 addresses')
+    parser.add_argument('-c', '--topo-config', default=DEFAULT_TOPOLOGY_FILE,
+                        help='Default topology config')
+    parser.add_argument('-p', '--path-policy', default=DEFAULT_PATH_POLICY_FILE,
+                        help='Path policy file')
+    parser.add_argument('-m', '--mininet', action='store_true',
+                        help='Use Mininet to create a virtual network topology')
+    parser.add_argument('-d', '--docker', action='store_true',
+                        help='Create a docker-compose configuration')
+    parser.add_argument('-n', '--network',
+                        help='Network to create subnets in (E.g. "127.0.0.0/8"')
+    parser.add_argument('-o', '--output-dir', default=GEN_PATH,
+                        help='Output directory')
+    parser.add_argument('-z', '--zk-config', default=DEFAULT_ZK_CONFIG,
+                        help='Zookeeper configuration file')
+    parser.add_argument('-b', '--bind-addr', default=GENERATE_BIND_ADDRESS,
+                        help='Generate bind addresses (E.g. "192.168.0.0/16"')
+    parser.add_argument('--pseg-ttl', type=int, default=DEFAULT_SEGMENT_TTL,
+                        help='Path segment TTL (in seconds)')
+    parser.add_argument('-cs', '--cert-server', default=DEFAULT_CERTIFICATE_SERVER,
+                        help='Certificate Server implementation to use ("go" or "py")')
+    parser.add_argument('-sd', '--sciond', default=DEFAULT_SCIOND,
+                        help='SCIOND implementation to use ("go" or "py")')
+    parser.add_argument('-ps', '--path-server', default=DEFAULT_PATH_SERVER,
+                        help='Path Server implementation to use ("go or "py")')
+    parser.add_argument('-ds', '--discovery', action='store_true',
+                        help='Generate discovery service')
+    return parser
+
+
 def main():
     """
     Main function.
     """
-    parser = ConfigGenArgs.create_parser()
+    parser = argparse.ArgumentParser()
+    add_arguments(parser)
     args = ConfigGenArgs(parser.parse_args())
     confgen = ConfigGenerator(args)
     confgen.generate_all()
diff --git a/python/topology/topo.py b/python/topology/topo.py
index f231a0b9a9..eafd9db715 100755
--- a/python/topology/topo.py
+++ b/python/topology/topo.py
@@ -54,7 +54,8 @@
 
 
 class TopoGenArgs(ArgsBase):
-    def __init__(self, args, topo_config, zk_config, subnet_gen, privnet_gen, default_mtu, port_gen):
+    def __init__(self, args, topo_config, zk_config, subnet_gen,
+                 privnet_gen, default_mtu, port_gen):
         """
         :param ArgsBase args: Contains the passed command line arguments.
         :param dict topo_config: The parsed topology config.