Added SSL certificates uploading. #164
Conversation
| # ssl_certificate(applications_root, name, app_info) # => /u/apps/my_app/shared/config/my_app.crt' | ||
| # | ||
| def ssl_certificate(applications_root, name, app_info) | ||
| Pathname.new(applications_root).join(name, 'shared', 'config', app_info["ssl_certificate"] || "#{name}.crt") |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| "key": "<ssl key>", | ||
| "crt": "<ssl crt>" | ||
| }, | ||
| "ssl_enabled": true, |
There was a problem hiding this comment.
What do you think about adding this information back into a key value hash like this:
"ssl_info" : {
"enabled": true
"certificate": "The cert file",
"certificate_key": "The key for the cert file"
}
| # Check if the app config has ssl_info section | ||
| # | ||
| def ssl_info?(app_info) | ||
| app_info.key?('ssl_info') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| @@ -33,6 +33,7 @@ | |||
|
|
|||
| # Include library helpers | |||
| ::Chef::Resource.send(:include, Rails::Helpers) | |||
| ::Chef::Recipe.send(:include, Rails::Helpers) | |||
There was a problem hiding this comment.
Just for my curiosity: what does this do?
There was a problem hiding this comment.
without this line, the following wouldn't work, method missing:
98: ssl_certificate_path = ssl_certificate(applications_root, app, app_info)
since it's a level of recipe, not resource such as:
cookbook_file pathname.to_s do
ssl_certificate_path = ssl_certificate(applications_root, app, app_info)
end
|
👍 We might need to deal with upgrades though. We don't want people who upgrade and have a config with the contents of the ssl-files (as was in the old situation) to suddenly break all their SSL on production (which won't really happen now, because you already raise an exception if the file is not found) Either we make the option a little smarter and have it detect if it is a path to a file or the content of a cert. Or we simply document and communicate this very well (as you already do with the |
|
What about we first go in a deprecation mode of the old way? So for this release we support both, and throw a big fat warning if people use the older version. And in the next release we remove the old way all together? |
ghost
added this to the
|
I'd still really love this feature to be in. The hardest part is to make it backwards compatible though. @jvanbaarsen @michiels how about a 3.x version that contains all such larger changes. |
|
@berkes I have to take a good look at this PR, and also see how we can incorporate this in https://github.com/intercity/intercity (The main product we created this for). I think I can free some time for this next friday (I've put it on my todo list for that day) |
As per #139 I removed the old approach of adding certificates as strings to node JSON file. Cookbook files are used instead.