Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2021-07-21 Thread Colin McCabe
Agreed. Flexible fields are the way to go here, I think. Thanks for the discussion, all. best, Colin On Tue, Jul 20, 2021, at 07:18, Ismael Juma wrote: > Hi Ron, > > Thanks for bringing this up. Thinking about it, the combination of flexible > fields and the principal type field gives us

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2021-07-20 Thread Ismael Juma
Hi Ron, Thanks for bringing this up. Thinking about it, the combination of flexible fields and the principal type field gives us enough flexibility that we don't need a magic number. Ismael P.S. For a magic number to be useful for third party implementations, we would need a mechanism to

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2021-07-13 Thread Ron Dagostino
Hi everyone. I know it has been 9 months since the last message appeared on this vote thread, but a potential oversight exists in the implementation of DefaultKafkaPrincipalBuilder.KafkaPrincipalSerde from https://github.com/apache/kafka/pull/9103. Specifically, there is no magic number at the

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-10-09 Thread Boyang Chen
Thanks Jason for the great thoughts, and we basically decided to shift the gear for a limited impersonation approach offline. The goal here is to simplify the handling logic by relying on the active controller to do the actual authorization for resources in the original client request. We are

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-25 Thread Jason Gustafson
Hey All, So the main thing the EnvelopeRequest gives us is a way to avoid converting older API versions in order to attach the initial principal name and the clientId. It also saves the need to add the initial principal and client id as a tagged field to all of the forwarded protocols, which is

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-25 Thread Colin McCabe
On Fri, Sep 25, 2020, at 10:49, Boyang Chen wrote: > Hey Jun, > > On Fri, Sep 25, 2020 at 10:19 AM Jun Rao wrote: > > > Hi, Boyang, > > > > Does EnvelopeRequest avoid the need for IBP? How do we know if the > > controller supports EnvelopeRequest or not? > > > > Unfortunately, the

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-25 Thread Boyang Chen
Hey Jun, On Fri, Sep 25, 2020 at 10:19 AM Jun Rao wrote: > Hi, Boyang, > > Does EnvelopeRequest avoid the need for IBP? How do we know if the > controller supports EnvelopeRequest or not? > > Unfortunately, the EnvelopeRequest is solving the inter-broker communication problem only. Admin

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-25 Thread Jun Rao
Hi, Boyang, Does EnvelopeRequest avoid the need for IBP? How do we know if the controller supports EnvelopeRequest or not? Thanks, Jun On Thu, Sep 24, 2020 at 6:22 PM Boyang Chen wrote: > Hey Jason and Jun, > > thanks for the reply. Actually after some offline discussion, we have seen >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-24 Thread Boyang Chen
Hey Jason and Jun, thanks for the reply. Actually after some offline discussion, we have seen hassles around upgrading and downgrading RPCs during redirection, which is an error-prone approach to coordinate all parties to choose the correct version to handle. Alternatively, we propose to bring

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-24 Thread Jun Rao
Hi, Jason, Yes, the most important thing is to be able to avoid two rolling restarts in the future. If we have a path to achieve that down the road, the changes here are fine. Thanks, Jun On Thu, Sep 24, 2020 at 3:20 PM Jason Gustafson wrote: > > One of the goals of KIP-584 (feature

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-24 Thread Jason Gustafson
> One of the goals of KIP-584 (feature versioning) is that we can get rid of IBP in the future. So does this change prevent us from removing IBP in the future? That is a good question. I think the problem here is that request forwarding puts an expectation on api version support which covers more

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-24 Thread Jun Rao
Hi, Boyang, One of the goals of KIP-584 (feature versioning) is that we can get rid of IBP in the future. So does this change prevent us from removing IBP in the future? Thanks, Jun On Thu, Sep 24, 2020 at 12:46 PM Jason Gustafson wrote: > Hey Boyang, > > Thanks for the update. This seems

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-24 Thread Jason Gustafson
Hey Boyang, Thanks for the update. This seems like the best thing we can do. The alternative would be to always ensure that the forwarded APIs are safe for conversion between versions, but that would restrict the flexibility that the versioning is providing. It would also be a large effort to

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-09-24 Thread Boyang Chen
Hey there, we spotted a necessary case to handle the redirect request versioning, and proposed the following changes: 1. For redirection RPCs (AlterConfig, Acl, Token etc), the corresponding allowed versions in the ApiVersionResponse will be affected by the entire cluster's versioning, not just

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-08-06 Thread Boyang Chen
Hey there, we are going to introduce a minor change to bump the version of several RPCs which are currently not supporting flexible versions. It is necessary because they need to be able to construct request header with initial principal name and client id as optional fields for redirection. The

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-31 Thread Boyang Chen
Hey David, After discussing with Colin offline, I would like to correct one case in the described workflow, where the CLUSTER_ACTION authorization would not be based on the initial principal field check, because it is not a secured condition which anyone could forge. The revised workflow shall

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-30 Thread Boyang Chen
On Thu, Jul 30, 2020 at 7:18 AM David Jacot wrote: > Hi Boyang, > > Thanks for your answers. > > > The point for using the listener name is more of a security purpose, to > > detect any forged request to our best effort. > > For throttling I think we could just check the request header for > >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-30 Thread David Jacot
Hi Boyang, Thanks for your answers. > The point for using the listener name is more of a security purpose, to > detect any forged request to our best effort. > For throttling I think we could just check the request header for > *InitialClientId* existence, to distinguish whether to apply >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-29 Thread Boyang Chen
Thanks David for the feedback! On Wed, Jul 29, 2020 at 7:53 AM David Jacot wrote: > Hi, Colin, Boyang, > > Colin, thanks for the clarification. Somehow, I thought that even if the > controller is ran independently, it > would still run the listeners of the broker and thus would be accessible by

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-29 Thread David Jacot
Hi, Colin, Boyang, Colin, thanks for the clarification. Somehow, I thought that even if the controller is ran independently, it would still run the listeners of the broker and thus would be accessible by redirecting on the loopback interface. My mistake. Boyang, I have few questions/comments

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread Boyang Chen
Thanks for the feedback Colin! On Tue, Jul 28, 2020 at 2:11 PM Colin McCabe wrote: > Hi Boyang, > > Thanks for updating this. A few comments below: > > In the "Routing Request Security" section, there is a reference to "older > requests that need redirection." But after these new revisions,

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread Colin McCabe
Hi Boyang, Thanks for updating this. A few comments below: In the "Routing Request Security" section, there is a reference to "older requests that need redirection." But after these new revisions, both new and old requests need redirection. So we should rephrase this. > In addition, to

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread Colin McCabe
Hi David, Thanks for bringing this up. This is indeed something that we overlooked, that we'll have to figure out. The active controller may not be co-located with a broker in the post-KIP-500 world. So it does not make sense to have the client communicate directly with the controller.

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread David Jacot
Hi Boyang, Thanks for the update. 1./2. In KIP-599 (accepted and already in trunk), we throttle the CreateTopicsRequest, CreatePartitionsRequest, and DeleteTopicsRequests by muting the channel used by the Admin client and setting the throttleTimeMs in the response. The change that you propose

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-27 Thread Boyang Chen
Hey there, I'm re-opening this thread because after some initial implementations started, we spotted some gaps in the approved KIP as well as some inconsistencies with KIP-631 controller. There are a couple of addendums to the existing KIP, specifically: 1. As the controller is foreseen to be

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-01 Thread Boyang Chen
Hey folks, I have also synced on the KIP-578 which was doing the partition limit, to make sure the partition limit error code would be properly propagated once it is done on top of KIP-590. Let me know if you have further questions or concerns. Boyang On Tue, Jun 23, 2020 at 5:08 PM Boyang Chen

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-23 Thread Boyang Chen
Thanks for the clarification, Colin and Ismael. Personally I also feel Option A is better to prioritize fixing the gap. Just to be clear, the proposed solution would be: 1. Bump the Metadata RPC version to return POLICY_VIOLATION. In the application level, we should swap the error message with

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-23 Thread Ismael Juma
Option A is basically what I was thinking. But with a slight adjustment: New versions of MetadataResponse return POLICY_VIOLATION, old versions return AUTHORIZATION_FAILED. The latter works correctly with old Java clients (i.e. the client fails fast and propagates the error), I've tested it.

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-23 Thread Colin McCabe
> > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma 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.

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-22 Thread Boyang Chen
On Mon, Jun 22, 2020 at 9:47 AM Ismael Juma wrote: > Thanks for the reply Boyang. Comments inline. > > On Mon, Jun 22, 2020 at 9:31 AM Boyang Chen > wrote: > > > Thanks for the questions Ismael. > > > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma wrote: > > > > > Hi Colin, > > > > > > The KIP

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-22 Thread Ismael Juma
Thanks for the reply Boyang. Comments inline. On Mon, Jun 22, 2020 at 9:31 AM Boyang Chen wrote: > Thanks for the questions Ismael. > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma wrote: > > > Hi Colin, > > > > The KIP states in the Compatibility section (not Future work): > > > > "To support

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-22 Thread Boyang Chen
Thanks for the questions Ismael. On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma 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

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-19 Thread Ismael Juma
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

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-19 Thread Colin McCabe
On Thu, Jun 18, 2020, at 10:20, Ismael Juma wrote: > Hi Boyang, > > Sorry for being late on this. I'm generally in favor of the KIP, but it > seems like it has two missing things: > > 1. It doesn't cover how this new channel will be configured. That's a > critical part of having a KIP that

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-18 Thread Ismael Juma
Hi Boyang, Sorry for being late on this. I'm generally in favor of the KIP, but it seems like it has two missing things: 1. It doesn't cover how this new channel will be configured. That's a critical part of having a KIP that comprises all that is needed to merge the relevant PR. 2. It doesn't

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-18 Thread Boyang Chen
Thanks everyone for the vote! We already got 3 binding votes (Colin, Guozhang, Rajini) and 2 non-binding votes (Jose, David). The KIP is now approved. Best, Boyang On Thu, Jun 18, 2020 at 1:43 AM Rajini Sivaram wrote: > +1 (binding) > > Thanks for the KIP, Boyang! > > Regards, > > Rajini > > >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-18 Thread Rajini Sivaram
+1 (binding) Thanks for the KIP, Boyang! Regards, Rajini On Thu, Jun 18, 2020 at 8:15 AM David Jacot wrote: > +1 (non-binding) > > Thanks for the KIP! > > On Thu, Jun 18, 2020 at 12:00 AM Jose Garcia Sancio > wrote: > > > +1. > > > > Thanks for the KIP and looking forward to the

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-18 Thread David Jacot
+1 (non-binding) Thanks for the KIP! On Thu, Jun 18, 2020 at 12:00 AM Jose Garcia Sancio wrote: > +1. > > Thanks for the KIP and looking forward to the improvement implementation. > > On Wed, Jun 17, 2020 at 2:24 PM Guozhang Wang wrote: > > > > Thanks for the KIP Boyang, +1 from me. > > > > >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-17 Thread Jose Garcia Sancio
+1. Thanks for the KIP and looking forward to the improvement implementation. On Wed, Jun 17, 2020 at 2:24 PM Guozhang Wang wrote: > > Thanks for the KIP Boyang, +1 from me. > > > Guozhang > > On Wed, Jun 17, 2020 at 1:40 PM Colin McCabe wrote: > > > Thanks, Boyang! +1 (binding) > > > > best,

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-17 Thread Guozhang Wang
Thanks for the KIP Boyang, +1 from me. Guozhang On Wed, Jun 17, 2020 at 1:40 PM Colin McCabe wrote: > Thanks, Boyang! +1 (binding) > > best, > Colin > > On Mon, Jun 15, 2020, at 12:59, Boyang Chen wrote: > > Thanks for more feedback Colin! I have addressed them in the KIP. > > > > Boyang > >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-17 Thread Colin McCabe
Thanks, Boyang! +1 (binding) best, Colin On Mon, Jun 15, 2020, at 12:59, Boyang Chen wrote: > Thanks for more feedback Colin! I have addressed them in the KIP. > > Boyang > > On Mon, Jun 15, 2020 at 11:29 AM Colin McCabe wrote: > > > On Fri, Jun 12, 2020, at 15:30, Boyang Chen wrote: > > >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-15 Thread Boyang Chen
Thanks for more feedback Colin! I have addressed them in the KIP. Boyang On Mon, Jun 15, 2020 at 11:29 AM Colin McCabe wrote: > On Fri, Jun 12, 2020, at 15:30, Boyang Chen wrote: > > Thanks Colin for the suggestions! > > > > On Fri, Jun 12, 2020 at 2:40 PM Colin McCabe wrote: > > > > > Hi

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-15 Thread Colin McCabe
On Fri, Jun 12, 2020, at 15:30, Boyang Chen wrote: > Thanks Colin for the suggestions! > > On Fri, Jun 12, 2020 at 2:40 PM Colin McCabe wrote: > > > Hi Boyang, > > > > Thanks for the KIP! I think it's getting close. > > > > > For older requests that need redirection, forwarding > > > broker

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-12 Thread Boyang Chen
Thanks Colin for the suggestions! On Fri, Jun 12, 2020 at 2:40 PM Colin McCabe wrote: > Hi Boyang, > > Thanks for the KIP! I think it's getting close. > > > For older requests that need redirection, forwarding > > broker will just use its own authorizer to verify the principals. When > the >

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-12 Thread Colin McCabe
Hi Boyang, Thanks for the KIP! I think it's getting close. > For older requests that need redirection, forwarding > broker will just use its own authorizer to verify the principals. When the > request looks good, it will just forward the request with its own > credentials, no second