Skip to content

Draft: Eliminate params.pp #426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Draft: Eliminate params.pp #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 35 additions & 81 deletions REFERENCE.md
View file
Open in desktop

Large diffs are not rendered by default.

51 changes: 45 additions & 6 deletions data/common.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,46 @@
---
postfix::params::aliasesseltype: ~
postfix::params::seltype: ~
postfix::params::mailx_package: 'mailx'
postfix::params::master_os_template: "postfix/master.cf.default.erb"
postfix::params::restart_cmd: '/etc/init.d/postfix reload'
...
postfix::alias_maps: hash:/etc/aliases
postfix::amavis_procs: 2
postfix::confdir: "/etc/postfix"
postfix::conffiles: {}
postfix::configs: {}
postfix::hashes: {}
postfix::inet_interfaces: all
postfix::inet_protocols: all
postfix::ldap: false
postfix::ldap_packages: []
postfix::lookup_table_type: hash
postfix::mail_user: vmail
postfix::mailaliases: {}
postfix::mailman: false
postfix::mailx_ensure: present
postfix::mailx_package: mailx
postfix::maincf_source: puppet:///modules/postfix/main.cf
postfix::manage_aliases: true
postfix::manage_conffiles: true
postfix::manage_mailname: true
postfix::manage_mailx: true
postfix::manage_root_alias: true
postfix::maps: {}
postfix::master_bounce_command: bounce
postfix::master_defer_command: bounce
postfix::master_entries: []
postfix::master_os_template: postfix/master.cf.default.erb
postfix::mta: false
postfix::mydestination: "$myhostname, localhost.$mydomain, localhost"
postfix::mynetworks: 127.0.0.0/8 [::ffff:127.0.0.0]/104 [::1]/128
postfix::myorigin: "%{facts.networking.fqdn}"
postfix::postfix_ensure: present
postfix::restart_cmd: "/etc/init.d/postfix reload"
postfix::root_group: root
postfix::root_mail_recipient: nobody
postfix::satellite: false
postfix::service_enabled: true
postfix::service_ensure: running
postfix::smtp_listen: 127.0.0.1
postfix::transports: {}
postfix::use_amavisd: false
postfix::use_dovecot_lda: false
postfix::use_schleuder: false
postfix::use_sympa: false
postfix::virtuals: {}
Comment on lines +2 to +46
Copy link
Member

@smortex smortex May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only values which are OS-dependent (i.e. present in multiple hiera .yaml files) should be in common.yaml. Values which are not OS-dependent must be kept in init.pp so that they are documented in REFERENCE.md.

Copy link
Contributor Author

@cocker-cc cocker-cc May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Values which are not OS-dependent must be kept in init.pp

I'm afraid having to disagree here. Since the Appearance of Hiera, APL (automatic Parameterlookup) is definitely the prefered Method for Defaultvalues. OS-Dependency is not a Criterion.
IMHO

Copy link
Member

@kenyon kenyon May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Vox Pupuli convention is as @smortex explained: OS-independent defaults as class parameter defaults so that they are documented in REFERENCE.md. OS-dependent defaults go into hiera data.

Copy link
Contributor Author

@cocker-cc cocker-cc May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Vox Pupuli convention is as @smortex explained

Where to find this?

The Link to Puppet Labs style guide in your README.md is dead, as well as the Link on voxpupuli.org

This Puppet-7-Style-Guide reads:

Use Hiera data in your module to set parameter defaults.

This exactly is, what I stick to, because

Only values which are OS-dependent (i.e. present in multiple hiera .yaml files) should be in common.yaml.

is plain inconsistent IMHO. The whole Sense of Hiera is to decouple Data from Code, to make the Code OS-agnostic. This very importand Paradigm should not be sacrificed for REFERENCE.md. (why not let puppet strings generate read from Hiera? *wink*)

Recognizing, that this is the wrong Location to discuss this Subject, feel free to adopt my Changes under your own Author-Name.

Copy link
Member

@kenyon kenyon May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Vox Pupuli convention is documented here: https://voxpupuli.org/docs/reviewing_pr/

Quoting that page:

Static data that is equal across every supported operating system must stay in the init.pp, it shouldn’t be moved to a common.yaml due to puppet-strings issue #250.

Yes, Perforce broke all of the puppet docs links.

