Skip to content

Commit

Permalink
raise default version to 1.16.0
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
Christoph Maser committed Jun 3, 2024
1 parent bbfff0a commit dfe9614
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 14 deletions.
1 change: 1 addition & 0 deletions .rspec
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

--format documentation
--color
--fail-fast
6 changes: 3 additions & 3 deletions REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -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'`

Expand Down
2 changes: 1 addition & 1 deletion manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions manifests/resource/mailhost.pp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -257,6 +257,7 @@
smtp_auth => $smtp_auth,
smtp_capabilities => $smtp_capabilities,
xclient => $xclient,
nginx_version => $nginx::nginx_version,
})

concat { $config_file:
Expand Down
2 changes: 1 addition & 1 deletion spec/acceptance/nginx_mail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' {
Expand Down
7 changes: 1 addition & 6 deletions spec/classes/nginx_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion spec/defines/resource_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions templates/mailhost/mailhost_common.epp
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>;
<%- } -%>
Expand Down

0 comments on commit dfe9614

Please sign in to comment.