opuppet/module-puppetlabs-apt
5
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

apt: Fix all strict variable cases.

Browse source 
A few of these fixes are absolutely horrendous but we have no choice as
we need to stay current- and future-parser compatible for now.

Once we can go Puppet 4 only we can use the `$facts` hash lookup instead
which will return undef/nil for things that aren't set instead of them
not being defined at all.
This commit is contained in:
Daniele Sluijters 2015-03-02 22:40:06 +01:00
parent 396036892d
commit c57d2dd5dd
13 changed files with 83 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
manifests/params.pp
View file

@ -1,9 +1,20 @@
class apt::params { class apt::params {
if $caller_module_name and $caller_module_name != $module_name { if defined('$caller_module_name') and $caller_module_name and $caller_module_name != $module_name {
fail('apt::params is a private class and cannot be accessed directly') fail('apt::params is a private class and cannot be accessed directly')
} }
if $::osfamily != 'Debian' {
fail('This module only works on Debian or derivatives like Ubuntu')
}
$xfacts = {
'lsbdistcodename' => defined('$lsbdistcodename') ? {
true => $::lsbdistcodename,
default => undef
},
}
$root = '/etc/apt' $root = '/etc/apt'
$provider = '/usr/bin/apt-get' $provider = '/usr/bin/apt-get'
$sources_list = "${root}/sources.list" $sources_list = "${root}/sources.list"
@ -13,10 +24,6 @@ class apt::params {
$preferences_d = "${root}/preferences.d" $preferences_d = "${root}/preferences.d"
$keyserver = 'keyserver.ubuntu.com' $keyserver = 'keyserver.ubuntu.com'
if $::osfamily != 'Debian' {
fail('This module only works on Debian or derivatives like Ubuntu')
}
$config_files = { $config_files = {
'conf' => { 'conf' => {
'path' => $conf_d, 'path' => $conf_d,
@ -68,7 +75,7 @@ class apt::params {
case $::lsbdistid { case $::lsbdistid {
'ubuntu', 'debian': { 'ubuntu', 'debian': {
$distid = $::lsbdistid $distid = $::lsbdistid
$distcodename = $::lsbdistcodename $distcodename = $xfacts['lsbdistcodename']
} }
'linuxmint': { 'linuxmint': {
if $::lsbdistcodename == 'debian' { if $::lsbdistcodename == 'debian' {
@ -113,5 +120,9 @@ class apt::params {
} }
} }
} }
'', default: {
$ppa_options = undef
$ppa_package = undef
}
} }
} }

12
manifests/pin.pp
View file

@ -3,7 +3,7 @@
define apt::pin( define apt::pin(
$ensure = present, $ensure = present,
$explanation = "${caller_module_name}: ${name}", $explanation = undef,
$order = undef, $order = undef,
$packages = '*', $packages = '*',
$priority = 0, $priority = 0,
@ -20,6 +20,16 @@ define apt::pin(
fail('Only integers are allowed in the apt::pin order param') fail('Only integers are allowed in the apt::pin order param')
} }
if $explanation {
$_explanation = $explanation
} else {
if defined('$caller_module_name') { # strict vars check
$_explanation = "${caller_module_name}: ${name}"
} else {
$_explanation = ": ${name}"
}
}
$pin_release_array = [ $pin_release_array = [
$release, $release,
$codename, $codename,

4
manifests/ppa.pp
View file

@ -1,12 +1,12 @@
# ppa.pp # ppa.pp
define apt::ppa( define apt::ppa(
$ensure = 'present', $ensure = 'present',
$release = $::lsbdistcodename,
$options = $::apt::ppa_options, $options = $::apt::ppa_options,
$release = $::apt::xfacts['lsbdistcodename'],
$package_name = $::apt::ppa_package, $package_name = $::apt::ppa_package,
$package_manage = false, $package_manage = false,
) { ) {
if ! $release { unless $release {
fail('lsbdistcodename fact not available: release parameter required') fail('lsbdistcodename fact not available: release parameter required')
} }

11
manifests/source.pp
View file

@ -4,7 +4,7 @@ define apt::source(
$comment = $name, $comment = $name,
$ensure = present, $ensure = present,
$location = '', $location = '',
$release = $::lsbdistcodename, $release = $::apt::xfacts['lsbdistcodename'],
$repos = 'main', $repos = 'main',
$include_src = false, $include_src = false,
$include_deb = true, $include_deb = true,
@ -13,10 +13,10 @@ define apt::source(
$architecture = undef, $architecture = undef,
$trusted_source = false, $trusted_source = false,
) { ) {
validate_string($architecture, $comment, $location, $release, $repos) validate_string($architecture, $comment, $location, $repos)
validate_bool($trusted_source, $include_src, $include_deb) validate_bool($trusted_source, $include_src, $include_deb)
if ! $release { unless $release {
fail('lsbdistcodename fact not available: release parameter required') fail('lsbdistcodename fact not available: release parameter required')
} }
@ -30,6 +30,7 @@ define apt::source(
$_key = merge($::apt::source_key_defaults, $key) $_key = merge($::apt::source_key_defaults, $key)
} else { } else {
validate_string($key) validate_string($key)
$_key = $key
} }
} }
@ -64,9 +65,9 @@ define apt::source(
before => $_before, before => $_before,
} }
} else { } else {
apt::key { "Add key: ${key} from Apt::Source ${title}": apt::key { "Add key: ${_key} from Apt::Source ${title}":
ensure => present, ensure => present,
id => $key, id => $_key,
before => $_before, before => $_before,
} }
} }

2
spec/classes/apt_spec.rb
View file

@ -1,6 +1,6 @@
require 'spec_helper' require 'spec_helper'
describe 'apt' do describe 'apt' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy'} }
context 'defaults' do context 'defaults' do
it { is_expected.to contain_file('sources.list').that_notifies('Exec[apt_update]').only_with({ it { is_expected.to contain_file('sources.list').that_notifies('Exec[apt_update]').only_with({

20
spec/classes/apt_update_spec.rb
View file

@ -4,7 +4,7 @@ require 'spec_helper'
describe 'apt::update', :type => :class do describe 'apt::update', :type => :class do
context "when update['always']=true" do context "when update['always']=true" do
#This should completely disable all of this logic. These tests are to guarantee that we don't somehow magically change the behavior. #This should completely disable all of this logic. These tests are to guarantee that we don't somehow magically change the behavior.
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{'::apt': update => {'always' => true},}" } let (:pre_condition) { "class{'::apt': update => {'always' => true},}" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
#set the apt_update exec's refreshonly attribute to false #set the apt_update exec's refreshonly attribute to false
@ -14,7 +14,7 @@ describe 'apt::update', :type => :class do
context "when apt::update['frequency'] has the value of #{update_frequency}" do context "when apt::update['frequency'] has the value of #{update_frequency}" do
{ 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval| { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
context "and $::apt_update_last_success indicates #{desc}" do context "and $::apt_update_last_success indicates #{desc}" do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" } let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
# set the apt_update exec's refreshonly attribute to false # set the apt_update exec's refreshonly attribute to false
@ -22,7 +22,7 @@ describe 'apt::update', :type => :class do
end end
end end
context 'when $::apt_update_last_success is nil' do context 'when $::apt_update_last_success is nil' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" } let (:pre_condition) { "class{'::apt': update => {'always' => true, 'frequency' => '#{update_frequency}'}, }" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
#set the apt_update exec\'s refreshonly attribute to false #set the apt_update exec\'s refreshonly attribute to false
@ -38,7 +38,7 @@ describe 'apt::update', :type => :class do
context "and apt::update['frequency']='always'" do context "and apt::update['frequency']='always'" do
{ 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval| { 'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
context "and $::apt_update_last_success indicates #{desc}" do context "and $::apt_update_last_success indicates #{desc}" do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{'::apt': update => {'always' => false, 'frequency' => 'always' },}" } let (:pre_condition) { "class{'::apt': update => {'always' => false, 'frequency' => 'always' },}" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
#set the apt_update exec's refreshonly attribute to false #set the apt_update exec's refreshonly attribute to false
@ -47,7 +47,7 @@ describe 'apt::update', :type => :class do
end end
end end
context 'when $::apt_update_last_success is nil' do context 'when $::apt_update_last_success is nil' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'always' },}" } let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'always' },}" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
#set the apt_update exec\'s refreshonly attribute to false #set the apt_update exec\'s refreshonly attribute to false
@ -58,7 +58,7 @@ describe 'apt::update', :type => :class do
context "and apt::update['frequency']='reluctantly'" do context "and apt::update['frequency']='reluctantly'" do
{'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval| {'a recent run' => Time.now.to_i, 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
context "and $::apt_update_last_success indicates #{desc}" do context "and $::apt_update_last_success indicates #{desc}" do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval} } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy'} }
let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" } let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" }
it 'should not trigger an apt-get update run' do it 'should not trigger an apt-get update run' do
#don't change the apt_update exec's refreshonly attribute. (it should be true) #don't change the apt_update exec's refreshonly attribute. (it should be true)
@ -67,7 +67,7 @@ describe 'apt::update', :type => :class do
end end
end end
context 'when $::apt_update_last_success is nil' do context 'when $::apt_update_last_success is nil' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" } let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => 'reluctantly' },}" }
it 'should not trigger an apt-get update run' do it 'should not trigger an apt-get update run' do
#don't change the apt_update exec's refreshonly attribute. (it should be true) #don't change the apt_update exec's refreshonly attribute. (it should be true)
@ -79,7 +79,7 @@ describe 'apt::update', :type => :class do
context "and apt::update['frequency'] has the value of #{update_frequency}" do context "and apt::update['frequency'] has the value of #{update_frequency}" do
{ 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval| { 'we are due for a run' => 1406660561,'the update-success-stamp file does not exist' => -1 }.each_pair do |desc, factval|
context "and $::apt_update_last_success indicates #{desc}" do context "and $::apt_update_last_success indicates #{desc}" do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => factval, :lsbdistcodename => 'wheezy' } }
let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" } let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
#set the apt_update exec\'s refreshonly attribute to false #set the apt_update exec\'s refreshonly attribute to false
@ -88,7 +88,7 @@ describe 'apt::update', :type => :class do
end end
end end
context 'when the $::apt_update_last_success fact has a recent value' do context 'when the $::apt_update_last_success fact has a recent value' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :apt_update_last_success => Time.now.to_i } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :apt_update_last_success => Time.now.to_i } }
let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" } let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
it 'should not trigger an apt-get update run' do it 'should not trigger an apt-get update run' do
#don't change the apt_update exec\'s refreshonly attribute. (it should be true) #don't change the apt_update exec\'s refreshonly attribute. (it should be true)
@ -96,7 +96,7 @@ describe 'apt::update', :type => :class do
end end
end end
context 'when $::apt_update_last_success is nil' do context 'when $::apt_update_last_success is nil' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy', :apt_update_last_success => nil } }
let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" } let (:pre_condition) { "class{ '::apt': update => {'always' => false, 'frequency' => '#{update_frequency}',} }" }
it 'should trigger an apt-get update run' do it 'should trigger an apt-get update run' do
#set the apt_update exec\'s refreshonly attribute to false #set the apt_update exec\'s refreshonly attribute to false

2
spec/classes/params_spec.rb
View file

@ -1,6 +1,6 @@
require 'spec_helper' require 'spec_helper'
describe 'apt::params', :type => :class do describe 'apt::params', :type => :class do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let (:title) { 'my_package' } let (:title) { 'my_package' }
it { is_expected.to contain_apt__params } it { is_expected.to contain_apt__params }

2
spec/defines/conf_spec.rb
View file

@ -3,7 +3,7 @@ describe 'apt::conf', :type => :define do
let :pre_condition do let :pre_condition do
'class { "apt": }' 'class { "apt": }'
end end
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let :title do let :title do
'norecommends' 'norecommends'
end end

21
spec/defines/key_spec.rb
View file

@ -1,7 +1,12 @@
require 'spec_helper' require 'spec_helper'
describe 'apt::key' do describe 'apt::key' do
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let :pre_condition do
'class { "apt": }'
end
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
GPG_KEY_ID = '47B320EB4C7C375AA9DAE1A01054B7A24BD6EC30' GPG_KEY_ID = '47B320EB4C7C375AA9DAE1A01054B7A24BD6EC30'
let :title do let :title do
@ -15,7 +20,7 @@ describe 'apt::key' do
:id => title, :id => title,
:ensure => 'present', :ensure => 'present',
:source => nil, :source => nil,
:server => nil, :server => 'keyserver.ubuntu.com',
:content => nil, :content => nil,
:options => nil, :options => nil,
}) })
@ -39,7 +44,7 @@ describe 'apt::key' do
:id => GPG_KEY_ID, :id => GPG_KEY_ID,
:ensure => 'present', :ensure => 'present',
:source => nil, :source => nil,
:server => nil, :server => 'keyserver.ubuntu.com',
:content => nil, :content => nil,
:options => nil, :options => nil,
}) })
@ -59,7 +64,7 @@ describe 'apt::key' do
:id => title, :id => title,
:ensure => 'absent', :ensure => 'absent',
:source => nil, :source => nil,
:server => nil, :server => 'keyserver.ubuntu.com',
:content => nil, :content => nil,
:keyserver => nil, :keyserver => nil,
}) })
@ -276,7 +281,8 @@ describe 'apt::key' do
describe 'duplication' do describe 'duplication' do
context 'two apt::key resources for same key, different titles' do context 'two apt::key resources for same key, different titles' do
let :pre_condition do let :pre_condition do
"apt::key { 'duplicate': id => '#{title}', }" "class { 'apt': }
apt::key { 'duplicate': id => '#{title}', }"
end end
it 'contains two apt::key resources' do it 'contains two apt::key resources' do
@ -295,7 +301,7 @@ describe 'apt::key' do
:id => title, :id => title,
:ensure => 'present', :ensure => 'present',
:source => nil, :source => nil,
:server => nil, :server => 'keyserver.ubuntu.com',
:content => nil, :content => nil,
:options => nil, :options => nil,
}) })
@ -305,7 +311,8 @@ describe 'apt::key' do
context 'two apt::key resources, different ensure' do context 'two apt::key resources, different ensure' do
let :pre_condition do let :pre_condition do
"apt::key { 'duplicate': id => '#{title}', ensure => 'absent', }" "class { 'apt': }
apt::key { 'duplicate': id => '#{title}', ensure => 'absent', }"
end end
it 'informs the user of the impossibility' do it 'informs the user of the impossibility' do
expect { subject }.to raise_error(/already ensured as absent/) expect { subject }.to raise_error(/already ensured as absent/)