2 changes: 1 addition & 1 deletion data/os/Alpine.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
---
postfix::params::master_os_template: "postfix/master.cf.debian.erb"
postfix::master_os_template: postfix/master.cf.debian.erb
7 changes: 3 additions & 4 deletions data/os/FreeBSD.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
postfix::confdir: "/usr/local/etc/postfix"
postfix::manage_mailname: false
postfix::manage_mailx: false
postfix::root_group: "wheel"
postfix::params::master_os_template: "postfix/master.cf.FreeBSD.erb"
postfix::params::restart_cmd: "/usr/local/etc/rc.d/postfix reload"
...
postfix::master_os_template: postfix/master.cf.FreeBSD.erb
postfix::restart_cmd: "/usr/local/etc/rc.d/postfix reload"
postfix::root_group: wheel
7 changes: 3 additions & 4 deletions data/os/Solaris.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
postfix::params::master_os_template: "postfix/master.cf.Solaris.erb"
postfix::params::restart_cmd: '/usr/sbin/svcadm refresh network/smtp:postfix'
postfix::params::mailx_package: 'mailx'
...
postfix::mailx_package: mailx
postfix::master_os_template: postfix/master.cf.Solaris.erb
postfix::restart_cmd: "/usr/sbin/svcadm refresh network/smtp:postfix"
8 changes: 4 additions & 4 deletions data/osfamily/Debian.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
postfix::params::mailx_package: 'bsd-mailx'
postfix::params::master_os_template: 'postfix/master.cf.debian.erb'
postfix::ldap_packages: ['postfix-ldap']
...
postfix::ldap_packages:
- postfix-ldap
postfix::mailx_package: bsd-mailx
postfix::master_os_template: postfix/master.cf.debian.erb
3 changes: 1 addition & 2 deletions data/osfamily/Debian/etch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
---
postfix::params::mailx_package: 'mailx'
...
postfix::mailx_package: mailx
3 changes: 1 addition & 2 deletions data/osfamily/Debian/lenny.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
---
postfix::params::mailx_package: 'mailx'
...
postfix::mailx_package: mailx
3 changes: 1 addition & 2 deletions data/osfamily/Debian/sarge.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
---
postfix::params::mailx_package: 'mailx'
...
postfix::mailx_package: mailx
11 changes: 5 additions & 6 deletions data/osfamily/RedHat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
postfix::params::aliasesseltype: 'etc_aliases_t'
postfix::params::seltype: 'postfix_etc_t'
postfix::params::restart_cmd: '/bin/systemctl reload postfix'
postfix::params::master_os_template: 'postfix/master.cf.redhat.erb'
postfix::mta_bin_path: '/usr/sbin/sendmail.postfix'
...
postfix::aliasesseltype: etc_aliases_t
postfix::master_os_template: postfix/master.cf.redhat.erb
postfix::mta_bin_path: "/usr/sbin/sendmail.postfix"
postfix::restart_cmd: "/bin/systemctl reload postfix"
postfix::seltype: postfix_etc_t
9 changes: 5 additions & 4 deletions data/osfamily/RedHat/10.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
postfix::alias_maps: 'lmdb:/etc/aliases'
postfix::ldap_packages: ['postfix-ldap']
postfix::lookup_table_type: 'lmdb'
postfix::params::mailx_package: 's-nail'
postfix::alias_maps: lmdb:/etc/aliases
postfix::ldap_packages:
- postfix-ldap
postfix::lookup_table_type: lmdb
postfix::mailx_package: s-nail
7 changes: 3 additions & 4 deletions data/osfamily/RedHat/4.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
postfix::params::aliasesseltype: 'etc_t'
postfix::params::seltype: 'etc_t'
postfix::params::restart_cmd: '/etc/init.d/postfix reload'
...
postfix::aliasesseltype: etc_t
postfix::restart_cmd: "/etc/init.d/postfix reload"
postfix::seltype: etc_t
5 changes: 2 additions & 3 deletions data/osfamily/RedHat/5.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
---
postfix::params::aliasesseltype: 'postfix_etc_t'
postfix::params::restart_cmd: '/etc/init.d/postfix reload'
...
postfix::aliasesseltype: postfix_etc_t
postfix::restart_cmd: "/etc/init.d/postfix reload"
3 changes: 1 addition & 2 deletions data/osfamily/RedHat/6.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
---
postfix::params::restart_cmd: '/etc/init.d/postfix reload'
...
postfix::restart_cmd: "/etc/init.d/postfix reload"
6 changes: 3 additions & 3 deletions data/osfamily/RedHat/9.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
---

