Merge pull request #446 from puppetlabs/daenney/apt-proxy

apt: Add proxy support on the class.
puppetlabs · Feb 27, 2015 · aae853b · aae853b
2 parents b6c70b7 + d81c3d9
commit aae853b
Show file tree

Hide file tree

Showing 6 changed files with 182 additions and 84 deletions.
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -1,5 +1,6 @@
 #
 class apt(
+  $proxy                = {},
   $always_apt_update    = false,
   $apt_update_frequency = 'reluctantly',
   $purge_sources_list   = false,
@@ -19,6 +20,28 @@
   validate_bool($purge_sources_list, $purge_sources_list_d,
                 $purge_preferences, $purge_preferences_d)
 
+  validate_hash($proxy)
+  if $proxy['host'] {
+    validate_string($proxy['host'])
+  }
+  if $proxy['port'] {
+    unless is_integer($proxy['port']) {
+      fail('$proxy port must be an integer')
+    }
+  }
+  if $proxy['https'] {
+    validate_bool($proxy['https'])
+  }
+
+  $_proxy = merge($apt::proxy_defaults, $proxy)
+
+  if $proxy['host'] {
+    apt::setting { 'conf-proxy':
+      priority => '01',
+      content  => template('apt/_header.erb', 'apt/proxy.erb'),
+    }
+  }
+
   $sources_list_content = $purge_sources_list ? {
     false => undef,
     true  => "# Repos managed by puppet.\n",

diff --git a/manifests/params.pp b/manifests/params.pp
@@ -31,9 +31,10 @@
     }
   }
 
