voxpupuli / puppet-postfix

Puppet postfix module
Apache License 2.0
70 stars 174 forks source link

Adjust postfix::config to use value type for appropriate action #290

Closed fincreg closed 3 years ago

fincreg commented 3 years ago

Allow create flexible profiles, that managed through hiera

If you do not specify ensure, than analyze $value and act accordingly. Remove if undef, clear if empty, set if string. Allows to create hiera managed multitype profiles

mx.yaml

---
profile::postfix::virtual_mailbox_domains: 'ldap:/etc/postfix/ldap-vmd.cf'
profile::postfix::virtual_mailbox_maps: 'ldap:/etc/postfix/ldap-vmm.cf'
profile::postfix::virtual_alias_maps: 'ldap:/etc/postfix/ldap-vam.cf'

relay.yaml

---
profile::postfix::smtpd_sasl_auth_enable: 'yes'
profile::postfix::smtpd_sasl_path: inet:127.0.0.1:5555

profile::postfix::virtual_mailbox_domains: 'ldap:/etc/postfix/ldap-vmd.cf'
profile::postfix::virtual_mailbox_maps: 'ldap:/etc/postfix/ldap-vmm.cf'
profile::postfix::virtual_alias_maps: 'ldap:/etc/postfix/ldap-vam.cf'

profile/manifests/postfix.pp

class profile::postfix (
  # Auth settings / Not for MX
  Variant[Undef, String] $smtpd_sasl_auth_enable                  = undef,
  Variant[Undef, String] $smtpd_sasl_path                         = undef,

  # Virtual settings
  Variant[Undef, String] $virtual_mailbox_domains                 = undef,
  Variant[Undef, String] $virtual_mailbox_maps                    = undef,
  Variant[Undef, String] $virtual_alias_maps                      = undef,
) {
  include ::postfix

  postfix::config {
    'smtpd_sasl_auth_enable':                    value => $smtpd_sasl_auth_enable;
    'smtpd_sasl_path':                           value => $smtpd_sasl_path;

    'virtual_mailbox_domains':                   value => $virtual_mailbox_domains;
    'virtual_mailbox_maps':                      value => $virtual_mailbox_maps;
    'virtual_alias_maps':                        value => $virtual_alias_maps;
  }
}
oleksandriegorov commented 3 years ago

@fincreg and myself worked hard to add new functionality and to fix all ruby/spec/acceptance tests. Once travis-ci catches up - we will have them all green. There are several conflicts, especially in .travis.yml (because I removed entire section for centos6, which fails due to EOL and no repos available). I can also offer a hand in reviewing "conflicting files", cause I think I was the one introducing changes there too. Please consider our pull request.

fincreg commented 3 years ago

Resolved merge conflicts

fincreg commented 3 years ago

@raphink, please review this PR

oleksandriegorov commented 3 years ago

Bump. Is there anything we need to discuss before this pull request can be merged?

raphink commented 3 years ago

I've left a few comments. I think some changes are out of the scope of your change (if required at all).

fincreg commented 3 years ago

Converted to draft until all suggestions reviewed.

raphink commented 3 years ago

Don't hesitate to mention me again if you need guidance for this PR. The amount of notifications tends to make me a bit hard of hearing!

fincreg commented 3 years ago

Reset till the initial update changes and rebased from camptocamp:master #291

raphink commented 3 years ago

After thinking about this again, I don't see a way to keep compatibility without making the interface unnecessarily complex.

My suggestion is to add a defined resource type to your profile:

define profile::postfix::config (
  $value,
) {
  $ensure = $value ? {
    /^.+$/  => 'present',
    /^$/    => 'blank',
    default => 'absent',
  }

  postfix::config { $title:
    ensure => $ensure,
    name   => $name,
    value  => value,
  }
}

and use that in your code instead of puppet::postfix.

If other people would be interested in that change, and willing to overgo a breaking change for this, I can reconsider, but for now it seems the most reasonable option.

fincreg commented 3 years ago

I believe we can stick to this solution. Thank you. Closing PR