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