Remove old cfg argument from Sydent constructor

- Move config file and dict handling over to SydentConfig
- Alter all places where Sydent object is constructed
- Remove parse_cfg_bool and set_from_comma_set_string from sydent.py
  as these functions were only used for config parsing
- Remove save_config from sydent.py as this can now be done from
  SydentConfig

scripts/casefold_db.py

@@ -23,7 +23,7 @@ from typing import Any, Dict, List, Tuple
 import signedjson.sign
 
 from sydent.config import SydentConfig
-from sydent.sydent import Sydent, parse_config_file
+from sydent.sydent import Sydent
 from sydent.util import json_decoder
 from sydent.util.emailutils import sendEmail
 from sydent.util.hash import sha256_and_url_safe_base64
@@ -252,13 +252,11 @@ if __name__ == "__main__":
         print(f"The config file '{args.config_path}' does not exist.")
         sys.exit(1)
 
-    config = parse_config_file(args.config_path)
-
     sydent_config = SydentConfig()
-    sydent_config.parse_from_config_parser(config)
+    sydent_config.parse_config_file(args.config_path)
 
     reactor = ResolvingMemoryReactorClock()
-    sydent = Sydent(config, sydent_config, reactor, False)
+    sydent = Sydent(sydent_config, reactor, False)
 
     update_global_associations(sydent, sydent.db, not args.no_email, args.dry_run)
     update_local_associations(sydent, sydent.db, not args.no_email, args.dry_run)

sydent/config/__init__.py

@@ -12,7 +12,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from configparser import ConfigParser
+import copy
+import logging
+import os
+from configparser import DEFAULTSECT, ConfigParser
+from typing import Dict
+
+from twisted.python import log
 
 from sydent.config.crypto import CryptoConfig
 from sydent.config.database import DatabaseConfig
@@ -21,6 +27,128 @@ from sydent.config.general import GeneralConfig
 from sydent.config.http import HTTPConfig
 from sydent.config.sms import SMSConfig
 
+logger = logging.getLogger(__name__)
+
+CONFIG_DEFAULTS = {
+    "general": {
+        "server.name": os.environ.get("SYDENT_SERVER_NAME", ""),
+        "log.path": "",
+        "log.level": "INFO",
+        "pidfile.path": os.environ.get("SYDENT_PID_FILE", "sydent.pid"),
+        "terms.path": "",
+        "address_lookup_limit": "10000",  # Maximum amount of addresses in a single /lookup request
+        # The root path to use for load templates. This should contain branded
+        # directories. Each directory should contain the following templates:
+        #
+        # * invite_template.eml
+        # * verification_template.eml
+        # * verify_response_template.html
+        "templates.path": "res",
+        # The brand directory to use if no brand hint (or an invalid brand hint)
+        # is provided by the request.
+        "brand.default": "matrix-org",
+        # The following can be added to your local config file to enable prometheus
+        # support.
+        # 'prometheus_port': '8080',  # The port to serve metrics on
+        # 'prometheus_addr': '',  # The address to bind to. Empty string means bind to all.
+        # The following can be added to your local config file to enable sentry support.
+        # 'sentry_dsn': 'https://...'  # The DSN has configured in the sentry instance project.
+        # Whether clients and homeservers can register an association using v1 endpoints.
+        "enable_v1_associations": "true",
+        "delete_tokens_on_bind": "true",
+        # Prevent outgoing requests from being sent to the following blacklisted
+        # IP address CIDR ranges. If this option is not specified or empty then
+        # it defaults to private IP address ranges.
+        #
+        # The blacklist applies to all outbound requests except replication
+        # requests.
+        #
+        # (0.0.0.0 and :: are always blacklisted, whether or not they are
+        # explicitly listed here, since they correspond to unroutable
+        # addresses.)
+        "ip.blacklist": "",
+        # List of IP address CIDR ranges that should be allowed for outbound
+        # requests. This is useful for specifying exceptions to wide-ranging
+        # blacklisted target IP ranges.
+        #
+        # This whitelist overrides `ip.blacklist` and defaults to an empty
+        # list.
+        "ip.whitelist": "",
+    },
+    "db": {
+        "db.file": os.environ.get("SYDENT_DB_PATH", "sydent.db"),
+    },
+    "http": {
+        "clientapi.http.bind_address": "::",
+        "clientapi.http.port": "8090",
+        "internalapi.http.bind_address": "::1",
+        "internalapi.http.port": "",
+        "replication.https.certfile": "",
+        "replication.https.cacert": "",  # This should only be used for testing
+        "replication.https.bind_address": "::",
+        "replication.https.port": "4434",
+        "obey_x_forwarded_for": "False",
+        "federation.verifycerts": "True",
+        # verify_response_template is deprecated, but still used if defined. Define
+        # templates.path and brand.default under general instead.
+        #
+        # 'verify_response_template': 'res/verify_response_page_template',
+        "client_http_base": "",
+    },
+    "email": {
+        # email.template and email.invite_template are deprecated, but still used
+        # if defined. Define templates.path and brand.default under general instead.
+        #
+        # 'email.template': 'res/verification_template.eml',
+        # 'email.invite_template': 'res/invite_template.eml',
+        "email.from": "Sydent Validation <noreply@{hostname}>",
+        "email.subject": "Your Validation Token",
+        "email.invite.subject": "%(sender_display_name)s has invited you to chat",
+        "email.invite.subject_space": "%(sender_display_name)s has invited you to a space",
+        "email.smtphost": "localhost",
+        "email.smtpport": "25",
+        "email.smtpusername": "",
+        "email.smtppassword": "",
+        "email.hostname": "",
+        "email.tlsmode": "0",
+        # The web client location which will be used if it is not provided by
+        # the homeserver.
+        #
+        # This should be the scheme and hostname only, see res/invite_template.eml
+        # for the full URL that gets generated.
+        "email.default_web_client_location": "https://app.element.io",
+        # When a user is invited to a room via their email address, that invite is
+        # displayed in the room list using an obfuscated version of the user's email
+        # address. These config options determine how much of the email address to
+        # obfuscate. Note that the '@' sign is always included.
+        #
+        # If the string is longer than a configured limit below, it is truncated to that limit
+        # with '...' added. Otherwise:
+        #
+        # * If the string is longer than 5 characters, it is truncated to 3 characters + '...'
+        # * If the string is longer than 1 character, it is truncated to 1 character + '...'
+        # * If the string is 1 character long, it is converted to '...'
+        #
+        # This ensures that a full email address is never shown, even if it is extremely
+        # short.
+        #
+        # The number of characters from the beginning to reveal of the email's username
+        # portion (left of the '@' sign)
+        "email.third_party_invite_username_obfuscate_characters": "3",
+        # The number of characters from the beginning to reveal of the email's domain
+        # portion (right of the '@' sign)
+        "email.third_party_invite_domain_obfuscate_characters": "3",
+    },
+    "sms": {
+        "bodyTemplate": "Your code is {token}",
+        "username": "",
+        "password": "",
+    },
+    "crypto": {
+        "ed25519.signingkey": "",
+    },
+}
+
 
 class ConfigError(Exception):
     pass
@@ -30,6 +158,10 @@ class SydentConfig:
     """This is the class in charge of handling Sydent's configuration.
     Handling of each individual section is delegated to other classes
     stored in a `config_sections` list.
+
+    To use this class, create a new object and then call one of
+    `parse_config_file` or `parse_config_dict` before creating the
+    Sydent object that uses it.
     """
 
     def __init__(self):
@@ -73,3 +205,97 @@ class SydentConfig:
         #       user has asked for this specifially (e.g. on first
         #       run only, or when specify --generate-config)
         return self.crypto.save_key
