From 25ee1eb0571f9eed405f7ad46f4fd9fff5907f1b Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Fri, 14 Oct 2016 16:55:50 +0100 Subject: [PATCH 1/2] (FM-5361) Remove all deprecated functionality This finishes the move to a cleaner puppet4 interface. Since this is a breaking change, it also bumps the major version. Users of the previous version who were running without receiving any deprecation warnings can use this version without any changes. --- README.markdown | 10 +- data/common.yaml | 1 - lib/puppet/functions/ntp_dirname.rb | 10 - lib/puppet/parser/functions/ntp_dirname.rb | 17 -- manifests/init.pp | 235 +++++---------------- metadata.json | 2 +- spec/acceptance/ntp_parameters_spec.rb | 16 +- spec/classes/ntp_spec.rb | 20 +- 8 files changed, 68 insertions(+), 243 deletions(-) delete mode 100644 lib/puppet/functions/ntp_dirname.rb delete mode 100644 lib/puppet/parser/functions/ntp_dirname.rb diff --git a/README.markdown b/README.markdown index 2a612ea0..74e68ed6 100644 --- a/README.markdown +++ b/README.markdown @@ -131,10 +131,6 @@ class { '::ntp': The following parameters are available in the `::ntp` class: -####`autoupdate` - -**Deprecated; replaced by the `package_ensure` parameter**. Tells Puppet whether to keep the ntp module updated to the latest version available. Valid options: true or false. Default value: false - ####`broadcastclient` Enable reception of broadcast server messages to any local interface. @@ -162,7 +158,7 @@ Specifies a file to act as a EPP template for the config file. Valid options: st ####`disable_auth` -Do not require cryptographic authentication for broadcast client, multicast +Do not require cryptographic authentication for broadcast client, multicast client and symmetric passive associations. ####`disable_auth` @@ -239,10 +235,10 @@ Tells Puppet to use non-standard maximal poll interval of upstream servers. Vali ####`ntpsigndsocket` Tells NTP to sign packets using the socket in the ntpsigndsocket path. NTP must be configured to sign sockets for this to work. -Valid option: a path to the socket directory; for example, for Samba it would be: +Valid option: a path to the socket directory; for example, for Samba it would be: ~~~~ -ntpsigndsocket = usr/local/samba/var/lib/ntp_signd/ +ntpsigndsocket = usr/local/samba/var/lib/ntp_signd/ ~~~~ Default value: undef. diff --git a/data/common.yaml b/data/common.yaml index 15b53e59..705e1ca2 100644 --- a/data/common.yaml +++ b/data/common.yaml @@ -1,6 +1,5 @@ --- ntp::authprov: ~ -ntp::autoupdate: false ntp::broadcastclient: false ntp::config_dir: ~ ntp::config_file_mode: '0644' diff --git a/lib/puppet/functions/ntp_dirname.rb b/lib/puppet/functions/ntp_dirname.rb deleted file mode 100644 index 5b0cfe63..00000000 --- a/lib/puppet/functions/ntp_dirname.rb +++ /dev/null @@ -1,10 +0,0 @@ -Puppet::Functions.create_function(:ntp_dirname, Puppet::Functions::InternalFunction) do - dispatch :ntp_dirname do - scope_param - optional_repeated_param 'Any', :args - end - def ntp_dirname(scope, *args) - call_function('deprecation', 'ntp_dirname_removal', "ntp_dirname(): this function is deprecated and will be removed at a later time.") - scope.send("function_ntp_dirname", args) - end -end diff --git a/lib/puppet/parser/functions/ntp_dirname.rb b/lib/puppet/parser/functions/ntp_dirname.rb deleted file mode 100644 index acce1fe4..00000000 --- a/lib/puppet/parser/functions/ntp_dirname.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Puppet::Parser::Functions - newfunction(:ntp_dirname, :type => :rvalue, :doc => <<-EOS - Returns the dirname of a path. - EOS - ) do |arguments| - - raise(Puppet::ParseError, "ntp_dirname(): Wrong number of arguments " + - "given (#{arguments.size} for 1)") if arguments.size < 1 - - function_deprecation([:ntp_dirname_removal, 'ntp_dirname(): this function is deprecated and will be removed at a later time.']) - - path = arguments[0] - return File.dirname(path) - end -end - -# vim: set ts=2 sw=2 et : diff --git a/manifests/init.pp b/manifests/init.pp index 8c468de0..06adab63 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,185 +1,58 @@ class ntp ( - Any $autoupdate, - Variant[Boolean, Stdlib::Compat::Bool] $broadcastclient, - Variant[Stdlib::Absolutepath, Stdlib::Compat::Absolute_path] $config, - Variant[Optional[Stdlib::Absolutepath], Any, Undef, Stdlib::Compat::Absolute_path] $config_dir, - Variant[String, Any] $config_file_mode, + Boolean $broadcastclient, + Stdlib::Absolutepath $config, + Optional[Stdlib::Absolutepath] $config_dir, + String $config_file_mode, Optional[String] $config_epp, - Variant[String[1], Stdlib::Compat::String] $config_template, - Variant[Boolean, Stdlib::Compat::Bool] $disable_auth, - Variant[Boolean, Stdlib::Compat::Bool] $disable_dhclient, - Variant[Boolean, Stdlib::Compat::Bool] $disable_kernel, - Variant[Boolean, Stdlib::Compat::Bool] $disable_monitor, - Variant[Optional[Array[String]], Stdlib::Compat::Array] $fudge, - Variant[Stdlib::Absolutepath, Stdlib::Compat::Absolute_path] $driftfile, - Variant[Optional[Stdlib::Absolutepath], Boolean, Undef, Stdlib::Compat::Absolute_path] $leapfile, - Variant[Optional[Stdlib::Absolutepath], Boolean, Undef, Stdlib::Compat::Absolute_path] $logfile, - Variant[Boolean, Stdlib::Compat::Bool] $iburst_enable, - Variant[Array[String], Stdlib::Compat::Array] $keys, - Variant[Boolean, Stdlib::Compat::Bool] $keys_enable, - Variant[Stdlib::Absolutepath, Any] $keys_file, - Variant[Optional[Ntp::Key_id], Pattern['^\d+$'], Pattern['']] $keys_controlkey, - Variant[Optional[Ntp::Key_id], Pattern['^\d+$'], Pattern['']] $keys_requestkey, - Variant[Optional[Array[Ntp::Key_id]], Stdlib::Compat::Array] $keys_trusted, - Variant[Optional[Ntp::Poll_interval], Boolean, Undef, Stdlib::Compat::Numeric] $minpoll, - Variant[Optional[Ntp::Poll_interval], Boolean, Undef, Stdlib::Compat::Numeric] $maxpoll, - Variant[String, Stdlib::Compat::String] $package_ensure, - Variant[Boolean, Stdlib::Compat::Bool] $package_manage, - Variant[Array[String], Stdlib::Compat::Array] $package_name, - Variant[Optional[Integer[0]], Undef, Stdlib::Compat::Numeric] $panic, - Variant[Array[String], Stdlib::Compat::Array] $peers, - Variant[Array[String], Stdlib::Compat::Array] $preferred_servers, - Variant[Array[String], Stdlib::Compat::Array] $restrict, - Variant[Array[String], Stdlib::Compat::Array] $interfaces, - Variant[Array[String], Any] $interfaces_ignore, - Variant[Array[String], Stdlib::Compat::Array] $servers, - Variant[Boolean, Stdlib::Compat::Bool] $service_enable, - Variant[String, Stdlib::Compat::String] $service_ensure, - Variant[Boolean, Stdlib::Compat::Bool] $service_manage, - Variant[String, Stdlib::Compat::String] $service_name, - Variant[String, Any] $service_provider, - Variant[Optional[Integer[0, 65535]], Boolean, Undef, Stdlib::Compat::Numeric] $stepout, - Variant[Optional[Stdlib::Absolutepath], Boolean, Undef, Stdlib::Compat::String] $step_tickers_file, + Optional[String] $config_template, + Boolean $disable_auth, + Boolean $disable_dhclient, + Boolean $disable_kernel, + Boolean $disable_monitor, + Optional[Array[String]] $fudge, + Stdlib::Absolutepath $driftfile, + Optional[Stdlib::Absolutepath] $leapfile, + Optional[Stdlib::Absolutepath] $logfile, + Boolean $iburst_enable, + Array[String] $keys, + Boolean $keys_enable, + Stdlib::Absolutepath $keys_file, + Optional[Ntp::Key_id] $keys_controlkey, + Optional[Ntp::Key_id] $keys_requestkey, + Optional[Array[Ntp::Key_id]] $keys_trusted, + Optional[Ntp::Poll_interval] $minpoll, + Optional[Ntp::Poll_interval] $maxpoll, + String $package_ensure, + Boolean $package_manage, + Array[String] $package_name, + Optional[Integer[0]] $panic, + Array[String] $peers, + Array[String] $preferred_servers, + Array[String] $restrict, + Array[String] $interfaces, + Array[String] $interfaces_ignore, + Array[String] $servers, + Boolean $service_enable, + String $service_ensure, + Boolean $service_manage, + String $service_name, + Optional[String] $service_provider, + Optional[Integer[0, 65535]] $stepout, + Optional[Stdlib::Absolutepath] $step_tickers_file, Optional[String] $step_tickers_epp, - Variant[Optional[String], Any, Undef, Stdlib::Compat::String] $step_tickers_template, - Variant[Optional[Boolean], Optional[Stdlib::Compat::Bool]] $tinker, - Variant[Boolean, Stdlib::Compat::Bool] $tos, - Variant[Optional[Integer[1]], Boolean, Undef, Stdlib::Compat::Numeric] $tos_minclock, - Variant[Optional[Integer[1]], Boolean, Undef, Stdlib::Compat::Numeric] $tos_minsane, - Variant[Optional[Integer[1]], Boolean, Undef, Stdlib::Compat::Numeric] $tos_floor, - Variant[Optional[Integer[1]], Boolean, Undef, Stdlib::Compat::Numeric] $tos_ceiling, - Variant[Variant[Boolean, Integer[0,1]], Boolean, Undef, Pattern['^[0|1]$']] $tos_cohort, - Variant[Boolean, Stdlib::Compat::Bool] $udlc, - Variant[Optional[Integer[1,15]], Any] $udlc_stratum, - Variant[Optional[Stdlib::Absolutepath], Boolean, Undef, Stdlib::Compat::Absolute_path] $ntpsigndsocket, - Variant[Optional[String], Any, Undef, Stdlib::Compat::String] $authprov, + Optional[String] $step_tickers_template, + Optional[Boolean] $tinker, + Boolean $tos, + Optional[Integer[1]] $tos_minclock, + Optional[Integer[1]] $tos_minsane, + Optional[Integer[1]] $tos_floor, + Optional[Integer[1]] $tos_ceiling, + Variant[Boolean, Integer[0,1]] $tos_cohort, + Boolean $udlc, + Optional[Integer[1,15]] $udlc_stratum, + Optional[Stdlib::Absolutepath] $ntpsigndsocket, + Optional[String] $authprov, ) { - - validate_legacy(Boolean, 'validate_bool', $broadcastclient) - validate_legacy(Stdlib::Absolutepath, 'validate_absolute_path', $config) - - assert_type(String, $config_file_mode) |$expected, $actual| { - deprecation("ntp::config_file_mode should be '${expected}', not '${actual}'") - } - - validate_legacy(String[1], 'validate_string', $config_template) - validate_legacy(Boolean, 'validate_bool', $disable_auth) - validate_legacy(Boolean, 'validate_bool', $disable_dhclient) - validate_legacy(Boolean, 'validate_bool', $disable_kernel) - validate_legacy(Boolean, 'validate_bool', $disable_monitor) - validate_legacy(Stdlib::Absolutepath, 'validate_absolute_path', $driftfile) - - if $logfile { validate_legacy(Optional[Stdlib::Absolutepath], 'validate_absolute_path', $logfile) } - elsif $logfile == undef { } - else { deprecation('ntp::logfile is false, but should be undef') } - - if $ntpsigndsocket { validate_legacy(Optional[Stdlib::Absolutepath], 'validate_absolute_path', $ntpsigndsocket) } - elsif $ntpsigndsocket == undef { } - else { deprecation('ntp::ntpsigndsocket is false, but should be undef') } - - if $leapfile { validate_legacy(Optional[Stdlib::Absolutepath], 'validate_absolute_path', $leapfile) } - elsif $leapfile == undef { } - else { deprecation('ntp::leapfile is false, but should be undef') } - - validate_legacy(Boolean, 'validate_bool', $iburst_enable) - validate_legacy(Array[String], 'validate_array', $keys) - validate_legacy(Boolean, 'validate_bool', $keys_enable) - validate_legacy(Optional[Ntp::Key_id], 'validate_re', $keys_controlkey, ['^\d+$', '']) - validate_legacy(Optional[Ntp::Key_id], 'validate_re', $keys_requestkey, ['^\d+$', '']) - validate_legacy(Optional[Array[Ntp::Key_id]], 'validate_array', $keys_trusted) - if $minpoll { validate_legacy(Optional[Ntp::Poll_interval], 'validate_numeric', $minpoll, 16, 3) } - elsif $minpoll == undef { } - else { deprecation('ntp::minpoll is false, but should be undef') } - - if $maxpoll { validate_legacy(Optional[Ntp::Poll_interval], 'validate_numeric', $maxpoll, 16, 3) } - elsif $maxpoll == undef { } - else { deprecation('ntp::maxpoll is false, but should be undef') } - - validate_legacy(String, 'validate_string', $package_ensure) - validate_legacy(Boolean, 'validate_bool', $package_manage) - validate_legacy(Array[String], 'validate_array', $package_name) - - validate_legacy(Array[String], 'validate_array', $preferred_servers) - validate_legacy(Array[String], 'validate_array', $restrict) - validate_legacy(Array[String], 'validate_array', $interfaces) - assert_type(Array[String], $interfaces_ignore) |$expected, $actual| { - deprecation("ntp::interfaces_ignore should be '${expected}', not '${actual}'") - } - validate_legacy(Array[String], 'validate_array', $servers) - validate_legacy(Array[String], 'validate_array', $fudge) - validate_legacy(Boolean, 'validate_bool', $service_enable) - validate_legacy(String, 'validate_string', $service_ensure) - validate_legacy(Boolean, 'validate_bool', $service_manage) - validate_legacy(String, 'validate_string', $service_name) - assert_type(String, $service_provider) |$expected, $actual| { - deprecation('puppet_3_type_check', "ntp::service_provider should be '${expected}', not '${actual}'") - } - - if $stepout { validate_legacy(Optional[Integer[0, 65535]], 'validate_numeric', $stepout, 65535, 0) } - elsif $stepout == undef { } - else { deprecation('puppet_3_type_check', 'ntp::stepout is false, but should be undef') } - - if $step_tickers_file { - validate_legacy(Optional[Stdlib::Absolutepath], 'validate_string', $step_tickers_file) - validate_legacy(Optional[String], 'validate_string', $step_tickers_template) - } - elsif $step_tickers_file == undef { } - else { deprecation('puppet_3_type_check', 'ntp::step_tickers_file is false, but should be undef') } - - # In cases where $step_tickers_file evaluated to false, anything could have been - # passed to $step_tickers_template. This deprecation removes that hole. - assert_type(Optional[String], $step_tickers_template) |$expected, $actual| { - deprecation('puppet_3_type_check', "ntp::step_tickers_template should be '${expected}', not '${actual}'") - } - - validate_legacy(Boolean, 'validate_bool', $tos) - if $tos_minclock { validate_legacy(Optional[Integer[1]], 'validate_numeric', $tos_minclock) } - elsif $tos_minclock == undef { } - else { deprecation('puppet_3_type_check', 'ntp::tos_minclock is false, but should be undef') } - - if $tos_minsane { validate_legacy(Optional[Integer[1]], 'validate_numeric', $tos_minsane) } - elsif $tos_minsane == undef { } - else { deprecation('puppet_3_type_check', 'ntp::tos_minsane is false, but should be undef') } - - if $tos_floor { validate_legacy(Optional[Integer[1]], 'validate_numeric', $tos_floor) } - elsif $tos_floor == undef { } - else { deprecation('puppet_3_type_check', 'ntp::tos_floor is false, but should be undef') } - - if $tos_ceiling { validate_legacy(Optional[Integer[1]], 'validate_numeric', $tos_ceiling) } - elsif $tos_ceiling == undef { } - else { deprecation('puppet_3_type_check', 'ntp::tos_ceiling is false, but should be undef') } - - if $tos_cohort { validate_legacy(Variant[Boolean, Integer[0,1]], 'validate_re', $tos_cohort, '^[0|1]$', "Must be 0 or 1, got: ${tos_cohort}") } - elsif $tos_cohort == undef { } - elsif $tos_cohort == false { } - else { - # No idea what else could come through here, but better safe than sorry, and - # with the next major release, this will go away, anyways. - assert_type(Optional[Variant[Boolean, Integer[0,1]]], $tos_cohort) |$expected, $actual| { - deprecation('puppet_3_type_check', "ntp::tos_cohort should be '${expected}', not '${actual}'") - } - } - - validate_legacy(Boolean, 'validate_bool', $udlc) - assert_type(Optional[Integer[1,15]], $udlc_stratum) |$expected, $actual| { - deprecation('puppet_3_type_check', "ntp::udlc_stratum should be '${expected}', not '${actual}'") - } - - validate_legacy(Array[String], 'validate_array', $peers) - if $authprov { validate_legacy(Optional[String], 'validate_string', $authprov) } - elsif $authprov == undef { } - else { deprecation('puppet_3_type_check', 'ntp::authprov is false, but should be undef') } - - - if $config_dir { validate_legacy(Optional[Stdlib::Absolutepath], 'validate_absolute_path', $config_dir) } - elsif $config_dir == undef { } - else { deprecation('puppet_3_type_check', 'ntp::config_dir is false, but should be undef') } - - - if $autoupdate { - deprecation('ntp::autoupdate', 'ntp: autoupdate parameter has been deprecated and replaced with package_ensure. Set package_ensure to latest for the same behavior as autoupdate => true.') - } - # defaults for tinker and panic are different, when running on virtual machines if str2bool($facts['is_virtual']) { $_tinker = pick($tinker, true) @@ -189,11 +62,6 @@ $_panic = $panic } - validate_legacy(Boolean, 'validate_bool', $_tinker) - if $_panic { validate_legacy(Optional[Integer[0]], 'validate_numeric', $_panic, 65535, 0) } - elsif $_panic == undef { } - else { deprecation('puppet_3_type_check', 'ntp::panic is false, but should be undef') } - # Anchor this as per #8040 - this ensures that classes won't float off and # mess everything up. You can read about this at: # http://docs.puppetlabs.com/puppet/2.7/reference/lang_containment.html#known-issues @@ -202,5 +70,4 @@ class { '::ntp::config': } ~> class { '::ntp::service': } -> anchor { 'ntp::end': } - } diff --git a/metadata.json b/metadata.json index 787b3044..31a08a30 100644 --- a/metadata.json +++ b/metadata.json @@ -1,6 +1,6 @@ { "name": "puppetlabs-ntp", - "version": "5.0.0", + "version": "6.0.0", "author": "Puppet Labs", "summary": "Installs, configures, and manages the NTP service.", "license": "Apache-2.0", diff --git a/spec/acceptance/ntp_parameters_spec.rb b/spec/acceptance/ntp_parameters_spec.rb index 4592ad6f..5fd616b0 100644 --- a/spec/acceptance/ntp_parameters_spec.rb +++ b/spec/acceptance/ntp_parameters_spec.rb @@ -59,16 +59,6 @@ end end - describe 'autoupdate' do - it 'raises a deprecation warning' do - pp = "class { 'ntp': autoupdate => true }" - - apply_manifest(pp, :catch_failures => true) do |r| - expect(r.stderr).to match(/autoupdate parameter has been deprecated and replaced with package_ensure/) - end - end - end - describe 'config' do it 'sets the ntp.conf location' do pp = "class { 'ntp': config => '/etc/antp.conf' }" @@ -145,9 +135,9 @@ pp = <<-EOS class { 'ntp': keys_enable => true, - keys_controlkey => '15', - keys_requestkey => '1', - keys_trusted => [ '1', '2' ], + keys_controlkey => 15, + keys_requestkey => 1, + keys_trusted => [ 1, 2 ], keys => [ '1 M AAAABBBB' ], } EOS diff --git a/spec/classes/ntp_spec.rb b/spec/classes/ntp_spec.rb index 52891005..566b4757 100644 --- a/spec/classes/ntp_spec.rb +++ b/spec/classes/ntp_spec.rb @@ -44,9 +44,9 @@ context "when enabled" do let(:params) {{ :keys_enable => true, - :keys_trusted => ['1', '2', '3'], - :keys_controlkey => '2', - :keys_requestkey => '3', + :keys_trusted => [1, 2, 3], + :keys_controlkey => 2, + :keys_requestkey => 3, }} it { should contain_file('/etc/ntp.conf').with({ @@ -64,9 +64,9 @@ context "when disabled" do let(:params) {{ :keys_enable => false, - :keys_trusted => ['1', '2', '3'], - :keys_controlkey => '2', - :keys_requestkey => '3', + :keys_trusted => [1, 2, 3], + :keys_controlkey => 2, + :keys_requestkey => 3, }} it { should_not contain_file('/etc/ntp.conf').with({ @@ -574,12 +574,12 @@ describe 'with parameters minpoll or maxpoll changed from default' do context 'when minpoll changed from default' do let(:params) {{ - :minpoll => 3, + :minpoll => 6, }} it do should contain_file('/etc/ntp.conf').with({ - 'content' => /minpoll 3/, + 'content' => /minpoll 6/, }) end end @@ -598,13 +598,13 @@ context 'when minpoll and maxpoll changed from default simultaneously' do let(:params) {{ - :minpoll => 3, + :minpoll => 6, :maxpoll => 12, }} it do should contain_file('/etc/ntp.conf').with({ - 'content' => /minpoll 3 maxpoll 12\n/, + 'content' => /minpoll 6 maxpoll 12\n/, }) end end From 77fbf3f5e57b9727f00ca8efeca20023d09fa802 Mon Sep 17 00:00:00 2001 From: Helen Campbell Date: Fri, 14 Oct 2016 15:06:45 +0100 Subject: [PATCH 2/2] Replacing anchor functionality with contain --- manifests/init.pp | 13 ++++++++----- spec/classes/contains_spec.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 spec/classes/contains_spec.rb diff --git a/manifests/init.pp b/manifests/init.pp index 06adab63..1a82bbd3 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -65,9 +65,12 @@ # Anchor this as per #8040 - this ensures that classes won't float off and # mess everything up. You can read about this at: # http://docs.puppetlabs.com/puppet/2.7/reference/lang_containment.html#known-issues - anchor { 'ntp::begin': } -> - class { '::ntp::install': } -> - class { '::ntp::config': } ~> - class { '::ntp::service': } -> - anchor { 'ntp::end': } + + contain ntp::install + contain ntp::config + contain ntp::service + + Class['::ntp::install'] -> + Class['::ntp::config'] ~> + Class['::ntp::service'] } diff --git a/spec/classes/contains_spec.rb b/spec/classes/contains_spec.rb new file mode 100644 index 00000000..9e0a0e02 --- /dev/null +++ b/spec/classes/contains_spec.rb @@ -0,0 +1,28 @@ +# To check the correct dependancies are set up for NTP. + +require 'spec_helper' +describe 'ntp' do + let(:facts) {{ :is_virtual => 'false' }} + let :pre_condition do + 'file { "foo.rb": + ensure => present, + path => "/etc/tmp", + notify => Service["ntp"] }' end + on_supported_os.select { |_, f| f[:os]['family'] != 'Solaris' }.each do |os, f| + context "on #{os}" do + let(:facts) do + f.merge(super()) + end + + it { is_expected.to compile.with_all_deps } + describe "Testing the dependancies between the classes" do + it { should contain_class('ntp::install') } + it { should contain_class('ntp::config') } + it { should contain_class('ntp::service') } + it { is_expected.to contain_class('ntp::install').that_comes_before('Class[ntp::config]') } + it { is_expected.to contain_class('ntp::service').that_subscribes_to('Class[ntp::config]') } + it { is_expected.to contain_file('foo.rb').that_notifies('Service[ntp]') } + end + end + end +end