Move the configuration file handling code into a separate module

sydent/config/__init__.py

@@ -1,4 +1,4 @@
-# Copyright 2021 New Vector Ltd
+# Copyright 2019-2021 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -181,30 +181,35 @@ def __init__(self):
             self.http,
         ]
 
-    def _parse_config(self, cfg: ConfigParser) -> None:
+    def _parse_config(self, cfg: ConfigParser) -> bool:
         """
         Run the parse_config method on each of the objects in
         self.config_sections
 
         :param cfg: the configuration to be parsed
+
+        :return: whether or not cfg has been altered. This method CAN
+            return True, but it *shouldn't* as this leads to altering the
+            config file.
         """
+        needs_saving = False
         for section in self.config_sections:
-            section.parse_config(cfg)
+            if section.parse_config(cfg):
+                needs_saving = True
+
+        return needs_saving
 
     def parse_from_config_parser(self, cfg: ConfigParser) -> bool:
         """
         Parse the configuration from a ConfigParser object
 
         :param cfg: the configuration to be parsed
 
-        :return: Whether or not cfg has been changed and needs saving
+        :return: whether or not cfg has been altered. This method CAN
+            return True, but it *shouldn't* as this leads to altering the
+            config file.
         """
-        self._parse_config(cfg)
-
-        # TODO: Don't alter config file when starting Sydent unless
-        #       user has asked for this specifially (e.g. on first
-        #       run only, or when specify --generate-config)
-        return self.crypto.save_key
+        return self._parse_config(cfg)
 
     def parse_config_file(self, config_file: str) -> None:
         """
@@ -234,6 +239,9 @@ def parse_config_file(self, config_file: str) -> None:
         # so that we can log while parsing the rest
         setup_logging(cfg)
 
+        # TODO: Don't alter config file when starting Sydent so that
+        #       it can be set to read-only
+
         needs_saving = self.parse_from_config_parser(cfg)
 
         if needs_saving:

sydent/config/_base.py

@@ -0,0 +1,31 @@
+# Copyright 2021 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from abc import ABC, abstractmethod
+from configparser import ConfigParser
+
+
+class BaseConfig(ABC):
+    @abstractmethod
+    def parse_config(self, cfg: ConfigParser) -> bool:
+        """
+        Parse the a section of the config
+
+        :param cfg: the configuration to be parsed
+
+        :return: whether or not cfg has been altered. This method CAN
+            return True, but it *shouldn't* as this leads to altering the
+            config file.
+        """
+        pass

sydent/config/crypto.py

@@ -18,11 +18,13 @@
 import nacl
 import signedjson.key
 
+from sydent.config._base import BaseConfig
+
 logger = logging.getLogger(__name__)
 
 
-class CryptoConfig:
-    def parse_config(self, cfg: "ConfigParser") -> None:
+class CryptoConfig(BaseConfig):
+    def parse_config(self, cfg: "ConfigParser") -> bool:
         """
         Parse the crypto section of the config
         :param cfg: the configuration to be parsed
@@ -31,7 +33,7 @@ def parse_config(self, cfg: "ConfigParser") -> None:
         signing_key_str = cfg.get("crypto", "ed25519.signingkey")
         signing_key_parts = signing_key_str.split(" ")
 
