Add ability to set ssl-settings globally - fixes #670 #1382
Conversation
nginx::config is a classThe enclosing module is declared in 11 of 578 indexed public Puppetfiles. Breaking changes to this file MAY impact these modules (near match):
The enclosing module is declared in 11 of 578 indexed public Puppetfiles. Breaking changes to this file WILL impact these modules (exact match):
Breaking changes to this file MAY impact these modules (near match):
The enclosing module is declared in 11 of 578 indexed public Puppetfiles. Breaking changes to this file WILL impact these modules (exact match):
Breaking changes to this file MAY impact these modules (near match): These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report. Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only. |
| @@ -171,32 +171,32 @@ | |||
| Optional[Variant[String, Boolean]] $ssl_cert = undef, | |||
| Optional[String] $ssl_client_cert = undef, | |||
| String $ssl_verify_client = 'on', | |||
| Optional[String] $ssl_dhparam = $nginx::ssl_dhparam, | |||
| Optional[String] $ssl_dhparam = undef, | |||
There was a problem hiding this comment.
Is this change ok, as if $nginx::ssl_dhparam is defined it will now get written to nginx.conf or is this considered a backward-incompatible change even if nginx should be working as before this change?
There was a problem hiding this comment.
I do not think this change is backwards-incompatible, because the value was undefbefore and can be overwritten any time. However, you are using a different data type in init.pp and server.pp.
Unfortunately I can not have a look at the failing Travis CI tests, because the website is under maintenance.
There was a problem hiding this comment.
I don't mind a breaking change for this since it makes sense IMHO.
There was a problem hiding this comment.
@dhoppe i made the settings in init.pp more strict as these are new parameters. I didn't want to break anything with my changes if somebody is already using nginx::resource::server.
There are also $ssl_stapling and $ssl_stapling_verify which are Enum['on', 'off'] on init.pp and Boolean in server.pp
Unifying them will be definitely a breaking-change and should probably be done in a separate ticket?
If there is any new data type to strict just tell me and i will change it :) I would like to see this change as soon as possible released as we would like to move some ssl-settings vom vhost/servers to the global config in our infrastructure.
|
A test worked timed out, restarted the build now. |
|
Sadly it timed out again. Did i introduce to many new test cases? But wouldn't explain why puppet 5.x ran only like 30 mins and on puppet 6.x it takes more than 60 minutes. |
|
Dear @TuningYourCode, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? |
|
@TuningYourCode I rebased it and it should be fine now |
…s_to_nginx_conf Add ability to set ssl-settings globally - fixes voxpupuli#670
Pull Request (PR) description
This PR enables to set ssl_* settings globally in nginx.conf to prevent setting them on all vhosts/servers manually.
This is also useful if you do not maintain all nginx vhosts/servers by puppet but still be able to have the same ssl_* settings (like ssl_ciphers, ssl_protocols etc.) on all of them.
This Pull Request (PR) fixes the following issues
Fixes #670