From df40baebedf5c0c15e08f3ec78adfd760b1371ca Mon Sep 17 00:00:00 2001 From: Helen Campbell Date: Tue, 4 Apr 2017 10:12:18 +0100 Subject: [PATCH] Removal of compat types and validate_legacy calls --- manifests/backports.pp | 16 ++---- manifests/conf.pp | 8 +-- manifests/init.pp | 89 +++++++++++++----------------- manifests/key.pp | 31 ++++------- manifests/pin.pp | 26 ++++----- manifests/ppa.pp | 10 ++-- manifests/setting.pp | 23 ++------ manifests/source.pp | 40 ++++++-------- spec/classes/apt_backports_spec.rb | 4 +- spec/defines/key_compat_spec.rb | 14 ++--- 10 files changed, 107 insertions(+), 154 deletions(-) diff --git a/manifests/backports.pp b/manifests/backports.pp index 06165b6..4faad61 100644 --- 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 364cae3..eea0686 100644 --- 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 40a40af..5654b3c 100644 --- 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 9a244b5..7d640a4 100644 --- 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 5b9dc1d..ee32de4 100644 --- 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 adce47c..a67e1a1 100644 --- 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 123bb2f..74ef1ea 100644 --- 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 8db3b28..bb498a3 100644 --- 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 2e8a9b4..c0d54a2 100644 --- 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 b298f8c..5b9e23f 100644 --- 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/) -- 2.32.3