Netflix / denominator

Portably control DNS clouds using java or bash
Apache License 2.0
580 stars 110 forks source link

DiscoveryDNS shouldn't have an opinion on how credentials are supplied #308

Closed codefromthecrypt closed 9 years ago

codefromthecrypt commented 9 years ago

Let's change the discoverydns credentials to accept what's actually needed. java.security.cert.Certificate and java.security.PrivateKey

The Credentials concept in denominator intentionally allows values to be objects. This is so that it doesn't have an opinion on how credentials which take forms besides string are supplied. DiscoveryDNS currently includes code to read Pems in the main source tree. We don't want to get into the business of maintaining Pem reading code if we can avoid it. This frees users to use bouncycastle or anything else to read the pems.

For the purposes of tests and the CLI, we'd probably still need to accept Pem format, just not in the base provider.

cc @MisterK

codefromthecrypt commented 9 years ago

cc @wnagele

MisterK commented 9 years ago

@adriancole So now, thanks to the 3rdparty folder, I can run the CLI using the DiscoveryDNS provider. Unfortunately, because of the changes you made in this issue, the DiscoveryDNSProvider expects an instance of a X509Certificate and a PrivateKey, which works well in the code. But the CLI seems to pass the credentials directly from the config file to the provider as Strings.

You mentioned above "For the purposes of tests and the CLI, we'd probably still need to accept Pem format, just not in the base provider.". How were you thinking of making that happen, without making the DiscoveryDNSProvider accepts both strings or objects? Is there another place where this could be done? Thanks in advance!

codefromthecrypt commented 9 years ago

I think the simplest way is to add a bouncycastle dep to the cli and mutate the credentials (if/)after they are loaded from yaml, but before they are used.

Peek at the credential for the START marker and then try to convert it using the pem reader.

I expect it won't be much code, and something we could paste into a credentials recipe wiki.

Want to give it a try?

Ps normally I am not a fan of deps, but the cli is not often used as a library. Bouncycastle is not terribly large and also doesn't have a lot of api drift.