-
Notifications
You must be signed in to change notification settings - Fork 387
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
test: add SSL test #620
test: add SSL test #620
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 for this PR. It seems like tests using this setup are failing, is this related to the Travis environment?
nudge @jeblair |
36ccd34
to
721a6c9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #620 +/- ##
==========================================
+ Coverage 96.62% 96.65% +0.02%
==========================================
Files 27 27
Lines 3557 3557
==========================================
+ Hits 3437 3438 +1
+ Misses 120 119 -1 ☔ View full report in Codecov by Sentry. |
721a6c9
to
c705139
Compare
c705139
to
7009dfe
Compare
I rebased this PR but it is not working because ZK is expecting a keystore and a truststore (java stuff) instead of .pem files. Will try to get this working soon and add some "cert auto generation". |
7009dfe
to
4b7b154
Compare
cbd104a
to
b8754f5
Compare
@jeffwidman @ceache @jeblair this one is finalized, would love a review to get it merged. |
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.
Looks great 👍
This adds a simple SSL test along with the framework for running the test Zookeeper in a mode where it listens on both SSL and non-SSL ports.
We need pypy 7.3.10+ anyway to perform the SSL test because cryptography is not available before.
a05f8ed
b8754f5
to
a05f8ed
Compare
Thanks for updating this. ZK can use a pem file that is just a concatenation of the cert and key as its "keystore" without any special jks functionality. But since that seems to have bitrotted in this PR somehow, doing all the actual jks stuff is probably going to be more robust in the long run. :) |
Thank you again for the PR @jeblair and thank you for the updated information. I remember using a product that would "weirdly" accept this kind of .pem files as a "keystore" and you are certainly right that it was ZK. My comment about the reason why the tests were failing was probably wrong. |
This adds a simple SSL test along with the framework for running
the test Zookeeper in a mode where it listens on both SSL and
non-SSL ports.
This is based on earlier work in #619.