-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added a role for the bridge mautrix-signal #686
Conversation
matrix_mautrix_signal_configuration_permissions: | ||
"YOUR_DOMAIN": user | ||
``` | ||
to allow all users registered to `YOUR_DOMAIN` access to the bridge (where `YOUR_DOMAIN` is your base domain, not the `matrix.` domain). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest making it the default configuration that only user from YOUR_DOMAIN
can access the bridge.
PS: I'm looking forward to using this bridge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for commenting! I thought about doing that, but then I thought, perhaps some admins consider this setting too liberal: what if you don't want all the users registered to your domain to be able to use the bridge? Having no default permissions at all is safer, but may be annoying if you forget to adapt. But maybe I'm too paranoid, and it's reasonable to assume that what you suggest will work for the vast majority of admins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The container will not start if you forget to adapt. I think it should be at least working. I think "[]" is not valid yaml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely. Fixed it, and added documentation for how one can get what @ebbertd proposed.
--env POSTGRES_USER={{ matrix_mautrix_signal_db_user }} \ | ||
--env POSTGRES_PASSWORD={{ matrix_mautrix_signal_db_password }} \ | ||
--env POSTGRES_DB={{ matrix_mautrix_signal_db_database }} \ | ||
-v {{ matrix_mautrix_signal_db_storage_path }}:/var/lib/postgresql/data:z \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add "-v /etc/passwd:/etc/passwd:ro" on the server for this to work. Otherwise it failed in chmod /var/run/postgres.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting that! I must have missed this during testing. I found this piece of documentation that you probably also found, which mentions the work-around you suggested as well as a second option. I committed your proposal as it was faster to do and shouldn't have any negative side-effects, even though the other options seems a bit cleaner.
Oh nice, thanks for making this PR! I'm excited to use it soon :-) |
I think we should look into ways of making this use our already existing (even if it takes some manual effort one the user's part). Otherwise, we're looking into maintaining multiple different Postgres instances, which also need to be backed up and upgraded (to new major Postgres versions) separately. It would be a pain to do for every service like this. Besides this bridge, we also have others which would benefit from using Postgres. Also, Dendrite and other services are coming, which would need their own Postgres as well. Each service using its own separate instance would be a pain. I still haven't figured out a great way for us to share the existing |
merge master
Sorry for having this blocked for so long! It seems like #740 is progressing, so I think it should be able to easily redo this role on top of it and make it use |
Now that #740 is merged, this bridge role can be greatly simplified. Please remove the Postgres server setup stuff that is part of this role and switch to using The way the other bridges do it is, they have a bunch of matrix-docker-ansible-deploy/roles/matrix-bridge-mautrix-facebook/defaults/main.yml Lines 36 to 61 in 067f12b
While the bridge roles define matrix-docker-ansible-deploy/group_vars/matrix_servers Lines 209 to 211 in 067f12b
The mautrix-signal bridge only supports
And finally, you'd need an entry in matrix-docker-ansible-deploy/group_vars/matrix_servers Lines 986 to 992 in 067f12b
Let me know if you need any help with this! |
Thanks @spantaleev and everyone who was involved in making the shared db server happen! I've updated the role now to work with this db server. Here are some instructions for anyone who's been running this role already (based on commit cea2faa) on how to port the db to the present version (commit 84cac25). Tested on CentOS 7 using the postgres server as installed by the role Instructions for porting the dbYou should probably start by running the playbook with the tags You should probably also make sure your homeserver's postgres is up to date. After that is done, feel free to check out the current version of this role. On your server, run: systemctl stop matrix-mautrix-signal
docker exec -ti matrix-mautrix-signal-db bash Inside the docker, run: createuser -U signal-db-user --superuser postgres
psql -U postgres postgres Assuming that Assuming that you want to keep the new defaults alter user "signal-db-user" rename to "matrix_mautrix_signal";
alter database "signal-db" rename to "matrix_mautrix_signal";
exit Back in the docker's bash, run: pg_dump --create -d matrix_mautrix_signal -U matrix_mautrix_signal > /var/lib/postgresql/data/dump.sql
exit Back on you server, you will find the file On your local machine, where you run the playbook, check out the current commit of this role if you haven't yet. Then run the playbook with the tag ansible-playbook -i inventory/hosts setup.yml \
--ask-become-pass \
--tags=setup-postgres This should create the new database for signal on the main postgres server. Then import the dumped database into this new database by running the playbook with the tag ansible-playbook -i inventory/hosts setup.yml \
--ask-become-pass \
--extra-vars="server_path_postgres_dump=/matrix/mautrix-signal/database/dump.sql" \
--tags=import-postgres or wherever on the remote machine your Run the playbook again to reinstall signal and start all services: ansible-playbook -i inventory/hosts setup.yml \
--ask-become-pass \
--tags=setup-all,start (Maybe it would suffice to run the tags Make sure the bridge works and all your data are there. Manually remove the old signal database (instructions copied from https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/master/docs/uninstalling.md). If you want to keep your systemctl stop matrix-mautrix-signal-db
rm -f /etc/systemd/system/matrix-mautrix-signal-db.service
systemctl daemon-reload
docker rm matrix-mautrix-signal-db
rm -rf /matrix/mautrix-signal/database Edit: Fixed the tag name |
The answer to these is: it's good to have them in both places. The role defines the obvious things it depends on (not knowing what setup it will find itself into), and then `group_vars/matrix_servers` "extends" it based on everything else it knows (the homeserver being Synapse, whether or not the internal Postgres server is being used, etc.)
While it's kind of nice having it, it's also somewhat raw and unnecessary. Having a good default and not even mentioning it seems better for most users. People who need a more exposed bridge (rare) can use override the default configuration using `matrix_mautrix_signal_configuration_extension_yaml`.
We try to only use console logging (going to journald) for everything, instead of logging things twice (or more).
Thanks a lot for all of your work on it, for the patience (for #740) and for redoing things for that! 👍 I've made a few minor changes (see above) and think this looks good for merging! Let me know if my last-minute changes broke something, etc. |
This role installs 3 services: the bridge itself, and its 2 dependencies: the signald daemon, and a postgres database. All run in docker containers.
The rest of the setup mirrors the mautrix-telegram bridge.
I also included documentation.