From a52e1a3b6c5a255f4a51d48dec739751238fa1f9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Jun 2019 18:37:20 +0100 Subject: [PATCH 1/8] Factor out "generate_config_from_template" ... and inline generate_secrets --- docker/start.py | 122 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 81 insertions(+), 41 deletions(-) diff --git a/docker/start.py b/docker/start.py index a7a54dacf7c4..eab39f3c9357 100755 --- a/docker/start.py +++ b/docker/start.py @@ -7,37 +7,109 @@ import glob import codecs + # 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 check_arguments(environ, args): for argument in args: if argument not in environ: - print("Environment variable %s is mandatory, exiting." % argument) - sys.exit(2) + error("Environment variable %s is mandatory, exiting." % argument) + + +def generate_config_from_template(environ, ownership): + """Generate a homeserver.yaml from environment variables + + Args: + environ (dict): environment dictionary + ownership (str): ":" string which will be used to set + ownership of the generated configs + 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 + # 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"] @@ -62,39 +134,7 @@ def generate_secrets(environ, secrets): 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"]) + config_path = generate_config_from_template(environ, ownership) args += [ "--config-path", From b1fddb7f699a595d39507e876d950f256ab0f8aa Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Jun 2019 18:55:37 +0100 Subject: [PATCH 2/8] Factor out a run_generate_config function --- docker/start.py | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/docker/start.py b/docker/start.py index eab39f3c9357..70942bcbeea0 100755 --- a/docker/start.py +++ b/docker/start.py @@ -33,12 +33,6 @@ def convert(src, dst, environ): outfile.write(rendered) -def check_arguments(environ, args): - for argument in args: - if argument not in environ: - error("Environment variable %s is mandatory, exiting." % argument) - - def generate_config_from_template(environ, ownership): """Generate a homeserver.yaml from environment variables @@ -108,17 +102,22 @@ def generate_config_from_template(environ, ownership): return config_path -# Prepare the configuration -mode = sys.argv[1] if len(sys.argv) > 1 else None -ownership = "{}:{}".format(environ.get("UID", 991), environ.get("GID", 991)) -args = ["python", "-m", "synapse.app.homeserver"] +def run_generate_config(environ): + """Run synapse with a --generate-config param to generate a template config file -# 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 += [ + Args: + environ (dict): environment dictionary + + 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", @@ -129,6 +128,15 @@ def generate_config_from_template(environ, ownership): ] os.execv("/usr/local/bin/python", args) + +# Prepare the configuration +mode = sys.argv[1] if len(sys.argv) > 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": + run_generate_config(environ) + # In normal mode, generate missing keys if any, then run synapse else: if "SYNAPSE_CONFIG_PATH" in environ: @@ -136,7 +144,10 @@ def generate_config_from_template(environ, ownership): else: config_path = generate_config_from_template(environ, ownership) - args += [ + args = [ + "python", + "-m", + "synapse.app.homeserver", "--config-path", config_path, # tell synapse to put any generated keys in /data rather than /compiled From 3f24e4dce7d4dfb50f0f8d958f6f4daf2dbdc70c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Jun 2019 14:23:24 +0100 Subject: [PATCH 3/8] Add a main() function --- docker/start.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/docker/start.py b/docker/start.py index 70942bcbeea0..df907fe15c3d 100755 --- a/docker/start.py +++ b/docker/start.py @@ -129,16 +129,15 @@ def run_generate_config(environ): os.execv("/usr/local/bin/python", args) -# Prepare the configuration -mode = sys.argv[1] if len(sys.argv) > 1 else None -ownership = "{}:{}".format(environ.get("UID", 991), environ.get("GID", 991)) +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": - run_generate_config(environ) + # 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 -else: + # In normal mode, generate missing keys if any, then run synapse if "SYNAPSE_CONFIG_PATH" in environ: config_path = environ["SYNAPSE_CONFIG_PATH"] else: @@ -158,3 +157,7 @@ def run_generate_config(environ): # 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) From 5375c3a9b8121d121cc68756654f4fc20c481995 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Jun 2019 15:10:36 +0100 Subject: [PATCH 4/8] isort --- docker/start.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docker/start.py b/docker/start.py index df907fe15c3d..bdb703aebd04 100755 --- a/docker/start.py +++ b/docker/start.py @@ -1,11 +1,12 @@ #!/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 From 043ab6da13cbc49d221b6c220ad0b360b2095643 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jun 2019 15:28:28 +0100 Subject: [PATCH 5/8] changelog --- changelog.d/5561.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5561.feature diff --git a/changelog.d/5561.feature b/changelog.d/5561.feature new file mode 100644 index 000000000000..85380bc51736 --- /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. From c58a6e6108d2891d10ce7f29e4b0169a3e026787 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Jun 2019 15:09:22 +0100 Subject: [PATCH 6/8] document supported env vars for docker 'generate' option --- docker/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docker/README.md b/docker/README.md index 5a596eecb977..10af88844bf3 100644 --- a/docker/README.md +++ b/docker/README.md @@ -179,3 +179,10 @@ docker run -d --name synapse \ -e SYNAPSE_CONFIG_PATH=/data/homeserver.yaml \ matrixdotorg/synapse:latest ``` + +The following environment variables are supported in this mode: + +* `SYNAPSE_SERVER_NAME` (mandatory): the server public hostname. +* `SYNAPSE_REPORT_STATS` (mandatory, `yes` or `no`): whether to enable + anonymous statistics reporting. +* `SYNAPSE_CONFIG_PATH` (mandatory): path to the file to be generated. From 7e433beb65946a39442f025705429227bfa3e429 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Jun 2019 15:18:30 +0100 Subject: [PATCH 7/8] Docker image: add support for SYNAPSE_DATA_DIR parameter Fixes #4830. --- docker/README.md | 4 ++++ docker/start.py | 17 +++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/docker/README.md b/docker/README.md index 10af88844bf3..5c85c538a8c9 100644 --- a/docker/README.md +++ b/docker/README.md @@ -186,3 +186,7 @@ The following environment variables are supported in this mode: * `SYNAPSE_REPORT_STATS` (mandatory, `yes` or `no`): whether to enable anonymous statistics reporting. * `SYNAPSE_CONFIG_PATH` (mandatory): path to the file to be generated. +* `SYNAPSE_DATA_DIR`: where the generated config will put persistent data + such as the datatase and media store. Defaults to `/data`. +* `UID`, `GID`: the user id and group id to use for creating the data + directories. Defaults to `991`, `991`. diff --git a/docker/start.py b/docker/start.py index bdb703aebd04..ad968b2a67e7 100755 --- a/docker/start.py +++ b/docker/start.py @@ -103,11 +103,12 @@ def generate_config_from_template(environ, ownership): return config_path -def run_generate_config(environ): +def run_generate_config(environ, ownership): """Run synapse with a --generate-config param to generate a template config file Args: - environ (dict): environment dictionary + environ (dict): env var dict + ownership (str): "userid:groupid" arg for chmod Never returns. """ @@ -115,6 +116,11 @@ def run_generate_config(environ): if v not in environ: error("Environment variable '%s' is mandatory in `generate` mode." % (v,)) + data_dir = environ.get("SYNAPSE_DATA_DIR", "/data") + + # make sure that synapse has perms to write to the data dir. + subprocess.check_output(["chown", ownership, data_dir]) + args = [ "python", "-m", @@ -125,8 +131,11 @@ def run_generate_config(environ): environ["SYNAPSE_REPORT_STATS"], "--config-path", environ["SYNAPSE_CONFIG_PATH"], + "--data-directory", + data_dir, "--generate-config", ] + # log("running %s" % (args, )) os.execv("/usr/local/bin/python", args) @@ -134,9 +143,9 @@ 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 + # In generate mode, generate a configuration and missing keys, then exit if mode == "generate": - return run_generate_config(environ) + return run_generate_config(environ, ownership) # In normal mode, generate missing keys if any, then run synapse if "SYNAPSE_CONFIG_PATH" in environ: From a0f2921ccfd5eac2b660274d4de3d05fdc0f390d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 26 Jun 2019 15:41:10 +0100 Subject: [PATCH 8/8] changelog --- changelog.d/5563.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5563.bugfix diff --git a/changelog.d/5563.bugfix b/changelog.d/5563.bugfix new file mode 100644 index 000000000000..09c4381a23c7 --- /dev/null +++ b/changelog.d/5563.bugfix @@ -0,0 +1 @@ +Docker: Use a sensible location for data files when generating a config file. \ No newline at end of file