-  $proxy = {
-    'host' => undef,
-    'port' => 8080,
+  $proxy_defaults = {
+    'host'  => undef,
+    'port'  => 8080,
+    'https' => false,
   }
 
   $file_defaults = {

diff --git a/manifests/ppa.pp b/manifests/ppa.pp
@@ -5,7 +5,6 @@
   $options        = $::apt::ppa_options,
   $package_name   = $::apt::ppa_package,
   $package_manage = false,
-  $proxy          = {},
 ) {
   if ! $release {
     fail('lsbdistcodename fact not available: release parameter required')
@@ -20,8 +19,6 @@
   $filename_without_ppa     = regsubst($filename_without_dots, '^ppa:', '', 'G')
   $sources_list_d_filename  = "${filename_without_ppa}-${release}.list"
 
-  $_proxy = merge($apt::proxy, $proxy)
-
   if $ensure == 'present' {
     if $package_manage {
       package { $package_name: }
@@ -31,13 +28,15 @@
       $_require = File['sources.list.d']
     }
 
-    case $_proxy['host'] {
-      false, '', undef: {
-        $_proxy_env = []
-      }
-      default: {
-        $_proxy_env = ["http_proxy=http://${_proxy['host']}:${_proxy['port']}", "https_proxy=http://${_proxy['host']}:${_proxy['port']}"]
+    $_proxy = $::apt::_proxy
+    if $_proxy['host'] {
+      if $_proxy['https'] {
+        $_proxy_env = ["http_proxy=http://${_proxy['host']}:${_proxy['port']}", "https_proxy=https://${_proxy['host']}:${_proxy['port']}"]
+      } else {
+        $_proxy_env = ["http_proxy=http://${_proxy['host']}:${_proxy['port']}"]
       }
+    } else {
+      $_proxy_env = []
     }
 
     exec { "add-apt-repository-${name}":

diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb
@@ -1,49 +1,85 @@
 require 'spec_helper'
-describe 'apt', :type => :class do
+describe 'apt' do
   let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } }
 
   context 'defaults' do
     it { is_expected.to contain_file('sources.list').that_notifies('Exec[apt_update]').only_with({
-      'ensure' => 'present',
-      'path'   => '/etc/apt/sources.list',
-      'owner'  => 'root',
-      'group'  => 'root',
-      'mode'   => '0644',
-      'notify' => 'Exec[apt_update]',
+      :ensure => 'present',
+      :path   => '/etc/apt/sources.list',
+      :owner  => 'root',
+      :group  => 'root',
+      :mode   => '0644',
+      :notify => 'Exec[apt_update]',
     })}
 
     it { is_expected.to contain_file('sources.list.d').that_notifies('Exec[apt_update]').only_with({
-      'ensure'  => 'directory',
-      'path'    => '/etc/apt/sources.list.d',
-      'owner'   => 'root',
-      'group'   => 'root',
-      'purge'   => false,
-      'recurse' => false,
-      'notify'  => 'Exec[apt_update]',
+      :ensure  => 'directory',
+      :path    => '/etc/apt/sources.list.d',
+      :owner   => 'root',
+      :group   => 'root',
+      :purge   => false,
+      :recurse => false,
+      :notify  => 'Exec[apt_update]',
     })}
 
     it { is_expected.to contain_file('preferences.d').only_with({
-      'ensure'  => 'directory',
-      'path'    => '/etc/apt/preferences.d',
-      'owner'   => 'root',
-      'group'   => 'root',
-      'purge'   => false,
-      'recurse' => false,
+      :ensure  => 'directory',
+      :path    => '/etc/apt/preferences.d',
+      :owner   => 'root',
+      :group   => 'root',
+      :purge   => false,
+      :recurse => false,
     })}
 
     it 'should lay down /etc/apt/apt.conf.d/15update-stamp' do
       is_expected.to contain_file('/etc/apt/apt.conf.d/15update-stamp').with({
-        'group' => 'root',
-        'mode'  => '0644',
-        'owner' => 'root',
+        :group => 'root',
+        :mode  => '0644',
+        :owner => 'root',
       }).with_content(/APT::Update::Post-Invoke-Success \{"touch \/var\/lib\/apt\/periodic\/update-success-stamp 2>\/dev\/null \|\| true";\};/)
     end
 
     it { is_expected.to contain_exec('apt_update').with({
-      'refreshonly' => 'true',
+      :refreshonly => 'true',
     })}
+
+    it { is_expected.not_to contain_apt__setting('conf-proxy') }
   end
 
+  describe 'proxy=' do
+    context 'host=localhost' do
+      let(:params) { { :proxy => { 'host' => 'localhost'} } }
+      it { is_expected.to contain_apt__setting('conf-proxy').with({
+        :priority => '01',
+      }).with_content(
+        /Acquire::http::proxy "http:\/\/localhost:8080\/";/
+      ).without_content(
+        /Acquire::https::proxy/
+      )}
+    end
+
+    context 'host=localhost and port=8180' do
+      let(:params) { { :proxy => { 'host' => 'localhost', 'port' => 8180} } }
+      it { is_expected.to contain_apt__setting('conf-proxy').with({
+        :priority => '01',
+      }).with_content(
+        /Acquire::http::proxy "http:\/\/localhost:8180\/";/
+      ).without_content(
+        /Acquire::https::proxy/
+      )}
+    end
+
+    context 'host=localhost and https=true' do
+      let(:params) { { :proxy => { 'host' => 'localhost', 'https' => true} } }
+      it { is_expected.to contain_apt__setting('conf-proxy').with({
+        :priority => '01',
+      }).with_content(
+        /Acquire::http::proxy "http:\/\/localhost:8080\/";/
+      ).with_content(
+        /Acquire::https::proxy "https:\/\/localhost:8080\/";/
+      )}
+    end
+  end
   context 'lots of non-defaults' do
     let :params do
       {
@@ -58,28 +94,28 @@
     end
 
     it { is_expected.to contain_file('sources.list').with({
-      'content' => "# Repos managed by puppet.\n"
+      :content => "# Repos managed by puppet.\n"
     })}
 
     it { is_expected.to contain_file('sources.list.d').with({
-      'purge'   => 'true',
-      'recurse' => 'true',
+      :purge   => 'true',
+      :recurse => 'true',
     })}
 
     it { is_expected.to contain_file('apt-preferences').only_with({
-      'ensure' => 'absent',
-      'path'   => '/etc/apt/preferences',
+      :ensure => 'absent',
+      :path   => '/etc/apt/preferences',
     })}
 
     it { is_expected.to contain_file('preferences.d').with({
-      'purge'   => 'true',
-      'recurse' => 'true',
+      :purge   => 'true',
+      :recurse => 'true',
     })}
 
     it { is_expected.to contain_exec('apt_update').with({
-      'refreshonly' => 'false',
-      'timeout'     => '1',
-      'tries'       => '3',
+      :refreshonly => 'false',
+      :timeout     => '1',
+      :tries       => '3',
     })}
 
   end
@@ -99,7 +135,7 @@
         'key'               => '55BE302B',
         'key_server'        => 'subkeys.pgp.net',
         'pin'               => '-10',
-        'include_src'       => true
+        'include_src'       => true,
       },
       'puppetlabs' => {
         'location'   => 'http://apt.puppetlabs.com',
@@ -111,7 +147,7 @@
 
     it {
       is_expected.to contain_apt__setting('list-debian_unstable').with({
-        'ensure'  => 'present',
+        :ensure => 'present',
       })
     }
 
@@ -120,7 +156,7 @@
 
     it {
       is_expected.to contain_apt__setting('list-puppetlabs').with({
-        'ensure'  => 'present',
+        :ensure => 'present',
       })
     }
 
@@ -131,7 +167,7 @@
     context 'bad purge_sources_list' do
       let :params do
         {
-          'purge_sources_list' => 'foo'
+          :purge_sources_list => 'foo'
         }
       end
       it do
@@ -144,7 +180,7 @@
     context 'bad purge_sources_list_d' do
       let :params do
         {
-          'purge_sources_list_d' => 'foo'
+          :purge_sources_list_d => 'foo'
         }
       end
       it do
@@ -157,7 +193,7 @@
     context 'bad purge_preferences' do
       let :params do
         {
-          'purge_preferences' => 'foo'
+          :purge_preferences => 'foo'
         }
       end
       it do
@@ -170,7 +206,7 @@
     context 'bad purge_preferences_d' do
       let :params do
         {
-          'purge_preferences_d' => 'foo'
+          :purge_preferences_d => 'foo'
         }
       end
       it do