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
> >
>
