-
Notifications
You must be signed in to change notification settings - Fork 139
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
[ADDED] TLS: natsOptions_TLSHandshakeFirst() #780
Conversation
This is to force a client to perform the TLS handshake first, that is, not wait for the INFO protocol from the server. This is needed if the server is configured to require clients to perform the TLS handshake first (before sending the INFO protocol). Resolves #779 Signed-off-by: Ivan Kozlovic <[email protected]>
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.
LGTM
Thank you!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #780 +/- ##
==========================================
+ Coverage 68.71% 68.88% +0.16%
==========================================
Files 39 39
Lines 15207 15251 +44
Branches 3143 3157 +14
==========================================
+ Hits 10449 10505 +56
+ Misses 1700 1674 -26
- Partials 3058 3072 +14 ☔ View full report in Codecov by Sentry. |
@levb Looking at a flapper on Windows (not specific to this PR it seems). If I find a fix (assuming it is just test), will add a commit here. At any rate, will ping when ready for review. |
If the cluster was not fully established when the rest of the test was running, the library may get additional client URLs to reconnect to (gossip protocol). Looks like this was happening more on Windows CI than Linux. Using some options to clamp all that. Signed-off-by: Ivan Kozlovic <[email protected]>
@levb Ok, I added a fix for the test. Ready for review. |
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.
LGTM
@kozlovic It's failing the test against an older server, was this a 2.10 feature? |
Maybe. Let me see. If so, we just need to add the check for server version. Give me a minute. |
Yes, it is a 2.10.0 thing. I will post a quick PR to fix the test. |
This is to force a client to perform the TLS handshake first, that is, not wait for the INFO protocol from the server. This is needed if the server is configured to require clients to perform the TLS handshake first (before sending the INFO protocol).
Resolves #779
Signed-off-by: Ivan Kozlovic [email protected]