-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat: support SaslHandshakeRequest v0 for SCRAM #1992
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, the fix looks good to me
@zhaomoran please can you take a look at the test failures? 2021-08-09T13:35:20.5596060Z --- FAIL: TestSASLSCRAMSHAXXX (0.01s) |
@dnwe I'm sorry,I ignored the test! |
@zhaomoran Looks like CI is still failing. Do you think you could take a look at it? |
It looked like it just needed |
@zhaomoran: #1944 has been solved or not? |
This PR changed the default behavior of |
@duedal conf.Net.SASL.Version had been defaulted to V0 for a while (if I remember correctly, due to a bug in Azure) — what changed here is that SCRAM had previously only implemented V1 handshaking and this PR added the missing capability and as a side effect conf.Net.SASL.Version is now being adhered to for SCRAM as well. When you say "breaking clients on sarama upgrades" — can you elaborate? Any Apache Kafka cluster that supports/supported SCRAM using SASLHandshakeV1 should also support it via the legacy SASLHandshakeV0 method? |
Fix #1918 ,Maybe other scram issues are also relevant!Thanks!