From aba776640b70ab8123d1d614dcc2892d5c42a408 Mon Sep 17 00:00:00 2001 From: Helen Campbell Date: Tue, 4 Apr 2017 11:57:35 +0100 Subject: [PATCH] Removal of deprecated functionality --- manifests/key.pp | 86 ++++++++---------------------- manifests/setting.pp | 2 +- manifests/source.pp | 70 ++++-------------------- spec/acceptance/apt_spec.rb | 4 +- spec/defines/key_compat_spec.rb | 39 +++++++------- spec/defines/pin_spec.rb | 2 +- spec/defines/source_compat_spec.rb | 29 +++------- spec/defines/source_spec.rb | 39 +++----------- 8 files changed, 68 insertions(+), 203 deletions(-) diff --git a/manifests/key.pp b/manifests/key.pp index 7d640a4..dfa1daf 100644 --- a/manifests/key.pp +++ b/manifests/key.pp @@ -6,90 +6,50 @@ define apt::key ( 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 { - deprecation('apt $key', '$key is deprecated and will be removed in the next major release. Please use $id instead.') - $_id = $key - } else { - $_id = $id - } - - if $key_content != undef { - deprecation('apt $key_content', '$key_content is deprecated and will be removed in the next major release. Please use $content instead.') - $_content = $key_content - } else { - $_content = $content - } - - if $key_source != undef { - deprecation('apt $key_source', '$key_source is deprecated and will be removed in the next major release. Please use $source instead.') - $_source = $key_source - } else { - $_source = $source - } - - if $key_server != undef { - deprecation('apt $key_server', '$key_server is deprecated and will be removed in the next major release. Please use $server instead.') - $_server = $key_server - } else { - $_server = $server - } - - if $key_options != undef { - deprecation('apt $key_options', '$key_options is deprecated and will be removed in the next major release. Please use $options instead.') - $_options = $key_options - } else { - $_options = $options - } - - 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($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']) - if $_source { - validate_re($_source, ['\Ahttps?:\/\/', '\Aftp:\/\/', '\A\/\w+']) + if $source { + validate_re($source, ['\Ahttps?:\/\/', '\Aftp:\/\/', '\A\/\w+']) } - if $_server { - validate_re($_server,['\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$']) + if $server { + validate_re($server,['\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$']) } case $ensure { present: { - if defined(Anchor["apt_key ${_id} absent"]){ - fail("key with id ${_id} already ensured as absent") + if defined(Anchor["apt_key ${id} absent"]){ + fail("key with id ${id} already ensured as absent") } - if !defined(Anchor["apt_key ${_id} present"]) { + if !defined(Anchor["apt_key ${id} present"]) { apt_key { $title: ensure => $ensure, - id => $_id, - source => $_source, - content => $_content, - server => $_server, - options => $_options, - } -> anchor { "apt_key ${_id} present": } + id => $id, + source => $source, + content => $content, + server => $server, + options => $options, + } -> anchor { "apt_key ${id} present": } } } absent: { - if defined(Anchor["apt_key ${_id} present"]){ - fail("key with id ${_id} already ensured as present") + if defined(Anchor["apt_key ${id} present"]){ + fail("key with id ${id} already ensured as present") } - if !defined(Anchor["apt_key ${_id} absent"]){ + if !defined(Anchor["apt_key ${id} absent"]){ apt_key { $title: ensure => $ensure, - id => $_id, - source => $_source, - content => $_content, - server => $_server, - options => $_options, - } -> anchor { "apt_key ${_id} absent": } + id => $id, + source => $source, + content => $content, + server => $server, + options => $options, + } -> anchor { "apt_key ${id} absent": } } } diff --git a/manifests/setting.pp b/manifests/setting.pp index 74ef1ea..507ec94 100644 --- a/manifests/setting.pp +++ b/manifests/setting.pp @@ -1,5 +1,5 @@ define apt::setting ( - Variant[String, Integer] $priority = 50, + Variant[String, Integer, Array] $priority = 50, Optional[Enum['file', 'present', 'absent']] $ensure = file, Optional[String] $source = undef, Optional[String] $content = undef, diff --git a/manifests/source.pp b/manifests/source.pp index bb498a3..ad80977 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -12,13 +12,6 @@ define apt::source( 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, ) { # This is needed for compat with 1.8.x @@ -26,25 +19,6 @@ define apt::source( $_before = Apt::Setting["list-${title}"] - if $required_packages != undef { - deprecation('apt $required_packages', '$required_packages is deprecated and will be removed in the next major release, please use package resources instead.') - exec { "Required packages: '${required_packages}' for ${name}": - command => "${::apt::provider} -y install ${required_packages}", - logoutput => 'on_failure', - refreshonly => true, - tries => 3, - try_sleep => 1, - before => $_before, - } - } - - if $trusted_source != undef { - deprecation('apt $trusted_source', '$trusted_source is deprecated and will be removed in the next major release, please use $allow_unsigned instead.') - $_allow_unsigned = $trusted_source - } else { - $_allow_unsigned = $allow_unsigned - } - if ! $release { if $facts['lsbdistcodename'] { $_release = $facts['lsbdistcodename'] @@ -59,36 +33,17 @@ define apt::source( fail('cannot create a source entry without specifying a location') } - if $include_src != undef and $include_deb != undef { - $_deprecated_include = { - 'src' => $include_src, - 'deb' => $include_deb, - } - } elsif $include_src != undef { - $_deprecated_include = { 'src' => $include_src } - } elsif $include_deb != undef { - $_deprecated_include = { 'deb' => $include_deb } - } else { - $_deprecated_include = {} - } - - $includes = merge($::apt::include_defaults, $_deprecated_include, $include) - - $_deprecated_key = { - 'key_server' => $key_server, - 'key_content' => $key_content, - 'key_source' => $key_source, - } + $includes = merge($::apt::include_defaults, $include) if $key { if is_hash($key) { unless $key['id'] { fail('key hash must contain at least an id entry') } - $_key = merge($::apt::source_key_defaults, $_deprecated_key, $key) + $_key = merge($::apt::source_key_defaults, $key) } else { validate_legacy(String, 'validate_string', $key) - $_key = merge( { 'id' => $key }, $_deprecated_key) + $_key = { 'id' => $key } } } @@ -98,7 +53,7 @@ define apt::source( 'comment' => $comment, 'includes' => $includes, 'architecture' => $architecture, - 'allow_unsigned' => $_allow_unsigned, + 'allow_unsigned' => $allow_unsigned, 'location' => $location, 'release' => $_release, 'repos' => $repos, @@ -132,16 +87,13 @@ define apt::source( if $key and ($ensure == 'present') { if is_hash($_key) { apt::key { "Add key: ${$_key['id']} from Apt::Source ${title}": - ensure => present, - id => $_key['id'], - server => $_key['server'], - content => $_key['content'], - source => $_key['source'], - options => $_key['options'], - key_server => $_key['key_server'], - key_content => $_key['key_content'], - key_source => $_key['key_source'], - before => $_before, + ensure => present, + id => $_key['id'], + server => $_key['server'], + content => $_key['content'], + source => $_key['source'], + options => $_key['options'], + before => $_before, } } } diff --git a/spec/acceptance/apt_spec.rb b/spec/acceptance/apt_spec.rb index 52415f5..df312a7 100644 --- a/spec/acceptance/apt_spec.rb +++ b/spec/acceptance/apt_spec.rb @@ -33,8 +33,8 @@ describe 'apt class' do class { 'apt': update => { 'frequency' => 'always', - 'timeout' => '400', - 'tries' => '3', + 'timeout' => 400, + 'tries' => 3, }, purge => { 'sources.list' => true, diff --git a/spec/defines/key_compat_spec.rb b/spec/defines/key_compat_spec.rb index 5b9e23f..f6013d3 100644 --- a/spec/defines/key_compat_spec.rb +++ b/spec/defines/key_compat_spec.rb @@ -26,7 +26,6 @@ describe 'apt::key', :type => :define do :source => nil, :server => 'keyserver.ubuntu.com', :content => nil, - :keyserver_options => nil, }) } it 'contains the apt_key present anchor' do @@ -50,7 +49,6 @@ describe 'apt::key', :type => :define do :source => nil, :server => 'keyserver.ubuntu.com', :content => nil, - :keyserver_options => nil, }) end it 'contains the apt_key present anchor' do @@ -70,7 +68,6 @@ describe 'apt::key', :type => :define do :source => nil, :server => 'keyserver.ubuntu.com', :content => nil, - :keyserver_options => nil, }) end it 'contains the apt_key absent anchor' do @@ -81,9 +78,9 @@ describe 'apt::key', :type => :define do describe 'set a bunch of things!' do let :params do { :content => 'GPG key content', - :key_source => 'http://apt.puppetlabs.com/pubkey.gpg', - :key_server => 'pgp.mit.edu', - :key_options => 'debug', + :source => 'http://apt.puppetlabs.com/pubkey.gpg', + :server => 'pgp.mit.edu', + :options => 'debug', } end it 'contains the apt_key' do @@ -103,7 +100,7 @@ describe 'apt::key', :type => :define do context "domain with dash" do let(:params) do{ - :key_server => 'p-gp.m-it.edu', + :server => 'p-gp.m-it.edu', } end it 'contains the apt_key' do is_expected.to contain_apt_key(title).with({ @@ -116,7 +113,7 @@ describe 'apt::key', :type => :define do context "url" do let :params do { - :key_server => 'hkp://pgp.mit.edu', + :server => 'hkp://pgp.mit.edu', } end it 'contains the apt_key' do @@ -129,7 +126,7 @@ describe 'apt::key', :type => :define do context "url with port number" do let :params do { - :key_server => 'hkp://pgp.mit.edu:80', + :server => 'hkp://pgp.mit.edu:80', } end it 'contains the apt_key' do @@ -144,7 +141,7 @@ describe 'apt::key', :type => :define do describe 'validation' do context "domain begin with dash" do let(:params) do{ - :key_server => '-pgp.mit.edu', + :server => '-pgp.mit.edu', } end it 'fails' do expect { subject.call } .to raise_error(/does not match/) @@ -153,7 +150,7 @@ describe 'apt::key', :type => :define do context "domain begin with dot" do let(:params) do{ - :key_server => '.pgp.mit.edu', + :server => '.pgp.mit.edu', } end it 'fails' do expect { subject.call } .to raise_error(/does not match/) @@ -162,7 +159,7 @@ describe 'apt::key', :type => :define do context "domain end with dot" do let(:params) do{ - :key_server => "pgp.mit.edu.", + :server => "pgp.mit.edu.", } end it 'fails' do expect { subject.call } .to raise_error(/does not match/) @@ -171,7 +168,7 @@ describe 'apt::key', :type => :define do context "exceed character url" do let :params do { - :key_server => 'hkp://pgpiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii.mit.edu' + :server => 'hkp://pgpiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii.mit.edu' } end it 'fails' do @@ -181,7 +178,7 @@ describe 'apt::key', :type => :define do context "incorrect port number url" do let :params do { - :key_server => 'hkp://pgp.mit.edu:8008080' + :server => 'hkp://pgp.mit.edu:8008080' } end it 'fails' do @@ -191,7 +188,7 @@ describe 'apt::key', :type => :define do context "incorrect protocol for url" do let :params do { - :key_server => 'abc://pgp.mit.edu:80' + :server => 'abc://pgp.mit.edu:80' } end it 'fails' do @@ -201,7 +198,7 @@ describe 'apt::key', :type => :define do context "missing port number url" do let :params do { - :key_server => 'hkp://pgp.mit.edu:' + :server => 'hkp://pgp.mit.edu:' } end it 'fails' do @@ -211,7 +208,7 @@ describe 'apt::key', :type => :define do context "url ending with a dot" do let :params do { - :key_server => 'hkp://pgp.mit.edu.' + :server => 'hkp://pgp.mit.edu.' } end it 'fails' do @@ -220,7 +217,7 @@ describe 'apt::key', :type => :define do end context "url begin with a dash" do let(:params) do{ - :key_server => "hkp://-pgp.mit.edu", + :server => "hkp://-pgp.mit.edu", } end it 'fails' do expect { subject.call }.to raise_error(/does not match/) @@ -237,7 +234,7 @@ describe 'apt::key', :type => :define do context 'invalid source' do let :params do { - :key_source => 'afp://puppetlabs.com/key.gpg', + :source => 'afp://puppetlabs.com/key.gpg', } end it 'fails' do expect { subject.call }.to raise_error(/does not match/) @@ -255,7 +252,7 @@ describe 'apt::key', :type => :define do context 'invalid server' do let :params do { - :key_server => 'two bottles of rum', + :server => 'two bottles of rum', } end it 'fails' do expect { subject.call }.to raise_error(/does not match/) @@ -264,7 +261,7 @@ describe 'apt::key', :type => :define do context 'invalid keyserver_options' do let :params do { - :key_options => {}, + :options => {}, } end it 'fails' do expect { subject.call }.to raise_error(/expects a String value/) diff --git a/spec/defines/pin_spec.rb b/spec/defines/pin_spec.rb index 4f51cf3..50890fc 100644 --- a/spec/defines/pin_spec.rb +++ b/spec/defines/pin_spec.rb @@ -84,7 +84,7 @@ describe 'apt::pin', :type => :define do it do expect { subject.call - }.to raise_error(Puppet::Error, /expects a value of type Integer/) + }.to raise_error(Puppet::Error, /expects an Integer value, got String/) end end diff --git a/spec/defines/source_compat_spec.rb b/spec/defines/source_compat_spec.rb index a384907..ad710a7 100644 --- a/spec/defines/source_compat_spec.rb +++ b/spec/defines/source_compat_spec.rb @@ -20,8 +20,7 @@ describe 'apt::source', :type => :define do let :params do { - 'include_deb' => false, - 'include_src' => true, + 'include' => { 'deb' => false, 'src' => true }, 'location' => 'http://debian.mirror.iweb.ca/debian/', } end @@ -46,15 +45,11 @@ describe 'apt::source', :type => :define do 'location' => 'http://debian.mirror.iweb.ca/debian/', 'release' => 'sid', 'repos' => 'testing', - 'include_src' => false, - 'required_packages' => 'vim', + 'include' => { 'src' => false }, 'key' => GPG_KEY_ID, - 'key_server' => 'pgp.mit.edu', - 'key_content' => 'GPG key content', - 'key_source' => 'http://apt.puppetlabs.com/pubkey.gpg', 'pin' => '10', 'architecture' => 'x86_64', - 'trusted_source' => true, + 'allow_unsigned' => true, } end @@ -68,26 +63,14 @@ describe 'apt::source', :type => :define do }) } - it { is_expected.to contain_exec("Required packages: 'vim' for my_source").that_comes_before('Apt::Setting[list-my_source]').with({ - 'command' => '/usr/bin/apt-get -y install vim', - 'logoutput' => 'on_failure', - 'refreshonly' => true, - 'tries' => '3', - 'try_sleep' => '1', - }) - } - it { is_expected.to contain_apt__key("Add key: #{GPG_KEY_ID} from Apt::Source my_source").that_comes_before('Apt::Setting[list-my_source]').with({ 'ensure' => 'present', 'id' => GPG_KEY_ID, - 'key_server' => 'pgp.mit.edu', - 'key_content' => 'GPG key content', - 'key_source' => 'http://apt.puppetlabs.com/pubkey.gpg', }) } end - context 'trusted_source true' do + context 'allow_unsigned true' do let :facts do { :os => { :family => 'Debian', :name => 'Debian', :release => { :major => '7', :full => '7.0' }}, @@ -99,9 +82,9 @@ describe 'apt::source', :type => :define do end let :params do { - 'include_src' => false, + 'include' => {'src' => false}, 'location' => 'http://debian.mirror.iweb.ca/debian/', - 'trusted_source' => true, + 'allow_unsigned' => true, } end diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb index 0972512..cb98035 100644 --- a/spec/defines/source_spec.rb +++ b/spec/defines/source_spec.rb @@ -226,7 +226,7 @@ describe 'apt::source' do let :params do { :location => 'hello.there', - :include => {'deb' => false, 'src' => true,}, + :include => {'deb' => false, 'src' => true}, :architecture => 'x86_64', } end @@ -250,7 +250,7 @@ describe 'apt::source' do let :params do { :location => 'hello.there', - :include_src => true, + :include => {'src' => true}, } end @@ -260,7 +260,7 @@ describe 'apt::source' do } end - context 'include_deb => false' do + context 'include deb => false' do let :facts do { :os => { :family => 'Debian', :name => 'Debian', :release => { :major => '7', :full => '7.0' }}, @@ -272,8 +272,8 @@ describe 'apt::source' do end let :params do { + :include => { 'deb' => false }, :location => 'hello.there', - :include_deb => false, } end @@ -284,7 +284,7 @@ describe 'apt::source' do it { is_expected.to contain_apt__setting('list-my_source').without_content(/deb hello.there wheezy main\n/) } end - context 'include_src => true and include_deb => false' do + context 'include src => true and include deb => false' do let :facts do { :os => { :family => 'Debian', :name => 'Debian', :release => { :major => '7', :full => '7.0' }}, @@ -296,35 +296,8 @@ describe 'apt::source' do end let :params do { + :include => { 'deb' => false, 'src' => true }, :location => 'hello.there', - :include_deb => false, - :include_src => true, - } - end - - it { is_expected.to contain_apt__setting('list-my_source').with({ - :ensure => 'present', - }).with_content(/deb-src hello.there wheezy main\n/) - } - it { is_expected.to contain_apt__setting('list-my_source').without_content(/deb hello.there wheezy main\n/) } - end - - context 'include precedence' do - let :facts do - { - :os => { :family => 'Debian', :name => 'Debian', :release => { :major => '7', :full => '7.0' }}, - :lsbdistid => 'debian', - :lsbdistcodename => 'wheezy', - :osfamily => 'debian', - :puppetversion => Puppet.version, - } - end - let :params do - { - :location => 'hello.there', - :include_deb => true, - :include_src => false, - :include => { 'deb' => false, 'src' => true }, } end -- 2.32.3