(FM-5361) Remove all the legacy/backwards-compat/deprecation logic

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.
puppetlabs · Sep 30, 2016 · eea5e2a · eea5e2a
1 parent f96aab7
commit eea5e2a
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 216 deletions.
diff --git 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
@@ -1,6 +1,5 @@
 ---
 ntp::authprov: ~
-ntp::autoupdate: false
 ntp::broadcastclient: false
 ntp::config_dir: ~
 ntp::config_file_mode: '0644'

diff --git a/manifests/init.pp b/manifests/init.pp
@@ -1,185 +1,58 @@
 class ntp (
-  Any $autoupdate,
-  Variant[Boolean, Stdlib::Compat::Bool] $broadcastclient,
-  Variant[Tea::Absolutepath, Stdlib::Compat::Absolute_path] $config,
-  Variant[Optional[Tea::Absolutepath], Any, Undef, Stdlib::Compat::Absolute_path] $config_dir,
-  Variant[String, Any] $config_file_mode,
+  Boolean $broadcastclient,
+  Tea::Absolutepath $config,
+  Optional[Tea::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[Tea::Absolutepath, Stdlib::Compat::Absolute_path] $driftfile,
-  Variant[Optional[Tea::Absolutepath], Boolean, Undef, Stdlib::Compat::Absolute_path] $leapfile,
-  Variant[Optional[Tea::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[Tea::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[Tea::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,
+  Tea::Absolutepath $driftfile,
+  Optional[Tea::Absolutepath] $leapfile,
+  Optional[Tea::Absolutepath] $logfile,
+  Boolean $iburst_enable,
+  Array[String] $keys,
+  Boolean $keys_enable,
+  Tea::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[Tea::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[Tea::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[Tea::Absolutepath] $ntpsigndsocket,
+  Optional[String] $authprov,
 ) {
-
-  validate_legacy(Boolean, 'validate_bool', $broadcastclient)
-  validate_legacy(Tea::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(Tea::Absolutepath, 'validate_absolute_path', $driftfile)
-
-  if $logfile { validate_legacy(Optional[Tea::Absolutepath], 'validate_absolute_path', $logfile) }
-  elsif $logfile == undef { }
-  else { deprecation('ntp::logfile is false, but should be undef') }
-
-  if $ntpsigndsocket { validate_legacy(Optional[Tea::Absolutepath], 'validate_absolute_path', $ntpsigndsocket) }
-  elsif $ntpsigndsocket == undef { }
-  else { deprecation('ntp::ntpsigndsocket is false, but should be undef') }
-
-  if $leapfile { validate_legacy(Optional[Tea::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[Tea::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[Tea::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
@@ -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
@@ -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
@@ -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