cmccabe commented on pull request #10550:
URL: https://github.com/apache/kafka/pull/10550#issuecomment-839281487
Thanks for working on this, @rondagostino , and sorry about the delays in
reviewing.
> We forward the Create/Remove operations to the controller, but this patch
actually short-circuits that if we are using KRaft with the ZooKeeper-based
`AclAuthorizer` via the changes to `RaftSupport.maybeForward()`. The reason for
short-circuiting it is because the KRaft controller doesn't have the code to
create or remove ACLs (`handle{Create,Delete}Acls` in `KafkaApis`). We could
add it, of course, in which case the changes to the `maybeForward()` method
would be unnecessary. Perhaps it would be simpler to do that instead of
delaying it to an additional PR -- is that what you were suggesting?
Yes, I think we should just do this in `ControllerApis.scala` and be done
with it. It's kind of annoying to do in a follow-on PR since we'd have to add
a lot of special-case code which we'd later have to undo, which is not usually
the right way to go. We might even be able to move this code into
`RequestHandlerHelper` or something since it would be the same between
`BrokerApis` and `ControllerApis` (I think?)
--
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