2
spec/defines/pin_spec.rb
View file

@ -3,7 +3,7 @@ describe 'apt::pin', :type => :define do
let :pre_condition do let :pre_condition do
'class { "apt": }' 'class { "apt": }'
end end
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let(:title) { 'my_pin' } let(:title) { 'my_pin' }
context 'defaults' do context 'defaults' do

11
spec/defines/ppa_spec.rb
View file

@ -1,10 +1,10 @@
require 'spec_helper' require 'spec_helper'
describe 'apt::ppa' do describe 'apt::ppa' do
describe 'defaults' do
let :pre_condition do let :pre_condition do
'class { "apt": }' 'class { "apt": }'
end end
describe 'defaults' do
let :facts do let :facts do
{ {
:lsbdistrelease => '11.04', :lsbdistrelease => '11.04',
@ -190,6 +190,7 @@ describe 'apt::ppa' do
:operatingsystem => 'Ubuntu', :operatingsystem => 'Ubuntu',
:lsbdistid => 'Ubuntu', :lsbdistid => 'Ubuntu',
:osfamily => 'Debian', :osfamily => 'Debian',
:lsbdistcodeanme => nil,
} }
end end
let(:title) { 'ppa:foo' } let(:title) { 'ppa:foo' }
@ -203,10 +204,10 @@ describe 'apt::ppa' do
describe 'not ubuntu' do describe 'not ubuntu' do
let :facts do let :facts do
{ {
:lsbdistrelease => '14.04', :lsbdistrelease => '6.0.7',
:lsbdistcodename => 'trusty', :lsbdistcodename => 'wheezy',
:operatingsystem => 'Debian', :operatingsystem => 'Debian',
:lsbdistid => 'Ubuntu', :lsbdistid => 'debian',
:osfamily => 'Debian', :osfamily => 'Debian',
} }
end end

2
spec/defines/setting_spec.rb
View file

@ -2,7 +2,7 @@ require 'spec_helper'
describe 'apt::setting' do describe 'apt::setting' do
let(:pre_condition) { 'class { "apt": }' } let(:pre_condition) { 'class { "apt": }' }
let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian', :lsbdistcodename => 'wheezy' } }
let(:title) { 'conf-teddybear' } let(:title) { 'conf-teddybear' }
let(:default_params) { { :content => 'di' } } let(:default_params) { { :content => 'di' } }

2
templates/pin.pref.erb
View file

@ -15,7 +15,7 @@ elsif @origin.length > 0
@pin = "origin #{@origin}" @pin = "origin #{@origin}"
end end
-%> -%>
Explanation: <%= @explanation %> Explanation: <%= @_explanation %>
Package: <%= @packages_string %> Package: <%= @packages_string %>
Pin: <%= @pin %> Pin: <%= @pin %>
Pin-Priority: <%= @priority %> Pin-Priority: <%= @priority %>