Hi Jun,
I think it's important to discuss the pluggable API versus the calling
implementation as two separate points.
>From an API perspective, are you suggesting that we should tell users that
they cannot block in authorize? Or are you saying that it's OK to block on
authorize on occasion and the recommendation would be for people to
increase the number of threads in the request thread pool? Architecturally,
this feels wrong in my opinion.
>From the calling implementation perspective, you don't need a callback. You
can basically say:
authorize(...).map { result =>
...
}
And then have a common method take that Future and handle it synchronously
if it's already complete or submit it to the purgatory if not. This is
similar to how many modern async web libraries work.
As Rajini said, this could be done later. The initial implementation could
simply do `toCompletableFuture().get()` to do it synchronously. But it
would mean that we could add this functionality without changing the
pluggable interface.
Ismael
On Wed, Sep 4, 2019 at 5:29 AM Jun Rao <j...@confluent.io> wrote:
> Hi, Rajini,
>
> Thanks for the KIP. I was concerned about #4 too. If we change the handling
> of all requests to use an async authorize() api, will that cause the code
> much harder to understand? There are quite a few callbacks already. I am
> not sure that we want to introduce more of those. The benefit from async
> authorize() api seems limited.
>
> Jun
>
> On Wed, Sep 4, 2019 at 5:38 PM Rajini Sivaram <rajinisiva...@gmail.com>
> wrote:
>
> > Hi Don,
> >
> > Thanks for your note.
> >
> > 1) The intention is to avoid blocking in the calling thread. We already
> > have several requests that are put into a purgatory when waiting for
> remote
> > communication, for example produce request waiting for replication. Since
> > we have a limited number of request threads in the broker, we want to
> make
> > progress with other requests, while one is waiting on any form of remote
> > communication.
> >
> > 2) Async management calls will be useful with the default authorizer when
> > KIP-500 removes ZK and we rely on Kafka instead. Our current ZK-based
> > implementation as well as any custom implementations that don't want to
> be
> > async will just need to return a sync'ed value. So instead of returning `
> > value`, the code would just return
> > `CompletableFuture.completedFuture(value)
> > `. So it would be just a single line change in the implementation with
> the
> > new API. The caller would treat completedFuture exactly as it does today,
> > processing the request synchronously without using a purgatory.
> >
> > 3) For implementations that return a completedFuture as described in 2),
> > the behaviour would remain exactly the same. No additional threads or
> > purgatory will be used for this case. So there would be no performance
> > penalty. For implementations that return a future that is not complete,
> we
> > prioritise running more requests concurrently. So in a deployment with a
> > large number of clients, we would improve performance by allowing other
> > requests to be processed on the request threads while some are waiting
> for
> > authorization metadata.
> >
> > 4) I was concerned about this too. The goal is to make the API flexible
> > enough to handle large scale deployments in future when caching all
> > authorization metadata in each broker is not viable. Using an async API
> > that returns CompletionStage, the caller has the option to handle the
> > result synchronously or asynchronously, so we don't necessarily need to
> > update the calling code right away. Custom authorizers using the async
> API
> > have full control over whether authorization is performed in-line since
> > completedFuture will always be handled synchronously. We do need to
> update
> > KafkaApis to take advantage of the asynchronous API to improve scale.
> Even
> > though this is a big change, since we will be doing the same for all
> > requests, it shouldn't be too hard to maintain since the same pattern
> will
> > be used for all requests.
> >
> > Regards,
> >
> > Rajini
> >
> > On Tue, Sep 3, 2019 at 11:48 PM Don Bosco Durai <bo...@apache.org>
> wrote:
> >
> > > Hi Rajini
> > >
> > > Help me understand this a bit more.
> > >
> > > 1. For all practical purpose, without authorization you can't go to the
> > > next step. The calling code needs to block anyway. So should we just
> let
> > > the implementation code do the async part?
> > > 2. If you feel management calls need to be async, then we should
> consider
> > > another set of async APIs. I don't feel we should complicate it
> further (
> > > 3. Another concern I have is wrt performance. Kafka has been built to
> > > handle 1000s per second requests. Not sure whether making it async will
> > add
> > > any unnecessary overhead.
> > > 4. How much complication would this add on the calling side? And is it
> > > worth it?
> > >
> > > Thanks
> > >
> > > Bosco
> > >
> > >
> > > On 9/3/19, 8:50 AM, "Rajini Sivaram" <rajinisiva...@gmail.com> wrote:
> > >
> > > Hi all,
> > >
> > > Ismael brought up a point that it will be good to make the
> Authorizer
> > > interface asynchronous to avoid blocking request threads during
> > remote
> > > operations.
> > >
> > > 1) Since we want to support different backends for authorization
> > > metadata,
> > > making createAcls() and deleteAcls() asynchronous makes sense since
> > > these
> > > always involve remote operations. When KIP-500 removes ZooKeeper,
> we
> > > would
> > > want to move ACLs to Kafka and async updates will avoid unnecessary
> > > blocking.
> > > 2) For authorize() method, we currently use cached ACLs in the
> > built-in
> > > authorizers, so synchronous authorise operations work well now. But
> > > async
> > > authorize() would support this scenario as well as authorizers in
> > large
> > > organisations where an LRU cache would enable a smaller cache even
> > > when the
> > > backend holds a large amount of ACLs for infrequently used
> resources
> > or
> > > users who don't use the system frequently.
> > >
> > > For both cases, the built-in authorizer will continue to be
> > > synchronous,
> > > using CompletableFuture.completedFuture() to return the actual
> > result.
> > > But
> > > async API will make custom authorizer implementations more
> flexible.
> > I
> > > would like to know if there are any concerns with these changes
> > before
> > > updating the KIP.
> > >
> > > *Proposed API:*
> > > public interface Authorizer extends Configurable, Closeable {
> > >
> > > Map<Endpoint, CompletionStage<Void>> start(AuthorizerServerInfo
> > > serverInfo);
> > > List<CompletionStage<AuthorizationResult>>
> > > authorize(AuthorizableRequestContext requestContext, List<Action>
> > > actions);
> > > List<CompletionStage<AclCreateResult>>
> > > createAcls(AuthorizableRequestContext requestContext,
> > List<AclBinding>
> > > aclBindings);
> > > List<CompletionStage<AclDeleteResult>>
> > > deleteAcls(AuthorizableRequestContext requestContext,
> > > List<AclBindingFilter> aclBindingFilters);
> > > CompletionStage<Collection<AclBinding>> acls(AclBindingFilter
> > > filter);
> > > }
> > >
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > > On Sun, Aug 18, 2019 at 6:25 PM Don Bosco Durai <bo...@apache.org>
> > > wrote:
> > >
> > > > Hi Rajini
> > > >
> > > > Thanks for clarifying. I am good for now.
> > > >
> > > > Regards
> > > >
> > > > Bosco
> > > >
> > > >
> > > > On 8/16/19, 11:30 AM, "Rajini Sivaram" <rajinisiva...@gmail.com
> >
> > > wrote:
> > > >
> > > > Hi Don,
> > > >
> > > > That should be fine. I guess Ranger loads policies from the
> > > database
> > > > synchronously when the authorizer is configured, similar to
> > > > SimpleAclAuthorizer loading from ZooKeeper. Ranger can
> continue
> > > to load
> > > > synchronously from `configure()` or `start()` and return an
> > > empty map
> > > > from
> > > > `start()`. That would retain the existing behaviour.. When
> the
> > > same
> > > > database stores policies for all listeners and the policies
> are
> > > not
> > > > stored
> > > > in Kafka, there is no value in making the load asynchronous.
> > > >
> > > > Regards,
> > > >
> > > > Rajini
> > > >
> > > >
> > > > On Fri, Aug 16, 2019 at 6:43 PM Don Bosco Durai <
> > > bo...@apache.org>
> > > > wrote:
> > > >
> > > > > Hi Rajini
> > > > >
> > > > > Assuming this doesn't affect custom plugins, I don't have
> any
> > > > concerns
> > > > > regarding this.
> > > > >
> > > > > I do have one question regarding:
> > > > >
> > > > > "For authorizers that don’t store metadata in ZooKeeper,
> > > ensure that
> > > > > authorizer metadata for each listener is available before
> > > starting
> > > > up the
> > > > > listener. This enables different authorization metadata
> > stores
> > > for
> > > > > different listeners."
> > > > >
> > > > > Since Ranger uses its own database for storing policies, do
> > you
> > > > anticipate
> > > > > any issues?
> > > > >
> > > > > Thanks
> > > > >
> > > > > Bosco
> > > > >
> > > > >
> > > > > On 8/16/19, 6:49 AM, "Rajini Sivaram" <
> > > rajinisiva...@gmail.com>
> > > > wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > I made another change to the KIP. The KIP was
> originally
> > > > proposing to
> > > > > extend SimpleAclAuthorizer to also implement the new
> API
> > > (in
> > > > addition
> > > > > to
> > > > > the existing API). But since we use the new API when
> > > available,
> > > > this
> > > > > can
> > > > > break custom authorizers that extend this class and
> > > override
> > > > specific
> > > > > methods of the old API. To avoid breaking any existing
> > > custom
> > > > > implementations that extend this class, particularly
> > > because it
> > > > is in
> > > > > the
> > > > > public package kafka.security.auth, the KIP now
> proposes
> > to
> > > > retain the
> > > > > old
> > > > > authorizer as-is. SimpleAclAuthorizer will continue to
> > > > implement the
> > > > > old
> > > > > API, but will be deprecated. A new authorizer
> > > implementation
> > > > > kafka.security.authorizer.AclAuthorizer will be added
> for
> > > the
> > > > new API
> > > > > (this
> > > > > will not be in the public package).
> > > > >
> > > > > Please let me know if you have any concerns.
> > > > >
> > > > > Regards,
> > > > >
> > > > > Rajini
> > > > >
> > > > >
> > > > > On Fri, Aug 16, 2019 at 8:48 AM Rajini Sivaram <
> > > > > rajinisiva...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Thanks Colin.
> > > > > >
> > > > > > If there are no other concerns, I will start vote
> later
> > > today.
> > > > Many
> > > > > thanks
> > > > > > to every one for the feedback.
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > >
> > > > > > On Thu, Aug 15, 2019 at 11:57 PM Colin McCabe <
> > > > cmcc...@apache.org>
> > > > > wrote:
> > > > > >
> > > > > >> Thanks, Rajini. It looks good to me.
> > > > > >>
> > > > > >> best,
> > > > > >> Colin
> > > > > >>
> > > > > >>
> > > > > >> On Thu, Aug 15, 2019, at 11:37, Rajini Sivaram
> wrote:
> > > > > >> > Hi Colin,
> > > > > >> >
> > > > > >> > Thanks for the review. I have updated the KIP to
> > move
> > > the
> > > > > interfaces for
> > > > > >> > request context and server info to the authorizer
> > > package.
> > > > These
> > > > > are now
> > > > > >> > called AuthorizableRequestContext and
> > > AuthorizerServerInfo.
> > > > > Endpoint is
> > > > > >> now
> > > > > >> > a class in org.apache.kafka.common to make it
> > reusable
> > > > since we
> > > > > already
> > > > > >> > have multiple implementations of it. I have
> removed
> > > > requestName
> > > > > from the
> > > > > >> > request context interface since authorizers can
> > > distinguish
> > > > > follower
> > > > > >> fetch
> > > > > >> > and consumer fetch from the operation being
> > > authorized. So
> > > > 16-bit
> > > > > >> request
> > > > > >> > type should be sufficient for audit logging. Also
> > > replaced
> > > > > AuditFlag
> > > > > >> with
> > > > > >> > two booleans as you suggested.
> > > > > >> >
> > > > > >> > Can you take another look and see if the KIP is
> > ready
> > > for
> > > > voting?
> > > > > >> >
> > > > > >> > Thanks for all your help!
> > > > > >> >
> > > > > >> > Regards,
> > > > > >> >
> > > > > >> > Rajini
> > > > > >> >
> > > > > >> > On Wed, Aug 14, 2019 at 8:59 PM Colin McCabe <
> > > > cmcc...@apache.org>
> > > > > >> wrote:
> > > > > >> >
> > > > > >> > > Hi Rajini,
> > > > > >> > >
> > > > > >> > > I think it would be good to rename
> > > KafkaRequestContext to
> > > > > something
> > > > > >> like
> > > > > >> > > AuthorizableRequestContext, and put it in the
> > > > > >> > > org.apache.kafka.server.authorizer namespace.
> If
> > > we put
> > > > it in
> > > > > the
> > > > > >> > > org.apache.kafka.common namespace, then it's not
> > > really
> > > > clear
> > > > > that
> > > > > >> it's
> > > > > >> > > part of the Authorizer API. Since this class is
> > > purely an
> > > > > interface,
> > > > > >> with
> > > > > >> > > no concrete implementation of anything, there's
> > > nothing
> > > > common
> > > > > to
> > > > > >> really
> > > > > >> > > reuse in any case. We definitely don't want
> > > someone to
> > > > > accidentally
> > > > > >> add or
> > > > > >> > > remove methods because they think this is just
> > > another
> > > > internal
> > > > > class
> > > > > >> used
> > > > > >> > > for requests.
> > > > > >> > >
> > > > > >> > > The BrokerInfo class is a nice improvement. It
> > > looks
> > > > like it
> > > > > will be
> > > > > >> > > useful for passing in information about the
> > context
> > > we're
> > > > > running
> > > > > >> in. It
> > > > > >> > > would be nice to call this class ServerInfo
> rather
> > > than
> > > > > BrokerInfo,
> > > > > >> since
> > > > > >> > > we will want to run the authorizer on
> controllers
> > > as well
> > > > as on
> > > > > >> brokers,
> > > > > >> > > and the controller may run as a separate process
> > > post
> > > > KIP-500.
> > > > > I also
> > > > > >> > > think that this class should be in the
> > > > > >> org.apache.kafka.server.authorizer
> > > > > >> > > namespace. Again, it is an interface, not a
> > > concrete
> > > > > implementation,
> > > > > >> and
> > > > > >> > > it's an interface that is very specifically for
> > the
> > > > authorizer.
> > > > > >> > >
> > > > > >> > > I agree that we probably don't have enough
> > > information
> > > > > preserved for
> > > > > >> > > requests currently to always know what entity
> made
> > > them.
> > > > So we
> > > > > can
> > > > > >> leave
> > > > > >> > > that out for now (except in the special case of
> > > Fetch).
> > > > > Perhaps we
> > > > > >> can add
> > > > > >> > > this later if it's needed.
> > > > > >> > >
> > > > > >> > > I understand the intention behind
> > AuthorizationMode
> > > > (which is
> > > > > now
> > > > > >> called
> > > > > >> > > AuditFlag in the latest revision). But it still
> > > feels
> > > > > complex. What
> > > > > >> if we
> > > > > >> > > just had two booleans in Action: logSuccesses
> and
> > > > logFailures?
> > > > > That
> > > > > >> seems
> > > > > >> > > to cover all the cases here.
> MANDATORY_AUTHORIZE
> > =
> > > true,
> > > > true.
> > > > > >> > > OPTIONAL_AUTHORIZE = true, false. FILTER =
> true,
> > > false.
> > > > > >> LIST_AUTHORIZED =
> > > > > >> > > false, false. Would there be anything lost
> versus
> > > having
> > > > the
> > > > > enum?
> > > > > >> > >
> > > > > >> > > best,
> > > > > >> > > Colin
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > On Wed, Aug 14, 2019, at 06:29, Mickael Maison
> > > wrote:
> > > > > >> > > > Hi Rajini,
> > > > > >> > > >
> > > > > >> > > > Thanks for the KIP!
> > > > > >> > > > I really like that authorize() will be able to
> > > take a
> > > > batch of
> > > > > >> > > > requests, this will speed up many
> > implementations!
> > > > > >> > > >
> > > > > >> > > > On Tue, Aug 13, 2019 at 5:57 PM Rajini
> Sivaram <
> > > > > >> rajinisiva...@gmail.com>
> > > > > >> > > wrote:
> > > > > >> > > > >
> > > > > >> > > > > Thanks David! I have fixed the typo.
> > > > > >> > > > >
> > > > > >> > > > > Also made a couple of changes to make the
> > > context
> > > > > interfaces more
> > > > > >> > > generic.
> > > > > >> > > > > KafkaRequestContext now returns the 16-bit
> API
> > > key as
> > > > Colin
> > > > > >> suggested
> > > > > >> > > as
> > > > > >> > > > > well as the friendly name used in metrics
> > which
> > > are
> > > > useful
> > > > > in
> > > > > >> audit
> > > > > >> > > logs.
> > > > > >> > > > > `Authorizer#start` is now provided a generic
> > > > `BrokerInfo`
> > > > > >> interface
> > > > > >> > > that
> > > > > >> > > > > gives cluster id, broker id and endpoint
> > > information.
> > > > The
> > > > > generic
> > > > > >> > > interface
> > > > > >> > > > > can potentially be used in other broker
> > plugins
> > > in
> > > > future
> > > > > and
> > > > > >> provides
> > > > > >> > > > > dynamically generated configs like broker id
> > > and ports
> > > > > which are
> > > > > >> > > currently
> > > > > >> > > > > not available to plugins unless these
> configs
> > > are
> > > > statically
> > > > > >> > > configured.
> > > > > >> > > > > Please let me know if there are any
> concerns.
> > > > > >> > > > >
> > > > > >> > > > > Regards,
> > > > > >> > > > >
> > > > > >> > > > > Rajini
> > > > > >> > > > >
> > > > > >> > > > > On Tue, Aug 13, 2019 at 4:30 PM David Jacot
> <
> > > > > dja...@confluent.io>
> > > > > >> > > wrote:
> > > > > >> > > > >
> > > > > >> > > > > > Hi Rajini,
> > > > > >> > > > > >
> > > > > >> > > > > > Thank you for the update! It looks good to
> > me.
> > > > There is a
> > > > > typo
> > > > > >> in the
> > > > > >> > > > > > `AuditFlag` enum: `MANDATORY_AUTHOEIZE` ->
> > > > > >> `MANDATORY_AUTHORIZE`.
> > > > > >> > > > > >
> > > > > >> > > > > > Regards,
> > > > > >> > > > > > David
> > > > > >> > > > > >
> > > > > >> > > > > > On Mon, Aug 12, 2019 at 2:54 PM Rajini
> > > Sivaram <
> > > > > >> > > rajinisiva...@gmail.com>
> > > > > >> > > > > > wrote:
> > > > > >> > > > > >
> > > > > >> > > > > > > Hi David,
> > > > > >> > > > > > >
> > > > > >> > > > > > > Thanks for reviewing the KIP! Since
> > > questions
> > > > about
> > > > > >> `authorization
> > > > > >> > > mode`
> > > > > >> > > > > > > and `count` have come up multiple
> times, I
> > > have
> > > > renamed
> > > > > both.
> > > > > >> > > > > > >
> > > > > >> > > > > > > 1) Renamed `count` to
> > > `resourceReferenceCount`.
> > > > It is
> > > > > the
> > > > > >> number
> > > > > >> > > of times
> > > > > >> > > > > > > the resource being authorized is
> > referenced
> > > > within the
> > > > > >> request.
> > > > > >> > > > > > >
> > > > > >> > > > > > > 2) Renamed `AuthorizationMode` to
> > > `AuditFlag`. It
> > > > is
> > > > > provided
> > > > > >> to
> > > > > >> > > improve
> > > > > >> > > > > > > audit logging in the authorizer. The
> enum
> > > values
> > > > have
> > > > > javadoc
> > > > > >> which
> > > > > >> > > > > > > indicate how the authorization result is
> > > used in
> > > > each
> > > > > of the
> > > > > >> modes
> > > > > >> > > to
> > > > > >> > > > > > > enable authorizers to log audit messages
> > at
> > > the
> > > > right
> > > > > severity
> > > > > >> > > level.
> > > > > >> > > > > > >
> > > > > >> > > > > > > Regards,
> > > > > >> > > > > > >
> > > > > >> > > > > > > Rajini
> > > > > >> > > > > > >
> > > > > >> > > > > > > On Mon, Aug 12, 2019 at 5:54 PM David
> > Jacot
> > > <
> > > > > >> dja...@confluent.io>
> > > > > >> > > wrote:
> > > > > >> > > > > > >
> > > > > >> > > > > > > > Hi Rajini,
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > Thank you for the KIP. Overall, it
> looks
> > > good
> > > > to me.
> > > > > I have
> > > > > >> few
> > > > > >> > > > > > > > questions/suggestions:
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > 1. It is hard to grasp what
> > > `Action#count` is
> > > > for. I
> > > > > guess I
> > > > > >> > > understand
> > > > > >> > > > > > > > where you want to go with it but it
> took
> > > me a
> > > > while to
> > > > > >> figure it
> > > > > >> > > out.
> > > > > >> > > > > > > > Perhaps, we could come up with a
> better
> > > name
> > > > than
> > > > > `count`?
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > 2. I had a hard time trying to
> > understand
> > > the
> > > > > >> `AuthorizationMode`
> > > > > >> > > > > > > concept,
> > > > > >> > > > > > > > especially wrt. the OPTIONAL one. My
> > > > understanding is
> > > > > that
> > > > > >> an
> > > > > >> > > ACL is
> > > > > >> > > > > > > either
> > > > > >> > > > > > > > defined or not. Could you elaborate a
> > bit
> > > more
> > > > on
> > > > > that?
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > Thanks,
> > > > > >> > > > > > > > David
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > On Fri, Aug 9, 2019 at 10:26 PM Don
> > Bosco
> > > Durai
> > > > <
> > > > > >> > > bo...@apache.org>
> > > > > >> > > > > > > wrote:
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > > Hi Rajini
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > 3.2 - This makes sense. Thanks for
> > > clarifying.
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > Rest looks fine. Once the
> > > implementations are
> > > > done,
> > > > > it
> > > > > >> will be
> > > > > >> > > more
> > > > > >> > > > > > > clear
> > > > > >> > > > > > > > > on the different values RequestType
> > and
> > > Mode.
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > Thanks
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > Bosco
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > On 8/9/19, 5:08 AM, "Rajini
> Sivaram"
> > <
> > > > > >> rajinisiva...@gmail.com
> > > > > >> > > >
> > > > > >> > > > > > wrote:
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > Hi Don,
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > Thanks for the suggestions. A
> few
> > > > responses
> > > > > below:
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > 3.1 Can rename and improve docs
> if
> > > we keep
> > > > > this. Let's
> > > > > >> > > finish the
> > > > > >> > > > > > > > > discussion on Colin's
> suggestions
> > > > regarding this
> > > > > >> first.
> > > > > >> > > > > > > > > 3.2 No, I was thinking of some
> > > requests
> > > > that
> > > > > have an
> > > > > >> old
> > > > > >> > > way of
> > > > > >> > > > > > > > > authorizing
> > > > > >> > > > > > > > > and a new way where we have
> > > retained the
> > > > old
> > > > > way for
> > > > > >> > > backward
> > > > > >> > > > > > > > > compatibility. One example is
> > > > Cluster:Create
> > > > > >> permission to
> > > > > >> > > create
> > > > > >> > > > > > > > > topics.
> > > > > >> > > > > > > > > We have replaced this with
> > > fine-grained
> > > > topic
> > > > > create
> > > > > >> > > access using
> > > > > >> > > > > > > > > Topic:Create
> > > > > >> > > > > > > > > for topic patterns. But we still
> > > check if
> > > > user
> > > > > has
> > > > > >> > > Cluster:Create
> > > > > >> > > > > > > > > first. If
> > > > > >> > > > > > > > > Denied, the deny is ignored and
> we
> > > check
> > > > > >> Topic:Create. We
> > > > > >> > > dont
> > > > > >> > > > > > want
> > > > > >> > > > > > > > to
> > > > > >> > > > > > > > > log
> > > > > >> > > > > > > > > DENY for Cluster:Create at INFO
> > > level for
> > > > this,
> > > > > since
> > > > > >> this
> > > > > >> > > is
> > > > > >> > > > > > not a
> > > > > >> > > > > > > > > mandatory ACL for creating
> topics.
> > > We
> > > > will get
> > > > > >> appropriate
> > > > > >> > > logs
> > > > > >> > > > > > > from
> > > > > >> > > > > > > > > the
> > > > > >> > > > > > > > > subsequent Topic:Create in this
> > > case.
> > > > > >> > > > > > > > > 3.3 They are not quite the same.
> > > FILTER
> > > > implies
> > > > > that
> > > > > >> user
> > > > > >> > > > > > actually
> > > > > >> > > > > > > > > requested access to and
> performed
> > > some
> > > > > operation on
> > > > > >> the
> > > > > >> > > filtered
> > > > > >> > > > > > > > > resources.
> > > > > >> > > > > > > > > LIST_AUTHORZED did not result in
> > any
> > > > actual
> > > > > access.
> > > > > >> User
> > > > > >> > > simply
> > > > > >> > > > > > > > wanted
> > > > > >> > > > > > > > > to
> > > > > >> > > > > > > > > know what they are allowed to
> > > access.
> > > > > >> > > > > > > > > 3.4 Request types are Produce,
> > > JoinGroup,
> > > > > OffsetCommit
> > > > > >> > > etc. So
> > > > > >> > > > > > that
> > > > > >> > > > > > > > is
> > > > > >> > > > > > > > > different from authorization
> mode,
> > > > operation
> > > > > etc.
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > On Thu, Aug 8, 2019 at 11:36 PM
> > Don
> > > Bosco
> > > > Durai
> > > > > <
> > > > > >> > > > > > bo...@apache.org>
> > > > > >> > > > > > > > > wrote:
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > > Hi Rajini
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > Thanks for clarifying. This is
> > > very
> > > > helpful...
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > #1 - On the Ranger side, we
> > > should be
> > > > able to
> > > > > handle
> > > > > >> > > multiple
> > > > > >> > > > > > > > > requests at
> > > > > >> > > > > > > > > > the same time. I was just not
> > > sure how
> > > > much
> > > > > >> processing
> > > > > >> > > overhead
> > > > > >> > > > > > > > will
> > > > > >> > > > > > > > > be
> > > > > >> > > > > > > > > > there on the Broker side to
> > split
> > > and
> > > > then
> > > > > >> consolidate
> > > > > >> > > the
> > > > > >> > > > > > > results.
> > > > > >> > > > > > > > > If it
> > > > > >> > > > > > > > > > is negligible, then this is
> the
> > > better
> > > > way.
> > > > > It will
> > > > > >> > > make it
> > > > > >> > > > > > > future
> > > > > >> > > > > > > > > proof.
> > > > > >> > > > > > > > > > #2 - I agree, having a single
> > > "start"
> > > > call
> > > > > makes it
> > > > > >> > > cleaner.
> > > > > >> > > > > > The
> > > > > >> > > > > > > > > > Authorization implementation
> > will
> > > only
> > > > have
> > > > > to do
> > > > > >> the
> > > > > >> > > > > > > > initialization
> > > > > >> > > > > > > > > only
> > > > > >> > > > > > > > > > once.
> > > > > >> > > > > > > > > > #3.1 - Thanks for the
> > > clarification. I
> > > > think
> > > > > it
> > > > > >> makes
> > > > > >> > > sense to
> > > > > >> > > > > > > have
> > > > > >> > > > > > > > > this.
> > > > > >> > > > > > > > > > The term "Mode" might not be
> > > explicit
> > > > enough.
> > > > > >> > > Essentially it
> > > > > >> > > > > > > seems
> > > > > >> > > > > > > > > you want
> > > > > >> > > > > > > > > > the Authorizer to know the
> > > > Intent/Purpose of
> > > > > the
> > > > > >> > > authorize call
> > > > > >> > > > > > > and
> > > > > >> > > > > > > > > let the
> > > > > >> > > > > > > > > > Authorizer decide what to log
> as
> > > Audit
> > > > event.
> > > > > >> Changing
> > > > > >> > > the
> > > > > >> > > > > > > > > class/field name
> > > > > >> > > > > > > > > > or giving more documentation
> > will
> > > do.
> > > > > >> > > > > > > > > > #3.2 - Regarding the option
> > > "OPTIONAL".
> > > > Are
> > > > > you
> > > > > >> thinking
> > > > > >> > > from
> > > > > >> > > > > > > > > chaining
> > > > > >> > > > > > > > > > multiple Authorizer? If so, I
> > am
> > > not
> > > > sure
> > > > > whether
> > > > > >> the
> > > > > >> > > Broker
> > > > > >> > > > > > > would
> > > > > >> > > > > > > > > have
> > > > > >> > > > > > > > > > enough information to make
> that
> > > > decision. I
> > > > > feel the
> > > > > >> > > Authorizer
> > > > > >> > > > > > > > will
> > > > > >> > > > > > > > > be the
> > > > > >> > > > > > > > > > one who would have that
> > > knowledge. E.g.
> > > > in
> > > > > Ranger
> > > > > >> we have
> > > > > >> > > > > > > explicit
> > > > > >> > > > > > > > > deny,
> > > > > >> > > > > > > > > > which means no matter what,
> the
> > > request
> > > > > should be
> > > > > >> denied
> > > > > >> > > for
> > > > > >> > > > > > the
> > > > > >> > > > > > > > > user/group
> > > > > >> > > > > > > > > > or condition. So if you are
> > > thinking of
> > > > > chaining
> > > > > >> > > Authorizers,
> > > > > >> > > > > > > then
> > > > > >> > > > > > > > > > responses should have the
> third
> > > state,
> > > > e.g.
> > > > > >> > > "DENIED_FINAL", in
> > > > > >> > > > > > > > which
> > > > > >> > > > > > > > > case
> > > > > >> > > > > > > > > > if there is an Authorization
> > > chain, it
> > > > will
> > > > > be stop
> > > > > >> and
> > > > > >> > > the
> > > > > >> > > > > > > request
> > > > > >> > > > > > > > > will be
> > > > > >> > > > > > > > > > denied and if it is just
> denied,
> > > then
> > > > you
> > > > > might fall
> > > > > >> > > back to
> > > > > >> > > > > > next
> > > > > >> > > > > > > > > > authorizer. If we don't have
> > > chaining of
> > > > > >> Authorizing in
> > > > > >> > > mind,
> > > > > >> > > > > > > then
> > > > > >> > > > > > > > we
> > > > > >> > > > > > > > > > should reconsider OPTIONAL for
> > > now. Or
> > > > > clarify under
> > > > > >> > > which
> > > > > >> > > > > > > scenario
> > > > > >> > > > > > > > > > OPTIONAL will be called.
> > > > > >> > > > > > > > > > #3.3 Regarding, FILTER v/s
> > > > LIST_AUTHORIZED,
> > > > > isn't
> > > > > >> > > > > > > LIST_AUTHORIZED a
> > > > > >> > > > > > > > > > special case for "FILTER"?
> > > > > >> > > > > > > > > > #3.4 KafkaRequestContext.
> > > requestType()
> > > > v/s
> > > > > Action.
> > > > > >> > > > > > > > > authorizationMode. I
> > > > > >> > > > > > > > > > am not sure about the overlap
> or
> > > > ambiguity.
> > > > > >> > > > > > > > > > #4 - Cool. This is good, it
> will
> > > be less
> > > > > stress on
> > > > > >> the
> > > > > >> > > > > > > Authorizer.
> > > > > >> > > > > > > > > Ranger
> > > > > >> > > > > > > > > > already supports the "count"
> > > concept
> > > > and also
> > > > > has
> > > > > >> > > batching
> > > > > >> > > > > > > > > capability to
> > > > > >> > > > > > > > > > aggregate similar requests to
> > > reduce the
> > > > > number of
> > > > > >> audit
> > > > > >> > > logs
> > > > > >> > > > > > to
> > > > > >> > > > > > > > > write. We
> > > > > >> > > > > > > > > > should be able to pass this
> > > through.
> > > > > >> > > > > > > > > > #5 - Assuming if the object
> > > instance is
> > > > going
> > > > > out of
> > > > > >> > > scope, we
> > > > > >> > > > > > > > > should be
> > > > > >> > > > > > > > > > fine. Not a super important
> ask.
> > > Ranger
> > > > is
> > > > > already
> > > > > >> > > catching
> > > > > >> > > > > > KILL
> > > > > >> > > > > > > > > signal for
> > > > > >> > > > > > > > > > clean up.
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > Thanks again. These are good
> > > > enhancements.
> > > > > >> Appreciate
> > > > > >> > > your
> > > > > >> > > > > > > efforts
> > > > > >> > > > > > > > > here.
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > Bosco
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > On 8/8/19, 2:03 AM, "Rajini
> > > Sivaram" <
> > > > > >> > > rajinisiva...@gmail.com
> > > > > >> > > > > > >
> > > > > >> > > > > > > > > wrote:
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > Hi Don,
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > Thanks for reviewing the
> > KIP.
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > 1. I had this originally
> as
> > a
> > > single
> > > > > Action, but
> > > > > >> > > thought it
> > > > > >> > > > > > > may
> > > > > >> > > > > > > > > be
> > > > > >> > > > > > > > > > useful
> > > > > >> > > > > > > > > > to support batched
> authorize
> > > calls
> > > > as
> > > > > well and
> > > > > >> keep
> > > > > >> > > it
> > > > > >> > > > > > > > > consistent with
> > > > > >> > > > > > > > > > other methods. Single
> > > requests can
> > > > contain
> > > > > >> multiple
> > > > > >> > > topics.
> > > > > >> > > > > > > For
> > > > > >> > > > > > > > > > example a
> > > > > >> > > > > > > > > > produce request can
> contain
> > > records
> > > > for
> > > > > several
> > > > > >> > > partitions
> > > > > >> > > > > > of
> > > > > >> > > > > > > > > different
> > > > > >> > > > > > > > > > topics. Broker could
> > > potentially
> > > > > authorize these
> > > > > >> > > together.
> > > > > >> > > > > > > For
> > > > > >> > > > > > > > > > SimpleAclAuthorizer,
> batched
> > > > authorize
> > > > > methods
> > > > > >> don't
> > > > > >> > > > > > provide
> > > > > >> > > > > > > > any
> > > > > >> > > > > > > > > > optimisation since lookup
> is
> > > based
> > > > on
> > > > > resources
> > > > > >> > > followed by
> > > > > >> > > > > > > the
> > > > > >> > > > > > > > > > matching
> > > > > >> > > > > > > > > > logic. But some
> authorizers
> > > may
> > > > manage
> > > > > ACLs by
> > > > > >> user
> > > > > >> > > > > > principal
> > > > > >> > > > > > > > > rather
> > > > > >> > > > > > > > > > than
> > > > > >> > > > > > > > > > resource and may be able
> to
> > > optimize
> > > > > batched
> > > > > >> > > requests. I am
> > > > > >> > > > > > > ok
> > > > > >> > > > > > > > > with
> > > > > >> > > > > > > > > > using
> > > > > >> > > > > > > > > > single Action if this is
> > > likely to
> > > > cause
> > > > > issues.
> > > > > >> > > > > > > > > > 2. If you have two
> > listeners,
> > > one
> > > > for
> > > > > >> inter-broker
> > > > > >> > > traffic
> > > > > >> > > > > > > and
> > > > > >> > > > > > > > > another
> > > > > >> > > > > > > > > > for
> > > > > >> > > > > > > > > > external clients, start
> > > method is
> > > > invoked
> > > > > twice,
> > > > > >> > > once for
> > > > > >> > > > > > > each
> > > > > >> > > > > > > > > > listener. On
> > > > > >> > > > > > > > > > second thought, that may
> be
> > > > confusing and
> > > > > a
> > > > > >> single
> > > > > >> > > start()
> > > > > >> > > > > > > > > invocation
> > > > > >> > > > > > > > > > that
> > > > > >> > > > > > > > > > provides all listener
> > > information
> > > > and
> > > > > returns
> > > > > >> > > multiple
> > > > > >> > > > > > > futures
> > > > > >> > > > > > > > > would be
> > > > > >> > > > > > > > > > better. Will update the
> KIP.
> > > > > >> > > > > > > > > > 3. A typical example is a
> > > consumer
> > > > > subscribing
> > > > > >> to a
> > > > > >> > > regex
> > > > > >> > > > > > > > > pattern. We
> > > > > >> > > > > > > > > > request all topic metadata
> > > from the
> > > > > broker in
> > > > > >> order
> > > > > >> > > to
> > > > > >> > > > > > decide
> > > > > >> > > > > > > > > whether
> > > > > >> > > > > > > > > > the
> > > > > >> > > > > > > > > > pattern matches, expecting
> > to
> > > > receive a
> > > > > list of
> > > > > >> > > authorised
> > > > > >> > > > > > > > > topics. The
> > > > > >> > > > > > > > > > user
> > > > > >> > > > > > > > > > is not asking to subscribe
> > to
> > > an
> > > > > unauthorized
> > > > > >> topic.
> > > > > >> > > If
> > > > > >> > > > > > there
> > > > > >> > > > > > > > > are 10000
> > > > > >> > > > > > > > > > topics in the cluster and
> > the
> > > user
> > > > has
> > > > > access
> > > > > >> to 100
> > > > > >> > > of
> > > > > >> > > > > > them,
> > > > > >> > > > > > > > at
> > > > > >> > > > > > > > > the
> > > > > >> > > > > > > > > > moment
> > > > > >> > > > > > > > > > we log 9900 DENIED log
> > > entries at
> > > > INFO
> > > > > level in
> > > > > >> > > > > > > > > SimpleAclAuthorizer.
> > > > > >> > > > > > > > > > The
> > > > > >> > > > > > > > > > proposal is to authorize
> > this
> > > > request with
> > > > > >> > > > > > > > > AuthorizationMode.FILTER, so
> > > > > >> > > > > > > > > > that authorizers can log
> > > resources
> > > > that
> > > > > are
> > > > > >> filtered
> > > > > >> > > out at
> > > > > >> > > > > > > > > lower level
> > > > > >> > > > > > > > > > like DEBUG since this is
> not
> > > an
> > > > attempt to
> > > > > >> access
> > > > > >> > > > > > > unauthorized
> > > > > >> > > > > > > > > > resources.
> > > > > >> > > > > > > > > > Brokers already handle
> these
> > > > differently
> > > > > since
> > > > > >> no
> > > > > >> > > > > > > authorization
> > > > > >> > > > > > > > > error
> > > > > >> > > > > > > > > > is
> > > > > >> > > > > > > > > > returned to the client in
> > > these
> > > > cases.
> > > > > Providing
> > > > > >> > > > > > > authorization
> > > > > >> > > > > > > > > mode to
> > > > > >> > > > > > > > > > authorizers enables
> > authorizer
> > > > > implementations
> > > > > >> to
> > > > > >> > > generate
> > > > > >> > > > > > > > > better audit
> > > > > >> > > > > > > > > > logs.
> > > > > >> > > > > > > > > > 4. Each request may
> contain
> > > multiple
> > > > > instances
> > > > > >> of
> > > > > >> > > the same
> > > > > >> > > > > > > > > authorizable
> > > > > >> > > > > > > > > > resource. For example a
> > > produce
> > > > request
> > > > > may
> > > > > >> contain
> > > > > >> > > records
> > > > > >> > > > > > > for
> > > > > >> > > > > > > > > 10
> > > > > >> > > > > > > > > > partitions of the same
> > topic.
> > > At the
> > > > > moment, we
> > > > > >> > > invoke
> > > > > >> > > > > > > > authorize
> > > > > >> > > > > > > > > > method 10
> > > > > >> > > > > > > > > > times. The proposal is to
> > > invoke it
> > > > once
> > > > > with
> > > > > >> > > count=10. The
> > > > > >> > > > > > > > > count is
> > > > > >> > > > > > > > > > provided to authorizer
> just
> > > for
> > > > audit
> > > > > logging
> > > > > >> > > purposes.
> > > > > >> > > > > > > > > > 5. Authorizer implements
> > > Closeable,
> > > > so you
> > > > > >> could use
> > > > > >> > > > > > close()
> > > > > >> > > > > > > to
> > > > > >> > > > > > > > > flush
> > > > > >> > > > > > > > > > audits?
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > On Thu, Aug 8, 2019 at
> 7:01
> > > AM Don
> > > > Bosco
> > > > > Durai <
> > > > > >> > > > > > > > bo...@apache.org
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > wrote:
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > > Rajini
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Thanks for putting this
> > > together.
> > > > It is
> > > > > >> looking
> > > > > >> > > good. I
> > > > > >> > > > > > > have
> > > > > >> > > > > > > > > few
> > > > > >> > > > > > > > > > > questions...
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > 1.
> > List<AuthorizationResult>
> > > > > authorize(...,
> > > > > >> > > List<Action>
> > > > > >> > > > > > > > > actions).
> > > > > >> > > > > > > > > > Do you
> > > > > >> > > > > > > > > > > see a scenario where the
> > > broker
> > > > will
> > > > > call
> > > > > >> > > authorize for
> > > > > >> > > > > > > > > multiple
> > > > > >> > > > > > > > > > topics at
> > > > > >> > > > > > > > > > > the same time? I can
> > > understand
> > > > that
> > > > > during
> > > > > >> > > > > > > creating/deleting
> > > > > >> > > > > > > > > ACLS,
> > > > > >> > > > > > > > > > > multiple permissions for
> > > multiple
> > > > > resources
> > > > > >> might
> > > > > >> > > be
> > > > > >> > > > > > done.
> > > > > >> > > > > > > > For
> > > > > >> > > > > > > > > > authorize
> > > > > >> > > > > > > > > > > call, would this be a
> > case?
> > > And
> > > > does the
> > > > > >> Authorize
> > > > > >> > > > > > > > > implementation
> > > > > >> > > > > > > > > > will be
> > > > > >> > > > > > > > > > > able to do performance
> > > > optimization
> > > > > because of
> > > > > >> > > this? Or
> > > > > >> > > > > > > > should
> > > > > >> > > > > > > > > we
> > > > > >> > > > > > > > > > just keep
> > > > > >> > > > > > > > > > > it simple? I don't see
> it
> > > as an
> > > > issue
> > > > > from
> > > > > >> Apache
> > > > > >> > > Ranger
> > > > > >> > > > > > > > side,
> > > > > >> > > > > > > > > but
> > > > > >> > > > > > > > > > just
> > > > > >> > > > > > > > > > > checking to see whether
> we
> > > need
> > > > to be
> > > > > aware of
> > > > > >> > > something.
> > > > > >> > > > > > > > > > > 2. Should I assume that
> > the
> > > > > SecurityProtocol
> > > > > >> passed
> > > > > >> > > > > > during
> > > > > >> > > > > > > > > start and
> > > > > >> > > > > > > > > > the
> > > > > >> > > > > > > > > > > one return by
> > > > > >> > > KafkaRequestContext.securityProtocol() will
> > > > > >> > > > > > > be
> > > > > >> > > > > > > > > the
> > > > > >> > > > > > > > > > same?
> > > > > >> > > > > > > > > > > CompletableFuture<Void>
> > > > start(String
> > > > > >> listenerName,
> > > > > >> > > > > > > > > SecurityProtocol
> > > > > >> > > > > > > > > > > securityProtocol);
> > > > > >> > > > > > > > > > >
> > > > KafkaRequestContext.securityProtocol()
> > > > > >> > > > > > > > > > > 3. What is the purpose
> of
> > > > > AuthorizationMode?
> > > > > >> How
> > > > > >> > > does the
> > > > > >> > > > > > > > > broker
> > > > > >> > > > > > > > > > decide
> > > > > >> > > > > > > > > > > what mode to use when
> the
> > > > authorize()
> > > > > method
> > > > > >> is
> > > > > >> > > called?
> > > > > >> > > > > > > > > > > 4. Can we clarify
> "count"
> > in
> > > > Action a
> > > > > bit
> > > > > >> more?
> > > > > >> > > How is it
> > > > > >> > > > > > > > used?
> > > > > >> > > > > > > > > > > 5. Do you feel having
> > "stop"
> > > > along with
> > > > > >> "start" be
> > > > > >> > > > > > helpful?
> > > > > >> > > > > > > > > E.g. In
> > > > > >> > > > > > > > > > Ranger
> > > > > >> > > > > > > > > > > we try to optimize the
> > Audit
> > > > writing by
> > > > > >> caching
> > > > > >> > > the logs
> > > > > >> > > > > > > for
> > > > > >> > > > > > > > a
> > > > > >> > > > > > > > > fixed
> > > > > >> > > > > > > > > > > interval. But when the
> > > Broker
> > > > > terminates, we
> > > > > >> do a
> > > > > >> > > forced
> > > > > >> > > > > > > > flush.
> > > > > >> > > > > > > > > > Having an
> > > > > >> > > > > > > > > > > explicit "stop" might
> give
> > > us a
> > > > formal
> > > > > way to
> > > > > >> > > flush our
> > > > > >> > > > > > > > audits.
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Thanks
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Bosco
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > On 8/7/19, 3:59 PM,
> > "Rajini
> > > > Sivaram" <
> > > > > >> > > > > > > > rajinisiva...@gmail.com
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > > wrote:
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Hi
> Ron/Harsha/Satish,
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Thanks for reviewing
> > > the KIP!
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > We should perhaps
> have
> > > a wider
> > > > > discussion
> > > > > >> > > outside
> > > > > >> > > > > > this
> > > > > >> > > > > > > > KIP
> > > > > >> > > > > > > > > for
> > > > > >> > > > > > > > > > > refactoring
> > > > > >> > > > > > > > > > > clients so that
> others
> > > who
> > > > are not
> > > > > >> following
> > > > > >> > > this KIP
> > > > > >> > > > > > > > also
> > > > > >> > > > > > > > > > notice the
> > > > > >> > > > > > > > > > > discussion. Satish,
> > > would you
> > > > like
> > > > > to
> > > > > >> start a
> > > > > >> > > > > > > discussion
> > > > > >> > > > > > > > > thread
> > > > > >> > > > > > > > > > on dev?
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Regards,
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > Rajini
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > On Wed, Aug 7, 2019
> at
> > > 6:21 PM
> > > > > Satish
> > > > > >> Duggana <
> > > > > >> > > > > > > > > > >
> satish.dugg...@gmail.com>
> > > > > >> > > > > > > > > > > wrote:
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > > > I felt the same
> need
> > > when
> > > > we want
> > > > > to
> > > > > >> add a
> > > > > >> > > > > > pluggable
> > > > > >> > > > > > > > API
> > > > > >> > > > > > > > > for
> > > > > >> > > > > > > > > > core
> > > > > >> > > > > > > > > > > > server
> > functionality.
> > > This
> > > > does
> > > > > not
> > > > > >> need to
> > > > > >> > > be part
> > > > > >> > > > > > > of
> > > > > >> > > > > > > > > this
> > > > > >> > > > > > > > > > KIP, it
> > > > > >> > > > > > > > > > > > can be a separate
> > > KIP. I can
> > > > > contribute
> > > > > >> those
> > > > > >> > > > > > > > refactoring
> > > > > >> > > > > > > > > > changes if
> > > > > >> > > > > > > > > > > > others are OK with
> > > that.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > It is better to
> > have a
> > > > structure
> > > > > like
> > > > > >> below.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > kafka-common:
> > > > > >> > > > > > > > > > > > common classes
> which
> > > can be
> > > > used
> > > > > in any
> > > > > >> of
> > > > > >> > > the
> > > > > >> > > > > > other
> > > > > >> > > > > > > > > modules
> > > > > >> > > > > > > > > > in Kafka
> > > > > >> > > > > > > > > > > > like client,
> > > > Kafka-server-common
> > > > > and
> > > > > >> server
> > > > > >> > > etc.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > >
> kafka-client-common:
> > > > > >> > > > > > > > > > > > common classes
> which
> > > can be
> > > > used
> > > > > in the
> > > > > >> > > client
> > > > > >> > > > > > > module.
> > > > > >> > > > > > > > > This
> > > > > >> > > > > > > > > > can be
> > > > > >> > > > > > > > > > > > part of client
> > module
> > > > itself.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > >
> kafka-server-common:
> > > > > >> > > > > > > > > > > > classes required
> > only
> > > for
> > > > > kafka-server.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > Thanks.
> > > > > >> > > > > > > > > > > > Satish.
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > On Wed, Aug 7,
> 2019
> > > at 9:28
> > > > PM
> > > > > Harsha
> > > > > >> > > Chintalapani
> > > > > >> > > > > > <
> > > > > >> > > > > > > > > > ka...@harsha.io>
> > > > > >> > > > > > > > > > > > wrote:
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > Thanks for the
> KIP
> > > Rajini.
> > > > > >> > > > > > > > > > > > > Quick thought,
> it
> > > would
> > > > be good
> > > > > to
> > > > > >> have a
> > > > > >> > > common
> > > > > >> > > > > > > > module
> > > > > >> > > > > > > > > > outside of
> > > > > >> > > > > > > > > > > > clients
> > > > > >> > > > > > > > > > > > > that only
> applies
> > to
> > > > server side
> > > > > >> > > interfaces &
> > > > > >> > > > > > > > changes.
> > > > > >> > > > > > > > > It
> > > > > >> > > > > > > > > > looks
> > > > > >> > > > > > > > > > > like we
> > > > > >> > > > > > > > > > > > are
> > > > > >> > > > > > > > > > > > > increasingly in
> > > favor of
> > > > using
> > > > > Java
> > > > > >> > > interface for
> > > > > >> > > > > > > > > pluggable
> > > > > >> > > > > > > > > > > modules on
> > > > > >> > > > > > > > > > > > the
> > > > > >> > > > > > > > > > > > > broker side.
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > Thanks,
> > > > > >> > > > > > > > > > > > > Harsha
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > On Tue, Aug 06,
> > > 2019 at
> > > > 2:31 PM,
> > > > > >> Rajini
> > > > > >> > > Sivaram <
> > > > > >> > > > > > > > > > > rajinisiva...@gmail.com
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > wrote:
> > > > > >> > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > > Hi all,
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > > I have
> created a
> > > KIP to
> > > > > replace the
> > > > > >> Scala
> > > > > >> > > > > > > > Authorizer
> > > > > >> > > > > > > > > API
> > > > > >> > > > > > > > > > with a
> > > > > >> > > > > > > > > > > new
> > > > > >> > > > > > > > > > > > Java
> > > > > >> > > > > > > > > > > > > > API:
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > > -
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > KIP-504+-+Add+new+Java+Authorizer+Interface
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > > This is
> > > replacement for
> > > > KIP-50
> > > > > >> which was
> > > > > >> > > > > > accepted
> > > > > >> > > > > > > > > but never
> > > > > >> > > > > > > > > > > merged.
> > > > > >> > > > > > > > > > > > Apart
> > > > > >> > > > > > > > > > > > > > from moving
> to a
> > > Java
> > > > API
> > > > > consistent
> > > > > >> > > with other
> > > > > >> > > > > > > > > pluggable
> > > > > >> > > > > > > > > > > interfaces
> > > > > >> > > > > > > > > > > > in the
> > > > > >> > > > > > > > > > > > > > broker,
> KIP-504
> > > also
> > > > attempts
> > > > > to
> > > > > >> address
> > > > > >> > > known
> > > > > >> > > > > > > > > limitations
> > > > > >> > > > > > > > > > in the
> > > > > >> > > > > > > > > > > > > > authorizer. If
> > > you have
> > > > come
> > > > > across
> > > > > >> other
> > > > > >> > > > > > > > > limitations that
> > > > > >> > > > > > > > > > you
> > > > > >> > > > > > > > > > > would
> > > > > >> > > > > > > > > > > > like
> > > > > >> > > > > > > > > > > > > > to see
> addressed
> > > in the
> > > > new
> > > > > API,
> > > > > >> please
> > > > > >> > > raise
> > > > > >> > > > > > > these
> > > > > >> > > > > > > > > on the
> > > > > >> > > > > > > > > > > discussion
> > > > > >> > > > > > > > > > > > > > thread so that
> > we
> > > can
> > > > > consider those
> > > > > >> > > too. All
> > > > > >> > > > > > > > > suggestions
> > > > > >> > > > > > > > > > and
> > > > > >> > > > > > > > > > > feedback
> > > > > >> > > > > > > > > > > > are
> > > > > >> > > > > > > > > > > > > > welcome.
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > > Thank you,
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > > > > Rajini
> > > > > >> > > > > > > > > > > > > >
> > > > > >> > > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> >
>
