dont deploy "ssl on" on nginx 1.15 or newer (for mailhost) #1281
Conversation
There was a problem hiding this comment.
Hi @rhykw, thanks for fixing this! can you provide a short acceptance test so we know for sure that the generated config file works? We have some examples at https://github.com/voxpupuli/puppet-nginx/tree/master/spec/acceptance. You can join us on IRC at #voxpupuli on freenode or on slack at https://slack.puppet.com if you've any questions.
|
Yeah, give me a sec! |
|
Hi @rhykw. Can you also provide an acceptance test for this? |
bastelfreak
force-pushed
the
ssl-mailhosts
branch
from
30f696a to
ce53786
Compare
|
Hi. I verifid this with the docs and rebased the branch. I will merge it after travis turns green. Thanks for the contribution! |
| @@ -20,10 +20,10 @@ server { | |||
| <% end -%> | |||
| <%- if @listen_ip.is_a?(Array) then -%> | |||
| <%- @listen_ip.each do |ip| -%> | |||
| listen <%= ip %>:<%= @ssl_port %>; | |||
| listen <%= ip %>:<%= @ssl_port %><% unless @add_listen_directive -%> ssl<% end -%>; | |||
There was a problem hiding this comment.
This is broken I think (will be fixed as part of #1330).
The problem is @add_listen_directive is not in scope. Since it's always nil and nil is falsey, ssl is always added to the line.
| @@ -485,7 +485,7 @@ | |||
| title: 'should set the IPv4 SSL listen port', | |||
| attr: 'ssl_port', | |||
| value: 45, | |||
| match: ' listen *:45;' | |||
| match: ' listen *:45 ssl;' | |||
There was a problem hiding this comment.
By default, ssl should not have appeared as the nginx_version fact isn't being set and add_listen_directive is therefore true.
dont deploy "ssl on" on nginx 1.15 or newer (for mailhost)
Pull Request (PR) description
The
sslon syntax is deprecated since nginx 1.15.This PR fixes it for mailhost.
This Pull Request (PR) fixes the following issues
fixes #1284