]> review.fuel-infra Code Review - puppet-modules/puppetlabs-apt.git/commitdiff
apt: Add proxy support on the class.
authorDaniele Sluijters <daenney@users.noreply.github.com>
Fri, 27 Feb 2015 11:34:05 +0000 (12:34 +0100)
committerDaniele Sluijters <daenney@users.noreply.github.com>
Fri, 27 Feb 2015 20:14:24 +0000 (21:14 +0100)
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.

manifests/init.pp
manifests/params.pp
manifests/ppa.pp
spec/classes/apt_spec.rb
spec/defines/ppa_spec.rb
templates/proxy.erb [new file with mode: 0644]

index 0b4a0bc6267aad080422786c4dea95c0f9fffecb..86e49c8f4ea48ff029e0251da34f01c39d0e5d44 100644 (file)
@@ -1,5 +1,6 @@
 #
 class apt(
+  $proxy                = {},
   $always_apt_update    = false,
   $apt_update_frequency = 'reluctantly',
   $purge_sources_list   = false,
@@ -19,6 +20,28 @@ class apt(
   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",
index 3c169e3fcfa8c05a32e395f0fee531f9dd6ae8fc..eda6adc93b3e4cd812a88484b7daab9b5db9f673 100644 (file)
@@ -31,9 +31,10 @@ class apt::params {
     }
   }
 
-  $proxy = {
-    'host' => undef,
-    'port' => 8080,
+  $proxy_defaults = {
+    'host'  => undef,
+    'port'  => 8080,
+    'https' => false,
   }
 
   $file_defaults = {
index 1dc8e26fce80519d67c00c21cb16c894f00b029b..4a08dce6fd413134590962f8ad11091b171f5958 100644 (file)
@@ -5,7 +5,6 @@ define apt::ppa(
   $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 @@ define apt::ppa(
   $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 @@ define apt::ppa(
       $_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}":
index b07e0d6a769e11d8b7b1c1da5ecc7900fef8f38a..c53e2a7dc67e40702926650ae97208fd69f06708 100644 (file)
@@ -42,8 +42,44 @@ describe 'apt' do
     it { is_expected.to contain_exec('apt_update').with({
       :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
       {
index dde015abad483c8de23c1e3b3605c8904edcaa9e..14a5139c3ba4d465b52c37f8bc3d98c8d9e03923 100644 (file)
@@ -60,7 +60,9 @@ describe 'apt::ppa' do
 
   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 @@ describe 'apt::ppa' do
       {
         '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::ppa' do
 
   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 @@ describe 'apt::ppa' do
       {
         :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
new file mode 100644 (file)
index 0000000..670e3a7
--- /dev/null
@@ -0,0 +1,4 @@
+Acquire::http::proxy "http://<%= @_proxy['host'] %>:<%= @_proxy['port'] %>/";
+<%- if @_proxy['https'] %>
+Acquire::https::proxy "https://<%= @_proxy['host'] %>:<%= @_proxy['port'] %>/";
+<%- end -%>