Hi, Boyang,

Thanks for the KIP. Just a few comments on the metrics.

1. It would be useful to document the full JMX metric names (package, type,
etc) of the new metrics. Also, for rates on the server side, we
typically use Yammer Meter.

2. For num-messages-redirected-rate, would num-requests-redirected-rate be
better?

3. num-client-forwarding-to-controller-rate: Is that based on clientId,
client IP, client request version or sth else? How do you plan to implement
that since it seems to require tracking the current unique client set
somehow. An alternative approach is to maintain a
num-requests-redirected-rate metric with a client tag.

Jun



On Mon, Jun 8, 2020 at 9:36 AM Boyang Chen <reluctanthero...@gmail.com>
wrote:

> Hey there,
>
> If no more question is raised, I will go ahead and start the vote shortly.
>
> On Thu, Jun 4, 2020 at 12:39 PM Boyang Chen <reluctanthero...@gmail.com>
> wrote:
>
> > Hey there,
> >
> > bumping this thread for any further KIP-590 discussion, since it's been
> > quiet for a while.
> >
> > Boyang
> >
> > On Thu, May 21, 2020 at 10:31 AM Boyang Chen <reluctanthero...@gmail.com
> >
> > wrote:
> >
> >> Thanks David, I agree the wording here is not clear, and the fellow
> >> broker should just send a new CreateTopicRequest in this case.
> >>
> >> In the meantime, we had some offline discussion about the Envelope API
> as
> >> well. Although it provides certain privileges like data embedding and
> >> principal embedding, it creates a security hole by letting a malicious
> user
> >> impersonate any forwarding broker, thus pretending to be any admin user.
> >> Passing the principal around also increases the vulnerability, compared
> >> with other standard ways such as passing a verified token, but it is
> >> unfortunately not fully supported with Kafka security.
> >>
> >> So for the security concerns, we are abandoning the Envelope approach
> and
> >> fallback to just forward the raw admin requests. The authentication will
> >> happen on the receiving broker side instead of on the controller, so
> that
> >> we are able to stripped off the principal fields and only include the
> >> principal in header as optional field for audit logging purpose.
> >> Furthermore, we shall propose adding a separate endpoint for
> >> broker-controller communication which should be recommended to enable
> >> secure connections so that a malicious client could not pretend to be a
> >> broker and perform impersonation.
> >>
> >> Let me know your thoughts.
> >>
> >> Best,
> >> Boyang
> >>
> >> On Tue, May 19, 2020 at 12:17 AM David Jacot <dja...@confluent.io>
> wrote:
> >>
> >>> Hi Boyang,
> >>>
> >>> I've got another question regarding the auto topic creation case. The
> KIP
> >>> says: "Currently the target broker shall just utilize its own ZK client
> >>> to
> >>> create
> >>> internal topics, which is disallowed in the bridge release. For above
> >>> scenarios,
> >>> non-controller broker shall just forward a CreateTopicRequest to the
> >>> controller
> >>> instead and let controller take care of the rest, while waiting for the
> >>> response
> >>> in the meantime." There will be no request to forward in this case,
> >>> right?
> >>> Instead,
> >>> a CreateTopicsRequest is created and sent to the controller node.
> >>>
> >>> When the CreateTopicsRequest is sent as a side effect of the
> >>> MetadataRequest,
> >>> it would be good to know the principal and the clientId in the
> controller
> >>> (quota,
> >>> audit, etc.). Do you plan to use the Envelope API for this case as well
> >>> or
> >>> to call
> >>> the regular API directly? Another was to phrase it would be: Shall the
> >>> internal
> >>> CreateTopicsRequest be sent with the original metadata (principal,
> >>> clientId, etc.)
> >>> of the MetadataRequest or as an admin request?
> >>>
> >>> Best,
> >>> David
> >>>
> >>> On Fri, May 8, 2020 at 2:04 AM Guozhang Wang <wangg...@gmail.com>
> wrote:
> >>>
> >>> > 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
> >>> >
> >>>
> >>
>

Reply via email to