Removal of compat types and validate_legacy calls

diff --git a/manifests/backports.pp b/manifests/backports.pp
index 06165b6c2210433522beb0804378861630bfd902..4faad6157a7f31664555471071c211e177e56ce2 100644 (file)
--- a/manifests/backports.pp
+++ b/manifests/backports.pp
@@ -1,26 +1,20 @@
 class apt::backports (
-  Optional[Variant[String, Stdlib::Compat::String]] $location = undef,
-  Optional[Variant[String, Stdlib::Compat::String]] $release = undef,
-  Optional[Variant[String, Stdlib::Compat::String]] $repos = undef,
-  Optional[Variant[String, Stdlib::Compat::String, Hash, Stdlib::Compat::Hash]] $key = undef,
-  Optional[Variant[Integer, Stdlib::Compat::Integer, String, Stdlib::Compat::String, Hash, Stdlib::Compat::Hash]] $pin  = 200,
+  Optional[String] $location                    = undef,
+  Optional[String] $release                     = undef,
+  Optional[String] $repos                       = undef,
+  Optional[Variant[String, Hash]] $key          = undef,
+  Optional[Variant[Integer, String, Hash]] $pin = 200,
 ){
   if $location {
-    validate_legacy(String, 'validate_string', $location)
     $_location = $location
   }
   if $release {
-    validate_legacy(String, 'validate_string', $release)
     $_release = $release
   }
   if $repos {
-    validate_legacy(String, 'validate_string', $repos)
     $_repos = $repos
   }
   if $key {
-    unless is_hash($key) {
-      validate_legacy(String, 'validate_string', $key)
-    }
     $_key = $key
   }
   if ($facts['lsbdistid'] == 'Debian' or $facts['lsbdistid'] == 'Ubuntu') {

diff --git a/manifests/conf.pp b/manifests/conf.pp
index 364cae383ce346a86d00524763a5a09c1caa35b0..eea06861d9e3793e807a9e77d5388d0897941d3c 100644 (file)
--- a/manifests/conf.pp
+++ b/manifests/conf.pp
@@ -1,8 +1,8 @@
 define apt::conf (
-  Optional[Variant[String, Stdlib::Compat::String]] $content                          = undef,
-  Enum['present', 'absent'] $ensure                                                   = present,
-  Variant[String, Stdlib::Compat::String, Integer, Stdlib::Compat::Integer] $priority = 50,
-  Optional[Boolean] $notify_update                                                    = undef,
+  Optional[String] $content          = undef,
+  Enum['present', 'absent'] $ensure  = present,
+  Variant[String, Integer] $priority = 50,
+  Optional[Boolean] $notify_update   = undef,
 ) {
 
   unless $ensure == 'absent' {

diff --git a/manifests/init.pp b/manifests/init.pp
index 40a40aff07650e457eabd3d56d1960f33176607a..5654b3c6261f058d54172fe224cc515c8d491f7f 100644 (file)
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -3,32 +3,32 @@
 # Manage APT (Advanced Packaging Tool)
 #
 class apt (
-  Variant[Hash, Stdlib::Compat::Hash] $update_defaults,
-  Variant[Hash, Stdlib::Compat::Hash] $purge_defaults,
-  Variant[Hash, Stdlib::Compat::Hash] $proxy_defaults,
-  Variant[Hash, Stdlib::Compat::Hash] $include_defaults,
-  Variant[String, Stdlib::Compat::String] $provider,
-  Variant[String, Stdlib::Compat::String] $keyserver,
-  Optional[Variant[String, Stdlib::Compat::String]] $ppa_options,
-  Optional[Variant[String, Stdlib::Compat::String]] $ppa_package,
-  Optional[Variant[Hash, Stdlib::Compat::Hash]] $backports,
-  Variant[Hash, Stdlib::Compat::Hash] $confs                = {},
-  Variant[Hash, Stdlib::Compat::Hash] $update               = {},
-  Variant[Hash, Stdlib::Compat::Hash] $purge                = {},
-  Variant[Hash, Stdlib::Compat::Hash] $proxy                = {},
-  Variant[Hash, Stdlib::Compat::Hash] $sources              = {},
-  Variant[Hash, Stdlib::Compat::Hash] $keys                 = {},
-  Variant[Hash, Stdlib::Compat::Hash] $ppas                 = {},
-  Variant[Hash, Stdlib::Compat::Hash] $pins                 = {},
-  Variant[Hash, Stdlib::Compat::Hash] $settings             = {},
-  Variant[String, Stdlib::Compat::String] $root             = '/etc/apt',
-  Variant[String, Stdlib::Compat::String] $sources_list     = "${root}/sources.list",
-  Variant[String, Stdlib::Compat::String] $sources_list_d   = "${root}/sources.list.d",
-  Variant[String, Stdlib::Compat::String] $conf_d           = "${root}/apt.conf.d",
-  Variant[String, Stdlib::Compat::String] $preferences      = "${root}/preferences",
-  Variant[String, Stdlib::Compat::String] $preferences_d    = "${root}/preferences.d",
-  Variant[Hash, Stdlib::Compat::Hash] $config_files         = { conf => { path => $conf_d, ext => '' }, pref => { path => $preferences_d, ext => '.pref' }, list => { path => $sources_list_d, ext => '.list' } },
-  Variant[Hash, Stdlib::Compat::Hash] $source_key_defaults  = { 'server' => $keyserver, 'options' => undef, 'content' => undef, 'source' => undef },
+  Hash $update_defaults,
+  Hash $purge_defaults,
+  Hash $proxy_defaults,
+  Hash $include_defaults,
+  String $provider,
+  String $keyserver,
+  Optional[String] $ppa_options,
+  Optional[String] $ppa_package,
+  Optional[Hash] $backports,
+  Hash $confs               = {},
+  Hash $update              = {},
+  Hash $purge               = {},
+  Hash $proxy               = {},
+  Hash $sources             = {},
+  Hash $keys                = {},
+  Hash $ppas                = {},
+  Hash $pins                = {},
+  Hash $settings            = {},
+  String $root              = '/etc/apt',
+  String $sources_list      = "${root}/sources.list",
+  String $sources_list_d    = "${root}/sources.list.d",
+  String $conf_d            = "${root}/apt.conf.d",
+  String $preferences       = "${root}/preferences",
+  String $preferences_d     = "${root}/preferences.d",
+  Hash $config_files        = { conf => { path => $conf_d, ext => '' }, pref => { path => $preferences_d, ext => '.pref' }, list => { path => $sources_list_d, ext => '.list' } },
+  Hash $source_key_defaults = { 'server' => $keyserver, 'options' => undef, 'content' => undef, 'source' => undef },
 ) {
 
   if $facts['osfamily'] != 'Debian' {
@@ -36,65 +36,50 @@ class apt (
   }
 
   $frequency_options = ['always','daily','weekly','reluctantly']
-  validate_legacy(Hash, 'validate_hash', $update)
+
   if $update['frequency'] {
     validate_re($update['frequency'], $frequency_options)
   }
   if $update['timeout'] {
-    unless is_integer($update['timeout']) {
-      fail('timeout value for update must be an integer')
-    }
+    assert_type(Integer, $update['timeout'])
   }
   if $update['tries'] {
-    unless is_integer($update['tries']) {
-      fail('tries value for update must be an integer')
-    }
+    assert_type(Integer, $update['tries'])
   }
 
   $_update = merge($::apt::update_defaults, $update)
   include ::apt::update
 
-  validate_legacy(Hash, 'validate_hash', $purge)
   if $purge['sources.list'] {
-    validate_legacy(Boolean, 'validate_bool', $purge['sources.list'])
+    assert_type(Boolean, $purge['sources.list'])
   }
   if $purge['sources.list.d'] {
-    validate_legacy(Boolean, 'validate_bool', $purge['sources.list.d'])
+    assert_type(Boolean, $purge['sources.list.d'])
   }
   if $purge['preferences'] {
-    validate_legacy(Boolean, 'validate_bool', $purge['preferences'])
+    assert_type(Boolean, $purge['preferences'])
   }
   if $purge['preferences.d'] {
-    validate_legacy(Boolean, 'validate_bool', $purge['preferences.d'])
+    assert_type(Boolean, $purge['preferences.d'])
   }
 
   $_purge = merge($::apt::purge_defaults, $purge)
 
-  validate_hash($proxy)
   if $proxy['ensure'] {
     validate_re($proxy['ensure'], ['file', 'present', 'absent'])
   }
   if $proxy['host'] {
-    validate_legacy(String, 'validate_string', $proxy['host'])
+    assert_type(String, $proxy['host'])
   }
   if $proxy['port'] {
-    unless is_integer($proxy['port']) {
-      fail('$proxy port must be an integer')
-    }
+    assert_type(Integer, $proxy['port'])
   }
-  if $proxy['https'] {
-    validate_legacy(Boolean, 'validate_bool', $proxy['https'])
+  if $proxy['https']{
+    assert_type(Boolean, $proxy['https'])
   }
 
   $_proxy = merge($apt::proxy_defaults, $proxy)
 
-  validate_legacy(Hash, 'validate_hash', $confs)
-  validate_legacy(Hash, 'validate_hash', $sources)
-  validate_legacy(Hash, 'validate_hash', $keys)
-  validate_legacy(Hash, 'validate_hash', $settings)
-  validate_legacy(Hash, 'validate_hash', $ppas)
-  validate_legacy(Hash, 'validate_hash', $pins)
-
   $confheadertmp = epp('apt/_conf_header.epp')
   $proxytmp = epp('apt/proxy.epp', {'proxies' => $_proxy})
   $updatestamptmp = epp('apt/15update-stamp.epp')

diff --git a/manifests/key.pp b/manifests/key.pp
index 9a244b5065ac049c58722fb8e56e8fb6258f1669..7d640a4537815602242df2cb2ba7cafa2aae9783 100644 (file)
--- a/manifests/key.pp
+++ b/manifests/key.pp
@@ -1,16 +1,16 @@
 # == Define: apt::key
 define apt::key (
-    Variant[String, Stdlib::Compat::String] $id                                         = $title,
-    Enum['present', 'absent'] $ensure                                                   = present,
-    Optional[Variant[String, Stdlib::Compat::String]] $content                          = undef,
-    Optional[Variant[String, Stdlib::Compat::String]] $source                           = undef,
-    Variant[String, Stdlib::Compat::String] $server                                     = $::apt::keyserver,
-    Optional[Variant[String, Stdlib::Compat::String]] $options                          = undef,
-    Optional[Variant[String, Stdlib::Compat::String, Hash, Stdlib::Compat::Hash]] $key  = undef,
-    Optional[Variant[String, Stdlib::Compat::String]] $key_content                      = undef,
-    Optional[Variant[String, Stdlib::Compat::String]] $key_source                       = undef,
-    Optional[Variant[String, Stdlib::Compat::String]] $key_server                       = undef,
-    Optional[Variant[String, Stdlib::Compat::String]] $key_options                      = undef,
+    String $id                           = $title,
+    Enum['present', 'absent'] $ensure    = present,
+    Optional[String] $content            = undef,
+    Optional[String] $source             = undef,
+    String $server                       = $::apt::keyserver,
+    Optional[String] $options            = undef,
+    Optional[Variant[String, Hash]] $key = undef,
+    Optional[String] $key_content        = undef,
+    Optional[String] $key_source         = undef,
+    Optional[String] $key_server         = undef,
+    Optional[String] $key_options        = undef,
     ) {
 
   if $key != undef {
@@ -49,11 +49,6 @@ define apt::key (
   }
 
   validate_re($_id, ['\A(0x)?[0-9a-fA-F]{8}\Z', '\A(0x)?[0-9a-fA-F]{16}\Z', '\A(0x)?[0-9a-fA-F]{40}\Z'])
-  validate_re($ensure, ['\A(absent|present)\Z',])
-
-  if $_content {
-    validate_legacy(String, 'validate_string', $_content)
-  }
 
   if $_source {
     validate_re($_source, ['\Ahttps?:\/\/', '\Aftp:\/\/', '\A\/\w+'])
@@ -63,10 +58,6 @@ define apt::key (
     validate_re($_server,['\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$'])
   }
 
-  if $_options {
-    validate_legacy(String, 'validate_string', $_options)
-  }
-
   case $ensure {
     present: {
       if defined(Anchor["apt_key ${_id} absent"]){

diff --git a/manifests/pin.pp b/manifests/pin.pp
index 5b9dc1d408e2daf8068c70a046c5e9d0074f5f9e..ee32de4694a5a50c7dda1cd48f89c8c1de214da7 100644 (file)
--- a/manifests/pin.pp
+++ b/manifests/pin.pp
@@ -2,19 +2,19 @@
 # pin a release in apt, useful for unstable repositories
 
 define apt::pin(
-  Optional[Enum['file', 'present', 'absent']] $ensure                             = present,
-  Optional[Variant[String, Stdlib::Compat::String]] $explanation                  = undef,
-  Variant[Integer,  Stdlib::Compat::Integer] $order                               = 50,
-  Variant[String, Stdlib::Compat::String, Stdlib::Compat::Array, Array] $packages = '*',
-  Variant[Numeric, String, Stdlib::Compat::String] $priority                      = 0,
-  Optional[Variant[String, Stdlib::Compat::String]] $release                      = '', # a=
-  Optional[Variant[String, Stdlib::Compat::String]] $origin                       = '',
-  Optional[Variant[String, Stdlib::Compat::String]] $version                      = '',
-  Optional[Variant[String, Stdlib::Compat::String]] $codename                     = '', # n=
-  Optional[Variant[String, Stdlib::Compat::String]] $release_version              = '', # v=
-  Optional[Variant[String, Stdlib::Compat::String]] $component                    = '', # c=
-  Optional[Variant[String, Stdlib::Compat::String]] $originator                   = '', # o=
-  Optional[Variant[String, Stdlib::Compat::String]] $label                        = '',  # l=
+  Optional[Enum['file', 'present', 'absent']] $ensure = present,
+  Optional[String] $explanation                       = undef,
+  Variant[Integer] $order                             = 50,
+  Variant[String, Array] $packages                    = '*',
+  Variant[Numeric, String] $priority                  = 0,
+  Optional[String] $release                           = '', # a=
+  Optional[String] $origin                            = '',
+  Optional[String] $version                           = '',
+  Optional[String] $codename                          = '', # n=
+  Optional[String] $release_version                   = '', # v=
+  Optional[String] $component                         = '', # c=
+  Optional[String] $originator                        = '', # o=
+  Optional[String] $label                             = '',  # l=
 ) {
 
   if $explanation {

diff --git a/manifests/ppa.pp b/manifests/ppa.pp
index adce47cae0645fdc4e7ef87ed88740a96bae8c27..a67e1a19457e204ce7e5cfb1237115b0081bbc0d 100644 (file)
--- a/manifests/ppa.pp
+++ b/manifests/ppa.pp
@@ -1,10 +1,10 @@
 # ppa.pp
 define apt::ppa(
-  Variant[String, Stdlib::Compat::String] $ensure                 = 'present',
-  Optional[Variant[String, Stdlib::Compat::String]] $options      = $::apt::ppa_options,
-  Optional[Variant[String, Stdlib::Compat::String]] $release      = $facts['lsbdistcodename'],
-  Optional[Variant[String, Stdlib::Compat::String]] $package_name = $::apt::ppa_package,
-  Boolean $package_manage                                         = false,
+  String $ensure                 = 'present',
+  Optional[String] $options      = $::apt::ppa_options,
+  Optional[String] $release      = $facts['lsbdistcodename'],
+  Optional[String] $package_name = $::apt::ppa_package,
+  Boolean $package_manage        = false,
 ) {
   unless $release {
     fail('lsbdistcodename fact not available: release parameter required')

diff --git a/manifests/setting.pp b/manifests/setting.pp
index 123bb2f70e29e9c2c559c1c3fd455e252b39863b..74ef1eabbc0523494156f3390e159d59ff195539 100644 (file)
--- a/manifests/setting.pp
+++ b/manifests/setting.pp
@@ -1,10 +1,9 @@
 define apt::setting (
-  Variant[String, Stdlib::Compat::String, Integer, Stdlib::Compat::Integer, Array, Stdlib::Compat::Array] $priority = 50,
-  Optional[Enum['file', 'present', 'absent']] $ensure                                                               = file,
-  Optional[Variant[String, Stdlib::Compat::String]] $source                                                         = undef,
-  Optional[Variant[String, Stdlib::Compat::String]] $content                                                        = undef,
-  Optional[Boolean] $notify_update                                                                                  = true,
-
+  Variant[String, Integer] $priority                  = 50,
+  Optional[Enum['file', 'present', 'absent']] $ensure = file,
+  Optional[String] $source                            = undef,
+  Optional[String] $content                           = undef,
+  Boolean $notify_update                              = true,
 ) {
 
   if $content and $source {
@@ -15,10 +14,6 @@ define apt::setting (
     fail('apt::setting needs either of content or source')
   }
 
-  if $notify_update {
-    validate_legacy(Boolean, 'validate_bool', $notify_update)
-  }
-
   $title_array = split($title, '-')
   $setting_type = $title_array[0]
   $base_name = join(delete_at($title_array, 0), '-')
@@ -30,14 +25,6 @@ define apt::setting (
     validate_re($priority, '^\d+$', 'apt::setting priority must be an integer or a zero-padded integer')
   }
 
-  if $source {
-    validate_legacy(String, 'validate_string', $source)
-  }
-
-  if $content {
-    validate_legacy(String, 'validate_string', $content)
-  }
-
   if ($setting_type == 'list') or ($setting_type == 'pref') {
     $_priority = ''
   } else {

diff --git a/manifests/source.pp b/manifests/source.pp
index 8db3b28ef12457c501f498017372fe4cd6720458..bb498a3546a6284fc95f7c9ab85b1dcf5290a74e 100644 (file)
--- a/manifests/source.pp
+++ b/manifests/source.pp
@@ -1,30 +1,26 @@
 # source.pp
 # add an apt source
 define apt::source(
-  Optional[Variant[String, Stdlib::Compat::String]] $location                         = undef,
-  Variant[String, Stdlib::Compat::String] $comment                                    = $name,
-  Variant[String, Stdlib::Compat::String] $ensure                                     = present,
-  Optional[Variant[String, Stdlib::Compat::String]] $release                          = undef,
-  Variant[String, Stdlib::Compat::String] $repos                                      = 'main',
-  Optional[Variant[Hash, Stdlib::Compat::Hash]] $include                              = {},
-  Optional[Variant[String, Stdlib::Compat::String, Hash, Stdlib::Compat::Hash]] $key  = undef,
-  $pin                                                                                = undef,
-  Optional[Variant[String, Stdlib::Compat::String]] $architecture                     = undef,
-  Boolean $allow_unsigned                                                             = false,
-  Boolean $notify_update                                                              = true,
-  Optional[Variant[String, Stdlib::Compat::String]] $key_server                       = undef,
-  Optional[Variant[String, Stdlib::Compat::String]] $key_content                      = undef,
-  Optional[Variant[String, Stdlib::Compat::String]] $key_source                       = undef,
-  Optional[Boolean] $include_src                                                      = undef,
-  Optional[Boolean] $include_deb                                                      = undef,
-  $required_packages                                                                  = undef,
-  $trusted_source                                                                     = undef,
+  Optional[String] $location           = undef,
+  String $comment                      = $name,
+  String $ensure                       = present,
+  Optional[String] $release            = undef,
+  String $repos                        = 'main',
+  Optional[Variant[Hash]] $include     = {},
+  Optional[Variant[String, Hash]] $key = undef,
+  $pin                                 = undef,
+  Optional[String] $architecture       = undef,
+  Boolean $allow_unsigned              = false,
+  Boolean $notify_update               = true,
+  Optional[String] $key_server         = undef,
+  Optional[String] $key_content        = undef,
+  Optional[String] $key_source         = undef,
+  Optional[Boolean] $include_src       = undef,
+  Optional[Boolean] $include_deb       = undef,
+  $required_packages                   = undef,
+  $trusted_source                      = undef,
 ) {
 
-  validate_legacy(String, 'validate_string', $architecture, $comment, $location, $repos)
-  validate_legacy(Boolean, 'validate_bool', $allow_unsigned)
-  validate_legacy(Hash, 'validate_hash', $include)
-
   # This is needed for compat with 1.8.x
   include ::apt
 

diff --git a/spec/classes/apt_backports_spec.rb b/spec/classes/apt_backports_spec.rb
index 2e8a9b4b09e65c90f45652d0db50e282342d5924..c0d54a21c990e5e2f18d4e7326ad45254b6a472a 100644 (file)
--- a/spec/classes/apt_backports_spec.rb
+++ b/spec/classes/apt_backports_spec.rb
@@ -260,7 +260,7 @@ describe 'apt::backports', :type => :class do
       it do
         expect {
           subject.call
-        }.to raise_error(Puppet::Error, /expects a value of type String, Hash,/)
+        }.to raise_error(Puppet::Error, /expects a value of type String or Hash, got Boolean/)
       end
     end
     context 'invalid pin' do
@@ -272,7 +272,7 @@ describe 'apt::backports', :type => :class do
       it do
         expect {
           subject.call
-        }.to raise_error(Puppet::Error, /parameter 'pin' expects a value of type Integer, Pattern/)
+        }.to raise_error(Puppet::Error, /expects a value of type Integer, String, or Hash, got Boolean/)
       end
     end
   end

diff --git a/spec/defines/key_compat_spec.rb b/spec/defines/key_compat_spec.rb
index b298f8cfbcb96362bad05213871fdf8f92aaa88d..5b9e23f2a198e08f1641160421f8a427ede42e32 100644 (file)
--- a/spec/defines/key_compat_spec.rb
+++ b/spec/defines/key_compat_spec.rb
@@ -40,7 +40,7 @@ describe 'apt::key', :type => :define do
       end
 
       let :params do {
-        :key => GPG_KEY_ID,
+        :id => GPG_KEY_ID,
       } end
 
       it 'contains the apt_key' do
@@ -80,7 +80,7 @@ describe 'apt::key', :type => :define do
 
     describe 'set a bunch of things!' do
       let :params do {
-        :key_content => 'GPG key content',
+        :content => 'GPG key content',
         :key_source => 'http://apt.puppetlabs.com/pubkey.gpg',
         :key_server => 'pgp.mit.edu',
         :key_options => 'debug',
@@ -92,7 +92,7 @@ describe 'apt::key', :type => :define do
           :ensure  => 'present',
           :source  => 'http://apt.puppetlabs.com/pubkey.gpg',
           :server  => 'pgp.mit.edu',
-          :content => params[:key_content],
+          :content => params[:content],
           :options => 'debug',
         })
       end
@@ -246,7 +246,7 @@ describe 'apt::key', :type => :define do
 
     context 'invalid content' do
       let :params do {
-        :key_content => [],
+        :content => [],
       } end
       it 'fails' do
         expect { subject.call }.to raise_error(/expects a String value/)
@@ -285,12 +285,12 @@ describe 'apt::key', :type => :define do
     describe 'duplication' do
       context 'two apt::key resources for same key, different titles' do
         let :pre_condition do
-          "#{super()}\napt::key { 'duplicate': key => '#{title}', }"
+          "#{super()}\napt::key { 'duplicate': id => '#{title}', }"
         end
 
         it 'contains the duplicate apt::key resource' do
           is_expected.to contain_apt__key('duplicate').with({
-            :key    => title,
+            :id    => title,
             :ensure => 'present',
           })
         end
@@ -320,7 +320,7 @@ describe 'apt::key', :type => :define do
 
       context 'two apt::key resources, different ensure' do
         let :pre_condition do
-          "#{super()}\napt::key { 'duplicate': key => '#{title}', ensure => 'absent', }"
+          "#{super()}\napt::key { 'duplicate': id => '#{title}', ensure => 'absent', }"
         end
         it 'informs the user of the impossibility' do
           expect { subject.call }.to raise_error(/already ensured as absent/)