cmccabe edited a comment on pull request #10550: URL: https://github.com/apache/kafka/pull/10550#issuecomment-822694203
Thanks for tackling this, @rondagostino ! We have to be a bit careful about the structure here. This is a plugin architecture, which means that the ZK-based authorizer shouldn't get any special treatment, nor should its setup be done outside its own code. The code and logic for accessing ZooKeeper for ACLs needs to be contained just in the AclAuthorizer itself. For example, KafkaRaftServer does not need to know anything about ZooKeeper or AclAuthorizer specifically. Maybe KafkaRaftServer's constructor needs to take an Authorizer object as an argument. (Although it's not even clear to me that that is true, since it seems like all authorization happens in KafkaApis and ControllerApis.) But certainly KafkaRaftServer does not need to be looking at `config.zkConnect` or messing around with ZkClient. Whatever setup the zookeeper authorizer needs to do should be done in its `start` function. So creating the relevant znodes, etc. We probably need to continue doing that setup in KafkaServer.scala as well, since not all users will be using KafkaServer + AclAuthorizer (again, plugin architecture, they could use using one but not the other.) A separate issue is that we need to start forwarding the ACL operations to the controller. You did one half of that work here (adding "controller" to the message JSONs) but not the other (supporting these calls on the controller-side). If it's easier, we could probably do this in a follow-up JIRA. However, we do need to do it before the bridge release. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
