albertito / chasquid

SMTP (email) server with a focus on simplicity, security, and ease of operation [mirror]
https://blitiri.com.ar/p/chasquid/
Other
868 stars 56 forks source link

Add -hostname CLI flag #7

Closed ThinkChaos closed 4 years ago

ThinkChaos commented 4 years ago

Allows overriding the configured hostname.

I'm using this in a docker container to have -hostname $HOSTNAME in the startup script, to be able to configure the hostname at runtime. This can also help to use the output of hostname --fqdn as the hostname as requested here.

PS: Once my container is all done, I'll send a message on the mailing list with a link and explanation for how it differs from the one in the repo :)

albertito commented 4 years ago

Thanks for sending this!

While I appreciate the patch, I want to understand first why the config option isn't good enough for your purposes.

Can you tell me more why that doesn't work? Is it because it's inconvenient or just impossible to do what you're trying to do?

The included Dockerfile uses a trick to set the domain in the config on startup, does that trick not work for your use case?

Thanks!

ThinkChaos commented 4 years ago

Thanks for the quick reply!

My use case is having an easy to use production ready docker image. I have env variables to configure the hostname and domains.

I did see that entrypoint.sh edits chasquid.conf, but intentionally departed from that because I don't think its acceptable for the image I'm making to edit the end-user's persistent configuration file, especially with a script that replaces the value at each container start. (By end-user I mean user of the image I'm making.) EDIT: Just to be 100% clear chasquid.conf is stored in a volume and is the end-user's.

I did keep the dovecot/auto-ssl.conf generation though as that is explicit: dovecot.conf contains an !include with a comment and is never modified directly. So if chasquid had an include mechanism I would've used that, but I don't think that's worth the trouble given how simple the configuration is.

I also considered these workarounds:

albertito commented 4 years ago

I see, thanks for the details! I understand the use case better now.

My gut feeling tells me it's either better to implement something more flexible (like passing any config option via flags), or nothing, rather than allowing it only for this particular value.

But it's just a feeling; I need to think and try a few things first, so let me do that and I'll be back to you soon.

ThinkChaos commented 4 years ago

Sure!

I think using reflect like this to parse tags on the protobuf generated struct could make this pretty easy to implement. I'm not familiar with reflection in Go so I don't know if this approach would have any drawbacks besides being dependent on protobuf's tag format.

albertito commented 4 years ago

So after some thought and experimentation, I came up with a set of changes (commit 4c28efcb being the final one) which implement a --config_overrides command-line flag.

It takes as value a text protobuf, so it can be used to override any value in the config. It's not super neat and there are some cases that are not supported (e.g. if you want an override to add things to a repeated element), but I think it covers the main use cases for something like this.

For your specific one, --config_overrides='hostname: "myhost"' should do the trick.

Can you give it a try and let me know if that works for you? It's on the next branch.

Thanks, Alberto

ThinkChaos commented 4 years ago

That's perfect thanks! Just want to note its -config_overrides (single dash, like all the other flags).

I'll close the PR as it is no longer useful.