Code review: real-postfix

[dehnert]

This is a placeholder PR for tracking the code review of the real-postfix role. It consists of the current master (commit 40aaee3), plus commits to ansible-realserver (commit b908d64) that modify ansible/roles/real-postfix (git filter-repo --path ansible/roles/real-postfix/ plus some cherry-picking onto master). It shouldn't actually get merged (and may or may not be a reasonable way to code review this...).

[dehnert]

/etc/postfix/virtual isn't new, but it doesn't seem to be referenced anywhere. Do you know if there's a reason it sticks around? It looks like it's an upstream default for virtual_alias_maps, but that's an option we're... rather emphatically not using the default value of.

[dehnert]

I've reviewed, I think, all of this except for the virtual_alias_{maps,domains} and smtp_generic_maps part -- which is to say, about half the files, and I think much less than that of the complexity. Files remaining to review:

• generic_strip_pool
• pass-scripts.mit.edu{,-suffix}
• main.cf.j2
• virtual-alias-domains-ldap.cf.j2
• virtual-alias-maps-ldap{,-reserved}.cf.j2
• virtual-alias-maps-relay{,-user,-user-suffix}-ldap.cf.j2

I've got some comments, but they're mostly ~style things -- I wouldn't have a problem with merging the portion I've reviewed as-is if you wanted.

[dehnert]

This feels unnecessarily confusing. While I appreciate a desire to match the authorized_submit_users config option name, I think it'd be better to name this file blocked-submit-users-ldap.cf or similar, which then wouldn't need a comment to understand.

[dehnert]

We talked about this some today, and I think the conclusion was that while renaming would make sense, there's a lack of enthusiasm to mess with what's not broken, which seems reasonable.

[dehnert]

I have more concerns with this batch, especially as it relates to domains on the other pool. If this was pre-commit code review, I'd want to see them addressed before merging.

[dehnert]

I also wrote some comments on what virtual_alias_domains is doing that may be useful -- it's at dehnert@05ea851.

[dehnert]

My little test script:

$ cat test_mail.py #!/usr/bin/python3
# Test Scripts mail forwarding setup
# Styles of email addresses
# $locker@scripts.mit.edu -> $locker
# *@locker.scripts.mit.edu
# *@$domain -> $domain's locker
# Mail enters scripts system at:
# F22 host
# F30 host
# Locker is configured with:
# F22
# F30
# Domain is configured with
# F22
# F30
# Same as above, but using force_pool
import smtplib
SCRIPTS_NAMES = ['scripts-f20.mit.edu', 'scripts-f30.mit.edu']
SCRIPTS_NAMES = ['w-e.mit.edu']
RECIPIENTS = [
'scripts-demo@scripts.mit.edu',
'test-f20@scripts-demo.scripts.mit.edu',
'test-f20@f20.scripts-demo.scripts.mit.edu',
'test-f30@f30.scripts-demo.scripts.mit.edu',
]
SENDER = 'adehnert@mit.edu'
MSG = """To: %(recipient)s
Subject: test to %(recipient)s via %(host)s
test to %(recipient)s via %(host)s
"""
for host in SCRIPTS_NAMES:
with smtplib.SMTP(host) as smtp:
for recipient in RECIPIENTS:
msg = MSG % dict(recipient=recipient, host=host)
smtp.sendmail(SENDER, [recipient], msg)