pykafka icon indicating copy to clipboard operation
pykafka copied to clipboard

Feature SASL SCRAM support

Open swenzel opened this issue 5 years ago • 6 comments

This pull request adds support for modular SASL authentication mechanisms with two mechanisms (PLAIN, SCRAM) already implemented.

closes #651

swenzel avatar Oct 07 '19 09:10 swenzel

Codecov Report

Merging #972 into master will decrease coverage by 1.54%. The diff coverage is 86.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #972      +/-   ##
==========================================
- Coverage   83.53%   81.98%   -1.55%     
==========================================
  Files          36       37       +1     
  Lines        3807     4468     +661     
  Branches      562      682     +120     
==========================================
+ Hits         3180     3663     +483     
- Misses        483      565      +82     
- Partials      144      240      +96
Impacted Files Coverage Δ
pykafka/client.py 88.46% <ø> (ø) :arrow_up:
pykafka/exceptions.py 100% <100%> (ø) :arrow_up:
pykafka/cluster.py 70.9% <100%> (+1.2%) :arrow_up:
pykafka/rdkafka/simple_consumer.py 87.82% <100%> (-1.56%) :arrow_down:
pykafka/connection.py 89.18% <100%> (+6.02%) :arrow_up:
pykafka/rdkafka/producer.py 91.22% <100%> (-3.32%) :arrow_down:
pykafka/protocol/__init__.py 100% <100%> (ø) :arrow_up:
pykafka/broker.py 92.22% <71.42%> (-1.42%) :arrow_down:
pykafka/protocol/sasl.py 84.93% <84.93%> (ø)
pykafka/sasl_authenticators.py 85.03% <85.03%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e7665bf...73f8e8e. Read the comment docs.

codecov-io avatar Oct 10 '19 06:10 codecov-io

Ditched kafka 0.8 because there were too many issues getting it to run. It neither supports SSL nor SASL, the configs differ significantly and there seems to be an issue with its zk client. I thought it's more useful to test kafka 2 instead.

Switched to librdkafka 0.11.5 because 0.9.5 doesn't support the SCRAM mechanisms. In the end I didn't choose a 1.x version since it introduces another configuration issue that I didn't want to solve.

I went far out of scope fixing several issues with CI and the tests which took me a lot longer than actually implementing the feature.
I've really put a lot of work into it but now is the point where I say it's enough. I'm open to refactoring certain things or to revert a few changes if you don't want them. But I won't continue fixing CI or other tests that do not belong to the code that I changed. Especially if it's for keeping python 2 or kafka 0 compatibility.

swenzel avatar Oct 10 '19 10:10 swenzel

Could someone please just take a look at this PR? I think @swenzel has done some great work here which really pushes pykafka forward at should be a high priority to work on.

hfjn avatar Nov 19 '19 10:11 hfjn

Hi, I would also like to see this merged! :D

garrettthomaskth avatar Mar 11 '20 14:03 garrettthomaskth

Any news?

Neustradamus avatar Mar 27 '20 01:03 Neustradamus

Look forward to this feature.

DoZX avatar Aug 21 '20 06:08 DoZX