Skip to content

Cfssl backend#54

Open
XANi wants to merge 7 commits into
duritong:mainfrom
efigence:cfssl-backend
Open

Cfssl backend#54
XANi wants to merge 7 commits into
duritong:mainfrom
efigence:cfssl-backend

Conversation

@XANi
Copy link
Copy Markdown

@XANi XANi commented Mar 26, 2019

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.

Copy link
Copy Markdown
Owner

@duritong duritong left a 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.

Comment thread lib/trocla/formats/cfssl.rb Outdated
certdata['not_before'] = cert.not_before
certdata['not_after'] = cert.not_after
return certdata
end
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation is wrong

Copy link
Copy Markdown
Author

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

Comment thread 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 ;/
Copy link
Copy Markdown
Owner

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...?

Copy link
Copy Markdown
Author

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
Copy link
Copy Markdown
Owner

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?

Copy link
Copy Markdown
Author

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

@XANi
Copy link
Copy Markdown
Author

XANi commented Mar 28, 2019

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.

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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants