Add puppetdb::ssl using puppetlabs/java_ks #55
Conversation
| I love the concept of what you're trying to do here @raphink its very interesting. However, won't we have two competing SSL management solutons over-writing each other here? puppetdb-ssl-setup being one of them, its set to run on package upgrade and install afaik. Have you tested the behaviour when that script fires? |
manifests/params.pp Outdated
There was a problem hiding this comment.
$settings::ssldir might work? Although this is server-side. This issue is not everyone has there certs in the ssl dir of the configuration path. Some people have them in /var/lib/puppet/ssl.
| @raphink yeah after reviewing I've noticed a couple of static pathing issues, nothing too major. My main concern is still how this interacts with puppetdb-ssl-setup, if you have a proposal let me know. |
| I've actually used that code in production for a few months, but tbh, I haven't resynced with |
| puppetdb-ssl-setup is not in the module, right? |
| @raphink no puppetdb-ssl-setup is in the package. |
| Ah yes, I remember this issue now. I had to remove the certificates generated by the package indeed. So I'd have to find a way to actually remove the certificates properly before replacing them, cause I'd really rather not use the certificates generated by the package. |
| Had a good chat to @raphink about this on IRC, we think we might be able to find a compatible way to work with puppetdb-ssl-setup that is less destructive and perhaps uses any existing java_ks store that is around instead of recreating it. Its an idea anyway :-). |
| Yes, so I will prepare a patch for java_ks to support On Thu, May 2, 2013 at 4:18 PM, Ken Barber notifications@github.com wrote:
|
| Can one of the admins verify this patch? |
1 similar comment
| Can one of the admins verify this patch? |
| puppetlabs/puppetlabs-java_ks#20 is now merged. I'll base myself on it, and if anyone could make a release of it, I could add a dependency on the new version. |
| ok to test |
| Merged build triggered. (Status: PENDING, Details: null) |
| Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/59/) |
| Merged build finished. (Status: FAILURE, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/59/) |
| Merged build triggered. (Status: PENDING, Details: null) |
| Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/60/) |
| Merged build finished. (Status: FAILURE, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/60/) |
| Merged build triggered. (Status: PENDING, Details: null) |
| Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/61/) |
| Merged build finished. (Status: SUCCESS, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/61/) |
| Can one of the admins verify this patch? |
| Any news on this? |
| Hi @raphink ... so we're still debating at the moment about this patch. We have an alternative approach and fix in the application that we are going to consider (probably for the next weekly sprint? - or at the very least soon as SSL issues are getting annoying) around support PEM files natively and building up a JKS in memory which might make this patch unnecessary. When we get around to looking at that work, I'll review this again. Sorry about the delay, the code is good we're just undecided about what the final solution should be. If you have any thoughts, I'd love to hear them. |
| Alright, sounds good. I'll be waiting for that. |
| I'm almost done - it will probably go up for review today, you can see the work here: https://github.com/kbarber/puppetdb/tree/feature/master/no-more-keystore |
| Alrighty, we've merged in the PEM file change which should simplify things going forward. Its not released, but will be in the next 1.x series most probably. I think this is a much simpler solution now, and obviates the need for this patch (although legacy might want it, I would instead ask people to upgrade, since its backwards compatible). I'm going to close this patch, its an awesome patch and a great idea, but simplifying upstream seemed like a better tactic for everyone. Thanks for the patch @raphink. |
3 participants
This PR adds a
puppetdb::sslclass which provides a simple way to configure SSL certificates for the PuppetDB using thepuppetlabs/java_ksmodule.