+
+    def parse_config_file(self, config_file: str) -> None:
+        """
+        Parse the given config from a filepath, populating missing items and
+        sections
+
+        :param config_file: the file to be parsed
+        """
+        # If the config file doesn't exist, prepopulate the config object
+        # with the defaults, in the right section.
+        #
+        # Otherwise, we have to put the defaults in the DEFAULT section,
+        # to ensure that they don't override anyone's settings which are
+        # in their config file in the default section (which is likely,
+        # because sydent used to be braindead).
+        use_defaults = not os.path.exists(config_file)
+
+        cfg = ConfigParser()
+        for sect, entries in CONFIG_DEFAULTS.items():
+            cfg.add_section(sect)
+            for k, v in entries.items():
+                cfg.set(DEFAULTSECT if use_defaults else sect, k, v)
+
+        cfg.read(config_file)
+
+        # Logging is configured in cfg, but these options must be parsed first
+        # so that we can log while parsing the rest
+        setup_logging(cfg)
+
+        needs_saving = self.parse_from_config_parser(cfg)
+
+        if needs_saving:
+            fp = open(config_file, "w")
+            cfg.write(fp)
+            fp.close()
+
+    def parse_config_dict(self, config_dict: Dict) -> None:
+        """
+        Parse the given config from a dictionary, populating missing items and sections
+
+        :param config_dict: the configuration dictionary to be parsed
+        """
+        # Build a config dictionary from the defaults merged with the given dictionary
+        config = copy.deepcopy(CONFIG_DEFAULTS)
+        for section, section_dict in config_dict.items():
+            if section not in config:
+                config[section] = {}
+            for option in section_dict.keys():
+                config[section][option] = config_dict[section][option]
+
+        # Build a ConfigParser from the merged dictionary
+        cfg = ConfigParser()
+        for section, section_dict in config.items():
+            cfg.add_section(section)
+            for option, value in section_dict.items():
+                cfg.set(section, option, value)
+
+        # This is only ever called by tests so don't configure logging
+        # as tests do this themselves
+
+        self.parse_from_config_parser(cfg)
+
+
+def setup_logging(cfg: ConfigParser) -> None:
+    """
+    Setup logging using the options selected in the config
+
+    :param cfg: the configuration
+    """
+    log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s"
+    formatter = logging.Formatter(log_format)
+
+    logPath = cfg.get("general", "log.path")
+    if logPath != "":
+        handler = logging.handlers.TimedRotatingFileHandler(
+            logPath, when="midnight", backupCount=365
+        )
+        handler.setFormatter(formatter)
+
+        def sighup(signum, stack):
+            logger.info("Closing log file due to SIGHUP")
+            handler.doRollover()
+            logger.info("Opened new log file due to SIGHUP")
+
+    else:
+        handler = logging.StreamHandler()
+
+    handler.setFormatter(formatter)
+    rootLogger = logging.getLogger("")
+    rootLogger.setLevel(cfg.get("general", "log.level"))
+    rootLogger.addHandler(handler)
+
+    observer = log.PythonLoggingObserver()
+    observer.start()

sydent/sydent.py

@@ -14,17 +14,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import configparser
-import copy
 import gc
 import logging
 import logging.handlers
 import os
-from typing import Optional, Set
+from typing import Optional
 
 import twisted.internet.reactor
 from twisted.internet import address, task
-from twisted.python import log
 
 from sydent.config import SydentConfig
 from sydent.db.hashing_metadata import HashingMetadataStore
@@ -76,140 +73,17 @@ from sydent.validators.msisdnvalidator import MsisdnValidator
 
 logger = logging.getLogger(__name__)
 
