Hi Rajini

Overall, it looks good. 

Thanks

Bosco


On 9/4/19, 3:39 AM, "Rajini Sivaram" <rajinisiva...@gmail.com> wrote:

    Hi Colin & Don,
    
    I have submitted a PR to help with reviewing the new API:
    https://github.com/apache/kafka/pull/7293/files.
    
       - Authorizer.java contains the updated API and includes javadoc
       update including the threading model.
       - AclAuthorizer.scala changes in the PR should give an idea of the
       implementation changes for custom authorizers that want to continue to be
       synchronous.
       - KafkaApis.scala continues to use the API synchronously for now. It can
       be updated later.
    
    
    Thank you,
    
    Rajini
    
    
    On Wed, Sep 4, 2019 at 9:38 AM 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
    >>     >     >     >> > > > > > > > >     >     >     > > >
    >>     >     >     >> > > > > > > > >     >     >     >
    >>     >     >     >> > > > > > > > >     >     >
    >>     >     >     >> > > > > > > > >     >     >
    >>     >     >     >> > > > > > > > >     >     >
    >>     >     >     >> > > > > > > > >     >     >
    >>     >     >     >> > > > > > > > >     >
    >>     >     >     >> > > > > > > > >     >
    >>     >     >     >> > > > > > > > >     >
    >>     >     >     >> > > > > > > > >     >
    >>     >     >     >> > > > > > > > >
    >>     >     >     >> > > > > > > > >
    >>     >     >     >> > > > > > > > >
    >>     >     >     >> > > > > > > > >
    >>     >     >     >> > > > > > > >
    >>     >     >     >> > > > > > >
    >>     >     >     >> > > > > >
    >>     >     >     >> > > >
    >>     >     >     >> > >
    >>     >     >     >> >
    >>     >     >     >>
    >>     >     >     >
    >>     >     >
    >>     >     >
    >>     >     >
    >>     >     >
    >>     >     >
    >>     >
    >>     >
    >>     >
    >>     >
    >>     >
    >>
    >>
    >>
    >>
    


Reply via email to