From a758247f2632b3204167a8058fbb8903f0438841 Mon Sep 17 00:00:00 2001 From: Ken Barber Date: Wed, 21 Mar 2012 13:20:13 +0000 Subject: [PATCH] (#13289) Clean up style violations and fix corresponding tests --- manifests/builddep.pp | 2 +- manifests/debian/testing.pp | 16 ++++++------ manifests/debian/unstable.pp | 28 ++++++++++---------- manifests/force.pp | 11 ++++---- manifests/init.pp | 50 ++++++++++++++++++------------------ manifests/key.pp | 34 ++++++++++++------------ manifests/pin.pp | 11 ++++---- manifests/ppa.pp | 8 +++--- manifests/release.pp | 6 ++--- manifests/source.pp | 22 ++++++++-------- spec/classes/apt_spec.rb | 4 +-- spec/classes/release_spec.rb | 2 +- spec/defines/pin_spec.rb | 10 ++++---- spec/defines/source_spec.rb | 10 ++++---- tests/source.pp | 42 +++++++++++++++--------------- 15 files changed, 129 insertions(+), 127 deletions(-) diff --git a/manifests/builddep.pp b/manifests/builddep.pp index 8aebc67..8891bd9 100644 --- a/manifests/builddep.pp +++ b/manifests/builddep.pp @@ -5,7 +5,7 @@ define apt::builddep() { Class['apt'] -> Apt::Builddep[$name] exec { "apt-update-${name}": - command => "/usr/bin/apt-get update", + command => '/usr/bin/apt-get update', refreshonly => true, } diff --git a/manifests/debian/testing.pp b/manifests/debian/testing.pp index 4eec1f8..cfdeb3c 100644 --- a/manifests/debian/testing.pp +++ b/manifests/debian/testing.pp @@ -8,14 +8,14 @@ class apt::debian::testing { # debian-keyring # debian-archive-keyring - apt::source { "debian_testing": - location => "http://debian.mirror.iweb.ca/debian/", - release => "testing", - repos => "main contrib non-free", - required_packages => "debian-keyring debian-archive-keyring", - key => "55BE302B", - key_server => "subkeys.pgp.net", - pin => "-10" + apt::source { 'debian_testing': + location => 'http://debian.mirror.iweb.ca/debian/', + release => 'testing', + repos => 'main contrib non-free', + required_packages => 'debian-keyring debian-archive-keyring', + key => '55BE302B', + key_server => 'subkeys.pgp.net', + pin => '-10', } } diff --git a/manifests/debian/unstable.pp b/manifests/debian/unstable.pp index 782440f..3991023 100644 --- a/manifests/debian/unstable.pp +++ b/manifests/debian/unstable.pp @@ -2,20 +2,20 @@ class apt::debian::unstable { - # deb http://debian.mirror.iweb.ca/debian/ unstable main contrib non-free - # deb-src http://debian.mirror.iweb.ca/debian/ unstable main contrib non-free - # Key: 55BE302B Server: subkeys.pgp.net - # debian-keyring - # debian-archive-keyring + # deb http://debian.mirror.iweb.ca/debian/ unstable main contrib non-free + # deb-src http://debian.mirror.iweb.ca/debian/ unstable main contrib non-free + # Key: 55BE302B Server: subkeys.pgp.net + # debian-keyring + # debian-archive-keyring - apt::source { "debian_unstable": - location => "http://debian.mirror.iweb.ca/debian/", - release => "unstable", - repos => "main contrib non-free", - required_packages => "debian-keyring debian-archive-keyring", - key => "55BE302B", - key_server => "subkeys.pgp.net", - pin => "-10" - } + apt::source { 'debian_unstable': + location => 'http://debian.mirror.iweb.ca/debian/', + release => 'unstable', + repos => 'main contrib non-free', + required_packages => 'debian-keyring debian-archive-keyring', + key => '55BE302B', + key_server => 'subkeys.pgp.net', + pin => '-10', + } } diff --git a/manifests/force.pp b/manifests/force.pp index ec6f57e..45c5679 100644 --- a/manifests/force.pp +++ b/manifests/force.pp @@ -7,15 +7,16 @@ define apt::force( ) { $version_string = $version ? { - false => undef, + false => undef, default => "=${version}", } + $install_check = $version ? { + false => "/usr/bin/dpkg -s ${name} | grep -q 'Status: install'", + default => "/usr/bin/dpkg -s ${name} | grep -q 'Version: ${version}'", + } exec { "/usr/bin/aptitude -y -t ${release} install ${name}${version_string}": - unless => $version ? { - false => "/usr/bin/dpkg -s ${name} | grep -q 'Status: install'", - default => "/usr/bin/dpkg -s ${name} | grep -q 'Version: ${version}'" - } + unless => $install_check, } } diff --git a/manifests/init.pp b/manifests/init.pp index b41d359..1a5c7e0 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -33,57 +33,57 @@ class apt( validate_bool($purge_sources_list, $purge_sources_list_d) $refresh_only_apt_update = $always_apt_update? { - true => false, - false => true + true => false, + false => true, } - if ! defined(Package["python-software-properties"]) { - package { "python-software-properties": } + if ! defined(Package['python-software-properties']) { + package { 'python-software-properties': } } - file { "sources.list": - path => "${apt::params::root}/sources.list", - ensure => present, - owner => root, - group => root, - mode => 644, + file { 'sources.list': + ensure => present, + path => "${apt::params::root}/sources.list", + owner => root, + group => root, + mode => '0644', content => $purge_sources_list ? { false => undef, true => "# Repos managed by puppet.\n", }, } - file { "sources.list.d": - path => "${apt::params::root}/sources.list.d", - ensure => directory, - owner => root, - group => root, - purge => $purge_sources_list_d, + file { 'sources.list.d': + ensure => directory, + path => "${apt::params::root}/sources.list.d", + owner => root, + group => root, + purge => $purge_sources_list_d, recurse => $purge_sources_list_d, } - exec { "apt_update": - command => "${apt::params::provider} update", - subscribe => [ File["sources.list"], File["sources.list.d"] ], + exec { 'apt_update': + command => "${apt::params::provider} update", + subscribe => [ File['sources.list'], File['sources.list.d'] ], refreshonly => $refresh_only_apt_update, } case $disable_keys { true: { - file { "99unauth": - content => "APT::Get::AllowUnauthenticated 1;\n", + file { '99unauth': ensure => present, - path => "/etc/apt/apt.conf.d/99unauth", + content => "APT::Get::AllowUnauthenticated 1;\n", + path => '/etc/apt/apt.conf.d/99unauth', } } false: { - file { "99unauth": + file { '99unauth': ensure => absent, - path => "/etc/apt/apt.conf.d/99unauth", + path => '/etc/apt/apt.conf.d/99unauth', } } undef: { } # do nothing - default: { fail("Valid values for disable_keys are true or false") } + default: { fail('Valid values for disable_keys are true or false') } } if($proxy_host) { diff --git a/manifests/key.pp b/manifests/key.pp index c6e5f6e..63165f7 100644 --- a/manifests/key.pp +++ b/manifests/key.pp @@ -3,7 +3,7 @@ define apt::key ( $ensure = present, $key_content = false, $key_source = false, - $key_server = "keyserver.ubuntu.com" + $key_server = 'keyserver.ubuntu.com' ) { include apt::params @@ -11,11 +11,11 @@ define apt::key ( $upkey = upcase($key) if $key_content { - $method = "content" + $method = 'content' } elsif $key_source { - $method = "source" + $method = 'source' } elsif $key_server { - $method = "server" + $method = 'server' } # This is a hash of the parts of the key definition that we care about. @@ -29,7 +29,7 @@ define apt::key ( case $ensure { present: { - anchor { "apt::key/$title":; } + anchor { "apt::key/${title}":; } if defined(Exec["apt::key $upkey absent"]) { fail ("Cannot ensure Apt::Key[$upkey] present; $upkey already ensured absent") @@ -41,14 +41,14 @@ define apt::key ( if !defined(Exec[$digest]) { exec { $digest: - path => "/bin:/usr/bin", + path => '/bin:/usr/bin', unless => "/usr/bin/apt-key list | /bin/grep '${upkey}'", - before => Anchor["apt::key $upkey present"], + before => Anchor["apt::key ${upkey} present"], command => $method ? { - "content" => "echo '${key_content}' | /usr/bin/apt-key add -", - "source" => "wget -q '${key_source}' -O- | apt-key add -", - "server" => "apt-key adv --keyserver '${key_server}' --recv-keys '${upkey}'", - }; + 'content' => "echo '${key_content}' | /usr/bin/apt-key add -", + 'source' => "wget -q '${key_source}' -O- | apt-key add -", + 'server' => "apt-key adv --keyserver '${key_server}' --recv-keys '${upkey}'", + }, } } @@ -62,16 +62,16 @@ define apt::key ( } exec { "apt::key $upkey absent": - path => "/bin:/usr/bin", - onlyif => "apt-key list | grep '$upkey'", - command => "apt-key del '$upkey'", - user => "root", - group => "root", + path => '/bin:/usr/bin', + onlyif => "apt-key list | grep '${upkey}'", + command => "apt-key del '${upkey}'", + user => 'root', + group => 'root', } } default: { - fail "Invalid 'ensure' value '$ensure' for aptkey" + fail "Invalid 'ensure' value '${ensure}' for aptkey" } } } diff --git a/manifests/pin.pp b/manifests/pin.pp index 40695af..95d445c 100644 --- a/manifests/pin.pp +++ b/manifests/pin.pp @@ -8,12 +8,13 @@ define apt::pin( include apt::params + $pcontent = "# ${name}\nPackage: ${packages}\nPin: release a=${name}\nPin-Priority: ${priority}" file { "${name}.pref": - path => "${apt::params::root}/preferences.d/${name}", - ensure => file, - owner => root, - group => root, - mode => 644, + ensure => file, + path => "${apt::params::root}/preferences.d/${name}", + owner => root, + group => root, + mode => '0644', content => "# ${name}\nPackage: ${packages}\nPin: release a=${name}\nPin-Priority: ${priority}", } } diff --git a/manifests/ppa.pp b/manifests/ppa.pp index 712f425..095d8f1 100644 --- a/manifests/ppa.pp +++ b/manifests/ppa.pp @@ -9,17 +9,17 @@ define apt::ppa( include apt::params if ! $release { - fail("lsbdistcodename fact not available: release parameter required") + fail('lsbdistcodename fact not available: release parameter required') } exec { "apt-update-${name}": - command => "/usr/bin/aptitude update", + command => '/usr/bin/aptitude update', refreshonly => true, } $filename_without_slashes = regsubst($name,'/','-','G') - $filename_without_ppa = regsubst($filename_without_slashes, '^ppa:','','G') - $sources_list_d_filename = "${filename_without_ppa}-${release}.list" + $filename_without_ppa = regsubst($filename_without_slashes, '^ppa:','','G') + $sources_list_d_filename = "${filename_without_ppa}-${release}.list" exec { "add-apt-repository-${name}": command => "/usr/bin/add-apt-repository ${name}", diff --git a/manifests/release.pp b/manifests/release.pp index 9fc0aa3..ee94e13 100644 --- a/manifests/release.pp +++ b/manifests/release.pp @@ -7,9 +7,9 @@ class apt::release ( include apt::params file { "${apt::params::root}/apt.conf.d/01release": - owner => root, - group => root, - mode => 644, + owner => root, + group => root, + mode => '0644', content => "APT::Default-Release \"${release_id}\";" } } diff --git a/manifests/source.pp b/manifests/source.pp index 42a21e9..95768dc 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -17,25 +17,25 @@ define apt::source( include apt::params if $release == undef { - fail("lsbdistcodename fact not available: release parameter required") + fail('lsbdistcodename fact not available: release parameter required') } file { "${name}.list": - path => "${apt::params::root}/sources.list.d/${name}.list", - ensure => file, - owner => root, - group => root, - mode => 644, - content => template("apt/source.list.erb"), + ensure => file, + path => "${apt::params::root}/sources.list.d/${name}.list", + owner => root, + group => root, + mode => '0644', + content => template("${module_name}/source.list.erb"), } if $pin != false { - apt::pin { "${release}": priority => "${pin}" } -> File["${name}.list"] + apt::pin { $release: priority => $pin } -> File["${name}.list"] } exec { "${name} apt update": - command => "${apt::params::provider} update", - subscribe => File["${name}.list"], + command => "${apt::params::provider} update", + subscribe => File["${name}.list"], refreshonly => true, } @@ -49,8 +49,8 @@ define apt::source( if $key != false { apt::key { "Add key: ${key} from Apt::Source ${title}": - key => $key, ensure => present, + key => $key, key_server => $key_server, key_content => $key_content, key_source => $key_source, diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb index 71a438e..000793d 100644 --- a/spec/classes/apt_spec.rb +++ b/spec/classes/apt_spec.rb @@ -50,7 +50,7 @@ describe 'apt', :type => :class do 'ensure' => "present", 'owner' => "root", 'group' => "root", - 'mode' => 644, + 'mode' => "0644", "content" => "# Repos managed by puppet.\n" }) else @@ -59,7 +59,7 @@ describe 'apt', :type => :class do 'ensure' => "present", 'owner' => "root", 'group' => "root", - 'mode' => 644, + 'mode' => "0644", 'content' => nil }) end diff --git a/spec/classes/release_spec.rb b/spec/classes/release_spec.rb index 221df03..d131b22 100644 --- a/spec/classes/release_spec.rb +++ b/spec/classes/release_spec.rb @@ -12,9 +12,9 @@ describe 'apt::release', :type => :class do it { should contain_file("/etc/apt/apt.conf.d/01release").with({ + "mode" => "0644", "owner" => "root", "group" => "root", - "mode" => 644, "content" => "APT::Default-Release \"#{param_set[:release_id]}\";" }) } diff --git a/spec/defines/pin_spec.rb b/spec/defines/pin_spec.rb index 52c6bfa..dbc595c 100644 --- a/spec/defines/pin_spec.rb +++ b/spec/defines/pin_spec.rb @@ -27,12 +27,12 @@ describe 'apt::pin', :type => :define do it { should include_class("apt::params") } it { should contain_file("#{title}.pref").with({ + 'ensure' => 'file', 'path' => "/etc/apt/preferences.d/#{title}", - 'ensure' => "file", - 'owner' => "root", - 'group' => "root", - 'mode' => "644", - 'content' => "# #{title}\nPackage: #{param_hash[:packages]}\nPin: release a=#{title}\nPin-Priority: #{param_hash[:priority]}" + 'owner' => 'root', + 'group' => 'root', + 'mode' => '0644', + 'content' => "# #{title}\nPackage: #{param_hash[:packages]}\nPin: release a=#{title}\nPin-Priority: #{param_hash[:priority]}", }) } end diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb index e93137e..2843429 100644 --- a/spec/defines/source_spec.rb +++ b/spec/defines/source_spec.rb @@ -66,12 +66,12 @@ describe 'apt::source', :type => :define do it { should contain_apt__params } it { should contain_file("#{title}.list").with({ + 'ensure' => 'file', 'path' => filename, - 'ensure' => "file", - 'owner' => "root", - 'group' => "root", - 'mode' => 644, - 'content' => content + 'owner' => 'root', + 'group' => 'root', + 'mode' => '0644', + 'content' => content, }) } diff --git a/tests/source.pp b/tests/source.pp index 4db740d..a8702cf 100644 --- a/tests/source.pp +++ b/tests/source.pp @@ -1,29 +1,29 @@ class { 'apt': } apt::source { 'foo': - location => '', - release => 'karmic', - repos => 'main', - include_src => true, + location => '', + release => 'karmic', + repos => 'main', + include_src => true, required_packages => false, - key => false, - key_server => 'keyserver.ubuntu.com', - pin => '600' + key => false, + key_server => 'keyserver.ubuntu.com', + pin => '600', } # test two sources with the same key -apt::source { "debian_testing": - location => "http://debian.mirror.iweb.ca/debian/", - release => "testing", - repos => "main contrib non-free", - key => "55BE302B", - key_server => "subkeys.pgp.net", - pin => "-10" +apt::source { 'debian_testing': + location => 'http://debian.mirror.iweb.ca/debian/', + release => 'testing', + repos => 'main contrib non-free', + key => '55BE302B', + key_server => 'subkeys.pgp.net', + pin => '-10', } -apt::source { "debian_unstable": - location => "http://debian.mirror.iweb.ca/debian/", - release => "unstable", - repos => "main contrib non-free", - key => "55BE302B", - key_server => "subkeys.pgp.net", - pin => "-10" +apt::source { 'debian_unstable': + location => 'http://debian.mirror.iweb.ca/debian/', + release => 'unstable', + repos => 'main contrib non-free', + key => '55BE302B', + key_server => 'subkeys.pgp.net', + pin => '-10', } -- 2.32.3