Skip to content
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

Jump to bottom

parameterize the restrict options #69

Merged
merged 2 commits into from
Feb 20, 2015
Merged

parameterize the restrict options #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7f3682
parameterize the restrict options
Feb 18, 2015
6681abb
parameterize driftfile & tinker sections
Feb 19, 2015
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
30 changes: 26 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ Ensure step tickers file. Valid values are 'present' and 'absent'.

- *Default*: based on OS

driftfile
---------
Path of the drift file. String with absolute path. Set to '' to disable drift file usage. 'USE_DEFAULTS' will choose the appropriate default for the system.

- *Default*: 'USE_DEFAULTS'

service_running
---------------
If service should be running
Expand All @@ -97,9 +103,9 @@ Service has a restart option

keys
----
Path of the symmetric key file. See ntpd(1).
Path of the symmetric key file. See ntpd(1). Set to '' to disable drift file usage. 'USE_DEFAULTS' will choose the appropriate default for the system.

- *Default*: based on OS
- *Default*: 'USE_DEFAULTS'

servers
-------
Expand Down Expand Up @@ -128,11 +134,21 @@ ntp::peers:

- *Default*: 'UNSET'

restrict_localhost
------------------
Array with options to provide to access control configuration (restrict) in ntp.conf.
'USE_DEFAULTS' will choose the appropriate default for the system to allow localhost access only.

- *Default*: 'USE_DEFAULTS'

restrict_options
----------------
String with options to provide to access control configuration (restrict) in ntp.conf
Array with options to provide to access control configuration (restrict) in ntp.conf.
'USE_DEFAULTS' will choose the appropriate default for the system.

For backward compatibility a string can still be used here. It will be used for IPv4 and IPv6 configuration.

- *Default*: 'default kod notrap nomodify nopeer noquery'
- *Default*: 'USE_DEFAULTS'

orphan_mode_stratum
-------------------
Expand All @@ -152,6 +168,12 @@ If statistics should be enabled.

- *Default*: false

enable_tinker
----------------
If tinker should be enabled (boolean).

- *Default*: true

