refactor: relay.sh
#3845
Merged
refactor: relay.sh
#3845
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
polarathene
added
service/postfix
area/scripts
area/features
kind/improvement
The functionality is effectively the same for the two configs for the most part when it comes to parsing out a domain from the target value. Virtual aliases is more flexible in value, which may not have a domain-part present (manual user edit).
polarathene
force-pushed
the
docs/refactor-relayhosts-feature
branch
from
012815c
to
366da36
Compare
- Moves the "handle changes" logic into it's own scoped function, out of the main change detection loop logic. - This will be benefit a future commit change that will rely on `VHOST_UPDATED=1`.
- Better phrasing of the current logic comments. - Regex patterns assigned to variables (easier to grok intention) - Bulk of the logic for generating `/etc/postfix/relayhost_map` wrapped into a separate function with Postfix config setting handled separately.
- Split the two distinct features that configure `/etc/postfix/relayhost_map` into separate functions (_`MATCH_VALID` var no longer needed for legacy support_). - Instead of extracting domains from `postfix-accounts.cf` + `postfix-virtual.cf`, this has already been handled at `/etc/postfix/vhost`, sourcing from there is far less complicated. - Rename loop var `DOMAIN_PART`to `SENDER_DOMAIN` for better context of what it represents when appended to the config file. - Revised maintenance notes + guidance towards a future refactor of this relayhost feature support.
- Remove comment regarding `smtp_sasl_password_maps = static:${RELAY_USER}:${RELAY_PASSWORD}`, it could be used but `main.cf` presently has `644` permissions vs the `sasl_passwd` file permissions of `600`, less secure at preventing leaking of secrets (ignoring the ENV exposure itself).
- Move the `main.cf` settings specific to relayhost credentials support / security into to the relevant function scope instead. This also allows for the configuration to be applied by a change detection event without container restart requirement.
- Outer functions for setup and change detection to call have a clearer config dependency guard, as does the `_legacy_support()`.
- These changes now support `DEFAULT_RELAY_HOST` to leverage the relay credentials ENV as well.
- `DATABASE_RELAYHOSTS` is available in scope to the functions called here that reference it.
polarathene
force-pushed
the
docs/refactor-relayhosts-feature
branch
from
366da36
to
02ec9d7
Compare
Tests are a bit lacking for this feature presently, although I'm fairly confident the PR isn't doing anything not covered in the changelog. |
polarathene
commented
Jan 28, 2024
polarathene
commented
Jan 29, 2024
Better quality guidance on configuring relay hosts.
polarathene
force-pushed
the
docs/refactor-relayhosts-feature
branch
from
44c5da0
to
a5feec5
Compare
polarathene
commented
Jan 29, 2024
ENV docs visual comparisonBeforeAfter |
|
I will review in an hour or so 🚀 |
georglauterbach
requested changes
Jan 29, 2024
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
polarathene
commented
Jan 29, 2024
georglauterbach
approved these changes
Jan 30, 2024
|
Documentation preview for this PR is ready! 🎉 Built with commit: 59f5638 |
This was referenced Jan 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
While revising the docs on this feature, I was reminded how frustratingly awkward the current implementation is 😮💨
So I've detoured to make some changes there:
relay.sh: Much better maintainer comments + simplified logic + fixes.check-for-changes.sh+postfix.sh, primarilyVHOST_UPDATEDaddition + comment revisions.DEFAULT_RELAY_HOSTto support credentials without redundantly needingRELAY_HOST, which should better help cater to this recent use-case bug report (the opposite,RELAY_HOSTwhen set enforced credentials).Changes have been staged out into scoped commits with individual commit messages for added context. If you want an easier set of diffs to review through, that'd be a nicer experience to work through 👍
Two other docs pages for the relay host feature will be revised as a separate follow-up PR.
Context
RELAY_PASSWORDENV for credentials and just handling that directly in the user configuration file instead.relay.shimplementation prior to this PR.relay.shcomments) and this follow-up commentcompose.yamllocal test environment with several DMS instances configured for relaying is available:compose.yamlfile: SettingRELAY_HOSTforces expectation to provide relay credentials #3842 (comment)compose.yaml+ multiple files for CoreDNS config: relaying: mails are not relayed with implicit tls (465) / improve scripts & configuration #2601 (comment)Type of change
Checklist:
docs/)CHANGELOG.md