-        self.save_key = False
+        save_key = False
 
         if signing_key_str == "":
             logger.info(
@@ -41,7 +43,7 @@ def parse_config(self, cfg: "ConfigParser") -> None:
 
             self.signing_key = signedjson.key.generate_signing_key("0")
 
-            self.save_key = True
+            save_key = True
         elif len(signing_key_parts) == 1:
             # old format key
             logger.info("Updating signing key format: brace yourselves")
@@ -52,16 +54,19 @@ def parse_config(self, cfg: "ConfigParser") -> None:
             self.signing_key.version = "0"
             self.signing_key.alg = signedjson.key.NACL_ED25519
 
-            self.save_key = True
+            save_key = True
         else:
             self.signing_key = signedjson.key.decode_signing_key_base64(
                 signing_key_parts[0], signing_key_parts[1], signing_key_parts[2]
             )
 
-        if self.save_key:
+        if save_key:
             signing_key_str = "%s %s %s" % (
                 self.signing_key.alg,
                 self.signing_key.version,
                 signedjson.key.encode_signing_key_base64(self.signing_key),
             )
             cfg.set("crypto", "ed25519.signingkey", signing_key_str)
+            return True
+        else:
+            return False

sydent/config/database.py

@@ -14,8 +14,10 @@
 
 from configparser import ConfigParser
 
+from sydent.config._base import BaseConfig
 
-class DatabaseConfig:
+
+class DatabaseConfig(BaseConfig):
     def parse_config(self, cfg: "ConfigParser") -> None:
         """
         Parse the database section of the config

sydent/config/email.py

@@ -15,9 +15,11 @@
 import socket
 from configparser import ConfigParser
 
+from sydent.config._base import BaseConfig
 
-class EmailConfig:
-    def parse_config(self, cfg: "ConfigParser") -> None:
+
+class EmailConfig(BaseConfig):
+    def parse_config(self, cfg: "ConfigParser") -> bool:
         """
         Parse the email section of the config
 
@@ -63,3 +65,5 @@ def parse_config(self, cfg: "ConfigParser") -> None:
         self.domain_obfuscate_characters = cfg.getint(
             "email", "email.third_party_invite_domain_obfuscate_characters"
         )
+
+        return False

sydent/config/general.py

@@ -20,13 +20,14 @@
 from jinja2.environment import Environment
 from jinja2.loaders import FileSystemLoader
 
+from sydent.config._base import BaseConfig
 from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set
 
 logger = logging.getLogger(__name__)
 
 
-class GeneralConfig:
-    def parse_config(self, cfg: "ConfigParser") -> None:
+class GeneralConfig(BaseConfig):
+    def parse_config(self, cfg: "ConfigParser") -> bool:
         """
         Parse the 'general' section of the config
 
@@ -78,9 +79,7 @@ def parse_config(self, cfg: "ConfigParser") -> None:
         )
 
         self.sentry_enabled = cfg.has_option("general", "sentry_dsn")
-        self.sentry_dsn = self.sentry_dsn = cfg.get(
-            "general", "sentry_dsn", fallback=None
-        )
+        self.sentry_dsn = cfg.get("general", "sentry_dsn", fallback=None)
 
         self.enable_v1_associations = parse_cfg_bool(
             cfg.get("general", "enable_v1_associations")
@@ -99,6 +98,8 @@ def parse_config(self, cfg: "ConfigParser") -> None:
         self.ip_blacklist = generate_ip_set(ip_blacklist)
         self.ip_whitelist = generate_ip_set(ip_whitelist)
 
+        return False
+
 
 def set_from_comma_sep_string(rawstr: str) -> Set[str]:
     """

sydent/config/http.py

@@ -14,8 +14,10 @@
 
 from configparser import ConfigParser
 
+from sydent.config._base import BaseConfig
 
-class HTTPConfig:
+
+class HTTPConfig(BaseConfig):
     def parse_config(self, cfg: "ConfigParser") -> None:
         """
         Parse the http section of the config
@@ -65,3 +67,5 @@ def parse_config(self, cfg: "ConfigParser") -> None:
                 if cfg.has_option(section, "base_replication_url"):
                     base_url = cfg.get(section, "base_replication_url")
                     self.base_replication_urls[peer] = base_url
+
+        return False

sydent/config/sms.py

@@ -15,8 +15,10 @@
 from configparser import ConfigParser
 from typing import Dict, List
 
+from sydent.config._base import BaseConfig
 
-class SMSConfig:
+
+class SMSConfig(BaseConfig):
     def parse_config(self, cfg: "ConfigParser") -> None:
         """
         Parse the sms section of the config
@@ -67,3 +69,5 @@ def parse_config(self, cfg: "ConfigParser") -> None:
                     )
 
                 self.smsRules[country] = action
+
+        return False