Merge pull request #5561 from matrix-org/rav/docker/refactor

Refactor the docker/start.py script

2 files changed, 118 insertions, 62 deletions

diff --git a/changelog.d/5561.feature b/changelog.d/5561.feature
new file mode 100644
index 0000000000..85380bc517
--- /dev/null
+++ b/changelog.d/5561.feature
@@ -0,0 +1 @@
+Update Docker image to deprecate the use of environment variables for configuration, and make the use of a static configuration the default.
diff --git a/docker/start.py b/docker/start.py
index a7a54dacf7..bdb703aebd 100755
--- a/docker/start.py
+++ b/docker/start.py
@@ -1,52 +1,124 @@
 #!/usr/local/bin/python
 
-import jinja2
+import codecs
+import glob
 import os
-import sys
 import subprocess
-import glob
-import codecs
+import sys
+
+import jinja2
+
 
 # Utility functions
-convert = lambda src, dst, environ: open(dst, "w").write(
-    jinja2.Template(open(src).read()).render(**environ)
-)
+def log(txt):
+    print(txt, file=sys.stderr)
+
+
+def error(txt):
+    log(txt)
+    sys.exit(2)
+
+
+def convert(src, dst, environ):
+    """Generate a file from a template
+
+    Args:
+        src (str): path to input file
+        dst (str): path to file to write
+        environ (dict): environment dictionary, for replacement mappings.
+    """
+    with open(src) as infile:
+        template = infile.read()
+    rendered = jinja2.Template(template).render(**environ)
+    with open(dst, "w") as outfile:
+        outfile.write(rendered)
+
+
+def generate_config_from_template(environ, ownership):
+    """Generate a homeserver.yaml from environment variables
 
+    Args:
+        environ (dict): environment dictionary
+        ownership (str): "<user>:<group>" string which will be used to set
+            ownership of the generated configs
 
-def check_arguments(environ, args):
-    for argument in args:
-        if argument not in environ:
-            print("Environment variable %s is mandatory, exiting." % argument)
-            sys.exit(2)
+    Returns:
+        path to generated config file
+    """
+    for v in ("SYNAPSE_SERVER_NAME", "SYNAPSE_REPORT_STATS"):
+        if v not in environ:
+            error(
+                "Environment variable '%s' is mandatory when generating a config "
+                "file on-the-fly." % (v,)
+            )
 
+    # populate some params from data files (if they exist, else create new ones)
+    environ = environ.copy()
+    secrets = {
+        "registration": "SYNAPSE_REGISTRATION_SHARED_SECRET",
+        "macaroon": "SYNAPSE_MACAROON_SECRET_KEY",
+    }
 
-def generate_secrets(environ, secrets):
     for name, secret in secrets.items():
         if secret not in environ:
             filename = "/data/%s.%s.key" % (environ["SYNAPSE_SERVER_NAME"], name)
+
+            # if the file already exists, load in the existing value; otherwise,
+            # generate a new secret and write it to a file
+
             if os.path.exists(filename):
                 with open(filename) as handle:
                     value = handle.read()
             else:
-                print("Generating a random secret for {}".format(name))
+                log("Generating a random secret for {}".format(name))
                 value = codecs.encode(os.urandom(32), "hex").decode()
                 with open(filename, "w") as handle:
                     handle.write(value)
             environ[secret] = value
 
