raise default version to 1.16.0

this fixes the acceptance tests. But there is a bigger issue here. The code in manifests/init.pp is not idempotent: ``` String[1] $nginx_version = pick(fact('nginx_version'), '1.6.0'), ``` Turns out on the first run the fact might not be set yet leading to a pre 1.15.0 compatible configuration on systems wich ship a newer version of nginx. Leading to ``` nginx: [emerg] unknown directive "ssl" in /etc/nginx/sites-enabled/www.puppetlabs.com.conf:25 ```
voxpupuli · Jun 3, 2024 · 7928a7d · 7928a7d
1 parent bbfff0a
commit 7928a7d
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 39 deletions.
diff --git a/.rspec b/.rspec
@@ -3,3 +3,4 @@
 
 --format documentation
 --color
+--fail-fast
diff --git a/REFERENCE.md b/REFERENCE.md
@@ -281,7 +281,7 @@ already installed.  If the fact is unavailable, it defaults to '1.6.0'.
 You may need to set this manually to get a working and idempotent
 configuration.
 
-Default value: `pick(fact('nginx_version'), '1.6.0')`
+Default value: `pick(fact('nginx_version'), '1.16.0')`
 
 ##### <a name="-nginx--debug_connections"></a>`debug_connections`
 
@@ -2918,15 +2918,15 @@ Default value: `'on'`
 
 Data type: `Enum['on', 'off']`
 
-Wheter to use proxy_protocol
+Wheter to use proxy_protocol, only suppported with nginx >= 1.19.8
 
 Default value: `'off'`
 
 ##### <a name="-nginx--resource--mailhost--proxy_smtp_auth"></a>`proxy_smtp_auth`
 
 Data type: `Enum['on', 'off']`
 
-Wheter to use proxy_smtp_auth
+Wheter to use proxy_smtp_auth, only suppported with nginx >= 1.19.4
 
 Default value: `'off'`
 

diff --git a/manifests/init.pp b/manifests/init.pp
@@ -240,7 +240,7 @@
   Hash $nginx_upstreams                                   = {},
   Nginx::UpstreamDefaults $nginx_upstreams_defaults       = {},
   Boolean $purge_passenger_repo                           = true,
