search

maxpert / marmot

A distributed SQLite replicator built on top of NATS
https://maxpert.github.io/marmot/
MIT License
1.86k stars 42 forks source link

Add support for TLS configuration for NATS #72

Closed arnarg closed 1 year ago

arnarg commented 1 year ago

This adds options ca_file, cert_file and key_file to NATS config.

ca_file is used to specify a CA to verify a TLS connection to NATS. cert_file and key_file is used to setup TLS authentication to NATS server.

In order to not having to restart marmot every time the client certificates need to be updated, SIGHUP can be sent to marmot to re-read and apply the certificates specified in the config.

closes #71

arnarg commented 1 year ago

Couple of changes. Overall I feel like this change looks good, what I want to sleep over is this SIGHUP change can spin up into bigger changes (reload whole config, DB schema, etc). I wanna be very intentional about the reload part. I would suggest to get TLS part in first and then have a different PR for SIGHUP part.

Sounds fair. I'll change the commit once I'm at a computer.

Do you still want to keep the ReloadCertificates function even if it's never called? Then it can just be called whenever the SIGHUP handling is added.

maxpert commented 1 year ago

Couple of changes. Overall I feel like this change looks good, what I want to sleep over is this SIGHUP change can spin up into bigger changes (reload whole config, DB schema, etc). I wanna be very intentional about the reload part. I would suggest to get TLS part in first and then have a different PR for SIGHUP part.

Sounds fair. I'll change the commit once I'm at a computer.

Do you still want to keep the ReloadCertificates function even if it's never called? Then it can just be called whenever the SIGHUP handling is added.

Let's keep the ReloadCertificates I don't see problem in keeping the method. Since it can be wired later.

maxpert commented 1 year ago

If you can get the changes in today, I will include it as part of beta build

arnarg commented 1 year ago

If you can get the changes in today, I will include it as part of beta build

I am planning on getting it done in the next few hours.

arnarg commented 1 year ago

@maxpert I force-pushed a new commit without the SIGHUP handling.