Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update templates/fcgi_site.erb #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mc0e
Copy link

@mc0e mc0e commented Oct 19, 2012

Change the determining factor for deciding to use SSL.

This allows a user to put more than just the port into the listen directive. eg:

nginx::fcgi::site { 'default-ssl':
listen => '443 default_server',
...
}

Later versions of nginx support this mode of configuration.

When using a wildcard SSL cert it is possible to have multiple domains on a single IP, so the 'default_server' switch is significant. If running multiple sites on different IPs, then the user would want to set listen => 'example.com:443' which is also handled better with this change.

Change the determining factor for deciding to use SSL.

This allows a user to put more than just the port into the listen directive.  eg:

   nginx::fcgi::site { 'default-ssl':
     listen          => '443 default_server ssl',
     ...
   }

Later versions of nginx support this mode of configuration.

When using a wildcard SSL cert it is possible to have multiple domains on a single IP, so the 'default_server' switch is significant.  If running multiple sites on different IPs, then the user would want to set listen => 'example.com:443' which is also handled better with this switch.
@BenoitCattie
Copy link
Owner

Hi,

can merge this request because 'ssl_certificate_key' could be empty if you want tje module to autogenerate the certs

@BenoitCattie
Copy link
Owner

Also changing value in $listen wont generate the cert if needed

Autogenerating ssl certs

if $listen == '443' [...]

I had to find an other way manging this scenario.

@mc0e
Copy link
Author

mc0e commented Nov 12, 2012

  1. Have a variable to explicitly state that the site is SSL, rather than overloading other variables with this assumption.
  2. Have an optional variable to add extra flags to the Listen directive, so that $listen contains only the port.
  3. Test for the presence of '443' as a substring of the $listen variable. It's possible to do this with regsubst(), though it's awkward.

Note that there are circumstances where people want to run HTTPS on a non standard port, which might be an argument for solution 1.

@mc0e
Copy link
Author

mc0e commented Nov 12, 2012

I wonder if autogenerating the ssl certs in the way you do it is ideal. It's pretty cool, but seems like it's as likely to be invoked by accident as on purpose. Would it be better to have the user set $ssl_certificate => 'snakeoil' where that's what's wanted?

@BenoitCattie
Copy link
Owner

I think the module listen variable must have the same content (or almost) as the one in the nginx config file with the possibility to set :

  • listen 192.168.1.1:443;
  • listen 443 ssl;
  • listen 443 default_server;
  • ....

I agree the way i do isnt ideal. Your proposal is I think a good alternative.

I also want the module to be able to manage HTTP/HTTPS vhost (http://nginx.org/en/docs/http/configuring_https_servers.html#single_http_https_server).

And have also news features to put into this module. (using parametrized classes rather than top-scope variables).

So may be i will work a bit on this module to manage all of those scenarios.

Benoit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants