-
Notifications
You must be signed in to change notification settings - Fork 17
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
Cfssl backend #54
base: main
Are you sure you want to change the base?
Cfssl backend #54
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot. Looks interesting!
I have some small comments inline.
Though, I would love to have tests and I guess the only thing we need to mock is the Open3 call, so should be pretty easy. But tests would show that a) it works in general and be explain some of the logic you have their and thus would prevent regressions.
lib/trocla/formats/cfssl.rb
Outdated
certdata['not_before'] = cert.not_before | ||
certdata['not_after'] = cert.not_after | ||
return certdata | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation is wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is "right" one ? I just let IDEA auto-indent it
lib/trocla/formats/cfssl.rb
Outdated
require 'open3' | ||
class Trocla::Formats::Cfssl < Trocla::Formats::Base | ||
def format(plain_password,options={}) | ||
#n o dig method on jruby 1.9 used by puppet ;/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this comment, should it read no dig...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've used dig in previous iteration then discovered JRuby version used by puppet doesn't have it yet. Ill push a change with that fixed and requires moved inside the class. I let the comment there so nobody steps in same landmine as me
|
||
formats: | ||
cfssl: | ||
server_url: http://localhost:8888 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm should we define that at all? Or is thi the default place where your cfssl daemon might be running?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default config of the daemon is localhost on port 8888:
cfssl serve --help
...
-address=127.0.0.1: Address to bind
-port=8888: Port to bind
...
so I used that as an example
Is there an example in code on how I could do that? Ruby is not exactly something I write often in (and I don't think I ever even wrote a test in its test frameworks) so I do not even know where to start with mocking. Inputs and outputs for cfssl are pretty easy to check/emulate, but honestly most of the complexisty with the setup is "set cfssl correctly in the first place" (this is why I attached full example config of it) and after that it is just "feed json, parse json" |
Adds Cloudflare's PKI toolkit support via its remote CA API. Basically feeds cfssl CLI tool with required options to generate key, csr, feed that to remote CA server, and put resulting data back into trocla, adding from-to dates extracted from the cert along the way.