Bad or partial config could fail faster and more noisily

[lampholder]

Setting up a TURN server, somehow I set the config in homeserver.yaml to read:

turn-shared-secret: 81ahb1ahBl4481ahb1ahBl4481ahb1ahBl4481ahb1ahBl44

When of course it should have read:

turn_shared_secret: 81ahb1ahBl4481ahb1ahBl4481ahb1ahBl4481ahb1ahBl44

Synapse uncritically accepted a turn_shared_secret of (presumably) empty string and generated (invalid) passwords using that. Coupled with coturn's super limp logging and frustrating debug/test tools, this silent failure frustrated my efforts to set up a functioning TURN server for just over one and a half years 😭

[michaelkaye]

Things related to this:

• config-test type option to test config and report errors (eg, values being strings when they should be lists) without/before restarting synapse
• show-config type option to read config and return the output of the config as understood by synapse, allowing a round-trip of config through the parser and back out into another yaml file (maybe just on stdout, maybe in the logs). That would then show a #turn_shared_secret line in the regurgitated config, as the parser thinks the value is not set, allowing better debugging.

[Half-Shot]

For @lampholder 's specific case we could just scream that turn is disabled, which should hopefully provoke some fear that his config is wrong. I'm not sure spitting the config back out is a particularly log friendly thing to do. Disabling turn is a fairly reasonable thing to do for people that don't want the pain of coturn.

[richvdh]

For @lampholder 's specific case we could just scream that turn is disabled,

scream where?

I'm not sure we could do a lot to solve @lampholder's problem beyond providing better test tools, which is covered by #1519.

[richvdh]

I don't think this is an actionable issue as it stands.

[michaelkaye]

Could i suggest an actionable change to synapse:

A commandline way of invoking synapse which does nothing except parse the config file, attempts to load modules as specified, but does not bind to any ports or open any database connections, emits a warning of any parameter in a config file is unused, and returns status code 0 if there were no errors or warnings, and status code 1 if there were any.

I'm aware this is not easy within the current flow of synapses' configuration, but it is an important feature for a reliable application. We are able to currently generate an entire config file from scratch, so we should have enough information about parameters to be able to check for invalid ones in a config file.

If you look at the options for high reliability services, we find things like haproxy -c or nginx -t, use of those flags has then been built into the automation tools that deploy them (eg ansible or systemd) so any change to config files can be checked to ensure that the service will at minimum restart before we shut down the old version and start downtime.

I can open this as a seperate issue if you want, but it would also cover @lampholder's case as well as the automation ones.

[richvdh]

agreed, sounds good. (and I don't think it's even that hard). new issue please!