+    environ["SYNAPSE_APPSERVICES"] = glob.glob("/data/appservices/*.yaml")
+    if not os.path.exists("/compiled"):
+        os.mkdir("/compiled")
+
+    config_path = "/compiled/homeserver.yaml"
+
+    # Convert SYNAPSE_NO_TLS to boolean if exists
+    if "SYNAPSE_NO_TLS" in environ:
+        tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"])
+        if tlsanswerstring in ("true", "on", "1", "yes"):
+            environ["SYNAPSE_NO_TLS"] = True
+        else:
+            if tlsanswerstring in ("false", "off", "0", "no"):
+                environ["SYNAPSE_NO_TLS"] = False
+            else:
+                error(
+                    'Environment variable "SYNAPSE_NO_TLS" found but value "'
+                    + tlsanswerstring
+                    + '" unrecognized; exiting.'
+                )
+
+    convert("/conf/homeserver.yaml", config_path, environ)
+    convert("/conf/log.config", "/compiled/log.config", environ)
+    subprocess.check_output(["chown", "-R", ownership, "/data"])
+    return config_path
+
+
+def run_generate_config(environ):
+    """Run synapse with a --generate-config param to generate a template config file
 
-# Prepare the configuration
-mode = sys.argv[1] if len(sys.argv) > 1 else None
-environ = os.environ.copy()
-ownership = "{}:{}".format(environ.get("UID", 991), environ.get("GID", 991))
-args = ["python", "-m", "synapse.app.homeserver"]
+    Args:
+        environ (dict): environment dictionary
 
-# In generate mode, generate a configuration, missing keys, then exit
-if mode == "generate":
-    check_arguments(
-        environ, ("SYNAPSE_SERVER_NAME", "SYNAPSE_REPORT_STATS", "SYNAPSE_CONFIG_PATH")
-    )
-    args += [
+    Never returns.
+    """
+    for v in ("SYNAPSE_SERVER_NAME", "SYNAPSE_REPORT_STATS", "SYNAPSE_CONFIG_PATH"):
+        if v not in environ:
+            error("Environment variable '%s' is mandatory in `generate` mode." % (v,))
+
+    args = [
+        "python",
+        "-m",
+        "synapse.app.homeserver",
         "--server-name",
         environ["SYNAPSE_SERVER_NAME"],
         "--report-stats",
@@ -57,46 +129,25 @@ if mode == "generate":
     ]
     os.execv("/usr/local/bin/python", args)
 
-# In normal mode, generate missing keys if any, then run synapse
-else:
+
+def main(args, environ):
+    mode = args[1] if len(args) > 1 else None
+    ownership = "{}:{}".format(environ.get("UID", 991), environ.get("GID", 991))
+
+    # In generate mode, generate a configuration, missing keys, then exit
+    if mode == "generate":
+        return run_generate_config(environ)
+
+    # In normal mode, generate missing keys if any, then run synapse
     if "SYNAPSE_CONFIG_PATH" in environ:
         config_path = environ["SYNAPSE_CONFIG_PATH"]
     else:
-        check_arguments(environ, ("SYNAPSE_SERVER_NAME", "SYNAPSE_REPORT_STATS"))
-        generate_secrets(
-            environ,
-            {
-                "registration": "SYNAPSE_REGISTRATION_SHARED_SECRET",
-                "macaroon": "SYNAPSE_MACAROON_SECRET_KEY",
-            },
-        )
-        environ["SYNAPSE_APPSERVICES"] = glob.glob("/data/appservices/*.yaml")
-        if not os.path.exists("/compiled"):
-            os.mkdir("/compiled")
-
-        config_path = "/compiled/homeserver.yaml"
-
-        # Convert SYNAPSE_NO_TLS to boolean if exists
-        if "SYNAPSE_NO_TLS" in environ:
-            tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"])
-            if tlsanswerstring in ("true", "on", "1", "yes"):
-                environ["SYNAPSE_NO_TLS"] = True
-            else:
-                if tlsanswerstring in ("false", "off", "0", "no"):
-                    environ["SYNAPSE_NO_TLS"] = False
-                else:
-                    print(
-                        'Environment variable "SYNAPSE_NO_TLS" found but value "'
-                        + tlsanswerstring
-                        + '" unrecognized; exiting.'
-                    )
-                    sys.exit(2)
-
-        convert("/conf/homeserver.yaml", config_path, environ)
-        convert("/conf/log.config", "/compiled/log.config", environ)
-        subprocess.check_output(["chown", "-R", ownership, "/data"])
-
-    args += [
+        config_path = generate_config_from_template(environ, ownership)
+
+    args = [
+        "python",
+        "-m",
+        "synapse.app.homeserver",
         "--config-path",
         config_path,
         # tell synapse to put any generated keys in /data rather than /compiled
@@ -107,3 +158,7 @@ else:
     # Generate missing keys and start synapse
     subprocess.check_output(args + ["--generate-keys"])
     os.execv("/sbin/su-exec", ["su-exec", ownership] + args)
+
+
+if __name__ == "__main__":
+    main(sys.argv, os.environ)