Thanks for the KIP Boyang, this looks good to me. Some minor comments: 1) I think in order to implement the forwarding mechanism the brokers needs some purgatory to keep the forwarded requests; if that's true, should we add some broker-side metrics for those purgatories for debugging purposes?
2) Should we also consider adding some extra metric counting old versioned admin client request rates (this goes beyond https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers since old versioned client would not report its Kafka version anyways); one use case I can think of besides debugging purposes, is that if we ever decides to break compatibility in future versions way after the bridge releases, to reject any v1 requests and hence can totally remove this forwarding logic on brokers, we can leverage on this metric to find a safe time to upgrade. Guozhang On Mon, Apr 6, 2020 at 3:50 PM Colin McCabe <cmcc...@apache.org> wrote: > Hi Sönke, > > Yeah, that was my thought too. The request has already been validated on > the forwarding broker, so we don't need to validate it again. However, you > make a good point that it's unfortunate that the audit log would lose the > principal information if we didn't forward it as well. > > Perhaps we could add a tagged field to the request header for all > messages. This field would contain the principal name. Of course, this > field should only be allowed if the request arrives with the highest > permission levels (Probably ClusterAction on Cluster, since that's what all > the brokers have.) > > regards, > Colin > > > On Mon, Apr 6, 2020, at 14:37, Sönke Liebau wrote: > > Hi Boyang, > > > > thanks for the KIP. Sounds good overall. > > > > @Tom: I thought about your remark a little and think that in principle we > > can get away without forwarding the principal at all. Brokers currently > > authenticate and authorize requests before performing writes to > Zookeeper - > > as long as we don't change that it shouldn't matter, whether the write > goes > > to ZK or the controller, as long as that request is properly > authenticated. > > So the broker would simply authorize and authenticate the original > request > > and then forward it to the controller using its own credentials. And the > > controller could simply trust that this is a bona-fide request, because > it > > came from a trusted peer. > > > > I can see two issues here, one is a bit academic I think.. > > > > 1. The controller would be unable to write a proper audit log, because it > > cannot know who sent the original request. > > > > 2. In theory, clusters could use Plaintext Listeners for inter broker > > traffic because that is on a separate, secure network or similar reasons. > > In that case, the forwarded request would be unauthenticated - then > again, > > so are all other requests between brokers, so nothing lost really. > > > > Overall though, I think that sending the principal along with the request > > shouldn't be a large issue though, it is just two Strings and a boolean. > > And the controller could bypass the PrincipalBuilder and just pass the > > Principal that was built and sent by the remote broker straight to the > > Authorizer. Since PrincipalBuilders are the same on all brokers it > > shouldn't matter who does the processing I think. > > > > Best regards, > > Sönke > > > > > > On Mon, 6 Apr 2020 at 22:30, Boyang Chen <reluctanthero...@gmail.com> > wrote: > > > > > Thanks Tom for the question! I'm not super familiar with the Principal > > > stuff, could you elaborate more on the two points you proposed here? > > > > > > I looked up Admin client and just take `createDelegationToken` API for > an > > > example, the request data encodes the principal information already, so > > > broker should also leverage that information to proxy the request IMHO. > > > > > > Boyang > > > > > > On Mon, Apr 6, 2020 at 9:21 AM Tom Bentley <tbent...@redhat.com> > wrote: > > > > > > > Hi Boyang, > > > > > > > > Thanks for the KIP! > > > > > > > > When a broker proxies a request to the controller how does the > > > > authenticated principal get propagated? I think a couple of things > might > > > > complicate this: > > > > > > > > 1. A PrincipalBuilder might be in use, > > > > 2. A Principal does not have to be serializable. > > > > > > > > > > > > Kind regards, > > > > > > > > Tom > > > > > > > > On Sat, Apr 4, 2020 at 12:52 AM Boyang Chen < > reluctanthero...@gmail.com> > > > > wrote: > > > > > > > > > Hey all, > > > > > > > > > > I would like to start off the discussion for KIP-590, a follow-up > > > > > initiative after KIP-500: > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-590%3A+Redirect+Zookeeper+Mutation+Protocols+to+The+Controller > > > > > > > > > > This KIP proposes to migrate existing Zookeeper mutation paths, > > > including > > > > > configuration, security and quota changes, to controller-only by > always > > > > > routing these alterations to the controller. > > > > > > > > > > Let me know your thoughts! > > > > > > > > > > Best, > > > > > Boyang > > > > > > > > > > > > > > > > > > -- > > Sönke Liebau > > Partner > > Tel. +49 179 7940878 > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany > > > -- -- Guozhang