postfix::params::mailx_package: 's-nail'
postfix::ldap_packages: ['postfix-ldap']
postfix::ldap_packages:
- postfix-ldap
postfix::mailx_package: s-nail
2 changes: 1 addition & 1 deletion manifests/conffile.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
mode => $mode,
owner => 'root',
group => 'postfix',
seltype => $postfix::params::seltype,
seltype => $postfix::seltype,
require => Package['postfix'],
source => $source,
content => $manage_content,
Expand Down
10 changes: 5 additions & 5 deletions manifests/files.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
ensure => 'file',
content => "${facts['networking']['fqdn']}\n",
mode => '0644',
seltype => $postfix::params::seltype,
seltype => $postfix::seltype,
}
}

Expand All @@ -62,7 +62,7 @@
content => "# file managed by puppet\n",
notify => Exec['newaliases'],
replace => false,
seltype => $postfix::params::aliasesseltype,
seltype => $postfix::aliasesseltype,
}
}

Expand All @@ -75,7 +75,7 @@
$_mastercf_content = epp($mastercf_template)
} else {
$_mastercf_content = template(
$postfix::params::master_os_template,
$postfix::master_os_template,
'postfix/master.cf.common.erb'
)
}
Expand All @@ -86,7 +86,7 @@
group => $postfix::root_group,
mode => '0644',
owner => 'root',
seltype => $postfix::params::seltype,
seltype => $postfix::seltype,
source => $mastercf_source,
}

Expand All @@ -97,7 +97,7 @@
mode => '0644',
owner => 'root',
replace => false,
seltype => $postfix::params::seltype,
seltype => $postfix::seltype,
source => $maincf_source,
}

Expand Down
2 changes: 1 addition & 1 deletion manifests/hash.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
Optional[Variant[Sensitive[String],String]] $content = undef,
Stdlib::Filemode $mode = '0640',
) {
include postfix::params
include postfix

assert_type(Stdlib::Absolutepath, $name)

Expand Down
115 changes: 68 additions & 47 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@
# @param manage_mailx
# A Boolean defining whether the puppet module should manage the mailx package. See also $mailx_ensure.
#
# @param mailx_package
# Name of package that provides mailx
#
# @param manage_root_alias
# Wheter to manage the mailalias for root user
#
Expand All @@ -148,6 +151,9 @@
# @param master_defer_command
# The defer command which should be used in master.cf
#
# @param master_os_template
# Path to the master template
#
# @param master_entries
# Array of strings containing additional entries for the /etc/postfix/master.cf file.
# Example: `['submission inet n - n - - smtpd']`.
Expand Down Expand Up @@ -235,6 +241,9 @@
# @param service_ensure
# Defines the service state of 'postfix' service
#
# @param restart_cmd
# Command to use when restarting postfix
#
# @param smtp_listen
# A string or an array of strings to define the IPs on which to listen in master.cf.
# This can also be set to 'all' to listen on all interfaces. If master_smtp is defined
Expand All @@ -260,65 +269,77 @@
# @param virtuals
# A hash of postfix::virtual resources
#
# @param aliasesseltype
# Selinux type for /etc/aliases
#
# @param seltype
# Selinux type for /etc/postfix/* config files
#
class postfix (
String $alias_maps = 'hash:/etc/aliases',
Integer $amavis_procs = 2,
String $alias_maps,
Integer $amavis_procs,
Stdlib::Absolutepath $confdir,
Hash $conffiles,
Hash $configs,
Hash $hashes,
String $inet_interfaces,
String $inet_protocols,
Boolean $ldap,
Array[String[1]] $ldap_packages,
String $lookup_table_type,
Hash $mailaliases,
String $mail_user, # postfix_mail_user
Boolean $mailman,
String $mailx_ensure,
String $mailx_package,
String $maincf_source,
Boolean $manage_aliases, # /etc/aliases
Boolean $manage_conffiles,
Boolean $manage_mailname,
Boolean $manage_mailx,
Boolean $manage_root_alias,
Hash $maps,
String $master_bounce_command,
String $master_defer_command,
String $master_os_template,
Array[String] $master_entries, # postfix_master_entries
Boolean $mta,
String $mydestination, # postfix_mydestination
String $mynetworks, # postfix_mynetworks
String $myorigin,
String $postfix_ensure,
String $root_group,
Variant[Array[String], String] $root_mail_recipient, # root_mail_recipient
Boolean $satellite,
Boolean $service_enabled,
String $service_ensure,
String $restart_cmd,
Variant[Array[String[1]], String[1]] $smtp_listen, # postfix_smtp_listen
Hash $transports,
Boolean $use_amavisd, # postfix_use_amavisd
Boolean $use_dovecot_lda, # postfix_use_dovecot_lda
Variant[Integer[2, 3], Boolean] $use_schleuder, # postfix_use_schleuder
Boolean $use_sympa, # postfix_use_sympa
Hash $virtuals,

Optional[Boolean] $chroot = undef,
Stdlib::Absolutepath $confdir = '/etc/postfix',
Hash $conffiles = {},
Hash $configs = {},
Hash $hashes = {},
String $inet_interfaces = 'all',
String $inet_protocols = 'all',
Boolean $ldap = false,
Optional[String] $ldap_base = undef,
Optional[String] $ldap_host = undef,
Optional[String] $ldap_options = undef,
Array[String[1]] $ldap_packages = [],
String $lookup_table_type = 'hash',
Hash $mailaliases = {},
String $mail_user = 'vmail', # postfix_mail_user
Boolean $mailman = false,
String $mailx_ensure = 'present',
String $maincf_source = "puppet:///modules/${module_name}/main.cf",
Boolean $manage_aliases = true, # /etc/aliases
Boolean $manage_conffiles = true,
Boolean $manage_mailname = true,
Boolean $manage_mailx = true,
Boolean $manage_root_alias = true,
Hash $maps = {},
String $master_bounce_command = 'bounce',
String $master_defer_command = 'bounce',
Array[String] $master_entries = [], # postfix_master_entries
Optional[String] $master_smtp = undef, # postfix_master_smtp
Optional[String] $master_smtps = undef, # postfix_master_smtps
Optional[String] $master_submission = undef, # postfix_master_submission
Optional[String] $master_smtp = undef, # postfix_master_smtp
Optional[String] $master_smtps = undef, # postfix_master_smtps
Optional[String] $master_submission = undef, # postfix_master_submission
Optional[String] $mastercf_content = undef,
Optional[String] $mastercf_source = undef,
Optional[String] $mastercf_template = undef,
Optional[Array[String[1]]] $masquerade_classes = undef,
Optional[Array[String[1]]] $masquerade_domains = undef,
Optional[Array[String[1]]] $masquerade_exceptions = undef,
Optional[Stdlib::Absolutepath] $mta_bin_path = undef,
Boolean $mta = false,
String $mydestination = '$myhostname, localhost.$mydomain, localhost', # postfix_mydestination
String $mynetworks = '127.0.0.0/8 [::ffff:127.0.0.0]/104 [::1]/128', # postfix_mynetworks
String $myorigin = $facts['networking']['fqdn'],
String $postfix_ensure = 'present',
Optional[String] $relayhost = undef, # postfix_relayhost
String $root_group = 'root',
Variant[Array[String], String] $root_mail_recipient = 'nobody', # root_mail_recipient
Boolean $satellite = false,
Boolean $service_enabled = true,
String $service_ensure = 'running',
Variant[Array[String[1]], String[1]] $smtp_listen = '127.0.0.1', # postfix_smtp_listen
Hash $transports = {},
Boolean $use_amavisd = false, # postfix_use_amavisd
Boolean $use_dovecot_lda = false, # postfix_use_dovecot_lda
Variant[Integer[2, 3], Boolean] $use_schleuder = false, # postfix_use_schleuder
Boolean $use_sympa = false, # postfix_use_sympa
Hash $virtuals = {},
) inherits postfix::params {
Optional[String] $relayhost = undef, # postfix_relayhost
Optional[String] $aliasesseltype = undef,
Optional[String] $seltype = undef,
) {
if (
($mastercf_source and $mastercf_content) or
($mastercf_source and $mastercf_template) or
Expand Down
1 change: 0 additions & 1 deletion manifests/map.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
Stdlib::Filemode $mode = '0640',
) {
include postfix
include postfix::params

$_path = pick($path, "${postfix::confdir}/${name}")

Expand Down
2 changes: 1 addition & 1 deletion manifests/packages.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
if ($postfix::manage_mailx) {
package { 'mailx':
ensure => $postfix::mailx_ensure,
name => $postfix::params::mailx_package,
name => $postfix::mailx_package,
}
}
}
Loading
Loading