StephenSorriaux / ansible-kafka-admin

Manage your topic's configuration (partitions, replication factor, parameters), ACLs, quotas, users and get stats, without any effort with this library. It does not use the Kafka scripts and does not require ssh connection to the remote broker.
Apache License 2.0
150 stars 46 forks source link

kafka_users module implementation #134

Closed saiello closed 2 years ago

saiello commented 2 years ago

Fixes #109

Proposed Changes

Discussion point

saiello commented 2 years ago

@ryarnyah @StephenSorriaux I am still working on it, but if you have any suggestion to handle properly updates of existing users let me know.

ryarnyah commented 2 years ago

@ryarnyah @StephenSorriaux I am still working on it, but if you have any suggestion to handle properly updates of existing users let me know.

Maybe trying open a simple KafkaClient will do the trick?

saiello commented 2 years ago

@ryarnyah Any update about the review?

ryarnyah commented 2 years ago

@ryarnyah Any update about the review?

Sorry, I will review it next week, i don't have much time now

Edit: if you are adding a molecule scenario you need to add it to our github actions to be running. We might prefer using only default scenario to parallelize tests efficiently and to not duplicate test helpers, what do you think @StephenSorriaux ?

saiello commented 2 years ago

if you are adding a molecule scenario you need to add it to our github actions to be running.

Yes, but I don't want editing github actions before the codereview

We might prefer using only default scenario to parallelize tests efficiently and to not duplicate test helpers, what do you think @StephenSorriaux ?

I see the current guideline, however I found easier writing test in this way: I don't need to write additional python code and using the real role in the converge playbook covers not only the module_utils but the library as well. Moreover is also an additional example.

To avoid duplications, the infra tests and playbooks ( i.e. converge.yaml, verify.yaml ) might be moved in a common folder. see sharing across scenarios

Notice however that kafka_user module currently works only with kafka >= 2.7.0. We might extends the support using the AlterConfigs kafka api, maybe after the implementation of #87

ryarnyah commented 2 years ago

We are also check Python compatibility with workers in python 2 to 3.10 with https://github.com/StephenSorriaux/ansible-kafka-admin/blob/master/molecule/default/molecule.yml#L13 That's why we are targetting "executors" in the tests (example https://github.com/StephenSorriaux/ansible-kafka-admin/blob/master/molecule/default/tests/test_acl_default.py#L19) So you might need to write something to do the same (we have some users that use pyth:on 2.7 and python 3.6...).

saiello commented 2 years ago

Hi, @ryarnyah I actually did something testing workers in python 2 to 3.10. I changed the gitactions as you suggested, adding the scenario's name into the matrix.

In the last checks' run you can see the scram-kafka-270 as well.

ryarnyah commented 2 years ago

Can you add some testinfra test to ensure that user password is usable to kafka client?

saiello commented 2 years ago

@ryarnyah With the previous push flake8, that had no version specified in requirements.txt, was upgraded to 5.0.4 and several issues came up.

I resolved the issues and fix the version to avoid this behaviour in the future.

When do you plan the 0.17 release?