smoeding / puppet-sendmail

Manage the Sendmail MTA using Puppet
BSD 2-Clause "Simplified" License
1 stars 11 forks source link

AliasFile default is /etc/mail/aliases on FreeBSD 11.2-RELEASE-p4 #24

Open decibelhertz opened 5 years ago

decibelhertz commented 5 years ago

On FreeBSD 11.2-RELEASE-p4, my sendmail.cf file ships with this...

$ fgrep AliasFile /etc/mail/sendmail.cf 
O AliasFile=/etc/mail/aliases

https://github.com/smoeding/puppet-sendmail/blob/master/manifests/aliases/file.pp#L29 is hard-coded to use the params class; https://github.com/smoeding/puppet-sendmail/blob/master/manifests/params.pp#L24 allows no adjustment.

My workaround is to just symlink and require/notify the appropriate classes.

  case $::kernel {
    'FreeBSD': {
      # The sendmail module does not yet have an easy way to adjust the
      # AliasFile parameter in sendmail.cf and in FreeBSD the default
      # is /etc/mail/aliases. We work around that by making a symlink.
      file { '/etc/mail/aliases':
        ensure => 'link',
        target => '../aliases',
        before => Class['sendmail::aliases'],
        notify => Class['sendmail::aliases::newaliases'],
      }

    } default: {
      #NOOP
    }
  }

It would probably be nice to be able to adjust the AliasFile line per a variable in the init class or the like.

That said, this tickles a broader issue, which I am not sure you'd want to tackle, here. That said, I'll mention it: with the conversion to no-Puppet-3-compatibility that happened this year... calling variables from params.pp should probably be avoided per https://www.devco.net/archives/2013/12/09/the-problem-with-params-pp.php . I would suggest a conversion to YAML similar to https://github.com/puppetlabs/puppetlabs-ntp/tree/master/data, which -- in my experience -- makes it easier to deal with multi-OS nuance like this for both coder and user alike, since it puts all variables into Hiera (and thus tunable by users).

Looks like there are ~22 classes to deal with per fgrep -R 'include ::sendmail::params' manifests/ | wc -l. I'd be willing to consider doing a PR to help, but will warn that it would take me a few weeks, based on my travel schedule.

smoeding commented 5 years ago

Thanks for your feedback. I planned to do the transformation from params.pp to hiera with the last major release but didn't have the time. As you found out it touches a lot of locations in the module. Unfortunately my current priorities won't allow me to to work on this in the near future.

I will try to provide the alias file as parameter to the main class to make it a little easier to get that working.

decibelhertz commented 5 years ago

Sounds fine. No rush on my end since my workaround is in place. If you can work this into your params.pp overhaul, I’m satisfied.

smoeding commented 3 years ago

I've just pushed v3.0.0 of the module to the forge. It includes the updated path to the aliases file for FreeBSD. I'm very sorry that it took so long!

I also tried to look at your point concerning params.pp. While I've converted all my other modules in the meantime, I found it difficult to do for this module. All the parameters would have to be parameters for the main class so that they could be managed in Hiera. Unfortunately this results in a major rewrite of the tests as almost all tests would then have to include the main class so that these parameters would have sensible values. I gave up on that since I was afraid to break something when changing code and tests at the same time. Sorry for that!