-  String[1] $nginx_version                                = pick(fact('nginx_version'), '1.6.0'),
+  String[1] $nginx_version                                = pick(fact('nginx_version'), '1.16.0'),
 
   ### END Hiera Lookups ###
 ) inherits nginx::params {

diff --git a/manifests/resource/mailhost.pp b/manifests/resource/mailhost.pp
@@ -73,9 +73,9 @@
 # @param xclient
 #   Whether to use xclient for smtp
 # @param proxy_protocol
-#   Wheter to use proxy_protocol
+#   Wheter to use proxy_protocol, only suppported with nginx >= 1.19.8
 # @param proxy_smtp_auth
-#   Wheter to use proxy_smtp_auth
+#   Wheter to use proxy_smtp_auth, only suppported with nginx >= 1.19.4
 # @param imap_auth
 #   Sets permitted methods of authentication for IMAP clients.
 # @param imap_capabilities
@@ -257,6 +257,7 @@
       smtp_auth                => $smtp_auth,
       smtp_capabilities        => $smtp_capabilities,
       xclient                  => $xclient,
+      nginx_version            => $nginx::nginx_version,
   })
 
   concat { $config_file:

diff --git a/spec/acceptance/nginx_mail_spec.rb b/spec/acceptance/nginx_mail_spec.rb
@@ -80,7 +80,7 @@ class { 'nginx':
       it { is_expected.to be_listening }
     end
 
-    context 'when configured for nginx 1.14' do
+    context 'when configured for nginx 1.14', if: !%w[Debian Archlinux].include?(fact('os.family')) do
       it 'runs successfully' do
         pp = "
       if fact('os.family') == 'RedHat' {

diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb
@@ -189,13 +189,8 @@
             let(:params) { { package_source: 'passenger' } }
 
             it { is_expected.to contain_package('nginx') }
+            it { is_expected.to contain_package('libnginx-mod-http-passenger') }
 
-            if (facts.dig(:os, 'name') == 'Debian' && %w[11].include?(facts.dig(:os, 'release', 'major'))) ||
-               (facts.dig(:os, 'name') == 'Ubuntu' && %w[bionic focal jammy].include?(facts.dig(:os, 'distro', 'codename')))
-              it { is_expected.to contain_package('libnginx-mod-http-passenger') }
-            else
-              it { is_expected.to contain_package('passenger') }
-            end
             it do
               is_expected.to contain_apt__source('nginx').with(
                 'location' => 'https://oss-binaries.phusionpassenger.com/apt/passenger',

diff --git a/spec/defines/resource_mailhost_spec.rb b/spec/defines/resource_mailhost_spec.rb
@@ -110,18 +110,6 @@
               value: 'off',
               match: '  xclient               off;'
             },
-            {
-              title: 'should set proxy_protocol',
-              attr: 'proxy_protocol',
-              value: 'off',
-              match: '  proxy_protocol        off;'
-            },
-            {
-              title: 'should set proxy_smtp_auth',
-              attr: 'proxy_smtp_auth',
-              value: 'off',
-              match: '  proxy_smtp_auth       off;'
-            },
             {
               title: 'should set auth_http',
               attr: 'auth_http',
@@ -254,6 +242,23 @@
               end
             end
           end
+          context 'mail proxy parameters' do
+            let(:pre_condition) { ['class { "nginx": nginx_version => "1.20.0"}'] }
+            let(:params) do
+              {
+                listen_port: 25,
+                ipv6_enable: true,
+                ssl_cert: 'dummy.crt',
+                ssl_key: 'dummy.key'
+              }
+            end
+
+            it 'configures mail proxy settings' do
+              content = catalogue.resource('concat::fragment', "#{title}-header").send(:parameters)[:content]
+              expect(content).to include('proxy_protocol        off;')
+              expect(content).to include('proxy_smtp_auth       off;')
+            end
+          end
         end
 
         describe 'mailhost template content for imap' do
@@ -548,7 +553,7 @@
               title: 'should set the IPv4 SSL listen port',
               attr: 'ssl_port',
               value: 45,
-              match: '  listen                *:45;'
+              match: '  listen                *:45 ssl;'
             },
             {
               title: 'should enable IPv6',
@@ -598,18 +603,6 @@
               value: 'off',
               match: '  xclient               off;'
             },
-            {
-              title: 'should set proxy_protocol',
-              attr: 'proxy_protocol',
-              value: 'off',
-              match: '  proxy_protocol        off;'
-            },
-            {
-              title: 'should set proxy_smtp_auth',
-              attr: 'proxy_smtp_auth',
-              value: 'off',
-              match: '  proxy_smtp_auth       off;'
-            },
             {
               title: 'should set auth_http',
               attr: 'auth_http',
@@ -712,6 +705,16 @@
                 expect(content).to include('listen                *:587 ssl;')
               end
             end
+
+            context 'mail proxy parameters' do
+              let(:pre_condition) { ['class { "nginx": nginx_version => "1.20.0"}'] }
+
+              it 'configures mail proxy settings' do
+                content = catalogue.resource('concat::fragment', "#{title}-ssl").send(:parameters)[:content]
+                expect(content).to include('proxy_protocol        off;')
+                expect(content).to include('proxy_smtp_auth       off;')
+              end
+            end
           end
         end
 

diff --git a/spec/defines/resource_server_spec.rb b/spec/defines/resource_server_spec.rb
@@ -672,7 +672,7 @@
                 facts[:nginx_version] ? facts.delete(:nginx_version) : facts
               end
 
-              it { is_expected.to contain_concat__fragment("#{title}-ssl-header").with_content(%r{  ssl on;}) }
+              it { is_expected.to contain_concat__fragment("#{title}-ssl-header").with_content(%r{listen       \*:443 ssl;}) }
             end
 
             context 'with fact nginx_version=1.14.1' do

diff --git a/templates/mailhost/mailhost_common.epp b/templates/mailhost/mailhost_common.epp
@@ -15,14 +15,19 @@
   Optional[String]  $smtp_auth,
   Optional[Array]   $smtp_capabilities,
   Enum['on', 'off'] $xclient,
+  String            $nginx_version,
 | -%>
   server_name           <%= $server_name.join(" ") %>;
 <%- if $protocol { -%>
   protocol              <%= $protocol %>;
 <%- } -%>
   xclient               <%= $xclient %>;
+<%- if versioncmp($nginx_version, '1.19.8') >= 0 { -%>
   proxy_protocol        <%= $proxy_protocol %>;
+<%- } -%>
+<%- if versioncmp($nginx_version, '1.19.4') >= 0 { -%>
   proxy_smtp_auth       <%= $proxy_smtp_auth %>;
+<%- } -%>
 <%- if $auth_http { -%>
   auth_http             <%= $auth_http %>;
 <%- } -%>