(#13289) Clean up style violations and fix corresponding tests

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,
   }
 

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',
   }
 
 }

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',
+  }
 
 }

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,
   }
 
 }

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) {

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"
     }
   }
 }

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}",
   }
 }

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}",

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}\";"
   }
 }

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,

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

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]}\";"
     })
   }

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

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,
         })
       }
 

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',
 }