On Mon, Jun 22, 2020 at 9:47 AM Ismael Juma <ism...@juma.me.uk> wrote:
> Thanks for the reply Boyang. Comments inline. > > On Mon, Jun 22, 2020 at 9:31 AM Boyang Chen <reluctanthero...@gmail.com> > wrote: > > > Thanks for the questions Ismael. > > > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma <ism...@juma.me.uk> wrote: > > > > > Hi Colin, > > > > > > The KIP states in the Compatibility section (not Future work): > > > > > > "To support the proxy of requests, we need to build a channel for > brokers > > > to talk directly to the controller. This part of the design is internal > > > change only and won’t block the KIP progress." > > > > > > I am clarifying that this is not internal only due to the config. If we > > say > > > that this KIP depends on another KIP before we can merge it, that's > fine > > > although it feels a bit unnecessary. > > > > > Thanks for pointing that out. I could emphasize that in the compatibility > > section such that for older client > > redirection, we still have a dependency upon the controller refactoring > > work. > > > > "Controller refactoring" is a bit misleading as refactoring is typically > used for internal changes without external impact. We should state that the > configuration of this external channel will be described in a separate KIP. > And we should add a reference to that KIP once we know its number. > > Agree. > > "However, I can't help but feel that the fact that create topic policy > can > > > be bypassed via auto topic creation is a bug." > > > > > > Yeah, I totally agree that it's a bug. And I think the change here is > > good. > > > But we need to explain the details. One suggestion is to return > > > TopicAuthorizationFailed for older clients (closest non retriable > > exception > > > I could find) and introduce logic in new clients to handle > > PolicyViolation > > > from MetadataRequest. And mention this in the release notes. > > > > > IIUC, when the authorization on the controller failed, we want to add the > > handling of Authorization error > > to the CreateTopicsRequest from the redirecting broker. However, this > > sounds like an issue with > > the redirecting broker's acl, not the original consumer/producer. So we > > probably should just use a > > UNKNOWN_SERVER_ERROR to respond to the client, making it clear that this > is > > not a client fault. > > > > Not sure if I understand the intention correctly, and the proposed > solution > > makes sense? > > > This is an issue with the user action (they tried to create a topic that > did not comply with the policy, the most likely reason being related to the > number of topics and/or partitions), so I am not sure why we would > return NKNOWN_SERVER_ERROR here. > > We are only opening the doors for specific internal topics (offsets, txn log), which I assume the client should have no possibility to mutate the topic policy? > Ismael >
