Skip to content
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

nixos/mautrix-whatsapp: init module Matrix<->WhatsApp Bridge #59211

Closed
wants to merge 398 commits into from

Conversation

Vskilet
Copy link
Contributor

@Vskilet Vskilet commented Apr 9, 2019

Motivation for this change

Add a service to use Matrix<->WhatsApp bridge.
I have some questions that I let here : https://discourse.nixos.org/t/add-path-from-the-nix-store-to-service-configuration-during-build/2629

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

nixos/modules/services/misc/mautrix-whatsapp.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/mautrix-whatsapp.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/mautrix-whatsapp.nix Outdated Show resolved Hide resolved
@flokli
Copy link
Contributor

flokli commented Apr 9, 2019

Thanks for the PR! Some nitpicks regarding DynamicUser, StateDirectory and configuration management, feel free to continue discussing services.matrix-synapse.app_service_config_files changes here, too :-)

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the contribution!

There are two thoughts of mine:

  • Isn't the registration.yaml required for the service? I currently generate those locally (that should be documented then) and add it to mautrix-whatsapp using -r ${./registration.yaml}. Also, matrix-synapse seems to require the app-service configuration file. In NixOS it can be added with services.matrix-synapse.app_service_config_files.

  • In the long-term I'd like to see some kind of services.matrix-bridges module where one can configure several bridges for different chat protocols. I guess that right now it should be possible to create some kind of "abstract" module that can be used for mautrix-whatsapp and mautrix-telegram, but not sure if that's out of scope here.

nixos/modules/services/misc/mautrix-whatsapp.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/mautrix-whatsapp.nix Outdated Show resolved Hide resolved
@Vskilet
Copy link
Contributor Author

Vskilet commented Apr 11, 2019

* In the long-term I'd like to see some kind of `services.matrix-bridges` module where one can configure several bridges for different chat protocols.  I guess that right now it should be possible to create some kind of "abstract" module that can be used for `mautrix-whatsapp` and `mautrix-telegram`, but not sure if that's out of scope here.

I start mautrix-telegram service too. I don't know exactly what you want to do ... Something like a json file with a set of configuration for all matrix's bridge ?

@Vskilet
Copy link
Contributor Author

Vskilet commented Apr 11, 2019

Hi @flokli and @Ma27 !
I try to apply your suggestions. Is the RFC42 mandatory for now ?
How can I convert easily JSON in YAML config file ?

@flokli
Copy link
Contributor

flokli commented Apr 11, 2019

I try to apply your suggestions. Is the RFC42 mandatory for now ?

RFC 42 isn't agreed upon yet, but it might some sense, and is surely better extendable than lines of text.

How can I convert easily JSON in YAML config file ?

See https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/databases/influxdb.nix#L99 for an example.

@Vskilet
Copy link
Contributor Author

Vskilet commented Apr 12, 2019

Hi @flokli and @Ma27 :)
I think we have something almost good. How can I get the StateDirectory to use it in config options ?
I will add the generation of registration.yaml on preStart

@Vskilet Vskilet marked this pull request as ready for review April 21, 2019 21:53
@Vskilet Vskilet requested a review from infinisil as a code owner April 21, 2019 21:53
@Vskilet
Copy link
Contributor Author

Vskilet commented Apr 22, 2019

@flokli is it satisfying ?

@Vskilet Vskilet changed the title [WIP] nixos/mautrix-whatsapp: init module Matrix<->WhatsApp Bridge nixos/mautrix-whatsapp: init module Matrix<->WhatsApp Bridge Apr 24, 2019
@Vskilet
Copy link
Contributor Author

Vskilet commented Apr 24, 2019

I added my module in module-list.nix but it seems there is a problem with this commit

@Vskilet
Copy link
Contributor Author

Vskilet commented Apr 25, 2019

It seems that we are OK on this PR, isn't it @Ma27 and @flokli ?

@flokli
Copy link
Contributor

flokli commented Apr 27, 2019

I guess we can't really cover this in a nixos vm test, as mautrix-webapp might fail on startup if there's no network connection, right?

@Ma27, did you give this a test run?

@Ma27
Copy link
Member

Ma27 commented Apr 27, 2019

did you give this a test run?

Not yet. Currently busy with some other stuff, but I'll try to deploy this to my Matrix instance tonight and see what happens :)

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all I'd like to note that we when using this module, all secrets will be persisted in the Nix store which means that any user on the target machine can read the secrets. I'm not sure what to do about here, IIRC you have to write everything into the YAML file (#59211 (review)), but I'm not sure.

There may be some workarounds (#59211 (review), https://elvishjerricco.github.io/2018/06/24/secure-declarative-key-management.html), but I haven't tried those out yet. I'm not sure if that's even a blocker, as I'm fairly sure that this is not the only module which handles secrets like this ATM and it's a known problem.

Regarding the module itself: I just deployed a pretty minimalistic config with this module to temporarily replace my current mautrix-whatsapp service with this module. A new HS token is generated, the service successfully starts up and is registered in my homeserver, hence 👍

nixos/modules/services/misc/mautrix-whatsapp.nix Outdated Show resolved Hide resolved
};
};
logging = {
directory = "/var/lib/mautrix-whatsapp/logs";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit neater to make this /var/log/mautrix-whatsapp instead and then change the unit below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExecStart = ''
${pkgs.mautrix-whatsapp}/bin/mautrix-whatsapp -c "${configFile}/config.yaml"
'';
Restart = "on-failure";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the logging to work:

LogsDirectory  = "mautrix-whatsapp";

@Vskilet
Copy link
Contributor Author

Vskilet commented Jun 3, 2022

Sorry for this big noise ... will re-open a new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet