Add ACL api to KafkaAdminClient

[ulrikjohansson]

This is a new attempt at creating user friendly ACL management methods to KafkaAdminClient.

It's building on top of the previous PR (#1646) that only got the protocol stuff merged b.c there were remaining issues to deal with in the api part, and not much time. I think I've covered all the feedback from that PR in this one.

I'm ready for a new round of review, hopefully I can find enough time for fixes and tweaks to land the whole thing this time =)

Fixes #1638

---

This change is 

[jeffwidman]

Thank you! I don't have time today to review, hopefully tomorrow. But I did restart all the travis flakes--looks like for some reason a bunch of them failed to download the Kafka/Zookeeper binaries used by the integration tests

[ulrikjohansson]

It looks like the downloads failed again on a few of the builds, would you be able to restart them again?

[mattdonders]

Is there any update on this PR? This is something that would help me out a great deal and glad to see it has been developed and hopefully passes the tests to be merged.

[jeffwidman]

Thanks for the nudge, I'll try to review it later today. I just restarted the travis tests that had failed to download things...

[mattdonders]

I am not a Travis expert, but it looks like 1 test fails in the pypy2.7-6 version.

Python: pypy2.7-6.0
KAFKA_VERSION=0.10.2.1

======= 1 failed, 517 passed, 35 skipped, 111 warnings in 440.99 seconds =======

Just wanted to give another nudge - thanks again for your hard work on this!

[ulrikjohansson]

Just a friendly nudge to check if you would have time to review this PR.
I'm currently on vacation and will have plenty of time during the next few weeks to address any issues.

The last test run seems to have a failed test b.c the test kafka cluster not starting properly.

More tests is actually something that might need to get fixed for this PR to land I guess. I'll have a look at that in a few days.

[ulrikjohansson]

Ping @jeffwidman. I'm sorry to bother you, but would you have time to have a look though this PR?

I've now also added a basic integration test for the KafkaAdminClient ACL handling methods, so at least there is now 1 basic check that it all works. And everything is finally green in travis as well now =)

[karthikkgc]

Is there any update on this PR? Waiting for more than 4 months :)

[ulrikjohansson]

For anyone impatient to try this out, it's possible to install this specific commit of kafka-python using

pip install git+https://github.com/ulrikjohansson/kafka-python@23c7123

I realise this might not be an option if you need this for a production environment, but for testing it should work.

Also, remember the maintainers are probably doing this for free on their spare time. I'm providing this PR with no expectation that they have to accept it, nor any expectation that they follow up within a certain time. There are any number of reasons for the wait, all of them valid. I'd be happy if it got landed, but that's as far as I'd go. They're the one's having to maintain this after the merge, not me. So, please be patient =)

[dpkp]

Hi all -- I reviewed and this looks great. Thanks so much for working on this and submitting the PR. And thanks for the patience!

I'm working to put together a new release, 1.4.7, and I'm going to merge this for inclusion.

[dpkp]

I have been unable to get the integration tests to pass on travis and have disabled them manually in test/test_admin_integration.py . I'm going to leave the code changes merged. But I would love it if you could take a look at the test failures and perhaps submit another PR to get them working!

Brokers are returning this error:

E           kafka.errors.SecurityDisabledError: [Error 54] SecurityDisabledError: Request 'DescribeAclsRequest_v0(resource_type=<ResourceType.TOPIC: 2>, resource_name='topic', principal=None, host='*', operation=<ACLOperation.ANY: 1>, permission_type=<ACLPermissionType.ANY: 1>)' failed with response 'DescribeAclsResponse_v0(throttle_time_ms=0, error_code=54, error_message='No Authorizer is configured on the broker', resources=[])'

See for example https://travis-ci.org/dpkp/kafka-python/jobs/590957368

[ulrikjohansson]

I recognize that error. It's because a test run is being made with a kafka config not setting an authorizer. Like this: https://github.com/dpkp/kafka-python/pull/1833/files#diff-204dbafdfb1e1695769a930f103f98ddR33-R34

I either missed something in my test config, or new versions of kafka is being tested against in master that I did not include the authorizer config for, or that I don't explicitly skip in the test.
In the failed build it says KAFKA_VERSION=1.1.1 though, and I've included a config change for that version.
I'll try and have a closer look at this sometime this week.

Edit:
I can see that 2 new versions tested against have been added to master since this branch was created: 0.10.2.2 and 0.11.0.3. They would fail this integration test with no authorizer class set, so that is probably what's happening.

[jeffwidman]

@ulrikjohansson when you fix that, do you mind making sure to fix in a way that's compatible with #1193? In particular, that PR removes all unittest dependencies in favor of pytest fixtures... I've already merged the new fixtures to master, so should be straightforward to switch to those... if you need help let me know and I can point to code examples of how I did it.

You can see how the tests added here are now breaking #1193 here: https://travis-ci.org/dpkp/kafka-python/builds/593248509

[ulrikjohansson]

@dpkp It looks like the missing kafka server config settings have been added into master already, along with a few other fixups.
I've tried running the test suite with the test_admin_integration enabled again, running KAFKA_VERSION=0.11.0.3 tox -e py36, and it passes for me locally.
Do you have a travis build that fails after adding the kafka config (and with the admin integration test enabled?)
@jeffwidman Looks like we've been making trouble for each other in parallel with these 2 PRs =) I'll see if I can get the tests converted this weekend so I'm not a blocker for your PR. How urgent is it?

Update: nevermind, unless I'm missing something, it was a pretty straightforward fix. It looks like the test works fine on all the travis kafka versions now as well.