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
