Fix synapse.config module "read" command (#11145)

`synapse.config.__main__` has the possibility to read a config item. This can be used to conveniently also validate the config is valid before trying to start Synapse.

 The "read" command broke in https://github.com/matrix-org/synapse/pull/10916 as it now requires passing in "server.server_name" for example.

 Also made the read command optional so one can just call this with just the confirm file reference and get a "Config parses OK" if things are ok.

Signed-off-by: Jason Robinson <jasonr@matrix.org>
Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

changelog.d/11145.bugfix

@@ -0,0 +1 @@
+Fix a bug introduced in Synapse v1.45.0 breaking the configuration file parsing script.

synapse/config/__main__.py

@@ -1,4 +1,5 @@
 # Copyright 2015, 2016 OpenMarket Ltd
+# 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.
@@ -11,25 +12,44 @@
 # 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.
+import sys
+
 from synapse.config._base import ConfigError
+from synapse.config.homeserver import HomeServerConfig
 
-if __name__ == "__main__":
-    import sys
 
-    from synapse.config.homeserver import HomeServerConfig
+def main(args):
+    action = args[1] if len(args) > 1 and args[1] == "read" else None
+    # If we're reading a key in the config file, then `args[1]` will be `read`  and `args[2]`
+    # will be the key to read.
+    # We'll want to rework this code if we want to support more actions than just `read`.
+    load_config_args = args[3:] if action else args[1:]
 
-    action = sys.argv[1]
+    try:
+        config = HomeServerConfig.load_config("", load_config_args)
+    except ConfigError as e:
+        sys.stderr.write("\n" + str(e) + "\n")
+        sys.exit(1)
+
+    print("Config parses OK!")
 
     if action == "read":
-        key = sys.argv[2]
+        key = args[2]
+        key_parts = key.split(".")
+
+        value = config
         try:
-            config = HomeServerConfig.load_config("", sys.argv[3:])
-        except ConfigError as e:
-            sys.stderr.write("\n" + str(e) + "\n")
+            while len(key_parts):
+                value = getattr(value, key_parts[0])
+                key_parts.pop(0)
+
+            print(f"\n{key}: {value}")
+        except AttributeError:
+            print(
+                f"\nNo '{key}' key could be found in the provided configuration file."
+            )
             sys.exit(1)
 
-        print(getattr(config, key))
-        sys.exit(0)
-    else:
-        sys.stderr.write("Unknown command %r\n" % (action,))
-        sys.exit(1)
+
+if __name__ == "__main__":
+    main(sys.argv)

tests/config/test___main__.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 synapse.config.__main__ import main
+
+from tests.config.utils import ConfigFileTestCase
+
+
+class ConfigMainFileTestCase(ConfigFileTestCase):
+    def test_executes_without_an_action(self):
+        self.generate_config()
+        main(["", "-c", self.config_file])
+
+    def test_read__error_if_key_not_found(self):
+        self.generate_config()
+        with self.assertRaises(SystemExit):
+            main(["", "read", "foo.bar.hello", "-c", self.config_file])
+
+    def test_read__passes_if_key_found(self):
+        self.generate_config()
+        main(["", "read", "server.server_name", "-c", self.config_file])

tests/config/test_load.py

@@ -1,4 +1,5 @@
 # Copyright 2016 OpenMarket Ltd
+# 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.
@@ -11,43 +12,30 @@
 # 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.
-import os.path
-import shutil
-import tempfile
-from contextlib import redirect_stdout
-from io import StringIO
-
 import yaml
 
 from synapse.config import ConfigError
 from synapse.config.homeserver import HomeServerConfig
 
-from tests import unittest
+from tests.config.utils import ConfigFileTestCase
 
 
-class ConfigLoadingTestCase(unittest.TestCase):
-    def setUp(self):
-        self.dir = tempfile.mkdtemp()
-        self.file = os.path.join(self.dir, "homeserver.yaml")
-
-    def tearDown(self):
-        shutil.rmtree(self.dir)
-
+class ConfigLoadingFileTestCase(ConfigFileTestCase):
     def test_load_fails_if_server_name_missing(self):
         self.generate_config_and_remove_lines_containing("server_name")
         with self.assertRaises(ConfigError):
-            HomeServerConfig.load_config("", ["-c", self.file])
+            HomeServerConfig.load_config("", ["-c", self.config_file])
         with self.assertRaises(ConfigError):
-            HomeServerConfig.load_or_generate_config("", ["-c", self.file])
+            HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
 
     def test_generates_and_loads_macaroon_secret_key(self):
         self.generate_config()
 
-        with open(self.file) as f:
+        with open(self.config_file) as f:
             raw = yaml.safe_load(f)
         self.assertIn("macaroon_secret_key", raw)
 
-        config = HomeServerConfig.load_config("", ["-c", self.file])
+        config = HomeServerConfig.load_config("", ["-c", self.config_file])
         self.assertTrue(
             hasattr(config.key, "macaroon_secret_key"),
             "Want config to have attr macaroon_secret_key",
@@ -58,7 +46,7 @@ class ConfigLoadingTestCase(unittest.TestCase):
                 "was: %r" % (config.key.macaroon_secret_key,)
             )
 
-        config = HomeServerConfig.load_or_generate_config("", ["-c", self.file])
+        config = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
         self.assertTrue(
             hasattr(config.key, "macaroon_secret_key"),
             "Want config to have attr macaroon_secret_key",
@@ -71,9 +59,9 @@ class ConfigLoadingTestCase(unittest.TestCase):
 
     def test_load_succeeds_if_macaroon_secret_key_missing(self):
         self.generate_config_and_remove_lines_containing("macaroon")
-        config1 = HomeServerConfig.load_config("", ["-c", self.file])
-        config2 = HomeServerConfig.load_config("", ["-c", self.file])
-        config3 = HomeServerConfig.load_or_generate_config("", ["-c", self.file])
+        config1 = HomeServerConfig.load_config("", ["-c", self.config_file])
+        config2 = HomeServerConfig.load_config("", ["-c", self.config_file])
+        config3 = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
         self.assertEqual(
             config1.key.macaroon_secret_key, config2.key.macaroon_secret_key
         )
@@ -87,15 +75,15 @@ class ConfigLoadingTestCase(unittest.TestCase):
             ["enable_registration: true", "disable_registration: true"]
         )
         # Check that disable_registration clobbers enable_registration.
-        config = HomeServerConfig.load_config("", ["-c", self.file])
+        config = HomeServerConfig.load_config("", ["-c", self.config_file])
         self.assertFalse(config.registration.enable_registration)
 
-        config = HomeServerConfig.load_or_generate_config("", ["-c", self.file])
+        config = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
         self.assertFalse(config.registration.enable_registration)
 
         # Check that either config value is clobbered by the command line.
         config = HomeServerConfig.load_or_generate_config(
-            "", ["-c", self.file, "--enable-registration"]
+            "", ["-c", self.config_file, "--enable-registration"]
         )
         self.assertTrue(config.registration.enable_registration)
 
@@ -104,33 +92,5 @@ class ConfigLoadingTestCase(unittest.TestCase):
         self.add_lines_to_config(["enable_metrics: true"])
 
         # The default Metrics Flags are off by default.
-        config = HomeServerConfig.load_config("", ["-c", self.file])
+        config = HomeServerConfig.load_config("", ["-c", self.config_file])
         self.assertFalse(config.metrics.metrics_flags.known_servers)
-
-    def generate_config(self):
-        with redirect_stdout(StringIO()):
-            HomeServerConfig.load_or_generate_config(
-                "",
-                [
-                    "--generate-config",
-                    "-c",
-                    self.file,
-                    "--report-stats=yes",
-                    "-H",
-                    "lemurs.win",
-                ],
-            )
-
-    def generate_config_and_remove_lines_containing(self, needle):
-        self.generate_config()
-
-        with open(self.file) as f:
-            contents = f.readlines()
-        contents = [line for line in contents if needle not in line]
-        with open(self.file, "w") as f:
-            f.write("".join(contents))
-
-    def add_lines_to_config(self, lines):
-        with open(self.file, "a") as f:
-            for line in lines:
-                f.write(line + "\n")

tests/config/utils.py

@@ -0,0 +1,58 @@
+# 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.
+import os
+import shutil
+import tempfile
+import unittest
+from contextlib import redirect_stdout
+from io import StringIO
+
+from synapse.config.homeserver import HomeServerConfig
+
+
+class ConfigFileTestCase(unittest.TestCase):
+    def setUp(self):
+        self.dir = tempfile.mkdtemp()
+        self.config_file = os.path.join(self.dir, "homeserver.yaml")
+
+    def tearDown(self):
+        shutil.rmtree(self.dir)
+
+    def generate_config(self):
+        with redirect_stdout(StringIO()):
+            HomeServerConfig.load_or_generate_config(
+                "",
+                [
+                    "--generate-config",
+                    "-c",
+                    self.config_file,
+                    "--report-stats=yes",
+                    "-H",
+                    "lemurs.win",
+                ],
+            )
+
+    def generate_config_and_remove_lines_containing(self, needle):
+        self.generate_config()
+
+        with open(self.config_file) as f:
+            contents = f.readlines()
+        contents = [line for line in contents if needle not in line]
+        with open(self.config_file, "w") as f:
+            f.write("".join(contents))
+
+    def add_lines_to_config(self, lines):
+        with open(self.config_file, "a") as f:
+            for line in lines:
+                f.write(line + "\n")