ZOOKEEPER-3160: Custom User SSLContext

[arankin-irl]

The Zookeeper libraries currently allow you to set up your SSL Context via system properties such as "zookeeper.ssl.keyStore.location" in the X509Util. This covers most simple use cases, where users have software keystores on their harddrive.

There are, however, a few additional scenarios that this doesn't cover. Two possible ones would be:

1. The user has a hardware keystore, loaded in using PKCS11 or something similar.
2. The user has no access to the software keystore, but can retrieve an already-constructed SSLContext from their container.

For this, I would propose that the X509Util be extended to allow a user to set a property "zookeeper.ssl.client.context" to provide a class which supplies a custom SSL context. This gives a lot more flexibility to the ZK client, and allows the user to construct the SSLContext in whatever way they please (which also future proofs the implementation somewhat).

I added a few simple tests to this class around setting the SSLContext, and setting an invalid one. I'm not testing the actual functionality of the SSLContext, etc.

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2328/

[eolivelli]

Very useful.

Left a minor comment

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2329/

[eolivelli]

Thank you
Looks good to me

[nkalmar]

Nica addition, Thanks!

[arankin-irl]

Merged the latest 3.5 branch in - looks like there was a significant structure rework, along with some changes to the ZKConfig properties - moved the additional property in with the others in X509Util, and updated the tests, javadocs, etc.

Just curious - what would be the average timeline to get this merged in?

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2471/

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2470/

[arankin-irl]

Updated again to resolve merge conflicts.

[arankin-irl]

@eolivelli - looks like the Jenkins build is failing due to the audit warnings, but I get a 404 when attempting to view the audit findings file.

[nkalmar]

Unfortunately the link at the Jenkins build are wrong in some cases. Just navigate manually from build artifacts in the jenkins build.
findbugs for example:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2645/artifact/build/test/findbugs/newPatchFindbugsWarnings.html

Oh, you need release audit. Same thing though, need to navigate manually :(
But here it is:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2645/artifact/patchprocess/patchReleaseAuditProblems.txt

You are missing the apache license headers in ZKClientSSLContext.java and in the test.
Added review comment. It should fix the build.

Thanks!

[arankin-irl]

Thanks @nkalmar - found the audit problems file and resolved the missing license headers.

[arankin-irl]

Addressed merge conflicts.

@nkalmar - Just wondering if there's anything else I can do to help this get merged?

[nkalmar]

I just realized this is for 3.5. Did you create a PR for the master branch? 3.5 is pretty much closed for new features, in order to stabilize it for the stable release.

[arankin-irl]

Ah, I hadn't realised that 3.5 was closed for stabilisation. So I should recreate this PR for the master branch then?

[nkalmar]

Yes please. But it should be fairly easy, most of the time 3.5 and master are compatible. So just cherry-picking your commits on master should work, hopefully without merge conflicts.

If you need any help, let me know!

[arankin-irl]

That's grand then. I'll close this PR and reopen a new one for master then 👍