-CONFIG_DEFAULTS = {
-    "general": {
-        "server.name": os.environ.get("SYDENT_SERVER_NAME", ""),
-        "log.path": "",
-        "log.level": "INFO",
-        "pidfile.path": os.environ.get("SYDENT_PID_FILE", "sydent.pid"),
-        "terms.path": "",
-        "address_lookup_limit": "10000",  # Maximum amount of addresses in a single /lookup request
-        # The root path to use for load templates. This should contain branded
-        # directories. Each directory should contain the following templates:
-        #
-        # * invite_template.eml
-        # * verification_template.eml
-        # * verify_response_template.html
-        "templates.path": "res",
-        # The brand directory to use if no brand hint (or an invalid brand hint)
-        # is provided by the request.
-        "brand.default": "matrix-org",
-        # The following can be added to your local config file to enable prometheus
-        # support.
-        # 'prometheus_port': '8080',  # The port to serve metrics on
-        # 'prometheus_addr': '',  # The address to bind to. Empty string means bind to all.
-        # The following can be added to your local config file to enable sentry support.
-        # 'sentry_dsn': 'https://...'  # The DSN has configured in the sentry instance project.
-        # Whether clients and homeservers can register an association using v1 endpoints.
-        "enable_v1_associations": "true",
-        "delete_tokens_on_bind": "true",
-        # Prevent outgoing requests from being sent to the following blacklisted
-        # IP address CIDR ranges. If this option is not specified or empty then
-        # it defaults to private IP address ranges.
-        #
-        # The blacklist applies to all outbound requests except replication
-        # requests.
-        #
-        # (0.0.0.0 and :: are always blacklisted, whether or not they are
-        # explicitly listed here, since they correspond to unroutable
-        # addresses.)
-        "ip.blacklist": "",
-        # List of IP address CIDR ranges that should be allowed for outbound
-        # requests. This is useful for specifying exceptions to wide-ranging
-        # blacklisted target IP ranges.
-        #
-        # This whitelist overrides `ip.blacklist` and defaults to an empty
-        # list.
-        "ip.whitelist": "",
-    },
-    "db": {
-        "db.file": os.environ.get("SYDENT_DB_PATH", "sydent.db"),
-    },
-    "http": {
-        "clientapi.http.bind_address": "::",
-        "clientapi.http.port": "8090",
-        "internalapi.http.bind_address": "::1",
-        "internalapi.http.port": "",
-        "replication.https.certfile": "",
-        "replication.https.cacert": "",  # This should only be used for testing
-        "replication.https.bind_address": "::",
-        "replication.https.port": "4434",
-        "obey_x_forwarded_for": "False",
-        "federation.verifycerts": "True",
-        # verify_response_template is deprecated, but still used if defined Define
-        # templates.path and brand.default under general instead.
-        #
-        # 'verify_response_template': 'res/verify_response_page_template',
-        "client_http_base": "",
-    },
-    "email": {
-        # email.template and email.invite_template are deprecated, but still used
-        # if defined. Define templates.path and brand.default under general instead.
-        #
-        # 'email.template': 'res/verification_template.eml',
-        # 'email.invite_template': 'res/invite_template.eml',
-        "email.from": "Sydent Validation <noreply@{hostname}>",
-        "email.subject": "Your Validation Token",
-        "email.invite.subject": "%(sender_display_name)s has invited you to chat",
-        "email.invite.subject_space": "%(sender_display_name)s has invited you to a space",
-        "email.smtphost": "localhost",
-        "email.smtpport": "25",
-        "email.smtpusername": "",
-        "email.smtppassword": "",
-        "email.hostname": "",
-        "email.tlsmode": "0",
-        # The web client location which will be used if it is not provided by
-        # the homeserver.
-        #
-        # This should be the scheme and hostname only, see res/invite_template.eml
-        # for the full URL that gets generated.
-        "email.default_web_client_location": "https://app.element.io",
-        # When a user is invited to a room via their email address, that invite is
-        # displayed in the room list using an obfuscated version of the user's email
-        # address. These config options determine how much of the email address to
-        # obfuscate. Note that the '@' sign is always included.
-        #
-        # If the string is longer than a configured limit below, it is truncated to that limit
-        # with '...' added. Otherwise:
-        #
-        # * If the string is longer than 5 characters, it is truncated to 3 characters + '...'
-        # * If the string is longer than 1 character, it is truncated to 1 character + '...'
-        # * If the string is 1 character long, it is converted to '...'
-        #
-        # This ensures that a full email address is never shown, even if it is extremely
-        # short.
-        #
-        # The number of characters from the beginning to reveal of the email's username
-        # portion (left of the '@' sign)
-        "email.third_party_invite_username_obfuscate_characters": "3",
-        # The number of characters from the beginning to reveal of the email's domain
-        # portion (right of the '@' sign)
-        "email.third_party_invite_domain_obfuscate_characters": "3",
-    },
-    "sms": {
-        "bodyTemplate": "Your code is {token}",
-        "username": "",
-        "password": "",
-    },
-    "crypto": {
-        "ed25519.signingkey": "",
-    },
-}
-
 
 class Sydent:
     def __init__(
         self,
-        cfg,
         sydent_config: SydentConfig,
         reactor=twisted.internet.reactor,
         use_tls_for_federation=True,
     ):
-        self.cfg = cfg
         self.config = sydent_config
 
         self.reactor = reactor
-        self.config_file = get_config_file_path()
         self.use_tls_for_federation = use_tls_for_federation
 
         logger.info("Starting Sydent server")
@@ -323,11 +197,6 @@ class Sydent:
         cb.clock = self.reactor
         cb.start(1.0)
 
-    def save_config(self):
-        fp = open(self.config_file, "w")
-        self.cfg.write(fp)
-        fp.close()
-
     def run(self):
         self.clientApiHttpServer.setup()
         self.replicationHttpsServer.setup()
@@ -423,97 +292,10 @@ class Keyring:
     pass
 
 
