intercity / chef-repo

Set up your server to host Ruby on Rails apps. - Follow us on Twitter: @intercityup
MIT License
416 stars 82 forks source link

Check if enable_ssl is set, otherwise check for certificate #180

Closed jvanbaarsen closed 9 years ago

jvanbaarsen commented 9 years ago

When I send over the SSL information in IC for the first time, chef doesn't see the certificate.crt file. In order to make SSL work directly after applying it for the first time we also need to check if the user is sending over the enable_ssl flag. If that is true we need to enable ssl, otherwise check if the certificate.crt file is present.

jvanbaarsen commented 9 years ago

/cc @michiels / @joshuajansen can you please review?

michiels commented 9 years ago

I believe there will be a problem with nginx. If you enable SSL, and per that option update the nginx template. Nginx will crash on next restart because it will not find the certificate file. The certificate file must really already be present on the server, before changing the nginx config.

jvanbaarsen commented 9 years ago

@michiels Yeah but the file is present when its given. But somehow chef doesn;t think it is (its created a couple files higher). I tested this on the vagrant box and with this fix it worked. Otherwise I had to apply the changes twice.

michiels commented 9 years ago

Ahh ok. Maybe the File.exists? thing is only executed on cookbook compilation, and not execution. Maybe there's a way to run the "File.exists?" on-cookbook-run? Not sure if Chef supports that.

If not: This fix is good I think, but people really need to realize that the file should already in place, otherwise it is going to break their sites.

jvanbaarsen commented 9 years ago

@michiels Yeah this is mostly for IC users. And they don't have to bother with Chef anyway. Even if we check on Cookbook run it won do any good. Since the file wont be present when the cookbook is run. It gets generated in this cookbook, and a couple of lines down it is checked it it exists. So that won't solve our problems in this case.

michiels commented 9 years ago

If the file is created a little lines up. Then when checking this line after that should result into True right? Probably some Chef documentation for this.

Yep! So make sure that all the chef-repo users will net set this flag to true, when they haven't uploaded their SSLs yet :)

michiels commented 9 years ago

Cool. Yeah so if we manually pass in ssl certificate with the attributes, it will now and automatically enable SSL. Otherwise, it will check for file existence.

Looks good! Maybe update the variables to be a bit more descriptive, instead of just ssl

:shipit:

jvanbaarsen commented 9 years ago

@michiels Will do! Thanks for the review :)