statdir
-------
Directory for storing ntpstats
Expand Down
63 changes: 57 additions & 6 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
'2.us.pool.ntp.org'],
$server_options = 'UNSET',
$peers = 'UNSET',
$restrict_options = 'default kod notrap nomodify nopeer noquery',
$restrict_options = 'USE_DEFAULTS',
$restrict_localhost = 'USE_DEFAULTS',
$step_tickers_ensure = 'USE_DEFAULTS',
$step_tickers_path = '/etc/ntp/step-tickers',
$step_tickers_owner = 'root',
Expand All @@ -32,6 +33,7 @@
$orphan_mode_stratum = 'UNSET',
$fudge_stratum = '10',
$enable_stats = false,
$enable_tinker = 'USE_DEFAULTS',
$statsdir = '/var/log/ntpstats/',
$logfile = 'UNSET',
$ignore_local_clock = false,
Expand Down Expand Up @@ -125,21 +127,27 @@
$default_package_noop = false
$default_package_source = undef
$default_package_adminfile = undef
$default_restrict_options = [ '-4 default kod notrap nomodify nopeer noquery', '-6 default kod notrap nomodify nopeer noquery', ]
$default_restrict_localhost = [ '127.0.0.1', '::1', ]
$default_step_tickers_ensure = 'absent'
$default_service_name = 'ntp'
$default_config_file = '/etc/ntp.conf'
$default_driftfile = '/var/lib/ntp/ntp.drift'
$default_keys = '/etc/ntp/keys'
$default_enable_tinker = true
}
'RedHat': {
$default_package_name = [ 'ntp' ]
$default_package_noop = false
$default_package_source = undef
$default_package_adminfile = undef
$default_restrict_options = [ '-4 default kod notrap nomodify nopeer noquery', '-6 default kod notrap nomodify nopeer noquery', ]
$default_restrict_localhost = [ '127.0.0.1', '::1', ]
$default_step_tickers_ensure = 'present'
$default_service_name = 'ntpd'
$default_config_file = '/etc/ntp.conf'
$default_keys = '/etc/ntp/keys'
$default_enable_tinker = true
if $::operatingsystemmajrelease == '7' or $::lsbmajdistrelease == '7' {
$default_driftfile = '/var/lib/ntp/drift'
} else {
Expand All @@ -150,11 +158,14 @@
$default_package_noop = false
$default_package_source = undef
$default_package_adminfile = undef
$default_restrict_options = [ '-4 default kod notrap nomodify nopeer noquery', '-6 default kod notrap nomodify nopeer noquery', ]
$default_restrict_localhost = [ '127.0.0.1', '::1', ]
$default_step_tickers_ensure = 'absent'
$default_service_name = 'ntp'
$default_config_file = '/etc/ntp.conf'
$default_driftfile = '/var/lib/ntp/drift/ntp.drift'
$default_keys = undef
$default_keys = ''
$default_enable_tinker = true

case $::lsbmajdistrelease {
'9','10': {
Expand All @@ -171,10 +182,14 @@
'Solaris': {
case $::kernelrelease {
'5.9','5.10': {
$default_package_name = [ 'SUNWntp4r', 'SUNWntp4u' ]
$default_package_name = [ 'SUNWntp4r', 'SUNWntp4u' ]
$default_restrict_options = [ 'default noserve noquery', ]
$default_restrict_localhost = [ '127.0.0.1', ]
}
'5.11': {
$default_package_name = [ 'network/ntp' ]
$default_package_name = [ 'network/ntp' ]
$default_restrict_options = [ 'default kod notrap nomodify nopeer noquery', ]
$default_restrict_localhost = [ '127.0.0.1', '::1', ]
}
default: {
fail("The ntp module supports Solaris kernel release 5.9, 5.10 and 5.11. You are running ${::kernelrelease}.")
Expand All @@ -188,6 +203,7 @@
$default_config_file = '/etc/inet/ntp.conf'
$default_driftfile = '/var/ntp/ntp.drift'
$default_keys = '/etc/inet/ntp.keys'
$default_enable_tinker = false
}
default: {
fail("The ntp module is supported by OS Families Debian, RedHat, Suse, and Solaris. Your operatingsystem, ${::operatingsystem}, is part of the osfamily, ${::osfamily}")
Expand Down Expand Up @@ -240,6 +256,10 @@
$driftfile_real = $driftfile
}

if $driftfile_real {
validate_absolute_path($driftfile_real)
}

if $step_tickers_ensure == 'USE_DEFAULTS' {
$step_tickers_ensure_real = $default_step_tickers_ensure
} else {
Expand All @@ -256,11 +276,42 @@
$keys_real = $keys
}

if $keys_real != undef {
if $keys_real {
validate_absolute_path($keys_real)
}

validate_string($restrict_options)
if is_bool($enable_tinker) == true {
$enable_tinker_real = $enable_tinker
} else {
$enable_tinker_real = $enable_tinker ? {
'USE_DEFAULTS' => $default_enable_tinker,
default => str2bool($enable_tinker)
}
}

if is_array($restrict_options) == true {
$restrict_options_real = $restrict_options
}
# needed for backward compatibility
elsif is_string($restrict_options) == true {
$restrict_options_real = $restrict_options ? {
'USE_DEFAULTS' => $default_restrict_options,
default => [ "-4 $restrict_options", "-6 $restrict_options", ]
}
}
else {
fail("restrict_options must be an array (prefered) or a string (will be deprecated).")
Copy link
Owner

Choose a reason for hiding this comment

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

Why will string be deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a suggestion only
Well, the new handling in the template is using an arrays. In my eyes it is preferable to have input and output variables in the same format. It avoids possible conversation errors and surprises on the user side. If you use a string it result in two lines with -4 and -6 as prefix. Using an array it will output exaxtly your input.

Above string to array conversation is a dirty workaround in my eyes. It's (hopefully) only there to not break things and expectations until the next major version update.

Copy link
Owner

Choose a reason for hiding this comment

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

Totally agree. If we bump the major version, we can introduce other breaking changes. Is there anything else we should change? I was thinking of merging this and your solaris code and making that the last of v3 and then we could move to v4, but I do not want to do that unless there are really compelling reasons to break compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On a major change, both restrict options could be merged into one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could rename $disable_monitor to $enable_monitor to have positive wording :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

technicaly restrict_options and restrict_localhost could get merged into one (would break backward compatibility now)

}

if is_array($restrict_localhost) == true {
$restrict_localhost_real = $restrict_localhost
}
elsif $restrict_localhost == 'USE_DEFAULTS' {
$restrict_localhost_real = $default_restrict_localhost
}
else {
fail("restrict_localhost must be an array or the string 'USE_DEFAULTS'.")
}

# validate $my_enable_stats - must be true or false
case $my_enable_stats {
Expand Down
Loading