apt: Add proxy support on the class.

Re-introduce proxy support at the class level. Needing to configure a proxy is such a common scenario that having it on the class is a reasonable thing. It also affects `apt::ppa`. Change `apt::ppa` to no longer have its own `proxy` parameter but use the proxy as configured on the main `apt` class.
puppetlabs · Feb 27, 2015 · fb908ec · fb908ec
1 parent b53ea1b
commit fb908ec
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 18 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,26 @@
   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)
+
+  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
@@ -44,6 +44,40 @@
     })}
   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
       {

diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb
@@ -60,7 +60,9 @@
 
   describe 'apt included, proxy host' do
     let :pre_condition do
-      'class { "apt": }'
+      'class { "apt":
+        proxy => { "host" => "localhost" },
+      }'
     end
     let :facts do
       {
@@ -75,13 +77,12 @@
       {
         'options' => '',
         'package_manage' => true,
-        'proxy' => { 'host' => 'localhost', }
       }
     end
     let(:title) { 'ppa:foo' }
     it { is_expected.to contain_package('software-properties-common') }
     it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({
-      :environment => ['http_proxy=http://localhost:8080', 'https_proxy=http://localhost:8080'],
+      :environment => ['http_proxy=http://localhost:8080'],
       :command     => '/usr/bin/add-apt-repository  ppa:foo',
       :unless      => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list',
       :user        => 'root',
@@ -92,7 +93,42 @@
 
   describe 'apt included, proxy host and port' do
     let :pre_condition do
-      'class { "apt": }'
+      'class { "apt":
+        proxy => { "host" => "localhost", "port" => 8180 },
+      }'
+    end
+    let :facts do
+      {
+        :lsbdistrelease  => '14.04',
+        :lsbdistcodename => 'trusty',
+        :operatingsystem => 'Ubuntu',
+        :lsbdistid       => 'Ubuntu',
+        :osfamily        => 'Debian',
+      }
+    end
+    let :params do
+      {
+        :options => '',
+        :package_manage => true,
+      }
+    end
+    let(:title) { 'ppa:foo' }
+    it { is_expected.to contain_package('software-properties-common') }
+    it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({
+      :environment => ['http_proxy=http://localhost:8180'],
+      :command     => '/usr/bin/add-apt-repository  ppa:foo',
+      :unless      => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list',
+      :user        => 'root',
+      :logoutput   => 'on_failure',
+    })
+    }
+  end
+
+  describe 'apt included, proxy host and port and https' do
+    let :pre_condition do
+      'class { "apt":
+        proxy => { "host" => "localhost", "port" => 8180, "https" => true },
+      }'
     end
     let :facts do
       {
@@ -107,13 +143,12 @@
       {
         :options => '',
         :package_manage => true,
-        :proxy => { 'host' => 'localhost', 'port' => 8180, }
       }
     end
     let(:title) { 'ppa:foo' }
     it { is_expected.to contain_package('software-properties-common') }
     it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Exec[apt_update]').with({
-      :environment => ['http_proxy=http://localhost:8180', 'https_proxy=http://localhost:8180'],
+      :environment => ['http_proxy=http://localhost:8180', 'https_proxy=https://localhost:8180'],
       :command     => '/usr/bin/add-apt-repository  ppa:foo',
       :unless      => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list',
       :user        => 'root',

diff --git a/templates/proxy.erb b/templates/proxy.erb
@@ -0,0 +1,4 @@
+Acquire::http::proxy "http://<%= @_proxy['host'] %>:<%= @_proxy['port'] %>/";
+<%- if @_proxy['https'] %>
+Acquire::https::proxy "https://<%= @_proxy['host'] %>:<%= @_proxy['port'] %>/";
+<%- end -%>