Attempt to parse Certificate and PrivateKey in CLI credentials #365
Conversation
|
NetflixOSS » denominator » denominator-pull-requests #130 SUCCESS |
|
I like what you've done. I'm going to make a few style notes so that the code reads easier with the rest. |
|
Thanks. |
| @@ -309,6 +323,43 @@ public void run() { | |||
| } | |||
| } | |||
|
|
|||
| private Function<Object, Object> attemptToTransformCredentials = new Function<Object, Object>() { | |||
There was a problem hiding this comment.
make things here static unless you are referencing an instance variable. also make the function final. I like the intent of "attemptToTransformCredentials" except that it doesn't say much about what it is doing. perhaps con
perhaps decodeAnyPems() as that implies that a pem may or may not be present.
|
ok done the review. please apply my style suggestions to all code, not just where I commented (particularly thinking about the "final" on locals). Once that's in and build passes, we are likely GTG! |
|
sounds familiar..
might need code like this somewhere?
https://github.com/Netflix/denominator/blob/master/example-daemon/build.gradle#L41
|
|
NetflixOSS » denominator » denominator-pull-requests #131 SUCCESS |
|
add to changelog for 4.6 and squash into a single commit. then, good to go! |
Before, finding source compatibility issues relied on building with an old JDK. This uses animal-sniffer to enforce java language level 6 for libraries and 7 for executables. closes Netflix#344
Before this change, `Zone.id()` implied that the provider supported multiple zones with the same name. This turned out to be false, as for example both CloudDNS and Designate use ids, but enforce zone names must be unique. This change introduces a series of changes to support both determining how zones are identified, as well if the provider supports zones that have the same name. One goal was to not add complexity. The overall approach to this is to only add a concept while removing one. `Zone.qualifier()` is added to disambiguate zones with the same name. This is paid for by deprecating `Zone.idOrName()`. `Provider.zoneIdentification()` is added to that callers can understand if zone names can be substituted for ids or if qualifiers are supported. This is paid for by deprecating `Provider.supportsDuplicateZoneNames()`.
`Zone.Identifier` doesn't pull its weight. The only thing it adds beyond `Provider.supportsDuplicateZones`, is that it can indicate a zone id is the same as its name. This isn't enough to warrant the added complexity. see Netflix#350
Zone.qualifier was only introduced to support creation of Route53 hosted zones. We were using this for the caller reference. Turns out that caller references can never be reused. This means that if you delete a zone, you can never create a new one with the same caller reference. As such, we shouldn't expose this to users. see Netflix#350
Reverts default zone ttl as it is not supportable across all providers. See Netflix#357
Zone.ttl is the SOA ttl. This is not always the default for new records. See Netflix#357
Adds Zone write support and backfills functionality for all supported providers. Adds usage notes to README files. closes Netflix#264
MisterK
force-pushed
the
parseCertInCLICredentials
branch
from
93b836a to
6e4a52b
Compare
|
NetflixOSS » denominator » denominator-pull-requests #132 FAILURE |
|
Mmm looks like I didn't rebase properly. I'm going to submit another proper pull request instead, from a different branch. Sorry about this... |
As mentioned in #308, peak at the CLI credentials for the START marker and then attempt to convert them to a X509Certificate or a PrivateKey, as required by DiscoveryDNS.