Fixed long names virtual hosts... #233
Conversation
…e1.domainname.org name2.domainname.org': } Add example nginx::vhost with HTTPS FastCGI and redirection of HTTP Add location_custom_cfg_prepend & location_custom_cfg_append. Used for logical structures such as if. Fix puppet-lint
|
Nice! Can I trouble you for some tests as well? |
|
Оf course. |
| target => $config_file, | ||
| content => $content_real, | ||
| order => "${ssl_priority}", | ||
| order => $ssl_priority, |
There was a problem hiding this comment.
concat::fragment expects a string, so this should be left as is (even though puppet-lint complains)
|
|
||
| ### Example puppet class calling nginx::vhost with HTTPS FastCGI and redirection of HTTP | ||
|
|
||
| ```define web::nginx_ssl_with_redirect ( |
There was a problem hiding this comment.
Did you preview this new section in the README? It doesn't render properly. The
```
must be on a line of its own, and it should read
```puppet
to get proper syntax highlighting.
|
I'm not convinced adding Also, I'm not sure this example in the README really belongs - it's very specific to a single use case and makes too many assumptions about the environment it would work in (e.g. SSL is turned on, the certificate and key paths are hard coded, fastcgi is hard coded to listen on 127.0.0.1). I think it would be better to explain how to fit the components together to get this end result instead of just putting the code there when it's not going to work as is for the majority of people. 👍 to sanitizing the vhost name in the location though. Would be great to get rspec tests for this, I know sanitizing vhost isn't tested at all right now. |
|
@3flex, thanks for your code review and feedback.
How could we extend location_cfg_* variables? We use location_cfg_* for simple Nginx options: But as for logical structures, we transmit the raw data from user to template without the symbol ';' etc. Same as location_custom_cfg_prepend => [
'if (-f $document_root$fastcgi_script_name){
set $fsn $fastcgi_script_name;
}',
],I think templating these structures will reduce transparency.
Well. My example is not ideal. However, php-fpm/nginx with FastCGI and HTTP -> HTTPS forwarding are applied ubiquitously. And it is not easy to implement that using the module. SSL certificate must be deployed on the server and determine the class, maybe you suggest an alternative to do this? It's just a simple example.
It is really better, but it's not simple for all cases. When this is done, my example can be removed.
I do not have enough experience on rspec tests creating. Could you help me? |
|
They're all fair points, though I still think the README example could be a bit more concise, and there must be a better way than adding more parameters - there will be four parameters for adding custom configuration to a location. I can't think of a good solution though. I raised a PR on your branch with tests. rspec can be tricky if you're new to it, it took me a long time to write the tests for this module and understand how everything fit together. |
test vhost name is sanitized
|
@3flex, thank you! |
|
@jfryman ? |
Fixed long names virtual hosts...
|
👍 |
|
molte grazie! |
Fixed long names virtual hosts, such as nginx::resource::vhost { 'name1.domainname.org name2.domainname.org': }
Add example nginx::vhost with HTTPS FastCGI and redirection of HTTP
Add location_custom_cfg_prepend & location_custom_cfg_append. Used for logical structures such as if.
Example:
location_custom_cfg_prepend => [
'if (-f $document_root$fastcgi_script_name){
set $fsn $fastcgi_script_name;
}',
],
Fix puppet-lint