Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Docker image no longer starts if you set SYNAPSE_SMTP_HOST #5430

Closed
richvdh opened this issue Jun 11, 2019 · 9 comments
Closed

Docker image no longer starts if you set SYNAPSE_SMTP_HOST #5430

richvdh opened this issue Jun 11, 2019 · 9 comments
Labels
A-Docker Docker images, or making it easier to run Synapse in a container.

Comments

@richvdh
Copy link
Member

richvdh commented Jun 11, 2019

As far as I can tell, the SYNAPSE_SMTP_* settings were never used, because email_notifs was set to false in the homeserver.yaml used by the docker image. Nevertheless the documentation kinda encourages you to set the SYNAPSE_SMTP_* settings. Those that did so will now find that their servers refuse to start, with:

RuntimeError: email.password_reset_behaviour is set to 'local' but no public_baseurl is set. This is necessary to generate password reset links
@richvdh
Copy link
Member Author

richvdh commented Jun 11, 2019

The workarounds for now are either:

  • don't set the SYNAPSE_SMTP_* env vars (will disable password reset on your server)
  • use a custom config file via SYNAPSE_CONFIG_PATH

@neilisfragile neilisfragile added p1 A-Docker Docker images, or making it easier to run Synapse in a container. labels Jun 13, 2019
@anoadragon453
Copy link
Member

anoadragon453 commented Jun 21, 2019

A quick fix before figuring out the whole Docker mess is just to add a SYNAPSE_PUBLIC_BASEURL environment variable. This behaviour has been implemented in #5519.

But we should really figure this out :/

@richvdh
Copy link
Member Author

richvdh commented Jun 21, 2019

I'm not convinced adding a SYNAPSE_PUBLIC_BASEURL is the best approach.

We're going to be telling people to switch to the SYNAPSE_CONFIG_PATH approach, so rather than having them add a SYNAPSE_PUBLIC_BASEURL, I think we should tell them to switch to a generated file.

The only question is whether we can do something better for people whose working configs have been broken by 1.0, like disabling password resets altogether?

Given the SYNAPSE_SMTP_ vars are unused in synapse <0.99, maybe we should just remove them, and skip generating the email config section?

@richvdh
Copy link
Member Author

richvdh commented Jun 21, 2019

(incidentally it might be good to do that against the release branch and we can update the docker image on dockerhub.)

@anoadragon453
Copy link
Member

Given the SYNAPSE_SMTP_ vars are unused in synapse <0.99, maybe we should just remove them, and skip generating the email config section?

This sounds best to me, considering, as you say, they were never actually used.

@hawkowl
Copy link
Contributor

hawkowl commented Jun 25, 2019

Because of #5518, I believe we can say "use a proper config" here, and close it?

@richvdh
Copy link
Member Author

richvdh commented Jun 25, 2019

Because of #5518, I believe we can say "use a proper config" here, and close it?

As per #5430 (comment): I'd prefer not to break peoples' working configs, and I think we can do better.

@xobs
Copy link

xobs commented Jul 3, 2019

The README.md on https://hub.docker.com/r/matrixdotorg/synapse still documents those variables, so that should first be removed prior to closing this issue.

richvdh pushed a commit that referenced this issue Jul 3, 2019
Removes any `SMTP_*` docker container environment variables from having any effect on the default config.

Fixes #5430
@richvdh
Copy link
Member Author

richvdh commented Jul 3, 2019

the readme will be updated at the same time as the docker image is updated on hub.docker.

fixed by #5596

@richvdh richvdh closed this as completed Jul 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Docker Docker images, or making it easier to run Synapse in a container.
Projects
None yet
Development

No branches or pull requests

5 participants