]> review.fuel-infra Code Review - puppet-modules/puppetlabs-apt.git/commitdiff
Add a 'direct' option to proxy settings
authorWill Meek <william.meek@puppet.com>
Mon, 6 Nov 2017 15:46:57 +0000 (15:46 +0000)
committerWill Meek <william.meek@puppet.com>
Wed, 8 Nov 2017 11:22:59 +0000 (11:22 +0000)
This commit adds a 'direct' boolean option to proxy settings.
When set to true, if https is not true, the https proxy is set
to 'DIRECT'.

README.md
manifests/init.pp
manifests/params.pp
spec/classes/apt_spec.rb
templates/proxy.epp

index 7c7a7b33fae65bcb5ab7921e696c9d537d4cbb98..a93d7bda7b687afe3ca3de7921d50f916a61dbb8 100644 (file)
--- a/README.md
+++ b/README.md
@@ -290,6 +290,8 @@ Main class, includes all other classes.
   * 'https': Specifies whether to enable https proxies. Valid options: 'true' and 'false'. Default: 'false'.
 
   * 'ensure': Optional parameter. Valid options: 'file', 'present', and 'absent'. Default: 'undef'. Prefer 'file' over 'present'.
+  
+  * 'direct': Specifies whether or not to use a 'DIRECT' https proxy if http proxy is used but https is not. Valid options: 'true' and 'false'. Default: 'false'.
 
 * `purge`: Specifies whether to purge any existing settings that aren't managed by Puppet. Valid options: a hash made up from the following keys:
 
index 581ce2d32db403b91eeda05a41a4db732a5e386d..aa61986d21fdc992b3b3cecf85c898f6ea35a7f1 100644 (file)
@@ -78,6 +78,9 @@ class apt (
   if $proxy['https']{
     assert_type(Boolean, $proxy['https'])
   }
+  if $proxy['direct']{
+    assert_type(Boolean, $proxy['direct'])
+  }
 
   $_proxy = merge($apt::proxy_defaults, $proxy)
 
index 8126c02be5ccb487776ce3d28071bcf2afa9d563..f85b9d186ec960a3527ae4c9f7460af736a02792 100644 (file)
@@ -49,6 +49,7 @@ class apt::params {
     'host'   => undef,
     'port'   => 8080,
     'https'  => false,
+    'direct' => false,
   }
 
   $purge_defaults = {
index e46ad050728dcb71fb14e490de3384b0981f6686..cbbe746b0edb364ef0908fe0bb81595d113c2960 100644 (file)
@@ -54,7 +54,7 @@ describe 'apt' do
     it 'lays 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').with_content(/APT::Update::Post-Invoke-Success \{"touch \/var\/lib\/apt\/periodic\/update-success-stamp 2>\/dev\/null \|\| true";\};/) # rubocop:disable Metrics/LineLength
+                                                                             owner: 'root').with_content(%r{APT::Update::Post-Invoke-Success {"touch /var/lib/apt/periodic/update-success-stamp 2>/dev/null || true";};}) # rubocop:disable Metrics/LineLength
     end
 
     it {
@@ -70,9 +70,9 @@ describe 'apt' do
 
       it {
         is_expected.to contain_apt__setting('conf-proxy').with(priority: '01').with_content(
-          /Acquire::http::proxy "http:\/\/localhost:8080\/";/,
+          %r{Acquire::http::proxy "http://localhost:8080/";},
         ).without_content(
-          %r{Acquire::https::proxy "DIRECT"},
+          %r{Acquire::https::proxy},
         )
       }
     end
@@ -82,9 +82,9 @@ describe 'apt' do
 
       it {
         is_expected.to contain_apt__setting('conf-proxy').with(priority: '01').with_content(
-          /Acquire::http::proxy "http:\/\/localhost:8180\/";/,
+          %r{Acquire::http::proxy "http://localhost:8180/";},
         ).without_content(
-          %r{Acquire::https::proxy "DIRECT"},
+          %r{Acquire::https::proxy},
         )
       }
     end
@@ -94,9 +94,40 @@ describe 'apt' do
 
       it {
         is_expected.to contain_apt__setting('conf-proxy').with(priority: '01').with_content(
-          /Acquire::http::proxy "http:\/\/localhost:8080\/";/,
+          %r{Acquire::http::proxy "http://localhost:8080/";},
         ).with_content(
-          /Acquire::https::proxy "https:\/\/localhost:8080\/";/,
+          %r{Acquire::https::proxy "https://localhost:8080/";},
+        )
+      }
+    end
+
+    context 'host=localhost and direct=true' do
+      let(:params) { { proxy: { 'host' => 'localhost', 'direct' => true } } }
+
+      it {
+        is_expected.to contain_apt__setting('conf-proxy').with(priority: '01').with_content(
+          %r{Acquire::http::proxy "http://localhost:8080/";},
+        ).with_content(
+          %r{Acquire::https::proxy "DIRECT";},
+        )
+      }
+    end
+
+    context 'host=localhost and https=true and direct=true' do
+      let(:params) { { proxy: { 'host' => 'localhost', 'https' => true, 'direct' => true } } }
+
+      it {
+        is_expected.to contain_apt__setting('conf-proxy').with(priority: '01').with_content(
+          %r{Acquire::http::proxy "http://localhost:8080/";},
+        ).with_content(
+          %r{Acquire::https::proxy "https://localhost:8080/";},
+        )
+      }
+      it {
+        is_expected.to contain_apt__setting('conf-proxy').with(priority: '01').with_content(
+          %r{Acquire::http::proxy "http://localhost:8080/";},
+        ).without_content(
+          %r{Acquire::https::proxy "DIRECT";},
         )
       }
     end
@@ -175,14 +206,14 @@ describe 'apt' do
       is_expected.to contain_apt__setting('list-debian_unstable').with(ensure: 'present')
     }
 
