nomad-cli / houston

Apple Push Notifications; No Dirigible Required
http://nomad-cli.com
MIT License
2.93k stars 229 forks source link

Read stored certificate file, or use the data in the ENV var as fallback #102

Closed kreeger closed 8 years ago

kreeger commented 9 years ago

I really miss the ability to feed a certificate's .pem data into an ENV var, since this is (at least) how we've got it setup on a few of our Heroku instances, but this was taken away as of v2.2.3 (I think). This adds that functionality back, but also retains the new functionality of providing the file path (and makes that a first-attempt priority).

kaneda commented 9 years ago

Why not use the dotenv gem?

kreeger commented 9 years ago

If we used the dotenv gem, we'd still need to work with a path to a local file somewhere, right? And we would rather not have our cert file sitting as a file somewhere (in order to get read in), and that includes not having it checked into our git repo.

Unless I'm misunderstanding the purpose of dotenv? I haven't used it that extensively.

kaneda commented 9 years ago

@kreeger I guess I'm confused as to the intention: where do you want to store the PEM file?

kreeger commented 9 years ago

@kaneda We're storing the already-read-in contents of the PEM file in the ENV setting itself. So instead of APN_CERTIFICATE containing a file path, it contains the entire contents of the file.

kaneda commented 9 years ago

@kreeger gotcha, so dotenv probably won't help with this. You could submit a patch to https://github.com/nomad/houston/blob/master/lib/houston/client.rb#L30 that allows it to take in a File obj or IO obj instead then read from it.

kreeger commented 9 years ago

@kaneda that's what this PR is for! :D

kaneda commented 9 years ago

@kreeger see comments.

kreeger commented 9 years ago

@kaneda Pull request updated; there wasn't a prior README section for adding configuration details, so I added one.

kaneda commented 9 years ago

Looks good to me, you've got my :shipit:

courtsimas commented 9 years ago

:+1:

kattrali commented 8 years ago

Thanks, @kreeger!