feat: allow adding arbitrary postfix config, allow running post-setup script (fixes #80)#139
feat: allow adding arbitrary postfix config, allow running post-setup script (fixes #80)#139Vosjedev wants to merge 1 commit into
Conversation
Vosjedev
force-pushed
the
master
branch
2 times, most recently
from
33a3aa2 to
d5f9607
Compare
|
I've added documentation. Let me know if anything else needs to happen before this is mergeable. |
|
Hi @Vosjedev thanks for the PR! I will review it and get back to you. |
|
Hi @Vosjedev I left you a review, please take a look at it and adjust documentation accordingly. |
|
Good afternoon, Sorry for the delayed response, I have gotten busier in the last week so my replies may be slow until the end of next week. |
juanluisbaptiste
left a comment
juanluisbaptiste
left a comment
There was a problem hiding this comment.
Oops, I forgot to submit the review.
|
No problem, that can happen. I integrated your requests, if there is anything else wrong, just ask.
I was thinking maybe it's a good idea to document these two variables then, so if you think that's a good idea too I can try to come up with reasonable phrasing for that without making it too confusing for someone who doesn't need the feature. Let me know in that case! |
|
@Vosjedev thanks, I will test and let you know. |
| # Custom configuration options | ||
|
|
||
| # via env vars for easy configuring | ||
| for var in "${!POSTFIX_@}"; do |
There was a problem hiding this comment.
Please add an echo before this loop, to let the user know that the options being set from this point onwards are custom. Something like "Setting custom Postfix configuration parameters using POSTFIX_* variables..." or similar.
There was a problem hiding this comment.
@Vosjedev I'm not sure why I had marked this comment as resolved, but I don't see the suggested message before the loop.
There was a problem hiding this comment.
my guess is that I marked it as resolved, assuming I would resolve it directly after. I guess I... didn't? I'll take another look
|
@Vosjedev it works fine, please add the last modification asked on my last comment and we're ready! |
|
Sorry for the delay, I had seen your email and completely forgot about it despite having time again... |
Sure, TY. |
… script Mostly follows the suggestions from juanluisbaptiste#80 usage: environment variables can be supplied in format POSTFIX_<option> to set them using postconf automatically. usage: mount or copy an executable as `/post-setup.sh` to have it automatically be run after init, and before postfix start Signed-off-by: Vosjedev <vosje@vosjedev.net>
2 participants
Mostly follows the suggestions from
#80
usage: environment variables can be supplied in format
POSTFIX_<option>to set them using postconf automatically.
usage: mount or copy an executable as
/post-setup.shto have it automatically be run after init, and before postfix startDescription of the change
This allows changing postfix (
main.cf) options via environment variables. The code used is modified from @flojon's suggestion in #80 to be using native bash instead of invocingtr. The used syntax is bash 4+, but the image already ships bash 5+, so it's fine.Next to that, a check has been added that executes
/post-setup.shwhen it exists and is executable. This doens't have to be a shellscript, it just have to be executable (maybe I should remove the.shsuffix then? idk.)Motivation and Context
The main motivation is because I would like to add ~5 options to my
main.cf, and reading the suggestions in #80, I thought I'd do the job and submit the changes as a PR.How Has This Been Tested?
It has been tested manually, both specifying uppercase and lowercase configuration options prefixed with
POSTFIX_in the env. The script was also tested manually, by mounting a shellscript with the content#!/bin/sh\necho HELLO WORLDand seeing whether we got the text back.Types of Changes
Checklist:
.env.examplefile accordingly.for now this PR is marked WIP, I have some more work to do (like updating docs and fixing my indentation) but I'll do that soon too.