-    it { is_expected.to contain_file('/etc/apt/sources.list.d/debian_unstable.list').with_content(/^deb http:\/\/debian.mirror.iweb.ca\/debian\/ unstable main contrib non-free$/) }
-    it { is_expected.to contain_file('/etc/apt/sources.list.d/debian_unstable.list').with_content(/^deb-src http:\/\/debian.mirror.iweb.ca\/debian\/ unstable main contrib non-free$/) }
+    it { is_expected.to contain_file('/etc/apt/sources.list.d/debian_unstable.list').with_content(%r{^deb http://debian.mirror.iweb.ca/debian/ unstable main contrib non-free$}) }
+    it { is_expected.to contain_file('/etc/apt/sources.list.d/debian_unstable.list').with_content(%r{^deb-src http://debian.mirror.iweb.ca/debian/ unstable main contrib non-free$}) }
 
     it {
       is_expected.to contain_apt__setting('list-puppetlabs').with(ensure: 'present')
     }
 
-    it { is_expected.to contain_file('/etc/apt/sources.list.d/puppetlabs.list').with_content(/^deb http:\/\/apt.puppetlabs.com precise main$/) }
+    it { is_expected.to contain_file('/etc/apt/sources.list.d/puppetlabs.list').with_content(%r{^deb http://apt.puppetlabs.com precise main$}) }
   end
 
   context 'with confs defined on valid osfamily' do
index 94e41af55d438404e0841871856694fa381abfef..ee663cb82ea80ccfe15ffc90d0ec3c967188d06f 100644 (file)
@@ -2,6 +2,6 @@
 Acquire::http::proxy "http://<%= $proxies['host'] %>:<%= $proxies['port'] %>/";
 <%- if $proxies['https'] { %>
 Acquire::https::proxy "https://<%= $proxies['host'] %>:<%= $proxies['port'] %>/";
-<%- } else { -%>
- Acquire::https::proxy "DIRECT";
+<%- } elsif $proxies['direct'] { -%>
+Acquire::https::proxy "DIRECT";
 <%- } -%>