New Parameters: ssl_timeout / nginx_vhosts_defaults

[fnerdwq]

Added new Parameters:

• $ssl_timeout (nginx: ssl_session_timeout) on nginx::resource::vhost define
• $nginx_vhosts_defaults on nginx class: This allows to set defaults for all created vhosts through the create_resource function call. This reduces the hiera yaml code immensely, when creating large lists of vhosts.

Analogous Parameter to $nginx_vhosts_defaults could/should be introduced for ...mailhosts, ...locations, ...

[3flex]

Could you name the new SSL parameter ssl_session_timeout to match the nginx directive name?

[fnerdwq]

I chose the name analogue to the existing ssl_cache parameter, which maps to the nginx parameter ssl_session_cacche

If you like, I rename the parameter.

[3flex]

Yeah, some parameters are named directly after the nginx directive, some aren't. But it's less confusing for users if the parameter matches the nginx directive name so it would be good to rename.

Thanks

[fnerdwq]

I agree, pull request updated.

So actually, e.g. ssl_cache should be changed to ssl_session_cache. It could be introduced additionally and the old one could be deprecated...

[fnerdwq]

Merged master for 'automerge'...

[fnerdwq]

Hi 3flex,
are you going to merge this patch?

Thanks...

[3flex]

Hi @fnerdwq, my apologies, there is other work going on in PR 423 and I didn't want to add anything that might affect that work. However this is unlikely to affect things so I'm happy to merge.

Can I just ask for two things first - please rebase, and have one commit for the new ssl parameter, and a second for the vhost_defaults parameter. Second, if possible, please add a test to that second commit to confirm that the listen_options parameter you added to the spec test is actually being inherited from the underlying vhost resource, e.g.

it { is_expected.to contain_nginx__resource__vhost("test2.local").with_listen_options('default_server') }

Thanks!

[fnerdwq]

Hi @3flex,
in #444 I rebased and created two singe commits. The test for the nginx_vhosts_defaults was already include. That's what you wanted, I suppose.