Hi, Rajini,

Thanks for the reply. The 4-step approach that you outlined seems to work.
Overall, I can see that the async authorize() api could lead to an overall
more efficient implementation. The tradeoff is that we have to code every
request with an extra stage. To me, this optimization seems too early. With
100K topics, each with 1KB worth of users, the required ACL space is about
100MB and we should be able to cache everything. Beyond that, we still have
the option of dividing up the topic and ACL metadata such that every broker
is only required to cache a subset of all metadata.
Regarding making create/delete api async, I still have mixed feelings on
this. I am wondering if the benefit is worth the added complexity. To me,
both operations are rare. If one request thread is blocked occasionally for
a few millisecs, it's probably ok since it mostly just affects one client.
In the case that a particular user issues many those operations in a short
window. Could we just use CPU throttling to prevent too much resource being
used? There are a few other similar types of requests such as create/alter
configs, create/alter topic, etc. Do we plan to add an extra processing
stage for each of them too in the future?

Thanks,

Jun

On Sun, Sep 8, 2019 at 10:04 AM Rajini Sivaram <rajinisiva...@gmail.com>
wrote:

> Thanks Ismael. That is good point. I have updated the javadoc.
>
> On Sun, Sep 8, 2019 at 5:38 PM Ismael Juma <ism...@juma.me.uk> wrote:
>
> > Hi Rajini,
> >
> > Can you please update the javadoc for every sync method to be explicit
> > about the fact that they should not block and the reasoning?
> >
> > Ismael
> >
> > On Sun, Sep 8, 2019, 9:21 AM Rajini Sivaram <rajinisiva...@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > Thanks everyone for the very useful discussion. I have updated the KIP
> to
> > > make *createAcls()* and *deleteAcls()* asynchronous. Also added a
> section
> > > under `*Rejected Alternatives*` for async *authorize()*, which we can
> > > revisit in future if requirement arises.
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > >
> > > On Fri, Sep 6, 2019 at 9:17 PM Ismael Juma <ism...@juma.me.uk> wrote:
> > >
> > > > It's not uncommon for people to do large numbers of admin operations
> > per
> > > > second. We've seen it many times in production. It's often a bug, but
> > the
> > > > system must cope with it without affecting other users of the same
> > Kafka
> > > > cluster.
> > > >
> > > > Ismael
> > > >
> > > > On Fri, Sep 6, 2019, 9:32 AM Don Bosco Durai <bo...@apache.org>
> wrote:
> > > >
> > > > > +1
> > > > >
> > > > > I might be wrong here, but here are few of my assumptions..
> > > > >
> > > > > 1. Unlike transactional services, in Kafka most of the users are
> > > > > application users, so once the connection is established, then they
> > are
> > > > > going to be long running sessions. So the caching is more
> predictable
> > > and
> > > > > heavy lifting can be done during initial session setup and cache
> can
> > be
> > > > > frequently updated using background thread. KSQL might have a
> > different
> > > > > flow, but I don't have enough datapoints for it.
> > > > > 2. For publish/consume authorization calls, providing async option
> > will
> > > > be
> > > > > misleading for Kafka like load
> > > > > 3. For admin calls, hopefully no one does hundreds of call in
> > multiple
> > > > > thread within few seconds. This would be a bad design.
> > > > >
> > > > > So I feel, we should be okay with sync calls for now. And consider
> > > async
> > > > > in the future if needed. At least in Apache Ranger implementation,
> we
> > > > would
> > > > > be okay using sync operation. I am not sure if any other plugin
> > > > > implementation would benefit from async implementation.
> > > > >
> > > > > Thanks
> > > > >
> > > > > Bosco
> > > > >
> > > > > On 9/6/19, 8:45 AM, "Ismael Juma" <ism...@juma.me.uk> wrote:
> > > > >
> > > > >     One more thing: if people have strong opinions that authorize
> has
> > > to
> > > > be
> > > > >     sync, we could leave it like that for now. If needed, we can
> > later
> > > > add
> > > > > an
> > > > >     authorizeAsync with another method returning a Boolean
> indicating
> > > > that
> > > > > it's
> > > > >     supported. That is, there is a path to evolve the interface
> (even
> > > if
> > > > a
> > > > > bit
> > > > >     ugly).
> > > > >
> > > > >     What about the other methods, is there consensus that they
> should
> > > be
> > > > >     updated to return CompletionStage?
> > > > >
> > > > >     Ismael
> > > > >
> > > > >     On Fri, Sep 6, 2019, 8:32 AM Ismael Juma <ism...@juma.me.uk>
> > > wrote:
> > > > >
> > > > >     > I'm on the move, but what Rajini said seems reasonable. I
> don't
> > > > think
> > > > >     > using SSDs solves the issue. They can still hang for seconds
> > when
> > > > > they
> > > > >     > fail. Also, many people may not have local SSDs available
> > (remote
> > > > > SSDs like
> > > > >     > EBS hang for tens of seconds when there are issues).
> > > > >     >
> > > > >     > We are currently vulnerable to all of these in the normal
> > > > read/write
> > > > > path,
> > > > >     > but it seems suboptimal to continue assuming non blocking
> > > behavior
> > > > > for
> > > > >     > things that actually do block.
> > > > >     >
> > > > >     > Ismael
> > > > >     >
> > > > >     > On Fri, Sep 6, 2019, 8:24 AM Rajini Sivaram <
> > > > rajinisiva...@gmail.com
> > > > > >
> > > > >     > wrote:
> > > > >     >
> > > > >     >> Hi Jun,
> > > > >     >>
> > > > >     >> For ACLs, the size is also dependent on the number of users.
> > So
> > > in
> > > > >     >> deployments with large number of users where some users are
> > not
> > > > > active, an
> > > > >     >> LRU cache may be useful. Agree that there may be other
> > solutions
> > > > > that help
> > > > >     >> avoid the requirement for an async API even for this case.
> > > > >     >>
> > > > >     >> I wasn't expecting our threading model to change very much.
> > > Having
> > > > > said
> > > > >     >> that, I haven't figured out how the purgatory can be adapted
> > to
> > > > work
> > > > >     >> efficiently for this. I was thinking:
> > > > >     >> 1) If authorization future is complete when returning from
> > > > > authorize()
> > > > >     >> call, we invoke the method immediately on the request thread
> > > > >     >> 2) If future is not complete, the request goes into a
> > purgatory
> > > > >     >> 3) When future completes, we mark the request ready to be
> run.
> > > > This
> > > > > can be
> > > > >     >> done on ForkJoinPool.commonPool or an executor. But we don't
> > > > handle
> > > > > the
> > > > >     >> request on that thread
> > > > >     >> 4) We handle the request on the request thread after 3).
> > > > >     >>
> > > > >     >> I totally agree that this adds complexity to the code. Will
> > wait
> > > > for
> > > > >     >> Ismael
> > > > >     >> to comment on whether he had a different model in mind.
> > > > >     >>
> > > > >     >> Thanks,
> > > > >     >>
> > > > >     >> Rajini
> > > > >     >>
> > > > >     >>
> > > > >     >> On Thu, Sep 5, 2019 at 10:29 PM Jun Rao <j...@confluent.io>
> > > wrote:
> > > > >     >>
> > > > >     >> > Hi, Ismael,
> > > > >     >> >
> > > > >     >> > Yes, I agree that there are 2 separate discussion points,
> > one
> > > on
> > > > > the API
> > > > >     >> > and the other on the implementation.
> > > > >     >> >
> > > > >     >> > First on the API, in general, my feeling is that the
> > > authorize()
> > > > > call is
> > > > >     >> > expected to be fast to reduce request latency. Adding an
> > async
> > > > API
> > > > >     >> could be
> > > > >     >> > mis-leading since it might give users the impression that
> it
> > > > > would be
> > > > >     >> > arbitrarily long. In the short term, currently, we cache
> all
> > > > > partition
> > > > >     >> > level metadata on every broker. The authorization metadata
> > is
> > > > >     >> proportional
> > > > >     >> > to the number of topics, which typically is less in size.
> So
> > > it
> > > > > we can
> > > > >     >> > cache all partition level metadata, we should be able to
> > cache
> > > > all
> > > > >     >> > authorization metadata. Longer term, if we want to support
> > > more
> > > > > metadata
> > > > >     >> > beyond memory, another option is to put all metadata in an
> > SSD
> > > > > backed
> > > > >     >> > key/value store. The lookup time could still be under 1ms,
> > > which
> > > > > we
> > > > >     >> could
> > > > >     >> > potentially absorb in the request threads. So, I am not
> sure
> > > > async
> > > > >     >> > authorize() api is strictly needed even in the long term.
> > > > >     >> >
> > > > >     >> > Second on the implementation. I was more concerned about
> the
> > > > > impact on
> > > > >     >> our
> > > > >     >> > threading model. If we make authorize() async, some
> threads
> > > have
> > > > > to
> > > > >     >> > complete the future when it's ready. It seems to me that
> > those
> > > > > threads
> > > > >     >> have
> > > > >     >> > to be inside the Authorizer implementation since only it
> > knows
> > > > > when to
> > > > >     >> > complete a future? If so, it feels weird that all the
> logic
> > of
> > > > > request
> > > > >     >> > handling post authorize() is now handled by external
> threads
> > > > > inside a
> > > > >     >> > plugin. It's not clear how we can configure and monitor
> > them,
> > > > and
> > > > > how
> > > > >     >> > throttling would work. Perhaps we can provide a bit more
> > > detail
> > > > > on the
> > > > >     >> new
> > > > >     >> > threading model.
> > > > >     >> >
> > > > >     >> > Thanks,
> > > > >     >> >
> > > > >     >> > Jun
> > > > >     >> >
> > > > >     >> > On Wed, Sep 4, 2019 at 9:56 PM Ismael Juma <
> > ism...@juma.me.uk
> > > >
> > > > > wrote:
> > > > >     >> >
> > > > >     >> > > 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.
> > > > >     >> > > > > >     >     >     >> > > > > > >
> > > > >     >> >
> >
>

Reply via email to