kafka-rest
kafka-rest copied to clipboard
Add feature to request consumer group information
Add feature to request consumer group information(start, end offsets, lags, consumer-instance-id, consumer-instance-ip) Added rest paths:
- /groups - returns consumer groups list
[{"groupId":"perf-consumer-46894","coordinator":{"host":"{some_host_name}","port":9496}}]
- /groups/{groupId}/topics - returns topics for wich group is subscribed
[{"name":"1"}]
- /groups/{groupId}/topics/{topic} - returns consumer group information(start, end offsets, lags, consumer-instance-id, consumer-instance-ip...) restricted by spesified topic
{"topicPartitionList":[{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":0,"currentOffset":15338734,"lag":113812,"endOffset":15452546},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":1,"currentOffset":15753823,"lag":136160,"endOffset":15889983},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":2,"currentOffset":15649419,"lag":133052,"endOffset":15782471}],"topicPartitionCount":3,"coordinator":{"host":"{coordinator_host_name}","port":9496}}
- /groups/{groupId}/partitions - returns consumer group information(start, end offsets, lags, consumer-instance-id, consumer-instance-ip...) for all topics for wich group is subscribed
{"topicPartitionList":[{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":0,"currentOffset":15338734,"lag":113812,"endOffset":15452546},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":1,"currentOffset":15753823,"lag":136160,"endOffset":15889983},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":2,"currentOffset":15649419,"lag":133052,"endOffset":15782471}],"topicPartitionCount":3,"coordinator":{"host":"{coordinator_host_name}","port":9496}}
Add unit and integration tests
It looks like @Ledostuff hasn't signed our Contributor License Agreement, yet.
The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia
You can read and sign our full Contributor License Agreement here.
Once you've signed reply with [clabot:check] to prove it.
Appreciation of efforts,
clabot
[clabot:check]
@confluentinc It looks like @Ledostuff just signed our Contributor License Agreement. :+1:
Always at your service,
clabot
@Ledostuff PR build is failing. Could you check?
The build failed:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.1:testCompile (default-testCompile) on project kafka-rest: Compilation failure
[ERROR] /home/jenkins/workspace/entinc-pr_kafka-rest_PR-435-42EK4ZKFX7WUDVKYQNLWCJBUA3J5IBRM4CNHID3FE32PTTTFMBRQ/kafka-rest/src/test/java/io/confluent/kafkarest/TestUtils.java:[263,34] cannot find symbol
[ERROR] symbol: method createConsumerProperties(java.lang.String,java.lang.String,java.lang.String,int)
[ERROR] location: class kafka.utils.TestUtils
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <goals> -rf :kafka-rest
This PR shouldn't need to depend on ZooKeeper. We should be able to use the public AdminClient.
What's the security model here? How do we decide who can and cannot access admin APIs?
@gAmUssA Thank you for review. I`ll fix build and push changes asap.
@cmccabe Thank you for review. I`ll fix build and push changes asap. Zookeeper needs to get broker information. I mean security protocol in EndPoint class. Do you mean I should move method from AdminClientWrapper to another place? Could you explain in detail your sentence about security model?
Zookeeper needs to get broker information. I mean security protocol in EndPoint class.
Why is this information needed for REST proxy? If it is needed, then we should at least think about providing it through the AdminClient.
Could you explain in detail your sentence about security model?
This PR seems to give rest-proxy users a lot of powerful abilities to access administrative APIs. I'm not sure that rest-proxy is the right place to manage the Kafka cluster.
If we did want to do this we would need to figure out how to give certain users the ability to use the admin APIs and block everyone else. For example, users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource.
To be honest, I'm not sure this is the right direction to go-- at least not without a lot more discussion.
@cmccabe Thank you for the answer.
This PR seems to give rest-proxy users a lot of powerful abilities to access administrative APIs. I'm not sure that rest-proxy is the right place to manage the Kafka cluster.
We are only show information about groups who consumes messges from clusters topics. We don`t try to change settings or do any other work with cluster.
If we did want to do this we would need to figure out how to give certain users the ability to use the admin APIs and block everyone else. For example, users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource.
I don't see any problem about that. Because we allow users to consume messages through the rest-proxy. But why do we can't show information about consumer groups? Even if we allow users DESCRIBE permission on groups or DESCRIBE permission on the Cluster resource . I'm open for discussion this PR)
Why is this information needed for REST proxy? If it is needed, then we should at least think about providing it through the AdminClient.
I didn't found similar method in the API of AdminClient. If you show me alternative method to get information about brokers in cluster with security protocol, I'll repair code in this PR gladly.)
Hey @Ledostuff, thanks for the PR!
I haven't had time to give this a proper look yet but I wanted to ask - do we need ZooKeeper to access the cluster information only ( {"brokerId":0,"endPointList":[{"protocol":"SSL","host":"127.0.0.1","port":9092}]}) ?
In regards to that, I feel it would be best if we do not re-introduce a ZooKeeper dependancy in the proxy.
The consumer information should be fetchable through the adminClient only, correct? To me, it makes sense to expose consumer information. I think endpoints 2., 3., 4. and 5. will be useful.
There is a question of whether that information should be limited to consumers managed by this proxy instance or consumers in the Kafka Cluster. As @cmccabe said, permissions are a valid concern:
For example, users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource. As far as I know, in the current model we allow a consumer to be created and join any arbitrary group present in the Kafka cluster. That means exposing consumer group information for the whole cluster would be consistent with the security model already present.
Hi @stanislavkozlovski,
"in the current model we allow a consumer to be created and join any arbitrary group present"
No, we don't. This is only valid for the case where authentication and acl's are off OR we elevate kafka-rest principal's resource permissions like READ/DESCRIBE on cluster/group/topic, depending on the operation type we want. We are free to choose which ACLs to give to kafka-rest's principal, should consumer groups be exposed or not. That is, users are free to choose their own security model.
users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource.
That's right, unless we provide DESCRIBE permission on the Cluster resource.
Viewing consumer group offsets or listing consumer groups is just a feature that can be enabled or disabled, according to a security model adapted by the user.
Hi @rootex-,
That's true. I agree with exposing consumer group information.
Not to be annoying, but... is there still interest in this? I'd welcome having a list of consumer groups available via REST proxy.
Not to be annoying, but... is there still interest in this? I'd welcome having a list of consumer groups available via REST proxy.
Yes, there is still a huge interest in this feature. So much useful work done and no one have ever reviewed this stuff. We are also sick and tired of endless patching Kafka Rest in Sberbank.
Could we merge this pull request?
Hello there! Please review this PR. Thanks)
Hi @stanislavkozlovski, Please, review my changes. Sorry for the delay)
@stanislavkozlovski , All notes are fixed. Will you be able review my PR again? Thanks)
Yes, I will try to review it fully this week
@stanislavkozlovski I've just fixed all notes. Review this PR again, please. Do you agree with my response about duplication consumerId and host for every topic-partition subscription information?
I want to discuss the data model and what we return a bit more in depth. Would you mind commenting on this PR each new endpoint and an example of what it returns?
I want to discuss the data model and what we return a bit more in depth. Would you mind commenting on this PR each new endpoint and an example of what it returns?
Hi, @stanislavkozlovski. I have already done it - link. Or do you mean something else?
@stanislavkozlovski, @rigelbm Please review may changes. To my mind all specified notes are fixed.
Hello guys. Could anybody review my changes?
I will do a second pass today. Sorry for the delay.
I will do a second pass today. Sorry for the delay.
Guys, do you have any notes to my PR?
Guys, we need this update, please review 🙏🏼
retest, please
@stanislavkozlovski Have you seen my comments to your notes?
Helooooo) Any suggestions?