-def parse_config_dict(config_dict):
-    """Parse the given config from a dictionary, populating missing items and sections
-
-    Args:
-        config_dict (dict): the configuration dictionary to be parsed
-    """
-    # Build a config dictionary from the defaults merged with the given dictionary
-    config = copy.deepcopy(CONFIG_DEFAULTS)
-    for section, section_dict in config_dict.items():
-        if section not in config:
-            config[section] = {}
-        for option in section_dict.keys():
-            config[section][option] = config_dict[section][option]
-
-    # Build a ConfigParser from the merged dictionary
-    cfg = configparser.ConfigParser()
-    for section, section_dict in config.items():
-        cfg.add_section(section)
-        for option, value in section_dict.items():
-            cfg.set(section, option, value)
-
-    return cfg
-
-
-def parse_config_file(config_file):
-    """Parse the given config from a filepath, populating missing items and
-    sections
-    Args:
-        config_file (str): the file to be parsed
-    """
-    # if the config file doesn't exist, prepopulate the config object
-    # with the defaults, in the right section.
-    #
-    # otherwise, we have to put the defaults in the DEFAULT section,
-    # to ensure that they don't override anyone's settings which are
-    # in their config file in the default section (which is likely,
-    # because sydent used to be braindead).
-    use_defaults = not os.path.exists(config_file)
-    cfg = configparser.ConfigParser()
-    for sect, entries in CONFIG_DEFAULTS.items():
-        cfg.add_section(sect)
-        for k, v in entries.items():
-            cfg.set(configparser.DEFAULTSECT if use_defaults else sect, k, v)
-
-    cfg.read(config_file)
-
-    return cfg
-
-
-def setup_logging(cfg):
-    log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s"
-    formatter = logging.Formatter(log_format)
-
-    logPath = cfg.get("general", "log.path")
-    if logPath != "":
-        handler = logging.handlers.TimedRotatingFileHandler(
-            logPath, when="midnight", backupCount=365
-        )
-        handler.setFormatter(formatter)
-
-        def sighup(signum, stack):
-            logger.info("Closing log file due to SIGHUP")
-            handler.doRollover()
-            logger.info("Opened new log file due to SIGHUP")
-
-    else:
-        handler = logging.StreamHandler()
-
-    handler.setFormatter(formatter)
-    rootLogger = logging.getLogger("")
-    rootLogger.setLevel(cfg.get("general", "log.level"))
-    rootLogger.addHandler(handler)
-
-    observer = log.PythonLoggingObserver()
-    observer.start()
-
-
 def get_config_file_path():
     return os.environ.get("SYDENT_CONF", "sydent.conf")
 
 
-def parse_cfg_bool(value):
-    return value.lower() == "true"
-
-
-def set_from_comma_sep_string(rawstr: str) -> Set[str]:
-    if rawstr == "":
-        return set()
-    return {x.strip() for x in rawstr.split(",")}
-
-
 def run_gc():
     threshold = gc.get_threshold()
     counts = gc.get_count()
@@ -523,15 +305,8 @@ def run_gc():
 
 
 if __name__ == "__main__":
-    cfg = parse_config_file(get_config_file_path())
-    setup_logging(cfg)
-
     sydent_config = SydentConfig()
-    cfg_needs_saving = sydent_config.parse_from_config_parser(cfg)
-
-    syd = Sydent(cfg, sydent_config=sydent_config)
-
-    if cfg_needs_saving:
-        syd.save_config()
+    sydent_config.parse_config_file(get_config_file_path())
 
+    syd = Sydent(sydent_config)
     syd.run()

tests/utils.py

@@ -24,7 +24,7 @@ from twisted.web.server import Request, Site
 from zope.interface import implementer
 
 from sydent.config import SydentConfig
-from sydent.sydent import Sydent, parse_config_dict
+from sydent.sydent import Sydent
 
 # Expires on Jan 11 2030 at 17:53:40 GMT
 FAKE_SERVER_CERT_PEM = """
@@ -70,14 +70,11 @@ def make_sydent(test_config={}):
 
     reactor = ResolvingMemoryReactorClock()
 
-    cfg = parse_config_dict(test_config)
-
     sydent_config = SydentConfig()
-    sydent_config.parse_from_config_parser(cfg)
+    sydent_config.parse_config_dict(test_config)
 
     return Sydent(
         reactor=reactor,
-        cfg=cfg,
         sydent_config=sydent_config,
         use_tls_for_federation=False,
     )