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

SSL Cert/Key Template #126

Closed
wesleytodd opened this issue Sep 3, 2013 · 4 comments · Fixed by #623
Closed

SSL Cert/Key Template #126

wesleytodd opened this issue Sep 3, 2013 · 4 comments · Fixed by #623

Comments

@wesleytodd
Copy link

It looks like the template for ssl virtual hosts is not including the ssl cert and key correctly. I am a total noob, so sorry if this is wrong and a waste of your time, but I applied a change to templates/vhost/vhost_ssl_header.erb:10-11 and it appears to work for me now. The change I made looks like this:

ssl_certificate           <%= @ssl_cert ? @ssl_cert : scope.lookupvar('nginx::params::nx_conf_dir') + '/' + @name.gsub(' ', '_') + '.crt' %>;
ssl_certificate_key       <%= @ssl_key ? @ssl_key : scope.lookupvar('nginx::params::nx_conf_dir') + '/' + @name.gsub(' ', '_') + '.key' %>;

If you want I can submit a PR, but I didn't want to unless this was an actually worthy change. Let me know! Thanks for the good work.

@jfryman
Copy link
Contributor

jfryman commented Sep 6, 2013

I'm good with the idea, except for the scope.lookupvar. I know this is scattered throughout the templates, but I really want to pull this out and rely on setting these variables in the manifest itself and have it pass through. (MVC style).

What do you think?

@wesleytodd
Copy link
Author

I would much rather them be set in the manifest, but didn't want to disrupt what was currently working so I made it work for both. If you don't need support for that only way, then it is as simple as removing the ternary statement.

@jfryman
Copy link
Contributor

jfryman commented Sep 6, 2013

I think you can get away with it w/o calling directly in the template. As long as the default is the params entry, the same functionality should be there, and we clean up at the same time. 👍

@3flex 3flex mentioned this issue Apr 10, 2015
5 tasks
@3flex
Copy link
Contributor

3flex commented Apr 10, 2015

Hi, please add any relevant comments to #599 regarding how the module will treat SSL certificates going forward. My proposal should mean this wouldn't be an issue anymore.

@3flex 3flex mentioned this issue May 6, 2015
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 a pull request may close this issue.

3 participants