search

linux-system-roles / postfix

An ansible role which configures postfix
https://linux-system-roles.github.io/postfix/
GNU General Public License v3.0
14 stars 20 forks source link

Add "previous: replaced" functionality to postfix_conf dict to reset postfix configuration #41

Closed spetrosi closed 2 years ago

spetrosi commented 2 years ago

This is a rework of #40.

This PR adds functionality to reset configuration. It does rpm -V to verify if configs has been changed and reinstall the package, similar to what Noriko did in https://github.com/linux-system-roles/logging/pull/264.

More info in commits.

spetrosi commented 2 years ago

[citest]

richm commented 2 years ago

This looks pretty good. But the big problem is that it isn't idempotent. Not sure how we can make this idempotent

richm commented 2 years ago

How about this? It isn't idempotent initially, but it will eventually be (mostly) idempotent: every time we do a reinstall, copy /etc/postfix/main.cf to /etc/postfix/lsr-orig/main.cf (or something like that - save the original RPM config to some safe location)

then

when user uses previous: replaced: mktmpdir tmpcur - copy current main.cf to this directory mktmpdir tmporig - copy orig main.cf to this directory for each key,value in postfix_conf: postconf -c tmpcur -e key=value postconf -c tmporig -e key=value if diff tmpcur/main.cf tmporig/main.cf copy tmporig/main.cf /etc/postfix/main.cf changed = true else no changes changed = false

If the changes in postfix_conf are already in main.cf then current main.cf and orig main.cf should be the same after applying the changes - but if there are any changes in the current main.cf that are not in the orig main.cf, then they will be different after applying the changes - and if we replace /etc/postfix/main.cf with the tmporig to which we have applied the changes, that will essentially be the same as erasing any previous values

spetrosi commented 2 years ago

@richm in your solution, the role still re-installs the RPM, right? The re-installation tasks will still result in a changed status, and setting changed_when: false on them seems not fair. I also think that it will be hard to read the output of the role with the proposed solution. What I did in #40 was completely idempotent. It ended up ugly because it is not possible to load the distribution defaults after they have been changed. And maintaining such a list of defaults manually might be cumbersome. I think that if the role re-installes the RPM, there is no need to simulate idempotency. There is also master.cf config file that will be re-written with re-installation, so we would need to work around it as well. It's more fair to say that previous: replaced re-installs the package and hence is not idempotent.

The main issue is unavailability of the distribution defaults. I can think of another solution:

  1. Identify what version of postfix is installed, if any. rpm -q postfix, let's imagine it's postfix-3.5.8-3.el8.x86_64.
  2. dnf download postfix-3.5.8-3.el8.x86_64 or yumdownloader postfix-3.5.8-3.el8.x86_64 to download the RPM on dnf or yum.
  3. rpm2cpio postfix-3.5.8-3.el8.x86_64 | cpio -ivd ./etc/postfix/main.cf ./etc/postfix/master.cf to extract config files.
  4. Proceed to what I did in #40, but instead of manually assigning red_hat_defaults, automatically collect them by comparing postconf -d and the distribution default config files that we now have. The tools used are available on fresh RHEL 7 for yum and RHEL 8 for dnf in 1minutetip.

Because #40 was completely idempotent, this will be too. Completely idempotent - if I set relayhost = example.com on a fresh postfix install and then run the role with postfix_conf: previous=replaced relayhost=example.com, the role finishes without changes. How does this sound?

richm commented 2 years ago

@richm in your solution, the role still re-installs the RPM, right?

Right. There is no other way to get back to the system default config file.

The re-installation tasks will still result in a changed status, and setting changed_when: false on them seems not fair.

Ok. Yes, if there is no saved copy of the original system default config file, then we will have to report changed: true.

I also think that it will be hard to read the output of the role with the proposed solution. What I did in #40 was completely idempotent. It ended up ugly because it is not possible to load the distribution defaults after they have been changed. And maintaining such a list of defaults manually might be cumbersome. I think that if the role re-installes the RPM, there is no need to simulate idempotency. There is also master.cf config file that will be re-written with re-installation, so we would need to work around it as well. It's more fair to say that previous: replaced re-installs the package and hence is not idempotent.

The main issue is unavailability of the distribution defaults. I can think of another solution:

1. Identify what version of postfix is installed, if any. `rpm -q postfix`, let's imagine it's `postfix-3.5.8-3.el8.x86_64`.

2. `dnf download postfix-3.5.8-3.el8.x86_64` or `yumdownloader postfix-3.5.8-3.el8.x86_64` to download the RPM on dnf or yum.

3. `rpm2cpio postfix-3.5.8-3.el8.x86_64  | cpio -ivd ./etc/postfix/main.cf ./etc/postfix/master.cf` to extract config files.

4. Proceed to what I did in [Add postfix_reset functionality that resets configuration to Red Hat defaults #40](https://github.com/linux-system-roles/postfix/pull/40), but instead of manually assigning [red_hat_defaults](https://github.com/linux-system-roles/postfix/pull/40/files#diff-3d0ff1709ca48add100327bb2a468e6c508fb92a159c64c4f99ad1df89d9bddeR26-R45), automatically collect them by comparing `postconf -d` and the distribution default config files that we now have.
   The tools used are available on fresh RHEL 7 for yum and RHEL 8 for dnf in 1minutetip.

Because #40 was completely idempotent, this will be too. Completely idempotent - if I set relayhost = example.com on a fresh postfix install and then run the role with postfix_conf: previous=replaced relayhost=example.com, the role finishes without changes. How does this sound?

Hmm - this is starting to get very complicated. I think for now that we should go with what you currently have in the PR, and document that using previous: replaced is not idempotent. Then let's see what feedback we get.

richm commented 2 years ago

ok - looks like need rebase on top of https://github.com/linux-system-roles/postfix/pull/44 ?

spetrosi commented 2 years ago

[citest bad]

spetrosi commented 2 years ago

Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2044657