From d81c3d9476b14892882620723a02617c344f703c Mon Sep 17 00:00:00 2001 From: Daniele Sluijters Date: Fri, 27 Feb 2015 12:34:05 +0100 Subject: [PATCH] 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. --- manifests/init.pp | 23 ++++++++++++++++++++ manifests/params.pp | 7 +++--- manifests/ppa.pp | 17 +++++++-------- spec/classes/apt_spec.rb | 36 ++++++++++++++++++++++++++++++ spec/defines/ppa_spec.rb | 47 +++++++++++++++++++++++++++++++++++----- templates/proxy.erb | 4 ++++ 6 files changed, 116 insertions(+), 18 deletions(-) create mode 100644 templates/proxy.erb diff --git a/manifests/init.pp b/manifests/init.pp index 0b4a0bc..86e49c8 100644 --- 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 @@ 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", diff --git a/manifests/params.pp b/manifests/params.pp index 3c169e3..eda6adc 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -31,9 +31,10 @@ class apt::params { } } - $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 index 1dc8e26..4a08dce 100644 --- a/manifests/ppa.pp +++ b/manifests/ppa.pp @@ -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}": diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb index b07e0d6..c53e2a7 100644 --- a/spec/classes/apt_spec.rb +++ b/spec/classes/apt_spec.rb @@ -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 { diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index dde015a..14a5139 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -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 index 0000000..670e3a7 --- /dev/null +++ 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 -%> -- 2.32.3