Just to adds a bit more FYI here related to the last question from David: in KIP-595 while implementing the new requests we are also adding a "KafkaNetworkChannel" which is used for brokers to send vote / fetch records, so besides the discussion on listeners I think implementation wise we can also consider consolidating a lot of those into the same call-trace as well -- of course this is not related to public APIs so maybe just needs to be coordinated among developments:
1. Broker -> Controller: ISR Change, Topic Creation, Admin Redirect (KIP-497). 2. Controller -> Broker: LeaderAndISR / MetadataUpdate; though these are likely going to be deprecated post KIP-500. 3. Txn Coordinator -> Broker: TxnMarker Guozhang On Wed, May 6, 2020 at 8:58 PM Boyang Chen <reluctanthero...@gmail.com> wrote: > Hey David, > > thanks for the feedbacks! > > On Wed, May 6, 2020 at 2:06 AM David Jacot <dja...@confluent.io> wrote: > > > Hi Boyang, > > > > While re-reading the KIP, I've got few small questions/comments: > > > > 1. When auto topic creation is enabled, brokers will send a > > CreateTopicRequest > > to the controller instead of writing to ZK directly. It means that > > creation of these > > topics are subject to be rejected with an error if a CreateTopicPolicy is > > used. Today, > > it bypasses the policy entirely. I suppose that clusters allowing auto > > topic creation > > don't have a policy in place so it is not a big deal. I suggest to call > > out explicitly the > > limitation in the KIP though. > > > > That's a good idea, will add to the KIP. > > > > 2. In the same vein as my first point. How do you plan to handle errors > > when internal > > topics are created by a broker? Do you plan to retry retryable errors > > indefinitely? > > > > I checked a bit on the admin client handling of the create topic RPC. It > seems that > the only retriable exceptions at the moment are NOT_CONTROLLER and > REQUEST_TIMEOUT. > So I guess we just need to retry on these exceptions? > > > > 3. Could you clarify which listener will be used for the internal > requests? > > Do you plan > > to use the control plane listener or perhaps the inter-broker listener? > > > > As we discussed in the KIP, currently the internal design for > broker->controller channel has not been > done yet, and I feel it makes sense to consolidate redirect RPC and > internal topic creation RPC to this future channel, > which are details to be filled in the near future, right now some > controller refactoring effort is still WIP. > > > > Thanks, > > David > > > > On Mon, May 4, 2020 at 9:37 AM Sönke Liebau > > <soenke.lie...@opencore.com.invalid> wrote: > > > > > Ah, I see, thanks for the clarification! > > > > > > Shouldn't be an issue I think. My understanding of KIPs was always that > > > they are mostly intended as a place to discuss and agree changes up > > front, > > > whereas tracking the actual releases that things go into should be > > handled > > > in Jira. > > > So maybe we just create new jiras for any subsequent work and either > link > > > those or make them subtasks (even though this jira is already a subtask > > > itself), that should allow us to properly track all releases that work > > goes > > > into. > > > > > > Thanks for your work on this!! > > > > > > Best, > > > Sönke > > > > > > > > > On Sat, 2 May 2020 at 00:31, Boyang Chen <reluctanthero...@gmail.com> > > > wrote: > > > > > > > Sure thing Sonke, what I suggest is that usual KIPs get accepted to > go > > > into > > > > next release. It could span for a couple of releases because of > > > engineering > > > > time, but no change has to be shipped in specific future releases, > like > > > the > > > > backward incompatible change for KafkaPrincipal. But I guess it's not > > > > really a blocker, as long as we stated clearly in the KIP how we are > > > going > > > > to roll things out, and let it partially finish in 2.6. > > > > > > > > Boyang > > > > > > > > On Fri, May 1, 2020 at 2:32 PM Sönke Liebau > > > > <soenke.lie...@opencore.com.invalid> wrote: > > > > > > > > > Hi Boyang, > > > > > > > > > > thanks for the update, sounds reasonable to me. Making it a > breaking > > > > change > > > > > is definitely the safer route to go. > > > > > > > > > > Just one quick question regarding your mail, I didn't fully > > understand > > > > what > > > > > you mean by "I think this is the first time we need to introduce a > > KIP > > > > > without having it > > > > > fully accepted in next release." - could you perhaps explain that > > some > > > > > more very briefly? > > > > > > > > > > Best regards, > > > > > Sönke > > > > > > > > > > > > > > > > > > > > On Fri, 1 May 2020 at 23:03, Boyang Chen < > reluctanthero...@gmail.com > > > > > > > > wrote: > > > > > > > > > > > Hey Tom, > > > > > > > > > > > > thanks for the suggestion. As long as we could correctly > serialize > > > the > > > > > > principal and embed in the Envelope, I think we could still > > leverage > > > > the > > > > > > controller to do the client request authentication. Although this > > > pays > > > > an > > > > > > extra round trip if the authorization is doomed to fail on the > > > receiver > > > > > > side, having a centralized processing unit is more favorable such > > as > > > > > > ensuring the audit log is consistent instead of scattering > between > > > > > > forwarder and receiver. > > > > > > > > > > > > Boyang > > > > > > > > > > > > On Wed, Apr 29, 2020 at 9:50 AM Tom Bentley <tbent...@redhat.com > > > > > > wrote: > > > > > > > > > > > > > Hi Boyang, > > > > > > > > > > > > > > Thanks for the update. In the EnvelopeRequest handling section > of > > > the > > > > > KIP > > > > > > > it might be worth saying explicitly that authorization of the > > > request > > > > > > will > > > > > > > happen as normal. Otherwise what you're proposing makes sense > to > > > me. > > > > > > > > > > > > > > Thanks again, > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Apr 29, 2020 at 5:27 PM Boyang Chen < > > > > > reluctanthero...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > Thanks for the proposed idea Sonke. I reviewed it and had > some > > > > > offline > > > > > > > > discussion with Colin, Rajini and Mathew. > > > > > > > > > > > > > > > > We do need to add serializability to the PrincipalBuilder > > > > interface, > > > > > > but > > > > > > > we > > > > > > > > should not make any default implementation which could go > wrong > > > and > > > > > > messy > > > > > > > > up with the security in a production environment if the user > > > > neglects > > > > > > it. > > > > > > > > Instead we need to make it required and backward > incompatible. > > > So I > > > > > > > > integrated your proposed methods and expand the Envelope RPC > > > with a > > > > > > > couple > > > > > > > > of more fields for audit log purpose as well. > > > > > > > > > > > > > > > > Since the KafkaPrincipal builder serializability is a binary > > > > > > incompatible > > > > > > > > change, I propose (also stated in the KIP) the following > > > > > implementation > > > > > > > > plan: > > > > > > > > > > > > > > > > 1. For next *2.x* release: > > > > > > > > 1. Get new admin client forwarding changes > > > > > > > > 2. Get the Envelope RPC implementation > > > > > > > > 3. Get the forwarding path working and validate the > > > function > > > > > with > > > > > > > > fake principals in testing environment, without actual > > > > > triggering > > > > > > > in > > > > > > > > the > > > > > > > > production system > > > > > > > > 2. For next *3.0 *release: > > > > > > > > 1. Introduce serializability to PrincipalBuilder > > > > > > > > 2. Turn on forwarding path in production and perform > > > > end-to-end > > > > > > > > testing > > > > > > > > > > > > > > > > > > > > > > > > I think this is the first time we need to introduce a KIP > > without > > > > > > having > > > > > > > it > > > > > > > > fully accepted in next release. Let me know if this sounds > > > > > reasonable. > > > > > > > > > > > > > > > > On Fri, Apr 24, 2020 at 1:00 AM Sönke Liebau > > > > > > > > <soenke.lie...@opencore.com.invalid> wrote: > > > > > > > > > > > > > > > > > After thinking on this a little bit, maybe this would be an > > > > option: > > > > > > > > > > > > > > > > > > add default methods serialize and deserialize to the > > > > > > > > KafkaPrincipalBuilder > > > > > > > > > interface, these could be very short: > > > > > > > > > > > > > > > > > > default String serialize(KafkaPrincipal principal) { > > > > > > > > > return principal.toString(); > > > > > > > > > } > > > > > > > > > > > > > > > > > > default KafkaPrincipal deserialize(String principalString) > { > > > > > > > > > return > > SecurityUtils.parseKafkaPrincipal(principalString); > > > > > > > > > } > > > > > > > > > > > > > > > > > > This would mean that all existing implementations of that > > > > interface > > > > > > > > > are unaffected, as this code is pretty much what is > currently > > > > being > > > > > > > > > used when their principals need to be serialized. > > > > > > > > > > > > > > > > > > But it offers people using custom principals the chance to > > > > override > > > > > > > > > these methods and ensure that all information gets > serialized > > > for > > > > > > > > > delegation tokens or request forwarding. > > > > > > > > > > > > > > > > > > > > > > > > > > > Wherever we need to de/serialize principals (for example in > > the > > > > > > > > > DelegationTokenManager [1]) we obtain an instance of the > > > > configured > > > > > > > > > PrincipalBuilder class and use that to do the actual work. > > > > > > > > > > > > > > > > > > What do you think? > > > > > > > > > > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > > > > > Sönke > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/33d06082117d971cdcddd4f01392006b543f3c01/core/src/main/scala/kafka/server/DelegationTokenManager.scala#L122 > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, 23 Apr 2020 at 17:42, Boyang Chen < > > > > > > reluctanthero...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Thanks all, > > > > > > > > > > > > > > > > > > > > IIUC, the necessity of doing the audit log on the > > controller > > > > side > > > > > > is > > > > > > > > > > because we need to make sure the authorized resource > > > > > modifications > > > > > > > > > > eventually arrive on the target broker side, but is that > > > really > > > > > > > > > necessary? > > > > > > > > > > > > > > > > > > > > I'm thinking the possibility of doing the audit log on > the > > > > > > forwarding > > > > > > > > > > broker side, which could simplify the discussion of > > principal > > > > > > > > > serialization > > > > > > > > > > here. The other option I could think of is to serialize > the > > > > > entire > > > > > > > > audit > > > > > > > > > > log message if we were supposed to approve, and pass it > as > > > part > > > > > of > > > > > > > the > > > > > > > > > > Envelope. > > > > > > > > > > > > > > > > > > > > Let me know if you think either of these approaches would > > > work. > > > > > > > > > > > > > > > > > > > > On Thu, Apr 23, 2020 at 7:01 AM Sönke Liebau > > > > > > > > > > <soenke.lie...@opencore.com.invalid> wrote: > > > > > > > > > > > > > > > > > > > > > I agree that this would be useful to have and shouldn't > > > > create > > > > > > > issues > > > > > > > > > in > > > > > > > > > > > 99% of all cases. But it would be a breaking change to > a > > > > public > > > > > > > API. > > > > > > > > > > > I had a quick look at the two large projects that come > to > > > > mind > > > > > > > which > > > > > > > > > > might > > > > > > > > > > > be affected: Ranger and Sentry - both seem to operate > > > > directly > > > > > > with > > > > > > > > > > > KafkaPrincipal instead of subclassing it. But anybody > who > > > > > > > > > > > extended KafkaPrincipal would probably need to update > > their > > > > > > code.. > > > > > > > > > > > > > > > > > > > > > > Writing this sparked the thought that this issue should > > > also > > > > > > > concern > > > > > > > > > > > delegation tokens, as Principals need to be stored/sent > > > > around > > > > > > for > > > > > > > > > those > > > > > > > > > > > too. > > > > > > > > > > > Had a brief look at the code and for Delegation Tokens > we > > > > seem > > > > > to > > > > > > > use > > > > > > > > > > > SecurityUtils#parseKafkaPrincipal[1] which would ignore > > any > > > > > > > > additional > > > > > > > > > > > information from custom Principals. > > > > > > > > > > > > > > > > > > > > > > We'll probably want to at least add a note on that to > the > > > > docs > > > > > - > > > > > > > > unless > > > > > > > > > > it > > > > > > > > > > > is there already, I've only looked for about 30 > seconds.. > > > > > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > Sönke > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/e9fcfe4fb7b9ae2f537ce355fe2ab51a58034c64/clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java#L52 > > > > > > > > > > > > > > > > > > > > > > On Thu, 23 Apr 2020 at 14:35, Colin McCabe < > > > > cmcc...@apache.org > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hmm... Maybe we need to add some way to serialize and > > > > > > deserialize > > > > > > > > > > > > KafkaPrincipal subclasses to/from string. We could > > add a > > > > > > method > > > > > > > to > > > > > > > > > > > > KafkaPrincipalBuilder#deserialize and a method > > > > > > > > > > KafkaPrincipal#serialize, > > > > > > > > > > > I > > > > > > > > > > > > suppose. > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Apr 23, 2020, at 02:14, Tom Bentley wrote: > > > > > > > > > > > > > Hi folks, > > > > > > > > > > > > > > > > > > > > > > > > > > Colin wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > The final broker knows it can trust the principal > > > name > > > > in > > > > > > the > > > > > > > > > > > envelope > > > > > > > > > > > > > > (since EnvelopeRequest requires CLUSTERACTION on > > > > > CLUSTER). > > > > > > > So > > > > > > > > it > > > > > > > > > > can > > > > > > > > > > > > use > > > > > > > > > > > > > > that principal name for authorization (given who > > you > > > > are, > > > > > > > what > > > > > > > > > can > > > > > > > > > > > you > > > > > > > > > > > > > > do?) The forwarded principal name will also be > > used > > > > for > > > > > > > > logging. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > My understanding (and I'm happy to be corrected) is > > > that > > > > a > > > > > > > custom > > > > > > > > > > > > > authoriser might rely on the KafkaPrincipal > instance > > > > being > > > > > a > > > > > > > > > subclass > > > > > > > > > > > of > > > > > > > > > > > > > KafkaPrincipal (e.g. the subclass has extra fields > > with > > > > the > > > > > > > > > > principal's > > > > > > > > > > > > > "roles"). So you can't construct a KafkaPrinicpal > on > > > the > > > > > > > > controller > > > > > > > > > > > which > > > > > > > > > > > > > would be guaranteed to work for arbitrary > > authorizers. > > > > You > > > > > > have > > > > > > > > to > > > > > > > > > > > > perform > > > > > > > > > > > > > authorization on the first broker (rejecting some > of > > > the > > > > > > > batched > > > > > > > > > > > > requests), > > > > > > > > > > > > > forward the authorized ones to the controller, > which > > > then > > > > > has > > > > > > > to > > > > > > > > > > trust > > > > > > > > > > > > that > > > > > > > > > > > > > the authorization has been done and make the ZK > > writes > > > > > > without > > > > > > > > > > > > > authorization. Because the controller cannot invoke > > the > > > > > > > > authorizer > > > > > > > > > > that > > > > > > > > > > > > > means that the authorizer audit logging (on the > > > > controller) > > > > > > > would > > > > > > > > > not > > > > > > > > > > > > > include these operations. But they would be in the > > > audit > > > > > > > logging > > > > > > > > > from > > > > > > > > > > > the > > > > > > > > > > > > > original broker, so the information would not be > > lost. > > > > > > > > > > > > > > > > > > > > > > > > > > There's also a problem with using the constructed > > > > principal > > > > > > for > > > > > > > > > other > > > > > > > > > > > > > logging (i.e. non authorizer logging) on the > > > controller: > > > > > > > There's > > > > > > > > > > > nothing > > > > > > > > > > > > > stopping a custom KafkaPrincipal subclass from > > > overriding > > > > > > > > > toString() > > > > > > > > > > to > > > > > > > > > > > > > return something different from "${type}:${name}". > If > > > > > you're > > > > > > > > > > building a > > > > > > > > > > > > > "fake" KafkaPrincipal on the controller from the > type > > > and > > > > > > name > > > > > > > > then > > > > > > > > > > its > > > > > > > > > > > > > toString() would be "wrong". A solution to this > would > > > be > > > > to > > > > > > > also > > > > > > > > > > > > serialize > > > > > > > > > > > > > the toString() into the envelope and have some > > > > > > > > > ProxiedKafkaPrincipal > > > > > > > > > > > > class > > > > > > > > > > > > > which you instantiate on the controller which has > > > > > overridden > > > > > > > > > toString > > > > > > > > > > > to > > > > > > > > > > > > > return the right value. Obviously you could > optimize > > > this > > > > > > using > > > > > > > > an > > > > > > > > > > > > optional > > > > > > > > > > > > > field so you only serialize the toString() if it > > > differed > > > > > > from > > > > > > > > the > > > > > > > > > > > value > > > > > > > > > > > > > you'd get from KafkaPrincipal.toString(). Maybe > > > non-audit > > > > > > > logging > > > > > > > > > > using > > > > > > > > > > > > the > > > > > > > > > > > > > wrong string value of a principal is sufficiently > > minor > > > > > that > > > > > > > this > > > > > > > > > > isn't > > > > > > > > > > > > > even worth trying to solve. > > > > > > > > > > > > > > > > > > > > > > > > > > Kind regards, > > > > > > > > > > > > > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Apr 22, 2020 at 10:59 PM Sönke Liebau > > > > > > > > > > > > > <soenke.lie...@opencore.com.invalid> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Colin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > thanks for your summary! Just one question - and > I > > > may > > > > be > > > > > > > > missing > > > > > > > > > > an > > > > > > > > > > > > > > obvious point here.. > > > > > > > > > > > > > > You write: > > > > > > > > > > > > > > > > > > > > > > > > > > > > "The initial broker should do authentication (who > > are > > > > > you?) > > > > > > > and > > > > > > > > > > come > > > > > > > > > > > up > > > > > > > > > > > > > > with a principal name. Then it creates an > envelope > > > > > > request, > > > > > > > > > which > > > > > > > > > > > will > > > > > > > > > > > > > > contain that principal name, and sends it along > > with > > > > the > > > > > > > > > unmodified > > > > > > > > > > > > > > original request to the final broker. [... ] > The > > > > final > > > > > > > broker > > > > > > > > > > knows > > > > > > > > > > > > it > > > > > > > > > > > > > > can trust the principal name in the envelope > (since > > > > > > > > > EnvelopeRequest > > > > > > > > > > > > > > requires CLUSTERACTION on CLUSTER). So it can > use > > > that > > > > > > > > principal > > > > > > > > > > > name > > > > > > > > > > > > for > > > > > > > > > > > > > > authorization (given who you are, what can you > > do?) " > > > > > > > > > > > > > > > > > > > > > > > > > > > > My understanding is, that you don't want to > > serialize > > > > the > > > > > > > > > Principal > > > > > > > > > > > > (due to > > > > > > > > > > > > > > the discussed issues with custom principals) but > > > reduce > > > > > the > > > > > > > > > > principal > > > > > > > > > > > > down > > > > > > > > > > > > > > to a string representation that would be used for > > > > logging > > > > > > and > > > > > > > > > > > > > > authorization? > > > > > > > > > > > > > > If that understanding is correct then I don't > think > > > we > > > > > > could > > > > > > > > use > > > > > > > > > > the > > > > > > > > > > > > > > regular Authorizer on the target broker, because > > that > > > > > would > > > > > > > > need > > > > > > > > > > the > > > > > > > > > > > > actual > > > > > > > > > > > > > > principal object to work on. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, a thought that just occurred to me, we > might > > > > > actually > > > > > > > > need > > > > > > > > > to > > > > > > > > > > > log > > > > > > > > > > > > > > different principal strings for the case of > queries > > > > like > > > > > > > > > > AlterConfigs > > > > > > > > > > > > > > (mentioned by Rajini) which may contain multiple > > > > > resources. > > > > > > > > Take > > > > > > > > > an > > > > > > > > > > > > LDAP > > > > > > > > > > > > > > authorizer that grants access based on group > > > > membership - > > > > > > the > > > > > > > > > same > > > > > > > > > > > > > > alterconfig request may contain resources that > are > > > > > > authorized > > > > > > > > > based > > > > > > > > > > > on > > > > > > > > > > > > > > group1 as well as resources authorized based on > > > > > membership > > > > > > in > > > > > > > > > > group 2 > > > > > > > > > > > > .. > > > > > > > > > > > > > > And in all cases we'd need to log the specific > > > reason I > > > > > > > think.. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Basically I think that we might have a hard time > > > > properly > > > > > > > > > > authorizing > > > > > > > > > > > > and > > > > > > > > > > > > > > logging without being able to forward the entire > > > > > > principal.. > > > > > > > > but > > > > > > > > > > > > again, I > > > > > > > > > > > > > > might be heading down an entirely wrong path here > > :) > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > Sönke > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, 22 Apr 2020 at 23:13, Guozhang Wang < > > > > > > > > wangg...@gmail.com> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Colin, Boyang: thanks for the updates, I agree > > that > > > > an > > > > > > > > > > > > EnvelopeRequest > > > > > > > > > > > > > > > would be a less vulnerable approach than > optional > > > > > fields, > > > > > > > and > > > > > > > > > I'm > > > > > > > > > > > > just > > > > > > > > > > > > > > > wondering if we would keep the EnvelopeRequest > > for > > > a > > > > > long > > > > > > > > > time. I > > > > > > > > > > > was > > > > > > > > > > > > > > > thinking that, potentially if we require > clients > > to > > > > be > > > > > on > > > > > > > > newer > > > > > > > > > > > > version > > > > > > > > > > > > > > > when talking to a 3.0+ (assuming 3.0 is the > > bridge > > > > > > release) > > > > > > > > > > > brokers, > > > > > > > > > > > > then > > > > > > > > > > > > > > > we do not need to keep this code for too long, > > but > > > I > > > > > > think > > > > > > > > that > > > > > > > > > > > > would be > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > very hasty compatibility breaking so maybe we > > > indeed > > > > > need > > > > > > > to > > > > > > > > > keep > > > > > > > > > > > > this > > > > > > > > > > > > > > > forwarding mechanism many years. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Regarding future use cases, I think the example > > > that > > > > > > Boyang > > > > > > > > > > > > mentioned may > > > > > > > > > > > > > > > not be very practical honestly, because when > > > there's > > > > a > > > > > > > > > > connectivity > > > > > > > > > > > > > > issue, > > > > > > > > > > > > > > > it is either a network partition between > > > "controller, > > > > > A | > > > > > > > B", > > > > > > > > > or > > > > > > > > > > > > > > > "controller | A, B". In other words, if the > > > > controller > > > > > > can > > > > > > > > talk > > > > > > > > > > to > > > > > > > > > > > A, > > > > > > > > > > > > > > then > > > > > > > > > > > > > > > very likely A would not be able to talk to B > > > > either... > > > > > > > > anyways, > > > > > > > > > > > > since the > > > > > > > > > > > > > > > forwarding would be there for a sufficiently > long > > > > > time, I > > > > > > > > think > > > > > > > > > > > > keeping > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > additional envelope makes sense. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Apr 22, 2020 at 1:47 PM Boyang Chen < > > > > > > > > > > > > reluctanthero...@gmail.com> > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks Colin for the summary! And Guozhang, > > > > regarding > > > > > > the > > > > > > > > > > future > > > > > > > > > > > > use > > > > > > > > > > > > > > > cases, > > > > > > > > > > > > > > > > consider a scenario where there are temporary > > > > > > > connectivity > > > > > > > > > > issue > > > > > > > > > > > > > > between > > > > > > > > > > > > > > > > controller to a fellow broker A, the > controller > > > > could > > > > > > > then > > > > > > > > > > > leverage > > > > > > > > > > > > > > > another > > > > > > > > > > > > > > > > healthy broker B to do a forwarding request > to > > A > > > in > > > > > > order > > > > > > > > to > > > > > > > > > > > > maintain a > > > > > > > > > > > > > > > > communication. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Even for KIP-590 scope, the forwarding > > mechanism > > > > > shall > > > > > > > not > > > > > > > > be > > > > > > > > > > > > transit > > > > > > > > > > > > > > as > > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > do need to support older version of admin > > clients > > > > > for a > > > > > > > > > couple > > > > > > > > > > of > > > > > > > > > > > > years > > > > > > > > > > > > > > > > IIUC. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Boyang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Apr 22, 2020 at 1:29 PM Colin McCabe > < > > > > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I guess the way I see this working is that > > the > > > > > > request > > > > > > > > gets > > > > > > > > > > > sent > > > > > > > > > > > > from > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > client, to the initial broker, and then > > > forwarded > > > > > to > > > > > > > the > > > > > > > > > > final > > > > > > > > > > > > > > broker. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The initial broker should do authentication > > > (who > > > > > are > > > > > > > > you?) > > > > > > > > > > and > > > > > > > > > > > > come > > > > > > > > > > > > > > up > > > > > > > > > > > > > > > > > with a principal name. Then it creates an > > > > envelope > > > > > > > > > request, > > > > > > > > > > > > which > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > contain that principal name, and sends it > > along > > > > > with > > > > > > > the > > > > > > > > > > > > unmodified > > > > > > > > > > > > > > > > > original request to the final broker. (I > > agree > > > > > with > > > > > > > Tom > > > > > > > > > and > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > > can't forward the information needed to do > > > > > > > authentication > > > > > > > > > on > > > > > > > > > > > the > > > > > > > > > > > > > > final > > > > > > > > > > > > > > > > > broker, but I also think we don't need to, > > > since > > > > we > > > > > > can > > > > > > > > do > > > > > > > > > it > > > > > > > > > > > on > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > initial broker.) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The final broker knows it can trust the > > > principal > > > > > > name > > > > > > > in > > > > > > > > > the > > > > > > > > > > > > > > envelope > > > > > > > > > > > > > > > > > (since EnvelopeRequest requires > CLUSTERACTION > > > on > > > > > > > > CLUSTER). > > > > > > > > > > So > > > > > > > > > > > > it can > > > > > > > > > > > > > > > use > > > > > > > > > > > > > > > > > that principal name for authorization > (given > > > who > > > > > you > > > > > > > are, > > > > > > > > > > what > > > > > > > > > > > > can > > > > > > > > > > > > > > you > > > > > > > > > > > > > > > > > do?) The forwarded principal name will > also > > be > > > > > used > > > > > > > for > > > > > > > > > > > logging. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > One question is why we need an > > EnvelopeRequest. > > > > > > Well, > > > > > > > if > > > > > > > > > we > > > > > > > > > > > > don't > > > > > > > > > > > > > > have > > > > > > > > > > > > > > > > an > > > > > > > > > > > > > > > > > EnvelopeRequest, we need somewhere else to > > put > > > > the > > > > > > > > > forwarded > > > > > > > > > > > > > > principal > > > > > > > > > > > > > > > > > name. I don't think overriding an existing > > > field > > > > > > (like > > > > > > > > > > > > clientId) is > > > > > > > > > > > > > > a > > > > > > > > > > > > > > > > good > > > > > > > > > > > > > > > > > option for this. It's messy, and loses > > > > > information. > > > > > > > It > > > > > > > > > also > > > > > > > > > > > > raises > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > question of how the final broker knows that > > the > > > > > > > clientId > > > > > > > > in > > > > > > > > > > the > > > > > > > > > > > > > > > received > > > > > > > > > > > > > > > > > message is not "really" a clientId, but is > a > > > > > > principal > > > > > > > > > name. > > > > > > > > > > > > Without > > > > > > > > > > > > > > > an > > > > > > > > > > > > > > > > > envelope, there's no way to clearly mark a > > > > request > > > > > as > > > > > > > > > > > forwarded, > > > > > > > > > > > > so > > > > > > > > > > > > > > > > there's > > > > > > > > > > > > > > > > > no reason for the final broker to treat > this > > > > > > > differently > > > > > > > > > > than a > > > > > > > > > > > > > > regular > > > > > > > > > > > > > > > > > clientId (or whatever). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > We talked about using optional fields to > > > contain > > > > > the > > > > > > > > > > forwarded > > > > > > > > > > > > > > > principal > > > > > > > > > > > > > > > > > name, but this is also messy and > potentially > > > > > > dangerous. > > > > > > > > > > Older > > > > > > > > > > > > > > brokers > > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > simply ignore the optional fields, which > > could > > > > > result > > > > > > > in > > > > > > > > > them > > > > > > > > > > > > > > executing > > > > > > > > > > > > > > > > > operations as the wrong principal. Of > > course, > > > > this > > > > > > > would > > > > > > > > > > > > require a > > > > > > > > > > > > > > > > > misconfiguration in order to happen, but it > > > still > > > > > > seems > > > > > > > > > > better > > > > > > > > > > > > to set > > > > > > > > > > > > > > > up > > > > > > > > > > > > > > > > > the system so that this misconfiguration is > > > > > detected, > > > > > > > > > rather > > > > > > > > > > > than > > > > > > > > > > > > > > > > silently > > > > > > > > > > > > > > > > > ignored. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It's true that the need for forwarding is > > > > > "temporary" > > > > > > > in > > > > > > > > > some > > > > > > > > > > > > sense, > > > > > > > > > > > > > > > > since > > > > > > > > > > > > > > > > > we only need it for older clients. > However, > > we > > > > > will > > > > > > > want > > > > > > > > > to > > > > > > > > > > > > support > > > > > > > > > > > > > > > > these > > > > > > > > > > > > > > > > > older clients for many years to come. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I agree that the usefulness of > > EnvelopeRequest > > > is > > > > > > > limited > > > > > > > > > by > > > > > > > > > > it > > > > > > > > > > > > > > being a > > > > > > > > > > > > > > > > > superuser-only request at the moment. > > Perhaps > > > > > there > > > > > > > are > > > > > > > > > some > > > > > > > > > > > > changes > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > how custom principals work that would allow > > us > > > to > > > > > get > > > > > > > > > around > > > > > > > > > > > > this in > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > future. We should think about that so that > > we > > > > have > > > > > > > this > > > > > > > > > > > > > > functionality > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > the future if it's needed. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Apr 22, 2020, at 11:21, Guozhang > Wang > > > > > wrote: > > > > > > > > > > > > > > > > > > Hello Gwen, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The purpose here is for maintaining > > > > compatibility > > > > > > old > > > > > > > > > > > clients, > > > > > > > > > > > > who > > > > > > > > > > > > > > do > > > > > > > > > > > > > > > > not > > > > > > > > > > > > > > > > > > have functionality to do re-routing admin > > > > > requests > > > > > > > > > > > themselves. > > > > > > > > > > > > New > > > > > > > > > > > > > > > > > clients > > > > > > > > > > > > > > > > > > can of course do this themselves by > > detecting > > > > > who's > > > > > > > the > > > > > > > > > > > > controller. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hello Colin / Boyang, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Regarding the usage of the envelope, I'm > > > > curious > > > > > > what > > > > > > > > are > > > > > > > > > > the > > > > > > > > > > > > > > > potential > > > > > > > > > > > > > > > > > > future use cases that would require > request > > > > > > > forwarding > > > > > > > > > and > > > > > > > > > > > > hence > > > > > > > > > > > > > > > > envelope > > > > > > > > > > > > > > > > > > would be useful? Originally I thought > that > > > the > > > > > > > > forwarding > > > > > > > > > > > > mechanism > > > > > > > > > > > > > > > is > > > > > > > > > > > > > > > > > only > > > > > > > > > > > > > > > > > > temporary as we need it for the bridge > > > release, > > > > > and > > > > > > > > > moving > > > > > > > > > > > > forward > > > > > > > > > > > > > > we > > > > > > > > > > > > > > > > > will > > > > > > > > > > > > > > > > > > get rid of this to simplify the code > base. > > If > > > > we > > > > > do > > > > > > > > have > > > > > > > > > > > valid > > > > > > > > > > > > use > > > > > > > > > > > > > > > > cases > > > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > > the future which makes us believe that > > > request > > > > > > > > forwarding > > > > > > > > > > > would > > > > > > > > > > > > > > > > actually > > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > a permanent feature retained on the > broker > > > > side, > > > > > > I'm > > > > > > > on > > > > > > > > > > board > > > > > > > > > > > > with > > > > > > > > > > > > > > > > adding > > > > > > > > > > > > > > > > > > the envelope request protocol. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Apr 22, 2020 at 8:22 AM Gwen > > Shapira > > > < > > > > > > > > > > > > g...@confluent.io> > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hey Boyang, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sorry if this was already discussed, > but > > I > > > > > didn't > > > > > > > see > > > > > > > > > > this > > > > > > > > > > > as > > > > > > > > > > > > > > > > rejected > > > > > > > > > > > > > > > > > > > alternative: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Until now, we always did client side > > > routing > > > > - > > > > > > the > > > > > > > > > client > > > > > > > > > > > > itself > > > > > > > > > > > > > > > > found > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > controller via metadata and directed > > > requests > > > > > > > > > > accordingly. > > > > > > > > > > > > > > Brokers > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > were not the controller, rejected those > > > > > requests. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Why did we decide to move to broker > side > > > > > routing? > > > > > > > Was > > > > > > > > > the > > > > > > > > > > > > > > > client-side > > > > > > > > > > > > > > > > > > > option discussed and rejected somewhere > > > and I > > > > > > > missed > > > > > > > > > it? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Gwen > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Apr 3, 2020, 4:45 PM 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > -- Guozhang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > -- Guozhang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > Sönke Liebau > > > > > > > > > > > > > > Partner > > > > > > > > > > > > > > Tel. +49 179 7940878 > > > > > > > > > > > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - > > 22880 > > > > > > Wedel - > > > > > > > > > > Germany > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > Sönke Liebau > > > > > > > > > > > Partner > > > > > > > > > > > Tel. +49 179 7940878 > > > > > > > > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 > > > Wedel - > > > > > > > Germany > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Sönke Liebau > > > > > > > > > Partner > > > > > > > > > Tel. +49 179 7940878 > > > > > > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 > Wedel - > > > > > Germany > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Sönke Liebau > > > > > Partner > > > > > Tel. +49 179 7940878 > > > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - > Germany > > > > > > > > > > > > > > > > > > -- > > > Sönke Liebau > > > Partner > > > Tel. +49 179 7940878 > > > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany > > > > > > -- -- Guozhang