[openSUSE] fix postfix config re ipv4, tlsmgr, & CA file settings. Fixes #2132

[phillxnet]

Thanks to @tyukh for initial reporting and initial diagnosis of the root causes. Due to differing defaults from our prior CentOS base, email notification configuration failed. Adjust configuration mechanism as below.

Fixes #2132
Fixes #2133
Fixes #2035

Includes:

• Un-remark tlsmgr line in master.cf when on openSUSE.
• Disable on-by-default mail sysconfig in openSUSE (see issue [openSUSE] disable config.postfix as we edit main.cf directly #2035)
• Update /etc files postfix/master.cf & sysconfig/mail only if required.
• Adapt Certificate Authority file bundle path to openSUSE contextually.
• Enforce "inet_protocols = ipv4" in postfix if a prior 'all' setting is found. This avoids postfix service failure on ipv4 only systems.
• Move to string.format in email_client.py.
• Black format email_client.py.
• Black format system/osi.py (large file) - in separate commit.
• Fix buggy use of os.chmod.
• Abstract "/root/.forward" for clarity.
• Abstract "/etc/postfix/generic" for clarity.
• Improve code comments.
• Add comments to existing system/osi inplace_replace() function.
• Add general purpose replace_line_if_found() facility to system/osi.py.
• Establish simple system/osi replace_line_if_found() mechanism.
• Avoid postfix log warnings by removing to-be duplicate config options.
• Ensure postfix service is enabled after each config save. The vendor default is 'disabled'.

Additional unrelated trivial fix without associated issue:

• A trivial debug log var refactor omission relating to pr Scheduled tasks: add feature to scan for network devices before shutting down #2039

Testing

No new unit tests were written. However functional testing was performed on all three target dirtos of "rockstor" (CentOS), and our two openSUSE targets: Leap15.1, and Tumbleweed.

From a clean source build, post postfix removal and install (fresh config files & to establish systemd vendor default status) the Rockstor Web-UI was able to successfully test email notification config entry, prior to submitting it, and successfully submit that config there after and subsequently perform the test email send.

All associated config files were inspected pre and post automated editing and the resulting file access rights were confirmed to be as intended.

All 3 platforms, post email notification configuration, were also confirmed to successfully forward self-sent root-directed email using the following procedure:

# cli mail client already installed on our CentOS variants
zypper in mailx
echo "Body text of email alert" | mail -s "Alert Email" root

@FroggyFlox Ready for review.
I was unfortunately unable to separate the black formatting in the case of the smaller file edited in this pr, but the much larger system/osi.py file does have it's black formatting in a separate commit to ease review.

[FroggyFlox]

@phillxnet,
A quick comment before I can test this one out: I noticed you are calling distro.id() every time here. In line with our discussion on a different PR related to using variables defined at build time, I actually used distro_id = settings.OS_DISTRO_ID instead in my recently submitted PR believing it would be slightly faster and/or more efficient.

Would that actually be the case? The difference is probably rather minor but I wanted to know what you think of it.

[phillxnet]

@FroggyFlox Re:

In line with our discussion on a different PR related to using variables defined at build time, I actually used distro_id = settings.OS_DISTRO_ID instead in my recently submitted PR believing it would be slightly faster and/or more efficient.

Yes, good point, I'd actually forgotten about that; however as you say it's a fairly trivial time saver in this case as it's only going to be called once/twice during each email setup, so maybe no more than a handful of times during an installs life time. My original intent with the settings.OS_DISTRO_ID was to save the "Built on ..." / "Uses ..." info and to avoid the repeated calls that are inevitable with it's use in the Web-UI header. I didn't want the Web-UI refresh to be calling the distro library at every refresh interval, though that's only something like every 10 to 30 minutes it's far more pressing a concern than what we have here.

So in short I'd suggest we absolutely use the settings var for frequent calls where we can, i.e. where Leap to Tumbleweed and back don't matter. But my most recent significant use of this library was in the packaging area where we have to account for folks moving their install from Leap to Tumbleweed and back, as they have different repositories. And that was more fresh than the Web-UI header work. Plus the header then denotes what the rpm was built on, or that was the intention, giving added debug value to a screen shot.

I say we leave this as is for now as it's 'mostly harmless' and have more of a talk on the best way to go in the future. In most cases the Django settings is probably best, so in turn I say we also leave that 'as-is' in your current pull request. But we should very definitely avoid frequent calls to distro if we can, and the setting var gives us that option. Plus there may be a related memory trade-off in pulling in the settings but I'm just not up enough of Django workings to know. So swings and roundabouts and I say we go with what we have both used thus far but definitely avoid, if possible, repeated calls. You samba config calls are also likely to only be used once per config, which is not that often. And if there turns out to be a difference, say between Leap / Tumbleweed and folks move there system under us then we will miss it, at least until a new rpm is pulled down. But that is unlikely currently. So yes lets chat on this but get these two pull requests sorted if we can in the mean time.

Nicely spotted though, and brings to the fore the question of how we use this. It's all a bit messy really but I couldn't thing of another way around it while we are in this transition. We might consider ripping out all uses of this now oft used library once we are done with the CentOS target all together.

Incidentally in the recent package work I also tried to call a minimum number of times by calling early and passing the result to the multiple child processes. I had though of doing that here but for a single call per email setup it seemed like unnecessary complexity. I should probably have noted this in the pr.

Thanks for bringing this up.

[FroggyFlox]

Thanks a lot for the very detailed answer!

[FroggyFlox]

Thanks a lot, @phillxnet , for taking care of this and in such a quick time as well!

I tested it to work as intended in a freshly built install on Leap 15.1:

1. Set up an email account (gmail based) and the "Test SMTP params" worked
2. Set up an email account (gmail based) using wrong settings and the "Test SMTP params" showed an error with useful information on possible fixes.
3. Send test email succeeded (upon clicking on the send test email icon)
4. I set up a scheduled task to fail and I do receive the email alert from cron in my gmail inbox as expected:
   
   As you can, the TLS encryption seems to have worked as well.

I couldn't further test the config as I'm not familiar with how postfix should behave, but at least I do receive the emails alerts as they're supposed to!

Thanks!

[FroggyFlox]

Thanks a lot for the very detailed answer!

[phillxnet]

@FroggyFlox Thanks for the review, much appreciated.

Glad you tested with gmail as I didn't, but I did use another TLS email service (with the 587 port) and that worked a treat as well. I'll go ahead and merge this one soon then. Thanks for submitting the preexisting table typo fix, I'd completely missed it.